Spaces:
Running
Running
| # Renderer Vocabulary: Richer Geometry, Motion, and Part Counts | |
| Review date: 2026-06-14. Reviewer: Claude (Opus 4.8), for OpenAI Codex to | |
| implement. Scope: one user goal — the vision prompt should be **more concise and | |
| descriptive**, and the pipeline should give the model **more freedom over the | |
| built-in Three.js geometry/motion vocabulary** and a **larger range of parts**. | |
| This document is findings + next steps. It does **not** authorize any deployment | |
| or Hugging Face changes on its own; follow the normal GitHub → HF sync and | |
| verification flow in `AGENTS.md`. File/line references are to the repo state at | |
| review time. | |
| User decisions captured for this spec (2026-06-14): | |
| - **Shapes:** add the *high-value set* — `cone`, `capsule`, `torus`, and a | |
| `spring` (helix) builder — on top of the existing 5. | |
| - **Motions:** add `screw` (helical: rotate + translate together), `orbit` | |
| (revolve around a pivot point), and `pulse` (scale breathing) on top of the | |
| existing 4. | |
| - **Parts:** raise the usable cap to **6** (the renderer already allows 6). | |
| --- | |
| ## TL;DR — the one architectural fact that drives everything | |
| **The prompt cannot grant freedom the renderer doesn't already have.** Because of | |
| the hard security invariant — *no model-authored HTML; render deterministically | |
| from validated JSON* (`AGENTS.md` Runtime Notes, `SECURITY.md` Agent Guidance, | |
| `docs/reviews/security-hardening.md` §3) — the only things that can appear on | |
| screen are shapes/motions | |
| that the **browser renderer** explicitly builds in `buildPartMesh` | |
| (`index.html:1141`) and `applyMotion` (`index.html:1195`). | |
| So today the vocabulary is capped **identically in four places**, and any | |
| expansion must change all four *in lockstep* (renderer is the source of truth): | |
| | Layer | Shapes | Motions | Parts cap | | |
| | --- | --- | --- | --- | | |
| | Prompt (`snap2sim/prompts.py:26-27`, `:32`) | box, cylinder, sphere, gear, rod | rotate, translate, oscillate, static | "2 to 4" | | |
| | Schema enum (`snap2sim/schema.py:136-139`, `:165-169`, `:214-215`) | same 5 | same 4 | no max | | |
| | Coercion (`snap2sim/model_io.py:166`, `:169`, `:117`) | same 5 (else→box) | same 4 (else→static) | `parts[:4]` | | |
| | Renderer (`index.html:842`, `:1145-1156`, `:1202-1210`, `:929`) | same 5 (else→box) | same 4 | `slice(0,6)` | | |
| Two consequences: | |
| 1. **Parts is already inconsistent**: renderer renders 6, but the prompt asks for | |
| ≤4 and coercion truncates to 4. Raising the usable count to 6 is mostly | |
| removing an artificial cap (see §3). | |
| 2. **Adding shapes/motions is a 4-layer change**, not a prompt edit. The order | |
| that avoids a broken intermediate state is: **renderer → schema → coercion → | |
| prompt** (build the capability, allow it, coerce toward it, then ask for it). | |
| --- | |
| ## Finding 1 — The prompt is verbose and under-describes the vocabulary it already has | |
| `build_vision_prompt` (`snap2sim/prompts.py:13-56`) is ~40 lines and embeds a | |
| full JSON skeleton, but: | |
| - It lists shapes/motions as **bare enums** (`prompts.py:26-27`) with **zero | |
| guidance on when to use each**. The model gets `box, cylinder, sphere, gear, | |
| rod` and must guess mapping. In practice the example payload | |
| (`EXAMPLE_ANALYSIS`, `schema.py:11-110`) only ever uses box/cylinder/gear, so | |
| `sphere` and `rod` are effectively invisible to the model. The vocabulary is | |
| underused before we even expand it. | |
| - It repeats constraints (e.g. "compact", "physically plausible", "short" appear | |
| multiple times across `:15-18`, `:31`, and the example) and spends many lines | |
| re-stating field rules already enforced by `schema.py` / coerced by | |
| `model_io.py`. | |
| - `VISION_SYSTEM_PROMPT` (`prompts.py:5-10`) is terse and doesn't frame the | |
| model's strengths (it's a *Reasoning* GGUF per `AGENTS.md`) or the "annotated | |
| cutaway" product goal. | |
| **Net:** the prompt is long on rules and short on *descriptive guidance*, which | |
| is the opposite of what helps a reasoning model pick good primitives. | |
| **A cheap win exists independent of any renderer change:** add a terse | |
| "shape → use for" and "motion → use for" guide so the model exploits the | |
| existing 5/4 vocabulary. That guide then grows naturally when we add the new | |
| primitives. | |
| --- | |
| ## Finding 2 — High-value Three.js primitives are unused; the cutaway aesthetic wants them | |
| The renderer hand-builds geometry in `buildPartMesh` (`index.html:1141-1167`) | |
| and `gearGeometry` (`index.html:1169-1187`). Three.js ships many more primitives | |
| that map cleanly onto real mechanism elements. The user picked the **high-value | |
| set**: | |
| | New shape | Three.js primitive | Real mechanism elements it covers | | |
| | --- | --- | --- | | |
| | `cone` | `ConeGeometry(radius, height, seg)` | valve cones, drill/screw tips, springs' seats, nozzles, pawl points | | |
| | `capsule` | `CapsuleGeometry(radius, length, …)` | pistons, dowel pins, rollers, bearings, plungers, shafts with rounded ends | | |
| | `torus` | `TorusGeometry(radius, tube, …)` | o-rings, retaining/snap rings, coils, washers (thick), seals | | |
| | `spring` | custom helix via `TubeGeometry` + `CatmullRomCurve3` (like `gearGeometry` is custom) | compression/extension springs, coils, helical elements — *very* common and currently impossible to depict | | |
| > Note on a latent inconsistency to clean up while here: the current `cylinder` | |
| > branch (`index.html:1146`) already secretly supports a **cone/taper** by using | |
| > `size[0]/2` as top radius and `size[2]/2` as bottom radius — but `size` is | |
| > documented everywhere else as `[x, y, z]` extents, and nothing tells the model | |
| > this. Introducing an explicit `cone` shape (with its own `size` semantics) | |
| > removes the need for that undocumented trick. Decide whether `cylinder` should | |
| > revert to a true cylinder (`radiusTop == radiusBottom`) once `cone` exists; | |
| > recommended yes, to keep `size` semantics consistent. | |
| ### `size` semantics for the new shapes (must be documented in the prompt) | |
| Keep the existing convention: `size: [x, y, z]` are bounding extents, and the | |
| builder derives radii/lengths from them so the model never has to send | |
| `radius`/`height` (which the prompt already forbids, `prompts.py:28-30`). Proposed: | |
| - `cone`: base diameter from `max(x, z)`, height from `y`. Apex +y. | |
| - `capsule`: diameter from `max(x, z)`, total length from `y` (long axis +y; | |
| reuse the `rod` pattern if a horizontal default reads better — match `rod`'s | |
| `rotateX` convention at `index.html:1151` for consistency). | |
| - `torus`: outer diameter from `max(x, z)`, tube thickness from `y` (clamp tube | |
| to a fraction of radius so it stays a ring, not a sphere). | |
| - `spring`: outer diameter from `max(x, z)`, length/height from `y`; expose | |
| `coils` (int, like `teeth` for gear, `schema.py:158`) and an optional | |
| `wire` thickness with sane defaults. | |
| All four must also be reachable through the same `color`, `rotation`, and | |
| `position` handling that already wraps `buildPartMesh` | |
| (`index.html:929-940`, color at `index.html:1158`/`colorFor` `:1228`). | |
| --- | |
| ## Finding 3 — Parts cap is artificially low and inconsistent | |
| - Prompt says **"Use 2 to 4 parts"** (`prompts.py:32`). | |
| - Coercion truncates with **`parts[:4]`** (`model_io.py:117`). | |
| - Renderer already maps **`slice(0, 6)`** (`index.html:929`) and labels project | |
| per mesh (`updateLabels`, `index.html:1213`). | |
| - Schema imposes **no max** on `parts` (`schema.py:123-125`). | |
| **Decision: usable cap = 6.** This is mostly *removing* a cap: | |
| - Prompt: change "Use 2 to 4 parts" → "Use **2 to 6** parts; prefer the fewest | |
| that explain the mechanism" (`prompts.py:32`). | |
| - Coercion: `parts[:4]` → `parts[:6]` (`model_io.py:117`). | |
| - Renderer: already 6 — no change, but double-check label legibility / overlap at | |
| 6 parts (`updateLabels` clamps at `index.html:1221-1222`). | |
| - Schema: optionally add `"maxItems": 6` to the `parts` array | |
| (`schema.py:123`) so over-long payloads fail validation predictably instead of | |
| being silently truncated only on the coercion path (note: `model_io.py` runs | |
| only on the Modal path, per `docs/reviews/interaction-and-fallback-review.md`; | |
| the Space `validate_analysis` path | |
| does **not** truncate, so a schema `maxItems` keeps both paths consistent). | |
| --- | |
| ## Finding 4 — Motions are hand-coded, not "built-in"; expand the custom set | |
| `applyMotion` (`index.html:1195-1211`) implements all motion by hand: `rotate` | |
| (continuous spin on `axis`), `oscillate` (sinusoidal rotation), `translate` | |
| (sinusoidal slide along `axis` within `range`), `static`. Three.js has no | |
| "motion" concept of its own, so "more built-in motions" = **add more primitives | |
| to `applyMotion`**. User picked: | |
| | New motion | Behavior | Mechanism examples | Params (extend schema) | | |
| | --- | --- | --- | --- | | |
| | `screw` | rotate **and** translate along the same `axis` together (helical) | screws, lead screws, drill bits, augers, twist mechanisms | reuse `axis`, `speed`, `phase`, `range` (translate extent) + e.g. `pitch` | | |
| | `orbit` | revolve the part **around a pivot point** (not its own center) | planetary/idler gears, cranks, eccentrics, governor weights | `axis`, `speed`, `phase` + a `pivot: [x,y,z]` | | |
| | `pulse` | sinusoidal **scale** breathing | diaphragms, bladders, pumps, bellows, valves opening/closing | `speed`, `phase`, `amplitude` | | |
| Implementation notes for `applyMotion`: | |
| - It already resets to base each frame (`mesh.position.copy(basePosition)`, | |
| `mesh.rotation.copy(baseRotation)`, `index.html:1200-1201`) — good; `screw` and | |
| `orbit` can compose translate+rotate cleanly on top of that. | |
| - `orbit` needs the pivot in world space; store it on `mesh.userData` at build | |
| time like `basePosition` (`index.html:936`). Offset = position − pivot, rotate | |
| the offset by `axis`, re-add pivot. | |
| - `pulse` should multiply the **reveal** scale (`revealMesh`, `index.html:1189`) | |
| rather than fight it — guard so the staggered reveal still plays first. | |
| - Keep the existing param defaults pattern (`speed||1`, `phase||0`, | |
| `amplitude||0.25`, `range||[-0.25,0.25]`, `index.html:1197-1207`). | |
| Schema additions: extend the `motion` enum (`schema.py:165-169`, `_MOTIONS` | |
| `:215`) and add optional `pitch` (number) and `pivot` (3-number list) to the | |
| `motion` properties (`schema.py:162-186`), validated with the existing | |
| `_require_number_list` helpers (`schema.py:326-329`). Coercion: extend the | |
| allowed set (`model_io.py:169`) and carry the new params through (mirror | |
| `_axis_vector`, `model_io.py:173-175`, `:290`). | |
| --- | |
| ## Finding 5 — The renderability gate and validators must learn the new vocabulary too | |
| Three guards currently encode the old vocabulary and will silently reject or | |
| downgrade the new shapes if missed: | |
| 1. **Browser renderability check** (`index.html:840-844`): the | |
| `["box","cylinder","sphere","gear","rod"].includes(geometry.shape)` test | |
| decides whether a part counts as renderable (feeds `render_mode` / | |
| annotate-fallback selection, `schema.py:select_render_mode`). Add the new | |
| shapes here or a part with `shape:"spring"` will be treated as non-renderable | |
| and fall back to annotate-only. | |
| 2. **Schema validator** (`schema.py:_SHAPES` `:214`, `_MOTIONS` `:215`, enums | |
| `:136-139`/`:165-169`) — extend both sets/enums. | |
| 3. **Coercion fallbacks** (`model_io.py:166-167`, `:169-170`) — extend the | |
| allowed sets so a valid new shape isn't rewritten to `box`/`static`. | |
| Also update `EXAMPLE_ANALYSIS` (`schema.py:11-110`) to exercise at least a couple | |
| of the new primitives (e.g. a `spring` and a `capsule`) so the in-prompt example | |
| actually demonstrates the wider vocabulary — the example is the single biggest | |
| lever on what the model emits. | |
| --- | |
| ## Proposed prompt rewrite (concise + descriptive) | |
| Goal: **shorter scaffolding, richer guidance.** Two concrete changes. | |
| ### A) `VISION_SYSTEM_PROMPT` (`prompts.py:5-10`) — frame the task and the model | |
| Keep it ~4 lines but add: this is a *reasoning* model building an **annotated | |
| technical cutaway**; reason briefly, then emit one JSON object; prefer the | |
| *simplest set of primitives* that truthfully explains the mechanism; lower | |
| confidence + annotate when unsure (already the policy — keep it). | |
| ### B) `build_vision_prompt` (`prompts.py:13-56`) — trim rules, add a vocabulary guide | |
| - **Cut redundancy**: state "final answer = one JSON object, no markdown" once; | |
| drop repeated "compact/short/plausible" restatements. | |
| - **Replace bare enums with a terse guide** the model can act on, e.g.: | |
| ``` | |
| Shapes (pick the closest; size = [x,y,z] extents): | |
| box plates, housings, blocks, levers, selectors | |
| cylinder shafts, sleeves, bushings, drums, pins | |
| cone valve cones, tips, nozzles, tapers | |
| capsule pistons, rollers, dowel pins, plungers, bearings | |
| sphere balls, detents, ball bearings, nodes | |
| rod links, tie rods, thin axles, connecting rods | |
| gear toothed wheels (set teeth); ratchets, cogs | |
| torus o-rings, snap/retaining rings, seals, coils (single) | |
| spring helical springs, coils (set coils) | |
| Motions (axis is a numeric vector like [0,1,0]): | |
| static fixed structure / housing | |
| rotate continuous spin (speed) | |
| oscillate sinusoidal twist (amplitude, speed) | |
| translate slide along axis (range [min,max]) | |
| screw spin + advance along axis together (pitch) — screws, drills | |
| orbit revolve around a pivot point (pivot [x,y,z]) — planetary/cranks | |
| pulse scale breathing (amplitude) — diaphragms, pumps | |
| ``` | |
| - **Update the part-count line** to "Use 2 to 6 parts; prefer the fewest that | |
| explain the mechanism." | |
| - **Keep** the hard rules that protect parsing/rendering: numeric `size`/ | |
| `position`/`axis` vectors only, no `radius`/`height`/string-axis | |
| (`prompts.py:28-30`), normalized `[0,1]` top-left annotation coords | |
| (`prompts.py:31-34`), and "reason first, final answer is one JSON object" | |
| (`prompts.py:14-17`, required by the `<think>`-stripping parser, | |
| `docs/reviews/interaction-and-fallback-review.md` | |
| Q3 / `model_io.py`). | |
| - **Keep the example** but shrink it to a 2–3 part skeleton that now includes a | |
| new primitive, and rely on `EXAMPLE_ANALYSIS` (the schema sample) to show the | |
| full shape. | |
| --- | |
| ## Security / invariants to preserve (do not regress) | |
| - **Deterministic rendering only.** All new shapes/motions are built in | |
| `buildPartMesh`/`applyMotion` from validated JSON. **Never** inject | |
| model-authored HTML/JS/markup (`AGENTS.md`, `SECURITY.md`, | |
| `docs/reviews/security-hardening.md` §3). | |
| `label`/`note` stay `textContent` (`index.html:1223`). | |
| - **Validate before render.** New enum values must be added to `schema.py` so | |
| `validate_analysis` (`schema.py:263`) still runs before scene generation; do | |
| not loosen validation to "accept anything." | |
| - **Coercion stays conservative.** Unknown shapes/motions must still fall back to | |
| `box`/`static` (`model_io.py:166-170`), never crash. | |
| - No change required to rate limiting, upload caps, decompression-bomb guards | |
| (`app.py`), Modal bearer auth, or the GitHub→HF sync. | |
| --- | |
| ## Suggested implementation order for Codex | |
| 1. **Renderer first** (`index.html`): add `cone`, `capsule`, `torus`, and a | |
| `spring` (helix) builder to `buildPartMesh`/a new helper next to | |
| `gearGeometry`; add `screw`, `orbit`, `pulse` to `applyMotion` | |
| (store `pivot` on `userData`). Update the renderability gate at | |
| `index.html:840-844`. Verify each renders before touching Python. | |
| 2. **Schema** (`snap2sim/schema.py`): extend shape/motion enums + `_SHAPES`/ | |
| `_MOTIONS`; add `pitch`/`pivot`/`coils` properties; optional `parts.maxItems: | |
| 6`; update `EXAMPLE_ANALYSIS` to use a couple of new primitives. | |
| 3. **Coercion** (`snap2sim/model_io.py`): extend allowed shape/motion sets; | |
| carry new params; change `parts[:4]` → `parts[:6]`. | |
| 4. **Prompt** (`snap2sim/prompts.py`): apply the concise rewrite (system + user), | |
| the shape/motion guide, and "2 to 6 parts". | |
| 5. **Local verification** (per `AGENTS.md`): schema/parser checks, FastAPI | |
| `TestClient` for `/`, `/analyze_image`, `/generate_scene`; and a **real | |
| browser** check that each new shape and motion renders and that 5–6-part | |
| scenes don't have unreadable overlapping labels. | |
| 6. **Optional Modal re-measure** | |
| (`docs/reviews/interaction-and-fallback-review.md` Q3 tuning note): the richer prompt | |
| may change token usage; confirm `run_analysis_endpoint_check` still returns | |
| valid JSON within the current `4096`/`8192`/`300s` budgets before deploy. | |
| 7. Standard PR -> GitHub Actions HF sync -> Space verification. The final | |
| public submission is under `build-small-hackathon/Snap2Sim`. | |
| ## Archived considerations | |
| Final submission note, 2026-06-15: the renderer vocabulary pass shipped before | |
| the public `build-small-hackathon/Snap2Sim` submission. | |
| - Whether `cylinder` should revert to a true (untapered) cylinder once explicit | |
| `cone` exists (recommended yes — see §2 note). | |
| - Whether `spring`/`torus` need extra params (`coils`, `wire`/`tube` thickness) | |
| exposed to the model or just sensible fixed defaults (recommend defaults first, | |
| expose only if visual quality needs it). | |