Skip to content

Commit 8d1c056

Browse files
committed
Skills: bound the diff and refuse a save that lost the race
Two defects in the review flow, both fixed at the root. The diff allocated a dense longest-common-subsequence table over the whole document, so a 12k-line skill cost gigabytes. It now normalizes newlines, trims the identical head and tail before building anything, refuses the quadratic walk past a hard cap (1500 changed lines per side, 400k characters) with an honest "too large to show line by line" summary, and collapses long unchanged runs to a single row. The table itself is Int32Array-backed, so the capped worst case is about 9MB. The script is computed once and both the rows and the change summary are read off that one result. The confirmed diff was computed against the version loaded when the page opened, so a save could silently bury whoever published in between. A save now carries the version it was reviewed against; PUT /:name refuses a stale one with a 409, and the page re-reads, keeps the edit, and re-opens the review against what is actually published, saying why. Also: the buffer is newline-normalized so the bytes reviewed are the bytes written, and the description is saved exactly as reviewed rather than trimmed on the way out; side actions (visibility, restore, compare) report failures next to themselves instead of replacing the page, and never reset an unsaved edit; failures read through describeApiError rather than rendering a server message, with a distinct "no skill named …" state for a 404; the new version read validates its id at the route and 404s a commit that isn't in the skill's history rather than dating it to 1970; and the versions list stacks at 1100px, as DESIGN.md specifies. Claude-Session: https://claude.ai/code/session_01Shhie5zM8L54bLHq5gFQti
1 parent 9b47abb commit 8d1c056

7 files changed

Lines changed: 497 additions & 145 deletions

File tree

‎apps/web/src/pages/diff-view.tsx‎

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,33 @@
11
// One diff renderer for every surface that shows "what changed": the
22
// save-confirmation step and the version comparison on a detail page both
3-
// mount this, so a diff always reads the same way. The line script itself
4-
// comes from `@corbits/text-diff`; this file is only its presentation.
3+
// mount this, so a diff always reads the same way. The line script comes
4+
// from `@corbits/text-diff`; this file is only its presentation, and it
5+
// computes the script exactly once per render — the change summary is read
6+
// off the same result the rows come from.
57

68
import { Badge } from "@corbits/react-ui";
7-
import { diffLines, diffTotals, hasChanges } from "@corbits/text-diff";
9+
import { diffText } from "@corbits/text-diff";
810
import type { DiffLine } from "@corbits/text-diff";
11+
import { useMemo } from "react";
912

1013
const MARKER: Record<DiffLine["kind"], string> = {
1114
context: " ",
1215
added: "+",
1316
removed: "-",
17+
skipped: "⋯",
1418
};
1519

1620
const ROW_CLASS: Record<DiffLine["kind"], string> = {
1721
context: "text-muted-foreground",
1822
added: "bg-success/10 text-foreground",
1923
removed: "bg-destructive/10 text-foreground",
24+
skipped: "text-muted-foreground italic",
2025
};
2126

2227
function lineNumber(value: number | null): string {
2328
return value === null ? "" : String(value);
2429
}
2530

26-
export function DiffSummary({
27-
before,
28-
after,
29-
}: {
30-
readonly before: string;
31-
readonly after: string;
32-
}) {
33-
const totals = diffTotals(diffLines(before, after));
34-
return (
35-
<p className="font-mono text-xs tabular-nums text-muted-foreground">
36-
{`+${String(totals.added)} added, −${String(totals.removed)} removed`}
37-
</p>
38-
);
39-
}
40-
4131
export function DiffView({
4232
before,
4333
after,
@@ -47,23 +37,45 @@ export function DiffView({
4737
readonly after: string;
4838
readonly unchangedNotice?: string;
4939
}) {
50-
const lines = diffLines(before, after);
40+
const diff = useMemo(() => diffText(before, after), [before, after]);
5141

52-
if (!hasChanges(lines)) {
42+
if (diff.status === "identical") {
5343
return (
5444
<p className="text-sm text-muted-foreground" data-testid="diff-unchanged">
5545
{unchangedNotice}
5646
</p>
5747
);
5848
}
5949

50+
if (diff.status === "too-large") {
51+
return (
52+
<div className="flex flex-col gap-1" data-testid="diff-too-large">
53+
<p className="text-sm text-foreground">
54+
This change is too large to show line by line — showing a summary
55+
only.
56+
</p>
57+
<p className="font-mono text-xs tabular-nums text-muted-foreground">
58+
{`${String(diff.beforeLines)} lines before, ${String(
59+
diff.afterLines,
60+
)} after — ${String(diff.changedBeforeLines)} rewritten to ${String(
61+
diff.changedAfterLines,
62+
)}`}
63+
</p>
64+
</div>
65+
);
66+
}
67+
6068
return (
6169
<div className="flex flex-col gap-2" data-testid="diff-view">
62-
<DiffSummary before={before} after={after} />
63-
<div className="overflow-x-auto rounded-md border border-border bg-muted/30">
70+
<p className="font-mono text-xs tabular-nums text-muted-foreground">
71+
{`+${String(diff.totals.added)} added, −${String(
72+
diff.totals.removed,
73+
)} removed`}
74+
</p>
75+
<div className="max-h-96 overflow-auto rounded-md border border-border bg-muted/30">
6476
<table className="w-full border-collapse font-mono text-xs leading-relaxed">
6577
<tbody>
66-
{lines.map((line, index) => (
78+
{diff.lines.map((line, index) => (
6779
<tr
6880
key={`${String(index)}:${line.kind}`}
6981
className={ROW_CLASS[line.kind]}
@@ -78,7 +90,7 @@ export function DiffView({
7890
{MARKER[line.kind]}
7991
</td>
8092
<td className="whitespace-pre-wrap break-words px-2 py-0.5">
81-
{line.text === "" ? " " : line.text}
93+
{line.text === "" ? " " : line.text}
8294
</td>
8395
</tr>
8496
))}

0 commit comments

Comments
 (0)