The census header has been printing `of the column guards, 0 are trait-fallback branches (already converted)` while sites of exactly that shape exist. I flagged this on #2842 as a suspected classifier gap; this confirms and fixes it. ## The miss Only `cond ? trait : literal` was recognised. The other spelling — a **negative** test with the literal on the **true** branch — is what a caller writes once it hoists its resolved lanes: ```ts complete: completeLanes === undefined ? columnId === "done" : completeLanes.includes(columnId) ``` That is `github-tracking-state.ts:245-246` — a fully converted resolver whose two degraded arms were reported as unconverted debt. **The backlog read higher than the remaining work**, and a reader chasing it was sent to lines that are already correct. Second half of the miss: `completeLanes` matches no hint. Adding `Lanes` to the hint list does **not** work, and the reason is itself a prior fix — hints are word-bounded because the unbounded form once let `hold` match `threshold` and `household`. `\bLanes\b` cannot match inside `completeLanes`, where the boundary does not exist. So resolved-lane identifiers get an explicit suffix rule. ## Both guards on the new rule exist because I broke them while writing it Worth stating, because each failure ran in the **dangerous direction** — marking a *live* line "already converted", which removes a real guard from a backlog people trust: | mistake | what it excused | |---|---| | widened the shared `testsTraitData` | fed the ancestor-walking rules too, which marked `step.status === "done" \|\| step.status === "in-progress"` at `register-task-workflow-routes.ts:941` — a step-**status** comparison, not a column guard — as converted | | let the new rule walk ancestors | excused any literal inside a block governed by a negative lane test | Measured: the count went to **6 with two of them wrong** before I caught it. The rule is now immediate-parent-only with its widened identifier match local to it, and reports exactly the **2 real sites**. ## Verification - Census: **176 guards, 2 trait-fallback** (was 176 / 0). The total is unchanged — this sub-count is diagnostic and does not move the ratchet, so `--strict` exits 0 with no baseline re-record. - 5 cases in `scripts/__tests__/lifecycle-census-inverted-fallback.test.mjs`, including both negatives that pin the mistakes above plus one for the suffix rule not over-reaching (`airplanes` is not a lane test). - `pnpm lint` clean; gate green (161/487/13/71). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved lifecycle analysis accuracy for trait fallback logic, including inverted conditions, legacy fallback syntax, and null or undefined checks. * Added safeguards to avoid misclassifying complex conditions, unrelated identifiers, and nested expressions. * Improved handling of lifecycle lane and column naming patterns. * **Tests** * Expanded coverage for valid and invalid fallback scenarios, identifier boundaries, parent-expression restrictions, and property-path checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
97 lines
4.2 KiB
JavaScript
97 lines
4.2 KiB
JavaScript
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-30-22:40:
|
|
|
|
THE CENSUS HEADER SAID "0 ARE TRAIT-FALLBACK BRANCHES" WHILE SITES OF THAT SHAPE EXISTED.
|
|
|
|
Only `cond ? trait : literal` was recognised. The other spelling — a NEGATIVE test with the literal on
|
|
the TRUE side — is what a caller writes once it hoists its resolved lanes:
|
|
|
|
completeLanes === undefined ? columnId === "done" : completeLanes.includes(columnId)
|
|
|
|
That is a fully converted resolver whose degraded arms were reported as unconverted debt, so the
|
|
backlog read higher than the remaining work and a reader chasing it was sent to correct lines.
|
|
|
|
BOTH GUARDS BELOW EXIST BECAUSE I BROKE THEM WHILE WRITING THE FIX, and each failure ran in the
|
|
dangerous direction — marking a LIVE line "already converted":
|
|
|
|
- widening the shared `testsTraitData` predicate fed the ancestor-walking rules too, which then
|
|
excused a step-STATUS comparison in `register-task-workflow-routes.ts:941`;
|
|
- letting the new rule walk ancestors excused any literal inside a block governed by a negative
|
|
lane test.
|
|
|
|
So the rule is immediate-parent-only and its widened identifier match is local to it.
|
|
*/
|
|
import test from "node:test";
|
|
import assert from "node:assert/strict";
|
|
|
|
import { findComparisons } from "../lib/lifecycle-column-census-ast.mjs";
|
|
|
|
const fallbacks = (src) =>
|
|
findComparisons("t.ts", src).filter((f) => f.traitFallback).map((f) => f.columnId);
|
|
|
|
test("an inverted fallback — negative test, literal on the TRUE branch — is recognised", () => {
|
|
const src = 'const c = completeLanes === undefined ? columnId === "done" : completeLanes.includes(columnId);';
|
|
assert.deepEqual(fallbacks(src), ["done"]);
|
|
});
|
|
|
|
test("the classic form still counts", () => {
|
|
const src = 'const c = columnFlags ? columnFlags.complete : columnId === "done";';
|
|
assert.deepEqual(fallbacks(src), ["done"]);
|
|
});
|
|
|
|
test("a POSITIVE condition with the literal on the true branch is a LIVE guard, not a fallback", () => {
|
|
/* The direction that must not be excused: nothing here says the trait data was absent. */
|
|
const src = 'const c = columnFlags ? columnId === "done" : other;';
|
|
assert.deepEqual(fallbacks(src), []);
|
|
});
|
|
|
|
test("a lane test does NOT excuse a literal elsewhere in the same block", () => {
|
|
/*
|
|
The ancestor-walk over-reach. `step.status === "done"` is not a column guard at all, and letting the
|
|
new rule climb marked exactly this shape as converted.
|
|
*/
|
|
const src = `
|
|
function f() {
|
|
if (completeLanes === undefined) {
|
|
return steps.find((step) => step.status === "done" || step.status === "in-progress");
|
|
}
|
|
return null;
|
|
}`;
|
|
assert.deepEqual(fallbacks(src), []);
|
|
});
|
|
|
|
test("an identifier merely CONTAINING a lane word is not a lane test", () => {
|
|
/* `\\bLanes\\b` cannot match inside `completeLanes`, which is why the suffix rule exists — but it
|
|
must not over-reach onto unrelated names either. */
|
|
const src = 'const c = airplanes === undefined ? columnId === "done" : other;';
|
|
assert.deepEqual(fallbacks(src), []);
|
|
});
|
|
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-30-23:20 (#2874 review — greptile P2):
|
|
A COMPOUND condition is not a simple absence test. `completeLanes === undefined || forceLegacy` has a
|
|
second disjunct that can select the true branch with lane data PRESENT, so the literal there is a live
|
|
guard. The unanchored check accepted it, which removes a real guard from the backlog — the direction
|
|
this rule exists to stop.
|
|
*/
|
|
|
|
test("a compound absence test is NOT treated as a fallback", () => {
|
|
const src = 'const c = completeLanes === undefined || forceLegacy ? columnId === "done" : other;';
|
|
assert.deepEqual(fallbacks(src), []);
|
|
});
|
|
|
|
test("a conjunction is refused too", () => {
|
|
const src = 'const c = completeLanes === undefined && legacyMode ? columnId === "done" : other;';
|
|
assert.deepEqual(fallbacks(src), []);
|
|
});
|
|
|
|
test("the simple negated form still counts", () => {
|
|
const src = 'const c = !completeLanes ? columnId === "done" : completeLanes.includes(columnId);';
|
|
assert.deepEqual(fallbacks(src), ["done"]);
|
|
});
|
|
|
|
test("a property-path absence test still counts", () => {
|
|
const src = 'const c = lifecycle?.completeLanes === undefined ? columnId === "done" : other;';
|
|
assert.deepEqual(fallbacks(src), ["done"]);
|
|
});
|