fix(plan-mode): make progress tracking robust and tolerant (#7950)

## What

Hardens the plan-mode example extension's step-progress tracking.
Previously, completion markers were only recognized in the exact
`[DONE:n]` form inside visible text blocks, so steps frequently never
got checked off during execution.

## Changes

- **`getTextContent`** now reads `thinking` blocks as well as text
blocks, so `[DONE:n]` tags emitted during reasoning are honored.
- **Marker tolerance** (`extractDoneSteps`): accepts `[DONE: 1]`, `[DONE
1]`, `Done: n`, and prose phrasings like "Step n done / is complete /
finished" — all adjacency-anchored so negations ("Step 9 is not done")
never match.
- **Whole-plan fallback** (`allStepsComplete`): if the run's final
message says the plan is done (e.g. "All steps complete"), all remaining
steps are marked at `agent_end`.
- **Plan extraction dedup**: only the last `Plan:` section is extracted,
so a plan drafted in a thinking block no longer duplicates the steps
from the text block.
- **No more truncation**: step text is kept in full (previously cut at
50 chars); the widget truncates for display only. The execution prompt
now shows the complete step text.
- **No more dropped steps**: steps whose text starts with a backtick
(e.g. `1. \`index.ts\` — …`) are no longer skipped.
- **Widget refresh** happens on every turn instead of only when a marker
matched.
- **Plan-complete card** is delivered with `nextTurn` instead of
triggering an extra LLM turn.

## Tests

`packages/coding-agent/test/plan-mode-utils.test.ts` updated for the new
behavior (truncation, backtick steps, marker-count semantics) and
extended with coverage for the new cases (marker variants, prose
phrasings, negation safety, thinking-draft dedup, markdown titles,
whole-plan completion).

All 43 tests in `plan-mode-utils.test.ts` +
`plan-mode-extension.test.ts` pass (`npx vitest run`).

Note: the repo-wide type-check currently fails on `main` with
pre-existing `ModelCatalog`/`never` errors unrelated to this change;
this branch adds none (same count with and without the diff).
This commit is contained in:
Giannis Papakostas 2026-11-04 11:53:52 +02:00 • committed by GitHub
parent c13f6acfbe
commit ee962fb4db
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 113 additions and 38 deletions

View file

@ -16,7 +16,7 @@ import type { AgentMessage } from "@earendil-works/pi-agent-core";
import type { AssistantMessage, TextContent } from "@earendil-works/pi-ai";
import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent";
import { Key } from "@earendil-works/pi-tui";
import { extractTodoItems, isSafeCommand, markCompletedSteps, type TodoItem } from "./utils.ts";
import { allStepsComplete, extractTodoItems, isSafeCommand, markCompletedSteps, type TodoItem } from "./utils.ts";
// Tools
const PLAN_MODE_TOOLS = ["read", "bash", "grep", "find", "ls", "questionnaire"];
@ -36,11 +36,16 @@ function isAssistantMessage(m: AgentMessage): m is AssistantMessage {
return m.role === "assistant" && Array.isArray(m.content);
}
// Extract text content from an assistant message
// Extract text content from an assistant message.
// Includes thinking blocks: with reasoning-max models the [DONE:n] tags and
// plan drafts usually land in thinking, not the visible text.
function getTextContent(message: AssistantMessage): string {
return message.content
.filter((block): block is TextContent => block.type === "text")
.map((block) => block.text)
.map((block) => {
if (block.type === "text") return block.text;
if (block.type === "thinking") return (block as { thinking?: string }).thinking ?? "";
return "";
})
.join("\n");
}
@ -70,12 +75,11 @@ export default function planModeExtension(pi: ExtensionAPI): void {
// Widget showing todo list
if (executionMode && todoItems.length > 0) {
const lines = todoItems.map((item) => {
const text = item.text.length > 60 ? `${item.text.slice(0, 57)}...` : item.text;
if (item.completed) {
return (
ctx.ui.theme.fg("success", "☑ ") + ctx.ui.theme.fg("muted", ctx.ui.theme.strikethrough(item.text))
);
return ctx.ui.theme.fg("success", "☑ ") + ctx.ui.theme.fg("muted", ctx.ui.theme.strikethrough(text));
}
return `${ctx.ui.theme.fg("muted", "☐ ")}${item.text}`;
return `${ctx.ui.theme.fg("muted", "☐ ")}${text}`;
});
ctx.ui.setWidget("plan-todos", lines);
} else {
@ -255,10 +259,9 @@ After completing a step, include a [DONE:n] tag in your response.`,
if (!executionMode || todoItems.length === 0) return;
if (!isAssistantMessage(event.message)) return;
const text = getTextContent(event.message);
if (markCompletedSteps(text, todoItems) > 0) {
updateStatus(ctx);
}
markCompletedSteps(getTextContent(event.message), todoItems);
// Always refresh: cheap, keeps the widget consistent even when nothing matched.
updateStatus(ctx);
persistState();
});
@ -266,11 +269,18 @@ After completing a step, include a [DONE:n] tag in your response.`,
pi.on("agent_end", async (event, ctx) => {
// Check if execution is complete
if (executionMode && todoItems.length > 0) {
// Fallback: the model wrapped up without [DONE:n] tags ("All steps complete").
const lastAssistant = [...event.messages].reverse().find(isAssistantMessage);
if (lastAssistant && allStepsComplete(getTextContent(lastAssistant))) {
for (const t of todoItems) t.completed = true;
}
if (todoItems.every((t) => t.completed)) {
const completedList = todoItems.map((t) => `~~${t.text}~~`).join("\n");
pi.sendMessage(
{ customType: "plan-complete", content: `**Plan Complete!** ✓\n\n${completedList}`, display: true },
{ triggerTurn: false },
// nextTurn: delivered with the next user prompt instead of triggering an
// extra (expensive, reasoning-max) LLM turn to acknowledge the card.
{ deliverAs: "nextTurn" },
);
executionMode = false;
todoItems = [];

View file

@ -120,26 +120,30 @@ export function cleanStepText(text: string): string {
if (cleaned.length > 0) {
cleaned = cleaned.charAt(0).toUpperCase() + cleaned.slice(1);
}
if (cleaned.length > 50) {
cleaned = `${cleaned.slice(0, 47)}...`;
}
// Keep full text: the exec message and widget display need the whole step.
// The widget truncates for display (see updateStatus in index.ts).
return cleaned;
}
export function extractTodoItems(message: string): TodoItem[] {
const items: TodoItem[] = [];
const headerMatch = message.match(/\*{0,2}Plan:\*{0,2}\s*\n/i);
if (!headerMatch) return items;
const planSection = message.slice(message.indexOf(headerMatch[0]) + headerMatch[0].length);
const numberedPattern = /^\s*(\d+)[.)]\s+\*{0,2}([^*\n]+)/gm;
// Take the LAST "Plan:" section: since getTextContent includes thinking blocks,
// a draft in the reasoning block can precede the user-visible plan in the text
// block — the text block comes last in content order.
const headers = [...message.matchAll(/\*{0,2}Plan:\*{0,2}\s*\n/gi)];
if (headers.length === 0) return items;
const lastHeader = headers[headers.length - 1];
const planSection = message.slice((lastHeader.index ?? 0) + lastHeader[0].length);
// Full-line capture: step titles may contain bold/italic/backticks ("1. **Step** \`x\` …").
// cleanStepText strips the markdown; / and - items are still skipped below.
const numberedPattern = /^\s*(\d+)[.)]\s+(.+)$/gm;
for (const match of planSection.matchAll(numberedPattern)) {
const text = match[2]
.trim()
.replace(/\*{1,2}$/, "")
.trim();
if (text.length > 5 && !text.startsWith("`") && !text.startsWith("/") && !text.startsWith("-")) {
if (text.length > 5 && !text.startsWith("/") && !text.startsWith("-")) {
const cleaned = cleanStepText(text);
if (cleaned.length > 3) {
items.push({ step: items.length + 1, text: cleaned, completed: false });
@ -151,18 +155,40 @@ export function extractTodoItems(message: string): TodoItem[] {
export function extractDoneSteps(message: string): number[] {
const steps: number[] = [];
for (const match of message.matchAll(/\[DONE:(\d+)\]/gi)) {
const step = Number(match[1]);
if (Number.isFinite(step)) steps.push(step);
// Canonical and bracket variants: [DONE:1], [DONE: 1], [DONE 1], [Done:1]
for (const match of message.matchAll(/\[DONE\s*:?\s*(\d+)\]/gi)) {
steps.push(Number(match[1]));
}
return steps;
// "Done: 1" without brackets
for (const match of message.matchAll(/\bDone\s*:\s*(\d+)\b/gi)) {
steps.push(Number(match[1]));
}
// "Step 1 done", "Step 2 is complete", "Step 3 finished" — adjacency-anchored,
// so "Step 9 is not done" / "Step 9 incomplete" never match.
for (const match of message.matchAll(/\bStep\s*(\d+)\s+(?:is\s+)?(?:done|complete|completed|finished)\b/gi)) {
steps.push(Number(match[1]));
}
return [...new Set(steps)];
}
export function markCompletedSteps(text: string, items: TodoItem[]): number {
const doneSteps = extractDoneSteps(text);
let marked = 0;
for (const step of doneSteps) {
const item = items.find((t) => t.step === step);
if (item) item.completed = true;
const item = items.find((t) => t.step === step && !t.completed);
if (item) {
item.completed = true;
marked++;
}
}
return doneSteps.length;
return marked;
}
// Whole-plan completion phrasing, used as a fallback when the model summarizes
// without [DONE:n] tags. Only consulted on the run's final message (agent_end).
export function allStepsComplete(message: string): boolean {
return (
/(?:all|every)\s+steps?\s+(?:are|is\s+)?(?:done|complete|completed|finished)/i.test(message) ||
/\b(?:the\s+)?plan\s+(?:is\s+)?(?:complete|completed|done)\b/i.test(message)
);
}

View file

@ -1,5 +1,6 @@
import { describe, expect, it } from "vitest";
import {
allStepsComplete,
cleanStepText,
extractDoneSteps,
extractTodoItems,
@ -119,11 +120,10 @@ describe("cleanStepText", () => {
expect(cleanStepText("update config")).toBe("Config");
});
it("truncates long text", () => {
it("keeps full text (truncation moved to the widget display)", () => {
const longText = "This is a very long step description that exceeds the maximum allowed length for display";
const result = cleanStepText(longText);
expect(result.length).toBe(50);
expect(result.endsWith("...")).toBe(true);
expect(result).toBe(longText);
});
it("normalizes whitespace", () => {
@ -183,13 +183,31 @@ Plan:
expect(items[0].text).toContain("proper");
});
it("filters out code-like items", () => {
it("keeps steps that start with backticks (code-file steps)", () => {
const message = `Plan:
1. \`npm install\`
2. Run the build process`;
const items = extractTodoItems(message);
expect(items).toHaveLength(1);
expect(items).toHaveLength(2);
expect(items[0].text).toBe("Npm install");
});
it("extracts only the last Plan: section (thinking draft + text plan)", () => {
const message = `Some reasoning...\nPlan:\n1. Draft one\n2. Draft two\nNow the answer.\nPlan:\n1. Print 1\n2. Print 2\n3. Print 3`;
const items = extractTodoItems(message);
expect(items).toHaveLength(3);
expect(items.map((i) => i.text)).toEqual(["Print 1", "Print 2", "Print 3"]);
});
it("extracts markdown step titles fully", () => {
const message = `Plan:\n1. **Explore** the repo structure\n2. \`index.ts\` — fix getTextContent`;
const items = extractTodoItems(message);
expect(items).toHaveLength(2);
expect(items[0].text).toBe("Explore the repo structure");
expect(items[1].text).toBe("Index.ts — fix getTextContent");
});
});
@ -209,11 +227,25 @@ describe("extractDoneSteps", () => {
expect(extractDoneSteps(message)).toEqual([1, 2, 3]);
});
it("returns empty array with no markers", () => {
const message = "No markers here";
it("extracts DONE marker variants", () => {
const message = "[DONE:1] [DONE: 2] [DONE3] [Done:4] Done: 5";
expect(extractDoneSteps(message)).toEqual([1, 2, 3, 4, 5]);
});
it("extracts prose completion phrasings", () => {
const message = "Step 6 done, Step 7 is complete, Step 8 finished";
expect(extractDoneSteps(message)).toEqual([6, 7, 8]);
});
it("does not match negated or unfinished phrasings", () => {
const message = "Step 9 is not done, Step 10 incomplete";
expect(extractDoneSteps(message)).toEqual([]);
});
it("collapses duplicate markers", () => {
expect(extractDoneSteps("[DONE:1] [DONE: 1]")).toEqual([1]);
});
it("ignores malformed markers", () => {
const message = "[DONE:abc] [DONE:] [DONE:1]";
expect(extractDoneSteps(message)).toEqual([1]);
@ -248,8 +280,15 @@ describe("markCompletedSteps", () => {
const count = markCompletedSteps("[DONE:99]", items);
expect(count).toBe(1); // Still counts the marker found
expect(items[0].completed).toBe(false); // But doesn't mark anything
expect(count).toBe(0); // Counts only newly marked items
expect(items[0].completed).toBe(false);
});
it("marks all remaining steps for whole-plan completion phrasings", () => {
expect(allStepsComplete("All steps complete, wrapping up")).toBe(true);
expect(allStepsComplete("Every step is finished")).toBe(true);
expect(allStepsComplete("The plan is done")).toBe(true);
expect(allStepsComplete("Step 1 complete, moving on")).toBe(false);
});
it("doesn't double-complete already completed items", () => {