fix(coding-agent): updated late diagnostics to batch messages and drop stale results

- Added per-path edit versioning in `EditTool` to drop stale late diagnostics after later edits. - Added deferred diagnostics queueing through `queueDeferredDiagnostics` and late-diagnostic yield batching. - Updated `UiHelpers` to render late diagnostic file path and summary lines in the chat transcript.
2026-06-08 05:46:43 +02:00
parent f11cbed185
commit 02dc03d03f
8 changed files with 135 additions and 84 deletions
@@ -1,66 +0,0 @@
-import { describe, expect, it } from "bun:test";
-import type { Message, Model } from "@oh-my-pi/pi-ai/types";
-import { buildOpenAiNativeHistory } from "../src/compaction/openai";
-
-const model = {
-	provider: "openai-codex",
-	id: "gpt-5.5",
-	api: "openai-responses",
-	contextWindow: 524288,
-	input: ["text"],
-} as unknown as Model;
-
-function makeCodexHistory(turns: number): Message[] {
-	const messages: Message[] = [];
-	for (let i = 0; i < turns; i++) {
-		messages.push({
-			role: "user",
-			content: [{ type: "text", text: `user turn ${i}` }],
-			timestamp: i,
-		} as unknown as Message);
-		messages.push({
-			role: "assistant",
-			provider: "openai-codex",
-			model: "gpt-5.5",
-			api: "openai-responses",
-			content: [{ type: "text", text: `assistant turn ${i}` }],
-			providerPayload: {
-				type: "openaiResponsesHistory",
-				provider: "openai-codex",
-				dt: true,
-				items: [
-					{ type: "reasoning", id: `rs_${i}`, summary: [] },
-					{
-						type: "message",
-						role: "assistant",
-						id: `msg_${i}`,
-						status: "completed",
-						content: [{ type: "output_text", text: `assistant turn ${i}`, annotations: [] }],
-					},
-					{ type: "function_call", id: `fc_${i}`, call_id: `call_${i}`, name: "read", arguments: "{}" },
-				],
-			},
-			timestamp: i,
-		} as unknown as Message);
-		messages.push({
-			role: "toolResult",
-			toolCallId: `call_${i}`,
-			content: [{ type: "text", text: `result ${i}` }],
-			timestamp: i,
-		} as unknown as Message);
-	}
-	return messages;
-}
-
-describe("buildOpenAiNativeHistory perf", () => {
-	it("times native history build for large codex contexts", () => {
-		for (const turns of [500, 1000, 2000, 4000]) {
-			const messages = makeCodexHistory(turns);
-			const t0 = performance.now();
-			const out = buildOpenAiNativeHistory(messages, model);
-			const dt = performance.now() - t0;
-			console.log(`turns=${turns} items=${out.length} ms=${dt.toFixed(1)}`);
-			expect(out.length).toBeGreaterThan(0);
-		}
-	});
-});
@@ -1,12 +1,15 @@
 # Changelog

 ## [Unreleased]
+
 ### Added

 - Added clickable file path hyperlinks to read tool outputs (read-call rows, grouped summaries, and inline previews) using resolved or absolute file targets with selector-based line anchors for quick navigation

 ### Changed

+- Changed late LSP diagnostics after edit or write to surface in the chat transcript as `Late diagnostics` entries showing file path, summary, and diagnostic lines
+- Changed delayed diagnostics delivery to batch late results in one message per flush instead of a raw hidden custom payload
 - Changed hidden custom messages and file-mention context to reach providers as `developer` messages instead of user-authored turns, so system reminders no longer pollute compacted user history.
 - Rewrote the plan-mode active prompt (`prompts/system/plan-mode-active.md`) from scratch to stop producing shallow plans. Reframed the artifact as an **execution spec** a fresh agent runs after the planning conversation is cleared/compacted (zero design decisions for the implementer) rather than a brevity-capped summary. Folded high-consensus requirements into the existing sections as inline, conditional rules — no new boilerplate sections: ordered Approach steps that keep the build/tests green after each step (sequencing); exact signatures/literals for new or load-bearing symbols (contracts); full callsite list + clean cutover for renames/signature-changes/removals; Verification that must exercise the new behavior (input → observable output) with run preconditions, not just build/typecheck; Assumptions restricted to user-overridable choices plus pre-decided fallbacks for load-bearing assumptions; a provenance rule (plan facts must come from a read this session; unverified claims flagged inline); and bans on conversation back-references and decision-free sections (Non-Goals/Alternatives/Risks/Future Work). Kept the decision-complete self-check and the brevity-vs-completeness tiebreak (completeness wins). Render contract (Handlebars vars/conditionals) unchanged; verified across all `planExists`/`reentry`/`iterative` branch combinations.

@@ -17,6 +20,7 @@

 ### Fixed

+- Fixed stale late diagnostics from older edits being shown after a file was edited again
 - Fixed read output paths so selector suffixes are preserved when corrected paths were returned without selectors
 - Fixed `read` surfacing a misleading red "Operation aborted" on a plain-file or directory read when a turn was interrupted mid-read. Those reads are deterministic and fast, so `execute` now runs them to completion instead of cancelling them; slower/non-deterministic reads (archive, sqlite, document, image, summary, conflict scan, URL) stay cancellable.
 - Fixed edit tool headers to hide first-change line suffixes, middle-elide long paths only when the header width needs it, show compact change stats, and target encoded `file://` hyperlinks.
@@ -14,7 +14,7 @@ import { getDiagnosticsLedger } from "../lsp/diagnostics-ledger";
 import applyPatchDescription from "../prompts/tools/apply-patch.md" with { type: "text" };
 import patchDescription from "../prompts/tools/patch.md" with { type: "text" };
 import replaceDescription from "../prompts/tools/replace.md" with { type: "text" };
-import type { ToolSession } from "../tools";
+import type { DeferredDiagnosticsEntry, ToolSession } from "../tools";
 import { truncateForPrompt } from "../tools/approval";
 import { isInternalUrlPath } from "../tools/path-utils";
 import { type EditMode, normalizeEditMode, resolveEditMode } from "../utils/edit-mode";
@@ -306,6 +306,9 @@ export class EditTool implements AgentTool<TInput> {
 	readonly #editMode?: EditMode;
 	readonly #dedupDiagnostics: boolean;
 	readonly #pendingDeferredFetches = new Map<string, AbortController>();
+	/** Per-path edit counter. A late-diagnostics entry captures the version at
+	 *  fetch time; a newer edit to the same path bumps it, marking the entry stale. */
+	readonly #editVersionByPath = new Map<string, number>();

 	constructor(private readonly session: ToolSession) {
 		const {
@@ -502,10 +505,12 @@ export class EditTool implements AgentTool<TInput> {
 		}

 		const deferredController = new AbortController();
+		const editVersion = (this.#editVersionByPath.get(path) ?? 0) + 1;
+		this.#editVersionByPath.set(path, editVersion);
 		return {
 			onDeferredDiagnostics: (lateDiagnostics: FileDiagnosticsResult) => {
 				this.#pendingDeferredFetches.delete(path);
-				this.#injectLateDiagnostics(path, lateDiagnostics);
+				this.#injectLateDiagnostics(path, lateDiagnostics, editVersion);
 			},
 			signal: deferredController.signal,
 			finalize: (diagnostics: FileDiagnosticsResult | undefined) => {
@@ -518,24 +523,20 @@ export class EditTool implements AgentTool<TInput> {
 		};
 	}

-	#injectLateDiagnostics(path: string, diagnostics: FileDiagnosticsResult): void {
+	#injectLateDiagnostics(path: string, diagnostics: FileDiagnosticsResult, editVersion: number): void {
 		const effective = this.#dedupDiagnostics
 			? getDiagnosticsLedger(this.session).reduce(path, diagnostics)
 			: diagnostics;
 		if (this.#dedupDiagnostics && effective.messages.length === 0) return;

-		const summary = effective.summary ?? "";
-		const lines = effective.messages ?? [];
-		const body = [`Late LSP diagnostics for ${path} (arrived after the edit tool returned):`, summary, ...lines]
-			.filter(Boolean)
-			.join("\n");
-
-		this.session.queueDeferredMessage?.({
-			role: "custom",
-			customType: "lsp-late-diagnostic",
-			content: body,
-			display: false,
-			timestamp: Date.now(),
-		});
+		const entry: DeferredDiagnosticsEntry = {
+			path,
+			summary: effective.summary ?? "",
+			messages: effective.messages ?? [],
+			errored: effective.errored,
+			// Drop at flush time if a later edit to the same file superseded this fetch.
+			isStale: () => this.#editVersionByPath.get(path) !== editVersion,
+		};
+		this.session.queueDeferredDiagnostics?.(entry);
 	}
 }
@@ -25,12 +25,13 @@ import type { CompactionQueuedMessage, InteractiveModeContext } from "../../mode
 import {
 	type CustomMessage,
 	isSilentAbort,
+	LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE,
 	resolveAbortLabel,
 	SKILL_PROMPT_MESSAGE_TYPE,
 	type SkillPromptDetails,
 } from "../../session/messages";
 import type { SessionContext } from "../../session/session-manager";
-import { formatBytes, formatDuration } from "../../tools/render-utils";
+import { formatBytes, formatDuration, replaceTabs, shortenPath } from "../../tools/render-utils";

 type TextBlock = { type: "text"; text: string };
 interface RenderInitialMessagesOptions {
@@ -168,6 +169,34 @@ export class UiHelpers {
 						this.ctx.chatContainer.addChild(block);
 						break;
 					}
+					if (message.customType === LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE) {
+						const details = (
+							message as CustomMessage<{
+								files?: Array<{
+									path?: string;
+									summary?: string;
+									errored?: boolean;
+									messages?: string[];
+								}>;
+							}>
+						).details;
+						const files = details?.files ?? [];
+						const block = new TranscriptBlock();
+						for (const file of files) {
+							const errored = file.errored ?? false;
+							const colorKey = errored ? "error" : "warning";
+							const icon = errored ? theme.status.error : theme.status.warning;
+							const shortPath = file.path ? shortenPath(file.path) : "unknown";
+							const summary = file.summary ? ` ${theme.fg("dim", `(${file.summary})`)}` : "";
+							const header = `${theme.fg(colorKey, `${icon} Late diagnostics`)} ${theme.fg("accent", shortPath)}${summary}`;
+							block.addChild(new Text(header, 1, 0));
+							for (const line of file.messages ?? []) {
+								block.addChild(new Text(theme.fg("muted", `  ${replaceTabs(line)}`), 0, 0));
+							}
+						}
+						this.ctx.chatContainer.addChild(block);
+						break;
+					}
 					if (message.customType === SKILL_PROMPT_MESSAGE_TYPE) {
 						const component = new SkillMessageComponent(message as CustomMessage<SkillPromptDetails>);
 						component.setExpanded(this.ctx.toolOutputExpanded);
@@ -0,0 +1,8 @@
+<system-notice>
+{{#if multiple}}Late LSP diagnostics arrived for {{files.length}} files after their edits returned:
+{{else}}Late LSP diagnostics arrived after the edit returned:
+{{/if}}
+{{#each files}}{{this.path}} — {{this.summary}}
+{{#each this.messages}}{{this}}
+{{/each}}{{#unless @last}}
+{{/unless}}{{/each}}</system-notice>
@@ -91,6 +91,7 @@ import { discoverAndLoadMCPTools, MCPManager, type MCPToolsLoadResult } from "./
 import { resolveMemoryBackend } from "./memory-backend";
 import type { MnemopiSessionState } from "./mnemopi/state";
 import asyncResultTemplate from "./prompts/tools/async-result.md" with { type: "text" };
+import lateDiagnosticTemplate from "./prompts/tools/lsp-late-diagnostic.md" with { type: "text" };
 import { AgentRegistry, MAIN_AGENT_ID } from "./registry/agent-registry";
 import {
 	collectEnvSecrets,
@@ -110,7 +111,12 @@ import {
 	type SnapshotResponse,
 	writeAuthBrokerSnapshotCache,
 } from "./session/auth-storage";
-import { type CustomMessage, convertToLlm, wrapSteeringForModel } from "./session/messages";
+import {
+	type CustomMessage,
+	convertToLlm,
+	LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE,
+	wrapSteeringForModel,
+} from "./session/messages";
 import { getRestorableSessionModels, SessionManager } from "./session/session-manager";
 import { closeAllConnections } from "./ssh/connection-manager";
 import { unmountAll } from "./ssh/sshfs-mount";
@@ -143,6 +149,7 @@ import {
 	BUILTIN_TOOLS,
 	computeEssentialBuiltinNames,
 	createTools,
+	type DeferredDiagnosticsEntry,
 	discoverStartupLspServers,
 	EditTool,
 	EvalTool,
@@ -229,6 +236,42 @@ function buildAsyncResultBatchMessage(entries: AsyncResultEntry[]): CustomMessag
 	};
 }

+type LateDiagnosticsDetails = {
+	files: Array<{ path: string; summary: string; errored: boolean; messages: string[] }>;
+};
+
+function buildLateDiagnosticsBatchMessage(
+	entries: DeferredDiagnosticsEntry[],
+): CustomMessage<LateDiagnosticsDetails> | null {
+	if (entries.length === 0) return null;
+	const files = entries.map(entry => ({
+		path: entry.path,
+		summary: entry.summary,
+		messages: entry.messages,
+		errored: entry.errored,
+	}));
+	const details: LateDiagnosticsDetails = {
+		files: files.map(file => ({
+			path: file.path,
+			summary: file.summary,
+			errored: file.errored,
+			messages: file.messages,
+		})),
+	};
+	return {
+		role: "custom",
+		customType: LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE,
+		content: prompt.render(lateDiagnosticTemplate, {
+			multiple: files.length > 1,
+			files,
+		}),
+		display: true,
+		attribution: "agent",
+		details,
+		timestamp: Date.now(),
+	};
+}
+
 function buildMcpNotificationBatchMessage(entries: McpNotificationEntry[]): AgentMessage | null {
 	const resources: McpNotificationEntry[] = [];
 	const seen = new Set<string>();
@@ -1312,6 +1355,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 			recordEvalSubagentUsage: output => sessionManager.recordEvalSubagentOutput(output),
 			getClientBridge: () => session?.clientBridge,
 			getCompactContext: () => session.formatCompactContext(),
+			queueDeferredDiagnostics: entry => session?.yieldQueue.enqueue(LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE, entry),
 			getTodoPhases: () => session.getTodoPhases(),
 			setTodoPhases: phases => session.setTodoPhases(phases),
 			isMCPDiscoveryEnabled: () => session.isMCPDiscoveryEnabled(),
@@ -2167,6 +2211,10 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 		session.yieldQueue.register<McpNotificationEntry>("mcp-notification", {
 			build: buildMcpNotificationBatchMessage,
 		});
+		session.yieldQueue.register<DeferredDiagnosticsEntry>(LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE, {
+			isStale: entry => entry.isStale(),
+			build: buildLateDiagnosticsBatchMessage,
+		});

 		// Attach the live session to the pre-registered ref so peers can route IRC
 		// messages here. Refresh sessionFile in case it was unavailable at pre-register
@@ -34,6 +34,7 @@ import type { OutputMeta } from "../tools/output-meta";
 import { formatOutputNotice } from "../tools/output-meta";

 export const SKILL_PROMPT_MESSAGE_TYPE = "skill-prompt";
+export const LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE = "lsp-late-diagnostic";

 export interface SkillPromptDetails {
 	name: string;
@@ -117,6 +117,28 @@ export type {
 	DiscoverableToolSource,
 } from "../tool-discovery/tool-index";

+/**
+ * A late LSP diagnostics result that arrived after the edit/write tool already
+ * returned. Surfaced to the model and the transcript via
+ * {@link ToolSession.queueDeferredDiagnostics}, batched through the session
+ * yield queue like background-job results.
+ */
+export interface DeferredDiagnosticsEntry {
+	/** Absolute path the diagnostics belong to (the renderer shortens it). */
+	path: string;
+	/** One-line severity summary, e.g. "2 errors". */
+	summary: string;
+	/** Formatted, ready-to-display diagnostic lines. */
+	messages: string[];
+	/** True when any message is error severity. */
+	errored: boolean;
+	/**
+	 * Evaluated at flush time: drop the entry when a newer edit to the same file
+	 * has superseded it, so the model never sees diagnostics for stale content.
+	 */
+	isStale(): boolean;
+}
+
 /** Session context for tool factories */
 export interface ToolSession {
 	/** Current working directory */
@@ -284,6 +306,10 @@ export interface ToolSession {

 	/** Queue a hidden message to be injected at the next agent turn. */
 	queueDeferredMessage?(message: CustomMessage): void;
+	/** Queue late LSP diagnostics (arrived after an edit/write returned) to be shown
+	 *  in the transcript and delivered to the model at the next yield, like background
+	 *  job results. */
+	queueDeferredDiagnostics?(entry: DeferredDiagnosticsEntry): void;
 	/** Get the active OpenTelemetry config so subagent dispatch can forward
 	 *  the parent's tracer/hooks with the subagent's own identity stamped. */
 	getTelemetry?: () => AgentTelemetryConfig | undefined;