fix(edit): recovered malformed local-model ranges

- Accepted comma-separated ranges and harmless malformed trailers emitted by local models.
- Added focused array-input diagnostics and reinforced canonical hashline syntax in the edit prompt.
- Covered the recovered forms with behavioral regression tests.

Fixes #5805
This commit is contained in:
roboomp
2026-07-17 07:46:25 +00:00
parent 0f9fceeea4
commit b65467291e
6 changed files with 56 additions and 10 deletions
+1
View File
@@ -26,6 +26,7 @@
### Fixed
- Fixed repeated edit-tool rejections from local models by recovering comma-separated ranges and malformed trailers, while clarifying canonical string input and `.=` syntax ([#5805](https://github.com/can1357/oh-my-pi/issues/5805)).
- Fixed loading issues for linked legacy extensions importing `DefaultPackageManager` or `linkedom`.
- Fixed the advisor retrying terminal, non-retriable provider failures (e.g., blocked prompts), ensuring they fail immediately while transient failures still retry.
- Fixed an issue where reassigning the `plan` role model mid-planning did not take effect until the next plan-mode entry; it now applies at the next turn boundary.
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed repeated edit-tool rejections by recovering comma-separated ranges and malformed local-model trailers, while steering agents to canonical string input and `.=` syntax ([#5805](https://github.com/can1357/oh-my-pi/issues/5805)).
## [17.0.0] - 2026-07-15
### Added
+5
View File
@@ -116,6 +116,11 @@ function parseHashlineHeaderLine(line: string, cwd?: string): RawSection | null
// the half-dozen variants models actually emit.
const recovered = tryParseRecoveryHeader(trimmed, cwd);
if (recovered !== null) return recovered;
if (trimmed === "[" || trimmed.startsWith('["') || trimmed.startsWith("[{")) {
throw new Error(
"Edit input must be one patch string, not a JSON array. Join patch lines with newlines inside the `input` string.",
);
}
throw new Error(
`Input header must be ${HL_FILE_PREFIX}PATH${HL_FILE_SUFFIX} or ${HL_FILE_PREFIX}PATH${HL_FILE_HASH_SEP}TAG${HL_FILE_SUFFIX} with a ${HL_FILE_HASH_LENGTH}-hex content-hash tag; got ${JSON.stringify(trimmed)}.`,
);
+17 -3
View File
@@ -1,5 +1,10 @@
Your patch language names lines to replace, delete, or insert at, then lists the new content. Rule of thumb: a header ending in `:` is followed by `+` body rows; `DEL` has no body.
<critical>
- Input is ONE patch string. NEVER pass an array.
- Ranges use `N.=M` exactly. NEVER commas or `:=:`.
</critical>
<headers>
Every file section starts with `[PATH#TAG]`. `TAG` = 4-hex snapshot tag from your latest `read`/`search`, REQUIRED on every section — no hashless form. Create new files with `write`; hashline only edits existing files.
</headers>
@@ -130,6 +135,13 @@ SWAP.BLK 1:
</example>
<anti-patterns>
# WRONG — comma range and `:=:` trailer. RIGHT: `SWAP 1.=17:`
SWAP 1,17:=:
+replacement
# RIGHT
SWAP 1.=17:
+replacement
# WRONG — empty `SWAP` to delete. RIGHT: DEL 4
SWAP 4.=4:
@@ -166,7 +178,9 @@ INS.POST 3:
<critical>
If you remember nothing else:
1. RE-GROUND AFTER EVERY EDIT. Every apply mints a fresh `#TAG` and renumbers — take the next edit's numbers from the edit response or a fresh `read`. Stale tag or surprise? STOP, re-`read`.
2. RANGES ARE TIGHT. Cover only lines that change; a stale wide range shreds everything it spans. Whole construct → `SWAP.BLK N`.
3. THE BODY IS THE FINAL CONTENT. Every body row starts with `+`; Markdown bullets use `+- item`, not `- item`.
1. INPUT IS ONE STRING. NEVER pass patch lines as an array.
2. RE-GROUND AFTER EVERY EDIT. Every apply mints a fresh `#TAG` and renumbers — take the next edit's numbers from the edit response or a fresh `read`. Stale tag or surprise? STOP, re-`read`.
3. RANGES ARE EXACT. Use `N.=M`; NEVER commas or `:=:`.
4. RANGES ARE TIGHT. Cover only lines that change; a stale wide range shreds everything it spans. Whole construct → `SWAP.BLK N`.
5. THE BODY IS THE FINAL CONTENT. Every body row starts with `+`; Markdown bullets use `+- item`, not `- item`.
</critical>
+15 -5
View File
@@ -40,6 +40,7 @@ const CHAR_HASH = 35;
const CHAR_TAB = 9;
const CHAR_SPACE = 32;
const CHAR_DOT = 46;
const CHAR_COMMA = 44;
const CHAR_HYPHEN = 45;
const CHAR_ELLIPSIS = 0x2026;
const CHAR_EQUALS = 61;
@@ -165,7 +166,7 @@ function scanRangeSeparator(line: string, index: number, end: number): number |
consumedSeparator = true;
continue;
}
if (code === CHAR_HYPHEN || code === CHAR_ELLIPSIS) {
if (code === CHAR_COMMA || code === CHAR_HYPHEN || code === CHAR_ELLIPSIS) {
cursor++;
consumedSeparator = true;
continue;
@@ -252,6 +253,17 @@ function consumeOptionalColon(line: string, index: number, end: number): number
cursor = skipStrayDot(line, cursor, end);
return cursor < end && line.charCodeAt(cursor) === CHAR_COLON ? skipWhitespace(line, cursor + 1, end) : cursor;
}
/**
* Recover local-model replace trailers that permute `:` and `=` as `:=:` or
* `=:`. The range has already been parsed, so these suffixes are unambiguous.
*/
function consumeReplaceColon(line: string, index: number, end: number): number {
const canonical = consumeOptionalColon(line, index, end);
if (canonical >= end || line.charCodeAt(canonical) !== CHAR_EQUALS) return canonical;
const afterEquals = skipWhitespace(line, canonical + 1, end);
if (afterEquals >= end || line.charCodeAt(afterEquals) !== CHAR_COLON) return canonical;
return skipWhitespace(line, afterEquals + 1, end);
}
function scanInsertTarget(line: string, index: number, end: number): TargetScan | null {
if (index >= end || line.charCodeAt(index) !== CHAR_DOT) return null;
@@ -341,7 +353,7 @@ function scanHunkAnchor(line: string, start: number, end: number): TargetScan |
if (range === null) return null;
return {
target: { kind: "replace", range: range.range },
nextIndex: consumeOptionalColon(line, range.nextIndex, end),
nextIndex: consumeReplaceColon(line, range.nextIndex, end),
};
}
// `delete_block N` — resolve N to a tree-sitter block range at apply time
@@ -360,9 +372,7 @@ function scanHunkAnchor(line: string, start: number, end: number): TargetScan |
if (deleteEnd !== null) {
const range = scanHeaderRange(line, deleteEnd, end, true);
if (range === null) return null;
let next = skipWhitespace(line, range.nextIndex, end);
next = skipStrayDot(line, next, end);
if (next < end && line.charCodeAt(next) === CHAR_COLON) return null;
const next = consumeOptionalColon(line, range.nextIndex, end);
return { target: { kind: "delete", range: range.range }, nextIndex: next };
}
// `insert_after_block N:` — insert after the last line of the tree-sitter
+14 -2
View File
@@ -57,6 +57,12 @@ describe("hashline section headers", () => {
expect(message).not.toContain("#0A3");
}
});
it("explains that array-shaped tool input must be one patch string", () => {
expect(() => Patch.parse('["[a.ts#1A2B]", "SWAP 1.=1:", "+after"]')).toThrow(
/one patch string, not a JSON array/,
);
});
});
describe("hashline core — verb header forms", () => {
@@ -87,6 +93,8 @@ describe("hashline core — verb header forms", () => {
expect(applyPatch(FILE, "SWAP 2\u20263:\n+X")).toBe("a\nX\nd\ne");
expect(applyPatch(FILE, "SWAP 2 3:\n+X")).toBe("a\nX\nd\ne");
expect(applyPatch(FILE, "SWAP 2..3:\n+X")).toBe("a\nX\nd\ne"); // legacy `..` still accepted
expect(applyPatch(FILE, "SWAP 2,3:\n+X")).toBe("a\nX\nd\ne");
expect(applyPatch(FILE, "SWAP 2,3:=:\n+X")).toBe("a\nX\nd\ne");
expect(applyPatch(FILE, "SWAP 2.=3\n+X")).toBe("a\nX\nd\ne"); // missing colon
});
@@ -187,8 +195,12 @@ describe("hashline body contracts", () => {
expect(() => parsePatch("DEL 2\n+X")).toThrow(/does not take body rows/);
});
it("rejects delete with a colon", () => {
expect(() => parsePatch("DEL 2:\n+X")).toThrow(/has no colon/);
it("accepts a trailing colon on bodyless delete headers", () => {
expect(applyPatch(FILE, "DEL 2,3:")).toBe("a\nd\ne");
});
it("still rejects delete body rows after a trailing colon", () => {
expect(() => parsePatch("DEL 2:\n+X")).toThrow(/does not take body rows/);
});
});