Make mention choices consistent and stable - #258
Conversation
ed77acf to
59a82eb
Compare
ffe9ca9 to
4ed2a70
Compare
59a82eb to
322ed68
Compare
3aea51d to
28d80d4
Compare
4ed2a70 to
f58b7b7
Compare
28d80d4 to
e0e2e7a
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord’s automated source review via Wes’s account.
Reviewed head e0e2e7a6f6693ef6a850ea34014e851e7d7be9d5 against base 216a81909210ef006a4d0040e82c366f3cda222d (the #257 stack base), including chooser/ranking, directory lifecycle, completion-host integration, insertion/send callers and changed test sources. Two actionable P2 findings are recorded inline. This is non-blocking Comment feedback, not approval or a changes-requested review.
Validation limits: source analysis only, with an independent source cross-check. No PR code, tests, builds or app workflows were executed; CI was not assessed. Browser editing/focus, actual directory/relay behavior and exact-head test results remain unverified. The three added browser cases describe native editing/focus or representative integration boundaries, but the PR description does not record exact-head validation/deferred-check evidence; test source is not a passing result.
e0e2e7a to
13d0b85
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Automated source review — Star Lord (via Wes’s account)
No new actionable source defects found in this revision.
- The prior mixed human/agent comparator cycle is addressed by per-choice sort keys and stable same-name blocks. The new permutation coverage targets both the reported cycle and interleaved human/agent cases.
- The prior loss of directory fallback on every inline keystroke is addressed by session-scoped chooser state. I traced the real
ComposerCompletionsremount boundary and the added host-level regression test, along with picker selection, live eligibility checks, and send-entry archive handling. - Ranking was assessed against the current
docs/agents.mdcontract: on otherwise equal matches, viewer-owned agents precede humans as well as other agents. This is a documented behavior assessment, not a separate product-approval claim.
Reviewed head: 13d0b85921748f0f848c2a76cb10f89426a6804e
Base: 17de590f1d574a9a7f2d9af218edfe75fc6f8caa
Diff merge-base: 24fcb1ed31e48558d0b127850ced79fbc6c80a7b
Prior reviewed head: e0e2e7a6f6693ef6a850ea34014e851e7d7be9d5
Validation limits: source-only review of pinned Git objects, with independent ranking review. No PR code, tests, builds, or app workflows were executed; CI was not assessed. Regression tests were inspected, not run. Browser editing/focus, live directory behavior, and packaged-app acceptance remain unverified. This non-blocking COMMENT is not a GitHub approval or merge authorization.
13d0b85 to
a8cb4b6
Compare
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Members and agent choices now establish the chooser at once. Directory people append below them, so a late page never moves a visible row. A new query keeps still-matching people from the last page while it loads, waits for a 200 ms typing pause, and settled pages are cached per session and query. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
…s and journey The browser journey and agent docs now use Close, Send anyway, and Invite, the actions of the outside-mention dialog on the base branch. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Ownership, same-name agent blocks, and presence tie-breaks are now per-choice sort keys. Owned agents lead equal matches; same-name agents stay one block at their first label. A permutation test covers mixed people and agents. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
The completion host remounts the mention provider on every keystroke, which dropped the hook-local page. The last settled page now lives per session and chooser lifetime: one picker, or one inline @ token. A test drives the real keyed host. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
…oser Main gained two mention changes while this branch was open: archived identities leave choices but never hide the viewer (#256), and prose after an unknown name closes the menu without extra directory reads (#303). - One archivedMention rule now owns the viewer exemption for candidates, disabled installed rows and the send-entry guard. The old per-surface predicate is removed. - The directory keeps #303's exhausted-prefix refutation. Only non-empty pages are cached, so an empty result is refuted by prefix evidence and a fresh search for the same query reads again. - Inline completion withdraws its menu for prose once local and directory evidence settle. - Tests from main follow this branch's rules: shown rows stay in place while disabled, and public keys match no choice. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
a8cb4b6 to
2adc990
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Automated source review — Star Lord (via Wes’s account)
One P3 recovery-contract gap is recorded inline: the new empty-result refresh policy does not reach the persistent toolbar picker. This is an incomplete part of the new fix, not a claim that this behavior first appeared in the previously reviewed revision.
Follow-up scope: the 2adc990 archive/prose-search integration and its supported callers. The archived-viewer exemption is consistently applied to candidate selection, disabled-row explanations and send entry; the prior transitive-ranking and inline-remount fixes remain intact. The directory/prose lane received an independent source cross-check, reconciled against the picker’s actual lifetime. No unrelated earlier review areas were reopened.
- Head:
2adc990779f86cb707d5ba47486fbb78a981aec9 - Base:
734949a62229c56b2e3368701a4b4d5413ac555a - Diff merge-base:
67fe94c750fc739e49caf3ee57e0f0940944cb04 - Earlier feedback-covered head:
13d0b85921748f0f848c2a76cb10f89426a6804e
Validation limits: source-only review of immutable Git objects; no PR code, tests, builds or app workflows were executed. Pinned diff whitespace checks passed. One exact-head CI snapshot showed JavaScript, browser measurements, DCO and security checks passing, with Rust and several browser shards still running; Windows was skipped. I did not wait or poll. Browser editing/focus, live directory behavior, complete CI and explicit human acceptance remain unverified. This non-blocking COMMENT is not approval, Request Changes, or merge authorization.
Main's archive fixture had no state(), so the shared archive rule threw and the product-ui catalogue rendered no composer. Unavailable archive reads report "unknown", as the real contract does. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
The toolbar picker stays mounted while closed, so an uncached empty page matched only by query survived close and reopen. Each opening is now its own directory lifetime, and the empty page belongs to that lifetime. The non-empty session cache is unchanged. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Automated source review — Star Lord (via Wes’s account)
No new actionable findings in this follow-up. The earlier P3 picker-reopen finding is addressed in source:
MentionPicker.tsx:42–51,85–91gives each opening its own directory lifetime.useMentionDirectory.ts:74–109,135–145binds uncached empty/error state and request cleanup to that lifetime while retaining the intended non-empty session/query cache.- The added persistent-picker regression (
session-agents.test.tsx:1837–1883) closes and reopens the same mounted picker, expects another directory call for the same query, and checks that the newly available person appears. - The composer-lab fixture now supplies
archives.state/ensure/refresh; returningunknownfor its unavailable archive snapshot matches the real read contract without weakening production archive filtering.
Scope: the four-file follow-up since 2adc990779f86cb707d5ba47486fbb78a981aec9 (63 additions / 5 deletions), its supported callers, cache/close/reopen lifecycle, and regression source. No unrelated earlier review areas were reopened. The target-branch changes since the merge-base do not overlap these four files.
- Head:
5075f7625ba6c46ce1be690d5e08e33d0f2b14be - Base:
734949a62229c56b2e3368701a4b4d5413ac555a - Diff merge-base:
67fe94c750fc739e49caf3ee57e0f0940944cb04
Validation limits: source-only review of immutable, blob-verified extracts; no PR code, tests, builds, or app workflows were executed. The four-file follow-up has no whitespace diagnostics. One exact-head CI snapshot showed browser measurements, DCO and security checks passing; JavaScript, Rust and all six browser-journey shards were still running, and Windows was skipped. No waiting or polling. Complete CI, actual browser/native interaction, live-directory recovery and explicit human acceptance remain unverified. This non-blocking COMMENT is not approval or merge authorization.
The spec was written against an early #258. The merged version orders your own agents before people too, keeps same-name agents in one block, keeps yourself visible when archived, closes the chooser for prose, and does not cache searches that find no one. Three new ranking fixtures cover the ownership and block cases. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
…ad-on-send * origin/main: (58 commits) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) fix(status): reopen a Today status as Today near 16:00 (#275) test: use current navigation for GIF send roundtrip (#309) Fix composer focus when selecting channels and DMs (#307) fix: retire mention searches after chips and refuted prose (#303) ... # Conflicts: # src/features/messages/MessageComposer.test.tsx # src/features/messages/MessageComposer.tsx
* origin/main: (45 commits) Use Blue 11 links with Blue 3 hover and explicit contrast exceptions (#322) perf(messages): index the emoji catalog for reaction lookups (#333) Polish search palette and add conversation search (#340) Use step-ten avatar colors with contrasting outlines (#320) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentCard.tsx # src/bundled/agents/AgentsPage.tsx
* origin/main: (36 commits) Delay message timestamp tooltips by 500 ms (#321) Use Blue 11 links with Blue 3 hover and explicit contrast exceptions (#322) perf(messages): index the emoji catalog for reaction lookups (#333) Polish search palette and add conversation search (#340) Use step-ten avatar colors with contrasting outlines (#320) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentEditor.tsx # src/bundled/profiles/ProfileAgentIdentity.test.tsx
🤖
Stacked on #257. Review that one first. This PR's diff has only the chooser changes.
Summary
@completion now use the same rules, so the same search gives the same list in both places.The rules
Who is in the list
Order
What does not match
When the list changes
Directory search
@or a reopened picker searches again. Typing more after that search does not search, because a longer name cannot match.@menu, text after an unknown name is treated as normal typing. The menu closes, and no search runs.Space
Screenshots
Members come first. People outside the channel come after them.
A directory search is still running. Members show at once, and a "Searching community…" line shows below them until people from outside the channel arrive.
Two identities have the same name. Each row shows a label and a short key, so you can tell them apart.
A person leaves the channel while the list is open. Their row stays in place, and you cannot choose it.
Details
docs/agents.mdunder "Chooser rules".