From 62ca571802678ef9985ee6ca42bd3caf2661843a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20T=C3=B6rcsv=C3=A1ri?= Date: Mon, 6 Jul 2026 08:35:18 +0200 Subject: [PATCH] =?UTF-8?q?feat(ysync):=20lib=5Fsymbols=20travel=20+=20lay?= =?UTF-8?q?out=20save-sync=20(miss=2008),=20TS=20hot-path=20opts=20(12),?= =?UTF-8?q?=20diff-on-rebind=20adopt=20(13)=20=E2=80=94=20doc=2018?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Miss 08A: the binding stores wire-carried (lib_symbols …) definitions in kdoc_libsymbols and prefixes them on apply wires — a joiner that never saw a symbol adopts it WITH its definition (new e2e ysync-libsymbols.spec.ts). Miss 08B: registerSaveHook gains onSavedText; WasmTool routes saved-file text to syncLayoutToY (per sheet room via the manager's syncLayoutFromSave, or the single-room doc) so title block / paper / setup edits converge instead of drifting. Opt 12 (TS half): zod off the observer hot path (yToItemUnchecked), children index built once per conversion. Opt 13: seed()'s adopt diffs the editor snapshot against the doc view and applies only the doc-authoritative difference — clean rebinds apply nothing, the adopt undo entry shrinks to the real changed set. Opt 14 deliberately deferred (doc 18). All TS-side; no wasm rebuild (the C++ blob/findLib sides already carried definitions). Verified: shared 107, standalone 79 (+2 known pre-existing wasm-assets), ysync e2e 21/21 chromium, collab regression 21/3-skip firefox. Bumps: web/pcbjam-shared (lib_symbols channel + syncLayoutToY + opts). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JgThWXtdvrYLK47EDFoGdq --- docs/features/ysync-review/00-overview.md | 1 + .../08-miss-layout-state-never-syncs.md | 2 +- .../12-opt-hot-path-full-model-work.md | 2 +- .../13-opt-parked-dirty-full-sheet-replace.md | 2 +- .../14-opt-item-granularity-bandwidth.md | 2 +- .../ysync-review/18-miss08-opts-12-13.md | 102 +++++++++ tests/kicad/ysync-libsymbols.spec.ts | 195 ++++++++++++++++++ web/pcbjam-shared | 2 +- web/standalone/src/components/WasmTool.tsx | 21 ++ .../src/wasm/collab/kicad-binding.test.ts | 97 +++++++++ .../src/wasm/collab/kicad-binding.ts | 95 +++++++-- .../src/wasm/collab/sheet-manager.ts | 24 ++- web/standalone/src/wasm/save-flow.test.ts | 35 ++++ web/standalone/src/wasm/save-flow.ts | 17 ++ 14 files changed, 570 insertions(+), 27 deletions(-) create mode 100644 docs/features/ysync-review/18-miss08-opts-12-13.md create mode 100644 tests/kicad/ysync-libsymbols.spec.ts diff --git a/docs/features/ysync-review/00-overview.md b/docs/features/ysync-review/00-overview.md index fd4c427..4496a0c 100644 --- a/docs/features/ysync-review/00-overview.md +++ b/docs/features/ysync-review/00-overview.md @@ -93,3 +93,4 @@ sending half emits NOTHING, not the bare removal the doc predicted). | 15 | [15-plan-repro-tests-and-v2-e2e.md](15-plan-repro-tests-and-v2-e2e.md) | The approved plan: repro tests for bugs 01–07 + the v2 e2e port | | 16 | [16-repro-suite-results-and-empirical-findings.md](16-repro-suite-results-and-empirical-findings.md) | Plan 15 executed (2026-07-03): suite map, verified failure sites, empirical findings F1–F4 | | 17 | [17-fixes-bugs-01-07.md](17-fixes-bugs-01-07.md) | Bugs 01–07 fixed & verified (2026-07-03); findings F5–F6; remaining follow-ups | +| 18 | [18-miss08-opts-12-13.md](18-miss08-opts-12-13.md) | Miss 08 (lib_symbols + layout save-sync) + opts 12/13 implemented; opt 14 deferred (2026-07-03) | diff --git a/docs/features/ysync-review/08-miss-layout-state-never-syncs.md b/docs/features/ysync-review/08-miss-layout-state-never-syncs.md index 6649c88..1ee0fc0 100644 --- a/docs/features/ysync-review/08-miss-layout-state-never-syncs.md +++ b/docs/features/ysync-review/08-miss-layout-state-never-syncs.md @@ -1,7 +1,7 @@ # Design miss 08 — Non-item document state only syncs at seed; `lib_symbols` is a landmine for the symbol-libraries milestone **Severity:** design gap (silent divergence for settings edits; structural blocker later) -**Status:** open decision +**Status:** IMPLEMENTED 2026-07-03 — see [18](18-miss08-opts-12-13.md) (lib_symbols channel + coarse layout save-sync; nets stay seed-frozen by design) ## Where diff --git a/docs/features/ysync-review/12-opt-hot-path-full-model-work.md b/docs/features/ysync-review/12-opt-hot-path-full-model-work.md index 955cded..ae69085 100644 --- a/docs/features/ysync-review/12-opt-hot-path-full-model-work.md +++ b/docs/features/ysync-review/12-opt-hot-path-full-model-work.md @@ -1,7 +1,7 @@ # Optimization 12 — O(full-model) work on every edit, apply, and remote batch **Severity:** performance (fine on demo boards; seconds-per-edit territory at 5–10k items) -**Status:** open +**Status:** TS items DONE 2026-07-03 (zod off the observer path, children index once — see [18](18-miss08-opts-12-13.md)); C++ item 1 delivered by [17](17-fixes-bugs-01-07.md) batch 3 except the scalar-scan/legacy-wire retirement (goes with the legacy-wire removal) ## The costs, per hot path diff --git a/docs/features/ysync-review/13-opt-parked-dirty-full-sheet-replace.md b/docs/features/ysync-review/13-opt-parked-dirty-full-sheet-replace.md index 52c5c85..d335f95 100644 --- a/docs/features/ysync-review/13-opt-parked-dirty-full-sheet-replace.md +++ b/docs/features/ysync-review/13-opt-parked-dirty-full-sheet-replace.md @@ -1,7 +1,7 @@ # Optimization 13 — Parked-dirty sheet rebind re-applies the entire sheet instead of the delta **Severity:** performance + UX (heavy on big sheets; creates the adopt undo-bomb) -**Status:** open +**Status:** FIXED 2026-07-03 — option 2 (diff on rebind) — see [18](18-miss08-opts-12-13.md) ## Where diff --git a/docs/features/ysync-review/14-opt-item-granularity-bandwidth.md b/docs/features/ysync-review/14-opt-item-granularity-bandwidth.md index a40fdad..a5ff5e2 100644 --- a/docs/features/ysync-review/14-opt-item-granularity-bandwidth.md +++ b/docs/features/ysync-review/14-opt-item-granularity-bandwidth.md @@ -1,7 +1,7 @@ # Optimization 14 — Item-level body granularity: whole-item payloads per nudge, LWW drops concurrent property edits **Severity:** known/documented v1 tradeoff — recorded here so its costs are visible when prioritizing -**Status:** open (deliberate design decision; revisit trigger below) +**Status:** open (deliberate design decision; revisit trigger below — reaffirmed 2026-07-03, see [18](18-miss08-opts-12-13.md)) ## Where diff --git a/docs/features/ysync-review/18-miss08-opts-12-13.md b/docs/features/ysync-review/18-miss08-opts-12-13.md new file mode 100644 index 0000000..0a35add --- /dev/null +++ b/docs/features/ysync-review/18-miss08-opts-12-13.md @@ -0,0 +1,102 @@ +# Miss 08 + opts 12/13 implemented; opt 14 deliberately deferred (2026-07-03) + +**Status:** miss [08](08-miss-layout-state-never-syncs.md) (both halves), opt +[12](12-opt-hot-path-full-model-work.md)'s TS items, and opt +[13](13-opt-parked-dirty-full-sheet-replace.md) implemented and verified. All +TS/shared-side — the C++ already did its part (eeschema's `itemBlob` emits the +`lib_symbols` context; `doApplyItems`' `findLib` was designed to prefer a +blob-carried definition), so no WASM rebuild was needed. + +## 08A — lib_symbols channel + +- New `kdoc_libsymbols` Y.Map (lib id → `(symbol …)` definition text): concurrent + placements of DIFFERENT symbols merge per definition instead of LWW-clobbering + one layout slot. +- `docToY` extracts the layout's `(lib_symbols …)` definitions into the map and + keeps the layout slot EMPTY (the injection point); `yToDoc` re-injects the + map's definitions sorted by lib id — the order KiCad's own writer emits, so + materialization matches editor saves. +- Emit direction: `wireLibSymbols(wire)` recovers definitions from the + multi-form clipboard blob (`unwrapWireItem` still strips them from the ITEM + payload); the binding stores them in the same transaction as the item delta. +- Apply direction: `deltaToItemsWire(delta, view, libDefs)` prefixes a root + item carrying `(lib_id …)` with its definition — exactly the multi-form shape + eeschema's paste path parses, so `findLib`'s first branch resolves it and a + peer that has never seen the symbol renders it correctly. +- e2e: `tests/kicad/ysync-libsymbols.spec.ts` — a joiner whose cold copy lacks + the symbol adopts it WITH its definition (chromium two-tab; F2 applies). + +## 08B — coarse layout save-sync + +- New shared `syncLayoutToY(fileDoc, ydoc, origin)`: reconciles non-item layout + slots per HEAD KEYWORD group (title block, paper, setup, settings…) from a + just-saved file — a changed group is replaced wholesale (LWW at head + granularity; different heads merge). `{item}` slots are never touched. +- Deliberate freezes: `net` (pcbnew's root net table — net-creating edits are + not possible in the standalone; repeated positional heads can't be reconciled + by name) stays seed-frozen; `lib_symbols` routes to the defs map ADDITIVELY + (a save can't know about a peer's not-yet-applied placement). +- Wiring: `registerSaveHook` gained `onSavedText` (decoded saved text); + WasmTool routes it — eeschema per sheet via the manager's new + `syncLayoutFromSave` (parked rooms sync too; the dirty mark it causes costs + only the real delta now, see opt 13), pcbnew/pl_editor via the single-room + doc. +- Known limitation (unchanged from the doc's "coarse but converging"): a peer's + EDITOR does not live-update non-item state — the room doc converges, so + materialization/reopen and drift-detect see it; live propagation into an open + editor's title block would need a C++ apply for non-item state. + +## Opt 12 — TS hot-path (items 2 + 4) + +- `yToItemUnchecked` (no zod) feeds the binding's `itemsView()` — both hot + directions (every local emit, every remote batch). zod stays at the trust + boundaries: wire parse, seed, materialize. +- `itemsWireToDelta` builds the parent→children index ONCE per conversion + (was: full-scan `descendants()` per wire item and per removed id). +- NOT taken from doc 12: retiring the C++ scalar scan + legacy emit (the scan + still feeds the legacy wire and covers eeschema's unreported post-Push + connectivity cleanup — retire it together with the legacy-wire removal, + miss 11 follow-up), and the lazy `deltaToItemsWire` view (item 3) — killing + zod removed the dominant cost; revisit if profiling says otherwise. + +## Opt 13 — diff-on-rebind adopt (option 2) + +`seed()`'s adopt no longer renders and re-applies EVERY doc root. It diffs the +editor snapshot against the doc view (`itemsWireToDelta` + `wireItemUuids`) and +applies only the doc-authoritative difference: + +- doc-only ROOTS → added (their sexprs embed descendants; a doc-only child makes + its shared parent differ and rides the parent's re-apply); +- differing items → the DOC's version re-applied, lifted to their root (the C++ + upsert replaces roots; a bare child apply would mis-parent); +- editor-only ROOTS → removed (editor-only children vanish with their parent's + re-apply). + +An empty diff degrades to baseline-only — the parked-dirty rebind, the clean +revisit, and the cold divergent adopt now share one code path, and the adopt's +single commit (and undo entry — the miss-09 "adopt undo bomb") shrinks to the +real changed set. + +## Opt 14 — deferred, on purpose + +The slot-level CRDT refinement is a meaningful rewrite of `kicad-y`'s write path +plus the whole conversion layer, with real interleaving-anomaly risk. Doc 14's +own revisit triggers (measured bandwidth/storage cost; concurrent same-item UX +complaints) are unmet, and the flatten already gives per-child granularity. +Nothing in this batch forecloses it — `body` stayed one JSON value. + +## Verification + +- pcbjam-shared: 107 (9 new in `test/layout-libsymbols-sync.test.ts`) +- standalone: 79 + 2 known pre-existing wasm-assets fixture failures + (collab suite 42, incl. 3 new diff-adopt + 2 lib_symbols binding tests; + save-flow +2) +- e2e (chromium): full ysync set + `ysync-libsymbols.spec.ts` green +- No C++ changes; no WASM rebuild. + +## Remaining from the review + +Misses [09](09-miss-undo-not-collab-aware.md) (collab-aware undo — the adopt +shrink here removes its worst case) and [10](10-miss-no-repair-path.md) (drift +repair), opt 12's C++ retirement half (with the legacy-wire removal), opt 14 +(triggers above), and doc 17's carried-over follow-ups. diff --git a/tests/kicad/ysync-libsymbols.spec.ts b/tests/kicad/ysync-libsymbols.spec.ts new file mode 100644 index 0000000..74a57e4 --- /dev/null +++ b/tests/kicad/ysync-libsymbols.spec.ts @@ -0,0 +1,195 @@ +import * as path from "path"; +import type { Page } from "@playwright/test"; +import { test, expect } from "./fixtures"; + +/** + * Miss 08A e2e — embedded library definitions travel the v2 wire. + * + * A schematic symbol is only renderable with its `(lib_symbols …)` definition. + * The TS layer used to strip that envelope ("sender context") so a peer that + * had never seen the symbol received a definition-less instance and the room's + * materialization referenced a lib id it didn't contain. Now definitions live + * in `kdoc_libsymbols` and are re-prefixed on every apply wire. + * + * Two kicad_editor instances → Firefox's per-process wasm budget (finding F2): + * chromium only, like the other two-tab specs. + */ + +const SYM1 = "44444444-0000-0000-0000-000000000001"; +const WIRE1 = "22222222-0000-0000-0000-000000000001"; + +const SCH_WITH_SYMBOL = `(kicad_sch +\t(version 20250114) +\t(generator "eeschema") +\t(generator_version "9.0") +\t(uuid "11111111-1111-1111-1111-111111111111") +\t(paper "A4") +\t(lib_symbols +\t\t(symbol "Device:R" (pin_numbers (hide yes)) (pin_names (offset 0)) (exclude_from_sim no) (in_bom yes) (on_board yes) +\t\t\t(property "Reference" "R" (at 2.032 0 90) (effects (font (size 1.27 1.27)))) +\t\t\t(property "Value" "R" (at 0 0 90) (effects (font (size 1.27 1.27)))) +\t\t\t(symbol "R_0_1" +\t\t\t\t(rectangle (start -1.016 -2.54) (end 1.016 2.54) (stroke (width 0.254) (type default)) (fill (type none))) +\t\t\t) +\t\t\t(symbol "R_1_1" +\t\t\t\t(pin passive line (at 0 3.81 270) (length 1.27) (name "~" (effects (font (size 1.27 1.27)))) (number "1" (effects (font (size 1.27 1.27))))) +\t\t\t\t(pin passive line (at 0 -3.81 90) (length 1.27) (name "~" (effects (font (size 1.27 1.27)))) (number "2" (effects (font (size 1.27 1.27))))) +\t\t\t) +\t\t) +\t) +\t(wire (pts (xy 50.8 50.8) (xy 101.6 50.8)) (stroke (width 0) (type default)) (uuid "${WIRE1}")) +\t(symbol (lib_id "Device:R") (at 63.5 63.5 0) (unit 1) (exclude_from_sim no) (in_bom yes) (on_board yes) (dnp no) +\t\t(uuid "${SYM1}") +\t\t(property "Reference" "R1" (at 66.04 62.23 0) (effects (font (size 1.27 1.27)) (justify left))) +\t\t(property "Value" "10k" (at 66.04 64.77 0) (effects (font (size 1.27 1.27)) (justify left))) +\t\t(property "Footprint" "" (at 0 0 0) (effects (font (size 1.27 1.27)) (hide yes))) +\t\t(property "Datasheet" "" (at 0 0 0) (effects (font (size 1.27 1.27)) (hide yes))) +\t\t(pin "1" (uuid "44444444-0000-0000-0000-0000000000a1")) +\t\t(pin "2" (uuid "44444444-0000-0000-0000-0000000000a2")) +\t\t(instances (project "rt" (path "/11111111-1111-1111-1111-111111111111" (reference "R1") (unit 1)))) +\t) +\t(sheet_instances (path "/" (page "1"))) +) +`; + +// B's cold copy: the same sheet BEFORE the symbol was placed — no instance, no +// definition. The adopt must deliver both. +const SCH_WITHOUT_SYMBOL = `(kicad_sch +\t(version 20250114) +\t(generator "eeschema") +\t(generator_version "9.0") +\t(uuid "11111111-1111-1111-1111-111111111111") +\t(paper "A4") +\t(lib_symbols) +\t(wire (pts (xy 50.8 50.8) (xy 101.6 50.8)) (stroke (width 0) (type default)) (uuid "${WIRE1}")) +\t(sheet_instances (path "/" (page "1"))) +) +`; + +type FS = { + mkdirTree(p: string): void; + writeFile(p: string, d: string): void; + readFile(p: string, o: { encoding: "utf8" }): string; +}; +type Mod = { + kicadOpenFile(p: string): unknown; + kicadCollabSnapshotItems(): string; + kicadCollabApplyItems(j: string): unknown; + kicadSaveSchematic(p: string): unknown; +}; + +const BOOT_TIMEOUT = 150000; +const BUNDLE = path.resolve(__dirname, "../apps/kicad/collab-bundle-v2.js"); + +function hasAbort(l: { consoleLogs: string[]; errors: string[] }): boolean { + return [...l.consoleLogs, ...l.errors].some((s) => s.includes("Aborted(")); +} + +async function bootOpen(page: Page, content: string, name: string): Promise { + await page.goto("/kicad/eeschema.html"); + await expect(page.locator("#canvas")).toBeVisible({ timeout: BOOT_TIMEOUT }); + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Mod }).Module; + return ( + !!m && + typeof m.kicadOpenFile === "function" && + typeof m.kicadCollabSnapshotItems === "function" && + typeof m.kicadSaveSchematic === "function" + ); + }, + null, + { timeout: BOOT_TIMEOUT }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || "").endsWith("Frame")), + null, + { timeout: BOOT_TIMEOUT }, + ); + await page.evaluate( + ({ content, name }) => { + const w = window as unknown as { FS: FS; Module: Mod }; + try { + w.FS.mkdirTree("/home/kicad/documents"); + } catch { + /* exists */ + } + const p = `/home/kicad/documents/${name}.kicad_sch`; + w.FS.writeFile(p, content); + w.Module.kicadOpenFile(p); + }, + { content, name }, + ); + await expect.poll(() => page.title(), { timeout: 30000 }).toMatch(new RegExp(name, "i")); + await page.addScriptTag({ path: BUNDLE }); +} + +function startV2( + page: Page, + opts: { room: string; settleMs?: number; seedText?: string }, +): Promise { + return page.evaluate(async (o) => { + const w = window as unknown as { + KicadCollabV2: { start: (m: unknown, win: unknown, o: unknown) => Promise }; + Module: unknown; + }; + await w.KicadCollabV2.start(w.Module, window, o); + }, opts); +} + +function saveText(page: Page): Promise { + return page.evaluate(() => { + const w = window as unknown as { FS: FS; Module: Mod }; + const out = "/home/kicad/documents/_dump.kicad_sch"; + w.Module.kicadSaveSchematic(out); + return w.FS.readFile(out, { encoding: "utf8" }); + }); +} + +test.describe("v2 items wire — lib_symbols travel (miss 08A)", () => { + test.describe.configure({ timeout: 420000 }); + + test("a joiner that never saw the symbol adopts it WITH its definition", async ({ + context, + testLogger, + }) => { + test.skip( + test.info().project.name === "firefox", + "two kicad_editor tabs exceed Firefox's per-process wasm budget", + ); + + const room = `ysync-libsym-${test.info().workerIndex}`; + const tabA = await context.newPage(); + const tabB = await context.newPage(); + await bootOpen(tabA, SCH_WITH_SYMBOL, "tabA"); + await bootOpen(tabB, SCH_WITHOUT_SYMBOL, "tabB"); + + await startV2(tabA, { room, seedText: SCH_WITH_SYMBOL }); // fresh → file-seed + await startV2(tabB, { room }); // joins → diff-adopt (doc authority) + + // The adopt delivered the INSTANCE… + await expect + .poll(async () => (await saveText(tabB)).includes(SYM1), { + timeout: 20000, + intervals: [400], + }) + .toBe(true); + // …AND its DEFINITION (findLib resolved it from the wire's lib_symbols + // context; before miss 08A the peer kept an empty lib_symbols and the + // instance rendered broken / materialized as an invalid file). + const saved = await saveText(tabB); + expect(saved).toContain(`(symbol "Device:R"`); + expect(saved).toContain(`(lib_id "Device:R")`); + + // The untouched wire survived the diff-adopt (only the difference applied). + expect(saved).toContain(WIRE1); + + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + await tabA.close(); + await tabB.close(); + }); +}); diff --git a/web/pcbjam-shared b/web/pcbjam-shared index f12d9af..2387cec 160000 --- a/web/pcbjam-shared +++ b/web/pcbjam-shared @@ -1 +1 @@ -Subproject commit f12d9af9595c915bb17a31bb59d156d38b256cbf +Subproject commit 2387cecba80221e463fde02636d9ce4c3c24a044 diff --git a/web/standalone/src/components/WasmTool.tsx b/web/standalone/src/components/WasmTool.tsx index 259ec30..c84d1bd 100644 --- a/web/standalone/src/components/WasmTool.tsx +++ b/web/standalone/src/components/WasmTool.tsx @@ -9,6 +9,7 @@ import { projectToolPath, toolSchema, ydocHasState, + syncLayoutToY, yToDoc, type KicadDoc, type Tool, @@ -570,6 +571,9 @@ export function WasmTool({ const startedRef = React.useRef(false); const driftRef = React.useRef<{ stop(): void } | null>(null); const sheetManagerRef = React.useRef(null); + // The single-room collab doc (pcbnew/pl_editor), for the layout save-sync + // (miss 08B); eeschema routes per sheet through the manager instead. + const collabDocRef = React.useRef(null); const [status, setStatus] = React.useState("Loading tool…"); const [logs, setLogs] = React.useState([]); const [showLog, setShowLog] = React.useState(false); @@ -795,6 +799,21 @@ export function WasmTool({ onSaved: (relPath) => { if (relPath.endsWith(".kicad_sch")) void sheetManagerRef.current?.onboard(relPath); }, + // Non-item document state (title block, paper, setup…) only reaches the + // room at seed time; reconcile it from every save (miss 08B). + onSavedText: (relPath, text) => { + if (sheetManagerRef.current) { + sheetManagerRef.current.syncLayoutFromSave(relPath, text); + return; + } + if (collabDocRef.current && relPath === targetPath) { + try { + syncLayoutToY(fileToDoc(text), collabDocRef.current, "layout-save"); + } catch (err) { + append(`[save] layout sync failed: ${String(err)}`); + } + } + }, }); const { session, targetBytes } = await maybeConnectDocSession(win, { docSource, @@ -865,6 +884,7 @@ export function WasmTool({ log: append, onStatus: setStatus, }); + collabDocRef.current = collabHandle?.doc ?? null; if (collabHandle && targetPath && COLLAB_TOOLS.has(tool)) { driftRef.current = startDriftDetection({ doc: collabHandle.doc, @@ -897,6 +917,7 @@ export function WasmTool({ // onActiveChange(null). sheetManagerRef.current?.destroy(); sheetManagerRef.current = null; + collabDocRef.current = null; oom.stop(); }; // Boot is one-shot per mount; deps intentionally exclude files/targetPath so diff --git a/web/standalone/src/wasm/collab/kicad-binding.test.ts b/web/standalone/src/wasm/collab/kicad-binding.test.ts index a848705..6758c45 100644 --- a/web/standalone/src/wasm/collab/kicad-binding.test.ts +++ b/web/standalone/src/wasm/collab/kicad-binding.test.ts @@ -4,6 +4,7 @@ import { docToFile, fileToDoc, itemsWireToDelta, + kicadLibSymbolsMap, parseItemsWireDelta, renderItem, sexprToItems, @@ -244,4 +245,100 @@ describe("bindKicadCollab — two editors over relayed Y.Docs", () => { sexprToItems(`(pad "1" smd (at 0 0) (uuid "pad-1"))`, "fp-1").items["pad-1"]!.body, ); }); + + it("adopt applies only the DIFFERENCE (opt 13) — identical items cost nothing", () => { + const { edA, edB, bindA, bindB } = setup(); + const SEG = `(segment (start 0 0) (end 1 1) (uuid "seg-1"))`; + seedEditor(edA, FP); + seedEditor(edA, SEG); + bindA.seed(); + + // B's editor already holds the IDENTICAL footprint but not the segment. + seedEditor(edB, FP); + bindB.seed(); + + expect(edB.store["seg-1"]).toBeDefined(); // caught up + expect(edB.applied).toHaveLength(1); + const wire = parseItemsWireDelta(edB.applied[0]!); + // Only the missing segment travelled — the matching footprint did not. + expect(wire.added).toHaveLength(1); + expect(wire.added[0]!.sexpr).toContain("seg-1"); + expect(wire.changed).toHaveLength(0); + expect(wire.removed).toHaveLength(0); + }); + + it("adopt with a fully matching editor applies NOTHING (clean rebind)", () => { + const { edA, edB, bindA, bindB } = setup(); + seedEditor(edA, FP); + bindA.seed(); + seedEditor(edB, FP); + bindB.seed(); + expect(edB.applied).toHaveLength(0); + }); + + it("adopt re-applies a differing item's DOC version, lifted to its root", () => { + const { edA, edB, bindA, bindB } = setup(); + seedEditor(edA, FP); + bindA.seed(); + + // B holds the same footprint but its pad drifted (never-synced local state). + seedEditor(edB, FP.replace(`(pad "1" smd (at 0 0)`, `(pad "1" smd (at 9 9)`)); + bindB.seed(); + + // Doc authority: B's editor converges on the doc's pad, via ONE root re-apply. + expect(edB.store["pad-1"]!.body).toEqual( + sexprToItems(`(pad "1" smd (at 0 0) (uuid "pad-1"))`, "fp-1").items["pad-1"]!.body, + ); + expect(edB.applied).toHaveLength(1); + const wire = parseItemsWireDelta(edB.applied[0]!); + expect(wire.changed).toHaveLength(1); + expect(wire.changed[0]!.sexpr).toContain(`(uuid "fp-1")`); // the root, not the bare pad + }); +}); + +describe("lib_symbols flow through the binding (miss 08A)", () => { + const DEF = `(symbol "Device:R" (property "Reference" "R" (at 2 0 90)))`; + const INSTANCE = `(symbol (lib_id "Device:R") (at 100 50 0) (uuid "sym-1"))`; + + it("an emitted placement's definition is stored and re-rendered for the peer", () => { + const { a, b } = pair(); + const edA = new FakeEditor(); + const edB = new FakeEditor(); + bindKicadCollab(a, edA).seed(); + bindKicadCollab(b, edB).seed(); + + // A places a symbol: the eeschema blob is multi-form (definition + instance). + edA.localUpsert(`(lib_symbols ${DEF}) ${INSTANCE}`, null, "added"); + + // B's editor received the instance WITH its definition prefixed (findLib's + // first branch), even though B has never seen this symbol. + expect(edB.store["sym-1"]).toBeDefined(); + const applied = edB.applied.map((j) => parseItemsWireDelta(j)); + const symWire = applied + .flatMap((w) => [...w.added, ...w.changed]) + .find((w) => w.sexpr.includes("sym-1")); + expect(symWire, "the symbol reached B").toBeTruthy(); + expect(symWire!.sexpr).toMatch(/^\(lib_symbols \(symbol "Device:R"/); + + // And the definition landed in the room's defs map on BOTH sides + // (materialization injection is covered by the shared-lib tests — this + // room was editor-snapshot-seeded, which carries no layout/meta). + expect(kicadLibSymbolsMap(b).get("Device:R")).toContain(`"Device:R"`); + expect(kicadLibSymbolsMap(a).get("Device:R")).toContain(`"Device:R"`); + }); + + it("adopt of a doc holding a symbol carries the definition too", () => { + const { a, b } = pair(); + const edA = new FakeEditor(); + const edB = new FakeEditor(); + bindKicadCollab(a, edA).seed(); + edA.localUpsert(`(lib_symbols ${DEF}) ${INSTANCE}`, null, "added"); + + // B joins with an empty editor → adopts the doc. + bindKicadCollab(b, edB).seed(); + expect(edB.store["sym-1"]).toBeDefined(); + const wire = parseItemsWireDelta(edB.applied[0]!); + const symWire = [...wire.added, ...wire.changed].find((w) => w.sexpr.includes("sym-1")); + expect(symWire!.sexpr).toMatch(/^\(lib_symbols \(symbol "Device:R"/); + }); }); diff --git a/web/standalone/src/wasm/collab/kicad-binding.ts b/web/standalone/src/wasm/collab/kicad-binding.ts index 402029f..bca3df5 100644 --- a/web/standalone/src/wasm/collab/kicad-binding.ts +++ b/web/standalone/src/wasm/collab/kicad-binding.ts @@ -7,13 +7,16 @@ import { isEmptyKicadDelta, itemsWireToDelta, kicadItemsMap, + kicadLibSymbolsMap, parseItemsWireDelta, - renderItem, seedDocToY, + upsertLibSymbolsToY, + wireItemUuids, + wireLibSymbols, Y_KDOC_META, Y_KDOC_SEED_NONCE, ydocHasState, - yToItem, + yToItemUnchecked, type ItemsWireDelta, type KicadDoc, type KicadItem, @@ -87,15 +90,24 @@ export function bindKicadCollab(doc: Y.Doc, bridge: KicadItemsBridge): KicadBind // Concurrent double-seed arbitration cleanup (bug 06); set by the file-seed branch. let detachSeedArbitration: (() => void) | undefined; - /** Plain snapshot of the Y items (the `current`/`view` the conversions need). */ + /** + * Plain snapshot of the Y items (the `current`/`view` the conversions need). + * Unchecked reads (opt 12): this runs on every local emit AND every remote + * batch; the zod walk of each body tree dominated at scale. The wire parse + * zod-validates at the trust boundary; seed/materialize keep checked reads. + */ const itemsView = (): Record => { const view: Record = {}; items.forEach((ym, uuid) => { - view[uuid] = yToItem(ym); + view[uuid] = yToItemUnchecked(ym); }); return view; }; + /** kdoc_libsymbols reader for the apply direction (miss 08). */ + const libDefs = (libId: string): string | undefined => + kicadLibSymbolsMap(doc).get(libId); + // DOWN: local editor change → Y.Doc bridge.onItems((json: string) => { if (destroyed) return; // stale hook (bug 07) — a destroyed binding is inert @@ -107,13 +119,19 @@ export function bindKicadCollab(doc: Y.Doc, bridge: KicadItemsBridge): KicadBind return; } const delta = itemsWireToDelta(wire, itemsView()); - if (isEmptyKicadDelta(delta)) return; + // Library definitions the blob carried (a placed symbol's lib_symbols + // context — miss 08): store them alongside the items, same transaction. + const defs = wireLibSymbols(wire); + if (isEmptyKicadDelta(delta) && Object.keys(defs).length === 0) return; clog("⬇ onItems (local edit):", { added: delta.added.length, updated: delta.updated.length, removed: delta.removed.length, }); - applyDeltaToY(doc, delta, ORIGIN); + doc.transact(() => { + applyDeltaToY(doc, delta, ORIGIN); + upsertLibSymbolsToY(doc, defs, ORIGIN); + }, ORIGIN); }); // UP: remote Y change → editor. The subscription + origin policy live HERE @@ -123,7 +141,7 @@ export function bindKicadCollab(doc: Y.Doc, bridge: KicadItemsBridge): KicadBind if (!seeded) return; // pre-seed state sync — seed()'s adopt covers it const delta = deltaFromYEvents(items, events); if (isEmptyKicadDelta(delta)) return; - const wire = deltaToItemsWire(delta, itemsView()); + const wire = deltaToItemsWire(delta, itemsView(), libDefs); if (isEmptyItemsWireDelta(wire)) return; clog("⬆ remote Y change → apply to editor:", { added: wire.added.length, @@ -203,32 +221,69 @@ export function bindKicadCollab(doc: Y.Doc, bridge: KicadItemsBridge): KicadBind cwarn("seed: snapshotItems unparseable", err); return; } - const local = itemsWireToDelta(wire, {}); const hasState = ydocHasState(doc); - clog( - `seed: doc has ${items.size} item(s), editor has ${local.added.length} →`, - hasState ? "ADOPTING doc (joining)" : "SEEDING doc (first tab)", - ); if (!hasState) { // First tab, no file source: seed the shared doc from the editor model. - applyDeltaToY(doc, local, ORIGIN); + const local = itemsWireToDelta(wire, {}); + clog(`seed: doc empty → SEEDING from editor snapshot (${local.added.length} item(s))`); + doc.transact(() => { + applyDeltaToY(doc, local, ORIGIN); + upsertLibSymbolsToY(doc, wireLibSymbols(wire), ORIGIN); + }, ORIGIN); return; } // Joining a populated doc: the editor adopts it (seed-once authority, same // rationale as the scalar reconciler §2 — divergent local uuids from a - // never-saved cold open must yield to the doc's identity). Apply the doc's - // ROOT items (their sexprs embed all descendants) and remove local-only roots. + // never-saved cold open must yield to the doc's identity). Diff the editor + // snapshot against the doc VIEW and apply only the DIFFERENCE (opt 13): + // identical items cost nothing, the apply commit (and its undo entry — the + // adopt undo-bomb, miss 09) shrinks to the real changed set, and a clean + // rebind degrades to baseline-only. const view = itemsView(); - const docRoots = Object.entries(view) - .filter(([, item]) => item.parent === null) - .map(([uuid]) => ({ sexpr: renderItem({ items: view }, uuid), parent: null })); - const removed = local.added + const editorDelta = itemsWireToDelta(wire, view); // editor state vs doc view + const editorUuids = wireItemUuids(wire); + + // Doc authority, inverted per class: + // - doc-only ROOTS → add to the editor (their sexprs embed descendants; + // a doc-only CHILD makes its shared parent's body differ → covered below); + // - items that DIFFER → re-apply the doc's version, lifted to their root + // (the C++ upsert replaces roots; a bare child apply would mis-parent); + // - editor-only ROOTS → remove (editor-only children disappear with their + // parent's re-apply). + const liftToRoot = (uuid: string): string => { + let cur = uuid; + while (view[cur]?.parent != null) cur = view[cur]!.parent!; + return cur; + }; + const docOnly = Object.entries(view) + .filter(([uuid, it]) => it.parent === null && !editorUuids.has(uuid)) + .map(([uuid, it]) => ({ uuid, ...it })); + const changedRoots = [ + ...new Set( + editorDelta.updated.filter((it) => it.uuid in view).map((it) => liftToRoot(it.uuid)), + ), + ] + .filter((uuid) => !docOnly.some((it) => it.uuid === uuid)) + .map((uuid) => ({ uuid, ...view[uuid]! })); + const removed = editorDelta.added .filter((it) => it.parent === null && !(it.uuid in view)) .map((it) => it.uuid); - bridge.applyItems(JSON.stringify({ added: docRoots, changed: [], removed })); + + const adoptWire = deltaToItemsWire( + { added: docOnly, updated: changedRoots, removed }, + view, + libDefs, + ); + + clog( + `seed: doc has ${items.size} item(s) → ADOPTING diff:`, + `+${adoptWire.added.length} ~${adoptWire.changed.length} -${adoptWire.removed.length}`, + ); + if (isEmptyItemsWireDelta(adoptWire)) return; // editor already matches — baseline only + bridge.applyItems(JSON.stringify(adoptWire)); } return { diff --git a/web/standalone/src/wasm/collab/sheet-manager.ts b/web/standalone/src/wasm/collab/sheet-manager.ts index f99bf81..0d45411 100644 --- a/web/standalone/src/wasm/collab/sheet-manager.ts +++ b/web/standalone/src/wasm/collab/sheet-manager.ts @@ -1,5 +1,5 @@ import type * as Y from "yjs"; -import { collabRoomId, type KicadDoc } from "@pcbjam/shared"; +import { collabRoomId, fileToDoc, syncLayoutToY, type KicadDoc } from "@pcbjam/shared"; import { connectKicadDoc, type KicadDocSession } from "./index"; import { bindKicadCollab, @@ -40,6 +40,12 @@ export interface SheetCollabManager { switchTo(sheetPath: string): Promise; /** Warm a sheet created mid-session (driven by the save hook on an unknown path). */ onboard(sheetPath: string): Promise; + /** + * Coarse non-item layout sync from a just-saved sheet file (miss 08B): title + * block / paper / settings edits reconcile into the sheet's room doc, which + * otherwise only carries them from seed time. No-op for unknown sheets. + */ + syncLayoutFromSave(sheetPath: string, fileText: string): void; /** The currently-bound sheet, for drift-detection wiring (null before first switch). */ active(): { sheetPath: string; doc: Y.Doc } | null; /** Tear down ALL bindings + providers + docs (session end / unmount). */ @@ -251,6 +257,20 @@ export function createSheetCollabManager(opts: SheetManagerOptions): SheetCollab } } + function syncLayoutFromSave(sheetPath: string, fileText: string): void { + const room = rooms.get(sheetPath); + if (!room) return; // not a collab sheet (or still onboarding) — nothing to sync + try { + // Writing to a PARKED room's doc marks it dirty via startWatch — fine: + // the diff-on-rebind adopt makes the catch-up cost the real delta only. + if (syncLayoutToY(fileToDoc(fileText), room.doc, "layout-save")) { + clog(`[sheet] layout save-sync: ${sheetPath} updated`); + } + } catch (err) { + cwarn(`[sheet] layout save-sync failed for ${sheetPath}`, err); + } + } + async function connectAll(sheetPaths: string[]): Promise { await Promise.all( sheetPaths.map((p) => @@ -289,7 +309,7 @@ export function createSheetCollabManager(opts: SheetManagerOptions): SheetCollab opts.onActiveChange?.(null); } - return { connectAll, switchTo, onboard, active, destroy }; + return { connectAll, switchTo, onboard, syncLayoutFromSave, active, destroy }; } export interface SheetChangedWindow { diff --git a/web/standalone/src/wasm/save-flow.test.ts b/web/standalone/src/wasm/save-flow.test.ts index 56097a1..fb3f7e8 100644 --- a/web/standalone/src/wasm/save-flow.test.ts +++ b/web/standalone/src/wasm/save-flow.test.ts @@ -45,4 +45,39 @@ describe("registerSaveHook path routing", () => { expect(onSaved).not.toHaveBeenCalled(); expect(saveBytes).not.toHaveBeenCalled(); }); + + it("onSavedText receives the decoded file text (layout save-sync, miss 08B)", () => { + const onSavedText = vi.fn(); + const win: SaveHookWindow = { + FS: { + readFile: () => new TextEncoder().encode("(kicad_sch (version 1))"), + } as unknown as SaveHookWindow["FS"], + kicadCollab: {}, + }; + registerSaveHook(win, { + slug: SLUG, + onSavedText, + log: () => {}, + onStatus: () => {}, + }); + win.kicadCollab!.onSave!(`${PROJ}/sheet.kicad_sch`); + expect(onSavedText).toHaveBeenCalledWith("sheet.kicad_sch", "(kicad_sch (version 1))"); + }); + + it("an onSavedText read failure is logged, not thrown", () => { + const onSavedText = vi.fn(); + const log = vi.fn(); + const win: SaveHookWindow = { + FS: { + readFile: () => { + throw new Error("gone"); + }, + } as unknown as SaveHookWindow["FS"], + kicadCollab: {}, + }; + registerSaveHook(win, { slug: SLUG, onSavedText, log, onStatus: () => {} }); + win.kicadCollab!.onSave!(`${PROJ}/sheet.kicad_sch`); + expect(onSavedText).not.toHaveBeenCalled(); + expect(log).toHaveBeenCalledWith(expect.stringContaining("onSavedText read failed")); + }); }); diff --git a/web/standalone/src/wasm/save-flow.ts b/web/standalone/src/wasm/save-flow.ts index ac68a03..d27ad1a 100644 --- a/web/standalone/src/wasm/save-flow.ts +++ b/web/standalone/src/wasm/save-flow.ts @@ -35,6 +35,12 @@ export function registerSaveHook( * ("Add Sheet"), which the page-load file list can't contain. */ onSaved?: (relPath: string) => void; + /** + * Like `onSaved` but with the saved file's TEXT (read back from MEMFS). + * The collab layout save-sync (miss 08B) uses it to reconcile non-item + * document state (title block, paper, setup…) into the room doc. + */ + onSavedText?: (relPath: string, text: string) => void; }, ): void { const projectPrefix = `${memfsProjectDir(opts.slug)}/`; @@ -64,6 +70,17 @@ export function registerSaveHook( opts.onSaved?.(relPath); + if (opts.onSavedText) { + try { + const data = win.FS?.readFile(absPath); + if (data instanceof Uint8Array) { + opts.onSavedText(relPath, new TextDecoder().decode(data)); + } + } catch (err) { + opts.log(`[save] onSavedText read failed for ${relPath}: ${String(err)}`); + } + } + if (!opts.saveBytes) { opts.log(`[save] ${relPath} saved in MEMFS (no external save target)`); return;