feat(coding-agent-tools): implemented auto-trimming for echo lines

- Implemented logic to automatically detect and trim redundant lines duplicating adjacent file content within conflict markers.
- Added delimiter balancing and boundary tracking to ensure accurate removal of echoed text while preserving EOL formatting.
- Updated user feedback to report the number of trimmed echo lines during write and conflict resolution operations.
- Expanded test coverage to include multi-line echo scenarios and integration validation of the repair process.
This commit is contained in:
can1357
2026-07-11 17:04:23 +02:00
parent f47fd93004
commit 7e5e7e864d
5 changed files with 308 additions and 12 deletions
+1
View File
@@ -29,6 +29,7 @@
### Fixed
- Fixed `write conflict://<N>` duplicating code when the model pastes the "whole resolved function" including lines adjacent to the marker block: replacement lines that exactly echo the context directly above/below the recorded region are now dropped (multi-line echoes always; single-line echoes only when removal restores the recorded sides' delimiter balance), with a note in the tool result. The conflict footer now also states that writes replace only the marker block and to prefer the minimal merge of the recorded sides.
- Fixed visible per-keystroke lag while searching in the `/resume` session picker. Literal matches now rank synchronously from a cached per-session haystack, fuzzy scoring runs in bounded background chunks that converge to the same ranking (large listings previously rebuilt a fuzzy index per token per session on every keystroke), and the prompt-history SQLite lookup — an FTS query plus a LIKE scan over every stored prompt — is debounced off the keystroke path.
- Fixed compiled Linux binary extension loading when bundled web-search header generation cannot read `header-generator` data files from the build-time path. ([#5178](https://github.com/can1357/oh-my-pi/issues/5178))
- Fixed `job` `list`/empty-poll snapshots returning empty output: a no-job result now says so explicitly, and both snapshots list running subagents that have no backing job (agents woken via `irc`, spawns owned by another agent) with an `irc` coordination hint, so the tool's picture matches the UI's running-agent badge. Polling an agent id that has no job now explains the agent's registry state instead of a bare "no matching jobs".
@@ -319,6 +319,15 @@ export function parseConflictUri(raw: string): ParsedConflictUri | null {
return recoveredPrefix !== undefined ? { id, scope, recoveredPrefix } : { id, scope };
}
/** Result of {@link spliceConflict}: the new file text plus any boundary-echo repair applied. */
export interface ConflictSplice {
text: string;
/** Replacement lines dropped because they duplicated the context directly above the region. */
trimmedLeading: number;
/** Replacement lines dropped because they duplicated the context directly below the region. */
trimmedTrailing: number;
}
/**
* Splice the conflict region recorded in `entry` out of `originalText`
* and replace it with `replacement` (markers and all sides included).
@@ -328,8 +337,16 @@ export function parseConflictUri(raw: string): ParsedConflictUri | null {
* match), so out-of-band edits earlier in the file that shift line
* numbers don't break resolution. Throws clearly when the marker block
* has actually been altered or removed.
*
* Boundary-echo repair (same philosophy as the edit tool's hashline
* keeper repair): models frequently paste the "whole resolved function"
* including the lines that live directly before/after the marker block,
* which the verbatim splice would duplicate. Replacement lines that
* exactly echo the adjacent context are dropped when the echo is
* unambiguous — two or more consecutive lines, or a single line whose
* removal fixes a delimiter-balance mismatch against the recorded sides.
*/
export function spliceConflict(originalText: string, entry: ConflictEntry, replacement: string): string {
export function spliceConflict(originalText: string, entry: ConflictEntry, replacement: string): ConflictSplice {
const lines = originalText.split("\n");
const expected = buildRecordedRegion(entry);
const match = locateRegion(lines, expected, entry.startLine - 1);
@@ -341,6 +358,8 @@ export function spliceConflict(originalText: string, entry: ConflictEntry, repla
const trimmed = normalizeTrailingNewline(replacement);
let replacementLines = trimmed.split("\n").map(stripTrailingCr);
const echo = trimBoundaryEcho(replacementLines, lines, match, entry);
replacementLines = echo.lines;
// Round-trip fidelity for CRLF files: recorded sections are LF-normalized,
// so re-apply \r to spliced lines when the matched region used CRLF. The
// final replacement line only carries \r when another line follows it.
@@ -351,7 +370,79 @@ export function spliceConflict(originalText: string, entry: ConflictEntry, repla
);
}
const next = [...lines.slice(0, match.startIdx), ...replacementLines, ...lines.slice(match.endIdx + 1)];
return next.join("\n");
return { text: next.join("\n"), trimmedLeading: echo.leading, trimmedTrailing: echo.trailing };
}
const MAX_ECHO_LINES = 12;
/**
* Net `{}`/`()`/`[]` count over `lines`. Crude (string/comment-blind) —
* used only to corroborate single-line echo trims, never alone.
*/
function delimiterBalance(lines: readonly string[]): number {
let balance = 0;
for (const line of lines) {
for (let i = 0; i < line.length; i++) {
const ch = line.charCodeAt(i);
if (ch === 123 /* { */ || ch === 40 /* ( */ || ch === 91 /* [ */) balance++;
else if (ch === 125 /* } */ || ch === 41 /* ) */ || ch === 93 /* ] */) balance--;
}
}
return balance;
}
/**
* Drop replacement lines that exactly echo the file lines adjacent to the
* located region. A multi-line echo is trimmed unconditionally (a correct
* resolution ending with the exact lines that already follow the region
* would mean intentionally duplicated code — vanishingly unlikely, and the
* untrimmed splice produces exactly that duplication). A single-line echo
* is trimmed only when the recorded sides agree on the region's delimiter
* balance and dropping the echo is what restores it.
*/
function trimBoundaryEcho(
replacement: string[],
fileLines: readonly string[],
match: { startIdx: number; endIdx: number },
entry: ConflictBlock,
): { lines: string[]; leading: number; trailing: number } {
const oursBalance = delimiterBalance(entry.oursLines);
const expectedBalance = oursBalance === delimiterBalance(entry.theirsLines) ? oursBalance : null;
const singleEchoJustified = (lines: string[], without: string[]) =>
expectedBalance !== null &&
delimiterBalance(lines) !== expectedBalance &&
delimiterBalance(without) === expectedBalance;
let lines = replacement;
let trailing = 0;
const after: string[] = [];
for (let i = match.endIdx + 1; i < fileLines.length && after.length < MAX_ECHO_LINES; i++) {
after.push(stripTrailingCr(fileLines[i]!));
}
for (let k = Math.min(after.length, lines.length - 1); k >= 1; k--) {
if (!after.slice(0, k).every((line, i) => lines[lines.length - k + i] === line)) continue;
if (k >= 2 || singleEchoJustified(lines, lines.slice(0, -1))) {
trailing = k;
lines = lines.slice(0, lines.length - k);
}
break;
}
let leading = 0;
const before: string[] = [];
for (let i = match.startIdx - 1; i >= 0 && before.length < MAX_ECHO_LINES; i--) {
before.unshift(stripTrailingCr(fileLines[i]!));
}
for (let k = Math.min(before.length, lines.length - 1); k >= 1; k--) {
if (!before.slice(before.length - k).every((line, i) => lines[i] === line)) continue;
if (k >= 2 || singleEchoJustified(lines, lines.slice(1))) {
leading = k;
lines = lines.slice(k);
}
break;
}
return { lines, leading, trailing };
}
/** Reconstruct the recorded marker block as it should appear in the file. */
@@ -608,11 +699,14 @@ export function formatConflictWarning(
if (theirsLabel) out.push(`- theirs = ${theirsLabel}`);
if (anyBase) out.push(`- base = ${baseLabel ?? "(no label)"}`);
out.push(
'NOTICE: Inspect a block by reading `conflict://<N>` (add `/ours` / `/theirs` / `/base` to render a single side). Resolve with `write({ path: "conflict://<N>", content })`, or bulk-resolve every registered conflict with `write({ path: "conflict://*", content })`. Writes replace the whole conflict region (markers + all sides).',
'NOTICE: Inspect a block by reading `conflict://<N>` (add `/ours` / `/theirs` / `/base` to render a single side). Resolve with `write({ path: "conflict://<N>", content })`, or bulk-resolve every registered conflict with `write({ path: "conflict://*", content })`. Writes replace ONLY the marker block (markers + all sides) — never repeat the lines before/after it; they stay in place.',
);
out.push(
'`content` shorthand: a line that is exactly `@ours` / `@theirs` / `@base` / `@both` expands to that recorded section. `@both` is ours-then-theirs with no separator. Lines that are not a token pass through verbatim, so `"// keep both\\n@ours\\n@theirs"` literally writes the comment, then ours, then theirs.',
);
out.push(
"Resolve with the minimal merge of the recorded sides — do not introduce logic that neither side contains.",
);
for (const entry of entries) {
const range = entry.startLine === entry.endLine ? `L${entry.startLine}` : `L${entry.startLine}-${entry.endLine}`;
@@ -674,7 +768,7 @@ export function formatConflictSummary(
'NOTICE: Bulk-resolve with `write({ path: "conflict://*", content })`, or address a single block with `write({ path: "conflict://<N>", content })`. Inspect a block by reading `conflict://<N>` (add `/ours` / `/theirs` / `/base` for a single side).',
);
lines.push(
"`content` shorthand: `@ours` / `@theirs` / `@base` / `@both` lines expand to the recorded sections; `@both` = ours-then-theirs. Non-token lines pass through verbatim.",
"`content` shorthand: `@ours` / `@theirs` / `@base` / `@both` lines expand to the recorded sections; `@both` = ours-then-theirs. Non-token lines pass through verbatim. Writes replace ONLY the marker block — never repeat the surrounding lines. Prefer the minimal merge of the recorded sides.",
);
lines.push("");
const idWidth = String(entries[entries.length - 1]?.id ?? 1).length;
+15 -2
View File
@@ -591,7 +591,8 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
const expanded = expandContentTokens(replacementContent, entry);
const originalText = await Bun.file(absolutePath).text();
const newContent = spliceConflict(originalText, entry, expanded);
const splice = spliceConflict(originalText, entry, expanded);
const newContent = splice.text;
await writethroughNoop(absolutePath, newContent, signal);
invalidateFsScanAfterWrite(absolutePath);
@@ -626,6 +627,10 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
if (stripped) {
resultText += `\nNote: auto-stripped hashline display prefixes from content before writing.`;
}
const echoTrimmed = splice.trimmedLeading + splice.trimmedTrailing;
if (echoTrimmed > 0) {
resultText += `\nNote: dropped ${echoTrimmed} content line(s) that duplicated the code adjacent to the conflict region — writes replace only the marker block; surrounding lines stay in place.`;
}
return {
content: [{ type: "text", text: resultText }],
@@ -690,6 +695,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
const succeededFiles: { displayPath: string; count: number; header?: string }[] = [];
const failedFiles: { displayPath: string; count: number; error: string }[] = [];
let totalResolvedIds = 0;
let totalEchoTrimmed = 0;
for (const [absolutePath, fileEntries] of byFile) {
const sample = fileEntries[0]!;
@@ -721,7 +727,9 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
for (const entry of fileEntries) {
try {
const expanded = expandContentTokens(replacementContent, entry);
text = spliceConflict(text, entry, expanded);
const splice = spliceConflict(text, entry, expanded);
text = splice.text;
totalEchoTrimmed += splice.trimmedLeading + splice.trimmedTrailing;
resolvedEntries.push(entry);
} catch (error) {
// A locate-miss for a region an earlier entry already spliced
@@ -766,6 +774,11 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
summaryLines.push(` ${file.displayPath}: ${file.count} ${conflictWord(file.count)}`);
}
}
if (totalEchoTrimmed > 0) {
summaryLines.push(
`Note: dropped ${totalEchoTrimmed} content line(s) that duplicated code adjacent to conflict regions — writes replace only the marker block; surrounding lines stay in place.`,
);
}
if (failedFiles.length > 0) {
summaryLines.push(
`Failed to resolve ${failedFiles.length} ${fileWord(failedFiles.length)} — registered entries left intact for retry:`,
@@ -273,23 +273,23 @@ describe("spliceConflict", () => {
it("replaces the marker region with the chosen content", () => {
const result = spliceConflict(file, entry, "resolved\n");
expect(result).toBe("before\nresolved\nafter\n");
expect(result.text).toBe("before\nresolved\nafter\n");
});
it("accepts multi-line replacement", () => {
const result = spliceConflict(file, entry, "alpha\nbeta\n");
expect(result).toBe("before\nalpha\nbeta\nafter\n");
expect(result.text).toBe("before\nalpha\nbeta\nafter\n");
});
it("accepts empty replacement", () => {
const result = spliceConflict(file, entry, "");
expect(result).toBe("before\n\nafter\n");
expect(result.text).toBe("before\n\nafter\n");
});
it("relocates the block when earlier lines have been added (line numbers shift)", () => {
const shifted = ["// new comment 1", "// new comment 2", ...file.split("\n")].join("\n");
const result = spliceConflict(shifted, entry, "resolved\n");
expect(result).toBe("// new comment 1\n// new comment 2\nbefore\nresolved\nafter\n");
expect(result.text).toBe("// new comment 1\n// new comment 2\nbefore\nresolved\nafter\n");
});
it("rejects when the recorded marker block has been edited away", () => {
@@ -306,13 +306,167 @@ describe("spliceConflict", () => {
"\r\n",
);
const result = spliceConflict(crlfFile, entry, "alpha\nbeta\n");
expect(result).toBe("before\r\nalpha\r\nbeta\r\nafter\r\n");
expect(result.text).toBe("before\r\nalpha\r\nbeta\r\nafter\r\n");
});
it("does not append \\r when the spliced region ends the file without a trailing newline", () => {
const crlfNoEof = ["before", "<<<<<<< HEAD", "ours", "=======", "theirs", ">>>>>>> feat"].join("\r\n");
const result = spliceConflict(crlfNoEof, entry, "resolved");
expect(result).toBe("before\r\nresolved");
expect(result.text).toBe("before\r\nresolved");
});
});
describe("spliceConflict boundary-echo repair", () => {
// The 08-multi-file-rename shape: the two lines after the closer are the
// function tail models love to re-emit when they paste the "whole
// resolved function" as the replacement.
const fnLines = [
"const queue = [];",
"<<<<<<< HEAD",
"export function scheduleTask(task, priority = 0) {",
"\tif (dupe(task)) {",
"\t\treturn;",
"\t}",
"=======",
"export function enqueueTask(task) {",
"\tif (queued.has(task.id)) {",
"\t\treturn;",
"\t}",
">>>>>>> feature",
"\tqueue.push(task);",
"}",
"",
];
const fnEntry = makeEntry({
startLine: 2,
separatorLine: 7,
endLine: 12,
oursLabel: "HEAD",
theirsLabel: "feature",
oursLines: fnLines.slice(2, 6),
theirsLines: fnLines.slice(7, 11),
});
it("drops a multi-line trailing echo of the context below the region", () => {
const replacement = [
"export function scheduleTask(task, priority = 0) {",
"\tif (queued.has(task.id)) {",
"\t\treturn;",
"\t}",
"\tqueue.push(task);",
"}",
].join("\n");
const result = spliceConflict(fnLines.join("\n"), fnEntry, replacement);
expect(result.trimmedTrailing).toBe(2);
expect(result.trimmedLeading).toBe(0);
expect(result.text).toBe(
[
"const queue = [];",
"export function scheduleTask(task, priority = 0) {",
"\tif (queued.has(task.id)) {",
"\t\treturn;",
"\t}",
"\tqueue.push(task);",
"}",
"",
].join("\n"),
);
});
// The 02-rename-vs-limits shape: a lone `}` echoed after a body-only region.
const bodyLines = [
"function nextDelay(a) {",
"<<<<<<< HEAD",
"\tconst delay = BASE * 2 ** a;",
"\treturn Math.min(delay, 10_000);",
"=======",
"\tconst d = B * 2 ** a;",
"\treturn Math.min(d, 30_000);",
">>>>>>> tune",
"}",
"",
];
const bodyEntry = makeEntry({
startLine: 2,
separatorLine: 5,
endLine: 8,
oursLabel: "HEAD",
theirsLabel: "tune",
oursLines: bodyLines.slice(2, 4),
theirsLines: bodyLines.slice(5, 7),
});
it("drops a single-line echo when it fixes the region's delimiter balance", () => {
const replacement = ["\tconst delay = BASE * 2 ** a;", "\treturn Math.min(delay, 30_000);", "}"].join("\n");
const result = spliceConflict(bodyLines.join("\n"), bodyEntry, replacement);
expect(result.trimmedTrailing).toBe(1);
expect(result.text).toBe(
[
"function nextDelay(a) {",
"\tconst delay = BASE * 2 ** a;",
"\treturn Math.min(delay, 30_000);",
"}",
"",
].join("\n"),
);
});
it("keeps a single-line echo when the delimiter balance is already consistent", () => {
const file = ["start", "<<<<<<< HEAD", "a", "=======", "b", ">>>>>>> x", "done();", ""].join("\n");
const entry = makeEntry({
startLine: 2,
separatorLine: 4,
endLine: 6,
oursLabel: "HEAD",
theirsLabel: "x",
oursLines: ["a"],
theirsLines: ["b"],
});
const result = spliceConflict(file, entry, "merged\ndone();");
expect(result.trimmedTrailing).toBe(0);
expect(result.text).toBe("start\nmerged\ndone();\ndone();\n");
});
it("drops a multi-line leading echo of the context above the region", () => {
const file = [
"// header",
"const queue = [];",
"<<<<<<< HEAD",
"a",
"=======",
"b",
">>>>>>> x",
"tail",
"",
].join("\n");
const entry = makeEntry({
startLine: 3,
separatorLine: 5,
endLine: 7,
oursLabel: "HEAD",
theirsLabel: "x",
oursLines: ["a"],
theirsLines: ["b"],
});
const result = spliceConflict(file, entry, "// header\nconst queue = [];\nmerged");
expect(result.trimmedLeading).toBe(2);
expect(result.text).toBe("// header\nconst queue = [];\nmerged\ntail\n");
});
it("repairs echoes in CRLF files without breaking EOL round-trip", () => {
const crlf = bodyLines.join("\r\n");
const replacement = ["\tconst delay = BASE * 2 ** a;", "\treturn Math.min(delay, 30_000);", "}"].join("\n");
const result = spliceConflict(crlf, bodyEntry, replacement);
expect(result.trimmedTrailing).toBe(1);
expect(result.text).toBe(
[
"function nextDelay(a) {",
"\tconst delay = BASE * 2 ** a;",
"\treturn Math.min(delay, 30_000);",
"}",
"",
].join("\r\n"),
);
});
});
@@ -332,6 +332,40 @@ describe("write resolves conflicts via conflict://N", () => {
expect(session.conflictHistory?.get(1)).toBeUndefined();
});
it("drops trailing lines that echo the context below the region and notes the repair", async () => {
const filePath = path.join(tempDir, "echo.ts");
const content = [
"function f() {",
"<<<<<<< HEAD",
"\tours();",
"=======",
"\ttheirs();",
">>>>>>> feature/x",
"\tdone();",
"}",
"",
].join("\n");
await Bun.write(filePath, content);
const session = createTestSession(tempDir);
const read = await getTool(session, "read");
const write = await getTool(session, "write");
await read.execute("read-echo", { path: "echo.ts" });
// The classic failure: the model pastes the whole resolved function,
// including the two lines that live below the marker block.
const result = await write.execute("write-echo", {
path: "conflict://1",
content: "\tours();\n\ttheirs();\n\tdone();\n}\n",
});
const text = getText(result);
expect(text).toContain("Resolved conflict #1");
expect(text).toContain("dropped 2 content line(s)");
expect(await Bun.file(filePath).text()).toBe(
["function f() {", "\tours();", "\ttheirs();", "\tdone();", "}", ""].join("\n"),
);
});
it("auto-recovers a `<file>:conflict://N` path and resolves the conflict", async () => {
const filePath = path.join(tempDir, "prefix.ts");
await Bun.write(filePath, TWO_WAY);