docs(solutions): complete the repo-wide resolver audit; correct a superseded note (#3236)
## What Completes the repo-wide resolver audit and **corrects a note of mine that had gone stale**. Docs only. Every `resolveProjectColumnsForRoles` call site in the repository has now been blinded individually. ## Final results | package | sites | outcome | |---|---|---| | `engine` | 10 files | scheduler, triage, evaluator uncovered → pinned; executor, restart-recovery, notification already covered; self-healing 21 pinned / 1 inert | | `core` | 14 | 9 covered, **5 uncovered → all 5 pinned** (#3225, #3227, #3233, #3234, #3235) | | `dashboard` | 4 | `register-task-workflow-routes.ts:1268` covered; `server.ts` ×3 flagged | | `cli` | 1 | flagged | ## The correction A note recorded `workflow-analytics.ts` and `team-analytics.ts` — 4 resolvers — as **unmeasurable**, because `pgDescribe` probes TCP and the `.pg` suites skip without it. The caution is real and stays: a skipped suite reads exactly like a passing one. But on an environment where those suites **do** run, all 4 were measured, and `team-analytics.ts` turned out to have a half-covered pair — `completeLanes` covered, **`activeLanes` not** — in a file named `team-analytics-renamed-lanes`. That is now pinned (#3227, merged). Left standing, the note converts a real finding into a **permanent excuse for not looking**. It now says: confirm the suite actually skips *here* before recording a site as unmeasurable for environment reasons. ## A fourth measurement failure mode — the opposite direction The three already recorded all produce false *uncovered*. This one produces false *covered*: **A COVERED verdict needs a baseline.** The dashboard sweep reported 5 failing files under the global blind. **4 of them fail on clean `main`** and have nothing to do with lanes — a docs-inventory test and a model-routes test among them. Read as-is, that is four resolvers falsely credited as covered. Only `register-task-workflow-routes.awaiting-planning.test.ts` passes clean and fails blinded, so it is the sole real detector. Second time today a baseline changed a conclusion (the first found a genuine red on main, #3229). ## Why 4 sites are flagged rather than pinned - **`server.ts:1922/1923/1938`** — inside the `/api/health/reliability` route closure. No route-level test exists, and the only way in is booting `createServer(store)` behind a mock-the-world shell, which the slow-test rule forbids. The alternative is a refactor to expose a seam — its own commit, since moving code and changing behaviour do not ride together. (A note already in this doc reached the same conclusion independently; this confirms it by measurement.) - **`cli/commands/task.ts:660`** — worth its own warning. Extracting a pure helper and testing it **would look like coverage and would not be**: blinding the resolver leaves such a test green, because the helper *receives* the lane set rather than resolving it. The uncovered thing is the resolve call, not the decision it feeds. Its sibling test file already records the same limit honestly for `boardColumnsForDisplay`. ## Reported, not fixed: 4 pre-existing red dashboard files on main `lazy-loaded-views-docs.test.ts` (AGENTS lazy-view inventory drifted — 24 actual vs 18 documented), `ResearchView.test.tsx`, `planning-browser-e2e.test.ts`, `register-model-routes-kimi-k3-supplemental.test.ts` — 7 failing tests, all in the non-blocking suite. I am not fixing them here: the lazy-views inventory is a curated list other workers are actively adding to, and rewriting it mid-flight would collide. Flagging so it is visible rather than silently absorbed into my blind's noise. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated workflow guidance to require baseline comparisons and verification that all relevant tests run. * Added safeguards for detecting ineffective changes and distinguishing pre-existing failures. * Expanded PostgreSQL audit documentation with measured coverage results, including uncovered resolver paths. * Recorded completed coverage sweeps across core, dashboard, and CLI areas, including pinned and non-pinnable sites. * Clarified limitations when testing extracted decision helpers instead of resolver calls. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -115,10 +115,20 @@ itself. Three in one session, each of which reads exactly like coverage:
|
||||
| `blind3.py <file> <var>` with an unmapped role | the script exited non-zero **silently**, `&&` skipped the check, `;` let vitest run against **unmodified source**. Reported 375/375 green "under blinding" with nothing blinded. |
|
||||
| `vitest run src/__tests__/notification` | missed `src/notification/__tests__/` entirely — a nested `__tests__` the glob never reached. Reported covered code as uncovered. |
|
||||
|
||||
So: **an UNCOVERED verdict is a claim about the whole tree and needs the whole tree's tests.** Before
|
||||
believing one, confirm (a) the blind actually modified the file — `git diff --stat`, not the tool's
|
||||
exit code, and (b) the run included every test file that imports the module, including nested
|
||||
`__tests__` directories. A tool that can no-op must say what it changed and fail loudly when it
|
||||
A fourth, in the opposite direction: **a COVERED verdict needs a baseline.** The dashboard sweep
|
||||
reported 5 failing files under the global blind; **4 of them fail on clean `main`** and had nothing
|
||||
to do with lanes (a docs-inventory test, a model-routes test). Read as-is, that is four resolvers
|
||||
falsely credited as covered. Run the failing set again *unblinded* and subtract — whatever fails in
|
||||
both is noise. Pre-existing red is common enough in a non-blocking suite that this is not an edge
|
||||
case; it is the second time today it changed a conclusion.
|
||||
|
||||
So: **an UNCOVERED verdict is a claim about the whole tree and needs the whole tree's tests**, and a
|
||||
COVERED verdict needs a baseline to subtract. Before believing either, confirm (a) the blind actually
|
||||
modified the TARGET file — `git diff --stat -- <path/to/target>`, not a bare `git diff --stat` and not
|
||||
the tool's exit code, since an unscoped diff reports any unrelated edit in the tree as though it were
|
||||
the blind's work, (b) the run included every test file
|
||||
that imports the module, including nested `__tests__` directories, and (c) the failures you are
|
||||
crediting do not also fail without the blind. A tool that can no-op must say what it changed and fail loudly when it
|
||||
cannot; the same standard this program applies to product guards applies to the audit's own
|
||||
instruments.
|
||||
|
||||
@@ -145,10 +155,14 @@ Three groups resisted blinding for reasons worth writing down, so the next perso
|
||||
them. Recording *why* a site cannot be measured is a result — the same stance #3212 took for a site
|
||||
that cannot be covered.
|
||||
|
||||
- **No TCP PostgreSQL.** `workflow-analytics.ts` and `team-analytics.ts` (4 resolvers) keep their
|
||||
renamed-lane coverage in `.pg` suites. `pgDescribe` probes **TCP**; `pg_isready` succeeding on a
|
||||
**Unix socket** is not the same thing, and mistaking one for the other turns 4 skipped suites into
|
||||
4 false "uncovered" readings.
|
||||
- **~~No TCP PostgreSQL.~~ SUPERSEDED — these were measurable and one hid a real gap.** The caution
|
||||
itself stands: `pgDescribe` probes **TCP**, `pg_isready` on a **Unix socket** is not the same
|
||||
thing, and a skipped suite reads exactly like a passing one. But on an environment where the `.pg`
|
||||
suites do run, all 4 resolvers were measured: `workflow-analytics.ts` has **both** halves covered,
|
||||
`team-analytics.ts` has `completeLanes` covered and **`activeLanes` uncovered** — a half-covered
|
||||
pair in a file named `team-analytics-renamed-lanes`, now pinned. **Before recording a site as
|
||||
unmeasurable for environment reasons, confirm the suite actually skips here** — otherwise the note
|
||||
converts a real finding into a permanent excuse for not looking.
|
||||
- **No injectable seam.** `reads.ts`'s incremental-sync scan composes Drizzle conditions against
|
||||
`layer.db` directly. A test there would assert the query that was built rather than the rows that
|
||||
were excluded — green, and blind to the bug.
|
||||
@@ -197,9 +211,34 @@ ones above, where a harness runs the code but cannot see the difference: **no am
|
||||
helps when the entry point is never called.** Check that something exercises the code at all before
|
||||
concluding a green blind means anything.
|
||||
|
||||
**`packages/core`'s 17 files are entirely unaudited** — including `store.ts`'s wip read behind the
|
||||
engine-downtime timing shift and the archived reads in `task-store/reads.ts`. Nothing is known about
|
||||
whether they are pinned; that is a gap in the audit, not a clean bill.
|
||||
`packages/core` is now fully blinded too — **15 sites, 9 already covered, 6 uncovered, all 6
|
||||
pinned**:
|
||||
|
||||
<!-- The `branch-and-pr-entities.ts` row carries TWO sites (`:445` and `:484`); counting the rows
|
||||
rather than the sites is what made this read 14/5. -->
|
||||
|
||||
| site | verdict |
|
||||
|---|---|
|
||||
| `store.ts:874` (downtime wip read) | uncovered → pinned |
|
||||
| `store.ts:1135` (open-undo finished lanes) | uncovered → pinned |
|
||||
| `branch-and-pr-entities.ts:445` / `:484` | uncovered → pinned |
|
||||
| `async-mission-store.ts:1179` (archived) | uncovered → pinned; `:1178` (complete) was covered |
|
||||
| `task-id-integrity.ts:502` (lineage gate) | uncovered → pinned |
|
||||
| `reads.ts` ×3, `productivity`/`github`/`gitlab` analytics, `eval-automation`, `task-artifacts-ops` | already covered |
|
||||
|
||||
`packages/dashboard` and `packages/cli` complete the sweep — **1 covered, 4 flagged**:
|
||||
|
||||
| site | verdict |
|
||||
|---|---|
|
||||
| `register-task-workflow-routes.ts:1268` (hold) | already covered |
|
||||
| `server.ts:1922` / `:1923` / `:1938` | uncovered, **not pinnable without a mock-the-world shell** (see the route-closure note above) |
|
||||
| `cli/commands/task.ts:660` | uncovered; the glyph decision is inline in `runTaskList`, whose own test file records that driving it needs the same forbidden shell |
|
||||
|
||||
The `cli` site deserves one warning. Extracting a pure helper and testing it would look like coverage
|
||||
and **would not be** — blinding the resolver leaves such a test green, because the helper receives
|
||||
the set rather than resolving it. The uncovered thing is the *resolve call*, not the decision.
|
||||
|
||||
Every `resolveProjectColumnsForRoles` call site in the repository has now been blinded individually.
|
||||
|
||||
## When you cannot pin it, say so at the site
|
||||
|
||||
|
||||
Reference in New Issue
Block a user