From 6fbb93b017531390f175df0b978e4d27e034e56a Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 12 May 2026 14:40:42 +0200 Subject: [PATCH] fix(coding-agent): allowed hashline parsing to accept flexible @@ section headers - Updated hashline section parsing to accept headers with any leading `@` characters, normalizing them to the path before validation. - Updated the hashline grammar and fallback errors to use canonical `@@ PATH` section headers. - Added coverage for mixed `@@` and `@@@` headers across multiple file sections in hashline parsing tests. --- docs/tools/edit.md | 24 +++++++-------- packages/coding-agent/src/edit/streaming.ts | 2 +- .../coding-agent/src/hashline/grammar.lark | 2 +- packages/coding-agent/src/hashline/input.ts | 16 ++++++---- .../src/prompts/tools/hashline.md | 30 +++++++++---------- .../coding-agent/test/core/hashline.test.ts | 8 +++++ 6 files changed, 48 insertions(+), 34 deletions(-) diff --git a/docs/tools/edit.md b/docs/tools/edit.md index 97e2aa4c0..4e5b60769 100644 --- a/docs/tools/edit.md +++ b/docs/tools/edit.md @@ -8,7 +8,7 @@ - Key collaborators: - `packages/coding-agent/src/utils/edit-mode.ts` — selects active edit mode - `packages/coding-agent/src/hashline/grammar.lark` — custom-tool grammar for hashline mode - - `packages/coding-agent/src/hashline/input.ts` — splits `@PATH` sections + - `packages/coding-agent/src/hashline/input.ts` — splits `@@ PATH` sections (legacy single-`@` headers are still accepted) - `packages/coding-agent/src/hashline/parser.ts` — parses ops and payload lines - `packages/coding-agent/src/hashline/apply.ts` — validates anchors and applies edits - `packages/coding-agent/src/hashline/anchors.ts` — stale-anchor mismatch formatting @@ -26,11 +26,11 @@ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `input` | `string` | Yes | One or more edit sections. First non-blank line must be `@PATH` unless the caller supplies the legacy fallback `path` outside the model schema and the body already looks like hashline ops (`packages/coding-agent/src/hashline/input.ts`). Optional `*** Begin Patch` / `*** End Patch` envelope is ignored if present. | +| `input` | `string` | Yes | One or more edit sections. First non-blank line must be `@@ PATH` (legacy single-`@` is still accepted) unless the caller supplies the legacy fallback `path` outside the model schema and the body already looks like hashline ops (`packages/coding-agent/src/hashline/input.ts`). Optional `*** Begin Patch` / `*** End Patch` envelope is ignored if present. | Patch language inside `input`: -- Section header: `@PATH` +- Section header: `@@ PATH` - Insert after: `+ ANCHOR` - Insert before: `< ANCHOR` - Delete range: `- A..B` @@ -102,24 +102,24 @@ Warnings: Hashline op examples: ```text -@src/a.ts +@@ src/a.ts + 4fb ~const added = true; ``` ```text -@src/a.ts +@@ src/a.ts < 4fb ~const addedBefore = true; ``` ```text -@src/a.ts +@@ src/a.ts - 4fb..6qx ``` ```text -@src/a.ts +@@ src/a.ts = 4fb..5dm ~const clean = (name || DEF).trim(); ~return clean.length === 0 ? DEF : clean.toUpperCase(); @@ -128,13 +128,13 @@ Hashline op examples: BOF/EOF examples: ```text -@src/a.ts +@@ src/a.ts + BOF ~const HEADER = true; ``` ```text -@src/a.ts +@@ src/a.ts + EOF ~export const done = true; ``` @@ -166,7 +166,7 @@ BOF/EOF examples: ## Errors - Missing section header: - - `input must begin with "@PATH" on the first non-blank line; got: ... Example: "@src/foo.ts" then edit ops.` + - `input must begin with "@@ PATH" on the first non-blank line; got: ... Example: "@@ src/foo.ts" then edit ops.` - Empty header: - `Input header "@" is empty; provide a file path.` - Bad anchor token: @@ -198,8 +198,8 @@ BOF/EOF examples: - Interior lines of a multi-line range use hash `**` (`RANGE_INTERIOR_HASH`) and are not individually verified; only the first and last anchor hashes are checked. - `computeLineHash()` trims trailing whitespace before hashing. Anchors survive line-ending changes and trailing-space-only changes, but not substantive line edits. - For punctuation-only lines, the hash mixes in the line number; identical `}` lines on different lines intentionally get different anchors. -- `splitHashlineInputs()` normalizes absolute `@PATH` headers back to a cwd-relative path when the file is inside the current working tree. -- Optional `*** Begin Patch` / `*** End Patch` markers are accepted in hashline mode, but the file sections are still `@PATH`-based, not Codex `*** Update File:` hunks. +- `splitHashlineInputs()` normalizes absolute `@@ PATH` headers back to a cwd-relative path when the file is inside the current working tree. Headers with any run of leading `@` chars (e.g. `@ foo.ts`, `@@ foo.ts`, `@@@foo.ts`) are accepted to absorb unified-diff-style drift; the canonical form is `@@ PATH`. +- Optional `*** Begin Patch` / `*** End Patch` markers are accepted in hashline mode, but the file sections are still `@@ PATH`-based, not Codex `*** Update File:` hunks. - `*** Abort` terminates parsing early and returns `ABORT_WARNING`; ops parsed before the marker still apply. - File-read cache invalidation is conflict-based, not write-through invalidation. If `read` later records content for a line that disagrees with the cached snapshot, the entire snapshot for that path is replaced with the newly observed lines (`packages/coding-agent/src/edit/file-read-cache.ts`). - There is no resolve-style apply/discard phase for hashline edits. The only preview path is the transient TUI diff preview in `packages/coding-agent/src/edit/streaming.ts`. diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts index a8ee0f6dc..02b743959 100644 --- a/packages/coding-agent/src/edit/streaming.ts +++ b/packages/coding-agent/src/edit/streaming.ts @@ -287,7 +287,7 @@ const hashlineStrategy: EditStreamingStrategy = { sections = splitHashlineInputs(args.input, { cwd: ctx.cwd, path: args.path }); } catch { // Single-section fallback keeps the original error rendering for the - // "haven't typed `@PATH` yet" case. + // "haven't typed `@@ PATH` yet" case. const result = await computeHashlineDiff({ input: args.input, path: args.path }, ctx.cwd, { autoDropPureInsertDuplicates: ctx.hashlineAutoDropPureInsertDuplicates, }); diff --git a/packages/coding-agent/src/hashline/grammar.lark b/packages/coding-agent/src/hashline/grammar.lark index 103333f7d..b2d571d40 100644 --- a/packages/coding-agent/src/hashline/grammar.lark +++ b/packages/coding-agent/src/hashline/grammar.lark @@ -3,7 +3,7 @@ begin_patch: "*** Begin Patch" LF end_patch: "*** End Patch" LF? hunk: update_hunk -update_hunk: "@" filename LF line_op* +update_hunk: "@@ " filename LF line_op* filename: /(.+)/ diff --git a/packages/coding-agent/src/hashline/input.ts b/packages/coding-agent/src/hashline/input.ts index e149f966e..00687fc76 100644 --- a/packages/coding-agent/src/hashline/input.ts +++ b/packages/coding-agent/src/hashline/input.ts @@ -27,11 +27,17 @@ function normalizeHashlinePath(rawPath: string, cwd?: string): string { function parseHashlineHeaderLine(line: string, cwd?: string): HashlineInputSection | null { const trimmed = line.trimEnd(); - if (trimmed === FILE_HEADER_PREFIX) { + if (!trimmed.startsWith(FILE_HEADER_PREFIX)) return null; + // Some models occasionally emit unified-diff-style "@@ path" (or even longer + // runs of "@"). Strip every leading "@" before resolving the path so those + // stray headers still route to the right file. + let prefixEnd = 0; + while (prefixEnd < trimmed.length && trimmed[prefixEnd] === FILE_HEADER_PREFIX) prefixEnd++; + const rest = trimmed.slice(prefixEnd); + if (rest.trim().length === 0) { throw new Error(`Input header "${FILE_HEADER_PREFIX}" is empty; provide a file path.`); } - if (!trimmed.startsWith(FILE_HEADER_PREFIX)) return null; - const parsedPath = normalizeHashlinePath(trimmed.slice(1), cwd); + const parsedPath = normalizeHashlinePath(rest, cwd); if (parsedPath.length === 0) { throw new Error(`Input header "${FILE_HEADER_PREFIX}" is empty; provide a file path.`); } @@ -91,8 +97,8 @@ export function splitHashlineInputs(input: string, options: SplitHashlineOptions if (parseHashlineHeaderLine(firstLine, options.cwd) === null) { const preview = JSON.stringify(firstLine.slice(0, 120)); throw new Error( - `input must begin with "@PATH" on the first non-blank line; got: ${preview}. ` + - `Example: "@src/foo.ts" then edit ops.`, + `input must begin with "@@ PATH" on the first non-blank line; got: ${preview}. ` + + `Example: "@@ src/foo.ts" then edit ops.`, ); } diff --git a/packages/coding-agent/src/prompts/tools/hashline.md b/packages/coding-agent/src/prompts/tools/hashline.md index 62068c0f6..0a551d715 100644 --- a/packages/coding-agent/src/prompts/tools/hashline.md +++ b/packages/coding-agent/src/prompts/tools/hashline.md @@ -1,13 +1,13 @@ Your patch language is a compact, line-anchored edit format. -A patch contains one or more file sections. The first non-blank line of every edit section **MUST** be `@PATH`. +A patch contains one or more file sections. The first non-blank line of every edit section **MUST** be `@@ PATH`. Operations reference lines in the file by their line number and hash, called "Anchors", e.g. `5th`, `123ab`. You **MUST** copy them verbatim from the latest output for the file you're editing. Purely textual format. The tool has NO awareness of language, indentation, brackets, fences, or table widths. Emit valid syntax in replacements/insertions. -@PATH header: subsequent ops apply to PATH +@@ PATH header: subsequent ops apply to PATH + ANCHOR insert lines AFTER the anchored line (or EOF); payload follows as `{{hsep}}TEXT` lines < ANCHOR insert lines BEFORE the anchored line (or BOF); payload follows as `{{hsep}}TEXT` lines - A..B delete the line range (inclusive). @@ -64,18 +64,18 @@ When your edit involves brace boundaries (`{` / `}`), prefer these shapes: # Replace one line (preserve the leading tab from the original) -@a.ts +@@ a.ts = {{hrefr 5}}..{{hrefr 5}} {{hsep}} return clean.trim().toUpperCase(); # Replace a contiguous range with multiple lines -@a.ts +@@ a.ts = {{hrefr 4}}..{{hrefr 5}} {{hsep}} const clean = (name || DEF).trim(); {{hsep}} return clean.length === 0 ? DEF : clean.toUpperCase(); # Replace a full multiline destructuring/call statement -@b.ts +@@ b.ts = {{hrefr 1}}..{{hrefr 8}} {{hsep}}const { {{hsep}} events, @@ -88,32 +88,32 @@ When your edit involves brace boundaries (`{` / `}`), prefer these shapes: {{hsep}}); # Insert BEFORE a line -@a.ts +@@ a.ts < {{hrefr 5}} {{hsep}} const debug = false; # Insert AFTER a line -@a.ts +@@ a.ts + {{hrefr 4}} {{hsep}} if (clean.length === 0) return DEF; # Append to end of file -@a.ts +@@ a.ts + EOF {{hsep}}export const done = true; # Delete a single line -@a.ts +@@ a.ts - {{hrefr 2}}..{{hrefr 2}} # Blank a line in place (no payload required) -@a.ts +@@ a.ts = {{hrefr 2}}..{{hrefr 2}} # WRONG — replaces 5 lines just to add one. Use `+` at the boundary instead. -@a.ts +@@ a.ts = {{hrefr 1}}..{{hrefr 5}} {{hsep}}const DEF = "guest"; {{hsep}}const DEBUG = false; @@ -123,12 +123,12 @@ When your edit involves brace boundaries (`{` / `}`), prefer these shapes: {{hsep}} return clean.trim(); # RIGHT — same effect, one-line insert -@a.ts +@@ a.ts + {{hrefr 1}} {{hsep}}const DEBUG = false; # WRONG — continuation-fragment payload from the middle of a larger statement. -@b.ts +@@ b.ts = {{hrefr 5}}..{{hrefr 7}} {{hsep}}} = await getStreamResponse( {{hsep}} request, @@ -136,7 +136,7 @@ When your edit involves brace boundaries (`{` / `}`), prefer these shapes: {{hsep}} onEvent, # RIGHT — widen to the full statement so the payload starts at a self-contained boundary. -@b.ts +@@ b.ts = {{hrefr 1}}..{{hrefr 8}} {{hsep}}const { {{hsep}} events, @@ -154,7 +154,7 @@ If your replacement payload would render with even one unchanged line in the dif - Always copy anchors exactly from tool output, but **NEVER** include line content after the `{{hsep}}` separator in the op line. - Every inserted/replacement content line **MUST** start with `{{hsep}}`; raw content lines are invalid. -- Do not write unified diff syntax (`@@`, `-OLD`, `+NEW`). +- Do not write unified diff syntax (`@@ -X,Y +X,Y @@`, `-OLD`, `+NEW`). The header is `@@ PATH`; line ops are `<`/`+`/`-`/`=`. - `= A..B` deletes the range; payload is what's written. If a payload edge line already exists immediately outside `A..B`, widen the range to cover it — otherwise it duplicates. - Multiple ops in one patch are cheap. Prefer two narrow ops over one wide `=`. - Before choosing a `= A..B` range, mentally delete lines A through B. If that would split an unclosed bracket, paren, brace, or string/template from a line above A, or orphan a closing delimiter that belongs to an opener inside the range, you are bisecting a syntactic construct. Widen the range to a self-contained boundary, or use `+`/`-` instead. diff --git a/packages/coding-agent/test/core/hashline.test.ts b/packages/coding-agent/test/core/hashline.test.ts index 0d0663bf0..3b37b900c 100644 --- a/packages/coding-agent/test/core/hashline.test.ts +++ b/packages/coding-agent/test/core/hashline.test.ts @@ -391,6 +391,14 @@ describe("splitHashlineInput — @ headers", () => { { path: "b.ts", diff: `+ EOF\n${pl("b")}` }, ]); }); + + it("tolerates extra '@' chars on the section header", () => { + const input = ["@@ a.ts", "+ BOF", pl("a"), "@@@b.ts", "+ EOF", pl("b")].join("\n"); + expect(splitHashlineInputs(input)).toEqual([ + { path: "a.ts", diff: `+ BOF\n${pl("a")}` }, + { path: "b.ts", diff: `+ EOF\n${pl("b")}` }, + ]); + }); }); describe("hashline executor", () => {