smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(coding-agent): removed mid-skip placeholder from diff output

Browse Source 
- Removed the mid-skip placeholder line that was output when skipping lines in the middle of a diff, relying instead on the line number jump to convey the gap. Removed the corresponding placeholder-filtering logic from the hashline diff preview that was handling these placeholders.
This commit is contained in:
can1357
2026-06-08 13:34:41 +02:00
parent d0d5b547ff
commit 2efd667916
8 changed files with 33 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -18,6 +18,7 @@
- Fixed follow-up handling to clear consumed clipboard image state after submission so pasted images are not silently carried into later messages
- Fixed clipboard-pasted images being rejected when steering or following up during compaction. Instead of bailing with "Retry after it completes to send images", the message and its images are now queued via `queueCompactionMessage` and forwarded to the session (steer/follow-up/prompt) when the compaction queue flushes.
- Fixed edit tool result previews to show only current-file lines and collapse long inserted blocks instead of echoing removed content.
- Fixed `generateDiffString` to omit the mid-skip `...` placeholder between two nearby edits, conveying the elided gap via the jump in line numbers instead (consistent with how leading/trailing context skips already render). The placeholder row was indistinguishable from a genuine `...` context line and wasted a row in compact previews.
## [15.10.4] - 2026-06-08
packages/coding-agent/src/edit/diff.ts
+3 -1
View File
@@ -133,8 +133,10 @@ export function generateDiffString(oldContent: string, newContent: string, conte
newLineNum++;
}
// Mid-skip placeholder is omitted too: the jump between the trailing
// number of the leading context and the leading number of the
// trailing context conveys the gap, just like leading/trailing skips.
if (middleSkip > 0) {
output.push(formatNumberedDiffLine(" ", oldLineNum, "..."));
oldLineNum += middleSkip;
newLineNum += middleSkip;
for (const line of linesToShow.slice(firstChunkLength)) {
packages/coding-agent/test/core/hashline.test.ts
+2 -19
View File
@@ -3,8 +3,8 @@ import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import {
type InMemorySnapshotStore as FileReadCache,
formatHashlineHeader,
InMemorySnapshotStore as FileReadCache,
MismatchError as HashlineMismatchError,
} from "@oh-my-pi/hashline";
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
@@ -15,9 +15,8 @@ import {
getFileSnapshotStore as getFileReadCache,
hashlineEditParamsSchema,
} from "@oh-my-pi/pi-coding-agent/edit";
import * as z from "zod/v4";
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
import * as z from "zod/v4";
beforeAll(async () => {
resetSettingsForTest();
@@ -49,7 +48,6 @@ function sameLineRange(anchor: string): string {
return `replace ${anchor}..${anchor}:`;
}
async function withTempDir(fn: (tempDir: string) => Promise<void>): Promise<void> {
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "hashline-edit-"));
try {
@@ -84,12 +82,6 @@ function hashlineExecuteOptions(
};
}
describe("hashline executor", () => {
it("rejects file creation and directs to the write tool", async () => {
await withTempDir(async tempDir => {
@@ -116,7 +108,6 @@ describe("hashline executor", () => {
});
});
it("emits an actionable no-op diagnostic when the payload matches the file byte-for-byte", async () => {
await withTempDir(async tempDir => {
const filePath = path.join(tempDir, "a.ts");
@@ -274,7 +265,6 @@ describe("hashlineEditParamsSchema — payload shape", () => {
});
});
describe("hashline — anchor-stale recovery via read snapshot cache", () => {
it("recovers when the file was modified out-of-band after a read", async () => {
await withTempDir(async tempDir => {
@@ -338,8 +328,6 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => {
});
});
it("captures the post-edit result so the next edit can recover from anchors against it", async () => {
await withTempDir(async tempDir => {
const filePath = path.join(tempDir, "a.ts");
@@ -411,9 +399,4 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => {
expect(await Bun.file(filePath).text()).toBe(`${v1Lines.join("\n")}\n`);
});
});
});
packages/coding-agent/test/tools/edit-diff.test.ts
+4 -1
View File
@@ -11,7 +11,10 @@ describe("generateDiffString", () => {
const result = generateDiffString(oldLines.join("\n"), newLines.join("\n"), 2);
const diffLines = result.diff.split("\n");
expect(diffLines).toContain(" 5|...");
// The mid-skip emits no placeholder row; the jump from the leading
// context (line 4) to the trailing context (line 16) conveys the gap.
expect(diffLines.some(line => line.endsWith("|...") || line.endsWith("|…"))).toBe(false);
expect(diffLines[diffLines.indexOf(" 4|line 4") + 1]).toBe(" 16|line 16");
expect(diffLines).toContain("-2|line 2");
expect(diffLines).toContain("+2|line 2 changed");
expect(diffLines).toContain("-18|line 18");
packages/hashline/CHANGELOG.md
-1
View File
@@ -14,7 +14,6 @@
### Fixed
- Fixed compact edit previews to omit deleted content, keep visible lines anchored to the current file, and collapse long inserted runs with a `+N…` elision marker.
- Fixed `buildCompactDiffPreview` to drop context-gap placeholder rows (`...`/`…`) from the model-facing preview, relying on the jump in emitted line numbers to convey the elision. The placeholder was byte-identical to a genuine context line whose text is `...`, and wasted a row; the diff producer still emits the marker for the human-facing TUI diff.
## [15.10.3] - 2026-06-08
packages/hashline/src/diff-preview.ts
-6
View File
@@ -96,12 +96,6 @@ export function buildCompactDiffPreview(diff: string, options: CompactDiffOption
break;
default: {
flushAddedRun();
// Context-gap placeholders (`...`/`…`) carry no real content and are
// byte-identical to a genuine context line whose text is "...". Drop
// them from the model-facing preview; the jump in emitted line
// numbers conveys the elision (matching how the diff producer already
// omits leading/trailing skips).
if (parsed.content === "..." || parsed.content === "…") break;
const newLineNumber = parsed.lineNumber + addedLines - removedLines;
formatted.push(` ${newLineNumber}:${parsed.content}`);
break;
packages/hashline/test/core-contracts.test.ts
+23 -9
View File
@@ -2,15 +2,15 @@ import { describe, expect, it } from "bun:test";
import {
applyEdits,
detectLineEnding,
type Edit,
formatHashlineHeader,
InMemoryFilesystem,
InMemorySnapshotStore,
Patch,
Patcher,
type PatchSection,
parsePatch,
Recovery,
type Edit,
type PatchSection,
type SplitOptions,
} from "@oh-my-pi/hashline";
@@ -117,9 +117,9 @@ describe("hashline parser — range-anchor contracts", () => {
it("preserves whitespace-bearing and sigil-leading payload exactly", () => {
const payload = "\tconst streamKeepaliveMs = opts.streamKeepaliveMs;";
expect(applyDiff(content, `insert after ${tag(2)}:\n${repl(payload)}`)).toBe(`aaa\nbbb\n${payload}\nccc`);
expect(applyDiff(content, `${sameLineRange(tag(2))}\n${repl("|literal")}\n${repl("^literal")}\n${repl("↓literal")}`)).toBe(
"aaa\n|literal\n^literal\n↓literal\nccc",
);
expect(
applyDiff(content, `${sameLineRange(tag(2))}\n${repl("|literal")}\n${repl("^literal")}\n${repl("↓literal")}`),
).toBe("aaa\n|literal\n^literal\n↓literal\nccc");
});
it("strips copied read-output prefixes only inside pasted bare body rows", () => {
@@ -259,7 +259,9 @@ describe("hashline abort sentinel", () => {
const sentinel = "*** Abort";
it("terminates parsing without surfacing a warning", () => {
const diff = [`insert after ${tag(1)}:`, repl("HELLO"), sentinel, `insert after ${tag(99)}:`, repl("never")].join("\n");
const diff = [`insert after ${tag(1)}:`, repl("HELLO"), sentinel, `insert after ${tag(99)}:`, repl("never")].join(
"\n",
);
const { edits, warnings } = parsePatch(diff);
expect(edits).toHaveLength(1);
expect(edits[0]).toMatchObject({ kind: "insert", text: "HELLO" });
@@ -267,7 +269,15 @@ describe("hashline abort sentinel", () => {
});
it("stops the input splitter before later sections", () => {
const input = ["[a.ts]", `insert after ${tag(1)}:`, repl("a-payload"), sentinel, "[b.ts]", `insert after ${tag(1)}:`, repl("never")].join("\n");
const input = [
"[a.ts]",
`insert after ${tag(1)}:`,
repl("a-payload"),
sentinel,
"[b.ts]",
`insert after ${tag(1)}:`,
repl("never"),
].join("\n");
const sections = splitHashlineInputs(input);
expect(sections).toHaveLength(1);
expect(sections[0].path).toBe("a.ts");
@@ -278,8 +288,12 @@ describe("hashline abort sentinel", () => {
describe("hashline parser — delete and blank payload semantics", () => {
it("applies inline delete and empty replace operations", () => {
expect(applyDiff("line1\nline2\nline3\n", splitHashlineInput("[a.ts]\ndelete 2\n").diff)).toBe("line1\nline3\n");
expect(applyDiff("line1\nline2\nline3\nline4\n", splitHashlineInput("[a.ts]\ndelete 2..3\n").diff)).toBe("line1\nline4\n");
expect(applyDiff("line1\nline2\nline3\n", splitHashlineInput("[a.ts]\nreplace 2..2:\n").diff)).toBe("line1\nline3\n");
expect(applyDiff("line1\nline2\nline3\nline4\n", splitHashlineInput("[a.ts]\ndelete 2..3\n").diff)).toBe(
"line1\nline4\n",
);
expect(applyDiff("line1\nline2\nline3\n", splitHashlineInput("[a.ts]\nreplace 2..2:\n").diff)).toBe(
"line1\nline3\n",
);
});
it("treats old inline replacement syntax as orphan body", () => {
packages/hashline/test/diff-preview.test.ts
-9
View File
@@ -46,13 +46,4 @@ describe("buildCompactDiffPreview", () => {
expect(preview.addedLines).toBe(7);
expect(preview.removedLines).toBe(0);
});
it("drops context-gap placeholders and conveys elision via the line-number jump", () => {
const preview = buildCompactDiffPreview(
[" 2|line 2", " 3|line 3", " 4|...", " 49|line 49", "+50|inserted"].join("\n"),
);
expect(preview.preview).toBe([" 2:line 2", " 3:line 3", " 49:line 49", "+50:inserted"].join("\n"));
expect(preview.addedLines).toBe(1);
});
});