docs(core): an empty role result has TWO meanings — correcting a rule I applied across three PRs (#2765)
## A correction to a rule I introduced, prompted by #2760 and then measured `synthesizeDefaultColumns` (`workflow-ir.ts`) upgrades a v1 graph by emitting `{ id, name: id, traits: [] }` for the five default ids — **placement only, by design**, with the real trait set living in `BUILTIN_CODING_WORKFLOW_IR`. So a v1-upgraded board arrives at every role resolver looking exactly like a v2 board that declares nothing. Measured on such an IR: | resolver | result | |---|---| | `resolveLifecycleColumns` | `{}` — every role undefined | | `resolveReviewColumns` | `[]` | | `columnsWithFlag(…, "countsTowardWip")` | `[]` | | `resolveTerminalColumns` | `["done","archived"]` — its own legacy fallback saves it | ## The correction I owe I introduced **"resolved and empty means this board declares no such lane"** deliberately across #2731, #2733 and #2734, to fix the *opposite* bug — a legacy fallback masking a genuinely absent lane. I argued for it repeatedly and applied it in several files. It is right for hand-written v2. It is **wrong for the upgrade path**, where it withdraws every role at once. And the two cases are indistinguishable at the call site: both arrive as an empty array. The contrast the test pins is the sharpest evidence — `resolveTerminalColumns` survives the identical IR purely because it never adopted that reading. **The data is the same; the reading is what differs.** ## Affected, named rather than left to be rediscovered - **on `main` today**: `default-workflow-hooks.ts` → `inRole` (from #2734) - **in my open PRs**: the store bypass guard (#2709) and the tracking-state terminal classifier (#2754) Consumers that kept a `length > 0 ? resolved : legacy` guard are unaffected — including the notifier (#2722), which I checked. ## Not fixed here The root repair is for the upgrade to carry real traits instead of placeholders. That changes behaviour for **every persisted v1 workflow**, so it wants its own change with its own measurement — not something slipped into a doc commit. Flagged at the source so the next consumer chooses knowingly. ## Verification 34 trait tests green · core `tsc` clean · lint clean (0 errors) · gate green (487 + 158 + 10 + 71). No production behaviour change; no census movement.
This commit is contained in:
@@ -8,7 +8,7 @@ byte-identical on the default workflow. The custom cases prove KTD-10 fallback.
|
||||
import { describe, expect, it } from "vitest";
|
||||
import "../builtin-traits.js"; // register built-in traits
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
|
||||
import { columnsWithFlag, columnHasFlag, resolveReboundTarget, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveLifecycleColumns, resolveReviewColumns, resolveTaskLifecycleColumns } from "../workflow-lifecycle-traits.js";
|
||||
import { columnsWithFlag, columnHasFlag, resolveReboundTarget, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveLifecycleColumns, resolveReviewColumns, resolveTaskLifecycleColumns, resolveTerminalColumns } from "../workflow-lifecycle-traits.js";
|
||||
import { BUILTIN_CODING_IDEAS_WORKFLOW_IR } from "../builtin-coding-ideas-workflow-ir.js";
|
||||
import type { WorkflowIr } from "../workflow-ir-types.js";
|
||||
import { getTraitRegistry } from "../trait-registry.js";
|
||||
@@ -435,3 +435,41 @@ describe("resolveReviewColumns", () => {
|
||||
expect(resolveReviewColumns(BUILTIN_CODING_WORKFLOW_IR)).toContain("in-review");
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-19:20:
|
||||
A v1 graph upgraded to v2 carries `traits: []` on every synthesized column (`synthesizeDefaultColumns`
|
||||
in workflow-ir.ts — placement only, by design). So every role resolver answers "nothing" for a board
|
||||
whose lanes are in fact the legacy ones.
|
||||
|
||||
This is pinned because the shape is INDISTINGUISHABLE at the call site from a hand-written v2 workflow
|
||||
that genuinely declares no such lane, and the two want opposite handling. A guard that reads empty as
|
||||
"no such lane" is right for the second and withdraws every role at once for the first.
|
||||
*/
|
||||
describe("a v1-upgraded IR resolves to NO roles — the other meaning of empty", () => {
|
||||
const v1Upgraded = {
|
||||
version: "v2",
|
||||
name: "upgraded",
|
||||
columns: ["todo", "in-progress", "in-review", "done", "archived"].map((id) => ({ id, name: id, traits: [] })),
|
||||
nodes: [],
|
||||
edges: [],
|
||||
} as never;
|
||||
|
||||
it("returns no lifecycle roles at all", () => {
|
||||
expect(resolveLifecycleColumns(v1Upgraded)).toEqual({});
|
||||
});
|
||||
|
||||
it("returns an EMPTY review set, not the legacy lane", () => {
|
||||
expect(resolveReviewColumns(v1Upgraded)).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns an EMPTY wip set", () => {
|
||||
expect(columnsWithFlag(v1Upgraded, "countsTowardWip")).toEqual([]);
|
||||
});
|
||||
|
||||
it("STILL yields the legacy terminal pair, because that resolver keeps its own fallback", () => {
|
||||
/* The contrast that makes the hazard concrete: same IR, and this one is unaffected purely because
|
||||
it never adopted the empty-means-absent reading. */
|
||||
expect(resolveTerminalColumns(v1Upgraded)).toEqual(["done", "archived"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -34,6 +34,57 @@ function columnsOf(ir: WorkflowIr): WorkflowIrColumn[] {
|
||||
* trait→columnIds expansion. Deterministic (declared column order). Empty for a
|
||||
* column-less IR or when no column carries the flag.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-21:15 (a DECLARATION is not a GUARD — I conflated them and
|
||||
published the mistake, so it is written down here):
|
||||
|
||||
The census counts COMPARISONS against a legacy column id. It does not count a workflow DECLARING a
|
||||
column with that id, and the two answer different questions:
|
||||
|
||||
triage column guards in the tree 0 (no code compares against the literal)
|
||||
`triage` declared by the default lineage yes (builtin-coding-workflow-ir.ts:49, the intake lane)
|
||||
|
||||
Both are true at once. "The backlog reached zero for `triage`" means nothing in the code branches on
|
||||
that NAME any more; it does not mean the column stopped existing, and a reader who takes it that way
|
||||
will conclude a resolver's `?? "triage"` fallback is dead when it is the default board's actual intake
|
||||
answer.
|
||||
|
||||
I asserted the stronger version in a review audit and it was wrong. One grep of
|
||||
`builtin-coding-workflow-ir.ts` would have caught it, which is the cheap check worth doing before any
|
||||
claim about what a lineage contains.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-19:20 (an EMPTY result has TWO meanings — measured, not assumed):
|
||||
|
||||
Everything below returns nothing for a column set that carries no traits, and there are two very
|
||||
different reasons a board can look like that:
|
||||
|
||||
DECLARED AND EMPTY a v2 workflow the operator wrote that genuinely has no complete lane. "No such
|
||||
lane" is the right answer, and a guard should act on it.
|
||||
|
||||
SYNTHESIZED a v1 graph upgraded to v2. `synthesizeDefaultColumns` (workflow-ir.ts) emits
|
||||
`{ id, name: id, traits: [] }` for the five default ids — placement only, by
|
||||
design, with the real trait set living in BUILTIN_CODING_WORKFLOW_IR. Those
|
||||
columns ARE the legacy lanes; the traits were simply never expressed.
|
||||
|
||||
MEASURED on such an IR:
|
||||
resolveLifecycleColumns -> {} (every role undefined)
|
||||
resolveReviewColumns -> []
|
||||
columnsWithFlag(wip) -> []
|
||||
resolveTerminalColumns -> ["done","archived"] (its own legacy fallback saves it)
|
||||
|
||||
CONSEQUENCE FOR CONVERTED GUARDS. A consumer that reads "resolved and empty" as "this board declares
|
||||
no such lane" is CORRECT for the first case and WRONG for the second — on a v1-upgraded board it
|
||||
withdraws every role at once. Callers that kept a `length > 0 ? resolved : legacy` guard are unaffected.
|
||||
|
||||
I introduced that reading deliberately in #2731/#2733/#2734 to fix the opposite bug (a legacy fallback
|
||||
masking a genuinely absent lane), and it is right for hand-written v2. This note exists because it is
|
||||
NOT right for the upgrade path, and the difference is invisible at the call site — both arrive here as
|
||||
an empty array.
|
||||
|
||||
The root fix would be for the upgrade to carry the real traits rather than placeholders; that changes
|
||||
behaviour for every persisted v1 workflow, so it is flagged here rather than made in passing.
|
||||
*/
|
||||
export function columnsWithFlag(ir: WorkflowIr, flag: keyof TraitFlags): string[] {
|
||||
const registry = getTraitRegistry();
|
||||
return columnsOf(ir)
|
||||
|
||||
Reference in New Issue
Block a user