Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions codev/plans/911-vscode-backlog-tree-title-coun.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# PIR Plan: Backlog tree title-count reflects active view mode

## Understanding

The Backlog view's title is rendered as `Backlog (<count>)` at `packages/vscode/src/extension.ts:184`, where `<count>` is the total spawnable issue count (`spawnableBacklog(data.backlog).length`). Since #809 added a mine-only / show-all toggle, the tree below the title only renders the **current view mode's** rows — but the title-count still shows the unfiltered total in **both** modes.

Result: a fresh install opens to mine-only mode and shows a title like `Backlog (47)` while only 3 rows are visible — disorienting. The eye icon hints at the mode, but the title-count is the most prominent integer on the view, and right now it disagrees with what's visible.

Two coupled gaps to close:
1. **The count itself** — it always reads the unfiltered total, ignoring the filter mode.
2. **The refresh trigger** — `updateListViewTitles()` is wired into the overview-data listener at `extension.ts:243-248`, but flipping `codev.backlogShowAll` doesn't go through that listener. So even if (1) is fixed, the title would still stay stale until the next overview refresh fired.

## Proposed Change

Implement **option 2** from the issue's proposed-fix list (the architect's mild preference): `Backlog (3 of 47)` when mine-only, `Backlog (47)` when show-all. Surfaces both numbers; the "of 47" signals "there are more, click the eye to see them" without needing the user to click the toggle to confirm. Doesn't add a redundant mode label (the eye icon already labels the mode).

Edge case: when mine-only is active but `currentUser` is null (gh unavailable), `filterMine` is a no-op (per #809's safety branch), so visible count == total. In that case, fall back to plain `Backlog (47)` — no point rendering `Backlog (47 of 47)`.

Concretely:

1. **New pure helper** `visibleBacklogCount(data, showAll)` in `packages/vscode/src/views/backlog-filter.ts`. Takes overview data and the show-all flag; returns `{ visible, total }`. Mirrors `orderedSpawnable`'s filter chain (`spawnableBacklog` → conditionally `filterMine`) but only counts. Lives next to `filterMine` (same file, same vscode-free constraints) so it's unit-testable in the vitest harness.

2. **Title formatter** `formatBacklogTitle(visible, total)` — also in `backlog-filter.ts`, also vscode-free. Returns `Backlog` (no data), `Backlog (N)` (visible == total), or `Backlog (V of T)` (visible < total). Pure string function — cheap to test exhaustively.

3. **Wire the helper into `updateListViewTitles()`** at `extension.ts:178-186`. Replace the inline `spawnableBacklog(data.backlog).length` with a call to `visibleBacklogCount` + `formatBacklogTitle`. Read `codev.backlogShowAll` via the existing `readBacklogShowAll` reader (lifted from its current inner-scope position at line 358-359 to module scope or hoisted earlier in `activate`, so `updateListViewTitles` can see it — see Files to Change).

4. **Add `updateListViewTitles()` to the showAll-config-change listener** at `extension.ts:361-367`. Right now that listener calls `backlogProvider.refresh()` to redraw the tree rows; it also needs to call `updateListViewTitles()` so the title-count updates in lockstep.

Why not put the helper in `backlog.ts`? Because `backlog.ts` imports `vscode` (it defines `BacklogProvider` which extends a vscode interface). The whole point of `backlog-filter.ts` is to keep pure helpers vitest-testable without the vscode mock dance. Same precedent as `filterMine`.

## Files to Change

- `packages/vscode/src/views/backlog-filter.ts` — add `visibleBacklogCount(data, showAll)` and `formatBacklogTitle(visible, total)` helpers next to `filterMine`. The `data` parameter is typed as `Pick<OverviewData, 'backlog' | 'currentUser'>` so the helper has the minimum surface area it needs and is easy to test with bare objects.
- `packages/vscode/src/extension.ts:178-186` — `updateListViewTitles()` reads showAll and calls the new helpers for the backlog row.
- `packages/vscode/src/extension.ts:353-367` — re-order so `readBacklogShowAll` is declared *before* `updateListViewTitles` is wired (or hoist `readBacklogShowAll` to a top-of-`activate` const, matching how `readFileViewAsTree` is already structured). Add `updateListViewTitles()` to the showAll-config-change listener body.
- `packages/vscode/src/__tests__/backlog-filter.test.ts` — extend the existing test file with cases for `visibleBacklogCount` and `formatBacklogTitle`:
- mine-only + currentUser known: visible reflects filterMine
- mine-only + currentUser null: visible == total (the safety fallthrough)
- show-all: visible == total
- title format: no data → `Backlog`; equal counts → `Backlog (N)`; unequal counts → `Backlog (V of T)`
- spawnable-vs-raw: items with `hasBuilder: true` excluded from both `visible` and `total` (mirrors `spawnableBacklog`'s contract)
- empty backlog: returns `{ visible: 0, total: 0 }`, title is `Backlog (0)`

No changes to:
- `backlog.ts` itself — `orderedSpawnable` already computes the visible rows; we just need a parallel pure-count path that doesn't go through the provider class.
- `BacklogProvider.refresh()` — already fires on showAll-change (#809); the tree redraws in lockstep with the title once `updateListViewTitles()` is also called.

## Risks & Alternatives Considered

- **Risk: drift between the title count and the actual rendered rows.** The title helper and `BacklogProvider.orderedSpawnable` could diverge over time. Mitigation: both call `spawnableBacklog` and `filterMine` (the existing pure helpers) — the shared primitives are the source of truth. Mild duplication of the filter-chain shape (two callers), but no duplicated *logic*. Worth it to keep the title computation independent of instantiating the provider.
- **Risk: `currentUser` semantics.** `filterMine` is a no-op when `currentUser` is null. The acceptance criterion says "title shows the full count, matching the visible rows" in that case. The proposed `formatBacklogTitle(visible, total)` handles this naturally — when `visible == total`, the format is `Backlog (N)`, no "of" suffix. Verified in the test plan.
- **Risk: showAll-change listener fires before the views are constructed.** `updateListViewTitles` already guards with `if (backlogView)` so a too-early call is a no-op. Safe.
- **Alternative: emit a `visibleCount` via `BacklogProvider`'s `onDidChangeTreeData`.** Rejected — would couple the title-update to the provider's internal change emitter and require either a custom event or recomputing the visible count in the provider. The pure-helper approach keeps the title formatter independent of provider lifecycle.
- **Alternative: title format option 3 (`Backlog · Mine (3)`).** Rejected per the issue's stated mild preference for option 2. Option 3 duplicates info that the eye icon already conveys.
- **Alternative: title format option 1 (`Backlog (3)` flat).** Rejected because it loses the "you have a filter on, the full set is bigger" affordance that option 2 surfaces. Acceptable but less informative.

## Test Plan

### Unit tests (vitest, `packages/vscode/src/__tests__/backlog-filter.test.ts`)

- `visibleBacklogCount`:
- showAll=true → visible == total == spawnable count
- showAll=false, currentUser known → visible == filterMine'd spawnable count, total == spawnable count
- showAll=false, currentUser null → visible == total (safety fallthrough)
- items with `hasBuilder: true` excluded from both counts
- empty `data.backlog` → `{ visible: 0, total: 0 }`
- `formatBacklogTitle`:
- `(undefined, undefined)` → `Backlog` (no-data fallback)
- `(5, 5)` → `Backlog (5)`
- `(3, 47)` → `Backlog (3 of 47)`
- `(0, 0)` → `Backlog (0)` (empty is still a known state, not the no-data case)
- `(0, 5)` → `Backlog (0 of 5)` (mine-only with nothing assigned to you — title still shows you what's available)

Run: `pnpm --filter @cluesmith/codev test` (vitest unit suite).

### Manual verification at the dev-approval gate

1. Reviewer runs `afx dev pir-911` from the main checkout.
2. Open Codev sidebar → Backlog view.
3. **Default mode (mine-only):** verify title reads `Backlog (V of T)` where V matches the row count and T matches the total spawnable backlog. If you have nothing assigned, expect `Backlog (0 of T)` — and the empty-state placeholder row.
4. **Toggle the eye icon to show-all:** title immediately updates to `Backlog (T)`, row count matches.
5. **Toggle back to mine-only:** title immediately reverts to `Backlog (V of T)`.
6. **`currentUser` unavailable case (harder to reproduce):** when no GitHub auth, mine-only falls through to show-all per #809's branch. Title should read `Backlog (T)`, not `Backlog (T of T)`.

### Cross-platform

VSCode extension code — same behavior on macOS / Linux / Windows VSCode hosts. No platform-specific paths.

### No regression

- Builders view title still reads `Builders (N)`.
- Pull Requests view title still reads `Pull Requests (N)`.
- Recently Closed view title still reads `Recently Closed (N)`.
- Empty-state placeholder for mine-only still renders when mine-only + currentUser known + no matches (unchanged from #809).
30 changes: 30 additions & 0 deletions codev/projects/911-vscode-backlog-tree-title-coun/status.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
id: '911'
title: vscode-backlog-tree-title-coun
protocol: pir
phase: verified
plan_phases: []
current_plan_phase: null
gates:
plan-approval:
status: approved
requested_at: '2026-05-28T08:18:39.996Z'
approved_at: '2026-05-28T08:21:30.631Z'
dev-approval:
status: approved
requested_at: '2026-05-28T08:27:03.161Z'
approved_at: '2026-05-28T09:10:15.134Z'
pr:
status: approved
requested_at: '2026-05-28T09:14:32.452Z'
approved_at: '2026-05-28T09:15:21.222Z'
iteration: 1
build_complete: false
history: []
started_at: '2026-05-28T08:16:25.739Z'
updated_at: '2026-05-28T09:15:32.050Z'
pr_history:
- phase: review
pr_number: 914
branch: builder/pir-911
created_at: '2026-05-28T09:11:36.159Z'
pr_ready_for_human: false
62 changes: 62 additions & 0 deletions codev/reviews/911-vscode-backlog-tree-title-coun.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# PIR Review: vscode Backlog title-count reflects the active view mode

Fixes #911

## Summary

The Backlog tree title used to read `Backlog (47)` (the unfiltered spawnable total) even when the view was in mine-only mode showing just 3 rows — the most prominent integer on the view disagreed with what was rendered below. This PR rewrites the title to `Backlog (3 of 47)` in mine-only mode and `Backlog (47)` in show-all mode (option 2 from the issue), and wires the title-update into the `codev.backlogShowAll` config-change listener so it refreshes in lockstep with the tree rather than waiting for the next overview tick.

## Files Changed

- `packages/vscode/src/views/backlog-filter.ts` (+54 / -3) — new `visibleBacklogCount` and `formatBacklogTitle` helpers; `spawnableBacklog` moved in from `backlog.ts` so the vscode-free module owns all three primitives the title and the provider share.
- `packages/vscode/src/views/backlog.ts` (+5 / -10) — drops the local `spawnableBacklog`; imports + re-exports it from `backlog-filter.ts` so existing call sites keep working.
- `packages/vscode/src/extension.ts` (+22 / -4) — `updateListViewTitles` now calls `visibleBacklogCount` + `formatBacklogTitle` for the backlog row; `readBacklogShowAll` hoisted above the title helper; showAll-config-change listener also calls `updateListViewTitles()`.
- `packages/vscode/src/__tests__/backlog-filter.test.ts` (+131 / -1) — coverage for `spawnableBacklog`, `visibleBacklogCount`, `formatBacklogTitle`. Existing `filterMine` coverage preserved.
- `codev/plans/911-vscode-backlog-tree-title-coun.md` (new, +94) — approved plan.
- `codev/state/pir-911_thread.md` (new, +25) — builder thread.

## Commits

- `b6e98cf4` [PIR #911] Plan draft
- `8ee5e308` [PIR #911] Thread: plan phase complete
- `c614bf76` [PIR #911] Backlog title-count reflects active view mode
- `9512bbd8` [PIR #911] Thread: implement phase complete

(porch-managed phase-transition / gate commits omitted.)

## Test Results

- `pnpm check-types`: ✓ pass
- `pnpm lint`: ✓ pass (eslint, zero warnings)
- `pnpm test:unit`: ✓ pass — 113 tests across 10 files, ~20 new for the title-count path
- `pnpm package` (esbuild bundle, production): ✓ pass
- Manual verification at the `dev-approval` gate: ✓ approved by the human

## Architecture Updates

No arch changes needed. This PR works inside the existing Backlog-view module boundary (`backlog.ts` provider + `backlog-filter.ts` pure helpers) — the only structural shift is moving `spawnableBacklog` from `backlog.ts` (vscode-dependent) into `backlog-filter.ts` (vscode-free) so the title helper can reuse it from the vitest harness. That mirrors the precedent `filterMine` established in #809; not a new pattern.

## Lessons Learned Updates

No new durable lessons. The "keep vscode-free helpers in `backlog-filter.ts` so vitest can test them without vscode mocks" pattern is already documented implicitly by `filterMine`'s existence — this PR just continues that pattern. A re-statement in `lessons-learned.md` would be churn, not signal.

One contextual observation worth recording in the PR body (not the lessons file): two coupled bugs were closed at once — the count itself, and the refresh trigger. The count being wrong would have been obvious; the refresh trigger being wrong would have been masked by the periodic overview poll (the title would have updated within 60s of any showAll toggle, so it looked "almost right" most of the time). Worth knowing for future title/badge-counter work: any view title that depends on user-toggleable state needs to be refreshed on the toggle's listener, not just on the data-change listener.

## Things to Look At During PR Review

- **`visibleBacklogCount`'s `currentUser` fallthrough** (`backlog-filter.ts`). When mine-only is active but `currentUser` is null (gh unavailable), `filterMine` is a no-op, so `visible == total`. The title helper short-circuits this to plain `Backlog (T)` rather than `Backlog (T of T)` — matches the safety branch in `BacklogProvider.orderedSpawnable` and the third acceptance criterion in the issue.
- **Hoisting `readBacklogShowAll`** in `extension.ts`. The reader was previously declared inside the backlog-toggle listener block (around line 358 pre-change). The hoist moves it above `updateListViewTitles` so the title helper can call it on every refresh. The original site (now the listener) uses the hoisted reader.
- **Title format choice**. Option 2 (`Backlog (V of T)`) was the issue's mild preference. Options 1 and 3 are also acceptable and the helper could be swapped to either with a one-line change in `formatBacklogTitle`. Worth one re-read by the architect at PR review to confirm.
- **`src/test/backlog.test.ts` left in place**. The vscode-test (Electron) harness still has the old `spawnableBacklog` test file; it imports from `backlog.ts` which re-exports the moved helper, so it still passes. Deleting it would be cleanup but goes beyond the issue's "purely a count/title-string fix" scope.

## How to Test Locally

For reviewers pulling the branch:

- **View diff**: VSCode sidebar → right-click builder pir-911 → **Review Diff** (auto-detects the default branch)
- **Run dev server**: VSCode sidebar → **Run Dev Server**, or `afx dev pir-911`
- **What to verify** (mapped from the plan's Test Plan):
- Open the Backlog view in mine-only mode (default): title reads `Backlog (V of T)` where V matches the row count and T matches the unfiltered spawnable total. With nothing assigned to you, expect `Backlog (0 of T)` plus the empty-state placeholder from #809.
- Toggle the eye icon to show-all: title immediately becomes `Backlog (T)`, row count matches.
- Toggle back to mine-only: title immediately reverts to `Backlog (V of T)`. No stale count waiting for the next overview tick.
- Other tree titles (`Builders`, `Pull Requests`, `Recently Closed`) still render with their existing `(N)` format — no regression.
34 changes: 34 additions & 0 deletions codev/state/pir-911_thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# PIR #911 — vscode backlog title-count

Issue: vscode Backlog tree title-count should reflect the active view mode, not the total spawnable set. Surfaced as a CMAP-2 nit on #910 / PIR #809.

## Plan phase

Wrote `codev/plans/911-vscode-backlog-tree-title-coun.md`. Picked option 2 (`Backlog (3 of 47)` mine-only / `Backlog (47)` show-all) per the architect's mild preference. Two pure helpers go into `backlog-filter.ts`:
- `visibleBacklogCount(data, showAll)` — mirrors `BacklogProvider.orderedSpawnable`'s filter chain (`spawnableBacklog` → conditionally `filterMine`) but only counts.
- `formatBacklogTitle(visible, total)` — returns `Backlog` / `Backlog (N)` / `Backlog (V of T)`.

Two coupled bugs being closed at once: (1) the count itself was unfiltered; (2) `updateListViewTitles` is wired only to the overview-data listener, so even a correct count would stay stale until the next overview tick when the user flipped showAll. Plan addresses both — the showAll-config-change listener at `extension.ts:361-367` will also call `updateListViewTitles()`.

Plan-approval gate granted.

## Implement phase

- Moved `spawnableBacklog` into `backlog-filter.ts` so the count helper and `BacklogProvider` share the same primitive without dragging `vscode` into the vitest harness. `backlog.ts` re-exports it (and uses it internally) so existing import paths in `extension.ts` and `src/test/backlog.test.ts` keep working.
- Added `visibleBacklogCount(data, showAll)` and `formatBacklogTitle(visible, total)` to `backlog-filter.ts`.
- `extension.ts`:
- Replaced the inline `spawnableBacklog(data.backlog).length` title computation with `visibleBacklogCount` + `formatBacklogTitle`.
- Hoisted `readBacklogShowAll` above `updateListViewTitles` so it can be read on every refresh.
- Wired `updateListViewTitles()` into the `codev.backlogShowAll` config-change listener so the title updates in lockstep with the tree (closes bug 2 from the plan).
- Tests: extended `__tests__/backlog-filter.test.ts` with new coverage for `spawnableBacklog`, `visibleBacklogCount`, and `formatBacklogTitle`. Vitest reports 113 passed (10 files), check-types and lint clean, esbuild bundle clean.

Dev-approval gate granted.

## Review phase

PR #914 opened with `codev/reviews/911-vscode-backlog-tree-title-coun.md` as the body. 3-way consultation single pass:
- Gemini: APPROVE (HIGH)
- Codex: APPROVE (HIGH)
- Claude: APPROVE (HIGH)

All three flagged zero issues. The `spawnableBacklog` move from `backlog.ts` into `backlog-filter.ts` was noted by Claude as a necessary deviation from the plan's "no changes to backlog.ts" line — the helper had to migrate to keep the vscode-free guarantee of `backlog-filter.ts`, and `backlog.ts`'s re-export preserves the existing import paths. Awaiting human pr-gate approval.
Loading
Loading