From 0e83e9244145c0a0bc2e65c3b962d967780c3486 Mon Sep 17 00:00:00 2001 From: Nikita Ashikhmin Date: Fri, 11 Sep 2026 15:10:02 +0400 Subject: [PATCH 1/3] perf: supply validated diff statistics to ACP clients Clients compared full file texts again to display line counts. Supply counts and the first changed line through com.intellij/diffStats metadata. Parse each update patch once and reuse it for application and statistics. Validate hunk coordinates and content before publishing the counts. Omit statistics when validation fails so clients can compare the texts. Type checking, the build, and all 610 active tests pass; 26 tests are skipped. The new coverage includes 34 statistics and file event cases. A real Codex history imported through ACP provides statistics for all 277 diffs. --- src/CodexToolCallMapper.ts | 40 +++-- src/DiffStats.ts | 149 ++++++++++++++++++ .../data/file-change-add-multiple-files.json | 16 +- .../data/file-change-add-new-file.json | 8 +- .../data/file-change-add-raw-content.json | 8 +- .../data/file-change-delete-file.json | 8 +- .../data/file-change-delete-raw-content.json | 8 +- .../data/load-session-history.json | 8 +- .../CodexACPAgent/file-change-events.test.ts | 25 +++ src/__tests__/DiffStats.test.ts | 130 +++++++++++++++ 10 files changed, 371 insertions(+), 29 deletions(-) create mode 100644 src/DiffStats.ts create mode 100644 src/__tests__/DiffStats.test.ts diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index d1b4cbb9..fe572b09 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -1,5 +1,6 @@ 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 { readFile } from "node:fs/promises"; import path from "node:path"; import type { UpdateSessionEvent } from "./ACPSessionConnection"; @@ -47,6 +48,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) { @@ -840,6 +842,7 @@ async function createAddFileContent(change: FileUpdateChange): Promise oldLineCount + 1 || newStart + newLines > newLineCount + 1 || + newStart - oldStart !== added - removed) return null; + let oldConsumed = 0; + let newConsumed = 0; + let previousWasContent = false; + for (const line of hunk.lines) { + switch (line[0]) { + case '+': + firstChangedLine ??= newStart + newConsumed; + added++; + newConsumed++; + previousWasContent = true; + break; + case '-': + firstChangedLine ??= newStart + newConsumed; + removed++; + oldConsumed++; + previousWasContent = true; + break; + case ' ': + case undefined: + oldConsumed++; + newConsumed++; + previousWasContent = true; + break; + case '\\': + if (!previousWasContent || line.replace(/\r$/, '') !== '\\ No newline at end of file') return null; + previousWasContent = false; + break; + default: + return null; + } + } + if (oldConsumed !== oldLines || newConsumed !== newLines) return null; + previousOldEnd = oldStart + oldLines; + previousNewEnd = newStart + newLines; + } + if (oldLineCount + added - removed !== newLineCount || + !this.matchesHunks(patch, oldText, false) || !this.matchesHunks(patch, newText, true)) return null; + if (this.sameLines(oldText, newText)) return { version: 1, added: 0, removed: 0, firstChangedLine: null }; + if (firstChangedLine === null) return null; + return { + version: 1, + added, + removed, + firstChangedLine: Math.max(1, Math.min(firstChangedLine, newLineCount)), + }; + } + + private matchesHunks(patch: StructuredPatch, text: string, newSide: boolean): boolean { + let offset = 0; + let lineNumber = 1; + for (const hunk of patch.hunks) { + const start = newSide ? hunk.newStart : hunk.oldStart; + while (lineNumber < start) { + offset = this.nextLine(text, offset); + lineNumber++; + } + for (const line of hunk.lines) { + if (line[0] === '\\' || line[0] === (newSide ? '-' : '+')) continue; + let end = offset; + while (end < text.length && text[end] !== '\n' && text[end] !== '\r') end++; + const expected = line.slice(1).replace(/\r$/, ''); + if (text.slice(offset, end) !== expected) return false; + offset = this.nextLine(text, offset); + lineNumber++; + } + } + return true; + } + + private nextLine(text: string, offset: number): number { + while (offset < text.length && text[offset] !== '\n' && text[offset] !== '\r') offset++; + if (offset === text.length) return offset; + return offset + (text[offset] === '\r' && text[offset + 1] === '\n' ? 2 : 1); + } + + private lineCount(text: string): number { + let count = 0; + let offset = 0; + while (offset < text.length) { + offset = this.nextLine(text, offset); + count++; + } + return count; + } + + private sameLines(left: string, right: string): boolean { + if (left === right) return true; + if (left.length === 0 || right.length === 0) return false; + const leftEnd = this.contentEnd(left); + const rightEnd = this.contentEnd(right); + let leftOffset = 0; + let rightOffset = 0; + while (leftOffset < leftEnd && rightOffset < rightEnd) { + let leftChar = left[leftOffset++]; + let rightChar = right[rightOffset++]; + if (leftChar === '\r') { + if (left[leftOffset] === '\n') leftOffset++; + leftChar = '\n'; + } + if (rightChar === '\r') { + if (right[rightOffset] === '\n') rightOffset++; + rightChar = '\n'; + } + if (leftChar !== rightChar) return false; + } + return leftOffset === leftEnd && rightOffset === rightEnd; + } + + private contentEnd(text: string): number { + if (text.endsWith('\r\n')) return text.length - 2; + if (text.endsWith('\n') || text.endsWith('\r')) return text.length - 1; + return text.length; + } +} 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..802cbbce 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,13 @@ "newText": "class FileA\n", "path": "/test/project/FileA.kt", "_meta": { - "kind": "add" + "kind": "add", + "com.intellij/diffStats": { + "version": 1, + "added": 1, + "removed": 0, + "firstChangedLine": 1 + } } }, { @@ -25,7 +31,13 @@ "newText": "class FileB\n", "path": "/test/project/FileB.kt", "_meta": { - "kind": "add" + "kind": "add", + "com.intellij/diffStats": { + "version": 1, + "added": 1, + "removed": 0, + "firstChangedLine": 1 + } } } ] 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..cf149add 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,13 @@ "newText": "package test.project\n\nclass NewFile {\n fun hello() = \"Hello\"\n}\n", "path": "/test/project/NewFile.kt", "_meta": { - "kind": "add" + "kind": "add", + "com.intellij/diffStats": { + "version": 1, + "added": 5, + "removed": 0, + "firstChangedLine": 1 + } } } ] 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..0f6dec66 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,13 @@ "newText": "fun main() {\n println(\"Hello, World!\")\n}\n", "path": "/test/project/RawFile.kt", "_meta": { - "kind": "add" + "kind": "add", + "com.intellij/diffStats": { + "version": 1, + "added": 3, + "removed": 0, + "firstChangedLine": 1 + } } } ] diff --git a/src/__tests__/CodexACPAgent/data/file-change-delete-file.json b/src/__tests__/CodexACPAgent/data/file-change-delete-file.json index ea6dd487..3f173780 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-delete-file.json +++ b/src/__tests__/CodexACPAgent/data/file-change-delete-file.json @@ -16,7 +16,13 @@ "newText": "", "path": "/test/project/OldFile.kt", "_meta": { - "kind": "delete" + "kind": "delete", + "com.intellij/diffStats": { + "version": 1, + "added": 0, + "removed": 3, + "firstChangedLine": 1 + } } } ] 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..1ef7dd3d 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,13 @@ "newText": "", "path": "/test/project/RawDeleteFile.kt", "_meta": { - "kind": "delete" + "kind": "delete", + "com.intellij/diffStats": { + "version": 1, + "added": 0, + "removed": 3, + "firstChangedLine": 1 + } } } ] diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index 69644617..5d8bdfa3 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -279,7 +279,13 @@ "newText": "Hello\nWorld\n", "path": "/test/project/Added.txt", "_meta": { - "kind": "add" + "kind": "add", + "com.intellij/diffStats": { + "version": 1, + "added": 2, + "removed": 0, + "firstChangedLine": 1 + } } } ] diff --git a/src/__tests__/CodexACPAgent/file-change-events.test.ts b/src/__tests__/CodexACPAgent/file-change-events.test.ts index 795d71e3..9c033baa 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, firstChangedLine: 1 } }, + { name: 'after application', disk: 'new\nextra\n', expected: { version: 1, added: 2, removed: 1, firstChangedLine: 1 } }, + { name: 'a relocated hunk', disk: 'prefix\nold\n', expected: undefined }, + ])('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 ? { 'com.intellij/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..0301fabd --- /dev/null +++ b/src/__tests__/DiffStats.test.ts @@ -0,0 +1,130 @@ +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, firstChangedLine: count === 0 ? null : 1, + }); + expect(calculator.deletedFile(text)).toEqual({ + version: 1, added: 0, removed: count, firstChangedLine: count === 0 ? null : 1, + }); + }); + + it.each([ + { + name: 'replacement with blank context', + old: 'first\n\nold\n', next: 'first\n\nnew\nextra\n', + patch: '@@ -1,3 +1,4 @@\n first\n \n-old\n+new\n+extra\n', + added: 2, removed: 1, firstChangedLine: 3, + }, + { + name: 'deletion at EOF clamps to the last remaining line', + old: 'first\nlast\n', next: 'first\n', + patch: '@@ -2 +1,0 @@\n-last\n', + added: 0, removed: 1, firstChangedLine: 1, + }, + { + name: 'deletion of the entire file clamps to line one', + old: 'first\nlast\n', next: '', + patch: '@@ -1,2 +0,0 @@\n-first\n-last\n', + added: 0, removed: 2, firstChangedLine: 1, + }, + { + name: 'insertion after EOF', + old: 'first\n', next: 'first\nsecond\nthird\n', + patch: '@@ -1,0 +2,2 @@\n+second\n+third\n', + added: 2, removed: 0, firstChangedLine: 2, + }, + { + name: 'multiple hunks with shifted coordinates', + old: 'one\ntwo\nthree\nfour\nfive\n', next: 'one\nTWO\ninserted\nthree\nfour\nFIVE\n', + patch: '@@ -2 +2,2 @@\n-two\n+TWO\n+inserted\n@@ -5 +6 @@\n-five\n+FIVE\n', + added: 3, removed: 2, firstChangedLine: 2, + }, + { + name: 'missing EOF newline markers do not count as lines', + old: 'old', next: 'new', + 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, firstChangedLine: 1, + }, + { + name: 'a single EOF newline does not change normalized lines', + old: 'same', next: 'same\n', + patch: '@@ -1 +1 @@\n-same\n\\ No newline at end of file\n+same\n', + added: 0, removed: 0, firstChangedLine: null, + }, + { + name: 'a blank line differs from an empty file', + old: '', next: '\n', + patch: '@@ -0,0 +1 @@\n+\n', + added: 1, removed: 0, firstChangedLine: 1, + }, + { + name: 'CRLF file contents', + old: 'first\r\nold\r\n', next: 'first\r\nnew\r\n', + patch: '@@ -1,2 +1,2 @@\n first\n-old\n+new\n', + added: 1, removed: 1, firstChangedLine: 2, + }, + { + name: 'CR file contents', + old: 'first\rold\r', next: 'first\rnew\r', + patch: '@@ -1,2 +1,2 @@\n first\n-old\n+new\n', + added: 1, removed: 1, firstChangedLine: 2, + }, + { + name: 'the patch counts are retained even when a minimal diff is smaller', + old: 'same\nold\n', next: 'same\nnew\n', + patch: '@@ -1,2 +1,2 @@\n-same\n-old\n+same\n+new\n', + added: 2, removed: 2, firstChangedLine: 1, + }, + ])('$name', ({ old, next, patch, added, removed, firstChangedLine }) => { + expect(calculator.update(parsePatch(patch)[0]!, old, next)).toEqual({ version: 1, added, removed, firstChangedLine }); + }); + + it('omits statistics if patch application relocated a hunk', () => { + const patch = parsePatch('@@ -1 +1 @@\n-old\n+new\n')[0]!; + expect(calculator.update(patch, 'prefix\nold\n', 'prefix\nnew\n')).toBeNull(); + }); + + 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, 'old\n', 'new\n')).toBeNull(); + }); + + 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, 'old\n', 'new\n')).toBeNull(); + }); + + it('omits binary patches and missing hunks', () => { + const patch = parsePatch('@@ -1 +1 @@\n-old\n+new\n')[0]!; + expect(calculator.update({ ...patch, hunks: [] }, 'old', 'new')).toBeNull(); + patch.isBinary = true; + expect(calculator.update(patch, 'old\n', 'new\n')).toBeNull(); + }); +}); From 7524f71fce2c4ae744f0490debfc8f96ae176557 Mon Sep 17 00:00:00 2001 From: Nikita Ashikhmin Date: Fri, 11 Sep 2026 15:16:36 +0400 Subject: [PATCH 2/3] perf: use the AIR namespace for diff statistics Publish diff statistics at _meta.jetbrains.air.diffStats through the existing AIR metadata helper. Preserve the diff kind and the versioned envelope. Document the wire format, count semantics, navigation, and fallback behavior. Type checking, the build, and all 610 active tests pass; 26 tests are skipped. --- docs/diff-statistics-extension.md | 96 +++++++++++++++++++ readme-dev.md | 5 + src/AirExtension.ts | 1 + src/CodexToolCallMapper.ts | 16 +--- .../data/file-change-add-multiple-files.json | 30 ++++-- .../data/file-change-add-new-file.json | 15 ++- .../data/file-change-add-raw-content.json | 15 ++- .../data/file-change-delete-file.json | 15 ++- .../data/file-change-delete-raw-content.json | 15 ++- .../data/load-session-history.json | 15 ++- .../CodexACPAgent/file-change-events.test.ts | 2 +- 11 files changed, 177 insertions(+), 48 deletions(-) create mode 100644 docs/diff-statistics-extension.md diff --git a/docs/diff-statistics-extension.md b/docs/diff-statistics-extension.md new file mode 100644 index 00000000..a58d1a3e --- /dev/null +++ b/docs/diff-statistics-extension.md @@ -0,0 +1,96 @@ +# AIR diff statistics extension + +Status: Experimental + +Agents can attach line counts and a navigation line to an ACP `diff` content block. +Clients can 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`. +It does not belong to the enclosing tool call or session notification. + +```json +{ + "type": "diff", + "path": "/project/file.txt", + "oldText": "keep\nold\n", + "newText": "keep\nnew\nextra\n", + "_meta": { + "kind": "update", + "jetbrains": { + "air": { + "version": 1, + "diffStats": { + "version": 1, + "added": 2, + "removed": 1, + "firstChangedLine": 2 + } + } + } + } +} +``` + +`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 the existing `kind` field. + +| 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. | +| `firstChangedLine` | integer or null | One-based navigation line in the block's new text. | + +All four fields are required. Numeric strings are invalid. +If both counts are zero, `firstChangedLine` must be null. +Otherwise, it must be positive. +For a deletion at the end, clamp it to the last new line. +For an empty new text, use line 1. +For a partial diff, the line is relative to that block, not the complete file. +ACP `locations` retain their separate file coordinates. + +## Count semantics + +Counts describe the patch that produced the emitted old and new texts. +A patch can contain more operations than a minimal comparison of the final texts. +Clients must preserve valid provider counts rather than replace them with a different comparison. + +For line boundaries, treat CRLF and CR as LF and ignore one final line terminator. +An empty string has zero lines. A string containing only one line terminator has one empty line. +Normalized equal texts have zero added and removed lines. +New files have zero removed lines; deleted files have zero added lines. + +Each diff block owns its statistics. Do not aggregate counts across blocks or tools. +A text revision must carry statistics for that revision, or omit the payload. +Clients must invalidate old statistics when the corresponding texts change. +A status-only update preserves the previous statistics. +Late statistics for unchanged texts may replace previously calculated values. + +## 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 must still send the usual `path`, `oldText`, and `newText` values. + +An agent omits `diffStats` if it cannot produce trustworthy counts. +An empty or invalid patch does not by itself mean that zero lines changed. +Clients fall back to their own comparison when the envelope or payload is missing, invalid, or unsupported. +Unknown fields do not invalidate an otherwise valid payload. + +The earlier experimental `com.intellij/diffStats` key is not part of this contract. +AIR ignores that key and uses its normal fallback. +Already persisted AIR statistics keep their existing format and need no migration. + +## Codex behavior + +For updates, Codex uses the parsed patch used to construct the emitted texts. +It checks hunk sizes, order, coordinates, and content before publishing statistics. +Fuzzy or relocated hunks omit statistics when these checks fail. +For additions and deletions, Codex counts the supplied file content. + +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 fe572b09..43fe8ebd 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -1,6 +1,7 @@ import type { ContentBlock, ToolCallContent } from "@agentclientprotocol/sdk"; 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"; @@ -840,10 +841,7 @@ async function createAddFileContent(change: FileUpdateChange): Promise { expect(event.content).toHaveLength(1); expect(event.content![0]!._meta).toEqual({ kind: 'update', - ...(expected ? { 'com.intellij/diffStats': expected } : {}), + ...(expected ? { jetbrains: { air: { version: 1, diffStats: expected } } } : {}), }); }); From 32dbec4030e8ee4a0fbf50518b2d41db98f8d60d Mon Sep 17 00:00:00 2001 From: Nikita Ashikhmin Date: Fri, 11 Sep 2026 16:13:42 +0400 Subject: [PATCH 3/3] perf: count diff operations without scanning file texts Use the parsed patch as the source of added and removed counts. Remove navigation coordinates and duplicate validation of complete file texts. Keep the existing patch application responsible for constructing ACP content. Count created and deleted files from the content already supplied by Codex. Update the extension specification and regression tests for counts-only metadata. Type checking, the build, and the full local test suite pass. A patch with 20 additions and 30 deletions takes about one microsecond to count. --- docs/diff-statistics-extension.md | 70 +++++++------- src/CodexToolCallMapper.ts | 2 +- src/DiffStats.ts | 91 ++----------------- .../data/file-change-add-multiple-files.json | 6 +- .../data/file-change-add-new-file.json | 3 +- .../data/file-change-add-raw-content.json | 3 +- .../data/file-change-delete-file.json | 3 +- .../data/file-change-delete-raw-content.json | 3 +- .../data/load-session-history.json | 3 +- .../CodexACPAgent/file-change-events.test.ts | 6 +- src/__tests__/DiffStats.test.ts | 72 ++++++--------- 11 files changed, 79 insertions(+), 183 deletions(-) diff --git a/docs/diff-statistics-extension.md b/docs/diff-statistics-extension.md index a58d1a3e..a493f2b9 100644 --- a/docs/diff-statistics-extension.md +++ b/docs/diff-statistics-extension.md @@ -2,21 +2,20 @@ Status: Experimental -Agents can attach line counts and a navigation line to an ACP `diff` content block. -Clients can use these values without comparing the block's texts again. +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`. -It does not belong to the enclosing tool call or session notification. ```json { "type": "diff", "path": "/project/file.txt", - "oldText": "keep\nold\n", - "newText": "keep\nnew\nextra\n", + "oldText": "old\n", + "newText": "new\nextra\n", "_meta": { "kind": "update", "jetbrains": { @@ -25,8 +24,7 @@ It does not belong to the enclosing tool call or session notification. "diffStats": { "version": 1, "added": 2, - "removed": 1, - "firstChangedLine": 2 + "removed": 1 } } } @@ -36,61 +34,57 @@ It does not belong to the enclosing tool call or session notification. `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 the existing `kind` field. +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. | -| `firstChangedLine` | integer or null | One-based navigation line in the block's new text. | -All four fields are required. Numeric strings are invalid. -If both counts are zero, `firstChangedLine` must be null. -Otherwise, it must be positive. -For a deletion at the end, clamp it to the last new line. -For an empty new text, use line 1. -For a partial diff, the line is relative to that block, not the complete file. -ACP `locations` retain their separate file coordinates. +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 -Counts describe the patch that produced the emitted old and new texts. -A patch can contain more operations than a minimal comparison of the final texts. -Clients must preserve valid provider counts rather than replace them with a different comparison. +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 line boundaries, treat CRLF and CR as LF and ignore one final line terminator. -An empty string has zero lines. A string containing only one line terminator has one empty line. -Normalized equal texts have zero added and removed lines. -New files have zero removed lines; deleted files have zero added lines. +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. Do not aggregate counts across blocks or tools. -A text revision must carry statistics for that revision, or omit the payload. -Clients must invalidate old statistics when the corresponding texts change. -A status-only update preserves the previous statistics. -Late statistics for unchanged texts may replace previously calculated values. +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 must still send the usual `path`, `oldText`, and `newText` values. +Agents still send the usual `path`, `oldText`, and `newText` values. -An agent omits `diffStats` if it cannot produce trustworthy counts. -An empty or invalid patch does not by itself mean that zero lines changed. -Clients fall back to their own comparison when the envelope or payload is missing, invalid, or unsupported. -Unknown fields do not invalidate an otherwise valid payload. +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. -Already persisted AIR statistics keep their existing format and need no migration. +Existing persisted statistics, including stored navigation lines, remain readable without migration. ## Codex behavior -For updates, Codex uses the parsed patch used to construct the emitted texts. -It checks hunk sizes, order, coordinates, and content before publishing statistics. -Fuzzy or relocated hunks omit statistics when these checks fail. -For additions and deletions, Codex counts the supplied file content. +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/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 43fe8ebd..d21bbaf4 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -880,7 +880,7 @@ async function createUpdateFileContent(change: FileUpdateChange): Promise oldLineCount + 1 || newStart + newLines > newLineCount + 1 || newStart - oldStart !== added - removed) return null; let oldConsumed = 0; let newConsumed = 0; @@ -39,13 +32,11 @@ export class DiffStatsCalculator { for (const line of hunk.lines) { switch (line[0]) { case '+': - firstChangedLine ??= newStart + newConsumed; added++; newConsumed++; previousWasContent = true; break; case '-': - firstChangedLine ??= newStart + newConsumed; removed++; oldConsumed++; previousWasContent = true; @@ -68,82 +59,16 @@ export class DiffStatsCalculator { previousOldEnd = oldStart + oldLines; previousNewEnd = newStart + newLines; } - if (oldLineCount + added - removed !== newLineCount || - !this.matchesHunks(patch, oldText, false) || !this.matchesHunks(patch, newText, true)) return null; - if (this.sameLines(oldText, newText)) return { version: 1, added: 0, removed: 0, firstChangedLine: null }; - if (firstChangedLine === null) return null; - return { - version: 1, - added, - removed, - firstChangedLine: Math.max(1, Math.min(firstChangedLine, newLineCount)), - }; - } - - private matchesHunks(patch: StructuredPatch, text: string, newSide: boolean): boolean { - let offset = 0; - let lineNumber = 1; - for (const hunk of patch.hunks) { - const start = newSide ? hunk.newStart : hunk.oldStart; - while (lineNumber < start) { - offset = this.nextLine(text, offset); - lineNumber++; - } - for (const line of hunk.lines) { - if (line[0] === '\\' || line[0] === (newSide ? '-' : '+')) continue; - let end = offset; - while (end < text.length && text[end] !== '\n' && text[end] !== '\r') end++; - const expected = line.slice(1).replace(/\r$/, ''); - if (text.slice(offset, end) !== expected) return false; - offset = this.nextLine(text, offset); - lineNumber++; - } - } - return true; - } - - private nextLine(text: string, offset: number): number { - while (offset < text.length && text[offset] !== '\n' && text[offset] !== '\r') offset++; - if (offset === text.length) return offset; - return offset + (text[offset] === '\r' && text[offset + 1] === '\n' ? 2 : 1); + return { version: 1, added, removed }; } private lineCount(text: string): number { let count = 0; - let offset = 0; - while (offset < text.length) { - offset = this.nextLine(text, offset); - count++; + for (let offset = text.indexOf('\n'); offset >= 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; } - - private sameLines(left: string, right: string): boolean { - if (left === right) return true; - if (left.length === 0 || right.length === 0) return false; - const leftEnd = this.contentEnd(left); - const rightEnd = this.contentEnd(right); - let leftOffset = 0; - let rightOffset = 0; - while (leftOffset < leftEnd && rightOffset < rightEnd) { - let leftChar = left[leftOffset++]; - let rightChar = right[rightOffset++]; - if (leftChar === '\r') { - if (left[leftOffset] === '\n') leftOffset++; - leftChar = '\n'; - } - if (rightChar === '\r') { - if (right[rightOffset] === '\n') rightOffset++; - rightChar = '\n'; - } - if (leftChar !== rightChar) return false; - } - return leftOffset === leftEnd && rightOffset === rightEnd; - } - - private contentEnd(text: string): number { - if (text.endsWith('\r\n')) return text.length - 2; - if (text.endsWith('\n') || text.endsWith('\r')) return text.length - 1; - return text.length; - } } 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 949f6682..26848a01 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json +++ b/src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json @@ -23,8 +23,7 @@ "diffStats": { "version": 1, "added": 1, - "removed": 0, - "firstChangedLine": 1 + "removed": 0 } } } @@ -43,8 +42,7 @@ "diffStats": { "version": 1, "added": 1, - "removed": 0, - "firstChangedLine": 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 b0ba442f..8ede27a2 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-add-new-file.json +++ b/src/__tests__/CodexACPAgent/data/file-change-add-new-file.json @@ -23,8 +23,7 @@ "diffStats": { "version": 1, "added": 5, - "removed": 0, - "firstChangedLine": 1 + "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 603cb99e..2e009c80 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json +++ b/src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json @@ -23,8 +23,7 @@ "diffStats": { "version": 1, "added": 3, - "removed": 0, - "firstChangedLine": 1 + "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 0335e478..63b0cb18 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-delete-file.json +++ b/src/__tests__/CodexACPAgent/data/file-change-delete-file.json @@ -23,8 +23,7 @@ "diffStats": { "version": 1, "added": 0, - "removed": 3, - "firstChangedLine": 1 + "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 a8e6757d..ba03757b 100644 --- a/src/__tests__/CodexACPAgent/data/file-change-delete-raw-content.json +++ b/src/__tests__/CodexACPAgent/data/file-change-delete-raw-content.json @@ -23,8 +23,7 @@ "diffStats": { "version": 1, "added": 0, - "removed": 3, - "firstChangedLine": 1 + "removed": 3 } } } diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index 869c5df3..fbdbe046 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -286,8 +286,7 @@ "diffStats": { "version": 1, "added": 2, - "removed": 0, - "firstChangedLine": 1 + "removed": 0 } } } diff --git a/src/__tests__/CodexACPAgent/file-change-events.test.ts b/src/__tests__/CodexACPAgent/file-change-events.test.ts index cd78d4c7..426fefd0 100644 --- a/src/__tests__/CodexACPAgent/file-change-events.test.ts +++ b/src/__tests__/CodexACPAgent/file-change-events.test.ts @@ -269,9 +269,9 @@ describe('CodexEventHandler - file change events', () => { }); it.each([ - { name: 'before application', disk: 'old\n', expected: { version: 1, added: 2, removed: 1, firstChangedLine: 1 } }, - { name: 'after application', disk: 'new\nextra\n', expected: { version: 1, added: 2, removed: 1, firstChangedLine: 1 } }, - { name: 'a relocated hunk', disk: 'prefix\nold\n', expected: undefined }, + { 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({ diff --git a/src/__tests__/DiffStats.test.ts b/src/__tests__/DiffStats.test.ts index 0301fabd..c553c95e 100644 --- a/src/__tests__/DiffStats.test.ts +++ b/src/__tests__/DiffStats.test.ts @@ -17,87 +17,66 @@ describe('ACP diff statistics', () => { ['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, firstChangedLine: count === 0 ? null : 1, + version: 1, added: count, removed: 0, }); expect(calculator.deletedFile(text)).toEqual({ - version: 1, added: 0, removed: count, firstChangedLine: count === 0 ? null : 1, + version: 1, added: 0, removed: count, }); }); it.each([ { name: 'replacement with blank context', - old: 'first\n\nold\n', next: 'first\n\nnew\nextra\n', patch: '@@ -1,3 +1,4 @@\n first\n \n-old\n+new\n+extra\n', - added: 2, removed: 1, firstChangedLine: 3, + added: 2, removed: 1, }, { - name: 'deletion at EOF clamps to the last remaining line', - old: 'first\nlast\n', next: 'first\n', + name: 'deletion at EOF', patch: '@@ -2 +1,0 @@\n-last\n', - added: 0, removed: 1, firstChangedLine: 1, + added: 0, removed: 1, }, { - name: 'deletion of the entire file clamps to line one', - old: 'first\nlast\n', next: '', + name: 'deletion of the entire file', patch: '@@ -1,2 +0,0 @@\n-first\n-last\n', - added: 0, removed: 2, firstChangedLine: 1, + added: 0, removed: 2, }, { name: 'insertion after EOF', - old: 'first\n', next: 'first\nsecond\nthird\n', patch: '@@ -1,0 +2,2 @@\n+second\n+third\n', - added: 2, removed: 0, firstChangedLine: 2, + added: 2, removed: 0, }, { name: 'multiple hunks with shifted coordinates', - old: 'one\ntwo\nthree\nfour\nfive\n', next: 'one\nTWO\ninserted\nthree\nfour\nFIVE\n', patch: '@@ -2 +2,2 @@\n-two\n+TWO\n+inserted\n@@ -5 +6 @@\n-five\n+FIVE\n', - added: 3, removed: 2, firstChangedLine: 2, + added: 3, removed: 2, }, { name: 'missing EOF newline markers do not count as lines', - old: 'old', next: 'new', 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, firstChangedLine: 1, + added: 1, removed: 1, }, { - name: 'a single EOF newline does not change normalized lines', - old: 'same', next: 'same\n', + 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: 0, removed: 0, firstChangedLine: null, + added: 1, removed: 1, }, { name: 'a blank line differs from an empty file', - old: '', next: '\n', patch: '@@ -0,0 +1 @@\n+\n', - added: 1, removed: 0, firstChangedLine: 1, + added: 1, removed: 0, }, { - name: 'CRLF file contents', - old: 'first\r\nold\r\n', next: 'first\r\nnew\r\n', - patch: '@@ -1,2 +1,2 @@\n first\n-old\n+new\n', - added: 1, removed: 1, firstChangedLine: 2, - }, - { - name: 'CR file contents', - old: 'first\rold\r', next: 'first\rnew\r', - patch: '@@ -1,2 +1,2 @@\n first\n-old\n+new\n', - added: 1, removed: 1, firstChangedLine: 2, + 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', - old: 'same\nold\n', next: 'same\nnew\n', patch: '@@ -1,2 +1,2 @@\n-same\n-old\n+same\n+new\n', - added: 2, removed: 2, firstChangedLine: 1, + added: 2, removed: 2, }, - ])('$name', ({ old, next, patch, added, removed, firstChangedLine }) => { - expect(calculator.update(parsePatch(patch)[0]!, old, next)).toEqual({ version: 1, added, removed, firstChangedLine }); - }); - - it('omits statistics if patch application relocated a hunk', () => { - const patch = parsePatch('@@ -1 +1 @@\n-old\n+new\n')[0]!; - expect(calculator.update(patch, 'prefix\nold\n', 'prefix\nnew\n')).toBeNull(); + ])('$name', ({ patch, added, removed }) => { + expect(calculator.update(parsePatch(patch)[0]!)).toEqual({ version: 1, added, removed }); }); it.each([ @@ -112,19 +91,24 @@ describe('ACP diff statistics', () => { ])('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, 'old\n', 'new\n')).toBeNull(); + 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, 'old\n', 'new\n')).toBeNull(); + 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: [] }, 'old', 'new')).toBeNull(); + expect(calculator.update({ ...patch, hunks: [] })).toBeNull(); patch.isBinary = true; - expect(calculator.update(patch, 'old\n', 'new\n')).toBeNull(); + expect(calculator.update(patch)).toBeNull(); }); });