From 72a30af57c53ed9ac3eea9a94d252041f2124059 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 08:05:12 +0000 Subject: [PATCH 1/5] fix(hashline): reveal seen-line-guard content and merge into snapshot on reject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Structural-summary reads (default for parseable code >100 lines) mint a `[path#tag]` that only marks declaration/boundary lines as displayed; edits anchored inside an elided body then hit `#assertSeenLines` in `packages/hashline/src/patcher.ts` and reject with "never displayed (it showed a partial range, a search hit, or a folded summary)". The existing message pointed at a range re-read, but that made every such recovery a three-turn round-trip (edit-fail → range read → edit-retry) and models frequently retried the same edit instead of following the hint — 5-8 out of 10 edits failed for the reporter. The rejection now: - Inlines the actual file content at the unseen anchor lines, from `matchedSnapshot.text` (which by definition equals the live normalized content on the no-drift path), up to `SEEN_LINE_REVEAL_CAP` (40) lines. - Merges the revealed lines into the snapshot's `seenLines` set, so a straight retry with the same `[path#tag]` header succeeds without a follow-up read. The content is inside the error the model receives, which is the proof it has now seen those lines. - For anchor ranges over the cap, only the revealed prefix is merged; the message keeps the range-re-read guidance for the remainder so runaway blind edits don't sneak past. Fixes #4224 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../test/edit/seen-line-guard.test.ts | 60 +++++++++++++++++++ packages/hashline/CHANGELOG.md | 4 ++ packages/hashline/src/messages.ts | 57 ++++++++++++++++-- packages/hashline/src/patcher.ts | 35 ++++++++++- packages/hashline/test/patcher.test.ts | 48 +++++++++++++++ 6 files changed, 201 insertions(+), 7 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 64c3c57ae..2b4fea2dc 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `edit` frequently failing with `never displayed (it showed a partial range, a search hit, or a folded summary)` after a structural-summary `read` on a >100-line source file: the rejection now inlines the actual file content at the unseen anchor lines and merges them into the snapshot's `seenLines` set, so a straight edit retry with the same `[path#tag]` header succeeds instead of demanding a follow-up range re-read. Wide (over 40-line) anchor ranges still fall back to a range re-read for the tail. ([#4224](https://github.com/can1357/oh-my-pi/issues/4224)) + ## [16.3.1] - 2026-07-02 ### Breaking Changes diff --git a/packages/coding-agent/test/edit/seen-line-guard.test.ts b/packages/coding-agent/test/edit/seen-line-guard.test.ts index 635b7abca..55a596cf2 100644 --- a/packages/coding-agent/test/edit/seen-line-guard.test.ts +++ b/packages/coding-agent/test/edit/seen-line-guard.test.ts @@ -169,6 +169,66 @@ describe("read → edit seen-line guard", () => { expect(edited).toContain("\tconfigure_gpio();\n\tbeep_3k8hz_on();\n\tk_sleep(K_MSEC(300));\n\tbeep_3k8hz_off();"); expect(edited).not.toContain("\tbeep_3k8hz_on();\n\tline_1289();\n\tk_sleep(K_MSEC(300));"); }); + + it("reveals the actual line content in the rejection and unblocks a same-tag retry", async () => { + const file = path.join(tmpDir, "notes.txt"); + await Bun.write(file, CONTENT); + const session = createSession(tmpDir); + + const read = await new ReadTool(session).execute("r1", { path: `${file}:1-3` }); + const tag = tagFromOutput(resultText(read)); + + let message: string | undefined; + try { + await executeHashlineSingle(execOptions(`[notes.txt#${tag}]\nSWAP 10.=12:\n+X10\n+X11\n+X12`, session)); + } catch (err) { + message = (err as Error).message; + } + // Rejection surfaces the ACTUAL file content at the unseen anchor lines. + expect(message).toMatch(/never displayed \(it showed/); + expect(message).toContain("Actual file content at those lines:"); + expect(message).toContain("10:line 10"); + expect(message).toContain("11:line 11"); + expect(message).toContain("12:line 12"); + // Snapshot text preserved verbatim — the reject is still a no-op on disk. + expect(await Bun.file(file).text()).toBe(CONTENT); + + // The revealed lines are now in the snapshot's seen set, so a straight + // retry with the same `[path#tag]` header succeeds without a re-read. + await executeHashlineSingle(execOptions(`[notes.txt#${tag}]\nSWAP 10.=12:\n+X10\n+X11\n+X12`, session)); + const after = await Bun.file(file).text(); + expect(after).toContain("X10\nX11\nX12"); + expect(after).not.toContain("line 10"); + }); + + it("keeps the re-read fallback when the anchor set exceeds the inline reveal cap", async () => { + const file = path.join(tmpDir, "long.txt"); + const lines = Array.from({ length: 200 }, (_, i) => `line ${i + 1}`); + await Bun.write(file, `${lines.join("\n")}\n`); + const session = createSession(tmpDir); + + // Partial read of the head — seenLines = 1..3 only. + const read = await new ReadTool(session).execute("r1", { path: `${file}:1-3` }); + const tag = tagFromOutput(resultText(read)); + + // Anchor 60 unseen lines — deliberately over the 40-line cap. + const dels = Array.from({ length: 60 }, (_, i) => `DEL ${100 + i}`).join("\n"); + let message: string | undefined; + try { + await executeHashlineSingle(execOptions(`[long.txt#${tag}]\n${dels}`, session)); + } catch (err) { + message = (err as Error).message; + } + expect(message).toMatch(/never displayed \(it showed/); + // Only the first cap-worth of lines are revealed. + expect(message).toContain("Preview of the actual file content at the first 40 unseen line(s)"); + expect(message).toContain("100:line 100"); + expect(message).toContain("139:line 139"); + expect(message).not.toContain("140:line 140"); + // Guidance directs at a range re-read of the FULL anchor range. + expect(message).toMatch(/long\.txt:100-159/); + expect(await Bun.file(file).text()).toBe(`${lines.join("\n")}\n`); + }); }); describe("search → edit seen-line guard", () => { diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 322c86b4f..272ee1572 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed frequent `hashline` edit rejections after a structural-summary read (default for parseable code >100 lines) by inlining the actual file content at the unseen anchor lines into the `never displayed (it showed a partial range, a search hit, or a folded summary)` error and merging those lines into the snapshot's `seenLines` set — a straight retry with the same `[path#tag]` header now succeeds instead of requiring a separate range re-read. Anchor ranges over the 40-line inline reveal cap still bounce back to the range re-read for the remainder. ([#4224](https://github.com/can1357/oh-my-pi/issues/4224)) + ## [16.3.0] - 2026-07-02 ### Changed diff --git a/packages/hashline/src/messages.ts b/packages/hashline/src/messages.ts index e86b0d748..55aa2a088 100644 --- a/packages/hashline/src/messages.ts +++ b/packages/hashline/src/messages.ts @@ -223,21 +223,66 @@ function formatLineRanges(lines: readonly number[]): string { return parts.join(", "); } +/** One anchored line whose actual content is being surfaced in an error message. */ +export interface RevealedLine { + line: number; + text: string; +} + +/** + * Content preview handed to {@link unseenLinesMessage}. `lines` are the + * unseen anchor lines whose actual file content we surface inline (from the + * tagged snapshot the caller matched). `truncated` = true means the anchor + * range exceeded the inline reveal cap; the caller only revealed a prefix + * and the remaining unseen lines still require a range re-read. + */ +export interface UnseenLinesReveal { + lines: readonly RevealedLine[]; + truncated: boolean; +} + /** * An anchored edit referenced lines the read that minted the cited tag never * displayed (a partial range, or a structural summary that collapsed bodies). * Editing lines you have not read is the off-by-memory failure that mangles - * files; reject and make the model re-read those exact lines first. + * files. When `reveal.lines` is non-empty, the caller has already inlined the + * actual file content at those lines and merged them into the snapshot's + * seen-line set, so the message points the model at a straight retry with the + * same `[path#tag]` header; when the reveal is empty or truncated, the + * message falls back to instructing a range re-read. */ -export function unseenLinesMessage(sectionPath: string, unseenLines: readonly number[], tag: string): string { +export function unseenLinesMessage( + sectionPath: string, + unseenLines: readonly number[], + tag: string, + reveal: UnseenLinesReveal = { lines: [], truncated: false }, +): string { const ranges = formatLineRanges(unseenLines); const selector = ranges.replace(/, /g, ","); - return ( + const header = `This edit anchors to lines ${ranges} of ${sectionPath} that ` + `${HL_FILE_PREFIX}${sectionPath}${HL_FILE_HASH_SEP}${tag}${HL_FILE_SUFFIX} never displayed (it showed a ` + - `partial range, a search hit, or a folded summary). Re-read them in full first with a ranged read like ` + - `\`${sectionPath}:${selector}\` — it skips summarization and mints a fresh tag (a plain re-read just re-folds ` + - `them) — then re-issue the edit.` + `partial range, a search hit, or a folded summary).`; + if (reveal.lines.length === 0) { + return ( + `${header} Re-read them in full first with a ranged read like ` + + `\`${sectionPath}:${selector}\` — it skips summarization and mints a fresh tag (a plain re-read just re-folds ` + + `them) — then re-issue the edit.` + ); + } + const preview = reveal.lines.map(({ line, text }) => ` ${formatNumberedLine(line, text)}`).join("\n"); + if (reveal.truncated) { + return ( + `${header} Preview of the actual file content at the first ${reveal.lines.length} unseen line(s):\n${preview}\n` + + `The range exceeds the inline preview cap — re-read the remainder with \`${sectionPath}:${selector}\` before ` + + `re-issuing the edit.` + ); + } + return ( + `${header} Actual file content at those lines:\n${preview}\n` + + `Verify the content matches what you intend to touch, then re-issue the edit with the same ` + + `${HL_FILE_PREFIX}path${HL_FILE_HASH_SEP}tag${HL_FILE_SUFFIX} header — a straight retry now succeeds without a re-read. ` + + `If the content does NOT match, fix your line numbers.` ); } diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index 2e3bc272d..5df8cfeda 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -33,6 +33,7 @@ import { HEADTAIL_DRIFT_WARNING, missingSnapshotTagMessage, pathRecoveredFromTagMessage, + type RevealedLine, unseenLinesMessage, } from "./messages"; import { MismatchError } from "./mismatch"; @@ -41,6 +42,16 @@ import { Recovery, type RecoveryResult } from "./recovery"; import type { Snapshot, SnapshotStore } from "./snapshots"; import type { ApplyResult, BlockResolution, BlockResolver, Edit, FileOp } from "./types"; +/** + * Upper bound on the number of unseen anchor lines whose actual file content + * we inline into a rejection error (see {@link Patcher.assertSeenLines}). Big + * enough to fit the common "edit a whole function body" retry path in one + * message, small enough to keep the error human-readable when the model + * over-anchors and to preserve the "re-read first" fallback for genuinely + * blind wide edits (only the revealed prefix gets merged into `seenLines`). + */ +const SEEN_LINE_REVEAL_CAP = 40; + export interface PatcherOptions { /** Storage backend used for all reads and writes. */ fs: Filesystem; @@ -486,13 +497,35 @@ export class Patcher { * externally minted or aged out), so the edit applies as before. Only runs * on the no-drift path, where anchor line numbers index the tagged content * 1:1. + * + * The rejection inlines the actual file content at the unseen anchor lines + * (from `matchedSnapshot.text`, which by definition equals the live + * normalized content) and merges those lines into the snapshot's + * seen-line set, so a straight retry with the same `[path#tag]` header + * succeeds without a follow-up range read — the content the model + * receives in the error IS proof it has now seen those lines. Ranges + * beyond {@link SEEN_LINE_REVEAL_CAP} still bounce back to a range + * re-read for the remainder; only the revealed prefix is merged. */ #assertSeenLines(section: PatchSection, expected: string, matchedSnapshot: Snapshot | null): void { const seen = matchedSnapshot?.seenLines; if (!seen || seen.size === 0) return; const unseen = section.collectAnchorLines().filter(line => !seen.has(line)); if (unseen.length === 0) return; - throw new Error(unseenLinesMessage(section.path, unseen, expected)); + const sourceLines = matchedSnapshot?.text.split("\n") ?? []; + const revealed: RevealedLine[] = []; + const revealCount = Math.min(unseen.length, SEEN_LINE_REVEAL_CAP); + for (let i = 0; i < revealCount; i++) { + const line = unseen[i]; + // Out-of-range anchors are caught by parse/apply with a better + // message; skip them here so they never join the revealed set or + // the seen-line merge. + if (line < 1 || line > sourceLines.length) continue; + revealed.push({ line, text: sourceLines[line - 1] ?? "" }); + } + const truncated = unseen.length > revealed.length; + for (const { line } of revealed) seen.add(line); + throw new Error(unseenLinesMessage(section.path, unseen, expected, { lines: revealed, truncated })); } #mismatchError( section: PatchSection, diff --git a/packages/hashline/test/patcher.test.ts b/packages/hashline/test/patcher.test.ts index f07ab03a2..f0e0f46c7 100644 --- a/packages/hashline/test/patcher.test.ts +++ b/packages/hashline/test/patcher.test.ts @@ -306,6 +306,54 @@ describe("Patcher seen-line provenance", () => { expect(fs.get(PATH)).toBe("l1\nl2\nl3\nL4\nl5\n"); }); + it("reveals the actual line content in the rejection and unblocks a same-tag retry", async () => { + const fs = new InMemoryFilesystem([[PATH, CONTENT]]); + const snapshots = new InMemorySnapshotStore(); + // Read only surfaced lines 1-2; anchor line 4 is unseen. + const tag = snapshots.record(PATH, CONTENT, [1, 2]); + const patcher = new Patcher({ fs, snapshots }); + + let message: string | undefined; + try { + await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 4.=4:\n+L4`)); + } catch (err) { + message = (err as Error).message; + } + expect(message).toMatch(/never displayed \(it showed/); + expect(message).toContain("Actual file content at those lines:"); + expect(message).toContain("4:l4"); + expect(fs.get(PATH)).toBe(CONTENT); + + // The revealed line joins the snapshot's seen set, so a straight retry + // with the same [path#tag] header applies — no extra read required. + const result = await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 4.=4:\n+L4`)); + expect(result.sections[0]?.op).toBe("update"); + expect(fs.get(PATH)).toBe("l1\nl2\nl3\nL4\nl5\n"); + }); + + it("truncates the reveal at the cap and directs the tail back to a range re-read", async () => { + const bigContent = `${Array.from({ length: 200 }, (_, i) => `l${i + 1}`).join("\n")}\n`; + const fs = new InMemoryFilesystem([[PATH, bigContent]]); + const snapshots = new InMemorySnapshotStore(); + const tag = snapshots.record(PATH, bigContent, [1]); + const patcher = new Patcher({ fs, snapshots }); + + // Anchor 60 unseen lines — over the 40-line inline reveal cap. + const dels = Array.from({ length: 60 }, (_, i) => `DEL ${100 + i}`).join("\n"); + let message: string | undefined; + try { + await patcher.apply(Patch.parse(`[${PATH}#${tag}]\n${dels}`)); + } catch (err) { + message = (err as Error).message; + } + expect(message).toContain("Preview of the actual file content at the first 40 unseen line(s)"); + expect(message).toContain("100:l100"); + expect(message).toContain("139:l139"); + expect(message).not.toContain("140:l140"); + expect(message).toMatch(new RegExp(`${PATH}:100-159`)); + expect(fs.get(PATH)).toBe(bigContent); + }); + it("skips the check when no seen lines were recorded (absent → allow)", async () => { const fs = new InMemoryFilesystem([[PATH, CONTENT]]); const snapshots = new InMemorySnapshotStore(); From f42fa38f7cdaec949c5a2831c238b97adbbe07c4 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 08:11:16 +0000 Subject: [PATCH 2/5] fix(hashline): do not merge revealed seen-lines on the truncated path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex reviewer flagged that when an anchor range exceeds `SEEN_LINE_REVEAL_CAP`, merging the revealed prefix into `seenLines` lets a model split a blind over-cap edit into two <=cap-line retries and slip past the range-re-read gate: attempt 1 reveals+merges lines 100-139, attempt 2 reveals+merges the 140-159 tail, attempt 3 applies — all without a single range read. The merge is now gated on `truncated === false`: only a reveal that covered EVERY unseen anchor line joins `seenLines`. Truncated reveals keep the range-re-read guidance and the reveal window stays anchored at the head across retries, so the same over-cap patch keeps rejecting until the model actually re-reads the range. --- packages/hashline/src/patcher.ts | 27 +++++++++++++++++--------- packages/hashline/test/patcher.test.ts | 17 ++++++++++++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index 5df8cfeda..e6291e9ee 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -500,12 +500,16 @@ export class Patcher { * * The rejection inlines the actual file content at the unseen anchor lines * (from `matchedSnapshot.text`, which by definition equals the live - * normalized content) and merges those lines into the snapshot's - * seen-line set, so a straight retry with the same `[path#tag]` header - * succeeds without a follow-up range read — the content the model - * receives in the error IS proof it has now seen those lines. Ranges - * beyond {@link SEEN_LINE_REVEAL_CAP} still bounce back to a range - * re-read for the remainder; only the revealed prefix is merged. + * normalized content) so the model can verify what it was about to touch. + * When the reveal covers EVERY unseen anchor line (`truncated === false`) + * those lines also merge into the snapshot's seen-line set, so a straight + * retry with the same `[path#tag]` header succeeds without a follow-up + * range read — the content the model received in the error IS proof it + * has now seen those lines. When the anchor range exceeds + * {@link SEEN_LINE_REVEAL_CAP} (`truncated === true`), NO lines merge: + * the message keeps the range-re-read guidance intact and the model + * cannot piecewise-reveal its way past the guard across multiple retries + * (over-cap retry → tail reveal → next retry applies). */ #assertSeenLines(section: PatchSection, expected: string, matchedSnapshot: Snapshot | null): void { const seen = matchedSnapshot?.seenLines; @@ -518,13 +522,18 @@ export class Patcher { for (let i = 0; i < revealCount; i++) { const line = unseen[i]; // Out-of-range anchors are caught by parse/apply with a better - // message; skip them here so they never join the revealed set or - // the seen-line merge. + // message; skip them here so they never join the revealed set. if (line < 1 || line > sourceLines.length) continue; revealed.push({ line, text: sourceLines[line - 1] ?? "" }); } const truncated = unseen.length > revealed.length; - for (const { line } of revealed) seen.add(line); + // Only merge when the reveal covered every unseen anchor line: a + // truncated reveal that merged its prefix would let the model split a + // blind edit into <=SEEN_LINE_REVEAL_CAP-line retries and land it + // without ever running the required range re-read. + if (!truncated) { + for (const { line } of revealed) seen.add(line); + } throw new Error(unseenLinesMessage(section.path, unseen, expected, { lines: revealed, truncated })); } #mismatchError( diff --git a/packages/hashline/test/patcher.test.ts b/packages/hashline/test/patcher.test.ts index f0e0f46c7..70b7f9c2d 100644 --- a/packages/hashline/test/patcher.test.ts +++ b/packages/hashline/test/patcher.test.ts @@ -352,6 +352,23 @@ describe("Patcher seen-line provenance", () => { expect(message).not.toContain("140:l140"); expect(message).toMatch(new RegExp(`${PATH}:100-159`)); expect(fs.get(PATH)).toBe(bigContent); + + // A straight retry of the same over-cap patch STILL rejects: the + // truncated reveal must not merge its prefix into seenLines, or the + // model could split a blind over-cap edit into <=cap-line retries and + // slip past the range-re-read gate. The reveal window stays anchored + // at the head (100..139), never advancing to the tail across retries. + let retryMessage: string | undefined; + try { + await patcher.apply(Patch.parse(`[${PATH}#${tag}]\n${dels}`)); + } catch (err) { + retryMessage = (err as Error).message; + } + expect(retryMessage).toContain("Preview of the actual file content at the first 40 unseen line(s)"); + expect(retryMessage).toContain("100:l100"); + expect(retryMessage).toContain("139:l139"); + expect(retryMessage).not.toContain("140:l140"); + expect(fs.get(PATH)).toBe(bigContent); }); it("skips the check when no seen lines were recorded (absent → allow)", async () => { From 367a002e481106b627b825901482a5cfa369137f Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 08:20:39 +0000 Subject: [PATCH 3/5] fix(hashline): column-cap revealed anchor lines and gate merge on width truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex reviewer flagged that the per-reveal cap only limits the line count, not each line's width. A minified-bundle-style wide line (megabytes on one row) would be stored verbatim in the RevealedLine and formatted straight into the thrown edit error — bypassing the column-truncation read/search already apply to long lines and dumping that much content into the tool result, TUI, and model context. assertSeenLines now clips each revealed line at SEEN_LINE_REVEAL_MAX_COLUMNS (512, matching search's DEFAULT_MAX_COLUMN) with a trailing ellipsis marker and treats any clip as truncated. The existing truncated-gated merge keeps the guard closed on clipped reveals so the model cannot land an edit having only seen the first 512 chars of a wide line, and the message keeps the range-re-read guidance. --- packages/hashline/src/patcher.ts | 51 ++++++++++++++++++-------- packages/hashline/test/patcher.test.ts | 43 ++++++++++++++++++++++ 2 files changed, 79 insertions(+), 15 deletions(-) diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index e6291e9ee..eceee2250 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -52,6 +52,16 @@ import type { ApplyResult, BlockResolution, BlockResolver, Edit, FileOp } from " */ const SEEN_LINE_REVEAL_CAP = 40; +/** + * Per-revealed-line character cap. Matches the read/search column cap so a + * revealed anchor line can never dump a minified megabyte-wide bundle line + * into the tool error, TUI, and model context. Lines longer than the cap + * are trimmed to `cap` characters plus an `…` marker AND flag the entire + * reveal as truncated so no line joins `seenLines` — the model must re-read + * the range to prove it saw the full width. + */ +const SEEN_LINE_REVEAL_MAX_COLUMNS = 512; + export interface PatcherOptions { /** Storage backend used for all reads and writes. */ fs: Filesystem; @@ -501,15 +511,18 @@ export class Patcher { * The rejection inlines the actual file content at the unseen anchor lines * (from `matchedSnapshot.text`, which by definition equals the live * normalized content) so the model can verify what it was about to touch. - * When the reveal covers EVERY unseen anchor line (`truncated === false`) - * those lines also merge into the snapshot's seen-line set, so a straight - * retry with the same `[path#tag]` header succeeds without a follow-up - * range read — the content the model received in the error IS proof it - * has now seen those lines. When the anchor range exceeds - * {@link SEEN_LINE_REVEAL_CAP} (`truncated === true`), NO lines merge: - * the message keeps the range-re-read guidance intact and the model - * cannot piecewise-reveal its way past the guard across multiple retries - * (over-cap retry → tail reveal → next retry applies). + * When the reveal covers EVERY unseen anchor line in full width + * (`truncated === false`) those lines also merge into the snapshot's + * seen-line set, so a straight retry with the same `[path#tag]` header + * succeeds without a follow-up range read — the content the model + * received in the error IS proof it has now seen those lines. When the + * anchor range exceeds {@link SEEN_LINE_REVEAL_CAP} lines OR any + * revealed line exceeds {@link SEEN_LINE_REVEAL_MAX_COLUMNS} characters + * (`truncated === true`), NO lines merge: the message keeps the + * range-re-read guidance intact and the model cannot piecewise-reveal + * its way past the guard across multiple retries + * (over-cap retry → tail reveal → next retry applies), nor coax the tool + * into dumping a minified megabyte-wide line into the error preview. */ #assertSeenLines(section: PatchSection, expected: string, matchedSnapshot: Snapshot | null): void { const seen = matchedSnapshot?.seenLines; @@ -519,18 +532,26 @@ export class Patcher { const sourceLines = matchedSnapshot?.text.split("\n") ?? []; const revealed: RevealedLine[] = []; const revealCount = Math.min(unseen.length, SEEN_LINE_REVEAL_CAP); + let columnTruncated = false; for (let i = 0; i < revealCount; i++) { const line = unseen[i]; // Out-of-range anchors are caught by parse/apply with a better // message; skip them here so they never join the revealed set. if (line < 1 || line > sourceLines.length) continue; - revealed.push({ line, text: sourceLines[line - 1] ?? "" }); + const source = sourceLines[line - 1] ?? ""; + if (source.length > SEEN_LINE_REVEAL_MAX_COLUMNS) { + revealed.push({ line, text: `${source.slice(0, SEEN_LINE_REVEAL_MAX_COLUMNS)}…` }); + columnTruncated = true; + } else { + revealed.push({ line, text: source }); + } } - const truncated = unseen.length > revealed.length; - // Only merge when the reveal covered every unseen anchor line: a - // truncated reveal that merged its prefix would let the model split a - // blind edit into <=SEEN_LINE_REVEAL_CAP-line retries and land it - // without ever running the required range re-read. + const truncated = unseen.length > revealed.length || columnTruncated; + // Only merge when the reveal covered every unseen anchor line in full + // width. A prefix-truncated reveal would let the model split a blind + // edit into <=cap-line retries and land it without ever running the + // required range re-read; a column-clipped reveal would leave part of + // each line unseen while the model receives an "ok to retry" signal. if (!truncated) { for (const { line } of revealed) seen.add(line); } diff --git a/packages/hashline/test/patcher.test.ts b/packages/hashline/test/patcher.test.ts index 70b7f9c2d..79ecebba3 100644 --- a/packages/hashline/test/patcher.test.ts +++ b/packages/hashline/test/patcher.test.ts @@ -371,6 +371,49 @@ describe("Patcher seen-line provenance", () => { expect(fs.get(PATH)).toBe(bigContent); }); + it("column-clips wide revealed lines, keeps the merge gate closed, and stays anchored across retries", async () => { + // Minified-bundle-style single wide line at anchor 2. Anchor 3 is a + // short line so we can see the width truncation applied only where + // needed. The 4KB cap is comfortably over SEEN_LINE_REVEAL_MAX_COLUMNS. + const wide = "a".repeat(4096); + const wideContent = `l1\n${wide}\nl3\nl4\n`; + const fs = new InMemoryFilesystem([[PATH, wideContent]]); + const snapshots = new InMemorySnapshotStore(); + const tag = snapshots.record(PATH, wideContent, [1]); + const patcher = new Patcher({ fs, snapshots }); + + let message: string | undefined; + try { + await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 2.=3:\n+X\n+Y`)); + } catch (err) { + message = (err as Error).message; + } + expect(message).toContain("Preview of the actual file content at the first 2 unseen line(s)"); + // Line 2 is clipped at 512 chars + ellipsis; the full 4KB never leaks + // into the error preview. + expect(message).toMatch(/2:a{512}…/); + expect(message).not.toContain("a".repeat(513)); + // Short line surfaces verbatim. + expect(message).toContain("3:l3"); + // Guidance routes to a range re-read. + expect(message).toMatch(new RegExp(`${PATH}:2-3`)); + expect(fs.get(PATH)).toBe(wideContent); + + // A straight retry STILL rejects: column-truncated reveals must not + // merge into seenLines, otherwise the model would land the edit + // having only seen the first 512 chars of a >4KB line. + let retryMessage: string | undefined; + try { + await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 2.=3:\n+X\n+Y`)); + } catch (err) { + retryMessage = (err as Error).message; + } + expect(retryMessage).toContain("Preview of the actual file content at the first 2 unseen line(s)"); + expect(retryMessage).toMatch(/2:a{512}…/); + expect(retryMessage).not.toContain("a".repeat(513)); + expect(fs.get(PATH)).toBe(wideContent); + }); + it("skips the check when no seen lines were recorded (absent → allow)", async () => { const fs = new InMemoryFilesystem([[PATH, CONTENT]]); const snapshots = new InMemorySnapshotStore(); From b38eba2be771c85b20d0376b222c6b43def53bc9 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 08:53:18 +0000 Subject: [PATCH 4/5] fix(coding-agent): exclude column-clipped lines from seen-line provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex reviewer flagged that the ranged-read fallback recommended by the patcher's over-cap reveal path (`path:N-M`) itself applies the 512-column cap and still records the displayed line numbers via `recordSeenLinesFromBody`. On a minified wide anchor line, `read file:N` shows only the clipped prefix but the line lands in the tag's seenLines — a subsequent edit anchored at N then slips past the seen-line guard. - `packages/coding-agent/src/edit/file-snapshot-store.ts`: `recordSeenLinesFromBody` grows an optional `excludedLines` set; parsed line numbers matching it are filtered before recording. - `packages/coding-agent/src/tools/read.ts`: `#readLocalFileMultiRange` and the single-range disk path build a `clippedLines` set alongside `columnTruncated` for both direct-range lines and `buildLineEntriesWithBlockContext` context lines, then pass it into `recordSeenLinesFromBody`. - `packages/coding-agent/src/tools/grep.ts`: same wiring for the match line via `match.truncated`, plus a conservative length+`...`-marker heuristic for context lines (native `crates/pi-natives/src/grep.rs` `truncate_line` doesn't propagate a per-line flag on `contextBefore`/`contextAfter`; a proper native-side flag is a follow-up). - `packages/coding-agent/test/edit/seen-line-guard.test.ts`: new case reads a 4KB single line and asserts the clipped line number stays out of `seenLines` and the edit against it still rejects with the seen-line guard. --- .../src/edit/file-snapshot-store.ts | 14 +++++++++- packages/coding-agent/src/tools/grep.ts | 16 +++++++++++- packages/coding-agent/src/tools/read.ts | 18 ++++++++++--- .../test/edit/seen-line-guard.test.ts | 26 +++++++++++++++++++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/coding-agent/src/edit/file-snapshot-store.ts b/packages/coding-agent/src/edit/file-snapshot-store.ts index 3428cc2bd..aaadc3bf2 100644 --- a/packages/coding-agent/src/edit/file-snapshot-store.ts +++ b/packages/coding-agent/src/edit/file-snapshot-store.ts @@ -132,12 +132,24 @@ export function recordSeenLines( * can reject edits anchored on lines the model never saw. Best-effort: a no-op * when the body has no numbered rows or the snapshot already aged out. `tag` * must be the tag returned when this exact content was recorded. + * + * `excludedLines` prunes 1-indexed line numbers whose displayed text was + * column-truncated (or otherwise not shown in full). A column-clipped row + * still carries a `NN:` prefix — the parser sees the number and would + * otherwise mark the line "seen" even though only its prefix ever reached + * the model. Producers that apply per-line column truncation MUST supply + * the clipped line set so the patcher's seen-line guard keeps rejecting + * edits against those lines until a full-width read of them occurs. */ export function recordSeenLinesFromBody( session: FileSnapshotStoreOwner, absolutePath: string, tag: string, body: string, + excludedLines?: ReadonlySet, ): void { - recordSeenLines(session, absolutePath, tag, parseSeenLinesFromHashlineBody(body)); + const parsed = parseSeenLinesFromHashlineBody(body); + const filtered = + excludedLines && excludedLines.size > 0 ? parsed.filter(line => !excludedLines.has(line)) : parsed; + recordSeenLines(session, absolutePath, tag, filtered); } diff --git a/packages/coding-agent/src/tools/grep.ts b/packages/coding-agent/src/tools/grep.ts index 23e4e1daf..6ad5804a5 100644 --- a/packages/coding-agent/src/tools/grep.ts +++ b/packages/coding-agent/src/tools/grep.ts @@ -1371,6 +1371,17 @@ export class GrepTool implements AgentTool }, 0); let lastEmittedLine: number | undefined; const gutterPad = " ".repeat(lineNumberWidth + 1); + // Track match/context lines whose displayed text was + // column-truncated by the native (see `crates/pi-natives/src/grep.rs` + // `truncate_line`, marker `...` at max_columns). Excluded from + // seenLines so a follow-up edit anchored at that line still + // requires a full-width re-read — the model saw only the + // prefix. The native currently propagates `truncated` only on + // the match line; context lines fall back to a length check + // against `DEFAULT_MAX_COLUMN` as a conservative heuristic. + const clippedLines = new Set(); + const isNativeTruncated = (line: string): boolean => + line.length >= DEFAULT_MAX_COLUMN && line.endsWith("..."); for (const match of fileMatches) { const pushLine = (lineNumber: number, line: string, isMatch: boolean) => { if (lastEmittedLine !== undefined && lineNumber > lastEmittedLine + 1) { @@ -1384,22 +1395,25 @@ export class GrepTool implements AgentTool if (match.contextBefore) { for (const ctx of match.contextBefore) { pushLine(ctx.lineNumber, ctx.line, false); + if (isNativeTruncated(ctx.line)) clippedLines.add(ctx.lineNumber); } } pushLine(match.lineNumber, match.line, true); if (match.truncated) { linesTruncated = true; + clippedLines.add(match.lineNumber); } if (match.contextAfter) { for (const ctx of match.contextAfter) { pushLine(ctx.lineNumber, ctx.line, false); + if (isNativeTruncated(ctx.line)) clippedLines.add(ctx.lineNumber); } } fileMatchCounts.set(relativePath, (fileMatchCounts.get(relativePath) ?? 0) + 1); } if (hashContext?.tag) { const absoluteFilePath = path.resolve(this.session.cwd, relativePath); - recordSeenLinesFromBody(this.session, absoluteFilePath, hashContext.tag, modelOut.join("\n")); + recordSeenLinesFromBody(this.session, absoluteFilePath, hashContext.tag, modelOut.join("\n"), clippedLines); } return { model: modelOut, display: displayOut }; }; diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index df93edd7e..b293c3543 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -1552,6 +1552,7 @@ export class ReadTool implements AgentTool { const displayLineByNumber = new Map(); const fullLines = rawSelector ? undefined : await readBracketContextFullLines(absolutePath, fileSize); let columnTruncated = 0; + const clippedLines = new Set(); let displayContent: { text: string; startLine: number; lineNumbers?: Array } | undefined; for (const range of ranges) { @@ -1599,6 +1600,7 @@ export class ReadTool implements AgentTool { if (!cloned) cloned = collectedLines.slice(); cloned[i] = text; columnTruncated = maxColumns; + clippedLines.add(range.startLine + i); } } if (cloned) displayLines = cloned; @@ -1626,7 +1628,10 @@ export class ReadTool implements AgentTool { if (visibleText !== undefined) return visibleText; if (maxColumns <= 0) return sourceText; const truncated = truncateLine(sourceText, maxColumns); - if (truncated.wasTruncated) columnTruncated = maxColumns; + if (truncated.wasTruncated) { + columnTruncated = maxColumns; + clippedLines.add(lineNumber); + } return truncated.text; }, }, @@ -1644,7 +1649,7 @@ export class ReadTool implements AgentTool { if (shouldAddHashLines && outputText) { const tag = await recordFileSnapshot(this.session, absolutePath); if (tag) { - recordSeenLinesFromBody(this.session, absolutePath, tag, outputText); + recordSeenLinesFromBody(this.session, absolutePath, tag, outputText, clippedLines); outputText = `${formatReadHashlineHeader(formatPathRelativeToCwd(absolutePath, this.session.cwd), tag)}\n${outputText}`; } } @@ -2483,6 +2488,7 @@ export class ReadTool implements AgentTool { // ellipsis-truncated text made every long-line file uneditable on // the next edit attempt. let displayLines: string[] = collectedLines; + const clippedLines = new Set(); if (!rawSelector && maxColumns > 0) { let cloned: string[] | undefined; for (let i = 0; i < collectedLines.length; i++) { @@ -2491,6 +2497,7 @@ export class ReadTool implements AgentTool { if (!cloned) cloned = collectedLines.slice(); cloned[i] = text; columnTruncated = maxColumns; + clippedLines.add(startLineDisplay + i); } } if (cloned) displayLines = cloned; @@ -2573,7 +2580,10 @@ export class ReadTool implements AgentTool { if (visibleText !== undefined) return visibleText; if (maxColumns <= 0) return sourceText; const truncated = truncateLine(sourceText, maxColumns); - if (truncated.wasTruncated) columnTruncated = maxColumns; + if (truncated.wasTruncated) { + columnTruncated = maxColumns; + clippedLines.add(lineNumber); + } return truncated.text; }, }, @@ -2647,7 +2657,7 @@ export class ReadTool implements AgentTool { } if (hashContext?.tag) { - recordSeenLinesFromBody(this.session, absolutePath, hashContext.tag, outputText); + recordSeenLinesFromBody(this.session, absolutePath, hashContext.tag, outputText, clippedLines); } if (capturedDisplayContent) { diff --git a/packages/coding-agent/test/edit/seen-line-guard.test.ts b/packages/coding-agent/test/edit/seen-line-guard.test.ts index 55a596cf2..22a2bf8cc 100644 --- a/packages/coding-agent/test/edit/seen-line-guard.test.ts +++ b/packages/coding-agent/test/edit/seen-line-guard.test.ts @@ -229,6 +229,32 @@ describe("read → edit seen-line guard", () => { expect(message).toMatch(/long\.txt:100-159/); expect(await Bun.file(file).text()).toBe(`${lines.join("\n")}\n`); }); + + it("does not mark column-clipped read lines as seen", async () => { + // A 4KB single line — the read tool's column cap (default 512 chars) + // clips this into `…` in the numbered output. The clipped line + // number MUST stay out of the tag's seenLines, or a subsequent edit + // anchored there would slip past the seen-line guard having seen only + // the first 512 chars. + const file = path.join(tmpDir, "wide.txt"); + const wide = "a".repeat(4096); + const content = `head\n${wide}\nfoot\n`; + await Bun.write(file, content); + const session = createSession(tmpDir); + + const read = await new ReadTool(session).execute("r1", { path: `${file}:2` }); + const tag = tagFromOutput(resultText(read)); + + const seen = getFileSnapshotStore(session).byHash(canonicalSnapshotKey(file), tag)?.seenLines; + expect(seen?.has(2)).toBe(false); + + // A straight edit anchored at the clipped line 2 is still rejected — + // the seen-line guard fires because the model only saw the prefix. + await expect( + executeHashlineSingle(execOptions(`[wide.txt#${tag}]\nSWAP 2.=2:\n+REPLACED`, session)), + ).rejects.toThrow(/never displayed \(it showed/); + expect(await Bun.file(file).text()).toBe(content); + }); }); describe("search → edit seen-line guard", () => { From fd6f7332904ae0623d5db5375659bb32dff7dc2b Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 08:53:25 +0000 Subject: [PATCH 5/5] style: bun run fix --- packages/coding-agent/src/edit/file-snapshot-store.ts | 3 +-- packages/coding-agent/src/tools/grep.ts | 8 +++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/src/edit/file-snapshot-store.ts b/packages/coding-agent/src/edit/file-snapshot-store.ts index aaadc3bf2..08fe6767f 100644 --- a/packages/coding-agent/src/edit/file-snapshot-store.ts +++ b/packages/coding-agent/src/edit/file-snapshot-store.ts @@ -149,7 +149,6 @@ export function recordSeenLinesFromBody( excludedLines?: ReadonlySet, ): void { const parsed = parseSeenLinesFromHashlineBody(body); - const filtered = - excludedLines && excludedLines.size > 0 ? parsed.filter(line => !excludedLines.has(line)) : parsed; + const filtered = excludedLines && excludedLines.size > 0 ? parsed.filter(line => !excludedLines.has(line)) : parsed; recordSeenLines(session, absolutePath, tag, filtered); } diff --git a/packages/coding-agent/src/tools/grep.ts b/packages/coding-agent/src/tools/grep.ts index 6ad5804a5..5c180e9d9 100644 --- a/packages/coding-agent/src/tools/grep.ts +++ b/packages/coding-agent/src/tools/grep.ts @@ -1413,7 +1413,13 @@ export class GrepTool implements AgentTool } if (hashContext?.tag) { const absoluteFilePath = path.resolve(this.session.cwd, relativePath); - recordSeenLinesFromBody(this.session, absoluteFilePath, hashContext.tag, modelOut.join("\n"), clippedLines); + recordSeenLinesFromBody( + this.session, + absoluteFilePath, + hashContext.tag, + modelOut.join("\n"), + clippedLines, + ); } return { model: modelOut, display: displayOut }; };