fix(glasses): the summary card reported five hardcoded lanes, so a renamed board read as empty - #2968
Conversation
…named board read as empty `boardSummaryCardFromCounts` interpolated `counts.triage`, `counts.todo`, `counts["in-progress"]`, `counts["in-review"]` and `counts.done`. `boardSummary` seeds exactly those five keys to 0 and then counts by real `task.column`, so on a board whose lanes are named anything else every value is the seeded 0: the summary card — the first card in every deck, and the entire body of `GET /board/summary` — reads "Triage 0 Todo 0 Doing 0 Review 0 Done 0" while the work sits in lanes it never names. It was also wrong on the default board: U11 (#2515) deleted the `triage` lane, so `Triage 0` was dead text occupying 9 of the 24 characters this display gets per line. Derived from `counts` rather than added as an optional resolved-lanes parameter: `counts` is already keyed by real column ids, so the vocabulary is in hand with no resolution and nothing that can be shipped unwired — the failure mode `unwired-lane-parameter-guard.test.ts` exists to catch. Reverted, the three new cases fail with the defect printed verbatim: 'Triage 0 Todo 0 Doing 0 Review 0 Done…' for a board holding four live cards. Verified: cards.test.ts 8/8; plugin suite 180 passed with 15 pre-existing agent-actions.test.ts failures that reproduce identically on clean origin/main; eslint, tsc, and the census/sql/fnxc ratchets green.
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
No automated review ran on this PR — the CodeRabbit returned The verification in the PR body is measured locally and stands on its own; treat the review status as absent, not passed. Happy to hand-walk any specific area if a human reviewer wants a second pair of eyes on the revert proof. |
…cards in review `statusBadge` returned `COLUMN_BADGES[column] ?? "todo"`, where `COLUMN_BADGES` mapped the six legacy ids to themselves. On a board whose lanes are named anything else every lookup missed and the badge read `todo` — a card sitting in review told the wearer it was un-started, and every card on the board carried the same badge. On a display with room for one word that is a confident wrong answer, which is worse than an unrecognised one. The badge IS the column id, so the function now says so, and the map goes with it: once the fallback is the id, a table mapping each legacy id to itself decides nothing, and six lane literals were sitting in this file doing no work. Behaviour on legacy boards is byte-identical either way — the map was an identity — so this is not a behaviour change riding along, it is the dead weight the fix exposed, removed rather than left as a decoy for the next reader. Mirrors `columnLabel` in the CLI (`COLUMN_LABELS[column] ?? column`): a board that calls its lane `checking` should read `checking`, which is true and already what its operator recognises. No resolution needed — the id is in hand at the call site. This is a miss in my own #2968. That PR fixed the summary card's counts in this same file and did not look one function further at the per-card badge those counts sit above — the practical form of the point I keep making about census counts: a file having had a defect fixed is not evidence about its neighbours, and this neighbour was nine lines away in a function I had read. Reverted: expected 'todo' to be 'checking'. Verified: plugin suite 198 passed / 19 files; tsc and eslint clean; census (unchanged — object keys are not comparisons), lane-wiring, inert-seam and fnxc gates green. No changeset — the plugin is private and not bundled into the published CLI.
…cards in review (#3015) ## Every card on a renamed board badged `todo` — including cards in review ```ts export function statusBadge(column: Task["column"]): string { return COLUMN_BADGES[column] ?? "todo"; } ``` `COLUMN_BADGES` maps the six legacy ids to themselves. On a board whose lanes are named anything else **every lookup misses**, so every card badges `todo` — a card sitting in review tells the wearer it is un-started, and the whole board carries one identical badge. On a display with room for a single word, that is worse than an unrecognised lane: it is a *confident wrong answer* rather than a missing one. Reached from `taskToCard` (the main card) and `notificationCard` (the notification badge). ## Fix, and the dead weight it exposed The badge **is** the column id, so the function now says so: ```ts export function statusBadge(column: Task["column"]): string { return column; } ``` That also retires `COLUMN_BADGES`. Once the fallback is the id, a table mapping each legacy id to *itself* decides nothing — six lane literals sat in this file doing no work. It was module-private with `statusBadge` as its only consumer and it was a pure identity, so behaviour on legacy boards is byte-identical: this is not a behaviour change riding along with a cleanup, it is the dead weight the fix exposed, removed rather than left as a decoy. Mirrors `columnLabel` in the CLI (`COLUMN_LABELS[column] ?? column`) for the same reason: a board that calls its lane `checking` should read `checking`. No resolution needed; the id is in hand at the call site. Note the census count for this file does **not** move — those six were object keys, not comparisons, which is exactly the scope the census documents for itself. ## This is a miss in my own #2968 That PR fixed the summary card's counts **in this same file** and never looked one function further at the per-card badge those counts sit above. Worth saying plainly, because it is the practical reminder behind the census finding I have been repeating all run: *a file having had a defect fixed is not evidence about its neighbours* — and here the neighbour was nine lines away, in a function I had read. ## Revert proof ``` AssertionError: expected 'todo' to be 'checking' Tests 1 failed | 9 passed (10) ``` The paired case ("still badges the legacy ids exactly as before") passes both ways by design — it guards against the fallback change altering known boards, so I am not counting it as coverage of the defect. ## Verification (measured) - plugin suite — **198 passed / 19 files** - `tsc --noEmit`, `eslint` — clean - `lifecycle-column-census --strict`, `check-lane-wiring`, `check-sql-column-literals`, `check-inert-flag-seams`, `check-fnxc-future-dates` — green No changeset: the plugin is `private: true` and is not bundled into the published CLI. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Status badges now accurately display a card’s actual lane, including previously unrecognized lanes. * Preserved existing badge behavior for known lanes. * **Tests** * Added coverage to verify accurate lane reporting and legacy behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
The defect
boardSummaryCardFromCountsbuilt its text from five hardcoded lane ids:`Triage ${counts.triage} Todo ${counts.todo} Doing ${counts["in-progress"]} Review ${counts["in-review"]} Done ${counts.done}`boardSummaryseeds exactly those five keys to0, then counts by realtask.column. On a board whose lanes are named anything else, every interpolated value is the seeded zero — so the summary card, which is the first card in every deck and the entire body ofGET /board/summary, reads:…while the real work sits in lanes it never mentions. The wearer is told the board is empty.
It is also wrong on the default board today: U11 (#2515) deleted the
triagelane, soTriage 0is permanently dead text — 9 of the 24 characters this display gets per line.Why there was no test
The only summary coverage in
cards.test.tsexercisedboardSummaryCard, an export no production file calls (its sole caller is that test). The function the deck actually ships had none. That gap is why five hardcoded ids survived here.The fix, and one deliberate choice
The lanes are derived from
countsinstead of taken as an optional resolved-lanes parameter.That is on purpose. This program's recurring defect is precisely the "optional lane answer + documented literal fallback" shape shipped without wiring the caller —
unwired-lane-parameter-guard.test.tsdocuments five live onmainat once, and in four of five the parameter was unreachable because the caller held a larger defect.countsis already keyed by realtask.columnvalues, so the vocabulary is in hand with no resolution, no new plumbing, and nothing that can be left unwired.Zero-count lanes are dropped so the scarce line budget goes to lanes with work; legacy ids keep their familiar labels and order, unknown ids sort after them alphabetically so output is deterministic.
countsitself is untouched — the route returns it as the API body and consumers still see every key.Revert proof
Restoring only
cards.tsfails all three new cases, printing the defect verbatim:Verification (measured)
cards.test.ts— 8/8 passedeslint,tsc --noEmit— cleanlifecycle-column-census --strict,check-sql-column-literals,check-fnxc-future-dates— greenNo changeset:
@fusion-plugin-examples/even-realities-glassesisprivate: trueand is not bundled into the published CLI.Pre-existing failure, NOT from this change
The plugin suite also has 15 failures in
src/__tests__/agent-actions.test.ts. They reproduce identically on cleanorigin/mainwith this branch stashed (15 failed | 41 passed), so they are not caused by this PR and are not fixed by it. Flagging separately — this suite is outside the merge gate, which is why it has been red unnoticed.