feat(dashboard): add a chat outline tab to jump to any turn - #9029
feat(dashboard): add a chat outline tab to jump to any turn#9029chenmingwei23 wants to merge 1 commit into
Conversation
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS The outline projects only the loaded page of a paginated transcript, so the deep sessions that motivate the feature open with most turns missing and no hint of it. Watch
Suggestions
[DESIGN-REVIEWED] e833705 |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of I have everything needed: the blind read, all three screenshots, the diff, and the sibling patterns. Reconciliation done — the primary control (outline rows) was read correctly by the blind reader; the current-marker ambiguity and three unshown surfaces are the findings. UX-Verdict: CONCERNS Rows read correctly cold, but the "current" marker is visually identical to hover, and the tab, + menu entry, and empty state were never shown. Watch
Evidence gaps
[UX-REVIEWED] e833705 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All checks are complete. The feature is derived from issue #8221, reuses the existing generic jump ( First-Principles-Verdict: PASS A requested navigation aid (issue #8221) built by projecting existing state onto the existing jump path — nothing duplicated, nothing persisted, off by default. What this change shipsIntent: let a user jump to any earlier turn in a long session from a clickable list — an ADDITION.
Subtractions
[FIRST-PRINCIPLES-REVIEWED] e833705 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsDiscovery found no candidates, and my independent verification confirms the integration is sound: the outline selector guards on No findings. [OPUS-REVIEWED] e833705 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsFINDING -- website/src/pages/chat/ActivityViewer.tsx:900 -- cold-opening a session with over 100 messages leaves False positive or not applicable? A repository writer can comment: |
de2b6c0 to
efc2fae
Compare
efc2fae to
ffb96cd
Compare
ffb96cd to
e833705
Compare
bolichen97
left a comment
There was a problem hiding this comment.
Tech Lead review: not approving — the outline silently truncates in exactly the case that motivated it.
The outline tab projects only selectSlotMessages, which for a freshly opened slot holds a single 100-row page (OLDER_PAGE_LIMIT) and never consults slotHasMore. In the "hundreds of turns deep" session that motivated #8221, the tab will silently list only the newest ~100 turns — contradicting both the PR body's claim that outline rows are "all the session's user turns" and the menu string "Jump to any turn in this session." Please either page in the rest of the session or make the outline honestly reflect a windowed view.
Second issue: the new outline-jump caller reuses onJumpToPin without adding a PendingJumpOrigin entry, so an unresolvable outline click (e.g. into the untruncated tail) renders the "pinned message unavailable" copy — the exact drift the PendingJumpOrigin enum and its comment at ChatPage.tsx:7557 exist to prevent. Please add an outline-specific origin.
Third, UX review flagged that the current-row marker reuses the bg-bg-hover token, making hover visually indistinguishable from the active/selected row — please use a distinct token.
|
Closing without merging. The reasoning is recorded on the issue so it outlives this PR: Short version: @bolichen97's tech-lead review is right that the outline silently truncates That is a wrong premise rather than a fixable defect at this size, so the work is being Thanks for the review; it caught the thing that mattered. |
Problem / Motivation
In a long single session the scrollback becomes unwieldy: finding an earlier
message means scrolling and scanning by eye, which is slow and easy to lose your
place in. Issue #8221 asks for a chat outline / table-of-contents of the current
session's turns, each entry showing the truncated user prompt, so a click jumps
to and highlights that turn.
Why it matters to the user
A jump-to-turn outline makes navigating a long conversation fast and lets you
skim the session's structure at a glance, instead of hand-scrolling a transcript
that may be hundreds of turns deep. The reporter left the surface open to design
("side panel, popover, or minimap rail -- easily accessible").
Screenshots
The Outline tab, populated with the session's user turns:
After activating an entry, that row is marked as current (aria-current), so a
screen reader conveys the user's place in the list:
Capture artifacts live in the repo's own pattern: the scene at
website/capture/chat-outline.html+website/capture/chat-outline.tsx(mounts the real
ChatOutlinePanel), driven bywebsite/scripts/capture-chat-outline.mjs, with the frames undertemp-screenshots/chat-outline/(a light-theme parity frame is there too).How the fix solves it (symptom -> root cause -> change)
The symptom is "no way to jump to a turn from a list." The transcript already had
the two hard parts:
meta.mid(ChatPage.tsxderives it per row at the render site; the chat slicedocuments
meta.midas message identity). No new data model or DOM attribute isneeded.
jumpToLoadedPinnedMessage(messageTs, mid?)resolves the row bymid(fallingback to
ts), maps it to the virtualized display index, callsnavToDisplayIndex(di, { behavior: 'smooth', align: 'center' }), and flags therow with
animate-msg-highlightfor 3s. It is passed down to the side panel asonJumpToPinand is not pin-specific.So the only missing piece is a list whose rows are all the session's user
turns rather than the pinned ones. This PR adds exactly that:
outline, following the existingpins/issuesshape (a
ViewKindreached from the+menu; deliberately NOT added toPINNED_VIEWS, so it never occupies the permanent pinned block). It defaultsoff -- nothing changes for anyone until they open it from the
+menu.ChatOutlinePanelwhose rows are projected from the redux transcript(
selectSlotMessages) filtered to user turns -- no server fetch, no persistence,no new store state. The projection is only computed while the Outline tab is shown.
onJumpToPin), so scrolling and the3s highlight are inherited rather than reinvented.
Accessibility: rows are real
<button>s inside a<ul>/<li>list, so theoutline is reachable and operable by keyboard by construction (Enter and Space
activate; Space is claimed so it does not scroll the list instead). The entry the
user last activated carries
aria-current="true".What tests we did
src/test/chat/ChatOutlinePanel.test.tsx(9 cases): the pure projection(
outlineEntrieskeeps user turns in order, drops a turn with neithermidnor
ts, keeps a legacyts-only turn;outlinePreviewcollapses whitespaceand truncates) and the rendered panel (empty state; one
<button>per turn;click and Space both jump with the exact
(ts, mid)argument; only theactivated row gets
aria-current; a mid-only turn passes('', mid)).unresolvable-turn guard, removing truncation, swapping the
(ts, mid)jumparguments, forcing
aria-currentalways-true, and removing the Space handlereach reddened a different, correct assertion; re-run on the merge with current
main and still red.
sidePanelAddMenu.test.tsxexpected menu-partition lists updated to includeoutline; the "partitions every catalogued view exactly once" invariant stillpasses.
docs/feature-map/README.md(verified
check_feature_map.pyfails without it and passes with it), whichcleared the GPT 5.6 blocking finding and the Feature Map Gate; translated the two
new multi-word catalog phrases across the flagged locales so
check-source-stringsand
labelKeyTablespass; and renamed the panel's test ids off theoutline-prefix (they were parsed by the phantom-classes gate as
outline-<color-token>utilities).
tsc -bclean for all touched files (the only 15 errors are a pre-existingstorybook baseline).
eslint srcclean. ExistingusePanelTabs,ActivityViewer,catalogParity,keyReference,changedValueQa,duplicateKeys,deadKeysall pass.
check-i18n-keysclean, no shadowing.merge-treeclean vs current main.Other suggestions
single-word "Outline" is left verbatim where locales share it (the gate accepts it).
now" tracking as the user scrolls (the current
aria-currenttracks the lastactivated entry); a minimap-style rail as an alternative surface; and persistence
of outline state across reloads. Each is independent of this change.
MarkdownToc.tsxis a precedent for a future scroll-synced rail but is an overlayrail bound to a scrollable viewport, not a drop-in for a side-panel tab.
Refs #8221