Skip to content

Commit 5bb2ac4

Browse files
committed
fix(PLA-121): annotate line refs with externalId, not internal id
groupAnnotatedLineRefsByFile was setting keyChangeId to keyChange.id (the internal DB id), but every other surface — view-state, the side panel's focus state, and the detail page's k.externalId === focusedKeyChangeId lookup — keys on externalId. The mismatch silently broke overlay clicks: clicking an overlay box would set focus to an id no other component could resolve, and the corresponding checkbox lookup against view.keyChangeIdSet would always miss. Reported by Cursor Bugbot.
1 parent 56a93ce commit 5bb2ac4

2 files changed

Lines changed: 53 additions & 2 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { describe, expect, it } from "vitest";
2+
import { DIFF_SIDE } from "../diff-types";
3+
import { groupAnnotatedLineRefsByFile, groupLineRefsByFile } from "../line-refs-by-file";
4+
5+
describe("groupLineRefsByFile", () => {
6+
it("returns null for empty input", () => {
7+
expect(groupLineRefsByFile([])).toBeNull();
8+
expect(groupLineRefsByFile(null)).toBeNull();
9+
expect(groupLineRefsByFile(undefined)).toBeNull();
10+
});
11+
12+
it("groups refs by filePath, preserving input order within a file", () => {
13+
const refs = [
14+
{ filePath: "a.ts", side: DIFF_SIDE.ADDITIONS, startLine: 1, endLine: 1 },
15+
{ filePath: "b.ts", side: DIFF_SIDE.ADDITIONS, startLine: 2, endLine: 2 },
16+
{ filePath: "a.ts", side: DIFF_SIDE.ADDITIONS, startLine: 5, endLine: 6 },
17+
];
18+
const result = groupLineRefsByFile(refs);
19+
expect(result?.get("a.ts")).toHaveLength(2);
20+
expect(result?.get("a.ts")?.[0]?.startLine).toBe(1);
21+
expect(result?.get("a.ts")?.[1]?.startLine).toBe(5);
22+
expect(result?.get("b.ts")).toHaveLength(1);
23+
});
24+
});
25+
26+
describe("groupAnnotatedLineRefsByFile", () => {
27+
it("annotates each line ref with the keyChange's externalId, not the internal id", () => {
28+
// Regression: previously this used `keyChange.id` (internal DB id), which
29+
// caused overlay clicks to set focus to a value that never matched the
30+
// side-panel's externalId-keyed focus state.
31+
const result = groupAnnotatedLineRefsByFile([
32+
{
33+
externalId: "ext-kc-1",
34+
lineRefs: [
35+
{ filePath: "src/foo.ts", side: DIFF_SIDE.ADDITIONS, startLine: 10, endLine: 10 },
36+
],
37+
},
38+
]);
39+
expect(result?.get("src/foo.ts")?.[0]?.keyChangeId).toBe("ext-kc-1");
40+
});
41+
42+
it("returns null when there are no key changes", () => {
43+
expect(groupAnnotatedLineRefsByFile([])).toBeNull();
44+
expect(groupAnnotatedLineRefsByFile(null)).toBeNull();
45+
});
46+
});

packages/web/src/lib/line-refs-by-file.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import type { LineRef } from "@stage-cli/types/chapters";
22
import type { AnnotatedLineRef } from "./diff-types";
33

4+
// `externalId` (not `id`) because that's what view-state, the side panel, and
5+
// the chapter detail page's focus state all key on. Annotating with the
6+
// internal DB id would silently break overlay ↔ side-panel sync: a side-panel
7+
// click sets focus to externalId, while an overlay click would set it to id,
8+
// and view-state checked-state lookups would never find the row.
49
interface KeyChangeWithLineRefs {
5-
id: string;
10+
externalId: string;
611
lineRefs: LineRef[];
712
}
813

@@ -30,7 +35,7 @@ export function groupAnnotatedLineRefsByFile(
3035
return groupLineRefsByFile(
3136
keyChanges.flatMap((keyChange) =>
3237
keyChange.lineRefs.map(
33-
(lineRef): AnnotatedLineRef => ({ ...lineRef, keyChangeId: keyChange.id }),
38+
(lineRef): AnnotatedLineRef => ({ ...lineRef, keyChangeId: keyChange.externalId }),
3439
),
3540
),
3641
);

0 commit comments

Comments
 (0)