Skip to content

feat: add a session summary panel to the chat side panel - #3169

Merged
michellemxm merged 1 commit into
mainfrom
feat/session-summary-panel
Aug 14, 2026
Merged

feat: add a session summary panel to the chat side panel#3169
michellemxm merged 1 commit into
mainfrom
feat/session-summary-panel

Conversation

@michellemxm

@michellemxm michellemxm commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem

A long session gives you no way to see what it was about. To recover the thread of a hundred-turn conversation — what you were trying to do, which threads finished, what is still waiting on you — you scroll back and re-read.

#2855 added the backend that derives that summary. Nothing renders it.

Why it matters

The summary is only useful if a reader can see it at the moment they return to a session. Without a surface it is a sidecar file on disk that nothing consumes.

Fix (symptom → root cause → change)

Symptom: no way to read a session's summary.

Root cause: the panel surface does not exist; GET /api/chat/slots/{slot}/summary has no consumer.

Change: a new Summary tab in the chat side panel.

  • Needs-you triage at the top. Open items are hoisted out of their intent cards into one block. Each is one headline, collapsed, expanding to its reasoning, its expected outcome, and the intent it came from — so the block answers "does this need me?" in a glance-sized space, and the context that makes an item decidable is one click away rather than pushing the intent list off screen. The item is deliberately duplicated rather than relocated — the card keeps its own copy, so the block reads as a summary of the panel, not a place things go missing from.
  • Collapsible intent cards, most recently touched first, with the newest expanded. Collapsed cards keep a one-line gist so the closed state still informs.
  • Project notes pinned outside the scroll region, collapsed by default with a count. They are durable background facts, not the answer to "does this session need me?" — open by default they cost every visit while being needed on few, so the count advertises them and the reader opts in.
  • Disclosure state persists per slot for all three, so re-opening the panel does not undo the reader's own collapsing.
  • No polling. Freshness comes from the session_summary websocket event invalidating the per-slot query. A panel on an interval would reward the refresh habit this feature exists to remove, and would cost a request per tick for data that changes once a turn.

One backend change rides along, and it is the reason that last point is true. #2855 added push_session_summary, but _broadcast had no typed WS branch for the event, so it fell through to the generic notification envelope — and because the client dispatches on the outer type, the panel's case 'session_summary' was unreachable. The summary only appeared after a manual reload, and the payload also landed in the bell feed as a ts-less notification. _broadcast now emits {"type": "session_summary", "data": {"key": ...}}, mirroring the artifact_update branch and the reason its comment gives. This is the only non-frontend code in the PR.

Registered across the seven places a side-panel view has to be declared (ViewKind, VIEW_TITLE_KEY, KIND_ICON, the two NEW_MENU_* maps, NEW_MENU_GROUPS, VIEW_KINDS), plus ActivityViewer's view union, its dispatch, and the segmented-control guard. It follows ContextBreakdownTab — the closest sibling, and the right precedent: a per-slot, read-only view of derived data, taking only slot and fetching its own.

Empty and error states

Three states a reader will hit before this feature is fully rolled out, each of which has to explain itself rather than look broken:

  • Off — the settings toggle ships separately, so this state is reachable before it can be turned on.
  • No summary yet — the copy names what actually produces one ("A summary is written when a turn finishes. Send a message to get one for this session."), because "wait and it appears" is false for an idle or historical session. A Reload summary button covers the case where a summary was written while the panel sat here; it deliberately cannot generate one, since the endpoint is read-only by design so that opening a panel cannot spend tokens.
  • Could not load — has an icon, a title, an explanation, and a Try again that calls the same refetch. Without it the failure branch returned before the header rendered, so the one control that could recover the panel disappeared exactly when it was needed.

Tests

  • 43 frontend tests. sessionSummaryTab.test.tsx covers all four states, ordering, triage hoisting with provenance, disclosure persistence across remount for both the intent cards and the triage items, the pinned notes, the plural chip, and that the panel fetches once and does not poll. Several assert recovery or reveal rather than presence: clicking Retry / Reload actually loads content, and expanding a collapsed triage item actually surfaces its reasoning and source intent — a collapsed-by-default element is easy to test into a state where it is merely absent.
  • sessionSummaryHelpers.test.ts — the pure helpers, collectTriage ordering especially (needs-you before merely-recent, dropped intents never hoisted, no double-count across the two passes).
  • sidePanelAddMenu.test.tsx fixtures updated for the new menu entry; that suite pins that every entry appears exactly once across the groups.
  • 9 Playwright specs (website/playwright/session-summary.spec.ts), MIN_EXECUTED_SPECS raised 210 → 219. They stub the summary route with page.route: a real summary costs a model call the credential-less CI gateway cannot make, and stubbing keeps the specs about the panel rather than about model output.

tsc -b, eslint, and all 16 i18n:check gates pass.

i18n

30 keys across the 12 shipped catalogs. The needs-you count uses i18next plural selection rather than a plural-neutral phrasing, so its base key is registered in pluralKeys.json and every locale carries exactly the CLDR categories it selects: Russian _few/_many, Spanish/French/Portuguese/Italian _many (these four select it for large and compact numbers), and zh-CN/ja/ko _other only. catalogParity.test.ts enforces both directions — a missing category and a category the language never selects.

Manual verification

Verified against a real session in an isolated dev gateway with the feature enabled — the panel rendered a live summary (3 intents from 6 user turns), not fixture data.

This depended on #3167 — the fix for the backend never storing a summary at all — which merged on 2026-08-13, so there is no longer a sequencing constraint.

Screenshots

Seven states plus the expanded interaction, captured by rendering the real component (dark theme; the harness has no gateway, so the custom color theme's variables are never injected and a light capture would be half-themed rather than accurate — the component holds no color literals, which is the property a light capture would have tested).

Populated — triage collapsed to headlines, notes collapsed behind their count

Expanded — one triage item open with its reasoning and source intent, notes open

No summary yet

Remaining states

One needs you (singular plural form)

Stale

No project notes

Feature off

Failed to load

Known gaps, deliberately left

  • A new-user usability review (run with no builder context) raised four issues. Two are addressed here: the needs-you block no longer repeats its card verbatim (it is a headline until expanded), and collapsing the notes by default keeps them from pushing the intent list down on every visit. Two remain: the pinned notes bar can still slice the next card's gist line where the scroll region meets it, and the staleness marker contradicts the footer's "Updated Nm ago". Both are layout/copy decisions worth taking separately rather than widening this PR.
  • The off state names no location for the settings toggle, because the toggle ships in a later PR and naming a control that does not exist yet would be worse than saying nothing.
  • Generate-on-demand for historical sessions (a session that finished before the feature existed can never get a summary) needs a POST endpoint and reverses feat: generate intent-level session summaries behind a flag #2855's deliberate read-only stance. Its own PR.

@michellemxm
michellemxm requested a review from a team August 13, 2026 00:45
@michellemxm
michellemxm requested a review from a team as a code owner August 13, 2026 00:45
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 0e790d5

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

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of 0e790d523fa7e25bc109d381d3857baeda3ec3c5 — updated in place on each push; does not block merge.

Design-Verdict: PASS

Real gap (backend from #2855 had no consumer), solved at the right layer with the closest precedent, the WS envelope fix in the same commit, and spec updated.

[DESIGN-REVIEWED] 0e790d5

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

Advisory UX-level review of 0e790d523fa7e25bc109d381d3857baeda3ec3c5 — updated in place on each push; does not block merge.

UX-Verdict: CONCERNS

Solid, state-complete panel — but its freshness warning is muted in the footer while the triage block above makes the claims that go stale.

Watch

  • Stale summaries are common (regeneration happens only at turn end), yet the sole cue — footer "Updated {{when}} — behind the conversation", muted mono, bottom edge — sits at maximal distance from the "Open items" block whose answer to "does this need me?" it invalidates; a reader acts on outdated triage without ever seeing the marker. Frequent × wrong-decision friction × every stale visit. Smallest fix: when stale, echo a small marker beside the "Open items" heading; keep the footer as the timestamp.
  • populated.png shows the pinned "How this project works" bar slicing the last card's gist line to a text sliver — reads as a rendering defect on every populated visit with 4+ intents (author acknowledges it). Smallest fix: bottom padding or a fade mask on the scroll region equal to the bar height.

Suggestions

  • In the empty state, "Check again" and the error state's "Try again" both call refetch(); the header reload icon reuses the "Check again" key (aria-label={...reload}) — unify on one verb pair so the icon's tooltip and the empty-state button don't read as different actions.

[UX-REVIEWED] 0e790d5

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

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

Review details

I've verified the single candidate. The SidePanel summary query at SidePanel.tsx:359 indeed lacks the enabled: !!slot guard the tab has, and the panel is mounted with slot={activeSlot || ''}, so an empty slot is theoretically possible. But the observable-wrong-outcome requirement (c) fails: summaryEnabled = summaryMeta?.enabled !== false fails OPEN, so an errored/empty-slot request produces no UI regression — only a wasted request that resolves to error. The candidate's own reasoning concedes it is "behaviourally harmless" and that it could not confirm the empty-slot mount actually occurs ("if activeSlot is always truthy... this is inert"). That is a "might," and the consequence is not a crash, data loss, corruption, or security hole. It dies under falsification and cannot reach the 80 confidence bar.

No findings.

[OPUS-REVIEWED] 0e790d5

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

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

@michellemxm
michellemxm enabled auto-merge (squash) August 13, 2026 01:05
@michellemxm
michellemxm force-pushed the feat/session-summary-panel branch from 9d751b0 to c7cab57 Compare August 13, 2026 01:16
@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: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@michellemxm
michellemxm force-pushed the feat/session-summary-panel branch from c7cab57 to 7026bbb Compare August 13, 2026 01:56
@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 Aug 13, 2026
@michellemxm

Copy link
Copy Markdown
Contributor Author

Dispositions for c7cab5786aaacd583cc07baa3c7b727cb13c36097026bbb85a8f

All three GPT blocking findings were legitimate and are fixed, not rebutted. Each was verified against the code before changing it.

1. SessionSummaryTab.tsx:57 — denied storage access crashes the panel · FIXED

Confirmed real: loadNotesOpen called localStorage.getItem bare, while its two siblings (loadOpen, loadTriageOpen) each wrapped the same call in try/catch. The project already owns the right helper — utils/safeStorage.ts exports safeGetItem — and this file was importing only safeSetItem from it, so writes were guarded and reads were not. All three loaders now read through safeGetItem. Because these run inside useState initializers, the throw took the whole panel down rather than degrading to the default, which is what made a Low-looking omission reachable as a blank panel.

New regression test: renders when storage is denied instead of taking the panel down, which stubs Storage.prototype.getItem to throw SecurityError. Verified non-inert — reverting just that one line to localStorage.getItem makes it fail.

2. ActivityViewer.tsx:1544 — slot switches retain another slot's disclosure state · FIXED

Confirmed real, and specific to this component rather than the sibling it follows. All three disclosure states are seeded by lazy useState initializers that read the slot's own storage key; React does not re-run an initializer when only a prop changes, so a slot switch with Summary already active would show slot A's disclosure choices and then persist them under B. ContextBreakdownTab on line 1540 has no key either, but it holds no slot-derived state, so it is not affected. Added key={slot}.

Being straight about the evidence: this one is verified by inspection, not by a new test. Testing it needs the ActivityViewer slot-switch path, and any test I write at component level would assert React's remount semantics rather than this fix — a test that cannot fail is worse than an honest note.

3. SessionSummaryTab.tsx:377 — duplicate headlines share disclosure state · FIXED

Confirmed real and reachable: collectTriage hoists next steps from multiple intents, and two intents can legitimately carry the same step text ("run the tests"). Keyed on item.what alone, one chevron expanded and persisted both. Disclosure is now keyed on source intent and step text, joined on NUL, which cannot occur in either value. The React list key moved to the same composite, replacing the array index.

New regression test: gives two intents the same step text independent disclosure. Verified non-inert — reverting triageKey to text-only makes it fail.

Also fixed in this push: six real CI failures that were mine

The Frontend Tests shards were red on all four, and the cause was mine — six i18n test failures that my local run had missed because catalogParity / zhStyle / hiStyle are vitest tests, not part of npm run i18n:check, and I had only run three test files locally. I have since run the full suite (17,711 tests) in both shards.

  • catalogParity es / fr / pt / it — these four require the CLDR many category, which my need_you plural was missing. Added need_you_many, mirroring _other per the convention already used by the 128 existing _many keys in es.json (many applies to large/compact numbers, where the wording is the plural one).
  • zhStylefrom_intent used corner brackets 「」, which style/zh-CN.md §1 forbids. Now curly quotes.
  • hiStyle — my 7 new Hindi strings used formal आप, pushing the ratcheted baseline from 119 to 126. All 7 rewritten in informal तुम forms. I diffed against origin/main's catalog to establish that main is exactly 119 and that all 7 additions were mine.

The remaining red is not from this diff

  • AppRootCoverage — "Kiro credits modal", 2 tests. Pre-existing. Proven, not assumed: I created a detached worktree at clean origin/main (b87198918, this PR's exact base) and ran the file there with none of my changes present — the same 2 tests fail identically.
  • DevFleetPageCoverage and MochiChatPanelCoverage, 1 test each. CI flakes. Both pass locally on my branch and on clean origin/main, and both passed on main's own CI run for b87198918.
  • Backend Tests (3.10, 3). Infrastructure, not a test: the job failed inside astral-sh/setup-uv with fetch failed, before any test ran. This PR touches only website/** and temp-screenshots/**.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 13, 2026
@michellemxm
michellemxm force-pushed the feat/session-summary-panel branch from 7026bbb to c90ff9f Compare August 13, 2026 02:32
@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 Aug 13, 2026
@michellemxm

Copy link
Copy Markdown
Contributor Author

Dispositions for 7026bbb85a8fc90ff9fad31f

GPT advisory FINDING — useWebSocket.ts:570, session_summary arrives in a notification envelope · FIXED

GPT was right, and I think this was under-severity rather than advisory. It is the PR's central freshness claim, and it was inoperative. Verified end to end rather than taken on faith:

  • push_session_summary emits _broadcast({"_type": "session_summary", "key": key}) (state.py:4874).
  • _broadcast's WS translation has typed branches for slots, slot_title, refresh, update_progress, artifact_update, chat_message — and no branch for session_summary, so it fell through to else: {"type": "notification", "data": note}.
  • The client dispatches on the outer type (useWebSocket.ts:509), so case 'session_summary' was unreachable dead code.

Two consequences, the second of which the finding did not mention:

  1. The panel was never invalidated. Because it deliberately does not poll, that is not a delayed update — it is no update until the user reloads. This matches a symptom I had already seen in live testing and had recorded as unverified: the summary only appeared after a manual reload. That observation now has a mechanism.
  2. The payload was also dispatched as a Notification. addNotification dedupes on ts and this payload has none, so one malformed entry landed in the bell feed per client.

Fix — backend, not frontend. I did not take the suggested data._type === 'session_summary' check inside the notification branch, because this repo already documents the opposite answer for this exact situation. The artifact_update branch three cases above carries the comment "Typed envelope (not the generic notification fallback) so useWebSocket and future consumers get a self-documenting event" — added for precisely this reason. So _broadcast now has a session_summary branch emitting {"type": "session_summary", "data": {"key": ...}}, which makes the already-reviewed frontend handler work as written and keeps the payload out of the notification feed. The frontend was correct; the envelope was missing.

This widens the PR into the backend by ~12 lines, which I want to flag rather than slip in: this PR was otherwise frontend-only. I judged the alternative worse — landing a panel whose documented no-polling freshness mechanism does not work, with a phantom notification as a side effect.

Test: test_session_summary_api.py::TestSessionSummaryBroadcast::test_ws_envelope_is_typed. Verified non-inert — removing the branch fails it with the bug in the assertion output: {'type': 'notification'} where the client expects {'type': 'session_summary'}. Spec updated in the same commit (docs/system-specs/modules/session-summary.md), since this is documented behavior.

UX Review suggestion 1 — returned_to is a dangling fragment that drops the computed count · FIXED

Accurate on both counts. The label read ↺ returned to with nothing after it, and IntentCard was already computing resumptionCount(intent) into resumed and then using it only as a > 0 visibility test — the number itself was discarded. Now Resumed ×{{times}} across all 12 catalogs.

One deliberate detail: the count is interpolated as times, not count. i18next treats count as a plural selector, so naming it that would have silently demanded a full set of plural forms in every catalog — including the _many category whose absence in es/fr/pt/it broke CI earlier in this PR. times keeps it a plain interpolation.

UX Review suggestion 2 — failed opens with filler and restates failed_title · FIXED

Accurate. The body was "Something went wrong loading it. Try again, or reload the page if it keeps failing." against the title "Could not load the summary" — the first sentence restated the title and led with the filler phrase the copy guide flags. Now the body leads with the recovery action: "Try again, or reload the page if it keeps failing." Applied across all 12 catalogs; en-XA regenerated.

Still-red checks: all four now proven not to come from this diff

Verified by running each on a clean detached worktree of origin/main with none of this branch's code present — not inferred from the file names:

  • AppRootCoverage — "Kiro credits modal", 2 tests: fails identically on clean main.
  • App.test.tsx — "Kiro credits pill … closes the modal on Escape", 1 test: appeared when main advanced to 5d6456004; fails identically on clean main at that SHA.
  • DevFleetPageCoverage, MochiChatPanelCoverage, 1 each: CI-only flakes — pass locally on this branch and on clean main, and MochiChatPanelCoverage already passed on the previous CI run.
  • Backend Tests (3.10, 3): infrastructure, not a test — the job died inside astral-sh/setup-uv with fetch failed before collection.

All are in the Kiro-credits area or the runner, none touch session summary.

Verification for this push

Full frontend suite in both shards (17,718 tests) — only the pre-existing Kiro-credits failures above. All 16 i18n gates PASS with I18N_BASE_REF=origin/main. tsc -b clean, 0 lint errors. Backend: 578 broadcast/notification tests pass, plus flake8 / mypy / isort clean on the touched files and docs-lint green. Rebased onto current main 5d6456004, still one commit.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 13, 2026
@michellemxm
michellemxm force-pushed the feat/session-summary-panel branch from c90ff9f to 37c2760 Compare August 13, 2026 03:24
@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 Aug 13, 2026
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Aug 13, 2026
@michellemxm
michellemxm force-pushed the feat/session-summary-panel branch from 5c4cd05 to a74368a Compare August 13, 2026 18:12
@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 Aug 13, 2026
@michellemxm

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #1636 (Git panel) landed, and dispositioned the local Opus mirror's findings against 5c4cd05bf. New head a74368a0abe4.

The rebase

#1636 registers a new panel tab in the same seven registries this branch touches, so 16 files conflicted: usePanelTabs.ts, ActivityViewer.tsx, SidePanel.tsx, sidePanelAddMenu.test.tsx, and all 12 locale catalogs. Both sides only add rows, so every conflict resolved as a union — Summary and Git now both register in ViewKind, VIEW_TITLE_KEY, KIND_ICON, NEW_MENU_LABEL_KEY, NEW_MENU_DESC_KEY, NEW_MENU_GROUPS and VIEW_KINDS, plus both ActivityViewer view unions and the segmented-control fall-through.

en-XA.json was not hand-merged — it is generated, so I took main's copy and re-ran scripts/gen-pseudolocale.mjs.

E2E: correcting my own earlier diagnosis

I previously suggested sourcing summaryEnabled synchronously, citing terminalEnabled as the precedent. That was wrong and I withdraw it. newMenuSections accepts terminalEnabled but never reads it — terminal gates no menu row, so its async flag never remounts a group and the precedent proves nothing about race-freedom. A synchronous store would also still flip once when its fetch resolves.

The actual cause was the group's React key being derived from its contents (section[0].kind). When the gate resolved, group 0's key changed summaryissues, React remounted the group, and the trigger detached mid-click. Each group now carries a declared id; filtering rows and dropping emptied groups both leave it untouched.

Proven non-inert: reverting the key to group.items[0].kind fails the new test with expected [ 'summary', 'side', 'logs' ] to deeply equal [...] — the identity change is the remount.

explains itself when the feature is off was unreachable by design (it navigated through the row the gate removes) and redundant — sessionSummaryTab.test.tsx:75 already asserts that copy. It now asserts the row is absent, and asserts a sibling row is visible first so it cannot pass by the menu simply failing to open.

Findings

1 — launcher left on the pre-reshape shape. FIXED. menuItems = menuSections.flat().flatMap(section => section.items). Legitimate and build-breaking; the fix existed in my worktree but was never amended into the reviewed commit, so the review was correct about the commit as pushed. My process error: I dispatched reviewers against a SHA that predated my own fix.

2 — bg-panel / bg-panel-strong are phantom utilities. FIXED. Verified independently: tailwind.config.js maps card and bg-elevated but has no panel key, so both classes emitted nothing and the two pinned bars painted the body colour behind a 1px border. Now bg-card / bg-bg-elevated.

This was visible in populated.png, a frame I had already reviewed and passed — the third time in this PR I missed something present in my own evidence. opsMissionControl.test.ts guards this exact mistake but over a hardcoded file list that cannot cover files added later, so the panel now carries its own assertion, which also pins that the replacements are present (or it would pass by the classes merely being deleted).

3, 4 — comment policy. FIXED. Dropped #2855 and #2981; rewrote three "used to / rendered as" comments in present tense.

5 — a comment stating something false. FIXED. The spec claimed the summary route "ships in a separate change, so on this branch's gateway it does not exist yet". It is registered at the base commit (routes/chat.py:57) — that landed with the backend PR and my comment went stale. Clause deleted; the model-call rationale stands on its own.

6 — helper only reshaped, not re-typed. FIXED. ContextBreakdownPanel.test.tsx passed summaryEnabled: undefined, silently exercising a gate state SidePanel never produces. Now { ...o, summaryEnabled: true }.

7 — orphaned doc comment. FIXED. Moved onto isOpenFor, which it describes.

8 — raw <button> vs <Btn>. PARTLY FIXED, PARTLY REBUTTED. The two pill buttons were already Btn's base contract (inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md border) and are now <Btn>. The two disclosure rows stay raw: they are full-width, borderless, unrounded, left-aligned rows, so <Btn> would need its display, width, padding, border and radius all overridden — the component would contribute nothing but indirection. The icon-only header reload likewise stays a 28px square with its aria-label.

Verification

npx tsc -b clean · npm run lint 0 errors (573 warnings, baseline) · I18N_BASE_REF=origin/main npm run i18n:check 16/16 PASS · sessionSummaryTab 28 ✓, sidePanelAddMenu 9 ✓, ContextBreakdownPanel ✓.

Two caveats, stated rather than buried:

  • Screenshots are stale — they show the bars before the token fix. I re-captured, but the dev server proxies /api/themes to a gateway that 403s without a token, so theme-boot fell back and the accent rendered green instead of purple across all 8 frames. Committing those would misstate the product's default accent, so the existing frames stand until an authenticated re-capture.
  • The full local suite is not a clean signal. The host was at load ~385 during the run; two files failed (CrewCompanionPet.coverage, MochiChatPanelCoverage) and both pass in isolation — neither is touched by this change. CI on a clean runner is the real arbiter.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Aug 13, 2026
@michellemxm
michellemxm force-pushed the feat/session-summary-panel branch from a74368a to 4d7bd62 Compare August 13, 2026 20:49
@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 Aug 13, 2026
@michellemxm

Copy link
Copy Markdown
Contributor Author

E2E: root cause and fix (4d7bd62dac4f)

Two specs were failing — collapses an intent card and remembers it across a reload and reports a failed load with a Retry that recovers the panel — both as locator.click: Timeout on the + ("Open side panel tab") inside the openAddMenu helper. The waitFor immediately above succeeded, then the click found nothing.

Cause. ChatPage opens the Activity panel by itself when the slot's project dir is a git repo (dispatch(openActivityPanel()), guarded once per slot+path by a localStorage marker), fired off an async git query that can resolve at any point after load. The helper dispatched toggle-activity-panel unconditionally, and a toggle against an already-open panel closes it, unmounting the strip the next line clicks. The marker suppressing the auto-open on the second attempt is why eight sibling specs were flaky-then-pass rather than failed.

Fix (test-only, website/playwright/session-summary.spec.ts): dispatch the toggle only when the strip is genuinely absent, and wrap the open-and-click in expect(...).toPass() so the pair retries as a unit instead of racing an auto-open that lands between a separate check and click. The retry budget is 12s, inside the 30s per-test timeout alongside the helper's goto.

This supersedes the menu-group React-key theory I posted earlier and then withdrew: the declared group id is correct on its own terms, but it was not the cause of these two failures.

No shipped code changed. The once-per-project late auto-open is a pre-existing interaction owned by the Git panel change and is unrelated to this feature.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 13, 2026
Renders the intent-level session summary as a side-panel tab: what each
thread of the session was trying to do, where it landed, and what still
needs the reader. Open items are hoisted into a triage block at the top,
each naming the intent it came from so lifting it out does not sever the
context that makes it decidable. Intent cards are collapsible with the
most recently touched one open, and the durable project facts are pinned
outside the scroll region so they do not have to be hunted for.

The panel does not poll. Freshness comes from the session_summary
websocket event, invalidating the per-slot query on regeneration. A panel
on an interval would reward the refresh habit the feature exists to
remove, and would cost a request per tick for data that changes once a
turn.

Reads the summary through GET /api/chat/slots/{slot}/summary. Off by
default: with the flag disabled the endpoint reports disabled and the
panel explains itself rather than looking broken. The settings toggle
ships separately, so the off state is reachable before it can be turned
on and has to stand on its own.

Every user-facing string is translated across the 12 shipped catalogs.
The needs-you count uses i18next plural selection rather than a
plural-neutral phrasing, so its base key is registered in pluralKeys.json
and Russian carries the few and many forms its rules require.

Adds 34 unit tests and 9 Playwright specs, and raises
MIN_EXECUTED_SPECS accordingly. The specs stub the summary route: a real
summary costs a model call the credential-less CI gateway cannot make,
and stubbing keeps them about the panel rather than about model output.

Screenshots under temp-screenshots/session-summary-panel/ are dark-theme
only. The capture harness has no gateway, so the custom color theme's
variables are never injected and a light capture would be half-themed
rather than accurate. The component holds no color literals; every color
is a theme token.
@michellemxm
michellemxm force-pushed the feat/session-summary-panel branch from 4d7bd62 to 0e790d5 Compare August 13, 2026 21:26
@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 Aug 13, 2026
@michellemxm

Copy link
Copy Markdown
Contributor Author

E2E round 2: the helper fix landed, two spec bugs behind it (0e790d523fa7)

The openAddMenu fix worked — the locator.click timeout is gone and both specs now run past the helper. What they hit next were two genuine bugs in my own specs, previously masked by the helper failing first. Both are test-only; neither is a defect in the panel.

1. collapses an intent card and remembers it across a reload — failed at line 154, getByText('Session summary panel') not found after page.reload(). The tab STRIP persists on a trailing 300ms debounce (PERSIST_DEBOUNCE_MS in usePanelTabs) with no beforeunload/pagehide flush, and clearTimeout restarts the timer on every write. Reloading immediately can therefore land before the flush, so the Summary tab is absent from the rehydrated strip and the panel returns without it. The disclosure state itself was never the problem — that one writes synchronously (safeSetItem in the toggle). Fixed by waiting for the mc-panel-tabs: write the assertion depends on before reloading, rather than sleeping.

2. reports a failed load with a Retry that recovers the panel — failed at line 219, getByText('Could not load the summary') not found. The spec failed only the first request to the summary route, on the assumption the tab issues it. It does not: SidePanel gates the + menu row on the same query key, ['session-summary', slot] (SidePanel.tsx:361 vs SessionSummaryTab.tsx:230), so the gate query consumed the 500 and the tab's refetch got the 200 — the error state never rendered. Fixed by failing every request until the spec flips a flag, which is what "Retry refetches" actually requires and is independent of how many components query the key.

Also rebased onto 3701948c1f54; tsc -b, gen-pseudolocale --check (10435 keys) and i18n:check are green on that base.

Worth noting as a separate, pre-existing wart owned by usePanelTabs rather than this PR: because that debounce has no unload flush, a user who reloads within 300ms of opening a panel tab loses the tab. Small, and not this change's to fix.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 13, 2026
@michellemxm

Copy link
Copy Markdown
Contributor Author

Dispositions for UX Review 🟡 CONCERNS on 0e790d523fa7

Taking the three items individually.

1. Stale marker sits at maximal distance from "Open items" — ACCEPTED, confirming treatment with the author before changing

The reasoning holds: regeneration happens only at turn end, so stale is a common state, and the block a reader uses to answer "does this need me?" carries no qualifier while the only cue is muted mono text at the bottom edge.

Flagging one thing before I change it, because it cuts against an earlier decision on this same PR rather than being new ground: an earlier round deliberately collapsed several freshness cues into the single footer line (updated_behind) precisely to stop the panel saying the same thing in two places. Adding a marker beside "Open items" partially reverses that. I think the two are reconcilable — the footer keeps the timestamp, the triage heading carries the qualifier, so they say different things rather than duplicating — but that is a design call on a panel whose freshness treatment was already litigated once, so I am confirming it with the author instead of quietly flipping it. It is a single conditional chip; no new payload field is needed, stale is already in the contract.

2. populated.png shows the pinned notes bar slicing the last card — REBUTTED (mechanism), with one correction to the framing

The proposed fix cannot work, because the bar does not overlay the list. From SessionSummaryTab.tsx: the panel is absolute inset-0 flex flex-col (line 364), the card list is flex-1 overflow-y-auto (385), and both the notes bar (455) and the footer (485) are shrink-0 flex siblings below it. Bottom padding or a fade mask inside the scroll region would add empty space after the final card; it would not change what the fold looks like mid-list, because nothing is being covered. What the screenshot shows is a scroll region with more content below the visible edge, which is what every scrollable list looks like when it is not scrolled to the end.

The "author acknowledges it" reference is to my own earlier note, where I dispositioned this and rebutted the same padding mechanism. Correcting one detail in the review's framing: it is not a rendering defect and it is not specific to 4+ intents — it is the ordinary appearance of a partially scrolled list.

3. Check again vs Try again vs the header reload tooltip — ACCEPTED AND DEFERRED

Real inconsistency, correctly spotted: all three paths call refetch() while presenting two different verbs, and the header icon reuses the empty-state key for its aria-label. Deferring it rather than folding it in: the fix edits the en catalog plus 11 translations (and regenerates en-XA) to settle a label nit, on a PR that is otherwise green after several CI rounds. That is a poor trade against the risk of re-opening the i18n gates for cosmetics. I will file it as a follow-up so it does not evaporate; it belongs with the settings-toggle change that is already queued behind this PR.

No code changed for this comment — items 2 and 3 need none, and item 1 is waiting on the author's call.

@michellemxm
michellemxm merged commit c7fdd9a into main Aug 14, 2026
63 of 65 checks passed
@michellemxm
michellemxm deleted the feat/session-summary-panel branch August 14, 2026 00:13
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 14, 2026
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
Renders the intent-level session summary as a side-panel tab: what each
thread of the session was trying to do, where it landed, and what still
needs the reader. Open items are hoisted into a triage block at the top,
each naming the intent it came from so lifting it out does not sever the
context that makes it decidable. Intent cards are collapsible with the
most recently touched one open, and the durable project facts are pinned
outside the scroll region so they do not have to be hunted for.

The panel does not poll. Freshness comes from the session_summary
websocket event, invalidating the per-slot query on regeneration. A panel
on an interval would reward the refresh habit the feature exists to
remove, and would cost a request per tick for data that changes once a
turn.

Reads the summary through GET /api/chat/slots/{slot}/summary. Off by
default: with the flag disabled the endpoint reports disabled and the
panel explains itself rather than looking broken. The settings toggle
ships separately, so the off state is reachable before it can be turned
on and has to stand on its own.

Every user-facing string is translated across the 12 shipped catalogs.
The needs-you count uses i18next plural selection rather than a
plural-neutral phrasing, so its base key is registered in pluralKeys.json
and Russian carries the few and many forms its rules require.

Adds 34 unit tests and 9 Playwright specs, and raises
MIN_EXECUTED_SPECS accordingly. The specs stub the summary route: a real
summary costs a model call the credential-less CI gateway cannot make,
and stubbing keeps them about the panel rather than about model output.

Screenshots under temp-screenshots/session-summary-panel/ are dark-theme
only. The capture harness has no gateway, so the custom color theme's
variables are never injected and a light capture would be half-themed
rather than accurate. The component holds no color literals; every color
is a theme token.

# Conflicts:
#	website/src/i18n/locales/bn.json
#	website/src/i18n/locales/de.json
#	website/src/i18n/locales/en-XA.json
#	website/src/i18n/locales/en.manual.json
#	website/src/i18n/locales/es.json
#	website/src/i18n/locales/fr.json
#	website/src/i18n/locales/hi.json
#	website/src/i18n/locales/it.json
#	website/src/i18n/locales/ja.json
#	website/src/i18n/locales/ko.json
#	website/src/i18n/locales/pt.json
#	website/src/i18n/locales/ru.json
#	website/src/i18n/locales/zh-CN.json
#	website/src/pages/chat/SidePanel.tsx
#	website/src/test/sidePanelAddMenu.test.tsx
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