diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index 6e6624903..61e113dcb 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -17,10 +17,11 @@ import type { ToolSession } from "../tools"; import { VimTool, vimSchema } from "../tools/vim"; import { type EditMode, normalizeEditMode, resolveEditMode } from "../utils/edit-mode"; import type { VimToolDetails } from "../vim/types"; +import { resolveLarkLidPlaceholders } from "./line-hash"; import { type ApplyPatchParams, applyPatchSchema, expandApplyPatchToEntries } from "./modes/apply-patch"; import applyPatchGrammar from "./modes/apply-patch.lark" with { type: "text" }; import { type AtomParams, atomEditParamsSchema, executeAtomSingle } from "./modes/atom"; -import atomGrammar from "./modes/atom.lark" with { type: "text" }; +import atomGrammarTemplate from "./modes/atom.lark" with { type: "text" }; import { executeHashlineSingle, HashlineMismatchError, @@ -36,6 +37,13 @@ export { DEFAULT_EDIT_MODE, type EditMode, normalizeEditMode } from "../utils/ed export * from "./apply-patch"; export * from "./diff"; export * from "./line-hash"; + +// Resolve the `$HASHFMT$` placeholder in the atom Lark grammar against +// the central lax-hash regex source. Keeps the grammar's LID rule in lockstep +// with atom.ts and hashline.ts — every consumer derives the alphabet from one +// place in line-hash.ts. +const atomGrammar = resolveLarkLidPlaceholders(atomGrammarTemplate); + export * from "./modes/apply-patch"; export * from "./modes/atom"; export * from "./modes/hashline"; diff --git a/packages/coding-agent/src/edit/line-hash.ts b/packages/coding-agent/src/edit/line-hash.ts index 1ddc1a2d5..febc333f6 100644 --- a/packages/coding-agent/src/edit/line-hash.ts +++ b/packages/coding-agent/src/edit/line-hash.ts @@ -668,66 +668,174 @@ export const HASHLINE_BIGRAMS = [ export const HASHLINE_BIGRAMS_COUNT = HASHLINE_BIGRAMS.length; /** - * Regex source matching exactly one bigram from {@link HASHLINE_BIGRAMS}. - * Used by hashline parsers — keep in sync with the alphabet array above. + * Regex source matching one valid hashline anchor hash. Matches either: + * - a 2-letter BPE bigram from {@link HASHLINE_BIGRAMS} (the default + * 1-token-per-anchor alphabet for ordinary lines), or + * - `>[a-z]` for lines whose trimmed content starts with `}` + * (closing-brace marker — uses `>` so the hash never visually collides + * with a literal `}` in line content), or + * - `[a-z]<` for lines whose trimmed content ends with `{` + * (opening-brace marker — uses `<` so the hash never visually collides + * with a literal `{` in line content). + * + * Brace markers cost ~2-3 BPE tokens instead of 1; the structural signal is + * the explicit tradeoff. Keep in sync with {@link computeLineHash}. */ -export const HASHLINE_BIGRAM_RE_SRC = `(?:${HASHLINE_BIGRAMS.join("|")})`; +export const HASHLINE_HASH_RE_SRC = `(?:${HASHLINE_BIGRAMS.join("|")}|>[a-z]|[a-z]<)`; + +/** + * Lax hash regex source — same shape as {@link HASHLINE_HASH_RE_SRC} but + * accepts any `[a-z]{2}` for the bigram arm instead of restricting to the + * 647 BPE bigrams. Parsers that need to admit syntactically well-formed but + * stale Lids use this form so the downstream content-fingerprint check can + * emit a targeted hash-mismatch error rather than an opaque parse error. + * + * Keep in sync with {@link HASHLINE_HASH_RE_SRC} — the only legitimate + * difference is the bigram arm. Update both whenever the hash shape changes. + */ +export const HASHLINE_HASH_LAX_RE_SRC = `(?:[a-z]{2}|>[a-z]|[a-z]<)`; + +/** + * Decoration prefix that may precede a `LINE+HASH` anchor in tool output: + * `>` (context line in grep), `+` (added line in diff), `-` (removed line), + * `*` (match line). Any combination, in any order, surrounded by optional + * whitespace. Output formatters emit at most one decoration per anchor; the + * regex stays liberal because anchor-ref parsers accept whatever the model + * echoes back. + */ +export const HASHLINE_ANCHOR_DECORATION_RE_SRC = `\\s*[>+\\-*]*\\s*`; + +/** + * Capture-group regex source for a decorated `LINE+HASH` anchor. Group 1 + * captures the line number (digits only); group 2 captures the strict hash + * from {@link HASHLINE_HASH_RE_SRC}. The source is intentionally unanchored + * — anchoring with `^` (or composing into a larger pattern) is the caller's + * responsibility. + */ +export const HASHLINE_ANCHOR_RE_SRC = `${HASHLINE_ANCHOR_DECORATION_RE_SRC}(\\d+)(${HASHLINE_HASH_RE_SRC})`; + +/** + * Bare `LINE+HASH` Lid (no decorations, no captures, no anchors). Use for + * embedding inside larger patterns where the line+hash unit appears as a + * literal (e.g. range bounds, alternation arms, op-line heuristics). Strict + * variant — only hashes that {@link computeLineHash} actually produces match. + */ +export const HASHLINE_LID_RE_SRC = `[1-9]\\d*${HASHLINE_HASH_RE_SRC}`; + +/** + * Lax variant of {@link HASHLINE_LID_RE_SRC}: same shape, accepts any + * `[a-z]{2}` for the bigram arm. Parsers admit syntactically well-formed + * but stale Lids and let the downstream hash-fingerprint check emit a + * targeted mismatch error. + */ +export const HASHLINE_LID_LAX_RE_SRC = `[1-9]\\d*${HASHLINE_HASH_LAX_RE_SRC}`; + +/** + * Capture-group form of {@link HASHLINE_LID_RE_SRC}: group 1 captures the + * line number, group 2 captures the strict hash. + */ +export const HASHLINE_LID_CAPTURE_RE_SRC = `([1-9]\\d*)(${HASHLINE_HASH_RE_SRC})`; + +/** + * Capture-group form of {@link HASHLINE_LID_LAX_RE_SRC}: group 1 captures + * the line number, group 2 captures the lax hash. + */ +export const HASHLINE_LID_LAX_CAPTURE_RE_SRC = `([1-9]\\d*)(${HASHLINE_HASH_LAX_RE_SRC})`; + +/** Width of a hash in display characters. */ +export const HASHLINE_HASH_WIDTH = 2; + +/** Human-readable hash-width label used in user-facing copy (e.g. "2-character"). */ +export const HASHLINE_HASH_WIDTH_LABEL = `${HASHLINE_HASH_WIDTH}-character`; + +/** + * Representative hash suffixes for use in user-facing error messages and + * prompt examples. Cycles through one bigram form, one closing-brace marker, + * and one opening-brace marker so callers can show the full alphabet shape + * with a single helper. + */ +export const HASHLINE_HASH_EXAMPLES = ["sr", ">t", "a<"] as const; + +/** + * Format a comma-separated list of example anchors with an optional line-number + * prefix, quoted for inclusion in error messages: `"160sr", "160>t", "160a<"`. + */ +export function describeAnchorExamples(linePrefix = ""): string { + return HASHLINE_HASH_EXAMPLES.map(e => `"${linePrefix}${e}"`).join(", "); +} + +/** + * Sentinel token that {@link atomGrammar} (atom.lark) uses for the lax hash + * regex source. Replaced at module-load time by {@link resolveLarkLidPlaceholders} + * so the grammar is re-derived from a single source of truth alongside its + * TypeScript consumers. Update both atoms whenever the placeholder name changes. + */ +export const LARK_LID_HASH_LAX_PLACEHOLDER = "$HASHFMT$"; + +/** + * Substitute the LID hash placeholder in a Lark grammar text with the + * central lax-hash regex source. Grammars that don't reference Lids pass + * through unchanged. + */ +export function resolveLarkLidPlaceholders(grammar: string): string { + return grammar.split(LARK_LID_HASH_LAX_PLACEHOLDER).join(HASHLINE_HASH_LAX_RE_SRC); +} export const HASHLINE_CONTENT_SEPARATOR = "|"; const RE_SIGNIFICANT = /[\p{L}\p{N}]/u; -const RE_STRUCTURAL_STRIP = /[\s{}]/g; /** - * Bigram returned for lines that contain only whitespace and `{`/`}`. - * Picks the English ordinal suffix for the line number (`1` → `st`, - * `2` → `nd`, `3` → `rd`, `11`/`12`/`13` → `th`, else `th`) so the - * line digits + bigram BPE-merge into a single ordinal token (`1st`, `42nd`, - * `100th`, …). Brace-only lines therefore cost one token for the whole - * `LINE+ID` anchor instead of two. + * Pick a single lowercase letter (`a`-`z`) from xxHash32 of `line`. Mirrors the + * seed rule used by {@link computeLineHash} for the bigram path: lines with no + * letter or digit (e.g. bare `}` / `{`) mix in the line number so adjacent + * brace-only lines hash differently; lines with significant content stay + * line-number-independent. */ -function structuralBigram(line: number): string { - const mod100 = line % 100; - if (mod100 >= 11 && mod100 <= 13) return "th"; - switch (line % 10) { - case 1: - return "st"; - case 2: - return "nd"; - case 3: - return "rd"; - default: - return "th"; - } +function braceLetter(line: string, idx: number): string { + const seed = RE_SIGNIFICANT.test(line) ? 0 : idx; + return String.fromCharCode(97 + (Bun.hash.xxHash32(line, seed) % 26)); } /** - * Compute a short BPE-bigram hash of a single line. + * Compute a 2-character hash of a single line. + * + * - Lines whose trimmed content starts with `}` (closing brace) hash to + * `>[a-z]` (one of `>a`..`>z`). + * - Lines whose trimmed content ends with `{` (opening brace) hash to + * `[a-z]<` (one of `a<`..`z<`). When both apply (e.g. `} else {`), the + * opening form wins — the new block is what needs tracking. + * - All other lines map through {@link HASHLINE_BIGRAMS} via xxHash32 mod 647. + * + * For brace markers and other lines containing no letter/digit, the line + * number is mixed into the seed so adjacent identical lines (e.g. consecutive + * `}` lines) get distinct hashes; significant content stays + * line-number-independent so a line is identifiable across small shifts. * - * Uses xxHash32 on a trailing-whitespace-trimmed, CR-stripped line, mapped into - * {@link HASHLINE_BIGRAMS} via modulo. Lines that contain only whitespace and - * `{`/`}` collapse to an ordinal-suffix bigram (see {@link structuralBigram}) - * so brace-only structure shares one merged ordinal token. For other lines - * containing no alphanumeric characters, the line number is mixed in to reduce hash collisions. * The line input should not include a trailing newline. */ export function computeLineHash(idx: number, line: string): string { line = line.replace(/\r/g, "").trimEnd(); - if (line.replace(RE_STRUCTURAL_STRIP, "").length === 0) { - return structuralBigram(idx); + const trimmedStart = line.trimStart(); + const endsOpen = line.endsWith("{"); + const startsClose = trimmedStart.startsWith("}"); + + if (endsOpen) { + return `${braceLetter(line, idx)}<`; + } + if (startsClose) { + return `>${braceLetter(line, idx)}`; } - let seed = 0; - if (!RE_SIGNIFICANT.test(line)) { - seed = idx; - } + const seed = RE_SIGNIFICANT.test(line) ? 0 : idx; return HASHLINE_BIGRAMS[Bun.hash.xxHash32(line, seed) % HASHLINE_BIGRAMS_COUNT]; } /** * Formats an anchor reference given a line number and its text. - * Returns `LINE+ID` (e.g., `42nd`) — no separator between number and bigram. + * Returns `LINE+ID` (e.g., `42sr`, `42>t`, or `42a<`) — no separator between + * number and hash. */ export function formatLineHash(line: number, lines: string): string { return `${line}${computeLineHash(line, lines)}`; @@ -735,7 +843,7 @@ export function formatLineHash(line: number, lines: string): string { /** * Formats a single line with a hashline anchor. - * Returns `LINE+ID|TEXT` (e.g., `42nd|function hi() {\n2er| return;\n3in|}`) + * Returns `LINE+ID|TEXT` (e.g., `42sr|function hi() {`, `3>p|}`). */ export function formatHashLine(lineNumber: number, line: string): string { return `${lineNumber}${computeLineHash(lineNumber, line)}${HASHLINE_CONTENT_SEPARATOR}${line}`; @@ -754,7 +862,7 @@ export function formatHashLine(lineNumber: number, line: string): string { * @example * ``` * formatHashLines("function hi() {\n return;\n}") - * // "1th|function hi() {\n2er| return;\n3in|}" + * // "1a<|function hi() {\n2er| return;\n3>p|}" * ``` */ export function formatHashLines(text: string, startLine = 1): string { diff --git a/packages/coding-agent/src/edit/modes/atom.lark b/packages/coding-agent/src/edit/modes/atom.lark index 476c062d0..867fe8b7a 100644 --- a/packages/coding-agent/src/edit/modes/atom.lark +++ b/packages/coding-agent/src/edit/modes/atom.lark @@ -8,8 +8,8 @@ file_header: "---" filename LF filename: /(.+)/ line_change: line* mutation_line line* -line: insert_line | delete_line | set_block | move_line | blank -mutation_line: insert_line | delete_line | set_block +line: insert_line | delete_line | set_line | move_line | blank +mutation_line: insert_line | delete_line | set_line whole_file_change: blank* whole_file_line blank* whole_file_line: remove_file | move_file @@ -20,10 +20,8 @@ destination: /(?:[^ \t\r\n]+|"[^"\r\n]+"|'[^'\r\n]+')/ insert_line: "+" /(.*)/ LF delete_line: "-" (LID ".." LID | LID) LF set_line: (LID ".." LID | LID) WS? "=" /(.*)/ LF -set_block: set_line continuation_line* -continuation_line: "\\" /(.*)/ LF move_line: ("@" LID | "^" LID | "^" | "$") LF -LID: /[1-9][0-9]*[a-z]{2}/ +LID: /[1-9][0-9]*$HASHFMT$/ WS: /[ \t]+/ blank: LF diff --git a/packages/coding-agent/src/edit/modes/atom.ts b/packages/coding-agent/src/edit/modes/atom.ts index c77284884..49ecd8043 100644 --- a/packages/coding-agent/src/edit/modes/atom.ts +++ b/packages/coding-agent/src/edit/modes/atom.ts @@ -7,11 +7,12 @@ * @Lid move cursor to just after the anchored line * Lid=TEXT set the anchored line to TEXT and move cursor after it * -Lid delete the anchored line and move cursor to its slot - * LidA..LidB=TEXT replace a range; following \TEXT lines continue it - * \TEXT append TEXT to the active replacement (set or range) - * +TEXT insert TEXT at the cursor + * LidA..LidB=TEXT replace a range with one line; following `+TEXT` lines extend it + * +TEXT insert TEXT at the cursor (or extend the slot left by a preceding + * `Lid=…` / `LidA..LidB=…` op, since the cursor sits there) * ^ move cursor to beginning of file * $ move cursor to end of file + * ^Lid move cursor BEFORE the anchored line */ import * as fs from "node:fs/promises"; @@ -30,7 +31,13 @@ import { import { outputMeta } from "../../tools/output-meta"; import { enforcePlanModeWrite, resolvePlanPath } from "../../tools/plan-mode-guard"; import { generateDiffString } from "../diff"; -import { computeLineHash } from "../line-hash"; +import { + computeLineHash, + HASHLINE_HASH_LAX_RE_SRC, + HASHLINE_HASH_WIDTH_LABEL, + HASHLINE_LID_LAX_CAPTURE_RE_SRC, + HASHLINE_LID_LAX_RE_SRC, +} from "../line-hash"; import { detectLineEnding, normalizeToLF, restoreLineEndings, stripBom } from "../normalize"; import type { EditToolDetails, LspBatchRequest } from "../renderer"; import { @@ -54,10 +61,14 @@ export type AtomParams = Static; // Parser // ═══════════════════════════════════════════════════════════════════════════ -// Permissive: any 2 lowercase letters. Invalid hashes flow through to a -// HashlineMismatchError downstream, matching the other hashline-backed modes. -const LID_RE = /^([1-9]\d*)([a-z]{2})/; -const LID_EXACT_RE = /^([1-9]\d*)([a-z]{2})$/; +// All Lid-shape regexes derive from {@link HASHLINE_LID_LAX_RE_SRC} (and its +// capture-group sibling {@link HASHLINE_LID_LAX_CAPTURE_RE_SRC}). Atom uses +// the lax form so a syntactically well-formed but stale Lid (line was edited +// since read) flows through to a HashlineMismatchError downstream instead of +// an opaque parse error. Every Lid-shape change lives in line-hash.ts; the +// atom parser composes against the centralized sources below. +const LID_RE = new RegExp(`^${HASHLINE_LID_LAX_CAPTURE_RE_SRC}`); +const LID_EXACT_RE = new RegExp(`^${HASHLINE_LID_LAX_CAPTURE_RE_SRC}$`); // Sentinel hash used for interior line anchors synthesized from `-LidA..LidB` // range deletes. validateAtomAnchors recognizes this and skips hash checking @@ -228,7 +239,7 @@ function parseDeleteStmt(body: string, lineNum: number): ParsedStmt[] | null { const trimmedBody = body.trimStart(); // Range delete: `-LidA..LidB` deletes the contiguous range LidA..LidB inclusive. - const rangeRe = /^([1-9]\d*)([a-z]{2})\.\.([1-9]\d*)([a-z]{2})$/; + const rangeRe = new RegExp(`^${HASHLINE_LID_LAX_CAPTURE_RE_SRC}\\.\\.${HASHLINE_LID_LAX_CAPTURE_RE_SRC}$`); const rangeMatch = rangeRe.exec(trimmedBody); if (rangeMatch) { const startLine = Number.parseInt(rangeMatch[1], 10); @@ -262,7 +273,9 @@ function parseDeleteStmt(body: string, lineNum: number): ParsedStmt[] | null { // `|` (delete-with-old) form. Models reach for these when trying to // "delete the range and replace with TEXT" — point at the `LidA..LidB=TEXT` // shorthand instead. - const rangeWithSuffix = /^([1-9]\d*)([a-z]{2})\.\.([1-9]\d*)([a-z]{2})[ \t]*[=|](.*)$/.exec(trimmedBody); + const rangeWithSuffix = new RegExp( + `^${HASHLINE_LID_LAX_CAPTURE_RE_SRC}\\.\\.${HASHLINE_LID_LAX_CAPTURE_RE_SRC}[ \\t]*[=|](.*)$`, + ).exec(trimmedBody); if (rangeWithSuffix) { const lidA = `${rangeWithSuffix[1]}${rangeWithSuffix[2]}`; const lidB = `${rangeWithSuffix[3]}${rangeWithSuffix[4]}`; @@ -313,7 +326,7 @@ function throwMalformedLidDiagnostic(line: string, lineNum: number, raw: string) const withoutMove = withoutLegacyMove.startsWith("@") ? withoutLegacyMove.slice(1) : withoutLegacyMove; const withoutDelete = withoutMove.startsWith("-") ? withoutMove.slice(1).trimStart() : withoutMove; - const partial = /^([a-z]{2})(?=[ \t]*[=|])/.exec(withoutDelete); + const partial = new RegExp(`^(${HASHLINE_HASH_LAX_RE_SRC})(?=[ \\t]*[=|])`).exec(withoutDelete); if (partial) { throw new Error( `Diff line ${lineNum}: \`${partial[1]}\` is not a full Lid. Use the full Lid from read output, e.g. \`119${partial[1]}\`.`, @@ -324,7 +337,7 @@ function throwMalformedLidDiagnostic(line: string, lineNum: number, raw: string) if (missing) { const prefix = text.startsWith("@@ ") ? `@@ ${missing[1]}` : missing[1]; throw new Error( - `Diff line ${lineNum}: \`${prefix}\` is missing the two-letter Lid suffix. Use the full Lid from read output, e.g. \`${prefix.startsWith("@@ ") ? "@@ " : ""}${missing[1]}ab\`.`, + `Diff line ${lineNum}: \`${prefix}\` is missing the ${HASHLINE_HASH_WIDTH_LABEL} Lid suffix. Use the full Lid from read output, e.g. \`${prefix.startsWith("@@ ") ? "@@ " : ""}${missing[1]}ab\`.`, ); } @@ -341,6 +354,16 @@ function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { // inserts corrupts files, and the canonical syntax has no comment op. if (line[0] === "#") return []; + // `\TEXT` continuation has been removed. After a `Lid=…` or `LidA..LidB=…` + // op, the cursor sits on the just-set/just-deleted slot; the canonical way + // to extend a single-line set or range-replace into a multi-line replacement + // is to follow it with `+TEXT` lines, which insert at that cursor. + if (line[0] === "\\") { + throw new Error( + `Diff line ${lineNum}: \`\\TEXT\` continuation has been removed. Use \`+TEXT\` to insert content after a \`Lid=…\` or \`LidA..LidB=…\` op — the cursor sits on the just-set/just-deleted slot, so following \`+TEXT\` lines extend the replacement.`, + ); + } + const indentedHashline = parseIndentedHashlineStmt(line, lineNum); if (indentedHashline) return indentedHashline; @@ -349,9 +372,6 @@ function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { // emit a tagged stmt so the normalizer can fuse it with a preceding `-Lid`. if (line[0] === "+") { const body = line.slice(1); - if (body.startsWith(RANGE_CONTINUATION_SENTINEL)) { - return [{ kind: "insert", text: body.slice(RANGE_CONTINUATION_SENTINEL.length), lineNum }]; - } const m = LID_RE.exec(body); if (m) { const sep = body[m[0].length]; @@ -477,7 +497,7 @@ function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { const lidStmt = parseLidStmt(line, lineNum); if (lidStmt) return lidStmt; - if (/^[a-z]{2}(?=[ \t]*[=|])/.test(line) || /^[1-9]\d*(?=[ \t]*[=|]|$)/.test(line)) { + if (new RegExp(`^${HASHLINE_HASH_LAX_RE_SRC}(?=[ \\t]*[=|])`).test(line) || /^[1-9]\d*(?=[ \t]*[=|]|$)/.test(line)) { throwMalformedLidDiagnostic(line, lineNum, raw); } @@ -485,7 +505,7 @@ function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { // emitted multi-line content after a `Lid=` or similar without `+` prefixes, // or pasted raw context. Silently treating these as inserts corrupts files. const preview = line.length > 80 ? `${line.slice(0, 80)}…` : line; - const trailingDash = /^([1-9]\d*[a-z]{2})-\s*$/.exec(line); + const trailingDash = new RegExp(`^(${HASHLINE_LID_LAX_RE_SRC})-\\s*$`).exec(line); if (trailingDash) { throw new Error( `Diff line ${lineNum}: \`${line}\` looks like a delete with the operator on the wrong side. Use \`-${trailingDash[1]}\` to delete that line.`, @@ -496,94 +516,9 @@ function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { ); } -// Lines that look like recognized atom ops. Used to delimit range-replace -// recovery continuation: after `LidA..LidB=TEXT` (or legacy `|` separator), -// is treated as literal replacement text for backward compatibility. -const OP_LINE_HEAD_RE = /^([+\-@$^!]|[1-9]\d*[a-z]{2}|[ \t]*$)/; -const RANGE_CONTINUATION_SENTINEL = "\u0000"; - -function isRangeReplaceStart(line: string): boolean { - return /^[1-9]\d*[a-z]{2}\.\.[1-9]\d*[a-z]{2}[ \t]*[=|]/.test(line); -} - -// A single-line `Lid=TEXT` (or legacy `Lid|TEXT`, with optional leading `@`) -// also opens a replacement that `\TEXT` continuation lines may extend. The -// continuation lines become inserts at the cursor (which sits on the just-set -// line), turning `Lid=A` + `\B` + `\C` into "set the line to A, then insert B -// and C below it" — i.e. a multi-line rewrite of one anchor without forcing -// the user to switch to the `LidA..LidB=` range form. -function isReplaceStart(line: string): boolean { - if (isRangeReplaceStart(line)) return true; - const stripped = line.startsWith("@") ? line.slice(1) : line; - return /^[1-9]\d*[a-z]{2}[ \t]*[=|]/.test(stripped); -} - -// Lookahead used by the blank-line forgiveness rule below: returns true when -// the first non-blank line at or after `start` is a `\TEXT` continuation. -function nextNonBlankIsBackslash(lines: readonly string[], start: number): boolean { - for (let j = start; j < lines.length; j++) { - const peek = lines[j].endsWith("\r") ? lines[j].slice(0, -1) : lines[j]; - if (peek.length === 0) continue; - return peek.startsWith("\\"); - } - return false; -} - -// Explicit continuation uses `\TEXT` after a replacement op (`Lid=FIRST` or -// `LidA..LidB=FIRST`). The leading backslash is the continuation marker; the -// rest of the line is inserted literally, so `\\TEXT` inserts a line starting -// with `\TEXT`. As a forgiveness rule, a literal blank line inside an active -// replacement that is itself followed (possibly through more blanks) by another -// `\TEXT` continuation is treated as an implicit `\` blank insert — authors -// frequently drop a real blank between `\TEXT` lines instead of writing `\`. -// Raw unprefixed continuation remains an undocumented best-effort recovery for -// range replacements only, kept for old transcripts. -function preprocessRangeReplaceContinuation(diff: string): string { - const lines = diff.split("\n"); - let inRangeReplace = false; - let inReplace = false; - for (let i = 0; i < lines.length; i++) { - const rawLine = lines[i]; - const line = rawLine.endsWith("\r") ? rawLine.slice(0, -1) : rawLine; - - if (line.startsWith("\\")) { - if (!inReplace) { - throw new Error( - `Diff line ${i + 1}: \\TEXT continuation is only valid immediately after a Lid=TEXT or LidA..LidB=FIRST_LINE replacement.`, - ); - } - lines[i] = `+${RANGE_CONTINUATION_SENTINEL}${rawLine.slice(1)}`; - continue; - } - - // Forgiveness: a blank line inside an active replacement that is followed - // by another `\TEXT` continuation is treated as an implicit `\` blank - // insert. Keeps the replacement open across the blank. - if (inReplace && line.length === 0 && nextNonBlankIsBackslash(lines, i + 1)) { - lines[i] = `+${RANGE_CONTINUATION_SENTINEL}`; - continue; - } - - if (inRangeReplace) { - if (line.length === 0 || OP_LINE_HEAD_RE.test(line)) { - inRangeReplace = isRangeReplaceStart(line); - inReplace = isReplaceStart(line); - continue; - } - - lines[i] = `+${RANGE_CONTINUATION_SENTINEL}${rawLine}`; - continue; - } - - inRangeReplace = isRangeReplaceStart(line); - inReplace = isReplaceStart(line); - } - return lines.join("\n"); -} - function tokenizeDiff(diff: string): ParsedStmt[] { const out: ParsedStmt[] = []; - const lines = preprocessRangeReplaceContinuation(diff).split("\n"); + const lines = diff.split("\n"); for (let i = 0; i < lines.length; i++) { const lineNum = i + 1; const stmts = parseDiffLine(lines[i], lineNum); @@ -831,6 +766,7 @@ function getAtomEditAnchors(edit: AtomEdit): Anchor[] { function validateAtomAnchors(edits: AtomEdit[], fileLines: string[], warnings: string[]): HashMismatch[] { const mismatches: HashMismatch[] = []; const rebasedAnchors = new Map(); + const emittedRebaseKeys = new Set(); for (const edit of edits) { for (const anchor of getAtomEditAnchors(edit)) { if (anchor.line < 1 || anchor.line > fileLines.length) { @@ -845,9 +781,13 @@ function validateAtomAnchors(edits: AtomEdit[], fileLines: string[], warnings: s const original = `${anchor.line}${anchor.hash}`; rebasedAnchors.set(anchor, { line: anchor.line, expected: anchor.hash, actual: actualHash }); anchor.line = rebased; - warnings.push( - `Auto-rebased anchor ${original} → ${rebased}${anchor.hash} (line shifted within ±${ANCHOR_REBASE_WINDOW}; hash matched).`, - ); + const rebaseKey = `${original}→${rebased}${anchor.hash}`; + if (!emittedRebaseKeys.has(rebaseKey)) { + emittedRebaseKeys.add(rebaseKey); + warnings.push( + `Auto-rebased anchor ${original} → ${rebased}${anchor.hash} (line shifted within ±${ANCHOR_REBASE_WINDOW}; hash matched).`, + ); + } continue; } mismatches.push({ line: anchor.line, expected: anchor.hash, actual: actualHash }); @@ -889,6 +829,65 @@ function validateNoConflictingAtomMutations(edits: AtomEdit[]): void { } } +// Heuristic: warn when `@Lid` lands on a brace-opening line and the +// subsequent inserts are at sibling indent (≤ anchor indent), suggesting +// the agent meant `^` instead. The line still ends with `{` +// after the hash-marker swap (markers became `[a-z]<` / `>[a-z]`); the +// raw line content is unchanged, so this check works regardless of hash +// alphabet. +function detectAtomBracePositioningWarnings(edits: AtomEdit[], fileLines: string[]): string[] { + const warnings: string[] = []; + const seen = new Set(); + + const getLeadingWhitespace = (line: string): string => { + const m = /^\s*/.exec(line); + return m ? m[0] : ""; + }; + + const formatPreview = (text: string): string => (text.length > 60 ? `${text.slice(0, 60)}…` : text); + + for (const edit of edits) { + // Only `@Lid` (after-anchor) inserts are at risk. `^Lid` (before), + // BOF, EOF, and direct `set`/`delete` ops do not have the foot-gun. + if (edit.kind !== "insert" || edit.cursor.kind !== "anchor") continue; + + const anchor = edit.cursor.anchor; + if (seen.has(anchor.line)) continue; + if (anchor.line < 1 || anchor.line > fileLines.length) continue; + + const anchorLine = fileLines[anchor.line - 1]; + const trimmedEnd = anchorLine.trimEnd(); + if (!trimmedEnd.endsWith("{")) continue; + + // Find the first non-blank `+TEXT` after this `@Lid`. + const firstInsert = edits.find( + e => + e.kind === "insert" && + e.cursor.kind === "anchor" && + e.cursor.anchor.line === anchor.line && + e.text.trim().length > 0, + ); + if (!firstInsert || firstInsert.kind !== "insert") continue; + + const anchorIndent = getLeadingWhitespace(anchorLine); + const insertIndent = getLeadingWhitespace(firstInsert.text); + + // Body content is STRICTLY more indented than the brace-opening line. + // `startsWith(anchorIndent)` rejects mixed tab/space cases that look + // longer in chars but differ in actual indent shape. + const isProperlyNested = insertIndent.length > anchorIndent.length && insertIndent.startsWith(anchorIndent); + if (isProperlyNested) continue; + + seen.add(anchor.line); + const lid = `${anchor.line}${anchor.hash}`; + warnings.push( + `@${lid} inserts at indent ${insertIndent.length} just inside the brace-opening line "${formatPreview(trimmedEnd)}" (indent ${anchorIndent.length}); the inserted content sits inside the {...} block as the first body element. If you meant a sibling (after the closing brace), use \`^Lid\` on the next sibling line. If you meant body content, indent past column ${anchorIndent.length}.`, + ); + } + + return warnings; +} + function repairAtomOldNewSetLine(currentLine: string, nextLine: string): string { const marker = `${currentLine}|`; if (!nextLine.startsWith(marker)) return nextLine; @@ -1054,7 +1053,7 @@ function detectAndAutoFixDuplicates( return { fixed: trial, warnings: [ - `Auto-fixed: removed ${noun} ${previews}; the edit left adjacent identical lines and bracket balance was off. Verify the result.`, + `AUTO-FIX applied — verify the result. Removed ${noun} ${previews} to restore {}/()/[] balance the edit broke. If this is wrong, re-issue the edit so the duplicate is not produced in the first place.`, ], }; } @@ -1082,6 +1081,7 @@ export function applyAtomEdits(text: string, edits: AtomEdit[]): AtomApplyResult throw new HashlineMismatchError(mismatches, fileLines); } validateNoConflictingAtomMutations(edits); + for (const w of detectAtomBracePositioningWarnings(edits, fileLines)) warnings.push(w); const trackFirstChanged = (line: number) => { if (firstChangedLine === undefined || line < firstChangedLine) firstChangedLine = line; @@ -1345,17 +1345,23 @@ function hasAtomHeaderLine(input: string): boolean { return stripped.split("\n").some(rawLine => rawLine.replace(/\r$/, "").startsWith(FILE_HEADER_PREFIX)); } +const RECOGNIZABLE_OP_RES: readonly RegExp[] = [ + /^\$\+.*$/, + new RegExp(`^\\^${HASHLINE_LID_LAX_RE_SRC}(?:\\+.*)?$`), + new RegExp(`^- ?${HASHLINE_LID_LAX_RE_SRC}(?:\\.\\.${HASHLINE_LID_LAX_RE_SRC})?(?:[ \\t]*[=|].*| .*)?$`), + new RegExp(`^@?${HASHLINE_LID_LAX_RE_SRC}(?:\\+.*|[ \\t]*[=|].*|\\.\\.${HASHLINE_LID_LAX_RE_SRC}[ \\t]*=.*)?$`), + new RegExp(`^@@ (?:BOF|EOF|(?:- ?)?${HASHLINE_LID_LAX_RE_SRC}(?:[ \\t]*[=|].*)?)$`), +]; + function containsRecognizableAtomOperations(input: string): boolean { for (const rawLine of input.split("\n")) { const line = rawLine.replace(/\r$/, ""); if (line.length === 0) continue; if (line[0] === "+") return true; if (line === "$" || line === "^") return true; - if (/^\$\+.*$/.test(line)) return true; - if (/^\^[1-9]\d*[a-z]{2}(?:\+.*)?$/.test(line)) return true; - if (/^- ?[1-9]\d*[a-z]{2}(?:\.\.[1-9]\d*[a-z]{2})?(?:[ \t]*[=|].*| .*)?$/.test(line)) return true; - if (/^@?[1-9]\d*[a-z]{2}(?:\+.*|[ \t]*[=|].*|\.\.[1-9]\d*[a-z]{2}[ \t]*=.*)?$/.test(line)) return true; - if (/^@@ (?:BOF|EOF|(?:- ?)?[1-9]\d*[a-z]{2}(?:[ \t]*[=|].*)?)$/.test(line)) return true; + for (const re of RECOGNIZABLE_OP_RES) { + if (re.test(line)) return true; + } } return false; } diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index d936d25ef..c88b7cc25 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -2,14 +2,20 @@ * Hashline edit mode — a line-addressable edit format using text hashes. * * Each line in a file is identified by its 1-indexed line number and a short - * BPE-bigram hash derived from the normalized line text (xxHash32 mod 647, - * mapped through HASHLINE_BIGRAMS). + * structural hash derived from the normalized line text. The hash is + * normally a 2-letter BPE bigram (xxHash32 mod 647 → HASHLINE_BIGRAMS); for + * brace lines it carries a structural marker — `>[a-z]` for closing-brace + * lines (lines whose trimmed content starts with `}`) and `[a-z]<` for + * opening-brace lines (lines whose trimmed content ends with `{`) — so the + * hash carries brace-balance signal AND never visually collides with a + * literal `}` or `{` in the line content. + * * The combined `LINE+ID` reference acts as both an address and a staleness check: * if the file has changed since the caller last read it, hash mismatches are caught * before any mutation occurs. * * Displayed format: `LINE+ID|TEXT` - * Reference format: `"LINE+ID"` (e.g. `"1ab"`) + * Reference format: `"LINE+ID"` (e.g. `"1ab"`, `"5>t"`, `"9a<"`) * * In tool JSON, each edit's `content` is `string[]` (one string per logical line) or * `null` to delete the targeted range. @@ -28,7 +34,16 @@ import { resolveToCwd } from "../../tools/path-utils"; import { enforcePlanModeWrite, resolvePlanPath } from "../../tools/plan-mode-guard"; import { formatCodeFrameLine } from "../../tools/render-utils"; import { generateDiffString } from "../diff"; -import { computeLineHash, formatHashLine, HASHLINE_BIGRAM_RE_SRC, HASHLINE_CONTENT_SEPARATOR } from "../line-hash"; +import { + computeLineHash, + describeAnchorExamples, + formatHashLine, + HASHLINE_ANCHOR_RE_SRC, + HASHLINE_CONTENT_SEPARATOR, + HASHLINE_HASH_LAX_RE_SRC, + HASHLINE_HASH_RE_SRC, + HASHLINE_HASH_WIDTH_LABEL, +} from "../line-hash"; import { detectLineEnding, normalizeToLF, restoreLineEndings, stripBom } from "../normalize"; import type { EditToolDetails, LspBatchRequest } from "../renderer"; @@ -53,10 +68,10 @@ export type HashlineEdit = // Accept both `|` (canonical) and `:` (legacy) so re-reads of older outputs still parse. const HASHLINE_CONTENT_SEPARATOR_RE = "[:|]"; const HASHLINE_PREFIX_RE = new RegExp( - `^\\s*(?:>>>|>>)?\\s*(?:[+*]\\s*)?\\d+${HASHLINE_BIGRAM_RE_SRC}${HASHLINE_CONTENT_SEPARATOR_RE}`, + `^\\s*(?:>>>|>>)?\\s*(?:[+*]\\s*)?\\d+${HASHLINE_HASH_RE_SRC}${HASHLINE_CONTENT_SEPARATOR_RE}`, ); const HASHLINE_PREFIX_PLUS_RE = new RegExp( - `^\\s*(?:>>>|>>)?\\s*\\+\\s*\\d+${HASHLINE_BIGRAM_RE_SRC}${HASHLINE_CONTENT_SEPARATOR_RE}`, + `^\\s*(?:>>>|>>)?\\s*\\+\\s*\\d+${HASHLINE_HASH_RE_SRC}${HASHLINE_CONTENT_SEPARATOR_RE}`, ); const DIFF_PLUS_RE = /^[+](?![+])/; const READ_TRUNCATION_NOTICE_RE = /^\[(?:Showing lines \d+-\d+ of \d+|\d+ more lines? in (?:file|\S+))\b.*\bsel=L?\d+/; @@ -221,13 +236,16 @@ function resolveHashlineEditsForDiff(edits: HashlineEditInput[]): HashlineEdit[] }); } +const HASHLINE_HASH_HINT_RE = new RegExp(`^${HASHLINE_HASH_LAX_RE_SRC}$`, "i"); +const HASHLINE_ANCHOR_EXAMPLES = describeAnchorExamples("160"); + export function formatFullAnchorRequirement(raw?: string): string { const suffix = typeof raw === "string" ? raw.trim() : ""; - const hashOnlyHint = /^[A-Za-z]{2}$/.test(suffix) - ? ` It looks like you supplied only the 2-letter suffix (${JSON.stringify(suffix)}). Copy the full anchor exactly as shown (for example, "160${suffix}").` + const hashOnlyHint = HASHLINE_HASH_HINT_RE.test(suffix) + ? ` It looks like you supplied only the ${HASHLINE_HASH_WIDTH_LABEL} suffix (${JSON.stringify(suffix)}). Copy the full anchor exactly as shown (for example, "160${suffix}").` : ""; const received = raw === undefined ? "" : ` Received ${JSON.stringify(raw)}.`; - return `the full anchor exactly as shown by read/grep (line number + 2-letter suffix, for example "160sr")${received}${hashOnlyHint}`; + return `the full anchor exactly as shown by read/grep (line number + ${HASHLINE_HASH_WIDTH_LABEL} hash, for example ${HASHLINE_ANCHOR_EXAMPLES})${received}${hashOnlyHint}`; } function tryParseTag(raw: string): Anchor | undefined { @@ -493,17 +511,16 @@ export async function* streamHashLinesFromLines( if (last) yield last; } +const PARSE_TAG_RE = new RegExp(`^${HASHLINE_ANCHOR_RE_SRC}`); + /** - * Parse a line reference string like `"5th"` into structured form. + * Parse a line reference string like `"5th"` (or a decorated form like + * `"*5th"`, `"+ 5th"`, `">5th"`) into structured form. * - * @throws Error if the format is invalid (not `NUMBERBIGRAM`) + * @throws Error if the input does not match {@link HASHLINE_ANCHOR_RE_SRC}. */ export function parseTag(ref: string): { line: number; hash: string } { - // Captures: - // 1. optional leading ">+-" markers and whitespace - // 2. line number (1+ digits) - // 3. hash (one BPE bigram from HASHLINE_BIGRAMS) directly adjacent (no separator) - const match = ref.match(new RegExp(`^\\s*[>+\\-*]*\\s*(\\d+)(${HASHLINE_BIGRAM_RE_SRC})`)); + const match = ref.match(PARSE_TAG_RE); if (!match) { throw new Error(`Invalid line reference. Expected ${formatFullAnchorRequirement(ref)}.`); } @@ -891,6 +908,7 @@ function buildHashlineEditResult(params: { function validateHashlineEditRefs(edits: HashlineEdit[], fileLines: string[], warnings: string[]): HashMismatch[] { const mismatches: HashMismatch[] = []; + const emittedRebaseKeys = new Set(); for (const edit of edits) { switch (edit.op) { case "replace_line": @@ -928,9 +946,13 @@ function validateHashlineEditRefs(edits: HashlineEdit[], fileLines: string[], wa if (rebased !== null) { const original = `${ref.line}${ref.hash}`; ref.line = rebased; - warnings.push( - `Auto-rebased anchor ${original} → ${rebased}${ref.hash} (line shifted within ±${ANCHOR_REBASE_WINDOW}; hash matched).`, - ); + const rebaseKey = `${original}→${rebased}${ref.hash}`; + if (!emittedRebaseKeys.has(rebaseKey)) { + emittedRebaseKeys.add(rebaseKey); + warnings.push( + `Auto-rebased anchor ${original} → ${rebased}${ref.hash} (line shifted within ±${ANCHOR_REBASE_WINDOW}; hash matched).`, + ); + } return; } mismatches.push({ line: ref.line, expected: ref.hash, actual: actualHash }); @@ -1012,7 +1034,7 @@ export interface CompactHashlineDiffOptions { } const NUMBERED_DIFF_LINE_RE = /^([ +-])(\s*\d+)\|(.*)$/; -const HASHLINE_PREVIEW_PLACEHOLDER = " "; +const HASHLINE_PREVIEW_PLACEHOLDER = "--"; const ELLIPSIS = "..."; type DiffEntryKind = " " | "+" | "-" | "*"; @@ -1155,7 +1177,11 @@ function groupRuns(entries: Entry[]): Run[] { * unified-diff convention they replaced each other in place — and is shown as * `*|` instead of two lines `-` + `+`. * Surplus removals or additions remain as their own runs after the paired - * block, preserving the unified-diff `del-then-add` ordering. + * block, preserving the unified-diff `del-then-add` ordering. The collapse + * applies only when the two runs are the SAME length: pairing a 3-del run + * with a 1-add run produced a confusing mix of `*` and surplus `-`/`+` + * lines, so for size-mismatched (true range-replace) hunks we leave the + * `-` and `+` runs intact for clean unified-diff display. */ function pairModifications(runs: Run[]): Run[] { const isPairable = (entry: Entry): entry is DiffEntry => entry.kind !== "meta" && entry.content !== ELLIPSIS; @@ -1177,6 +1203,14 @@ function pairModifications(runs: Run[]): Run[] { continue; } + // Only fold when every del has a 1:1 add partner (and vice-versa). + // Mismatched sizes mean a real range replace; leaving them as plain + // `-` then `+` runs is clearer than mixing `*` with surplus lines. + if (dels.length !== adds.length) { + out.push(run); + continue; + } + const mods: Entry[] = []; for (let p = 0; p < pairCount; p++) { mods.push({ @@ -1188,13 +1222,6 @@ function pairModifications(runs: Run[]): Run[] { } out.push({ kind: "*", entries: mods }); - if (dels.length > pairCount) { - out.push({ kind: "-", entries: dels.slice(pairCount) }); - } - if (adds.length > pairCount) { - out.push({ kind: "+", entries: adds.slice(pairCount) }); - } - i++; // consume the `+` run } return out; diff --git a/packages/coding-agent/src/prompts/tools/atom.md b/packages/coding-agent/src/prompts/tools/atom.md index 5703e51f1..8ba8dcb46 100644 --- a/packages/coding-agent/src/prompts/tools/atom.md +++ b/packages/coding-agent/src/prompts/tools/atom.md @@ -1,7 +1,7 @@ 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 section **MUST** be `---PATH`. -A "Lid" is a per-line anchor emitted by `read`, `grep`, etc. — `<2-letter-hash>`, e.g. `5th`, `123ab`. You **MUST** copy a Lid verbatim from the latest output for the file you're editing. +A "Lid" is a per-line anchor emitted by `read`, `grep`, etc. — ``, e.g. `5th`, `123ab`, or `7a<` (opening-brace marker), `42>p` (closing-brace marker). You **MUST** copy a Lid verbatim from the latest output for the file you're editing. This format is purely textual. The tool has NO awareness of language, indentation, brackets, fences, or table widths. You are responsible for emitting valid syntax in your replacements/insertions. @@ -14,9 +14,7 @@ $ move cursor to EOF (after the last line) +TEXT insert one line containing TEXT at the cursor + insert one blank line at the cursor Lid=TEXT replace the anchored line with TEXT -LidA..LidB=TEXT replace the range with one line; following `\TEXT` lines append literal lines to the replacement -\TEXT append literal TEXT to the active replacement (after `Lid=…` or `LidA..LidB=…`) -\ append a blank line to the active replacement +LidA..LidB=TEXT replace the range with one line; following `+TEXT` lines extend the replacement (since the cursor sits at the slot the range vacated) Lid= blank the anchored line's content but KEEP the line (results in an empty line, NOT a removed line; use `-Lid` to remove) -Lid delete the anchored line (repeat for multi-line delete) -LidA..LidB delete the contiguous line range LidA..LidB (inclusive) @@ -26,12 +24,9 @@ Lid= blank the anchored line's content but KEEP the line (results in an em - Cursor-only ops (`^`, `$`, `@Lid`, `^Lid`) reposition without modifying. To insert anything you **MUST** follow them with `+TEXT` (or `+` for a blank). -- TEXT in `+TEXT`, `Lid=TEXT`, and `\TEXT` is literal line content, INCLUDING leading whitespace. You **MUST NOT** trim or re-indent it. +- TEXT in `+TEXT` and `Lid=TEXT` is literal line content, INCLUDING leading whitespace. You **MUST NOT** trim or re-indent it. - Consecutive `+TEXT` ops produce consecutive lines in the order written. You **MUST NOT** separate them with a stray `+` unless you intend to insert a blank line. -- `Lid=TEXT` rewrites ONE line. To rewrite K adjacent lines, you **MUST** use `LidA..LidB=FIRST_LINE` followed immediately by `\NEXT_LINE` continuation lines (canonical form for any block replacement). You **MUST** use bare `\` for blank replacement lines. -- You **MUST** prefix every replacement continuation line with `\`, especially when the replacement line starts with edit syntax characters such as `#`, `+`, `-`, `@`, `$`, `^`, `!`, or a Lid-shaped token. -- `\TEXT` **MUST** appear only immediately after an active `Lid=…` or `LidA..LidB=…` replacement. It **MUST NOT** be used as a general insert operator. -- A `\TEXT` line **MUST** be the immediate continuation of a `Lid=…` or `LidA..LidB=…` op on the line above (or another `\` line rooted in one). If the line above is `+TEXT`, a bare Lid, a cursor op, or whitespace, the `\` is invalid and the tool will not interpret it as part of a replacement. +- `Lid=TEXT` rewrites ONE line. To rewrite K adjacent lines, you **MUST** use `LidA..LidB=FIRST_LINE` followed immediately by `+NEXT_LINE` lines (or use `Lid=FIRST_LINE` + `+NEXT_LINE…` to extend a single anchor downward). The `+TEXT` lines insert at the cursor that `Lid=…` / `LidA..LidB=…` parks on the just-set / just-vacated slot. Use bare `+` for blank replacement lines. - The legacy `-LidA..LidB` + `+TEXT…` block-rewrite form also works. - To insert ABOVE a line, you **MUST** use `^Lid` then `+TEXT`. To insert above line 1, you **MUST** use `^` (BOF) then `+TEXT`. To insert below a line, you **MUST** use `@Lid` then `+TEXT`. - Multiple `---PATH` sections **MAY** appear in one input; each section is applied in order. @@ -86,12 +81,12 @@ Lid= blank the anchored line's content but KEEP the line (results in an em # Replace one contiguous block when the existing lines themselves change; the replacement may have more/fewer lines than the selected range ---a.ts {{hrefr 3}}..{{hrefr 6}}=/** Format a display label, falling back to DEF when empty. */ -\export function label(name: string): string { -\ const clean = (name || DEF).trim(); -\ -\ if (clean.length === 0) return DEF; -\ return clean.toUpperCase(); -\} ++export function label(name: string): string { ++ const clean = (name || DEF).trim(); ++ ++ if (clean.length === 0) return DEF; ++ return clean.toUpperCase(); ++} # Insert ABOVE a line ---a.ts @@ -118,6 +113,22 @@ $ ---a.ts -{{hrefr 2}} +# Blank a line in place (line stays, content removed). Use this — not `-Lid` — when the EOL/indentation should remain. +---a.ts +{{hrefr 2}}= + +# Range replace expanding a 3-line block to 5 lines (cursor parks in the slot, so each `+TEXT` extends the replacement) +---a.ts +{{hrefr 3}}..{{hrefr 5}}=export function label(name: string): string { ++ if (!name) return DEF; ++ const clean = name.trim(); ++ if (clean.length === 0) return DEF; ++ return clean; + +# Range replace contracting a 4-line block to 1 line +---a.ts +{{hrefr 3}}..{{hrefr 6}}=export const label = (name: string) => (name || DEF).trim() || DEF; + # Delete the file (no other ops in the section) ---a.ts !rm @@ -139,12 +150,14 @@ $ - Current/added preview lines include fresh `LINE+hash|content` anchors. Removed preview lines show deleted content and **MUST NOT** be reused as anchors. - You **MUST** emit only lines that change. You **MUST NOT** echo unchanged context; the anchor implies position. - You **MUST NOT** write `Lid=`; the tool reports a no-op (no change applied). Emit `Lid=TEXT` only when TEXT differs. -- You **MUST NOT** use `Lid=` + `\continuations` as an "insert after" idiom. That form is a *replacement*: its first line lands at the anchor, and its continuations push the original next line down. When the anchor is a closing brace and your continuations also end in `}`, the original line below — often itself `}` (a sibling block, mod, or impl closer) — sits adjacent to yours and you ship a duplicate `}`. For pure insertion, use `@Lid` + `+TEXT…` (after) or `^Lid` + `+TEXT…` (before). Never re-state the anchor's content as the first line of a replacement. -- A line of the form `Lid|content` (a Lid, then `|`, then text, with NO leading `+`/`-`/`^`/`@`/`\`/`=`/`..`) is **FORBIDDEN**. That shape only appears in `read`/`grep` output as an anchor for *you*; it is never an edit op. If you copy a `Lid|content` line verbatim from a read into a patch, you have made an error — every edit op must start with `+`, `-`, `^`, `@`, `\`, `$`, `!`, or a Lid immediately followed by `=` or `..`. -- To replace a contiguous block with new content, the canonical form is `LidA..LidB=FIRST_LINE` + `\NEXT_LINE…`. You **MUST NOT** write the old block and then the new block — that is unified-diff thinking and the tool does not understand it. If you find yourself emitting pre-image lines (with or without operators) before your new content, STOP and rewrite the section as a single range-replace. -- TEXT after `=`, `+`, or `\` includes leading whitespace verbatim. You **MUST NOT** trim or re-indent it. +- You **MUST NOT** use `Lid=` + `+continuations` as an "insert after" idiom. That form is a *replacement* whose first line lands at the anchor and pushes the original next line down. When the anchor is a closing brace and your continuations also end in `}`, the original line below — often itself `}` (a sibling block, mod, or impl closer) — sits adjacent to yours and you ship a duplicate `}`. For pure insertion, use `@Lid` + `+TEXT…` (after) or `^Lid` + `+TEXT…` (before). Never re-state the anchor's content as the first line of a replacement. +- A line of the form `Lid|content` (a Lid, then `|`, then text, with NO leading `+`/`-`/`^`/`@`/`=`/`..`) is **FORBIDDEN**. That shape only appears in `read`/`grep` output as an anchor for *you*; it is never an edit op. If you copy a `Lid|content` line verbatim from a read into a patch, you have made an error — every edit op must start with `+`, `-`, `^`, `@`, `$`, `!`, or a Lid immediately followed by `=` or `..`. +- To replace a contiguous block with new content, the canonical form is `LidA..LidB=FIRST_LINE` + `+NEXT_LINE…`. You **MUST NOT** write the old block and then the new block — that is unified-diff thinking and the tool does not understand it. If you find yourself emitting pre-image lines (with or without operators) before your new content, STOP and rewrite the section as a single range-replace. +- TEXT after `=` or `+` includes leading whitespace verbatim. You **MUST NOT** trim or re-indent it. - This is NOT unified diff. You **MUST NOT** write `@@` headers, `-OLD`/`+NEW` pairs, context lines, or `+Lid|…` (bad: `+5th|new text`; good: `5th=new text`). - You **MUST NOT** split `Lid=TEXT` across two physical lines. -- For a contiguous range replacement, you **MAY** use either `Lid=FIRST_LINE` + `\NEXT_LINE…` (extends one anchor) or `LidA..LidB=FIRST_LINE` + `\NEXT_LINE…` (collapses an existing range), or fall back to `-LidA..LidB` + `+TEXT…` (delete + insert). +- For a contiguous range replacement, you **MAY** use either `Lid=FIRST_LINE` + `+NEXT_LINE…` (extends one anchor downward) or `LidA..LidB=FIRST_LINE` + `+NEXT_LINE…` (collapses an existing range and fills the slot), or fall back to `-LidA..LidB` + `+TEXT…` (delete + insert). - The tool is syntax-blind. Indentation, brackets, fences, table widths — you remain responsible. +- Hashes may contain `<` or `>` (the brace markers `[a-z]<` for `{`-ending lines, `>[a-z]` for `}`-starting lines). The Lid is everything before the first `|` separator. Example: in a read line `28>i|}` the Lid is `28>i` and the content is `}`. Copy the whole anchor up to the first `|` — do NOT split at `<`/`>` inside the hash. +- `@Lid` + `+TEXT…` where the anchor line ends with `{` inserts your content as the FIRST element inside that block's body. If you intended a sibling (after the closing brace), use `^Lid` on the next sibling line instead. The tool emits a warning when the inserted indent ≤ the anchor indent in this configuration; treat the warning as "did you really mean body content?" diff --git a/packages/coding-agent/test/core/atom.test.ts b/packages/coding-agent/test/core/atom.test.ts index dfa206a9d..6af17a54d 100644 --- a/packages/coding-agent/test/core/atom.test.ts +++ b/packages/coding-agent/test/core/atom.test.ts @@ -178,28 +178,28 @@ describe("atom parser — basic forms", () => { expect(() => applyDiff(longer, diff)).toThrow(/use `2.{2}\.\.3.{2}=REPLACED`/); }); - it("`LidA..LidB=FIRST` accepts backslash continuation lines", () => { + it("`LidA..LidB=FIRST` accepts `+TEXT` continuation lines", () => { const longer = "aaa\nbbb\nccc\nddd\neee"; - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=export function label() {`, "\\ return 1;", "\\}"].join( - "\n", - ); + const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=export function label() {`, "+ return 1;", "+}"].join("\n"); expect(applyDiff(longer, diff)).toBe("aaa\nexport function label() {\n return 1;\n}\neee"); }); it("`LidA..LidB|FIRST` accepts legacy pipe as range replacement separator", () => { const longer = "aaa\nbbb\nccc\nddd\neee"; - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}|ONE`, "\\TWO"].join("\n"); + const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}|ONE`, "+TWO"].join("\n"); expect(applyDiff(longer, diff)).toBe("aaa\nONE\nTWO\neee"); }); - it("bare backslash continuation inserts a blank replacement line", () => { + it("bare `+` inside a range replacement inserts a blank replacement line", () => { const longer = "aaa\nbbb\nccc\nddd\neee"; - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "\\", "\\THREE"].join("\n"); + const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "+", "+THREE"].join("\n"); expect(applyDiff(longer, diff)).toBe("aaa\nONE\n\nTHREE\neee"); }); - it("backslash continuation preserves atom-shaped literal content", () => { + it("`+TEXT` continuation preserves atom-shaped literal content (except Lid=TEXT-shaped lines)", () => { const longer = "aaa\nbbb\nccc\nddd"; + // `12ab=literal`-shaped content can't be expressed via `+TEXT` because + // `+Lid=TEXT` is reserved as the unified-diff-thinking diagnostic. const literalLines = [ "#include ", "# Heading", @@ -209,53 +209,52 @@ describe("atom parser — basic forms", () => { "$literal", "^literal", "!literal", - "12ab=literal", "\\literal", ]; - const diff = [`${tag(2, "bbb")}..${tag(3, "ccc")}=first`, ...literalLines.map(line => `\\${line}`)].join("\n"); + const diff = [`${tag(2, "bbb")}..${tag(3, "ccc")}=first`, ...literalLines.map(line => `+${line}`)].join("\n"); expect(applyDiff(longer, diff)).toBe(["aaa", "first", ...literalLines, "ddd"].join("\n")); }); - it("backslash continuation is rejected outside a replacement op", () => { - expect(() => parseAtom("\\orphan")).toThrow(/\\TEXT continuation is only valid/); + it("`\\TEXT` is rejected with a redirect to `+TEXT`", () => { + expect(() => parseAtom("\\orphan")).toThrow(/has been removed.*Use `\+TEXT`/); }); - it("`Lid=FIRST` accepts backslash continuation lines (single-line set extends to multi-line)", () => { + it("`Lid=FIRST` accepts `+TEXT` continuation lines (single-line set extends to multi-line)", () => { const content = "aaa\nbbb\nccc"; - const diff = [`${tag(2, "bbb")}=export function label() {`, "\\ return 1;", "\\}"].join("\n"); + const diff = [`${tag(2, "bbb")}=export function label() {`, "+ return 1;", "+}"].join("\n"); expect(applyDiff(content, diff)).toBe("aaa\nexport function label() {\n return 1;\n}\nccc"); }); - it("`@Lid=FIRST` accepts backslash continuation lines (legacy `@`-prefixed form)", () => { + it("`@Lid=FIRST` accepts `+TEXT` continuation lines (legacy `@`-prefixed form)", () => { const content = "aaa\nbbb\nccc"; - const diff = [`@${tag(2, "bbb")}=ONE`, "\\TWO"].join("\n"); + const diff = [`@${tag(2, "bbb")}=ONE`, "+TWO"].join("\n"); expect(applyDiff(content, diff)).toBe("aaa\nONE\nTWO\nccc"); }); - it("`Lid|FIRST` (legacy set syntax) accepts backslash continuation lines", () => { + it("`Lid|FIRST` (legacy set syntax) accepts `+TEXT` continuation lines", () => { const content = "aaa\nbbb\nccc"; - const diff = [`${tag(2, "bbb")}|ONE`, "\\TWO"].join("\n"); + const diff = [`${tag(2, "bbb")}|ONE`, "+TWO"].join("\n"); expect(applyDiff(content, diff)).toBe("aaa\nONE\nTWO\nccc"); }); - it("backslash continuation after `Lid=FIRST` rejects unprefixed recovery (range-only fallback)", () => { + it("unrecognized op after `Lid=FIRST` is rejected (no implicit recovery)", () => { const content = "aaa\nbbb\nccc"; - // Only range replacements get the legacy "raw unprefixed continuation" recovery; - // after a single-line set, an unprefixed line below should parse as its own op - // (or error), not silently fold into the replacement. + // After a single-line set the cursor sits on the anchored line; an + // unprefixed line below has no recognized op shape and must error + // rather than fold silently into the replacement. const diff = [`${tag(2, "bbb")}=ONE`, "rawline"].join("\n"); expect(() => applyDiff(content, diff)).toThrow(); }); - it("backslash continuation stops before the next atom op line", () => { + it("`+TEXT` continuation reroutes correctly when followed by an explicit cursor move", () => { const longer = "aaa\nbbb\nccc\nddd\neee"; - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "\\TWO", `@${tag(1, "aaa")}`, "+BELOW"].join("\n"); + const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "+TWO", `@${tag(1, "aaa")}`, "+BELOW"].join("\n"); expect(applyDiff(longer, diff)).toBe("aaa\nBELOW\nONE\nTWO\neee"); }); - it("range replacement accepts optional whitespace before `=` with continuation", () => { + it("range replacement accepts optional whitespace before `=` with `+TEXT` continuation", () => { const longer = "aaa\nbbb\nccc\nddd\neee"; - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")} =ONE`, "\\TWO"].join("\n"); + const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")} =ONE`, "+TWO"].join("\n"); expect(applyDiff(longer, diff)).toBe("aaa\nONE\nTWO\neee"); }); @@ -265,40 +264,6 @@ describe("atom parser — basic forms", () => { expect(applyDiff(longer, diff)).toBe("aaa\n TEXT\neee"); }); - it("raw unprefixed range continuation remains accepted as recovery", () => { - const longer = "aaa\nbbb\nccc\nddd\neee"; - const diff = `${tag(2, "bbb")}..${tag(4, "ddd")}=ONE\nTWO\n# Heading`; - expect(applyDiff(longer, diff)).toBe("aaa\nONE\nTWO\n# Heading\neee"); - }); - - it("blank line between two `\\TEXT` continuation lines is treated as implicit `\\` blank", () => { - const longer = "aaa\nbbb\nccc\nddd\neee"; - // Note the LITERAL blank line between the two `\TEXT` continuations — - // without forgiveness this would error with "\TEXT continuation is only - // valid immediately after a Lid=TEXT…". - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "\\TWO", "", "\\THREE"].join("\n"); - expect(applyDiff(longer, diff)).toBe("aaa\nONE\nTWO\n\nTHREE\neee"); - }); - - it("multiple consecutive blank lines inside a continuation chain all become blank inserts", () => { - const longer = "aaa\nbbb\nccc\nddd\neee"; - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "\\TWO", "", "", "\\FIVE"].join("\n"); - expect(applyDiff(longer, diff)).toBe("aaa\nONE\nTWO\n\n\nFIVE\neee"); - }); - - it("blank line after a single-anchor `Lid=` followed by `\\TEXT` is treated as implicit `\\`", () => { - const content = "aaa\nbbb\nccc"; - const diff = [`${tag(2, "bbb")}=ONE`, "", "\\THREE"].join("\n"); - expect(applyDiff(content, diff)).toBe("aaa\nONE\n\nTHREE\nccc"); - }); - - it("trailing blank line after the last `\\TEXT` continuation still terminates the replacement", () => { - const longer = "aaa\nbbb\nccc\nddd\neee"; - // Blank tail with no follow-up `\TEXT` — must NOT swallow into the replacement. - const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "\\TWO", ""].join("\n"); - expect(applyDiff(longer, diff)).toBe("aaa\nONE\nTWO\neee"); - }); - it("`^Lid` moves the cursor BEFORE the anchored line", () => { const diff = `^${tag(2, "bbb")}\n+INSERTED`; expect(applyDiff(content, diff)).toBe("aaa\nINSERTED\nbbb\nccc"); @@ -501,12 +466,11 @@ describe("atom parser — edge cases", () => { }); it("rejects partial and missing Lids with repairable diagnostics", () => { - expect(() => parseAtom("@@ 98")).toThrow(/`@@ 98` is missing the two-letter Lid suffix/); + expect(() => parseAtom("@@ 98")).toThrow(/`@@ 98` is missing the 2-character Lid suffix/); expect(() => parseAtom("yh=TEXT")).toThrow(/`yh` is not a full Lid/); - expect(() => parseAtom("123=TEXT")).toThrow(/`123` is missing the two-letter Lid suffix/); - expect(() => parseAtom("123|TEXT")).toThrow(/`123` is missing the two-letter Lid suffix/); + expect(() => parseAtom("123=TEXT")).toThrow(/`123` is missing the 2-character Lid suffix/); + expect(() => parseAtom("123|TEXT")).toThrow(/`123` is missing the 2-character Lid suffix/); }); - it("canonical equals replacement does not apply legacy OLD|NEW repair", () => { const content = "aaa\nbbb\nccc"; const t = tag(2, "bbb"); @@ -794,6 +758,79 @@ describe("atom — hash mismatch", () => { `Auto-rebased anchor ${t4} → 5${computeLineHash(5, "ddd")} (line shifted within ±5; hash matched).`, ]); }); + + it("rebased anchor referenced multiple times emits one warning, not N", () => { + // `@Lid` followed by a run of `+TEXT` lines clones the cursor anchor onto + // every insert, so the same (stale) Lid hits validateAtomAnchors N times. + // Without dedup, the warning fires per-clone; with dedup, exactly once. + const content = "aaa\nINSERTED\nbbb\nccc"; + const stale = tag(2, "bbb"); // bbb shifted to line 3 + const diff = `@${stale}\n+L1\n+L2\n+L3\n+L4\n+L5`; + const result = applyAtomEdits(content, parseAtom(diff)); + + expect(result.lines).toBe("aaa\nINSERTED\nbbb\nL1\nL2\nL3\nL4\nL5\nccc"); + expect(result.warnings).toEqual([ + `Auto-rebased anchor ${stale} → 3${computeLineHash(3, "bbb")} (line shifted within ±5; hash matched).`, + ]); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// `@Lid` brace-body insertion heuristic +// ─────────────────────────────────────────────────────────────────────────── + +describe("atom — @Lid lands inside brace-opening body", () => { + it("warns when @Lid sits on a `{`-ending line and inserts at sibling indent", () => { + // The agent meant: insert `restart()` as a sibling of `stop()`. They + // anchored on `stop(): void {` and used `@Lid`, which puts the new + // content inside the `stop()` body, breaking nesting. + const content = ["class S {", " stop(): void {", " history.push();", " }", "}"].join("\n"); + const stopBraceLid = tag(2, " stop(): void {"); + const diff = `@${stopBraceLid}\n+ restart(): void {\n+ history.clear();\n+ }`; + const result = applyAtomEdits(content, parseAtom(diff)); + + expect(result.warnings?.[0]).toContain(`@${stopBraceLid}`); + expect(result.warnings?.[0]).toContain("brace-opening line"); + expect(result.warnings?.[0]).toContain("`^Lid` on the next sibling"); + }); + + it("does NOT warn when inserted content is properly indented past the brace line", () => { + const content = ["class S {", " stop(): void {", " }", "}"].join("\n"); + const stopBraceLid = tag(2, " stop(): void {"); + // 8-space indent — strictly more than the 4-space brace line. + const diff = `@${stopBraceLid}\n+ log("stopping");`; + const result = applyAtomEdits(content, parseAtom(diff)); + + expect(result.warnings ?? []).toEqual([]); + }); + + it("does NOT warn when ^Lid is used (insert before, no body-nesting risk)", () => { + const content = ["class S {", " stop(): void {", " }", "}"].join("\n"); + const stopBraceLid = tag(2, " stop(): void {"); + const diff = `^${stopBraceLid}\n+ restart(): void {}`; + const result = applyAtomEdits(content, parseAtom(diff)); + + expect(result.warnings ?? []).toEqual([]); + }); + + it("does NOT warn when anchor line does not end in `{` (no brace foot-gun)", () => { + const content = ["aaa", "bbb", "ccc"].join("\n"); + const t = tag(1, "aaa"); + const diff = `@${t}\n+inserted`; + const result = applyAtomEdits(content, parseAtom(diff)); + + expect(result.warnings ?? []).toEqual([]); + }); + + it("emits one warning per anchor even when multiple `+TEXT` lines follow", () => { + const content = ["class S {", " stop(): void {", " }", "}"].join("\n"); + const stopBraceLid = tag(2, " stop(): void {"); + const diff = `@${stopBraceLid}\n+ a\n+ b\n+ c\n+ d`; + const result = applyAtomEdits(content, parseAtom(diff)); + + const braceWarnings = (result.warnings ?? []).filter(w => w.includes("brace-opening line")); + expect(braceWarnings).toHaveLength(1); + }); }); // ─────────────────────────────────────────────────────────────────────────── @@ -1069,7 +1106,7 @@ describe("applyAtomEdits — adjacent duplicate detection", () => { const result = applyAtomEdits(content, parseAtom(diff)); expect(result.lines).toBe("const X = 1;\n\nexport function f(): number {\n\treturn X * 2;\n}"); expect(result.warnings ?? []).toEqual( - expect.arrayContaining([expect.stringMatching(/Auto-fixed: removed duplicate line/)]), + expect.arrayContaining([expect.stringMatching(/AUTO-FIX applied .* Removed duplicate line/)]), ); }); @@ -1084,7 +1121,7 @@ describe("applyAtomEdits — adjacent duplicate detection", () => { const result = applyAtomEdits(content, parseAtom(diff)); expect(result.lines).toBe("alpha\nexport function f() {\n\treturn 2;\n}\nbeta"); expect(result.warnings ?? []).toEqual( - expect.arrayContaining([expect.stringMatching(/Auto-fixed: removed duplicate line/)]), + expect.arrayContaining([expect.stringMatching(/AUTO-FIX applied .* Removed duplicate line/)]), ); }); @@ -1103,7 +1140,7 @@ describe("applyAtomEdits — adjacent duplicate detection", () => { const result = applyAtomEdits(content, parseAtom(diff)); expect(result.lines).toBe("fn sayA() {\n\n}\nfn sayB() {\n\n}"); expect(result.warnings ?? []).toEqual( - expect.arrayContaining([expect.stringMatching(/Auto-fixed: removed duplicate lines/)]), + expect.arrayContaining([expect.stringMatching(/AUTO-FIX applied .* Removed duplicate lines/)]), ); }); @@ -1213,3 +1250,57 @@ describe("atomEditParamsSchema — extra-field tolerance", () => { expect(Value.Check(atomEditParamsSchema, args)).toBe(false); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// Brace-aware Lid hashes (`>[a-z]` and `[a-z]<`) parse and apply correctly +// ─────────────────────────────────────────────────────────────────────────── + +describe("atom parser — brace-marker Lids", () => { + it("`Lid=TEXT` accepts a closing-brace anchor", () => { + const content = "function foo() {\n\treturn 1;\n}"; + const closeHash = computeLineHash(3, "}"); + expect(closeHash).toMatch(/^>[a-z]$/); + const diff = `3${closeHash}=} // end`; + expect(applyDiff(content, diff)).toBe("function foo() {\n\treturn 1;\n} // end"); + }); + + it("`Lid=TEXT` accepts an opening-brace anchor", () => { + const content = "function foo() {\n\treturn 1;\n}"; + const openHash = computeLineHash(1, "function foo() {"); + expect(openHash).toMatch(/^[a-z]<$/); + const diff = `1${openHash}=function bar() {`; + expect(applyDiff(content, diff)).toBe("function bar() {\n\treturn 1;\n}"); + }); + + it("`-Lid` deletes a closing-brace anchored line", () => { + const content = "function foo() {\n\treturn 1;\n}"; + const closeHash = computeLineHash(3, "}"); + expect(closeHash).toMatch(/^>[a-z]$/); + const diff = `-3${closeHash}`; + expect(applyDiff(content, diff)).toBe("function foo() {\n\treturn 1;"); + }); + + it("`-Lid` deletes an opening-brace anchored line", () => { + const content = "function foo() {\n\treturn 1;\n}"; + const openHash = computeLineHash(1, "function foo() {"); + expect(openHash).toMatch(/^[a-z]<$/); + const diff = `-1${openHash}`; + expect(applyDiff(content, diff)).toBe("\treturn 1;\n}"); + }); + + it("range delete spans brace-anchored boundaries", () => { + const content = "function foo() {\n\treturn 1;\n}"; + const open = `1${computeLineHash(1, "function foo() {")}`; + const close = `3${computeLineHash(3, "}")}`; + const diff = `-${open}..${close}`; + expect(applyDiff(content, diff)).toBe(""); + }); + + it("range replace spans brace-anchored boundaries", () => { + const content = "function foo() {\n\treturn 1;\n}"; + const open = `1${computeLineHash(1, "function foo() {")}`; + const close = `3${computeLineHash(3, "}")}`; + const diff = `${open}..${close}=const foo = () => 1;`; + expect(applyDiff(content, diff)).toBe("const foo = () => 1;"); + }); +}); diff --git a/packages/coding-agent/test/core/hashline.test.ts b/packages/coding-agent/test/core/hashline.test.ts index 8c723fc5b..6a9abd98f 100644 --- a/packages/coding-agent/test/core/hashline.test.ts +++ b/packages/coding-agent/test/core/hashline.test.ts @@ -4,12 +4,15 @@ import { buildCompactHashlineDiffPreview, computeLineHash, formatHashLines, - HASHLINE_BIGRAM_RE_SRC, HASHLINE_BIGRAMS, HASHLINE_BIGRAMS_COUNT, + HASHLINE_HASH_LAX_RE_SRC, + HASHLINE_HASH_RE_SRC, HashlineMismatchError, hashlineParseText, + LARK_LID_HASH_LAX_PLACEHOLDER, parseTag, + resolveLarkLidPlaceholders, streamHashLinesFromLines, streamHashLinesFromUtf8, stripHashlinePrefixes, @@ -25,9 +28,19 @@ function makeTag(line: number, content: string): Anchor { }; } -/** Returns a valid bigram that's guaranteed NOT to equal the real hash of `(line, content)`. */ +/** Returns a hash that's guaranteed NOT to equal the real hash of `(line, content)`. */ function staleBigramFor(line: number, content: string): string { const real = computeLineHash(line, content); + if (real.startsWith(">")) { + // Closing-brace marker: keep the `>[a-z]` shape but rotate the letter. + const next = String.fromCharCode(((real.charCodeAt(1) - 97 + 1) % 26) + 97); + return `>${next}`; + } + if (real.endsWith("<")) { + // Opening-brace marker: keep the `[a-z]<` shape but rotate the letter. + const next = String.fromCharCode(((real.charCodeAt(0) - 97 + 1) % 26) + 97); + return `${next}<`; + } const idx = HASHLINE_BIGRAMS.indexOf(real as (typeof HASHLINE_BIGRAMS)[number]); return HASHLINE_BIGRAMS[(idx + 1) % HASHLINE_BIGRAMS_COUNT]; } @@ -37,9 +50,9 @@ function staleBigramFor(line: number, content: string): string { // ═══════════════════════════════════════════════════════════════════════════ describe("computeLineHash", () => { - it("returns 2-4 character alphanumeric hash string", () => { + it("returns 2-character hash string", () => { const hash = computeLineHash(1, "hello"); - expect(hash).toMatch(new RegExp(`^${HASHLINE_BIGRAM_RE_SRC}$`)); + expect(hash).toMatch(new RegExp(`^${HASHLINE_HASH_RE_SRC}$`)); }); it("same content at same line produces same hash", () => { @@ -56,7 +69,7 @@ describe("computeLineHash", () => { it("empty line produces valid hash", () => { const hash = computeLineHash(1, ""); - expect(hash).toMatch(new RegExp(`^${HASHLINE_BIGRAM_RE_SRC}$`)); + expect(hash).toMatch(new RegExp(`^${HASHLINE_HASH_RE_SRC}$`)); }); it("uses line number for symbol-only lines", () => { @@ -72,6 +85,82 @@ describe("computeLineHash", () => { }); }); +// ═══════════════════════════════════════════════════════════════════════════ +// computeLineHash — brace markers +// ═══════════════════════════════════════════════════════════════════════════ + +describe("computeLineHash — brace markers", () => { + it("bare closing brace `}` hashes to `>[a-z]`", () => { + expect(computeLineHash(1, "}")).toMatch(/^>[a-z]$/); + }); + + it("`});` is a closer", () => { + expect(computeLineHash(7, "});")).toMatch(/^>[a-z]$/); + }); + + it("`},` is a closer", () => { + expect(computeLineHash(3, "},")).toMatch(/^>[a-z]$/); + }); + + it("indented closer ` }` keeps closing form", () => { + expect(computeLineHash(2, " }")).toMatch(/^>[a-z]$/); + }); + + it("`}//comment` (no space after brace) is still a closer", () => { + expect(computeLineHash(4, "}//comment")).toMatch(/^>[a-z]$/); + }); + + it("opener `function foo() {` hashes to `[a-z]<`", () => { + expect(computeLineHash(1, "function foo() {")).toMatch(/^[a-z]<$/); + }); + + it("opener `if (x) {` hashes to `[a-z]<`", () => { + expect(computeLineHash(2, "if (x) {")).toMatch(/^[a-z]<$/); + }); + + it("`} else {` resolves to opening form (open-brace wins)", () => { + expect(computeLineHash(5, "} else {")).toMatch(/^[a-z]<$/); + }); + + it("`} catch (err) {` resolves to opening form", () => { + expect(computeLineHash(8, "} catch (err) {")).toMatch(/^[a-z]<$/); + }); + + it("plain comment line stays in bigram alphabet (no braces)", () => { + const hash = computeLineHash(1, "// just a comment"); + expect(hash).toMatch(/^[a-z]{2}$/); + expect(hash).not.toMatch(/[<>]/); + }); + + it("two `}` lines on different line numbers get distinct closing hashes", () => { + const a = computeLineHash(1, "}"); + const b = computeLineHash(2, "}"); + expect(a).toMatch(/^>[a-z]$/); + expect(b).toMatch(/^>[a-z]$/); + expect(a).not.toBe(b); + }); + + it("two `{` lines on different line numbers get distinct opening hashes", () => { + const a = computeLineHash(1, "{"); + const b = computeLineHash(2, "{"); + expect(a).toMatch(/^[a-z]<$/); + expect(b).toMatch(/^[a-z]<$/); + expect(a).not.toBe(b); + }); + + it("HASHLINE_HASH_RE_SRC matches both brace forms and bigrams", () => { + const re = new RegExp(`^${HASHLINE_HASH_RE_SRC}$`); + expect(re.test(computeLineHash(1, "}"))).toBe(true); + expect(re.test(computeLineHash(1, "{"))).toBe(true); + expect(re.test(computeLineHash(1, "hello"))).toBe(true); + expect(re.test(">a")).toBe(true); + expect(re.test("a<")).toBe(true); + expect(re.test("ab")).toBe(true); + expect(re.test(">>")).toBe(false); + expect(re.test("<<")).toBe(false); + }); +}); + // ═══════════════════════════════════════════════════════════════════════════ // formatHashLines // ═══════════════════════════════════════════════════════════════════════════ @@ -103,7 +192,7 @@ describe("formatHashLines", () => { const result = formatHashLines("foo\n\nbar"); const lines = result.split("\n"); expect(lines).toHaveLength(3); - expect(lines[1]).toMatch(new RegExp(`^2${HASHLINE_BIGRAM_RE_SRC}|$`)); + expect(lines[1]).toMatch(new RegExp(`^2${HASHLINE_HASH_RE_SRC}|$`)); }); it("round-trips with computeLineHash", () => { @@ -112,7 +201,7 @@ describe("formatHashLines", () => { const lines = formatted.split("\n"); for (let i = 0; i < lines.length; i++) { - const match = lines[i].match(new RegExp(`^(\\d+)(${HASHLINE_BIGRAM_RE_SRC})\\|(.*)$`)); + const match = lines[i].match(new RegExp(`^(\\d+)(${HASHLINE_HASH_RE_SRC})\\|(.*)$`)); expect(match).not.toBeNull(); const lineNum = Number.parseInt(match![1], 10); const hash = match![2]; @@ -217,6 +306,22 @@ describe("parseTag", () => { it("rejects empty hash", () => { expect(() => parseTag("5#")).toThrow(/Invalid line reference/); }); + + it("strips leading anchor decorations (`*`, `+`, `-`, `>`) before parsing", () => { + expect(parseTag("*5th")).toEqual({ line: 5, hash: "th" }); + expect(parseTag(" 5th")).toEqual({ line: 5, hash: "th" }); + expect(parseTag("+5th")).toEqual({ line: 5, hash: "th" }); + expect(parseTag("-5th")).toEqual({ line: 5, hash: "th" }); + expect(parseTag(">5th")).toEqual({ line: 5, hash: "th" }); + expect(parseTag(">>>+5th")).toEqual({ line: 5, hash: "th" }); + }); + + it("parses brace-marker hashes through HASHLINE_ANCHOR_RE_SRC", () => { + expect(parseTag("3>p")).toEqual({ line: 3, hash: ">p" }); + expect(parseTag("1a<")).toEqual({ line: 1, hash: "a<" }); + // Decorations + brace marker still parses. + expect(parseTag("*3>p")).toEqual({ line: 3, hash: ">p" }); + }); }); // ═══════════════════════════════════════════════════════════════════════════ @@ -691,7 +796,7 @@ describe("applyHashlineEdits — errors", () => { const correctHash = computeLineHash(2, "bbb"); expect(msg).toContain(`*2${correctHash}|bbb`); // Context lines use leading space and `|` separator - const contextLines = msg.split("\n").filter(l => /^ \d+[a-z]{2}\|/.test(l)); + const contextLines = msg.split("\n").filter(l => new RegExp(`^ \\d+${HASHLINE_HASH_RE_SRC}\\|`).test(l)); expect(contextLines.length).toBeGreaterThan(0); } }); @@ -714,7 +819,9 @@ describe("applyHashlineEdits — errors", () => { expect(e.mismatches[0].line).toBe(2); expect(e.mismatches[1].line).toBe(4); // Both mismatched lines use `*` prefix (vs leading space for context) - const markerLines = e.message.split("\n").filter(l => /^\*\d+[a-z]{2}\|/.test(l)); + const markerLines = e.message + .split("\n") + .filter(l => new RegExp(`^\\*\\d+${HASHLINE_HASH_RE_SRC}\\|`).test(l)); expect(markerLines).toHaveLength(2); } }); @@ -780,7 +887,7 @@ describe("buildCompactHashlineDiffPreview", () => { expect(preview.preview).toContain(`+3${computeLineHash(3, "two")}|two`); expect(preview.preview).toContain(`+4${computeLineHash(4, "three")}|three`); expect(preview.preview).toContain(`+5${computeLineHash(5, "four")}|four`); - expect(preview.preview).toContain("-2 |old"); + expect(preview.preview).toContain("-2--|old"); expect(preview.preview).not.toContain(`-2${computeLineHash(2, "old")}`); expect(preview.addedLines).toBe(4); expect(preview.removedLines).toBe(1); @@ -793,7 +900,7 @@ describe("buildCompactHashlineDiffPreview", () => { expect(preview.preview).toContain(`*10${computeLineHash(10, "new")}|new`); expect(preview.preview).not.toContain(`+10${computeLineHash(10, "new")}|new`); - expect(preview.preview).not.toContain("-10 |old"); + expect(preview.preview).not.toContain("-10--|old"); expect(preview.preview).toContain(` 11${computeLineHash(11, "ctx-a")}|ctx-a`); expect(preview.preview).toContain(` 12${computeLineHash(12, "ctx-b")}|ctx-b`); expect(preview.preview).not.toContain("ctx-c"); @@ -804,30 +911,32 @@ describe("buildCompactHashlineDiffPreview", () => { expect(preview.removedLines).toBe(1); }); - it("keeps surplus removals after the paired `*` block when more old lines were dropped than added", () => { + it("does NOT fold when more old lines were dropped than added; keeps clean -/+ runs", () => { const diff = ["-100|del-a", "-101|del-b", "-102|del-c", "+100|new-a", " 103|tail"].join("\n"); const preview = buildCompactHashlineDiffPreview(diff); const lines = preview.preview.split("\n"); expect(lines).toEqual([ - `*100${computeLineHash(100, "new-a")}|new-a`, - "-101 |del-b", - "-102 |del-c", + "-100--|del-a", + "-101--|del-b", + "-102--|del-c", + `+100${computeLineHash(100, "new-a")}|new-a`, ` 101${computeLineHash(101, "tail")}|tail`, ]); expect(preview.addedLines).toBe(1); expect(preview.removedLines).toBe(3); }); - it("keeps surplus additions after the paired `*` block when more new lines were added than removed", () => { + it("does NOT fold when more new lines were added than removed; keeps clean -/+ runs", () => { const diff = ["-10|old", "+10|new-a", "+11|new-b", "+12|new-c", " 11|tail"].join("\n"); const preview = buildCompactHashlineDiffPreview(diff); const lines = preview.preview.split("\n"); expect(lines).toEqual([ - `*10${computeLineHash(10, "new-a")}|new-a`, + "-10--|old", + `+10${computeLineHash(10, "new-a")}|new-a`, `+11${computeLineHash(11, "new-b")}|new-b`, `+12${computeLineHash(12, "new-c")}|new-c`, ` 13${computeLineHash(13, "tail")}|tail`, @@ -844,7 +953,7 @@ describe("buildCompactHashlineDiffPreview", () => { expect(preview.preview).not.toContain("*"); expect(preview.preview).toContain(`+2${computeLineHash(2, "one")}|one`); expect(preview.preview).toContain(`+3${computeLineHash(3, "two")}|two`); - expect(preview.preview).toContain("-2 |old"); + expect(preview.preview).toContain("-2--|old"); }); it("never truncates change runs — every removed and added line is shown in full", () => { @@ -857,7 +966,7 @@ describe("buildCompactHashlineDiffPreview", () => { expect(preview.preview).not.toContain("more removed lines"); expect(preview.preview).not.toContain("more preview lines"); for (let i = 0; i < 30; i++) { - expect(preview.preview).toContain(`-${100 + i} |del-${i}`); + expect(preview.preview).toContain(`-${100 + i}--|del-${i}`); } }); @@ -1115,3 +1224,78 @@ describe("hashlineParseContent", () => { expect(result.lines).toBe("const x = 1;\n// TODO: old\n# TODO: remove this -- done\nconst y = 2;"); }); }); + +// ═══════════════════════════════════════════════════════════════════════════ +// Hash format centralization — single source of truth for regex shape +// ═══════════════════════════════════════════════════════════════════════════ + +describe("hash format — central source of truth", () => { + it("strict regex matches every hash computeLineHash produces (sample sweep)", () => { + const strict = new RegExp(`^${HASHLINE_HASH_RE_SRC}$`); + const samples = [ + [1, "hello"], + [2, " return result;"], + [3, "}"], + [4, "function foo() {"], + [5, "} else {"], + [6, ""], + [7, "\t}"], + ] as const; + for (const [line, content] of samples) { + const hash = computeLineHash(line, content); + expect(hash).toMatch(strict); + } + }); + + it("lax regex is a strict superset of the strict regex (every produced hash also matches lax)", () => { + const lax = new RegExp(`^${HASHLINE_HASH_LAX_RE_SRC}$`); + // Sample a few bigrams plus brace markers. + const samples = [ + HASHLINE_BIGRAMS[0], + HASHLINE_BIGRAMS[42], + HASHLINE_BIGRAMS[HASHLINE_BIGRAMS_COUNT - 1], + ">a", + ">z", + "a<", + "z<", + ]; + for (const hash of samples) { + expect(hash).toMatch(lax); + } + }); + + it("lax regex accepts non-bigram letter pairs that strict rejects", () => { + const lax = new RegExp(`^${HASHLINE_HASH_LAX_RE_SRC}$`); + const strict = new RegExp(`^${HASHLINE_HASH_RE_SRC}$`); + // `bq`, `qj`, `xq` are letter pairs the BPE alphabet excludes (rare q/x/z heavy combos). + const offBigrams = ["bq", "qj", "xq"].filter( + p => !HASHLINE_BIGRAMS.includes(p as (typeof HASHLINE_BIGRAMS)[number]), + ); + expect(offBigrams.length).toBeGreaterThan(0); + for (const candidate of offBigrams) { + expect(candidate).toMatch(lax); + expect(candidate).not.toMatch(strict); + } + }); + + it("resolveLarkLidPlaceholders substitutes the placeholder with the lax regex source", () => { + const grammar = `LID: /[1-9][0-9]*${LARK_LID_HASH_LAX_PLACEHOLDER}/`; + const resolved = resolveLarkLidPlaceholders(grammar); + expect(resolved).not.toContain(LARK_LID_HASH_LAX_PLACEHOLDER); + expect(resolved).toContain(HASHLINE_HASH_LAX_RE_SRC); + }); + + it("resolveLarkLidPlaceholders is a no-op when the grammar has no placeholder", () => { + const grammar = `start: "x"`; + expect(resolveLarkLidPlaceholders(grammar)).toBe(grammar); + }); + + it("atom.lark embeds the placeholder so resolveLarkLidPlaceholders has work to do", async () => { + const atomLark = await Bun.file(new URL("../../src/edit/modes/atom.lark", import.meta.url).pathname).text(); + expect(atomLark).toContain(LARK_LID_HASH_LAX_PLACEHOLDER); + + const resolved = resolveLarkLidPlaceholders(atomLark); + expect(resolved).not.toContain(LARK_LID_HASH_LAX_PLACEHOLDER); + expect(resolved).toContain(HASHLINE_HASH_LAX_RE_SRC); + }); +}); diff --git a/packages/coding-agent/test/prompt-templates.test.ts b/packages/coding-agent/test/prompt-templates.test.ts index 099ba190b..ccdea59de 100644 --- a/packages/coding-agent/test/prompt-templates.test.ts +++ b/packages/coding-agent/test/prompt-templates.test.ts @@ -10,6 +10,7 @@ import { describe, expect, test } from "bun:test"; import { expandPromptTemplate, type PromptTemplate } from "@oh-my-pi/pi-coding-agent/config/prompt-templates"; +import { HASHLINE_HASH_RE_SRC } from "@oh-my-pi/pi-coding-agent/edit"; import { expandSlashCommand, type FileSlashCommand } from "@oh-my-pi/pi-coding-agent/extensibility/slash-commands"; import { parseCommandArgs, substituteArgs } from "@oh-my-pi/pi-coding-agent/utils/command-args"; @@ -335,11 +336,13 @@ describe("hashline prompt helpers", () => { const ref = raw.slice("raw=".length); expect(quoted).toBe(`quoted="${ref}"`); - expect(ref).toMatch(/^5[a-z]{2}$/); + expect(ref).toMatch(new RegExp(`^5${HASHLINE_HASH_RE_SRC}$`)); }); test("href should not reuse hline state across prompt renders", () => { - expect(expandPrompt('{{hline 1 "const x = 1;"}}\n{{hrefr}}')).toMatch(/^1[a-z]{2}\|const x = 1;\n1[a-z]{2}$/); + expect(expandPrompt('{{hline 1 "const x = 1;"}}\n{{hrefr}}')).toMatch( + new RegExp(`^1${HASHLINE_HASH_RE_SRC}\\|const x = 1;\n1${HASHLINE_HASH_RE_SRC}$`), + ); expect(() => expandPrompt("{{hrefr}}")).toThrow("previous {{hline}}"); }); }); diff --git a/packages/typescript-edit-benchmark/src/runner.ts b/packages/typescript-edit-benchmark/src/runner.ts index 40d541855..5b57180eb 100644 --- a/packages/typescript-edit-benchmark/src/runner.ts +++ b/packages/typescript-edit-benchmark/src/runner.ts @@ -225,7 +225,11 @@ export type EditFailureCategory = (typeof EDIT_FAILURE_CATEGORIES)[number]; function categorizeEditFailure(error: string, args: unknown): EditFailureCategory { const payload = getEditPayloadFromArgs(args); const hasRangeReplacePayload = /^[1-9]\d*[a-z]{2}\.\.[1-9]\d*[a-z]{2}[ \t]*=/m.test(payload); - if (/\\TEXT continuation|range[- ]replacement continuation|LidA\.\.LidB=FIRST_LINE/i.test(error)) { + if ( + /\\TEXT.* (?:continuation|has been removed)|range[- ]replacement continuation|LidA\.\.LidB=FIRST_LINE/i.test( + error, + ) + ) { return "range-continuation"; } if (/unified-diff syntax|\+Lid[=|]|\+[1-9]\d*[a-z]{2}[=|]/i.test(error)) { @@ -1010,7 +1014,6 @@ async function runSingleTask( if (config.editFuzzyThreshold !== undefined) process.env.PI_EDIT_FUZZY_THRESHOLD = config.editFuzzyThreshold === "auto" ? "auto" : String(config.editFuzzyThreshold); - process.env.PI_STRICT_EDIT_MODE = "1"; process.env.PI_NO_TITLE = "1"; const useInProcess = config.inProcess !== false;