fix: restore macOS UX and complete memory graph - #11035
Merged
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
17 issues found across 81 files
Confidence score: 2/5
desktop/macos/Desktop/Sources/Stores/APIClient+Tasks.swiftanddesktop/macos/Desktop/Sources/Stores/TasksStore.swiftcurrently use a No Deadline boundary/pagination approach that can skip tasks (especially once dated incomplete tasks exceed backend caps), so users may never see valid tasks and list sections can be mis-partitioned — switch to a backend-backed pagination contract with stable ordering/cursors instead ofdatedCount + offsetassumptions.- In
desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskDetailPanel.swift, the “Work on this with Omi” path for tasks without a workstream opens chat UI without creating a thread, which makes the action appear broken — route this branch throughopenChat(for:)while keepingopenExistingThreadfor linked tasks. desktop/macos/Desktop/Sources/MainWindow/Pages/MemoriesPage.swiftcan render a blank list for empty layers becausememoriesremains scoped to defaults whilefilteredMemoriesexcludes them, creating a mismatch that looks like missing UI state — apply layer filtering to the authoritative page dataset before rendering.desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskMultiSelection.swiftanddesktop/macos/Desktop/Sources/MainWindow/Tasks/TaskBulkOperationCoordinator.swiftappear unhooked from productionTasksPageflows, so multi-select and coordinated bulk behavior are effectively unreachable for users — wireTaskMultiSelectionState/coordinator into task-page events and retire the separatedeleteSelectedTasks()path.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/routers/knowledge_graph.py">
<violation number="1" location="backend/routers/knowledge_graph.py:141">
P3: Canonical read failures have no route-level regression test, so a future broad exception or status mapping change can silently turn the intended retryable 503 into a 500/400. Add a test that makes `get_canonical_knowledge_graph_payload` raise `CanonicalGraphReadUnavailable` and asserts `503` plus `canonical_graph_unavailable`.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/Rewind/Core/ActionItemStorage.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/Rewind/Core/ActionItemStorage.swift:333">
P3: Tasks-page bucket/pagination policy now lives in `ActionItemStorage`, coupling the persistence actor to one UI surface and making future task readers inherit an ambiguous contract. Keeping the two filtered reads and surface assembly in `TasksStore` (or a dedicated service) would keep storage focused on reusable persistence queries.
(Based on your team's feedback about keeping DB-layer logic out.)</violation>
</file>
<file name="desktop/macos/Desktop/Sources/Stores/APIClient+Tasks.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/Stores/APIClient+Tasks.swift:105">
P1: No Deadline pagination can skip tasks because `datedCount + offset` assumes the general endpoint's global order has every dated row before every null-due row. The current backend only applies that sort after a bounded, unordered Firestore read; add a backend null-due cursor/filter contract before using this boundary.</violation>
</file>
<file name="desktop/macos/Desktop/Tests/MemoryLayerFilterTests.swift">
<violation number="1" location="desktop/macos/Desktop/Tests/MemoryLayerFilterTests.swift:154">
P3: These modified source-inspection assertions grep the MemoriesPage.swift source text without the `// omi-test-quality: source-inspection` tripwire reason that AGENTS.md requires for this file. The behavioral test (`testEmptyAuthoritativeServerPageDoesNotDisplayNewerCachedMemory`) already proves behavior by calling `MemoryPageProjection.visibleMemories`, so the parallel `source.contains(...)` greps (e.g. `source: .authoritativeServer`, `hasAuthoritativeServerProjection`, `canRenderCacheBeforeAuthoritativeFetch`) add brittle, tripwire-free coverage. Consider either asserting viewed behavior via the production API or annotating these greps with the mandated tripwire reason.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/Chat/AgentContextAdmissionGate.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/Chat/AgentContextAdmissionGate.swift:34">
P3: Large admission bursts pay quadratic queue-maintenance cost because every handoff shifts all remaining waiters. Use a deque or head index with periodic compaction for FIFO dequeue.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskDetailPanel.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskDetailPanel.swift:603">
P2: For tasks without a workstream, this action opens the chat panel but creates no thread, despite its “Work on this with Omi” label. Wire this branch to `openChat(for:)` (keeping `openExistingThread` for linked tasks), or label it as a non-creating action.</violation>
</file>
<file name="desktop/macos/Desktop/Tests/KnowledgeGraphPaginationTests.swift">
<violation number="1" location="desktop/macos/Desktop/Tests/KnowledgeGraphPaginationTests.swift:71">
P3: The busy-poll in waitUntilStarted has no upper bound, so a future change that causes getKnowledgeGraph to fence out before startLoading() will make this test hang the whole suite instead of failing. Consider bounding the poll (loop count or a real continuation/expectation) so the test fails rather than stalls.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/Stores/TasksStore.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/Stores/TasksStore.swift:1226">
P1: Accounts with more than 2,000 dated incomplete tasks stop at the backend read cap, so later dated tasks never reach the surface and the subsequent No Deadline boundary is wrong. Use a backend pagination contract that can advance beyond the bounded offset window (for example a cursor), or make this route expose an explicit complete-dated query before claiming a full scan.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Components/ChatMessagesView.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatMessagesView.swift:522">
P3: The placement is marked `.completed` based on `scrollMode == .followingBottom` without confirming the final `scrollToBottom` actually moved the viewport. Since `scrollToBottom` internally no-ops when `userIsScrolling`/`messages.isEmpty`, this can mark the one-shot placement complete without the bottom placement having executed — which the INV-CHAT-2 invariant added in this PR explicitly forbids. Consider capturing whether the scroll executed (e.g., only completing when the scroll command actually ran) so a no-op final stage doesn't permanently close the retry window.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskMultiSelection.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskMultiSelection.swift:29">
P2: Task multi-selection is not reachable in the macOS app: this state model is only exercised by tests and no Tasks surface creates or calls it. Wire `TaskMultiSelectionState` into the task-page event and bulk-action flow, or omit this unused implementation until that integration is included.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/CaptureListeningControls.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/CaptureListeningControls.swift:63">
P2: When transcription is unavailable the title becomes 'Transcription unavailable', which is much wider than the 104pt 'Listening' label the 136pt slot was sized for. Because the slot is a fixed width and the pill's text has .lineLimit(1), that longer error title will now truncate/ellipsize inside the reserved slot (a regression vs. the previous content-sized layout). Consider letting the slot size to the widest needed resting pill while still reserving the 31pt hover affordance on top of it, or pick a slot width that fits the unavailable-state title.</violation>
</file>
<file name="desktop/macos/Desktop/Tests/DesktopChatDriftGuardTests.swift">
<violation number="1" location="desktop/macos/Desktop/Tests/DesktopChatDriftGuardTests.swift:75">
P2: These new/updated tests assert behavioral outcomes via source-string inspection rather than exercising the restored production behavior. Per the documented test-quality standard, source inspection is allowed only for narrow forbidden-pattern/static-wiring tripwires guarded by a reason in the `// omi-test-quality: source-inspection -- static contract:` format; these comments (e.g. "the transcript's default placement is bottom-first, while user interaction is the only path that cancels it") omit the required `static contract:` prefix and describe runtime behavior (bottom-first launch anchor, after-disappear restore, scroll-observer rebinding) that should be covered with behavioral assertions calling the production API. Each added test also adds source-inspection sites, and `check_desktop_test_quality.py` is expected to "baselines only decrease."</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskBulkOperationCoordinator.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskBulkOperationCoordinator.swift:103">
P2: Bulk-operation behavior is unreachable in the Tasks UI: no production code constructs this coordinator, while `TasksPage` continues using its separate `deleteSelectedTasks()` path. Wire `TasksPage` actions to this coordinator (including confirmation/report handling), or remove it until that integration is added.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Components/ChatScrollBehavior.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatScrollBehavior.swift:198">
P3: Scroll-view hierarchy traversal now has three identical private implementations. A shared helper would keep attachment and anchor lookup behavior consistent when SwiftUI hierarchy handling changes.</violation>
</file>
<file name="desktop/macos/e2e/flows/tasks.yaml">
<violation number="1" location="desktop/macos/e2e/flows/tasks.yaml:61">
P3: S2c's expect block only asserts `Tasks` is visible — that text is already on screen before the bulk action runs (and is asserted in S1 too), so this step cannot detect a failed delete/complete, a missing confirmation, or a stale selected count. The prose `do` claims to verify removal/result, but the machine-checked assertions don't. Add at least one outcome-specific check (e.g., the selected counter returning to `0 selected`, or the per-row result text) so the step fails when the bulk operation doesn't execute.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift:1087">
P2: Selecting an empty layer such as Archive can render a blank list rather than the empty state: `memories` retains default-scope rows even though `filteredMemories` excludes them. Filter the authoritative page by `layerAllowed(_:for:)` before assigning it, so the backing list matches the selected layer.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Pages/ConversationDetailView.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Pages/ConversationDetailView.swift:211">
P3: This new automation-driven open path sets `showTranscriptDrawer = true` without wrapping it in `OmiMotion.withGated`, so the transcript drawer pops in instantly instead of sliding, unlike every other drawer-open path in this view (the NotificationCenter handler at line 251 and the View Transcript button at 392/648/680 all use `OmiMotion.withGated`). Consider animating it the same way for a consistent macOS chat-first UX, which is the stated goal of this PR.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| guard !overflow else { throw APIError.invalidResponse } | ||
| return try await getActionItems( | ||
| limit: limit, | ||
| offset: generalOffset, |
There was a problem hiding this comment.
P1: No Deadline pagination can skip tasks because datedCount + offset assumes the general endpoint's global order has every dated row before every null-due row. The current backend only applies that sort after a bounded, unordered Firestore read; add a backend null-due cursor/filter contract before using this boundary.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/Stores/APIClient+Tasks.swift, line 105:
<comment>No Deadline pagination can skip tasks because `datedCount + offset` assumes the general endpoint's global order has every dated row before every null-due row. The current backend only applies that sort after a bounded, unordered Firestore read; add a backend null-due cursor/filter contract before using this boundary.</comment>
<file context>
@@ -55,6 +62,53 @@ extension APIClient {
+ guard !overflow else { throw APIError.invalidResponse }
+ return try await getActionItems(
+ limit: limit,
+ offset: generalOffset,
+ completed: completed,
+ expectedOwnerId: expectedOwnerId,
</file context>
| var datedOffset = 0 | ||
| while true { | ||
| guard isCurrent(lease) else { throw LocalMutationAuthorizationError.revoked } | ||
| let page = try await APIClient.shared.getDatedActionItems( |
There was a problem hiding this comment.
P1: Accounts with more than 2,000 dated incomplete tasks stop at the backend read cap, so later dated tasks never reach the surface and the subsequent No Deadline boundary is wrong. Use a backend pagination contract that can advance beyond the bounded offset window (for example a cursor), or make this route expose an explicit complete-dated query before claiming a full scan.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/Stores/TasksStore.swift, line 1226:
<comment>Accounts with more than 2,000 dated incomplete tasks stop at the backend read cap, so later dated tasks never reach the surface and the subsequent No Deadline boundary is wrong. Use a backend pagination contract that can advance beyond the bounded offset window (for example a cursor), or make this route expose an explicit complete-dated query before claiming a full scan.</comment>
<file context>
@@ -1025,6 +1079,239 @@ class TasksStore: ObservableObject {
+ var datedOffset = 0
+ while true {
+ guard isCurrent(lease) else { throw LocalMutationAuthorizationError.revoked }
+ let page = try await APIClient.shared.getDatedActionItems(
+ limit: Self.apiPageLimitCap,
+ offset: datedOffset,
</file context>
| /// invalidating a selection. Callers provide an optional complete task-ID set | ||
| /// when rows are deleted or ownership changes so stale IDs can be pruned while | ||
| /// filtered-out but still-existing selections remain selected. | ||
| struct TaskMultiSelectionState: Equatable, Sendable { |
There was a problem hiding this comment.
P2: Task multi-selection is not reachable in the macOS app: this state model is only exercised by tests and no Tasks surface creates or calls it. Wire TaskMultiSelectionState into the task-page event and bulk-action flow, or omit this unused implementation until that integration is included.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/MainWindow/Tasks/TaskMultiSelection.swift, line 29:
<comment>Task multi-selection is not reachable in the macOS app: this state model is only exercised by tests and no Tasks surface creates or calls it. Wire `TaskMultiSelectionState` into the task-page event and bulk-action flow, or omit this unused implementation until that integration is included.</comment>
<file context>
@@ -0,0 +1,245 @@
+/// invalidating a selection. Callers provide an optional complete task-ID set
+/// when rows are deleted or ownership changes so stale IDs can be pruned while
+/// filtered-out but still-existing selections remain selected.
+struct TaskMultiSelectionState: Equatable, Sendable {
+ private(set) var isActive = false
+ private(set) var selectedIDs: Set<String> = []
</file context>
| // Only fire if still following — user may have scrolled during settling | ||
| if scrollMode == .followingBottom { | ||
| scrollToBottom(proxy: proxy) | ||
| if completesInitialRestore, initialRestoreState == .pending { |
There was a problem hiding this comment.
P3: The placement is marked .completed based on scrollMode == .followingBottom without confirming the final scrollToBottom actually moved the viewport. Since scrollToBottom internally no-ops when userIsScrolling/messages.isEmpty, this can mark the one-shot placement complete without the bottom placement having executed — which the INV-CHAT-2 invariant added in this PR explicitly forbids. Consider capturing whether the scroll executed (e.g., only completing when the scroll command actually ran) so a no-op final stage doesn't permanently close the retry window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/MainWindow/Components/ChatMessagesView.swift, line 522:
<comment>The placement is marked `.completed` based on `scrollMode == .followingBottom` without confirming the final `scrollToBottom` actually moved the viewport. Since `scrollToBottom` internally no-ops when `userIsScrolling`/`messages.isEmpty`, this can mark the one-shot placement complete without the bottom placement having executed — which the INV-CHAT-2 invariant added in this PR explicitly forbids. Consider capturing whether the scroll executed (e.g., only completing when the scroll command actually ran) so a no-op final stage doesn't permanently close the retry window.</comment>
<file context>
@@ -486,12 +509,19 @@ struct ChatMessagesView<WelcomeContent: View>: View {
// Only fire if still following — user may have scrolled during settling
if scrollMode == .followingBottom {
scrollToBottom(proxy: proxy)
+ if completesInitialRestore, initialRestoreState == .pending {
+ initialRestoreState = .completed
+ }
</file context>
| } | ||
| .onChange(of: automation.transcriptDrawerOpen) { _, isOpen in | ||
| guard automation.openConversationId == conversation.id, isOpen else { return } | ||
| showTranscriptDrawer = true |
There was a problem hiding this comment.
P3: This new automation-driven open path sets showTranscriptDrawer = true without wrapping it in OmiMotion.withGated, so the transcript drawer pops in instantly instead of sliding, unlike every other drawer-open path in this view (the NotificationCenter handler at line 251 and the View Transcript button at 392/648/680 all use OmiMotion.withGated). Consider animating it the same way for a consistent macOS chat-first UX, which is the stated goal of this PR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/MainWindow/Pages/ConversationDetailView.swift, line 211:
<comment>This new automation-driven open path sets `showTranscriptDrawer = true` without wrapping it in `OmiMotion.withGated`, so the transcript drawer pops in instantly instead of sliding, unlike every other drawer-open path in this view (the NotificationCenter handler at line 251 and the View Transcript button at 392/648/680 all use `OmiMotion.withGated`). Consider animating it the same way for a consistent macOS chat-first UX, which is the stated goal of this PR.</comment>
<file context>
@@ -205,6 +206,10 @@ struct ConversationDetailView: View {
}
+ .onChange(of: automation.transcriptDrawerOpen) { _, isOpen in
+ guard automation.openConversationId == conversation.id, isOpen else { return }
+ showTranscriptDrawer = true
+ }
.task {
</file context>
Integrate TaskMultiSelectionState into Tasks UI with a Select/Done affordance, batch bulk delete through deleteMultipleTasks for correct score compaction, split TaskDetailPanel into policy/navigator/view, extract knowledge graph pagination from APIClient+Settings, consolidate TasksStore incomplete offsets, and align AgentClient admission gate coverage for Chat-first receipt paths.
Failure-Class: FC-single-input-device-release
`userIsScrolling` had no single owner. Three writers disagreed about what it means, and the resume path consulted the weakest of them: 1. Returning to the live edge could never resume following. Commit 7b70061 replaced the 360ms settlement guess with AppKit's `didEndLiveScroll`, which delivers on the very next main-thread turn. `userIsScrolling` is cleared by a 0.3s `asyncAfter`, so it was still true whenever the settle callback ran and `canResumeFollowing` vetoed every resume. The 360ms guess had happened to outlast the latch, so replacing it silently removed the only path back into live following. The end-of-input signal is the authority for "the gesture finished", so it now releases the latch instead of being vetoed by it. 2. `isSending` flipping under a surface with no local send token cleared the latch and seized the viewport, so a send the reader did not make (poll, sync, another surface) teleported a mid-gesture reader to the bottom. 3. The local-send path did the same unconditionally. Both send paths now leave a reader whose gesture is in flight where they are and raise the existing jump-to-latest affordance instead. `ChatScrollContainer` gets the same latch fix so the shared container does not drift from the main transcript. Guard surface (this is the recurring-class artifact FC-split-mutation-authority asks for): `ChatTranscriptGestureHarnessTests` mounts the real `ChatMessagesView` in an `NSHostingView`, finds the `NSScrollView` SwiftUI actually built, and drives it with real `NSEvent` scroll wheels through `NSApplication.sendEvent` — the same call that feeds `UserScrollDetector`'s local monitor, carrying the window identity and in-bounds location the production guards check. The existing suites drive `NSClipView` bounds and post notifications by hand and never instantiate the view, which is why 31 of them passed while both defects shipped. Verification: - `xcrun swift test --package-path Desktop --filter 'UserScrollDetectorTests|ChatScrollLiveEdgeTests|ScrollPositionDetectorTests|DesktopChatDriftGuardTests|ChatPromptTimelineTests|ChatTranscriptGestureHarnessTests'` -> 38 tests, 0 failures. - Reverting only the source fixes and re-running the harness fails `testDeliberateReturnToTheLiveEdgeResumesFollowing` (4951.0 of 4965.0, never re-followed) and `testSendStartingWhileTheReaderScrollsDoesNotSeizeTheViewport` (3751.0 -> 4951.0). Both pass with the fixes. - Live QA bundle `omi-macos-ux-qa-20260802` (dev backend), real Home chat with a real `ChatProvider`: reader scrolled away mid-stream held 121785.0 exactly while the document grew 123375.0 -> 124377.0; a deliberate return to the live edge then tracked the next streamed turn (122992.0 -> 123083.0 as the document grew 123582.0 -> 123673.0, `is_at_bottom=true` throughout). - `python3 scripts/check_desktop_test_quality.py` -> at/below baseline. Not verified: a physical trackpad gesture. Posting synthetic HID events needs Accessibility, which this session's responsible process does not hold (`AXIsProcessTrusted=false`); every gesture above was injected in-process instead. The harness documents the one boundary that leaves synthetic: a test process is never the active app, so SwiftUI's `ScrollView` will not apply synthetic scroll deltas and the harness moves the clip view itself in lockstep with the events. INV-CHAT-2 Failure-Class: FC-split-mutation-authority Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`AgentPillLifecycleTests` is a guard test for INV-CHAT-2, and two of its source-string tripwires were asserting shapes that no longer exist: - `scheduleSettledBottomChecks` has never existed in `ChatScrollBehavior`; 7b70061 renamed the real function to `scheduleSettledBottomFollow` but added the tripwire under the old name, so `testStreamingResponseGrowthIsSteppedAndNonAnimated` has been red on this branch ever since. - `testSharedChatMessagesOpenAndSendFollowLatest` pinned the exact three-line body of `handleLocalSend` including `userIsScrolling = false`, i.e. it pinned the defect the previous commit removes. Replaced with the contract it was reaching for: a local send may enter follow mode, and must not clear the reader's in-flight gesture latch to do it. Verification: `xcrun swift test --package-path Desktop --filter 'AgentPillLifecycleTests'` -> 79 tests, 0 failures (was 2 failures). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Narrow AgentContextAdmissionGate to refresh/admit only (not full query streams), fix Memories layer-filter reload to use tier cache off default access, correct Tasks detail/chat/escape behavior, align No Deadline API boundary with retired dated rows, add canonical graph revision retry plus 503 route test, retry task deep-link scroll until proxy mounts, cancel stale initial-scroll timers on conversation switch, and tighten tasks e2e expectations.
Review fixes added bounded UX and pagination-boundary logic without splitting the oversized page/store owners.
`UserScrollDetector`'s local event monitor matched `.leftMouseDown` and
`.leftMouseDragged` and treated either as a scroll gesture as long as the
location was inside the scroll view's bounds. It never checked whether the
viewport had actually moved.
That routed a plain click into `cancelPendingScrollsForUserInteraction()`,
which moves `initialRestoreState` to the terminal `.userInterrupted` and cancels
the pending `scrollTo("bottom-anchor")` work items. `canStart` is `.waiting`
only, so nothing could ever retry the launch placement for that view's lifetime
and the transcript stayed wherever SwiftUI left it — the top of the history.
The window is wide: the transcript mounts while the journal is still loading, so
any click anywhere in that region before the snapshot arrives permanently kills
the placement that had not yet had content to place. That is the reported
"chat opens scrolled to the top".
A press now only records where the viewport was. A drag takes ownership once it
has genuinely moved the viewport, which keeps scrollbar drags and
selection-autoscroll working; a click that moves nothing changes no state.
Verification:
- New `testAClickBeforeTheHistoryLoadsDoesNotStrandTheTranscriptAtTheTop`
mounts the real `ChatMessagesView`, sends a real `.leftMouseDown` NSEvent
through `NSApplication.sendEvent` while `isLoadingInitial` is true, then
delivers the history. Before this fix it lands at `scrollTop=0.0 of 6610.0`;
after, at the live edge.
- `testAMouseDragThatMovesTheViewportStillTakesOwnership` pins the other side:
a drag that moves the viewport still takes it away from live-follow.
- A separate probe confirmed the mechanism directly: a synthetic `.leftMouseDown`
into a real `NSScrollView` with the production coordinator installed fires
`onUserScroll` exactly once.
- `xcrun swift test --package-path Desktop --filter
'UserScrollDetectorTests|ChatScrollLiveEdgeTests|ScrollPositionDetectorTests|DesktopChatDriftGuardTests|ChatPromptTimelineTests|ChatTranscriptGestureHarnessTests|AgentPillLifecycleTests'`
-> 120 tests, 0 failures.
INV-CHAT-2
Failure-Class: FC-split-mutation-authority
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to cb3f41c, which made a press stop claiming the viewport but still keyed the transfer on receiving a `.leftMouseDragged`. A second-model review pointed out two gaps that leaves, and both are real: - A scrollbar *track click* repositions the viewport during the press and may never produce a drag event at all. Ownership never transferred, so the next streamed token pulled the reader straight back. - The per-event `bounds.contains(location)` guard dropped drag events the moment the pointer left the transcript — which is exactly what a scrollbar drag and a selection autoscroll do. Ownership was lost precisely when the reader was moving fastest. Ownership is now defined by the thing it actually means: a press opens a candidate interaction, and the candidate is promoted exactly once when this clip view's normalized offset genuinely moves. That is observed from the clip view's own bounds notifications, so it does not matter which input produced the motion. `.leftMouseUp` closes the candidate and runs the existing settle check. The offset comes from `ChatScrollLiveEdge.topBasedScrollOffset`, so flipped and non-flipped document views agree. Verification: - New `testAScrollbarTrackClickThatMovesTheViewportTakesOwnership` moves the viewport during a press with no drag event, then streams. With the clip-bounds observer neutralised it fails (reader at 3443.0 is yanked to 4957.0); with it, the position holds. - `testAClickBeforeTheHistoryLoadsDoesNotStrandTheTranscriptAtTheTop` and `testAMouseDragThatMovesTheViewportStillTakesOwnership` still pass, so the click fix and drag ownership are both intact. - Full desktop suite: `xcrun swift test --package-path Desktop` -> 3809 tests, 1 skipped (pre-existing), 0 failures. Known gap, deliberately not in this change: keyboard navigation still claims ownership on key-down without confirming displacement, so an arrow key that only moves a caret inside selectable text counts as a scroll. Page Up/Down/Home/End are genuine viewport commands, so the fix there is narrower than it looks and is worth its own change. INV-CHAT-2 Failure-Class: FC-split-mutation-authority Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claim reader ownership once per wheel gesture (not per delta), add unphased wheel settle fallback, and extend scroll ownership tests. Format settings floating-bar/chat section for pinned swift-format.
Nested horizontal Markdown code scrollers consumed vertical trackpad gestures before the transcript could continue. Route vertical intent to the enclosing chat scroller while preserving horizontal code navigation, and make the transcript claim reader ownership before forwarding. INV-CHAT-2 Failure-Class: FC-single-input-device-release
Failure-Class: new
Git-on-my-level
added a commit
that referenced
this pull request
Aug 3, 2026
## Summary - add a reusable macOS SwiftUI/AppKit runtime-debugging playbook distilled from the investigation that landed in #11035 - map failures from product authority through state and identity, delivery, native-object lifetime, layout, and rendered outcome - teach first-impossible-transition tracing, privacy-safe evidence collection, and common cross-layer failure patterns - document mounted native-boundary regressions, adversarial fixtures, behavioral guards, and named-bundle QA - retain chat scrolling as a worked example and route agents to the guide with one line in the macOS app's `AGENTS.md` ## Why this is repository documentation The method is reusable, but its operational details depend on Omi's native logger, test harnesses, named-bundle workflow, and SwiftUI/AppKit ownership boundaries. Keeping the playbook beside the app makes code paths and examples reviewable with the implementation and avoids a separate debugging skill drifting out of date. ## Product invariants affected none ## Verification - `git diff --check` - local PR preflight for the exact PR body - agent-document reference and desktop changelog checks run through the pre-push gate
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root causes addressed
Product invariants affected
Failure-Class: FC-single-input-device-release
Verification
QA / deployment note
The installed app remains compatible with the currently deployed backend and falls back only when the new canonical route is unavailable. Full multi-page Brain Map QA requires this PR's backend and Firestore index to deploy. If historical canonical memory-item/assertion materialization is incomplete, that is a separate backfill issue; this change removes request-side truncation but does not synthesize missing canonical graph data.
The broad
desktop/macos/test.shlauncher run reachestests/test-omi-dev.sh, whose temporary worktree under/private/var/...is rejected by the managed Git wrapper because linked worktrees must live on Ephemeral. Earlier launcher checks passed; product Swift coverage above is complete.