One chat shell: every content block renders everywhere, plus chat ergonomics (#12598) - #12607
Open
Git-on-my-level wants to merge 17 commits into
Open
One chat shell: every content block renders everywhere, plus chat ergonomics (#12598)#12607Git-on-my-level wants to merge 17 commits into
Git-on-my-level wants to merge 17 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
40 issues found across 211 files
Confidence score: 2/5
DashboardViewModel.swiftcan publish the previous account’s goals after an account switch, and failed progress updates can leave rejected optimistic values persisted; snapshot ownership and roll back both published and stored state on failure.memory_link_block.dart,message.dart, andai_message.dartexpose several chat-content regressions: memory links can remain disabled, rich blocks without IDs can disappear, and mixed fallback content can hide plain text; initialize providers, assign stable IDs, and preserve non-interactive blocks.TaskNavigationRequestStore.swiftcurrently has no reachable callers, so chat-originated task and candidate targets cannot reachTasksPage; wire the producers to the request APIs.QueryShellHome.swiftandChatFirstShell.swiftcan leave users on the wrong route or an indefinite “Loading goals” state when asynchronous resolution fails or completes late; validate navigation before changing routes and reload goals after capability activation.
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="desktop/macos/Desktop/Sources/MainWindow/Dashboard/DashboardViewModel.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Dashboard/DashboardViewModel.swift:85">
P1: When an account switches while a dashboard load is in flight, this code can sync and publish the previous account's goals after `resetSessionState()` clears the view model. Capture an owner authorization snapshot or generation for every dashboard/local load and guard it before storage sync and every UI publication.</violation>
<violation number="2" location="desktop/macos/Desktop/Sources/MainWindow/Dashboard/DashboardViewModel.swift:149">
P2: When the progress PATCH fails, the optimistic value remains visible and persisted even though the server rejected it. Save the prior goal value and roll back both the published goal and `GoalStorage` when the remote update fails.</violation>
<violation number="3" location="desktop/macos/Desktop/Sources/MainWindow/Dashboard/DashboardViewModel.swift:203">
P2: When the backend delete fails after the local soft-delete, the goal stays hidden until a later refresh makes it reappear. Restore the local record on delete failure, or queue the deletion durably so local visibility cannot claim a deletion that the server rejected.</violation>
</file>
<file name="app/lib/pages/chat/widgets/content_blocks/memory_link_block.dart">
<violation number="1" location="app/lib/pages/chat/widgets/content_blocks/memory_link_block.dart:51">
P1: When a user encounters a memory link before visiting the Memories page, `MemoriesProvider` is never initialized, so this button remains disabled with a spinner and the memory cannot be opened. Initialize the provider for the chat lifecycle, or add a single-flight `ensureLoaded` path before resolving the memory.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Dashboard/TaskNavigationRequestStore.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Dashboard/TaskNavigationRequestStore.swift:33">
P1: No caller invokes either `request` overload, so the exact task/candidate handoff is unreachable. Since `pendingTarget` is `private(set)`, `TasksPage` can never receive a queued target; wire the chat producers to these APIs before relying on this navigation path.</violation>
</file>
<file name="app/lib/backend/schema/message.dart">
<violation number="1" location="app/lib/backend/schema/message.dart:445">
P1: When the API returns a rich block without an `id`, `typedContentBlocks` drops it before rendering, so legacy conversation/task/etc. cards appear only as plain fallback text. Assign a stable message-scoped ID for missing block IDs before decoding, or otherwise allow the decoder to render these valid first-class blocks.</violation>
</file>
<file name="app/lib/pages/chat/widgets/ai_message.dart">
<violation number="1" location="app/lib/pages/chat/widgets/ai_message.dart:298">
P1: When a fallback contains both interactive and non-interactive blocks, this condition hides the non-interactive block text because `ChatContentBlockList` renders only interactive blocks. Replace the body only when every raw block is valid and represented by one of the six interactive widgets; otherwise append the widgets to the fallback text.</violation>
</file>
<file name="desktop/macos/scripts/check-single-chat-shell.py">
<violation number="1" location="desktop/macos/scripts/check-single-chat-shell.py:88">
P1: When an auxiliary surface uses a property name other than `chatFirstRichBlockContext`, this tripwire misses the optional context and its `if let` rendering fork. It currently passes the production `floatingSurface` implementation, allowing rich blocks to disappear; match the type annotation itself and add a self-test for this form.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/QueryShell/QueryShellHome.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/QueryShell/QueryShellHome.swift:476">
P2: When a referenced memory is not in the local cache, this navigates away before confirming it can be opened. `ChatFirstShell.resolvePendingMemoryFocus` returns on failure without reverting the route, leaving the user on an empty or stale Memories pane; preflight both memory-opening paths before calling `navigation.open`.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/ChatFirst/ChatFirstShell.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/ChatFirst/ChatFirstShell.swift:124">
P2: When a user opens Goals before the asynchronous capability resolves, the page can remain on “Loading goals” indefinitely. Trigger a goals reload after capability activation, or keep the initial load pending until the capability resolves.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/FloatingControlBar/FloatingControlBarView.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/FloatingControlBar/FloatingControlBarView.swift:2179">
P2: When an agent transcript contains a question card, this context gates it against the main-chat transcript instead of the agent message being displayed. Pass a surface-specific question state/handler, or keep the agent card explicitly passive, so the floating card does not present a misleading non-functional control.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/DesktopHomeView.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/DesktopHomeView.swift:328">
P2: If this task runs during a transient missing-owner or account transition, the sample becomes permanently resolved with capability off and the next account never gets a capability fetch. Reset the sample's resolved state or explicitly retry after the new owner is available.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubbleSupport.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubbleSupport.swift:350">
P2: When an assistant message contains only a rich block, this branch hides the metadata band and removes the row's only timestamp. The ChatFirst cards do not display their own time, so task, question, and memory history loses temporal context; keep a timestamp-only band for these rows or render the timestamp inside the rich block card.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Components/ChatSelectableTextPopover.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatSelectableTextPopover.swift:91">
P2: When the popover content is mounted before its window is attached, this one-shot focus attempt does nothing and the advertised ⌘A/⌘C shortcuts are unavailable until a click. Claim first responder from `viewDidMoveToWindow` or an attached-view update path instead of silently dropping the request.</violation>
<violation number="2" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatSelectableTextPopover.swift:100">
P2: When `Select Text…` is opened during an in-progress answer, each streamed token replaces `NSTextView.string` and interrupts native selection. Snapshot the text when opening the popover, or preserve the selection and scroll position while applying streaming updates.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift:90">
P2: The new context affects rich-card rendering, but the bubble identity ignores it and the rich-block view does not observe its `ChatProvider`. Include the relevant context/capability identity in the bubble refresh path or observe the provider so capability changes update existing cards.</violation>
<violation number="2" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift:286">
P2: While an assistant is streaming after a tool call but before an answer arrives, this fallback exposes pre-tool commentary as copyable text. Use `message.copyableText` directly so copy and selection preserve the answer-only projection.</violation>
<violation number="3" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift:723">
P3: The copy shortcut activates when any metadata control is focused because they share one Boolean focus state. Track the focused metadata control with distinct focus values and enable `⌘C` only for the copy control.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/Components/ChatMessagesView.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatMessagesView.swift:36">
P2: When a settled row is later rewritten by journal replay or its failure status changes, the new adjacent duplicate pass is not rerun, so the transcript can retain an obsolete “Duplicate message” chip or miss a newly eligible collapse. Include the relevant row text/status fingerprint in the invalidation key, or refresh when those values change.</violation>
<violation number="2" location="desktop/macos/Desktop/Sources/MainWindow/Components/ChatMessagesView.swift:60">
P2: Consecutive user turns now collapse as duplicate answers. Restrict the adjacent-collapse pass to assistant rows so repeating a long user question does not hide the later turn behind the duplicate chip.</violation>
</file>
<file name="desktop/macos/e2e/flows/home.yaml">
<violation number="1" location="desktop/macos/e2e/flows/home.yaml:15">
P2: `home.yaml` lists `DesktopAutomationHomeStageActions.swift` in `covers:` claiming it exercises the four `home_*` bridge actions, but no step in this flow invokes any of them, and the flow's preconditions only include `auth_ready` (no `automation_bridge_ready`). The S1–S6 steps are direct UI interactions (typing in the composer, clicking buttons, hovering markers), so the bridge actions' behavior is never driven or verified here. Per the e2e `covers:` contract, either drop this entry or add a step that drives a `home_*` action through the bridge (and add the `automation_bridge_ready` precondition).</violation>
</file>
<file name="app/lib/pages/chat/widgets/content_blocks/task_card_block.dart">
<violation number="1" location="app/lib/pages/chat/widgets/content_blocks/task_card_block.dart:94">
P2: When `item.isLocked` is true, this control remains enabled and `_toggle` optimistically changes the task before the API rejects the update. Disable the toggle for locked items so chat does not show a flicker and failed mutation on paywalled tasks.</violation>
</file>
<file name="app/lib/backend/schema/chat_content_block.dart">
<violation number="1" location="app/lib/backend/schema/chat_content_block.dart:114">
P2: When a citation carries `momentTimestampMs`, `createdAt`, `appName`, or `url`, this decoder silently drops that metadata from `typedContentBlocks`. Preserve the canonical citation fields, including their snake_case aliases, in `CitationContentBlock`.</violation>
<violation number="2" location="app/lib/backend/schema/chat_content_block.dart:137">
P2: When an agent completion includes `promptSnippet`, this projection drops it even though the canonical and Swift codecs preserve it. Add `promptSnippet` to `AgentCompletionContentBlock` and decode the wire field.</violation>
<violation number="3" location="app/lib/backend/schema/chat_content_block.dart:181">
P2: When a cold-start question reaches mobile, `_decodeQuestionCard` discards its canonical `coldStartSequence`, so consumers of the typed block cannot identify the script or step. Decode and expose this field, including the snake_case representation.</violation>
</file>
<file name="app/lib/pages/chat/widgets/content_blocks/conversation_link_blocks.dart">
<violation number="1" location="app/lib/pages/chat/widgets/content_blocks/conversation_link_blocks.dart:63">
P2: When a failed lookup is followed by a different block at the same list position, Flutter reuses this state and `_isUnavailable` remains true, so the new block incorrectly renders unavailable. Reset state in `didUpdateWidget` when the block identity changes, or key the stateful block widgets themselves.</violation>
</file>
<file name="desktop/macos/e2e/flows/tasks.yaml">
<violation number="1" location="desktop/macos/e2e/flows/tasks.yaml:9">
P2: The new covers entry claims the tasks flow exercises TaskNavigationRequestStore's exact-record handoff, but no step in tasks.yaml produces a navigation request: S1 navigates via the nav-bar tab button and S2a clicks a task row already inside the page. The store's request(task:)/request(candidate:) also have no callers in the codebase, so consumeIfAvailable always returns nil. Per the repo rule, list a source in covers only when the flow actually exercises and verifies its behavior — remove this entry (or add a step that hands the Tasks page an exact record and verifies it opens on arrival).</violation>
</file>
<file name="desktop/macos/e2e/flows/floating-bar-functional.yaml">
<violation number="1" location="desktop/macos/e2e/flows/floating-bar-functional.yaml:14">
P2: This Ask-Omi-only flow never drives the notch's agent-exit routing, so listing DesktopShellPresentationPolicy.swift in covers claims coverage the flow does not exercise. S2-S7 only open Ask Omi, send a stubbed query, and close it via closeAskOmiForAutomation -> closeAIConversation; the policy's sole call site is showMainConversationFromAgent (FloatingControlBarWindow.swift:1621) -> routePrimaryTextInputToMainAppAfterAgentExit, which no step triggers. List it only in a flow that backs out of an agent chat in the notch, or drop the entry.</violation>
</file>
<file name="app/lib/pages/chat/widgets/content_blocks/goal_link_block.dart">
<violation number="1" location="app/lib/pages/chat/widgets/content_blocks/goal_link_block.dart:96">
P2: When a goal uses more than one decimal place, `_format` rounds both values before displaying them, so the sheet reports inaccurate progress. Preserve the value's precision instead of forcing every non-integer to one decimal place.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/ViewExporter.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/ViewExporter.swift:55">
P2: After removing the dashboard and daily-score standalone views and the full-dashboard page, `standaloneViewCount` (18) and `fullPageCount` (11) no longer match the registry arrays (13 and 8). `runBatch` spawns a subprocess for every index up to count, and `standaloneViewAt`/`fullPageViewAt` return nil past the end, so the subprocess exits(1) and `--export-views`/`--export-fullpages` log spurious FAILED exports for nonexistent views. Update the two counts (or derive them from the arrays) when removing registry entries.</violation>
<violation number="2" location="desktop/macos/Desktop/Sources/ViewExporter.swift:55">
P3: `previewChatProvider()` is now dead code: its only caller was the removed "02-dashboard" registry entry. Remove the unused private helper to avoid a stale build-time dependency on ChatProvider.</violation>
</file>
<file name="desktop/macos/e2e/flows/chat-first-cohesive.yaml">
<violation number="1" location="desktop/macos/e2e/flows/chat-first-cohesive.yaml:47">
P2: ChatSelectableTextPopover.swift is listed in `covers:`, but no step in this flow exercises it. The popover is only instantiated when `showSelectableText` becomes true (the "Select Text…" context-menu or hover-strip path); the `.popover(isPresented:)` anchor here is a `Color.clear` view that is always present, so it never renders `ChatSelectableTextPopover`/`OmiSelectableTextView`. Steps S1–S23 contain no interaction that presents the popover or triggers copy/select-text behavior. The added comment even concedes the file's behavior is asserted hermetically in `ChatSelectableTextSurfaceTests`, "not here". Per the repo convention that `covers:` lists only sources the flow actually exercises and verifies, this entry should be dropped (or a step added that opens and dismisses the popover) so the manifest does not claim coverage it does not provide.</violation>
</file>
<file name="desktop/macos/e2e/flows/home-spine.yaml">
<violation number="1" location="desktop/macos/e2e/flows/home-spine.yaml:24">
P2: home-spine.yaml lists DashboardViewModel.swift in `covers:`, but the flow never exercises it. The comment claims Home's corner counts and goal rows read it; in fact the corner count comes from QueryShellHome's `total` (appState conversations + tasksStore.tasks + memoriesViewModel.memories + screenCount), DashboardViewModel is only used by StartupWarmupCoordinator/ViewModelContainer, and no S1-S10 step touches goal rows or scores. Per the repo's covers contract, only list sources the flow actually exercises and verifies. Drop the entry (and the comment) or add a step that drives a goal row and asserts the dashboard-derived count.</violation>
</file>
<file name="desktop/macos/e2e/CORE_E2E.md">
<violation number="1" location="desktop/macos/e2e/CORE_E2E.md:72">
P2: This PR makes ChatFirstShell the universal shell for every account, yet deleting the 'Home stage (hub/chat/connect), chat-first shell | T2' row from the change→tier map drops the only documented minimum qualification tier for that surface. The replacement flows home.yaml and home-spine.yaml are only `tier: manual` and were not added to the flow audit baseline, so the now-universal chat shell has no traceable coverage requirement. Replace the removed row with a reference to the new home/chat-first flows (and list them in the flow audit) instead of deleting it outright.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/MainWindow/ChatFirst/Blocks/ChatFirstContentBlockViews.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/MainWindow/ChatFirst/Blocks/ChatFirstContentBlockViews.swift:32">
P2: The `.disabled` branch in `ChatFirstQuestionCardOptionsPolicy.presentation` is reached whenever `isActionable` is false and capability is off, but `isActionable` is false both for a live question with capability off and for a question that has been retired (no longer the transcript tail). The code comment says capability-off is "the only reason to show unpressable options: the question is live," yet the view passes only `isActionable`, which cannot tell the two apart. As a result a superseded question card renders dimmed options plus the "Answering is unavailable right now" label even though the real reason is that the question is no longer active, contradicting the intent and the `hidden` policy used for retired questions when capability is on.</violation>
</file>
<file name="app/lib/pages/chat/widgets/content_blocks/chat_block_chrome.dart">
<violation number="1" location="app/lib/pages/chat/widgets/content_blocks/chat_block_chrome.dart:133">
P3: While task data is hydrating, `ChatBlockLoading` ignores its `icon` and `label`, so the card shows only a spinner and loading message without identifying the block as a task. Render `ChatBlockEyebrow` (or otherwise include the icon and label) in the loading state.</violation>
</file>
<file name="desktop/macos/e2e/feature-vector.md">
<violation number="1" location="desktop/macos/e2e/feature-vector.md:40">
P3: The new coverage claim says `home.yaml (nav + conversation_list_snapshot)`, but `home.yaml` never exercises `conversation_list_snapshot` and contains no snapshot/bridge action, and it is `tier: manual` rather than a T2 deep flow. The row now misattributes coverage that the flow does not provide, which misleads the coverage tracker. Drop the `conversation_list_snapshot` parenthetical (or add the snapshot step to `home.yaml`) so the status reflects what the flow actually verifies.</violation>
</file>
<file name="desktop/macos/e2e/flows/chat-first-capability-isolation.yaml">
<violation number="1" location="desktop/macos/e2e/flows/chat-first-capability-isolation.yaml:4">
P3: The updated description and the S2 comment claim this flow proves content blocks "still render", task check-off works, and only the question card's options dim on a capability-off bundle, but the steps only assert `state.shellVariant: chat_first` and `state.visibleChatFirstRoute: chat`. No step actually renders or samples any block, so running this manual flow yields no evidence for those claims and the text overstates what it verifies. Align the description/comment with the shell-mount-and-route assertion the steps actually make, or add steps that exercise a block.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/Chat/ChatContinuityInvariants.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/Chat/ChatContinuityInvariants.swift:59">
P3: The `isJournaled` documentation reverses the property's behavior: `true` permits journal persistence, while only trial and onboarding return `false`. Describe the property as whether this kind should be journaled, and call out trial/onboarding as presentation-only exceptions.</violation>
</file>
<file name="desktop/macos/Desktop/Tests/HomeStageCloseSemanticsTests.swift">
<violation number="1" location="desktop/macos/Desktop/Tests/HomeStageCloseSemanticsTests.swift:102">
P3: Removing the three DashboardPage static-contract tests leaves the class doc comment factually wrong and an empty MARK section behind. The comment still claims HomeAskFocusPolicyTests unit-tests the deferred focus policy and that 'the DashboardPage assertions below' are static-contract tripwires, but both that test file and DashboardPage.swift were removed. Delete the now-empty '// MARK: Flow (static contract over DashboardPage wiring)' section and trim the stale references in the header comment.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Re-trigger cubic
| // 1. Show local data first (already loaded in init) | ||
| // 2. Fetch from API | ||
| do { | ||
| let apiGoals = try await APIClient.shared.getGoals() |
There was a problem hiding this comment.
P1: When an account switches while a dashboard load is in flight, this code can sync and publish the previous account's goals after resetSessionState() clears the view model. Capture an owner authorization snapshot or generation for every dashboard/local load and guard it before storage sync and every UI publication.
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/Dashboard/DashboardViewModel.swift, line 85:
<comment>When an account switches while a dashboard load is in flight, this code can sync and publish the previous account's goals after `resetSessionState()` clears the view model. Capture an owner authorization snapshot or generation for every dashboard/local load and guard it before storage sync and every UI publication.</comment>
<file context>
@@ -0,0 +1,208 @@
+ // 1. Show local data first (already loaded in init)
+ // 2. Fetch from API
+ do {
+ let apiGoals = try await APIClient.shared.getGoals()
+ // 3. Sync to SQLite
+ try await GoalStorage.shared.syncServerGoals(apiGoals)
</file context>
| summary: block.summary, | ||
| actionTitle: l10n.chatBlockOpenInMemories, | ||
| actionKey: Key('chat-block-memoryLink-${block.id}-open'), | ||
| isOpening: memory == null, |
There was a problem hiding this comment.
P1: When a user encounters a memory link before visiting the Memories page, MemoriesProvider is never initialized, so this button remains disabled with a spinner and the memory cannot be opened. Initialize the provider for the chat lifecycle, or add a single-flight ensureLoaded path before resolving the memory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/lib/pages/chat/widgets/content_blocks/memory_link_block.dart, line 51:
<comment>When a user encounters a memory link before visiting the Memories page, `MemoriesProvider` is never initialized, so this button remains disabled with a spinner and the memory cannot be opened. Initialize the provider for the chat lifecycle, or add a single-flight `ensureLoaded` path before resolving the memory.</comment>
<file context>
@@ -0,0 +1,57 @@
+ summary: block.summary,
+ actionTitle: l10n.chatBlockOpenInMemories,
+ actionKey: Key('chat-block-memoryLink-${block.id}-open'),
+ isOpening: memory == null,
+ onAction: memory == null ? null : () => showMemoryDialog(context, provider, memory: memory),
+ );
</file context>
| } | ||
| } | ||
|
|
||
| func request(task: TaskActionItem) { |
There was a problem hiding this comment.
P1: No caller invokes either request overload, so the exact task/candidate handoff is unreachable. Since pendingTarget is private(set), TasksPage can never receive a queued target; wire the chat producers to these APIs before relying on this navigation path.
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/Dashboard/TaskNavigationRequestStore.swift, line 33:
<comment>No caller invokes either `request` overload, so the exact task/candidate handoff is unreachable. Since `pendingTarget` is `private(set)`, `TasksPage` can never receive a queued target; wire the chat producers to these APIs before relying on this navigation path.</comment>
<file context>
@@ -0,0 +1,66 @@
+ }
+ }
+
+ func request(task: TaskActionItem) {
+ pendingTarget = .task(task.id)
+ pendingTask = task
</file context>
| /// | ||
| /// The raw list stays authoritative on the wire (see [toJson]); this is the | ||
| /// renderable view used by the chat content-block widgets. | ||
| List<ChatContentBlock> get typedContentBlocks => _typedContentBlocks ??= ChatContentBlock.decodeList(contentBlocks); |
There was a problem hiding this comment.
P1: When the API returns a rich block without an id, typedContentBlocks drops it before rendering, so legacy conversation/task/etc. cards appear only as plain fallback text. Assign a stable message-scoped ID for missing block IDs before decoding, or otherwise allow the decoder to render these valid first-class blocks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/lib/backend/schema/message.dart, line 445:
<comment>When the API returns a rich block without an `id`, `typedContentBlocks` drops it before rendering, so legacy conversation/task/etc. cards appear only as plain fallback text. Assign a stable message-scoped ID for missing block IDs before decoding, or otherwise allow the decoder to render these valid first-class blocks.</comment>
<file context>
@@ -435,38 +436,25 @@ class ServerMessage {
+ ///
+ /// The raw list stays authoritative on the wire (see [toJson]); this is the
+ /// renderable view used by the chat content-block widgets.
+ List<ChatContentBlock> get typedContentBlocks => _typedContentBlocks ??= ChatContentBlock.decodeList(contentBlocks);
+
+ /// True when [text] carries nothing beyond the fallback text synthesized from
</file context>
Suggested change
| List<ChatContentBlock> get typedContentBlocks => _typedContentBlocks ??= ChatContentBlock.decodeList(contentBlocks); | |
| List<ChatContentBlock> get typedContentBlocks => _typedContentBlocks ??= ChatContentBlock.decodeList( | |
| contentBlocks.asMap().entries.map((entry) { | |
| final block = Map<String, dynamic>.from(entry.value); | |
| if ((block['id'] as String?)?.trim().isNotEmpty != true) { | |
| block['id'] = '$id:content-block:${entry.key}'; | |
| } | |
| return block; | |
| }).toList(growable: false), | |
| ); |
Comment on lines
+298
to
+302
| final blocksReplaceBody = contentBlocks != null && | ||
| message.memories.isEmpty && | ||
| message.type != MessageType.daySummary && | ||
| !displayOptions && | ||
| message.textIsStructuredFallback; |
There was a problem hiding this comment.
P1: When a fallback contains both interactive and non-interactive blocks, this condition hides the non-interactive block text because ChatContentBlockList renders only interactive blocks. Replace the body only when every raw block is valid and represented by one of the six interactive widgets; otherwise append the widgets to the fallback text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/lib/pages/chat/widgets/ai_message.dart, line 298:
<comment>When a fallback contains both interactive and non-interactive blocks, this condition hides the non-interactive block text because `ChatContentBlockList` renders only interactive blocks. Replace the body only when every raw block is valid and represented by one of the six interactive widgets; otherwise append the widgets to the fallback text.</comment>
<file context>
@@ -284,8 +285,26 @@ Widget buildMessageWidget(
+ // A message whose text is only the fallback synthesized from its blocks has
+ // nothing to say that the components do not already show, so the components
+ // replace the body instead of repeating it.
+ final blocksReplaceBody = contentBlocks != null &&
+ message.memories.isEmpty &&
+ message.type != MessageType.daySummary &&
</file context>
Suggested change
| final blocksReplaceBody = contentBlocks != null && | |
| message.memories.isEmpty && | |
| message.type != MessageType.daySummary && | |
| !displayOptions && | |
| message.textIsStructuredFallback; | |
| final blocksReplaceBody = contentBlocks != null && | |
| message.memories.isEmpty && | |
| message.type != MessageType.daySummary && | |
| !displayOptions && | |
| message.textIsStructuredFallback && | |
| message.typedContentBlocks.length == message.contentBlocks.length && | |
| message.typedContentBlocks.every( | |
| (block) => const { | |
| 'taskCard', | |
| 'goalLink', | |
| 'captureLink', | |
| 'conversationLink', | |
| 'memoryLink', | |
| 'questionCard', | |
| }.contains(block.type), | |
| ); |
| name: chat-first-capability-isolation | ||
| tier: manual | ||
| description: "Manual local/offline capability-off assertion. Run it once for each isolated fixture case in the two-launch matrix below." | ||
| description: "Manual local/offline capability-off assertion: the one shell still mounts and its content blocks still render, non-actionable. Run it once for each isolated fixture case in the two-launch matrix below." |
There was a problem hiding this comment.
P3: The updated description and the S2 comment claim this flow proves content blocks "still render", task check-off works, and only the question card's options dim on a capability-off bundle, but the steps only assert state.shellVariant: chat_first and state.visibleChatFirstRoute: chat. No step actually renders or samples any block, so running this manual flow yields no evidence for those claims and the text overstates what it verifies. Align the description/comment with the shell-mount-and-route assertion the steps actually make, or add steps that exercise a block.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/e2e/flows/chat-first-capability-isolation.yaml, line 4:
<comment>The updated description and the S2 comment claim this flow proves content blocks "still render", task check-off works, and only the question card's options dim on a capability-off bundle, but the steps only assert `state.shellVariant: chat_first` and `state.visibleChatFirstRoute: chat`. No step actually renders or samples any block, so running this manual flow yields no evidence for those claims and the text overstates what it verifies. Align the description/comment with the shell-mount-and-route assertion the steps actually make, or add steps that exercise a block.</comment>
<file context>
@@ -1,7 +1,7 @@
name: chat-first-capability-isolation
tier: manual
-description: "Manual local/offline capability-off assertion. Run it once for each isolated fixture case in the two-launch matrix below."
+description: "Manual local/offline capability-off assertion: the one shell still mounts and its content blocks still render, non-actionable. Run it once for each isolated fixture case in the two-launch matrix below."
app: non-prod
# A harness invocation owns one automation port, hence one named app. It cannot
</file context>
| .focused($isMetadataControlFocused) | ||
| // Only while this row's control strip holds keyboard focus. A window-wide | ||
| // ⌘C would take the shortcut away from selected prose and the composer. | ||
| .modifier(ChatCopyKeyboardShortcut(isActive: isMetadataControlFocused)) |
There was a problem hiding this comment.
P3: The copy shortcut activates when any metadata control is focused because they share one Boolean focus state. Track the focused metadata control with distinct focus values and enable ⌘C only for the copy control.
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/ChatBubble.swift, line 723:
<comment>The copy shortcut activates when any metadata control is focused because they share one Boolean focus state. Track the focused metadata control with distinct focus values and enable `⌘C` only for the copy control.</comment>
<file context>
@@ -709,9 +718,32 @@ struct ChatBubble: View {
.focused($isMetadataControlFocused)
+ // Only while this row's control strip holds keyboard focus. A window-wide
+ // ⌘C would take the shortcut away from selected prose and the composer.
+ .modifier(ChatCopyKeyboardShortcut(isActive: isMetadataControlFocused))
.help("Copy message")
}
</file context>
Comment on lines
+59
to
+60
| /// Kinds whose cards are presentation only and must never enter the chat | ||
| /// journal. See `FloatingControlBarManager.persistNotificationMessageIfNeeded`. |
There was a problem hiding this comment.
P3: The isJournaled documentation reverses the property's behavior: true permits journal persistence, while only trial and onboarding return false. Describe the property as whether this kind should be journaled, and call out trial/onboarding as presentation-only exceptions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/Chat/ChatContinuityInvariants.swift, line 59:
<comment>The `isJournaled` documentation reverses the property's behavior: `true` permits journal persistence, while only trial and onboarding return `false`. Describe the property as whether this kind should be journaled, and call out trial/onboarding as presentation-only exceptions.</comment>
<file context>
@@ -35,7 +50,20 @@ enum ProactiveNotificationKind: String, Equatable, CaseIterable {
+ }
+ }
+
+ /// Kinds whose cards are presentation only and must never enter the chat
+ /// journal. See `FloatingControlBarManager.persistNotificationMessageIfNeeded`.
+ var isJournaled: Bool {
</file context>
Suggested change
| /// Kinds whose cards are presentation only and must never enter the chat | |
| /// journal. See `FloatingControlBarManager.persistNotificationMessageIfNeeded`. | |
| /// Whether this kind's card should enter the chat journal. Trial and onboarding cards | |
| /// are presentation only and must never be journaled. |
| ), | ||
|
|
||
| ( | ||
| "02-dashboard", |
There was a problem hiding this comment.
P3: previewChatProvider() is now dead code: its only caller was the removed "02-dashboard" registry entry. Remove the unused private helper to avoid a stale build-time dependency on ChatProvider.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/ViewExporter.swift, line 55:
<comment>`previewChatProvider()` is now dead code: its only caller was the removed "02-dashboard" registry entry. Remove the unused private helper to avoid a stale build-time dependency on ChatProvider.</comment>
<file context>
@@ -51,21 +51,6 @@ enum ViewExporter {
- ),
-
(
"04-conversations",
{ AnyView(ConversationsPage(appState: AppState(), selectedConversation: .constant(nil))) },
@@ -102,12 +87,6 @@ enum ViewExporter {
</file context>
|
|
||
| /// hub → chat → connect → close must collapse to the resting surface, and a | ||
| /// later `home_ask` must rest in chat — never force-jump to the hub. | ||
| func testAutomationCloseRoutesToUserCollapseNotHubJump() throws { |
There was a problem hiding this comment.
P3: Removing the three DashboardPage static-contract tests leaves the class doc comment factually wrong and an empty MARK section behind. The comment still claims HomeAskFocusPolicyTests unit-tests the deferred focus policy and that 'the DashboardPage assertions below' are static-contract tripwires, but both that test file and DashboardPage.swift were removed. Delete the now-empty '// MARK: Flow (static contract over DashboardPage wiring)' section and trim the stale references in the header comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Tests/HomeStageCloseSemanticsTests.swift, line 102:
<comment>Removing the three DashboardPage static-contract tests leaves the class doc comment factually wrong and an empty MARK section behind. The comment still claims HomeAskFocusPolicyTests unit-tests the deferred focus policy and that 'the DashboardPage assertions below' are static-contract tripwires, but both that test file and DashboardPage.swift were removed. Delete the now-empty '// MARK: Flow (static contract over DashboardPage wiring)' section and trim the stale references in the header comment.</comment>
<file context>
@@ -97,97 +97,4 @@ final class HomeStageCloseSemanticsTests: XCTestCase {
// MARK: Flow (static contract over DashboardPage wiring)
- /// hub → chat → connect → close must collapse to the resting surface, and a
- /// later `home_ask` must rest in chat — never force-jump to the hub.
- func testAutomationCloseRoutesToUserCollapseNotHubJump() throws {
- let source = try dashboardSource()
-
- XCTAssertFalse(
</file context>
DesktopHomeView held the app on a "Preparing Omi…" card until a network call decided which of two shells to mount, then rendered either ChatFirstShell or a legacy sidebar + DashboardPage tree. Both were the same product with different chrome, and the legacy branch was the only reason `useLegacyHomeDesign`, `useOldestHomeDesign`, DashboardPage's inline chat, SidebarView, and the widget hub still existed. The shell now mounts immediately for everyone. The server-owned capability still resolves — same request, same analytics event, same ChatProvider projection gate — but alongside the mounted shell rather than in front of it, and it now only decides whether the capability-gated kernel features engage. Capability-off renders the same shell. `navigate help` named a "Help from Founder" page no shell had mounted for a long time: the bridge resolved a title and then timed out. It now resolves to Settings → About, where getting help from a person actually lives. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Six of the journal's block kinds — question card, task card, goal link, capture link, conversation link, memory link — were dropped on the floor by every Chat surface except one. `ContentBlockGroup.group` skipped them unless `richBlockRenderingEnabled`, and `ChatBubble.blockView` returned `EmptyView` for each of them again. A turn whose whole content was a task card therefore read as an empty assistant reply in the task panel and in the notch, and as a card you could tick off in the main window. `ChatFirstRichBlockContext` is now non-optional on `QueryShellHome`, `QueryAnswerThread`, `ChatMessagesView` and `ChatBubble`; the task panel and the floating/notch renderers bind the shell's process-wide owners through `.auxiliary`, so a card tapped in the notch summons the main window and routes the one shell. `ChatFirstRichBlockGroupView` is the single renderer all three hosts share. Capability-off degrades rather than disappears: cards render, task check-off works (it binds `TasksStore`, not the projection), links navigate, and a question card shows its options dimmed and unpressable with an explicit "Answering is unavailable right now" line, instead of a question with no visible answers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`showNotification` took an optional `kind:` and `FloatingBarNotification` quietly filled it in from `assistantId`, whose default arm is `.general`. Five producers never passed one — trial messaging, onboarding permission help, both notch moments, and the whole generic proactive path — so their cards journaled a bare `notification:<uuid>` continuity key and came back in the transcript badged "Notification" with a bell, a row that says nothing about what Omi actually noticed. `kind:` is now required and never derived inside the value type. The generic proactive path derives it once at the producer edge, from the same `from(assistantId:)` call the category gate already makes three lines earlier. `.trial` and `.onboarding` are new kinds and are excluded from journaling alongside the integration nudge: billing copy and permission help are not observations. `.functional` carries the system notices that used to ride on `.general`. `.general` survives as decode-only so historical bare keys keep reading back, and its badge arm stays for exactly those rows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nd-shell flows The tests that protected the old shape are the reason it would come back: `ChatFirstRichBlockTests` asserted that a caller without an explicit context got *nothing*, and three flows waited on `shellVariant: legacy`. - `OneChatShellRichBlockTests` builds one turn carrying prose plus all six interactable kinds and asserts the grouping keeps every one of them, in transcript order, on the same entry point the notch and task panel call. It then drives each link's typed navigation target through the real navigation owner, and pins capability-off to "options dimmed", never "options gone" — via a new `ChatFirstQuestionCardOptionsPolicy` that separates *answered* and *retired* (hide) from *capability-off* (disable). - `ProactiveNotificationKindTests` walks every assistant id a producer ships and proves none of them derives `.general`, so no producer can mint a bare `notification:<uuid>` key; historical bare keys still decode. - `check-single-chat-shell.py` (+ manifest entries, with a self-test) is the tripwire for the vocabulary that made a second shell expressible. - `home-stage.yaml` and `dashboard.yaml` described `DashboardPage` and are deleted with it; `chat-first-capability-isolation.yaml` is repurposed to the assertion that now matters — capability-off mounts the same shell. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`ChatFirstRichBlockContext.auxiliary` binds `ChatFirstShellNavigation.shared` so a card tapped in the notch or the task panel routes the shell. The root was still creating its own instance, so those taps would have moved a navigation object nothing rendered — the card would appear to do nothing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…more `DashboardIntelligenceStore` fetched recommendations, projected them, and kept a feedback outbox for a section that only `DashboardPage` mounted. With the page gone its 750 lines had no renderer and no caller — the grep is exact: the class was referenced only by its own file and its own tests. Its 1,117-line test file went with it, because a test for a store nothing mounts is coverage of nothing. What stays is the part other surfaces still use: `TaskNavigationRequestStore`, the exact-record handoff `QueryShellHome` and the chat-first task card give the Tasks page instead of a tab index. The file is named for it now. Three source-reading tests still pointed at `DashboardPage.swift` and failed on the missing file rather than on anything real; they move onto `QueryAnswerThread` / `QueryShellHome`, which is where Home's error card and its colour tokens actually live. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…efault `navigate help` pre-selects About because that is where getting help from a person lives. A caller that also names a section meant that section. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…as cut off Three things a reader could not do with a reply on screen. Select it. `OmiMarkdown` disabled native text selection outright, so a date or a name in an answer could only be retyped. The reason was real — one AppKit selection overlay per `Text`, on a row that rewrites its body every streaming flush, is a non-converging layout loop (400 segments, 2 s hangs) — but it is a reason about *streaming* rows. Selection is now opt-in through `\.chatTextSelectable`, and `ChatTextSelectionPolicy` grants it to settled rows only, on every surface that shows chat prose: the transcript, the notch, the expanded floating bar, and onboarding. The three `.textSelection(.enabled)` calls outside `OmiMarkdown` in the floating surfaces were dead — the inner `.disabled` won — and are replaced rather than left as decoration. Copy it without hunting. The copy button lived only in the hover-revealed strip, and a user turn had no copy affordance at all. Every row now has a "Copy Message" context menu over the same pasteboard write, and the copy button takes ⌘C while its row's strip holds keyboard focus — not window-wide, which would take the shortcut from selected prose and the composer. See that it stopped mid-sentence. A voice barge-in persists the partial answer with a terminal failed status, and the only failure affordance was a stamp for a row with no text — so "…arrive on Saturday," rendered exactly like a finished reply. `ChatTurnFailurePresentation` decides between that stamp and a quiet trailing "Interrupted" mark, and the empty-row case is unchanged. Also: the hover strip is now `accessibilityHidden` when it is invisible (opacity and hit-testing hid it from the eye and the mouse but not VoiceOver), each row carries a You/Omi label, and a row whose whole content is a rich block reserves no metadata band — a memory card stamps its own time and has nothing to copy or rate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two consecutive one-line answers sat roughly 100 device pixels apart, and a memory card floated in symmetric dead space. Measured on the real views: 44 pt between two settled replies, and a 48 pt card row inflated to 69 pt. Two causes, both double-charges. The hover strip is 28 pt of real reserved height under every settled reply. The stack then added a full 16 pt inter-exchange gap on top of it, so the separation the band already provides was paid for twice. `ChatTranscriptLayout.spacing` now asks whether the row above reserves a band and takes a hairline when it does; every other rung of the ladder is unchanged, and a reply still binds to its question more tightly than to the next exchange. `ChatOmiMarkPlacement.rowHeight` reserved 32 pt on every assistant row for a mark that only needs it when an empty streaming reply has no height of its own. On a settled row the reservation did nothing but centre short content in a box taller than itself — which is what put equal dead space above and below the memory card. It now applies while streaming, top-aligned. Measured after: 32 pt between two replies, 48 pt for the card row, and a five-row transcript 296 pt tall instead of 341. Also collapses adjacent repeats. Dedup only ran on messages over 200 characters, so three push-to-talk tries at the same ~90-character question stuttered down the transcript untouched — each press mints a distinct `voice:<uuid>` turn, so those are three legitimate journal rows and journal identity is not the place to fix it. `adjacentDuplicateIDs` collapses a short answer repeated in the row immediately below it within ten minutes, and folds a failed barge-in fragment into the answer it is a strict prefix of. It stays behind the existing expandable "Duplicate message" chip, so nothing is hidden outright, and non-adjacent, distant, or cross-sender repeats are left alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Mobile stored `content_blocks` as raw maps and, since #12015, hid any message whose blocks were only desktop chat-first chrome (goal/task/ question) because there was no renderer for them. Both halves are now wrong: the components are coming, so the schema needs a typed projection and the hide filter has to go. Add `ChatContentBlock`, a sealed model mirroring the canonical schema in `desktop/macos/agent/src/runtime/types.ts` and the Swift codec's required-field rules, decoding both the camelCase (desktop/agent) and snake_case (chat-first spec) dialects. Malformed blocks are dropped; unknown types become `UnknownContentBlock` so the message keeps its synthesized fallback text instead of losing content. The raw list stays authoritative on the wire — `toJson` is unchanged. Delete `hideFromMobileChat` / `visibleOnMobile` and their four call sites in MessageProvider, replacing the "is this body only the fallback dump?" test with `textIsStructuredFallback`, which the renderer uses to decide whether components replace the body or sit beside it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ten new keys for the block eyebrows, destination actions, unavailable state, and the conversation link's recommended-steps header, translated into all 48 non-template locales. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every block type that macOS renders as a control now has a mobile component, driven from the same wire schema (#12598): - taskCard: a live checkbox wired to the single tasks mutation path, ActionItemsProvider.updateActionItemState. The tasks API is list-only, so the card resolves against the loaded list and mirrors the macOS loading / unavailable states rather than inventing a fetch-by-id. - goalLink: mobile has no goal detail route, so the card opens a bottom sheet with the goal's title and progress resolved from GoalsProvider, and shows the unavailable state when the id is not in the list. - captureLink / conversationLink: push ConversationDetailPage through the citation preamble already shipped in chat (grouped-map hit, then fetch by id). conversationLink also lists its recommended action items as plain rows; mobile creates tasks from the tasks surface, so the block mutates nothing. - memoryLink: opens the existing memory sheet for the resolved memory. - questionCard: options send their preparedAnswer down the normal chat send path, so the runtime stays authoritative for what an answer means. A deferral option is not special — it sends its own prepared answer. Once selectedOptionId is set only the chosen option remains, disabled, so no stale chip ever looks tappable. text/thinking/toolCall/discoveryCard/citation/agentSpawn/agentCompletion and unknown types render nothing extra — the body (or its synthesized fallback) already carries them — but they never hide the message. Where the body is only that fallback, the components replace it instead of repeating it. Every interactive element carries Key('chat-block-<type>-<id>...'). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…class Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…le transcript The selection change this branch shipped is reverted. `OmiMarkdown` disables native text selection again at both sites, explicitly, and the `\.chatTextSelectable` environment key, `ChatTextSelectionPolicy` and `OmiChatTextSelectability` are gone along with the host wiring in `AIResponseView`, `FloatingControlBarView` and `OnboardingChatView` — those hosts now carry no `textSelection` modifier around `OmiMarkdown` at all, since the ones that were there before this branch were dead code under the inner `.disabled`. The settled-row gate was not enough. PR #10834 made the same argument and reopened FC-selection-overlay-layout-loop in Omi Beta 0.12.146: every sampled main-thread stack sat in `SelectionOverlay`, `setFont`, intrinsic-size invalidation and AttributeGraph, and memory grew without bound. A settled row is still rebuilt by transcript loading, scrolling, window resize and parent-state updates, which is all that loop needs. `.github/scripts/check_chat_selection_boundary.py` rejects the escape hatch and names the remedy: the existing copy actions, or a separate non-live reading surface. So this adds the reading surface. "Select Text…" sits on the row's context menu next to "Copy Message" and as an ibeam button in the hover strip, on assistant and user rows alike. It opens `ChatSelectableTextPopover`: one `NSTextView` over one message's copyable text — `isEditable` false, `isSelectable` true, ⌘A and ⌘C native, Escape closes, sized to content with a 360 pt cap and internal scrolling. It is outside the transcript's layout and does not mount until the reader asks for it, so it cannot take part in the loading, scrolling and resize passes that made in-place selection unsafe. No SwiftUI `textSelection` anywhere in it — AppKit selection is what an `NSTextView` already is. The strip now carries four controls plus the timestamp (thumbs, thumbs, copy, select, info-when-present). At 24 pt each that still leaves the timestamp its own room, so both affordances stay rather than context-menu only. Everything else on this branch is unchanged: right-click Copy, focus-gated ⌘C, the metadata-band policy, the transcript rhythm, the interrupted-turn marker, adjacent duplicate collapse, and the accessibility fixes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
RealtimeConversationToolProjection.swift was folded into ChatToolExecutor in 758cd3f and the flow kept the stale path, which fails desktop-flow-lint. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Git-on-my-level
force-pushed
the
fix/one-chat-shell-ergonomics
branch
from
September 2, 2026 10:30
482cde7 to
803c4dd
Compare
#12608 (first-48h activation) landed upstream while this branch deleted the second shell. Three conflicts, all of them the same shape: main put new behaviour on surfaces this branch removes. - `DashboardPage.swift` (modify/delete): kept our deletion. Everything #12608 added to it was a *second* wiring of behaviour it had already wired into the shared surfaces — the day-0 chip composition, the prefilled-draft consumption, the draft-chip branch. Those live in `ChatProvider` / `OnboardingOpenerView` / `QueryShellHome` and survive. - `home-stage.yaml` (modify/delete): kept our deletion. Its S1-S8 assert `state.homeMode`, which only `DashboardPage` ever published; its new S9-S12 (prefilled draft, daily-summary snapshot) are real and are re-homed onto `chat-first.yaml`, on the route that owns them now. - `ptt-lifecycle.yaml` (content): took main's version. Both sides fixed the same stale covers path; main dropped the entry, we repointed it. Main's is the one already on the base branch. `QueryShellHome`'s new prefilled-draft path arrived guarded on `usesLegacyPresentation`, a symbol this branch deletes; the guard is dropped, which is also the correct behaviour — it is the only composer. `HomeDailySummarySection` was the hub's rendering of the summary and had no renderer left; its `HomeDailySummaryStatsRow`, which the surviving Chat card uses, stays and now names the file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Closes #12598.
What this does
One chat shell. Every macOS account now mounts
ChatFirstShell. TheuseLegacyHomeDesign/useOldestHomeDesignsettings, theChatFirstShellCapabilitySamplecohort switch, the.legacyvariant,QueryShellHome.usesLegacyPresentation,DashboardPageand its inline chat, the legacy sidebar shell, and the "Preparing Omi…" network-gated loading view are deleted. The shells already shared their chrome (ChatFirstShell's chat route wasQueryShellHome), so nothing a user could see moves except the things the legacy path was still hiding.Every block renders everywhere.
ChatFirstRichBlockContextis no longer optional. Task card, goal link, capture link, conversation link, memory link and question card render as their interactable component in the main chat, the task chat panel, the floating bar and the notch (which now share one navigation owner and summon the main window for deep links). The server capability (controlGeneration) is still resolved and still gates the kernel features it always gated; with it absent, cards still render, task check-off still works, links still navigate, and question-card options are disabled rather than hidden. No backend change:chat_first_uiwas a derived response field only the shell switch read.No bare "Notification" rows.
kind:is required onshowNotification; the assistant-id fallback is gone; trial and onboarding notices are not journaled; functional notices get a.functionalkind..generalsurvives decode-only for historical rows.Mobile renders the same block set. New typed
ChatContentBlockdecoder (camelCase and snake_case), six block widgets bound toActionItemsProvider.updateActionItemState,ConversationDetailPage,showMemoryDialog, a goal bottom sheet, and the chat send path for question-card answers. The #12015 filter that hid task/goal/question messages from the phone is removed.Chat ergonomics (macOS).
desktop-chat-selection-boundarygate exists because feat(macOS): enable drag selection for completed chat messages #10834 reopened FC-selection-overlay-layout-loop in Beta 0.12.146.…and "Interrupted" instead of looking finished.Proofs
OneChatShellRichBlockTests— one turn carrying all six block types renders six typed groups on the shared renderer; each action reachesChatFirstShellNavigation/ question selection; capability-off keeps the task card actionable and disables (not hides) options.ProactiveNotificationKindTests— no producer derives.general; every kinded key round-trips; historical bare keys still decode.ChatRowErgonomicsTests— band policy, failure presentation, adjacent dedup, rendered row heights viaNSHostingView;ChatSelectableTextSurfaceTests— the popover's text view is selectable, not editable, and updates in place.desktop/macos/scripts/check-single-chat-shell.pywired into.github/checks-manifest.yaml(with self-test) forbids every retired symbol.app/test/widgets/chat_content_blocks_test.dart— one fixture carrying every block; task toggle and question answer recorded on recording providers.server_message_content_blocks_test.dartinverted.dashboard.yaml,home-stage.yamlremoved;check-e2e-flow-coverage.py --strictanddesktop-flow-lint.pypass;agent-logic-harness.sh --swift-onlypassed.Known pre-existing failures on
origin/main, not touched here:check-sources-root-layout.py(90 vs baseline 77) andChatDiscoverabilityTests/testDesktopCapabilitiesExistInAgentToolDeclarations(look_at_frame).Merged with #12608 (first-48h activation)
origin/mainlanded the activation work while this branch deleted the second shell. Everything user-facing in it survives, on the surface that remains:ChatMessagesView(showsDailySummary, admitted only while the transcript follows the live edge), which both shells shared. It renders inQueryAnswerThread; the task panel opts out.HomeDailySummarySection, the hub's second rendering of the same record, had no renderer left and is deleted — itsHomeDailySummaryStatsRow, which the surviving Chat card uses, stays and names the file.ChatProvider.onboardingOpenerviaHomeSuggestionComposer.compose(dayZero: .live())and rendered byOnboardingOpenerView, which also owns the "Remember that I…" draft branch.DashboardPage's copy of both was the duplicate.MainChatNavigationRequestStore.request(draft:)→QueryShellHome.takePendingDraftIfAny(). It arrived guarded onusesLegacyPresentation, a symbol this branch deletes; the guard is gone, which is also correct — it is the only composer.OmiApp,FloatingControlBarWindow,RealtimeHubControllerand Settings; untouched by the shell deletion.home-stage.yamlS1–S8 assertstate.homeMode, which onlyDashboardPagepublished, so the flow goes with the page. Its new S9–S12 (open_chat_prefilled,chat_drafts_snapshot,daily_summary_snapshot) are real and move tochat-first.yamlS11a–S11c, on the route that owns them, with the activationcovers:entries that still name live files.Product invariants affected
ChatProvidergains no auth authority; the capability resolve still runs under the existing owner/authorization snapshot and fails to a nil projection, never to a session change.ChatProvider.messages, and dedup/interrupted marking are display-only over kernel turn identity.ChatMessagesViewis untouched; only the row spacing ladder and the dedup helper changed.ChatFirstShellowners (Tasks, Memories, Goals, Apps, Settings); the deleted legacy pages were the reduced copies.Failure-Class: new
Line-Count-Exception: desktop/macos/Desktop/Sources/DesktopAutomationBridge.swift | 4829 -> 4833 | shellVariant snapshot pinned to chat_first for flows; legacy field removed
Line-Count-Exception: desktop/macos/Desktop/Sources/FloatingControlBar/FloatingControlBarView.swift | 3086 -> 3095 | floating chat renders rich blocks through the shared renderer
Line-Count-Exception: desktop/macos/Desktop/Sources/FloatingControlBar/FloatingControlBarWindow.swift | 5689 -> 5696 | notification kind is required; trial/onboarding excluded from journaling
Line-Count-Exception: desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift | 2032 -> 2054 | Select Text affordance: context-menu item, hover-strip button, and one shared popover anchor per row
Line-Count-Exception: desktop/macos/Desktop/Sources/MainWindow/Components/OmiMarkdown.swift | 1643 -> 1649 | the explicit selection boundary and why a settled row is not an exception to it
Line-Count-Exception: desktop/macos/Desktop/Sources/Onboarding/OnboardingChatView.swift | 2175 -> 2176 | onboarding notice passes an explicit notification kind
Line-Count-Exception: desktop/macos/Desktop/Sources/ProactiveAssistants/Services/NotificationService.swift | 1528 -> 1535 | every proactive card passes an explicit kind
Line-Count-Exception: desktop/macos/Desktop/Sources/Providers/ChatProvider.swift | 7154 -> 7163 | capability resolve moved out of the shell switch; agent-card opening wired into Home
🤖 Generated with Claude Code