Skip to content

Add StlDef and ThreeMfDef FileDef subclasses for 3D model files - #5658

Open
FadhlanR wants to merge 13 commits into
mainfrom
cs-12053-cs-12054-model-filedef-subclasses
Open

Add StlDef and ThreeMfDef FileDef subclasses for 3D model files#5658
FadhlanR wants to merge 13 commits into
mainfrom
cs-12053-cs-12054-model-filedef-subclasses

Conversation

@FadhlanR

@FadhlanR FadhlanR commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What

Adds .stl and .3mf support as base-realm FileDef subclasses, following the PngDef → ImageDef → FileDef pattern. Implements CS-12053 (StlDef) and CS-12054 (ThreeMfDef).

  • ModelDef (packages/base/model-file-def.gts) — shared base for 3D model formats. Owns the generic Model3DInfoField scene facts (meshes / materials / vertices / triangles / generator), the four format templates, the live WebGL viewer, the SVG-silhouette preview, and the shared getExtension / Model3dData / ModelInspectorSection helpers.
  • StlDef (packages/base/stl-model-def.gts, CS-12053) — extractAttributes runs a bounded STL parser (stl-meta-extractor.ts): ASCII/binary detection, facet / normal / degenerate-facet counts, color-data flag, bounding-box extents. Adds StlMetadataField.
  • ThreeMfDef (packages/base/three-mf-def.gts, CS-12054) — extractAttributes unzips the OPC package (fflate) and parses the model part(s) + slicer config (three-mf-meta-extractor.ts): objects, materials, plates, print parts, extruders, bounds. Adds ThreeMfMetadataField / ThreeMfPrintPartField.
  • Registers .stl / .3mf in runtime-common/file-def-code-ref.ts; adds fflate (base dep + host/app/lib/externals.ts shims for fflate and @cardstack/runtime-common/constants).
  • Six sample files under experiments/model-samples/ (stl + three-mf × simple/moderate/complex) for manual verification, plus StlDef/ThreeMfDef assertions in file-def-code-ref-test.ts and unit tests for the metadata parsers.

Supersedes #5627 and #5628 (the per-format draft PRs), unified here because both leaves share ModelDef.

Review follow-ups (latest revision)

Addressing the review of the initial revision:

  1. Parsers extracted to pure .ts modulesstl-meta-extractor.ts / three-mf-meta-extractor.ts (mirroring png-meta-extractor.ts), so they're directly unit-testable without the card-api/.gts harness, and the .gts leaves shrink to templates + extractAttributes.
  2. STL parsing is now a single streaming pass over the DataView with no per-vertex array — index-time transient memory stays flat regardless of model size. Degeneracy is counted per facet as read, fixing a miscount that occurred when non-finite vertices were dropped and the remainder regrouped into triples.
  3. Corrupt 3MF degrades cleanlyparseThreeMf returns undefined for any unparseable input (non-ZIP via try/catch, no .model part, malformed XML) so the leaf falls back to base FileDef via FileContentMismatchError, instead of a bare unzipSync throw. (The parser stays free of a card-api import.)
  4. Index-time parse cost is bounded — both leaves accept an optional fileSizeLimitBytes (threaded seam), defaulting to the realm's own DEFAULT_FILE_SIZE_LIMIT_BYTES (the same 5 MB ceiling the write path enforces, so the two can't drift). Over the cap the parse is skipped and only base attributes are returned — the file keeps its StlDef / ThreeMfDef type and the live client-side viewer (which parses its own geometry); it just shows empty inspector panels and a generic silhouette. It never throws (throwing would demote the file to a plain FileDef).
  5. model3d.vertices documented as "vertex records as the format stores them" (STL triangle-soup ≈ 3× triangles; 3MF indexed = unique) — not comparable across formats.
  6. De-duplication — shared getExtension + Model3dData, and a data-driven ModelInspectorSection component replaces the ~40-line inspector <style> block that was copied across the three isolated templates.

Rendering per format

  • atom — icon + filename.
  • fitted — renders the live model client-side, but lazily (an IntersectionObserver boots WebGL only when the tile scrolls on-screen, so a grid of many models stays within the browser's WebGL-context budget) and non-interactive (no orbit controls, no gesture trapping, no hint — a static rendered model that lets the enclosing grid scroll normally). Off-screen and during prerender it shows the silhouette.
  • embedded + isolatedlive interactive three.js orbit viewer (drag / scroll-zoom). three.js + OrbitControls + STL/3MFLoader load lazily from a CDN (esm.sh) at client render time only — the sanctioned Boxel pattern for card libraries (the loader resolves https:// specifiers). The engine never runs during extraction/indexing.
  • isolated layout mirrors the handoff realm: a header (icon + name + extension pill), a bordered live-viewer stage, and a boxed property inspector with grouped sections ("3D model" + "STL mesh" / "3MF package") and per-row hairline separators. Two-column when wide, single-column when narrow.

The SVG silhouette (drawn from the extracted bounding box) is the loading placeholder, the off-screen state for fitted, and the graceful fallback whenever WebGL / the CDN engine is unavailable (e.g. the headless prerender) — so the stored/prerendered HTML never breaks. Full GPU teardown on unmount.

Design decisions

  • Extraction is pure-JS and prerender-safe — STL/3MF metadata is parsed with DataView / DOMParser / fflate, no GPU, so indexing never loads three.js.
  • Two-tier library loading, matching the loader. Bare imports in base modules resolve only via the host shim registry (externals.ts), not node_modules — so fflate (used in extraction) is shimmed there. three.js (used only by the client viewer) uses the CDN URL-import path, the blessed pattern for card libraries, which keeps it out of the indexer entirely.
  • Parse-cost cap reuses the realm's file-size limit rather than an invented number — since a file over that limit can't be written to the realm in the first place, the cap is a backstop for environments that raise the limit, and base stays decoupled from host by taking the value through options.
  • Cross-origin byte fetch uses no credentials. The viewer fetches the file bytes with a plain fetch(url) (no credentials: 'include') — the host auth service worker injects the realm Authorization header, the same path that lets <img src> load realm images. A credentialed request is illegal against the realm's wildcard Access-Control-Allow-Origin, which is why the naive port initially failed cross-origin.
  • No linksTo for a future thumbnail — a FileDef can't store a concrete relationship, so the thumbnailUrl seam is a computed convention URL rather than a link.
  • Styling follows the repo conventions: semantic role tokens (--foreground / --muted-foreground / --card / --muted / --border), rem units, no font shorthand.

Verification

  • lint:types (ember-tsc) clean for the changed files, and prettier --check clean.
  • Unit tests (packages/host/tests/unit/model-meta-extractor-test.ts) cover parseStl (binary/ASCII, color detection, degenerate-facet counting, mismatch), parseThreeMf (package parse, slicer-config parts/plates/extruders, malformed→undefined, non-ZIP→undefined, built with fflate.zipSync), and silhouettePath.
  • Acceptance test (packages/host/tests/acceptance/model-file-extract-test.gts) drives the real render/file-extract route (the indexer's path) over an ASCII STL and a real 3MF ZIP served from a test realm, asserting the file-meta search doc carries the parsed model3d / stlMetadata / threeMfMetadata, plus the mismatch-fallback for a .stl whose bytes aren't STL.
  • Manually verified in the app — STL and 3MF render across atom/fitted/embedded/isolated, extraction populates the metadata (confirmed in the index DB), and the cross-origin fetch works via the auth service worker.

packages/base is not eslinted in CI (it has no lint:js); the pre-commit eslint "Parsing error" on .gts is a known local-only parser quirk, not a CI failure.

Reviewer focus: these are the first base FileDefs to populate a contained compound field (model3d, stlMetadata, threeMfMetadata) from extractAttributes. It uses the same deserialization as CardDef contains(FieldDef); the file-meta round-trip is now covered by the acceptance test above as well as manual verification.

Follow-ups / not in this PR

  • CS-12401 — shaded raster preview for fitted tiles / thumbnails (CPU rasterizer or capture of the live WebGL canvas, written to an ignored .derived/ dir, populating the thumbnailUrl seam).

Screen recording

Screen.Recording.2026-07-31.at.19.30.42.mov
Screen.Recording.2026-07-31.at.19.29.44.mov

🤖 Generated with Claude Code

Introduce a shared ModelDef base (extends FileDef) with two leaves:
StlDef (.stl) and ThreeMfDef (.3mf). Each extracts format metadata in
extractAttributes (pure-JS parsers ported from the handoff realm) and
renders a deterministic inline SVG silhouette across atom/fitted/
embedded/isolated — no WebGL, prerender-safe.

- ModelDef: shared Model3DInfoField scene facts, silhouette preview,
  and a thumbnailUrl seam for the shaded-PNG follow-up (CS-12401).
- StlDef: ASCII/binary detection, facet/normal/degenerate counts,
  color-data flag, bounding-box extents.
- ThreeMfDef: OPC ZIP unzip (fflate), model-part + slicer-config parse
  (objects, materials, plates, print parts, extruders, bounds).
- Register .stl/.3mf in file-def-code-ref; add fflate dependency.

Shaded raster PNG previews are intentionally deferred to CS-12401; the
SVG silhouette is the preview until then.

CS-12053, CS-12054

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files      1 suites   2h 56m 41s ⏱️
3 821 tests 3 807 ✅ 14 💤 0 ❌
3 840 runs  3 826 ✅ 14 💤 0 ❌

Results for commit 1dcc629.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   14m 26s ⏱️ +34s
2 032 tests ±0  2 032 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 111 runs  ±0  2 111 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 1dcc629. ± Comparison against earlier commit 739608c.

FadhlanR and others added 10 commits July 31, 2026 14:32
- Register `fflate` as an async module shim in host externals.ts and add
  it to host deps. Bare imports in realm-served base modules resolve only
  via the shim registry, not node_modules, so the extractor's
  `import 'fflate'` would 404 at runtime without this.
- Render 3MF materialNames via a joined getter instead of interpolating a
  string[] directly (glint: string[] is not a ContentValue).
- prettier formatting.

CS-12053, CS-12054
isolated + embedded now mount an interactive three.js orbit viewer
(ModelViewer + renderModel modifier), matching the handoff prototype.
three.js + OrbitControls + STL/3MFLoader load lazily from a CDN
(esm.sh) at client render time only — the sanctioned Boxel pattern for
card libraries — so the engine never runs during extraction/indexing.

- fitted keeps the pure SVG silhouette (collection-tile budget).
- The silhouette is the loading placeholder and the fallback whenever
  WebGL/the CDN engine is unavailable (e.g. prerender), so the static
  prerendered HTML degrades gracefully.
- Full GPU teardown on unmount (dispose + forceContextLoss).

CS-12053, CS-12054
The WebGL viewer's `fetch(url, { credentials: 'include' })` was blocked
cross-origin (host 4200 → realm 4201): a credentialed CORS request is
illegal against the realm's wildcard Access-Control-Allow-Origin, so the
STL/3MF bytes never loaded and every model fell back to the silhouette.

Drop `credentials: 'include'` — realm auth is carried by an Authorization
header injected by the host auth service worker on the GET (the same path
that lets <img src> load realm images), not by cookies. Verified in the
browser: complex STL and moderate 3MF now render in the live orbit viewer.

CS-12053, CS-12054
Six sample model files (stl + three-mf, simple/moderate/complex) under
experiments/model-samples/ so StlDef/ThreeMfDef can be exercised manually
in the app — each is indexed directly as its FileDef subtype.

CS-12053, CS-12054
Rework the isolated view to mirror the handoff prototype: a header bar
with a mono extension pill, a bordered live-viewer stage, and a two-column
(stage + property inspector) body that collapses to one column on narrow
containers. Metadata reads as grouped "3D model" / "STL mesh" / "3MF
package" sections with uppercase mono labels; the viewer hint is a small
mono chip bottom-right.

Also aligns tokens with the design conventions: semantic role tokens
(--foreground/--muted-foreground/--card/--muted/--border) instead of
numbered palette values, rem units, and no `font` shorthand.

CS-12053, CS-12054
Match the handoff realm's inspector: wrap the property groups in a
bordered, rounded --card panel and separate rows with hairline top
borders (first row none), with mono uppercase group headings. Applied to
the shared "3D model" group and the leaf STL/3MF groups.

CS-12053, CS-12054
Fitted now mounts the WebGL viewer instead of only the silhouette, but:
- lazy via IntersectionObserver — the engine boots only when the tile is
  on-screen, so a grid of many models doesn't exhaust the browser's WebGL
  context budget; off-screen tiles keep the silhouette.
- non-interactive — no orbit controls, no gesture trapping, no hint, so
  fitted tiles render a static model and the enclosing grid scrolls
  normally.

During prerender/indexing the engine never loads (no CDN/WebGL), so fitted
still falls back to the silhouette there. Verified in the browser across
badge/strip tile sizes.

CS-12053, CS-12054
Extract the STL/3MF metadata parsers into pure `.ts` modules
(stl-meta-extractor / three-mf-meta-extractor, mirroring
png-meta-extractor) so they're unit-testable without the card-api
harness, and harden them per review:

- STL parsing is now a single streaming pass over the DataView with no
  per-vertex array, so index-time memory stays flat regardless of model
  size; degeneracy is counted per facet as read, fixing the misaligned
  count when non-finite vertices were dropped.
- parseThreeMf returns undefined for any unparseable input (non-ZIP,
  no model part, malformed XML) so the leaf falls back cleanly to base
  FileDef; removed a redundant modelPart regex clause.
- Both leaves take an optional fileSizeLimitBytes, defaulting to the
  realm's standard DEFAULT_FILE_SIZE_LIMIT_BYTES. Over the cap they skip
  the parse and return base attributes only (keeping the 3D type + live
  viewer), rather than throwing and demoting to a plain file.
- Share Model3dData and getExtension; replace the ~40-line inspector
  style block duplicated across the three isolated templates with a
  data-driven ModelInspectorSection component.
- Document that model3d.vertices counts vertex records as the format
  stores them (not comparable across formats).
- Add unit tests for parseStl, parseThreeMf, and silhouettePath.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drives the real render/file-extract route (the indexer's path) over an
ASCII STL and a real 3MF ZIP served from a test realm, asserting the
file-meta search doc carries the parsed model3d / stlMetadata /
threeMfMetadata, plus the mismatch-fallback for a .stl whose bytes
aren't STL.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "Build test web assets" (rolldown) job failed with MISSING_EXPORT
cascading from card-api.gts: the unit test value-imported silhouettePath
from model-file-def.gts, which pulled model-file-def -> card-api into the
host test bundle, where card-api's long-standing loose type imports
(CodeRef/ToolContext/LocalPath/SingleCardDocument imported without the
`type` keyword) become hard rolldown errors.

Move silhouettePath into a pure model-silhouette.ts (no card-api
dependency) and import the test from there; model-file-def re-exports it
for back-compat. No host test now value-imports a card-api-chained .gts,
so card-api is no longer bundled into the test assets.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FadhlanR
FadhlanR marked this pull request as ready for review July 31, 2026 13:24
@FadhlanR
FadhlanR requested a review from a team July 31, 2026 13:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b56856a21

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

): { model3d: Model3dData; threeMfMetadata: ThreeMfMetadata } | undefined {
let files: Record<string, Uint8Array>;
try {
files = unzipSync(new Uint8Array(buf)) as Record<string, Uint8Array>;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bound decompressed 3MF data before unzipping

For a user-supplied 3MF containing a highly compressible entry, the caller's size cap covers only the compressed archive, while unzipSync eagerly allocates every decompressed entry. A sub-5 MB ZIP bomb can therefore consume hundreds of megabytes or more and stall or OOM the indexing process; inspect declared uncompressed sizes and enforce an aggregate/output limit, or use bounded streaming extraction.

Useful? React with 👍 / 👎.

Comment on lines +122 to +126
let vertices = elements('vertex')
.map((element) =>
['x', 'y', 'z'].map((axis) => Number(element.getAttribute(axis))),
)
.filter((vertex) => vertex.every(Number.isFinite));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply 3MF transforms when calculating bounds

For 3MF files that place objects through <item transform> or compose them through <component transform>, these raw resource coordinates are not the coordinates of the assembled model. For example, two instances of a 10-unit object placed 100 units apart still report sizeX as 10, so the indexed bounds and generated silhouette are wrong; compute bounds by traversing the build/component graph with accumulated transforms rather than aggregating every stored vertex directly.

Useful? React with 👍 / 👎.

// geometry); it just has empty inspector panels and a generic silhouette.
// Do NOT throw FileContentMismatchError here: that would demote the file to
// a plain FileDef and lose the 3D card entirely.
let sizeCap = options.fileSizeLimitBytes ?? DEFAULT_FILE_SIZE_LIMIT_BYTES;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the configured file-size limit for extraction

In deployments that configure a realm file limit above the default, valid STL and 3MF files between that configured limit and DEFAULT_FILE_SIZE_LIMIT_BYTES are accepted but silently lose all model metadata. FileDefAttributesExtractor currently passes only contentHash, contentSize, and toolContext in its options, so fileSizeLimitBytes is never populated and both new extractors always fall back to the hard-coded default; thread the configured value through the extraction call.

Useful? React with 👍 / 👎.

Comment thread packages/base/model-file-def.gts Outdated
Comment on lines +369 to +373
io = new IntersectionObserver((entries) => {
if (entries.some((entry) => entry.isIntersecting)) {
io?.disconnect();
start();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Release lazy viewers when tiles leave the viewport

When fitted tiles remain mounted while a user scrolls through a collection, the observer disconnects permanently after first intersection, while each viewer keeps its WebGL context and animation loop until component teardown. Consequently the number of active contexts grows with every tile ever viewed rather than the number currently visible, eventually exhausting browser context limits and breaking later previews; observe exit events and suspend/dispose off-screen viewers.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Resolved in 1dcc629 — and reframed: the fix isn't format-specific, it's prerender vs live client.

  • Live client: the viewer's modifier now lazy-boots when the tile scrolls on-screen and disposes the WebGL context (renderer.dispose() + forceContextLoss(), canvas removed) when it scrolls off, re-booting from cached bytes on re-entry. So live contexts track the currently visible set, not "tiles ever viewed" — the monotonic growth is gone.
  • Prerender/indexing: the modifier now no-ops entirely when globalThis.__boxelRenderContext is set, so server-side rendering never boots WebGL or fetches the CDN engine (deterministic, rather than relying on the CDN import failing).
  • Fitted specifically: it no longer mounts the viewer at all. A grid can show more tiles than the browser's ~16-context cap at once, which lazy+dispose can't solve (that only bounds scrolling, not simultaneous visibility), so fitted uses the static thumbnail. A pooled/shared-context renderer for interactive fitted is tracked as Option B in CS-12401.

Comment on lines +73 to +77
let document = new DOMParser().parseFromString(
strFromU8(bytes),
'application/xml',
);
if (document.getElementsByTagName('parsererror').length) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate that model parts are actually 3MF XML

A ZIP containing any well-formed XML file whose name ends in .model passes this check even if its root is unrelated to 3MF, such as <document/>. The parser then returns zero-valued metadata instead of undefined, so mislabeled or corrupt uploads are stamped as ThreeMfDef and sent to a viewer that cannot parse them; require the 3MF model root/core namespace and valid model structure before accepting the package.

Useful? React with 👍 / 👎.

Comment thread packages/base/model-file-def.gts Outdated
// the thumbnail seam, and the four format templates. Leaves (StlDef,
// ThreeMfDef) add their format-specific metadata field + extraction and may
// override `isolated` to append that metadata below the shared body.
export class ModelDef extends FileDef {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's name this 3DModelDef (and rename file accordingly) -- ModelDef sounds too much like some kind of base class.

@lukemelia

lukemelia commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What do we lose if we only read the headers of these files and not the whole thing? Parsing 5MB to index should be avoided if possible.

@habdelra habdelra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖]

Lens. I focused on the index-time extraction path — where the parsers actually run and where the feature's cost/correctness live — and on how the new size-cap machinery wires into the existing FileDefAttributesExtractor, since that seam isn't visible from the diff alone. I checked out the head branch and traced every extraction call from runFileExtract down through both leaves; the template/viewer layer I reviewed more lightly.

Bottom line. Clean, well-tested feature — the pure .ts parsers, the card-api-free silhouette, the unit + real-route acceptance tests, and the mismatch→base-FileDef fallback are all nicely done, and CI is green. No blocking bug for a default deployment. The one thing worth fixing before merge-and-forget is documentation-level: the fileSizeLimitBytes "threaded seam" is never populated by its only caller, and its "can't drift" comment is false once FILE_SIZE_LIMIT_BYTES is raised. The five automated P1/P2 flags are all reachable — I verified each and added mechanism + fix in-thread; none block, but the ZIP-bomb and lazy-viewer ones deserve a follow-up.

What lands right.

  • Extracting the parsers to pure .ts modules (mirroring png-meta-extractor.ts) makes them directly unit-testable and keeps card-api out of the test bundle — and the model-silhouette.ts split correctly resolved the rolldown MISSING_EXPORT cascade.
  • The STL single-pass streaming parse with per-facet degeneracy counting is the right shape, and the degenerate-facet test pins exactly the misalignment the old grouping had.
  • Over-cap behavior keeps the 3D type + live viewer and only drops the inspector metadata (rather than throwing and demoting to a plain file) — good instinct, well-commented.

On the open (automated) threads. All five are real and reachable; verification and recommended fixes are in each thread. Priority order: (1) is the one I'd act on in this PR (cheap, and the comment is currently misleading); (2) and (5) are the hardening ones; (3) and (4) are metadata-accuracy nits.

Recommendations (all non-blocking; none require re-architecture):

  1. Either thread environmentService.fileSizeLimitBytes through to extractAttributes, or drop the parameter and reword the comment — thread on stl-model-def.gts.
  2. Bound 3MF decompression via fflate's filter/originalSize and restrict to the entries you parse — thread on three-mf-meta-extractor.ts:58.
  3. Treat STL/3MF sizeX/Y/Z as un-transformed resource-space extents, not assembled dimensions (viewer is correct) — thread on three-mf-meta-extractor.ts:126.
  4. Require a <model> root before accepting a 3MF, and add the negative-space test — thread on three-mf-meta-extractor.ts:77.
  5. Release/suspend fitted WebGL viewers on viewport exit if the grid doesn't virtualize — thread on model-file-def.gts:373.

Adjacent, out of scope. Binary-STL color detection treats any non-zero attribute-byte-count as "has color," which over-reports for STLs that use those bytes for non-color data — fine for a metadata flag, just noting it. And there are now three small extension-parsing helpers (getExtension, ModelIsolatedBody.extension, and file-def-code-ref's extensionOfName); not worth consolidating across packages here.

This is a COMMENT review — approve / request-changes is the maintainers' call.


Generated by Claude Code

Comment thread packages/base/stl-model-def.gts Outdated
Comment on lines +166 to +170
// Backstop bound on the bytes we're willing to parse at index time,
// threaded from the host; defaults to the realm's standard file-size
// limit (`DEFAULT_FILE_SIZE_LIMIT_BYTES`), the same ceiling the write
// path enforces, so the two can't drift.
fileSizeLimitBytes?: number;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirming the automated P2 flag on fileSizeLimitBytes (the codex thread on three-mf-def.gts:286), and one step further: the seam is never populated by any caller, and this comment's "can't drift" claim is currently false. Blocking-wise: non-blocking (default deployments are unaffected), but worth resolving since the PR description advertises this as a threaded seam. Regression class: introduced by this PR.

What I traced. The only production caller of a leaf extractAttributes is FileDefAttributesExtractor in packages/host/app/utils/file-def-attributes-extractor.ts. Its tryExtract calls klass.extractAttributes(url, stream, { contentHash, contentSize, toolContext }) — no fileSizeLimitBytes. The FileDefExport type in that same file doesn't declare fileSizeLimitBytes either, so TypeScript never flags the omission. So options.fileSizeLimitBytes here is always undefined and sizeCap is always DEFAULT_FILE_SIZE_LIMIT_BYTES, regardless of realm configuration.

Why the comment is wrong. The write path's ceiling is not fixed at the default: realm.ts and server.ts both read Number(process.env.FILE_SIZE_LIMIT_BYTES ?? DEFAULT_FILE_SIZE_LIMIT_BYTES). A deployment that sets FILE_SIZE_LIMIT_BYTES above 5 MB accepts STL/3MF files up to that limit on write, but this extractor still caps the parse at the hard-coded 5 MB — so files between 5 MB and the configured limit are stamped with the 3D type but silently get no model3d/metadata. That's exactly the drift the comment says can't happen.

The fix is cheap because the host already has the value. serve-index.ts injects the server's fileSizeLimitBytes into host config, and environment-service.ts exposes it as environmentService.fileSizeLimitBytes. Thread that through runFileExtractFileDefAttributesExtractor → the tryExtract options (and add the field to FileDefExport), or — if you'd rather not wire it now — drop the fileSizeLimitBytes parameter and reword this comment to say the cap is a fixed 5 MB backstop. Either is fine; the in-between state documents behavior that doesn't exist.

The identical comment + read applies verbatim in three-mf-def.gts.


Generated by Claude Code

Comment on lines +56 to +62
let files: Record<string, Uint8Array>;
try {
files = unzipSync(new Uint8Array(buf)) as Record<string, Uint8Array>;
} catch {
// Not a valid ZIP / OPC package.
return undefined;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirming the automated P1 (bound decompressed 3MF before unzipping). unzipSync here eagerly decompresses every entry in the archive into memory, and the only size gate upstream is on the compressed bytes (the write limit, default 5 MB). A ~5 MB archive of highly compressible data can expand to hundreds of MB–GBs; since this runs on the indexing/prerender path, an OOM there stalls indexing for the whole worker, not just this file.

Scope/severity. Realm files come from authenticated realm members, not anonymous public uploads, so this is hardening rather than an open DoS — non-blocking, follow-up. But it's a real gap and the fix is small.

Concrete mitigation with the library you already use. fflate's unzipSync(data, opts) takes filter: (file: UnzipFileInfo) => boolean, which runs before each entry is decompressed and receives { name, originalSize, compression, ... }. Two cheap wins: (1) you only ever read *.model and model_settings.config, so filter to just those entries — everything else is skipped, not decompressed; (2) reject any entry whose declared originalSize exceeds a sane per-entry ceiling. Declared sizes can be forged, so for a hard guarantee you'd stream (unzip) with an output-byte counter, but the filter alone removes the obvious bomb and bounds the work to the entries you actually parse.


Generated by Claude Code

Comment on lines +122 to +138
let vertices = elements('vertex')
.map((element) =>
['x', 'y', 'z'].map((axis) => Number(element.getAttribute(axis))),
)
.filter((vertex) => vertex.every(Number.isFinite));
let mins = [Infinity, Infinity, Infinity];
let maxs = [-Infinity, -Infinity, -Infinity];
for (let vertex of vertices) {
for (let axis = 0; axis < 3; axis++) {
mins[axis] = Math.min(mins[axis], vertex[axis]);
maxs[axis] = Math.max(maxs[axis], vertex[axis]);
}
}
let dimension = (axis: number) =>
vertices.length
? Math.round((maxs[axis] - mins[axis]) * 1_000_000) / 1_000_000
: undefined;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirming the automated P2 (bounds ignore 3MF transforms). dimension() is computed from the min/max of every <vertex> aggregated across all objects/model parts in raw resource coordinates — <build><item transform> and <component transform> are never applied. For a single-object identity-transform model (the common slicer export, and what the tests cover) this is correct, but for a multi-instance or composed build it's wrong: the classic case is two instances of a 10-unit object placed 100 units apart, which still reports sizeX = 10 instead of ~110. It also unions unrelated objects' local coordinate systems overlaid at the origin.

What it affects / doesn't. Only the indexed sizeX/Y/Z and the silhouette proportions (which read extents). The live WebGL viewer is unaffected — 3MFLoader applies the build/component transforms itself, so the on-screen model is correct; it's just the extracted numbers that are approximate.

Follow-up, not this PR — a correct implementation means walking the build graph with accumulated transforms, which is real work for a metadata nicety. Non-blocking; I'd just avoid treating sizeX/Y/Z as authoritative assembled dimensions until then.


Generated by Claude Code

Comment on lines +77 to +84
if (document.getElementsByTagName('parsererror').length) {
// Malformed model XML — treat the whole package as unparseable.
return undefined;
}
modelDocuments.push({ path, document, root: document.documentElement });
}
let primary =
modelDocuments.find(({ path }) => path === modelPart) ?? modelDocuments[0];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirming the automated P2 (accepts any .model XML). The only gates to "this is a 3MF" are: it's a ZIP, it has an entry matching /\.model$/i, and that entry is well-formed XML (no <parsererror>). A ZIP containing <document/> in a file named x.model passes all three — parseThreeMf then returns an all-zero/empty metadata object (not undefined), so the file is stamped ThreeMfDef and handed to a viewer that can't parse it.

Fix. Require the 3MF root before accepting: check primary.root.localName === 'model' (and ideally the core namespace http://schemas.microsoft.com/3dmanufacturing/core/...), returning undefined otherwise so it falls back to base FileDef like the other unparseable cases.

Test gap. model-meta-extractor-test.ts covers malformed XML, missing model part, and non-ZIP, but not "well-formed XML that isn't a 3MF <model> root" — that's the negative-space test that would pin this. Worth adding alongside the fix. Minor / non-blocking.


Generated by Claude Code

Comment thread packages/base/model-file-def.gts Outdated
Comment on lines +367 to +378
let io: IntersectionObserver | undefined;
if (lazy) {
io = new IntersectionObserver((entries) => {
if (entries.some((entry) => entry.isIntersecting)) {
io?.disconnect();
start();
}
});
io.observe(element);
} else {
start();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirming the automated P2 (lazy viewers never released), with one conditional. The IntersectionObserver calls io.disconnect() on first intersection (line 371) and boots the viewer; the WebGL context + RAF loop it creates are only torn down in the modifier's destructor — i.e. on component unmount. So the number of live WebGL contexts tracks "tiles ever scrolled past," not "tiles currently visible."

Whether this bites depends on the container. If the enclosing collection virtualizes (unmounts off-screen tiles), the destructor runs on scroll-away and this is fine. If it keeps all tiles mounted, contexts grow monotonically and will hit the browser's ~16-context ceiling, after which new viewers fail and later previews break (WebGL context-lost / silhouette fallback). Worth confirming which case the fitted grid is in.

Fix if needed. Keep the lazy boot, but don't disconnect() permanently — keep the observer and react to both isIntersecting transitions, disposing/suspending the renderer on exit and re-booting on re-entry. Non-blocking; follow-up unless the target grid is non-virtualized.


Generated by Claude Code

…ent-side size

Rework STL/ThreeMf FileDefs per review feedback:

- Rename ModelDef -> ThreeDModelDef (and model-file-def.gts ->
  three-d-model-def.gts) so the base class no longer reads like a shared
  base type.
- Fitted view now renders a static cube icon + filename (mirroring the audio
  FileDef) instead of a live per-tile WebGL viewer, so a grid of tiles can no
  longer exhaust the browser's WebGL context budget. The shaded-thumbnail seam
  (`thumbnailUrl`) is kept for CS-12401.
- Drop the deterministic SVG silhouette (model-silhouette.ts) and the
  index-time bounding box (sizeX/Y/Z). Physical dimensions now come from the
  live client-side viewer, which reads the true, transform-correct bounds off
  the loaded geometry — so the 3MF-transform approximation disappears too.
- STL extraction is header-only: it reads the binary header (facet count,
  COLOR= flag) or the ASCII prologue (solid name) and never scans the facet
  body.
- 3MF extraction is a bounded, DOM-free prologue read: fflate's filter
  decompresses only the `.model` + `model_settings.config` entries (skipping
  embedded thumbnails/textures and refusing oversized entries as a ZIP-bomb
  backstop), and metadata is regex-read from the model part's prologue rather
  than DOM-parsing the geometry. Also validates the 3MF `<model>` core root so
  unrelated `.model` XML is rejected.
- Thread the realm's configured fileSizeLimitBytes through
  FileDefAttributesExtractor -> the leaf extractAttributes, so the size cap
  tracks the write-path limit instead of a hard-coded default.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FadhlanR

FadhlanR commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖]

Good push on the header question — we reworked extraction and previews around it (739608c).

On "avoid parsing 5MB to index": the answer is asymmetric between the two formats, so we changed both.

  • STL is now header-only. We read the binary header (facet count, COLOR= flag) or the ASCII prologue (solid name) and never scan the facet body. Neither format's header carries a bounding box, so dimensions can't come from a header read at all — they now come from the live client-side viewer, which reads the true bounds off the geometry it already loads (and is transform-correct, unlike the old index-time scan).
  • 3MF is now a bounded prologue read. The useful metadata sits at the top of the model part (root namespaces/unit, <metadata>, <basematerials>), before the geometry. So fflate's filter decompresses only the .model + model_settings.config entries (skipping embedded thumbnails/textures), and we regex-read the metadata prologue instead of DOM-parsing the vertex/triangle body. One caveat I'll be straight about: we still inflate the model entry's compressed stream (fast, and capped), so 3MF's win is "no geometry DOM-parse + skip unrelated entries," not a literal zero-read. A true zero-geometry-decompress would need entry-level streaming, which we can add if profiling shows the inflate itself matters.

Previews: fitted now shows a static cube icon + filename (mirroring the audio FileDef) instead of booting a live WebGL viewer per tile — the shaded-thumbnail seam (thumbnailUrl) stays for CS-12401. Embedded/isolated keep the live viewer and surface the client-side dimensions.

Nice side effect: this also closes the automated flags in one go — the ZIP-bomb (bounded decompression + oversized-entry rejection), the WebGL-context exhaustion (see the follow-up below), the transform-incorrect bounds (dimensions are client-side now), and the <model>-root validation (3MF now requires the core namespace before it's accepted). And fileSizeLimitBytes is now actually threaded from realm config, so the cap tracks the write limit instead of a hard-coded default.

Also renamed ModelDefThreeDModelDef per your other comment — 3DModelDef can't be the literal class name (it's the adoptsFrom export identifier, and JS identifiers can't start with a digit), so I spelled out the leading digit to match the existing ThreeMfDef convention.


Follow-up (1dcc629) — correcting the WebGL point above. Two things: (a) 739608c alone did not fully close the WebGL-context finding — fitted was static, but embedded still booted eagerly and never released its context, so an embedded strip could still leak. (b) The real fix generalizes the gate to prerender vs live client, not format:

  • Server-side prerender never boots WebGL or the CDN import (globalThis.__boxelRenderContext guard) — deterministic, not fail-fallback.
  • The live viewer now disposes its WebGL context when a tile scrolls off-screen and re-boots from cached bytes on re-entry, so embedded strips are bounded too, not just fitted.
  • Fitted stays on the static thumbnail because a grid can exceed the browser's ~16-context cap simultaneously (lazy+dispose only bounds scrolling). A pooled/shared-context renderer for interactive fitted is Option B in CS-12401.

…creen

Reframe the WebGL/static split around render context rather than format, and
fix the context-budget leak the review flagged:

- Prerender gate: `ModelViewer`'s modifier no-ops when
  `globalThis.__boxelRenderContext` is set (server-side indexing/prerender), so
  WebGL and the CDN engine import are never started there — the static thumbnail
  is the deterministic prerender representation, instead of relying on a doomed
  CDN import failing.
- Viewport gate: the viewer now lazy-boots when the element scrolls on-screen
  and disposes its WebGL context (renderer.dispose + forceContextLoss) when it
  scrolls off, re-booting from cached bytes on re-entry. So an embedded strip of
  models holds a context only for the visible ones and never exhausts the
  browser's ~16-context budget — the actual fix to the "contexts grow
  monotonically" finding.

Fitted stays on the static thumbnail (a grid can show more tiles than the
context cap at once; pooled-renderer live fitted is tracked in CS-12401).

Co-Authored-By: Claude Opus 4.8 (1M context) <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.

3 participants