Two things landed in the same place while stating the KernelBackend::patch
batch contract in #373. The first is a concrete bug worth fixing on its own.
The second is the larger question of what patch is for, which wants
deliberate time rather than an incremental fix.
The bug: a byte offset inside a multi-byte character aborts the kernel
PatchOp::{Insert,Delete,Replace} validate only that the offset is within
content.len(). They never check that it lands on a UTF-8 character boundary.
String::insert_str, &content[a..b], drain, and replace_range all abort
on a mid-codepoint index.
Confirmed by probe, not read off the source — PatchOp::Insert { offset: 1 }
into a file beginning with é (two bytes):
thread '...' panicked at crates/kaish-kernel/src/backend/local.rs:66:25:
assertion failed: self.is_char_boundary(idx)
An embedder that computes its own byte offset takes down the kernel instead of
receiving BackendError::InvalidOperation. The file is left untouched, so the
batch contract stated in #373 still holds — but "if an operation fails, the
error returns" does not, which is why that doc was worded to promise only that
a failing operation stops the batch before the write.
The fix is a character-boundary check in the three byte-offset arms, returning
InvalidOperation with the offset and the character it split.
A smaller sibling from the same review, unverified: line: 0 is mapped to line
1 by saturating_sub(1) rather than rejected, even though the PatchOp docs
say line numbers are 1-indexed.
The larger question: how faithful should patch be?
The concern is that a partly-faithful patch is worse than none — an agent
carries expectations from patch(1), and every place kaish quietly differs is
a place those expectations produce a wrong result rather than an error.
Known divergences today:
- No
-F / --fuzz. GNU patch takes it; kaish does not. DEFAULT_FUZZ is
hardcoded to 2 at patch.rs:423, matching GNU's default, but there is no way
to ask for anything else — including 0.
max_fuzz = 0 does not mean "here or nowhere". It still searches
file-wide for a position. There is no way to express "apply at line 42,
refuse otherwise", which is exactly what an anchor-based edit needs. This is
the capability gap that reopened the edit design (see the 0.15.1 slate).
Deciding this shapes the edit work: if patch becomes faithful enough to
carry anchored edits, edit needs less; if it stays a convenience, edit
carries the exact-position contract instead.
Deferred deliberately — filing so it does not get rediscovered a third time.
Two things landed in the same place while stating the
KernelBackend::patchbatch contract in #373. The first is a concrete bug worth fixing on its own.
The second is the larger question of what
patchis for, which wantsdeliberate time rather than an incremental fix.
The bug: a byte offset inside a multi-byte character aborts the kernel
PatchOp::{Insert,Delete,Replace}validate only that the offset is withincontent.len(). They never check that it lands on a UTF-8 character boundary.String::insert_str,&content[a..b],drain, andreplace_rangeall aborton a mid-codepoint index.
Confirmed by probe, not read off the source —
PatchOp::Insert { offset: 1 }into a file beginning with
é(two bytes):An embedder that computes its own byte offset takes down the kernel instead of
receiving
BackendError::InvalidOperation. The file is left untouched, so thebatch contract stated in #373 still holds — but "if an operation fails, the
error returns" does not, which is why that doc was worded to promise only that
a failing operation stops the batch before the write.
The fix is a character-boundary check in the three byte-offset arms, returning
InvalidOperationwith the offset and the character it split.A smaller sibling from the same review, unverified:
line: 0is mapped to line1 by
saturating_sub(1)rather than rejected, even though thePatchOpdocssay line numbers are 1-indexed.
The larger question: how faithful should
patchbe?The concern is that a partly-faithful
patchis worse than none — an agentcarries expectations from
patch(1), and every place kaish quietly differs isa place those expectations produce a wrong result rather than an error.
Known divergences today:
-F/--fuzz. GNU patch takes it; kaish does not.DEFAULT_FUZZishardcoded to 2 at
patch.rs:423, matching GNU's default, but there is no wayto ask for anything else — including 0.
max_fuzz = 0does not mean "here or nowhere". It still searchesfile-wide for a position. There is no way to express "apply at line 42,
refuse otherwise", which is exactly what an anchor-based edit needs. This is
the capability gap that reopened the
editdesign (see the 0.15.1 slate).Deciding this shapes the
editwork: ifpatchbecomes faithful enough tocarry anchored edits,
editneeds less; if it stays a convenience,editcarries the exact-position contract instead.
Deferred deliberately — filing so it does not get rediscovered a third time.