From a319e35a67eeabd4f28bdc918e6ced89d972d175 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 10:18:18 -0700 Subject: [PATCH] =?UTF-8?q?fix(dashboard):=20the=20card's=20completion=20t?= =?UTF-8?q?imestamp=20reads=20the=20resolved=20complete=20lane=20(census?= =?UTF-8?q?=2013=20=E2=86=92=2012)=20(#3146)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TaskCard.tsx` 1 → 0. **Census 13 → 12**, baseline re-recorded in-commit. ## The defect `getInReviewCompletionMs` gated on `task.column === "done"`, so on a board whose completion lane is renamed, a finished card rendered its execution time **without the completion half** — the `Completed ` part of the indicator's `title` / `aria-label` never appeared. Nobody reported it because the card does not look broken. It looks like a card whose completion time was never recorded. ## The recorded blocker had expired, and I trusted it twice The note on that helper read: > Module-scope, takes only a `Task`, and has no flags to consult. Converting it means either threading resolved flags through a pure duration helper or resolving a workflow inside it. True when written (2026-07-30). False within a day, and the evidence is in the same file: - `taskColumnFlags` is a **prop of this component**, destructured and already consumed by `isWipColumnRole` / `isReviewColumnRole`. - The **sibling duration helpers were threaded for exactly this purpose** — `getTotalAgentActiveMs` carries the note *"THREADED SO THE CONVERSION IS NOT INERT"*. - This helper has **one caller**, inside the component, where the flags are in scope. The threading the note called prohibitive was already done; only this helper was left behind. I read that note twice this week and took it at face value both times — and what finally prompted the check was main landing `taskRevert 2 → 0 — **the recorded blocker named the wrong variable**` (#3129), someone else finding the same class of decay in a note I had also accepted. This program's own learnings say a deferral's stated blocker is a claim that ages like any measurement. I had applied every other entry in that document this week except that one. ## A dependency-array bug the conversion would have introduced The memo now reads `taskColumnFlags`, so it joins the dependency array. Flags arrive **asynchronously** — the board resolves workflows after first paint — so a card rendered before they load and re-rendered after would otherwise keep the pre-flag answer, since none of the memo's other inputs changed. This repo has **no `react-hooks/exhaustive-deps` rule**, so nothing would have flagged the omission. ## Two wrong probes before a correct one, both caught by controls and mutation Recording these because the fix was right from the start and my instruments were not: 1. **`textContent` matched nothing.** The completion time lands in `title`/`aria-label`, never in visible text. The **control failed too** — the signature of a broken probe rather than a broken fix. 2. **`innerHTML` on the whole card matched always.** The lifecycle-dates footer renders its own `Completed ` line, and *that* path already resolves the complete lane correctly. The probe was reading a different, already-converted feature. **Mutation exposed it: reverting the fix left all six green.** The final assertion queries `.card-time-indicator` and reads its `title`, which is the only form that can tell the two apart. ## Verification | | result | |---|---| | suite | **6 passed** | | mutation (restore `=== "done"`) | **1 failed \| 5 passed** — the renamed case only, control still green | | dashboard `tsc -p tsconfig.app.json` | **0 errors** | | census `--strict` | exit 0, baseline re-recorded in-commit | Flags stay optional with the legacy id as fallback (`isCompleteColumnRole`), so any caller without resolved flags behaves exactly as before. ## Note on `check-fnxc-future-dates` It fails on this branch, but **not because of it** — `scheduler.ts` and one PG test carry future stamps on `main` itself. #3139 fixes that. None of my files appear in the report. Co-authored-by: Claude Opus 5 (1M context) --- .../dashboard/app/components/TaskCard.tsx | 36 +++++++- ...Card.time-indicator-renamed-lanes.test.tsx | 88 +++++++++++++++++++ .../lib/lifecycle-column-census-baseline.json | 1 - 3 files changed, 120 insertions(+), 5 deletions(-) diff --git a/packages/dashboard/app/components/TaskCard.tsx b/packages/dashboard/app/components/TaskCard.tsx index f0f47b2162..94d6a7180e 100644 --- a/packages/dashboard/app/components/TaskCard.tsx +++ b/packages/dashboard/app/components/TaskCard.tsx @@ -401,9 +401,32 @@ Module-scope, takes only a `Task`, and has no flags to consult. Converting it me resolved flags through a pure duration helper or resolving a workflow inside it — the same shape flagged at `project-engine.ts:2555` and `github-tracking-comments.ts:165`. Left counted so the census keeps pointing at the class rather than at me having decided it away. + +FNXC:WorkflowResolvedColumns 2026-07-31-23:59 — THAT BLOCKER HAS SINCE EXPIRED, and the evidence is in +this file. + +"Has no flags to consult" was true when written and is not true now. `taskColumnFlags` is a prop of +this component, destructured and already consumed by `isWipColumnRole` / `isReviewColumnRole` a few +hundred lines below, and `TaskContextMenuColumnFlags` carries `complete`. The sibling duration helpers +here were threaded for exactly this reason — `getTotalAgentActiveMs` carries the note "THREADED SO THE +CONVERSION IS NOT INERT". This helper has ONE caller, inside the component, where the flags are in +scope. + +So the threading the note called prohibitive is already done; only this helper was left behind. The +flags are OPTIONAL and the legacy id remains the fallback (`isCompleteColumnRole`), so a caller without +resolved flags behaves exactly as before. + +WHAT THE LITERAL COST: on a board whose complete lane is renamed, `task.column === "done"` matched +nothing, so a finished card showed its execution time WITHOUT the completion timestamp — the "done N +ago" half of the label simply never appeared. Cosmetic, but only visible on renamed boards, which is +why nobody reported it. + +A DECAYED DEFERRAL, recorded as such: this program's learnings say a deferral's stated blocker is a +claim that ages like any measurement. Mine aged out in one day, and I re-read it twice this week and +took it at face value both times. */ -function getInReviewCompletionMs(task: Task): number | null { - return task.column === "done" ? getDoneCompletionMs(task) : null; +function getInReviewCompletionMs(task: Task, columnFlags?: TaskContextMenuColumnFlags): number | null { + return isCompleteColumnRole(columnFlags, task.column) ? getDoneCompletionMs(task) : null; } function getMergeElapsedMs(task: Task, nowMs: number): number | null { @@ -1838,7 +1861,7 @@ function TaskCardComponent({ return null; } - const completionMs = getInReviewCompletionMs(task); + const completionMs = getInReviewCompletionMs(task, taskColumnFlags); if (completionMs == null) { return { label: elapsedLabel, @@ -1853,7 +1876,12 @@ function TaskCardComponent({ title: t("tasks.executionTimeCompleted", "Execution time {{elapsed}}. Completed {{completedAt}}", { elapsed: elapsedLabel, completedAt }), ariaLabel: t("tasks.executionTimeCompleted", "Execution time {{elapsed}}. Completed {{completedAt}}", { elapsed: elapsedLabel, completedAt }), }; - }, [task.column, task.status, task.columnMovedAt, task.timedExecutionMs, task.updatedAt, task.workflowStepResults, task.log, task.firstExecutionAt, task.cumulativeActiveMs, task.cumulativePlanningMs, task.planningStartedAt, task.executionStartedAt, task.executionCompletedAt, timeIndicatorNowMs]); + /* FNXC:WorkflowResolvedColumns 2026-07-31-23:59: `taskColumnFlags` joins the deps because this memo + now READS it. Flags arrive asynchronously (the board resolves workflows after first paint), so a + card that renders before they load and re-renders after would otherwise keep the pre-flag answer + — the memo's inputs would be unchanged. This repo has no `react-hooks/exhaustive-deps` rule, so + nothing would have flagged the omission. */ + }, [task.column, task.status, task.columnMovedAt, task.timedExecutionMs, task.updatedAt, task.workflowStepResults, task.log, task.firstExecutionAt, task.cumulativeActiveMs, task.cumulativePlanningMs, task.planningStartedAt, task.executionStartedAt, task.executionCompletedAt, timeIndicatorNowMs, taskColumnFlags]); const lifecycleDates = useMemo(() => { const created = formatCompactLifecycleDate(task.createdAt, locale, new Date(lifecycleNowMs)); diff --git a/packages/dashboard/app/components/__tests__/TaskCard.time-indicator-renamed-lanes.test.tsx b/packages/dashboard/app/components/__tests__/TaskCard.time-indicator-renamed-lanes.test.tsx index 0a80d47432..c24c11bd4f 100644 --- a/packages/dashboard/app/components/__tests__/TaskCard.time-indicator-renamed-lanes.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskCard.time-indicator-renamed-lanes.test.tsx @@ -88,3 +88,91 @@ describe("the card time indicator under a renamed board vocabulary", () => { expect(hasDuration(container as unknown as HTMLElement)).toBe(false); }); }); + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:59: +THE COMPLETION HALF OF THE SAME LABEL, which the cases above do not reach. + +`getInReviewCompletionMs` gated on `task.column === "done"`, so on a board with a renamed completion +lane a finished card rendered its execution time WITHOUT the "done N ago" suffix — the label appears, +just permanently missing half of itself. That is why nobody reported it: the card does not look +broken, it looks like a card whose completion time has not been recorded. + +The deferral note on that helper said it had "no flags to consult". That was true when written and +expired within a day: `taskColumnFlags` is a prop of this component, the sibling duration helpers in +that file were threaded for exactly this purpose, and this helper's single caller sits inside the +component where the flags are in scope. + +DIFFERENTIAL BY CONSTRUCTION: `shipped` collides with no legacy id, so a surviving `=== "done"` cannot +pass by luck, and the control below pins that the default vocabulary still works. +*/ +function finishedTaskIn(column: string): Task { + return { + ...runningTaskIn(column), + executionCompletedAt: "2026-06-01T00:30:00.000Z", + columnMovedAt: "2026-06-01T00:30:00.000Z", + updatedAt: "2026-06-01T00:30:00.000Z", + } as unknown as Task; +} + +/* +SCOPED TO `.card-time-indicator`, and it took two wrong probes to get here — both caught by controls +and by mutation rather than by reading the code. + + 1. `textContent` matched nothing: the completion time lands in the indicator's `title` / + `aria-label`, never in visible text. The CONTROL failed too, which is the signature of a broken + probe rather than a broken fix. + 2. `innerHTML` on the whole card matched ALWAYS: the lifecycle-dates footer renders its own + "Completed " line, and that path resolves the complete lane CORRECTLY already. So the probe + was reading a different, already-converted feature. Mutation exposed it — reverting the fix left + all six green. + +Querying the indicator element and reading its `title` is the only assertion that can distinguish the +two, which is the whole point of the test. +*/ +const completionTitle = (root: HTMLElement) => + root.querySelector(".card-time-indicator")?.getAttribute("title") ?? ""; +const hasCompletionSuffix = (root: HTMLElement) => /Completed/i.test(completionTitle(root)); + +describe("the card completion timestamp under a renamed board vocabulary", () => { + /* Control: the legacy `done` lane renders the completion suffix with no flags supplied. */ + it("default vocabulary: a card in `done` shows when it completed", () => { + const { container } = render( + , + ); + + expect(hasCompletionSuffix(container as unknown as HTMLElement)).toBe(true); + }); + + /* The defect: `shipped` matched no legacy id, so the completion half never rendered. */ + it("renamed vocabulary: a card whose traits say COMPLETE shows when it completed", () => { + const { container } = render( + , + ); + + expect(hasCompletionSuffix(container as unknown as HTMLElement)).toBe(true); + }); + + /* + The paired negative: resolving traits must not stamp a completion time on a card that has not + finished. A renamed WIP card is still running, so the suffix must stay absent — otherwise the fix + trades a missing timestamp for a false one. + */ + it("renamed vocabulary: a running card in the WIP lane shows no completion time", () => { + const { container } = render( + , + ); + + expect(hasCompletionSuffix(container as unknown as HTMLElement)).toBe(false); + }); +}); diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 3034a7ab4a..1e0976bd49 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -8,7 +8,6 @@ "packages/core/src/task-store/moves.ts": 1, "packages/core/src/task-store/task-id-integrity.ts": 1, "packages/dashboard/app/components/ResearchTaskActionModal.tsx": 1, - "packages/dashboard/app/components/TaskCard.tsx": 1, "packages/engine/src/notification/notification-service.ts": 1, "packages/engine/src/self-healing.ts": 1, "packages/engine/src/triage.ts": 1