From 3146a745bf4e75465b22853a3d1c50cdf2713ee9 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 11:50:25 -0700 Subject: [PATCH] test(engine): cover two reporter resolvers that no test could tell from the literal (#3217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Applied #3214's blinding procedure **outside `self-healing.ts`**, where that measurement has never been run. Two of the five resolvers across the two reporters were uncovered; this covers both. ## The measurement One resolver at a time, blinded back to its legacy ids, against each file's existing suite: | site | blinded to | result | |---|---|---| | `backlog-pressure-reporter.ts:87` hold | `["todo"]` | 2 failed — covered | | **`backlog-pressure-reporter.ts:88` wip** | `["in-progress"]` | **0 failed of 11 — UNCOVERED** | | `backlog-pressure-reporter.ts:89` terminal | `["done","archived"]` | 1 failed — covered | | `stale-task-reporter.ts:59` wip | `["in-progress"]` | 1 failed — covered | | **`stale-task-reporter.ts:60` review** | `["in-review"]` | **0 failed of 7 — UNCOVERED** | Both uncovered resolvers sit in a `Promise.all` **beside one that is covered**, so each sweep reads as converted while half of it was held by nothing. That is rule 1 in the doc — coverage is per-resolver, not per-sweep — and it is why the census cannot answer this: a syntactic scan sees five resolved sites and five is what it counts. `stale-task-reporter.ts` is the sharper case. Its describe block **already declared `signoff` in the fixture IR** and no case ever put a card there, so the review resolver was decorative. ## What they cost on a renamed board - **wip** feeds `inProgressCount`, the *denominator* of `ratio = todoCount / max(inProgressCount, 1)`. Against the literal, busy work in a renamed lane counts as **zero**, the ratio inflates, and the backlog-pressure alert fires on a queue that is draining normally — the operator is paged that the board is jammed while agents work through it. - **review** decides which rows the staleness read *fetches at all*. A review stalled for days in a renamed lane is never queried and never surfaced — precisely the condition this reporter exists to report. ## Following the four rules **Rule 2 — the fixture reaches the guarded branch.** 12 hold cards over 2 wip cards is a ratio of 6, *under* the default threshold of 10, so the correct answer is "no alert"; blinding collapses the denominator to 1, the ratio becomes 12, and it alerts. A fixture whose ratio cleared the threshold either way would exercise the sweep and never touch the line under test. **Rule 3 — assert the path-specific side effect.** `upsertInsight` not called, and `logEntry` called with `column=signoff`. Asserting `alerted === false` alone would also pass if the run bailed for an unrelated reason — missing insight store, cooldown, too few candidates — none of which involve the wip lane. **Rule 4 — the store fake honours `options.column`.** Both harnesses already did; reused rather than replaced. Each new case is paired with a negative so it cannot pass vacuously: the "does not alert" case is backed by a *same renamed board still alerts when in-progress work really is thin* case, so a reporter broken into never firing fails. ## Census **Unchanged — `CONVERSION QUEUE EMPTY`, `AVAILABLE: 0` before and after.** This converts nothing. It closes coverage on conversions the census already counts as done, which is the gap #3214 names: *"the census counts comparisons; it cannot tell a working conversion from one a later merge silently reverted."* ## Verification Blind-verified in both directions — blinding each resolver fails **exactly** the new case and nothing else: ``` backlog-pressure BLIND wip -> 1 failed | 12 passed (13) restored: 13 passed stale-task BLIND review -> 1 failed | 7 passed (8) restored: 8 passed combined 21 passed (2 files) ``` No changeset: test-only, behavior-preserving, no published-package surface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .../backlog-pressure-reporter.test.ts | 85 +++++++++++++++++++ .../src/__tests__/stale-task-reporter.test.ts | 30 +++++++ 2 files changed, 115 insertions(+) diff --git a/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts b/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts index efe0a5d732..2c892fea2a 100644 --- a/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts +++ b/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts @@ -359,4 +359,89 @@ describe("backlog pressure resolves the board's own lanes", () => { /* The paired negative: a dependency still waiting must still block. */ expect(ids).not.toContain("FN-BLOCKED"); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-18:02 (found by BLINDING, not by the census): + THE WIP RESOLVER IN THIS REPORTER WAS UNCOVERED — the census counts it as converted, and nothing + in the tree could tell it from the literal it replaced. + + Measured with the #3214 procedure, one resolver at a time against this file's 11 cases: + + hold -> ["todo"] 2 failed covered + wip -> ["in-progress"] 0 failed UNCOVERED + terminal -> ["done","archived"] 1 failed covered + + Every pre-existing renamed-board case here supplies `inProgressSlim` under the resolved wip lane + AND asserts an outcome the wip count does not change, so blinding that one resolver was invisible. + + WHAT IT COSTS ON A RENAMED BOARD, which is why this is a real alert bug and not bookkeeping: + `wipColumns` feeds `inProgressCount`, the DENOMINATOR of + `ratio = todoCount / max(inProgressCount, 1)`. Against the literal, a board whose wip lane is + `brewing` matches no rows, so busy in-progress work counts as ZERO, the ratio inflates to + `todoCount`, and the backlog-pressure alert fires on a queue that is draining normally. The + operator is paged that the board is jammed while agents are working through it. + + THE FIXTURE REACHES THE BRANCH (rule 2): 12 hold cards over 2 wip cards is a ratio of 6, UNDER the + default threshold of 10, so the correct answer is "no alert". Blinded, the denominator collapses to + 1 and the ratio becomes 12 — over the threshold, past the candidate gate, and alerting. A fixture + whose ratio cleared the threshold either way could not see this. + + ASSERTED ON THE SIDE EFFECT (rule 3): `upsertInsight` is what the alerting path DOES. Asserting + only `alerted === false` would also pass if the run bailed early for an unrelated reason — a + missing insight store, a cooldown hit, too few candidates — none of which involve the wip lane. + */ + it("does NOT alert when a renamed WIP lane holds the in-progress work", async () => { + /* 12 waiting cards in the renamed hold lane. */ + const todoSlim = Array.from({ length: 12 }, (_, i) => + createTask({ id: `FN-W${i}`, column: "drafting" })); + /* 2 cards genuinely in progress, in the renamed wip lane the literal cannot see. */ + const inProgressSlim = [ + createTask({ id: "FN-BREW-1", column: "brewing" }), + createTask({ id: "FN-BREW-2", column: "brewing" }), + ]; + /* Dependency-free, so the blinded run reaches the alert rather than stopping at candidates. */ + const todoFull = todoSlim; + const insightStore = { upsertInsight: vi.fn(), listInsights: vi.fn().mockResolvedValue([]) }; + const reporter = new BacklogPressureReporter({ + store: createStore({ + todoSlim, inProgressSlim, todoFull, allTasks: todoSlim, insightStore, + workflowIr: RENAMED_DEPENDENCY_IR, holdColumn: "drafting", wipColumn: "brewing", + }), + projectId: "/tmp/project", + logger: { warn: vi.fn(), error: vi.fn() }, + now: () => Date.parse("2026-05-18T12:00:00.000Z"), + }); + + const result = await reporter.report(); + + /* 12 / 2 = 6, under the threshold of 10. Against the literal this is 12 / 1 and alerts. */ + expect(result).toEqual({ alerted: false, reason: "under-threshold" }); + expect(insightStore.upsertInsight).not.toHaveBeenCalled(); + }); + + /* + THE PAIRED POSITIVE. The case above is a "does not fire" assertion, which a reporter that never + fired at all would satisfy — including one broken so badly it always returns under-threshold. This + pins that the SAME renamed board still alerts when the ratio genuinely warrants it, so the case + above is measuring the wip lane rather than a dead reporter. + */ + it("still alerts on the same renamed board when in-progress work really is thin", async () => { + const todoSlim = Array.from({ length: 12 }, (_, i) => + createTask({ id: `FN-T${i}`, column: "drafting" })); + const insightStore = { upsertInsight: vi.fn(), listInsights: vi.fn().mockResolvedValue([]) }; + const reporter = new BacklogPressureReporter({ + store: createStore({ + todoSlim, inProgressSlim: [], todoFull: todoSlim, allTasks: todoSlim, insightStore, + workflowIr: RENAMED_DEPENDENCY_IR, holdColumn: "drafting", wipColumn: "brewing", + }), + projectId: "/tmp/project", + logger: { warn: vi.fn(), error: vi.fn() }, + now: () => Date.parse("2026-05-18T12:00:00.000Z"), + }); + + const result = await reporter.report(); + + expect(result.alerted).toBe(true); + expect(insightStore.upsertInsight).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/engine/src/__tests__/stale-task-reporter.test.ts b/packages/engine/src/__tests__/stale-task-reporter.test.ts index 2847fb19e4..3229317bd5 100644 --- a/packages/engine/src/__tests__/stale-task-reporter.test.ts +++ b/packages/engine/src/__tests__/stale-task-reporter.test.ts @@ -162,6 +162,36 @@ describe("stale-task reporting resolves the board's own lanes", () => { expect(result.surfaced).toBeGreaterThan(0); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-18:14 (found by BLINDING — the review half was uncovered): + This describe already declared `signoff` in its IR and no case ever put a card there, so the + REVIEW resolver was doing nothing any test could see. Measured with the #3214 procedure against + this file's 7 cases: + + wipColumns -> ["in-progress"] 1 failed covered + reviewColumns -> ["in-review"] 0 failed UNCOVERED + + Rule 1 in that doc is exactly this: coverage is PER-RESOLVER, not per-sweep. Both resolvers sit in + one `Promise.all` and read as a single converted sweep; only one of them was held by anything. + + WHAT IT COSTS: `reviewColumns` decides which rows the staleness read even FETCHES. Against the + literal, a card parked in a renamed review lane is never queried, so a review that has silently + stalled for days is never surfaced — the precise condition this reporter exists to report. + + The card must be stale by the IN-REVIEW thresholds, which the harness above already supplies; a + fixture leaning on the in-progress ones would surface through the wip path and prove nothing. + */ + it("surfaces a stale card sitting in a RENAMED review lane", async () => { + const store = renamedStore({ signoff: [staleCard("FN-R", "signoff")] }); + const reporter = new StaleTaskReporter({ store, now: () => NOW }); + + const result = await reporter.report(); + + expect(result.surfaced).toBeGreaterThan(0); + /* Path-specific: the surfacing side effect names the lane the card is actually in. */ + expect(store.logEntry).toHaveBeenCalledWith("FN-R", expect.stringContaining("column=signoff")); + }); + it("keeps surfacing legacy-board cards when no workflow resolves", async () => { /* My first version of this case asserted that a card in `in-progress` is surfaced on the RENAMED