Align render loop with Ghostty - #5
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ddf0eb6bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| let start = CACurrentMediaTime() | ||
| ghostty_app_tick(app) | ||
| AppDelegate.shared?.tabManager?.tickRender() | ||
| let elapsedMs = (CACurrentMediaTime() - start) * 1000 |
There was a problem hiding this comment.
Ensure Ghostty surfaces are still drawn each tick
With tickRender() removed, tick() now only calls ghostty_app_tick(app). I couldn’t find any remaining call to ghostty_surface_draw in the repo, which was previously the only render entry point. If GhosttyKit still expects the host to invoke ghostty_surface_draw to present frames, visible surfaces won’t repaint on output/cursor blink (only on size refresh via ghostty_surface_refresh). Please confirm GhosttyKit now draws internally on ghostty_app_tick, or reintroduce an explicit draw for visible surfaces to avoid frozen renders.
Useful? React with 👍 / 👎.
`browser_accessibility_cocoa.mm` declares NSAccessibilityUIElementsForSearchPredicateParameterizedAttribute and NSAccessibilityScrollToVisibleAction in an anonymous namespace as private backports. The macOS 26.2 SDK now declares the same names publicly (@available(macos 26.0)), so unqualified uses are ambiguous. This is a hard compile error, not a -W warning a pragma can silence, so patch 0005 renames the anon-namespace backports to CmuxNS* and fixes the 2 use sites of each. Also retracts the strategic note in patches/README.md that proposed switching to refs/branch-heads/7204. ls-remote shows 7204 resolves to the same commit (72a51d14...) the checkout is already on -- the LKGM shape of the HEAD message hid that fact. M148 stable IS the current base; there is no cheaper alternative. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…-ai#2 manaflow-ai#3 + tests) Three real bugs from the review of bc76ffe, all fixed in one go since they touch the same per-binding state machine: manaflow-ai#1 — HerdrTabRegistry.remove now calls HerdrInboundLayoutSync.forgetBinding (and HerdrDividerSync.reset). The throttle/suppress commit added three static maps keyed by binding.rootCmuxPaneId (lastDividerApplyAt, pendingDividerSpec, dividerApplyScheduled, plus the new pendingResizeRetries / per- binding suppression state) — none of them were cleared on teardown. A future binding reusing the same UUID inherited stale throttle timestamps and could have applied dividers from a tree that no longer existed. manaflow-ai#3 — Suppression is now per-binding instead of one global static. inboundApplyActiveByBinding + inboundApplySuppressUntilByBinding are keyed by binding.rootCmuxPaneId; shouldSuppressOutboundResize takes an optional binding key (nil falls back to the global aggregate, used by the legacy getter and the unresolved-binding defensive path in forwardPanelSize). HerdrPanelOpener. forwardPanelSize resolves the binding key for the panel via HerdrTabRegistry.binding(forCmuxPaneId:) and consults the per-binding state. A drag in workspace A no longer suppresses legitimate user-driven cmux window resizes in workspace B. manaflow-ai#2 — When forwardPanelSize gets suppressed, it now hands a retry closure to markPendingResize. After the trailing window closes we re-fire the latest pending retry per panel so the user-driven resize is not silently dropped. Each markPendingResize schedules a flush Task that waits inboundApplySuppressTrailingMs + 50ms; if the suppression window has been re-armed by another apply, the flush no-ops and the next apply schedules another. forgetBinding also drains the pendingResizeRetries dict so a torn-down panel doesn't leak its closure. Tests: - testPerBindingSuppressionDoesNotBleedToOtherBindings — locks contract that workspace A's drag doesn't suppress workspace B. - testForgetBindingClearsSuppressionState — locks the teardown cleanup that manaflow-ai#1 was missing. - testPendingResizeFiresAfterSuppressionReleases — locks that a suppressed user resize is re-fired exactly once after the trailing window expires. Test seam `_withInboundApplyActiveForTesting` now accepts an optional bindingKey so the per-binding-scoping test can drive both the global and per-binding state from one helper. Open items from the review NOT addressed in this commit: - Throttle still has no test (reviewer manaflow-ai#7). Next. - 33ms/250ms hardcoded constants undocumented (reviewer manaflow-ai#10). - The trailing window deadline can shrink across re-entrant apply boundaries on wall-clock backwards step (reviewer manaflow-ai#5). Use a monotonic clock; deferred. - Bandwidth premise still unproven (reviewer A). Need instrumentation pass to confirm SSH-master is actually the bottleneck before claiming this commit "fixes" the symptom.
Auto-scroll-to-bottom now targets the last visible chunk's id with anchor:.bottom instead of the LazyVStack container's .id() sentinel. Sidesteps the documented "lazy stacks trade some degree of layout correctness for performance" trade-off by asking ScrollViewReader to position a specific row, not to compute the container bottom from total contentSize. Empirically reduces blank-screen-on-collapse substantially; the residual on `-> .topLevelExpanded` after accumulated off-screen drift is accepted until user-impact escalates. AppKit migration ruled out per DECISIONS.md Don't-re-walk manaflow-ai#4 (List(.plain) had unacceptable initial-render lag; a hand-rolled representable would inherit the same NSHostingView.intrinsicContentSize per-row cost). Handover docs catch up: - DECISIONS.md / FORK_NOTES.md rewritten as canonical handover; new Mitigations-currently-shipped section documents the partial fix and its residual. - AGENT_WORKFLOW.md adds Xcode 26.5 reality: no downloadable DocC archives for system frameworks; canonical fetch is the JSON DocC endpoint at developer.apple.com/tutorials/data/documentation/<path>.json via curl + SOCKS5. - docs/apple-swiftui-scroll/README.md drops the stale ".defaultScrollAnchor(.bottom) is the systematic fix" claim that contradicted DECISIONS.md Don't-re-walk manaflow-ai#5; flags the cached snapshots as starting reference only. 141-test inspector subset green.
First slice of extending chat to plain terminals. Platform-free, fully tested, no mac/PTY wiring yet. TerminalCommandBlock: the terminal analogue of a chat message (command, output, exitCode, isRunning, isInteractive) — rendered later as a single-column monospace log, not opposing bubbles. OSC133CommandParser: a pure, incremental state machine that segments a shell PTY stream into blocks via OSC 133 semantic-prompt marks (A/B/C/D), strips other ANSI/OSC sequences, folds carriage-return progress redraws to final per-line state, flags alt-screen programs interactive, and carries partial escape sequences across chunk boundaries. 10 swift-testing cases (happy path, failure, running-until-next-prompt, two commands, split escape, streaming, CR fold, alt-screen, sequence stripping, bare command). 92 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Renders TerminalCommandBlock as a single-column monospace LOG (not opposing bubbles) — the visual differentiator of a terminal session from an agent chat. TerminalCommandBlockView: a ❯-prompted command row over its monospace output, with long-output collapse (head/tail + 'more lines' expander), a run-status footer (green check / red exit code), a red left rail for failures, and a running spinner. Interactive (alt-screen) blocks render TerminalInteractiveCardView with an Open-Terminal escape hatch instead of the raw screen. Wired into a DEBUG Settings > Developer > Terminal Log Demo screen so the log is verifiable on the simulator before a Mac host parses real PTY streams. Verified: ls/git/swift-build/cat-fail/npm-dev/vim sample blocks all render correctly (/tmp/tgchat-sliceB-demo2.png). 3 new localized strings (en+ja). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add ChatTranscriptRow.terminalCommand(TerminalCommandBlock) and dispatch it in ChatTranscriptRowView to TerminalCommandBlockView, so a terminal session's log renders through the same ChatTranscriptListView as agent chats (shared scroll, expansion, pill, composer) rather than a separate view. Row id is 'term-<n>', distinct from msg-/pending- ids so the ForEach never sees duplicate ids (the duplicate-id case is what thrashes lazy layout into the 100% CPU freeze). The Terminal Log Demo now feeds .terminalCommand rows through the real ChatTranscriptListView; sim-verified the log renders correctly in the actual transcript machinery (/tmp/tgchat-sliceC-demo.png). 3 row tests (distinct/ prefixed ids, no cross-kind collision, output change breaks equality). 95 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
From an adversarial review of Slice A: - CRLF blanking: "\r\n" is a single grapheme in Swift, so split-on-"\n" never saw CRLF and the fold either no-op'd or blanked lines. Normalize CRLF to LF before folding standalone \r progress redraws. (real bug, corrupts CRLF output) - O(n^2) output build: foldCarriageReturns ran on the whole buffer per character. Append raw and fold once per consume() (and at close). (perf, large output) - Unbounded pending: an unterminated OSC/CSI grew the carryover buffer without bound and wedged the parser. Cap escape-sequence scan at 8 KiB and resync. (DoS) - Batched alt-screen modes: ESC[?1049;2004h (alt screen + bracketed paste) was missed by the exact "?1049" check; now parse the ;-separated private modes. - De-fanged the openIndex force-unwrap in the output path (guarded flush). 5 new tests (CRLF, CRLF+progress mix, batched alt-screen, unterminated-OSC resync, large output). 100 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lower-priority items from the UI review: - VoiceOver: .accessibilityElement(.combine) absorbed the inline more-lines button, so a VoiceOver user could not expand collapsed output. Add an .accessibilityActions toggle (Show all output / Show less output) when the output is collapsible. 2 new localized strings (en+ja). - Perf: cap each output line at 4000 chars (with an ellipsis) so one pathologically long line cannot force an unbounded-width Text layout on every render. Multi-line collapse already bounds line count; this bounds width. Localization audit: 68 keys, all en+ja translated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add ChatSessionKind (.agent/.terminal) on ChatSessionDescriptor (declaration default, not yet a CodingKey, so existing wire payloads keep decoding; Slice D adds the wire field for real terminal sessions). ChatScreen passes descriptor.kind to the composer, which for a terminal session shows a monospace '> command' placeholder and a monospace input instead of 'Message <Agent>'. The Terminal Log Demo now mounts the real ChatComposerView under the log, so the complete terminal-chat surface (monospace log + terminal composer) is sim-verified (/tmp/tgchat-c2-demo.png). Codable backward-compat covered by the existing descriptor round-trip tests (100 pass). 1 new localized string (en+ja). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reserve the failure-rail gutter on every row (leading padding 8 always) so a failed command's content does not shift 8pt right relative to successful rows in the scrolling log. The red rail now sits in the reserved gutter when failed. (deferred cosmetic item from the UI review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per the architecture review, the iOS side carries already-parsed TerminalCommandBlock deltas through the EXISTING ChatEventSource/store seam (additive, gated on descriptor.kind == .terminal; the agent path is untouched). - TerminalCommandBlock: + Codable (snake_case exit_code/is_running/is_interactive). - ChatSessionEvent: + .terminalBlocks([TerminalCommandBlock]) (wire 'terminal_blocks'/'blocks'); receivers upsert by id; whole-block values are idempotent on reconnect replay. - ChatHistoryPage: + optional terminalBlocks (additive; agent payloads still decode). - ChatConversationStore: terminal mode — terminalBlocks/order, kind-branched reproject() (flat log, bypasses the bubble-grouping projector), .terminalBlocks upsert arm, history/resync seeding. Agent reproject/apply paths verbatim. - FixtureChatEventSource: terminal backlog init + emitTerminalBlocks + terminal history. - TerminalLogDemoScreen: now a real ChatScreen over a terminal fixture store, so the full path (history -> rows -> log + terminal composer + header) is sim-verified end to end. 6 new tests (wire round-trips incl agent backward-compat; store history-seeds-rows and upsert-by-id). 106 tests pass. Only the Mac source (OSC133 injection + PTY parse + serve) remains for #5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two real bugs in the new terminal store mode, found by review: - reset left stale blocks: apply(.reset) cleared messages but not terminalBlocks/terminalBlockOrder, so the synchronous terminal reproject() re-rendered old blocks (and would persist permanently if the post-reset history fetch failed). Clear them synchronously, mirroring messages = []. - terminal sends were invisible and leaked: the terminal reproject() ignored pending, so an optimistic send showed no row (and no failure/retry), and it never reconciled because the echo arrives as a command block, not a message. Now the terminal log includes pending rows, and reconcileTerminalPending clears a pending once its command echoes back as a block. Failed pendings keep their retry row. FixtureChatEventSource.send is a no-op in terminal mode (real echoes are blocks). 2 regression tests (reset clears rows; send shows pending then reconciles on the matching block). 108 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…plemented) A terminal session can't page older blocks yet (loadOlder pages messages by seq, terminal history carries blocks). Force hasMoreHistory = false when seeding a terminal page in both loadInitialHistoryIfNeeded and resyncTail, so the top sentinel never fires and the UI can't flip into a wrong 'earlier history on your Mac' truncated state if a producer returns hasMore. Fixture gains terminalHasMore to exercise it. 109 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Perf review: flushOpenOutput folded the ENTIRE accumulated output buffer on
every consume() call, so a long command streamed over many chunks was O(total)
per chunk = quadratic in total output — on the Mac PTY-drain path, the streaming
killer.
Now the parser keeps foldedOutput (completed lines, already folded) + openLine
(current line); only the open line is folded per chunk, so consume() is
O(chunk) and a whole command is O(n). foldLine folds one line and drops a single
trailing CR (the CRLF whose newline is the terminator), which also fixes a CRLF
SPLIT ACROSS consume chunks ('ab\r' then '\ncd') that the whole-buffer fold had
masked. 2 new tests (split-CRLF across chunks; byte-at-a-time streaming);
existing CRLF/progress/streaming tests still green. 111 tests.
Follow-up (with the Mac stream wiring): cap retained per-block output for
infinite producers (memory + per-tick wire payload).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ines) Two low-risk items from the perf review: - TerminalCommandBlock declares LAST so the synthesized Equatable compares the cheap scalar fields (id/exitCode/isRunning/isInteractive) before the full output string. A streaming block diffs against its prior self every tick across all rows, so checking scalars first avoids a per-tick O(output) string compare for unchanged blocks. - TerminalCommandBlockView splits block.output once per render (a hoisted ) instead of recomputing the computed 5+ times per body pass; outputBlock/collapsedOutput now take the lines. No behavior change; 111 tests pass. Remaining perf follow-ups (with the Mac stream): per-block output cap; reproject single-row mutation on upsert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Plain terminal tabs (and agent sessions whose hooks did not fire) now get the chat toggle, not just hook-registered agent sessions. - TerminalController+MobileChat: terminalChatDescriptors enumerates every open terminal surface across all main windows (kind: .terminal, id = surface UUID); v2MobileChatSessions merges them with agent sessions (deduped by surface). terminalChatHistoryPage parses the surface's MobileTerminalByteTee replay ring with OSC133CommandParser into a command-block page; v2MobileChatHistory routes terminal surface ids there. No god-file edits (uses workspace.panels + AppDelegate window enumeration), so the build stays incremental. - ChatSessionDescriptor: custom Codable so kind travels on the wire (decodes .agent when absent for back-compat). Without this, terminal descriptors arrived as .agent on the phone (wrong composer, bubble rendering). Verified end-to-end on the simulator against a plain terminal tab: toggle appears, the command log renders from real PTY output (commands + exit status), and the terminal composer shows. 112 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ss toast
Send error: v2MobileChatSend/interrupt/answer resolved the terminal via the
agent record, which a terminal session has none of, so they returned the
'agent terminal moved' error. mobileChatTerminalParams now resolves a terminal
surface directly (sessionID is the surface UUID -> its workspace via
terminalSurfaceWorkspaceID) and returns {workspace_id, surface_id}, so the
command injects through the same v2MobileTerminalPaste path normal terminal
input uses. isKnownTerminalSurface reuses the same lookup.
Error toast can now be swiped up to dismiss (DragGesture, animates out via the
existing move(edge:.top) transition), in addition to tap and auto-dismiss.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… polish - #4 (toggle for agents launched mid-session): the Mac now adopts a detected agent the instant its terminal title becomes the agent's (AgentChatTranscriptService observes .ghosttyDidSetTitle and calls TerminalController.adoptDetectedAgentSessions), so the session registers and pushes to the phone live, not only on next open. - #1 (scroll-to-bottom no longer dismisses keyboard): the dismiss tap excludes the button's frame (ChatScrollButtonFramePreferenceKey + excludedRegion). - #3 (smooth scroll): the button does a single animated proxy.scrollTo to the bottom anchor instead of stacked non-animated jumps. - #5 (toggle eases in): the session-list update is wrapped in withAnimation. - #2 (cramped grouping): intraGroupSpacing 2 -> 5 so a code block isn't flush against the next message bubble. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Testing