diff --git a/packages/core/src/__tests__/eval-signal-collector.test.ts b/packages/core/src/__tests__/eval-signal-collector.test.ts index 93521817b3..312b841b61 100644 --- a/packages/core/src/__tests__/eval-signal-collector.test.ts +++ b/packages/core/src/__tests__/eval-signal-collector.test.ts @@ -55,4 +55,46 @@ describe("collectDeterministicSignals", () => { expect(signals.commitSummary.commitCount).toBe(0); expect(signals.logSummary).toEqual({ errorCount: 0, warningCount: 0, timingEntries: 0 }); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:58: + `column` is a two-value eval-record field, and against the `archived` literal a card resting in a + RENAMED archive lane was recorded as `"done"`. + + Not a crash and not a lifecycle decision — a mislabelled row in the eval corpus, which is a dataset + every later comparison reads. That is the expensive kind of quiet: nothing fails, the numbers just + drift. + + Only the ARCHIVED arm was ever wrong. A renamed COMPLETE lane was, and remains, recorded as + `"done"`, which is correct — so only the archived answer is resolved, and the third case below + pins that the widening did not turn every renamed lane into `"archived"`. + */ + it("records a RENAMED archive lane as archived", () => { + const task = makeTask({ column: "filed", log: [] }); + const signals = collectDeterministicSignals( + task, + { runId: "ER-3", startedAt: "2026-05-02T00:00:00.000Z" }, + { archivedColumns: new Set(["archived", "filed"]) }, + ); + expect(signals.column).toBe("archived"); + }); + + it("still records the legacy id as archived when no resolved answer is supplied", () => { + /* CONTROL: the parameter is optional, so an unwired caller must behave exactly as before. */ + const task = makeTask({ column: "archived", log: [] }); + const signals = collectDeterministicSignals(task, { runId: "ER-4", startedAt: "2026-05-02T00:00:00.000Z" }); + expect(signals.column).toBe("archived"); + }); + + it("records a renamed COMPLETE lane as done, not archived", () => { + /* The paired negative: the resolved set names the archive lanes only. A card in the board's + completion lane is `done`, which is what it always was and must stay. */ + const task = makeTask({ column: "shipped", log: [] }); + const signals = collectDeterministicSignals( + task, + { runId: "ER-5", startedAt: "2026-05-02T00:00:00.000Z" }, + { archivedColumns: new Set(["archived", "filed"]) }, + ); + expect(signals.column).toBe("done"); + }); }); diff --git a/packages/core/src/eval-signal-collector.ts b/packages/core/src/eval-signal-collector.ts index c5c766a15c..1c3034c75b 100644 --- a/packages/core/src/eval-signal-collector.ts +++ b/packages/core/src/eval-signal-collector.ts @@ -69,7 +69,28 @@ function collectCommitSummary(task: TaskDetail): DeterministicSignals["commitSum }; } -export function collectDeterministicSignals(task: TaskDetail, _run: EvalRunContext): DeterministicSignals { +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:55: +`archivedColumns` is an optional RESOLVED answer supplied by the caller; omitted, the `archived` +literal answers exactly as before. + +This collector is SYNC and pure — no store, no workflow — so the lane answer has to arrive as a +parameter. `HybridEvaluatorService.evaluateTask` is async and already holds an optional store, which +is where the resolution is paid. + +WHAT THE LITERAL COST. `column` is a two-value eval-record field, so a renamed ARCHIVE lane was +recorded as `"done"`. Not a crash and not a lifecycle decision — a mislabelled row in the eval +corpus, which is a dataset every later comparison reads. Wrong labels in evaluation data are quiet +in exactly the way that makes them expensive: nothing fails, the numbers just drift. + +A renamed COMPLETE lane is unaffected either way — it was, and remains, `"done"`, which is correct. +Only the archived arm was ever wrong, so only it is resolved. +*/ +export function collectDeterministicSignals( + task: TaskDetail, + _run: EvalRunContext, + options?: { archivedColumns?: ReadonlySet }, +): DeterministicSignals { const workflowSummary = countWorkflow(task.workflowStepResults); const logSummaryWithEvidence = summarizeLogs(task.log ?? []); const commitSummary = collectCommitSummary(task); @@ -110,7 +131,9 @@ export function collectDeterministicSignals(task: TaskDetail, _run: EvalRunConte return { taskId: task.id, - column: task.column === "archived" ? "archived" : "done", + column: (options?.archivedColumns ? options.archivedColumns.has(task.column) : task.column === "archived") + ? "archived" + : "done", executionStartedAt: task.executionStartedAt, executionCompletedAt: task.executionCompletedAt, timedExecutionMs: task.timedExecutionMs, diff --git a/packages/engine/src/evaluator.ts b/packages/engine/src/evaluator.ts index 7acbb1687d..c05a1f157f 100644 --- a/packages/engine/src/evaluator.ts +++ b/packages/engine/src/evaluator.ts @@ -4,6 +4,7 @@ import { normalizeCategoryScore, resolveScoreBand, resolveValidatorSettingsModel, + resolveProjectColumnsForRoles, EVAL_SCORE_CATEGORIES, type DeterministicSignals, type EvalScoreCategory, @@ -72,7 +73,16 @@ export class HybridEvaluatorService { settings: Partial, modelOverride?: EvaluatorModelOverride, ): Promise> { - const deterministicSignals = collectDeterministicSignals(task, run); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:55: + Resolve the archive lanes here, where an await is legal, and hand them to the sync collector. The + store is OPTIONAL on this service, so a store-less evaluator degrades to the legacy `archived` + literal — the collector's documented default — rather than failing. + */ + const archivedColumns = this.deps.store + ? await resolveProjectColumnsForRoles(this.deps.store, ["archived"]).catch(() => undefined) + : undefined; + const deterministicSignals = collectDeterministicSignals(task, run, { archivedColumns }); const model = resolveEvaluatorModel(settings, modelOverride); const evidenceBundle = this.deps.store ? await (this.deps.collectEvidence ?? collectTaskEvaluationEvidence)({ diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index b767593dd4..ec823f3570 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -1128,6 +1128,24 @@ export class Scheduler { this.lastAutoClaimFingerprint.set(task.id, nextFingerprint); this.options.snapshotManager?.invalidate("task:updated"); } + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:58 (FLAGGED AND LEFT COUNTED — do NOT convert with + `resolveTaskParkedColumnsSync`): + This literal and the `in-review` one further down are the two the sync-lane pass did not take, + and nothing in this file said why. Converting them the way the other ten were converted would + make them INERT, not fixed: `getTaskWorkflowSelectionImpl` returns `undefined` unconditionally + under PostgreSQL, so `resolveTaskWorkflowIrSync` always answers with the DEFAULT builtin IR and + every lane it yields is the legacy id (proved in `postgres/sync-workflow-ir-is-always-default.pg.test.ts`; + `check-inert-sync-lane-conversions` baselines the twenty guards already in that state here). + + They stay literal and COUNTED, which is the honest state: an unconverted literal is at least + visible to the census, while an inert conversion leaves the backlog and takes the evidence with + it. Both live in a synchronous `task:updated` listener, so the async resolver is unavailable + without reordering this handler against every other subscriber. + + Unblocking needs a sync-capable workflow-selection reader — one change that un-inerts every + sync-path conversion in this file at once. + */ // Track mission failure signals before moveTask clears failure metadata. if (task.sliceId && task.status === "failed") { if (task.column === "in-progress") this.failedTaskIds.add(task.id); @@ -1204,6 +1222,8 @@ export class Scheduler { } if (!this.options.prMonitor) return; + /* FNXC:WorkflowResolvedColumns 2026-07-31-23:58: the second of the two honest literals — see + the note on the mission-failure guard above for why converting it here would be inert. */ if (task.column !== "in-review") return; if (!task.prInfo) return;