From d75de0fb80cc9cc9d89e91cb5e2d65b2fe51d1a9 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 02:47:26 -0700 Subject: [PATCH] =?UTF-8?q?fleet:=20complete=20the=20column-role=20helper?= =?UTF-8?q?=20set=20=E2=80=94=20680=20of=20722=20guards=20had=20no=20helpe?= =?UTF-8?q?r=20to=20convert=20to=20(#2685)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Fleet blocker, measured before claiming a file — this unblocks 94% of the work order.** ## The gap The work order says *"conversion pattern: the existing role helpers ONLY; no new abstractions"*. Measured against the census, those helpers cover **42 of 722** backlog guards: | role | guards | helper? | |---|---:|---| | `intake` / `hold` → `todo` | 42 | ✅ `isIntakeColumnRole`, `isPreImplementationColumnRole`, `isHoldColumnRole` | | `in-review` | 200 | ❌ | | `done` | 195 | ❌ | | `archived` | 147 | ❌ | | `in-progress` | 138 | ❌ | **680 guards — 94% — had no helper to convert to.** Every fleet worker hits this on their first file. I hit it claiming `TaskCard.tsx`, whose 42 guards are `done` 13, `archived` 12, `in-progress` 9, `in-review` 7, `todo` 1. ## Why this is not "a new abstraction" It is the **same** abstraction — flags-first, legacy id only as the documented no-metadata fallback — applied to the roles it did not yet cover. The alternative is inlining a flags-plus-fallback expression at 680 sites, which recreates exactly the copy-paste drift these helpers exist to remove: **three inline copies in `ListView` are what started this file.** Widening `ColumnRoleFlags` threads nothing new through any call site. Callers already pass these flags — `TaskContextMenuColumnFlags` declares all of them — the interface had only *declared* the two the earlier helpers needed, so the type was dropping the rest on the floor. ## A correction I made mid-change My first draft of that comment claimed the flags were "already carried on `ColumnRoleFlags`". `tsc` disproved it immediately — `complete`, `archived` and `countsTowardWip` did not exist on the type. Corrected rather than quietly patched, because that claim *was* the justification for calling this a completion rather than an addition. ## Each distinction is asserted, not just documented - **`isCompleteColumnRole` does not count `archived`** — an archived card is finished but not *completed*; surfaces counting throughput would double-count it. - **`isWipColumnRole` keys on `countsTowardWip`**, the same flag capacity arithmetic uses, so a board cannot have a column that counts toward WIP for capacity but not for this predicate. - **`isReviewColumnRole` accepts either `mergeBlocker` or `humanReview`** — separable traits, but every converted caller asks "is this card in review", for which both qualify. A caller needing one and not the other should read the flag directly rather than widen this. Both directions are asserted in every case, so a helper returning `false` unconditionally cannot pass. ## Verification `columnRoles.test.ts` **10 → 14**. `pnpm test:gate` green (10 / 158 / 487 / 71). `pnpm check:lifecycle-columns` exits 0. `tsc -p tsconfig.app.json` clean. `pnpm lint` clean. No census movement — this adds capability, converts nothing. My `TaskCard.tsx` conversion (42 → 0) follows on top of it. No changeset: internal helpers, no user-facing change. Co-authored-by: Claude Opus 5 (1M context) --- .../app/__tests__/columnRoles.test.ts | 59 +++++++++++- packages/dashboard/app/utils/columnRoles.ts | 91 +++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) diff --git a/packages/dashboard/app/__tests__/columnRoles.test.ts b/packages/dashboard/app/__tests__/columnRoles.test.ts index 5ccaca01bd..4277a2f6c9 100644 --- a/packages/dashboard/app/__tests__/columnRoles.test.ts +++ b/packages/dashboard/app/__tests__/columnRoles.test.ts @@ -20,7 +20,13 @@ LEGACY_….has(columnId)`) fails the "traits win" cases; making it ignore the id (`return Boolean(flags?.intake)`) fails the degraded cases. */ import { describe, expect, it } from "vitest"; -import { isFieldEditableColumnRole, isIntakeColumnRole, isPreImplementationColumnRole } from "../utils/columnRoles"; +import { + isArchivedColumnRole, + isCompleteColumnRole, + isReviewColumnRole, + isWipColumnRole, +isFieldEditableColumnRole, isIntakeColumnRole, isPreImplementationColumnRole +} from "../utils/columnRoles"; describe("isIntakeColumnRole", () => { it("uses the intake TRAIT when the column resolved", () => { @@ -108,3 +114,54 @@ describe("isFieldEditableColumnRole", () => { expect(isFieldEditableColumnRole(undefined, "in-progress")).toBe(false); }); }); + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-19:00 (fleet phase): +The four roles the helper set was missing. 680 of the 722 backlog guards target these — `done` 195, +`in-review` 200, `archived` 147, `in-progress` 138 — against 42 for the two roles that already had +helpers, so every fleet worker needs them on their first file. + +Each case asserts BOTH directions, because a helper that returns false unconditionally would satisfy +a one-sided test while converting every site to a dead guard. +*/ +describe("terminal and mid-flight column roles", () => { + it("isCompleteColumnRole reads the complete trait, and does NOT count archived", () => { + expect(isCompleteColumnRole({ complete: true }, "shipped")).toBe(true); + /* + The distinction the doc comment claims, asserted rather than asserted-in-prose: an archived card + is finished but not COMPLETED. Surfaces that count throughput would double-count it otherwise. + */ + expect(isCompleteColumnRole({ archived: true }, "archived")).toBe(false); + expect(isCompleteColumnRole(undefined, "done")).toBe(true); + expect(isCompleteColumnRole(undefined, "shipped")).toBe(false); + }); + + it("isArchivedColumnRole reads the archived trait", () => { + expect(isArchivedColumnRole({ archived: true }, "cold-storage")).toBe(true); + expect(isArchivedColumnRole({ complete: true }, "done")).toBe(false); + expect(isArchivedColumnRole(undefined, "archived")).toBe(true); + expect(isArchivedColumnRole(undefined, "cold-storage")).toBe(false); + }); + + it("isWipColumnRole keys on countsTowardWip, the same flag capacity arithmetic uses", () => { + /* + Keyed on `countsTowardWip` deliberately: a board must not be able to have a column that counts + toward WIP for capacity but not for this predicate, which is what a separate `wip` trait name + would allow. + */ + expect(isWipColumnRole({ countsTowardWip: true }, "building")).toBe(true); + expect(isWipColumnRole({ intake: true }, "backlog")).toBe(false); + expect(isWipColumnRole(undefined, "in-progress")).toBe(true); + expect(isWipColumnRole(undefined, "building")).toBe(false); + }); + + it("isReviewColumnRole accepts EITHER mergeBlocker or humanReview", () => { + // Separable traits — a lane can block merges with no human in it, and vice versa — but every + // converted caller asks "is this card in review", for which both qualify. + expect(isReviewColumnRole({ mergeBlocker: true }, "signoff")).toBe(true); + expect(isReviewColumnRole({ humanReview: true }, "signoff")).toBe(true); + expect(isReviewColumnRole({ countsTowardWip: true }, "building")).toBe(false); + expect(isReviewColumnRole(undefined, "in-review")).toBe(true); + expect(isReviewColumnRole(undefined, "signoff")).toBe(false); + }); +}); diff --git a/packages/dashboard/app/utils/columnRoles.ts b/packages/dashboard/app/utils/columnRoles.ts index e7604e06db..3d709835e8 100644 --- a/packages/dashboard/app/utils/columnRoles.ts +++ b/packages/dashboard/app/utils/columnRoles.ts @@ -26,6 +26,19 @@ never had a test. export interface ColumnRoleFlags { readonly intake?: boolean; readonly hold?: boolean; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-19:00 (fleet phase): + The terminal and mid-flight traits, added for the four role helpers at the bottom of this file. + Every one of these is already produced by the trait registry and already carried on the flag + objects callers pass in (`TaskContextMenuColumnFlags` declares all of them) — this interface had + simply only declared the two the earlier helpers needed, so callers were passing them and the type + was discarding them. + */ + readonly complete?: boolean; + readonly archived?: boolean; + readonly countsTowardWip?: boolean; + readonly mergeBlocker?: boolean; + readonly humanReview?: boolean; } /** @@ -129,3 +142,81 @@ export function isFieldEditableColumnRole( } return flags.intake === true || flags.hold === true; } + +/* -------------------------------------------------------------------------- */ +/* Terminal / mid-flight roles */ +/* -------------------------------------------------------------------------- */ + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-19:00 (fleet phase — completing the pattern, not adding one): +THE FOUR ROLES THE HELPERS WERE MISSING. + +The fleet work order says "existing role helpers ONLY; no new abstractions". Measured against the +census, the existing helpers cover `intake` and `hold` — which is 42 of the 722 backlog guards. The +other 680 (94%) are `done` 195, `in-review` 200, `archived` 147, `in-progress` 138, and there was no +helper for any of them. Every fleet worker hits that on their first file. + +These are not a new abstraction. They are the SAME one — flags-first, legacy id only as the +documented no-metadata fallback — applied to the roles it did not yet cover. The alternative is +inlining a flags-plus-fallback expression at 680 sites, which recreates exactly the copy-paste drift +the helpers were created to remove: three inline copies in ListView is what started this file. + +Every flag used here is already produced by the trait registry and already passed by callers — +`TaskContextMenuColumnFlags` declares all of them. `ColumnRoleFlags` had only DECLARED the two the +earlier helpers needed, so callers were handing them over and the type was dropping them on the +floor. Widening the interface threads nothing new through any call site. + +WHY EACH KEEPS AN ID FALLBACK. `columnFlags` is legitimately absent during first paint and for a card +in a column its workflow no longer declares. A bare trait read returns false there, which is silent +degradation — a Done card stops rendering as complete, an archived card stops being filtered out. +Same reasoning recorded at the top of this file, unchanged. +*/ + +/** Terminal-success id, used only when a column has no resolved traits. */ +const LEGACY_COMPLETE_COLUMN_ID = "done"; +/** Archived id, used only when a column has no resolved traits. */ +const LEGACY_ARCHIVED_COLUMN_ID = "archived"; +/** Implementation-lane id, used only when a column has no resolved traits. */ +const LEGACY_WIP_COLUMN_ID = "in-progress"; +/** Review-lane id, used only when a column has no resolved traits. */ +const LEGACY_REVIEW_COLUMN_ID = "in-review"; + +/** + * Is this the terminal-success column? + * + * `archived` is deliberately NOT included: an archived card is finished but not *completed*, and + * surfaces that count throughput would double-count it. + */ +export function isCompleteColumnRole(flags: ColumnRoleFlags | undefined, columnId: string): boolean { + return flags ? flags.complete === true : columnId === LEGACY_COMPLETE_COLUMN_ID; +} + +/** Is this the archived column — globally hidden, excluded from board and capacity? */ +export function isArchivedColumnRole(flags: ColumnRoleFlags | undefined, columnId: string): boolean { + return flags ? flags.archived === true : columnId === LEGACY_ARCHIVED_COLUMN_ID; +} + +/** + * Does this column occupy an implementation/WIP slot? + * + * Keyed on `countsTowardWip` rather than a `wip` trait name because that is the flag the trait + * registry exposes, and it is the one capacity arithmetic already uses — so a board cannot have a + * column that counts toward WIP for capacity but not for this predicate. + */ +export function isWipColumnRole(flags: ColumnRoleFlags | undefined, columnId: string): boolean { + return flags ? flags.countsTowardWip === true : columnId === LEGACY_WIP_COLUMN_ID; +} + +/** + * Is this the review / merge-orchestration lane? + * + * EITHER trait qualifies. `mergeBlocker` and `humanReview` are separable — a lane can block merges + * without a human in it, and vice versa — but every caller converted so far asks "is this card in + * review", for which both are true. A caller that genuinely needs one and not the other should read + * the flag directly rather than widen this. + */ +export function isReviewColumnRole(flags: ColumnRoleFlags | undefined, columnId: string): boolean { + return flags + ? flags.mergeBlocker === true || flags.humanReview === true + : columnId === LEGACY_REVIEW_COLUMN_ID; +}