Repository navigation
perf(cmux-next): persist diff comments off the UI actor - #24610
Conversation
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 1effa89 to compare; do not merge until green; see lane-rules Tour of |
|
Found 10 test failures on Blacksmith runners: Failures
|
79e5d92 to
9f52824
Compare
|
Bead |
9f52824 to
cc6272c
Compare
cc6272c to
17fa872
Compare
… compile red) b9c80fa (#24610) sets latestCommentsSequence from the +Comments extension in another file, but the property is private: "'latestCommentsSequence' is inaccessible due to 'private' protection level" (fleet build c245113c75ddae0600ce28ab). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…'s daemon; the comment sequence is internal #24627 (f8c226e) wrote `daemon.supports(...)` in TabMoves.newWorkspaceName, a static func with no daemon in scope, so `daemon` was Darwin's daemon(Int32, Int32) and the app did not compile. It now reads the source workspace's daemon as WorkspaceAutoNameWiring.allowsLegacyDefaults does (no daemon: legacy defaults apply). #24610 (b9c80fa) set FilePageProvider's private latestCommentsSequence from FilePageProvider+Comments.swift, another file; it is internal now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…der's comment sequence is visible to its extension Two more compile errors on feat-cmux-next from #24627 and #24610 (fleet compile-check job 0c66846f79c254a414e309b4): - TabMoves.newWorkspaceName called `daemon.supports(...)` in a static func, which resolved to libc's daemon(3); it now asks the source workspace's daemon through services.machines.daemon(forWorkspace:), as WorkspaceAutoNameWiring does. - FilePageProvider+Comments.swift wrote the private latestCommentsSequence of FilePageProvider.swift; it is internal now.
…NextWorkspaceNaming, TabMoves) - b9c80fa (#24610) reads latestCommentsSequence from FilePageProvider+Comments.swift while it stayed private to FilePageProvider.swift: now internal to the module. - f8c226e (#24627) and 70be1be (#24628) put WorkspaceNameSource (CmuxNextDaemon) in public signatures of NewWorkspaceName and WorkspaceAutoName under an internal import: now a public import. - f8c226e (#24627) read `daemon` in TabMoves.newWorkspaceName, where no such value exists (it resolved to Darwin's daemon(3)): it now asks the source workspace's own daemon, as WorkspaceAutoNameWiring.allowsLegacyDefaults does (no daemon: legacy defaults allowed). CmuxNextApp and CmuxNextWorkspaceNaming did not compile on feat-cmux-next (cmux-ci steps f1cbf88c2b78e129fd383836, 7ddea01f7ac1bccd493aebbb, compile check 197c7d71ec98e5f8efdb2136). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…der's comment sequence is visible to its extension Two more compile errors on feat-cmux-next from #24627 and #24610 (fleet compile-check job 0c66846f79c254a414e309b4): - TabMoves.newWorkspaceName called `daemon.supports(...)` in a static func, which resolved to libc's daemon(3); it now asks the source workspace's daemon through services.machines.daemon(forWorkspace:), as WorkspaceAutoNameWiring does. - FilePageProvider+Comments.swift wrote the private latestCommentsSequence of FilePageProvider.swift; it is internal now.
…der's comment sequence is visible to its extension Two more compile errors on feat-cmux-next from #24627 and #24610 (fleet compile-check job 0c66846f79c254a414e309b4): - TabMoves.newWorkspaceName called `daemon.supports(...)` in a static func, which resolved to libc's daemon(3); it now asks the source workspace's daemon through services.machines.daemon(forWorkspace:), as WorkspaceAutoNameWiring does. - FilePageProvider+Comments.swift wrote the private latestCommentsSequence of FilePageProvider.swift; it is internal now.
…; the comment sequence is readable from its extension
- TabMoves.newWorkspaceName(for:services:) called daemon.supports(...) with no
daemon in scope, so Swift resolved libc's daemon(_:_:) ("value of type
'@sendable (Int32, Int32) -> Int32' has no member 'supports'"); it asks
services.machines.daemon(forTab:) like the other moves. Broken by
f8c226e (#24627).
- FilePageProvider.latestCommentsSequence was private while
FilePageProvider+Comments.swift (another file) reads and writes it; it is
internal. Broken by b9c80fa (#24610).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…'s daemon; the comment sequence is internal daemon in scope, so `daemon` was Darwin's daemon(Int32, Int32) and the app did not compile. It now reads the source workspace's daemon as WorkspaceAutoNameWiring.allowsLegacyDefaults does (no daemon: legacy defaults apply). #24610 (b9c80fa) set FilePageProvider's private latestCommentsSequence from FilePageProvider+Comments.swift, another file; it is internal now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mpiles #24627 (f8c226e) wrote `daemon.supports(...)` in TabMoves.newWorkspaceName, a static func with no daemon in scope, so `daemon` was Darwin's daemon(Int32, Int32) and the app did not compile. It now reads the source workspace's daemon as WorkspaceAutoNameWiring.allowsLegacyDefaults does (no daemon: legacy defaults apply). (The FilePageProvider comment-sequence error from #24610 was fixed upstream by #24646.) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sitoryTests imports, SidebarGroupLineLayoutTests var) feat-cmux-next's CmuxNextAppTests did not compile (fleet compile-tests step bbf3c4c87fb08a4528741f48 on aws-m4pro-2): - MarkdownCommentRepositoryTests (#24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesign. - SidebarGroupLineLayoutTests (#24635) sets child.parent on a let SidebarGroup (a struct).
…sitoryTests imports, SidebarGroupLineLayoutTests var) feat-cmux-next's CmuxNextAppTests did not compile (fleet compile-tests step bbf3c4c87fb08a4528741f48 on aws-m4pro-2): - MarkdownCommentRepositoryTests (#24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesign. - SidebarGroupLineLayoutTests (#24635) sets child.parent on a let SidebarGroup (a struct).
…; the comment sequence is readable from its extension
- TabMoves.newWorkspaceName(for:services:) called daemon.supports(...) with no
daemon in scope, so Swift resolved libc's daemon(_:_:) ("value of type
'@sendable (Int32, Int32) -> Int32' has no member 'supports'"); it asks
services.machines.daemon(forTab:) like the other moves. Broken by
f8c226e (#24627).
- FilePageProvider.latestCommentsSequence was private while
FilePageProvider+Comments.swift (another file) reads and writes it; it is
internal. Broken by b9c80fa (#24610).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sitoryTests imports and typed sequences, SidebarGroupLineLayoutTests var) feat-cmux-next's CmuxNextAppTests did not compile (fleet compile-tests steps bbf3c4c87fb08a4528741f48 and 90534172dbdd07c288701002 on aws-m4pro-2): - MarkdownCommentRepositoryTests (#24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesign, and its sequence check [Int] == [1, 2, 3] was an ambiguous use of == (the literal also fits IndexSet and IndexPath); the sequences are typed [Int]. - SidebarGroupLineLayoutTests (#24635) sets child.parent on a let SidebarGroup (a struct).
…sitoryTests imports and typed sequences, SidebarGroupLineLayoutTests var) feat-cmux-next's CmuxNextAppTests did not compile (fleet compile-tests steps bbf3c4c87fb08a4528741f48 and 90534172dbdd07c288701002 on aws-m4pro-2): - MarkdownCommentRepositoryTests (#24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesign, and its sequence check [Int] == [1, 2, 3] was an ambiguous use of == (the literal also fits IndexSet and IndexPath); the sequences are typed [Int]. - SidebarGroupLineLayoutTests (#24635) sets child.parent on a let SidebarGroup (a struct).
Test code: - AAGroupLineGalleryTests, SidebarStatusCleanupGalleryTests: ThemeStore.shared is gone since B3 (73c3d0d); they apply the theme through ThemeScope.app.store like the other galleries. - SidebarGroupLineLayoutTests: SidebarGroup is a struct; the child the test re-parents is a var. - MarkdownCommentRepositoryTests: its FilePageHosting stub uses JSONValue and SettingWriter, which come from CmuxNextSettings (missing import since b9c80fa, #24610). Product, import-only build fix in the sidebar area (a590's), no behavior change: CmuxNextAgentLanes/SidebarLocalLanes.swift's public extension of AgentLaneStore takes WorkspaceModel and AgentTurnStateStore, so its imports of CmuxNextAgentActivity, CmuxNextBridge and CmuxNextDaemon are public (InternalImportsByDefault; c2e943f, #24651). Product, one word: CmuxNextFeed/Views/FeedView.swift's FeedView declared onReportsReply as a let with a default, which is not in the memberwise initializer, so FeedHostView's FeedView(..., onReportsReply:) did not compile ("extra argument 'onReportsReply' in call"); it is a var (9222c97, #24616). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test code: - AAGroupLineGalleryTests, SidebarStatusCleanupGalleryTests: ThemeStore.shared is gone since B3 (73c3d0d); they apply the theme through ThemeScope.app.store like the other galleries. - SidebarGroupLineLayoutTests: SidebarGroup is a struct; the child the test re-parents is a var. - MarkdownCommentRepositoryTests: its FilePageHosting stub uses types from CmuxNextSettings and CmuxNextDesign, which it never imported (b9c80fa, #24610). Product, minimal build fixes, no behavior change: - Import-only build fix in the sidebar area (a590's): CmuxNextAgentLanes/SidebarLocalLanes.swift's public extension of AgentLaneStore takes WorkspaceModel and AgentTurnStateStore, so its imports of CmuxNextAgentActivity, CmuxNextBridge and CmuxNextDaemon are public (InternalImportsByDefault; c2e943f, #24651); CmuxNextApp/SidebarBridge.swift imports CmuxNextAgentActivity for the members it uses. - CmuxNextFeed/Views/FeedView.swift: onReportsReply is a var optional (no explicit nil, swiftlint) so the memberwise init takes it; a let with a default is not an init parameter (9222c97, #24616). - CmuxNextAgentPane/AcpmuxMailboxClient.swift: public import Foundation (a public Date in its API). - CmuxNextApp/Feed/FeedPageService.swift: the reply closure captures [services] explicitly (Swift 6). Its Process.run() on the main thread is unchanged (cmux-4f's PL-main owns it). Errors found together with session cmux-4f's PL-main (origin/pl-main-testfix4 085eaa6), which dropped its push for this one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test code: - AAGroupLineGalleryTests, SidebarStatusCleanupGalleryTests: ThemeStore.shared is gone since B3 (73c3d0d); they apply the theme through ThemeScope.app.store like the other galleries. - SidebarGroupLineLayoutTests: SidebarGroup is a struct; the child the test re-parents is a var. - MarkdownCommentRepositoryTests: its FilePageHosting stub uses types from CmuxNextSettings and CmuxNextDesign, which it never imported (b9c80fa, #24610). Product, minimal build fixes, no behavior change: - Import-only build fix in the sidebar area (a590's): CmuxNextAgentLanes/SidebarLocalLanes.swift's public extension of AgentLaneStore takes WorkspaceModel and AgentTurnStateStore, so its imports of CmuxNextAgentActivity, CmuxNextBridge and CmuxNextDaemon are public (InternalImportsByDefault; c2e943f, #24651); CmuxNextApp/SidebarBridge.swift imports CmuxNextAgentActivity for the members it uses. - CmuxNextFeed/Views/FeedView.swift: onReportsReply is a var optional (no explicit nil, swiftlint) so the memberwise init takes it; a let with a default is not an init parameter (9222c97, #24616). - CmuxNextAgentPane/AcpmuxMailboxClient.swift: public import Foundation (a public Date in its API). - CmuxNextApp/Feed/FeedPageService.swift: the reply closure captures [services] explicitly (Swift 6). Its Process.run() on the main thread is unchanged (cmux-4f's PL-main owns it). Errors found together with session cmux-4f's PL-main (origin/pl-main-testfix4 085eaa6), which dropped its push for this one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=codex&session=01a16ae4-6466-71c0-b22c-5513fa7573e9&host=huge-blue
…sitoryTests imports and typed sequences, SidebarGroupLineLayoutTests var) feat-cmux-next's CmuxNextAppTests did not compile (fleet compile-tests steps bbf3c4c87fb08a4528741f48 and 90534172dbdd07c288701002 on aws-m4pro-2): - MarkdownCommentRepositoryTests (#24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesign, and its sequence check [Int] == [1, 2, 3] was an ambiguous use of == (the literal also fits IndexSet and IndexPath); the sequences are typed [Int]. - SidebarGroupLineLayoutTests (#24635) sets child.parent on a let SidebarGroup (a struct).
Test code: - AAGroupLineGalleryTests, SidebarStatusCleanupGalleryTests: ThemeStore.shared is gone since B3 (73c3d0d); they apply the theme through ThemeScope.app.store like the other galleries. - SidebarGroupLineLayoutTests: SidebarGroup is a struct; the child the test re-parents is a var. - MarkdownCommentRepositoryTests: its FilePageHosting stub uses types from CmuxNextSettings and CmuxNextDesign, which it never imported (b9c80fa, #24610). Product, minimal build fixes, no behavior change: - CmuxNextFeed/Views/FeedView.swift: onReportsReply optionals without an explicit nil (swiftlint), so the memberwise init still takes them. - CmuxNextApp/Feed/FeedPageService.swift: the reply closure captures [services] explicitly (Swift 6). Its Process.run() on main is unchanged (cmux-4f's PL-main owns it). The import fixes for SidebarLocalLanes, SidebarBridge and AcpmuxMailboxClient and the acpmux titles.rs fix (6999224) are already on the tip. Errors found together with session cmux-4f's PL-main (085eaa6), which dropped its push. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test code: - AAGroupLineGalleryTests, SidebarStatusCleanupGalleryTests: ThemeStore.shared is gone since B3 (73c3d0d); they apply the theme through ThemeScope.app.store like the other galleries. - SidebarGroupLineLayoutTests: SidebarGroup is a struct; the child the test re-parents is a var. - MarkdownCommentRepositoryTests: the sequence assertion types its literal ([Int]: an untyped array literal is ambiguous for == inside #expect), and its FilePageHosting stub uses types from CmuxNextSettings and CmuxNextDesign, which it never imported (b9c80fa, #24610). Product, minimal build fixes, no behavior change: - CmuxNextFeed/Views/FeedView.swift: onReportsReply optionals without an explicit nil (swiftlint), so the memberwise init still takes them. - CmuxNextApp/Feed/FeedPageService.swift: the reply closure captures [services] explicitly (Swift 6). Its Process.run() on main is unchanged (cmux-4f's PL-main owns it). The import fixes for SidebarLocalLanes, SidebarBridge and AcpmuxMailboxClient and the acpmux titles.rs fix (6999224) are already on the tip. Errors found together with session cmux-4f's PL-main (085eaa6), which dropped its push. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hared instance; comment repository test imports; a nested group test's child) 73c3d0d (B3) removed ThemeStore.shared; two sidebar gallery tests still applied their theme through it. MarkdownCommentRepositoryTests (b9c80fa, #24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesignTokens, and its awaited array comparison was ambiguous. SidebarGroupLineLayoutTests (#24635) sets the parent of a let group. The test target did not compile on feat-cmux-next (cmux-ci steps 1bb712123db802d37528912d, f012101c57927662812e05a8). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Settings and CmuxNextDesign #24610 added the test with a FilePageHosting fake that names JSONValue, SettingWriter (CmuxNextSettings) and RecoveryDraftAcceptance (CmuxNextDesignTokens, re-exported by CmuxNextDesign) without importing them, so CmuxNextAppTests does not compile. Import them the way EditorRevealConfigTests does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hared instance; comment repository test imports; a nested group test's child) 73c3d0d (B3) removed ThemeStore.shared; two sidebar gallery tests still applied their theme through it. MarkdownCommentRepositoryTests (b9c80fa, #24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesignTokens, and its awaited array comparison was ambiguous. SidebarGroupLineLayoutTests (#24635) sets the parent of a let group. The test target did not compile on feat-cmux-next (cmux-ci steps 1bb712123db802d37528912d, f012101c57927662812e05a8). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sitoryTests imports and typed sequences, SidebarGroupLineLayoutTests var) feat-cmux-next's CmuxNextAppTests did not compile (fleet compile-tests steps bbf3c4c87fb08a4528741f48 and 90534172dbdd07c288701002 on aws-m4pro-2): - MarkdownCommentRepositoryTests (#24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesign, and its sequence check [Int] == [1, 2, 3] was an ambiguous use of == (the literal also fits IndexSet and IndexPath); the sequences are typed [Int]. - SidebarGroupLineLayoutTests (#24635) sets child.parent on a let SidebarGroup (a struct).
…sitoryTests imports and typed sequences, SidebarGroupLineLayoutTests var) feat-cmux-next's CmuxNextAppTests did not compile (fleet compile-tests steps bbf3c4c87fb08a4528741f48 and 90534172dbdd07c288701002 on aws-m4pro-2): - MarkdownCommentRepositoryTests (#24610) names JSONValue, SettingWriter and RecoveryDraftAcceptance without importing CmuxNextSettings and CmuxNextDesign, and its sequence check [Int] == [1, 2, 3] was an ambiguous use of == (the literal also fits IndexSet and IndexPath); the sequences are typed [Int]. - SidebarGroupLineLayoutTests (#24635) sets child.parent on a let SidebarGroup (a struct).
|
Shipped in:
|
|
Shipped in:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
Diff-comment list decoding, mutation, trimming, and encoding now run in a dedicated
MarkdownCommentRepositoryactor. The page provider awaits that boundary and publishes the persisted JSON without doing comment work on the UI actor. A FIFO mutation queue and monotonic publication sequence preserve add/delete order across storage and JSON awaits; the current-file guard suppresses results after navigation.Leo said yes.
Performance evidence
Same fixed harness: 100 comments added in order to one transcript-sized comment list, using the same injected storage and Xcode 26.6 fleet lane:
The metric is total persistence work for the fixed sequence; parsing, mutation, encoding, and I/O are actor-isolated from the page actor. The final head adds ordering and navigation guards without changing the measured persistence path.
Validation
0253d85f9a65201e156b7c3f:MarkdownCommentRepositoryTests, 2 tests passed onaws-m4pro-3, Xcode 26.6;ThreadProbewas[false,false,false]and ordering/durability assertions passed.99647b4b47210ad624cb7d29: same harness on parent, 2 tests passed onaws-m4pro-6, Xcode 26.6.host_lock; no native receipt exists for17fa8720cebeaa95122b32712f24db4779648073.17fa8720cebeaa95122b32712f24db4779648073.FilePageProvider.swiftandFilePageService.swiftfor unrelated file-line routing; this PR changes only the comment persistence path and has no shared symbols.Agent session: big-red