DRAFT / INCOMPLETE: "Ask Claude about this" native reference UX - #263
Draft
itsdestin wants to merge 27 commits into
Draft
DRAFT / INCOMPLETE: "Ask Claude about this" native reference UX#263itsdestin wants to merge 27 commits into
itsdestin wants to merge 27 commits into
Conversation
Holds the 'Ask Claude about this' reference as state instead of composer text. Parked per session like InputBar's draftsRef so a reference cannot leak from one conversation into another's next message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Inverts v1's askAboutThis()/scaffold() — same prompt strings, returned as data rather than dispatched at the composer as text. Tags the source element and (where possible) the selected runs with marker attributes so the overlay can re-measure geometry later without storing stale DOMRects. describeArtifactSelection moves to build-reference.ts as module-private (not exported, to avoid a circular import once Task 3 has build-menu.ts import the builders). build-menu.ts's artifactMenu keeps a temporary pure duplicate (describeArtifactSelectionForAskMenu) rather than importing buildArtifactReference back, because that builder also tags DOM markers for the future overlay — calling it from every right-click, not just an actual "Ask" click, would give the live menu a new DOM-mutation side effect before Task 3 is ready for it. Task 3 deletes the duplicate when it rewires the call site. Also fixes a bug in the buildChatReference draft: it fell back to only bubble?.textContent when locating the quote, so a null bubble (target has no bubble ancestor) always produced an empty quote even when target itself had text — inconsistent with the `host = bubble ?? target` fallback used two lines later for anchoring. Now uses the same bubble-or-target fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviewer caught a design defect in the PendingReference anchor: tagHost()/ tagSelectionRuns() set a data-reference-host attribute and wrapped the selection in a marker <span> via Range.surroundContents(). Chat bubbles render their text as plain React-managed JSX, so surroundContents() splits a text node out from under React's fiber, and the next reconcile throws NotFoundError: Failed to execute 'removeChild', crashing the chat view. Repeated right-clicks also nested marker spans with no cleanup. Anchor now holds a live host Element + a cloned Range instead of CSS selectors, so building a reference never mutates the DOM. Safe because this state is renderer-local and never serialized/persisted/sent over IPC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uildArtifactReference
Code review flagged that only buildChatReference had a regression test proving
the old DOM-mutating implementation (setAttribute + Range.surroundContents)
wasn't reintroduced. The other two builders had no equivalent guard, so a
reintroduced mutation would pass every existing test for them.
Verified by temporarily reintroducing setAttribute('data-reference-host', ...)
into both builders: both new assertions failed as expected, then reverted.
…ng turns buildContextMenu now takes an onReference callback instead of dispatching youcoded:compose-insert. Ask about this is DISABLED (not hidden) on the in-flight turn, with a title hint explaining why — the reference card is a static clone and would freeze a streaming message mid-sentence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SESSION_PROCESS_EXITED and NATIVE_SESSION_ERROR call endTurn(session), flipping isThinking false without replacing the turn object in session.assistantTurns (unlike TRANSCRIPT_TURN_COMPLETE / TRANSCRIPT_INTERRUPT, which both create a new turn reference). For a text-only turn the memo comparator's group-ID loop never runs, so prev.turn === next.turn let a streaming true->false change through undetected, leaving data-streaming stuck at "true" and "Ask about this" permanently disabled after a crash mid-response. Add streaming to assistantTurnPropsAreEqual, and a real-React-render test that keeps the same turn reference across renders to prove the memo actually re-renders when only streaming flips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The composer now holds ONLY the user's words: placeholderFor() announces the held reference, composeOutgoing() prepends its promptText at send, and the youcoded:compose-insert CustomEvent is retired (0 producers, 0 consumers). Feature is functionally complete here; Tasks 5-9 add the visual layer. Also updates InputBar.test.tsx: InputBar now calls useReference() unconditionally, so every render site needs a ReferenceProvider ancestor to avoid throwing (matches how App.tsx scopes the real provider by sessionId). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gap 1: a failed async native send restored the draft but not the held reference (clearReference() had already run synchronously before the ack settled). Restore both together now, guarded against clobbering a newer reference set during the round-trip. Gap 2: minimal (terminal view) send paths write straight to the PTY and never consume a held reference, so it stayed parked invisibly if the user switched from chat to terminal view. placeholderFor no longer announces a reference in minimal mode, and an effect clears it outright rather than attempting to prepend the scaffold to a PTY write (which desktop/CLAUDE.md documents as unsafe under ConPTY chunking). Widened ReferenceApi.setReference to also accept a (prev) => next updater, needed for the gap-1 guarded restore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s-session reference leak Critical: BuddyChatApp/BuddyOverlayApp mount InputBar under ThemeProvider > ChatProvider with no ReferenceProvider ancestor, so useReference() threw on every render and the Buddy companion window rendered blank. It now soft-fails to an inert API, matching the useEscClose convention. Important: a native-send failure restored a held reference via setReference((cur) => cur ?? reference) unconditionally. If the user switched sessions while the send was in flight, this wrote the old session's reference into the new session's live slot on the single app-wide ReferenceProvider. Guarded on session identity via a live activeSessionIdRef. Adds the first tests under src/renderer/components/buddy/, plus a session-switch regression test in InputBar.test.tsx. Each test verified to fail against the unfixed code (see task-4-report.md). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
buildUnionPath walks down the right edges of every line box then back up the left, giving a partial selection its real notched shape instead of a bounding box. Pure so the trickiest logic in the feature is testable without a DOM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reuses the L2 band rather than inventing an L5, with REFERENCE_COMPOSER_Z exported from Overlay.tsx so design rule 11 holds. Opening any overlay on top cancels the reference (new useEscStackDepth), which makes the two states mutually exclusive and sidesteps the z-ordering question entirely. Fixes an off-by-one in the depth-baseline capture: useEscStackDepth() reads the stack BEFORE this component's own useEscClose registers, so comparing later (post-registration) depth against that pre-registration baseline made the overlay cancel itself the instant it opened. Verified with a throwaway scratch test against the original formula before landing the +1 fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…review Finding 1 (Critical): InputBar.tsx's inline z-index on .input-bar-container could never beat the reference scrim — it's a descendant of .bottom-float, which already forms its own stacking context (position:absolute + z-index + transform/will-change), so a descendant's z-index only orders it against siblings inside that context, never against .bottom-float itself. Fix lifts the actual stacking-context-forming ancestor instead: globals.css now raises .bottom-float's z-index via var(--reference-composer-z) whenever body[data-reference-held] is set, and ReferenceOverlay.tsx publishes that var (Overlay.tsx stays the sole source of the layer number, design rule 11). Finding 2: the data-reference-held attribute is now actually consumed by CSS (was dead code before). Finding 3: documented the same-commit useEscClose registration race as an accepted, deliberate behavior (any contention for the L2 band yields — safer than a rewrite of the shared, app-wide Esc stack) and pinned it with a test that reproduces the exact race via React 18 batching. Finding 4: added tests for the attribute/CSS-var contract and a source-text guard on the consuming CSS rule. Real paint order still needs a dev-instance visual check (not provable in jsdom, and out of scope per task constraints). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Traces the real selection when there is one and the whole host element when there isn't (Destin's 9B call). Geometry is re-derived from the live DOM on scroll/resize rather than snapshotted, because stored rects go stale the instant anything moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clones the source and transforms it from its real rect to the viewport centre. A clone rather than a scroll because the newest message — the most likely right-click target — sits directly above the composer with no scroll room and can never reach centre by scrolling. Artifact references don't travel; the clone stays pinned over the source and is clipped to the selection instead, so only the referenced lines read at full --fg above the dim. Fixed a real coordinate bug along the way: clip-path: path() resolves against the clipped element's OWN border box, not the viewport, so the viewport-relative outline path (`d`) must be shifted by the source's own rect before use (new `shiftPath` helper in reference-geometry.ts) — using it unshifted, as the task brief's literal snippet did, would have clipped the wrong region whenever the source isn't pinned at the viewport origin. Also deletes `rects` from useReferenceGeometry's return value: nothing outside the hook's own tests ever consumed it once the artifact case switched from redrawing selected runs to clipping the clone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The FLIP positioning effect depended on `d` (the traced-outline path, recomputed on every scroll/resize), but travelling chat references never read `d` at all -- only the artifact clip-path branch does. Sharing one effect meant scrolling mid-travel reset `transform` back to the source position and re-scheduled the RAF, visibly restarting the 460ms lift. Split into two effects: the travel FLIP now keys on `[reference, travels]` only, so it runs exactly once per reference; the artifact clip-path keeps `[reference, travels, d]` so it still tracks the selection as the page scrolls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eNode comment The Cancel button's wrapper div is a sibling of .reference-lift-card inside .reference-lift, which sets pointer-events: none. Only .reference-lift-card had it restored, so the button was never a real hit-test target — clicks appeared to work only because they fell through to the full-viewport scrim behind it. Add pointer-events-auto to both the travelling and non-travelling wrapper (same idiom as Toast.tsx's action slot). Also corrects a WHY comment on the cloneNode(true) call that incorrectly claimed canvas/scroll state survives cloning — it doesn't (cloneNode copies DOM attributes only, not drawn bitmaps or scrollTop/scrollLeft). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Outline only — no trace animation, breathing pulse, glow, or travel easing. Stamps data-reduced on .reference-trace/.reference-lift from useTheme()'s reducedEffects (there's no data-reduced-effects attribute on <html> to key CSS off directly). Corrected the plan brief's stale `.reference-lift[data- reduced="true"] > *` selector (predates Task 8's clone-markup split) to target .reference-lift-card, and scoped the lift-shadow override to the travelling case only so it doesn't clobber the artifact clone's unconditional box-shadow: none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…view Issues A/B — a detached source positioned the card wrongly. Both the FLIP travel effect and the artifact clip effect called getBoundingClientRect() on anchor.host unconditionally. When the host is gone (session switched and back, or the file tab closed) that returns an all-zero rect, so the chat card landed near the top-left corner and the artifact clone was pinned at (0,0) fully unclipped. Both now detect !host.isConnected and render the non-anchored centred card the spec's section 7 already specified, with no animation from a meaningless origin. Issue C — "right-click again to replace the held reference" (spec section 7) was dead for chat references. The window-wide scrim intercepts the click, so buildContextMenu bailed at its .chat-scroll ancestry gate. Artifact references only worked by accident, because cloneNode copies data-artifact-viewer onto the clone and that branch is checked before the .chat-scroll gate. ContextMenuHost now resolves the true element under the pointer via elementsFromPoint when the raw target is the scrim, so replacement works uniformly. Left-click on the scrim still cancels; the transcript stays dimmed. Also: object-wrapper workaround for the TS 5.9.3 never-narrowing quirk in the new test, and the elementsFromPoint stub is now restored after each test rather than left on the shared document. Both detached-source tests verified RED against the unfixed overlay. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rapper Three defects from Destin's dev-instance review. 1. "Black box around where the message bubble was originally." The trace SVG draws `d`, measured from anchor.host at its ORIGINAL position — but a chat reference's card flies to the viewport centre, so the outline stayed behind and boxed the empty space the bubble vacated. On a dark-accent theme that reads as a hard black rectangle. The source-anchored trace is now rendered only for NON-travelling (artifact) references, where nothing moves and the outline is exactly right. The travelling card carries its own ring + glow in CSS, so the highlight hugs the card and travels with it by construction. 2. "Weird extra clear space to the right of the centered message bubble." Chat bubbles carry `max-w-[85%]`. The lift wrapper is sized to the bubble's own measured rect, so that percentage resolved a SECOND time against it, leaving the card 15% narrower than its wrapper — dead space on the right, with the cancel button pinned out in it. The clone now fills the wrapper. 3. The ring sits on the clone rather than the holder, so it follows the bubble's own border-radius instead of boxing a rounded card in a square. Reduced-effects keeps a plain ring and drops the glow + amplified shadow. Task 9's data-reduced tests now cover both kinds, since the trace only exists for one of them. New regression test verified RED against the old behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.bubble-timestamp renders INSIDE the bubble div (UserMessage.tsx:75, AssistantTurnBubble.tsx:437), so reading textContent swept it into the quote — Destin's dev review caught a scaffold ending `...ready to use or delete as needed.12:55 AM"`. Strips chrome from a CLONE, never the live node: the reference path must not mutate React-managed DOM. Pinned by a test asserting outerHTML is unchanged alongside the content assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem 1: InputBar dispatched the optimistic USER_PROMPT/QUEUED_MESSAGE_ADDED with the user's raw draft while the actual sent text carried the reference scaffold prepended. chat-reducer's TRANSCRIPT_USER_MESSAGE dedup matches on exact content, so the mismatch meant no pending entry was ever found and the transcript event appended a second bubble. Fix: dispatch outgoing.content (the exact string sent) at all three call sites instead of the draft-only bubbleMessage. Problem 2: since the timeline entry now legitimately holds the scaffold, UserMessage rendered it as raw boilerplate text. Added context-menu/reference-prompt.ts as the single source of truth for the scaffold format (lead-ins, follow-up marker, buildScaffold/ buildArtifactScaffold, and a whitespace-tolerant parseReferencePrompt — it has to tolerate InputBar's newline-flattening sanitize, not just the builder's own multi-line output). build-reference.ts now calls the shared builder instead of holding its own copy. UserMessage renders a quoted reply strip (collapsed past ~3 lines/~240 chars, via a real <Button> toggle) for chat-text/chat-code references and a compact descriptor line for artifact references, falling through to the existing plain-text path otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…chip Two dev-review defects. 1. Weird corner artifacts around the lifted card. box-shadow paints OUTSIDE the border box and is clipped by an ANCESTOR's overflow — .reference-lift-card carried max-height/overflow-y, which sliced the clone's ring off everywhere except where it bled past the corners. The scroll clamp moves onto the clone itself; an element's own overflow never clips its own shadow. 2. The cancel X was the default ghost Button — transparent, so it disappeared against a wallpaper theme. Now a solid circular chip on bg-panel/border-edge (theme tokens, no literals). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…panel
Destin picked options B+D from the dev-review mockup. They compose into one
control rather than two: D's pill IS the collapsed state, B's labelled panel
IS the expanded one.
Collapsed by default and unconditionally — not length-gated — so the bubble
stays roughly the size of what the user actually typed and the reference never
competes with their own words. Clicking the pill reveals the full quote in a
tinted panel headed 'Claude said', with a Hide control to collapse again.
An artifact reference renders as the same pill shape but static: it is already
a short descriptor ('lines 12-14 of chat-reducer.ts'), not a quoted body, so
there is nothing to expand.
Removes the now-dead JS clamp helpers (isLongQuote/clampQuote/COLLAPSE_*) —
the pill ellipsises in CSS, so nothing truncates in JS any more.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…traced outline Dev-instance review flagged four defects in the held reference feature: source and travelling clone both visible at once, no indication of which part of a message was selected, a "weird black box" around artifact selections, and an "uneven and janky" traced outline. - Hide the source (visibility:hidden, not display:none) for the duration a CHAT reference is held, so the clone reads as the bubble having moved rather than a second copy. Restored to its exact prior inline state (including dropping a leftover empty style attribute) on clear. - Capture the selection's character offsets at build time (computeSelectionOffsets) and re-apply them as a <mark class="reference- mark"> inside the detached clone (applyHighlightMark), for both chat and artifact references. - Delete the traced SVG outline entirely for both kinds. The artifact clip-path (and its underlying geometry hook) stays — it never rendered through the SVG, just shared its path data — along with the dead CSS (keyframes, --ref-wash, reduced-motion branch) that only the SVG used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ble artifact selections Dev-review follow-up on the held-reference feature: - Cancelling (Esc, scrim click, x button) now flies the clone back to the source's current rect before actually clearing the reference, mirroring the entry FLIP's timing/easing. The source stays hidden until the flight lands, so the "two copies visible" bug this feature already fixed once can't come back on the way out. Sending is untouched — InputBar's send() still calls clearReference() directly, so it clears immediately with no animation; the call SITE is what distinguishes cancel from send, not a flag. Detached sources, artifact references (which never travelled), reducedEffects, and prefers-reduced-motion all skip straight to an immediate clear. - Artifact selections now get a visible ring (theme-token box-shadow), not just a translucent background tint layered on a clip-path region that was already the bright part of the screen. Investigated first: the offsets and the mark were already reaching the artifact clone correctly (the offset capture and the clip-path share the same captured Range) — the gap was purely that the mark had no edge of its own to read as "selected" once it sat inside an already-undimmed area. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… clone Commit 2a89452 deleted the outline entirely after dev review flagged it as "uneven and janky" with a "weird black box" around artifact selections. Two different bugs were tangled together there and deletion papered over both: - Geometry: the union path was raw, unsnapped, unmerged client rects joined with hard 90 degree steps. reference-geometry.ts now snaps every edge to a whole pixel, merges near-identical adjacent line-box edges (2px tolerance), drops near-zero boxes, and rounds every corner (quadratic, clamped per vertex to half the shorter adjacent edge) via a new buildRoundedOutlinePath. - Anchor: the outline used to measure the SOURCE's position, so a travelling chat reference painted a box around the empty space the bubble left behind — worked around at the time by just never rendering it for that case. useReferenceGeometry now measures the `.reference-mark` elements inside the CLONE instead, which is wherever the highlight actually is for both a travelling and a pinned clone, by construction. A transitionrun/end-driven rAF loop keeps it tracking during the FLIP travel without ever touching transform/left/top itself, so it can't restart the travel the way sharing an effect with `d` used to. .reference-mark's inset ring (added as a stand-in while there was no outline) is now a literal duplicate of the restored outline and is removed; the background tint stays as a distinct signal. The travelling card's own ring+glow is kept — it answers a different question ("this is the referenced message" vs. the outline's "this is the selected span within it"). Full reasoning, tolerances, and verification output in .superpowers/sdd/outline-restore-report.md (gitignored, local only). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A selection that crosses a syntax-highlighting token boundary makes apply-highlight.ts emit one <mark> per covered text node, so a single visual line can hand back more than one client rect. toBoxes fed every rect straight to buildRoundedOutlinePath's down-right/up-left traversal, which only forms a valid simple polygon with at most one box per line -- feeding it 2 boxes on one line produced a self-intersecting path whose fill bled into a bounding-box-shaped region behind the correctly-tight mark highlights (Destin's 2026-07-28 report: outline enclosing text that was never selected, alongside what looked like a second, larger box). toBoxes now groups rects landing on the same line (by raw vertical midpoint, pre-padding) and unions each group into one box first. Co-Authored-By: Claude Opus 5 (1M context) <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.
Replaces the v1 "Ask about this" behaviour (shipped #169), which pasted a raw prompt
scaffold into the composer, with a held reference kept as state.
youcoded-dev/docs/active/specs/2026-07-26-ask-claude-reference-ux-design.mdyoucoded-dev/docs/active/plans/2026-07-26-ask-claude-reference-ux.mdbed5f7ea(design),d8a70e4e(inline-reply options)What works
The composer holds only the user's own words; the scaffold is assembled at send time and
rendered back as a collapsible reply (a pill that expands into a labelled panel). The
window dims behind a scrim, the referenced chat message clones and FLIP-animates to the
viewport centre and back out on cancel, the source hides so it reads as one bubble moving,
and the selection is marked inside the clone. Artifact selections stay in place, clipped so
only the referenced lines stay bright.
3551 tests passing,
tsc --noEmitclean. ~50 tests are new and specific to this work.Honest status — why this is a draft
The state, builder, menu wiring, and composer plumbing (roughly the first half) are
well-tested and settled. The visual overlay layer is not. It was reworked in response
to five separate rounds of dev-instance feedback, and each round found a real defect that
static analysis and jsdom could not catch:
max-w-[85%]and left dead space beside a "centred" cardoverflowclipped the card's ring into corner fragmentspointer-events: noneand only appeared to work becauseclicks fell through to the scrim
That cadence is the actual signal here: jsdom cannot verify any of this. Every remaining
risk in this PR is a paint-order, layout, or animation question, and the test suite is
structurally incapable of covering it. The tests pin mechanisms (attributes set, styles
written, path strings emitted), not appearance.
A reviewer should treat the overlay as a working prototype whose architecture is sound but
whose implementation has absorbed a lot of point fixes. Rebuilding it against the now
well-understood constraints may be cheaper than continuing to patch it.
Known-open items
defaults, never visually tuned.
not directly observed — the fix is correct for the general case regardless.
outgoing-message.tsflattens newlines before dispatch, so Claude receives a single-linescaffold. Pre-existing, not introduced here; the parser tolerates both forms.
and falls back to plain rendering.
setText/setAttachmentsrestores on the native-send-failure path share a cross-sessionrace that
setReferencewas explicitly guarded against.measure()in the geometry hook is unthrottled — a fast scroll drives many re-renders.clip-path.touch path at all, a pre-existing limitation.
Not to be lost if this is rewritten
Three constraints were learned the expensive way and are documented in code comments:
used
Range.surroundContents(), which splits text nodes; React's fiber keeps the stalereference and the next reconcile throws
NotFoundError: removeChild, taking down thechat view. Reversible inline style is fine — structure is not.
.bottom-floatforms a stacking context (position: absolute; z-index: 20plus atransform), so the composer cannot out-z-index a portalled scrim from inside it. It is
raised via a CSS variable published from
Overlay.tsx, which stays the single authorityon layer numbers.
chat-reducer.ts:331), so theoptimistic dispatch and what is actually sent must be the same string or the message
echoes as two bubbles.
🤖 Generated with Claude Code