diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index de08b9d74..ce7abd651 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,12 +1,15 @@ # Changelog ## [Unreleased] + ### Added - Visualize leading whitespace (indentation) in diff output with dim glyphs—tabs display as ` → ` and spaces as `·` for improved readability ### Fixed +- Fixed patch applicator to correctly handle context-only hunks (pure context lines between @@ markers) without altering indentation in tab-indented files +- Fixed indentation conversion logic to infer tab width from space-to-tab patterns using linear regression (ax+b model) when pattern uses spaces and actual file uses tabs - Fixed tab character rendering in tool output previews and code cell displays, ensuring tabs are properly converted to spaces for consistent terminal display - Fixed `newSession()` to properly await session manager operations, ensuring new session is fully initialized before returning - Fixed session formatting to use XML structure for tools and tool invocations instead of YAML, improving compatibility with structured output parsing diff --git a/packages/coding-agent/src/patch/applicator.ts b/packages/coding-agent/src/patch/applicator.ts index f6883e3bf..54ae2d585 100644 --- a/packages/coding-agent/src/patch/applicator.ts +++ b/packages/coding-agent/src/patch/applicator.ts @@ -126,6 +126,8 @@ function adjustLinesIndentation(patternLines: string[], actualLines: string[], n let patternTabOnly = true; let actualSpaceOnly = true; + let patternSpaceOnly = true; + let actualTabOnly = true; let patternMixed = false; let actualMixed = false; @@ -133,6 +135,7 @@ function adjustLinesIndentation(patternLines: string[], actualLines: string[], n if (line.trim().length === 0) continue; const ws = getLeadingWhitespace(line); if (ws.includes(" ")) patternTabOnly = false; + if (ws.includes("\t")) patternSpaceOnly = false; if (ws.includes(" ") && ws.includes("\t")) patternMixed = true; } @@ -140,6 +143,7 @@ function adjustLinesIndentation(patternLines: string[], actualLines: string[], n if (line.trim().length === 0) continue; const ws = getLeadingWhitespace(line); if (ws.includes("\t")) actualSpaceOnly = false; + if (ws.includes(" ")) actualTabOnly = false; if (ws.includes(" ") && ws.includes("\t")) actualMixed = true; } @@ -173,6 +177,88 @@ function adjustLinesIndentation(patternLines: string[], actualLines: string[], n } } + // Reverse: pattern uses spaces, actual uses tabs — infer spaces = tabs * width + offset + // Collect (tabs, spaces) pairs from matched lines to solve for the model's tab rendering. + // With one data point: spaces = tabs * width (offset=0). + // With two+: solve ax + b via pairs with distinct tab counts. + if (!patternMixed && !actualMixed && patternSpaceOnly && actualTabOnly) { + const samples = new Map(); // tabs -> spaces + const lineCount = Math.min(patternLines.length, actualLines.length); + let consistent = true; + for (let i = 0; i < lineCount; i++) { + const patternLine = patternLines[i]; + const actualLine = actualLines[i]; + if (patternLine.trim().length === 0 || actualLine.trim().length === 0) continue; + const spaces = countLeadingWhitespace(patternLine); + const tabs = countLeadingWhitespace(actualLine); + if (tabs === 0) continue; + const existing = samples.get(tabs); + if (existing !== undefined && existing !== spaces) { + consistent = false; + break; + } + samples.set(tabs, spaces); + } + + if (consistent && samples.size > 0) { + let tabWidth: number | undefined; + let offset = 0; + + if (samples.size === 1) { + // One level: assume offset=0, width = spaces / tabs + const [[tabs, spaces]] = samples; + if (spaces % tabs === 0) { + tabWidth = spaces / tabs; + } + } else { + // Two+ levels: solve via any two distinct pairs + // spaces = tabs * width + offset => width = (s2 - s1) / (t2 - t1) + const entries = [...samples.entries()]; + const [t1, s1] = entries[0]; + const [t2, s2] = entries[1]; + if (t1 !== t2) { + const w = (s2 - s1) / (t2 - t1); + if (w > 0 && Number.isInteger(w)) { + const b = s1 - t1 * w; + // Validate all samples against this model + let valid = true; + for (const [t, s] of samples) { + if (t * w + b !== s) { + valid = false; + break; + } + } + if (valid) { + tabWidth = w; + offset = b; + } + } + } + } + + if (tabWidth !== undefined && tabWidth > 0) { + const converted = newLines.map(line => { + if (line.trim().length === 0) return line; + const ws = countLeadingWhitespace(line); + if (ws === 0) return line; + // Reverse: tabs = (spaces - offset) / width + const adjusted = ws - offset; + if (adjusted >= 0 && adjusted % tabWidth! === 0) { + return "\t".repeat(adjusted / tabWidth!) + line.slice(ws); + } + // Partial tab — keep remainder as spaces + const tabCount = Math.floor(adjusted / tabWidth!); + const remainder = adjusted - tabCount * tabWidth!; + if (tabCount >= 0) { + return "\t".repeat(tabCount) + " ".repeat(remainder) + line.slice(ws); + } + return line; + }); + return converted; + } + } + } + // Build a map from trimmed content to actual lines (by content, not position) // This handles fuzzy matches where pattern and actual may not be positionally aligned const contentToActualLines = new Map(); @@ -1118,8 +1204,25 @@ function computeReplacements( // Adjust indentation if needed (handles fuzzy matches where indentation differs) const actualMatchedLines = originalLines.slice(found, found + pattern.length); - const adjustedNewLines = adjustLinesIndentation(pattern, actualMatchedLines, newSlice); + // Skip pure-context hunks (no +/- lines — oldLines === newLines). + // They serve only to advance lineIndex for subsequent hunks. + let isNoOp = pattern.length === newSlice.length; + if (isNoOp) { + for (let i = 0; i < pattern.length; i++) { + if (pattern[i] !== newSlice[i]) { + isNoOp = false; + break; + } + } + } + + if (isNoOp) { + lineIndex = found + pattern.length; + continue; + } + + const adjustedNewLines = adjustLinesIndentation(pattern, actualMatchedLines, newSlice); replacements.push({ startIndex: found, oldLen: pattern.length, newLines: adjustedNewLines }); lineIndex = found + pattern.length; } diff --git a/packages/coding-agent/src/prompts/tools/patch.md b/packages/coding-agent/src/prompts/tools/patch.md index 2128031f3..851e2a034 100644 --- a/packages/coding-agent/src/prompts/tools/patch.md +++ b/packages/coding-agent/src/prompts/tools/patch.md @@ -48,7 +48,7 @@ Returns success/failure; on failure, error message indicates: - Never use anchors as comments (no line numbers, location labels, placeholders like `@@ @@`) - Do not place new lines outside intended block - If edit fails or breaks structure, re-read file and produce new patch from current content—do not retry same diff -- If indentation/alignment wrong after editing, run formatter (`go fmt`, `cargo fmt`, `ruff format`, `biome`, etc.)—never make repeated edit attempts to fix whitespace +- **NEVER** use edit to fix indentation or reformat code—run the project's formatter instead diff --git a/packages/coding-agent/test/core/apply-patch-regression.test.ts b/packages/coding-agent/test/core/apply-patch-regression.test.ts index c65219988..e8943ff2e 100644 --- a/packages/coding-agent/test/core/apply-patch-regression.test.ts +++ b/packages/coding-agent/test/core/apply-patch-regression.test.ts @@ -1796,3 +1796,107 @@ describe("regression: trailing context lines don't delete file content", () => { expect(result).toContain("return this._kittyProtocolActive;"); }); }); + +describe("regression: context-only hunks between @@ markers must not change indentation (agent-session.ts)", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = path.join(os.tmpdir(), `regression-ctx-noop-${Date.now()}-${Math.random().toString(36).slice(2)}`); + fs.mkdirSync(tempDir, { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + test("pure context hunk (no +/- lines) does not alter tab-indented file content", async () => { + const filePath = path.join(tempDir, "agent-session.ts"); + // Actual file uses tab indentation (\t\t for method body). + // Pre-patch state: three callsites lack `await`. + const fileContent = [ + "class AgentSession {", + "\tasync newSession(options?: NewSessionOptions): Promise {", + "\t\tconst previousSessionFile = this.sessionFile;", + "", + "\t\tthis._disconnectFromAgent();", + "\t\tawait this.abort();", + "\t\tthis.agent.reset();", + "\t\tawait this.sessionManager.flush();", + "\t\tthis.sessionManager.newSession(options);", + "\t\tthis.agent.sessionId = this.sessionManager.getSessionId();", + "\t\tthis._steeringMessages = [];", + "\t\tthis._followUpMessages = [];", + "\t\tthis._pendingNextTurnMessages = [];", + "\t}", + "}", + "", + ].join("\n"); + await Bun.write(filePath, fileContent); + + // Exact patch from the regression case (spaces in diff, tabs in file). + // The lines between the first @@ and second @@ are pure context. + const diff = [ + "@@", + " async newSession(options?: NewSessionOptions): Promise {", + " const previousSessionFile = this.sessionFile;", + "@@", + " this._disconnectFromAgent();", + " await this.abort();", + " this.agent.reset();", + " await this.sessionManager.flush();", + "- this.sessionManager.newSession(options);", + "+ await this.sessionManager.newSession(options);", + " this.agent.sessionId = this.sessionManager.getSessionId();", + " this._steeringMessages = [];", + " this._followUpMessages = [];", + " this._pendingNextTurnMessages = [];", + ].join("\n"); + + await applyPatch({ path: "agent-session.ts", op: "update", diff }, { cwd: tempDir }); + + const result = await Bun.file(filePath).text(); + // The change should be applied + expect(result).toContain("\t\tawait this.sessionManager.newSession(options);"); + // Lines covered by the pure-context hunk must keep their original tab indentation + expect(result).toContain("\tasync newSession(options?: NewSessionOptions): Promise {"); + expect(result).toContain("\t\tconst previousSessionFile = this.sessionFile;"); + expect(result).toContain("\t\tthis._disconnectFromAgent();"); + }); + + test("space-to-tab conversion with offset (ax+b model)", async () => { + const filePath = path.join(tempDir, "offset.ts"); + // File uses tabs: 1 tab for class body, 2 tabs for method body, 3 for nested + const fileContent = [ + "class Foo {", + "\tbar() {", + "\t\tif (true) {", + "\t\t\tthis.x = 1;", + "\t\t}", + "\t}", + "}", + "", + ].join("\n"); + await Bun.write(filePath, fileContent); + + // Model rendered tabs as 3 cols with 1 extra offset: + // 1 tab -> 4 spaces, 2 tabs -> 7 spaces, 3 tabs -> 10 spaces + // => width=3, offset=1 + const diff = [ + "@@ class Foo {", + " bar() {", + "- if (true) {", + "- this.x = 1;", + "+ if (ready) {", + "+ this.x = 42;", + " }", + ].join("\n"); + + await applyPatch({ path: "offset.ts", op: "update", diff }, { cwd: tempDir }); + + const result = await Bun.file(filePath).text(); + expect(result).toContain("\t\tif (ready) {"); + expect(result).toContain("\t\t\tthis.x = 42;"); + // Unchanged lines must keep tabs + expect(result).toContain("\tbar() {"); + }); +});