From 9e5ae8baf62dd2d59e2953e75c9a7e336ec60240 Mon Sep 17 00:00:00 2001 From: David Montero Crespo Date: Mon, 20 Jul 2026 17:39:29 +0200 Subject: [PATCH] =?UTF-8?q?fix(breadboard):=20derive=20seating=20at=20elem?= =?UTF-8?q?ent=20mount=20=E2=80=94=20closes=20run-before-seating=20race?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A part can land in the store at its FINAL position before its element mounts: the agent streams add_component and the seating move in one batch, and updateComponent's reseat then finds no DOM (computeSeating null) and keeps the empty seating. Nothing re-derived it afterwards — the agent-side seat correction skips when the position needs no nudge, and 'pininfo-change' only fires on pin-SET swaps, not on plain init. Meanwhile run_simulation executes right after the SSE round, before the correction's animation frame. Net effect, reported by a user as a suspicion that turned out exactly right: a clock the agent built and ran in one turn showed a dead display, while reloading the project and running it worked — bb seating wires are persisted, so on reload they exist before Run is pressed. DynamicComponent now reseats once the element's pinInfo first becomes measurable (same polling cadence as the pinInfo-ready effect), which closes the hole for every path that stores a final position before mount: agent batches, project load, undo. To keep that free on load, reseatComponentOnBreadboard skips the store write when there is nothing seated and nothing to clear — otherwise every off-board part would churn the wires array identity once per mount. Verified live end-to-end: agent adds + seats + wires + compiles + RUNS in a single turn; the seated LED blinks immediately (4 transitions sampled), with all 4 seated-pin markers present — no reload needed. Co-Authored-By: Claude Fable 5 --- .../src/__tests__/breadboard-seating.test.ts | 50 +++++++++++++++++++ frontend/src/components/DynamicComponent.tsx | 40 +++++++++++++++ frontend/src/store/useSimulatorStore.ts | 10 ++++ 3 files changed, 100 insertions(+) diff --git a/frontend/src/__tests__/breadboard-seating.test.ts b/frontend/src/__tests__/breadboard-seating.test.ts index af89dae8..6b5ce0ee 100644 --- a/frontend/src/__tests__/breadboard-seating.test.ts +++ b/frontend/src/__tests__/breadboard-seating.test.ts @@ -258,3 +258,53 @@ describe('resolveSeatPosition', () => { expect(bbWires.map((w) => w.end.pinName).sort()).toEqual(['11t.a', '5t.a']); }); }); + +/** + * The run-before-seating race: a part can land in the store at its final + * position BEFORE its element mounts (the agent streams add_component and the + * seating move in one batch). The reseat inside updateComponent then finds no + * DOM and keeps an empty seating — and nothing re-derived it, so a simulation + * started in that window ran against a part with no bb wires, while reloading + * the project (bb wires are persisted) worked. DynamicComponent now reseats + * at mount, as soon as pinInfo is measurable; these tests cover the store + * half of that contract. + */ +describe('reseat after late element mount', () => { + beforeEach(() => { + document.getElementById('res1')?.remove(); + const s = useSimulatorStore.getState(); + s.setComponents([ + { id: 'bb1', metadataId: 'breadboard', x: 0, y: 0, properties: {} }, + { id: 'res1', metadataId: 'resistor', x: 900, y: 900, properties: {} }, + ] as never); + s.setWires([]); + }); + + it('a seating move with NO mounted element creates no bb wires (the race)', () => { + // Element deliberately NOT mounted — this is the agent-batch window. + useSimulatorStore.getState().updateComponent('res1', resistorPosFor('5t.a', 0, 0)); + expect(useSimulatorStore.getState().wires.filter((w) => w.bb)).toHaveLength(0); + }); + + it('reseating once the element mounts creates the missing bb wires', () => { + useSimulatorStore.getState().updateComponent('res1', resistorPosFor('5t.a', 0, 0)); + expect(useSimulatorStore.getState().wires.filter((w) => w.bb)).toHaveLength(0); + + // Element appears (custom-element upgrade) — the mount-time effect fires: + mountFakeElement('res1', RES_PIN_INFO); + useSimulatorStore.getState().reseatComponentOnBreadboard('res1'); + + const bbWires = useSimulatorStore.getState().wires.filter((w) => w.bb); + expect(bbWires).toHaveLength(2); + expect(bbWires.map((w) => w.end.pinName).sort()).toEqual(['11t.a', '5t.a']); + }); + + it('mount-reseat of an off-board part is a no-op that keeps array identity', () => { + // Every component reseats at mount now — a project load must not churn + // the wires array once per off-board part. + mountFakeElement('res1', RES_PIN_INFO); // far from the board at (900,900) + const before = useSimulatorStore.getState().wires; + useSimulatorStore.getState().reseatComponentOnBreadboard('res1'); + expect(useSimulatorStore.getState().wires).toBe(before); + }); +}); diff --git a/frontend/src/components/DynamicComponent.tsx b/frontend/src/components/DynamicComponent.tsx index 8bd28d42..5dbbc6d0 100644 --- a/frontend/src/components/DynamicComponent.tsx +++ b/frontend/src/components/DynamicComponent.tsx @@ -346,6 +346,46 @@ export const DynamicComponent: React.FC = ({ return () => el.removeEventListener('pininfo-change', onPinInfoChange); }, [id, metadata.tagName]); + /** + * Reseat once the element's geometry first becomes measurable. + * + * A part can land in the store at its FINAL position before its element + * mounts — the agent streams add_component + a seating move in one batch, + * and `updateComponent`'s reseat then finds no DOM (computeSeating null) + * and keeps the (empty) seating. Nothing re-derived it afterwards: the + * seat-correction skips when the position needs no nudge, and + * 'pininfo-change' only fires on pin-SET swaps, not on plain init. So the + * part had no bb wires until the user dragged it or reloaded — a clock + * started by the agent in that window ran against a dead display, while + * reload+run worked (bb wires are persisted). Deriving the seating at + * mount closes that hole for every path (agent, load, undo). + */ + useEffect(() => { + const tryReseat = () => { + try { + const pinInfo = (elementRef.current as any)?.pinInfo; + if (pinInfo && Array.isArray(pinInfo) && pinInfo.length > 0) { + useSimulatorStore.getState().reseatComponentOnBreadboard(id); + return true; + } + } catch { + // element not ready yet / headless tests + } + return false; + }; + if (tryReseat()) return; + // Same cadence as the pinInfo-ready poll above: the custom element may + // upgrade a few frames after React commits. + const interval = setInterval(() => { + if (tryReseat()) clearInterval(interval); + }, 100); + const timeout = setTimeout(() => clearInterval(interval), 2000); + return () => { + clearInterval(interval); + clearTimeout(timeout); + }; + }, [id, metadata.tagName]); + /** * Extract pinInfo from web component after it initializes */ diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts index fa340088..4db1d304 100644 --- a/frontend/src/store/useSimulatorStore.ts +++ b/frontend/src/store/useSimulatorStore.ts @@ -2515,6 +2515,16 @@ export const useSimulatorStore = create((set, get) => { // null = geometry unmeasurable (unmounted DOM) — keep whatever // seating exists rather than tearing out live connections. if (seating === null) return; + // Nothing seated and nothing to clear: skip the store write. The + // mount-time reseat (DynamicComponent) calls this for EVERY part as + // its element becomes measurable — on a project load that would churn + // the wires array identity once per off-board component for no change. + if ( + seating.length === 0 && + !get().wires.some((w) => w.bb && (w.start.componentId === id || w.end.componentId === id)) + ) { + return; + } set((state) => { const kept = state.wires.filter( (w) => !(w.bb && (w.start.componentId === id || w.end.componentId === id)),