Skip to content

fix(desktop): a check that runs beside a change no longer hides it from the view - #359

Open
ManuelStaggl wants to merge 1 commit into
mainfrom
fix/quiet-check-race
Open

ManuelStaggl wants to merge 1 commit into
mainfrom
fix/quiet-check-race

Conversation

@ManuelStaggl

Copy link
Copy Markdown
Collaborator

Symptom

The contract test "a slot built into a hinged flap stays in it when the flap turns (ADR 0027)" fails in about half of its runs: the log says "built the function block slot", SolidCraft has cut the slot (volume 4855.69 → 4683.37 mm³, faces 18 → 22), but the view stays at "2 bodies · 35 faces" until some later change. Print check and export preview stay on the old design with it.

Cause

HeadlessEngine.checkForChange(quiet) always recorded the revision it read and told the listeners only when it was not quiet and the revision had moved since the last check of either kind.

  1. A change starts (applyBlock): it waits for running trials once, then reads sketch names, the component, and runs applySteps.
  2. Meanwhile the print check of the hinge that was just mirrored starts a trial (joint sweep).
  3. The trial ends between applySteps and the change's own check; its quiet check records the revision the block made.
  4. The change's loud check finds the revision where the last check left it, tells nobody, nothing is mirrored.

The same holds for the autosave (save ends with a quiet check) and for tool calls of the assistant, whose check in callTool's finally can be overtaken by a quiet one in the same way.

Fix: bookkeeping in the engine (way b), not one queue for everything (way a)

The engine keeps apart what the revision alone cannot tell, at the one place where it is recorded (HeadlessEngine.check):

  • changed(): after a change PartBuddy made itself (the nine places in the supervisor). The listeners are told even when a quiet check recorded the revision first. MirrorScheduler already drops a revision it has delivered, so a change that left the revision alone costs nothing.
  • unchanged(): after a trial or a save. Tells nobody. When it records a moved revision while a tool call of the assistant is under way, it leaves a note, and that call's own check tells the listeners.
  • checkForChange is gone from the public surface, so the supervisor cannot ask for the ambiguous "maybe" check any more; only callTool uses it.

Why not the queue: it would put every tool call of the assistant, every trial and every autosave behind one another, a change in timing for the whole product that cannot be tried here without the app, with self-blocking to guard against (applyBlock runs a trial for the bold font before it builds). And it holds only as long as every caller remembers to queue; the autosave does not go through probing today. The bookkeeping never makes anything wait and can only tell more often than before, never less.

Known cost: a read of the assistant during which a trial ends is mirrored once more than needed, because the engine cannot know that the call only read. A trial alone, and a read after it, still mirror nothing.

Not changed: trials still run interleaved with tool calls of the assistant. Each trial is one batch that SolidCraft takes back itself, except the snap-fit test of a joint sweep, which reads where the joint stands and puts it back in a second batch; a joint.drive of the assistant between the two would be taken back. That needs the queue and is left for its own change.

The window mode is not affected: there SolidCraft reports each change itself with design.changed, and PartBuddy keeps no revision of its own that a quiet check could move.

A line in the log ("revision N was recorded before its change was told; telling it now") shows when the guard was needed.

Tests

apps/desktop/test/headless.test.ts, against a stand-in whose revision the test moves by hand (the first two fail without the fix):

  • tells a change of PartBuddy whose revision a trial recorded first
  • tells a change of the assistant whose revision a trial recorded while the call ran
  • tells nothing after a trial alone, nor after the assistant reads when the trial is over
  • tells each of two changes that follow one another
  • counts a call as over with its check, so a later trial is not taken for its change

typecheck, eslint and vitest run (721 passed) are green.

🤖 Generated with Claude Code

…om the view

A trial (print check, joint sweep, dimension probe) and the autosave end with a quiet
revision check. When one ended between a change and the change's own check, it recorded
the change's revision, the loud check saw no movement and the view kept the old design.

The headless engine now keeps the two apart where the revision is recorded: a change
PartBuddy made itself is always told (changed()), and a quiet check that records a moved
revision while a tool call of the assistant is under way leaves it for that call's check.
A trial alone still tells nobody.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant