docs(learnings): a blocker described four times, wrong twice — instrument before you file (#3200)
Records the method that moved a `triage.ts` site flagged unconvertible for four cycles. The method transfers; the three conversions do not. ## Four mechanisms, split by derivation rather than care | # | claimed mechanism | derived from | held? | |---|---|---|---| | 1 | merged intake/hold vocabularies | reading | no | | 2 | orphan arm scoped to `source === "selection"` | reading + one test run | partly | | 3 | provenance verifies by `ir.id`, which builtins lack | reading a **comment** | **no — filed as #3187, closed as wrong** | | 4 | two test harnesses cannot answer a selection query | instrumented isolation | **yes** | (3) is the expensive one. The text I quoted was **historical prose describing code that had been removed**, sitting directly above a paragraph saying exactly that. I read a rationale as an implementation, and it reached an issue other lanes could have acted on. ## The isolation took three runs ``` flag only, no conversion 8 passed -> the orphan arm is not the cause flag + conversion 5 failed -> the conversion is same, with a realistic mock store 8 passed -> the mock was the cause ``` Change one variable, let the suite answer. Available from cycle one. ## Why this is not just "test more" Every wrong mechanism was plausible, specific, and consistent with the code as read. **Plausibility is what made them dangerous** — each was good enough to write down, publish and act on. The failure mode is not sloppiness; it is that a careful reading of a large file *feels* like evidence. The tell is grammatical: **a claim that can be written without running anything is a hypothesis, not a measurement.** "This cannot be converted because X" versus "reverting X fails these 3 of 8 cases." ## The corollary, including its negative result Once the harness was the suspect, a class fell out: a test that stubs a reader **broken in production** proves the call site's logic while unable to see that production resolves nothing. Eight files stubbed `resolveTaskWorkflowIrSync` — one masking a live defect, four redundant (#3198), one legitimate. The doc also records that the obvious generalisation **fails**: `getTaskWorkflowSelection` is equally degraded under PostgreSQL but stubbing it masks nothing, because the resolver prefers the async twin and both answer the same. The distinguishing property is that the reader returns something *incorrect*, not merely *unused*. Written down so nobody repeats the 120-file sweep. Docs only; `check-fnxc-future-dates` exit 0. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a case study for diagnosing an unconvertible workflow site. * Documented controlled-run findings identifying the realistic mock store as the cause. * Clarified the difference between reading-based hypotheses and instrumented evidence. * Added guidance for distinguishing conversion, orphan-arm, and mock-store issues. * Recorded an audit of related test stubs, including redundant, masking, legitimate, and unresolved cases. <!-- 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:
@@ -428,6 +428,63 @@ Two corollaries worth keeping:
|
||||
are the same shape — they need the emitter-side / async-threading work, not another conversion pass.
|
||||
`--claims` now marks sync-resolver files as inert-risk and keeps them out of the start-here list.
|
||||
|
||||
## A blocker described four times, wrong twice: instrument before you file
|
||||
|
||||
One `triage.ts` site sat flagged as unconvertible for four cycles. Each cycle produced a confident
|
||||
mechanism, and the mechanisms split cleanly by method:
|
||||
|
||||
| # | claimed mechanism | derived from | held? |
|
||||
|---|---|---|---|
|
||||
| 1 | merged intake/hold vocabularies | reading | no |
|
||||
| 2 | the orphan arm is scoped to `source === "selection"` | reading + one test run | partly |
|
||||
| 3 | provenance verifies by `ir.id`, which builtins lack | reading a **comment** | **no — filed as an issue, closed as wrong** |
|
||||
| 4 | two test harnesses cannot answer a selection query | instrumented isolation | **yes** |
|
||||
|
||||
Mechanism 1, derived from reading, was wrong outright; mechanism 2, derived from reading plus a
|
||||
single test run, held only partly — the arm is scoped as claimed, but that scoping was not what made
|
||||
the conversion fail. Neither reading-derived claim survived intact. The one derived from a comment was wrong in the most
|
||||
expensive way: the text quoted was historical prose explaining code that had been REMOVED, sitting
|
||||
directly above a paragraph saying so. A rationale was read as an implementation.
|
||||
|
||||
**The isolation that settled it took three runs and about a minute:**
|
||||
|
||||
```text
|
||||
flag only, no conversion 8 passed -> the orphan arm is not the cause
|
||||
flag + conversion 5 failed -> the conversion is
|
||||
same, with a realistic mock store 8 passed -> the mock was the cause
|
||||
```
|
||||
|
||||
That is the whole technique: change one variable at a time and let the suite answer. It was available
|
||||
from cycle one.
|
||||
|
||||
### Why this is not just "test more"
|
||||
|
||||
Every wrong mechanism was *plausible*, *specific*, and *consistent with the code as read*. Plausibility
|
||||
is what made them dangerous — each one was good enough to write down, publish, and act on. The failure
|
||||
mode is not sloppiness; it is that a careful reading of a large file feels like evidence and is not.
|
||||
|
||||
The tell is grammatical: **if a claim can be written without running anything, it has not been tested.**
|
||||
Statements like "this cannot be converted because X" are hypotheses. Statements like "reverting X fails
|
||||
these 3 of 8 cases" are measurements. This document is full of the second kind because the first kind
|
||||
kept being wrong.
|
||||
|
||||
### The corollary that found real bugs
|
||||
|
||||
Once the harness was the suspect rather than the code, a class fell out: **a test that stubs a reader
|
||||
which is broken in production proves the call site's logic while being unable to see that production
|
||||
resolves nothing.** Eight files stubbed `resolveTaskWorkflowIrSync`. Auditing them — delete the stub,
|
||||
see whether the suite still discriminates — classified six: one masking a live defect, four redundant,
|
||||
one legitimate. The other two are unaccounted for in this write-up — the outcomes were not recorded
|
||||
at the time, and inventing a classification now would be the same unearned confidence this document
|
||||
is about. Either eight is the wrong total or two audits went unrecorded; whoever re-runs the audit
|
||||
should treat the six as the only claim it supports.
|
||||
|
||||
The distinguishing property is narrow, and worth stating because the obvious generalisation fails:
|
||||
the reader must return something **incorrect**, not merely **unused**. `getTaskWorkflowSelection` is
|
||||
also degraded under PostgreSQL, and stubbing it masks nothing, because the resolver simply prefers the
|
||||
async twin and both answer the same. Measured: 120 files stub it; adding the async twin to the
|
||||
highest-signal one changed nothing.
|
||||
|
||||
## The rule that produced every fix above
|
||||
|
||||
**A green guard is evidence only once you have watched it go red.**
|
||||
|
||||
Reference in New Issue
Block a user