smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(hashline): enabled content hash matching to resolve tag collisions

Browse Source 
- Simplified match logic to rely exclusively on content hash equality.
- Removed strict validation that rejected colliding snapshot tags.
- Updated recovery behavior to resolve collisions to the most-recently recorded snapshot.
- Refactored tests to expect successful preview and patching despite tag ambiguity.
This commit is contained in:
can1357
2026-07-02 22:43:28 +02:00
parent d82b9bdc5f
commit 0059aed4f0
10 changed files with 56 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Improved reliability of edits when file snapshots share identical 16-bit hash tags
## [16.3.2] - 2026-07-02
### Breaking Changes
packages/coding-agent/src/edit/hashline/diff.ts
+4 -13
View File
@@ -180,8 +180,7 @@ function resolvePreviewEdits(args: {
}): readonly Edit[] {
const { section, absolutePath, normalized, snapshots, expected, liveMatches, edits } = args;
if (!hasBlockEdit(edits)) return edits;
const baseText =
expected === undefined || liveMatches ? normalized : snapshots.byHashExact(absolutePath, expected)?.text;
const baseText = expected === undefined || liveMatches ? normalized : snapshots.byHash(absolutePath, expected)?.text;
if (baseText === undefined) {
throw createMismatchError(section, absolutePath, normalized, snapshots, expected ?? "");
}
@@ -200,17 +199,9 @@ function applyPreviewEdits(args: {
if (!options.skipHashValidation && expected === undefined) {
throw new Error(missingSnapshotTagMessage(section.path));
}
// A 16-bit tag can collide across two different file states, so hash
// equality alone does not prove the live text IS the snapshot the model's
// anchors were minted against (mirrors Patcher's apply-time guard). When
// the store retains text for the tag, require it to be unambiguous and
// byte-identical to live; otherwise fall through to recovery/reject below
// exactly as if the hash had not matched.
const liveMatches =
expected !== undefined &&
computeFileHash(normalized) === expected &&
(snapshots.byHash(absolutePath, expected) === null ||
snapshots.byHashExact(absolutePath, expected)?.text === normalized);
// The 4-hex tag is content-derived: when the live text hashes to it, trust
// the match and preview directly (mirrors Patcher's apply-time behavior).
const liveMatches = expected !== undefined && computeFileHash(normalized) === expected;
const edits = parsePreviewEdits(section, options.streaming);
const resolved = resolvePreviewEdits({ section, absolutePath, normalized, snapshots, expected, liveMatches, edits });
if (options.skipHashValidation || expected === undefined || liveMatches) return applyEdits(normalized, resolved);
packages/coding-agent/test/edit-diff.test.ts
+10 -38
View File
@@ -300,9 +300,10 @@ describe("computeHashlineDiff", () => {
});
// A 16-bit snapshot tag can collide across two different file states. The
// preview must mirror Patcher's apply-time guard: hash equality alone never
// proves the live text IS the snapshot the anchors were minted against.
test("rejects the no-drift path when the live text is a colliding ambiguous tag", async () => {
// preview mirrors Patcher's apply-time behavior: tag equality with the
// live content is trusted as-is — a colliding retained snapshot must not
// reject the preview, since a forced re-read would mint the very same tag.
test("previews onto live content when the tag matches a colliding retained snapshot", async () => {
// Both texts hash to `1D84` (pinned in hashline's collision tests).
const SNAPSHOT_TEXT = "line one 263\nline two 4471\n";
const LIVE_TEXT = "line one 410\nline two 6970\n";
@@ -311,48 +312,19 @@ describe("computeHashlineDiff", () => {
const snapshotStore = new InMemorySnapshotStore();
// Anchors were minted against SNAPSHOT_TEXT; the live file is the
// colliding LIVE_TEXT, also retained (e.g. read after an external
// write). The tag is ambiguous — previewing the SWAP against live
// would show the model's payload landing on unrelated content.
// colliding LIVE_TEXT. The tag still matches live, so the SWAP lands
// on live line 2.
const tag = snapshotStore.record(sourcePath, SNAPSHOT_TEXT);
snapshotStore.record(sourcePath, LIVE_TEXT);
const result = await computeHashlineDiff(
{ input: `${formatHashlineHeader(sourcePath, tag)}\nSWAP 2.=2:\n+edited from snapshot` },
{ input: `${formatHashlineHeader(sourcePath, tag)}\nSWAP 2.=2:\n+edited live` },
tempDir,
snapshotStore,
);
expect("error" in result).toBe(true);
if ("error" in result) {
expect(result.error).toContain("file changed between read and edit");
}
});
test("rejects the no-drift path when the live text collides with the single retained snapshot", async () => {
const SNAPSHOT_TEXT = "line one 263\nline two 4471\n";
const LIVE_TEXT = "line one 410\nline two 6970\n";
const sourcePath = path.join(tempDir, "source.txt");
await Bun.write(sourcePath, LIVE_TEXT);
const snapshotStore = new InMemorySnapshotStore();
// Only SNAPSHOT_TEXT is retained; live drifted to a colliding text the
// store never saw. computeFileHash(live) === tag, but the retained
// text differs — recovery (3-way merge from SNAPSHOT_TEXT) must run
// instead of anchoring directly onto the collider. Here the merge
// cannot apply (every line differs), so the preview surfaces the
// drift error rather than a bogus diff.
const tag = snapshotStore.record(sourcePath, SNAPSHOT_TEXT);
const result = await computeHashlineDiff(
{ input: `${formatHashlineHeader(sourcePath, tag)}\nSWAP 2.=2:\n+edited from snapshot` },
tempDir,
snapshotStore,
);
expect("error" in result).toBe(true);
if ("error" in result) {
expect(result.error).toContain("file changed between read and edit");
expect("diff" in result).toBe(true);
if ("diff" in result) {
expect(result.diff).toContain("edited live");
}
});
});
packages/hashline/CHANGELOG.md
+8
View File
@@ -2,6 +2,14 @@
## [Unreleased]
### Breaking Changes
- Removed `SnapshotStore.byHashExact`; consumers resolve tags via `byHash`, which returns the most recently recorded version on a collision.
### Changed
- Improved robustness of patch application by resolving 16-bit snapshot tag collisions to the most recent version rather than rejecting them.
## [16.3.0] - 2026-07-02
### Changed
packages/hashline/src/patcher.ts
+6 -22
View File
@@ -521,23 +521,13 @@ export class Patcher {
}): ApplyResult {
const { section, canonicalPath, exists, normalized, edits } = args;
const expected = exists ? section.fileHash : undefined;
// A 16-bit tag can collide across two different file states, so equality
// on `computeFileHash(normalized) === expected` alone is not enough to
// prove the live text IS the snapshot the tag names. Also require that,
// when a snapshot for `(path, expected)` is retained, exactly one stored
// version carries the tag and its full text matches the live text. If
// multiple versions share the tag, the header is ambiguous: there is no
// safe way to know which stored text the model's line anchors came from.
const storedSnapshotsForTag =
expected === undefined
? []
: this.snapshots.findByHash(expected).filter(snapshot => snapshot.path === canonicalPath);
const ambiguousStoredTag = storedSnapshotsForTag.length > 1;
// The 4-hex tag is content-derived: when the live text hashes to it,
// trust the match and apply directly. `storedSnapshotForTag` feeds the
// drift paths below (block resolution, 3-way recovery); on a 16-bit
// tag collision it resolves to the most-recently recorded text.
const storedSnapshotForTag = expected === undefined ? null : this.snapshots.byHash(canonicalPath, expected);
const hashMatches = expected !== undefined && computeFileHash(normalized) === expected;
const matchedSnapshot = hashMatches ? this.snapshots.byContent(canonicalPath, normalized) : null;
const liveMatches =
hashMatches && !ambiguousStoredTag && (storedSnapshotForTag === null || matchedSnapshot !== null);
const liveMatches = expected !== undefined && computeFileHash(normalized) === expected;
const matchedSnapshot = liveMatches ? this.snapshots.byContent(canonicalPath, normalized) : null;
// Resolve `replace_block N:` edits to concrete ranges before recovery
// runs. Block anchors are expressed against the snapshot the section tag
@@ -552,9 +542,6 @@ export class Patcher {
const resolveWarnings: string[] = [];
let resolved: readonly Edit[] = edits;
if (hasBlockEdit(edits)) {
if (ambiguousStoredTag) {
throw this.#mismatchError(section, canonicalPath, normalized, expected ?? "", true);
}
const baseText = expected === undefined || liveMatches ? normalized : storedSnapshotForTag?.text;
if (baseText === undefined) {
throw this.#mismatchError(section, canonicalPath, normalized, expected ?? "", false);
@@ -590,9 +577,6 @@ export class Patcher {
const result = applyEdits(normalized, resolved);
return withResolveWarnings({ ...result, warnings: [HEADTAIL_DRIFT_WARNING, ...(result.warnings ?? [])] });
}
if (ambiguousStoredTag) {
throw this.#mismatchError(section, canonicalPath, normalized, expected ?? "", true);
}
// File drifted: try to replay the edit against the version the tag
// names and 3-way-merge it onto the live content.
const recovered = this.recovery.tryRecover({
packages/hashline/src/recovery.ts
+4 -5
View File
@@ -392,11 +392,10 @@ export class Recovery {
*/
tryRecover(args: RecoveryArgs): RecoveryResult | null {
const { path, currentText, fileHash, edits } = args;
// Collision-safe lookup: when two retained texts share the 16-bit tag
// there is no way to know which one the model's anchors were minted
// against — replaying against the wrong collider would land the edit
// on unrelated content. Refuse and let the caller reject (re-read).
const snapshot = this.store.byHashExact(path, fileHash);
// When two retained texts collide on the 16-bit tag, resolve to the
// most-recently recorded one; a wrong pick can only land if one of the
// merge/remap/session-chain strategies below applies it cleanly.
const snapshot = this.store.byHash(path, fileHash);
if (!snapshot) return null;
const isHead = isHeadSnapshot(this.store.head(path), snapshot);
const recoveryWarning = isHead ? RECOVERY_EXTERNAL_WARNING : RECOVERY_SESSION_CHAIN_WARNING;
packages/hashline/src/snapshots.ts
+6 -33
View File
@@ -11,7 +11,7 @@
* {@link SnapshotStore.record} with the full normalized text they observed.
* The store hashes it, dedups against the per-path history, and returns the
* tag. Consumers (recovery, the patcher) resolve a stale tag back to the
* recorded full text via {@link SnapshotStore.byHashExact} and 3-way-merge the
* recorded full text via {@link SnapshotStore.byHash} and 3-way-merge the
* would-be edit onto the live content.
*
* The abstract base class lets callers plug in whatever storage they like
@@ -49,8 +49,8 @@ export interface Snapshot {
/**
* Storage seam for full-file version snapshots. The patcher calls {@link head}
* for the latest version of a path and {@link byHashExact} when it needs the
* specific historical version a section's stale tag names.
* for the latest version of a path and {@link byHash} when it needs the
* historical version a section's stale tag names.
*/
export abstract class SnapshotStore {
/** Most-recently recorded version for `path`, or `null` if none. */
@@ -59,27 +59,14 @@ export abstract class SnapshotStore {
/**
* Recorded version for `path` whose tag equals `hash`, or `null`. When two
* distinct texts collide on the 16-bit tag, returns the most-recently
* recorded one; callers that treat the tag as content identity must use
* {@link byHashExact} (or verify {@link Snapshot.text} via {@link byContent}).
* recorded one.
*/
abstract byHash(path: string, hash: string): Snapshot | null;
/**
* Collision-safe {@link byHash}: the single retained version for `path`
* whose tag equals `hash`, or `null` when none is retained OR when two or
* more distinct texts collide on the tag. In the collision case there is
* no way to know which retained text the model's line anchors were minted
* against, so consumers that replay anchors (recovery, previews) must
* refuse rather than pick one.
*/
abstract byHashExact(path: string, hash: string): Snapshot | null;
/**
* Recorded version for `path` whose {@link Snapshot.text} equals `fullText`,
* or `null`. Disambiguates hash collisions where two distinct file states
* share the same 4-hex tag: the patcher consults this before taking the
* no-drift path so a colliding live text is never accepted as the exact
* snapshot the model's line anchors were minted against.
* or `null`. The patcher uses it on the no-drift path to attach seen-line
* provenance to the exact text the model read.
*/
abstract byContent(path: string, fullText: string): Snapshot | null;
@@ -189,20 +176,6 @@ export class InMemorySnapshotStore extends SnapshotStore {
return history?.find(version => version.hash === hash) ?? null;
}
byHashExact(path: string, hash: string): Snapshot | null {
const history = this.#versions.get(path);
if (history === undefined) return null;
let match: Snapshot | null = null;
for (const version of history) {
if (version.hash !== hash) continue;
// Two retained versions with one tag are distinct texts by
// construction (record() dedups on full-text equality) — ambiguous.
if (match !== null) return null;
match = version;
}
return match;
}
byContent(path: string, fullText: string): Snapshot | null {
const history = this.#versions.get(path);
return history?.find(version => version.text === fullText) ?? null;
packages/hashline/test/patcher.test.ts
+7 -60
View File
@@ -127,13 +127,11 @@ describe("Patcher snapshot tag integrity", () => {
expect(fs.get(PATH)).toBe("current\n");
});
// A 16-bit snapshot tag can collide across two different file states.
// When the model authored line-anchored edits against snapshot A but the
// live file is a colliding text B, `computeFileHash` alone cannot tell them
// apart. The patcher must NOT take the no-drift path against B: it either
// recovers against the stored A (3-way merge) or rejects as stale.
// Regression for issue #4075.
it("refuses to accept a colliding live text as the tagged snapshot", async () => {
// A 16-bit snapshot tag can collide across two different file states. Tag
// equality with the live content is trusted as-is: the model did nothing
// wrong, and a forced re-read would mint the very same tag. Line anchors
// therefore index the live text, colliding retained snapshots notwithstanding.
it("applies onto live content when the tag matches, even against a retained colliding snapshot", async () => {
// These two texts both hash to `1D84`.
const SNAPSHOT_TEXT = "line one 263\nline two 4471\n";
const LIVE_TEXT = "line one 410\nline two 6970\n";
@@ -145,59 +143,8 @@ describe("Patcher snapshot tag integrity", () => {
const tag = snapshots.record(PATH, SNAPSHOT_TEXT, [1, 2]);
const patcher = new Patcher({ fs, snapshots });
try {
await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 2.=2:\n+edited from snapshot`));
throw new Error("expected MismatchError");
} catch (error) {
expect(error).toBeInstanceOf(MismatchError);
const message = (error as MismatchError).displayMessage;
// The tag IS a known snapshot, so we land on the drift branch.
expect(message).toMatch(/file changed between read and edit/);
}
// Live file untouched: line 2 must NOT have been overwritten with the
// model's edit anchored against the collider.
expect(fs.get(PATH)).toBe(LIVE_TEXT);
});
it("rejects an ambiguous colliding tag even when live text matches one retained collider", async () => {
const SNAPSHOT_TEXT = "line one 263\nline two 4471\n";
const LIVE_TEXT = "line one 410\nline two 6970\n";
expect(computeFileHash(SNAPSHOT_TEXT)).toBe(computeFileHash(LIVE_TEXT));
const snapshots = new InMemorySnapshotStore();
const tag = snapshots.record(PATH, SNAPSHOT_TEXT, [1, 2]);
snapshots.record(PATH, LIVE_TEXT, [1, 2]);
const fs = new InMemoryFilesystem([[PATH, LIVE_TEXT]]);
const patcher = new Patcher({ fs, snapshots });
try {
await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 2.=2:\n+edited from snapshot`));
throw new Error("expected MismatchError");
} catch (error) {
expect(error).toBeInstanceOf(MismatchError);
}
expect(fs.get(PATH)).toBe(LIVE_TEXT);
});
it("rejects an ambiguous colliding tag before stale recovery can target the wrong snapshot", async () => {
const SNAPSHOT_TEXT = "line one 263\nline two 4471\n";
const COLLIDING_TEXT = "line one 410\nline two 6970\n";
const LIVE_TEXT = "line one 410\nline two 6970\nlive-added\n";
expect(computeFileHash(SNAPSHOT_TEXT)).toBe(computeFileHash(COLLIDING_TEXT));
const snapshots = new InMemorySnapshotStore();
const tag = snapshots.record(PATH, SNAPSHOT_TEXT, [1, 2]);
snapshots.record(PATH, COLLIDING_TEXT, [1, 2]);
const fs = new InMemoryFilesystem([[PATH, LIVE_TEXT]]);
const patcher = new Patcher({ fs, snapshots });
try {
await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 2.=2:\n+edited from snapshot`));
throw new Error("expected MismatchError");
} catch (error) {
expect(error).toBeInstanceOf(MismatchError);
}
expect(fs.get(PATH)).toBe(LIVE_TEXT);
await patcher.apply(Patch.parse(`[${PATH}#${tag}]\nSWAP 2.=2:\n+edited live`));
expect(fs.get(PATH)).toBe("line one 410\nedited live\n");
});
});
packages/hashline/test/recovery-session-chain.test.ts
+7 -11
View File
@@ -203,7 +203,7 @@ function findCollidingTexts(): { older: string; newer: string } {
}
describe("Recovery — colliding snapshot tags", () => {
it("refuses recovery when two retained texts share the section's tag", () => {
it("recovers against the most-recently retained text when two colliders share the tag", () => {
const { older, newer } = findCollidingTexts();
const tag = computeFileHash(older);
expect(computeFileHash(newer)).toBe(tag);
@@ -213,12 +213,9 @@ describe("Recovery — colliding snapshot tags", () => {
store.record(PATH, older);
store.record(PATH, newer);
// Live file drifted away from both colliders, so recovery cannot
// shortcut via live==snapshot; it must pick a base text for the tag.
// The model's edit was authored against `older` (line 2 = its unique
// payload). Resolving the tag to the most-recent collider would 3-way
// merge the stale payload onto `newer`'s unrelated line 2 — silent
// corruption. The ambiguous tag must refuse instead.
// Live drifted away from both colliders, so recovery cannot shortcut
// via live==snapshot. The tag cannot name a unique base; it resolves
// to the most-recently recorded collider and 3-way merges from there.
const currentText = `${newer}drifted trailer\n`;
const recovered = new Recovery(store).tryRecover({
path: PATH,
@@ -227,13 +224,12 @@ describe("Recovery — colliding snapshot tags", () => {
edits: parsePatch("SWAP 2.=2:\n+model payload").edits,
});
expect(recovered).toBeNull();
expect(recovered?.text).toBe(lines("shared head", "model payload", "shared tail", "drifted trailer"));
});
it("still recovers when exactly one retained text carries the tag", () => {
// Same drift scenario minus the collision: the unambiguous tag keeps
// recovering via 3-way merge, proving the collision gate above does
// not overreach.
// Same drift scenario with a single retained text for the tag: the
// plain 3-way merge path.
const { older } = findCollidingTexts();
const store = new InMemorySnapshotStore();
const tag = store.record(PATH, older);
packages/hashline/test/snapshots.test.ts
-20
View File
@@ -151,25 +151,5 @@ describe("InMemorySnapshotStore", () => {
expect(store.byContent(PATH, COLLIDE_A)?.seenLines).toEqual(new Set([1, 2]));
expect(store.byContent(PATH, COLLIDE_B)).toBeNull();
});
it("byHashExact returns the single retained collider, or null when the tag is ambiguous", () => {
const store = new InMemorySnapshotStore();
expect(store.byHashExact(PATH, computeFileHash(COLLIDE_A))).toBeNull();
const tag = store.record(PATH, COLLIDE_A);
// Exactly one retained text for the tag → safe to resolve.
expect(store.byHashExact(PATH, tag)?.text).toBe(COLLIDE_A);
store.record(PATH, COLLIDE_B);
// Two distinct texts now share the tag: there is no way to know
// which one a section's anchors were minted against, so the
// collision-safe lookup must refuse while byHash still surfaces
// the most-recent collider.
expect(store.byHashExact(PATH, tag)).toBeNull();
expect(store.byHash(PATH, tag)?.text).toBe(COLLIDE_B);
// Other paths and unknown tags stay null.
expect(store.byHashExact(OTHER, tag)).toBeNull();
expect(store.byHashExact(PATH, tag === "0000" ? "FFFF" : "0000")).toBeNull();
});
});
});