diff --git a/.changeset/step-narration-one-based.md b/.changeset/step-narration-one-based.md new file mode 100644 index 0000000000..fada6d3852 --- /dev/null +++ b/.changeset/step-narration-one-based.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Task chat step narration now shows 1-based step numbers matching the task card's step count. +category: fix +dev: Display-only change in proactive-status.ts builders and merge-queue-ops proactiveStepStatusMessage; the 0-based step-index contract (tools, PROMPT.md headings, run-audit) is unchanged. diff --git a/packages/core/src/__tests__/postgres/proactive-step-status.pg.test.ts b/packages/core/src/__tests__/postgres/proactive-step-status.pg.test.ts index 4f5cd15681..8fe30ca98b 100644 --- a/packages/core/src/__tests__/postgres/proactive-step-status.pg.test.ts +++ b/packages/core/src/__tests__/postgres/proactive-step-status.pg.test.ts @@ -36,8 +36,9 @@ pgTest("proactive step-status chat entries (PostgreSQL)", () => { const statuses = (await harness.store.getAgentLogs(task.id, { type: "status" })).map((entry) => entry.text); expect(statuses).toEqual([ - "Step 0 finished — Implement the change.", - "Step 0 was returned to pending — Implement the change.", + // Narration displays 1-based numbers (0-based index 0 → "Step 1") to match the task card. + "Step 1 finished — Implement the change.", + "Step 1 was returned to pending — Implement the change.", ]); }); }); diff --git a/packages/core/src/task-store/merge-queue-ops.ts b/packages/core/src/task-store/merge-queue-ops.ts index 57fc406d22..1ae80726e9 100644 --- a/packages/core/src/task-store/merge-queue-ops.ts +++ b/packages/core/src/task-store/merge-queue-ops.ts @@ -41,16 +41,20 @@ function proactiveStepStatusMessage( status: import("../types.js").StepStatus, ): string | null { if (previousStatus === status) return null; - const label = stepName.trim() || `Step ${stepIndex}`; + // FNXC:ProactiveChatStatus 2026-07-23-10:30: + // Chat narration must display 1-based step numbers to match the task card's "N/M steps" + // counting; stepIndex stays 0-based in the store/tool contract (see proactive-status.ts). + const display = stepIndex + 1; + const label = stepName.trim() || `Step ${display}`; switch (status) { case "in-progress": - return `Starting Step ${stepIndex}: ${label}`; + return `Starting Step ${display}: ${label}`; case "done": - return `Step ${stepIndex} finished — ${label}.`; + return `Step ${display} finished — ${label}.`; case "skipped": - return `Step ${stepIndex} was skipped — ${label}.`; + return `Step ${display} was skipped — ${label}.`; case "pending": - return `Step ${stepIndex} was returned to pending — ${label}.`; + return `Step ${display} was returned to pending — ${label}.`; } } diff --git a/packages/engine/src/__tests__/proactive-status.test.ts b/packages/engine/src/__tests__/proactive-status.test.ts index 0959696257..1b2e25825a 100644 --- a/packages/engine/src/__tests__/proactive-status.test.ts +++ b/packages/engine/src/__tests__/proactive-status.test.ts @@ -33,10 +33,12 @@ describe("proactive status narration", () => { }); it("builds complete status messages and safely reports unavailable reviews", () => { - expect(buildStepStartMessage(2, "Ship it")).toBe("Starting Step 2: Ship it"); - expect(buildStepSuccessMessage(2, "Ship it")).toBe("Step 2 finished — Ship it."); - expect(buildStepSkippedMessage(2, "No code change needed")).toBe("Step 2 was skipped — No code change needed."); - expect(buildStepSkippedMessage(2, "Step 2")).toBe("Step 2 was skipped."); + // Narration displays 1-based step numbers (0-based index 2 → "Step 3") to match the task card. + expect(buildStepStartMessage(2, "Ship it")).toBe("Starting Step 3: Ship it"); + expect(buildStepSuccessMessage(2, "Ship it")).toBe("Step 3 finished — Ship it."); + expect(buildStepSkippedMessage(2, "No code change needed")).toBe("Step 3 was skipped — No code change needed."); + // A caller-defaulted name of "Step <0-based index>" is treated as unnamed, not echoed. + expect(buildStepSkippedMessage(2, "Step 2")).toBe("Step 3 was skipped."); expect(buildStepFailureMessage(2, "Ship it", sanitizeFailureReason(undefined))).toContain("No failure reason"); expect(buildPlanVerifiedMessage()).toBe("The plan was written and verified."); expect(buildReviewVerdictMessage("UNAVAILABLE", "nope")).toBeNull(); diff --git a/packages/engine/src/proactive-status.ts b/packages/engine/src/proactive-status.ts index a4a47bb744..44caafd1fd 100644 --- a/packages/engine/src/proactive-status.ts +++ b/packages/engine/src/proactive-status.ts @@ -39,18 +39,33 @@ export function sanitizeFailureReason(rawError: unknown): string { return sanitized; } -function stepLabel(stepIndex: number, stepName?: string): string { - const fallback = `Step ${stepIndex}`; - return stepName?.trim() || fallback; +/* +FNXC:ProactiveChatStatus 2026-07-23-10:30: +Step indices are 0-based everywhere in the agent-facing contract (fn_task_update, PROMPT.md +"### Step 0:" headings, run-audit rows), but the task card counts steps 1-based ("12/13"). +Chat narration is read next to the card, so it must display stepIndex + 1 or the final step of a +13-step task announces itself as "Step 12" while the card shows 13 steps. Only the display number +converts; callers keep passing the 0-based index, and a fallback name of "Step <0-based index>" +(produced by callers that default the name from the index) is treated as "unnamed". +*/ +function stepDisplayNumber(stepIndex: number): number { + return stepIndex + 1; +} + +function stepDisplayName(stepIndex: number, stepName?: string): string | undefined { + const name = stepName?.trim(); + return name && name !== `Step ${stepIndex}` ? name : undefined; } export function buildStepStartMessage(stepIndex: number, stepName?: string): string { - return `Starting Step ${stepIndex}: ${stepLabel(stepIndex, stepName)}`; + const display = stepDisplayNumber(stepIndex); + return `Starting Step ${display}: ${stepDisplayName(stepIndex, stepName) ?? `Step ${display}`}`; } export function buildStepSuccessMessage(stepIndex: number, stepName?: string): string { - const name = stepName?.trim(); - return name && name !== `Step ${stepIndex}` ? `Step ${stepIndex} finished — ${name}.` : `Step ${stepIndex} finished.`; + const name = stepDisplayName(stepIndex, stepName); + const display = stepDisplayNumber(stepIndex); + return name ? `Step ${display} finished — ${name}.` : `Step ${display} finished.`; } /** @@ -59,12 +74,15 @@ export function buildStepSuccessMessage(stepIndex: number, stepName?: string): s * preflight and intentional no-op flows remain visible without fabricating a failure reason. */ export function buildStepSkippedMessage(stepIndex: number, stepName?: string): string { - const name = stepName?.trim(); - return name && name !== `Step ${stepIndex}` ? `Step ${stepIndex} was skipped — ${name}.` : `Step ${stepIndex} was skipped.`; + const name = stepDisplayName(stepIndex, stepName); + const display = stepDisplayNumber(stepIndex); + return name ? `Step ${display} was skipped — ${name}.` : `Step ${display} was skipped.`; } export function buildStepFailureMessage(stepIndex: number, stepName: string | undefined, safeReason: string): string { - const prefix = stepName?.trim() && stepName.trim() !== `Step ${stepIndex}` ? `Step ${stepIndex} (${stepName.trim()})` : `Step ${stepIndex}`; + const name = stepDisplayName(stepIndex, stepName); + const display = stepDisplayNumber(stepIndex); + const prefix = name ? `Step ${display} (${name})` : `Step ${display}`; return `${prefix} did not complete: ${safeReason}`; }