diff --git a/docs/diff-statistics-extension.md b/docs/diff-statistics-extension.md new file mode 100644 index 00000000..6b12e849 --- /dev/null +++ b/docs/diff-statistics-extension.md @@ -0,0 +1,30 @@ +# AIR diff statistics + +The adapter supplies counts in each ACP diff block through `_meta.jetbrains.air.diffStats`. +This uses the same contract as the Codex adapter. No capability negotiation is required. + +```json +{ + "_meta": { + "jetbrains": { + "air": { + "version": 1, + "diffStats": { "version": 1, "added": 20, "removed": 30 } + } + } + } +} +``` + +Both counts are nonnegative integers. They describe the operations in that block's patch. +Context lines and EOF markers do not increase the counts. +The extension carries no navigation position. + +For `Edit` and `Write`, the adapter counts operations while it builds diff texts from the SDK's `structuredPatch`. +Each hunk produces one block with its own counts. The adapter does not compare or scan the full file texts for statistics. + +When a patch has unsafe, unordered, or mutually inconsistent coordinates, inconsistent line counts, +misplaced EOF markers, or unsupported prefixes, the adapter omits the statistics. +Empty patches, initial tool inputs, and history without structured patches retain their existing behavior. +They provide no statistics because the adapter has no patch operations to count. +AIR uses its existing comparison when statistics are absent. diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 992ed36d..e7eb064d 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -232,10 +232,10 @@ import { TaskState, taskStateToPlanEntries, toolInfoFromToolUse, - toolUpdateFromDiffToolResponse, toolUpdateFromToolResult, unregisterHookCallback, } from "./tools.js"; +import { toolUpdateFromDiffToolResponse } from "./diff.js"; import { nodeToWebReadable, nodeToWebWritable, Pushable, unreachable } from "./utils.js"; import { acceptedPlanToolResult, diff --git a/src/air-extension.ts b/src/air-extension.ts index 347b835f..7309f8db 100644 --- a/src/air-extension.ts +++ b/src/air-extension.ts @@ -2,6 +2,7 @@ export const AIR_NATIVE_SUBAGENT_SESSIONS_CAPABILITY = "nativeSubagentSessions"; export const AIR_ASYNC_TASKS_CAPABILITY = "asyncTasks"; export const AIR_SESSION_FAILURE_CAPABILITY = "sessionFailure"; export const AIR_RECOMMENDED_CONFIG_VALUE_CAPABILITY = "recommendedValue"; +export const AIR_DIFF_STATS_KEY = "diffStats"; const JETBRAINS_META_KEY = "jetbrains"; const AIR_META_KEY = "air"; diff --git a/src/diff.ts b/src/diff.ts new file mode 100644 index 00000000..03f010d1 --- /dev/null +++ b/src/diff.ts @@ -0,0 +1,158 @@ +import { ToolCallContent, ToolCallLocation } from "@agentclientprotocol/sdk"; +import { AIR_DIFF_STATS_KEY, withAirMeta } from "./air-extension.js"; + +interface DiffToolResponseHunk { + oldStart: number; + oldLines: number; + newStart: number; + newLines: number; + lines: string[]; +} + +interface DiffToolResponse { + filePath?: string; + structuredPatch?: DiffToolResponseHunk[]; + /** FileWriteOutput only (FileEditOutput carries no `type`): whether the + * write created the file or overwrote an existing one. */ + type?: "create" | "update"; + /** FileWriteOutput only: the content that was written. */ + content?: string; + /** FileWriteOutput only: the pre-write content — null on create, or on an + * update whose previous content was too large to include. */ + originalFile?: string | null; +} + +/** + * Builds diff ToolUpdate content from the structured toolResponse provided by + * the PostToolUse hook for diff-producing tools (Edit, Write). Unlike parsing + * the plain unified diff string, this uses the pre-parsed structuredPatch + * which supports multiple replacement sites (replaceAll) and always includes + * context lines for better readability. + */ +export function toolUpdateFromDiffToolResponse(toolResponse: unknown): { + content?: ToolCallContent[]; + locations?: ToolCallLocation[]; +} { + if (!toolResponse || typeof toolResponse !== "object") return {}; + const response = toolResponse as DiffToolResponse; + if (!response.filePath || !Array.isArray(response.structuredPatch)) return {}; + + const content: ToolCallContent[] = []; + const locations: ToolCallLocation[] = []; + let previousOldEnd = 0; + let previousNewEnd = 0; + let accumulatedDelta = 0; + let coordinatesRemainConsistent = true; + + for (const { lines, oldStart, newStart, oldLines, newLines } of response.structuredPatch) { + const oldText: string[] = []; + const newText: string[] = []; + let added = 0; + let removed = 0; + let validPrefixes = true; + let validEofMarkers = true; + const oldPosition = oldStart + (oldLines === 0 ? 1 : 0); + const newPosition = newStart + (newLines === 0 ? 1 : 0); + const oldEnd = oldPosition + oldLines; + const newEnd = newPosition + newLines; + const nextDelta = accumulatedDelta + newLines - oldLines; + const validCoordinates: boolean = + coordinatesRemainConsistent && + Number.isSafeInteger(oldStart) && + Number.isSafeInteger(newStart) && + Number.isSafeInteger(oldLines) && + Number.isSafeInteger(newLines) && + oldStart >= (oldLines === 0 ? 0 : 1) && + newStart >= (newLines === 0 ? 0 : 1) && + oldLines >= 0 && + newLines >= 0 && + Number.isSafeInteger(oldPosition) && + Number.isSafeInteger(newPosition) && + Number.isSafeInteger(oldEnd) && + Number.isSafeInteger(newEnd) && + Number.isSafeInteger(nextDelta) && + oldPosition >= previousOldEnd && + newPosition >= previousNewEnd && + newPosition - oldPosition === accumulatedDelta; + coordinatesRemainConsistent = validCoordinates; + if (validCoordinates) { + previousOldEnd = oldEnd; + previousNewEnd = newEnd; + accumulatedDelta = nextDelta; + } + for (const [index, line] of lines.entries()) { + if (line.startsWith("-")) { + oldText.push(line.slice(1)); + removed++; + } else if (line.startsWith("+")) { + newText.push(line.slice(1)); + added++; + } else if (line === "\\ No newline at end of file") { + const previousLine = lines[index - 1]; + if ( + typeof previousLine !== "string" || + (!previousLine.startsWith("-") && + !previousLine.startsWith("+") && + !previousLine.startsWith(" ")) + ) { + validEofMarkers = false; + } + continue; + } else { + if (!line.startsWith(" ")) validPrefixes = false; + oldText.push(line.slice(1)); + newText.push(line.slice(1)); + } + } + if (oldText.length > 0 || newText.length > 0) { + locations.push({ path: response.filePath, line: newStart }); + content.push({ + type: "diff", + path: response.filePath, + oldText: oldText.join("\n") || null, + newText: newText.join("\n"), + ...(validCoordinates && + validPrefixes && + validEofMarkers && + oldText.length === oldLines && + newText.length === newLines + ? { _meta: withAirMeta(undefined, AIR_DIFF_STATS_KEY, { version: 1, added, removed }) } + : {}), + }); + } + } + + // A Write `update` can arrive with an empty structuredPatch — nothing + // changed, the diff timed out, or the previous content was too large to + // diff (originalFile null; SDK 0.3.252 documents the lane). Returning `{}` + // would leave Write's optimistic tool_use-time content standing, and that + // was built with `oldText: null` — "creation" semantics — so an overwrite + // of a large existing file would render as creating it. Emit a truthful + // replacement instead. Gated on `type` so Edit (whose output carries no + // `type` and whose optimistic old/new diff is already truthful) keeps the + // empty-return behavior. + if (content.length === 0 && response.type === "update" && typeof response.content === "string") { + locations.push({ path: response.filePath }); + content.push( + typeof response.originalFile === "string" + ? { + type: "diff", + path: response.filePath, + oldText: response.originalFile, + newText: response.content, + } + : { + type: "content", + content: { + type: "text", + text: `Updated \`${response.filePath}\` (previous content too large to diff)`, + }, + }, + ); + } + + const result: { content?: ToolCallContent[]; locations?: ToolCallLocation[] } = {}; + if (content.length > 0) result.content = content; + if (locations.length > 0) result.locations = locations; + return result; +} diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index dbb2cfa3..0b661479 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -27,8 +27,8 @@ import { toolInfoFromToolUse, toDisplayPath, toolUpdateFromToolResult, - toolUpdateFromDiffToolResponse, } from "../tools.js"; +import { toolUpdateFromDiffToolResponse } from "../diff.js"; import { toAcpNotifications, promptToClaude, @@ -1676,6 +1676,9 @@ describe("toolUpdateFromDiffToolResponse", () => { path: "/Users/test/project/test.txt", oldText: "context before\nold line\ncontext after", newText: "context before\nnew line\ncontext after", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, }, ], locations: [{ path: "/Users/test/project/test.txt", line: 1 }], @@ -1710,12 +1713,18 @@ describe("toolUpdateFromDiffToolResponse", () => { path: "/Users/test/project/file.ts", oldText: "oldValue", newText: "newValue", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, }, { type: "diff", path: "/Users/test/project/file.ts", oldText: "oldValue", newText: "newValue", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, }, ], locations: [ @@ -1746,6 +1755,9 @@ describe("toolUpdateFromDiffToolResponse", () => { path: "/Users/test/project/file.ts", oldText: "context\nremoved line", newText: "context", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 0, removed: 1 } } }, + }, }, ], locations: [{ path: "/Users/test/project/file.ts", line: 10 }], diff --git a/src/tests/diff-statistics.test.ts b/src/tests/diff-statistics.test.ts new file mode 100644 index 00000000..1e3738e1 --- /dev/null +++ b/src/tests/diff-statistics.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from "vitest"; +import { toolUpdateFromDiffToolResponse } from "../diff.js"; + +describe("diff statistics", () => { + it("counts each block independently without reading full file content", () => { + const result = toolUpdateFromDiffToolResponse({ + filePath: "/file.ts", + get originalFile() { + throw new Error("The patch already contains the changes"); + }, + get content() { + throw new Error("The patch already contains the changes"); + }, + structuredPatch: [ + { + oldStart: 1, + oldLines: 3, + newStart: 1, + newLines: 2, + lines: [" context", "-first", "-second", "+replacement"], + }, + { + oldStart: 1_000_000, + oldLines: 0, + newStart: 1_000_000, + newLines: 2, + lines: ["+first", "+second"], + }, + ], + }); + + expect(result.content?.map((block) => block._meta)).toEqual([ + { jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 2 } } } }, + { jetbrains: { air: { version: 1, diffStats: { version: 1, added: 2, removed: 0 } } } }, + ]); + }); + + it("counts patch operations when an EOF change produces equal displayed texts", () => { + const result = toolUpdateFromDiffToolResponse({ + filePath: "/file.ts", + structuredPatch: [ + { + oldStart: 1, + oldLines: 1, + newStart: 1, + newLines: 1, + lines: ["-same", "\\ No newline at end of file", "+same"], + }, + ], + }); + + expect(result.content).toEqual([ + { + type: "diff", + path: "/file.ts", + oldText: "same", + newText: "same", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, + }, + ]); + }); + + it("publishes zero counts for a block with only context", () => { + const result = toolUpdateFromDiffToolResponse({ + filePath: "/file.ts", + structuredPatch: [ + { oldStart: 1, oldLines: 1, newStart: 1, newLines: 1, lines: [" unchanged"] }, + ], + }); + + expect(result.content?.[0]._meta).toEqual({ + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 0, removed: 0 } } }, + }); + }); + + it.each([ + { oldLines: 2, newLines: 1, lines: ["-old", "+new"] }, + { oldLines: 1, newLines: 2, lines: ["-old", "+new"] }, + { oldLines: 1, newLines: 1, lines: ["?unsupported"] }, + { oldLines: "1", newLines: 1, lines: ["-old", "+new"] }, + { oldStart: -1, oldLines: 1, newLines: 1, lines: ["-old", "+new"] }, + { newStart: Number.NaN, oldLines: 1, newLines: 1, lines: ["-old", "+new"] }, + { + oldLines: 1, + newLines: 1, + lines: ["\\ No newline at end of file", "-old", "+new"], + }, + ])("omits statistics for an inconsistent patch: %j", (hunk) => { + const result = toolUpdateFromDiffToolResponse({ + filePath: "/file.ts", + structuredPatch: [{ oldStart: 1, newStart: 1, ...hunk }], + }); + + expect(result.content).toHaveLength(1); + expect(result.content?.[0]).not.toHaveProperty("_meta"); + }); + + it("omits statistics once hunk coordinates stop increasing", () => { + const result = toolUpdateFromDiffToolResponse({ + filePath: "/file.ts", + structuredPatch: [ + { oldStart: 10, oldLines: 1, newStart: 10, newLines: 1, lines: ["-old", "+new"] }, + { oldStart: 5, oldLines: 1, newStart: 5, newLines: 1, lines: ["-old", "+new"] }, + ], + }); + + expect(result.content?.[0]).toHaveProperty("_meta"); + expect(result.content?.[1]).not.toHaveProperty("_meta"); + }); + + it("omits statistics when later hunk coordinates ignore the accumulated line delta", () => { + const result = toolUpdateFromDiffToolResponse({ + filePath: "/file.ts", + structuredPatch: [ + { + oldStart: 10, + oldLines: 1, + newStart: 10, + newLines: 2, + lines: ["-old", "+new", "+extra"], + }, + { oldStart: 20, oldLines: 1, newStart: 20, newLines: 1, lines: ["-old", "+new"] }, + ], + }); + + expect(result.content?.[0]).toHaveProperty("_meta"); + expect(result.content?.[1]).not.toHaveProperty("_meta"); + }); +}); diff --git a/src/tests/tools.test.ts b/src/tests/tools.test.ts index acca35b1..24ce4c41 100644 --- a/src/tests/tools.test.ts +++ b/src/tests/tools.test.ts @@ -1191,6 +1191,9 @@ describe("Bash terminal output", () => { path: "/Users/test/project/file.ts", oldText: "context before\nold text\ncontext after", newText: "context before\nnew text\ncontext after", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, }, ]); expect(hookUpdate.locations).toEqual([{ path: "/Users/test/project/file.ts", line: 5 }]); @@ -1272,8 +1275,24 @@ describe("Bash terminal output", () => { expect(hookUpdates).toHaveLength(1); const hookUpdate = hookUpdates[0].update; expect(hookUpdate.content).toEqual([ - { type: "diff", path: "/Users/test/project/file.ts", oldText: "foo", newText: "bar" }, - { type: "diff", path: "/Users/test/project/file.ts", oldText: "foo", newText: "bar" }, + { + type: "diff", + path: "/Users/test/project/file.ts", + oldText: "foo", + newText: "bar", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, + }, + { + type: "diff", + path: "/Users/test/project/file.ts", + oldText: "foo", + newText: "bar", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, + }, ]); expect(hookUpdate.locations).toEqual([ { path: "/Users/test/project/file.ts", line: 3 }, @@ -1437,6 +1456,9 @@ describe("Bash terminal output", () => { path: "/Users/test/project/file.ts", oldText: "line1\nold line2\nline3", newText: "line1\nNEW line2\nline3", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 1, removed: 1 } } }, + }, }, ]); expect(hookUpdate.locations).toEqual([{ path: "/Users/test/project/file.ts", line: 1 }]); @@ -1511,6 +1533,9 @@ describe("Bash terminal output", () => { path: "/Users/test/project/new.ts", oldText: null, newText: "first\nsecond", + _meta: { + jetbrains: { air: { version: 1, diffStats: { version: 1, added: 2, removed: 0 } } }, + }, }, ]); }); diff --git a/src/tools.ts b/src/tools.ts index 1424731f..cf55bbff 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -1390,104 +1390,6 @@ export function markdownEscape(text: string): string { return escape + "\n" + text + (text.endsWith("\n") ? "" : "\n") + escape; } -interface DiffToolResponseHunk { - oldStart: number; - oldLines: number; - newStart: number; - newLines: number; - lines: string[]; -} - -interface DiffToolResponse { - filePath?: string; - structuredPatch?: DiffToolResponseHunk[]; - /** FileWriteOutput only (FileEditOutput carries no `type`): whether the - * write created the file or overwrote an existing one. */ - type?: "create" | "update"; - /** FileWriteOutput only: the content that was written. */ - content?: string; - /** FileWriteOutput only: the pre-write content — null on create, or on an - * update whose previous content was too large to include. */ - originalFile?: string | null; -} - -/** - * Builds diff ToolUpdate content from the structured toolResponse provided by - * the PostToolUse hook for diff-producing tools (Edit, Write). Unlike parsing - * the plain unified diff string, this uses the pre-parsed structuredPatch - * which supports multiple replacement sites (replaceAll) and always includes - * context lines for better readability. - */ -export function toolUpdateFromDiffToolResponse(toolResponse: unknown): { - content?: ToolCallContent[]; - locations?: ToolCallLocation[]; -} { - if (!toolResponse || typeof toolResponse !== "object") return {}; - const response = toolResponse as DiffToolResponse; - if (!response.filePath || !Array.isArray(response.structuredPatch)) return {}; - - const content: ToolCallContent[] = []; - const locations: ToolCallLocation[] = []; - - for (const { lines, newStart } of response.structuredPatch) { - const oldText: string[] = []; - const newText: string[] = []; - for (const line of lines) { - if (line.startsWith("-")) { - oldText.push(line.slice(1)); - } else if (line.startsWith("+")) { - newText.push(line.slice(1)); - } else { - oldText.push(line.slice(1)); - newText.push(line.slice(1)); - } - } - if (oldText.length > 0 || newText.length > 0) { - locations.push({ path: response.filePath, line: newStart }); - content.push({ - type: "diff", - path: response.filePath, - oldText: oldText.join("\n") || null, - newText: newText.join("\n"), - }); - } - } - - // A Write `update` can arrive with an empty structuredPatch — nothing - // changed, the diff timed out, or the previous content was too large to - // diff (originalFile null; SDK 0.3.252 documents the lane). Returning `{}` - // would leave Write's optimistic tool_use-time content standing, and that - // was built with `oldText: null` — "creation" semantics — so an overwrite - // of a large existing file would render as creating it. Emit a truthful - // replacement instead. Gated on `type` so Edit (whose output carries no - // `type` and whose optimistic old/new diff is already truthful) keeps the - // empty-return behavior. - if (content.length === 0 && response.type === "update" && typeof response.content === "string") { - locations.push({ path: response.filePath }); - content.push( - typeof response.originalFile === "string" - ? { - type: "diff", - path: response.filePath, - oldText: response.originalFile, - newText: response.content, - } - : { - type: "content", - content: { - type: "text", - text: `Updated \`${response.filePath}\` (previous content too large to diff)`, - }, - }, - ); - } - - const result: { content?: ToolCallContent[]; locations?: ToolCallLocation[] } = {}; - if (content.length > 0) result.content = content; - if (locations.length > 0) result.locations = locations; - return result; -} - /* Callbacks are keyed globally because the SDK hook is process-wide, but each * entry retains its owning ACP session so cancellation/teardown can release it. */ const toolUseCallbacks = new Map<