diff --git a/web/standalone/src/components/CmmsAiWidget.tsx b/web/standalone/src/components/CmmsAiWidget.tsx index a4953f3..3aabf5f 100644 --- a/web/standalone/src/components/CmmsAiWidget.tsx +++ b/web/standalone/src/components/CmmsAiWidget.tsx @@ -20,18 +20,27 @@ export function CmmsAiWidget(): React.ReactElement | null { React.useEffect(() => { let cancelled = false; - fetchCmmsConfig().then((c) => { - if (cancelled) return; - setConfig(c); - if (c.aiEnabled) installAiBridge(); - // Belt-and-braces alongside the `?theme=` hand-off (lib/theme.ts) — that - // param already wins whenever edit.php sets it, so this only matters if - // it's ever absent. Skip if the URL param already ran; setTheme is - // otherwise idempotent (persists + toggles the same `.dark` class). - if (c.dark && !new URLSearchParams(window.location.search).get("theme")) { - setTheme("dark"); - } - }); + // fetchCmmsConfig() never rejects (its own internal .catch() falls back + // to NO_WIDGET) — this .catch() guards the callback body itself instead + // (setConfig/installAiBridge/setTheme), so a mistake here becomes a + // logged no-op, not a silent unhandled rejection with nothing to say why + // the widget didn't appear. + fetchCmmsConfig() + .then((c) => { + if (cancelled) return; + setConfig(c); + if (c.aiEnabled) installAiBridge(); + // Belt-and-braces alongside the `?theme=` hand-off (lib/theme.ts) — + // that param already wins whenever edit.php sets it, so this only + // matters if it's ever absent. Skip if the URL param already ran; + // setTheme is otherwise idempotent (persists + toggles `.dark`). + if (c.dark && !new URLSearchParams(window.location.search).get("theme")) { + setTheme("dark"); + } + }) + .catch((err: unknown) => { + console.error("[cmms-ai-widget] boot failed:", err); + }); return () => { cancelled = true; }; diff --git a/web/standalone/src/lib/cmms-config.ts b/web/standalone/src/lib/cmms-config.ts index 754e157..4ede25a 100644 --- a/web/standalone/src/lib/cmms-config.ts +++ b/web/standalone/src/lib/cmms-config.ts @@ -15,17 +15,22 @@ export interface CmmsConfig { entityId: string; } -function isCmmsConfig(v: unknown): v is CmmsConfig { +// Only `dark` / `aiEnabled` / `aiEditorKind` actually gate behavior — a +// bug found 2026-09-05 in browser verification required all six fields as +// strict strings, so the real server response (which omits `entityType` / +// `entityId`) failed validation and silently fell back to NO_WIDGET despite +// `aiEnabled: true` — the widget never mounted and nothing logged why. +// `csrfToken` / `entityType` / `entityId` are only used to build the iframe's +// query string (harmless as `""`), so a missing one shouldn't disable the +// widget entirely. +function isCmmsConfigCore(v: unknown): v is Pick { if (!v || typeof v !== "object") return false; const c = v as Record; - return ( - typeof c.dark === "boolean" && - typeof c.aiEnabled === "boolean" && - c.aiEditorKind === "kicad" && - typeof c.csrfToken === "string" && - typeof c.entityType === "string" && - typeof c.entityId === "string" - ); + return typeof c.dark === "boolean" && typeof c.aiEnabled === "boolean" && c.aiEditorKind === "kicad"; +} + +function optionalString(v: unknown): string { + return typeof v === "string" ? v : ""; } const NO_WIDGET: CmmsConfig = { @@ -47,7 +52,18 @@ export function fetchCmmsConfig(): Promise { if (!cached) { cached = fetch(`${API_BASE_URL}/api/cmms-config`, { credentials: "include" }) .then((r) => (r.ok ? r.json() : null)) - .then((v) => (isCmmsConfig(v) ? v : NO_WIDGET)) + .then((v): CmmsConfig => { + if (!isCmmsConfigCore(v)) return NO_WIDGET; + const c = v as Record; + return { + dark: v.dark, + aiEnabled: v.aiEnabled, + aiEditorKind: v.aiEditorKind, + csrfToken: optionalString(c.csrfToken), + entityType: optionalString(c.entityType), + entityId: optionalString(c.entityId), + }; + }) .catch(() => NO_WIDGET); } return cached; diff --git a/web/standalone/src/wasm/collab/ai-bridge.test.ts b/web/standalone/src/wasm/collab/ai-bridge.test.ts new file mode 100644 index 0000000..535a8da --- /dev/null +++ b/web/standalone/src/wasm/collab/ai-bridge.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from "vitest"; +import * as Y from "yjs"; +import { docToFile, docToY, kicadLibSymbolsMap, fileToDoc, yToDoc } from "@pcbjam/shared"; +import { applyToActiveBuffer, readActiveBuffer, registerActiveCollabDoc } from "./ai-bridge"; + +// A symbol instance is only renderable with its `(lib_symbols …)` definition. +// `docDelta` (the apply path's item differ) only diffs uuid-bearing items, and a +// `(symbol "Device:R" …)` lib def has no uuid — so an AI proposal that places a +// part the sheet has never carried used to land the instance with no definition, +// and KiCad drew nothing. `applyToActiveBuffer` now upserts the proposal's lib +// defs into `kdoc_libsymbols` alongside the item delta. + +const WIRE1 = "22222222-0000-0000-0000-000000000001"; +const SYM1 = "44444444-0000-0000-0000-000000000001"; + +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"))) +) +`; + +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(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"))) +) +`; + +function seed(src: string): Y.Doc { + const ydoc = new Y.Doc(); + docToY(fileToDoc(src), ydoc); + return ydoc; +} + +describe("ai-bridge applyToActiveBuffer", () => { + it("carries a newly-placed symbol's (lib_symbols) definition, not just the instance", () => { + const ydoc = seed(SCH_WITHOUT_SYMBOL); + registerActiveCollabDoc(ydoc, "board.kicad_sch"); + try { + expect(kicadLibSymbolsMap(ydoc).get("Device:R")).toBeUndefined(); + + applyToActiveBuffer(SCH_WITH_SYMBOL); + + const def = kicadLibSymbolsMap(ydoc).get("Device:R"); + expect(def).toBeDefined(); + expect(def).toContain("R_0_1"); + + // and the instance item itself landed via the normal item delta + const out = docToFile(yToDoc(ydoc)); + expect(out).toContain(SYM1); + expect(out).toContain('(lib_id "Device:R")'); + } finally { + registerActiveCollabDoc(null); + } + }); + + it("is a no-op on lib_symbols when the proposal adds no new definitions", () => { + const ydoc = seed(SCH_WITH_SYMBOL); + registerActiveCollabDoc(ydoc, "board.kicad_sch"); + try { + const before = kicadLibSymbolsMap(ydoc).get("Device:R"); + expect(before).toBeDefined(); + + // same doc, wire nudged — no new parts + applyToActiveBuffer(SCH_WITH_SYMBOL.replace("101.6 50.8", "127 50.8")); + + expect(kicadLibSymbolsMap(ydoc).get("Device:R")).toBe(before); + expect(readActiveBuffer()).toContain("127 50.8"); + } finally { + registerActiveCollabDoc(null); + } + }); +}); diff --git a/web/standalone/src/wasm/collab/ai-bridge.ts b/web/standalone/src/wasm/collab/ai-bridge.ts index 85a46cd..4040831 100644 --- a/web/standalone/src/wasm/collab/ai-bridge.ts +++ b/web/standalone/src/wasm/collab/ai-bridge.ts @@ -28,6 +28,8 @@ import { docDelta, docToFile, fileToDoc, + libSymbolsFromLayout, + upsertLibSymbolsToY, yToDoc, type KicadDoc, } from "@pcbjam/shared"; @@ -82,6 +84,20 @@ export function applyToActiveBuffer(newSource: string): void { const prev = yToDoc(activeDoc); const next = fileToDoc(newSource); const delta = docDelta(prev, next); + // `docDelta` only diffs uuid-bearing items — a `(lib_symbols …)` def has no + // uuid, so a proposal that places a symbol the sheet has never carried adds + // the instance item but drops its library definition, leaving KiCad an + // instance pointing at an undefined symbol (renders nothing). Push the + // proposal's lib defs into the kdoc_libsymbols map FIRST (upsert by lib id, + // never deletes — the server always splices into the existing block, so this + // is existing ∪ additions), so the items observer's `deltaToItemsWire` can + // prefix each new instance with its `(lib_symbols …)` on the way to the + // editor — exactly what `kicad-binding.ts`'s own DOWN path does with + // `upsertLibSymbolsToY(doc, defs, ORIGIN)`. Deliberately NOT `syncLayoutToY`: + // that also reconciles non-item layout heads (root `uuid`, `sheet_instances`, + // `paper`…) wholesale-LWW from the proposal, which would corrupt the sheet's + // identity if the server's spliced preamble deviates at all. + upsertLibSymbolsToY(activeDoc, libSymbolsFromLayout(next.layout, next.items), APPLY_ORIGIN); applyDeltaToY(activeDoc, delta, APPLY_ORIGIN); }