diff --git a/.changeset/workflow-analytics-renamed-lanes.md b/.changeset/workflow-analytics-renamed-lanes.md new file mode 100644 index 0000000000..0009ad0370 --- /dev/null +++ b/.changeset/workflow-analytics-renamed-lanes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Per-workflow Command Center metrics now count work on renamed boards. +category: fix +dev: `aggregateWorkflowAnalytics` takes an optional lane store and resolves complete / wip / human-review columns via `resolveProjectColumnsForRoles`; its SQL previously filtered on the literal `'done'` and `('in-progress','in-review')`. diff --git a/packages/core/src/__tests__/postgres/workflow-analytics-renamed-lanes.pg.test.ts b/packages/core/src/__tests__/postgres/workflow-analytics-renamed-lanes.pg.test.ts new file mode 100644 index 0000000000..6dc67a605d --- /dev/null +++ b/packages/core/src/__tests__/postgres/workflow-analytics-renamed-lanes.pg.test.ts @@ -0,0 +1,160 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-18:50 (per-workflow metrics read zero on a renamed board): + +`aggregateWorkflowAnalytics` filtered in SQL on `t."column" = 'done'` and +`IN ('in-progress','in-review')`. On a board whose lanes are renamed those match nothing, so +`tasksCompleted`, `tasksInProgress` and `tasksInReview` all come back ZERO for every workflow while +the board is busy. Nothing errors. + +WHY NO EXISTING CHECK SAW IT. The lifecycle census parses TypeScript comparisons; these ids live +inside SQL strings. The sweep that converted this file's TypeScript guards left the queries alone, +and the file scored as converted. + +The cases are DIFFERENTIAL: the same seeded work aggregated under two vocabularies whose roles are +identical and only the ids differ. `shipped` and `checking` collide with no legacy id, so a surviving +`'done'` cannot pass by luck. + +`tasksInReview` is asserted as well as `tasksCompleted` because the two queries take DIFFERENT +resolved sets — complete for one, wip+human-review for the other. Asserting only the completed count +would leave the second conversion unproven, which is the partial-supply shape this program keeps +re-finding. +*/ + +import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest"; +import { sql } from "drizzle-orm"; +import { + pgDescribe, + createSharedPgTaskStoreTestHarness, + type SharedPgTaskStoreHarness, +} from "../../__test-utils__/pg-test-harness.js"; +import { aggregateWorkflowAnalytics } from "../../workflow-analytics.js"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../../index.js"; + +const IN_RANGE = "2026-06-15T12:00:00.000Z"; +const RANGE = { from: "2026-06-01T00:00:00.000Z", to: "2026-06-30T23:59:59.999Z" }; + +/* +The per-column trait map production supplies (`resolveColumnFlagsByName(store)` at the Command Center +route). The SQL fix decides WHICH rows come back; this map decides which bucket each row lands in via +`isWipColumnRole` / `isReviewColumnRole`. Both halves have to be right — omitting this map was my +first mistake here, and the renamed case failed even with the query fixed, which is exactly the +partial-conversion shape the note above warns about. +*/ +const RENAMED_FLAGS = new Map([ + ["building", { countsTowardWip: true }], + ["checking", { countsTowardWip: true, humanReview: true }], + ["shipped", { complete: true }], +]); +const LEGACY_FLAGS = new Map([ + ["in-progress", { countsTowardWip: true }], + ["in-review", { countsTowardWip: true, humanReview: true }], + ["done", { complete: true }], +]); + +pgDescribe("workflow analytics under a renamed board vocabulary", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_workflow_analytics_lanes", + }); + + beforeAll(h.beforeAll); + beforeEach(h.beforeEach); + afterEach(h.afterEach); + afterAll(h.afterAll); + + /** The builtin coding workflow with only its column ids renamed. */ + async function seedRenamedWorkflow(): Promise { + const RENAME: Record = { + todo: "drafting", + "in-progress": "building", + "in-review": "checking", + done: "shipped", + }; + const rename = (id: string | undefined) => (id && RENAME[id]) ?? id; + const ir = JSON.parse(JSON.stringify(BUILTIN_CODING_WORKFLOW_IR)) as { + id: string; + nodes?: { column?: string }[]; + columns?: { id: string }[]; + }; + ir.id = "custom:renamed-workflow-analytics"; + for (const node of ir.nodes ?? []) node.column = rename(node.column); + for (const column of ir.columns ?? []) column.id = rename(column.id) as string; + + const ids = (ir.columns ?? []).map((column) => column.id); + expect(ids).toContain("shipped"); + expect(ids).not.toContain("done"); + + await h.store().createWorkflowDefinition({ name: "Renamed", kind: "workflow", ir } as never); + } + + /** One finished task and one sitting in the review lane. */ + async function seedWork(completeLane: string, reviewLane: string): Promise { + const store = h.store(); + const adminDb = h.adminDb(); + for (const [id, lane] of [["KB-DONE", completeLane], ["KB-REVIEW", reviewLane]] as const) { + await store.createTaskWithReservedId( + { description: id, column: "todo" }, + { taskId: id, createdAt: IN_RANGE, updatedAt: IN_RANGE, applyDefaultWorkflowSteps: false }, + ); + /* Seeded directly: the aggregator reads column_moved_at, and moveTask would stamp it with + `now` rather than a date inside the query range. */ + await adminDb.execute(sql` + UPDATE project.tasks + SET "column" = ${lane}, column_moved_at = ${IN_RANGE}, updated_at = ${IN_RANGE} + WHERE id = ${id}`); + store.taskCache.delete(id); + } + } + + const run = (withStore: boolean, renamed = false) => aggregateWorkflowAnalytics( + Object.assign(h.layer(), { projectId: "p1" }), + { ...RANGE, columnFlagsByName: renamed ? RENAMED_FLAGS : LEGACY_FLAGS }, + withStore ? h.store() : undefined, + ); + + /* Control: the default vocabulary counts both. Passes before and after the fix, so a generally + broken aggregator cannot hide behind the renamed case below. */ + it("default vocabulary: completed and in-review work are counted", async () => { + await seedWork("done", "in-review"); + + const wf = await run(true); + + expect(wf.totals.tasksCompleted).toBe(1); + expect(wf.totals.tasksInReview).toBe(1); + }); + + /* The defect: before the fix both queries matched nothing on this board. */ + it("renamed vocabulary: completed and in-review work are counted", async () => { + await seedRenamedWorkflow(); + await seedWork("shipped", "checking"); + + const wf = await run(true, true); + + expect(wf.totals.tasksCompleted).toBe(1); + expect(wf.totals.tasksInReview).toBe(1); + }); + + /* + The paired negative: resolving real lanes must not degrade into "every column counts". A card still + in the hold lane is neither completed nor in review — otherwise the fix trades an undercount for an + overcount, which is harder to notice. + */ + it("renamed vocabulary: a card in the HOLD lane counts as neither", async () => { + await seedRenamedWorkflow(); + await seedWork("drafting", "drafting"); + + const wf = await run(true, true); + + expect(wf.totals.tasksCompleted).toBe(0); + expect(wf.totals.tasksInReview).toBe(0); + }); + + /* Omitting the store must keep the legacy answer, so an unconverted caller is byte-identical. */ + it("without a lane store, the legacy ids still answer", async () => { + await seedWork("done", "in-review"); + + const wf = await run(false); + + expect(wf.totals.tasksCompleted).toBe(1); + expect(wf.totals.tasksInReview).toBe(1); + }); +}); diff --git a/packages/core/src/workflow-analytics.ts b/packages/core/src/workflow-analytics.ts index b6e1002eb5..6095a5d4c5 100644 --- a/packages/core/src/workflow-analytics.ts +++ b/packages/core/src/workflow-analytics.ts @@ -1,4 +1,5 @@ import { isReviewColumnRole, isWipColumnRole, type ColumnRoleTraitFlags } from "./column-roles.js"; +import { resolveProjectColumnsForRoles, type ProjectLaneVocabularyStore } from "./project-lane-vocabulary.js"; import { sql } from "drizzle-orm"; import type { Database } from "./db.js"; import type { AsyncDataLayer } from "./postgres/data-layer.js"; @@ -350,10 +351,27 @@ function buildWorkflowAnalytics( export async function aggregateWorkflowAnalytics( dbOrLayer: Database | AsyncDataLayer, query: WorkflowAnalyticsQuery = {}, + /* + FNXC:WorkflowResolvedColumns 2026-07-30-18:30: + The store, used ONLY to resolve which columns carry the complete / wip / human-review traits. + + These queries filtered on `t."column" = 'done'` and `IN ('in-progress','in-review')`. Those ids sit + inside SQL strings, which the lifecycle census cannot see because it parses TypeScript comparisons — + so the sweep that converted this file's TS guards left the queries alone and the file scored as + converted. On a renamed board every per-workflow completed count and throughput figure reads ZERO + while the board is busy, with no error. + + Resolved per PROJECT: this aggregates a whole project, so the union of a role's columns across its + workflows is the right set and a bound IN list is enough. (Per-task lanes need the + superset-then-decide-in-JS shape instead — see cleanupStaleMergeQueueRowsInTransaction.) + + Omitted, the legacy ids answer, so an unconverted caller is byte-identical. + */ + laneStore?: ProjectLaneVocabularyStore, ): Promise { const defaultWorkflowId = query.defaultWorkflowId ?? "builtin:coding"; if ("ping" in dbOrLayer) { - return aggregateWorkflowAnalyticsAsync(dbOrLayer, query, defaultWorkflowId); + return aggregateWorkflowAnalyticsAsync(dbOrLayer, query, defaultWorkflowId, laneStore); } const db = dbOrLayer as Database; @@ -440,11 +458,62 @@ export async function aggregateWorkflowAnalytics( * returns it parsed) so it is re-stringified to feed the shared * countModifiedFiles helper unchanged. */ +/* The legacy active ids are always retained: the classifier's fallback recognises them, so they can + never be the "selected but unclassifiable" case this filter exists to prevent. */ +const LEGACY_ACTIVE_LANES: readonly string[] = ["in-progress", "in-review"]; + async function aggregateWorkflowAnalyticsAsync( layer: AsyncDataLayer, query: WorkflowAnalyticsQuery, defaultWorkflowId: string, + laneStore?: ProjectLaneVocabularyStore, ): Promise { + /* + FNXC:PostgresCommandCenterAnalytics 2026-07-30-21:20 (#2866 review — greptile P1 x2, and the second + one is a self-contradiction rather than an imprecision): + + THE SQL FILTER AND THE ROW CLASSIFIER MUST DROP THE SAME COLUMNS. + + `resolveProjectColumnsForRoles` unions every column any workflow gives the role, so a column id two + workflows reuse with DIFFERENT traits stays in the filter. `columnFlagsByName` — built by the + Command Center route and consumed ~200 lines below — deliberately DROPS such an id as conflicting, + on the argument that a merged entry double-counts (see its own note, #2803 review). + + The two together produce rows that are SELECTED and then classified by nothing: + `columnFlagsByName.get("checking")` is undefined, so `isWipColumnRole` and `isReviewColumnRole` both + fall back to the legacy ids, `checking` matches neither, and the count silently evaporates. Worse + than either half alone — the filter says the lane counts, the classifier says it cannot say, and the + operator sees a workflow sitting at zero. + + Aligning on the CLASSIFIER's answer is the conservative direction: a column it refuses to judge is + removed from the filter too, so the rows are never selected rather than selected and discarded. The + totals are unchanged (they were already lost); what changes is that the two halves now agree, and a + reader is not left hunting for where the rows went. + + CONSEQUENTLY THIS IS UNCOVERED, AND THAT IS INHERENT, NOT AN OMISSION. Mutating the filter away + leaves the renamed-lane suite green, because the counts were already zero on both sides — the only + observable difference is whether a workflow with no other rows appears with zeros or is absent. + A test pinning THAT would be asserting an artifact of , not the invariant. The + invariant worth covering is the one the first finding names, and it needs the per-workflow map. + + The FIRST finding — a project-wide union admitting one workflow's rows into another's completed + count — is real and NOT fixed here: unlike `team-analytics`, these rows carry a workflow id, so a + per-workflow lane map is feasible and is the right fix. It needs the lane resolution keyed by + workflow rather than by project, which changes this function's contract with its caller. Recorded on + the PR rather than folded into a review-response commit. + */ + const droppedAsConflicting = (lane: string): boolean => + query.columnFlagsByName !== undefined && !query.columnFlagsByName.has(lane); + const completeLanes = laneStore + ? [...await resolveProjectColumnsForRoles(laneStore, ["complete"])] + : ["done"]; + const activeLanes = (laneStore + ? [...await resolveProjectColumnsForRoles(laneStore, ["countsTowardWip", "humanReview"])] + : ["in-progress", "in-review"] + ).filter((lane) => LEGACY_ACTIVE_LANES.includes(lane) || !droppedAsConflicting(lane)); + /* An IN list of bound parameters, not `= ANY(${array})`: drizzle expands a JS array in a template + into a tuple, which PostgreSQL rejects for ANY. Each id stays a parameter. */ + const inList = (lanes: readonly string[]) => sql.join(lanes.map((lane) => sql`${lane}`), sql`, `); const wfExpr = sql`COALESCE(NULLIF(s.workflow_id, ''), ${defaultWorkflowId})`; const tokFrom = query.from !== undefined ? sql`AND t.token_usage_last_used_at >= ${query.from}` : sql``; @@ -484,7 +553,7 @@ async function aggregateWorkflowAnalyticsAsync( sql`SELECT ${wfExpr} AS "workflowId", count(*)::int AS count FROM project.tasks t LEFT JOIN project.task_workflow_selection s ON s.task_id = t.id - WHERE t."column" = 'done' AND t.column_moved_at IS NOT NULL ${compFrom} ${compTo} + WHERE t."column" IN (${inList(completeLanes)}) AND t.column_moved_at IS NOT NULL ${compFrom} ${compTo} GROUP BY 1`, )) as Array<{ workflowId: string; count: number }>; const completedRows: CountByWorkflowRow[] = completedRowsRaw.map((r) => ({ @@ -498,7 +567,7 @@ async function aggregateWorkflowAnalyticsAsync( sql`SELECT ${wfExpr} AS "workflowId", t."column" AS "columnName", count(*)::int AS count FROM project.tasks t LEFT JOIN project.task_workflow_selection s ON s.task_id = t.id - WHERE t."column" IN ('in-progress', 'in-review') ${curFrom} ${curTo} + WHERE t."column" IN (${inList(activeLanes)}) ${curFrom} ${curTo} GROUP BY 1, t."column"`, )) as Array<{ workflowId: string; columnName: string; count: number }>; const currentRows: Array = currentRowsRaw.map((r) => ({ diff --git a/packages/dashboard/src/routes/register-command-center-routes.ts b/packages/dashboard/src/routes/register-command-center-routes.ts index fd2387e56e..ed3110a9ce 100644 --- a/packages/dashboard/src/routes/register-command-center-routes.ts +++ b/packages/dashboard/src/routes/register-command-center-routes.ts @@ -519,7 +519,9 @@ async function resolveColumnFlagsByName( pricingOverrides: settings.modelPricingOverrides, defaultWorkflowId, columnFlagsByName: await resolveColumnFlagsByName(store), - }); + /* FNXC:WorkflowResolvedColumns 2026-07-30-18:35: store supplied so the per-workflow completed + and in-flight queries resolve the board's real lanes instead of 'done'/'in-progress'. */ + }, store); if (wantsCsv(req.query)) { sendCsv(res, "command-center-workflows.csv", workflowAnalyticsToTable(result)); return; diff --git a/scripts/lib/sql-column-literals-baseline.json b/scripts/lib/sql-column-literals-baseline.json index 43890a1b3a..5afd8218b7 100644 --- a/scripts/lib/sql-column-literals-baseline.json +++ b/scripts/lib/sql-column-literals-baseline.json @@ -12,5 +12,5 @@ "packages/core/src/task-store/task-artifacts-ops.ts": 1, "packages/core/src/task-store/workflow-definitions.ts": 2, "packages/core/src/team-analytics.ts": 3, - "packages/core/src/workflow-analytics.ts": 6 + "packages/core/src/workflow-analytics.ts": 3 }