fix(edit): reduced hashline failures for weak models

- Added actionable reversed-range and block-anchor diagnostics.
- Accepted unambiguous single-dot ranges and routed affected models to replace mode.
- Preserved edit safety checks and added regression coverage.

Fixes #6671
This commit is contained in:
can1357
2026-07-27 05:52:30 +02:00
parent 7d3f9d382c
commit 972ddb4e5e
12 changed files with 300 additions and 33 deletions
+7
View File
@@ -2,6 +2,13 @@
## [Unreleased]
## [17.1.5] - 2026-07-27
### Changed
- Improved reversed-range and invalid block-anchor diagnostics with absolute endpoint corrections plus nearby syntactic opener suggestions, without auto-applying the suggested edit ([#6671](https://github.com/can1357/oh-my-pi/issues/6671)).
- Accepted a single dot between integer range endpoints, such as `DEL 235.258`, as an unambiguous range separator ([#6671](https://github.com/can1357/oh-my-pi/issues/6671)).
## [17.1.2] - 2026-07-24
### Changed
+64 -7
View File
@@ -15,12 +15,57 @@
import { STRUCTURAL_CLOSER_RE } from "./apply";
import {
BLOCK_RESOLVER_UNAVAILABLE,
type BlockDiagnosticSuggestions,
blockSingleLineMessage,
blockUnresolvedMessage,
insertAfterBlockCloserLoweredWarning,
insertAfterBlockUnresolvedLoweredWarning,
} from "./messages";
import type { BlockResolution, BlockResolver, Cursor, Edit } from "./types";
import type { BlockResolution, BlockResolver, BlockSpan, Cursor, Edit } from "./types";
/** Maximum nearby lines inspected only after a block anchor has already failed. */
const BLOCK_SUGGESTION_SCAN_LIMIT = 64;
function resolveDiagnosticBlock(resolver: BlockResolver, path: string, text: string, line: number): BlockSpan | null {
try {
return resolver({ path, text, line });
} catch {
// Suggestions are best-effort and must never hide the authoritative anchor error.
return null;
}
}
function findNextBlock(
anchorLine: number,
lines: readonly string[],
path: string,
text: string,
resolver: BlockResolver,
): BlockSpan | null {
const lastLine = Math.min(lines.length, anchorLine + BLOCK_SUGGESTION_SCAN_LIMIT);
for (let line = anchorLine + 1; line <= lastLine; line++) {
if (lines[line - 1]?.trim().length === 0) continue;
const span = resolveDiagnosticBlock(resolver, path, text, line);
if (span?.start === line && span.end > line) return span;
}
return null;
}
function findEnclosingBlock(
anchorLine: number,
lines: readonly string[],
path: string,
text: string,
resolver: BlockResolver,
): BlockSpan | null {
const firstLine = Math.max(1, anchorLine - BLOCK_SUGGESTION_SCAN_LIMIT);
for (let line = anchorLine - 1; line >= firstLine; line--) {
if (lines[line - 1]?.trim().length === 0) continue;
const span = resolveDiagnosticBlock(resolver, path, text, line);
if (span?.start === line && span.end >= anchorLine && span.end > line) return span;
}
return null;
}
export interface ResolveBlockEditsOptions {
/**
@@ -105,11 +150,18 @@ export function resolveBlockEdits(
continue;
}
if (onUnresolved === "drop") continue;
throw new Error(
`line ${edit.lineNum}: ${
resolver ? blockUnresolvedMessage(edit.anchor.line, op, text.split("\n")) : BLOCK_RESOLVER_UNAVAILABLE
}`,
);
if (!resolver) throw new Error(`line ${edit.lineNum}: ${BLOCK_RESOLVER_UNAVAILABLE}`);
const lines = text.split("\n");
const nextBlock =
lines[edit.anchor.line - 1]?.trim().length === 0
? findNextBlock(edit.anchor.line, lines, path, text, resolver)
: null;
const enclosingBlock =
nextBlock === null ? findEnclosingBlock(edit.anchor.line, lines, path, text, resolver) : null;
const suggestions: BlockDiagnosticSuggestions = {};
if (nextBlock) suggestions.nextBlock = nextBlock;
if (enclosingBlock) suggestions.enclosingBlock = enclosingBlock;
throw new Error(`line ${edit.lineNum}: ${blockUnresolvedMessage(edit.anchor.line, op, lines, suggestions)}`);
}
if (span.start === span.end) {
// A single-line block resolution means line N is a bare statement, not
@@ -118,7 +170,12 @@ export function resolveBlockEdits(
// and its `break;`). The plain op is exact for one line, so reject and
// point at it; drop instead on the lenient preview path.
if (onUnresolved === "drop") continue;
throw new Error(`line ${edit.lineNum}: ${blockSingleLineMessage(edit.anchor.line, op)}`);
const enclosingBlock = resolver
? findEnclosingBlock(edit.anchor.line, text.split("\n"), path, text, resolver)
: null;
throw new Error(
`line ${edit.lineNum}: ${blockSingleLineMessage(edit.anchor.line, op, enclosingBlock ?? undefined)}`,
);
}
options.onResolved?.({
anchorLine: edit.anchor.line,
+79 -8
View File
@@ -1,6 +1,7 @@
/** Centralized error/warning text for the hashline parser, applier, and patcher. */
import { formatNumberedLine, HL_FILE_HASH_SEP, HL_FILE_PREFIX, HL_FILE_SUFFIX, HL_RANGE_SEP } from "./format";
import type { BlockSpan } from "./types";
/** Lines of context shown either side of a hash mismatch. */
export const MISMATCH_CONTEXT = 2;
@@ -29,6 +30,39 @@ export function formatAnchoredContext(anchorLines: readonly number[], fileLines:
}
return rows;
}
/** Concrete range operation rejected because its absolute end precedes its start. */
export type AbsoluteRangeOp = "replace" | "delete";
/** Explain absolute range endpoints and provide safe, non-applying retry forms. */
export function invalidAbsoluteRangeMessage(
patchLine: number,
start: number,
end: number,
op: AbsoluteRangeOp,
block?: BlockSpan,
): string {
const single = op === "replace" ? `SWAP ${start}${HL_RANGE_SEP}${start}:` : `DEL ${start}`;
const countedEnd = start + end - 1;
const counted =
Number.isSafeInteger(countedEnd) && countedEnd >= start
? op === "replace"
? `SWAP ${start}${HL_RANGE_SEP}${countedEnd}:`
: `DEL ${start}${HL_RANGE_SEP}${countedEnd}`
: null;
const blockForm = op === "replace" ? `SWAP.BLK ${start}:` : `DEL.BLK ${start}`;
let message =
`line ${patchLine}: Invalid absolute range: start ${start}, end ${end}. ` +
`The value after \`${HL_RANGE_SEP}\` is an absolute source line, not a line count or replacement length. ` +
`For one line use \`${single}\`.`;
if (counted !== null) {
message += ` For ${end} lines starting at ${start}, use \`${counted}\`.`;
}
if (block?.start === start && block.end > start) {
message +=
` The syntactic block beginning at ${start} ends at ${block.end}, ` + `so \`${blockForm}\` is also valid.`;
}
return message;
}
/** Optional patch envelope start marker; silently consumed. */
export const BEGIN_PATCH_MARKER = "*** Begin Patch";
@@ -70,6 +104,14 @@ export const EMPTY_REPLACE = `\`SWAP N${HL_RANGE_SEP}M:\` needs at least one \`+
/** `replace_block N:` hunk with no body. */
export const EMPTY_BLOCK = "`SWAP.BLK N:` needs at least one `+TEXT` body row. To delete a block, use `DEL.BLK N`.";
/** Optional source-aware suggestions appended to block-anchor diagnostics. */
export interface BlockDiagnosticSuggestions {
/** Closest following multi-line block that begins after the authored anchor. */
nextBlock?: BlockSpan;
/** Closest preceding multi-line block whose span contains the authored anchor. */
enclosingBlock?: BlockSpan;
}
/**
* Block-anchored replace/delete could not resolve to a syntactic block
* (unsupported language, blank/out-of-range line, no node beginning on N, or
@@ -82,12 +124,31 @@ export function blockUnresolvedMessage(
line: number,
op: "replace" | "delete" = "replace",
fileLines?: readonly string[],
suggestions: BlockDiagnosticSuggestions = {},
): string {
const phrase = op === "delete" ? `DEL.BLK ${line}` : `SWAP.BLK ${line}:`;
const fallback = op === "delete" ? `DEL ${line}${HL_RANGE_SEP}M` : `SWAP ${line}${HL_RANGE_SEP}M:`;
let message =
`\`${phrase}\` could not resolve a syntactic block beginning on line ${line} ` +
`(unsupported language, blank/closer line, or parse error). Use \`${fallback}\` with explicit lines.`;
const anchorText = fileLines?.[line - 1];
const nextBlock = suggestions.nextBlock;
let message: string;
if (anchorText !== undefined && anchorText.trim().length === 0 && nextBlock) {
const retry = op === "delete" ? `DEL.BLK ${nextBlock.start}` : `SWAP.BLK ${nextBlock.start}:`;
message =
`Line ${line} is blank; no syntactic block can begin there. ` +
`The next multi-line block begins at line ${nextBlock.start} and ends at line ${nextBlock.end}. ` +
`Retry \`${retry}\`.`;
} else {
message =
`\`${phrase}\` could not resolve a syntactic block beginning on line ${line} ` +
`(unsupported language, blank/closer line, or parse error). Use \`${fallback}\` with explicit lines.`;
}
const enclosingBlock = suggestions.enclosingBlock;
if (enclosingBlock) {
const retry = op === "delete" ? `DEL.BLK ${enclosingBlock.start}` : `SWAP.BLK ${enclosingBlock.start}:`;
message +=
` The nearest enclosing multi-line block begins at line ${enclosingBlock.start} ` +
`and ends at line ${enclosingBlock.end}; use \`${retry}\` to target it.`;
}
if (fileLines) {
const context = formatAnchoredContext([line], fileLines);
if (context.length > 0) message += `\n\n${context.join("\n")}`;
@@ -344,7 +405,7 @@ export type BlockOp = "replace" | "delete" | "insert_after";
* form only earns its keep when it spares counting a closing line you cannot
* see. Reject and point at both fixes.
*/
export function blockSingleLineMessage(line: number, op: BlockOp): string {
export function blockSingleLineMessage(line: number, op: BlockOp, enclosingBlock?: BlockSpan): string {
const blockForm = op === "insert_after" ? "INS.BLK.POST" : op === "delete" ? "DEL.BLK" : "SWAP.BLK";
const plainForm =
op === "insert_after"
@@ -352,9 +413,19 @@ export function blockSingleLineMessage(line: number, op: BlockOp): string {
: op === "delete"
? `DEL ${line}`
: `SWAP ${line}${HL_RANGE_SEP}${line}:`;
return (
let message =
`\`${blockForm} ${line}\` resolved a single-line block — line ${line} is a bare statement, not the opening line ` +
`of a multi-line construct. For that one line use \`${plainForm}\`; to act on an enclosing construct, anchor ${blockForm} ` +
`on the line that OPENS it (e.g. its \`function\`/\`if\`/\`case\` header), never a statement inside it.`
);
`of a multi-line construct. For only this statement use \`${plainForm}\`.`;
if (enclosingBlock) {
const enclosingForm =
op === "insert_after"
? `INS.BLK.POST ${enclosingBlock.start}:`
: op === "delete"
? `DEL.BLK ${enclosingBlock.start}`
: `SWAP.BLK ${enclosingBlock.start}:`;
message +=
` The nearest enclosing multi-line block begins at line ${enclosingBlock.start} ` +
`and ends at line ${enclosingBlock.end}; use \`${enclosingForm}\` to target it.`;
}
return message;
}
+31 -6
View File
@@ -5,11 +5,13 @@
*/
import { HL_PAYLOAD_REPLACE, HL_RANGE_SEP } from "./format";
import {
type AbsoluteRangeOp,
BARE_BODY_AUTO_PIPED_WARNING,
DELETE_BLOCK_TAKES_NO_BODY,
DELETE_TAKES_NO_BODY,
EMPTY_BLOCK,
EMPTY_INSERT,
invalidAbsoluteRangeMessage,
MINUS_BULLET_AUTO_PIPED_WARNING,
MINUS_ROW_REJECTED,
MOVE_TAKES_NO_BODY,
@@ -17,13 +19,36 @@ import {
} from "./messages";
import { stripOneLeadingHashlinePrefix } from "./prefixes";
import { type BlockTarget, cloneCursor, type ParsedRange, type Token, Tokenizer } from "./tokenizer";
import type { Anchor, Cursor, Edit, FileOp } from "./types";
import type { Anchor, BlockSpan, Cursor, Edit, FileOp } from "./types";
/** Parser error carrying enough range metadata for source-aware diagnostic enrichment. */
export class InvalidAbsoluteRangeError extends Error {
/** Patch-language line containing the invalid range header. */
readonly patchLine: number;
/** Absolute first source line authored in the range. */
readonly startLine: number;
/** Invalid absolute last source line authored in the range. */
readonly endLine: number;
/** Operation whose range was invalid. */
readonly op: AbsoluteRangeOp;
function validateRangeOrder(range: ParsedRange, lineNum: number): void {
constructor(patchLine: number, startLine: number, endLine: number, op: AbsoluteRangeOp, block?: BlockSpan) {
super(invalidAbsoluteRangeMessage(patchLine, startLine, endLine, op, block));
this.name = "InvalidAbsoluteRangeError";
this.patchLine = patchLine;
this.startLine = startLine;
this.endLine = endLine;
this.op = op;
}
/** Rebuild this error with a proven syntactic-block endpoint suggestion. */
withBlock(block: BlockSpan): InvalidAbsoluteRangeError {
return new InvalidAbsoluteRangeError(this.patchLine, this.startLine, this.endLine, this.op, block);
}
}
function validateRangeOrder(range: ParsedRange, lineNum: number, op: AbsoluteRangeOp): void {
if (range.end.line < range.start.line) {
throw new Error(
`line ${lineNum}: range ${range.start.line}${HL_RANGE_SEP}${range.end.line} ends before it starts.`,
);
throw new InvalidAbsoluteRangeError(lineNum, range.start.line, range.end.line, op);
}
}
@@ -170,7 +195,7 @@ export class Executor {
case "op-block":
this.#discardPendingSkippableComments();
if (token.target.kind === "replace" || token.target.kind === "delete") {
validateRangeOrder(token.target.range, token.lineNum);
validateRangeOrder(token.target.range, token.lineNum, token.target.kind);
}
if (token.target.kind === "rem") {
this.#flushPending();
+22 -2
View File
@@ -38,9 +38,10 @@ import {
} from "./messages";
import { MismatchError } from "./mismatch";
import { detectLineEnding, type LineEnding, normalizeToLF, restoreLineEndings, stripBom } from "./normalize";
import { InvalidAbsoluteRangeError } from "./parser";
import { Recovery, type RecoveryResult } from "./recovery";
import type { Snapshot, SnapshotStore } from "./snapshots";
import type { ApplyResult, BlockResolution, BlockResolver, Edit, FileOp } from "./types";
import type { ApplyResult, BlockResolution, BlockResolver, BlockSpan, Edit, FileOp } from "./types";
/**
* Upper bound on the number of unseen anchor lines whose actual file content
@@ -277,6 +278,25 @@ export class Patcher {
}
}
async #parseWithRangeDiagnostics(section: PatchSection) {
try {
return section.parse();
} catch (error) {
if (!(error instanceof InvalidAbsoluteRangeError) || !this.blockResolver) throw error;
let span: BlockSpan | null = null;
try {
const read = await this.#tryRead(section.path);
if (read.exists) {
const normalized = normalizeToLF(stripBom(read.rawContent).text);
span = this.blockResolver({ path: section.path, text: normalized, line: error.startLine });
}
} catch {
// Source-aware enrichment is best-effort; preserve the actionable parser error.
}
throw span?.start === error.startLine && span.end > span.start ? error.withBlock(span) : error;
}
}
/**
* Read a section's target file, parse the section, validate the snapshot
* tag (with recovery), and apply the edits in memory. Returns a
@@ -287,7 +307,7 @@ export class Patcher {
* tag mismatch ({@link MismatchError}).
*/
async prepare(section: PatchSection): Promise<PreparedSection> {
const parsed = section.parse();
const parsed = await this.#parseWithRangeDiagnostics(section);
const parseWarnings = [...parsed.warnings];
const fileOp = parsed.fileOp;
assertSectionHashPresent(section.path, section.fileHash);
+12 -8
View File
@@ -171,14 +171,18 @@ function scanRangeSeparator(line: string, index: number, end: number): number |
consumedSeparator = true;
continue;
}
if (
code === CHAR_DOT &&
cursor + 1 < end &&
(line.charCodeAt(cursor + 1) === CHAR_DOT || line.charCodeAt(cursor + 1) === CHAR_EQUALS)
) {
cursor += 2;
consumedSeparator = true;
continue;
if (code === CHAR_DOT && cursor + 1 < end) {
const next = line.charCodeAt(cursor + 1);
if (next === CHAR_DOT || next === CHAR_EQUALS) {
cursor += 2;
consumedSeparator = true;
continue;
}
if (isNonZeroDigitCode(next)) {
cursor++;
consumedSeparator = true;
continue;
}
}
break;
}
+38
View File
@@ -104,6 +104,30 @@ describe("resolveBlockEdits", () => {
expect(error?.message).not.toContain("foxtrot");
});
it("suggests the next multi-line opener for a blank anchor without applying it", () => {
const text = "alpha\n\nfunction x() {\n return 1;\n}";
const resolver: BlockResolver = ({ line }) => (line === 3 ? { start: 3, end: 5 } : null);
const edits = parsePatch("SWAP.BLK 2:\n+function y() {}").edits;
expect(() => resolveBlockEdits(edits, text, PATH, resolver)).toThrow(
"Line 2 is blank; no syntactic block can begin there. The next multi-line block begins at line 3 and ends at line 5. Retry `SWAP.BLK 3:`.",
);
});
it("suggests both the exact statement range and nearest enclosing block", () => {
const text = "function x() {\n run();\n}";
const resolver: BlockResolver = ({ line }) => {
if (line === 2) return { start: 2, end: 2 };
if (line === 1) return { start: 1, end: 3 };
return null;
};
const edits = parsePatch("SWAP.BLK 2:\n+ stop();").edits;
expect(() => resolveBlockEdits(edits, text, PATH, resolver)).toThrow(
"For only this statement use `SWAP 2.=2:`. The nearest enclosing multi-line block begins at line 1 and ends at line 3; use `SWAP.BLK 1:` to target it.",
);
});
it("omits the context preview when the anchor line is out of range", () => {
const edits = parsePatch("SWAP.BLK 9:\n+X").edits;
let error: Error | undefined;
@@ -219,6 +243,20 @@ describe("Patcher with a block resolver", () => {
expect(result.sections[0]?.blockResolutions).toEqual([{ anchorLine: 2, start: 2, end: 3, op: "replace" }]);
});
it("enriches reversed absolute ranges with the resolved block endpoint without writing", async () => {
const source = Array.from({ length: 255 }, (_, index) => `line ${index + 1}`).join("\n");
const fs = new InMemoryFilesystem([[PATH, source]]);
const snapshots = new InMemorySnapshotStore();
const tag = snapshots.record(PATH, source);
const resolver: BlockResolver = ({ line }) => (line === 195 ? { start: 195, end: 255 } : null);
const patcher = new Patcher({ fs, snapshots, blockResolver: resolver });
await expect(patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 195.=61:\n+replacement`))).rejects.toThrow(
"Invalid absolute range: start 195, end 61. The value after `.=` is an absolute source line, not a line count or replacement length. For one line use `SWAP 195.=195:`. For 61 lines starting at 195, use `SWAP 195.=255:`. The syntactic block beginning at 195 ends at 255, so `SWAP.BLK 195:` is also valid.",
);
expect(fs.get(PATH)).toBe(source);
});
it("resolves against the tagged snapshot and recovers onto drifted content", async () => {
const snapshotText = "line0\nline1\nline2\nline3\nline4\n";
// The live file gained a trailing line after the read minted the tag.
+6
View File
@@ -23,6 +23,12 @@ describe("hashline format v4", () => {
expect(applyPatch(text, "DEL 2.=3")).toBe("a\nd");
});
it("accepts a single dot between integer range endpoints", () => {
const text = "a\nb\nc\nd";
expect(applyPatch(text, "DEL 2.3")).toBe("a\nd");
expect(applyPatch(text, "SWAP 2.3:\n+middle")).toBe("a\nmiddle\nd");
});
it("inserts before and after concrete anchors", () => {
const text = "a\nb\nc";
const diff = ["INS.PRE 2:", "+before", "INS.POST 2:", "+after"].join("\n");