Skip to content

Commit 8cc77e8

Browse files
authored
fix(notion-fetch): stabilize sidebar order on partial sync (#125)
* fix(notion-fetch): stabilize sidebar order on partial sync Fixes #118. - Preserve existing sidebar_position when Order missing - Use Order for toggle category positions - Insert/dedupe subpages deterministically Testing: bunx vitest run scripts/fetchNotionData.test.ts scripts/notion-fetch/generateBlocks.test.ts * docs(qa): add issue 118 QA script Adds a repeatable QA checklist to validate sidebar/category ordering stability across partial sync runs. Refs: #118 * fix(notion-fetch): stabilize sidebar positions * fix(notion-fetch): recompute sidebar position on full sync
1 parent 3254f9c commit 8cc77e8

File tree

6 files changed

+487
-10
lines changed

6 files changed

+487
-10
lines changed
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# QA Script: Issue 118 — Stable Sidebar Order on Partial Syncs
2+
3+
## Goal
4+
Verify that a *partial* Notion sync (processing only a subset of pages) does **not** reshuffle:
5+
- `sidebar_position` for pages missing Notion `Order`
6+
- `_category_.json.position` for toggle sections
7+
- ordering of sub-pages relative to parents
8+
9+
This QA is designed to mimic the “filtered/tagged” CI behavior by running `notion:fetch-all` twice with different `--max-pages` values.
10+
11+
## Preconditions
12+
- You are on PR branch `fix/issue-118-stable-order` (PR #125).
13+
- You have valid Notion env vars available (via `.env` or environment):
14+
- `NOTION_API_KEY`
15+
- `DATABASE_ID` or `NOTION_DATABASE_ID`
16+
- (optional) `DATA_SOURCE_ID`
17+
- (optional) `BASE_URL=/comapeo-docs/`
18+
19+
## Safety notes
20+
- These commands will generate content under `docs/`, `i18n/`, and `static/images/`. Do not commit generated content changes.
21+
- Prefer running this QA in a throwaway worktree.
22+
23+
## Step 1 — Install deps (if needed)
24+
```bash
25+
bun i
26+
```
27+
28+
## Step 2 — Script/unit verification
29+
```bash
30+
bunx vitest run scripts/fetchNotionData.test.ts scripts/notion-fetch/generateBlocks.test.ts
31+
```
32+
Expected: green.
33+
34+
## Step 3 — Baseline full-ish run (establish stable positions)
35+
Run a bigger batch to populate cache and write initial frontmatter.
36+
```bash
37+
rm -rf .cache/page-metadata.json 2>/dev/null || true
38+
bun run notion:fetch-all --force --max-pages 20
39+
```
40+
41+
Snapshot sidebar/category positions after the baseline:
42+
```bash
43+
rg -n \"^sidebar_position:\" docs i18n -S > /tmp/sidebar_positions.before.txt
44+
rg -n '\"position\"\\s*:' docs -S --glob \"**/_category_.json\" > /tmp/category_positions.before.txt
45+
```
46+
47+
## Step 4 — Partial run (simulate filtered sync)
48+
Run a smaller batch without `--force` (this simulates a filtered subset run where index-based fallbacks used to drift).
49+
```bash
50+
bun run notion:fetch-all --max-pages 5
51+
```
52+
53+
Snapshot again:
54+
```bash
55+
rg -n \"^sidebar_position:\" docs i18n -S > /tmp/sidebar_positions.after.txt
56+
rg -n '\"position\"\\s*:' docs -S --glob \"**/_category_.json\" > /tmp/category_positions.after.txt
57+
```
58+
59+
## Step 5 — Assertions (what must be true)
60+
1) **No sidebar reshuffle for existing pages missing `Order`:**
61+
```bash
62+
diff -u /tmp/sidebar_positions.before.txt /tmp/sidebar_positions.after.txt || true
63+
```
64+
Expected: either no diff, or only diffs attributable to *newly generated* files/pages in the smaller run (not re-numbering existing pages).
65+
66+
2) **No `_category_.json` reshuffle due to partial indexing:**
67+
```bash
68+
diff -u /tmp/category_positions.before.txt /tmp/category_positions.after.txt || true
69+
```
70+
Expected: no diff for existing categories.
71+
72+
3) **Git diff sanity check (generated content shouldn’t get reordered):**
73+
```bash
74+
git diff -- docs i18n static/images | rg -n \"sidebar_position|_category_\\.json|position\" -S || true
75+
```
76+
Expected: no “position churn” across existing files.
77+
78+
## Step 6 — Sub-page placement spot check (manual)
79+
In the logs of the partial run, confirm at least one case where a parent page and its sub-page(s) are processed consecutively (sub-pages immediately after parent). If logs are too noisy, spot-check output:
80+
- Pick a known parent doc and a sub-page doc.
81+
- Confirm their sidebar positions do not jump unexpectedly and that the sub-page appears directly under/near its parent in the sidebar for a local build (optional).
82+
83+
Optional local UI verification (only if requested):
84+
```bash
85+
bun run dev
86+
```
87+
88+
## Reporting back
89+
Post a short QA result in the PR:
90+
- ✅/❌ for steps 2–5
91+
- Paste any diffs from the `diff -u` checks (trimmed)
92+
- Mention any observed sidebar/category position churn
93+

scripts/fetchNotionData.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,55 @@ describe("fetchNotionData", () => {
718718
consoleWarnSpy.mockRestore();
719719
});
720720

721+
it("should insert sub-pages after parent and sort by Order when present", async () => {
722+
const data = [
723+
{
724+
id: "parent",
725+
url: "url",
726+
properties: {
727+
Order: { number: 1 },
728+
Title: { title: [{ plain_text: "Parent" }] },
729+
"Sub-item": { relation: [{ id: "sub1" }, { id: "sub2" }] },
730+
},
731+
},
732+
];
733+
734+
vi.mocked(enhancedNotion.pagesRetrieve)
735+
.mockResolvedValueOnce({ id: "sub1", properties: {} })
736+
.mockResolvedValueOnce({
737+
id: "sub2",
738+
properties: { Order: { number: 2 } },
739+
});
740+
741+
const result = await sortAndExpandNotionData(data);
742+
743+
expect(result.map((r) => r.id)).toEqual(["parent", "sub2", "sub1"]);
744+
});
745+
746+
it("should dedupe duplicate Sub-item relations", async () => {
747+
const data = [
748+
{
749+
id: "parent",
750+
url: "url",
751+
properties: {
752+
Title: { title: [{ plain_text: "Parent" }] },
753+
"Sub-item": {
754+
relation: [{ id: "sub1" }, { id: "sub1" }, { id: "sub2" }],
755+
},
756+
},
757+
},
758+
];
759+
760+
vi.mocked(enhancedNotion.pagesRetrieve).mockImplementation(
761+
async ({ page_id }) => ({ id: page_id, properties: {} })
762+
);
763+
764+
const result = await sortAndExpandNotionData(data);
765+
766+
expect(result.map((r) => r.id)).toEqual(["parent", "sub1", "sub2"]);
767+
expect(vi.mocked(enhancedNotion.pagesRetrieve)).toHaveBeenCalledTimes(2);
768+
});
769+
721770
it("should log progress every 10 items", async () => {
722771
const relations = Array.from({ length: 15 }, (_, i) => ({
723772
id: `sub${i}`,

scripts/fetchNotionData.ts

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export async function sortAndExpandNotionData(
126126
subId: string;
127127
parentTitle: string;
128128
}> = [];
129+
const seenRelations = new Set<string>();
129130

130131
for (const item of data) {
131132
const relations = item?.properties?.["Sub-item"]?.relation ?? [];
@@ -135,6 +136,11 @@ export async function sortAndExpandNotionData(
135136
"Unknown";
136137

137138
for (const rel of relations) {
139+
const key = `${String(item.id)}:${String(rel.id)}`;
140+
if (seenRelations.has(key)) {
141+
continue;
142+
}
143+
seenRelations.add(key);
138144
allRelations.push({
139145
parentId: item.id as string,
140146
subId: rel.id,
@@ -253,11 +259,61 @@ export async function sortAndExpandNotionData(
253259
`✅ Fetched ${subpages.length}/${allRelations.length} sub-pages (${successRate}%) in ${fetchDuration}s`
254260
);
255261

256-
// Add all fetched sub-pages to data array
262+
const subpageById = new Map<string, any>();
257263
for (const subpage of subpages) {
258-
data.push(subpage);
264+
if (subpage?.id) {
265+
subpageById.set(subpage.id, subpage);
266+
}
267+
}
268+
269+
const subpagesByParent = new Map<string, any[]>();
270+
for (const relation of allRelations) {
271+
const subpage = subpageById.get(relation.subId);
272+
if (!subpage) {
273+
continue;
274+
}
275+
const list = subpagesByParent.get(relation.parentId) ?? [];
276+
list.push(subpage);
277+
subpagesByParent.set(relation.parentId, list);
259278
}
260279

280+
const expanded: Array<Record<string, unknown>> = [];
281+
for (const parent of data) {
282+
expanded.push(parent);
283+
const list = subpagesByParent.get(parent.id as string);
284+
if (!list || list.length === 0) {
285+
continue;
286+
}
287+
288+
const sortedSubpages = list
289+
.map((subpage, index) => {
290+
const orderValue = subpage?.properties?.["Order"]?.number;
291+
return {
292+
subpage,
293+
index,
294+
orderValue,
295+
hasOrder: Number.isFinite(orderValue),
296+
};
297+
})
298+
.sort((a, b) => {
299+
if (a.hasOrder && b.hasOrder) {
300+
return a.orderValue - b.orderValue;
301+
}
302+
if (a.hasOrder) {
303+
return -1;
304+
}
305+
if (b.hasOrder) {
306+
return 1;
307+
}
308+
return a.index - b.index;
309+
})
310+
.map((entry) => entry.subpage);
311+
312+
expanded.push(...sortedSubpages);
313+
}
314+
315+
data = expanded;
316+
261317
data.forEach((item, index) => {
262318
// Use optional chaining in case url is missing
263319
console.log(`Item ${index + 1}:`, item?.url);

0 commit comments

Comments
 (0)