Repository navigation
Chats: delete the app's copy of the chat index; every reader asks acpmux (step 2 of #28055) - #28090
Conversation
… rule; Swift is the platform shell Rules 1-3 and L7: a client asks Rust for its visible window and gets row diffs with a revision; a native Swift list holds only its visible rows as value snapshots; Swift is never a relay or a mirror for data a webview shows (Lawrence 2026-10-30, P-RUST).
…hange; main holds only the window Rust: acpmux answers _acpmux/chats_window over 3,000 chats with the rows sorted and filtered, and one change is a diff of one row with the next revision. Swift: after one chat change among 3,000, the main thread holds the sidebar history's window, not the whole index (v2.2.9 hangs of 570 to 1,148 ms in AcpmuxChatsStore.apply and SidebarChatHistoryFeed.chats).
…w, never the whole index
v2.2.9 hangs of 570 to 1,148 ms (Lawrence, debug.hangs 2026-10-30):
AcpmuxChatsStore.apply copied every chat because SidebarChatHistoryFeed.chats
held the same storage, and that setter ran the derived == over every chat and
freed the old copy, all on the main thread.
Rust (acpmux) now owns the list a client shows (P-RUST, layer-ownership.md
rules 1-3 and L7):
- _acpmux/chats_window {window, filter, limit, topLevel, exclude, keys}: the
window's rows, sorted, filtered and paged by acpmux; then, per index or chat
activity change, _acpmux/chats_window_changed with the next revision, the
window's keys in order and only the rows whose value changed. Level-triggered:
each window is recomputed from the index as it is now (chats/window.rs,
server/chat_windows.rs).
- _acpmux/chat_projects: each harness's project folders grouped by acpmux.
Swift:
- AcpmuxChatWindows holds only each window's visible rows, as value snapshots,
and applies a diff in O(window keys + changed rows); a revision gap, a
missing row or _acpmux/lagged asks for the window again (once).
- The sidebar chat history (Earlier and Pinned) reads two windows; its whole-
index setter is gone.
- ProjectsImport relays acpmux's grouped projects instead of taking a snapshot
of the mirror (which made the next main-actor change copy every chat).
- ChatsFeed's remaining mirror (palette chats page, Open Chat, cookie import,
New Tab cards) is private to it: no other holder, so a change applies in
place. Its removal is P-RUST step 2.
Tests: chats_window.rs (3,000 chats: sorted, filtered window; one change is
one row; a change outside the filter sends nothing; chat_projects groups),
window.rs unit tests, ChatsWindowMainThreadTests (one change among 3,000
touches one row on main; missed diffs ask again once).
…d state The guardrails count observations and main hops per file: the specs and the state now come from one ObservationStream, as before the windows.
…nd its connection (R6) chats_window_owned.rs counts live window tasks (a process-wide drop-guard count). Before the fix the task runs until the connection closes, so it outlives the client's last chats_window_close.
…d its connection (R6) The connection's one chats-window task ends when the client closes its last window (a later open starts a new one), when the connection closes, and with the runtime at daemon exit. Its running flag is set and cleared under the windows lock, so an open never misses a task that is ending.
…ux runs the requests in Review F1 of #28055: the daemon runs each request as its own task, so two specs of one window sent back to back may apply out of order, and the client then accepts the older spec's full message. Rust: 20 rounds of five specs in one burst must end on the last (limit 5); an older spec or close is refused as stale. Swift: each spec names a newer generation, and a message for an older one is not applied.
…28055 F1: the daemon runs each request as its own task, so two specs of one window sent back to back could apply out of order and leave the window on the older spec. Each open and close now names the window's generation (`gen`); acpmux keeps the highest per window, refuses an older one as `stale` and echoes `gen` in every message, and the client applies only its current spec's rows. F4: the windows lock is no longer held across an index read: rows are read off the executor, and each unchanged spec takes its diff under the lock (one recompute at a time, an async lock); an activity change takes no lock. F5: the batches waiting when a recompute starts are drained first, so one recompute follows a burst. F8: a recompute that fails sends `_acpmux/chats_lagged`, so the client asks again. F3: an error answer to a window request names its window (JSON-RPC id); the window is asked again at the next index change or reconnect, not left waiting. F7: an `_acpmux/lagged` for a message too large is not answered by asking again (it would loop). F2: `_acpmux/chat_projects` has no app reply cap (one row per chat folder); a failed read is logged and relayed again at the next index change (F6). F10: AcpmuxChatsStore.newestFirst and Window.hasMore (no readers) removed. F9 (one copy of the mirror per page) is left for step 2, which deletes it.
…its chat Step 2 of P-RUST (R7): the app's Chats feed must hold no copy of acpmux's index. ChatsFeedHoldsNoIndexTests: a 3,000-chat watch page and one index change reach the main actor with no AcpmuxChat in the message (today: 3,000 and 1). chats_open.rs: _acpmux/chat_open also answers the chat itself, so Open Chat names it without the mirror.
…ux (P-RUST step 2, R7) Step 2 of #28055: the app holds no copy of acpmux's chat index. The mirror (AcpmuxChatsStore) is deleted with its last readers: - New Tab cards: a window over the index (`newtab.cards`, the newest 8 top-level chats); only that window's messages push to the pages. - Command palette chats page: a window (`palette.chats`) whose spec carries the query; acpmux searches its own index (title or folder) and answers its newest 200 matches, then their changes while the page shows. The window closes with the page. No main-actor task: the rows arrive as window messages. - Open Chat: `_acpmux/chat_open` answers the chat with its plan; a running chat is found by its key (`harness:sessionId`). - Cookie import's requester: the caller's acpmux session lane by id, O(1) (the lane now carries the summary's `preview`). - ChatsFeed keeps the one connection, the windows and an index-change signal for the projects relay; the watch asks for a one-chat page, and its lines reach the main actor with no chat in them. This also removes review F9 of #28055 (one copy of the mirror per page). Behavior change: the palette's chats page lists acpmux's newest 200 matches of title or folder per query, instead of every chat filtered in the app by title, key, harness, folder, accounts and roots.
…first answer Review finding 1 of #28090: the page's queryResults stream never ended, so the palette spinner ran for as long as the page showed and an empty answer never showed the empty state.
…answer Review finding 1 of #28090: the palette runs its spinner until a query's stream ends; the stream now ends after acpmux's first answer for the query, so an empty answer shows the empty state. The window stays open until the page closes; an edit opens the next query's spec. Also removes the unused AgentPaneChatsPage.read (finding 3) and a stale test comment.
…ow many matched No-regression gate (lead, review of #28090): the palette's chats page found a chat by its harness ("codex", "claude") and by words that are not next to each other, with the palette ranker; the window's substring search found neither. 30 Claude chats: "claude" finds them all (total 30, five shown, hasMore), "fix build" finds the 15 whose title holds both words.
No-regression gate (review of #28090): acpmux reports how many chats matched; when more match than the page shows, the page's last row says so and runs nothing. The spec also sends the page's untitled word, so acpmux finds an untitled chat by it.
…s the total (no regression) Review of #28090 under the no-regression gate: the palette's chats page found a chat by its harness ("codex") and by words anywhere in its title or folder, with the palette ranker; the window's substring search found neither. - acpmux: a window with a `query` ranks every chat its filter and exclusions pass with cmux-palette-ranker (the ranker the palette itself runs through its C ABI): the same entry shape the page built (title or the client's `untitled` word, the folder with ~ as subtitle, the harness and folder as keywords), best match first; it sends the first `limit` (200) and `total`, how many matched. A window with no query is unchanged (newest first). - Swift: the spec sends `untitled`; a window keeps `total`; the palette page adds a last, disabled row "The best 200 of N matching chats. Type more to narrow." when more match than it shows (all nine macOS locales). Not the same as before: no frecency (the page passed none it had for chats), and the ranker runs over the chats' metadata in acpmux, off the main thread.
…r-fix3 # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentRecents/ChatsFeed+Start.swift
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Review (claude-opus-review-subagent-4): FIX at 48c88de Scope: the whole PR against merge-base 25c1584, and the repair 184f853..48c88de. Read only. I ran no build and no test. I read the three earlier FIX reviews (-6541340463, -6542219906, -6543785296) and the P2 comment from teamleaderleo. Finding (fix before merge)F1. Search shows at most 10 agent chats for every query, and nothing tells the person that more chats match.
Checked, no defect found
Notes (do not block)
|
…tale refusal, a not-ready index and a reconnect (red) Review 4 of #28090 (issuecomment-6546769110): F1 Search cut every chats answer to 10 rows with no row saying more match (30 chats on a page with room showed 10); note 2 a ready:false answer left Return on a timer; note 3 a refusal of an older spec ended the current query; note 4 End stopped on a stand-in on a full page (selectableRows compared counts); note 6 the chats page did not ask again when acpmux came. The seams these call (PalettePageSpec.moreRow and onSourceChange, AcpmuxChatWindows.requestFailed(_:generation:), requestNotReady, followAvailability) are inert here.
…imer: not-ready ends the wait, a stale refusal is ignored, a reconnect asks again Review 4 of #28090 (issuecomment-6546769110): - F1: a non-leading answer gets every row the ranked rows leave under the 400-row page and at least 10, taken from the weakest ranked rows. A cut answer ends with its page's more row (PalettePageSpec.moreRow): the chats page's says how many more chats match (the cut rows plus those past acpmux's 200) and opens the Agent Chats page with the same query. The page's own notice rows stay while nothing is cut. New string handlers.agentChats.moreInSearch, 21 locales. - Note 2: no hold timer. acpmux's ready:false answer ends the query's answer at once (requestNotReady), as a refusal does; every request ends with rows, none, or the query moving on. - Note 3: ChatsFeed keeps each window request's generation; requestFailed and requestNotReady ignore an older spec's answer. - Note 4: selectableRows filters by row id, so End and Down never stop on a stand-in on a full page. - Note 5: follow and followRefusal drop the entries of dead owners as they add one. - Note 6: PalettePageSpec.onSourceChange: the chats page asks the shown query again when acpmux connects or its not-ready index fills the window (AcpmuxChatWindows.followAvailability); the subscription lives while the page is open. - Note 7: the PaletteContributedAnswers doc and sidebar-search.md say when an answer holds the rows.
…ttling on its query; End must move off the first row
…200 virtual main display, so STRESS's 1600x1000 window fits (cmux23's 1920x1080 display left 963 pt)
|
Fix of review 4 (issuecomment-6546769110). Head 08f4143. The bench table follows in a later comment. Commits:
Fleet:
|
…r-fix3 # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentRecents/ChatsFeed+Start.swift
…wn acpmux for the typed word and prints its index state and matches (why no chats answer landed)
|
STRESS A/B for the perf-hot
Not measured: Not a result: the scheduled run at 20b3334 (job b9cd52e9a2a2458e514b919c) stopped in the STRESS build, because cmux23's display left 963 pt (fixed with |
|
Review (claude-opus-review-subagent-5): FIX at 733bd91 Scope: the whole PR against merge-base b0e10a0, and the repair 48c88de..733bd91 (a464fdd, 0b331e6, a895a65, 08f4143, 733bd91). Read only. I ran no build and no test. I read reviews -6541340463, -6542219906, -6543785296 and -6546769110, the fleet comment -6547205694, the A/B comment -6556134645, and the logs of jobs 84b28b44de05d9aa3cbcd8c4 (before) and f7e2a0be41b5b9de9cc24c01 (after). Findings (fix before merge)F1. Missing required measurement: the new answer path never ran, on the bench or in a GUI pass.
F2.
Earlier findings: none came back
Checked, no defect found
Notes (do not block)
|
# Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextAgentActivity/AgentActivityLane.swift # cmux-tui/crates/acpmux/src/server/chats.rs
…een with stand-ins; the more row keeps its count; ChatsFeed not-ready and generation (red) Review 5 of #28090 (issuecomment-6556223157): F2 an empty answer ran two full deliveries (list reload and reducer pass) that changed nothing; note 3 with stand-ins on a full page the selectable list kept ranked rows the shown list had cut; note 2 the more row read the live window, which the pushed page's cancel closes; note 4 ChatsFeed's not-ready and stale decoding and the generation it keeps had no test. ChatsFeed.windowRequest(method:params:) is an inert seam here.
…t for shown and selectable; the more row counts from the answer Review 5 of #28090 (issuecomment-6556223157): - F2: answer changes are redelivered once per main-thread turn (scheduleRedelivery), and only when what a delivery would show or hold changed (answerFingerprint vs the last delivery's): an empty answer with no stand-ins costs no list reload and no reducer pass; a yield and the stream's end no longer deliver twice. - Note 3: the selectable rows are the shown rows without the stand-in sections, so both cut the ranked rows at the same row and the selection is never off screen. - Note 2: the chats page records how many matches each answer left past its page (PastPageCounts) and the more row reads that, not the live window, which the pushed page's cancel closes. - Note 4: ChatsFeed.windowRequest(method:params:) is the one place a request's window and generation are kept. - Note 1: the awaitedAnswers doc no longer names the deleted holdLimit.
…k rows before typing; count chat rows per key Review 5 F1: --seed-chats wrote acpmux's own session store, which the chat index never scans, so no answer landed on either side. It now writes Claude Code transcripts in <HOME>/.claude/projects (a default root of cmux-chat-index's roots table) of a HOME the app gets of its own under /tmp; the account's ~/.claude is never touched. Before typing, the bench asks the app's acpmux for the word until the index answers with rows, and fails the run otherwise. After each key it reads the chat rows on screen (debug.palette.capture rows:true, a DEBUG and bench-hooks route), so the base, whose chats are ranked rows, is measured on the same check: key.palette_query_rows is its frame that first shows chat rows.
…e's rows (one App line, no new route branch)
…es the choice history; the App's chats page only wires it
…hecker crashed on the chained form, signal 6)
…nto chats-mirror-fix3
… (Swift 6.3.3 type-checker crash) The merge-head compile 5d75c0a66d245630f25fea38 crashed swift-frontend 6.3.3 (signal 6, ConstraintSystem.cpp:4138 recordArgumentList) in theMoreRowKeepsItsCountAfterTheWindowCloses. Every windows.connected closure states (String, [String: any Sendable]), and the heterogeneous row literal's map closure states (Int) -> [String: Any]. Same assertions.
|
Review (claude-opus-review-subagent-6): FIX at 6745efb Scope: the whole PR against merge-base 0e4ed40, and the repair 733bd91..6745efb (84b690b, 7ea1a33, 0525a8d, aeafd3c, b5ad89a, 04c407b, the merges e08a074 and 24939af, and 6745efb). I read reviews -6541340463, -6542219906, -6543785296, -6546769110 and -6556223157, the teamleaderleo P2, the PR body, and the logs of fleet steps 18f71a1f6c0aa1c8f4375ac1 and 9e19c1b1878a14e485b60be6. I ran no build and no test. Lead decision applied: this PR lands as a correctness fix with no perf claim. The missing answer-path measurement (review 5 F1) is not a finding. The PR body reports the STRESS A/B as measured, says "Perf claim: none", and says that no chats answer landed on either side. That is honest. Finding (fix before merge)F1. The repair of review 5 F2 is not complete, and its own regression test fails on the fleet.
Earlier findings: none came back
Checked, no defect found
Notes (do not block)
|
There was a problem hiding this comment.
18 issues found across 52 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextSignposts/SignpostMetricKitMirror.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextSignposts/SignpostMetricKitMirror.swift:47">
P3: The new palette-query exclusion has no test. Add `#expect(!SignpostMetricKitMirror.mirrors(.paletteQueryRows))` and the `.paletteQueryAnswer` equivalent next to the existing `motionFrame`/`cefPump` checks so a future change cannot silently re-enable them.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Package.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Package.swift:1012">
P3: The dependency map comment above `CmuxNextAgentLanes` still lists only AgentActivity, Apps, Bridge, Sidebar and Wakeups. Add Palette (the chats palette page) and CmuxAgentBrands to that list so the comment matches the target.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextPaletteTests/ContributedAnswersTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextPaletteTests/ContributedAnswersTests.swift:89">
P2: `until` returns silently on timeout, and `answer`/`refuse` then do nothing when the ask never registered. Tests such as `anEmptyAnswerDeliversNothing` can pass without exercising the path they name. Have `until` report failure (for example `Issue.record` or a `#require` on the ask) when the condition never holds.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextAgentActivity/AgentActivityLineConnection.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextAgentActivity/AgentActivityLineConnection.swift:91">
P3: The new `send(_ encode:)` runs the closure before checking whether the connection is still open. After `finish()` a request's JSON encoding is wasted work, and the closure's captured params stay alive until the queue drains. Check `!closed` before calling `encode()`.</violation>
</file>
<file name="cmux-tui/crates/acpmux/src/server/chats.rs">
<violation number="1" location="cmux-tui/crates/acpmux/src/server/chats.rs:216">
P2: The new `chat` field in the `_acpmux/chat_open` response is built with plain `chat_value`, so it omits `attention`, `preview` and `parent`. Every other chat value this module returns is decorated with them via `hub.chat_activity()`. Compute the activity before `blocking` (as `page` does) and decorate the value here.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/FrecencyStore+RankerHistory.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/FrecencyStore+RankerHistory.swift:15">
P3: The doc says this keeps picks that "begin `query`", but the filter keeps picks whose stored prefix the query begins with. Reword it to match the predicate.</violation>
</file>
<file name="scripts/cmux-next/palette-open-stages.py">
<violation number="1" location="scripts/cmux-next/palette-open-stages.py:56">
P3: `--seed-chats` and `--stress` do nothing unless `--search-keys` is also given, and nothing reports that. Reject them with a parser error when `--search-keys` is absent.</violation>
<violation number="2" location="scripts/cmux-next/palette-open-stages.py:194">
P3: The final answer wait happens after the last row capture, so late chat answers are omitted from `CHAT_ROWS` and can falsely fail the seeded benchmark. Capture the palette again after this wait and update the final key's count.</violation>
<violation number="3" location="scripts/cmux-next/palette-open-stages.py:317">
P3: Each `--seed-chats` run leaves its seeded HOME under /tmp because `cleanup()` never removes it. Track the directory and remove it in `cleanup()`.</violation>
<violation number="4" location="scripts/cmux-next/palette-open-stages.py:470">
P3: A missing chat-index precondition does not stop the benchmark: this branch records a failure and then `search_report` still measures. Raise or return after the failed check so invalid runs cannot produce timing samples.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AgentChatsJoinSearchTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AgentChatsJoinSearchTests.swift:61">
P3: This spins on `Task.yield()` up to 500 times to wait for acpmux's request, so the result depends on scheduler timing rather than a deterministic signal. Prefer an explicit signal from the fake `connected` send closure (e.g. an `AsyncStream` or continuation resumed on the matching request) so the test does not depend on how many turns the palette takes.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteModel+Search.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteModel+Search.swift:285">
P2: `held` never reaches `awaitsRank` or the spinner. A chats query with no ranked rows and an answer still pending shows "No Results" until acpmux answers. Set `awaitsRank` from `held` as well as `provisional`.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteQueryFrameMarks.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteQueryFrameMarks.swift:32">
P3: `pendingAnswer` records the raw answer count, not the answer rows actually rendered. Pass the count from the joined/displayed sections so `rows=<n>` reflects what is on screen after Search's share limit and More row.</violation>
</file>
<file name="cmux-tui/crates/acpmux/src/chats/window.rs">
<violation number="1" location="cmux-tui/crates/acpmux/src/chats/window.rs:287">
P2: `ranked_rows` materializes and retains a full `Value` for every matching chat before applying the window limit, so each query's memory and serialization work scales with the entire index despite returning at most 200 rows. Keep lightweight rank candidates for all matches, then build decorated chat values only for the selected indices.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PalettePageState.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PalettePageState.swift:181">
P2: Reloading a contributed page clears its answer rows but leaves `answeredQuery` set, so the next search skips `askContributions` and never restores those rows. Reset `answeredQuery` whenever `cancelAnswers()` clears the answer state.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/ChatsFeedHoldsNoIndexTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/ChatsFeedHoldsNoIndexTests.swift:33">
P3: This test passes whenever the watch page decodes to anything without chats, including `nil`, so it does not confirm the page was handled as the watch answer. Assert the decoded case directly, e.g. `#expect(message == .watching)`, before the chat count.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteContentView.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteContentView.swift:148">
P2: The answer signpost ends with the message-only overload, so a `CommitBoundary` that fires as `.missed` is recorded as a real commit. Rows ends via the outcome-aware overload and get `commit=missed`. Pass `outcome` for the answer too, and add an outcome+message overload to `Signposter` so the `rows=` detail is kept.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AgentChatsMoreChatsPluralsTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AgentChatsMoreChatsPluralsTests.swift:20">
P2: The `2,999` case asserts a thousands separator that `moreChats` probably does not produce. The count is a plain `%lld` interpolation, so the row likely reads `2999 more chats`. Either assert the actual output or drop this case.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| if marks.rows != nil || marks.answer != nil { | ||
| CommitBoundary().after { outcome in | ||
| if let rows = marks.rows { Signposter.key.end(.paletteQueryRows, rows, outcome) } | ||
| if let answer = marks.answer { Signposter.key.end(.paletteQueryAnswer, answer.interval, message: "rows=\(answer.rows)") } |
There was a problem hiding this comment.
P2: The answer signpost ends with the message-only overload, so a CommitBoundary that fires as .missed is recorded as a real commit. Rows ends via the outcome-aware overload and get commit=missed. Pass outcome for the answer too, and add an outcome+message overload to Signposter so the rows= detail is kept.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteContentView.swift, line 148:
<comment>The answer signpost ends with the message-only overload, so a `CommitBoundary` that fires as `.missed` is recorded as a real commit. Rows ends via the outcome-aware overload and get `commit=missed`. Pass `outcome` for the answer too, and add an outcome+message overload to `Signposter` so the `rows=` detail is kept.</comment>
<file context>
@@ -139,6 +140,14 @@ final class PaletteContentView: NSView {
+ if marks.rows != nil || marks.answer != nil {
+ CommitBoundary().after { outcome in
+ if let rows = marks.rows { Signposter.key.end(.paletteQueryRows, rows, outcome) }
+ if let answer = marks.answer { Signposter.key.end(.paletteQueryAnswer, answer.interval, message: "rows=\(answer.rows)") }
+ }
+ }
</file context>
| } | ||
|
|
||
| @Test(arguments: [(1, "1 more chat. Type to search."), (2, "2 more chats. Type to search."), | ||
| (2_999, "2,999 more chats. Type to search.")]) |
There was a problem hiding this comment.
P2: The 2,999 case asserts a thousands separator that moreChats probably does not produce. The count is a plain %lld interpolation, so the row likely reads 2999 more chats. Either assert the actual output or drop this case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AgentChatsMoreChatsPluralsTests.swift, line 20:
<comment>The `2,999` case asserts a thousands separator that `moreChats` probably does not produce. The count is a plain `%lld` interpolation, so the row likely reads `2999 more chats`. Either assert the actual output or drop this case.</comment>
<file context>
@@ -0,0 +1,40 @@
+ }
+
+ @Test(arguments: [(1, "1 more chat. Type to search."), (2, "2 more chats. Type to search."),
+ (2_999, "2,999 more chats. Type to search.")])
+ func theEnglishRow(count: Int, expected: String) throws {
+ let english = try table("en")
</file context>
| } | ||
|
|
||
| /// Waits until `check` holds (an answer lands on a later main-actor turn), or gives up. | ||
| static func until(_ check: () -> Bool) async { |
There was a problem hiding this comment.
P2: until returns silently on timeout, and answer/refuse then do nothing when the ask never registered. Tests such as anEmptyAnswerDeliversNothing can pass without exercising the path they name. Have until report failure (for example Issue.record or a #require on the ask) when the condition never holds.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextPaletteTests/ContributedAnswersTests.swift, line 89:
<comment>`until` returns silently on timeout, and `answer`/`refuse` then do nothing when the ask never registered. Tests such as `anEmptyAnswerDeliversNothing` can pass without exercising the path they name. Have `until` report failure (for example `Issue.record` or a `#require` on the ask) when the condition never holds.</comment>
<file context>
@@ -0,0 +1,346 @@
+ }
+
+ /// Waits until `check` holds (an answer lands on a later main-actor turn), or gives up.
+ static func until(_ check: () -> Bool) async {
+ for _ in 0..<500 where !check() { await Task.yield() }
+ }
</file context>
| .map_err(RpcError::invalid_params)?; | ||
| // The chat itself (title, folder, harness), so a client that opens it holds no | ||
| // copy of the index to name it. | ||
| plan.set("chat", crate::chats::chat_value(&chat)); |
There was a problem hiding this comment.
P2: The new chat field in the _acpmux/chat_open response is built with plain chat_value, so it omits attention, preview and parent. Every other chat value this module returns is decorated with them via hub.chat_activity(). Compute the activity before blocking (as page does) and decorate the value here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/acpmux/src/server/chats.rs, line 216:
<comment>The new `chat` field in the `_acpmux/chat_open` response is built with plain `chat_value`, so it omits `attention`, `preview` and `parent`. Every other chat value this module returns is decorated with them via `hub.chat_activity()`. Compute the activity before `blocking` (as `page` does) and decorate the value here.</comment>
<file context>
@@ -208,8 +209,12 @@ pub(super) async fn route(
+ .map_err(RpcError::invalid_params)?;
+ // The chat itself (title, folder, harness), so a client that opens it holds no
+ // copy of the index to name it.
+ plan.set("chat", crate::chats::chat_value(&chat));
+ Ok(plan)
})
</file context>
| state.rankedComplete = complete | ||
| let joined = Self.joined(sections, state: state) | ||
| state.deliveredAnswerFingerprint = Self.answerFingerprint(state) | ||
| let held = joined.held |
There was a problem hiding this comment.
P2: held never reaches awaitsRank or the spinner. A chats query with no ranked rows and an answer still pending shows "No Results" until acpmux answers. Set awaitsRank from held as well as provisional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteModel+Search.swift, line 285:
<comment>`held` never reaches `awaitsRank` or the spinner. A chats query with no ranked rows and an answer still pending shows "No Results" until acpmux answers. Set `awaitsRank` from `held` as well as `provisional`.</comment>
<file context>
@@ -272,13 +274,26 @@ extension PaletteModel {
+ state.rankedComplete = complete
+ let joined = Self.joined(sections, state: state)
+ state.deliveredAnswerFingerprint = Self.answerFingerprint(state)
+ let held = joined.held
+ let sections = joined.shown
state.lastSections = sections
</file context>
| parser.add_argument("--json", help="write the samples here") | ||
| parser.add_argument("--no-gate", action="store_true", help="report only; exit 0 when every open was timed") | ||
| parser.add_argument("--search-keys", help="time typed keys of this word on Search (Cmd-Shift-A), not palette opens") | ||
| parser.add_argument("--seed-chats", type=int, default=0, help="stored chats in the app's acpmux store (--search-keys)") |
There was a problem hiding this comment.
P3: --seed-chats and --stress do nothing unless --search-keys is also given, and nothing reports that. Reject them with a parser error when --search-keys is absent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/palette-open-stages.py, line 56:
<comment>`--seed-chats` and `--stress` do nothing unless `--search-keys` is also given, and nothing reports that. Reject them with a parser error when `--search-keys` is absent.</comment>
<file context>
@@ -32,6 +52,10 @@
parser.add_argument("--json", help="write the samples here")
parser.add_argument("--no-gate", action="store_true", help="report only; exit 0 when every open was timed")
+parser.add_argument("--search-keys", help="time typed keys of this word on Search (Cmd-Shift-A), not palette opens")
+parser.add_argument("--seed-chats", type=int, default=0, help="stored chats in the app's acpmux store (--search-keys)")
+parser.add_argument("--stress", action="store_true", help="build the STRESS session before typing (--search-keys)")
+parser.add_argument("--vdisplay", type=float, help="run on a 1920x1200 virtual main display at this refresh rate (Hz)")
</file context>
| palette.model.send(.setQuery("zebra")) | ||
| await palette.model.settle() | ||
| // The page is asked in a later turn than the key's, after the choice history is encoded off main. | ||
| for _ in 0..<500 where !sent.contains(where: { $0["query"] as? String == "zebra" }) { await Task.yield() } |
There was a problem hiding this comment.
P3: This spins on Task.yield() up to 500 times to wait for acpmux's request, so the result depends on scheduler timing rather than a deterministic signal. Prefer an explicit signal from the fake connected send closure (e.g. an AsyncStream or continuation resumed on the matching request) so the test does not depend on how many turns the palette takes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AgentChatsJoinSearchTests.swift, line 61:
<comment>This spins on `Task.yield()` up to 500 times to wait for acpmux's request, so the result depends on scheduler timing rather than a deterministic signal. Prefer an explicit signal from the fake `connected` send closure (e.g. an `AsyncStream` or continuation resumed on the matching request) so the test does not depend on how many turns the palette takes.</comment>
<file context>
@@ -0,0 +1,88 @@
+ palette.model.send(.setQuery("zebra"))
+ await palette.model.settle()
+ // The page is asked in a later turn than the key's, after the choice history is encoded off main.
+ for _ in 0..<500 where !sent.contains(where: { $0["query"] as? String == "zebra" }) { await Task.yield() }
+ let asked = sent.last { $0["window"] as? String == AgentChatsPalettePage.window }
+ #expect(asked?["query"] as? String == "zebra", "Search asks acpmux for the query: \(sent)")
</file context>
| self.rows = nil | ||
| } | ||
| if answerRows > 0, let answer { | ||
| pendingAnswer = (answer, answerRows) |
There was a problem hiding this comment.
P3: pendingAnswer records the raw answer count, not the answer rows actually rendered. Pass the count from the joined/displayed sections so rows=<n> reflects what is on screen after Search's share limit and More row.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextPalette/PaletteQueryFrameMarks.swift, line 32:
<comment>`pendingAnswer` records the raw answer count, not the answer rows actually rendered. Pass the count from the joined/displayed sections so `rows=<n>` reflects what is on screen after Search's share limit and More row.</comment>
<file context>
@@ -0,0 +1,45 @@
+ self.rows = nil
+ }
+ if answerRows > 0, let answer {
+ pendingAnswer = (answer, answerRows)
+ self.answer = nil
+ }
</file context>
| # The chat rows on screen for this key (Search's Agent Chats section; at the base, ranked chat rows). | ||
| shown = (rpc("debug.palette.capture", {"path": os.path.join(SCRATCH, "key.png")}) or {}).get("rows") or [] | ||
| CHAT_ROWS.append(sum(1 for row in shown if str(row).startswith("chat:"))) | ||
| time.sleep(0.4) # test harness: the last key's answer lands |
There was a problem hiding this comment.
P3: The final answer wait happens after the last row capture, so late chat answers are omitted from CHAT_ROWS and can falsely fail the seeded benchmark. Capture the palette again after this wait and update the final key's count.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/palette-open-stages.py, line 194:
<comment>The final answer wait happens after the last row capture, so late chat answers are omitted from `CHAT_ROWS` and can falsely fail the seeded benchmark. Capture the palette again after this wait and update the final key's count.</comment>
<file context>
@@ -152,6 +176,175 @@ def one_open(params):
+ # The chat rows on screen for this key (Search's Agent Chats section; at the base, ranked chat rows).
+ shown = (rpc("debug.palette.capture", {"path": os.path.join(SCRATCH, "key.png")}) or {}).get("rows") or []
+ CHAT_ROWS.append(sum(1 for row in shown if str(row).startswith("chat:")))
+ time.sleep(0.4) # test harness: the last key's answer lands
+ hangs = rpc("debug.hangs") or {}
+ closed = rpc("debug.key", {"key": "escape", "target": "palette"})
</file context>
| time.sleep(0.4) # test harness: the last key's answer lands | |
| time.sleep(0.4) # test harness: the last key's answer lands | |
| if keys: | |
| shown = (rpc("debug.palette.capture", {"path": os.path.join(SCRATCH, "key.png")}) or {}).get("rows") or [] | |
| CHAT_ROWS[-1] = sum(1 for row in shown if str(row).startswith("chat:")) |
| print("STRESS-SESSION", json.dumps(summary)[:600], flush=True) | ||
| if opts.seed_chats: | ||
| index = wait_for_index(opts.search_keys.split()[0]) | ||
| if not ((index.get("window_total") or index.get("window_rows") or 0) > 0) and not str(index.get("error") or "").startswith("no acpmux"): |
There was a problem hiding this comment.
P3: A missing chat-index precondition does not stop the benchmark: this branch records a failure and then search_report still measures. Raise or return after the failed check so invalid runs cannot produce timing samples.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/palette-open-stages.py, line 470:
<comment>A missing chat-index precondition does not stop the benchmark: this branch records a failure and then `search_report` still measures. Raise or return after the failed check so invalid runs cannot produce timing samples.</comment>
<file context>
@@ -224,6 +457,23 @@ def main():
+ print("STRESS-SESSION", json.dumps(summary)[:600], flush=True)
+ if opts.seed_chats:
+ index = wait_for_index(opts.search_keys.split()[0])
+ if not ((index.get("window_total") or index.get("window_rows") or 0) > 0) and not str(index.get("error") or "").startswith("no acpmux"):
+ row("acpmux index", f"rows for {opts.search_keys!r} before typing", json.dumps(index)[:300], False)
+ report = {"tag": TAG, "stress": opts.stress, "seed_chats": opts.seed_chats, "search_keys": search_report(opts.search_keys)}
</file context>
…en (red) Review 6 of #28090: the skip of a no-change delivery compared only answer row ids, so a re-answer with the same chats under new titles was dropped.
…the same Review 6 of #28090: the answer fingerprint held the raw awaited set, so an empty answer ending under ranked rows still reloaded the list and ran the reducer (anEmptyAnswerDeliversNothing failed, step 9e19c1b1878a14e485b60be6). It now holds whether the rows are held (one shared rule, isHeld, which joined() uses too) and each answer row as it shows, so a retitled chat with the same id still reaches the screen. Builds on the fork's 7e82205 (bench/pr28090-fp-fix).
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Owner of the pushes to this PR is now the perf lead's agent (lead, 13:33Z); the fix agent and the conflict resolver no longer push here. Review 6 F1 (a no-change redelivery still ran on an empty answer) and a gap found while fixing it:
Fleet:
Per the lead's decision (13:33Z, confirming 11:46Z), this PR lands as a correctness fix with no perf claim. The measured A/B is reported as it is, and no new A/B gates it. |
|
Review (claude-opus-review-subagent-7): SHIP at c568b63 Scope: the whole PR against merge-base 0e4ed40, and the repair 6745efb..c568b63 (ad00bfa adds only the test, c568b63 changes only Lead decision applied: this PR lands as a correctness fix with no perf claim. The A/B in the body is reported as measured. A missing measurement is not a finding. Review 6 F1: closed
Fingerprint: no real change is skipped
Red and green evidence (checked in the fleet logs)
Earlier findings: none came back
Checked, no defect found
Notes (do not block)
|
Summary
Step 2 of #28055 (P-RUST, R7: no permanent second path). The app holds no copy of acpmux's chat index.
AcpmuxChatsStore(the mirror whose whole-dictionary copy and deep==on the main thread were the v2.2.9 hangs, base measurement in #28055) is deleted with its last readers:newtab.cards: the newest 24 top-level chats,AgentPaneDeviceChat.maximumPushed). Only that window's messages push to the pages.palette.chats) whose spec carries the query. acpmux ranks every chat with the palette's own ranker (cmux-palette-ranker, the crate the palette runs through its C ABI) over the same fields the page searched: title, folder and harness, every word anywhere, best match first. It answers the best 200 and the total that matched; when more match, the page's last row says so ("The best 200 of N matching chats. Type more to narrow."). The query's result ends after that answer (the spinner stops; an empty answer shows the empty state). The window closes with the page.queryResults) has no providers, so the palette asks it for each query through one shared path (PaletteContributedAnswersin CmuxNextPalette), in a later turn than the key's (the chats' choice history is encoded off the main thread, and ChatsFeed encodes each request on the connection's serial queue).ready: false), the answer ends at once with no rows and Return runs the best agent session. When acpmux connects or its index fills the window, the shown query is asked again._acpmux/chat_opennow answers the chat itself with its plan. A running chat is found by its key (harness:sessionId).preview.#28055 has landed; the base is
feat-cmux-next. After the base merges, the page moved from CmuxNextApp into CmuxNextAgentLanes (5e88f79) and AppServices holds the lane store (63702fa).Behavior change: a harness name ("codex") and words that are not next to each other match as before, and a chat the person chose before ranks first among equal matches as before (the page sends its choice history of chats; acpmux ranks with the same palette ranker and the same frecency). The page shows the best 200 matches and says when more match; the empty query shows the newest 200 and says how many more chats there are ("N more chats. Type to search."). Known difference (lead decision): the relative-date text ("yesterday") is no longer searched; it is follow-up 2 in followups.md.
Changelog
Changed: Search Agent Chats shows the best 200 matching chats (or the newest 200) and says how many more there are; it no longer finds a chat by its relative date. In Search (Cmd-Shift-A), Agent Chats is its own section after the best match, in the order the chats index ranks them, and comes in a moment after the other rows; when the page is full it shows the best 10 and a row that opens Agent Chats with the rest.
Testing
Rust:
tests/chats_window.rsa_query_window_ranks_like_the_palette_and_reports_how_many_matched("claude" finds every claude-code chat with total 30; "fix build" finds the 15 chats that hold both words) andwindow.rsunit tests ("codex" finds codex chats; non-adjacent words; the folder; an untitled chat). Red at 5c5380f, green at the head.Swift:
AgentChatsPaletteAnswerTests(the stream ends after the first answer; the notice row past the page;untitledsent). Red at 833106b and 59af11d, green at the head.Swift:
ChatsFeedHoldsNoIndexTests(a 3,000-chat watch page and one index change reach the main thread with noAcpmuxChat). Red at 1790b10, green at the head (job ids in a comment).Rust:
tests/chats_open.rschat_open_then_session_new_adopts_from_the_profile_storechecks that the plan names its chat. Red at 1790b10, green at the head.Swift:
ContributedAnswersTests(review 2 of the fix: a Return before the new query's rank runs its best row; a late chats answer keeps the selected best row first; the chats stay until the next answer replaces them; the Chats scope's Return waits for the chats) andaWindowIsNotAnsweredAgainUntilTheNewConnectionAnswers. Red at 26fb2bc, green at 184f853 (job ids in a comment).Swift:
AgentChatsJoinSearchTests(Search and the Chats & agents scope, with the realAgentChatsPalettePageas the App contributes it, ask acpmux and list its chat). Red at 6150416, green at fdab8c7 withAgentChatsPaletteAnswerTests,CmdShiftASearchesEverythingTestsandCmdPListsOpenItemsTests(job ids in a comment).Compile:
cmux-ci compile-check --app --package CmuxNext --recipe-version 2at the head.Lockfile: acpmux now depends on
cmux-palette-ranker. Thecmux-tui/Cargo.lockchange (one line in the acpmux entry) is the output ofcargo update -p cmux-palette-ranker --offline(rewrites the lock only, builds nothing), run on the base lock; it is byte-identical to the committed lock.cargo metadata --locked --offline --format-version 1accepts the head's lock (exit 0) and refuses the base lock with the new dependency (exit 101).Observed / Not run
Results of the jobs go in comments; the last ones are in issuecomment-6547205694 and the A/B comment.
Observed, STRESS A/B (bench build, 60-workspace STRESS session on a 1920x1200 virtual display, 3000 seeded chats, 220 typed keys of "idle" per side, same Mac cmux23, through the mini scheduler). Before = merge-base 25c1584 plus the bench harness (68c8e00), job 84b28b44de05d9aa3cbcd8c4. After = 8dc940d, job f7e2a0be41b5b9de9cc24c01:
debug.key)key.palette_query_rows(edit to the frame with the final ranked rows)Perf claim: none beyond #28055's counts. This is a correctness change. The table reports the A/B as it was measured; it is not a claim that the change makes Search faster or slower.
Not run:
key.palette_query_answer(the edit to the frame that shows the chats answer) was not measured. No chats answer landed on either side, so that interval has no samples. Why acpmux gave no rows for the 3000 seeded chats is under investigation separately (diagnosis probe 9692561c9f3f8d03130e1e45). At the base, chats were ranked rows from the deleted mirror, so the base can have no answer frame.No capture: the rows look the same; only their source changes.
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Deletes the app's mirror of acpmux's chat index (
AcpmuxChatsStore) — the whole-dictionary copy and deep equality on the main thread behind the v2.2.9 hangs in #28055. Every reader now asks acpmux for the chats it needs, so the feed holds no chat of the index.What changed
untitledword. acpmux ranks with the palette's own ranker by title, folder and harness (every word anywhere; a chat chosen before ranks above an equal match) and answers the best 200 matches and how many matched; a last, disabled row says how many more matched than the page shows, in each locale's plural form.PaletteContributedAnswers). An answer shows only with the current query's ranked rows; an older query's answer is a stand-in that navigation never selects and never holds Return. A non-leading answer gets at least its best 10 rows in Search, and when the page cuts it, ends with a "more" row that says how many more match and opens the Agent Chats page with the query; in the Chats scope the chats lead, so Return waits for the best chat. A query's answer ends at once with no rows when acpmux is not connected, is not ready or refuses the window's request (an older spec's refusal is ignored), and there is no timer: every request ends with rows, with none, or when the query moves on. An answer that changes nothing delivers nothing — the skip compares the shown rows and the held state, so a re-answer with the same chats under new titles still reaches the screen — and shown and selectable rows cut the page at the same place. The page asks the shown query again when acpmux connects or its not-ready index fills the window._acpmux/chat_openanswers the chat itself with its plan; Open Chat and reveal find a running chat by its key (harness:sessionId).preview.Testing
ChatsFeedHoldsNoIndexTests: a 3,000-chat watch page and one index change reach the main thread with noAcpmuxChat.AgentChatsPaletteAnswerTestsandAgentChatsMoreChatsPluralsTests: a query stream ends after acpmux's first answer, and the more-notice row shows and pluralizes per locale.AgentChatsJoinSearchTestsandContributedAnswersTests: Search and the Chats & agents scope list the chats acpmux answers; old answers are never returned on, chats keep a minimum share of a full Search page (with a more row when cut), a missing, not-ready or refused answer never holds Return, a stale refusal is ignored, the page asks again when acpmux connects, navigation never stops on a stand-in, a retitled re-answer reaches the screen, and an empty answer with no stand-ins changes nothing.tests/chats_open.rschecks that_acpmux/chat_opennames its chat in the plan;tests/chats_window.rscovers a query window's rank order (choice history, harness names, non-adjacent words) and its match total.cmux-ci compile-checkpasses at the head;palette-open-stages.py --search-keystimes typed keys on Search to the frames that show the query's rows and its contributed answer (signpostskey.palette_query_rowsandkey.palette_query_answer), seeding chats where acpmux's index reads them (transcripts under a tempHOME), asking the app's own acpmux for the word before typing and counting the chat rows each key shows;--vdisplayon a 1920x1200 virtual display so the STRESS window fits, report only, no CI.Written for commit c568b63. Summary will update on new commits.