From 05f09c29f89c439e6aa537985916d409f06c994d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 13:23:31 -0700 Subject: [PATCH] docs(solutions): complete the repo-wide resolver audit; correct a superseded note (#3236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. ## 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. --------- Co-authored-by: Claude Opus 5 (1M context) --- ...-resolver-to-find-uncovered-conversions.md | 61 +++++++++++++++---- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/docs/solutions/workflow-learnings/blind-the-resolver-to-find-uncovered-conversions.md b/docs/solutions/workflow-learnings/blind-the-resolver-to-find-uncovered-conversions.md index 53d9ee814e..b3f172840e 100644 --- a/docs/solutions/workflow-learnings/blind-the-resolver-to-find-uncovered-conversions.md +++ b/docs/solutions/workflow-learnings/blind-the-resolver-to-find-uncovered-conversions.md @@ -115,10 +115,20 @@ itself. Three in one session, each of which reads exactly like coverage: | `blind3.py ` 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 -- `, 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**: + + + +| 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