Route worker responses by correlation id#50
Open
kmatzen wants to merge 1 commit into
Open
Conversation
The WorkerBridge held a single responseHandler slot and dispatched worker responses by message *type* alone, so concurrent requests displaced and settled each other. TLA+ models of the old and new protocols are in specs/; TLC produced two counterexamples against the old design, both replayed as tests in workerBridge.test.ts: - Two evaluates in flight: the first's `sdf` response reached the second's handler, passed its `seq === evalSeq` check (it *was* the newest), and resolved the second promise with the first's geometry. The staleness guard checked whether the handler was current, never whether the response matched it. - An export displaced by an edit: `useEvaluator` fires on any store change and overwrote the export's handler, so the `exportResult` matched neither branch of the evaluate handler and was discarded. Toolbar's await never returned. Every request now carries an rid that the worker echoes on all responses including progress and errors, and the handler slot becomes a map keyed by rid. Superseded evaluates resolve(null) instead of returning unsettled — useEvaluator already has its own evalSeq guard. worker.onerror now rejects everything outstanding rather than only logging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
WorkerBridgeheld a singleresponseHandlerslot and dispatched worker responses by message type alone, so concurrent requests displaced and settled each other.TLA+ models of the old and new protocols are in
specs/. TLC produced two counterexamples against the old design, both replayed as tests inworkerBridge.test.ts:Cross-talk between evaluates. Two evaluates in flight: the first's
sdfresponse reached the second's handler, passed itsseq === evalSeqcheck (it was the newest), and resolved the second promise with the first's geometry. The staleness guard checked whether the handler was current, never whether the response matched it — so wrong geometry could reach the viewport.Export orphaned by an edit.
useEvaluatorfires on any store change and overwrote the export's handler, so theexportResultmatched neither branch of the evaluate handler and was discarded.Toolbar'sawaitnever returned.Changes
ridthat the worker echoes on all responses, including progress and errors.rid. Routing never consults message type.resolve(null)instead of returning unsettled —useEvaluatoralready has its ownevalSeqguard.worker.onerrorrejects everything outstanding rather than only logging.evaluatenow bumpsevalSeqbefore awaitingreadyPromiserather than after, so sequence numbers follow call order rather than ready-resolution order.Verification
tsc --noEmitmainbaselineThe single full-suite failure is
simplify.test.ts > preserves mesh validity, a pre-existing CPU-bound test with a marginal 5000ms timeout. It passes in isolation and its failure count varied run-to-run on identical code; nothing in this change touchessimplifyMeshor its imports.specs/check.shruns both models through TLC. The fixed spec checks exhaustively atMaxReq = 4(2,535 states, depth 13) including liveness under weak fairness.tla2tools.jaris gitignored and fetched on first run.Follow-ups
#51 — cancel in-flight worker jobs. There is still no cancellation. Every request now settles, but a 256³ export still blocks every evaluate queued behind it, since the worker runs each message to completion. The viewport will stall during exports as before — it just won't hang permanently. Approaches and the race worth specifying are laid out in the issue.
#52 — a more rigorous way to verify rendering correctness. Out of scope here, but surfaced while reading the SDF pipeline:
evaluate.tsandcodegen.tsare two independent implementations of every operation, and nothing checks they agree.🤖 Generated with Claude Code