Skip to content

feat(dashboard): add a chat outline tab to jump to any turn - #9029

Closed
chenmingwei23 wants to merge 1 commit into
mainfrom
feat/chat-outline-panel-8221
Closed

feat(dashboard): add a chat outline tab to jump to any turn#9029
chenmingwei23 wants to merge 1 commit into
mainfrom
feat/chat-outline-panel-8221

Conversation

@chenmingwei23

@chenmingwei23 chenmingwei23 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

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:

Chat Outline tab populated with seven 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:

Chat Outline tab with the third entry marked as the current entry

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 by
website/scripts/capture-chat-outline.mjs, with the frames under
temp-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:

  • Every user turn already carries a stable identifier, the server-minted
    meta.mid (ChatPage.tsx derives it per row at the render site; the chat slice
    documents meta.mid as message identity). No new data model or DOM attribute is
    needed.
  • A scroll-to-turn-and-highlight path already exists and is generic:
    jumpToLoadedPinnedMessage(messageTs, mid?) resolves the row by mid (falling
    back to ts), maps it to the virtualized display index, calls
    navToDisplayIndex(di, { behavior: 'smooth', align: 'center' }), and flags the
    row with animate-msg-highlight for 3s. It is passed down to the side panel as
    onJumpToPin and 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:

  • A new non-pinned side-panel view outline, following the existing pins / issues
    shape (a ViewKind reached from the + menu; deliberately NOT added to
    PINNED_VIEWS, so it never occupies the permanent pinned block). It defaults
    off
    -- nothing changes for anyone until they open it from the + menu.
  • A new ChatOutlinePanel whose 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.
  • Each row reuses the existing generic jump (onJumpToPin), so scrolling and the
    3s highlight are inherited rather than reinvented.

Accessibility: rows are real <button>s inside a <ul>/<li> list, so the
outline 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

  • New src/test/chat/ChatOutlinePanel.test.tsx (9 cases): the pure projection
    (outlineEntries keeps user turns in order, drops a turn with neither mid
    nor ts, keeps a legacy ts-only turn; outlinePreview collapses whitespace
    and truncates) and the rendered panel (empty state; one <button> per turn;
    click and Space both jump with the exact (ts, mid) argument; only the
    activated row gets aria-current; a mid-only turn passes ('', mid)).
  • Mutation-verified by hand (5 mutations, each reverted): dropping the
    unresolvable-turn guard, removing truncation, swapping the (ts, mid) jump
    arguments, forcing aria-current always-true, and removing the Space handler
    each reddened a different, correct assertion; re-run on the merge with current
    main and still red.
  • sidePanelAddMenu.test.tsx expected menu-partition lists updated to include
    outline; the "partitions every catalogued view exactly once" invariant still
    passes.
  • Review-round fixes: added the "Chat outline" row to docs/feature-map/README.md
    (verified check_feature_map.py fails without it and passes with it), which
    cleared 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-strings
    and labelKeyTables pass; and renamed the panel's test ids off the outline-
    prefix (they were parsed by the phantom-classes gate as outline-<color-token>
    utilities).
  • tsc -b clean for all touched files (the only 15 errors are a pre-existing
    storybook baseline). eslint src clean. Existing usePanelTabs, ActivityViewer,
    catalogParity, keyReference, changedValueQa, duplicateKeys, deadKeys
    all pass. check-i18n-keys clean, no shadowing. merge-tree clean vs current main.

Other suggestions

  • The two new multi-word phrases are translated in every flagged locale;
    single-word "Outline" is left verbatim where locales share it (the gate accepts it).
  • Follow-on scope, deliberately not built here: live "which turn is on screen
    now" tracking as the user scrolls (the current aria-current tracks the last
    activated entry); a minimap-style rail as an alternative surface; and persistence
    of outline state across reloads. Each is independent of this change.
  • MarkdownToc.tsx is a precedent for a future scroll-synced rail but is an overlay
    rail bound to a scrollable viewport, not a drop-in for a side-panel tab.

Refs #8221

@chenmingwei23
chenmingwei23 requested a review from a team September 6, 2026 13:09
@chenmingwei23
chenmingwei23 requested a review from a team as a code owner September 6, 2026 13:09
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of e833705709d17f2d8ed0c66b01fd56f2c3d2f84f — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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

  • A slot opens with page one of history (OLDER_PAGE_LIMIT = 100 rows, "the first page a slot opens with is simply page one of the same pagination loadOlderMessages runs") and selectSlotMessages returns only what's loaded. For the issue's motivating case — a session "hundreds of turns deep" — the Outline tab therefore lists only the user turns among the newest ~100 rows; the older turns a table of contents exists to reach appear only after the user hand-scrolls to page them in, the very act the feature is meant to replace. The panel never consults slotHasMore, so the truncation is silent, and the PR's claim that "rows are all the session's user turns" doesn't hold in exactly this case. The pins panel avoids this because pins come from a server API and the jump path pages history in on demand.

Suggestions

  • Cheapest closure of the gap within this shape: when slotHasMore is true, render an "earlier turns not listed — load earlier history" row at the top of the outline (reusing loadOlderMessages), so the truncation is at least visible and actionable; a server-side turn projection can stay a follow-up.

[DESIGN-REVIEWED] e833705

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of e833705709d17f2d8ed0c66b01fd56f2c3d2f84f — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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

  • Current marker uses the hover token: isCurrent ? 'bg-bg-hover' is the same class every row gets on hover:bg-bg-hover, so the blind reader's "I can't tell hover from selected from a still picture" is true live too — mousing over any row makes it look current. Every use of the panel, mild persistent confusion. Give the current row a distinct treatment (accent left bar or selected-state token), not the hover fill.
  • Menu description "Jump to any turn in this session" over-promises: outlineEntries drops every non-user turn, so someone hunting an assistant reply opens the tab and finds only their own prompts; it also drifts against the empty state's "No messages to outline yet". Say "Jump to any of your messages in this session" and keep "messages" in both strings.

Evidence gaps

  • The "Outline" tab label/icon in the tab strip appears in no screenshot — all three crop to the bare list (blind reader: "no title, header… to say" what the list is); one frame with the panel chrome would close it.
  • The + menu showing "Outline — Jump to any turn in this session" (the discovery path) appears in no screenshot.
  • The empty state "No messages to outline yet" is introduced by the diff but shown in no screenshot.

[UX-REVIEWED] e833705

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of e833705709d17f2d8ed0c66b01fd56f2c3d2f84f — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are complete. The feature is derived from issue #8221, reuses the existing generic jump (jumpToLoadedPinnedMessage via onJumpToPin) rather than duplicating it, the transcript search is a meaningfully different mechanism (text match vs. structural TOC), the capture artifacts follow a 1000+-file repo convention, and the locale/feature-map edits are gate-mandated. The only unjustified surface is two zero-consumer micro-exports.

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 ships

Intent: let a user jump to any earlier turn in a long session from a clickable list — an ADDITION.

  1. "Outline" entry in the side panel's + menu, session-output group — justified (issue Chat outline panel to navigate to any turn in the current session #8221; off until opened)
  2. Outline tab lists the session's user turns, one trimmed line each — justified (client-only projection, no new store state)
  3. Click / Enter / Space on an entry scrolls to and highlights that turn — justified (reuses generic onJumpToPin, counted: same callback the pins tab uses)
  4. Last-activated entry carries aria-current — justified (declared accessibility contract, test-pinned)
  5. Empty state when the session has no turns — justified
  6. Feature-map README row — justified (check_feature_map.py gate mandates it)
  7. New labels across 13 locale files — justified (i18n gates mandate catalog parity)
  8. Capture scene + script + 3 committed frames — justified (repo's documented evidence pattern; 219 sibling scenes, 1053 committed frames)

Subtractions

  • Drop data-current on the row button (ChatOutlinePanel.tsx:135) — zero consumers (grepped data-current across website/: only aws-control's own page/test plus this defining site; this PR's tests and capture both assert aria-current instead).
  • Drop export type { ChatOutlinePanelProps } (ChatOutlinePanel.tsx:156) — zero consumers (3 grep hits, all in the defining file; the capture entry imports only OutlineEntry).

[FIRST-PRINCIPLES-REVIEWED] e833705

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed e833705709d17f2d8ed0c66b01fd56f2c3d2f84f — this comment is updated in place on each push.

Review details

Discovery found no candidates, and my independent verification confirms the integration is sound: the outline selector guards on view === 'outline' with a stable empty-list fallback so non-outline renders don't re-project; onJumpToPin is forwarded to ActivityViewer (SidePanel:805) and reused generically; rows are real <button>s with text content (no icon-buttons-need-labels/accessible-interactive-elements violation); ChatOutlinePanel.tsx lives under src/pages/chat/ so the src/pages/*.tsx-scoped blocking rules (narrow-viewport-required, page-layout-pattern) don't match it, and the panel is single-column regardless; no HTML injection or dangerouslySetInnerHTML. Untranslated locale strings fall to the i18n gate, not this pass. Nothing survives falsification.

No findings.

[OPUS-REVIEWED] e833705

Verdict parsed from the review's SHA-scoped output markers for commit e833705709d17f2d8ed0c66b01fd56f2c3d2f84f.

False positive or not applicable? A repository writer can comment:
/ai-review override fable e833705709d17f2d8ed0c66b01fd56f2c3d2f84f: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of e833705709d17f2d8ed0c66b01fd56f2c3d2f84f and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/src/pages/chat/ActivityViewer.tsx:900 -- cold-opening a session with over 100 messages leaves selectSlotMessages bounded to the newest page, so Outline omits older turns despite promising “Jump to any turn” -> Fix: derive entries from the complete session transcript. (origin: validation)
[GPT-REVIEWED] e833705

False positive or not applicable? A repository writer can comment:
/ai-review override gpt e833705709d17f2d8ed0c66b01fd56f2c3d2f84f: <one-sentence reason>

@chenmingwei23
chenmingwei23 force-pushed the feat/chat-outline-panel-8221 branch from de2b6c0 to efc2fae Compare September 6, 2026 13:22
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@chenmingwei23
chenmingwei23 force-pushed the feat/chat-outline-panel-8221 branch from efc2fae to ffb96cd Compare September 6, 2026 13:40
@chenmingwei23
chenmingwei23 force-pushed the feat/chat-outline-panel-8221 branch from ffb96cd to e833705 Compare September 6, 2026 13:55
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 6, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chenmingwei23

Copy link
Copy Markdown
Contributor Author

Closing without merging. The reasoning is recorded on the issue so it outlives this PR:
#8221 (comment)

Short version: @bolichen97's tech-lead review is right that the outline silently truncates
in the exact case that motivated #8221. The rows come from selectSlotMessages, which
returns only the in-memory page -- a single OLDER_PAGE_LIMIT (100) page on a slot switch --
and nothing in the outline path consults slotHasMore. The comment at
ActivityViewer.tsx:898 confirms that is deliberate: "derived from the same redux messages
the transcript renders -- no server fetch." So in a session hundreds of turns deep the tab
shows only the newest slice, with no signal anything is missing, while this PR's body claimed
"all the session's user turns" and the menu string says "Jump to any turn in this session."

That is a wrong premise rather than a fixable defect at this size, so the work is being
retired rather than patched. His two other findings -- the outline click reusing the 'pin'
PendingJumpOrigin and so rendering "pinned message unavailable", and the current-row marker
reusing the bg-bg-hover token -- are recorded on the issue too, along with what a future
attempt has to handle.

Thanks for the review; it caught the thing that mattered.

@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants