From 55e146fa3c03d4f02806bea8365454f3e5616c18 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 30 May 2026 15:59:58 +0200 Subject: [PATCH] fix: synchronized snapshot-tag checks and fixed ANSI width emoji handling - Introduced a shared missingSnapshotTagMessage helper and reused it from both hashline patcher and preview validation. - Required snapshot tags in preview hashline sections for all edits, including head/tail inserts, so preview failures now match apply-time rejections. - Updated TUI width calculation to strip ANSI escapes before grapheme splitting, preserving styled ZWJ emoji width behavior. --- .../coding-agent/src/edit/hashline/diff.ts | 12 +++---- packages/coding-agent/test/edit-diff.test.ts | 30 ++++++++++++++--- packages/hashline/src/messages.ts | 12 +++++++ packages/hashline/src/patcher.ts | 8 ++--- packages/hashline/src/prompt.md | 10 +++--- packages/tui/src/utils.ts | 32 +++++++++++++++++-- packages/tui/test/text-utils.test.ts | 17 ++++++++++ 7 files changed, 98 insertions(+), 23 deletions(-) diff --git a/packages/coding-agent/src/edit/hashline/diff.ts b/packages/coding-agent/src/edit/hashline/diff.ts index 38f3cbf68..b354ca723 100644 --- a/packages/coding-agent/src/edit/hashline/diff.ts +++ b/packages/coding-agent/src/edit/hashline/diff.ts @@ -11,6 +11,7 @@ */ import { Patch as HashlinePatch, + missingSnapshotTagMessage, normalizeToLF, type Patch, type PatchSection, @@ -40,10 +41,6 @@ async function readSectionText(absolutePath: string, sectionPath: string): Promi } } -function hasAnchorScoped(section: PatchSection): boolean { - return section.hasAnchorScopedEdit; -} - function snapshotMatchesCurrent(snapshot: Snapshot, currentText: string): boolean { return snapshot.text === currentText; } @@ -54,9 +51,10 @@ function validateSectionHash( snapshots: SnapshotStore, ): string | null { if (section.fileHash === undefined) { - return hasAnchorScoped(section) - ? `Missing hashline snapshot tag for anchored edit to ${section.path}; use \`¶${section.path}#tag\` from your latest read.` - : null; + // The snapshot tag is mandatory on every section — head/tail inserts + // included — to keep this preview path in lockstep with the apply path + // (`Patcher.prepare`), which rejects tagless sections unconditionally. + return missingSnapshotTagMessage(section.path); } const snapshot = snapshots.byHash(absolutePath, section.fileHash); if (snapshot && snapshotMatchesCurrent(snapshot, text)) return null; diff --git a/packages/coding-agent/test/edit-diff.test.ts b/packages/coding-agent/test/edit-diff.test.ts index 634902777..594322c15 100644 --- a/packages/coding-agent/test/edit-diff.test.ts +++ b/packages/coding-agent/test/edit-diff.test.ts @@ -2,7 +2,7 @@ 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 { formatHashlineHeader, InMemorySnapshotStore, missingSnapshotTagMessage } from "@oh-my-pi/hashline"; import { adjustIndentation, computeEditDiff, @@ -251,18 +251,40 @@ describe("computeHashlineDiff", () => { test("accepts hashline input edits", async () => { const sourcePath = path.join(tempDir, "source.txt"); - await Bun.write(sourcePath, "first\n"); + const text = "first\n"; + await Bun.write(sourcePath, text); + const snapshotStore = new InMemorySnapshotStore(); + const tag = snapshotStore.record(sourcePath, text); const result = await computeHashlineDiff( - { input: `¶${sourcePath}\ninsert tail:\n+second` }, + { input: `${formatHashlineHeader(sourcePath, tag)}\ninsert tail:\n+second` }, tempDir, - new InMemorySnapshotStore(), + snapshotStore, ); expect("diff" in result).toBe(true); if ("diff" in result) { expect(result.diff).toContain("second"); } }); + + test("rejects a tagless head/tail insert in the preview path, matching apply", async () => { + const relativePath = "source.txt"; + await Bun.write(path.join(tempDir, relativePath), "first\n"); + + // A tagless `insert tail:` carries no anchored edit, yet the apply path + // (Patcher.prepare) rejects it for the missing mandatory tag. The + // preview/diff path MUST emit the SAME rejection so a successful preview + // never precedes a failing apply. + const result = await computeHashlineDiff( + { input: `¶${relativePath}\ninsert tail:\n+second` }, + tempDir, + new InMemorySnapshotStore(), + ); + expect("error" in result).toBe(true); + if ("error" in result) { + expect(result.error).toBe(missingSnapshotTagMessage(relativePath)); + } + }); test("returns a handled error when the source path is a local URL", async () => { const result = await computeHashlineDiff( { input: "¶local://PLAN.md\ninsert tail:\n+x" }, diff --git a/packages/hashline/src/messages.ts b/packages/hashline/src/messages.ts index 174f25622..d7490cd3b 100644 --- a/packages/hashline/src/messages.ts +++ b/packages/hashline/src/messages.ts @@ -5,6 +5,8 @@ * them. */ +import { HL_FILE_HASH_SEP, HL_FILE_PREFIX } from "./format"; + /** Lines of context shown either side of a hash mismatch. */ export const MISMATCH_CONTEXT = 2; @@ -75,3 +77,13 @@ export const RECOVERY_SESSION_REPLAY_WARNING = */ export const HEADTAIL_DRIFT_WARNING = "Applied an `insert head:`/`insert tail:` edit onto the current file content even though the snapshot tag was stale (the file changed since your read). Head/tail position is content-independent, so the insert was not rejected — but re-read if the drift was unexpected."; + +/** + * Error text emitted when a hashline section omits the mandatory snapshot tag. + * The tag is REQUIRED on every section, enforced identically by the apply path + * ({@link Patcher.prepare}) and the preview/diff path, so both surfaces reuse + * this single builder to stay in lockstep. + */ +export function missingSnapshotTagMessage(sectionPath: string): string { + return `Missing hashline snapshot tag for edit to ${sectionPath}; use \`${HL_FILE_PREFIX}${sectionPath}${HL_FILE_HASH_SEP}tag\` from your latest read/search output. To create a new file, use the write tool.`; +} diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index 566aaa928..f7f9974c3 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -23,11 +23,11 @@ * filesystem configuration. */ import { applyEdits } from "./apply"; -import { computeFileHash, formatHashlineHeader, HL_FILE_HASH_SEP, HL_FILE_PREFIX } from "./format"; +import { computeFileHash, formatHashlineHeader } from "./format"; import type { Filesystem, WriteResult } from "./fs"; import { isNotFound } from "./fs"; import type { Patch, PatchSection } from "./input"; -import { HEADTAIL_DRIFT_WARNING } from "./messages"; +import { HEADTAIL_DRIFT_WARNING, missingSnapshotTagMessage } from "./messages"; import { MismatchError } from "./mismatch"; import { detectLineEnding, type LineEnding, normalizeToLF, restoreLineEndings, stripBom } from "./normalize"; import { Recovery, type RecoveryResult } from "./recovery"; @@ -105,9 +105,7 @@ function hasAnchorScopedEdit(edits: readonly Edit[]): boolean { function assertSectionHashPresent(sectionPath: string, fileHash: string | undefined): void { if (fileHash !== undefined) return; - throw new Error( - `Missing hashline snapshot tag for edit to ${sectionPath}; use \`${HL_FILE_PREFIX}${sectionPath}${HL_FILE_HASH_SEP}tag\` from your latest read/search output. To create a new file, use the write tool.`, - ); + throw new Error(missingSnapshotTagMessage(sectionPath)); } function recoveryToApplyResult(result: RecoveryResult): ApplyResult { diff --git a/packages/hashline/src/prompt.md b/packages/hashline/src/prompt.md index a664c7778..3f8f1873f 100644 --- a/packages/hashline/src/prompt.md +++ b/packages/hashline/src/prompt.md @@ -33,7 +33,7 @@ There is NO other body row kind. NEVER write `-old` or a bare/context line. To k Original (the exact shape `read` returns): ``` -¶greet.py#A1 +¶greet.py#A1B2 1:def greet(name): 2: msg = "Hello, " + name 3: print(msg) @@ -42,14 +42,14 @@ Original (the exact shape `read` returns): Insert a guard after line 1: ``` -¶greet.py#A1 +¶greet.py#A1B2 insert after 1: + if not name: name = "stranger" ``` Replace line 2 with two lines: ``` -¶greet.py#A1 +¶greet.py#A1B2 replace 2..2: + greeting = "Hi" + msg = f"{greeting}, {name}" @@ -57,13 +57,13 @@ replace 2..2: Delete line 3: ``` -¶greet.py#A1 +¶greet.py#A1B2 delete 3 ``` Add a header and trailer: ``` -¶greet.py#A1 +¶greet.py#A1B2 insert head: +# generated header insert tail: diff --git a/packages/tui/src/utils.ts b/packages/tui/src/utils.ts index 9c3986502..5546d938f 100644 --- a/packages/tui/src/utils.ts +++ b/packages/tui/src/utils.ts @@ -87,10 +87,38 @@ const segmenter = new Intl.Segmenter(undefined, { granularity: "grapheme" }); const EXTENDED_PICTOGRAPHIC_REGEX = /\p{Extended_Pictographic}/u; +// Matches CSI (`\x1b[…`) and OSC (`\x1b]…` terminated by BEL/ST) escape +// sequences. Mirrors the standard ansi-regex coverage so visible-span +// segmentation lines up with the native ANSI scanner. +const ANSI_ESCAPE_REGEX = + /[\u001b\u009b][[\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\d/#&.:=?%@~_]+)*|[a-zA-Z\d]+(?:;[-a-zA-Z\d/#&.:=?%@~_]*)*)?\u0007)|(?:(?:\d{1,4}(?:;\d{0,4})*)?[\dA-PR-TZcf-nq-uy=><~]))/g; + +function pictographicSpanWidth(span: string): number { + let width = 0; + for (const { segment } of segmenter.segment(span)) { + width += EXTENDED_PICTOGRAPHIC_REGEX.test(segment) ? 2 : nativeVisibleWidth(segment, getDefaultTabWidth()); + } + return width; +} + +// Width fallback for strings that mix ANSI styling with ZWJ pictographic +// emoji. `Intl.Segmenter` would split an escape sequence into individual +// graphemes, so the native scanner (which only skips ANSI when handed the +// complete sequence) double-counts the printable SGR bytes. Excise the ANSI +// spans first — they contribute zero cells — and apply the pictographic +// grapheme override only to the visible spans, then sum. function visibleWidthByGrapheme(str: string): number { let width = 0; - for (const { segment } of segmenter.segment(str)) { - width += EXTENDED_PICTOGRAPHIC_REGEX.test(segment) ? 2 : nativeVisibleWidth(segment, getDefaultTabWidth()); + let lastIndex = 0; + ANSI_ESCAPE_REGEX.lastIndex = 0; + for (let match = ANSI_ESCAPE_REGEX.exec(str); match !== null; match = ANSI_ESCAPE_REGEX.exec(str)) { + if (match.index > lastIndex) { + width += pictographicSpanWidth(str.slice(lastIndex, match.index)); + } + lastIndex = ANSI_ESCAPE_REGEX.lastIndex; + } + if (lastIndex < str.length) { + width += lastIndex === 0 ? pictographicSpanWidth(str) : pictographicSpanWidth(str.slice(lastIndex)); } return width; } diff --git a/packages/tui/test/text-utils.test.ts b/packages/tui/test/text-utils.test.ts index 23f101d47..3bb20f04d 100644 --- a/packages/tui/test/text-utils.test.ts +++ b/packages/tui/test/text-utils.test.ts @@ -19,6 +19,23 @@ describe("text utils", () => { expect(visibleWidth(text)).toBe(4); }); + it("counts a styled ZWJ emoji the same as the unstyled emoji (ANSI is zero-width)", () => { + // Family emoji built from ZWJ-joined code points renders as a single + // 2-cell grapheme. Wrapping it in SGR styling must not change its width: + // the grapheme fallback splits ANSI into separate segments, and the + // native scanner only skips ANSI when handed the complete escape — so + // the SGR bytes (`[`, `3`, `1`, `m`, …) must be excised before + // segmentation, not counted as visible cells. + const emoji = "\u{1F468}\u200d\u{1F469}\u200d\u{1F467}"; + const styled = `\x1b[31m${emoji}\x1b[0m`; + expect(visibleWidth(emoji)).toBe(2); + expect(visibleWidth(styled)).toBe(visibleWidth(emoji)); + // Styling around only part of a ZWJ-containing span is also zero-width. + expect(visibleWidth(`a\x1b[1m${emoji}\x1b[22mb`)).toBe(1 + 2 + 1); + // Plain styled ASCII is unaffected — ANSI strips to its visible text. + expect(visibleWidth("\x1b[31mhello\x1b[0m")).toBe(visibleWidth("hello")); + }); + it("truncates ANSI text with ellipsis", () => { const text = "\x1b[31mhello world\x1b[0m"; const result = truncateToWidth(text, 6);