Conversation
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: one P2 mention round-trip regression. Reviewed head 65f700cdc7a6cd47de18df3f962343e431a08732 against stacked base ab358b85628608e3ce73c6773c5e402a0fdf201d (PR #140), not main.
Completion and the mention picker now serialize a resolved native name that the sent-message renderer cannot bind. With public name Mic, legacy name Legacy Mic, and a ready matching-community native name Native Mic, selecting the agent produces a decorated draft but a plain @Native Mic after send. The signed recipient pubkey remains correct; the regression is loss of the mention chip, exact-profile navigation, and live display-name updates for that mention. See the inline finding for the complete path.
Please retain a renderable exact-key mention through both selection paths. The smallest option appears to be separating the resolved display label from a public/legacy source label already understood by the renderer, rather than expanding name-based identity inference. Add a select → send → render regression with distinct native/public/legacy names, asserting the same recipient key and profile target.
All three independent review lanes are integrated. No additional actionable regression found in notification targeting, DM labels, activity/typing, agent selection, or timeline name invalidation. Namesake qualifiers remain explicitly out of scope.
Validation: exact-head source tracing and archive-byte verification, plus existing successful hosted JavaScript, Rust/tool integration, Chromium/WebKit journeys and measurements, DCO, and security checks. No local suites or live UI workflow were run; live Virtua remeasurement remains unverified. Windows native validation was skipped by CI.
| choices.set(pubkey, { | ||
| pubkey, | ||
| name: | ||
| name: resolveName( | ||
| pubkey, | ||
| profiles.get(pubkey)?.name ?? | ||
| choices.get(pubkey)?.name ?? | ||
| pubkey.slice(0, 12), | ||
| choices.get(pubkey)?.name ?? | ||
| pubkey.slice(0, 12), | ||
| ), |
There was a problem hiding this comment.
[P2] Keep newly selected resolved names bindable after send
The resolved label is also the serialized MentionRecipient.name (edit: { mention: recipient } at line 141); MentionPicker.tsx:92-107 does the same. For a member with public name Mic, legacy name Legacy Mic, and a ready same-community native name Native Mic, selecting it now inserts @Native Mic. MessageComposer.tsx:347-369,437-471 sends that text plus the pubkey, and relay/messages.ts:26-47 retains only the text and p tag, not the draft label mapping.
The draft looks correct because RichComposerInput.tsx:30-35 injects the selected name into its temporary profile map. The sent row has no such map: profileMentionParts.ts:16-31 recognizes only public/legacy labels, before MessageMarkdown.tsx:356-361,400 can apply the resolved display name. Consequently this ordinary selection becomes plain text after sending, without its mention chip/profile link or subsequent resolved-name updates. The exact notification recipient is still correct. The base inserts the public name for this member and remains bindable.
Preserve a renderer-recognizable exact-key mention through both completion and picker, keeping display-label resolution separate from source serialization if possible. Cover this with a native-name-differs-from-public-and-legacy round-trip test; the added mounted test only relabels an already-bindable @Mic.
There was a problem hiding this comment.
Fixed at 1e2fd78. Picker and completion now use one shared choice builder with separate recipient and display label. Only the original public/library name is serialized; native names remain UI labels. No sent-text matching rules changed. Added both selection paths with Mic / Legacy Mic / Native Mic, then draft → createMessages send → exact p tag → mounted MessageMarkdown profile button → live rename with the same target. Both regressions fail against the old picker/completion and pass with the fix. Normal push gate passed types, 1,024 tests, and design checks. #142 is restacked at af37798; hosted checks are running.
There was a problem hiding this comment.
CI exposed a browser fixture contract error: its production-broker stub returned [] for agentLibrary rather than {definitions: [], identities: []}. Fixed the stub at 1ba499a (no production guards added). The affected navigation/unread/initial-position files pass in both Chromium and WebKit (52 tests) on #141; live reconnect and agent-activity files also pass in both engines on the restacked #142. Final hosted checks are running. #140 required CI/DCO are green.
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
ab358b8 to
000495d
Compare
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
65f700c to
1e2fd78
Compare
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
The prior P2 is fixed. No new production-code blocker found, but CI is not green. Re-reviewed head 1e2fd7812ae4f18185abdb490a70da9683cbe952 against stacked base 000495d3d1eb4b7345cc5bf4b08c6992ccf5a377, not main. The original feature commit is patch-equivalent after rebase; both independent review lanes are integrated.
Mention repair
The shared choice builder now separates the serialized recipient name from its resolved display label in both completion and picker. I traced both through actual composer insertion, draft handling, send/reply serialization, and sent rendering. Exact pubkeys, signed text, and the existing conservative binding rules are preserved.
The new two-surface regressions exercise distinct public/legacy/native names, real createMessages output and exact p tags, mounted sent rendering, and live rename with the same profile target. They use draft helpers rather than mounting the whole composer; that wiring was source-reviewed separately. Hosted JavaScript passed 208 files / 2,192 tests, including all six mention-selection tests, on merge 323750911dfdab49979ee4377d4b0869535ba58b of this exact head/base.
CI follow-up, not a new production P2
Chromium shard 2 fails reconnect/stream-repair journeys (live.spec.mjs:114, message-navigation.spec.mjs:474) with Cannot read properties of undefined (reading 'find') in name resolution, then missing timeline rows.
tests/browser/fixture.mjs:985 injects agentLibrary: () => []. The broker returns it unchanged; the library publishes it as ready; directory.ts:40-41 accesses the absent identities. The real producer returns { definitions, identities } or throws (dev/agent-library.mjs:21-60,112-117), with errors mapped to HTTP 503. These producer/fixture files are inherited from the stacked base; #141's timeline consumers expose the mismatch.
Smallest next step: correct the fixture to { definitions: [], identities: [] } and obtain passing affected browser checks. Do not weaken assertions or expand production machinery on this evidence. This is not established as a production reconnect defect or a flake, but red CI still prevents a merge-ready verdict.
Inherited limitation
Ordinary-channel local-nonmember choices can still serialize a native name that the sent renderer cannot bind. Both original and current stacked bases already do this. It is a separate follow-up, not the member/library regression introduced by #141; it does not expand this re-review's exit criterion. Namesake qualifiers remain deferred.
No local broad suites or live UI run. Live Virtua rename remeasurement remains unverified; Windows native CI is skipped. This COMMENT review does not approve or authorize merging the PR.
…ixture Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
2 participants
🤖
Summary
The same agent could appear under different names in messages, mentions, profiles, and conversation controls. Use the shared resolved name across these surfaces so readers can follow who is speaking and who will receive a message.
Details
Second of three stacked PRs; depends on the shared-name-service PR and targets
feat/agent-name-service.Validation
Focused mounted-component, message-geometry, and service checks cover live label updates and exact targets. This branch head typechecks. No browser cases added or removed. Hosted JavaScript, Rust/tool integration, Chromium/WebKit journeys, browser measurements, DCO, and security checks passed. Windows native validation was skipped by the workflow.