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.
This commit is contained in:
+12
-12
@@ -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`.
|
||||
|
||||
@@ -287,7 +287,7 @@ const hashlineStrategy: EditStreamingStrategy<HashlineArgs> = {
|
||||
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,
|
||||
});
|
||||
|
||||
@@ -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: /(.+)/
|
||||
|
||||
|
||||
@@ -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.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
<ops>
|
||||
@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:
|
||||
|
||||
<examples>
|
||||
# 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}}
|
||||
</examples>
|
||||
|
||||
<anti-pattern>
|
||||
# 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
|
||||
<critical>
|
||||
- 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.
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user