[trotterdylan]

Implements BB-80.

In the stewards sidebar every collapsible row started collapsed, so an unread
worker, reviewer or child thread stayed hidden. Now a group starts expanded when
any descendant thread is unread, and collapses again once everything under it is
read; the user's own toggle still wins.

Merging this moves the bb-plugins pin, builds a new bb-server image and
stages a restart for every developer's server (Restart now, or the
03:00-05:00 ET quiet window).

Classification

owner-reviewed: it changes the default collapse behaviour of existing sidebar UI (a default people rely on changes meaning), per AGENTS.md's change classification.

What changes

• sidebar.ts: hasUnread(children, byId) walks a child tree and is true when any thread at any depth is unread; isUnread(t) is the shared predicate (isUnread, or an unread-success/unread-error indicator). countThreads moves here beside them (pure tree helpers).
• app.tsx: oneOffGlyph now calls the shared isUnread. SidebarThreadRow, SidebarEffortRow and the section SidebarCollapsible hold userOpen: boolean | null (null = untouched) and compute open = userOpen ?? (revealed || hasUnread(...)); a manual toggle always wins, and an untouched group follows unread state as it changes. sectionHasUnread covers an effort's root and its descendants for the section rows.
• The BB-85 navigation reveal (revealed) stays as part of the default.

Testing

• npm test in plugins/stewards: pass (723 tests; stewards is in the CI matrix).
• npm run typecheck in plugins/stewards: pass.
• Added hasUnread/isUnread unit tests in sidebar.test.ts.
• Added render tests in sidebar-app.test.tsx for all three levels: unread child row + Threads section, unread effort + Monitors section, all-read collapsed, untouched flip, manual collapse wins on new unread, manual expand survives everything read.
• Two existing tests that assumed an unread group stayed collapsed by default were updated to the new default.

[trotterdylan]

Independent review running in bb thread @thread:thr_fs936ra62u.

— from bb thread thr_gj9jqeu7tc

[trotterdylan]

Review round 1 (GLM 5.3, at d83b9b3)

Owner review required: yes — the sidebar's default collapse behaviour changes (a default people rely on changes meaning: groups that always started collapsed now start expanded when they contain unread threads). No decide-list entry in stewards/project.json matches the changed files (plugins/stewards/app.tsx, sidebar.ts and the two test files touch none of the path-scoped decide: project entries), so @trotterdylan, this one needs your approval.

Suggestions

• app.tsx (SidebarThreadRow/SidebarEffortRow): revealed is now only part of the default, so a manually collapsed group no longer re-expands when navigation later targets a thread inside it (BB-85's old useEffect forced it open on each reveal flip). The PR body declares this and the ticket says the manual toggle always wins, so it is not a blocker — just be aware BB-85's reveal now loses to a manual collapse permanently, not just until the reveal toggles.
• hasUnread re-walks the full subtree on every render of every row. Fine at sidebar scale, but if efforts ever grow deep child trees this is O(n·depth) per render pass.

Checked

• Ran: npm run typecheck and npm test in plugins/stewards locally — pass (21 files, 723 tests). Stewards is in the CI matrix, so CI covers it; no plugin suite was skipped by CI for this diff.
• Verified against ticket:
  • Criterion 1 (section/effort/child row with an unread descendant at any depth expanded on first render): met — sectionHasUnread (root + hasUnread over the effort tree) for the three sections, hasUnread(effort.children, byId) for effort rows, hasUnread(children, byId) for thread rows; render tests cover a Threads child row, a Monitors effort row and both sections.
  • Criterion 2 (all-read groups collapsed): met — default is false when nothing is unread and nothing is revealed; covered by the all-read render test and the updated "collapses Monitors and Recently ended until opened" test.
  • Criterion 3 (manual toggle wins in both directions): met — userOpen ?? (revealed || hasUnread(...)); tested for collapse-then-new-unread and expand-then-all-read.
  • Criterion 4 (untouched group flips with unread state): met — the default recomputes from live data on each realtime refetch; tested both flip directions.
  • Criterion 5 (unit tests for hasUnread in sidebar.test.ts, render tests for all three levels in sidebar-app.test.tsx, existing tests pass): met — isUnread covers the three unread shapes and the non-unread ones, hasUnread covers depth, empty tree and threads missing from byId (counted as read, documented); all 723 pass.
• Scope: nothing beyond the ticket — no persistence, no thread-list preference changes, unread semantics unchanged (oneOffGlyph now delegates to the same predicate it duplicated), and the countThreads move is behaviour-neutral. No bb core or contract files touched.

— from bb thread thr_fs936ra62u

[trotterdylan]

Addressed round 1 (pushed e34bbd2):

• Suggestion 1 (BB-85 reveal vs manual collapse): fixed in e34bbd2. I restored the reveal effect so revealed sets userOpen = true when navigation targets a thread inside a group, exactly as before; the unread default and the manual toggle still drive open otherwise. Added a render test that manually collapses an effort and then re-renders with activeThreadId on its worker, asserting it re-expands.
• Suggestion 2 (hasUnread re-walk cost): declined. Sidebar trees are a handful of workers/reviewers per effort, so the walk is negligible; memoizing per row would add state for no measured benefit.

Re-ran npm run typecheck and npm test in plugins/stewards: pass (724 tests).

— from bb thread thr_gj9jqeu7tc

[trotterdylan]

Review round 2 (GLM 5.3, at e34bbd2)

Owner review required: yes — unchanged from round 1: the sidebar's default collapse behaviour changes (a default people rely on changes meaning). No decide-list entry in stewards/project.json matches the changed files, so @trotterdylan, this one needs your approval.

The round 1 approval covers this push: it is the reveal-suggestion fix I asked for plus its test, nothing else. This marker repeats the verdict on the new head.

Resolved since round 1

• Suggestion 1 (a manual collapse permanently hid a group navigation later targeted, weakening BB-85): resolved at e34bbd2 — the reveal useEffect is back and writes userOpen sticky-true, so navigation re-expands a manually collapsed group, while the untouched default (userOpen ?? hasUnread(...)) is now purely unread-driven. Cleaner than my original suggestion, and the new render test covers collapse-then-navigate-reveals.
• Suggestion 2 (hasUnread walk cost): declined with a reason I accept at sidebar scale — fine.

Checked

• Ran: npm run typecheck and npm test in plugins/stewards at e34bbd2 — pass (21 files, 724 tests, the new reveal test included). Stewards is in the CI matrix.
• Verified against ticket: all five criteria still met at this head; the fix narrows the behaviour change (BB-85 reveal semantics restored) and touches no out-of-scope area.

— from bb thread thr_fs936ra62u

[trotterdylan]

Approved e34bbd2 from the Stewards page

[trotterdylan]

Merged: owner-reviewed, approved by trotterdylan on e34bbd2 (comment 2026-10-27T22:26:01Z); reviewer approved on the same head (round 2). Checks green. merge commit 791fb1b

This merge moves the bb-plugins pin, builds a new bb-server image, and stages a restart for every developer's server (Restart now, or the 03:00-05:00 ET quiet window).

— from bb thread thr_gj9jqeu7tc