diff --git a/docs/diff-statistics-extension.md b/docs/diff-statistics-extension.md new file mode 100644 index 00000000..a493f2b9 --- /dev/null +++ b/docs/diff-statistics-extension.md @@ -0,0 +1,90 @@ +# AIR diff statistics extension + +Status: Experimental + +Agents can attach added and removed line counts to an ACP `diff` content block. +Clients use these values without comparing the block's texts again. +The extension applies to any ACP agent, including Codex. + +## Wire format + +The payload belongs to the individual diff block at `_meta.jetbrains.air.diffStats`. + +```json +{ + "type": "diff", + "path": "/project/file.txt", + "oldText": "old\n", + "newText": "new\nextra\n", + "_meta": { + "kind": "update", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 2, + "removed": 1 + } + } + } + } +} +``` + +`jetbrains.air.version` identifies the AIR envelope. Clients accept integer versions of at least 1. +`diffStats.version` identifies this payload. This specification defines version 1 only. +Agents preserve other metadata, including `kind`. + +| Field | Type | Meaning | +| --- | --- | --- | +| `version` | integer | Must equal `1`. | +| `added` | integer | Number of added lines, between 0 and 2147483647. | +| `removed` | integer | Number of removed lines, between 0 and 2147483647. | + +All three fields are required. Numeric strings are invalid. +Statistics contain no navigation coordinates. Clients must not compare texts to obtain coordinates when they receive valid counts. + +## Count semantics + +For updates, counts describe the addition and deletion operations in the supplied patch. +Context lines and `No newline at end of file` markers do not contribute to counts. +A replacement contributes both added and removed lines. +A patch can contain operations that leave the normalized file content unchanged. +Clients preserve the patch counts instead of recomputing a minimal diff. +Relocating an exact hunk does not change its counts. + +For creation and deletion, count the supplied file content. +Treat CRLF and CR as line boundaries and do not count an extra line after the final terminator. +An empty string has zero lines. One line terminator represents one empty line. +Creation has zero removed lines; deletion has zero added lines. + +Each diff block owns its statistics. +A text revision carries statistics for that revision, or omits the payload. +Clients invalidate old statistics when the texts change. +Status-only updates preserve previous statistics. +Late statistics may replace calculated values for unchanged texts. + +## Availability and compatibility + +This is optional display metadata. No capability negotiation is required. +Clients that do not understand it can ignore it and render the standard diff content. +Agents still send the usual `path`, `oldText`, and `newText` values. + +An agent omits statistics when it cannot produce valid counts. +Clients use their normal comparison when metadata is missing, malformed, or unsupported. +Unknown fields do not invalidate a valid payload. + +The earlier experimental `com.intellij/diffStats` key is not part of this contract. +AIR ignores that key and uses its normal fallback. +Existing persisted statistics, including stored navigation lines, remain readable without migration. + +## Codex behavior + +The existing patch application validates the file change and produces the texts for ACP. +The statistics calculator then reads only the parsed patch. It receives no file texts. +It validates hunk sizes and coordinates and counts `+` and `-` operations. +It does not verify file contents again or locate a navigation line. + +Tests: `src/__tests__/DiffStats.test.ts` and +`src/__tests__/CodexACPAgent/file-change-events.test.ts`. diff --git a/readme-dev.md b/readme-dev.md index bc147807..135fb112 100644 --- a/readme-dev.md +++ b/readme-dev.md @@ -84,3 +84,8 @@ npm run package:all 1. Update the `@openai/codex` version in `package.json` (under `dependencies`). 2. Regenerate Codex types in `src/app-server/`: `npm run generate-types` 3. Ensure there are no type errors or failed tests: `npm run typecheck` and `npm run test` + +### AIR diff statistics + +See the [diff statistics specification](docs/diff-statistics-extension.md) for the +`_meta.jetbrains.air.diffStats` payload and its compatibility rules. diff --git a/src/AirExtension.ts b/src/AirExtension.ts index 5ffe2ee2..28af8784 100644 --- a/src/AirExtension.ts +++ b/src/AirExtension.ts @@ -12,6 +12,7 @@ export const JETBRAINS_META_KEY = "jetbrains"; export const AIR_META_KEY = "air"; export const AIR_EXTENSION_VERSION_KEY = "version"; export const AIR_EXTENSION_CAPABILITIES_KEY = "capabilities"; +export const AIR_DIFF_STATS_KEY = "diffStats"; export const AIR_SESSION_FAILURE_KEY = "sessionFailure"; export const AIR_AGENT_FILE_CHANGE_REPORT_KEY = "agentFileChangeReport"; export const AIR_NATIVE_SUBAGENT_SESSIONS_KEY = "nativeSubagentSessions"; diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index d1b4cbb9..d21bbaf4 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -1,5 +1,7 @@ import type { ContentBlock, ToolCallContent } from "@agentclientprotocol/sdk"; -import { applyPatch, parsePatch, reversePatch } from "diff"; +import { applyPatch, parsePatch, reversePatch, type StructuredPatch } from "diff"; +import { DiffStatsCalculator } from "./DiffStats"; +import { AIR_DIFF_STATS_KEY, withAirMeta } from "./AirExtension"; import { readFile } from "node:fs/promises"; import path from "node:path"; import type { UpdateSessionEvent } from "./ACPSessionConnection"; @@ -47,6 +49,7 @@ type ContextCompactionItem = ThreadItem & { type: "contextCompaction" }; type AcpToolCallEvent = Extract; const CONTEXT_COMPACTION_META = createContextCompactionMeta(); +const DIFF_STATS = new DiffStatsCalculator(); function toAcpStatus(status: CodexItemStatus): AcpToolCallStatus { switch (status) { @@ -838,9 +841,7 @@ async function createAddFileContent(change: FileUpdateChange): Promise= 0; offset = text.indexOf('\n', offset + 1)) count++; + for (let offset = text.indexOf('\r'); offset >= 0; offset = text.indexOf('\r', offset + 1)) { + if (text[offset + 1] !== '\n') count++; + } + if (text.length > 0 && !text.endsWith('\n') && !text.endsWith('\r')) count++; + return count; + } +} diff --git a/src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json b/src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json index 41b58b87..26848a01 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json +++ b/src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json @@ -16,7 +16,17 @@ "newText": "class FileA\n", "path": "/test/project/FileA.kt", "_meta": { - "kind": "add" + "kind": "add", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 1, + "removed": 0 + } + } + } } }, { @@ -25,7 +35,17 @@ "newText": "class FileB\n", "path": "/test/project/FileB.kt", "_meta": { - "kind": "add" + "kind": "add", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 1, + "removed": 0 + } + } + } } } ] diff --git a/src/__tests__/CodexACPAgent/data/file-change-add-new-file.json b/src/__tests__/CodexACPAgent/data/file-change-add-new-file.json index 4bf59ca9..8ede27a2 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-add-new-file.json +++ b/src/__tests__/CodexACPAgent/data/file-change-add-new-file.json @@ -16,7 +16,17 @@ "newText": "package test.project\n\nclass NewFile {\n fun hello() = \"Hello\"\n}\n", "path": "/test/project/NewFile.kt", "_meta": { - "kind": "add" + "kind": "add", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 5, + "removed": 0 + } + } + } } } ] diff --git a/src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json b/src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json index 31f18f5a..2e009c80 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json +++ b/src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json @@ -16,7 +16,17 @@ "newText": "fun main() {\n println(\"Hello, World!\")\n}\n", "path": "/test/project/RawFile.kt", "_meta": { - "kind": "add" + "kind": "add", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 3, + "removed": 0 + } + } + } } } ] diff --git a/src/__tests__/CodexACPAgent/data/file-change-delete-file.json b/src/__tests__/CodexACPAgent/data/file-change-delete-file.json index ea6dd487..63b0cb18 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-delete-file.json +++ b/src/__tests__/CodexACPAgent/data/file-change-delete-file.json @@ -16,7 +16,17 @@ "newText": "", "path": "/test/project/OldFile.kt", "_meta": { - "kind": "delete" + "kind": "delete", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 0, + "removed": 3 + } + } + } } } ] diff --git a/src/__tests__/CodexACPAgent/data/file-change-delete-raw-content.json b/src/__tests__/CodexACPAgent/data/file-change-delete-raw-content.json index 60f83da7..ba03757b 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-delete-raw-content.json +++ b/src/__tests__/CodexACPAgent/data/file-change-delete-raw-content.json @@ -16,7 +16,17 @@ "newText": "", "path": "/test/project/RawDeleteFile.kt", "_meta": { - "kind": "delete" + "kind": "delete", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 0, + "removed": 3 + } + } + } } } ] diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index 69644617..fbdbe046 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -279,7 +279,17 @@ "newText": "Hello\nWorld\n", "path": "/test/project/Added.txt", "_meta": { - "kind": "add" + "kind": "add", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 2, + "removed": 0 + } + } + } } } ] diff --git a/src/__tests__/CodexACPAgent/file-change-events.test.ts b/src/__tests__/CodexACPAgent/file-change-events.test.ts index 795d71e3..426fefd0 100644 --- a/src/__tests__/CodexACPAgent/file-change-events.test.ts +++ b/src/__tests__/CodexACPAgent/file-change-events.test.ts @@ -268,6 +268,31 @@ describe('CodexEventHandler - file change events', () => { ); }); + it.each([ + { name: 'before application', disk: 'old\n', expected: { version: 1, added: 2, removed: 1 } }, + { name: 'after application', disk: 'new\nextra\n', expected: { version: 1, added: 2, removed: 1 } }, + { name: 'a relocated hunk', disk: 'prefix\nold\n', expected: { version: 1, added: 2, removed: 1 } }, + ])('publishes reliable update statistics $name', async ({ disk, expected }) => { + mockFileContent('/test/project/OldFile.kt', disk); + const event = await createFileChangeUpdate({ + type: 'fileChange', + id: 'stats-update', + changes: [{ + path: '/test/project/OldFile.kt', + kind: { type: 'update', move_path: null }, + diff: '@@ -1 +1,2 @@\n-old\n+new\n+extra\n', + }], + status: 'completed', + }); + expect(event.sessionUpdate).toBe('tool_call'); + if (event.sessionUpdate !== 'tool_call') throw new Error('Expected a file-change tool call'); + expect(event.content).toHaveLength(1); + expect(event.content![0]!._meta).toEqual({ + kind: 'update', + ...(expected ? { jetbrains: { air: { version: 1, diffStats: expected } } } : {}), + }); + }); + it('should ignore broken unified diffs in update file changes', async () => { const fileChange: ThreadItem & { type: 'fileChange' } = { type: 'fileChange', diff --git a/src/__tests__/DiffStats.test.ts b/src/__tests__/DiffStats.test.ts new file mode 100644 index 00000000..c553c95e --- /dev/null +++ b/src/__tests__/DiffStats.test.ts @@ -0,0 +1,114 @@ +import { parsePatch } from 'diff'; +import { describe, expect, it } from 'vitest'; +import { DiffStatsCalculator } from '../DiffStats'; + +describe('ACP diff statistics', () => { + const calculator = new DiffStatsCalculator(); + + it.each([ + ['', 0], + ['line', 1], + ['line\n', 1], + ['\n', 1], + ['\n\n', 2], + ['first\n\nlast\n', 3], + ['first\r\n\r\nlast\r\n', 3], + ['first\r\rlast\r', 3], + ['first\r\nsecond\rthird\n', 3], + ])('counts added and deleted lines in %j', (text, count) => { + expect(calculator.addedFile(text)).toEqual({ + version: 1, added: count, removed: 0, + }); + expect(calculator.deletedFile(text)).toEqual({ + version: 1, added: 0, removed: count, + }); + }); + + it.each([ + { + name: 'replacement with blank context', + patch: '@@ -1,3 +1,4 @@\n first\n \n-old\n+new\n+extra\n', + added: 2, removed: 1, + }, + { + name: 'deletion at EOF', + patch: '@@ -2 +1,0 @@\n-last\n', + added: 0, removed: 1, + }, + { + name: 'deletion of the entire file', + patch: '@@ -1,2 +0,0 @@\n-first\n-last\n', + added: 0, removed: 2, + }, + { + name: 'insertion after EOF', + patch: '@@ -1,0 +2,2 @@\n+second\n+third\n', + added: 2, removed: 0, + }, + { + name: 'multiple hunks with shifted coordinates', + patch: '@@ -2 +2,2 @@\n-two\n+TWO\n+inserted\n@@ -5 +6 @@\n-five\n+FIVE\n', + added: 3, removed: 2, + }, + { + name: 'missing EOF newline markers do not count as lines', + patch: '@@ -1 +1 @@\n-old\n\\ No newline at end of file\n+new\n\\ No newline at end of file\n', + added: 1, removed: 1, + }, + { + name: 'counts the patch operations for an EOF newline change', + patch: '@@ -1 +1 @@\n-same\n\\ No newline at end of file\n+same\n', + added: 1, removed: 1, + }, + { + name: 'a blank line differs from an empty file', + patch: '@@ -0,0 +1 @@\n+\n', + added: 1, removed: 0, + }, + { + name: 'CRLF patch lines', + patch: '@@ -1,2 +1,2 @@\r\n first\r\n-old\r\n+new\r\n', + added: 1, removed: 1, + }, + { + name: 'the patch counts are retained even when a minimal diff is smaller', + patch: '@@ -1,2 +1,2 @@\n-same\n-old\n+same\n+new\n', + added: 2, removed: 2, + }, + ])('$name', ({ patch, added, removed }) => { + expect(calculator.update(parsePatch(patch)[0]!)).toEqual({ version: 1, added, removed }); + }); + + it.each([ + { oldStart: -1 }, + { newStart: NaN }, + { newStart: 1.5 }, + { oldLines: 2 }, + { newLines: 0 }, + { lines: ['-old', '+new', '?garbage'] }, + { lines: ['\\ No newline at end of file', '-old', '+new'] }, + { lines: ['-old', '\\ invalid marker', '+new'] }, + ])('omits malformed hunk statistics: %j', (change) => { + const patch = parsePatch('@@ -1 +1 @@\n-old\n+new\n')[0]!; + patch.hunks[0] = { ...patch.hunks[0]!, ...change }; + expect(calculator.update(patch)).toBeNull(); + }); + + it('counts a patch at large coordinates without requiring file texts', () => { + const patch = parsePatch('@@ -1000000 +1000000 @@\n-old\n+new\n')[0]!; + expect(calculator.update(patch)).toEqual({ version: 1, added: 1, removed: 1 }); + }); + + it('omits overlapping hunks', () => { + const patch = parsePatch('@@ -1 +1 @@\n-old\n+new\n')[0]!; + patch.hunks.push({ ...patch.hunks[0]! }); + expect(calculator.update(patch)).toBeNull(); + }); + + it('omits binary patches and missing hunks', () => { + const patch = parsePatch('@@ -1 +1 @@\n-old\n+new\n')[0]!; + expect(calculator.update({ ...patch, hunks: [] })).toBeNull(); + patch.isBinary = true; + expect(calculator.update(patch)).toBeNull(); + }); +});