feat: bound the summary panel height caps and raise the storage rails

[michellemxm]

Problem / Motivation

The session-summary sidecar trimmed every payload to 8 intents and 5 project notes
before writing it to disk. That trim is not a display limit — nothing collapses,
nothing is hidden behind a control, and nothing reports it. The dropped intents and
notes are simply absent from the stored record, and no later reader can tell a session
that produced 20 intents from one that produced 8.

Two things made it worse than a plain cap:

• The panel already withholds nothing. It renders every intent it receives and
  collapses all but the most recently touched one, so the config's stated rationale
  ("the panel collapses them anyway") argued for a storage deletion from a display
  premise.
• The counts a reader trusts were the post-trim counts. The header chip and the
  project-notes badge both report what survived, so a clipped session shows a
  confident, wrong number.

Why it matters

A returning reader uses this panel to decide what still needs them. Silent truncation
makes it quietly unreliable on exactly the long sessions where it is most needed, and
the loss is unrecoverable — the payload is only rebuilt when the transcript changes, so
nothing backfills what the trim removed.

What changed (motivation → approach → change)

Raise the rails so binding is pathological, not routine. max_intents and
max_constraints both move to 50. This costs nothing measurable: only three call
sites read the payload and none injects it into a model prompt, so a larger payload
carries zero token cost. At ~1.1 KB per intent (measured across 26 real sidecars, 2.2 KB
worst case) 50 intents is ~54 KB typical, and every loop over intents is linear.

Report a rail that binds. normalize_payload now logs at WARNING with the counts.
WARNING rather than INFO because the loss is otherwise invisible and the rail should
essentially never bind — a line that fires on ordinary sessions is one an operator
learns to ignore, so a test asserts it stays silent when the rail does not bind.

Fix the layout the higher notes rail would have broken. The project-notes footer is
shrink-0 with no ceiling, so a long list grew the footer and squeezed the intent list
toward zero instead of scrolling — max_constraints: 5 was the only thing preventing
that. Both the open-items card and the notes footer are now bounded at 33vh.

The cap is on the card, not the list inside it. Capping the list leaves the card
measurably larger than its stated limit (heading, padding and toggle add ~86px, so a
"33vh" list sat inside a card that was 43% of the window), which reads as the cap not
working. Charging the chrome against the third instead makes the box a reader can point
at the box the cap governs. The list is flex-1 min-h-0 overflow-y-auto, and min-h-0
is load-bearing: a flex child's default min-height:auto refuses to shrink below its
content and would push the card straight past the cap. The disclosure control sits
outside the scroll region so it is reachable at any list length, and below the cap the
height stays natural with no scrollbar.

Give the bounded edge a cue. A cap that lands on a clean row boundary is
indistinguishable from a list that simply ended, and the platform offers no help —
macOS overlay scrollbars stay hidden until you scroll. So a reader who expands
"+25 more", sees six rows and "Show less", can reasonably conclude the rest never
loaded. Both bounded regions now carry a bottom fade, following the treatment already
used in four places in the dashboard (Accordion, PrDetail, PrList, IssueList):
h-6, bg-gradient-to-t from the region's own surface colour so content dissolves into
it rather than banding over it.

It differs from those four in one way, and that difference is the point: they are static,
shown whenever the region is open, while this one is gated on measurement. A fade over a
list with nothing beneath it invents hidden content, which is the same dishonesty as
hiding content told the other way round — so it appears only while something is genuinely
below the fold, and clears when the reader reaches the end. That also makes it correct in
the short-session case, where the cap does not bind and there is nothing to hint at.

Stop finished intents feeding the open-items block. collectTriage's second pass
hoisted steps from every intent except needs-you and dropped, which included
done — completed and verified. Such an intent needs nothing, so its residual next
steps are residue, and counting them inflated the header chip on precisely the long
sessions the higher rail now preserves. The pass is now in-progress only; needs-you
is still covered by the first pass.

Config descriptions now describe the code. Both keys claimed to be display-driven
upper bounds; they are storage rails, and the text says so.

Tests

• test_trimming_intents_is_reported / test_trimming_project_notes_is_reported — the
  WARNING fires with the dropped and total counts when each rail binds.

• test_a_rail_that_does_not_bind_stays_quiet — no log record on an ordinary session.
  This is what keeps the warning a signal rather than noise.

• test_the_suppress_the_section_setting_does_not_warn — max_constraints: 0 is the
  documented "suppress the section entirely" setting, so any emitted note trips
  len(constraints) > 0 and would have fired the WARNING on every regeneration of every
  session with that config — precisely the noise the test above exists to forbid, missed
  because it only exercised the 50/50 case. The trim site now exempts 0, since
  discarding notes there is the operator's intent rather than an anomaly.

• test_documented_defaults / test_non_numeric_values_fall_back_to_defaults — updated
  to 50. These caught that the default lived in two places: the dataclass field
  and an independent literal in the disk-parse fallback, so changing only the dataclass
  would have left a config-absent install still capped at 8.

• never hoists from a done intent (vitest) — rewritten as a contract test; it
  previously asserted the opposite. The existing ordering test already pins that
  in-progress still contributes, so the narrowing is fenced on both sides.

• website/scripts/capture-session-summary-height-caps.mjs — a new capture harness that
  asserts the geometry it photographs, because a still frame cannot distinguish
  "capped at 33vh" from "happens to be short". 19 assertions across a tall and a short
  fixture in both themes: the card sits at 33vh and its list has real height and
  scrolls; the toggle survives scrolling the list to its end; the notes header is not
  squeezed out; the fade is present while content is below the fold, resolves to the
  region's own surface colour, and clears at the end of the scroll; and on the short
  fixture the card sits below the cap with nothing scrolling and no fade at all —
  the checks that would catch an implementation pinning every expanded list to 33vh or
  showing the cue unconditionally.

  Two of these assertions were strengthened after they failed to catch a real regression
  I introduced mid-review. Copying the sibling Accordion's absolute inset-0 scroller
  took the list out of flow, so the card — whose height is a max-h ceiling rather than a
  fixed height — had no content left to measure and collapsed from 297px to 86px. The
  one-sided card <= 33vh and scroll > client checks both still passed (a collapsed
  card is under the cap; client=0 is less than any scrollHeight). They are now two-sided,
  so that failure mode is caught directly rather than only via the short fixture.

  The harness also seeds localStorage through stubDashboardApi's localStorageEntries
  option rather than its own addInitScript, which would race that helper's
  localStorage.clear() and depend on undocumented registration order.

Manual verification

Geometry is machine-asserted by the harness above rather than eyeballed. Measured at a
1400x900 viewport: open-items card 297px (exactly 33vh) with 969px of content
scrolling inside a 211px list; notes footer 297px with 402px of content in a 261px
list; short fixture 246px with nothing scrolling. Both themes. Adding the fade left every
one of those numbers unchanged.

The fade's own colour resolution is asserted rather than assumed, because Chrome reports
a withAlpha() theme colour as color(srgb 0.1098 0.098 0.1333) while reporting the
resolved gradient stop as rgb(28, 25, 34) — the same colour in different units, so a
naive string comparison would have reported a mismatch that does not exist, and a genuine
mismatch (a stop resolving to transparent, rendering nothing) would be invisible.

Worth recording for whoever tunes this next: 33vh of the window is 43% of the
panel's 685px scroll area, because the panel is shorter than the window by its header
and two footers. The harness prints both shares so that difference is visible rather
than rediscovered.

Full backend suite: 58,560 passed. The 143 failures on this host are pre-existing and
environmental — verified by running the highest-count files against pristine
origin/main in the same venv, which fails identically (80 failed / 84 passed).
Causes: defusedxml absent from the local venv, the xdist worker budget being
cores/memory-dependent, and AF_UNIX path too long from the worktree path length. All
175 session-summary tests pass. Frontend: 22,295 passed; 3 unrelated files timed out
while both suites ran concurrently and pass in isolation (77 tests).

Screenshots / video

Open items expanded — the card bounded at 33vh with its list scrolling inside it, and
"Show less" pinned below the scroll region:

The same block with the inner list scrolled to its end — this is what shows the toggle
lives outside the scroll region rather than merely below it:

"How this project works" expanded — 18 notes, footer bounded at 33vh, header intact:

Collapsed default, the short-session case, and light theme

Collapsed default — 3 items and the withheld count, unchanged by this PR:

Short session expanded — natural height, no scrollbar, cap not binding:

Light theme, the same capped state:

Related Issues

no linked issue: this came from a session-summary improvement tracker rather than a
filed issue, so a merge has nothing to close.

Heads-up on a known textual conflict: #4533 reformats the same four
max_intents / max_constraints parse-fallback lines in config/loader.py (onto one
line each, keeping 8 and 5). Whichever PR lands second will conflict there, and the
correct resolution is this PR's 50 / 50 values in #4533's single-line form. The two
PRs' edits to docs/system-specs/modules/session-summary.md are disjoint — #4533 is in
the Storage/payload region, this is the Configuration table.

Checklist

• Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] e385bb6

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

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

Analysis complete. The change is well-executed UX-wise — measurement-gated fades, persistent disclosure state, honest counts — with one behavioral accessibility gap on the new notes scroller.

UX-Verdict: CONCERNS

Solid capped-scroll treatment with honest overflow cues, but the bounded notes list is unreachable past the fold for keyboard-only users.

Watch

• Keyboard users cannot scroll the capped project-notes list. The new scroller is <ul ref={notesFade.ref} className="flex-1 min-h-0 overflow-y-auto …"> containing only plain <li> text — no focusable element inside and no tabIndex on the container, so once 18 notes are bounded at 33vh (screenshot tall-04-notes-expanded-dark.png), everything below the fold is pointer-only. Low-frequency cohort × total content inaccessibility × every long session = CONCERNS. Smallest fix: tabIndex={0} + visible focus ring on the scroller (arrow keys then scroll it natively). The open-items region is fine — its rows are buttons, so tabbing scrolls them into view.

Suggestions

• In the max_constraints config help, "A storage rail, not a display limit" leads with the PR's internal vocabulary; the max_intents sibling already carries the plain-language version ("dropped from the record rather than hidden") — reuse that phrasing and drop "rail" from both help strings.

[UX-REVIEWED] e385bb6

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

All hunks are accounted for and the approach checks out (I verified the prompt in chat_summary.py is built from render_input(turns), not the stored payload, so the "zero token cost" claim backing the 8→50 raise is real). Final review:

Design-Verdict: PASS

Real, unrecoverable data loss with a false count; fixed at the right layer, layout and chip semantics repaired in the same motion.

Suggestions

• The WARNING makes the rail visible to an operator reading logs, but the named victim — the panel's returning reader — still sees a confident wrong count if the rail ever binds; a small truncated: true field in the stored payload would let the panel say so at any rail value, structurally rather than probabilistically.

[DESIGN-REVIEWED] e385bb6

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of e385bb6f4c3c46822ead7263b3a69691561b2819 — 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 repo checks complete: I verified the existing scroll-cue mechanism (useScrollEdges, horizontal-only), counted payload readers, checked for sibling silent trims in session_summary.py, and confirmed the capture-harness/temp-screenshots convention is mandated by the PR template. Final review below.

First-Principles-Verdict: CONCERNS

A measured overflow-fade cue (new hook, two overlays, ~90 harness lines) ships undeclared in a change described entirely as rails + height caps.

What this change ships

Intent: make the summary panel trustworthy on long sessions by stopping silent, unrecoverable truncation of the stored record — a FIX, with the layout that trim was masking bounded in the same change.

1. Stored summaries keep up to 50 intents, was 8 — justified (silent data loss, named defect)
2. Stored project notes keep up to 50, was 5 — justified (same defect)
3. A WARNING logs when either rail trims — justified (loss otherwise leaves no trace)
4. Expanded open-items card capped at 33vh, list scrolls inside, toggle stays reachable — justified (item 1 makes long lists real)
5. Expanded notes footer capped at 33vh — justified (shrink-0 footer squeezed the intent list to zero)
6. Gradient fade appears at the bottom of a capped list while more is below — undeclared, rides along
7. Finished (done) intents no longer feed the open-items block or its count — justified, declared, fenced by tests
8. Config descriptions now state the caps are storage rails — justified (they described display behavior the panel doesn't have)
9. Geometry-asserting capture harness + 6 committed screenshots — mandated by .github/PULL_REQUEST_TEMPLATE.md
10. Spec table updated to 50/50 — mandated (same-commit spec rule)

Watch

• The fade cue is nowhere in the description — not in "What changed", the test list, or the screenshot captions — yet it is a new visible surface (useOverflowFade + two data-testid overlays in SessionSummaryTab.tsx) plus the harness's fadeProbe/rgbTriples/fadeDissolvesInto machinery. Its harm is derived (this repo's own useScrollEdges doc: a hidden-scrollbar scroller "reads as COMPLETE"; grep useScrollEdges — 8 component consumers of the horizontal twin), so it deserves to exist — but it is a distinct capability a human approved this PR without reading about. Declare it, or split it out.
• The warning is operator-log-only: a panel reader still cannot tell a trimmed session from a complete one. The description says this honestly (binding becomes "pathological"), so this sits at mechanism level with the remainder acknowledged — noting the level, not demanding more.

[FIRST-PRINCIPLES-REVIEWED] e385bb6

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

Both the discovery pass and my independent falsification converge. Let me confirm the collectTriage state semantics don't produce an unintended regression by checking the state type — done above: states are done | needs-you | in-progress | dropped. Old second pass skipped needs-you and dropped (kept done + in-progress); new pass keeps only in-progress, deliberately dropping done. This matches the updated test and doc comment. Not a defect.

The backend trim/warn logic, rail default changes, and the useOverflowFade hook are all consistent and tested. Nothing grounds a concrete input → call path → wrong-outcome chain in the changed lines.

No findings.

[OPUS-REVIEWED] e385bb6

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

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

[michellemxm]

First Principles Review — disposition

• Item 6 is a separable behaviour reversal; confirm the old inclusion was accident, not decision.

  The deleted assertion pinned the opposite on purpose, per its old name "includes steps from done
  intents only via the recent pass". It changes what the chip counts for every session today, rail
  or no rail — a human should confirm the old inclusion was accident, not decision.

  rebutted — the concern is correctly identified as separable, and the maintainer has explicitly
  ruled on it, so it is not an unreviewed rider. Three pieces of evidence say the old inclusion was
  incidental rather than a decision:

  1. done was included by omission from an exclusion list, not by positive selection. The old
     pass-2 condition was if (intent.state === 'needs-you' || intent.state === 'dropped') continue
     — nothing named done, so it fell through by default. The new form selects positively
     (if (intent.state !== 'in-progress') continue), which is why the change reads as a reversal.
  2. The old test name describes the mechanism, not the policy. "includes steps from done intents
     only via the recent pass, not the needs-you pass" pins which of the two passes hoists such
     an intent — it is an argument about pass ordering, not a claim that a finished-and-verified
     intent ought to contribute open items.
  3. The function's own docstring was silent on status: "the open next steps of the most recently
     touched intents". A deliberate decision to count finished work would have been stated there,
     since every other ordering decision in that function is.

  On the merits: done means completed and verified (derive_state), and the block answers
  "does this need me?" — so its residual next_steps are residue. The generation prompt already
  instructs that "an intent that needs nothing gets an empty list", so a done intent carrying open
  steps is a generation artefact, and counting it inflates the header chip precisely on the long
  sessions the raised rail now preserves. The replacement test states the contract in those terms,
  and the pre-existing ordering test still pins that in-progress contributes, so the narrowing is
  fenced on both sides rather than merely loosened.

[michellemxm]

Disposition — UX Review (🟡 CONCERNS): accepted, fixed in e385bb6f4

The finding. The expanded open-items card ends on a clean row boundary at the 33vh
cap with no scrollbar, clipped row, or fade, so a reader who clicks "+25 more", sees six
rows and "Show less", can reasonably conclude the rest never loaded. Correct, and the
platform makes it worse rather than better: macOS overlay scrollbars stay hidden until
you actually scroll, so at rest there is genuinely no cue at all.

Worth naming why this landed rather than being argued: this PR replaced silent storage
truncation with silent visual truncation. Nothing is lost and scrolling reaches
everything, but the discoverability failure is the same shape as the complaint the work
started from, so "the content is reachable" is not a sufficient answer.

The fix. Both bounded regions — the open-items card and the "How this project works"
footer — now carry a bottom fade. It follows the treatment already used in four places in
the dashboard (Accordion, PrDetail, PrList, IssueList): h-6, bg-gradient-to-t
from the region's own surface colour, pointer-events-none, aria-hidden (it reports
what the scroll position already carries, and a screen reader is never in doubt about how
much of a list remains).

One deliberate departure from those four, and it is the substance of the fix. They are
static — shown whenever the region is open. This one is gated on measurement. A fade over a
list with nothing beneath it invents hidden content, which is the same dishonesty as
hiding content, told the other way round. So it appears only while something is genuinely
below the fold and clears once the reader reaches the end, which also makes it correct in
the short-session case where the cap does not bind.

Assertions added, so both halves of that contract are pinned rather than described:

• fade present while the list continues, and its gradient resolves to the region's own
  surface colour (a stop resolving to transparent would render nothing and be invisible in
  a screenshot)
• fade clears at the end of the scroll — a cue that persists at the bottom is a smaller
  version of the failure this fixes
• no fade on the short fixture, in either region

This also caught a real regression of mine, and two of my assertions were too weak to
notice. Copying Accordion's structure literally put absolute inset-0 on the scroller,
which takes it out of flow; a card whose height is a max-h ceiling rather than a fixed
height then has no content left to measure and collapsed from 297px to 86px. The one-sided
card <= 33vh and scroll > client checks both still passed — a collapsed card is under
the cap, and client=0 is less than any scrollHeight. They are now two-sided (the card
must sit at the cap; the list must have real height), so that failure mode fails loudly.
Accordion gets away with absolute because its parent chain supplies a definite height.

Geometry is unchanged by the fade: card 297px / list 211px, notes 297px / 261px, short
card 246px with nothing scrolling, both themes. The six committed frames are re-captured
and the PR body's pinned URLs updated to the new commit.

[michellemxm]

Disposition — GPT 5.6 Review (🔴 blocking): accepted, fixed in e385bb6f4

The finding. The capture harness seeded localStorage through its own
addInitScript, racing stubDashboardApi's localStorage.clear().

Legitimate, and the helper says so itself. stub-dashboard-api.mjs exposes a
localStorageEntries option specifically for this, with a comment above it noting that a
caller's own addInitScript races the clear below. The harness worked only because it
happened to register the stub before its own initializer, and init scripts run in
registration order — so the passing runs rested on undocumented ordering, not a guarantee.
Reorder those two calls and the seeds are wiped, the panel never opens, and it surfaces as
a role-lookup timeout that names nothing about the cause. In evidence-producing code that
is worth fixing rather than rebutting: a harness that fails obscurely is worse than one
that fails loudly.

The fix. Seeds moved into stubDashboardApi's localStorageEntries, and the harness's
own addInitScript is gone. All geometry assertions still pass in both themes; eslint
clean. No production code involved, so panel behaviour is unaffected.

Two further changes rode along in this push and are dispositioned separately: the
max_constraints: 0 warning-noise defect raised by the First Principles lane (guarded, with
a test), and the UX lane's hidden-overflow concern (a measurement-gated bottom fade).

[michellemxm]

Disposition — UX Review (🟡 CONCERNS) on e385bb6f4

Two items, taken separately.

Watch — keyboard users cannot scroll the capped project-notes list: accepted, fix prepared, awaiting the maintainer's authorization to push

Legitimate, and it is a regression this PR introduced rather than a pre-existing gap.
Before this change the notes footer had no ceiling, so a long list grew the footer and
every note was on screen; bounding it at 33vh turned it into a scroll region whose items
are plain <li> text, with no focusable child and no tabIndex. So everything past the
fold became pointer-only. The open-items region is unaffected for exactly the reason the
review gives — its rows are buttons, so tabbing scrolls them into view.

The prepared fix is the smallest one that satisfies WCAG 2.4.7 and matches an existing
precedent in this codebase (SubagentCompletionCard's bounded transcript region):

• tabIndex={0} on the scroller, so arrow keys scroll it natively
• role="region" + an accessible name, so a keyboard user who lands there is told what
  they landed in. The name reuses the existing project_notes catalog key rather than
  adding a string, so no new entry lands in 13 catalogs and the dead-key ratchet is
  unaffected
• focus-visible:ring-2 ring-inset rather than an outline — an outline on a scroll
  container is clipped to a hairline on one edge, which is not a visible focus indicator

jsx-a11y/no-noninteractive-tabindex fires on this and is a false positive here: a
bounded scroll region genuinely needs a tab stop. It is suppressed with a scoped
eslint-disable block carrying the reason, following the same treatment in CodeBlock,
ActivityViewer and ui.tsx. Zero new lint warnings.

Pinned by a new test, the bounded notes list is reachable by keyboard, which asserts
the named region is the <ul> and that it takes focus. Mutation-verified: removing
tabIndex fails exactly that one test and nothing else.

Suggestion — drop "rail" from the config help strings: accepted, prepared with the same push

Fair, and worth taking rather than deferring: "storage rail" is vocabulary invented while
writing this change, not language a reader of the settings UI has any reason to know. Both
help strings now lead with the plain-language consequence the review pointed at — that
what exceeds the ceiling is dropped from the record rather than hidden — and neither uses
"rail".

Why this is not already on the PR

This branch has a standing constraint that each push is individually authorized, and the
current authorization is spent. Both changes are complete and verified locally (tsc,
eslint, 45 frontend tests, 60 backend tests, flake8, and the 38 geometry/fade assertions
in the capture harness, with no strict-mode locator collision from the new accessible
name). They land on the next amend, which will also re-pin the six screenshot URLs to the
new commit.