U12 drift: ListView.tsx — one tested column-role helper (5 -> 0) (#2620)
**File claimed: `packages/dashboard/app/components/ListView.tsx`.** Per-file lifecycle-column guard count: **5 → 0** (3 live, 2 in comment prose that described the deleted code). ## What was actually wrong All three live sites were *already* flags-first. The defect was that each carried its own inline copy of the same fallback: ```ts targetFlags ? Boolean(targetFlags.intake || targetFlags.hold) : column === "todo" || column === "triage" ``` Three copies, none reachable from a test, each reading like a lifecycle rule rather than the degraded mode it is. A fourth copy was the natural next step. ## Why the fallback survives instead of being deleted `columnFlagsById` is legitimately empty in two states: the pre-load window before the workflows fetch resolves, and a card stranded in a column its workflow no longer declares. A bare `flags.intake === true` returns false in both, and **both failures are silent** — the Planning badge stops appearing, and a backwards move stops asking whether to preserve step progress, so the operator loses completed steps with no prompt and no error. Deleting the fallback is not the cleanup it looks like. So it is kept, named (`isPreImplementationColumnRole`, `isIntakeColumnRole` in `app/utils/columnRoles.ts`), defined once, and documented with that reason at the definition. The legacy ids now live in a named `LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS` set — a last-resort guess, not a comparison masquerading as a rule. ## Tests, and the case that never had one `app/__tests__/columnRoles.test.ts` (6). The degraded branch is now covered for the first time — it was unreachable while inline inside two `handleMove` closures and a `useCallback`. It also pins the **inversion** a fourth copy would eventually get wrong: a resolved column whose traits say it is *not* pre-implementation must not be overridden by an id that happens to be `todo` or `triage`. That is the direction that trains operators to dismiss the prompt. Mutation-checked, measured: | mutation | result | |---|---| | ignore the flags argument (`return LEGACY_….has(columnId)`) | **4 failed / 2 passed** | | ignore the id fallback (`return Boolean(flags?.intake \|\| flags?.hold)`) | **4 failed / 2 passed** | ## Behaviour preservation `ListView.test.tsx` + `workflow-resolved-columns.test.tsx`: **260 passed**, unchanged. The extraction is a pure move — the two helper bodies are the inline expressions verbatim, with the id set hoisted. `pnpm lint` clean. `tsc -p tsconfig.app.json` clean (the app config, not the root one that silently skips `app/`). No changeset: behaviour-preserving refactor. ## Backlog measured on `origin/main` at time of writing 48 total. `self-healing.ts` (10) is the capacity worker's; `register-task-workflow-routes.ts` (7) is my #2614. Remaining unowned in this area after this PR: `TaskCard.tsx` 4, `TaskDetailModal.tsx` 3, `TaskContextMenu.tsx` 2, `Column.tsx` 2, `taskActivity.ts` 2. Several of those hold the *same* fallback pattern and can now call this helper rather than grow another copy. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
73
packages/dashboard/app/__tests__/columnRoles.test.ts
Normal file
73
packages/dashboard/app/__tests__/columnRoles.test.ts
Normal file
@@ -0,0 +1,73 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
Coverage for the column-ROLE helpers extracted from ListView's three copy-pasted id
|
||||
fallbacks.
|
||||
|
||||
WHAT THIS PINS THAT THE INLINE COPIES COULD NOT. The fallback branch — "no resolved
|
||||
traits, guess from the id" — was unreachable from any test while it lived inline inside
|
||||
two `handleMove` closures and a `useCallback`. It is also the branch that matters most: it
|
||||
runs during first paint and for a stranded card, and when it is wrong the failure is
|
||||
SILENT (a badge that stops appearing, a preserve-progress prompt that stops asking before
|
||||
a move discards completed steps). Nothing throws.
|
||||
|
||||
So both directions are asserted for both helpers: traits win when present, ids are used
|
||||
only when they are absent, and — the case that would otherwise rot — a resolved column
|
||||
whose traits say "not pre-implementation" is NOT overridden by an id that happens to be
|
||||
`todo`. That inversion is what a fourth inline copy would eventually get wrong.
|
||||
|
||||
REVERT CHECK, measured: making either helper ignore its flags argument (`return
|
||||
LEGACY_….has(columnId)`) fails the "traits win" cases; making it ignore the id fallback
|
||||
(`return Boolean(flags?.intake)`) fails the degraded cases.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isIntakeColumnRole, isPreImplementationColumnRole } from "../utils/columnRoles";
|
||||
|
||||
describe("isIntakeColumnRole", () => {
|
||||
it("uses the intake TRAIT when the column resolved", () => {
|
||||
// The point of the whole conversion: a workflow-named intake column with no legacy id.
|
||||
expect(isIntakeColumnRole({ intake: true }, "backlog")).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for a resolved column that is not intake, whatever its id", () => {
|
||||
/*
|
||||
The inversion. `triage` is the legacy intake id, so a helper that consulted the id
|
||||
first — or fell through to it — would answer true here and put a Planning badge on a
|
||||
column its own workflow says is mid-flight.
|
||||
*/
|
||||
expect(isIntakeColumnRole({ intake: false, hold: true }, "triage")).toBe(false);
|
||||
});
|
||||
|
||||
it("falls back to the legacy intake id when the column has NO resolved traits", () => {
|
||||
// First paint, or a card stranded in a column the workflow no longer declares.
|
||||
expect(isIntakeColumnRole(undefined, "triage")).toBe(true);
|
||||
expect(isIntakeColumnRole(undefined, "in-progress")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isPreImplementationColumnRole", () => {
|
||||
it("treats EITHER intake or hold as pre-implementation", () => {
|
||||
// Both mean work has not started, so moving a part-done card in risks its steps.
|
||||
expect(isPreImplementationColumnRole({ intake: true }, "backlog")).toBe(true);
|
||||
expect(isPreImplementationColumnRole({ hold: true }, "parked")).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for a resolved WIP column even when its id is a legacy one", () => {
|
||||
/*
|
||||
The regression this guards: `todo` is the post-U11 merged planning id, so an
|
||||
id-consulting fallback would prompt on a move into a column whose traits say
|
||||
implementation happens there — training operators to dismiss the prompt.
|
||||
*/
|
||||
expect(isPreImplementationColumnRole({ intake: false, hold: false }, "todo")).toBe(false);
|
||||
});
|
||||
|
||||
it("falls back to the legacy pre-implementation ids when traits are absent", () => {
|
||||
/*
|
||||
THE SILENT-LOSS CASE. Without this branch a move during first paint skips the
|
||||
preserve-progress prompt entirely and the operator loses completed steps with no
|
||||
error. It is the reason the fallback survives the conversion.
|
||||
*/
|
||||
expect(isPreImplementationColumnRole(undefined, "todo")).toBe(true);
|
||||
expect(isPreImplementationColumnRole(undefined, "triage")).toBe(true);
|
||||
expect(isPreImplementationColumnRole(undefined, "in-review")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -8,6 +8,7 @@ import type { Task, TaskDetail, Column, ColumnId, TaskCreateInput, MergeResult,
|
||||
import { DEFAULT_COLUMN, THINKING_LEVELS, getErrorMessage, isColumn } from "@fusion/core";
|
||||
import { resolveEffectiveAutoMerge } from "../../../core/src/task-merge";
|
||||
import { useColumnLabel } from "../i18n/labels";
|
||||
import { isIntakeColumnRole, isPreImplementationColumnRole } from "../utils/columnRoles";
|
||||
import { sortTasksForDisplayColumn } from "./taskSorting";
|
||||
import { batchUpdateTaskModels, fetchNodes, fetchTaskDetail, rebuildTaskSpec, refreshPrStatus, updateTask } from "../api";
|
||||
import { TaskDetailContent } from "./TaskDetailModal";
|
||||
@@ -657,8 +658,8 @@ export function ListView({
|
||||
/*
|
||||
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, R8):
|
||||
`LEGACY_LIST_COLUMNS` is DELETED. It synthesised trait flags onto the six
|
||||
hardcoded legacy column ids (`intake: column === "triage"`, `hold: column ===
|
||||
"todo"`, …) — the same defect U10 removed from Board's aggregate lane union,
|
||||
hardcoded legacy column ids (synthesising `intake` onto the legacy intake id,
|
||||
`hold` onto `todo`, …) — the same defect U10 removed from Board's aggregate lane union,
|
||||
surviving in the ListView copy. It only ever fed this arm, which the skeleton
|
||||
gate below makes unreachable: that gate returns unless a lane resolved, and a
|
||||
resolved lane always yields a non-null `selectedWorkflow`. Empty columns render
|
||||
@@ -858,16 +859,14 @@ export function ListView({
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The card's INTAKE role, from its own column's traits. Both grouped-list render paths
|
||||
gated the transient Planning badge on `task.column === "triage"`, which U11 deletes —
|
||||
the badge would simply stop appearing on planning rows, with nothing failing.
|
||||
gated the transient Planning badge on the legacy intake id, which U11 deletes — the
|
||||
badge would simply stop appearing on planning rows, with nothing failing.
|
||||
|
||||
ONE fallback, matching TaskCard: `columnFlagsById` has no entry for a column the
|
||||
resolved workflow does not declare (a stranded card, or the pre-load window), and a
|
||||
bare trait read would drop the badge there too.
|
||||
The id fallback now lives once in `isIntakeColumnRole`, together with the reason it
|
||||
cannot be deleted; see `utils/columnRoles.ts`.
|
||||
*/
|
||||
const isIntakeColumnForTask = useCallback((task: Task): boolean => {
|
||||
const flags = columnFlagsById.get(task.column);
|
||||
return flags ? flags.intake === true : task.column === "triage";
|
||||
return isIntakeColumnRole(columnFlagsById.get(task.column), task.column);
|
||||
}, [columnFlagsById]);
|
||||
|
||||
const isArchivedColumn = useCallback((column: ColumnId): boolean => {
|
||||
@@ -1927,11 +1926,7 @@ export function ListView({
|
||||
traits when they exist makes the rule mean "moving back into a pre-implementation
|
||||
lane", which is the thing worth warning about.
|
||||
*/
|
||||
const shouldPrompt = hasStepProgress && (
|
||||
targetFlags
|
||||
? Boolean(targetFlags.intake || targetFlags.hold)
|
||||
: column === "todo" || column === "triage"
|
||||
);
|
||||
const shouldPrompt = hasStepProgress && isPreImplementationColumnRole(targetFlags, column);
|
||||
let moveOptions: { preserveProgress?: boolean } | undefined;
|
||||
|
||||
if (shouldPrompt) {
|
||||
@@ -2477,13 +2472,8 @@ export function ListView({
|
||||
const task = tasks.find((candidate) => candidate.id === taskId);
|
||||
const hasStepProgress = task?.steps.some((step) => step.status !== "pending") ?? false;
|
||||
const targetFlags = columnFlagsById.get(column);
|
||||
// Same rule as the context-menu move above: flags first, ids only as the
|
||||
// no-metadata fallback.
|
||||
const shouldPrompt = hasStepProgress && (
|
||||
targetFlags
|
||||
? Boolean(targetFlags.intake || targetFlags.hold)
|
||||
: column === "todo" || column === "triage"
|
||||
);
|
||||
// Same rule as the context-menu move above, and now literally the same function.
|
||||
const shouldPrompt = hasStepProgress && isPreImplementationColumnRole(targetFlags, column);
|
||||
|
||||
let moveOptions: { preserveProgress?: boolean } | undefined;
|
||||
if (shouldPrompt) {
|
||||
|
||||
65
packages/dashboard/app/utils/columnRoles.ts
Normal file
65
packages/dashboard/app/utils/columnRoles.ts
Normal file
@@ -0,0 +1,65 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
ONE place that answers "what ROLE does this column play?" when trait metadata may be
|
||||
missing, replacing three copy-pasted id fallbacks in ListView.
|
||||
|
||||
WHY A HELPER AND NOT A BARE TRAIT READ. Trait flags come from the resolved workflow, so
|
||||
`columnFlagsById` legitimately has no entry in two states:
|
||||
|
||||
1. the pre-load window — the board renders before the workflows fetch resolves;
|
||||
2. a stranded card resting in a column its workflow no longer declares.
|
||||
|
||||
A bare `flags.intake === true` returns false in both, which is silent degradation rather
|
||||
than a visible failure: the Planning badge just stops appearing, and a move back into a
|
||||
pre-implementation lane stops asking whether to preserve step progress — so an operator
|
||||
loses completed steps with no prompt and no error. That is why the fallback exists and
|
||||
why deleting it is not the cleanup it looks like.
|
||||
|
||||
What was wrong was having the fallback THREE TIMES, inline, as `column === "todo" ||
|
||||
column === "triage"`. Copies drift, none of them were reachable from a test, and each
|
||||
read like a lifecycle rule rather than the degraded mode it is. Here it is named, has one
|
||||
definition, and is covered — including the degraded path itself, which is the part that
|
||||
never had a test.
|
||||
*/
|
||||
|
||||
/** The subset of a column's resolved trait flags these role questions need. */
|
||||
export interface ColumnRoleFlags {
|
||||
readonly intake?: boolean;
|
||||
readonly hold?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* The pre-graph column ids that behaved as pre-implementation lanes.
|
||||
*
|
||||
* NOT a lifecycle rule — a last-resort guess used only when a column has no resolved
|
||||
* traits. `todo` is the post-U11 merged planning column; `triage` is its pre-merge
|
||||
* predecessor, retained because a project upgraded mid-flight can still hold cards there
|
||||
* while its workflow no longer declares it.
|
||||
*/
|
||||
const LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS: ReadonlySet<string> = new Set(["todo", "triage"]);
|
||||
|
||||
/** Pre-merge intake id, used only when a column has no resolved traits. */
|
||||
const LEGACY_INTAKE_COLUMN_ID = "triage";
|
||||
|
||||
/**
|
||||
* Does this column play the INTAKE role — the lane a card enters before implementation?
|
||||
*
|
||||
* Drives the transient Planning badge. Traits when resolved; the legacy intake id only
|
||||
* when they are absent, so the badge does not vanish during first paint.
|
||||
*/
|
||||
export function isIntakeColumnRole(flags: ColumnRoleFlags | undefined, columnId: string): boolean {
|
||||
return flags ? flags.intake === true : columnId === LEGACY_INTAKE_COLUMN_ID;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this column a PRE-IMPLEMENTATION lane — intake or a hold?
|
||||
*
|
||||
* Drives the "preserve progress?" prompt when a card with completed steps is moved
|
||||
* backwards. Either trait qualifies: both mean work has not started there, so moving a
|
||||
* part-done card in risks discarding steps.
|
||||
*/
|
||||
export function isPreImplementationColumnRole(flags: ColumnRoleFlags | undefined, columnId: string): boolean {
|
||||
return flags
|
||||
? Boolean(flags.intake || flags.hold)
|
||||
: LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS.has(columnId);
|
||||
}
|
||||
Reference in New Issue
Block a user