Repository navigation
Stream agent prose to the iOS chat as it generates (default-off) - #6731
Merged
Merged
Conversation
The iOS chat showed an agent answer only when its JSONL line was written, which both Claude Code 2.1 and Codex do per content block at block/turn completion, never per token. A single long final answer therefore appeared all at once. Confirmed by watching the on-disk file grow during a live turn (a 647-char answer went absent->complete in one 100ms tick) and by capturing Claude's interactive pty, which paints the answer as a synchronized-output frame with absolute-column cursor moves rather than a linear text append. Since the only token-grained source for an interactive turn is the rendered screen, add a live preview path that scrapes Ghostty's emulated screen grid while a turn is in flight, extracts the in-progress prose, and pushes it as a new streamingProse wire event. The preview lives outside the message window and is superseded the instant the authoritative JSONL line lands, so it never duplicates a committed message. - ChatSessionEvent.streamingProse(ChatMessage?): whole-value preview, nil clears. - ChatConversationStore: render the preview as a trailing agent bubble; clear it on authoritative agent prose or reset (no duplicate). - AgentChatProseScreenExtractor: pure, spinner-anchored screen->prose; returns nil unless a turn is actively streaming. Unit tested over Claude/Codex fixtures. - AgentChatProseStreamer + AgentChatTranscriptService: poll the surface grid on the turn lifecycle (UserPromptSubmit..Stop), gated by a default-off flag (CMUXAgentChatProseStreaming) and chat subscribers, off the keystroke path. Default-off pending iOS dogfood. The extractor is heuristic by nature; the JSONL reconciliation guarantees the committed state is always correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds live agent prose previewing from terminal snapshots through transient wire events, store projection, and an iOS debug preview surface. ChangesAgent Prose Streaming Preview
Sequence Diagram(s)sequenceDiagram
participant TranscriptService as AgentChatTranscriptService
participant Streamer as AgentChatProseStreamer
participant Extractor as AgentChatProseScreenExtractor
participant Store as ChatConversationStore
participant MobileView as StreamingChatPreviewView
TranscriptService->>Streamer: turnStarted(sessionID, agentKind, surfaceID)
loop active turn polling
Streamer->>Extractor: extract(lines:agentKind:)
Extractor-->>Streamer: prose preview or nil
Streamer->>Store: .streamingProse(preview)
end
TranscriptService->>Streamer: authoritativeProseArrived(sessionID)
Streamer->>Store: .streamingProse(nil)
TranscriptService->>Store: .appended(committed prose)
Store-->>MobileView: rendered preview bubble updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 2 warnings)
✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryAdds a default-off live streaming preview for agent prose on iOS chat: while a turn is in-flight the Mac host polls Ghostty's emulated screen grid at 150 ms, extracts the in-progress answer via a spinner-anchored heuristic extractor, and pushes it to iOS clients as a new
Confidence Score: 4/5Safe to merge behind the default-off flag; the streaming preview is always superseded by the authoritative JSONL message, so any mis-extraction is transient and self-correcting within one turn. The wire/store/lifecycle plumbing is correct and well-tested. Two open items from the prior review thread remain unaddressed in the diff: the
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Hook as AgentChatTranscriptService
participant Streamer as AgentChatProseStreamer
participant Ghostty as Ghostty Screen Grid
participant Wire as ChatSessionEvent Wire
participant Store as ChatConversationStore (iOS)
Hook->>Streamer: turnStarted(sessionID, surfaceID, agentKind)
loop every 150 ms while turn active
Streamer->>Ghostty: snapshot(surfaceID) → [String]
Ghostty-->>Streamer: screen rows
Streamer->>Streamer: AgentChatProseScreenExtractor.extract()
alt prose changed
Streamer->>Wire: .streamingProse(ChatMessage)
Wire-->>Store: "streamingMessage = preview"
Store->>Store: reproject() → trailing bubble
end
end
Hook->>Streamer: authoritativeProseArrived(sessionID)
Streamer->>Wire: .streamingProse(nil) [clear]
Wire-->>Store: "streamingMessage = nil"
Hook->>Wire: .appended([committedMessages])
Wire-->>Store: committed message takes over
Hook->>Streamer: turnEnded(sessionID)
Streamer->>Streamer: cancel loop task
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Hook as AgentChatTranscriptService
participant Streamer as AgentChatProseStreamer
participant Ghostty as Ghostty Screen Grid
participant Wire as ChatSessionEvent Wire
participant Store as ChatConversationStore (iOS)
Hook->>Streamer: turnStarted(sessionID, surfaceID, agentKind)
loop every 150 ms while turn active
Streamer->>Ghostty: snapshot(surfaceID) → [String]
Ghostty-->>Streamer: screen rows
Streamer->>Streamer: AgentChatProseScreenExtractor.extract()
alt prose changed
Streamer->>Wire: .streamingProse(ChatMessage)
Wire-->>Store: "streamingMessage = preview"
Store->>Store: reproject() → trailing bubble
end
end
Hook->>Streamer: authoritativeProseArrived(sessionID)
Streamer->>Wire: .streamingProse(nil) [clear]
Wire-->>Store: "streamingMessage = nil"
Hook->>Wire: .appended([committedMessages])
Wire-->>Store: committed message takes over
Hook->>Streamer: turnEnded(sessionID)
Streamer->>Streamer: cancel loop task
Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+15
to
+22
| enum AgentChatProseStreamingFlag { | ||
| static let defaultsKey = "CMUXAgentChatProseStreaming" | ||
|
|
||
| /// Whether the live streaming preview is enabled. Defaults to `false`. | ||
| static var isEnabled: Bool { | ||
| UserDefaults.standard.bool(forKey: defaultsKey) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
cmux-no-ambient-global-state
enum AgentChatProseStreamingFlag has no cases and exists solely to hold static let defaultsKey and static var isEnabled, which is exactly the pattern the rule flags. AgentChatProseStreamer.init already accepts isEnabled as an injectable closure (correct), but AgentChatTranscriptService.noteHookEvent reads AgentChatProseStreamingFlag.isEnabled directly from the global instead of using the same injected seam. Preferred shape: a fileprivate or private constant for the defaults key, read directly at the call site, or a small injectable struct if shared re-use is needed.
Rule Used: Flag new ambient global state in production Swift:... (source)
Comment on lines
+18
to
+19
| /// Drives the live agent-prose streaming preview (default-off feature flag). | ||
| private var proseStreamer: AgentChatProseStreamer! |
Contributor
There was a problem hiding this comment.
proseStreamer IUO (!) is avoidable — proseStreamer is always assigned in every designated init path, so the implicitly-unwrapped optional exists only to satisfy the two-phase init ordering. Declaring it private let and moving the AgentChatProseStreamer(...) construction before registry.onRecordChanged (which already captures self weakly) removes the crash-at-nil risk of the ! and makes the always-initialized invariant visible at the declaration site.
Suggested change
| /// Drives the live agent-prose streaming preview (default-off feature flag). | |
| private var proseStreamer: AgentChatProseStreamer! | |
| /// Drives the live agent-prose streaming preview (default-off feature flag). | |
| private let proseStreamer: AgentChatProseStreamer |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 517-523: In the `.streamingProse` case within
ChatConversationStore, the message validation on line 521 only checks if the
message is prose using `Self.isProse($0)` but does not verify it is specifically
`.agent` prose. Modify the flatMap condition in the `let next = message.flatMap
{ ... }` line to additionally validate that the message kind is `.agent` in
addition to passing the isProse check, so that only valid agent prose is
accepted and malformed non-agent frames are ignored.
In
`@Packages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/AgentChatProseScreenExtractorTests.swift`:
- Around line 129-131: The test assertion for the extracted line count is too
weak because the nil coalescing operator converts a nil result to 0, which still
passes the `<= 200` check. This allows broken extraction to pass silently.
Remove the nil coalescing operator and instead assert that the extraction result
from extractor.extract is non-nil, then assert the exact expected line count for
this fixture to properly catch any regressions in the extraction logic.
In `@Sources/Mobile/AgentChat/AgentChatProseStreamer.swift`:
- Line 61: Remove Task.sleep-driven polling from the production runtime path in
AgentChatProseStreamer by eliminating the sleep parameter default that uses
Task.sleep(for:) on line 61 and refactoring the polling loop at lines 112-116
that relies on Task.sleep cadencing. Replace the timing-based polling mechanism
with an alternative implementation that does not depend on Task.sleep or
sleep-driven polling primitives in the production code path.
- Around line 119-127: The emitPreviewIfChanged method returns early when
snapshot or extraction fails (lines 122-123) without emitting a nil prose event
to clear stale UI state. Instead of returning when snapshot is nil or extraction
fails, emit a ChatSessionEventFrame with .streamingProse(nil) to clear any
previously emitted preview and ensure the UI shows an empty/disabled state when
the signal is unavailable, following the fail-closed pattern described in the
path instructions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e405f061-aa9e-4459-aff4-ef71230f4153
📒 Files selected for processing (12)
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/AgentChatProseScreenExtractor.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Source/FixtureChatEventSource.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatSessionListReducer.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatSessionEvent.swiftPackages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/AgentChatProseScreenExtractorTests.swiftPackages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatConversationStoreTests.swiftSources/Mobile/AgentChat/AgentChatProseStreamer.swiftSources/Mobile/AgentChat/AgentChatProseStreamingFlag.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swiftcmux.xcodeproj/project.pbxprojdocs/streaming-agent-updates.md
Comment on lines
+517
to
+523
| case .streamingProse(let message): | ||
| // The preview is a whole-value replace; an agent session only. A | ||
| // terminal session has no agent prose, so ignore it there. | ||
| guard descriptor.kind != .terminal else { break } | ||
| let next = message.flatMap { Self.isProse($0) ? $0 : nil } | ||
| guard next != streamingMessage else { break } | ||
| streamingMessage = next |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail closed on malformed .streamingProse payloads.
Line 521 only checks message kind (.prose) and will accept non-agent prose. Restrict this path to .agent prose so invalid frames are ignored instead of rendered.
As per path instructions, “Correctness-critical state must come from a single authoritative source … fail closed (disable/empty state) rather than guessing.”
🔧 Suggested fix
- let next = message.flatMap { Self.isProse($0) ? $0 : nil }
+ let next = message.flatMap { ($0.role == .agent && Self.isProse($0)) ? $0 : nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`
around lines 517 - 523, In the `.streamingProse` case within
ChatConversationStore, the message validation on line 521 only checks if the
message is prose using `Self.isProse($0)` but does not verify it is specifically
`.agent` prose. Modify the flatMap condition in the `let next = message.flatMap
{ ... }` line to additionally validate that the message kind is `.agent` in
addition to passing the isProse check, so that only valid agent prose is
accepted and malformed non-agent frames are ignored.
Source: Path instructions
Comment on lines
+129
to
+131
| let result = extractor.extract(lines: rows, agentKind: .claude) | ||
| let lineCount = result?.split(separator: "\n", omittingEmptySubsequences: false).count ?? 0 | ||
| #expect(lineCount <= 200) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Strengthen the cap test so extractor regressions cannot pass silently.
Line 130 converts nil to 0, so a broken extraction path still passes <= 200. Require non-nil extraction (and ideally assert the exact capped line count for this fixture).
🔧 Suggested fix
let result = extractor.extract(lines: rows, agentKind: .claude)
- let lineCount = result?.split(separator: "\n", omittingEmptySubsequences: false).count ?? 0
- `#expect`(lineCount <= 200)
+ guard let result else {
+ Issue.record("expected capped prose extraction, got nil")
+ return
+ }
+ let lineCount = result.split(separator: "\n", omittingEmptySubsequences: false).count
+ `#expect`(lineCount == 200)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/AgentChatProseScreenExtractorTests.swift`
around lines 129 - 131, The test assertion for the extracted line count is too
weak because the nil coalescing operator converts a nil result to 0, which still
passes the `<= 200` check. This allows broken extraction to pass silently.
Remove the nil coalescing operator and instead assert that the extraction result
from extractor.extract is non-nil, then assert the exact expected line count for
this fixture to properly catch any regressions in the extraction logic.
| hasSubscribers: @escaping @MainActor () -> Bool, | ||
| now: @escaping @MainActor () -> Date = { Date() }, | ||
| pollInterval: Duration = .milliseconds(150), | ||
| sleep: @escaping @Sendable (Duration) async -> Void = { try? await Task.sleep(for: $0) } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Remove Task.sleep-driven polling from this production runtime path.
Line 61 and Lines 112-116 introduce a Task.sleep cadence loop in shipped Swift runtime code. This conflicts with the repo rule set that treats timing/polling primitives in production paths as failures by default.
As per coding guidelines, "Task.sleep/timing-based polling in production Swift code should be treated as failures by default."
Also applies to: 112-116
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Mobile/AgentChat/AgentChatProseStreamer.swift` at line 61, Remove
Task.sleep-driven polling from the production runtime path in
AgentChatProseStreamer by eliminating the sleep parameter default that uses
Task.sleep(for:) on line 61 and refactoring the polling loop at lines 112-116
that relies on Task.sleep cadencing. Replace the timing-based polling mechanism
with an alternative implementation that does not depend on Task.sleep or
sleep-driven polling primitives in the production code path.
Source: Coding guidelines
Comment on lines
+119
to
+127
| private func emitPreviewIfChanged(sessionID: String) { | ||
| guard let turn = turns[sessionID], !turn.settled else { return } | ||
| guard isEnabled(), hasSubscribers() else { return } | ||
| guard let lines = snapshot(turn.surfaceID) else { return } | ||
| guard let prose = extractor.extract(lines: lines, agentKind: turn.agentKind) else { return } | ||
| guard prose != turn.lastEmitted else { return } | ||
| turns[sessionID]?.lastEmitted = prose | ||
| emit(ChatSessionEventFrame(sessionID: sessionID, event: .streamingProse(previewMessage(sessionID: sessionID, text: prose)))) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear stale preview when extraction/snapshot signal disappears.
On Line 123, a failed extraction path returns early without emitting .streamingProse(nil). If a preview was already emitted, the UI can keep showing stale prose until turnEnded or authoritative append arrives, instead of failing closed.
💡 Proposed fix
private func emitPreviewIfChanged(sessionID: String) {
guard let turn = turns[sessionID], !turn.settled else { return }
- guard isEnabled(), hasSubscribers() else { return }
- guard let lines = snapshot(turn.surfaceID) else { return }
- guard let prose = extractor.extract(lines: lines, agentKind: turn.agentKind) else { return }
+ guard isEnabled(), hasSubscribers(),
+ let lines = snapshot(turn.surfaceID),
+ let prose = extractor.extract(lines: lines, agentKind: turn.agentKind)
+ else {
+ clearPreviewIfNeeded(sessionID: sessionID)
+ return
+ }
guard prose != turn.lastEmitted else { return }
turns[sessionID]?.lastEmitted = prose
emit(ChatSessionEventFrame(sessionID: sessionID, event: .streamingProse(previewMessage(sessionID: sessionID, text: prose))))
}
+
+private func clearPreviewIfNeeded(sessionID: String) {
+ guard turns[sessionID]?.lastEmitted != nil else { return }
+ turns[sessionID]?.lastEmitted = nil
+ clearPreview(sessionID: sessionID)
+}As per path instructions, "If a reliable signal is missing, fail closed (disable/empty state) rather than guessing."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Mobile/AgentChat/AgentChatProseStreamer.swift` around lines 119 -
127, The emitPreviewIfChanged method returns early when snapshot or extraction
fails (lines 122-123) without emitting a nil prose event to clear stale UI
state. Instead of returning when snapshot is nil or extraction fails, emit a
ChatSessionEventFrame with .streamingProse(nil) to clear any previously emitted
preview and ensure the UI shows an empty/disabled state when the signal is
unavailable, following the fail-closed pattern described in the path
instructions.
Source: Path instructions
A self-playing agent chat mounted at the root when CMUX_UITEST_STREAMING_CHAT_PREVIEW=1, mirroring the existing terminal/ workspace layout previews. It drives the real ChatConversationStore + ChatScreen with live streamingProse events that grow word by word then clear, so the incremental streaming preview can be recorded and verified on a simulator with no sign-in or Mac pairing. Used to capture frame-by-frame proof that the agent bubble builds incrementally and is superseded cleanly with no duplicate. DEBUG-only; Release compiles the branch to EmptyView. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/StreamingChatPreviewView.swift`:
- Around line 32-58: The preview title and scripted chat copy in
StreamingChatPreviewView are still hardcoded English user-facing strings. Update
the ChatSessionDescriptor title, the ChatMessage prose text, and the drive()
narration text to use String(localized:defaultValue:) with stable keys, and add
matching entries to Resources/Localizable.xcstrings so the preview renders
localized text instead of literals.
- Around line 52-73: The preview loop in StreamingChatPreviewView.drive only
exercises incremental streaming updates and clearing, but it never simulates the
authoritative committed message that should reconcile the trailing preview
bubble. Update the FixtureChatEventSource sequence in drive so it emits the
final committed agent message event before calling streamingProse(nil), using
the existing previewMessage helper and the same event source flow, so ChatScreen
can verify settlement/reconciliation without duplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 03a96197-10cd-47cd-886b-3bfaebb1927e
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/StreamingChatPreviewView.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swift
Comment on lines
+32
to
+58
| let descriptor = ChatSessionDescriptor( | ||
| id: "streaming-preview", | ||
| agentKind: .claude, | ||
| title: "Streaming preview" | ||
| ) | ||
| let prompt = ChatMessage( | ||
| id: "preview-user-0", | ||
| seq: 0, | ||
| role: .user, | ||
| timestamp: Date(), | ||
| kind: .prose(ChatProse(text: "Reply with three short sentences about the color blue.")) | ||
| ) | ||
| let source = FixtureChatEventSource(backlog: [prompt]) | ||
| let store = ChatConversationStore(descriptor: descriptor, source: source) | ||
| let model = Model(store: store, source: source) | ||
| model.runTask = Task { await store.run() } | ||
| model.driveTask = Task { await Self.drive(source: source) } | ||
| self.model = model | ||
| } | ||
|
|
||
| /// Loops the streaming lifecycle so any recording window captures a full | ||
| /// build: grow the answer word by word via `streamingProse`, hold, then | ||
| /// clear with `streamingProse(nil)`, and repeat. | ||
| private static func drive(source: FixtureChatEventSource) async { | ||
| let full = "The sky looks blue because air scatters short blue wavelengths most. " | ||
| + "Blue is widely tied to calm, depth, and quiet focus. " | ||
| + "From sapphires to the open ocean, it runs through the natural world." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the preview title and scripted chat text.
Line 35, Line 42, and Lines 56-58 introduce bare user-facing strings that will render directly in the simulator preview. These need String(localized:defaultValue:) keys plus matching Resources/Localizable.xcstrings entries instead of raw English literals. As per coding guidelines, "All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text")" and per path instructions, "Swift text must use localized APIs with matching translated string-catalog entries."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/StreamingChatPreviewView.swift`
around lines 32 - 58, The preview title and scripted chat copy in
StreamingChatPreviewView are still hardcoded English user-facing strings. Update
the ChatSessionDescriptor title, the ChatMessage prose text, and the drive()
narration text to use String(localized:defaultValue:) with stable keys, and add
matching entries to Resources/Localizable.xcstrings so the preview renders
localized text instead of literals.
Sources: Coding guidelines, Path instructions
Comment on lines
+52
to
+73
| /// Loops the streaming lifecycle so any recording window captures a full | ||
| /// build: grow the answer word by word via `streamingProse`, hold, then | ||
| /// clear with `streamingProse(nil)`, and repeat. | ||
| private static func drive(source: FixtureChatEventSource) async { | ||
| let full = "The sky looks blue because air scatters short blue wavelengths most. " | ||
| + "Blue is widely tied to calm, depth, and quiet focus. " | ||
| + "From sapphires to the open ocean, it runs through the natural world." | ||
| let words = full.split(separator: " ").map(String.init) | ||
| // Let ChatScreen subscribe and load the seeded prompt first. | ||
| try? await Task.sleep(for: .milliseconds(1200)) | ||
| while !Task.isCancelled { | ||
| var accumulated = "" | ||
| for word in words { | ||
| if Task.isCancelled { return } | ||
| accumulated += accumulated.isEmpty ? word : " " + word | ||
| await source.emit(.streamingProse(previewMessage(text: accumulated))) | ||
| try? await Task.sleep(for: .milliseconds(130)) | ||
| } | ||
| try? await Task.sleep(for: .milliseconds(1500)) | ||
| await source.emit(.streamingProse(nil)) | ||
| try? await Task.sleep(for: .milliseconds(800)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This preview never exercises the settlement/reconciliation path.
The loop only emits incremental .streamingProse(...) updates and then .streamingProse(nil). That verifies preview growth/clear, but it does not simulate the authoritative committed prose event that is supposed to replace the preview without duplication, so the main behavior called out in this PR is still untested here. Emit the final committed agent message before clearing the preview so this surface actually covers reconciliation. Based on PR objectives, the preview should validate that the trailing bubble is reconciled when the authoritative JSONL message arrives.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/StreamingChatPreviewView.swift`
around lines 52 - 73, The preview loop in StreamingChatPreviewView.drive only
exercises incremental streaming updates and clearing, but it never simulates the
authoritative committed message that should reconcile the trailing preview
bubble. Update the FixtureChatEventSource sequence in drive so it emits the
final committed agent message event before calling streamingProse(nil), using
the existing previewMessage helper and the same event source flow, so ChatScreen
can verify settlement/reconciliation without duplication.
This comment has been minimized.
This comment has been minimized.
Captured a live `claude` turn over the debug socket and replayed the raw
read-screen frames through the extractor. The synthetic fixtures had been
unrealistic in three ways the real TUI exposed, each of which made the
extractor return garbage or leak the prompt instead of the answer:
- The in-progress answer is itself prefixed with Claude's "⏺ " bullet, and
the persistent bottom mode bar carries "esc to interrupt" *below* the input
box. The old anchor picked that bottom bar and folded the input-box chrome
(a divider row) into the preview.
- During the first seconds the spinner line is a bare gerund ("✻ Nebulizing… ")
with no timer, so it was missed entirely and collection walked up into the
wrapped user prompt, leaking "...three sentences." as a fake answer.
- The "running stop hooks… 0/3 · 3s · ↓ 56 tokens" status drops the paren
around the elapsed timer.
Fixes:
- Three-tier anchor: timer line, then bare gerund line (leading animated
spinner glyph + "…", which excludes the post-turn "Brewed for 3s" summary),
then an interrupt hint *on the working line only* (never the footer mode bar).
- Treat the answer's own "⏺ " as an inclusive top, and for Claude require it:
if collection reaches a prompt boundary without it, there is no answer yet so
return nil (the thinking phase previews nothing).
- Strip the 2-space hanging indent so wrapped answer lines read as one paragraph.
- Relaxed timer scanner (bare "· 3s") plus a strict parenthesized variant used
to tell a live "Forming… (9s)" from the bare "Brewed for 3s" summary.
Tests now replay the verbatim live frames (thinking→partial→full→settled) and
assert nil until "⏺ "+words appear, the growing partial, the clean full answer,
and nil once settled.
…updates # Conflicts: # Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swift
The required `workflow-guard-tests` "Validate Swift file length budget" step failed: the streaming feature legitimately grew four tracked files past their budgets. Surgically bump only those entries (rather than a full --write-budget rewrite, which churns unrelated recounts): - ChatConversationStoreTests.swift 877 → 948 (streaming reconciliation tests) - ChatConversationStore.swift 722 → 764 (streamingMessage projection) - CMUXMobileRootView.swift 505 → 518 (streaming preview branch) - AgentChatTranscriptService.swift: newly tracked at 524 (prose streamer wiring)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swift (1)
103-129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMirror the launch-argument fallback on the new preview flags.
workspaceListLayoutPreviewEnablednow accepts both environment and launch-argument forms, but these new preview toggles still read environment only. Any UI-test/dev launcher that passesCMUX_UITEST_*_PREVIEW=1as an argument will never activate the streaming or agent-chat previews, so the new harnesses won't come up under the same invocation style.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swift` around lines 103 - 129, The new preview toggles in UITestConfig currently read only from ProcessInfo.processInfo.environment, so they miss the launch-argument fallback used by workspaceListLayoutPreviewEnabled. Update streamingChatPreviewEnabled, agentChatPreviewEnabled, and agentChatInlinePreviewEnabled to check both environment and ProcessInfo.processInfo.arguments for their CMUX_UITEST_*_PREVIEW flags, using the same pattern as the existing preview flag logic in UITestEnvironmentConfig so UI-test/dev launchers can activate them consistently.Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift (1)
199-208: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftKeep the streaming preview out of the production root routing path.
This branch adds a new debug/UI-test-only surface to
rootContentin a productionSources/file. The repo rules treat new test/debug seams in production Swift source as failures; mount this from a dedicated debug-only composition point instead of the shipping root view. As per path instructions,**/Sources/**/*.swift: "Do not add test-only or debug-only seams in production Swift source files" and "isolate genuinely debug-only facilities in a dedicated debug file or folder."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift` around lines 199 - 208, The root routing in CMUXMobileRootView/rootContent is exposing a debug-only streamingChatPreview from production Sources code. Remove that branch from the shipping root view and move the streaming preview selection into a dedicated debug-only composition point or debug file/folder, keeping only production navigation paths in rootContent. Ensure any references to shouldShowStreamingChatPreview and streamingChatPreview are isolated outside the production root routing path.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift`:
- Around line 199-208: The root routing in CMUXMobileRootView/rootContent is
exposing a debug-only streamingChatPreview from production Sources code. Remove
that branch from the shipping root view and move the streaming preview selection
into a dedicated debug-only composition point or debug file/folder, keeping only
production navigation paths in rootContent. Ensure any references to
shouldShowStreamingChatPreview and streamingChatPreview are isolated outside the
production root routing path.
In `@Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swift`:
- Around line 103-129: The new preview toggles in UITestConfig currently read
only from ProcessInfo.processInfo.environment, so they miss the launch-argument
fallback used by workspaceListLayoutPreviewEnabled. Update
streamingChatPreviewEnabled, agentChatPreviewEnabled, and
agentChatInlinePreviewEnabled to check both environment and
ProcessInfo.processInfo.arguments for their CMUX_UITEST_*_PREVIEW flags, using
the same pattern as the existing preview flag logic in UITestEnvironmentConfig
so UI-test/dev launchers can activate them consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0987b679-e885-4cc9-9e2c-285b3b9c210b
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swiftcmux.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
- cmux.xcodeproj/project.pbxproj
…updates # Conflicts: # .github/swift-file-length-budget.tsv
…updates # Conflicts: # .github/swift-file-length-budget.tsv # Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatSessionListReducer.swift # Sources/Mobile/AgentChat/AgentChatTranscriptService.swift
This branch was successfully deployed
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.
What
The iOS chat showed an agent's answer only once its JSONL transcript line was written. Both installed agents write prose to that file per content block at block/turn completion, never per token, so a single long final answer appeared all at once instead of building as it generated.
This adds a live streaming preview: while a turn is in flight, the host scrapes Ghostty's emulated screen grid for the agent surface, extracts the in-progress prose, and pushes it to chat clients as a new
streamingProsewire event. The preview is reconciled to the authoritative JSONL line the instant it lands, so it never duplicates a committed message.Behind a default-off flag (
CMUXAgentChatProseStreaming) pending iOS dogfood.Why this path (investigation)
Documented in
docs/streaming-agent-updates.md. Confirmed against the installed versions (Claude Code 2.1.191, Codex 0.142.0):agent_message_delta. So this is not a parser change.ESC[?2026h/l) with absolute-column cursor moves, repainting a bottom live region per frame. Recovering text means running a terminal emulator, which Ghostty already is.stream-jsondeltas exist only in non-interactive print mode, incompatible with the interactive TUI the user runs.So the one viable source is Ghostty's emulated screen text, reconciled to JSONL.
How
ChatSessionEvent.streamingProse(ChatMessage?): whole-value preview replace;nilclears. Lives outside the message window.ChatConversationStore: renders the preview as a trailing agent bubble; clears it on authoritative agent prose append orreset.AgentChatProseScreenExtractor: pure, spinner-anchored screen to prose. Anchors on the agent's working line, takes the contiguous answer block above it up to the previous committed block, strips TUI chrome. Returnsnilunless a turn is actively streaming.AgentChatProseStreamer+AgentChatTranscriptService: polls the surface render-grid on the turn lifecycle (UserPromptSubmittoStop), gated by the default-off flag and live chat subscribers, off the keystroke hot path. Settles on the authoritative prose append.Verified against a real live turn
The extractor was hardened against raw
read-screencaptures of a liveclaudeturn (Claude Code 2.1.191), driven over the debug socket. Replaying the verbatim frames exposed three things the original synthetic fixtures had wrong, each of which made the extractor return chrome or leak the prompt:⏺bullet, and the persistent bottom mode bar carriesesc to interruptbelow the input box (the old anchor picked that bar and folded a divider row into the preview);✻ Nebulizing…) with no timer, so collection walked up into the wrapped user prompt and leaked...three sentences.as a fake answer;running stop hooks… 0/3 · 3s · ↓ 56 tokensstatus drops the paren around the timer.Fixed via a three-tier anchor (timer line, bare gerund line, interrupt hint on the working line only, never the footer), requiring Claude's
⏺answer-top (so the thinking phase previews nothing), and stripping the 2-space hanging indent. The test suite now replays the live thinking→partial→full→settled frames and asserts:niluntil⏺+words appear, the growing partial, the clean full answer,nilonce settled.Principled vs hacky
The wire/store/lifecycle plumbing is principled and reuses existing seams (the render-grid snapshot, the surface→session registry, the
.appended/.updatedreconciliation discipline). The screen extractor is inherently heuristic — it pattern-matches per-agent TUI chrome, which drifts with CLI versions and locale. That risk is bounded: the preview is always superseded by the JSONL line at turn end, so any mis-extraction is transient and self-corrects within one turn, and the feature is default-off.Known limitations (v1)
Tests
swift test --package-path Packages/Shared/CmuxAgentChat: 120 pass (0 failures), including the real-frameAgentChatProseScreenExtractorfixtures and store reconciliation (streaming prose renders/clears,authoritative agent prose supersedes the preview without a duplicate).CmuxMobileSupport: 63 pass.CmuxMobileShellUIbuilds for the iOS simulator.Dogfood
Default-off. To try:
defaults write <bundle-id> CMUXAgentChatProseStreaming -bool YES, pair an iPhone, open an agent's chat, send a prompt that yields a multi-sentence answer, and watch the bubble build as it generates, then match the committed message exactly at turn end.🤖 Generated with Claude Code
Summary by CodeRabbit