feat(chat): adaptive annotation pill for the Quote action - #7336
feat(chat): adaptive annotation pill for the Quote action#7336solnikhil wants to merge 1 commit into
Conversation
github-actions
Bot
added
merge conflict
bolichen97
added
needs-pr-triage
|
🤖 Kiro Crew [operator: bolichen97#66809557]: This PR has been inactive for 7+ days with a merge conflict against main. I've assessed the blockers and they appear resolvable — I'll push fixes directly to this branch as a co-author. Assessment: The branch is behind main and conflicts in three files: If you'd prefer I don't touch this PR, add the |
Quote injected raw '> markdown' straight into the composer. That text had no boundary the user could not half-delete, carried no attribution, and merged several quotes into one run-on blockquote with no way to drop just one.
Stage each selection as a structured quote instead, drawn as an adaptive pill above the composer: role plus excerpt at one quote, count plus distinct roles beyond that. Click pins a popover offering per-row remove, clear all, and jump to source; Escape closes it and returns focus to the pill.
Quotes are per chat slot, survive a slot switch, and serialize on the wire as attributed blockquotes ('> [Role - time] text'), so the agent contract is unchanged. They clear atomically with the composer they annotated and are restored into their originating slot when a send is refused or steering is rejected. A quote-only composer is sendable.
Rebased onto main and reconciled by Kiro Crew (operator bolichen97): merge conflicts in ChatPage.tsx, AssistantMessage.tsx resolved keeping main's structure; en-XA.json regenerated; an unused eslint-disable directive removed. Original feature authored by Nikhil Solanki.
Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io>
b8c6aa4 to
0268caa
Compare
|
🤖 Kiro Crew [operator: bolichen97#66809557]: Rebased this branch onto current main and pushed the reconciled head
No design, approach, or scope changes — the feature is exactly as @solnikhil authored it. Local gates before push: |
github-actions
Bot
added
readiness: action required
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of Design-Verdict: PASS Right seam (the established staged-chip pattern), unchanged wire contract, and failure/slot lifecycle mirroring the session-ref machinery — a sound, proportionate design. Suggestions
[DESIGN-REVIEWED] 0268caa |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All evidence gathered. The one raw- First-Principles-Verdict: CONCERNS Every core item earns its place, but the change orphans What this change shipsIntent: make a quoted selection a bounded, attributed, individually removable object instead of raw editable markdown — an ADDITION (declared
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 0268caa |
UX Review (Fable 5, fork) — 🟡 CONCERNSUX-level review of All evidence is gathered. The feature itself is well-built (staged pill matches the existing strip pattern, keyboard/a11y contract is solid, quote-only sends and failure restores are handled), but two error paths added by this diff carry real UX risk. Final review: UX-Verdict: CONCERNS Solid staged-quote flow, but a failed jump speaks in the Pins feature's voice, and a failed steer restores only half the user's message. Watch
Suggestions
[UX-REVIEWED] 0268caa |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. The sole candidate — a staged-quote key collision from [OPUS-REVIEWED] 0268caa |
GPT 5.6 Review (fork) — 🔴 changes requested (blocking)Reviewed BLOCKING -- website/src/utils/quoteRefs.ts:35 -- Fixed fence corrupts quoted Markdown examples |
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
github-actions
Bot
added
the
merge conflict
Problem / Motivation
Selecting assistant text and choosing Quote injected raw
> markdownstraight into the composer. That leaves the user editing a quotation as if it
were their own prose: the
>prefix has no boundary, so a stray backspacesilently turns the quote into ordinary text; it carries no attribution, so
neither the reader nor the agent can tell who said it or when; and quoting a
second passage appends into the same run-on blockquote with no way to drop just
one of them.
Why it matters
Quote is the cheapest way to make a follow-up unambiguous — "this line, not the
whole answer" — so its output is exactly what the next turn is grounded in.
Today that grounding is fragile: the reference is indistinguishable from the
instruction, half-deletions are invisible, and multi-quote replies degrade into
one merged block. Users respond by re-typing the passage by hand, which
defeats the action.
What changed (motivation → approach → change)
Goal — make a quote a first-class staged object with a boundary, provenance,
and individual removal, without changing what the agent receives.
Approach — the composer is a plain
<textarea>that renderstext/plainonly, so true inline chips would need a mirrored overlay or a contenteditable
swap (the app's only composer — an outsized blast radius). The chip pattern
already established for staged session refs and file attachments is the right
seam, so quotes are staged above the composer and folded into the payload at
send time, exactly like
appendSessionRefLinksdoes for session refs.What was built — a
QuoteAnnotationPillwhose resting label is adaptiverather than a bare count: at one quote it shows the role plus a ~50-char excerpt
(the common case, where "1 quote" would hide the only thing worth confirming),
and at two or more it collapses to a count plus the distinct roles. Hover opens
the disclosure transiently; click, tap, Enter and Space pin it — the path
that works on touch and with a keyboard — exposing per-row remove, clear all,
and jump to source. Escape closes and returns focus to the pill.
On the wire a quote serializes as an attributed blockquote,
> [Role · time] text— deliberately the same>form the agent alreadyparses, just carrying provenance, so this is a presentation upgrade with zero
agent-side change. Code selections are wrapped in a blockquoted fence so
indentation survives. Display truncation never reaches the wire: the full text
is always sent.
Lifecycle details that took most of the care: quotes are per chat slot and
survive a slot switch; they clear atomically with the composer they annotated;
and they are merged back into their originating slot when a send is refused
or a mid-turn steer is rejected, so a rejected request never silently discards
the reference. A quote-only composer (no typed prose) is sendable.
Tests
website/src/test/quoteRefs.test.ts— the wire contract: multi-line prose asone attributed blockquote, code indentation preserved inside a blockquoted
fence, empty collection serializes to nothing, and typed text is preserved
byte-for-byte when no quote is staged.
website/src/test/QuoteAnnotationPill.test.tsx— adaptive label, per-rowremove by stable key, clear all, jump-to-source passing the right quote, the
Escape and second-activation dismissal contracts, and that an emptied
collection does not retain pinned-open state.
website/src/utils/chatQuoteDrafts.test.ts— per-slot isolation, snapshotindependence from later mutation, malformed-record rejection, and the
merge-behind-newer ordering used by send recovery.
website/src/test/ChatInput.quoteAnnotation.test.tsx— a quote-only composerenables Send, and the measured strip height is added to the composer floor.
website/src/test/ChatPageMoreCoverage.test.tsx— staging instead of composerinjection, stacking, per-slot ownership across switches, quote-only send then
clear, restore on a rejected transport, steer serialization and consumption,
and merge-behind-newer on a rejected steer.
Three pre-existing specs asserted
toHaveBeenCalledWith(text, rect)againstthe
SelectionToolbarcallback this PR widens to carry an optional sourcedescriptor. They now read
mock.callsso they are arity-tolerant, rather thanthe component contorting to preserve the old arity.
Manual verification
Driven in a real browser against the dashboard, and it caught a defect no unit
test could: the popover was originally an
absolute bottom-fullchild ofChatInput'sinput-wrapper, which ownsoverflow-hiddenfor its roundedframe and drag-resize. The panel opens above the strip, so it was clipped to
zero visible pixels while remaining present and focusable — every assertion
passed because jsdom performs no layout. Measured
popover.top = 632againstwrapper.top = 724withoverflow: hiddento confirm it, then moved thepopover into a
createPortalwith a viewport-anchored fixed position (thepattern
SelectionToolbaralready uses here), re-measured on open, resize andscroll, and re-armed the hover handlers on the portaled node so the pointer
crossing into it no longer dismisses it. Re-captured to confirm it renders.
The composer value was read directly at each step, which is the sharpest
statement of the change: on
mainit ends up holding"> Once it is exhausted … \n\n", and on this branch it is""with the quotestaged as a pill instead.
Screenshots / video
Before — Quote injects raw
> markdownas editable text, no attribution:After — the quote is staged as an adaptive pill; the composer stays empty:
Pinned popover — row attribution (jump to source), quote text, per-row remove
Related Issues
Fixes #6983
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)