Skip to content

Commit 57c2fb0

Browse files
authored
fix(artifacts): make edits more robust (lenient id linking + typography-tolerant matching) (#2379)
* fix(artifacts): link renamed/mismatched edit ids instead of dead-carding When the model renames or drifts an artifact identifier on edit (e.g. green-button -> blue-button), the update orphaned into a dead 'This edit couldn't be linked to an artifact' card. collectArtifacts now falls back to the most-recently-created artifact when an update id doesn't match (and an artifact exists), attaching the edit as a proper new version. Also hardens the artifacts system prompt: keep the identifier byte-identical across versions, copy old_str verbatim, and don't call tools mid-artifact. * fix(artifacts): tolerate curly quotes and dashes in edit old_str matching findMatch now normalizes curly quotes and en/em dashes to ASCII (1:1, length-preserving, so offsets still index raw content) before matching, so a model that emits or omits smart quotes/dashes doesn't fail the edit. Unicode spaces are already covered by the existing whitespace-tolerant regex. Inspired by pi-mono's edit tool; the apply-against-original/reverse-order and hard-error ideas were deliberately not ported (we re-search per pair so there's no stale-offset bug, and have no tool-call retry loop). * fix(artifacts): match drifted update id by normalized key before latest-artifact fallback Addresses review: with multiple artifacts, a case/whitespace-drifted id (e.g. 'App' for 'app') was attached to the most-recently-created artifact instead of the one it meant. Now try a normalized (trim+lowercase) id match first; fall back to the latest only when there's no unambiguous normalized match.
1 parent 448892f commit 57c2fb0

3 files changed

Lines changed: 150 additions & 11 deletions

File tree

src/lib/server/textGeneration/artifacts.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,11 @@ Editing an artifact you created earlier in the conversation:
3636
<new_str>replacement text</new_str>
3737
</artifact>
3838
39-
- Each old_str must match the latest version EXACTLY (including whitespace/indentation) and must be unique within it.
39+
- Each old_str must match the latest version EXACTLY (including whitespace/indentation) and must be unique within it. Copy it verbatim from the latest version; do not retype, reformat, or re-indent it.
4040
- Emit at most ONE update block per reply, with all the pairs (up to 4) inside that single block — never one block per pair.
4141
- For larger changes, re-emit the full artifact with the SAME identifier (this creates a new version).
42-
- Reuse the same identifier for every version of one artifact; pick a new identifier only for a genuinely different artifact.
42+
- Keep the identifier BYTE-IDENTICAL across every version, even when the title or content changes (renaming a green button to blue keeps the same identifier). Use a new identifier only for a genuinely different artifact.
43+
- Do not call tools while creating or editing an artifact; emit the artifact in your reply first, then use tools in a later turn if needed.
4344
4445
Around the tags, briefly tell the user in plain text what you built or changed.`;
4546

src/lib/utils/artifacts.spec.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,30 @@ describe("applyArtifactUpdate", () => {
149149
]);
150150
expect(result.content).toBe("v3");
151151
});
152+
153+
it("matches across smart quotes in content, preserving surrounding text", () => {
154+
// content has curly double quotes; old_str uses straight quotes
155+
const content = `const greeting = “Hello”; // tag`;
156+
const result = applyArtifactUpdate(content, [{ old: `"Hello"`, new: `"Goodbye"` }]);
157+
expect(result).toMatchObject({ applied: 1, failed: 0 });
158+
expect(result.content).toBe(`const greeting = "Goodbye"; // tag`);
159+
});
160+
161+
it("matches when old_str uses smart quotes and content is straight", () => {
162+
const result = applyArtifactUpdate(`x = "v"`, [{ old: `x = “v”`, new: `x = "w"` }]);
163+
expect(result).toMatchObject({ applied: 1, failed: 0, content: `x = "w"` });
164+
});
165+
166+
it("matches an em-dash in content against a hyphen in old_str", () => {
167+
const result = applyArtifactUpdate(`a — b`, [{ old: `a - b`, new: `a + b` }]);
168+
expect(result).toMatchObject({ applied: 1, failed: 0, content: `a + b` });
169+
});
170+
171+
it("matches an NBSP in content against a normal space (whitespace tolerance)", () => {
172+
const nbsp = String.fromCharCode(0xa0);
173+
const result = applyArtifactUpdate(`foo${nbsp}bar`, [{ old: `foo bar`, new: `foo baz` }]);
174+
expect(result).toMatchObject({ applied: 1, failed: 0, content: `foo baz` });
175+
});
152176
});
153177

154178
describe("collectArtifacts", () => {
@@ -252,6 +276,76 @@ describe("collectArtifacts", () => {
252276
expect(registry.byMessageOp.get("m1:0")).toEqual({ identifier: "ghost", version: -1 });
253277
});
254278

279+
it("links a renamed-identifier update to the existing artifact", () => {
280+
const registry = collectArtifacts([
281+
msg(
282+
"m1",
283+
`<artifact identifier="green-button" type="html" title="Green Button">a green btn</artifact>`
284+
),
285+
msg(
286+
"m2",
287+
`<artifact identifier="blue-button" type="update" title="Blue Button"><old_str>green</old_str><new_str>blue</new_str></artifact>`
288+
),
289+
]);
290+
// The update linked to the existing artifact instead of orphaning into a dead card
291+
expect(registry.artifacts.size).toBe(1);
292+
const versions = registry.artifacts.get("green-button")?.versions;
293+
expect(versions).toHaveLength(2);
294+
expect(versions?.[1]).toMatchObject({
295+
identifier: "green-button",
296+
op: "update",
297+
title: "Blue Button",
298+
content: "a blue btn",
299+
});
300+
expect(registry.byMessageOp.get("m2:0")).toEqual({ identifier: "green-button", version: 2 });
301+
});
302+
303+
it("links a case-drifted identifier update to the existing artifact", () => {
304+
const registry = collectArtifacts([
305+
msg("m1", `<artifact identifier="app" type="html" title="App">alpha beta</artifact>`),
306+
msg(
307+
"m2",
308+
`<artifact identifier="App" type="update"><old_str>alpha</old_str><new_str>gamma</new_str></artifact>`
309+
),
310+
]);
311+
expect(registry.artifacts.size).toBe(1);
312+
expect(registry.artifacts.get("app")?.versions).toHaveLength(2);
313+
expect(registry.artifacts.get("app")?.versions[1]?.content).toBe("gamma beta");
314+
});
315+
316+
it("links an orphan update to the most-recently-created artifact when several exist", () => {
317+
const registry = collectArtifacts([
318+
msg("m1", `<artifact identifier="first" type="html" title="First">one</artifact>`),
319+
msg("m2", `<artifact identifier="second" type="html" title="Second">two</artifact>`),
320+
msg(
321+
"m3",
322+
`<artifact identifier="ghost" type="update"><old_str>two</old_str><new_str>three</new_str></artifact>`
323+
),
324+
]);
325+
// No "ghost" artifact created; the update attached to the most recent ("second")
326+
expect(registry.artifacts.has("ghost")).toBe(false);
327+
expect(registry.artifacts.get("first")?.versions).toHaveLength(1);
328+
expect(registry.artifacts.get("second")?.versions).toHaveLength(2);
329+
expect(registry.artifacts.get("second")?.versions[1]?.content).toBe("three");
330+
expect(registry.byMessageOp.get("m3:0")).toEqual({ identifier: "second", version: 2 });
331+
});
332+
333+
it("prefers a normalized identifier match over the latest artifact for drifted ids", () => {
334+
const registry = collectArtifacts([
335+
msg("m1", `<artifact identifier="app" type="html" title="App">alpha</artifact>`),
336+
msg("m2", `<artifact identifier="chart" type="html" title="Chart">bravo</artifact>`),
337+
msg(
338+
"m3",
339+
`<artifact identifier="App" type="update"><old_str>alpha</old_str><new_str>gamma</new_str></artifact>`
340+
),
341+
]);
342+
// "App" drifts from "app" by case, so it links there — not the latest ("chart")
343+
expect(registry.artifacts.get("app")?.versions).toHaveLength(2);
344+
expect(registry.artifacts.get("app")?.versions[1]?.content).toBe("gamma");
345+
expect(registry.artifacts.get("chart")?.versions).toHaveLength(1);
346+
expect(registry.byMessageOp.get("m3:0")).toEqual({ identifier: "app", version: 2 });
347+
});
348+
255349
it("applies streaming update pairs progressively", () => {
256350
const registry = collectArtifacts(
257351
[

src/lib/utils/artifacts.ts

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -215,24 +215,46 @@ function escapeRegExp(s: string): string {
215215
return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
216216
}
217217

218+
/**
219+
* Collapse curly quotes and en/em dashes to their ASCII forms so an old_str that
220+
* uses (or omits) them still matches content stored the other way. Every
221+
* substitution is 1:1, so length is preserved and an offset found in the
222+
* normalized string maps straight back to the raw string (this is what lets
223+
* findMatch search the normalized text but slice the raw content). Unicode
224+
* spaces (NBSP etc.) need no handling here — they are already covered by the
225+
* whitespace-tolerant regex below, since JS `\\s` matches them.
226+
*/
227+
function normalizeTypography(s: string): string {
228+
return s
229+
.replace(/[]/g, "'")
230+
.replace(/[]/g, '"')
231+
.replace(/[]/g, "-");
232+
}
233+
218234
/**
219235
* Locate `needle` in `content`. Exact match first; if that fails, retry with
220236
* every whitespace run collapsed to \s+ — models routinely mis-copy
221237
* indentation in old_str, and a strict-only match would turn the whole edit
222238
* into a silent no-op (observed live with Kimi-K2.6 counting spaces).
223239
* Bracket runs are matched loosely the same way ("<<svg" ≈ "<svg"), since the
224-
* stored content is normalized but models quote their own raw output.
240+
* stored content is normalized but models quote their own raw output. Both
241+
* sides are also typography-normalized (curly quotes, en/em dashes) before
242+
* matching.
225243
*/
226244
function findMatch(content: string, needle: string): { start: number; end: number } | null {
227-
const exact = content.indexOf(needle);
228-
if (exact !== -1) return { start: exact, end: exact + needle.length };
229-
const trimmed = needle.trim();
245+
// Substitutions are 1:1, so offsets in the normalized strings index the raw
246+
// `content` directly — no offset remapping needed.
247+
const nContent = normalizeTypography(content);
248+
const nNeedle = normalizeTypography(needle);
249+
const exact = nContent.indexOf(nNeedle);
250+
if (exact !== -1) return { start: exact, end: exact + nNeedle.length };
251+
const trimmed = nNeedle.trim();
230252
if (!trimmed) return null;
231253
try {
232254
const pattern = new RegExp(
233255
escapeRegExp(trimmed.replace(/<{2,}/g, "<")).replace(/</g, "<+").replace(/\s+/g, "\\s+")
234256
);
235-
const match = pattern.exec(content);
257+
const match = pattern.exec(nContent);
236258
if (match) return { start: match.index, end: match.index + match[0].length };
237259
} catch {
238260
// needle too large/odd for a regex — treat as not found
@@ -307,6 +329,11 @@ export function artifactOpKey(messageId: Message["id"], opIndex: number): string
307329
return `${messageId}:${opIndex}`;
308330
}
309331

332+
/** Case/whitespace-insensitive identifier key, for tolerant update linking. */
333+
function normalizeIdentifier(id: string): string {
334+
return id.trim().toLowerCase();
335+
}
336+
310337
/**
311338
* Walk the active conversation path and fold artifact operations into
312339
* versioned artifacts. Updates apply onto the latest version of the same
@@ -371,15 +398,32 @@ export function collectArtifacts(
371398
}
372399

373400
// update op
401+
// Models sometimes rename the identifier when editing (e.g. green-button ->
402+
// blue-button) or drift its case/whitespace, which would orphan the update as a
403+
// dead "couldn't be linked" card. When the id doesn't match an existing artifact,
404+
// first try a normalized (case/whitespace-insensitive) match so a drifted id hits
405+
// the artifact it meant; otherwise fall back to the most-recently-created one (an
406+
// update is by definition an edit to existing content, and the Map preserves
407+
// insertion order). Only leave a disabled card when there's nothing to link to.
408+
if (!artifact && artifacts.size > 0) {
409+
const wanted = normalizeIdentifier(op.identifier);
410+
const normalizedMatches = [...artifacts.values()].filter(
411+
(a) => normalizeIdentifier(a.identifier) === wanted
412+
);
413+
artifact =
414+
normalizedMatches.length === 1 ? normalizedMatches[0] : [...artifacts.values()].at(-1);
415+
}
374416
const base = artifact?.versions.at(-1);
375417
if (!artifact || !base) {
376-
// Update referencing an unknown artifact: surface a disabled card
377418
byMessageOp.set(key, { identifier: op.identifier, version: -1 });
378419
continue;
379420
}
380421
const result = applyArtifactUpdate(base.content, op.pairs);
422+
// Use the resolved artifact's identifier (not op.identifier, which may be the
423+
// renamed/mismatched one) so the new version attaches to the right artifact and
424+
// the card resolves it.
381425
const version: ArtifactVersion = {
382-
identifier: op.identifier,
426+
identifier: artifact.identifier,
383427
type: base.type,
384428
title: op.title || base.title,
385429
language: base.language,
@@ -395,9 +439,9 @@ export function collectArtifacts(
395439
failedPairs: Math.max(result.failed, !isLive && op.pairs.length === 0 ? 1 : 0),
396440
};
397441
artifact.versions.push(version);
398-
byMessageOp.set(key, { identifier: op.identifier, version: version.version });
442+
byMessageOp.set(key, { identifier: artifact.identifier, version: version.version });
399443
if (isLive) {
400-
streaming = { identifier: op.identifier, version: version.version };
444+
streaming = { identifier: artifact.identifier, version: version.version };
401445
}
402446
}
403447
}

0 commit comments

Comments
 (0)