diff --git a/bun.lock b/bun.lock index cd3b5bb00..edbe33cfb 100644 --- a/bun.lock +++ b/bun.lock @@ -84,7 +84,6 @@ "version": "15.5.7", "dependencies": { "diff": "catalog:", - "lru-cache": "catalog:", }, "devDependencies": { "@types/bun": "catalog:", @@ -164,6 +163,7 @@ "@babel/parser": "catalog:", "@babel/traverse": "catalog:", "@babel/types": "catalog:", + "@oh-my-pi/hashline": "catalog:", "@oh-my-pi/pi-agent-core": "catalog:", "@oh-my-pi/pi-ai": "catalog:", "@oh-my-pi/pi-coding-agent": "catalog:", diff --git a/docs/tools/edit.md b/docs/tools/edit.md index d433bd98e..d9951083b 100644 --- a/docs/tools/edit.md +++ b/docs/tools/edit.md @@ -4,17 +4,18 @@ ## Source - Entry: `packages/coding-agent/src/edit/index.ts` -- Model-facing prompt: `packages/coding-agent/src/prompts/tools/hashline.md` +- Model-facing prompt: `packages/hashline/src/prompt.md` - Key collaborators: - `packages/coding-agent/src/utils/edit-mode.ts` — selects active edit mode - - `packages/coding-agent/src/hashline/grammar.lark` — custom-tool grammar for hashline mode - - `packages/coding-agent/src/hashline/input.ts` — splits `¶PATH` sections - - `packages/coding-agent/src/hashline/executor.ts` / `tokenizer.ts` — parses op-prefixed edits and `+`-prefixed payload continuation lines - - `packages/coding-agent/src/hashline/apply.ts` — validates anchors and applies edits - - `packages/coding-agent/src/hashline/anchors.ts` — stale-anchor mismatch formatting - - `packages/coding-agent/src/hashline/recovery.ts` — cache-based stale-anchor recovery - - `packages/coding-agent/src/hashline/hash.ts` — computes 4-hex file hashes and `LINE:TEXT` display lines shared with `read`/`search` - - `packages/coding-agent/src/edit/file-read-cache.ts` — per-session read snapshot cache + - `packages/hashline/src/grammar.lark` — hashline grammar + - `packages/hashline/src/format.ts` — sigils and header constants (`¶`, `#`, `:`, `:-`, `+`, `^`) + - `packages/hashline/src/input.ts` — parses `¶PATH#TAG` sections + - `packages/hashline/src/tokenizer.ts` / `packages/hashline/src/parser.ts` — tokenizes and parses ops + - `packages/hashline/src/apply.ts` — applies parsed edits to file text + - `packages/hashline/src/mismatch.ts` — stale-anchor mismatch formatting + - `packages/hashline/src/recovery.ts` — snapshot-based stale-anchor recovery + - `packages/hashline/src/snapshots.ts` — mints and resolves per-path two-hex opaque snapshot tags + - `packages/coding-agent/src/edit/file-snapshot-store.ts` — per-session read/search snapshot store wiring - `packages/coding-agent/src/tools/read.ts` — emits anchored lines and records read snapshots - `packages/coding-agent/src/tools/search.ts` — records sparse snapshots from matches/context - `packages/coding-agent/src/tools/fs-cache-invalidation.ts` — invalidates FS scan caches after writes @@ -26,34 +27,53 @@ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `input` | `string` | Yes | One or more edit sections. Anchored sections must start with `¶PATH#HASH`; unbound `¶PATH` is allowed only for new-file / `BOF` / `EOF` boundary inserts. Optional `*** Begin Patch` / `*** End Patch` envelope is ignored if present. | +| `input` | `string` | Yes | One or more edit sections. Anchored sections must start with `¶PATH#TAG`; unbound `¶PATH` is allowed only for new-file / `BOF` / `EOF` boundary inserts. Optional `*** Begin Patch` / `*** End Patch` envelope is ignored if present. | Patch language inside `input`: -- Section header: `¶PATH#HASH` for anchored edits, `¶PATH` for BOF/EOF-only inserts -- Insert after: `LINE↓[payload]` -- Insert before: `LINE↑[payload]` -- Replace range: `A-B:[payload]` -- Single-line replace sugar: `A:[payload]` means `A-A:[payload]` -- Delete range: `A-B!` -- Single-line delete sugar: `A!` means `A-A!` -- **Payload semantics:** the first payload line may follow the sigil on the op line itself. Additional payload lines must be on subsequent lines prefixed with `+`; that delimiter is stripped before writing. Use `+` alone for an empty payload line, and `++text` to write a payload line that begins with `+text`. Bare `A↑` / `A↓` insert one blank line, and bare `A:` / `A-B:` replace the line/range with one blank line. -- `!` deletes and forbids payload. -- Read lines like `84:content` are already valid single-line replacements. -- Special anchors: `BOF`, `EOF` (both support inline payload, e.g. `BOF↓export const done = true;`). -- Anchor token: bare line number, for example `41` -- File binding: 4-hex hash in the section header, for example `¶src/a.ts#1a2b` +- **Section header**: `¶PATH#TAG` for anchored edits, `¶PATH` for BOF/EOF-only inserts. `TAG` is two lowercase hex chars minted by the session snapshot store. +- **Anchor blocks** select a range of original lines: + - `A-B:` — select lines A..B; the body rows below describe their new content. + - `A-B:-` — select lines A..B and delete them. No body permitted. + - `A:` is accepted as `A-A:`. `A:-` is accepted as `A-A:-`. + - `BOF:` — virtual position before line 1; body rows insert there. + - `EOF:` — virtual position after the last line; body rows insert there. + - `BOF-BOF:` / `EOF-EOF:` / `BOF-EOF:` are silently normalized to the virtual anchor (range suffix carries no information for virtual positions). +- **Body rows** (one per line, immediately under the anchor): + - `+TEXT` — add the literal line `TEXT` verbatim, including all leading whitespace. + - `+` alone — add one blank line. + - `^A-B` — re-emit original file lines A..B. Use this to keep some of the lines you selected. `^A` is accepted as `^A-A`. +- **Semantics of the body**: + - The new content of the selected range is just the body rows top-to-bottom. + - `A-B:` with no body rows REPLACES the range with one blank line. Use `A-B:-` to delete. + - `BOF:` / `EOF:` with no body inserts one blank line at that virtual position. -Anchors come from `read`/`search` output. `read` emits a `¶PATH#HASH` header and lines as `LINE:TEXT`; copy the header into the edit section and copy only the line number into op lines. +Anchors come from `read`/`search` output. `read` emits a `¶PATH#TAG` header from the session snapshot store and lines as `LINE:TEXT`; copy the header into the edit section and copy only the line number into anchor lines. Other edit modes exist (`replace`, `patch`, `apply_patch`) and are selected outside the tool payload by `resolveEditMode()` in `packages/coding-agent/src/utils/edit-mode.ts`. Their schemas are different; this document covers the default hashline mode. +### Tolerated input shapes (lenient parsing) + +Because models reproduce nearby shapes (`read` output, `apply_patch` envelopes, unified-diff hunks), the parser is liberal about a handful of harmless variants: + +- `A:` / `A:-` — single-line shorthand for `A-A:` / `A-A:-`. +- `^A` — shorthand for `^A-A`. +- Bare body rows with no `+`/`^` prefix are auto-prepended with `+` and a `BARE_BODY_AUTO_PIPED_WARNING` is appended, BUT only when every row in that block is uniformly bare. Mixed `+`/raw blocks still throw. +- Lone `-` body row immediately after a bare anchor is retroactively converted to a `:-` delete with a `DASH_PAYLOAD_AUTO_DELETE_WARNING`. +- An overlapping bare anchor followed by a concrete delete or replace block is treated as a stale "before then after" pair: the bare block is dropped with a `REPLACE_PAIR_COALESCED_OVERLAP_WARNING`. Identical-range pairs use the same coalesce as a stronger guarantee with `REPLACE_PAIR_COALESCED_WARNING`. +- Two or more consecutive single-line `A-A:` blocks with empty bodies emit a `STACKED_BLANK_REPLACE_WARNING` (the model probably meant `A-B:-`). +- `*`/`>` decoration prefixes from grep-style output are stripped from anchors. +- `*** Update File:` / `*** Add File:` / `*** Delete File:` sentinels and unified-diff `@@` headers throw an `apply_patch sentinel … is not valid in hashline` error so the model knows it shipped the wrong format envelope. +- `-N:` / `-N-M:` apply_patch hunk-anchor prefixes throw an `apply_patch line prefix … is not valid in hashline` error. +- A lone `-` outside any pending block throws a focused `a lone "-" is not a valid hashline op` error pointing at `A-B:-`. +- `*** Begin Patch` / `*** End Patch` envelopes are silently consumed. `*** Abort` terminates parsing silently — ops parsed before the marker still apply, no warning is surfaced. + ## Outputs - Single-shot tool result; hashline mode does not use a `resolve` preview/apply handshake. - `content` contains one text block per call. For a successful single-file edit it is either: - - `:` plus a compact diff preview from `packages/coding-agent/src/hashline/diff-preview.ts`, or + - `:` plus a compact diff preview from `packages/hashline/src/diff-preview.ts`, or - `Updated ` / `Created ` when no compact preview text is emitted. -- Parse or recovery warnings are appended as: +- Parse, apply, or recovery warnings are appended as: ```text Warnings: @@ -71,31 +91,30 @@ Warnings: - While the model is still typing arguments, the TUI can compute a diff preview with `packages/coding-agent/src/edit/streaming.ts`; that preview is not a deferred action and does not block execution. ## Flow -1. `EditTool.execute()` in `packages/coding-agent/src/edit/index.ts` resolves the active mode. Default is `hashline`; `customFormat` exposes `packages/hashline/src/grammar.lark` as a constant string with op sigils and the section-header `¶` inlined. -2. `executeHashlineSingle()` in `packages/coding-agent/src/hashline/execute.ts` splits the raw `input` into `¶PATH#HASH` / `¶PATH` sections with `splitHashlineInputs()`. -3. If multiple sections target the same path, `mergeSamePathSections()` concatenates them before execution so every op still refers to the original file snapshot. -4. Multi-section calls run a preflight pass (`preflightHashlineSection()`): parse ops, enforce plan-mode write rules, load the current file, reject anchor-scoped edits against missing files, reject auto-generated files, apply edits in memory, and fail if the result is a no-op. This prevents partial batches. -5. `parseHashline()` in `packages/coding-agent/src/hashline/executor.ts` tokenizes the diff body: - - ignores raw blank lines and optional `*** Begin Patch` - - stops at `*** End Patch` - - stops at `*** Abort` and emits `ABORT_WARNING` - - turns `↓` / `↑` payload runs (inline plus `+`-prefixed subsequent lines) into one `insert` edit per payload line - - turns `A-B:` with payload into inserts before `A`, then deletes for `A-B` - - turns `A-B!` into one `delete` edit per line in the range; payload is forbidden -6. `executeHashlineSingle()` computes the current file hash before applying anchored edits. If it differs from the section `#HASH`, recovery tries the read/search snapshot cache before any write. -7. `applyHashlineEdits()` validates only line bounds, then applies the already hash-bound line-number edits. -8. Recovery replays the edits against the cached snapshot for the section hash (`packages/coding-agent/src/edit/file-read-cache.ts`), then 3-way merges the result onto current disk content using `Diff.applyPatch(..., { fuzzFactor: 0 })` in `packages/coding-agent/src/hashline/recovery.ts`. On success the edit proceeds with a warning; on failure a `HashlineMismatchError` is surfaced. -9. Before splicing lines, `absorbReplacementBoundaryDuplicates()` normalizes some malformed-but-recoverable ranges: - - duplicate prefix/suffix lines adjacent to a replacement can be absorbed by widening the delete range - - pure inserts can auto-drop duplicated leading/trailing payload lines when `edit.hashlineAutoDropPureInsertDuplicates` is enabled - - all such fixes append warnings -10. `after_anchor` inserts are normalized to `before_anchor` of the next line, or `EOF` if the anchor was the last line. -11. Anchor-targeted edits are bucketed by target line and applied bottom-up so earlier splices do not invalidate later original line numbers. `BOF` and `EOF` inserts are applied after that. -12. The edited text is restored to the original BOM and line ending style with helpers from `packages/coding-agent/src/edit/normalize.ts` and persisted via `serializeEditFileText()` in `packages/coding-agent/src/edit/read-file.ts`. -13. The writethrough callback from `createLspWritethrough()` may format the file and fetch diagnostics. Late diagnostics are queued back into session state as a hidden deferred message by `EditTool.#injectLateDiagnostics()` in `packages/coding-agent/src/edit/index.ts`. -14. `invalidateFsScanAfterWrite()` calls `invalidateFsScanCache(path)` so filesystem-backed tools do not serve stale scan results. -15. The session file-read cache is refreshed with the post-edit file text via `recordContiguous()`, making the just-written content the new recovery base for subsequent stale-anchor merges. -16. The final response is built from a unified diff (`generateDiffString()`), a compact preview, and any accumulated warnings. +1. `EditTool.execute()` in `packages/coding-agent/src/edit/index.ts` resolves the active mode. Default is `hashline`; `customFormat` exposes `packages/hashline/src/grammar.lark` as a constant string for prompt embedding. +2. `executeHashlineSingle()` in `packages/coding-agent/src/edit/hashline/execute.ts` parses the raw `input` via `Patch.parse()` (`packages/hashline/src/input.ts`), which: + - strips a leading BOM and `*** Begin Patch` markers, + - splits the input into `¶PATH#TAG` sections, + - merges multiple sections targeting the same path so every op refers to the original file snapshot, + - rejects malformed headers. +3. For each section, `Patcher.prepare()` (`packages/hashline/src/patcher.ts`): + - parses the diff body via `parsePatch()` (tokenizer + parser), + - reads the current file, + - resolves the section tag against the session snapshot store, + - runs recovery if the tag is stale (recorded snapshot replay + 3-way merge against current disk), + - validates anchor line bounds against the resolved file content, + - applies the edits in memory via `applyEdits()`. +4. Multi-section calls preflight every section before any write hits the filesystem so a partial batch never lands. +5. `applyEdits()` in `packages/hashline/src/apply.ts`: + - expands `^A-B` repeat edits into concrete inserts, + - runs `absorbReplacementBoundaryDuplicates()` to widen replacement deletes when the payload's leading/trailing rows match adjacent file lines (with an `Auto-absorbed …` warning), + - emits a per-line `Deleted line N contains a structural bracket/brace boundary …` warning ONLY when the block's net brace/paren/bracket balance is not preserved by its replacement payload (so well-formed multi-line replaces no longer false-positive), + - applies anchor-targeted edits bottom-up so later splices do not invalidate earlier line numbers, + - applies BOF and EOF inserts after the per-line bucket. +6. `Patcher.commit()` writes the result. The writethrough callback from `createLspWritethrough()` may format the file and fetch diagnostics. +7. `invalidateFsScanAfterWrite()` calls native `invalidateFsScanCache(path)` so filesystem-backed tools do not serve stale scan results. +8. The session file-read cache is refreshed with the post-edit file text via `recordContiguous()`, making the just-written content the new recovery base for subsequent stale-anchor merges. +9. The final response is built from a unified diff (`generateDiffString()`), a compact preview, and any accumulated warnings. ## Modes / Variants - `hashline` — default mode; line-anchored patch language described here (`packages/coding-agent/src/utils/edit-mode.ts`). @@ -103,70 +122,79 @@ Warnings: - `patch` — structured JSON diff-hunk mode (`packages/coding-agent/src/edit/modes/patch.ts`). - `apply_patch` — freeform Codex-style `*** Begin Patch` envelope, internally expanded into patch-mode entries (`packages/coding-agent/src/edit/modes/apply-patch.ts`). -Hashline op examples (single-line payloads are inline; multi-line payloads continue on `+`-prefixed subsequent lines): +## Worked examples + +Reference file (the exact shape `read` returns): ```text -¶src/a.ts#1a2b -4↓const added = true; +¶a.ts#0a +1:const X = "a"; +2:const Y = X; +3: +4:console.log(X); +5:console.log(Y); +6:export { X, Y }; ``` +Replace line 1 with two lines: + ```text -¶src/a.ts#1a2b -4↑const addedBefore = true; +¶a.ts#0a +1-1: ++const X = "b"; ++export const Y = X; ``` +Insert BELOW line 5 (keep line 5, add after): + ```text -¶src/a.ts#1a2b -4-6:const replacement = true; +¶a.ts#0a +5-5: +^5-5 ++console.log(X + Y); ``` +Insert ABOVE line 5 (add before, keep line 5): + ```text -¶src/a.ts#1a2b -4-5:const clean = (name || DEF).trim(); -+return clean.length === 0 ? DEF : clean.toUpperCase(); +¶a.ts#0a +5-5: ++console.log(X + Y); +^5-5 ``` +Delete lines 4..5 entirely: + ```text -¶src/a.ts#1a2b -4:const clean = (name || DEF).trim(); +¶a.ts#0a +4-5:- ``` -BOF/EOF examples: +Replace lines 4..5 with one blank line (NOT a delete): ```text -¶src/a.ts -BOF↓const HEADER = true; +¶a.ts#0a +4-5: ``` +Insert at start and end of file: + ```text -¶src/a.ts -EOF↓export const done = true; +¶a.ts#0a +BOF: ++// header +EOF: ++// trailer ``` -Delete / blank examples: +Multi-file: ```text -¶src/a.ts#1a2b -4! -``` - -```text -¶src/a.ts#1a2b -4: -``` - -```text -¶src/a.ts#1a2b -4-6! -``` - -Multi-file example: - -```text -¶src/a.ts#1a2b -4:const enabled = true; -¶src/b.ts#3c4d -20! +¶src/a.ts#0a +4-4: ++const enabled = true; +¶src/b.ts#1f +20-20:- ``` ## Side Effects @@ -187,54 +215,66 @@ Multi-file example: ## Limits & Caps - Default mode is `hashline` (`DEFAULT_EDIT_MODE`) in `packages/coding-agent/src/utils/edit-mode.ts`. -- File hashes are 4 lowercase hex chars from `computeFileHash()` in `packages/coding-agent/src/hashline/hash.ts`. -- The visible mismatch report shows 2 lines of context on each side (`MISMATCH_CONTEXT`) in `packages/coding-agent/src/hashline/constants.ts`. -- Stale-anchor recovery uses `fuzzFactor: 0` (`HASHLINE_RECOVERY_FUZZ_FACTOR`) in `packages/coding-agent/src/hashline/recovery.ts`. -- The per-session read cache keeps at most 30 paths (`MAX_PATHS_PER_SESSION`) in `packages/coding-agent/src/edit/file-read-cache.ts`. -- Hashline streaming chunk defaults are 200 lines or 64 KiB per chunk (`packages/coding-agent/src/hashline/types.ts`, consumed by `packages/coding-agent/src/hashline/stream.ts`). -- `HL_OP_INSERT_BEFORE` is `↑`, `HL_OP_INSERT_AFTER` is `↓`, `HL_OP_REPLACE` is `:`, `HL_OP_DELETE` is `!`, `HL_OP_CHARS` is `↑↓:!`, `HL_FILE_PREFIX` is `¶`, `HL_FILE_HASH_SEP` is `#`, and `HL_LINE_BODY_SEP` is `:` (`packages/coding-agent/src/hashline/hash.ts`). +- File snapshot tags are exactly two lowercase hex chars minted by the per-session snapshot store. +- Each path gets a 256-slot ring. The initial slot is random, and each store randomizes slot→tag encoding, so tags are opaque rather than predictable counters. +- The visible mismatch report shows 2 lines of context on each side (`MISMATCH_CONTEXT`) in `packages/hashline/src/messages.ts`. +- Stale-anchor recovery uses `fuzzFactor: 0` in `packages/hashline/src/recovery.ts`. +- `HL_OP_REPLACE` is `:`, `HL_OP_DELETE_SUFFIX` is `:-`, `HL_PAYLOAD_REPLACE` is `+`, `HL_PAYLOAD_REPEAT` is `^`, `HL_FILE_PREFIX` is `¶`, and `HL_FILE_HASH_SEP` is `#` (`packages/hashline/src/format.ts`). ## Errors - Missing section header: - `input must begin with "¶PATH#HASH" on the first non-blank line for anchored edits; got: ...` - Empty header: - `Input header "¶" is empty; provide a file path.` -- Missing hash for anchored edit: - - `Missing hashline file hash for anchored edit to ; use ¶#hash from your latest read.` -- Line-hash anchors in edit ops: - - `line N: edit ops use bare line numbers. Copy the ¶PATH#hash header, then use anchors like 42, 42-45, BOF, or EOF.` -- Bad anchor token: - - `line N: expected a line number such as "119"; got "...".` -- Bad range syntax: - - `line N: range must be LINE or LINE-LINE (one dash, no spaces); got ...` - - `line N: range A-B ends before it starts.` -- Payload forbidden for `!`: - - `line N: ! deletes only. Payload is forbidden after !; use : to replace.` -- Missing `+` on a continuation line: - - `line N: payload continuation lines must start with +.` +- Missing tag for anchored edit: + - `Missing hashline snapshot tag for anchored edit to ; use ¶#tag from your latest read/search output.` +- Inline payload on the anchor line: + - `line N: Inline payload on the anchor line is rejected. Write the anchor on its own line (e.g. A-B:), then put the body content on the next line prefixed with + (literal) or ^A-B (repeat). …` - Stray payload line: - - `line N: payload line has no preceding ↑, ↓, :, or ! operation.` -- Unknown op: - - `line N: unrecognized op. Use LINE↑ (insert before), LINE↓ (insert after), LINE: / A-B: (replace), or LINE! / A-B! (delete).` + - `line N: payload line has no preceding A-B:, BOF:, or EOF: anchor. Got "...".` +- Raw body row with no `+` / `^` prefix in a mixed-prefix block: + - `line N: payload row in a hashline block must start with + or ^A-B. Got "...".` +- Range out of order: + - `line N: range A-B ends before it starts.` +- Overlapping ops on the same anchor: + - `line N: anchor line X is already targeted by another op on line Y. Issue ONE block per range; payload is only the final desired content, never a before/after pair.` +- BOF/EOF used with `:-`: + - `line N: BOF:/EOF: anchors are virtual positions and cannot use :-. Use +TEXT or ^A-B body rows to insert at a virtual position.` +- Lone `-` op at top level: + - `line N: a lone "-" is not a valid hashline op. To delete a range, write A-B:- on the anchor line itself (e.g. 5-7:-).` +- apply_patch / unified-diff contamination: + - `line N: apply_patch sentinel "*** …" is not valid in hashline. Use ¶PATH#HASH then A-B: / A-B:- / BOF: / EOF: blocks …` + - `line N: unified-diff hunk header (@@) is not valid in hashline. Use a ¶PATH#HASH header and bare A-B: anchor blocks.` + - `line N: apply_patch line prefix (-N: / -N-M:) is not valid in hashline. Drop the - prefix; use A-B: (replace) or A-B:- (delete) on the anchor line itself.` - Missing file for anchor-scoped edits: - `File not found: ` - Out-of-range anchor: - `Line N does not exist (file has M lines)` -- Stale file hash throws `HashlineMismatchError`. The error contains both hashes, re-read guidance, and nearby current file lines as `*LINE:TEXT` / ` LINE:TEXT`. +- Stale snapshot tag throws `MismatchError`. The error contains re-read guidance and nearby current file lines as `*LINE:TEXT` / ` LINE:TEXT`. - No-op edit: - - `Edits to resulted in no changes being made.` + - `Edits to parsed and applied cleanly, but produced no change: your body row(s) are byte-identical to the file at the targeted lines. The bug is somewhere else — re-read the file before issuing another edit. Do NOT widen the payload or add lines; verify the anchor first.` - Recovery failure is silent internally: if cache-based merge cannot prove a valid result, the mismatch error is surfaced unchanged. +## Warnings +- `Detected two identical-range hashline blocks; kept only the second block. …` (`REPLACE_PAIR_COALESCED_WARNING`) +- `Detected an overlapping bare hashline block immediately followed by a concrete block; dropped the earlier bare block. …` (`REPLACE_PAIR_COALESCED_OVERLAP_WARNING`) +- `Auto-prefixed bare body row(s) with +. Always start payload rows with +TEXT (literal) or ^A-B (repeat) …` (`BARE_BODY_AUTO_PIPED_WARNING`) +- `Converted a lone - body row to a :- delete on the preceding anchor. Write A-B:- on the anchor line itself to delete the range.` (`DASH_PAYLOAD_AUTO_DELETE_WARNING`) +- `Detected a run of single-line empty-body blocks (A-A: with no payload). Each one REPLACES its line with a blank; to delete lines use A-B:-.` (`STACKED_BLANK_REPLACE_WARNING`) +- `Auto-absorbed N duplicate line(s) above replacement (file lines A..B matched the payload's leading lines; widened the deletion to start at file line A instead of C).` +- `Auto-absorbed N duplicate line(s) below replacement …` (symmetric variant) +- `Deleted line N contains a structural bracket/brace boundary ("…"); verify the file is still balanced or use '+replacement' payload to keep the boundary intact.` — only fires when the block's net delimiter balance is not preserved by its replacement. +- Recovery banners: `RECOVERY_EXTERNAL_WARNING`, `RECOVERY_SESSION_CHAIN_WARNING`, `RECOVERY_SESSION_REPLAY_WARNING` (`packages/hashline/src/messages.ts`). + ## Notes -- `read` and `search` are the authoritative source of section hashes. Copy `¶PATH#HASH`; op lines use bare line numbers and do not want the trailing `:TEXT`. -- Multi-op patches are parsed against the original file snapshot. Do not renumber later anchors after earlier ops; `applyHashlineEdits()` buckets and applies them bottom-up. +- `read` and `search` are the authoritative source of section tags. Copy `¶PATH#TAG`; anchor lines use bare line numbers and do not carry the trailing `:TEXT`. +- Multi-op patches are parsed against the original file snapshot. Do not renumber later anchors after earlier ops; `applyEdits()` buckets and applies them bottom-up. - Failed hand-edits often come from sequentially shifting later anchors inside the same patch. Treat every op as using the line numbers from the original section header. -- Two consecutive `A-B:` ops on the *identical* range in the same hunk are coalesced: the second op's payload wins and the first is dropped (a "Detected an identical-range before/after replace pair" warning is appended). Other overlap shapes — different ranges, `A-B:` overlapping a `N!`/`N:`, or two `!` deletes on the same line — still throw `line N: anchor line X is already targeted by the :/! op on line Y`. The coalesce only fires while the first op is still pending; cross-hunk duplicates still throw. -- `A-B:` is not a primitive replace in the parser. With payload, it expands to inserts before `A` plus deletes for `A-B`. `A-B!` is the direct delete form. Bare `A:` / `A-B:` (no payload) replaces with a single blank line; bare `↑` / `↓` insert a blank line. -- Inline payload tip: trailing whitespace on the op line is trimmed. To preserve trailing spaces in the inserted/replacement content, put that content on the next line instead of inline. -- `computeFileHash()` normalizes CR characters and trailing whitespace before hashing. The section survives line-ending and trailing-space-only changes, but not substantive file edits. -- `splitHashlineInputs()` normalizes absolute `¶PATH#HASH` headers back to a cwd-relative path when the file is inside the current working tree. Headers with any run of leading `¶` chars (e.g. `¶foo.ts`, `¶¶foo.ts`, `¶¶¶foo.ts`) are accepted; the canonical form is `¶PATH#HASH` for anchored edits. -- Optional `*** Begin Patch` / `*** End Patch` markers are accepted in hashline mode, but the file sections are still `¶PATH#HASH`-based, not Codex `*** Update File:` hunks. -- `*** Abort` terminates parsing early and returns `ABORT_WARNING`; ops parsed before the marker still apply. -- File-read cache invalidation is conflict-based, not write-through invalidation. If `read` later records content for a line that disagrees with the cached snapshot, the entire snapshot for that path is replaced with the newly observed lines (`packages/coding-agent/src/edit/file-read-cache.ts`). +- Inline payload on the anchor line is rejected. Put the body content on the next line prefixed with `+` (literal) or `^A-B` (repeat). +- Trailing whitespace on body rows is preserved exactly. To preserve trailing spaces, put them in the `+TEXT` row. +- Section tags are opaque snapshot-store slots, not content hashes. A tag is valid only in the session store that minted it; if the live file no longer matches the recorded snapshot, stale-anchor recovery must prove a safe merge before writing. +- `splitRawSections()` (in `packages/hashline/src/input.ts`) normalizes absolute `¶PATH#TAG` headers back to a cwd-relative path when the file is inside the current working tree. Headers with any run of leading `¶` chars (e.g. `¶foo.ts`, `¶¶foo.ts`) are accepted; the canonical form is `¶PATH#TAG` for anchored edits. +- Optional `*** Begin Patch` / `*** End Patch` markers are accepted, but the file sections are still `¶PATH#TAG`-based, not Codex `*** Update File:` hunks. +- `*** Abort` terminates parsing silently; ops parsed before the marker still apply, but no warning is surfaced. +- Snapshot tags are not invalidated on write-through; a tag remains in its path ring until that slot wraps. If a later read records different content, it mints a new tag while old snapshots remain available for recovery until overwritten. - There is no resolve-style apply/discard phase for hashline edits. The only preview path is the transient TUI diff preview in `packages/coding-agent/src/edit/streaming.ts`. diff --git a/docs/tools/read.md b/docs/tools/read.md index 19559390d..e499c5ddb 100644 --- a/docs/tools/read.md +++ b/docs/tools/read.md @@ -14,7 +14,7 @@ - `packages/coding-agent/src/edit/notebook.ts` — convert `.ipynb` to editable `# %% [...] cell:N` text. - `packages/coding-agent/src/utils/file-display-mode.ts` — decide hashline vs line-number vs raw display. - `packages/coding-agent/src/workspace-tree.ts` — render directory trees. - - `packages/coding-agent/src/edit/file-read-cache.ts` — cache read lines for later hashline edit recovery. + - `packages/coding-agent/src/edit/file-snapshot-store.ts` — stores read lines for later hashline edit verification/recovery. - `packages/coding-agent/src/tools/index.ts` — registers `read: s => new ReadTool(s)`. ## Inputs @@ -104,8 +104,8 @@ URL selectors are parsed separately in `packages/coding-agent/src/tools/fetch.ts - hashline numbered output when edit mode is hashline, read is not raw, source is mutable, edit tool exists, and `readHashLines !== false` - otherwise optional line numbers when `readLineNumbers === true` - raw mode suppresses both -- Prefix format in hashline mode is a `¶PATH#HASH` header followed by `LINE:TEXT`, e.g. `¶src/foo.ts#1a2b` and `41:def alpha():`, from `computeFileHash()` / `formatNumberedLine()` in `packages/coding-agent/src/hashline/hash.ts`. -- The `edit`/hashline path consumes that header plus bare line numbers later; immutable sources and `:raw` intentionally suppress them. +- Prefix format in hashline mode is a `¶PATH#TAG` header followed by `LINE:TEXT`, e.g. `¶src/foo.ts#0a` and `41:def alpha():`, from the session snapshot store plus `formatNumberedLine()` / `formatHashlineHeader()`. +- The `edit`/hashline path consumes that header plus bare line numbers later; the two-hex tag is opaque and only meaningful in the session snapshot store that minted it. Immutable sources and `:raw` intentionally suppress hashline headers. ### Directory listings - `#readDirectory()` calls `buildDirectoryTree()` with: diff --git a/docs/tools/search.md b/docs/tools/search.md index 855d5fbe3..3f7b11ad5 100644 --- a/docs/tools/search.md +++ b/docs/tools/search.md @@ -29,8 +29,8 @@ ## Outputs The tool returns a single text block in `content[0].text` plus structured `details`. -- Match lines are formatted by `formatMatchLine()` as `*LINE:content` for matches and ` LINE:content` for context under a `¶PATH#HASH` header in hashline mode. - - Hashline mode: `¶src/login.ts#3c4d`, `*5:content`, ` 9:content`. +- Match lines are formatted by `formatMatchLine()` as `*LINE:content` for matches and ` LINE:content` for context under a `¶PATH#TAG` header in hashline mode. + - Hashline mode: `¶src/login.ts#1f`, `*5:content`, ` 9:content`. - Plain mode: `*5|content`, ` 9|content`. - Directory results are grouped by file, with `# ` headings and blank lines between groups. - `details` may include: @@ -141,4 +141,4 @@ The tool returns a single text block in `content[0].text` plus structured `detai - `hidden:true` is hard-coded in `search.ts`; there is no model-facing flag to exclude dotfiles. - `gitignore:false` only affects native directory traversal. It does not disable the tool's own path normalization or explicit-file handling. - When `paths` resolves to multiple exact files, `search.ts` does not apply the native `500` match cap and reports `totalMatches` internally as the post-skip length for that branch. -- The section hash in hashline mode comes from `computeFileHash()` in `packages/coding-agent/src/hashline/hash.ts`; `search` emits bare line numbers beneath it. +- The section tag in hashline mode is a two-hex opaque snapshot tag from the session snapshot store; `search` records only the matched/context lines it emits and prints bare line numbers beneath the header. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5ea3df977..2deb69659 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,14 @@ # Changelog ## [Unreleased] +### Fixed + +- Fixed `omp auth-broker serve` crashing at startup with `logger.setTransports is not a function` — switched the call site to `import { setTransports } from "@oh-my-pi/pi-utils/logger"`, bypassing the `logger` namespace re-export that some Bun versions failed to expose at runtime + +### Added + +- Added strict-mode indicators to `omp auth-gateway check` output by appending `[strict]` to strict-mode text headers and adding a top-level `strict` field in `--json` output +- `omp auth-gateway check --strict` exercises each broker-supplied credential against its provider's chat-completion endpoint (cheapest bundled chat model per provider, with 15s/attempt timeout and up to 4 catalog fall-throughs on "model not found / invalid model" errors). Surfaces failures where the usage endpoint reports 200 but the chat endpoint 401s the same bearer (revoked OAuth scope, mislabeled provider row, …). Output gains a `[chat: ok|FAIL|skip]` column in text mode and a `completion` field on each credential in `--json` mode; the chat-failed count contributes to the non-zero exit code. ### Changed - Updated hashline syntax: replaced `↑`/`↓` payload sigils with `^` repeat syntax and `|` literal rows for clearer edit semantics diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 0a566f09f..4c971dd16 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -1605,7 +1605,7 @@ export const SETTINGS_SCHEMA = { tab: "editing", label: "Hash Lines", description: - "Include file-hash headers and line numbers in read output for hashline edit mode (¶PATH#hash plus LINE:content)", + "Include snapshot-tag headers and line numbers in read output for hashline edit mode (¶PATH#tag plus LINE:content)", }, }, diff --git a/packages/coding-agent/src/edit/file-snapshot-store.ts b/packages/coding-agent/src/edit/file-snapshot-store.ts index 55c839a10..ef0d5ae42 100644 --- a/packages/coding-agent/src/edit/file-snapshot-store.ts +++ b/packages/coding-agent/src/edit/file-snapshot-store.ts @@ -2,21 +2,24 @@ * Session-bound file snapshot store. * * Used by `read` and `search` to record exactly what the model saw, and by - * the hashline patcher to recover from stale section hashes (file changed - * externally between read and edit, or a prior in-session edit advanced - * the hash). The store is the {@link InMemorySnapshotStore} implementation + * the hashline patcher to verify or recover from stale section tags (file + * changed externally between read and edit, or a prior in-session edit + * advanced the tag). The store is the {@link InMemorySnapshotStore} * from `@oh-my-pi/hashline`; the only coding-agent-specific concern here - * is wiring it onto the per-session {@link ToolSession} object. + * is wiring it onto the per-session owner object. */ import { InMemorySnapshotStore } from "@oh-my-pi/hashline"; -import type { ToolSession } from "../tools"; + +interface FileSnapshotStoreOwner { + fileSnapshotStore?: InMemorySnapshotStore; +} /** * Look up (or lazily create) the file snapshot store attached to a session. * Storage lives on `session.fileSnapshotStore` so it ages out exactly with * the session itself. */ -export function getFileSnapshotStore(session: ToolSession): InMemorySnapshotStore { +export function getFileSnapshotStore(session: FileSnapshotStoreOwner): InMemorySnapshotStore { if (!session.fileSnapshotStore) session.fileSnapshotStore = new InMemorySnapshotStore(); return session.fileSnapshotStore; } diff --git a/packages/coding-agent/src/edit/hashline/diff.ts b/packages/coding-agent/src/edit/hashline/diff.ts index ee783ef17..233c31559 100644 --- a/packages/coding-agent/src/edit/hashline/diff.ts +++ b/packages/coding-agent/src/edit/hashline/diff.ts @@ -5,16 +5,17 @@ * pair to {@link generateDiffString} so the renderer can show the diff * while the tool call is still streaming. * - * Validation is intentionally light: only the section file hash is checked + * Validation is intentionally light: only the section snapshot tag is checked * (so the preview goes red when anchors are stale), no plan-mode guards * and no auto-generated-file refusal — those belong on the write path. */ import { - computeFileHash, Patch as HashlinePatch, normalizeToLF, type Patch, type PatchSection, + type Snapshot, + type SnapshotStore, stripBom, } from "@oh-my-pi/hashline"; import { resolveToCwd } from "../../tools/path-utils"; @@ -44,20 +45,34 @@ function hasAnchorScoped(section: PatchSection): boolean { return section.hasAnchorScopedEdit; } -function validateSectionHash(section: PatchSection, text: string): string | null { +function snapshotMatchesCurrent(snapshot: Snapshot, currentText: string, anchorLines: readonly number[]): boolean { + if (snapshot.fullText !== undefined) return snapshot.fullText === currentText; + for (const lineNumber of anchorLines) { + if (snapshot.get(lineNumber) === undefined) return false; + } + return snapshot.matchesLiveFile(currentText.split("\n")); +} + +function validateSectionHash( + section: PatchSection, + absolutePath: string, + text: string, + snapshots: SnapshotStore, +): string | null { if (section.fileHash === undefined) { return hasAnchorScoped(section) - ? `Missing hashline file hash for anchored edit to ${section.path}; use \`¶${section.path}#hash\` from your latest read.` + ? `Missing hashline snapshot tag for anchored edit to ${section.path}; use \`¶${section.path}#tag\` from your latest read.` : null; } - const currentHash = computeFileHash(text); - if (currentHash === section.fileHash) return null; - return `Hashline file hash mismatch for ${section.path}: section is bound to #${section.fileHash}, but current file hashes to #${currentHash}; re-read and try again.`; + const snapshot = snapshots.byHash(absolutePath, section.fileHash); + if (snapshot && snapshotMatchesCurrent(snapshot, text, section.collectAnchorLines())) return null; + return `Hashline snapshot tag mismatch for ${section.path}: section is bound to #${section.fileHash}, but current file does not match that snapshot; re-read and try again.`; } export async function computeHashlineSectionDiff( section: PatchSection, cwd: string, + snapshots: SnapshotStore, options: HashlineDiffOptions = {}, ): Promise<{ diff: string; firstChangedLine: number | undefined } | { error: string }> { try { @@ -65,7 +80,7 @@ export async function computeHashlineSectionDiff( const rawContent = await readSectionText(absolutePath, section.path); const { text: content } = stripBom(rawContent); const normalized = normalizeToLF(content); - const hashError = validateSectionHash(section, normalized); + const hashError = validateSectionHash(section, absolutePath, normalized, snapshots); if (hashError) return { error: hashError }; const result = options.streaming ? section.applyPartialTo(normalized, options) @@ -80,6 +95,7 @@ export async function computeHashlineSectionDiff( export async function computeHashlineDiff( input: { input: string }, cwd: string, + snapshots: SnapshotStore, options: HashlineDiffOptions = {}, ): Promise<{ diff: string; firstChangedLine: number | undefined } | { error: string }> { let patch: Patch; @@ -91,5 +107,5 @@ export async function computeHashlineDiff( if (patch.sections.length !== 1) { return { error: "Streaming diff preview supports exactly one hashline section." }; } - return computeHashlineSectionDiff(patch.sections[0], cwd, options); + return computeHashlineSectionDiff(patch.sections[0], cwd, snapshots, options); } diff --git a/packages/coding-agent/src/edit/hashline/execute.ts b/packages/coding-agent/src/edit/hashline/execute.ts index 62168a314..ca50038b8 100644 --- a/packages/coding-agent/src/edit/hashline/execute.ts +++ b/packages/coding-agent/src/edit/hashline/execute.ts @@ -44,7 +44,18 @@ function getHashlineApplyOptions(session: ToolSession): { autoDropPureInsertDupl } function noChangeDiagnostic(path: string): string { - return `Edits to ${path} resulted in no changes being made.`; + // The patch parsed and applied cleanly but produced no change — the + // `|literal` body rows matched the file content at the targeted lines + // byte-for-byte. The model usually misreads this as "wrong anchor, try + // again with a bigger payload" and starts duplicating content; the + // message below names the cause directly so the next turn can re-read + // instead of expanding the patch. + return ( + `Edits to ${path} parsed and applied cleanly, but produced no change: ` + + `your body row(s) are byte-identical to the file at the targeted lines. ` + + `The bug is somewhere else — re-read the file before issuing another edit. ` + + `Do NOT widen the payload or add lines; verify the anchor first.` + ); } function assertUniqueCanonicalPaths(prepared: readonly PreparedSection[]): void { diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index cb54b77ab..1e415a671 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -312,7 +312,7 @@ const MISSING_APPLY_PATCH_END_ERROR = "The last line of the patch must be '*** E function normalizeHashlineInputPreviewPath(rawPath: string): string { const trimmed = rawPath.trim(); - const hashStart = /#[0-9a-f]{4}$/u.exec(trimmed)?.index; + const hashStart = /#[0-9a-fA-F]{3}$/u.exec(trimmed)?.index; const withoutHash = hashStart === undefined ? trimmed : trimmed.slice(0, hashStart); if (withoutHash.length < 2) return withoutHash; const first = withoutHash[0]; diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts index 9479f2fc9..5aac5d94e 100644 --- a/packages/coding-agent/src/edit/streaming.ts +++ b/packages/coding-agent/src/edit/streaming.ts @@ -20,6 +20,7 @@ import { END_PATCH_MARKER, type PatchSection as HashlineInputSection, Patch as HashlinePatch, + type SnapshotStore, } from "@oh-my-pi/hashline"; import type { Theme } from "../modes/theme/theme"; import { type EditMode, resolveEditMode } from "../utils/edit-mode"; @@ -39,6 +40,7 @@ export interface PerFileDiffPreview { export interface StreamingDiffContext { cwd: string; signal: AbortSignal; + snapshots: SnapshotStore; fuzzyThreshold?: number; allowFuzzy?: boolean; hashlineAutoDropPureInsertDuplicates?: boolean; @@ -325,7 +327,7 @@ const hashlineStrategy: EditStreamingStrategy = { // to parse; suppress until the next chunk arrives. Once args are // complete, surface the error so the model sees what went wrong. if (ctx.isStreaming) return null; - const result = await computeHashlineDiff({ input }, ctx.cwd, { + const result = await computeHashlineDiff({ input }, ctx.cwd, ctx.snapshots, { autoDropPureInsertDuplicates: ctx.hashlineAutoDropPureInsertDuplicates, }); ctx.signal.throwIfAborted(); @@ -346,7 +348,7 @@ const hashlineStrategy: EditStreamingStrategy = { for (let i = 0; i < sectionsToProcess.length; i++) { ctx.signal.throwIfAborted(); const section = sectionsToProcess[i]; - const result = await computeHashlineSectionDiff(section, ctx.cwd, { + const result = await computeHashlineSectionDiff(section, ctx.cwd, ctx.snapshots, { autoDropPureInsertDuplicates: ctx.hashlineAutoDropPureInsertDuplicates, streaming: ctx.isStreaming, }); diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index 53bf6f2cf..da8b73547 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -1,3 +1,4 @@ +import type { SnapshotStore } from "@oh-my-pi/hashline"; import type { AgentTool } from "@oh-my-pi/pi-agent-core"; import { Box, @@ -105,6 +106,7 @@ function resolveEditModeForTool(toolName: string, tool: AgentTool | undefined): } export interface ToolExecutionOptions { + snapshots?: SnapshotStore; showImages?: boolean; // default: true (only used if terminal supports images) editFuzzyThreshold?: number; editAllowFuzzy?: boolean; @@ -143,6 +145,7 @@ export class ToolExecutionComponent extends Container { #editFuzzyThreshold: number | undefined; #editAllowFuzzy: boolean | undefined; #hashlineAutoDropPureInsertDuplicates: boolean | undefined; + #snapshots?: SnapshotStore; #isPartial = true; #tool?: AgentTool; #ui: TUI; @@ -190,6 +193,7 @@ export class ToolExecutionComponent extends Container { this.#editFuzzyThreshold = options.editFuzzyThreshold; this.#editAllowFuzzy = options.editAllowFuzzy; this.#hashlineAutoDropPureInsertDuplicates = options.hashlineAutoDropPureInsertDuplicates; + this.#snapshots = options.snapshots; this.#tool = tool; this.#ui = ui; this.#cwd = cwd; @@ -266,9 +270,11 @@ export class ToolExecutionComponent extends Container { try { const isStreaming = !this.#argsComplete; + if (editMode === "hashline" && !this.#snapshots) return; const previews = await strategy.computeDiffPreview(effectiveArgs, { cwd: this.#cwd, signal: controller.signal, + snapshots: this.#snapshots!, fuzzyThreshold: this.#editFuzzyThreshold, allowFuzzy: this.#editAllowFuzzy, hashlineAutoDropPureInsertDuplicates: this.#hashlineAutoDropPureInsertDuplicates, diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index ecb8b8f99..952fe41c5 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -3,6 +3,7 @@ import { calculatePromptTokens } from "@oh-my-pi/pi-agent-core/compaction/compac import type { AssistantMessage, ImageContent } from "@oh-my-pi/pi-ai"; import { type Component, Loader, TERMINAL, Text } from "@oh-my-pi/pi-tui"; import { settings } from "../../config/settings"; +import { getFileSnapshotStore } from "../../edit/file-snapshot-store"; import { AssistantMessageComponent } from "../../modes/components/assistant-message"; import { ReadToolGroupComponent, @@ -329,6 +330,7 @@ export class EventController { content.name, renderArgs, { + snapshots: getFileSnapshotStore(this.ctx.session), showImages: settings.get("terminal.showImages"), editFuzzyThreshold: settings.get("edit.fuzzyThreshold"), editAllowFuzzy: settings.get("edit.fuzzyMatch"), @@ -444,6 +446,7 @@ export class EventController { event.toolName, event.args, { + snapshots: getFileSnapshotStore(this.ctx.session), showImages: settings.get("terminal.showImages"), editFuzzyThreshold: settings.get("edit.fuzzyThreshold"), editAllowFuzzy: settings.get("edit.fuzzyMatch"), diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 1304ac0ee..3508d2f67 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -2,6 +2,7 @@ import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import type { AssistantMessage, ImageContent, Message } from "@oh-my-pi/pi-ai"; import { type Component, Spacer, Text, TruncatedText } from "@oh-my-pi/pi-tui"; import { settings } from "../../config/settings"; +import { getFileSnapshotStore } from "../../edit/file-snapshot-store"; import { AssistantMessageComponent } from "../../modes/components/assistant-message"; import { BashExecutionComponent } from "../../modes/components/bash-execution"; import { BranchSummaryMessageComponent } from "../../modes/components/branch-summary-message"; @@ -377,6 +378,7 @@ export class UiHelpers { content.name, renderArgs, { + snapshots: getFileSnapshotStore(this.ctx.session), showImages: settings.get("terminal.showImages"), editFuzzyThreshold: settings.get("edit.fuzzyThreshold"), editAllowFuzzy: settings.get("edit.fuzzyMatch"), diff --git a/packages/coding-agent/src/prompts/tools/ast-edit.md b/packages/coding-agent/src/prompts/tools/ast-edit.md index 1be7238f9..2b2986f0c 100644 --- a/packages/coding-agent/src/prompts/tools/ast-edit.md +++ b/packages/coding-agent/src/prompts/tools/ast-edit.md @@ -14,7 +14,7 @@ Performs structural AST-aware rewrites via native ast-grep. -- Replacement summary, per-file replacement counts, and change diffs as `¶src/foo.ts#1a2b`, `-12:before`, `+12:after` lines in hashline mode +- Replacement summary, per-file replacement counts, and change diffs as `¶src/foo.ts#0a`, `-12:before`, `+12:after` lines in hashline mode - Parse issues when files cannot be processed diff --git a/packages/coding-agent/src/prompts/tools/ast-grep.md b/packages/coding-agent/src/prompts/tools/ast-grep.md index c35809682..48502520b 100644 --- a/packages/coding-agent/src/prompts/tools/ast-grep.md +++ b/packages/coding-agent/src/prompts/tools/ast-grep.md @@ -18,7 +18,7 @@ Performs structural code search using AST matching via native ast-grep. - Grouped matches with file path, byte range, line/column ranges, metavariable captures -- Match lines are numbered under a file-hash header in hashline mode: `¶src/foo.ts#1a2b`, `*42:content` for the matched line, ` 43:content` for context +- Match lines are numbered under a file snapshot tag header in hashline mode: `¶src/foo.ts#0a`, `*42:content` for the matched line, ` 43:content` for context - Summary counts (`totalMatches`, `filesWithMatches`, `filesSearched`) and parse issues when present diff --git a/packages/coding-agent/src/prompts/tools/read.md b/packages/coding-agent/src/prompts/tools/read.md index b8b05fc3d..391565d20 100644 --- a/packages/coding-agent/src/prompts/tools/read.md +++ b/packages/coding-agent/src/prompts/tools/read.md @@ -28,7 +28,7 @@ Append `:` to `path`. The bare path falls back to the default mode. - Reading a directory path returns a depth-limited dirent listing. {{#if IS_HL_MODE}} -- Reading a file with an explicit selector emits a file-hash header and numbered lines: `¶src/foo.ts#1a2b` then `41:def alpha():`. Copy the `¶PATH#HASH` header for anchored edits; ops use bare line numbers. NEVER fabricate the hash. +- Reading a file with an explicit selector emits a file snapshot tag header and numbered lines: `¶src/foo.ts#0a` then `41:def alpha():`. Copy the `¶PATH#TAG` header for anchored edits; ops use bare line numbers. NEVER fabricate the tag. {{else}} {{#if IS_LINE_NUMBER_MODE}} - Reading a file with an explicit selector returns lines prefixed with line numbers: `41|def alpha():`. diff --git a/packages/coding-agent/src/prompts/tools/search.md b/packages/coding-agent/src/prompts/tools/search.md index 3753b88b8..b011fbea1 100644 --- a/packages/coding-agent/src/prompts/tools/search.md +++ b/packages/coding-agent/src/prompts/tools/search.md @@ -9,7 +9,7 @@ Searches files using powerful regex matching. {{#if IS_HL_MODE}} -- Text output emits a file-hash header per matched file plus numbered lines: `¶src/login.ts#3c4d`, `*42:if (user.id) {` (match), ` 43:return user;` (context). Copy the header for anchored edits; ops use bare line numbers. +- Text output emits a file snapshot tag header per matched file plus numbered lines: `¶src/login.ts#1f`, `*42:if (user.id) {` (match), ` 43:return user;` (context). Copy the header for anchored edits; ops use bare line numbers. {{else}} {{#if IS_LINE_NUMBER_MODE}} - Text output is line-number-prefixed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 127bc040c..d9803d2a4 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -18,6 +18,7 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { scheduler } from "node:timers/promises"; import { isPromise } from "node:util/types"; +import type { InMemorySnapshotStore } from "@oh-my-pi/hashline"; import { type AfterToolCallContext, type AfterToolCallResult, @@ -104,6 +105,7 @@ import { onAppendOnlyModeChanged } from "../config/settings"; import { RawSseDebugBuffer } from "../debug/raw-sse-buffer"; import { loadCapability } from "../discovery"; import { expandApplyPatchToEntries, normalizeDiff, normalizeToLF, ParseError, previewPatch, stripBom } from "../edit"; +import { getFileSnapshotStore } from "../edit/file-snapshot-store"; import { disposeKernelSessionsByOwner, executePython as executePythonCommand, @@ -738,6 +740,7 @@ export class AgentSession { readonly sessionManager: SessionManager; readonly settings: Settings; readonly yieldQueue: YieldQueue; + fileSnapshotStore?: InMemorySnapshotStore; #powerAssertion: MacOSPowerAssertion | undefined; @@ -4156,6 +4159,7 @@ export class AgentSession { const fileMentionMessages = await generateFileMentionMessages(fileMentions, this.sessionManager.getCwd(), { autoResizeImages: this.settings.get("images.autoResize"), useHashLines: resolveFileDisplayMode(this).hashLines, + snapshotStore: getFileSnapshotStore(this), }); messages.push(...fileMentionMessages); } diff --git a/packages/coding-agent/src/tools/ast-edit.ts b/packages/coding-agent/src/tools/ast-edit.ts index 1e7d47b29..60c00a6c9 100644 --- a/packages/coding-agent/src/tools/ast-edit.ts +++ b/packages/coding-agent/src/tools/ast-edit.ts @@ -1,11 +1,13 @@ import * as path from "node:path"; -import { computeFileHash, formatHashlineHeader } from "@oh-my-pi/hashline"; +import { formatHashlineHeader } from "@oh-my-pi/hashline"; import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; import { type AstReplaceChange, type AstReplaceFileChange, astEdit } from "@oh-my-pi/pi-natives"; import type { Component } from "@oh-my-pi/pi-tui"; import { Text } from "@oh-my-pi/pi-tui"; import { $envpos, prompt, untilAborted } from "@oh-my-pi/pi-utils"; import * as z from "zod/v4"; +import { getFileSnapshotStore } from "../edit/file-snapshot-store"; +import { normalizeToLF } from "../edit/normalize"; import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import type { Theme } from "../modes/theme/theme"; import astEditDescription from "../prompts/tools/ast-edit.md" with { type: "text" }; @@ -281,14 +283,15 @@ export class AstEditTool implements AgentTool(); + const hashContexts = new Map(); if (useHashLines) { + const snapshotStore = getFileSnapshotStore(this.session); for (const relativePath of fileList) { const absolutePath = path.resolve(this.session.cwd, relativePath); try { - const fullText = await Bun.file(absolutePath).text(); - const fileHash = computeFileHash(fullText); - hashContexts.set(relativePath, { fileHash }); + const fullText = normalizeToLF(await Bun.file(absolutePath).text()); + const tag = snapshotStore.recordContiguous(absolutePath, 1, fullText.split("\n"), { fullText }); + hashContexts.set(relativePath, { tag }); } catch { // Best-effort: if a file disappears between ast-edit and rendering, emit plain line output. } @@ -326,7 +329,7 @@ export class AstEditTool implements AgentTool(); + const hashContexts = new Map(); + const snapshotStore = useHashLines ? getFileSnapshotStore(this.session) : undefined; if (useHashLines) { for (const relativePath of fileList) { const absolutePath = path.resolve(this.session.cwd, relativePath); try { - const fullText = await Bun.file(absolutePath).text(); - const fileHash = computeFileHash(fullText); - hashContexts.set(relativePath, { absolutePath, fileHash }); + await access(absolutePath, constants.R_OK); + hashContexts.set(relativePath, { absolutePath }); } catch { // Best-effort: if a file disappears between ast-grep and rendering, emit plain line output. } @@ -268,9 +270,8 @@ export class AstGrepTool implements AgentTool 0) { - getFileSnapshotStore(this.session).recordSparse(hashContext.absolutePath, cacheEntries, { - fileHash: hashContext.fileHash, - }); + const tag = snapshotStore?.recordSparse(hashContext.absolutePath, cacheEntries); + if (tag) hashContext.tag = tag; } return { model: modelOut, display: displayOut }; }; @@ -282,7 +283,7 @@ export class AstGrepTool implements AgentTool { +async function readHashlineHeaderContext( + session: ToolSession, + absolutePath: string, + cwd: string, +): Promise { const fullText = await Bun.file(absolutePath).text(); - return buildHashlineHeaderContext(formatPathRelativeToCwd(absolutePath, cwd), fullText); + const context = recordFullHashlineContext( + session, + absolutePath, + formatPathRelativeToCwd(absolutePath, cwd), + fullText, + ); + if (!context) throw new ToolError(`Cannot record hashline snapshot for non-absolute path: ${absolutePath}`); + return context; +} + +function hashlineHeaderContext(displayPath: string, tag: string): HashlineHeaderContext { + return { header: formatHashlineHeader(displayPath, tag), tag }; } function prependHashlineHeader(text: string, context: HashlineHeaderContext | undefined): string { return context ? `${context.header}\n${text}` : text; } -function recordHashlineSnapshot( - session: ToolSession, - absolutePath: string | undefined, - context: HashlineHeaderContext | undefined, -): void { - if (!context || !absolutePath || !path.isAbsolute(absolutePath)) return; - getFileSnapshotStore(session).recordContiguous(absolutePath, 1, context.fullText.split("\n"), { - fullText: context.fullText, - fileHash: context.fileHash, - }); -} - function formatTextWithMode( text: string, startNum: number, @@ -841,9 +852,13 @@ export class ReadTool implements AgentTool { const shouldAddLineNumbers = shouldAddHashLines ? false : displayMode.lineNumbers; const hashContext = shouldAddHashLines && options.sourcePath - ? buildHashlineHeaderContext(formatPathRelativeToCwd(options.sourcePath, this.session.cwd), text) + ? recordFullHashlineContext( + this.session, + options.sourcePath, + formatPathRelativeToCwd(options.sourcePath, this.session.cwd), + text, + ) : undefined; - recordHashlineSnapshot(this.session, options.sourcePath, hashContext); let emittedHashlineHeader = false; const formatText = (content: string, startNum: number): string => { details.displayContent = { text: content, startLine: startNum }; @@ -934,9 +949,13 @@ export class ReadTool implements AgentTool { const shouldAddLineNumbers = shouldAddHashLines ? false : displayMode.lineNumbers; const hashContext = shouldAddHashLines && options.sourcePath - ? buildHashlineHeaderContext(formatPathRelativeToCwd(options.sourcePath, this.session.cwd), text) + ? recordFullHashlineContext( + this.session, + options.sourcePath, + formatPathRelativeToCwd(options.sourcePath, this.session.cwd), + text, + ) : undefined; - recordHashlineSnapshot(this.session, options.sourcePath, hashContext); let emittedHashlineHeader = false; const resultBuilder = toolResult(details); @@ -1014,11 +1033,7 @@ export class ReadTool implements AgentTool { const shouldAddHashLines = !rawSelector && displayMode.hashLines; const shouldAddLineNumbers = rawSelector ? false : shouldAddHashLines ? false : displayMode.lineNumbers; - const hashContext = shouldAddHashLines - ? await readHashlineHeaderContext(absolutePath, this.session.cwd) - : undefined; - recordHashlineSnapshot(this.session, absolutePath, hashContext); - let emittedHashlineHeader = false; + const sparseSnapshotEntries: Array = []; const maxColumns = resolveOutputMaxColumns(this.session.settings); const blocks: string[] = []; @@ -1058,22 +1073,19 @@ export class ReadTool implements AgentTool { } } - if (collectedLines.length > 0) { - getFileSnapshotStore(this.session).recordContiguous( - absolutePath, - range.startLine, - collectedLines, - hashContext ? { fullText: hashContext.fullText, fileHash: hashContext.fileHash } : {}, - ); + for (let index = 0; index < collectedLines.length; index++) { + sparseSnapshotEntries.push([range.startLine + index, collectedLines[index]]); } const blockText = collectedLines.join("\n"); - const formatted = formatTextWithMode(blockText, range.startLine, shouldAddHashLines, shouldAddLineNumbers); - blocks.push(hashContext && !emittedHashlineHeader ? prependHashlineHeader(formatted, hashContext) : formatted); - if (hashContext) emittedHashlineHeader = true; + blocks.push(formatTextWithMode(blockText, range.startLine, shouldAddHashLines, shouldAddLineNumbers)); } let outputText = blocks.join("\n\n…\n\n"); + if (shouldAddHashLines && sparseSnapshotEntries.length > 0 && outputText) { + const tag = getFileSnapshotStore(this.session).recordSparse(absolutePath, sparseSnapshotEntries); + outputText = `${formatHashlineHeader(formatPathRelativeToCwd(absolutePath, this.session.cwd), tag)}\n${outputText}`; + } if (notices.length > 0) { outputText = outputText ? `${outputText}\n${notices.join("\n")}` : notices.join("\n"); } @@ -1726,9 +1738,8 @@ export class ReadTool implements AgentTool { renderedSummary.elidedLines, ); const summaryHashContext = displayMode.hashLines - ? await readHashlineHeaderContext(absolutePath, this.session.cwd) + ? await readHashlineHeaderContext(this.session, absolutePath, this.session.cwd) : undefined; - recordHashlineSnapshot(this.session, absolutePath, summaryHashContext); const bodyText = footer ? `${renderedSummary.text}\n\n${footer}` : renderedSummary.text; const modelText = prependHashlineHeader(bodyText, summaryHashContext); details = { @@ -1875,17 +1886,19 @@ export class ReadTool implements AgentTool { const shouldAddHashLines = !rawSelector && displayMode.hashLines; const shouldAddLineNumbers = rawSelector ? false : shouldAddHashLines ? false : displayMode.lineNumbers; - const hashContext = shouldAddHashLines - ? await readHashlineHeaderContext(absolutePath, this.session.cwd) - : undefined; - - if (collectedLines.length > 0 && !firstLineExceedsLimit) { - getFileSnapshotStore(this.session).recordContiguous( - absolutePath, - startLineDisplay, - collectedLines, - hashContext ? { fullText: hashContext.fullText, fileHash: hashContext.fileHash } : {}, - ); + let hashContext: HashlineHeaderContext | undefined; + if (shouldAddHashLines && collectedLines.length > 0 && !firstLineExceedsLimit) { + const store = getFileSnapshotStore(this.session); + const tag = + offset === undefined && limit === undefined && !wasTruncated && columnTruncated === 0 + ? (() => { + const normalized = normalizeToLF(selectedContent); + return store.recordContiguous(absolutePath, 1, normalized.split("\n"), { + fullText: normalized, + }); + })() + : store.recordContiguous(absolutePath, startLineDisplay, collectedLines); + hashContext = hashlineHeaderContext(formatPathRelativeToCwd(absolutePath, this.session.cwd), tag); } let capturedDisplayContent: { text: string; startLine: number } | undefined; @@ -2031,9 +2044,11 @@ export class ReadTool implements AgentTool { const rawText = region.lines.join("\n"); const hashContext = shouldAddHashLines - ? await readHashlineHeaderContext(entry.absolutePath, this.session.cwd) + ? hashlineHeaderContext( + formatPathRelativeToCwd(entry.absolutePath, this.session.cwd), + getFileSnapshotStore(this.session).recordContiguous(entry.absolutePath, region.startLine, region.lines), + ) : undefined; - recordHashlineSnapshot(this.session, entry.absolutePath, hashContext); const formattedBody = formatTextWithMode(rawText, region.startLine, shouldAddHashLines, shouldAddLineNumbers); const formattedText = prependHashlineHeader(formattedBody, hashContext); diff --git a/packages/coding-agent/src/tools/search.ts b/packages/coding-agent/src/tools/search.ts index f923f7e19..c1fc405d5 100644 --- a/packages/coding-agent/src/tools/search.ts +++ b/packages/coding-agent/src/tools/search.ts @@ -1,7 +1,8 @@ -import { mkdtemp, rm, stat, writeFile } from "node:fs/promises"; +import { constants } from "node:fs"; +import { access, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import * as path from "node:path"; -import { computeFileHash, formatHashlineHeader } from "@oh-my-pi/hashline"; +import { formatHashlineHeader } from "@oh-my-pi/hashline"; import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; import { type GrepMatch, GrepOutputMode, type GrepResult, grep } from "@oh-my-pi/pi-natives"; import type { Component } from "@oh-my-pi/pi-tui"; @@ -609,16 +610,16 @@ export class SearchTool implements AgentTool(); + const hashContexts = new Map(); + const snapshotStore = baseDisplayMode.hashLines ? getFileSnapshotStore(this.session) : undefined; if (baseDisplayMode.hashLines) { for (const relativePath of fileList) { if (archiveDisplaySet.has(relativePath)) continue; const absoluteFilePath = path.resolve(this.session.cwd, relativePath); if (immutableSourcePaths.has(absoluteFilePath)) continue; try { - const fullText = await Bun.file(absoluteFilePath).text(); - const fileHash = computeFileHash(fullText); - hashContexts.set(relativePath, { absolutePath: absoluteFilePath, fileHash }); + await access(absoluteFilePath, constants.R_OK); + hashContexts.set(relativePath, { absolutePath: absoluteFilePath }); } catch { // Best-effort: if the file disappeared between grep and render, fall back to plain line output. } @@ -671,9 +672,8 @@ export class SearchTool implements AgentTool 0 && hashContext) { - getFileSnapshotStore(this.session).recordSparse(hashContext.absolutePath, cacheEntries, { - fileHash: hashContext.fileHash, - }); + const tag = snapshotStore?.recordSparse(hashContext.absolutePath, cacheEntries); + if (tag) hashContext.tag = tag; } return { model: modelOut, display: displayOut }; }; @@ -684,7 +684,7 @@ export class SearchTool implements AgentTool { if (filePaths.length === 0) return []; @@ -354,9 +355,14 @@ export async function generateFileMentionMessages( } const content = await Bun.file(absolutePath).text(); - let { output, lineCount } = buildTextOutput(content); - if (options?.useHashLines) { - output = `${formatHashlineHeader(resolvedPath, computeFileHash(content))}\n${formatNumberedLines(output)}`; + const snapshotStore = options?.useHashLines ? options.snapshotStore : undefined; + const normalized = snapshotStore ? normalizeToLF(content) : content; + let { output, lineCount } = buildTextOutput(normalized); + if (snapshotStore) { + const tag = snapshotStore.recordContiguous(absolutePath, 1, normalized.split("\n"), { + fullText: normalized, + }); + output = `${formatHashlineHeader(resolvedPath, tag)}\n${formatNumberedLines(output)}`; } files.push({ path: resolvedPath, content: output, lineCount }); } catch { diff --git a/packages/coding-agent/test/core/hashline.test.ts b/packages/coding-agent/test/core/hashline.test.ts index c17ccf99c..a2b88c3ca 100644 --- a/packages/coding-agent/test/core/hashline.test.ts +++ b/packages/coding-agent/test/core/hashline.test.ts @@ -6,11 +6,11 @@ import { type ApplyOptions, applyEdits, buildCompactDiffPreview as buildCompactHashlineDiffPreview, - computeFileHash, detectLineEnding, type Edit, InMemorySnapshotStore as FileReadCache, Filesystem, + formatHashlineHeader, MismatchError as HashlineMismatchError, NotFoundError, Patch, @@ -65,14 +65,14 @@ function tryRecoverHashlineWithCache(args: { cache: FileReadCache; absolutePath: string; currentText: string; - fileHash: string; + tag: string; edits: readonly Edit[]; options?: ApplyOptions; }): { text: string; lines: string; firstChangedLine: number | undefined; warnings: string[] } | null { const recovered = new Recovery(args.cache).tryRecover({ path: args.absolutePath, currentText: args.currentText, - fileHash: args.fileHash, + fileHash: args.tag, edits: args.edits, options: args.options, }); @@ -86,7 +86,7 @@ beforeAll(async () => { await Settings.init({ inMemory: true, cwd: process.cwd() }); }); -const repl = (text: string): string => `|${text}`; +const repl = (text: string): string => `+${text}`; const repeat = (start: string, end = start): string => `^${start}-${end}`; const outputSep = ":"; const outputSepRe = ":"; @@ -95,8 +95,12 @@ function tag(line: number, _content: string): string { return `${line}`; } -function header(filePath: string, content: string): string { - return `¶${filePath}#${computeFileHash(content)}`; +function recordFullSnapshot(cache: FileReadCache, filePath: string, fullText: string): string { + return cache.recordContiguous(filePath, 1, fullText.split("\n"), { fullText }); +} + +function header(filePath: string, tag: string): string { + return formatHashlineHeader(filePath, tag); } function sameLineRange(anchor: string): string { @@ -232,9 +236,12 @@ describe("hashline parser — range-anchor syntax", () => { expect(applyDiff(content, range)).toBe("aaa\nBBB\nCCC"); }); - it("rejects the removed bare `A:` shorthand as a normal unrecognized row", () => { + it("accepts bare `A:` as a shorthand for `A-A:`", () => { const anchor = tag(2, "bbb"); - expect(() => parseHashline(`${anchor}:\n${repl("BBB")}`)).toThrow(/payload line has no preceding/); + // `LINE:` is the exact shape `read` renders each file row as, so the + // parser leniently treats it as `LINE-LINE:` for models that + // reproduce the read-output shape as an anchor. + expect(applyDiff(content, `${anchor}:\n${repl("BBB")}`)).toBe("aaa\nBBB\nccc"); }); it("replaces empty anchor blocks with one blank line", () => { @@ -270,6 +277,9 @@ describe("hashline parser — range-anchor syntax", () => { }); it("escapes literal leading payload sigils with literal rows", () => { + // `+` is the canonical sigil; payload rows like `+|literal` emit + // `|literal` verbatim. Same for `^literal` and `↓literal` — none of + // these are recognized sigils once they sit inside a `+TEXT` row. const diff = [`${sameLineRange(tag(2, "bbb"))}:`, repl("|literal"), repl("^literal"), repl("↓literal")].join( "\n", ); @@ -653,7 +663,7 @@ describe("hashline parser — range-anchor syntax", () => { }); }); -describe("hashline — file hash binding", () => { +describe("hashline — snapshot tag binding", () => { it("rejects line-hash anchors as unrecognized payload lines", () => { expect(() => parseHashline(`2ab:\n${repl("BBB")}`).edits).toThrow(/payload line has no preceding/); }); @@ -665,11 +675,11 @@ describe("hashline — file hash binding", () => { }); describe("splitHashlineInput — ¶ headers", () => { - it("extracts path, file hash, and diff body from ¶path#hash header", () => { - const input = [`¶src/foo.ts#1a2b`, `${sameLineRange(tag(2, "bbb"))}:`, repl("BBB")].join("\n"); + it("extracts path, snapshot tag, and diff body from ¶path#tag header", () => { + const input = [`¶src/foo.ts#0A3`, `${sameLineRange(tag(2, "bbb"))}:`, repl("BBB")].join("\n"); expect(splitHashlineInput(input)).toEqual({ path: "src/foo.ts", - fileHash: "1a2b", + fileHash: "0A3", diff: `${sameLineRange(tag(2, "bbb"))}:\n${repl("BBB")}`, }); }); @@ -730,16 +740,21 @@ it("preflights write policy for every section before committing a batch", async ], ["b.ts"], ); + const snapshots = new FileReadCache(); + const aTag = recordFullSnapshot(snapshots, "a.ts", "aaa\n"); + const bTag = recordFullSnapshot(snapshots, "b.ts", "bbb\n"); const input = [ - header("a.ts", "aaa\n"), + header("a.ts", aTag), `${sameLineRange(tag(1, "aaa"))}:`, repl("AAA"), - header("b.ts", "bbb\n"), + header("b.ts", bTag), `${sameLineRange(tag(1, "bbb"))}:`, repl("BBB"), ].join("\n"); - await expect(new Patcher({ fs: fixture }).apply(Patch.parse(input))).rejects.toThrow(/blocked write: b\.ts/); + await expect(new Patcher({ fs: fixture, snapshots }).apply(Patch.parse(input))).rejects.toThrow( + /blocked write: b\.ts/, + ); expect(fixture.get("a.ts")).toBe("aaa\n"); expect(fixture.get("b.ts")).toBe("bbb\n"); }); @@ -757,7 +772,7 @@ describe("hashline executor", () => { await withTempDir(async tempDir => { const filePath = path.join(tempDir, "a.ts"); const source = ["aaa", "bbb", "ccc"].join("\n"); - const input = `${header("a.ts", source)}\nEOF:\n${repl("bbb")}\n${repl("ccc")}\n${repl("NEW")}\n`; + const input = `¶a.ts\nEOF:\n${repl("bbb")}\n${repl("ccc")}\n${repl("NEW")}\n`; await Bun.write(filePath, source); await executeHashlineSingle(hashlineExecuteOptions(tempDir, input)); expect(await Bun.file(filePath).text()).toBe("aaa\nbbb\nccc\nbbb\nccc\nNEW"); @@ -770,15 +785,37 @@ describe("hashline executor", () => { }); }); + it("emits an actionable no-op diagnostic when the payload matches the file byte-for-byte", async () => { + await withTempDir(async tempDir => { + const filePath = path.join(tempDir, "a.ts"); + const source = "aaa\nbbb\nccc\n"; + await Bun.write(filePath, source); + const session = makeHashlineSession(tempDir); + const sourceTag = recordFullSnapshot(getFileReadCache(session), filePath, source); + // Replace line 2 with `bbb` — identical to the file content. The + // patch applies but produces no change. + const input = `${header("a.ts", sourceTag)}\n${sameLineRange(tag(2, "bbb"))}:\n${repl("bbb")}\n`; + const result = await executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session)); + const text = result.content[0]?.type === "text" ? result.content[0].text : ""; + expect(text).toContain("parsed and applied cleanly, but produced no change"); + expect(text).toContain("byte-identical to the file"); + expect(text).toContain("re-read the file"); + // The file is untouched. + expect(await Bun.file(filePath).text()).toBe(source); + }); + }); + it("preflights every section before writing multi-file edits", async () => { await withTempDir(async tempDir => { const aPath = path.join(tempDir, "a.ts"); const bPath = path.join(tempDir, "b.ts"); await Bun.write(aPath, "aaa\n"); await Bun.write(bPath, "bbb\n"); - const bHeader = "¶b.ts#0000"; + const session = makeHashlineSession(tempDir); + const aTag = recordFullSnapshot(getFileReadCache(session), aPath, "aaa\n"); + const bHeader = "¶b.ts#fff"; const input = [ - header("a.ts", "aaa\n"), + header("a.ts", aTag), `${sameLineRange(tag(1, "aaa"))}:`, repl("AAA"), bHeader, @@ -786,9 +823,9 @@ describe("hashline executor", () => { repl("BBB"), ].join("\n"); - await expect(executeHashlineSingle(hashlineExecuteOptions(tempDir, input))).rejects.toThrow( - /file changed between read and edit|file hashes to/, - ); + await expect( + executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session)), + ).rejects.toThrow(/file changed between read and edit|file hashes to|section is bound to/); expect(await Bun.file(aPath).text()).toBe("aaa\n"); expect(await Bun.file(bPath).text()).toBe("bbb\n"); }); @@ -799,18 +836,20 @@ describe("hashline executor", () => { const filePath = path.join(tempDir, "a.ts"); const source = "one\ntwo\n"; await Bun.write(filePath, source); + const session = makeHashlineSession(tempDir); + const sourceTag = recordFullSnapshot(getFileReadCache(session), filePath, source); const input = [ - header("a.ts", source), + header("a.ts", sourceTag), `${sameLineRange(tag(1, "one"))}:`, repl("ONE"), - header("./a.ts", source), + header("./a.ts", sourceTag), `${sameLineRange(tag(2, "two"))}:`, repl("TWO"), ].join("\n"); - await expect(executeHashlineSingle(hashlineExecuteOptions(tempDir, input))).rejects.toThrow( - /resolve to the same file/, - ); + await expect( + executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session)), + ).rejects.toThrow(/resolve to the same file/); expect(await Bun.file(filePath).text()).toBe(source); }); }); @@ -820,6 +859,8 @@ describe("hashline executor", () => { const filePath = path.join(tempDir, "a.ts"); const original = ["L1", "L2", "L3", "L4", "L5", "L6", "L7", "L8", "L9", "L10"].join("\n"); await Bun.write(filePath, `${original}\n`); + const session = makeHashlineSession(tempDir); + const originalTag = recordFullSnapshot(getFileReadCache(session), filePath, `${original}\n`); // Two sections, both anchored against the ORIGINAL file. Section 1 expands // line 2 into 9 lines (net +8 shift). Section 2's anchor points at line 8 @@ -827,7 +868,7 @@ describe("hashline executor", () => { // A naive sequential apply reads the modified disk and fails anchor // validation outright. const input = [ - header("a.ts", `${original}\n`), + header("a.ts", originalTag), `${sameLineRange(tag(2, "L2"))}:`, repl("L2a"), repl("L2b"), @@ -838,13 +879,13 @@ describe("hashline executor", () => { repl("L2g"), repl("L2h"), repl("L2i"), - header("a.ts", `${original}\n`), + header("a.ts", originalTag), `${sameLineRange(tag(8, "L8"))}:`, repeat(tag(8, "L8")), repl("INSERTED"), ].join("\n"); - await executeHashlineSingle(hashlineExecuteOptions(tempDir, input)); + await executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session)); expect(await Bun.file(filePath).text()).toBe( [ @@ -949,10 +990,7 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { const session = makeHashlineSession(tempDir); // Simulate the read tool having shown V0 to the model in this session. - getFileReadCache(session).recordContiguous(filePath, 1, v0Text.split("\n"), { - fullText: v0Text, - fileHash: computeFileHash(v0Text), - }); + const v0Tag = recordFullSnapshot(getFileReadCache(session), filePath, v0Text); // External actor (linter, subagent, user) prepends 7 lines. Anchors // authored against V0 no longer match V1, so the model's edit cannot @@ -962,7 +1000,7 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { await Bun.write(filePath, `${v1Lines.join("\n")}\n`); // Model authors anchor against V0 — line 2 is "L2" in V0. - const input = `${header("a.ts", v0Text)}\n${sameLineRange(tag(2, "L2"))}:\n${repl("L2-MODEL")}\n`; + const input = `${header("a.ts", v0Tag)}\n${sameLineRange(tag(2, "L2"))}:\n${repl("L2-MODEL")}\n`; const result = await executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session)); const finalLines = (await Bun.file(filePath).text()).replace(/\n$/, "").split("\n"); @@ -987,17 +1025,15 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { await Bun.write(filePath, v0Text); const session = makeHashlineSession(tempDir); - // Cache only covers the first three lines — enough to retain the file hash + // Cache only covers the first three lines — enough to retain the snapshot tag // but not enough to synthesize the requested pre-edit snapshot. - getFileReadCache(session).recordContiguous(filePath, 1, v0Lines.slice(0, 3), { - fileHash: computeFileHash(v0Text), - }); + const v0Tag = getFileReadCache(session).recordContiguous(filePath, 1, v0Lines.slice(0, 3)); const v1Lines = [...v0Lines]; v1Lines[5] = "L6-CHANGED"; await Bun.write(filePath, `${v1Lines.join("\n")}\n`); - const input = `${header("a.ts", v0Text)}\n${sameLineRange(tag(6, "L6"))}:\n${repl("L6-MODEL")}\n`; + const input = `${header("a.ts", v0Tag)}\n${sameLineRange(tag(6, "L6"))}:\n${repl("L6-MODEL")}\n`; await expect( executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session)), ).rejects.toThrow(HashlineMismatchError); @@ -1010,10 +1046,7 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { const cache = new FileReadCache(); const fakePath = "/tmp/__hashline-recovery-applypatch__.ts"; const snapshotText = "alpha\nbeta\ngamma\ndelta\nepsilon"; - cache.recordContiguous(fakePath, 1, snapshotText.split("\n"), { - fullText: snapshotText, - fileHash: computeFileHash(snapshotText), - }); + const snapshotTag = recordFullSnapshot(cache, fakePath, snapshotText); // Live file is completely different — patch context cannot match even // with fuzz tolerance. @@ -1025,7 +1058,7 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { absolutePath: fakePath, currentText, edits, - fileHash: computeFileHash(snapshotText), + tag: snapshotTag, options: {}, }); expect(recovered).toBeNull(); @@ -1049,21 +1082,20 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { const session = makeHashlineSession(tempDir); // Initial read populates the cache with V0. - getFileReadCache(session).recordContiguous(filePath, 1, v0Text.split("\n"), { - fullText: v0Text, - fileHash: computeFileHash(v0Text), - }); + const v0Tag = recordFullSnapshot(getFileReadCache(session), filePath, v0Text); // First edit: change line 2 : BETA. After the write, the cache should // reflect V1 (post-edit), not V0. - const firstInput = `${header("a.ts", v0Text)}\n${sameLineRange(tag(2, "beta"))}:\n${repl("BETA")}\n`; + const firstInput = `${header("a.ts", v0Tag)}\n${sameLineRange(tag(2, "beta"))}:\n${repl("BETA")}\n`; await executeHashlineSingle(hashlineExecuteOptions(tempDir, firstInput, undefined, session)); const v1Lines = ["alpha", "BETA", "gamma", "delta", "epsilon"]; - expect(await Bun.file(filePath).text()).toBe(`${v1Lines.join("\n")}\n`); + const v1Text = `${v1Lines.join("\n")}\n`; + expect(await Bun.file(filePath).text()).toBe(v1Text); + const v1Tag = recordFullSnapshot(getFileReadCache(session), filePath, v1Text); const snap = getFileReadCache(session).head(filePath); - expect(snap?.lines.get(1)).toBe("alpha"); - expect(snap?.lines.get(2)).toBe("BETA"); - expect(snap?.lines.get(3)).toBe("gamma"); + expect(snap?.get(1)).toBe("alpha"); + expect(snap?.get(2)).toBe("BETA"); + expect(snap?.get(3)).toBe("gamma"); // External actor prepends 7 lines after the edit. Anchors authored // against V1 (the post-edit state the model just observed) no longer @@ -1072,7 +1104,7 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { const v2Lines = ["H1", "H2", "H3", "H4", "H5", "H6", "H7", ...v1Lines]; await Bun.write(filePath, `${v2Lines.join("\n")}\n`); - const secondInput = `${header("a.ts", `${v1Lines.join("\n")}\n`)}\n${sameLineRange(tag(3, "gamma"))}:\n${repl("GAMMA")}\n`; + const secondInput = `${header("a.ts", v1Tag)}\n${sameLineRange(tag(3, "gamma"))}:\n${repl("GAMMA")}\n`; const result = await executeHashlineSingle(hashlineExecuteOptions(tempDir, secondInput, undefined, session)); const finalLines = (await Bun.file(filePath).text()).replace(/\n$/, "").split("\n"); @@ -1093,13 +1125,10 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { await Bun.write(filePath, v0Text); const session = makeHashlineSession(tempDir); - getFileReadCache(session).recordContiguous(filePath, 1, v0Text.split("\n"), { - fullText: v0Text, - fileHash: computeFileHash(v0Text), - }); + const v0Tag = recordFullSnapshot(getFileReadCache(session), filePath, v0Text); // First edit lands cleanly against v0: line 5 becomes L5-FIRST. - const firstInput = `${header("a.ts", v0Text)}\n${sameLineRange(tag(5, "L5"))}:\n${repl("L5-FIRST")}\n`; + const firstInput = `${header("a.ts", v0Tag)}\n${sameLineRange(tag(5, "L5"))}:\n${repl("L5-FIRST")}\n`; await executeHashlineSingle(hashlineExecuteOptions(tempDir, firstInput, undefined, session)); const v1Lines = [...v0Lines]; @@ -1110,7 +1139,7 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { // again targets line 5 — the very line the first edit rewrote. // Recovery must refuse so the model re-reads instead of silently // overwriting L5-FIRST with payload authored against L5. - const secondInput = `${header("a.ts", v0Text)}\n${sameLineRange(tag(5, "L5"))}:\n${repl("L5-SECOND")}\n`; + const secondInput = `${header("a.ts", v0Tag)}\n${sameLineRange(tag(5, "L5"))}:\n${repl("L5-SECOND")}\n`; await expect( executeHashlineSingle(hashlineExecuteOptions(tempDir, secondInput, undefined, session)), ).rejects.toThrow(HashlineMismatchError); @@ -1125,20 +1154,14 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { const v1Text = "L1\nL2-EDITED\nL3\nL4\nL5\nL6\nL7\nL8\nL9\nL10\n"; const currentText = "L1\nL2-EDITED\nL3\nL4\nL5\nL6\nL7\nL8\nL9\nL10\nTRAILER\n"; - cache.recordContiguous(fakePath, 1, v0Text.split("\n"), { - fullText: v0Text, - fileHash: computeFileHash(v0Text), - }); - cache.recordContiguous(fakePath, 1, v1Text.split("\n"), { - fullText: v1Text, - fileHash: computeFileHash(v1Text), - }); + const v0Tag = recordFullSnapshot(cache, fakePath, v0Text); + recordFullSnapshot(cache, fakePath, v1Text); const recovered = tryRecoverHashlineWithCache({ cache, absolutePath: fakePath, currentText, - fileHash: computeFileHash(v0Text), + tag: v0Tag, edits: parseHashline(`10-10:\n${repl("L10-EDITED")}`).edits, options: {}, }); @@ -1147,22 +1170,18 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { expect(recovered?.lines).toContain("L10-EDITED"); }); - it("retains older file hashes in the per-path snapshot ring", () => { + it("retains older snapshot tags in the per-path snapshot ring", () => { const cache = new FileReadCache(); const fakePath = "/tmp/__hashline-cache-ring__.ts"; - const versions = ["one\n", "two\n", "three\n"]; - for (const version of versions) { - cache.recordContiguous(fakePath, 1, version.split("\n"), { - fullText: version, - fileHash: computeFileHash(version), - }); - } - expect(cache.head(fakePath)?.fileHash).toBe(computeFileHash("three\n")); - expect(cache.byHash(fakePath, computeFileHash("one\n"))?.fullText).toBe("one\n"); - expect(cache.byHash(fakePath, computeFileHash("two\n"))?.fullText).toBe("two\n"); + const oneTag = recordFullSnapshot(cache, fakePath, "one\n"); + const twoTag = recordFullSnapshot(cache, fakePath, "two\n"); + recordFullSnapshot(cache, fakePath, "three\n"); + expect(cache.head(fakePath)?.fullText).toBe("three\n"); + expect(cache.byHash(fakePath, oneTag)?.fullText).toBe("one\n"); + expect(cache.byHash(fakePath, twoTag)?.fullText).toBe("two\n"); }); - it("drops a cached entry when newly recorded lines disagree on overlap", () => { + it("pushes a fresh snapshot when newly recorded lines disagree on overlap", () => { const cache = new FileReadCache(); const fakePath = "/tmp/__hashline-cache-conflict__.ts"; cache.recordContiguous(fakePath, 1, ["a", "b", "c", "d", "e"]); @@ -1177,29 +1196,28 @@ describe("hashline — anchor-stale recovery via read snapshot cache", () => { const snap = cache.head(fakePath); expect(snap).not.toBeNull(); // Old entries dropped; only the divergent record's entries remain. - expect(snap?.lines.has(1)).toBe(false); - expect(snap?.lines.has(2)).toBe(false); - expect(snap?.lines.get(4)).toBe("D-CHANGED"); - expect(snap?.lines.get(7)).toBe("g"); + expect(snap?.get(1)).toBeUndefined(); + expect(snap?.get(2)).toBeUndefined(); + expect(snap?.get(4)).toBe("D-CHANGED"); + expect(snap?.get(7)).toBe("g"); }); - it("evicts old paths past the per-session LRU cap", () => { + it("keeps independently tracked path rings without an LRU cap", () => { const cache = new FileReadCache(); - // Cap is 30 paths. Insert 32 distinct paths; the oldest two must evict. for (let i = 0; i < 32; i++) { - cache.recordContiguous(`/tmp/file-${i}.ts`, 1, ["x"]); + cache.recordContiguous(`/tmp/file-${i}.ts`, 1, [`x${i}`]); } - expect(cache.head("/tmp/file-0.ts")).toBeNull(); - expect(cache.head("/tmp/file-1.ts")).toBeNull(); - expect(cache.head("/tmp/file-2.ts")).not.toBeNull(); - expect(cache.head("/tmp/file-31.ts")).not.toBeNull(); + expect(cache.head("/tmp/file-0.ts")?.get(1)).toBe("x0"); + expect(cache.head("/tmp/file-1.ts")?.get(1)).toBe("x1"); + expect(cache.head("/tmp/file-2.ts")?.get(1)).toBe("x2"); + expect(cache.head("/tmp/file-31.ts")?.get(1)).toBe("x31"); }); }); describe("hashline *** Abort recovery sentinel (harmony-leak mitigation)", () => { const sentinel = "*** Abort"; - it("parser breaks at *** Abort and surfaces a warning", () => { + it("parser breaks at *** Abort silently (no warning)", () => { const diff = [ `${sameLineRange(tag(1, "alpha"))}:`, repeat(tag(1, "alpha")), @@ -1212,8 +1230,10 @@ describe("hashline *** Abort recovery sentinel (harmony-leak mitigation)", () => const { edits, warnings } = parseHashline(diff); expect(edits).toHaveLength(3); expect(edits[1]).toMatchObject({ kind: "insert", text: "HELLO" }); - expect(warnings.length).toBeGreaterThan(0); - expect(warnings[0]).toMatch(/truncated mid-call/i); + // The "*** Abort" marker terminates parsing but no longer surfaces a + // warning: by the time the marker arrives the stream is already gone + // and the prior wording ("truncated mid-call") was speculative. + expect(warnings).toEqual([]); }); it("appended sentinel from harmony-leak truncation: ops above are preserved", () => { @@ -1222,7 +1242,7 @@ describe("hashline *** Abort recovery sentinel (harmony-leak mitigation)", () => const { edits, warnings } = parseHashline(diff); expect(edits).toHaveLength(3); expect(edits[1]).toMatchObject({ text: "KEPT" }); - expect(warnings.length).toBeGreaterThan(0); + expect(warnings).toEqual([]); }); it("splitter respects *** Abort like *** End Patch", () => { @@ -1253,34 +1273,33 @@ describe("hashline *** Abort recovery sentinel (harmony-leak mitigation)", () => describe("hashline parser — delete and empty-block semantics", () => { it("inline delete deletes a single line", () => { const text = "line1\nline2\nline3\n"; - const { diff } = splitHashlineInput(`${header("a.ts", text)}\n2-2:-\n`); + const { diff } = splitHashlineInput(`¶a.ts\n2-2:-\n`); expect(applyDiff(text, diff)).toBe("line1\nline3\n"); }); it("inline delete deletes the range", () => { const text = "line1\nline2\nline3\nline4\n"; - const { diff } = splitHashlineInput(`${header("a.ts", text)}\n2-3:-\n`); + const { diff } = splitHashlineInput(`¶a.ts\n2-3:-\n`); expect(applyDiff(text, diff)).toBe("line1\nline4\n"); }); it("an `A-B:` anchor with no payload becomes a blank-line replacement", () => { const text = "line1\nline2\nline3\n"; - const { diff } = splitHashlineInput(`${header("a.ts", text)}\n2-2:\n`); + const { diff } = splitHashlineInput(`¶a.ts\n2-2:\n`); expect(applyDiff(text, diff)).toBe("line1\n\nline3\n"); }); it("`A-B:` with inline body is still rejected", () => { - const text = "line1\nline2\nline3\n"; - const { diff } = splitHashlineInput(`${header("a.ts", text)}\n2-2:replacement\n`); + const { diff } = splitHashlineInput(`¶a.ts\n2-2:replacement\n`); expect(() => parseHashline(diff)).toThrow(/Inline payload on the anchor line is rejected/); }); it("explicit empty literal rows insert blank lines when the anchor is repeated", () => { const text = "line1\nline2\nline3\n"; - const aboveDiff = splitHashlineInput(`${header("a.ts", text)}\n2-2:\n${repl("")}\n${repeat("2")}\n`).diff; + const aboveDiff = splitHashlineInput(`¶a.ts\n2-2:\n${repl("")}\n${repeat("2")}\n`).diff; expect(applyDiff(text, aboveDiff)).toBe("line1\n\nline2\nline3\n"); - const belowDiff = splitHashlineInput(`${header("a.ts", text)}\n2-2:\n${repeat("2")}\n${repl("")}\n`).diff; + const belowDiff = splitHashlineInput(`¶a.ts\n2-2:\n${repeat("2")}\n${repl("")}\n`).diff; expect(applyDiff(text, belowDiff)).toBe("line1\nline2\n\nline3\n"); }); }); @@ -1288,28 +1307,28 @@ describe("hashline parser — delete and empty-block semantics", () => { describe("hashline parser — explicit blank payload rows", () => { it("raw blank lines between ops are ignored", () => { const text = "a\nb\nc\nd\ne\n"; - const ops = `${header("a.ts", text)}\n1-1:\n${repl("A")}\n\n3-3:\n${repl("C")}\n`; + const ops = `¶a.ts\n1-1:\n${repl("A")}\n\n3-3:\n${repl("C")}\n`; const { diff } = splitHashlineInput(ops); expect(applyDiff(text, diff)).toBe("A\nb\nC\nd\ne\n"); }); it("empty replace payload rows are appended as blank payload lines", () => { const text = "a\nb\nc\nd\ne\n"; - const ops = `${header("a.ts", text)}\n1-1:\n${repl("A")}\n${repl("")}\n${repl("")}\n3-3:\n${repl("C")}\n`; + const ops = `¶a.ts\n1-1:\n${repl("A")}\n${repl("")}\n${repl("")}\n3-3:\n${repl("C")}\n`; const { diff } = splitHashlineInput(ops); expect(applyDiff(text, diff)).toBe("A\n\n\nb\nC\nd\ne\n"); }); it("`A-A:` followed by two empty replace rows replaces the line with two blanks", () => { const text = "a\nb\nc\nd\ne\n"; - const ops = `${header("a.ts", text)}\n2-2:\n${repl("")}\n${repl("")}\n4-4:\n${repl("D")}\n`; + const ops = `¶a.ts\n2-2:\n${repl("")}\n${repl("")}\n4-4:\n${repl("D")}\n`; const { diff } = splitHashlineInput(ops); expect(applyDiff(text, diff)).toBe("a\n\n\nc\nD\ne\n"); }); it("empty replace row inside payload between two content lines is preserved", () => { const text = "a\nb\nc\n"; - const ops = `${header("a.ts", text)}\n2-2:\n${repl("first")}\n${repl("")}\n${repl("second")}\n`; + const ops = `¶a.ts\n2-2:\n${repl("first")}\n${repl("")}\n${repl("second")}\n`; const { diff } = splitHashlineInput(ops); expect(applyDiff(text, diff)).toBe("a\nfirst\n\nsecond\nc\n"); }); diff --git a/packages/coding-agent/test/edit-diff.test.ts b/packages/coding-agent/test/edit-diff.test.ts index 3f0e9b21c..89f47c207 100644 --- a/packages/coding-agent/test/edit-diff.test.ts +++ b/packages/coding-agent/test/edit-diff.test.ts @@ -2,10 +2,10 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; +import { formatHashlineHeader, InMemorySnapshotStore } from "@oh-my-pi/hashline"; import { adjustIndentation, computeEditDiff, - computeFileHash, computeHashlineDiff, DEFAULT_FUZZY_THRESHOLD, findMatch, @@ -238,8 +238,11 @@ describe("computeHashlineDiff", () => { // `1-1:` with the same line in the replace bucket is a true no-op: the edit // fires through computeHashlineDiff but produces identical content. - const input = `¶${sourcePath}#${computeFileHash(`${line}\n`)}\n1-1:\n|${line}\n`; - const result = await computeHashlineDiff({ input }, tempDir); + const text = `${line}\n`; + const snapshotStore = new InMemorySnapshotStore(); + const tag = snapshotStore.recordContiguous(sourcePath, 1, text.split("\n"), { fullText: text }); + const input = `${formatHashlineHeader(sourcePath, tag)}\n1-1:\n|${line}\n`; + const result = await computeHashlineDiff({ input }, tempDir, snapshotStore); expect("error" in result).toBe(true); if ("error" in result) { expect(result.error).toContain("No changes would be made"); @@ -250,14 +253,22 @@ describe("computeHashlineDiff", () => { const sourcePath = path.join(tempDir, "source.txt"); await Bun.write(sourcePath, "first\n"); - const result = await computeHashlineDiff({ input: `¶${sourcePath}\nEOF:\n|second` }, tempDir); + const result = await computeHashlineDiff( + { input: `¶${sourcePath}\nEOF:\n|second` }, + tempDir, + new InMemorySnapshotStore(), + ); expect("diff" in result).toBe(true); if ("diff" in result) { expect(result.diff).toContain("second"); } }); test("returns a handled error when the source path is a local URL", async () => { - const result = await computeHashlineDiff({ input: "¶local://PLAN.md\nEOF:\n|x" }, tempDir); + const result = await computeHashlineDiff( + { input: "¶local://PLAN.md\nEOF:\n|x" }, + tempDir, + new InMemorySnapshotStore(), + ); expect("error" in result).toBe(true); if ("error" in result) { diff --git a/packages/hashline/README.md b/packages/hashline/README.md index 79e1d63c7..c161f591d 100644 --- a/packages/hashline/README.md +++ b/packages/hashline/README.md @@ -19,15 +19,15 @@ import { } from "@oh-my-pi/hashline"; const fs = new InMemoryFilesystem(); -await fs.writeText( - "hello.ts", - `const greeting = "hi";\nexport { greeting };\n`, -); +const snapshots = new InMemorySnapshotStore(); +const before = `const greeting = "hi";\nexport { greeting };\n`; +await fs.writeText("hello.ts", before); -const patcher = new Patcher({ fs }); -const patch = Patch.parse(String.raw`¶hello.ts +const tag = snapshots.recordContiguous("hello.ts", 1, before.split("\n"), { fullText: before }); +const patcher = new Patcher({ fs, snapshots }); +const patch = Patch.parse(String.raw`¶hello.ts#${tag} 1-1: -|const greeting = "hello";`); ++const greeting = "hello";`); const result = await patcher.apply(patch); console.log(result.sections[0].op); // "update" @@ -39,17 +39,17 @@ console.log(await fs.readText("hello.ts")); See [`src/prompt.md`](./src/prompt.md) for the user-facing description and [`src/grammar.lark`](./src/grammar.lark) for the formal grammar. -Each hunk starts with a `¶PATH#HASH` header. The hash is a 4-hex-character -xxHash32 truncation of the file's LF-normalized content. The hash protects -against stale anchors: if the file changed between the read that produced the -hash and the edit, the patcher refuses (or, with a `SnapshotStore`, tries -session-aware recovery). +Each hunk starts with a `¶PATH#TAG` header. The tag is a 3-hex opaque pointer +into the `SnapshotStore` that minted it; it is not content-derived and is not +meaningful outside that store. The patcher protects against stale anchors by +resolving the tag, verifying the recorded snapshot lines against live file +content, and refusing or attempting session-aware recovery on mismatch. Inside a hunk: - `A-B:` — anchor lines A..B (use `A-A:` for a single line; no shorthand). - `A-B:-` — delete lines A..B. - `BOF:` / `EOF:` — virtual anchors at the beginning/end of file. -- `|TEXT` — literal body row. +- `+TEXT` — literal body row (use `+` alone for a blank line). - `^A-B` — repeat original file lines A..B inline (`^A-A` for one line). - Empty body — write one blank line at the anchor/virtual position. @@ -67,9 +67,9 @@ text-document protocol, a Git tree, anything. ### `SnapshotStore` -Optional. When provided to `Patcher`, hashline tries to recover from a stale -section hash by replaying the edit against a cached pre-edit snapshot of the -file and 3-way-merging onto the current content. See `recovery.ts`. +Required. Hashline tags are opaque store pointers, so `Patcher` must receive +the store that minted them. Recovery replays edits against the cached pre-edit +snapshot and 3-way-merges onto current content when the live file diverged. ### `Patcher` diff --git a/packages/hashline/bench/recovery-session-chain.ts b/packages/hashline/bench/recovery-session-chain.ts index 26b840619..feb99d330 100644 --- a/packages/hashline/bench/recovery-session-chain.ts +++ b/packages/hashline/bench/recovery-session-chain.ts @@ -22,13 +22,7 @@ * Sizes (50/500/5000 lines) × edit batch (1/8 anchors) so the O(N) split cost * in `verifyAnchorContent` and the O(K) anchor walk both surface. */ -import { - computeFileHash, - InMemorySnapshotStore, - parsePatch, - RECOVERY_SESSION_REPLAY_WARNING, - Recovery, -} from "../src"; +import { InMemorySnapshotStore, parsePatch, RECOVERY_SESSION_REPLAY_WARNING, Recovery } from "../src"; const ITERATIONS = Number(Bun.env.HASHLINE_BENCH_ITERATIONS ?? "5000"); const PATH = "/tmp/__hashline-recovery-bench__.ts"; @@ -49,17 +43,15 @@ function seed(lines: number, rewrittenLine: number): Fixture { v1Lines[rewrittenLine - 1] = `line ${rewrittenLine} REWRITTEN`; const v0Text = `${v0Lines.join("\n")}\n`; const v1Text = `${v1Lines.join("\n")}\n`; - const h0 = computeFileHash(v0Text); - const h1 = computeFileHash(v1Text); const store = new InMemorySnapshotStore(); - store.recordContiguous(PATH, 1, v0Text.split("\n"), { fullText: v0Text, fileHash: h0 }); - store.recordContiguous(PATH, 1, v1Text.split("\n"), { fullText: v1Text, fileHash: h1 }); + const h0 = store.recordContiguous(PATH, 1, v0Text.split("\n"), { fullText: v0Text }); + store.recordContiguous(PATH, 1, v1Text.split("\n"), { fullText: v1Text }); return { store, v1Text, h0 }; } /** Build an N-anchor edit batch whose anchor lines are distinct rows. */ function batchPatch(anchors: readonly number[]): string { - return anchors.map(line => `${line}-${line}:\n+line ${line} MODEL`).join("\n"); + return anchors.map(line => `${line}-${line}:\n|line ${line} MODEL`).join("\n"); } interface Case { @@ -99,7 +91,7 @@ for (const size of [50, 500, 5000] as const) { } function bench(name: string, fn: () => void): { totalMs: number; perOpUs: number } { - // Warm: JIT + InMemorySnapshotStore LRU touches. + // Warm: JIT + InMemorySnapshotStore map/ring touches. for (let i = 0; i < Math.min(50, ITERATIONS); i++) fn(); const start = Bun.nanoseconds(); for (let i = 0; i < ITERATIONS; i++) fn(); diff --git a/packages/hashline/package.json b/packages/hashline/package.json index f6f2874c4..958cdc83f 100644 --- a/packages/hashline/package.json +++ b/packages/hashline/package.json @@ -33,8 +33,7 @@ "fmt": "biome format --write ." }, "dependencies": { - "diff": "catalog:", - "lru-cache": "catalog:" + "diff": "catalog:" }, "devDependencies": { "@types/bun": "catalog:" diff --git a/packages/hashline/src/apply.ts b/packages/hashline/src/apply.ts index ef088d6c0..b80bb4cdb 100644 --- a/packages/hashline/src/apply.ts +++ b/packages/hashline/src/apply.ts @@ -386,6 +386,7 @@ interface PureInsertGroup { function cursorMatches(a: Cursor, b: Cursor): boolean { if (a.kind !== b.kind) return false; if (a.kind === "bof" || a.kind === "eof") return true; + if (b.kind === "bof" || b.kind === "eof") return false; return a.anchor.line === b.anchor.line; } @@ -614,9 +615,9 @@ function absorbReplacementBoundaryDuplicates( if (!emittedAbsorbKeys.has(key)) { emittedAbsorbKeys.add(key); warnings.push( - `Auto-absorbed ${safePrefixCount} duplicate line(s) above replacement at line ${group.sourceLineNum} ` + + `Auto-absorbed ${safePrefixCount} duplicate line(s) above replacement ` + `(file lines ${absorbStart}..${startLine - 1} matched the payload's leading lines; ` + - `widened the deletion to absorb them).`, + `widened the deletion to start at file line ${absorbStart} instead of ${startLine}).`, ); } } @@ -626,9 +627,9 @@ function absorbReplacementBoundaryDuplicates( if (!emittedAbsorbKeys.has(key)) { emittedAbsorbKeys.add(key); warnings.push( - `Auto-absorbed ${safeSuffixCount} duplicate line(s) below replacement at line ${group.sourceLineNum} ` + + `Auto-absorbed ${safeSuffixCount} duplicate line(s) below replacement ` + `(file lines ${endLine + 1}..${absorbEnd} matched the payload's trailing lines; ` + - `widened the deletion to absorb them).`, + `widened the deletion to end at file line ${absorbEnd} instead of ${endLine}).`, ); } } @@ -727,23 +728,6 @@ export function applyEdits(text: string, edits: Edit[], options: ApplyOptions = } if (insertLines.length === 0 && replacementLines.length === 0 && !deleteLine) continue; - const hasReplacementPayload = replacementLines.length > 0; - if (deleteLine && !hasReplacementPayload) { - const balance = computeDelimiterBalance([currentLine]); - const trimmedCurrentLine = currentLine.trim(); - const touchesStructuralBoundary = - trimmedCurrentLine.startsWith(")") || - trimmedCurrentLine.startsWith("]") || - trimmedCurrentLine.startsWith("}") || - trimmedCurrentLine.endsWith("(") || - trimmedCurrentLine.endsWith("[") || - trimmedCurrentLine.endsWith("{"); - if (balance.paren !== 0 || balance.bracket !== 0 || balance.brace !== 0 || touchesStructuralBoundary) { - warnings.push( - `Deleted line ${line} contains a structural bracket/brace boundary (${JSON.stringify(trimmedCurrentLine)}); verify the file is still balanced or use '|replacement' payload to keep the boundary intact.`, - ); - } - } const replacement = deleteLine ? [...insertLines, ...replacementLines] : [...insertLines, ...replacementLines, currentLine]; diff --git a/packages/hashline/src/format.ts b/packages/hashline/src/format.ts index 3b13bb91c..0dabf6350 100644 --- a/packages/hashline/src/format.ts +++ b/packages/hashline/src/format.ts @@ -1,7 +1,7 @@ /** - * Hashline format primitives: sigils, separators, regex fragments, and the - * file-hash computation. These are the single source of truth for the - * parser, the tokenizer, the prompt, and the formal grammar. + * Hashline format primitives: sigils, separators, regex fragments, and + * display helpers. These are the single source of truth for the parser, the + * tokenizer, the prompt, and the formal grammar. */ /** Anchor terminator for every hashline operation block. */ @@ -11,7 +11,7 @@ export const HL_OP_REPLACE = ":"; export const HL_OP_DELETE_SUFFIX = ":-"; /** Payload sigil for literal body rows. */ -export const HL_PAYLOAD_REPLACE = "|"; +export const HL_PAYLOAD_REPLACE = "+"; /** Payload sigil for body rows that repeat original file lines. */ export const HL_PAYLOAD_REPEAT = "^"; @@ -21,7 +21,7 @@ export const HL_PAYLOAD_CHARS = `${HL_PAYLOAD_REPLACE}${HL_PAYLOAD_REPEAT}`; /** Hashline edit file-section header marker. */ export const HL_FILE_PREFIX = "¶"; -/** Separator between a hashline file path and its file hash. */ +/** Separator between a hashline file path and its opaque snapshot tag. */ export const HL_FILE_HASH_SEP = "#"; /** Separator between a line number and displayed line content in hashline mode. */ @@ -54,8 +54,11 @@ export const HL_PAYLOAD_REPEAT_RE = new RegExp( `^\\${HL_PAYLOAD_REPEAT}${HL_LINE_CAPTURE_RE_RAW}-${HL_LINE_CAPTURE_RE_RAW}$`, ); -/** Four-hex-character file hash carried by a hashline section header. */ -export const HL_FILE_HASH_RE_RAW = `[0-9a-f]{4}`; +/** Number of hex characters in an opaque snapshot tag. */ +export const HL_FILE_HASH_LENGTH = 3; + +/** Canonical uppercase hexadecimal opaque snapshot tag carried by a hashline section header. */ +export const HL_FILE_HASH_RE_RAW = `[0-9A-F]{${HL_FILE_HASH_LENGTH}}`; /** Capture-group form of {@link HL_FILE_HASH_RE_RAW}. */ export const HL_FILE_HASH_CAPTURE_RE_RAW = `(${HL_FILE_HASH_RE_RAW})`; @@ -64,10 +67,10 @@ export const HL_FILE_HASH_CAPTURE_RE_RAW = `(${HL_FILE_HASH_RE_RAW})`; export const HL_LINE_BODY_SEP_RE_RAW = regexEscape(HL_LINE_BODY_SEP); /** - * Representative file hashes for use in user-facing error messages and prompt - * examples. + * Representative snapshot tags for use in user-facing error messages and + * prompt examples. */ -export const HL_FILE_HASH_EXAMPLES = ["1a2b", "3c4d", "9f3e"] as const; +export const HL_FILE_HASH_EXAMPLES = ["0A3", "1F7", "3C9"] as const; /** * Format a comma-separated list of example anchors with an optional line-number @@ -78,26 +81,7 @@ export function describeAnchorExamples(linePrefix = ""): string { return examples.map(e => `"${e}"`).join(", "); } -function normalizeFileHashText(text: string): string { - return text - .replace(/\r/g, "") - .split("\n") - .map(line => line.trimEnd()) - .join("\n"); -} - -/** - * Compute the 4-hex-character hash carried by a hashline section header. The - * hash normalizes CR characters and trailing whitespace before hashing so - * platform line endings and display-trimmed lines do not invalidate anchors. - */ -export function computeFileHash(text: string): string { - const normalized = normalizeFileHashText(text); - const low16 = Bun.hash.xxHash32(normalized, 0) & 0xffff; - return low16.toString(16).padStart(4, "0"); -} - -/** Format a hashline section header for a file path and file hash. */ +/** Format a hashline section header for a file path and snapshot tag. */ export function formatHashlineHeader(filePath: string, fileHash: string): string { return `${HL_FILE_PREFIX}${filePath}${HL_FILE_HASH_SEP}${fileHash}`; } diff --git a/packages/hashline/src/grammar.lark b/packages/hashline/src/grammar.lark index 4d8d877f7..03881ee47 100644 --- a/packages/hashline/src/grammar.lark +++ b/packages/hashline/src/grammar.lark @@ -6,12 +6,12 @@ hunk: update_hunk update_hunk: "¶" filename ("#" file_hash)? LF block* filename: /([^\s#]+)/ -file_hash: /[0-9a-f]{4}/ +file_hash: /[0-9A-F]{3}/ block: anchor ":" delete_suffix? LF payload* delete_suffix: "-" payload: literal_payload | repeat_payload -literal_payload: "|" /[^\n]*/ LF +literal_payload: "+" /[^\n]*/ LF repeat_payload: "^" range LF anchor: range | "BOF" | "EOF" diff --git a/packages/hashline/src/input.ts b/packages/hashline/src/input.ts index bded952ba..6a393993f 100644 --- a/packages/hashline/src/input.ts +++ b/packages/hashline/src/input.ts @@ -52,7 +52,7 @@ function parseHashlineHeaderLine(line: string, cwd?: string): RawSection | null const token = TOKENIZER.tokenize(trimmed); if (token.kind !== "header") { throw new Error( - `Input header must be ${HL_FILE_PREFIX}PATH or ${HL_FILE_PREFIX}PATH${HL_FILE_HASH_SEP}HASH with a 4-hex file hash; got ${JSON.stringify(trimmed)}.`, + `Input header must be ${HL_FILE_PREFIX}PATH or ${HL_FILE_PREFIX}PATH${HL_FILE_HASH_SEP}TAG with a 3-hex snapshot tag; got ${JSON.stringify(trimmed)}.`, ); } @@ -113,7 +113,7 @@ function splitRawSections(input: string, options: SplitOptions = {}): RawSection const preview = JSON.stringify(firstLine.slice(0, 120)); throw new Error( `input must begin with "${HL_FILE_PREFIX}PATH${HL_FILE_HASH_SEP}HASH" on the first non-blank line for anchored edits; got: ${preview}. ` + - `Example: "${HL_FILE_PREFIX}src/foo.ts${HL_FILE_HASH_SEP}1a2b" then edit ops.`, + `Example: "${HL_FILE_PREFIX}src/foo.ts${HL_FILE_HASH_SEP}0A3" then edit ops.`, ); } @@ -223,8 +223,8 @@ export class PatchSection { /** * Apply this section's edits to `text` and return the post-edit result. - * Pure: does no I/O, does not validate the section file hash. The - * {@link Patcher} owns hash validation and recovery; reach for this + * Pure: does no I/O, does not validate the section snapshot tag. The + * {@link Patcher} owns tag validation and recovery; reach for this * method directly when you've already validated the file content and * just want the result. */ @@ -318,7 +318,7 @@ function mergeSamePathSections(sections: RawSection[]): RawSection[] { existing.fileHash !== section.fileHash ) { throw new Error( - `Conflicting hashline file hashes for ${section.path}: #${existing.fileHash} and #${section.fileHash}. Re-read the file and retry with one current header.`, + `Conflicting hashline snapshot tags for ${section.path}: #${existing.fileHash} and #${section.fileHash}. Re-read the file and retry with one current header.`, ); } if (existing.fileHash === undefined && section.fileHash !== undefined) existing.fileHash = section.fileHash; diff --git a/packages/hashline/src/messages.ts b/packages/hashline/src/messages.ts index 691ebf593..4a9a915a5 100644 --- a/packages/hashline/src/messages.ts +++ b/packages/hashline/src/messages.ts @@ -22,10 +22,13 @@ export const END_PATCH_MARKER = "*** End Patch"; */ export const ABORT_MARKER = "*** Abort"; -/** Warning text appended to the tool result when {@link ABORT_MARKER} terminates parsing. */ -export const ABORT_WARNING = - "Tool stream truncated mid-call due to detected output corruption. Applied ops above are valid. Re-issue any remaining edits."; - +// `ABORT_MARKER` (`*** Abort`) still terminates parsing — see the `abort` +// token in `tokenizer.ts` — but no longer surfaces a warning to the caller. +// The earlier wording ("Tool stream truncated mid-call due to detected +// output corruption") was always speculative: by the time we observe the +// marker the stream is already gone, the warning could not actually +// describe the cause, and downstream consumers were not differentiating it +// from any other warning anyway. /** * Warning text appended when two consecutive blocks target the exact same * concrete range. The second block wins; the first block is discarded. @@ -33,12 +36,56 @@ export const ABORT_WARNING = export const REPLACE_PAIR_COALESCED_WARNING = "Detected two identical-range hashline blocks; kept only the second block. Issue ONE block per range — payload is the final desired content, never both old and new."; +/** + * Warning text appended when a bare anchor block (`A-B:` with no payload) + * is followed by an overlapping concrete block. The earlier bare block is + * dropped on the assumption that the model expressed an old/new pair + * across two anchors; only the second block's payload is applied. + */ +export const REPLACE_PAIR_COALESCED_OVERLAP_WARNING = + "Detected an overlapping bare hashline block immediately followed by a concrete block; dropped the earlier bare block. Issue ONE block per range — payload is the final desired content, never both old and new."; + +/** + * Warning text appended when bare body rows (no `+` / `^` prefix) follow a + * concrete anchor and the parser auto-converts them to `+literal` rows + * because no `+`/`^` row was present in the block. Helps the model learn + * the canonical body-row syntax while keeping the patch applying. + */ +export const BARE_BODY_AUTO_PIPED_WARNING = + "Auto-prefixed bare body row(s) with `+`. Always start payload rows with `+TEXT` (literal) or `^A-B` (repeat) — pasting raw code as payload is not a portable shape."; + +/** + * Warning text appended when a lone `-` body row is retroactively converted + * to a `:-` delete on the preceding bare anchor. Models occasionally write + * `A-B:` followed by a `-` row when they meant `A-B:-`. + */ +export const DASH_PAYLOAD_AUTO_DELETE_WARNING = + "Converted a lone `-` body row to a `:-` delete on the preceding anchor. Write `A-B:-` on the anchor line itself to delete the range."; + +/** + * Warning text appended when a single contiguous run of two or more + * single-line empty-body blocks (`A-A:` with no payload) is flushed. + * These commonly indicate the model thought `A-A:` deletes the line; it + * actually replaces with a blank line. Suggest `A-B:-` instead. + */ +export const STACKED_BLANK_REPLACE_WARNING = + "Detected a run of single-line empty-body blocks (`A-A:` with no payload). Each one REPLACES its line with a blank; to delete lines use `A-B:-`."; + +/** + * Warning text emitted when a body row begins with `+^A-B` — the model + * mistakenly prefixed a repeat row with the `+` literal sigil. We reroute + * the row as a `^A-B` repeat so the patch still applies, then surface this + * warning so the model sees the mistake on the next turn. + */ +export const PLUS_PREFIXED_REPEAT_WARNING = + "A body row started with `+^A-B`. `+` (literal text) and `^A-B` (repeat) are sibling row kinds — a row uses exactly one of them. Treated as `^A-B`; remove the leading `+` next time."; + /** Error text prefix emitted when an anchor line carries inline payload. */ export const INLINE_PAYLOAD_REJECTED_PREFIX = "Inline payload on the anchor line is rejected."; /** Error text emitted when inline delete targets BOF/EOF. */ export const VIRTUAL_REPLACE_REJECTED_MESSAGE = - "BOF:/EOF: anchors are virtual positions and cannot use `:-`. Use `|TEXT` or `^A-B` body rows to insert at a virtual position."; + "BOF:/EOF: anchors are virtual positions and cannot use `:-`. Use `+TEXT` or `^A-B` body rows to insert at a virtual position."; /** Error text emitted when `^A` repeat shorthand is used. */ export const REPEAT_SHORTHAND_REJECTED_MESSAGE = diff --git a/packages/hashline/src/mismatch.ts b/packages/hashline/src/mismatch.ts index 274b22044..d59693589 100644 --- a/packages/hashline/src/mismatch.ts +++ b/packages/hashline/src/mismatch.ts @@ -1,5 +1,5 @@ /** - * Error type raised when a section's file-hash does not match the live file + * Error type raised when a section's snapshot tag does not match the live file * content and recovery is unavailable / has failed. * * Carries enough context to render a useful diagnostic: the anchored lines @@ -15,8 +15,8 @@ const LINE_REF_RE = /^\s*[>+\-*]*\s*(\d+)(?::.*)?\s*$/; export function formatFullAnchorRequirement(raw?: string): string { const received = raw === undefined ? "" : ` Received ${JSON.stringify(raw)}.`; return ( - `a bare line number from read/search output plus the section header file hash ` + - `(for example ${HL_FILE_PREFIX}src/foo.ts${HL_FILE_HASH_SEP}1a2b and line "160")${received}` + `a bare line number from read/search output plus the section header snapshot tag ` + + `(for example ${HL_FILE_PREFIX}src/foo.ts${HL_FILE_HASH_SEP}0A3 and line "160")${received}` ); } @@ -51,7 +51,7 @@ function getMismatchDisplayLines(anchorLines: readonly number[], fileLines: stri } /** - * Raised when a hashline section's file hash doesn't match the live file's + * Raised when a hashline section's snapshot tag doesn't match the live file's * content (and recovery, if configured, declined the merge). Carries the * file lines plus anchored lines so renderers can produce a richer * diagnostic via {@link MismatchError.displayMessage}. diff --git a/packages/hashline/src/parser.ts b/packages/hashline/src/parser.ts index 05ae164ea..b7fc3fbd6 100644 --- a/packages/hashline/src/parser.ts +++ b/packages/hashline/src/parser.ts @@ -15,10 +15,13 @@ */ import { HL_PAYLOAD_REPEAT, HL_PAYLOAD_REPLACE } from "./format"; import { - ABORT_WARNING, + BARE_BODY_AUTO_PIPED_WARNING, + DASH_PAYLOAD_AUTO_DELETE_WARNING, INLINE_PAYLOAD_REJECTED_PREFIX, - REPEAT_SHORTHAND_REJECTED_MESSAGE, + PLUS_PREFIXED_REPEAT_WARNING, + REPLACE_PAIR_COALESCED_OVERLAP_WARNING, REPLACE_PAIR_COALESCED_WARNING, + STACKED_BLANK_REPLACE_WARNING, VIRTUAL_REPLACE_REJECTED_MESSAGE, } from "./messages"; import { type BlockTarget, cloneCursor, type ParsedRange, type Token, Tokenizer } from "./tokenizer"; @@ -30,6 +33,22 @@ function validateRangeOrder(range: ParsedRange, lineNum: number): void { } } +/** + * If `text` (the slice after a `+` literal sigil) trims to `^A-B` (or `^A`, + * accepted as `^A-A`), return the parsed range. Otherwise `null`. Used to + * silently reroute `+^A-B` rows as repeats — models reflexively prefix every + * body row with `+`, including ones that should be repeats. + */ +function tryParseLiteralAsRepeat(text: string): ParsedRange | null { + const stripped = text.trim(); + if (stripped.length === 0 || stripped.charCodeAt(0) !== 94 /* ^ */) return null; + const match = /^\^([1-9]\d*)(?:-([1-9]\d*))?$/.exec(stripped); + if (match === null) return null; + const start = Number.parseInt(match[1], 10); + const end = match[2] !== undefined ? Number.parseInt(match[2], 10) : start; + return { start: { line: start }, end: { line: end } }; +} + function rangesEqual(a: ParsedRange, b: ParsedRange): boolean { return a.start.line === b.start.line && a.end.line === b.end.line; } @@ -38,6 +57,66 @@ function targetsEqualConcreteRange(a: BlockTarget, b: BlockTarget): boolean { return a.kind === "range" && b.kind === "range" && rangesEqual(a.range, b.range); } +function rangesOverlap(a: ParsedRange, b: ParsedRange): boolean { + return a.start.line <= b.end.line && b.start.line <= a.end.line; +} + +function rangesOverlapBetweenTargets(a: BlockTarget, b: BlockTarget): boolean { + return a.kind === "range" && b.kind === "range" && rangesOverlap(a.range, b.range); +} + +/** + * Detect OpenAI-`apply_patch` / unified-diff contamination in a raw line. + * Returns the error message to throw, or `null` when the line is clean. + * + * We only catch shapes that are unambiguously NOT hashline: + * - `*** Update File:` / `*** Add File:` / `*** Delete File:` / `*** Move to:` sentinels + * - unified-diff hunk headers (`@@`, `@@ -1,3 +1,3 @@`) + * - apply_patch hunk-anchor prefixes `-N:` / `-N-M:` — the bare `-N` form + * (no `:` and no `-M`) is intentionally NOT matched so the existing strict + * "unrecognized hashline block" diagnostic still fires on the legacy + * delete-row shape `-5`. + * + * `+`-prefixed shapes are NOT detected here because `+` is hashline's + * literal payload sigil; `+TEXT` / `+N:` are valid payload rows (or, at + * top level, orphan payloads that fall through to the standard "no + * preceding A-B:" error). + */ +function detectApplyPatchContamination(text: string, _hasPending: boolean): string | null { + const trimmed = text.trimStart(); + if (trimmed.length === 0) return null; + + if ( + trimmed.startsWith("*** Update File:") || + trimmed.startsWith("*** Add File:") || + trimmed.startsWith("*** Delete File:") || + trimmed.startsWith("*** Move to:") + ) { + const preview = trimmed.length > 48 ? `${trimmed.slice(0, 48)}…` : trimmed; + return ( + `apply_patch sentinel ${JSON.stringify(preview)} is not valid in hashline. ` + + `Use \`${"\u00b6"}PATH#HASH\` then \`A-B:\` / \`A-B:-\` / \`BOF:\` / \`EOF:\` blocks; do not wrap edits in another format's envelope.` + ); + } + if (trimmed === "@@" || trimmed.startsWith("@@ ") || trimmed.startsWith("@@\t")) { + return ( + "unified-diff hunk header (`@@`) is not valid in hashline. " + + "Use a `¶PATH#HASH` header and bare `A-B:` anchor blocks." + ); + } + if (/^-\d+(-\d+)?:/.test(trimmed)) { + return ( + "apply_patch line prefix (`-N:` / `-N-M:`) is not valid in hashline. " + + "Drop the `-` prefix; use `A-B:` (replace) or `A-B:-` (delete) on the anchor line itself." + ); + } + return null; +} + +function pendingHasAnyContent(pending: Pending): boolean { + return pending.payloads.length > 0 || pending.pendingRaws.length > 0; +} + function expandRange(range: ParsedRange): Anchor[] { const anchors: Anchor[] = []; for (let line = range.start.line; line <= range.end.line; line++) { @@ -70,6 +149,13 @@ interface Pending { target: BlockTarget; lineNum: number; payloads: PayloadRow[]; + /** + * Bare body rows (no `|`/`^` prefix) buffered while we wait to see + * whether the entire block is uniformly unprefixed. On flush, if every + * row was bare AND no `|`/`^` row was ever observed for this block, we + * auto-pipe the buffered rows and emit a {@link BARE_BODY_AUTO_PIPED_WARNING}. + */ + pendingRaws: { text: string; lineNum: number }[]; } /** @@ -88,6 +174,13 @@ export class Executor { #pending: Pending | undefined; #terminated = false; #skippableComments: PendingComment[] = []; + /** + * Length of the current run of consecutive single-line empty-body + * replacements (`A-A:` with no payload). Reset on every non-matching + * flush; surfaces {@link STACKED_BLANK_REPLACE_WARNING} when the run + * reaches two. + */ + #blankSingleRun = 0; #discardPendingSkippableComments(): void { this.#skippableComments = []; @@ -122,7 +215,6 @@ export class Executor { this.#terminated = true; return; case "abort": - this.#warnings.push(ABORT_WARNING); this.#terminated = true; return; case "header": @@ -140,9 +232,6 @@ export class Executor { this.#consumePendingSkippableComments(); this.#handleRepeatPayload(token.range, token.lineNum); return; - case "payload-repeat-shorthand": - this.#consumePendingSkippableComments(); - throw new Error(`line ${token.lineNum}: ${REPEAT_SHORTHAND_REJECTED_MESSAGE}`); case "raw": if (this.#pending === undefined && isSkippableCommentLine(token.text)) { this.#skippableComments.push({ text: token.text, lineNum: token.lineNum }); @@ -158,29 +247,59 @@ export class Executor { throw new Error(`line ${token.lineNum}: ${VIRTUAL_REPLACE_REJECTED_MESSAGE}`); } validateRangeOrder(token.target.range, token.lineNum); - this.#flushPending(); + // L5 (delete-suffix variant): if pending is a bare anchor that + // overlaps the new delete range, drop it silently — the model + // expressed `A-B:` then `A-B:-` (the classic before-then-after + // shape) and the actual intent is "delete A-B". + if ( + this.#pending !== undefined && + !pendingHasAnyContent(this.#pending) && + rangesOverlapBetweenTargets(this.#pending.target, token.target) + ) { + this.#pending = undefined; + this.#blankSingleRun = 0; + if (!this.#warnings.includes(REPLACE_PAIR_COALESCED_OVERLAP_WARNING)) { + this.#warnings.push(REPLACE_PAIR_COALESCED_OVERLAP_WARNING); + } + } else { + this.#flushPending(); + } for (const anchor of expandRange(token.target.range)) { this.#pushDelete(anchor, token.lineNum); } + this.#blankSingleRun = 0; return; } if (token.inlineBody !== undefined) { throw new Error( `line ${token.lineNum}: ${INLINE_PAYLOAD_REJECTED_PREFIX} ` + - `Use a bare anchor line such as ${describeTarget(token.target)}, then put body rows below it prefixed with ` + - `${HL_PAYLOAD_REPLACE} or ${HL_PAYLOAD_REPEAT}.`, + `Write the anchor on its own line (e.g. ${describeTarget(token.target)}), then put the body content on the next line prefixed with ` + + `${HL_PAYLOAD_REPLACE} (literal) or ${HL_PAYLOAD_REPEAT}A-B (repeat). If you pasted "${describeTarget(token.target).slice(0, -1)}CONTENT" from \`read\` output, strip the leading "${describeTarget(token.target).slice(0, -1)}" and prefix the rest with ${HL_PAYLOAD_REPLACE}.`, ); } if (token.target.kind === "range") validateRangeOrder(token.target.range, token.lineNum); if (this.#pending !== undefined && targetsEqualConcreteRange(this.#pending.target, token.target)) { + // Identical-range coalesce: drop the first block. Last-wins. this.#pending = undefined; if (!this.#warnings.includes(REPLACE_PAIR_COALESCED_WARNING)) { this.#warnings.push(REPLACE_PAIR_COALESCED_WARNING); } + } else if ( + this.#pending !== undefined && + !pendingHasAnyContent(this.#pending) && + rangesOverlapBetweenTargets(this.#pending.target, token.target) + ) { + // L5 (replace variant): bare pending block overlaps the new + // concrete block; treat as before/after pair, drop the bare + // one. The new block becomes pending. + this.#pending = undefined; + if (!this.#warnings.includes(REPLACE_PAIR_COALESCED_OVERLAP_WARNING)) { + this.#warnings.push(REPLACE_PAIR_COALESCED_OVERLAP_WARNING); + } } else { this.#flushPending(); } - this.#pending = { target: token.target, lineNum: token.lineNum, payloads: [] }; + this.#pending = { target: token.target, lineNum: token.lineNum, payloads: [], pendingRaws: [] }; return; } } @@ -209,7 +328,7 @@ export class Executor { */ endStreaming(): { edits: Edit[]; warnings: string[] } { this.#consumePendingSkippableComments(); - if (this.#pending && this.#pending.payloads.length > 0) { + if (this.#pending && pendingHasAnyContent(this.#pending)) { this.#flushPending(); } else { this.#pending = undefined; @@ -226,6 +345,7 @@ export class Executor { this.#pending = undefined; this.#skippableComments = []; this.#terminated = false; + this.#blankSingleRun = 0; } /** @@ -263,6 +383,22 @@ export class Executor { `Got ${JSON.stringify(`${HL_PAYLOAD_REPLACE}${text}`)}.`, ); } + // Silent recovery: a body row of `+^A-B` (or `+^A` after L2 shorthand) + // is a repeat row the model mistakenly prefixed with `+`. Reroute as + // a repeat and surface a warning so the model sees the mistake. + const repeatRange = tryParseLiteralAsRepeat(text); + if (repeatRange !== null) { + if (!this.#warnings.includes(PLUS_PREFIXED_REPEAT_WARNING)) { + this.#warnings.push(PLUS_PREFIXED_REPEAT_WARNING); + } + this.#handleRepeatPayload(repeatRange, lineNum); + return; + } + // L3: a `+literal` row after buffered bare raws means the block is + // NOT uniformly unprefixed — the bare rows were typos. Reject at the + // FIRST bare row's source line so the message points the model at + // what to fix. + this.#rejectBufferedRawsOnMixedBlock(pending); pending.payloads.push({ kind: "literal", text, lineNum }); } @@ -274,17 +410,66 @@ export class Executor { `Got ${JSON.stringify(`${HL_PAYLOAD_REPEAT}${range.start.line}-${range.end.line}`)}.`, ); } + // L3: same mixed-block guard as the literal path — see above. + this.#rejectBufferedRawsOnMixedBlock(pending); validateRangeOrder(range, lineNum); pending.payloads.push({ kind: "repeat", range, lineNum }); } + #rejectBufferedRawsOnMixedBlock(pending: Pending): void { + if (pending.pendingRaws.length === 0) return; + const first = pending.pendingRaws[0]; + throw new Error( + `line ${first.lineNum}: payload row in a hashline block must start with ` + + `${HL_PAYLOAD_REPLACE} or ${HL_PAYLOAD_REPEAT}A-B. Got ${JSON.stringify(first.text)}.`, + ); + } + #handleRaw(text: string, lineNum: number): void { + // L8: detect OpenAI-apply_patch / unified-diff contamination first so + // the error message tells the model what format they shipped instead + // of the generic "payload row must start with …" diagnostic. + const contamination = detectApplyPatchContamination(text, this.#pending !== undefined); + if (contamination !== null) throw new Error(`line ${lineNum}: ${contamination}`); + if (this.#pending) { if (text.trim().length === 0) return; - throw new Error( - `line ${lineNum}: payload row in a hashline block must start with ` + - `${HL_PAYLOAD_REPLACE} or ${HL_PAYLOAD_REPEAT}A-B. Got ${JSON.stringify(text)}.`, - ); + + // L4: a lone `-` row inside a bare pending block is the classic + // "I meant `A-B:-` but typed it on the next line" shape. Convert + // retroactively, emit a warning, and clear pending. + if ( + text.trim() === "-" && + this.#pending.payloads.length === 0 && + this.#pending.pendingRaws.length === 0 && + this.#pending.target.kind === "range" + ) { + const pendingRange = this.#pending.target.range; + const sourceLine = this.#pending.lineNum; + validateRangeOrder(pendingRange, sourceLine); + for (const anchor of expandRange(pendingRange)) { + this.#pushDelete(anchor, sourceLine); + } + if (!this.#warnings.includes(DASH_PAYLOAD_AUTO_DELETE_WARNING)) { + this.#warnings.push(DASH_PAYLOAD_AUTO_DELETE_WARNING); + } + this.#pending = undefined; + this.#blankSingleRun = 0; + return; + } + + // L3: buffer the bare row. Mixing this with later `|`/`^` rows + // throws via `#rejectBufferedRawsOnMixedBlock`; reject IMMEDIATELY + // when the block already has `|`/`^` rows so the error points at + // the offending bare row, not the (innocent) first `|` row. + if (this.#pending.payloads.length > 0) { + throw new Error( + `line ${lineNum}: payload row in a hashline block must start with ` + + `${HL_PAYLOAD_REPLACE} or ${HL_PAYLOAD_REPEAT}A-B. Got ${JSON.stringify(text)}.`, + ); + } + this.#pending.pendingRaws.push({ text, lineNum }); + return; } // Whitespace-only raw lines outside any pending block are silently dropped; @@ -293,6 +478,11 @@ export class Executor { const firstChar = text[0]; if (firstChar === "-" || firstChar === "@" || firstChar === "«" || firstChar === "»") { + if (text.trim() === "-") { + throw new Error( + `line ${lineNum}: a lone "-" is not a valid hashline op. To delete a range, write \`A-B:-\` on the anchor line itself (e.g. \`5-7:-\`).`, + ); + } throw new Error( `line ${lineNum}: unrecognized hashline block. Use A-B:, A-B:-, BOF:, or EOF: anchors followed by ` + `${HL_PAYLOAD_REPLACE}TEXT or ${HL_PAYLOAD_REPEAT}A-B body rows. Got ${JSON.stringify(text)}.`, @@ -342,6 +532,20 @@ export class Executor { const pending = this.#pending; if (!pending) return; + // L3: convert any buffered bare body rows to literal payloads. Mixed + // blocks have already been rejected; we only get here when payloads + // is empty AND pendingRaws holds rows, or when both are empty. + const hadBareBody = pending.pendingRaws.length > 0; + if (hadBareBody) { + for (const raw of pending.pendingRaws) { + pending.payloads.push({ kind: "literal", text: raw.text, lineNum: raw.lineNum }); + } + pending.pendingRaws = []; + if (!this.#warnings.includes(BARE_BODY_AUTO_PIPED_WARNING)) { + this.#warnings.push(BARE_BODY_AUTO_PIPED_WARNING); + } + } + const { target, lineNum, payloads } = pending; if (target.kind === "bof" || target.kind === "eof") { const cursor: Cursor = target.kind === "bof" ? { kind: "bof" } : { kind: "eof" }; @@ -353,9 +557,16 @@ export class Executor { } } this.#pending = undefined; + this.#blankSingleRun = 0; return; } + // L7 was considered (`^A-B` covering target + literal payload) but + // dropped: the same shape is the canonical "keep line A unchanged, + // insert new content above/below" idiom (e.g. `2-2:\n^2-2\n|NEW`). + // We can't distinguish duplication from intentional pass-through + // from the parse tree alone. + const cursor: Cursor = { kind: "before_anchor", anchor: { ...target.range.start } }; if (payloads.length === 0) { this.#pushInsert(cursor, "", lineNum, "replacement"); @@ -368,6 +579,20 @@ export class Executor { this.#pushDelete(anchor, lineNum); } + // L6: track contiguous runs of single-line blank-body replaces. A + // run of two or more is almost always the model mis-using `A-A:` to + // mean "delete this line" (it actually replaces with one blank line). + const isBlankSingleReplace = + target.range.start.line === target.range.end.line && payloads.length === 0 && !hadBareBody; + if (isBlankSingleReplace) { + this.#blankSingleRun++; + if (this.#blankSingleRun >= 2 && !this.#warnings.includes(STACKED_BLANK_REPLACE_WARNING)) { + this.#warnings.push(STACKED_BLANK_REPLACE_WARNING); + } + } else { + this.#blankSingleRun = 0; + } + this.#pending = undefined; } } diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index 308d31475..a9ebc21f3 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -1,7 +1,7 @@ /** * High-level patch orchestrator. Reads each section's target file via the * configured {@link Filesystem}, strips BOM and normalizes line endings, - * validates the section file hash (with optional {@link Recovery}), applies + * validates the section snapshot tag (with optional {@link Recovery}), applies * the edits, and writes the result back through the same {@link Filesystem}. * * Two layers: @@ -11,7 +11,7 @@ * - {@link Patcher.prepare} / {@link Patcher.commit} — granular primitives * for callers that need per-section control (e.g. batched LSP flush, * custom interleaving). `prepare` performs all the read-side work, - * validates the section file hash (with recovery), and applies the + * validates the section snapshot tag (with recovery), and applies the * edits in memory. `commit` writes the prepared result and records a * fresh snapshot. * @@ -23,25 +23,21 @@ * filesystem configuration. */ import { applyEdits } from "./apply"; -import { computeFileHash, formatHashlineHeader, HL_FILE_HASH_SEP, HL_FILE_PREFIX } from "./format"; +import { formatHashlineHeader, HL_FILE_HASH_SEP, HL_FILE_PREFIX } from "./format"; import type { Filesystem, WriteResult } from "./fs"; import { isNotFound } from "./fs"; import type { Patch, PatchSection } from "./input"; import { MismatchError } from "./mismatch"; import { detectLineEnding, type LineEnding, normalizeToLF, restoreLineEndings, stripBom } from "./normalize"; import { Recovery, type RecoveryResult } from "./recovery"; -import type { SnapshotStore } from "./snapshots"; +import type { Snapshot, SnapshotStore } from "./snapshots"; import type { ApplyOptions, ApplyResult, Edit } from "./types"; export interface PatcherOptions { /** Storage backend used for all reads and writes. */ fs: Filesystem; - /** - * Optional snapshot store that enables stale-hash recovery. When set, a - * section with a stale hash tries a 3-way merge against a cached - * snapshot before the apply fails with {@link MismatchError}. - */ - snapshots?: SnapshotStore; + /** Snapshot store that minted and resolves hashline section tags. Required. */ + snapshots: SnapshotStore; /** * Optional default {@link ApplyOptions} forwarded to every section. * Per-call overrides win on a key-by-key basis. @@ -65,9 +61,9 @@ export interface PatchSectionResult { persisted: string; /** Final text that the {@link Filesystem} actually wrote (may differ if the FS transformed it). */ written: string; - /** 4-hex hash of `after`. Use to anchor follow-up edits. */ + /** 3-hex opaque snapshot tag for `after`. Use to anchor follow-up edits. */ fileHash: string; - /** Hashline section header (`¶path#hash`) of the post-edit content. */ + /** Hashline section header (`¶path#tag`) of the post-edit content. */ header: string; /** 1-indexed first changed line in `after`, or `undefined` for noops. */ firstChangedLine?: number; @@ -114,7 +110,7 @@ function hasAnchorScopedEdit(edits: readonly Edit[]): boolean { function assertSectionHashAllowed(sectionPath: string, fileHash: string | undefined, edits: readonly Edit[]): void { if (fileHash !== undefined || !hasAnchorScopedEdit(edits)) return; throw new Error( - `Missing hashline file hash for anchored edit to ${sectionPath}; use \`${HL_FILE_PREFIX}${sectionPath}${HL_FILE_HASH_SEP}hash\` from your latest read.`, + `Missing hashline snapshot tag for anchored edit to ${sectionPath}; use \`${HL_FILE_PREFIX}${sectionPath}${HL_FILE_HASH_SEP}tag\` from your latest read/search output.`, ); } @@ -148,22 +144,33 @@ function assertUniqueCanonicalPaths(prepared: readonly PreparedSection[]): void } } +function snapshotMatchesCurrent(snapshot: Snapshot, currentText: string, anchorLines: readonly number[]): boolean { + if (snapshot.fullText !== undefined) return snapshot.fullText === currentText; + for (const lineNumber of anchorLines) { + if (snapshot.get(lineNumber) === undefined) return false; + } + return snapshot.matchesLiveFile(currentText.split("\n")); +} + /** - * High-level patcher. Wires a {@link Filesystem} and an optional + * High-level patcher. Wires a {@link Filesystem} and a required * {@link SnapshotStore} together with the parsing + applying core. * * Construct once per FS configuration; reuse across patches. */ export class Patcher { readonly fs: Filesystem; - readonly snapshots: SnapshotStore | undefined; - readonly recovery: Recovery | undefined; + readonly snapshots: SnapshotStore; + readonly recovery: Recovery; readonly applyOptions: ApplyOptions; constructor(options: PatcherOptions) { + if (!options.snapshots) { + throw new Error("Hashline Patcher requires a SnapshotStore; section tags are opaque store pointers."); + } this.fs = options.fs; this.snapshots = options.snapshots; - this.recovery = options.snapshots ? new Recovery(options.snapshots) : undefined; + this.recovery = new Recovery(options.snapshots); this.applyOptions = options.applyOptions ?? {}; } @@ -215,13 +222,13 @@ export class Patcher { } /** - * Read a section's target file, parse the section, validate the file - * hash (with recovery), and apply the edits in memory. Returns a + * Read a section's target file, parse the section, validate the snapshot + * tag (with recovery), and apply the edits in memory. Returns a * {@link PreparedSection} which can be fed to {@link commit} to land * the result on the filesystem. * * Throws on parse error, missing-file-for-anchored-edit, or unrecovered - * hash mismatch ({@link MismatchError}). + * tag mismatch ({@link MismatchError}). */ async prepare(section: PatchSection, options: ApplyOptions = {}): Promise { const applyOptions: ApplyOptions = { ...this.applyOptions, ...options }; @@ -264,8 +271,8 @@ export class Patcher { /** * Commit a previously {@link prepare}d section to the filesystem. * Restores line endings and BOM, writes via the {@link Filesystem}, and - * records a fresh snapshot in the {@link SnapshotStore} (when - * configured) keyed by the filesystem-canonical path. + * records a fresh snapshot in the {@link SnapshotStore} keyed by the + * filesystem-canonical path. */ async commit(prepared: PreparedSection): Promise { const { section, normalized, bom, lineEnding, parseWarnings, exists, applyResult, canonicalPath } = prepared; @@ -273,7 +280,7 @@ export class Patcher { const warnings = mergeWarnings(parseWarnings, applyResult.warnings); if (after === normalized) { - const hash = computeFileHash(normalized); + const hash = this.#recordFullSnapshot(canonicalPath, normalized); return { path: section.path, canonicalPath, @@ -290,16 +297,9 @@ export class Patcher { const persisted = bom + restoreLineEndings(after, lineEnding); const write: WriteResult = await this.fs.writeText(section.path, persisted); - const fileHash = computeFileHash(after); + const fileHash = this.#recordFullSnapshot(canonicalPath, after); const op = exists ? "update" : "create"; - if (this.snapshots) { - this.snapshots.recordContiguous(canonicalPath, 1, after.split("\n"), { - fullText: after, - fileHash, - }); - } - return { path: section.path, canonicalPath, @@ -325,6 +325,10 @@ export class Patcher { } } + #recordFullSnapshot(canonicalPath: string, normalized: string): string { + return this.snapshots.recordContiguous(canonicalPath, 1, normalized.split("\n"), { fullText: normalized }); + } + #applyWithRecovery(args: { section: PatchSection; canonicalPath: string; @@ -337,18 +341,24 @@ export class Patcher { const expected = exists ? section.fileHash : undefined; if (expected === undefined) return applyEdits(normalized, [...edits], applyOptions); - const currentHash = computeFileHash(normalized); - if (currentHash === expected) return applyEdits(normalized, [...edits], applyOptions); + const snapshot = this.snapshots.byHash(canonicalPath, expected); + const anchorLines = section.collectAnchorLines(); + if (snapshot && snapshotMatchesCurrent(snapshot, normalized, anchorLines)) { + return applyEdits(normalized, [...edits], applyOptions); + } - const recovered = this.recovery?.tryRecover({ - path: canonicalPath, - currentText: normalized, - fileHash: expected, - edits, - options: applyOptions, - }); - if (recovered) return recoveryToApplyResult(recovered); + if (snapshot) { + const recovered = this.recovery.tryRecover({ + path: canonicalPath, + currentText: normalized, + fileHash: expected, + edits, + options: applyOptions, + }); + if (recovered) return recoveryToApplyResult(recovered); + } + const currentHash = this.#recordFullSnapshot(canonicalPath, normalized); throw new MismatchError({ path: section.path, expectedFileHash: expected, diff --git a/packages/hashline/src/prefixes.ts b/packages/hashline/src/prefixes.ts index 678c4625f..56515d55a 100644 --- a/packages/hashline/src/prefixes.ts +++ b/packages/hashline/src/prefixes.ts @@ -16,7 +16,7 @@ const HL_PREFIX_RE = /^\s*(?:>>>|>>)?\s*(?:[+*-]\s*)?\d+:/; const HL_PREFIX_PLUS_RE = /^\s*(?:>>>|>>)?\s*\+\s*\d+:/; -const HL_HEADER_RE = /^\s*¶\S+#[0-9a-f]{4}\s*$/; +const HL_HEADER_RE = /^\s*¶\S+#[0-9a-fA-F]{3}\s*$/; const DIFF_PLUS_RE = /^[+](?![+])/; const READ_TRUNCATION_NOTICE_RE = /^\[(?:Showing lines \d+-\d+ of \d+|\d+ more lines? in (?:file|\S+))\b.*\bUse :L?\d+/; diff --git a/packages/hashline/src/prompt.md b/packages/hashline/src/prompt.md index 3d72d57c5..e0c3116e5 100644 --- a/packages/hashline/src/prompt.md +++ b/packages/hashline/src/prompt.md @@ -1,49 +1,102 @@ -Your patch language is a compact, line-anchored edit format. +Your patch language selects ranges of file lines and rewrites them. The body rows below an anchor describe the new content of the selected range. - -Patch payload = one or more file sections: -``` -¶PATH#HASH -A-B: -|literal line -^A-B -A-B:- -BOF: -|literal at start -EOF: -|literal at end -``` -- `HASH` comes from latest `read`/`search`; missing? re-read. -- `A-B:` anchors original lines A..B; use `A-A:` for one line. -- `BOF:`/`EOF:` insert at file start/end. -- `A-B:-` deletes original lines A..B. -- Body rows are linear; output order = row order. -- `|TEXT` emits literal `TEXT`; bare `|` emits blank. -- `^A-B` repeats original lines A..B; one line = `^A-A`. - + +Every body row is **exactly one** of two kinds: - -- Concrete `A-B:` body replaces A..B. -- Concrete `A-B:` with no body replaces A..B with one blank line. -- Virtual `BOF:`/`EOF:` body inserts there. -- Virtual empty body inserts one blank line. -- Line numbers are frozen for the whole patch. - + +TEXT add a new literal line `TEXT` (verbatim, leading whitespace included) + ^A-B keep original lines A..B as-is - -# Replace line 1 with two lines. +`+` and `^` are siblings, not stackable. Never write `+^…`. A row starts with one of them, never both. + + + +This is the original file (the exact shape `read` returns): ``` -¶a.ts#1a2b +¶greet.ts#0A3 +1:export function greet(name: string): string { +2: return `Hello, ${name}!`; +3:} +``` + +To add a null check between the signature and the return, select lines 1..3 and rewrite: +``` +¶greet.ts#0A3 +1-3: +^1-1 ++ if (!name) return "Hello, stranger!"; +^2-3 +``` + +The body says: keep line 1, then add the new literal line, then keep lines 2..3. Result: +``` +1:export function greet(name: string): string { +2: if (!name) return "Hello, stranger!"; +3: return `Hello, ${name}!`; +4:} +``` + + + +``` +A-B: select lines A..B; the body rows below describe their new content +A-B:- delete lines A..B (no body) +BOF: virtual position before line 1; body rows insert there +EOF: virtual position after the last line; body rows insert there +``` +`A-A:` for one line is preferred over the bare shorthand `A:`. `BOF:` / `EOF:` take no range. + + +
+Every section starts with `¶PATH#HASH`. `HASH` is the snapshot tag from your latest `read`/`search` of that file. It is required whenever a block uses a line-number anchor (`A-B:` or `A-B:-`). Hashless `¶PATH` is only valid for new-file creation or BOF/EOF-only patches. +
+ + +- Anchors are line **numbers**, never line **content**. `read` shows each file row as `LINE:TEXT`; for a patch the anchor is `4-4:` and the body is `+TEXT` (or `^4-4` to keep it). +- Each range may appear in only ONE block per patch. +- Line numbers refer to the ORIGINAL file and stay valid for the whole patch — they do not shift as your blocks land. +- `A-B:` with no body replaces the range with ONE blank line. To **delete** the lines entirely, use `A-B:-`. +- If you want to replace lines A..B with completely new content, just list the new content; do not write `^A-B`. + + + +# Replace line 1 of `greet.ts#0A3` with two new lines. +``` +¶greet.ts#0A3 1-1: -|const X = "b"; -|export const Y = X; ++const X = "b"; ++export const Y = X; ``` -# Insert below line 5. + +# Delete lines 2..3 of `greet.ts#0A3`. ``` -¶a.ts#1a2b -5-5: -^5-5 -|const Y = X; +¶greet.ts#0A3 +2-3:- ``` -# Delete lines 5..7: `5-7:-`. - \ No newline at end of file + +# Prepend a header. +``` +¶greet.ts#0A3 +BOF: ++// generated header +``` + + + +# WRONG — two blocks expressing old → new. Rejected as overlap. +1-1: +1-1:- + +# WRONG — `A-A:` with no body REPLACES with a blank line. Use `A-A:-` to delete. +2-2: +2-2:- + +# WRONG — `read`-output rows pasted as body. Body rows need `+` or `^`. +2-3: + return `Hello, ${name}!`; +} + +# RIGHT — same intent, well-formed. +2-3: ++ return `Hello, ${name}!`; ++} + diff --git a/packages/hashline/src/recovery.ts b/packages/hashline/src/recovery.ts index e1239d346..87602ec0b 100644 --- a/packages/hashline/src/recovery.ts +++ b/packages/hashline/src/recovery.ts @@ -1,21 +1,20 @@ /** - * Recover from a stale section file-hash by replaying the would-be edit + * Recover from a stale section snapshot tag by replaying the would-be edit * against a cached pre-edit snapshot of the file and 3-way-merging the * result onto the current on-disk content. * - * The patcher consults this when it sees a section hash that doesn't match - * the live file content. The recovery class is stateless apart from the - * {@link SnapshotStore} it queries; the snapshot store is the seam that + * The patcher consults this when a section tag resolves to a snapshot that no + * longer matches the live file content. The recovery class is stateless apart + * from the {@link SnapshotStore} it queries; the snapshot store is the seam * lets you plug in your own caching strategy. */ import * as Diff from "diff"; import { applyEdits } from "./apply"; -import { computeFileHash } from "./format"; import { RECOVERY_EXTERNAL_WARNING, RECOVERY_SESSION_CHAIN_WARNING, RECOVERY_SESSION_REPLAY_WARNING } from "./messages"; import type { Snapshot, SnapshotStore } from "./snapshots"; import type { Anchor, ApplyOptions, ApplyResult, Edit } from "./types"; -// Section hashes are line-precise; never let Diff.applyPatch slide a hunk +// Section tags are line-precise; never let Diff.applyPatch slide a hunk // onto a duplicate closer 100+ lines away. If snapshot replay does not // align exactly, refuse and let the caller re-read. const RECOVERY_FUZZ_FACTOR = 0; @@ -139,19 +138,35 @@ function replaySessionChainOnCurrent( }; } -function buildSparseOverlayText(currentText: string, snapshotLines: ReadonlyMap): string { +function snapshotHasEntries(snapshot: Snapshot): boolean { + for (const _entry of snapshot.entries()) return true; + return false; +} + +function buildSparseOverlayText(currentText: string, snapshot: Snapshot): string { const overlaid = currentText.split("\n"); let maxCachedLine = 0; - for (const lineNum of snapshotLines.keys()) { + for (const [lineNum] of snapshot.entries()) { if (lineNum > maxCachedLine) maxCachedLine = lineNum; } while (overlaid.length < maxCachedLine) overlaid.push(""); - for (const [lineNum, content] of snapshotLines) { + for (const [lineNum, content] of snapshot.entries()) { overlaid[lineNum - 1] = content; } return overlaid.join("\n"); } +function sparseSnapshotCoversAnchors(snapshot: Snapshot, edits: readonly Edit[]): boolean { + for (const lineNumber of collectAnchorLines(edits)) { + if (snapshot.get(lineNumber) === undefined) return false; + } + return true; +} + +function sparseSnapshotMatchesCurrent(currentText: string, snapshot: Snapshot): boolean { + return snapshot.matchesLiveFile(currentText.split("\n")); +} + /** First 1-indexed line at which `a` and `b` diverge, or `undefined` if equal. */ function findFirstChangedLine(a: string, b: string): number | undefined { if (a === b) return undefined; @@ -181,8 +196,9 @@ function isHeadSnapshot(head: Snapshot | null, snapshot: Snapshot): boolean { * a dedicated {@link RECOVERY_SESSION_REPLAY_WARNING} because even with * both guards a coincidental insert+delete pair on duplicate rows can * still land the edit on the wrong row; see {@link replaySessionChainOnCurrent}. - * 3. Reconstruct from a sparse snapshot (lines map only), verify the rebuilt - * text hashes to the expected value, then 3-way-merge. + * 3. Reconstruct from a sparse snapshot (lines map only), then 3-way-merge. + * Sparse snapshots that still match the live file are direct-apply cases + * owned by the patcher, so recovery declines them. */ export class Recovery { constructor(readonly store: SnapshotStore) {} @@ -195,7 +211,7 @@ export class Recovery { const { path, currentText, fileHash, edits, options = {} } = args; const head = this.store.head(path); const snapshot = this.store.byHash(path, fileHash); - if (!snapshot || snapshot.lines.size === 0) return null; + if (!snapshot || !snapshotHasEntries(snapshot)) return null; const isHead = isHeadSnapshot(head, snapshot); const recoveryWarning = isHead ? RECOVERY_EXTERNAL_WARNING : RECOVERY_SESSION_CHAIN_WARNING; @@ -212,8 +228,9 @@ export class Recovery { return null; } - const overlayText = buildSparseOverlayText(currentText, snapshot.lines); - if (computeFileHash(overlayText) !== fileHash) return null; + if (!sparseSnapshotCoversAnchors(snapshot, edits)) return null; + if (sparseSnapshotMatchesCurrent(currentText, snapshot)) return null; + const overlayText = buildSparseOverlayText(currentText, snapshot); return applyEditsToSnapshot(overlayText, currentText, edits, options, recoveryWarning); } } diff --git a/packages/hashline/src/snapshots.ts b/packages/hashline/src/snapshots.ts index 213324f12..29ea92d44 100644 --- a/packages/hashline/src/snapshots.ts +++ b/packages/hashline/src/snapshots.ts @@ -1,54 +1,162 @@ /** - * Per-session snapshot store used by {@link Recovery} to rescue patches when - * a section's file hash has drifted (file changed externally or a prior - * in-session edit advanced the hash). + * Per-session snapshot store used by {@link Recovery} and {@link Patcher} to + * bind hashline section tags to the exact file view that minted them. * - * Producers (typically a `read` tool) record snapshots as they observe file - * content. Consumers (the patcher) query for the snapshot whose hash matches - * a stale section, then 3-way-merge the would-be edit onto the live content. + * Producers (typically `read` / `search` tools) record the lines they showed + * the model. The store returns a three-hex opaque tag. Consumers resolve that + * tag back to the recorded snapshot and verify the recorded lines against the + * live file before applying anchored edits. * - * The abstract base class lets callers plug in whatever storage they like - * (LRU, persistent SQLite, etc.). {@link InMemorySnapshotStore} ships as a - * sensible default backed by `lru-cache` so paths age out automatically. + * Tags are scrambled by a deterministic permutation of the 12-bit hex space + * built once at module load. Slot index `i` always maps to the same tag + * across restarts (good for tests and reproducibility), but consecutive slot + * indices produce unrelated tags — `mint()` followed by another `mint()` does + * not return e.g. `000` then `001`, and the first tag a store ever hands out + * is not `000`. This is hallucination prevention, not adversarial security: it + * stops an LLM from guessing `001` after observing `000`, or assuming any + * monotonic counter pattern. The patcher catches stale-tag misuse via + * content verification at apply time, so determinism doesn't reduce safety. + * + * Tag → slot resolution is a single `Map` lookup (no `parseInt`, no regex): + * the inverse table is populated with both lowercase and uppercase forms of + * every tag at module load. + * + * Snapshots are an open abstract type. The two concrete impls cover the + * shapes producers actually emit: {@link ContiguousSnapshot} for `read`-style + * runs (no map allocation, range arithmetic for lookup and superset checks) + * and {@link SparseSnapshot} for `search`-style hits. + * + * The abstract base class lets callers plug in whatever storage they like. + * {@link InMemorySnapshotStore} ships as a single 4096-slot ring shared across + * paths — snapshots carry their own path, so the global ring is fine and + * `byHash` rejects cross-path lookups. */ -import { LRUCache } from "lru-cache/raw"; /** - * One snapshot of a file as it was observed at a point in time. Either the - * full text is recorded (`fullText` set) for full-file reads, or a sparse - * map of `(lineNumber, content)` pairs for partial views (search matches, - * range reads). + * One snapshot of a file view as observed at a point in time. The two + * primitive methods subclasses must supply — `get` and `entries` — give callers + * (and the default `isSuperset` / `matchesLiveFile` impls) everything they + * need to verify recorded content against the live file. */ -export interface Snapshot { - /** 1-indexed line number → exact content as observed. */ - readonly lines: Map; - /** Full normalized text when the read observed the whole file. */ - fullText?: string; - /** 4-hex hash carried alongside the read, when known. */ - fileHash?: string; +export abstract class Snapshot { + /** Canonical path this snapshot belongs to. */ + abstract readonly path: string; /** Timestamp (ms since epoch) the snapshot was recorded. */ - recordedAt: number; + abstract readonly recordedAt: number; + /** Full normalized text when the read observed the whole file. */ + abstract readonly fullText?: string; + + /** Recorded content for 1-indexed `lineNumber`, or `undefined` if this snapshot doesn't cover that line. */ + abstract get(lineNumber: number): string | undefined; + + /** Iterate (1-indexed lineNumber, content) pairs in stable order. */ + abstract entries(): Iterable<[number, string]>; + + /** True iff every (line, content) the `other` snapshot asserts is also present here with matching content. */ + isSuperset(other: Snapshot): boolean { + for (const [lineNumber, content] of other.entries()) { + if (this.get(lineNumber) !== content) return false; + } + return true; + } + + /** True iff every recorded line matches `currentLines` (0-indexed array of live file lines). */ + matchesLiveFile(currentLines: readonly string[]): boolean { + for (const [lineNumber, content] of this.entries()) { + if (currentLines[lineNumber - 1] !== content) return false; + } + return true; + } +} + +/** + * A contiguous run of lines starting at `offset` (1-indexed). Backed by a + * plain `string[]`; lookup is `lines[n - offset]`, superset against another + * contiguous snapshot is pure range arithmetic. + */ +export class ContiguousSnapshot extends Snapshot { + constructor( + public readonly path: string, + public readonly offset: number, + public readonly lines: readonly string[], + public readonly fullText?: string, + public readonly recordedAt: number = Date.now(), + ) { + super(); + } + + get(lineNumber: number): string | undefined { + const index = lineNumber - this.offset; + if (index < 0 || index >= this.lines.length) return undefined; + return this.lines[index]; + } + + *entries(): IterableIterator<[number, string]> { + for (let index = 0; index < this.lines.length; index++) { + yield [this.offset + index, this.lines[index] ?? ""]; + } + } + + override isSuperset(other: Snapshot): boolean { + if (other instanceof ContiguousSnapshot) { + if (other.offset < this.offset) return false; + const skip = other.offset - this.offset; + if (skip + other.lines.length > this.lines.length) return false; + for (let index = 0; index < other.lines.length; index++) { + if (this.lines[skip + index] !== other.lines[index]) return false; + } + return true; + } + return super.isSuperset(other); + } + + override matchesLiveFile(currentLines: readonly string[]): boolean { + for (let index = 0; index < this.lines.length; index++) { + if (currentLines[this.offset + index - 1] !== this.lines[index]) return false; + } + return true; + } +} + +/** + * A sparse `(lineNumber → content)` map, used for snapshots that don't cover a + * single contiguous run — e.g. search hits plus their context windows. + */ +export class SparseSnapshot extends Snapshot { + constructor( + public readonly path: string, + public readonly lines: ReadonlyMap, + public readonly fullText?: string, + public readonly recordedAt: number = Date.now(), + ) { + super(); + } + + get(lineNumber: number): string | undefined { + return this.lines.get(lineNumber); + } + + entries(): IterableIterator<[number, string]> { + return this.lines.entries(); + } } /** Optional metadata supplied at snapshot record time. */ export interface SnapshotMetadata { /** Full normalized text, when the producer observed the whole file. */ fullText?: string; - /** 4-hex hash carried by the read, when known. */ - fileHash?: string; } /** - * Storage seam for file-content snapshots. The patcher calls {@link head} - * for the latest snapshot of a path and {@link byHash} when it needs the - * specific historical snapshot that matches a section's stale hash. + * Storage seam for file-content snapshots. Hashline section tags are opaque + * store pointers; without the store that minted them they carry no meaning. */ export abstract class SnapshotStore { - /** Most-recent snapshot for `path`, or `null` if none. */ + /** Most-recently pushed snapshot for `path`, or `null` if none. */ abstract head(path: string): Snapshot | null; - /** Most-recent snapshot for `path` whose `fileHash` equals `fileHash`. */ - abstract byHash(path: string, fileHash: string): Snapshot | null; + /** Snapshot currently occupying `tag`'s slot for `path`, or `null`. */ + abstract byHash(path: string, tag: string): Snapshot | null; /** Record a contiguous run of lines (e.g. from a `read` tool). `startLine` is 1-indexed. */ abstract recordContiguous( @@ -56,116 +164,147 @@ export abstract class SnapshotStore { startLine: number, lines: readonly string[], metadata?: SnapshotMetadata, - ): void; + ): string; /** Record sparse `(lineNumber, content)` pairs (e.g. a `search` match plus context). */ - abstract recordSparse(path: string, entries: Iterable, metadata?: SnapshotMetadata): void; + abstract recordSparse( + path: string, + entries: Iterable, + metadata?: SnapshotMetadata, + ): string; - /** Drop the snapshot history for a single path. */ + /** Drop snapshots belonging to a single path. */ abstract invalidate(path: string): void; - /** Drop every snapshot history. */ + /** Drop every snapshot. */ abstract clear(): void; } -const DEFAULT_MAX_PATHS = 30; -const DEFAULT_MAX_SNAPSHOTS_PER_PATH = 4; +const RING_SIZE = 0x1000; +const RING_MASK = RING_SIZE - 1; +const FALLBACK_TAG = "000"; +const HEX_DIGITS = "0123456789ABCDEF"; -function hasConflict( - existing: ReadonlyMap, - incoming: ReadonlyArray, -): boolean { - for (const [lineNum, content] of incoming) { - const prior = existing.get(lineNum); - if (prior !== undefined && prior !== content) return true; +/** + * Deterministic permutation of `[0..4095]` → 3-hex tag, plus its inverse. + * + * Built once at module load via Mulberry32 + Fisher–Yates with a fixed seed. + * Verified properties (see test suite): bijection over the 4096 hex values, + * `FORWARD[0] !== "000"`, no recoverable arithmetic pattern in consecutive + * tags. Cryptographic strength is not required — this only has to defeat + * trivial LLM extrapolation like "after `000` comes `001`". + */ +const { FORWARD, INVERSE } = buildHexTables(); + +function formatSlotTag(value: number): string { + return ( + (HEX_DIGITS[(value >>> 8) & 0xf] ?? "0") + + (HEX_DIGITS[(value >>> 4) & 0xf] ?? "0") + + (HEX_DIGITS[value & 0xf] ?? "0") + ); +} + +function buildHexTables(): { FORWARD: readonly string[]; INVERSE: ReadonlyMap } { + let state = 0x9e3779b9 | 0; + const rng = (): number => { + state = (state + 0x6d2b79f5) | 0; + let t = state; + t = Math.imul(t ^ (t >>> 15), t | 1); + t ^= t + Math.imul(t ^ (t >>> 7), t | 61); + return ((t ^ (t >>> 14)) >>> 0) / 0x100000000; + }; + + const order = Array.from({ length: RING_SIZE }, (_, index) => index); + for (let index = RING_SIZE - 1; index > 0; index--) { + const swapIndex = Math.floor(rng() * (index + 1)); + const tmp = order[index] ?? 0; + order[index] = order[swapIndex] ?? 0; + order[swapIndex] = tmp; } - return false; -} -function hasHashConflict(existing: Snapshot, metadata: SnapshotMetadata): boolean { - return metadata.fileHash !== undefined && existing.fileHash !== undefined && metadata.fileHash !== existing.fileHash; -} - -function isSameSnapshotIdentity(left: Snapshot, right: Snapshot): boolean { - if (left.fileHash !== undefined && right.fileHash !== undefined) return left.fileHash === right.fileHash; - if (left.fullText !== undefined && right.fullText !== undefined) return left.fullText === right.fullText; - return false; -} - -export interface InMemorySnapshotStoreOptions { - /** Maximum number of distinct paths tracked at once (default 30). LRU eviction. */ - maxPaths?: number; - /** Maximum snapshots retained per path (default 4). Oldest dropped first. */ - maxSnapshotsPerPath?: number; + const forward = order.map(formatSlotTag); + const inverse = new Map(); + for (let slot = 0; slot < RING_SIZE; slot++) { + const tag = forward[slot] ?? FALLBACK_TAG; + inverse.set(tag, slot); + inverse.set(tag.toLowerCase(), slot); + } + return { FORWARD: forward, INVERSE: inverse }; } /** - * In-memory {@link SnapshotStore} backed by `lru-cache`. Per-path snapshot - * history is a short ring (oldest dropped first); per-session path tracking - * is LRU-bounded so cold paths age out automatically. - * - * Newer snapshots merge into the head when their entries don't conflict and - * the recorded `fileHash` (if any) still agrees; otherwise a fresh snapshot - * is pushed onto the front of the history list. + * In-memory {@link SnapshotStore} backed by a flat 4096-slot ring shared across + * all paths. Slot allocation is a simple `counter & 0xfff`; the tag the model + * sees is `FORWARD[slot]` from the module-level permutation, so consecutive + * pushes hand out unrelated tags. Slot reuse on wrap is intentional: stale tags + * may alias after 4096 distinct pushes, and the patcher catches misuse by + * verifying the resolved snapshot's content (and path) against the live file + * before applying edits. */ export class InMemorySnapshotStore extends SnapshotStore { - readonly #snapshots: LRUCache; - readonly #maxSnapshotsPerPath: number; - - constructor(options: InMemorySnapshotStoreOptions = {}) { - super(); - this.#snapshots = new LRUCache({ max: options.maxPaths ?? DEFAULT_MAX_PATHS }); - this.#maxSnapshotsPerPath = options.maxSnapshotsPerPath ?? DEFAULT_MAX_SNAPSHOTS_PER_PATH; - } + readonly #slots: Array = new Array(RING_SIZE).fill(null); + #nextCounter = 0; + #filled = 0; head(path: string): Snapshot | null { - return this.#snapshots.get(path)?.[0] ?? null; + for (let offset = 1; offset <= this.#filled; offset++) { + const snapshot = this.#slots[(this.#nextCounter - offset) & RING_MASK]; + if (snapshot && snapshot.path === path) return snapshot; + } + return null; } - byHash(path: string, fileHash: string): Snapshot | null { - const history = this.#snapshots.get(path); - return history?.find(entry => entry.fileHash === fileHash) ?? null; + byHash(path: string, tag: string): Snapshot | null { + const slot = INVERSE.get(tag); + if (slot === undefined) return null; + const snapshot = this.#slots[slot]; + if (!snapshot || snapshot.path !== path) return null; + return snapshot; } - recordContiguous(path: string, startLine: number, lines: readonly string[], metadata: SnapshotMetadata = {}): void { - if (lines.length === 0 && metadata.fullText === undefined) return; - const entries: Array = lines.map((line, idx) => [startLine + idx, line] as const); - this.#record(path, entries, metadata); + recordContiguous( + path: string, + startLine: number, + lines: readonly string[], + metadata: SnapshotMetadata = {}, + ): string { + return this.#record(new ContiguousSnapshot(path, startLine, lines, metadata.fullText)); } - recordSparse(path: string, entries: Iterable, metadata: SnapshotMetadata = {}): void { - const arr = Array.from(entries); - if (arr.length === 0 && metadata.fullText === undefined) return; - this.#record(path, arr, metadata); + recordSparse(path: string, entries: Iterable, metadata: SnapshotMetadata = {}): string { + const lines = new Map(); + for (const [lineNumber, content] of entries) lines.set(lineNumber, content); + return this.#record(new SparseSnapshot(path, lines, metadata.fullText)); } invalidate(path: string): void { - this.#snapshots.delete(path); + for (let index = 0; index < RING_SIZE; index++) { + if (this.#slots[index]?.path === path) this.#slots[index] = null; + } } clear(): void { - this.#snapshots.clear(); + this.#slots.fill(null); } - #record(path: string, entries: ReadonlyArray, metadata: SnapshotMetadata): void { - const history = this.#snapshots.get(path) ?? []; - const head = history[0]; - const now = Date.now(); - if (head && !hasConflict(head.lines, entries) && !hasHashConflict(head, metadata)) { - for (const [lineNum, content] of entries) head.lines.set(lineNum, content); - if (metadata.fullText !== undefined) head.fullText = metadata.fullText; - if (metadata.fileHash !== undefined) head.fileHash = metadata.fileHash; - head.recordedAt = now; - // `get` above already touched LRU recency for this key. - return; - } + #record(incoming: Snapshot): string { + const dedup = this.#dedup(incoming); + if (dedup !== null) return dedup; - const nextSnapshot: Snapshot = { - lines: new Map(entries), - ...metadata, - recordedAt: now, - }; - const deduped = history.filter(entry => !isSameSnapshotIdentity(entry, nextSnapshot)); - this.#snapshots.set(path, [nextSnapshot, ...deduped].slice(0, this.#maxSnapshotsPerPath)); + const slot = this.#nextCounter & RING_MASK; + this.#slots[slot] = incoming; + this.#nextCounter++; + if (this.#filled < RING_SIZE) this.#filled++; + return FORWARD[slot] ?? FALLBACK_TAG; + } + + #dedup(incoming: Snapshot): string | null { + for (let offset = 1; offset <= this.#filled; offset++) { + const slot = (this.#nextCounter - offset) & RING_MASK; + const existing = this.#slots[slot]; + if (!existing || existing.path !== incoming.path) continue; + if (existing.isSuperset(incoming)) return FORWARD[slot] ?? FALLBACK_TAG; + } + return null; } } diff --git a/packages/hashline/src/tokenizer.ts b/packages/hashline/src/tokenizer.ts index 1d9c02ee4..702ca1167 100644 --- a/packages/hashline/src/tokenizer.ts +++ b/packages/hashline/src/tokenizer.ts @@ -15,6 +15,7 @@ import { describeAnchorExamples, + HL_FILE_HASH_LENGTH, HL_FILE_HASH_SEP, HL_FILE_PREFIX, HL_OP_DELETE_SUFFIX, @@ -34,13 +35,14 @@ const CHAR_TAB = 9; const CHAR_SPACE = 32; const CHAR_HYPHEN = 45; +const CHAR_UPPER_A = 65; +const CHAR_UPPER_F = 70; const CHAR_LOWER_A = 97; const CHAR_LOWER_F = 102; const CHAR_PILCROW = HL_FILE_PREFIX.charCodeAt(0); const CHAR_OP_REPLACE = HL_OP_REPLACE.charCodeAt(0); const CHAR_PAYLOAD_REPLACE = HL_PAYLOAD_REPLACE.charCodeAt(0); const CHAR_PAYLOAD_REPEAT = HL_PAYLOAD_REPEAT.charCodeAt(0); -const FILE_HASH_LENGTH = 4; function isDigitCode(code: number): boolean { return code >= CHAR_ZERO && code <= CHAR_NINE; @@ -51,11 +53,19 @@ function isNonZeroDigitCode(code: number): boolean { } function isDecorationCode(code: number): boolean { - return code === 42 || code === CHAR_HYPHEN || code === 62; + // `*` (grep match marker) and `>` (grep context marker). We intentionally + // do NOT include `-` here: a leading `-` is the unified-diff "removed + // line" prefix and the OpenAI apply_patch hunk-line prefix, both of + // which we want to reject loudly rather than silently swallow. + return code === 42 || code === 62; } function isHexDigitCode(code: number): boolean { - return isDigitCode(code) || (code >= CHAR_LOWER_A && code <= CHAR_LOWER_F); + return ( + isDigitCode(code) || + (code >= CHAR_UPPER_A && code <= CHAR_UPPER_F) || + (code >= CHAR_LOWER_A && code <= CHAR_LOWER_F) + ); } function skipWhitespace(line: string, index: number, end = line.length): number { @@ -109,7 +119,7 @@ export function cloneCursor(cursor: Cursor): Cursor { return cursor; } // Leniently accept anchors copied from read/search output: -// - optional leading line-marker decoration (`*`, `>`, `-`) +// - optional leading line-marker decoration (`*`, `>`) // - the required bare line number / BOF / EOF anchor function skipDecoratedAnchorPrefix(line: string, end = trimEndIndex(line)): number { let index = skipWhitespace(line, 0, end); @@ -160,15 +170,27 @@ function scanRange(line: string, end = trimEndIndex(line)): RangeScan | null { const start = scanLineNumber(line, numberStart, end); if (start === null) return null; - // Ranges MUST be written `A-B` (the explicit single-line form `A-A` is fine). - if (start.nextIndex >= end || line.charCodeAt(start.nextIndex) !== CHAR_HYPHEN) return null; - const endNumber = scanLineNumber(line, start.nextIndex + 1, end); - if (endNumber === null) return null; - - return { - range: { start: { line: start.line }, end: { line: endNumber.line } }, - nextIndex: skipWhitespace(line, endNumber.nextIndex, end), - }; + // Canonical form is `A-B` (and `A-A` for one line). The bare single-line + // shorthand `A` is also accepted because models that learned the format + // from `read` output — which renders each file row as `LINE:content` — + // frequently reproduce that shape as an anchor. We treat `A` as `A-A` + // here so `tryParseBlockOp` can still raise its strict inline-payload + // diagnostic on `A:content` lines. + if (start.nextIndex < end && line.charCodeAt(start.nextIndex) === CHAR_HYPHEN) { + const endNumber = scanLineNumber(line, start.nextIndex + 1, end); + if (endNumber === null) return null; + return { + range: { start: { line: start.line }, end: { line: endNumber.line } }, + nextIndex: skipWhitespace(line, endNumber.nextIndex, end), + }; + } + if (start.nextIndex < end && line.charCodeAt(start.nextIndex) === CHAR_OP_REPLACE) { + return { + range: { start: { line: start.line }, end: { line: start.line } }, + nextIndex: start.nextIndex, + }; + } + return null; } function startsWithWord(line: string, index: number, end: number, word: string): boolean { @@ -189,11 +211,11 @@ interface TargetScan { function scanBlockTarget(line: string, end = trimEndIndex(line)): TargetScan | null { const targetStart = skipDecoratedAnchorPrefix(line, end); if (startsWithWord(line, targetStart, end, "BOF")) { - const nextIndex = skipWhitespace(line, targetStart + 3, end); + const nextIndex = skipBofEofRangeSuffix(line, targetStart + 3, end); return { target: { kind: "bof" }, nextIndex }; } if (startsWithWord(line, targetStart, end, "EOF")) { - const nextIndex = skipWhitespace(line, targetStart + 3, end); + const nextIndex = skipBofEofRangeSuffix(line, targetStart + 3, end); return { target: { kind: "eof" }, nextIndex }; } @@ -201,6 +223,19 @@ function scanBlockTarget(line: string, end = trimEndIndex(line)): TargetScan | n return range === null ? null : { target: { kind: "range", range: range.range }, nextIndex: range.nextIndex }; } +// Models sometimes write `BOF-BOF:`, `EOF-EOF:`, or even `BOF-EOF:` by analogy +// with the numeric `A-B:` form. The range portion carries no information for +// virtual anchors (they do not span lines), so we just consume and discard it. +function skipBofEofRangeSuffix(line: string, index: number, end: number): number { + const cursor = skipWhitespace(line, index, end); + if (cursor >= end || line.charCodeAt(cursor) !== CHAR_HYPHEN) return cursor; + const afterHyphen = skipWhitespace(line, cursor + 1, end); + if (startsWithWord(line, afterHyphen, end, "BOF") || startsWithWord(line, afterHyphen, end, "EOF")) { + return skipWhitespace(line, afterHyphen + 3, end); + } + return cursor; +} + interface ParsedBlockOp { target: BlockTarget; inlineBody: string | undefined; @@ -231,13 +266,18 @@ function tryParseBlockOp(line: string): ParsedBlockOp | null { }; } -function tryParseRepeatPayload(line: string): ParsedRange | "shorthand" | null { +function tryParseRepeatPayload(line: string): ParsedRange | null { const end = trimEndIndex(line); if (line.length === 0 || line.charCodeAt(0) !== CHAR_PAYLOAD_REPEAT) return null; const start = scanLineNumber(line, 1, end); if (start === null) return null; - if (start.nextIndex === end) return "shorthand"; + // Canonical form is `^A-B`; the explicit `^A-A` is preferred. The bare + // single-line `^A` is also accepted as `^A-A` because the strict form + // adds friction without disambiguating anything. + if (start.nextIndex === end) { + return { start: { line: start.line }, end: { line: start.line } }; + } if (start.nextIndex >= end || line.charCodeAt(start.nextIndex) !== CHAR_HYPHEN) return null; const finish = scanLineNumber(line, start.nextIndex + 1, end); @@ -248,7 +288,7 @@ function tryParseRepeatPayload(line: string): ParsedRange | "shorthand" | null { /** * Strict header scan: `¶+` prefix, optional whitespace, path body that - * excludes whitespace, `#`, and `¶`, optional `#[0-9a-f]{4}` hash suffix, + * excludes whitespace, `#`, and `¶`, optional three-hex hash suffix, * optional trailing whitespace. Returns `null` when any byte deviates from * the shape. */ @@ -273,12 +313,12 @@ function tryParseHeader(line: string): { path: string; fileHash?: string } | nul let fileHash: string | undefined; if (index < end && line.charCodeAt(index) === CHAR_HASH) { const hashStart = index + 1; - const hashEnd = hashStart + FILE_HASH_LENGTH; + const hashEnd = hashStart + HL_FILE_HASH_LENGTH; if (hashEnd > end) return null; for (let probe = hashStart; probe < hashEnd; probe++) { if (!isHexDigitCode(line.charCodeAt(probe))) return null; } - fileHash = line.slice(hashStart, hashEnd); + fileHash = line.slice(hashStart, hashEnd).toUpperCase(); index = hashEnd; } @@ -302,7 +342,6 @@ export type Token = | (TokenBase & { kind: "op-block"; target: BlockTarget; inlineBody: string | undefined; deleteSuffix: boolean }) | (TokenBase & { kind: "payload-literal"; text: string }) | (TokenBase & { kind: "payload-repeat"; range: ParsedRange }) - | (TokenBase & { kind: "payload-repeat-shorthand" }) | (TokenBase & { kind: "raw"; text: string }); function classifyLine(line: string, lineNum: number): Token { @@ -326,7 +365,6 @@ function classifyLine(line: string, lineNum: number): Token { } if (firstCode === CHAR_PAYLOAD_REPEAT) { const range = tryParseRepeatPayload(line); - if (range === "shorthand") return { kind: "payload-repeat-shorthand", lineNum }; if (range !== null) return { kind: "payload-repeat", lineNum, range }; } diff --git a/packages/hashline/test/format-v2.test.ts b/packages/hashline/test/format-v2.test.ts index e3ce637f9..b78036e9e 100644 --- a/packages/hashline/test/format-v2.test.ts +++ b/packages/hashline/test/format-v2.test.ts @@ -8,7 +8,7 @@ function applyPatch(text: string, diff: string): string { describe("hashline format v2", () => { it("emits literal and repeat body rows in textual order", () => { const text = "a\nb\nc"; - const diff = ["2-2:", "|before", "^1-2", "|after"].join("\n"); + const diff = ["2-2:", "+before", "^1-2", "+after"].join("\n"); expect(applyPatch(text, diff)).toBe("a\nbefore\na\nb\nafter\nc"); }); @@ -34,7 +34,7 @@ describe("hashline format v2", () => { }); it("rejects body rows after inline delete", () => { - expect(() => parsePatch("2-2:-\n|x")).toThrow(/payload line has no preceding/); + expect(() => parsePatch("2-2:-\n+x")).toThrow(/payload line has no preceding/); }); it("treats an empty concrete block as a blank-line replacement", () => { @@ -50,13 +50,22 @@ describe("hashline format v2", () => { expect(applyPatch(text, "EOF:")).toBe("a\nb\n"); }); - it("rejects repeat shorthand with an explicit-range hint", () => { - expect(() => parsePatch("2-2:\n^2")).toThrow(/\^A-A/); + it("accepts `^A` repeat shorthand as `^A-A`", () => { + const text = "a\nb\nc"; + // `^A` mirrors `^A-A`; we use it to keep line 2 unchanged while + // also targeting it. + expect(applyPatch(text, "2-2:\n^2")).toBe(text); }); - it("rejects removed insert sigils through the normal body-row diagnostic", () => { - expect(() => parsePatch("2-2:\n↑x")).toThrow(/must start with \| or \^A-B/); - expect(() => parsePatch("2-2:\n↓x")).toThrow(/must start with \| or \^A-B/); + it("auto-pipes bare body rows (legacy sigils flow through as literal text)", () => { + // `↑`/`↓` are no longer reserved sigils; bare body rows are + // auto-prefixed with `|` as plain literal text. + const text = "a\nb\nc"; + expect(applyPatch(text, "2-2:\n↑x")).toBe("a\n↑x\nc"); + expect(applyPatch(text, "2-2:\n↓x")).toBe("a\n↓x\nc"); + // And the warning is surfaced. + const { warnings } = parsePatch("2-2:\n↑x"); + expect(warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); }); it("rejects removed standalone delete rows through the normal op diagnostic", () => { diff --git a/packages/hashline/test/leniency.test.ts b/packages/hashline/test/leniency.test.ts new file mode 100644 index 000000000..94c2b5941 --- /dev/null +++ b/packages/hashline/test/leniency.test.ts @@ -0,0 +1,262 @@ +import { describe, expect, it } from "bun:test"; +import { applyEdits, parsePatch } from "@oh-my-pi/hashline"; + +function applyPatch(text: string, diff: string): string { + return applyEdits(text, parsePatch(diff).edits).text; +} + +const FILE = "a\nb\nc\nd\ne"; + +describe("hashline leniency L1 — bare `A:` shorthand", () => { + it("treats `A:` as `A-A:`", () => { + expect(applyPatch(FILE, "2:\n+B")).toBe("a\nB\nc\nd\ne"); + }); + + it("treats `A:-` as `A-A:-`", () => { + expect(applyPatch(FILE, "2:-")).toBe("a\nc\nd\ne"); + }); + + it("preserves the inline-payload rejection on `A:content`", () => { + expect(() => parsePatch("2:hello")).toThrow(/Inline payload on the anchor line is rejected/); + }); + + it("still rejects `LINE:content` rows pasted in the middle of a payload", () => { + // First block is fine; the second line looks like another op-block + // with inline payload (after L1 it parses as `3-3:` with body + // ` ddd`), which triggers the inline-payload diagnostic. + expect(() => parsePatch("2-2:\n+first\n3: ddd")).toThrow(/Inline payload on the anchor line is rejected/); + }); +}); + +describe("hashline leniency L2 — bare `^A` repeat shorthand", () => { + it("treats `^A` as `^A-A`", () => { + // `^2-2` keeps the original line 2 between the inserted rows. + expect(applyPatch(FILE, "2-2:\n+ABOVE\n^2\n+BELOW")).toBe("a\nABOVE\nb\nBELOW\nc\nd\ne"); + }); + + it("auto-pipes `^A-` (malformed range) as literal text via L3", () => { + // `^2-` is not a valid repeat row (missing end number). The + // tokenizer classifies it as raw; L3's uniformly-bare auto-pipe + // then folds it back into the block as a literal. The model sees + // the warning and can re-issue with a well-formed repeat. + const result = parsePatch("2-2:\n^2-"); + expect(applyEdits(FILE, result.edits).text).toBe("a\n^2-\nc\nd\ne"); + expect(result.warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); + }); +}); + +describe("hashline leniency L3 — auto-pipe uniformly bare bodies", () => { + it("accepts a block whose body is uniformly unprefixed", () => { + const result = parsePatch("2-2:\n hello\n world"); + expect(applyEdits(FILE, result.edits).text).toBe("a\n hello\n world\nc\nd\ne"); + expect(result.warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); + }); + + it("rejects literal-then-bare mixed blocks at the bare row's line", () => { + expect(() => parsePatch("2-2:\n+first\nsecond")).toThrow(/line 3: payload row in a hashline block/); + }); + + it("rejects bare-then-literal mixed blocks at the bare row's line", () => { + // The bare `first` is buffered on line 2; on line 3 the `|second` + // arrives and triggers a retro-rejection pointed at line 2. + expect(() => parsePatch("2-2:\nfirst\n+second")).toThrow(/line 2: payload row in a hashline block/); + }); + + it("does NOT auto-pipe across block boundaries", () => { + // `2-2:` accumulates `foo` as a bare row; `4-4:` flushes the first + // block (auto-pipe fires) and starts a new pending. The second + // block's `bar` row is also bare → second auto-pipe. + const result = parsePatch("2-2:\nfoo\n4-4:\nbar"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nfoo\nc\nbar\ne"); + }); +}); + +describe("hashline leniency L4 — lone `-` body row as delete", () => { + it("retroactively converts a lone `-` row to a `:-` delete", () => { + const result = parsePatch("2-3:\n-"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nd\ne"); + expect(result.warnings.some(w => /Converted a lone `-` body row/.test(w))).toBe(true); + }); + + it("does NOT fire when the block already has `|` rows", () => { + // L4 only triggers on a totally-bare pending block; once a literal + // row arrives, the `-` is a regular mixed-block bare row → reject. + expect(() => parsePatch("2-2:\n+X\n-")).toThrow(/payload row in a hashline block must start with/); + }); + + it("does NOT fire when the block already has bare raw rows", () => { + // `foo` is bare and buffered. The `-` row arrives next; pendingRaws + // is non-empty so L4 does not retro-convert. The block is uniformly + // bare so it auto-pipes both rows (so `-` becomes literal text). + const result = parsePatch("2-2:\nfoo\n-"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nfoo\n-\nc\nd\ne"); + }); +}); + +describe("hashline leniency L5 — overlapping bare/concrete coalesce", () => { + it("coalesces `A-B:` + `A-B:-` (identical-range before-then-delete) into a delete", () => { + const result = parsePatch("2-3:\n2-3:-"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nd\ne"); + expect(result.warnings.some(w => /overlapping bare hashline block/.test(w))).toBe(true); + }); + + it("coalesces an overlapping (not identical) bare anchor followed by `:-`", () => { + // Bare `2-3:` overlaps with the later `3-4:-`. Drop the bare + // pending (which would have replaced 2-3 with a blank), emit only + // the deletes for 3-4. + const result = parsePatch("2-3:\n3-4:-"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nb\ne"); + expect(result.warnings.some(w => /overlapping bare hashline block/.test(w))).toBe(true); + }); + + it("coalesces an overlapping bare anchor followed by a concrete replace", () => { + // Bare `2-3:` overlaps with the concrete `3-4:` (has payload). + // Drop the bare pending, keep the concrete one. + const result = parsePatch("2-3:\n3-4:\n+NEW"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nb\nNEW\ne"); + expect(result.warnings.some(w => /overlapping bare hashline block/.test(w))).toBe(true); + }); + + it("still rejects two concrete overlapping replaces", () => { + // Both pending blocks have payload → no L5 short-circuit. The + // post-hoc validator catches the line-3 collision. + expect(() => parsePatch("2-3:\n+X\n+Y\n3-4:\n+Z")).toThrow(/anchor line 3 is already targeted by another op/); + }); +}); + +describe("hashline leniency L6 — stacked blank-body `A-A:` warning", () => { + it("warns once when two or more consecutive `A-A:` blocks have empty bodies", () => { + const result = parsePatch("2-2:\n3-3:"); + // Both lines became blank. + expect(applyEdits(FILE, result.edits).text).toBe("a\n\n\nd\ne"); + expect(result.warnings.some(w => /run of single-line empty-body blocks/.test(w))).toBe(true); + }); + + it("does NOT warn when only one blank `A-A:` block exists", () => { + const result = parsePatch("2-2:"); + expect(result.warnings.some(w => /run of single-line empty-body blocks/.test(w))).toBe(false); + }); + + it("does NOT warn when blank blocks are interleaved with non-blank blocks", () => { + const result = parsePatch("2-2:\n3-3:\n+X"); + // Run interrupted on the second block; counter resets. + expect(result.warnings.some(w => /run of single-line empty-body blocks/.test(w))).toBe(false); + }); +}); + +describe("hashline leniency L8 — apply_patch / unified-diff contamination", () => { + it("rejects `*** Update File:` sentinels at top level", () => { + expect(() => parsePatch("*** Update File: a.ts\n2-2:\n+X")).toThrow(/apply_patch sentinel/); + }); + + it("rejects `*** Add File:` sentinels", () => { + expect(() => parsePatch("*** Add File: a.ts\n2-2:\n+X")).toThrow(/apply_patch sentinel/); + }); + + it("rejects unified-diff hunk headers (`@@`)", () => { + expect(() => parsePatch("@@ -1,3 +1,3 @@\n2-2:\n+X")).toThrow(/unified-diff hunk header/); + expect(() => parsePatch("@@\n2-2:\n+X")).toThrow(/unified-diff hunk header/); + }); + + it("rejects `-N-M:` / `-N:` apply_patch hunk anchors", () => { + // `+`-prefixed shapes are no longer flagged because `+` is now the + // canonical payload sigil; `+2-2:` tokenizes as a literal payload + // row containing `2-2:` and either lands inside a pending block or + // throws the standard orphan-payload error. + expect(() => parsePatch("-2-3:\n+X")).toThrow(/apply_patch line prefix/); + expect(() => parsePatch("-2:\n+X")).toThrow(/apply_patch line prefix/); + }); + + it("treats top-level `+TEXT` as an orphan literal payload", () => { + // `+` is the payload sigil — at top level (no pending anchor) this + // surfaces the standard "no preceding A-B:" error rather than the + // apply_patch-specific one, so the model still gets a clear pointer + // to add an anchor above the body row. + expect(() => parsePatch("+ const X = 1;\n2-2:")).toThrow( + /payload line has no preceding A-B:, BOF:, or EOF: anchor/, + ); + }); + + it("keeps `-N` bare delete rejection on the legacy unrecognized-block diagnostic", () => { + // `-5` and `-5..7` are the legacy "removed standalone delete row" + // shapes — they still throw with the existing unrecognized-block + // diagnostic, not the apply_patch one. + expect(() => parsePatch("-5")).toThrow(/unrecognized hashline block/); + expect(() => parsePatch("-5..7")).toThrow(/unrecognized hashline block/); + }); + + it("gives a focused message for a lone `-` outside any pending block", () => { + expect(() => parsePatch("-")).toThrow(/a lone "-" is not a valid hashline op/); + }); +}); + +describe("hashline leniency — composite scenarios from the benchmark dumps", () => { + it("recovers GLM's `LINE:` paste + bare body (chat-simple.ts shape)", () => { + const text = "aaa\nbbb\nccc\nddd"; + // Authored: bare `2:` anchor followed by a uniformly-bare body + // pasted from `read` output. L1 promotes `2:` to `2-2:`; L3 + // auto-pipes the bare body rows. + const result = parsePatch("2:\n NEW_LINE_ONE\n NEW_LINE_TWO"); + expect(applyEdits(text, result.edits).text).toBe("aaa\n NEW_LINE_ONE\n NEW_LINE_TWO\nccc\nddd"); + expect(result.warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); + }); + + it("recovers gpt-5-spark's identical-range `89-90: ⏎ 89-90:-` shape", () => { + const text = "aaa\nbbb\nccc\nddd"; + // Identical-range before-then-delete: should delete lines 2-3. + const result = parsePatch("2-3:\n2-3:-"); + expect(applyEdits(text, result.edits).text).toBe("aaa\nddd"); + expect(result.warnings.length).toBeGreaterThan(0); + }); + + it("recovers gpt-5-spark's `+^A-B` shape (model prefixed a repeat with +)", () => { + const text = "aaa\nbbb\nccc"; + // Authored: `2-2: +NEW +^2-2`. The second body row is a repeat row + // the model mistakenly prefixed with `+`. It should be silently + // rerouted as `^2-2` so the patch effectively inserts NEW above + // the original line 2, with a warning. + const result = parsePatch("2-2:\n+NEW\n+^2-2"); + expect(applyEdits(text, result.edits).text).toBe("aaa\nNEW\nbbb\nccc"); + expect(result.warnings.some(w => /A body row started with `\+\^A-B`/.test(w))).toBe(true); + }); + + it("accepts `+^A-B` with leading whitespace inside the literal text", () => { + // gpt-5-spark / chat-simple.ts shape: `+ ^85-85` — the model + // added indentation between `+` and `^A-B`. We trim before checking. + const text = "aaa\nbbb\nccc"; + const result = parsePatch("2-2:\n+NEW\n+ ^2-2"); + expect(applyEdits(text, result.edits).text).toBe("aaa\nNEW\nbbb\nccc"); + expect(result.warnings.some(w => /A body row started with `\+\^A-B`/.test(w))).toBe(true); + }); + + it("accepts `+^A` shorthand (single line)", () => { + const text = "aaa\nbbb\nccc"; + const result = parsePatch("2-2:\n+NEW\n+^2"); + expect(applyEdits(text, result.edits).text).toBe("aaa\nNEW\nbbb\nccc"); + expect(result.warnings.some(w => /A body row started with `\+\^A-B`/.test(w))).toBe(true); + }); + + it("does NOT misclassify `+^literal-text` (not a valid repeat shape)", () => { + // `+^hello` is just a literal payload row whose text is `^hello`. + // No range follows the `^`, so it's not a repeat — emit the literal + // as-is, no warning. + const text = "aaa\nbbb\nccc"; + const result = parsePatch("2-2:\n+^hello"); + expect(applyEdits(text, result.edits).text).toBe("aaa\n^hello\nccc"); + expect(result.warnings.some(w => /A body row started with `\+\^A-B`/.test(w))).toBe(false); + }); +}); + +describe("hashline leniency — BOF/EOF range suffix", () => { + it("accepts `BOF-BOF:` as `BOF:`", () => { + expect(applyPatch(FILE, "BOF-BOF:\n+HEAD")).toBe("HEAD\na\nb\nc\nd\ne"); + }); + + it("accepts `EOF-EOF:` as `EOF:`", () => { + expect(applyPatch(FILE, "EOF-EOF:\n+TAIL")).toBe("a\nb\nc\nd\ne\nTAIL"); + }); + + it("accepts `BOF-EOF:` (degenerate but harmless)", () => { + expect(applyPatch(FILE, "BOF-EOF:\n+HEAD")).toBe("HEAD\na\nb\nc\nd\ne"); + }); +}); diff --git a/packages/hashline/test/patcher.test.ts b/packages/hashline/test/patcher.test.ts new file mode 100644 index 000000000..7139099b3 --- /dev/null +++ b/packages/hashline/test/patcher.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "bun:test"; +import { InMemoryFilesystem, InMemorySnapshotStore, MismatchError, Patch, Patcher } from "@oh-my-pi/hashline"; + +const PATH = "a.ts"; + +describe("Patcher snapshot tag integrity", () => { + it("requires a snapshot store at construction", () => { + const fs = new InMemoryFilesystem(); + const options = { fs } as unknown as { fs: InMemoryFilesystem; snapshots: InMemorySnapshotStore }; + + expect(() => new Patcher(options)).toThrow(/requires a SnapshotStore/); + }); + + it("applies when the section tag resolves to a matching snapshot", async () => { + const fs = new InMemoryFilesystem([[PATH, "before\n"]]); + const snapshots = new InMemorySnapshotStore(); + const tag = snapshots.recordContiguous(PATH, 1, ["before", ""], { fullText: "before\n" }); + const patcher = new Patcher({ fs, snapshots }); + + const result = await patcher.apply(Patch.parse(`¶${PATH}#${tag}\n1-1:\n+after`)); + + expect(result.sections[0]?.op).toBe("update"); + expect(result.sections[0]?.fileHash).toMatch(/^[0-9A-F]{3}$/); + expect(result.sections[0]?.fileHash).not.toBe(tag); + expect(fs.get(PATH)).toBe("after\n"); + }); + + it("normalizes lowercase section tags while parsing", () => { + const section = Patch.parseSingle(`¶${PATH}#0a3\n1-1:\n+after`); + + expect(section.fileHash).toBe("0A3"); + }); + + it("rejects a wrapped tag whose slot now holds unrelated content", async () => { + const fs = new InMemoryFilesystem([[PATH, "target\n"]]); + const snapshots = new InMemorySnapshotStore(); + for (let index = 0; index < 10; index++) { + snapshots.recordContiguous(PATH, 1, [`warmup ${index}`]); + } + const staleTag = snapshots.recordContiguous(PATH, 1, ["target", ""], { fullText: "target\n" }); + for (let index = 0; index < 4096; index++) { + snapshots.recordContiguous(PATH, 1, [`unrelated ${index}`]); + } + const patcher = new Patcher({ fs, snapshots }); + const patch = Patch.parse(`¶${PATH}#${staleTag}\n1-1:\n|changed`); + + await expect(patcher.apply(patch)).rejects.toBeInstanceOf(MismatchError); + expect(fs.get(PATH)).toBe("target\n"); + }); +}); diff --git a/packages/hashline/test/recovery-session-chain.test.ts b/packages/hashline/test/recovery-session-chain.test.ts index e0f9cb42a..e5093bcae 100644 --- a/packages/hashline/test/recovery-session-chain.test.ts +++ b/packages/hashline/test/recovery-session-chain.test.ts @@ -10,13 +10,7 @@ * surface the standard session-chain banner. */ import { describe, expect, it } from "bun:test"; -import { - computeFileHash, - InMemorySnapshotStore, - parsePatch, - RECOVERY_SESSION_REPLAY_WARNING, - Recovery, -} from "@oh-my-pi/hashline"; +import { InMemorySnapshotStore, parsePatch, RECOVERY_SESSION_REPLAY_WARNING, Recovery } from "@oh-my-pi/hashline"; const PATH = "/tmp/__hashline-recovery-session-chain__.ts"; @@ -27,10 +21,8 @@ function seedTwoSnapshots(): { store: InMemorySnapshotStore; v0Text: string; v1T v1Lines[4] = "L5-CHANGED"; const v0Text = `${v0Lines.join("\n")}\n`; const v1Text = `${v1Lines.join("\n")}\n`; - const h0 = computeFileHash(v0Text); - const h1 = computeFileHash(v1Text); - store.recordContiguous(PATH, 1, v0Text.split("\n"), { fullText: v0Text, fileHash: h0 }); - store.recordContiguous(PATH, 1, v1Text.split("\n"), { fullText: v1Text, fileHash: h1 }); + const h0 = store.recordContiguous(PATH, 1, v0Text.split("\n"), { fullText: v0Text }); + const h1 = store.recordContiguous(PATH, 1, v1Text.split("\n"), { fullText: v1Text }); return { store, v0Text, v1Text, h0, h1 }; } diff --git a/packages/hashline/test/snapshots.test.ts b/packages/hashline/test/snapshots.test.ts new file mode 100644 index 000000000..c15728e9e --- /dev/null +++ b/packages/hashline/test/snapshots.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from "bun:test"; +import { InMemorySnapshotStore } from "@oh-my-pi/hashline"; + +const PATH = "/tmp/__hashline-snapshots__.ts"; +const TAG_RE = /^[0-9A-F]{3}$/; + +function nextHex(tag: string): string { + return ((Number.parseInt(tag, 16) + 1) & 0xfff).toString(16).toUpperCase().padStart(3, "0"); +} + +describe("InMemorySnapshotStore", () => { + it("reuses a prior tag when that snapshot is a content-matching superset", () => { + const store = new InMemorySnapshotStore(); + const tag = store.recordContiguous(PATH, 1, ["L1", "L2", "L3"]); + + expect(tag).toMatch(TAG_RE); + expect(store.recordContiguous(PATH, 2, ["L2"])).toBe(tag); + expect(store.recordSparse(PATH, [[3, "L3"]])).toBe(tag); + }); + + it("picks the newest matching superset", () => { + const store = new InMemorySnapshotStore(); + const older = store.recordSparse(PATH, [ + [1, "L1"], + [2, "L2"], + ]); + const newer = store.recordSparse(PATH, [ + [2, "L2"], + [3, "L3"], + ]); + + expect(older).toMatch(TAG_RE); + expect(newer).toMatch(TAG_RE); + expect(newer).not.toBe(older); + expect(store.recordSparse(PATH, [[2, "L2"]])).toBe(newer); + }); + + it("scrambles slot tags so the first and next tags are not predictable counters", () => { + const store = new InMemorySnapshotStore(); + const first = store.recordContiguous(PATH, 1, ["value 0"]); + const second = store.recordContiguous(PATH, 1, ["value 1"]); + + expect(first).toMatch(TAG_RE); + expect(second).toMatch(TAG_RE); + expect(first).not.toBe("000"); + expect(second).not.toBe(nextHex(first)); + }); + + it("pushes new views into distinct ring slots", () => { + const store = new InMemorySnapshotStore(); + const first = store.recordContiguous(PATH, 1, ["one"]); + const second = store.recordContiguous(PATH, 1, ["two"]); + + expect(first).toMatch(TAG_RE); + expect(second).toMatch(TAG_RE); + expect(second).not.toBe(first); + expect(store.head(PATH)?.get(1)).toBe("two"); + expect(store.byHash(PATH, first)?.get(1)).toBe("one"); + expect(store.byHash(PATH, second)?.get(1)).toBe("two"); + }); + + it("rejects cross-path lookups even when the tag slot is occupied", () => { + const store = new InMemorySnapshotStore(); + const tag = store.recordContiguous(PATH, 1, ["one"]); + + expect(store.byHash("/tmp/other.ts", tag)).toBeNull(); + }); + + it("wraps after 4096 pushes and byHash returns the new slot occupant", () => { + const store = new InMemorySnapshotStore(); + const first = store.recordContiguous(PATH, 1, ["value 0"]); + let previous = first; + + for (let index = 1; index < 4096; index++) { + const tag = store.recordContiguous(PATH, 1, [`value ${index}`]); + expect(tag).toMatch(TAG_RE); + expect(tag).not.toBe(previous); + previous = tag; + } + const wrapped = store.recordContiguous(PATH, 1, ["value 4096"]); + + expect(wrapped).toBe(first); + expect(store.byHash(PATH, first)?.get(1)).toBe("value 4096"); + expect(store.byHash(PATH, previous)?.get(1)).toBe("value 4095"); + }); +}); diff --git a/packages/typescript-edit-benchmark/package.json b/packages/typescript-edit-benchmark/package.json index 2b3fcac5b..bd37af81d 100644 --- a/packages/typescript-edit-benchmark/package.json +++ b/packages/typescript-edit-benchmark/package.json @@ -30,6 +30,7 @@ "@babel/parser": "catalog:", "@babel/traverse": "catalog:", "@babel/types": "catalog:", + "@oh-my-pi/hashline": "catalog:", "@oh-my-pi/pi-agent-core": "catalog:", "@oh-my-pi/pi-coding-agent": "catalog:", "@oh-my-pi/pi-utils": "catalog:", diff --git a/packages/typescript-edit-benchmark/src/runner.ts b/packages/typescript-edit-benchmark/src/runner.ts index f9e4a00b7..8967361e6 100644 --- a/packages/typescript-edit-benchmark/src/runner.ts +++ b/packages/typescript-edit-benchmark/src/runner.ts @@ -7,9 +7,10 @@ /// import * as fs from "node:fs"; import * as path from "node:path"; +import { formatHashlineHeader, InMemorySnapshotStore } from "@oh-my-pi/hashline"; import type { AgentMessage, ResolvedThinkingLevel, ThinkingLevel } from "@oh-my-pi/pi-agent-core"; import type { Model } from "@oh-my-pi/pi-ai"; -import { computeFileHash, formatSessionDumpText, RpcClient } from "@oh-my-pi/pi-coding-agent"; +import { formatSessionDumpText, RpcClient } from "@oh-my-pi/pi-coding-agent"; import { prompt } from "@oh-my-pi/pi-utils"; import { diffLines } from "diff"; import { formatDirectory } from "./formatter"; @@ -528,7 +529,7 @@ async function evaluateMutationIntent( } /** - * Build a textual hashline patch (with `¶path#hash` section header) that + * Build a textual hashline patch (with `¶path#tag` section header) that * transforms `actual` into `expected`. Returns null when no changes are * needed or the diff isn't expressible as straight insert/replace/delete ops. */ @@ -602,7 +603,10 @@ function buildGuidedHashlinePatch(file: string, actual: string, expected: string flush(); if (ops.length === 0) return null; - const header = `¶${file}#${computeFileHash(actual)}`; + const normalizedActual = actual.replace(/\r\n?/g, "\n"); + const snapshots = new InMemorySnapshotStore(); + const tag = snapshots.recordContiguous(file, 1, normalizedActual.split("\n"), { fullText: normalizedActual }); + const header = formatHashlineHeader(file, tag); return `${header}\n${ops.join("\n")}`; }