Repository navigation
Conversation
Every call to a model's worker goes through FragmentsModel._invoke() instead of spelling out threads.invoke(model.modelId, …) at each of the 89 call sites. The model is now the one place that knows how its worker is addressed, which is what the typing of these calls and the internal uid build on. Mechanical, with one behavior detail: the calls to a delta model go through the delta model itself, looked up with _getDeltaModel(), instead of invoking its modelId. A deltaModelId whose model is no longer loaded now reads as "no edits" where it sent a request for a missing model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FragmentsModel._invoke() only takes names of VirtualFragmentsModel's methods, with arguments matching their parameters, and resolves to their return type. A renamed or retyped worker method no longer fails at runtime only. It showed worker methods declaring narrower parameters than they handle: the highlight and visibility methods take no ids to mean every item, and getItemsData() takes GUIDs too. Their signatures now say so. Results are still cast at the call sites: they are copied between threads, so class instances in them arrive as plain objects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FragmentsModel.getItemsGeometry() and getGeometries() returned MeshData whose `transform`, typed as a THREE.Matrix4, was the plain object the worker's copy arrives as, so calling a Matrix4 method on it threw. They now rebuild it, as ItemGeometry.get() already did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_invoke is typed now, and every one of these arguments matches its worker method, so the comments only switched off the argument checking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_invoke claimed to resolve to what the worker method returns, but the
worker's answer is a structured-clone copy: class instances in it arrive
without their prototype. And nothing checked the claim, because
FragmentsConnection.fetch/invoke resolved to `any`.
Cloned<T> describes that copy: methods are dropped, so a THREE.Matrix4 is
typed as `{ elements, isMatrix4 }` and no longer passes for one. invoke<R>()
resolves to Cloned<R>, with R (default any) the method's return type, and
FragmentsConnection.fetch keeps the request's type instead of dropping it,
so the execute and box requests now declare the fields the worker answers
with.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_invoke accepted any public method of VirtualFragmentsModel, including ones that break over the wire: dispose() deleted the worker-side model behind the main thread's back, and getItemsByConfig() takes a function, which can't be copied to the worker. It now takes only the methods listed in RemoteMethods, which also documents what crosses the thread boundary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getHighlight() and getItemsMaterialDefinition() returned definitions whose `color`, typed as a THREE.Color, was the plain copy the worker's answer arrives as, so calling a THREE.Color method on it threw. resetColors() meant to rebuild it but checked `isColor`, which the copy keeps as an own property, so it never did, and getItemsMaterialDefinition() didn't call it. MaterialManager.restoreColor() rebuilds the color from a copied definition and is typed that way, so a definition can't be handed out without it. It keeps the components as they are: the worker already converted them from sRGB, and the old fallback would have converted them again. Definitions the worker transfers for rendering go through it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…copy EditManager.restoreTransforms() takes the geometries as they arrive from the worker and returns them with their transform rebuilt, so dropping the rebuild is now a type error rather than a cast that hides it. ItemGeometry uses it instead of its own copy of the loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getBuffer() is compressed by default, and compressed it resolves to a Uint8Array, but it was cast to an ArrayBuffer, so `new DataView(buffer)` type-checked and threw. The cast couldn't simply go: the worker method's inferred type, ArrayBufferLike, absorbs Uint8Array too. The worker method now declares both, which is also what IFragmentsModel says. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Item.getAttributes() cast the worker's attributes to `type?: number`, and AttributeData and ItemAttributes.setType() took a number too, but the types are the names the importer writes, such as "IFCLABEL", and the worker declares them as strings. A numeric comparison against them never matched. They are strings now, and the cast is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getSpatialStructure(), getMetadata(), getCRS() and getSequenced() on the worker returned `any`, so the casts on the main thread were the only type their results had. They now declare what they return. That showed getSequenced() handing out results as the worker's copies: "mergedBoxes" typed as a THREE.Box3, "geometry" transforms as THREE.Matrix4 and "highlight" colors as THREE.Color were all plain objects. Each kind of result is now rebuilt from the copy, by a map keyed by the result kind, so a kind that needs rebuilding can't be left out. It resolves to null when the worker doesn't know the result kind, as its documentation says. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Most of them claimed exactly what _invoke already resolves to, each as a separate unchecked claim, so a worker method changing its result would go unnoticed. Where a result is handed out under a named type (ItemData, SpatialTreeItem, the raw data maps, ...), a return annotation keeps that name in the declarations and is checked against the worker's result. The casts that stay are the index methods' and getMetadata()'s: their type parameters are the caller's word for an index's or the metadata's shape, which the worker can't check either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getMetadata() promises an object (IFragmentsModel types it as T), but the
worker answered null for a model whose file has no metadata, which the
main thread's cast passed through. It answers {} now, so the promise
holds without every caller having to check for null. getCRS() still
answers null for a model without a CRS.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The answer was posted outside the try/catch, so a result postMessage() can't copy, such as one holding a function, threw a DataCloneError as an unhandled rejection and the request never settled. The request is now answered with that error instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A model is registered on its worker before its generate() loop ends, so a DELETE_MODEL that landed mid-load, from disposing a model while it loads, disposed the half-built model from under that loop. The delete now aborts the load, waits for it to unwind and clean up after itself, and answers after that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both threads key a model's state by its modelId, so a second load under an ID that was loaded or still loading silently clobbered the first model. load() now throws for it. abort(modelId) relies on it too: an ID names at most one load in flight. Only models in `models.list` count. An ID whose model was disposed can be loaded again as before, including while its worker still deletes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abort(modelId) kept its own account of the loads in flight and of the ones already aborted, in two sets of IDs next to the caller's signal. A load now owns one AbortController that the signal and abort(modelId) both go through. abort(modelId) only looks up the load running under that ID, so it needs no bookkeeping of its own, and an abort can't outlive its load or reach a later load of the same ID. An abort also ends the load's wait for the worker's step right away instead of when the worker's answer comes back, and goes on to dispose the partial model. load() still rejects once that model is disposed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Disposing a model while it loads, with dispose(), disposeModel() or FragmentsModels.dispose(), left its load running: load() carried on and, with autoCoordinate off, fired onModelLoaded for the dead model. dispose() now aborts the model's load through the same controller as the signal and abort(modelId), so load() rejects with a LoadAbortedError. dispose() returns the same promise when it is called again. That lets the aborted load wait for the disposal that aborted it instead of starting a second one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every model gets a uid from FragmentsModels that is never reused, and all internal routing uses it instead of the modelId: messages to and from the workers, thread assignment, progress callbacks, the tile queue, material definitions and the worker's mesh cache. Late work of a disposed model can no longer land on a newer model loaded under the same modelId. The public `models.list` stays keyed by modelId. dispose() now takes effect before it returns: the model leaves the models list and the scene, its thread slot is freed and later requests to it reject. The returned promise resolves once the worker has deleted the model. The modelId can be loaded again right away. - A failed or aborted load disposes its partial model without waiting for the worker, so load() rejects right away. - Terminating a worker rejects the requests still waiting on it. - A FINISH from a disposed model still settles update fences. - editor.save() no longer reuses the old model's materials; they are disposed with it in finalizeDispose(). - The per-model fence of update(true) is keyed by uid too: a FINISH carries the uid now, so a fence still keyed by modelId never saw its model finish and update(true) never resolved. It is cleared when the model is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The main thread answers a worker's request with the request itself. RequestsManager empties a RECOMPUTE_MESHES request's tile list once it has taken the tiles, but a disposed model's request was dropped with its list in place, so every tile batch its worker still flushed was copied back, buffers and all. It is emptied there too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A call on a disposed model, e.g. through an Item or Element kept from before, was rejected by the connection as "model 7 is not loaded": 7 is the model's internal uid, which the caller has never seen. _invoke now rejects it first as 'model "arq" is disposed'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 11, 2026
This branch has not been deployed
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.
Part of a stack of six PRs that replaces #305. The branches live in a fork, so GitHub can't chain them: every PR is based on
main, and the diff of each one includes the PRs below it. Each PR names the commits that are its own and links a comparison that shows only those. They are meant to merge in the order below, and PRs 2 to 6 stay drafts until the one before them is in.The work. All model state, on both threads, is keyed by the consumer's
modelId, and amodelIdcan be reused. Late async work of a model that was disposed or replaced (a worker reply, queued tiles, a material transfer, a progress event, an in-flight dispose) therefore lands on the next model loaded under the same ID. The stack gives each model an internal uid that is never reused and routes everything by it, addsload({ signal })(#173), and types the messages between the threads, where most of the bugs it fixes were hidden behindany.The stack, bottom to top, in merge order:
refactor(): route model calls through _invoke: mechanical groundwork, no behavior changefix(): type calls to a model's worker, rebuild what the worker copiesfeat(): load abort signal + harden the message layerfix(): key models by an internal uid, dispose synchronously: the core fixfix(edit): key editor state by the model's uidrefactor(): type the requests workers send to the main threadEvery commit in the stack type-checks and passes the test suite on its own, and each PR builds. The top of the stack matches #305's branch merged with today's
main, apart from one fix that merge needs (see PR 4) and two reworded lines.This PR's own diff: its last 3 commits, shown here on their own. The commits before them belong to the PRs below.
Description
PR 4 of 6, the core fix of the stack. Each model gets a uid from a counter in
FragmentsModelsthat is never reused, and all internal routing uses it instead of themodelId:modelIdstays the consumer-facing name:models.list,disposeModel()andabort()are still keyed by it. Work addressed to a uid that is no longer live finds no model and is dropped.dispose()takes effect before it returns. The model leavesmodels.listand the scene, its worker slot is freed, and later requests to it reject. The returned promise resolves once the worker has deleted the model. ThemodelIdcan be loaded again right away.From the review of #305. The review set one race aside as its own issue: a load that starts before the previous model under the same
modelIdhas finished its dispose round trip. This PR is that fix. The old model finishes its teardown under its own uid, so nothing of it can reach the new one, and messages for a disposed model are dropped before anything can throw.Bugs fixed (all reproducible on
main)modelIdagain: the old model's late tiles, materials and progress events were applied to the new one, and the old dispose'sthreads.delete(modelId)could remove the new model's worker assignment.modelId. Two models with the samemodelIdon one worker read, and evicted, each other's meshes.forceUpdateFinish(), and the tile batches its worker still flushed were copied back to it, buffers and all.editor.save()reused the old model's materials, which were never tracked for disposal, so they leaked. The new model gets its own.Behavior changes
dispose(),disposeModel()andFragmentsModels.dispose()apply their effects synchronously. They still return a promise.load()that fails or is aborted rejects right away. It used to wait for the worker to delete the partial model first, which a synchronous dispose no longer needs.ItemorElementkept from before, rejects withmodel "<modelId>" is disposed. Before, it went to whichever model held themodelId.model.modelIdno longer readsmodel.object.name, so renaming the object doesn't change it.uid. The worker bundle must match the library version, whichFragmentsModels.getWorker()already guarantees.Internal API changes. These are documented as "don't use directly" but are public on their classes:
new FragmentsModel({ modelId, uid, meshManager, threads, editor, threadGroup })takes a single object.new Editor(core)no longer takes the connection.model.threads.invoke(uid, …),RequestsManager.clean(uid)andFragmentsConnection.delete(uid)take the uid, and the last one returns a promise.MaterialManager.releaseModelSlot()is removed.Aborting is not part of this PR
All of the abort work is in PR 3. In the load and abort code, on both threads, this PR only swaps the key from
modelIdto uid and makes the dispose call inload()'s error path synchronous.One fix that isn't in #305
Since #305 was written,
maingained a per-model fence forupdate(true), keyed byrequest.modelId(7516383). After this PR a FINISH carriesuidinstead, so that fence would never see its model finish andupdate(true)would never resolve. Git merges the two changes cleanly andtscpasses, so only a test catches it. The uid commit keys the fence by uid and clears it when the model is removed, andmesh-manager-multi-model-fence.test.tsis ported to uid along with it.Additional context
Worth a close look:
dispose()contract inFragmentsModel.dispose,DataManager.disposeandFragmentsConnection.delete.MeshManager.The first commit changes the key both threads use to address each other, so it can't be split further: it touches all 34 source files of this PR. The other two commits are small.
Tests: 186 pass, 8 of them new. They cover reloading an ID while its old model is still being deleted and the late messages of a disposed model (
load-abort.test.ts), connection deletion and worker termination (fragments-connection.test.ts), and a disposed model's FINISH (mesh-manager-fence.test.ts). None of this has been exercised in a browser with real workers yet.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following:
feat(examples): add hello-world example).fixes #123).🤖 Generated with Claude Code