fix(tests): the load-lane guard matched a VARIABLE NAME — make it AST-based (#2804)
## A red that no CI run can see Found by pre-flighting #2796 against current `main`. Merged, the engine suite fails: ``` scheduler-load-lane-union.test.ts > the scheduler builds this same union expected 'import {…' to contain '...columnsWithFlag(loadLaneIr, "intake")' ``` **Neither side is red on its own.** `main` is green (10913 passed, 0 failed) and #2796's own CI is green — this test landed on `main` via **#2787**, *after* #2796 was cut, and #2796 does not touch the file. It fails only in the merged state, which is exactly the shape no branch's CI checks. ## #2796 is not at fault The union is still built (`scheduler.ts`, the `columnsWithFlag(ir, ...)` spread). Resolving assignment load per task renamed the local from `loadLaneIr` to `ir`, and the guard hardcoded that name: ```ts expect(source).toContain(`...columnsWithFlag(loadLaneIr, "${flag}")`); ``` It would fail identically on a reformat, a line wrap, or any rename — reporting drift that did not happen. And the reflex fix is to edit the string to match, which protects nothing and teaches nobody anything. ## The fix Parse `scheduler.ts` and collect the string literal passed as the **second** argument to every `columnsWithFlag(...)` call, whatever the first argument is called. Same invariant — every legacy role is unioned somewhere in the scheduler — now actually checked. It also asserts the parse found **something** before checking the six flags. A visitor that matched nothing would make every assertion below it vacuous, which is the specific failure mode this guard family keeps producing. Still structural rather than behavioural, for the reason the file header already gives: the call site sits inside a dispatch path a unit test has no business standing up. The three sibling cases cover the resolver's behaviour; this one covers the wiring. ## Evidence — the discrimination is the right way round | mutation | expected | result | |---|---|---| | rename `ir` → `loadLaneIr` (behaviour identical) | pass | **4/4 passed** | | drop `...columnsWithFlag(ir, "hold")` from the union | fail | **fails**: `scheduler.ts no longer passes "hold" to columnsWithFlag` | Engine **10936 passed / 0 failed** · gate **732 green** · lint clean · engine `tsc --noEmit` **0 errors**. Test-only; `scheduler.ts` restored clean after the mutations. **Unblocks #2796 with no change needed on its side** — commented there. ## Pre-flight results for the rest of the queue Same method (merge with current `main`, run the suites), since batch-engine's earlier landing put 32 failures on main that were only caught post-merge: | PR | result | |---|---| | #2785 batch-engine-tail | clean — 10903 passed | | #2783 batch-core-2 | clean — core 4751 passed; its one api failure is pre-existing on main | | #2797 engine tail | clean — 10941 passed | | #2772 batch-dashboard-app | clean — backfill total 112 → **111**, no lane regresses | | #2796 assignment load | **this failure only** | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -62,12 +62,82 @@ describe("the scheduler's load-lane union covers every legacy role", () => {
|
||||
expect(loadLanes(RENAMED_IR).has("shipped")).toBe(false);
|
||||
});
|
||||
|
||||
it("the scheduler builds this same union", () => {
|
||||
// Guards the mirror above against drift: if scheduler.ts stops unioning a role, this fails.
|
||||
it("the scheduler builds this same union", async () => {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-04:20:
|
||||
AST, NOT A SOURCE-TEXT MATCH — the previous form hardcoded a local VARIABLE NAME.
|
||||
|
||||
It asserted `source.toContain('...columnsWithFlag(loadLaneIr, "<flag>")')`. #2796 resolves
|
||||
assignment load per task and renamed that local from `loadLaneIr` to `ir`; the union it builds is
|
||||
unchanged (scheduler.ts, the `columnsWithFlag(ir, ...)` spread), but the literal stopped matching
|
||||
and this failed. Neither PR's CI could see it — the test landed on main via #2787 after #2796 was
|
||||
cut, and #2796 does not touch this file, so it only breaks in the merged state.
|
||||
|
||||
A guard that fails on a rename, a reformat, or a line wrap while the behaviour is untouched costs
|
||||
more than it protects: it reports drift that did not happen, and the reflex fix is to edit the
|
||||
string, which teaches nobody anything.
|
||||
|
||||
So: parse `scheduler.ts` and collect the string literal passed as the SECOND argument to every
|
||||
`columnsWithFlag(...)` call, whatever the first argument is called. The invariant — every legacy
|
||||
role is unioned somewhere in the scheduler — is preserved and is what actually gets checked.
|
||||
|
||||
Still a structural assertion rather than a behavioural one, for the reason the file header gives:
|
||||
the call site sits inside a dispatch path a unit test has no business standing up. The three
|
||||
cases above cover the resolver's behaviour; this one covers the wiring.
|
||||
*/
|
||||
const ts = await import("typescript");
|
||||
const source = readFileSync(new URL("../scheduler.ts", import.meta.url), "utf8");
|
||||
const sf = ts.createSourceFile("scheduler.ts", source, ts.ScriptTarget.Latest, true);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-16:45 (#2804 review — greptile):
|
||||
SCOPED TO `resolveLoadLanes`, not the whole file.
|
||||
|
||||
The first version collected EVERY `columnsWithFlag` call in scheduler.ts. `resolveDependencySatisfactionColumns`
|
||||
in the same file passes `mergeBlocker` and `humanReview` for its own, unrelated question — so
|
||||
deleting either from the load-lane union left them in the set anyway and every assertion below
|
||||
still passed. The test could not fail for the regression it exists to catch.
|
||||
|
||||
Scoping means two things have to be sanity-checked, not one: that the function was FOUND, and that
|
||||
it contained calls. Either being false is a silent pass, which is the failure mode this whole
|
||||
exercise keeps producing.
|
||||
*/
|
||||
const flagsPassed = new Set<string>();
|
||||
let loadLaneFnFound = false;
|
||||
|
||||
const collectWithin = (node: import("typescript").Node): void => {
|
||||
if (ts.isCallExpression(node)
|
||||
&& ts.isIdentifier(node.expression)
|
||||
&& node.expression.text === "columnsWithFlag"
|
||||
&& node.arguments.length >= 2
|
||||
&& ts.isStringLiteral(node.arguments[1])) {
|
||||
flagsPassed.add(node.arguments[1].text);
|
||||
}
|
||||
ts.forEachChild(node, collectWithin);
|
||||
};
|
||||
|
||||
const findLoadLaneFn = (node: import("typescript").Node): void => {
|
||||
if (ts.isVariableDeclaration(node)
|
||||
&& ts.isIdentifier(node.name)
|
||||
&& node.name.text === "resolveLoadLanes"
|
||||
&& node.initializer) {
|
||||
loadLaneFnFound = true;
|
||||
collectWithin(node.initializer);
|
||||
return;
|
||||
}
|
||||
ts.forEachChild(node, findLoadLaneFn);
|
||||
};
|
||||
findLoadLaneFn(sf);
|
||||
|
||||
/*
|
||||
Both sanity checks are load-bearing. A rename of `resolveLoadLanes` would otherwise leave the set
|
||||
empty and the loop below would assert nothing — the exact vacuity this scoping was meant to remove.
|
||||
*/
|
||||
expect(loadLaneFnFound, "scheduler.ts no longer declares resolveLoadLanes — this test is scoped to it").toBe(true);
|
||||
expect(flagsPassed.size).toBeGreaterThan(0);
|
||||
|
||||
for (const flag of ["intake", "hold", "countsTowardWip", "mergeOrchestration", "mergeBlocker", "humanReview"]) {
|
||||
expect(source).toContain(`...columnsWithFlag(loadLaneIr, "${flag}")`);
|
||||
expect(flagsPassed, `scheduler.ts no longer passes "${flag}" to columnsWithFlag`).toContain(flag);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user