fix(mac): AppKit markdown emphasis/appearance and sidebar selection fixes - #441
Merged
Merged
Conversation
Two defects in the AppKit Markdown renderer's styling layer. Emphasis was never applied. MarkdownSemanticModel encodes bold/italic purely as `inlinePresentationIntent`, which SwiftUI's `Text` honors but TextKit ignores — `AppKitMarkdownStyle.attributed` only branched on `.code` and `link`, then back-filled every unstyled run with the plain body font. Every `**bold**` and `*italic*` span in the transcript, in table cells, and in list/quote/heading bodies rendered as regular 14pt text. Emphasis intents are now resolved into real symbolic traits and composed with the code font, so `***both***` and ``**bold `code`**`` render correctly. Code-block and table chrome froze dynamic system colors into CGColors in `init`. `NSColor.cgColor` resolves against the current *drawing* appearance, which outside a draw pass is the system appearance, not the view's — so on a Light Mode Mac the dark-pinned transcript drew white text on a white `textBackgroundColor` plate. Nothing implemented `updateLayer`/`viewDidChangeEffectiveAppearance` either, so a runtime theme switch never repaired the cached fragment views. Both views now re-resolve their layer colors in `updateLayer` under the view's own effective appearance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`AppKitMarkdownTableView.measuredHeight` ignored its width argument and returned `rowCount * 30 + 12`, but the cells it builds are NSTextViews with `widthTracksTextView` and only a minimum height constraint, so any cell whose text wraps grows past that budget. The transcript row was sized for the estimate and the lower table rows were clipped; because the same value is what the height cache stores, the clipping was stable rather than self-correcting. The laid-out document height is now authoritative, with the old fixed estimate kept as a floor so an empty or not-yet-laid-out table measures exactly as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two selection defects in the native outline. Delegated automation children are projected as `.automation(scope)` and never get a `.thread(scope)` node, but `synchronizeSelection` and the keyed differ's selection plan both looked the scope up as `.thread`. Clicking a nested "Agent: …" row opened the child thread and then immediately ran `deselectAll`, leaving the sidebar with no selected row and the differ unable to retain selection across moves. Scope-to-node resolution now goes through one place, `SidebarOutlineSnapshot .selectionNodeID(for:)`, which returns whichever node actually carries that scope as its thread action target. `outlineViewSelectionDidChange` only ignored notifications while `isApplyingSelection` was set, and that flag was set exclusively inside `synchronizeSelection`. AppKit moves or clears `selectedRow` while rows are removed inside `beginUpdates`/`endUpdates` and while a collapsing parent hides them, so a session that settled, snoozed, got archived, or fell out of a search query would post a selection notification mid-batch and the delegate would read it as user intent: it re-entrantly cleared the presentation selection and dispatched `navigation.clearSelection()`, emptying the transcript, or activated whatever neighbouring row AppKit had landed on. Controller-driven structural updates are now guarded, and `synchronizeSelection` runs after every apply as the sole authority. It clears the highlighted row only when the session selection actually became nil, so it never steals an outline selection it did not set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both menu paths anchored the transient NSPopover to the row's `SidebarRowView` — `presentMenu(forRow:)` via `view(atColumn:row:)` and the ellipsis button by passing the row view itself. A targeted `.reload(id)` calls `reloadItem`, which re-invokes `viewFor:item:` and swaps in a different `SidebarRowView`; a transient popover whose positioning view leaves the window is dismissed. Opening Snooze/Settle/ Pin on a running session therefore closed the menu on the next provider mutation for that thread, and even an idle row reconfigured within a minute when the visible-row clock refreshed its relative time. The controller now hands the presenter the outline view plus the row's rect in its coordinate space. The outline view outlives every cell-view replacement, so the menu stays open and keeps pointing at its row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The row is projected unselectable and unexpandable, so `shouldSelectItem` rejects it, it can never become `selectedRow` for `handleKeyboardCommand`, and its disclosure/menu buttons are hidden. The only remaining route to `revealAll` was `SidebarRowView.mouseUp` gated on a double click, which never arrives because NSTableView's own tracking loop consumes the click sequence for a row the row view does not handle. A project with more than five active sessions therefore hid the rest behind a row that did nothing at all. The outline view now routes a primary click on an unselectable action row to the controller, which activates it. Selectable rows keep AppKit's click-to-select behavior untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`SidebarOutlineSnapshot.nextUpdateAt` is the minimum over *materialized* nodes, and the controller's shared clock is fed from the visible subset of those. A collapsed Snoozed disclosure materializes no thread rows, so the wake time of everything inside it was absent from the snapshot: the visible-row clock was scheduled at nil, `refresh()` never ran, and a session snoozed behind the default-collapsed disclosure never woke until an unrelated topology change or an app restart forced a reprojection. The same gap hid auto-settle deadlines for rows past the "Show more" cap and for every row under a collapsed project. The collapsed disclosure, show-more, and project rows now carry the earliest classification deadline of the scopes they hide. Only classification deadlines count — relative-time labels of rows nobody can see must not wake the clock — so a collapsed row reconfigures once, when its hidden content actually changes bucket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`StreamingMarkdownCache.document` short-circuited on byte count plus a trailing 64-byte window. A same-length in-place edit that differed anywhere before those last 64 bytes therefore returned the previous document unchanged — the engine had already detected the source change, so the stale document became the snapshot for a bumped revision and the transcript kept rendering the old wording. The same window gated the append-only decision, so a longer non-append replacement whose bytes at [n-64, n) happened to match skipped `reset` and kept a permanently wrong settled prefix. The comparison is now exact. It is a linear byte scan of the accumulated message, which is far below the tail re-parse this same call gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`PlainStreamingState.append` tracked `tailCharacterCount + delta.count`, but grapheme counts are not additive across the join: a trailing `\r` meeting a leading `\n`, or a base letter meeting a combining mark (both admitted by the plain-stream predicate), collapse into one character. The tracked count drifted above the real length and was carried forward, while the split used `index(_:offsetBy:)` with no `limitedBy:` — enough accumulated drift traps with "String index is out of bounds" mid-stream, and below that threshold it mis-sizes the frozen fragment. The running total is now treated as the upper bound it is: `limitedBy` makes the split total, and every split resets the count to the truth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… settles `markdownFileTargets` and `markdownExternalLinks` each walk every block and every attributed run of the document and parse a `URL` per link run. Because the real streaming path always has `plainStreamingState == nil`, they ran on every provider delta over the entire accumulated message — quadratic main-actor work over a stream whose results nothing reads: no AppKit consumer touches `MarkdownDocumentSnapshot.fileTargets` or `.externalLinks` while a message is live. They are now resolved when the message settles, which `finish()` already reprojects with `isStreaming == false`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Post-Gate-F bug-fix batch for the AppKit renderer's Markdown and sidebar surfaces, from the 2026-08-02 bug hunt:
Deliberately skipped (need product decisions or their own pass): transcript context-menu restoration, syntax-highlight restoration in code blocks, plain-stream fast-path predicate widening, project identity stabilization. Tracked in the session's finding inventory.
Area
apps/mac— native macOS appapps/windows— Windows desktop appapps/mobile— iPhone companion appapps/server— backend serverRelease size
size:XSsize:Ssize:Msize:Lsize:XLVerification
pnpm run verifyafter the final edit (resolved to the Swift suite for this Swift-only diff): pass🤖 Generated with Claude Code