Spaces:
Running
Running
jasondo OpenAI Codex commited on
Commit ·
5e65d06
1
Parent(s): 7a8df56
Reduce motion axis bias
Browse filesCo-authored-by: OpenAI Codex <codex@openai.com>
- AGENTS.md +67 -29
- README.md +141 -240
- docs/README.md +30 -0
- docs/features/annotated-photo-fallback.md +284 -0
- FEATURE.md → docs/features/confidence-threshold.md +3 -2
- docs/features/renderer-vocabulary.md +314 -0
- docs/features/vision-prompt-roles.md +169 -0
- docs/fixtures/mini-fan-analysis.json +105 -0
- docs/reviews/interaction-and-fallback-review.md +325 -0
- docs/reviews/motion-axis-bias.md +147 -0
- docs/reviews/scene-geometry-review.md +212 -0
- docs/reviews/security-hardening.md +290 -0
- index.html +19 -2
- snap2sim/model_io.py +1 -1
- snap2sim/prompts.py +13 -2
AGENTS.md
CHANGED
|
@@ -42,6 +42,7 @@ technical cutaway animation.
|
|
| 42 |
## Project Structure
|
| 43 |
|
| 44 |
- `PROMPT.md` - original product, model, design, and delivery requirements.
|
|
|
|
| 45 |
- `README.md` - quickstart, runtime decision, and repo map.
|
| 46 |
- `SECURITY.md` - public/private data handling rules for humans and agents.
|
| 47 |
- `.env.example` - public Hugging Face Space / local demo variables.
|
|
@@ -189,7 +190,8 @@ technical cutaway animation.
|
|
| 189 |
on June 14, 2026: `/analyze_image` returned a validated mechanism payload
|
| 190 |
through the secured Modal bearer-token flow, and `/generate_scene` returned
|
| 191 |
an A-Frame `<a-scene>` block.
|
| 192 |
-
- Implemented the `
|
|
|
|
| 193 |
Space-layer rate limiting for `/analyze_image` and `/generate_scene`,
|
| 194 |
upload-size caps, PIL decompression-bomb handling, clean image decode errors,
|
| 195 |
unique Modal temp image files with cleanup, and no model-authored scene HTML.
|
|
@@ -209,7 +211,8 @@ technical cutaway animation.
|
|
| 209 |
`/analyze_image` returned a validated mechanism payload through the secured
|
| 210 |
Modal bearer-token flow, and `/generate_scene` returned
|
| 211 |
`{ "renderer": "three", "analysis": ... }` with no HTML field.
|
| 212 |
-
- Implemented the `
|
|
|
|
| 213 |
label-overlay pointer-events bug that blocked OrbitControls, added bounded
|
| 214 |
camera controls plus Reset view and orbit/zoom hint, changed the source-photo
|
| 215 |
preview to a stable contained image box, added optional normalized per-part
|
|
@@ -220,7 +223,8 @@ technical cutaway animation.
|
|
| 220 |
the last complete JSON object, and llama.cpp analysis now uses larger
|
| 221 |
token/context/timeout budgets (`4096` output tokens, `8192` context, `300s`
|
| 222 |
timeout at the analysis call site).
|
| 223 |
-
- Local verification after the `
|
|
|
|
| 224 |
FastAPI `TestClient` confirmed `/`, `/analyze_image`, and `/generate_scene`,
|
| 225 |
and browser verification confirmed left-drag orbit, right-drag pan, wheel
|
| 226 |
zoom, mobile canvas event targeting, contained source preview, annotated-photo
|
|
@@ -235,7 +239,8 @@ technical cutaway animation.
|
|
| 235 |
`run_analysis_raw_check` parsed strict JSON successfully in `35.12s`, and
|
| 236 |
`run_analysis_endpoint_check` returned a parsed per-image `targeting reticle`
|
| 237 |
payload at `0.7` confidence.
|
| 238 |
-
- User decision after the `
|
|
|
|
| 239 |
`analyze_image_llamacpp` analysis path for the demo. `INFERENCE_BACKEND` now
|
| 240 |
defaults to `modal`; `INFERENCE_BACKEND=local` is an explicit UI-development
|
| 241 |
sample mode only and should not be used for public submission.
|
|
@@ -261,7 +266,8 @@ technical cutaway animation.
|
|
| 261 |
reveal. Local browser verification confirmed the scene still renders, orbit
|
| 262 |
drag still works, the canvas remains the pointer target, the source preview
|
| 263 |
remains contained, and the mobile layout has no horizontal overflow.
|
| 264 |
-
- Implemented the `
|
|
|
|
| 265 |
added a toolbar slider defaulting to `0.5`, made the browser recompute render
|
| 266 |
mode from cached analysis plus the current slider value, and kept slider
|
| 267 |
changes local with no extra `/analyze_image` or `/generate_scene` calls.
|
|
@@ -278,46 +284,54 @@ technical cutaway animation.
|
|
| 278 |
returned real Modal analysis for `Target Reticle` at `0.7` confidence with 3
|
| 279 |
parts, and `/generate_scene` returned `renderer: three`, `render_mode: three`,
|
| 280 |
and no HTML field.
|
| 281 |
-
- Implemented the `
|
|
|
|
| 282 |
the slider is enabled before the first upload, sends `confidence_threshold`
|
| 283 |
only with `/generate_scene`, no longer re-renders cached analysis on slider
|
| 284 |
movement, and the server clamps/coerces the threshold before selecting
|
| 285 |
`render_mode`. The browser now trusts the server's `render_mode` and only
|
| 286 |
downgrades when geometry or annotation data is missing.
|
| 287 |
-
- Implemented the `
|
|
|
|
| 288 |
and landscape image containment by giving the annotation frame a definite
|
| 289 |
viewport-bounded box, added numbered marker dots, leader lines, and optional
|
| 290 |
annotation bounding boxes, and kept all annotation text rendered through
|
| 291 |
`textContent`.
|
| 292 |
-
- Implemented the `
|
|
|
|
| 293 |
deterministic `cone`, `capsule`, `torus`, and `spring` shapes; added `screw`,
|
| 294 |
`orbit`, and `pulse` motions; raised the usable part cap to 6 across prompt,
|
| 295 |
schema, coercion, and browser renderability; and updated the sample analysis
|
| 296 |
to exercise all four new primitives.
|
| 297 |
-
- Local verification after the `
|
| 298 |
-
|
| 299 |
-
|
|
|
|
| 300 |
landscape annotated-photo fallbacks with labels in bounds and no mobile
|
| 301 |
-
horizontal overflow, and the deterministic Three.js path reached
|
| 302 |
-
|
| 303 |
- Committed and pushed `682623a` (`Expand renderer vocabulary and annotation
|
| 304 |
fallback`) to GitHub `main`. GitHub Actions sync run `27518719440` completed
|
| 305 |
successfully, and the private Hugging Face Space reported SHA
|
| 306 |
`682623a979e351c3fb6d5b5b9ade907c7a979aea`.
|
| 307 |
-
- Deployed the updated Modal app after the
|
| 308 |
-
|
| 309 |
-
|
| 310 |
-
|
| 311 |
-
|
| 312 |
-
|
| 313 |
-
|
| 314 |
-
|
|
|
|
|
|
|
|
|
|
| 315 |
invariant analysis contract into `VISION_SYSTEM_PROMPT`, reduced
|
| 316 |
`build_vision_prompt()` to a backward-compatible per-image user-prompt alias,
|
| 317 |
added `build_vision_messages()`, and wired Modal llama.cpp analysis calls to
|
| 318 |
pass `-sys` with a fallback that prepends the system text only if the deployed
|
| 319 |
binary rejects the system-prompt flag.
|
| 320 |
-
- Verification after the `
|
|
|
|
| 321 |
schema/parser/coercion checks, FastAPI `TestClient` checks for `/`,
|
| 322 |
`/analyze_image`, and `/generate_scene`, and a `modal_app.py` import/helper
|
| 323 |
check all passed locally. Modal dev raw analysis parsed strict JSON in
|
|
@@ -334,7 +348,8 @@ technical cutaway animation.
|
|
| 334 |
returned a real Modal analysis for `cutaway mechanism` at `0.7` confidence
|
| 335 |
with 3 parts, and `/generate_scene` returned `renderer: three`,
|
| 336 |
`render_mode: three`, and no HTML field.
|
| 337 |
-
- Implemented the `
|
|
|
|
| 338 |
axial and disk-like primitives now infer their axis from `geometry.size`,
|
| 339 |
radius comes from the cross-section pair, explicit `geometry.rotation`
|
| 340 |
remains additive, the Three.js assembly recenters before reveal animation,
|
|
@@ -342,11 +357,35 @@ technical cutaway animation.
|
|
| 342 |
aspect, the grid moves to the recentered assembly bottom, and label
|
| 343 |
projection uses world positions. Prompt wording now tells the model that the
|
| 344 |
renderer infers primitive orientation from the odd-one-out size dimension.
|
| 345 |
-
- Local verification after the `
|
|
|
|
| 346 |
schema validation, parser/coercion validation, FastAPI `TestClient` checks
|
| 347 |
-
in `INFERENCE_BACKEND=local`, browser verification for
|
| 348 |
-
and `EXAMPLE_ANALYSIS`,
|
|
|
|
| 349 |
no-overflow measurement, orbit drag, and Reset view all passed.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 350 |
|
| 351 |
## Next Work
|
| 352 |
|
|
@@ -357,6 +396,5 @@ technical cutaway animation.
|
|
| 357 |
- Keep the Hugging Face Space private until the user explicitly approves making
|
| 358 |
it public for submission.
|
| 359 |
- Keep the README opening hook intact: "You find a small metal cylinder at a
|
| 360 |
-
flea market. What is it? How does it work inside?"
|
| 361 |
-
|
| 362 |
-
bonus quest claims from `PROMPT.md`.
|
|
|
|
| 42 |
## Project Structure
|
| 43 |
|
| 44 |
- `PROMPT.md` - original product, model, design, and delivery requirements.
|
| 45 |
+
- `docs/` - implementation notes, review passes, and representative fixtures.
|
| 46 |
- `README.md` - quickstart, runtime decision, and repo map.
|
| 47 |
- `SECURITY.md` - public/private data handling rules for humans and agents.
|
| 48 |
- `.env.example` - public Hugging Face Space / local demo variables.
|
|
|
|
| 190 |
on June 14, 2026: `/analyze_image` returned a validated mechanism payload
|
| 191 |
through the secured Modal bearer-token flow, and `/generate_scene` returned
|
| 192 |
an A-Frame `<a-scene>` block.
|
| 193 |
+
- Implemented the `docs/reviews/security-hardening.md` security pass on June
|
| 194 |
+
14, 2026:
|
| 195 |
Space-layer rate limiting for `/analyze_image` and `/generate_scene`,
|
| 196 |
upload-size caps, PIL decompression-bomb handling, clean image decode errors,
|
| 197 |
unique Modal temp image files with cleanup, and no model-authored scene HTML.
|
|
|
|
| 211 |
`/analyze_image` returned a validated mechanism payload through the secured
|
| 212 |
Modal bearer-token flow, and `/generate_scene` returned
|
| 213 |
`{ "renderer": "three", "analysis": ... }` with no HTML field.
|
| 214 |
+
- Implemented the `docs/reviews/interaction-and-fallback-review.md` local code
|
| 215 |
+
pass on June 14, 2026: fixed the
|
| 216 |
label-overlay pointer-events bug that blocked OrbitControls, added bounded
|
| 217 |
camera controls plus Reset view and orbit/zoom hint, changed the source-photo
|
| 218 |
preview to a stable contained image box, added optional normalized per-part
|
|
|
|
| 223 |
the last complete JSON object, and llama.cpp analysis now uses larger
|
| 224 |
token/context/timeout budgets (`4096` output tokens, `8192` context, `300s`
|
| 225 |
timeout at the analysis call site).
|
| 226 |
+
- Local verification after the `docs/reviews/interaction-and-fallback-review.md`
|
| 227 |
+
pass: schema/parser checks passed,
|
| 228 |
FastAPI `TestClient` confirmed `/`, `/analyze_image`, and `/generate_scene`,
|
| 229 |
and browser verification confirmed left-drag orbit, right-drag pan, wheel
|
| 230 |
zoom, mobile canvas event targeting, contained source preview, annotated-photo
|
|
|
|
| 239 |
`run_analysis_raw_check` parsed strict JSON successfully in `35.12s`, and
|
| 240 |
`run_analysis_endpoint_check` returned a parsed per-image `targeting reticle`
|
| 241 |
payload at `0.7` confidence.
|
| 242 |
+
- User decision after the `docs/reviews/interaction-and-fallback-review.md`
|
| 243 |
+
pass: use the real Modal
|
| 244 |
`analyze_image_llamacpp` analysis path for the demo. `INFERENCE_BACKEND` now
|
| 245 |
defaults to `modal`; `INFERENCE_BACKEND=local` is an explicit UI-development
|
| 246 |
sample mode only and should not be used for public submission.
|
|
|
|
| 266 |
reveal. Local browser verification confirmed the scene still renders, orbit
|
| 267 |
drag still works, the canvas remains the pointer target, the source preview
|
| 268 |
remains contained, and the mobile layout has no horizontal overflow.
|
| 269 |
+
- Implemented the `docs/features/confidence-threshold.md`
|
| 270 |
+
confidence-threshold control on June 14, 2026:
|
| 271 |
added a toolbar slider defaulting to `0.5`, made the browser recompute render
|
| 272 |
mode from cached analysis plus the current slider value, and kept slider
|
| 273 |
changes local with no extra `/analyze_image` or `/generate_scene` calls.
|
|
|
|
| 284 |
returned real Modal analysis for `Target Reticle` at `0.7` confidence with 3
|
| 285 |
parts, and `/generate_scene` returned `renderer: three`, `render_mode: three`,
|
| 286 |
and no HTML field.
|
| 287 |
+
- Implemented the `docs/features/confidence-threshold.md`
|
| 288 |
+
confidence-threshold re-spec on June 14, 2026:
|
| 289 |
the slider is enabled before the first upload, sends `confidence_threshold`
|
| 290 |
only with `/generate_scene`, no longer re-renders cached analysis on slider
|
| 291 |
movement, and the server clamps/coerces the threshold before selecting
|
| 292 |
`render_mode`. The browser now trusts the server's `render_mode` and only
|
| 293 |
downgrades when geometry or annotation data is missing.
|
| 294 |
+
- Implemented the `docs/features/annotated-photo-fallback.md` fallback pass:
|
| 295 |
+
fixed portrait
|
| 296 |
and landscape image containment by giving the annotation frame a definite
|
| 297 |
viewport-bounded box, added numbered marker dots, leader lines, and optional
|
| 298 |
annotation bounding boxes, and kept all annotation text rendered through
|
| 299 |
`textContent`.
|
| 300 |
+
- Implemented the `docs/features/renderer-vocabulary.md` renderer/prompt
|
| 301 |
+
vocabulary pass: added
|
| 302 |
deterministic `cone`, `capsule`, `torus`, and `spring` shapes; added `screw`,
|
| 303 |
`orbit`, and `pulse` motions; raised the usable part cap to 6 across prompt,
|
| 304 |
schema, coercion, and browser renderability; and updated the sample analysis
|
| 305 |
to exercise all four new primitives.
|
| 306 |
+
- Local verification after the `docs/features/annotated-photo-fallback.md` /
|
| 307 |
+
`docs/features/renderer-vocabulary.md` pass: schema and coercion checks
|
| 308 |
+
passed, FastAPI `TestClient` confirmed `/`, `/analyze_image`, and
|
| 309 |
+
`/generate_scene`, browser verification confirmed contained portrait and
|
| 310 |
landscape annotated-photo fallbacks with labels in bounds and no mobile
|
| 311 |
+
horizontal overflow, and the deterministic Three.js path reached `CUTAWAY
|
| 312 |
+
READY` with a nonblank six-part scene.
|
| 313 |
- Committed and pushed `682623a` (`Expand renderer vocabulary and annotation
|
| 314 |
fallback`) to GitHub `main`. GitHub Actions sync run `27518719440` completed
|
| 315 |
successfully, and the private Hugging Face Space reported SHA
|
| 316 |
`682623a979e351c3fb6d5b5b9ade907c7a979aea`.
|
| 317 |
+
- Deployed the updated Modal app after the
|
| 318 |
+
`docs/features/annotated-photo-fallback.md` /
|
| 319 |
+
`docs/features/renderer-vocabulary.md` pass so `analyze_image_llamacpp` uses
|
| 320 |
+
the richer prompt plus expanded schema and coercion. Authenticated private
|
| 321 |
+
Space verification passed: root returned `200` with the updated vocabulary
|
| 322 |
+
shell, a synthetic target image returned a real Modal analysis for `piston
|
| 323 |
+
mechanism` at `0.85` confidence with 3 parts, `/generate_scene` returned
|
| 324 |
+
`renderer: three`, `render_mode: three`, and no HTML field, and a high
|
| 325 |
+
confidence threshold returned `render_mode: annotate`.
|
| 326 |
+
- Implemented the `docs/features/vision-prompt-roles.md` prompt-role split on
|
| 327 |
+
June 15, 2026: moved the
|
| 328 |
invariant analysis contract into `VISION_SYSTEM_PROMPT`, reduced
|
| 329 |
`build_vision_prompt()` to a backward-compatible per-image user-prompt alias,
|
| 330 |
added `build_vision_messages()`, and wired Modal llama.cpp analysis calls to
|
| 331 |
pass `-sys` with a fallback that prepends the system text only if the deployed
|
| 332 |
binary rejects the system-prompt flag.
|
| 333 |
+
- Verification after the `docs/features/vision-prompt-roles.md` pass:
|
| 334 |
+
prompt-helper checks,
|
| 335 |
schema/parser/coercion checks, FastAPI `TestClient` checks for `/`,
|
| 336 |
`/analyze_image`, and `/generate_scene`, and a `modal_app.py` import/helper
|
| 337 |
check all passed locally. Modal dev raw analysis parsed strict JSON in
|
|
|
|
| 348 |
returned a real Modal analysis for `cutaway mechanism` at `0.7` confidence
|
| 349 |
with 3 parts, and `/generate_scene` returned `renderer: three`,
|
| 350 |
`render_mode: three`, and no HTML field.
|
| 351 |
+
- Implemented the `docs/reviews/scene-geometry-review.md` deterministic
|
| 352 |
+
renderer pass on June 15, 2026:
|
| 353 |
axial and disk-like primitives now infer their axis from `geometry.size`,
|
| 354 |
radius comes from the cross-section pair, explicit `geometry.rotation`
|
| 355 |
remains additive, the Three.js assembly recenters before reveal animation,
|
|
|
|
| 357 |
aspect, the grid moves to the recentered assembly bottom, and label
|
| 358 |
projection uses world positions. Prompt wording now tells the model that the
|
| 359 |
renderer infers primitive orientation from the odd-one-out size dimension.
|
| 360 |
+
- Local verification after the `docs/reviews/scene-geometry-review.md` pass:
|
| 361 |
+
prompt/schema imports,
|
| 362 |
schema validation, parser/coercion validation, FastAPI `TestClient` checks
|
| 363 |
+
in `INFERENCE_BACKEND=local`, browser verification for
|
| 364 |
+
`docs/fixtures/mini-fan-analysis.json` and `EXAMPLE_ANALYSIS`,
|
| 365 |
+
desktop/mobile nonblank screenshots, mobile
|
| 366 |
no-overflow measurement, orbit drag, and Reset view all passed.
|
| 367 |
+
- Reworked `README.md` for Build Small judging on June 15, 2026: added
|
| 368 |
+
judge-facing YAML tags for the Backyard AI track plus Llama Champion,
|
| 369 |
+
NVIDIA Nemotron Quest, Modal Award, OpenAI Codex, Off-Brand, Best Demo, and
|
| 370 |
+
Sharing is Caring; shifted the body from a maintainer runbook to a public
|
| 371 |
+
pitch with concise idea, demo, tech stack, verification, and setup sections;
|
| 372 |
+
kept private Modal endpoint URLs and credentials out of public docs.
|
| 373 |
+
- Cleaned up repository notes on June 15, 2026: moved feature specs to
|
| 374 |
+
`docs/features/`, review passes to `docs/reviews/`, and the mini-fan model
|
| 375 |
+
output fixture to `docs/fixtures/mini-fan-analysis.json`; added
|
| 376 |
+
`docs/README.md` as an index.
|
| 377 |
+
- Implemented the `docs/reviews/motion-axis-bias.md` pass on June 15, 2026:
|
| 378 |
+
de-anchored the vision prompt from `[0, 1, 0]`, added balanced X/Y/Z axis
|
| 379 |
+
examples and a shaft exemplar, made missing browser-side motion axes derive
|
| 380 |
+
from the same size-based primitive axis used for geometry orientation, and
|
| 381 |
+
removed the hard-coded Y axis from the generic fallback rotor.
|
| 382 |
+
- Verification after the motion-axis pass: schema/parser/coercion checks
|
| 383 |
+
passed, FastAPI `TestClient` checks passed in `INFERENCE_BACKEND=local`,
|
| 384 |
+
Playwright renderer probes confirmed missing-axis X, Y, and Z parts infer
|
| 385 |
+
the expected axes with no horizontal overflow, Modal dev
|
| 386 |
+
`run_analysis_endpoint_check` returned a rotating `dial` with
|
| 387 |
+
`axis: [1, 0, 0]`, stable Modal deploy completed, and unauthenticated stable
|
| 388 |
+
`runtime_probe` still returned `401 Unauthorized`.
|
| 389 |
|
| 390 |
## Next Work
|
| 391 |
|
|
|
|
| 396 |
- Keep the Hugging Face Space private until the user explicitly approves making
|
| 397 |
it public for submission.
|
| 398 |
- Keep the README opening hook intact: "You find a small metal cylinder at a
|
| 399 |
+
flea market. What is it? How does it work inside?" Maintain the judge-facing
|
| 400 |
+
YAML tags and public pitch if future README updates are needed.
|
|
|
README.md
CHANGED
|
@@ -5,208 +5,160 @@ app_file: app.py
|
|
| 5 |
license: mit
|
| 6 |
colorFrom: blue
|
| 7 |
colorTo: yellow
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 8 |
---
|
| 9 |
|
| 10 |
-
# Snap2Sim
|
| 11 |
|
| 12 |
You find a small metal cylinder at a flea market. What is it? How does it work
|
| 13 |
inside?
|
| 14 |
|
| 15 |
-
|
| 16 |
-
|
| 17 |
-
|
| 18 |
-
|
|
|
|
| 19 |
|
| 20 |
-
The
|
| 21 |
-
|
| 22 |
-
|
| 23 |
-
sample backend is available only when `INFERENCE_BACKEND=local` is explicitly
|
| 24 |
-
set for development.
|
| 25 |
|
| 26 |
-
##
|
| 27 |
|
| 28 |
-
|
| 29 |
-
|
| 30 |
-
|
| 31 |
-
```
|
| 32 |
|
| 33 |
-
|
| 34 |
-
|
| 35 |
-
|
| 36 |
-
validated scene descriptor.
|
| 37 |
|
| 38 |
-
|
| 39 |
-
`INFERENCE_BACKEND=local`. That mode returns a sample mechanism and should not
|
| 40 |
-
be used for the public demo.
|
| 41 |
|
| 42 |
-
|
| 43 |
-
|
| 44 |
-
|
| 45 |
|
| 46 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 47 |
|
| 48 |
-
Run
|
| 49 |
|
| 50 |
```powershell
|
| 51 |
-
|
|
|
|
| 52 |
```
|
| 53 |
|
| 54 |
-
|
| 55 |
-
Hugging Face repo without downloading the model. It does not prove image input
|
| 56 |
-
works in llama.cpp.
|
| 57 |
-
|
| 58 |
-
## Runtime Decision
|
| 59 |
-
|
| 60 |
-
Verification on June 13, 2026 found:
|
| 61 |
-
|
| 62 |
-
- `unsloth/NVIDIA-Nemotron-3-Nano-Omni-30B-A3B-Reasoning-GGUF` includes GGUF
|
| 63 |
-
quant files, `mmproj-BF16.gguf`, `mmproj-F16.gguf`, and `mmproj-F32.gguf`.
|
| 64 |
-
- The same model card includes llama.cpp launch instructions.
|
| 65 |
-
- llama.cpp source includes the `nemotron_h_moe` text architecture.
|
| 66 |
-
- llama.cpp public docs list multimodal server support, but the visible
|
| 67 |
-
supported-model list does not explicitly name Nemotron Omni or CRADIO v4-H.
|
| 68 |
-
|
| 69 |
-
So the preferred path is single-model GGUF via llama.cpp. The Modal GPU smoke
|
| 70 |
-
test passed on June 13, 2026 with `UD_Q4_K_M` and `mmproj-F16.gguf`, confirming
|
| 71 |
-
that `llama-mtmd-cli` accepts image input for this pairing.
|
| 72 |
-
|
| 73 |
-
If endpoint quality is not strong enough for the demo, use
|
| 74 |
-
`nvidia/Nemotron-3-Nano-Omni-30B-A3B-Reasoning-NVFP4` through vLLM or
|
| 75 |
-
Transformers/custom code for vision analysis, and keep the GGUF path for
|
| 76 |
-
text-only Three.js generation.
|
| 77 |
|
| 78 |
-
|
| 79 |
-
|
| 80 |
-
|
| 81 |
-
|
| 82 |
-
|
| 83 |
-
## Model Stack and Quest Claims
|
| 84 |
|
| 85 |
-
|
| 86 |
-
|
| 87 |
-
|
| 88 |
|
| 89 |
-
|
| 90 |
-
NVIDIA Nemotron Nano V2 VL 12B for vision plus Qwen2.5-Coder-14B for scene
|
| 91 |
-
generation, for about 26B total parameters.
|
| 92 |
|
| 93 |
-
|
| 94 |
-
|---|---|
|
| 95 |
-
| Llama Champion | Confirmed via llama.cpp / GGUF runtime |
|
| 96 |
-
| NVIDIA Nemotron Quest | Confirmed via Nemotron primary model |
|
| 97 |
-
| Off-Brand | Confirmed |
|
| 98 |
-
| Modal Award | Confirmed via deployed Modal endpoints |
|
| 99 |
-
| Off the Grid | Not claimed while inference runs on Modal |
|
| 100 |
-
| Field Notes | Stretch |
|
| 101 |
-
|
| 102 |
-
## Project Structure
|
| 103 |
-
|
| 104 |
-
- `.env.example` - public runtime variables for the deployed Modal demo.
|
| 105 |
-
- `SECURITY.md` - public/private data handling guidance for humans and agents.
|
| 106 |
-
- `app.py` - Hugging Face Space entry point using `gradio.Server`; serves
|
| 107 |
-
`index.html` at `/` and exposes `/analyze_image` plus `/generate_scene`.
|
| 108 |
-
- `index.html` - plain HTML/CSS/JS shell with upload orchestration,
|
| 109 |
-
source-photo preview, and deterministic browser-side Three.js rendering.
|
| 110 |
-
- `modal_app.py` - Modal web endpoint scaffold.
|
| 111 |
-
- `scripts/verify_runtime_assets.py` - GGUF/mmproj metadata preflight.
|
| 112 |
-
- `snap2sim/backend.py` - backend selection and HTTP client.
|
| 113 |
-
- `snap2sim/prompts.py` - prompt template for vision analysis.
|
| 114 |
-
- `snap2sim/schema.py` - JSON schema and sample analysis payload.
|
| 115 |
-
|
| 116 |
-
## Current Rendering Architecture
|
| 117 |
-
|
| 118 |
-
The app uses `gradio.Server` to serve `index.html` directly. This avoids
|
| 119 |
-
Gradio component script stripping and lets the page load the trusted Three.js
|
| 120 |
-
renderer normally.
|
| 121 |
-
|
| 122 |
-
Runtime flow:
|
| 123 |
-
|
| 124 |
-
1. Browser encodes the uploaded photo and posts it to `/analyze_image`.
|
| 125 |
-
2. Backend returns the validated mechanism JSON.
|
| 126 |
-
3. Browser posts the analysis plus the current confidence threshold to
|
| 127 |
-
`/generate_scene` for a validated scene descriptor.
|
| 128 |
-
4. Browser renders deterministic Three.js primitives when geometry is usable,
|
| 129 |
-
or overlays text-only callouts on the uploaded photo when the model only has
|
| 130 |
-
image-space annotations.
|
| 131 |
-
5. A browser-side confidence slider controls the minimum analysis confidence
|
| 132 |
-
needed for the 3D cutaway. Moving it only updates the next-run value; it does
|
| 133 |
-
not re-render cached analysis or call the model.
|
| 134 |
-
6. `/generate_scene` returns a validated
|
| 135 |
-
`{ "renderer": "...", "render_mode": "...", "analysis": ... }` descriptor
|
| 136 |
-
instead of model-authored HTML. The returned `render_mode` is the source of
|
| 137 |
-
truth for the visible renderer, with the browser only downgrading when
|
| 138 |
-
required data is missing.
|
| 139 |
-
|
| 140 |
-
The shell uses Chakra Petch and Fira Code from Bunny Fonts, an asymmetric
|
| 141 |
-
63/37 viewport/readout split, a blueprint grid, amber/cyan instrument-panel
|
| 142 |
-
colors, explicit Modal cold-start messaging, source-photo preview, and a
|
| 143 |
-
play/pause control. The confidence slider defaults to 50%, matching the
|
| 144 |
-
server's fallback threshold, and the generation route is authoritative for the
|
| 145 |
-
visible render mode.
|
| 146 |
-
|
| 147 |
-
The deterministic renderer supports `box`, `cylinder`, `sphere`, `gear`, `rod`,
|
| 148 |
-
`cone`, `capsule`, `torus`, and `spring` primitives, plus `static`, `rotate`,
|
| 149 |
-
`oscillate`, `translate`, `screw`, `orbit`, and `pulse` motions. The
|
| 150 |
-
annotated-photo fallback keeps the uploaded image contained in the viewport and
|
| 151 |
-
draws marker dots, leader lines, and optional normalized bounding boxes from
|
| 152 |
-
validated annotation data.
|
| 153 |
-
|
| 154 |
-
The browser no longer injects model-authored HTML into the DOM. The model's
|
| 155 |
-
job is limited to the structured analysis JSON contract in `snap2sim/schema.py`.
|
| 156 |
-
|
| 157 |
-
## Modal Deployment Path
|
| 158 |
-
|
| 159 |
-
Deploy the Modal app before wiring a public demo so the llama.cpp endpoint uses
|
| 160 |
-
the current prompt, parsing, and coercion code:
|
| 161 |
|
| 162 |
```powershell
|
| 163 |
-
python
|
| 164 |
```
|
| 165 |
|
| 166 |
-
|
| 167 |
-
first.
|
| 168 |
-
|
| 169 |
-
Current production deployment:
|
| 170 |
-
|
| 171 |
-
- Deployment: https://modal.com/apps/bigstonks1/main/deployed/snap2sim-inside-the-machine
|
| 172 |
-
- Web endpoint URLs are configured in Hugging Face Space variables and protected
|
| 173 |
-
by the `SNAP2SIM_API_TOKEN` bearer-token secret.
|
| 174 |
-
|
| 175 |
-
Useful deployment functions/endpoints:
|
| 176 |
-
|
| 177 |
-
- `check_remote_imports` verifies Modal can import the local `snap2sim` package
|
| 178 |
-
before running expensive GPU/model work.
|
| 179 |
-
- `download_runtime_assets` caches the selected GGUF quant and `mmproj` file in
|
| 180 |
-
the `snap2sim-hf-cache` Modal Volume.
|
| 181 |
-
- `smoke_test_llamacpp_image` builds llama.cpp with CUDA and runs one image
|
| 182 |
-
prompt through `llama-mtmd-cli` using the cached GGUF and `mmproj`.
|
| 183 |
-
- `run_analysis_endpoint_check` calls the real llama.cpp analysis task with a
|
| 184 |
-
synthetic image and confirms it returns a validated mechanism payload.
|
| 185 |
-
- `runtime_probe` reports the configured model repo, quant, projector file, and
|
| 186 |
-
whether placeholder inference is active on the compatibility endpoint.
|
| 187 |
-
- `analyze_image` preserves the HTTP contract for old wiring, but the demo
|
| 188 |
-
should use `analyze_image_llamacpp`.
|
| 189 |
-
- `generate_scene` and `generate_scene_llamacpp` are compatibility endpoints
|
| 190 |
-
that return the deterministic Three.js scene descriptor.
|
| 191 |
-
- `analyze_image_llamacpp` is the experimental GPU endpoint for the llama.cpp
|
| 192 |
-
runtime path after the smoke test passes.
|
| 193 |
-
|
| 194 |
-
Runtime environment knobs:
|
| 195 |
-
|
| 196 |
-
- `SNAP2SIM_MODEL_REPO`, default
|
| 197 |
-
`unsloth/NVIDIA-Nemotron-3-Nano-Omni-30B-A3B-Reasoning-GGUF`
|
| 198 |
-
- `SNAP2SIM_GGUF_QUANT`, default `UD-Q4_K_M`
|
| 199 |
-
- `SNAP2SIM_MMPROJ_FILE`, default `mmproj-F16.gguf`
|
| 200 |
-
- `SNAP2SIM_RUNTIME_MODE`, default `placeholder`
|
| 201 |
-
- `SNAP2SIM_SMOKE_GPU`, default `L40S`
|
| 202 |
-
- `SNAP2SIM_RUNTIME_GPU`, default `L40S`
|
| 203 |
-
|
| 204 |
-
For the public demo, point `MODAL_ANALYZE_URL` at the validated
|
| 205 |
-
`analyze_image_llamacpp` endpoint. `SNAP2SIM_RUNTIME_MODE=placeholder` can stay
|
| 206 |
-
set because only the legacy `analyze_image` compatibility endpoint reads it.
|
| 207 |
-
The browser renders scenes deterministically from the validated analysis JSON.
|
| 208 |
-
|
| 209 |
-
Run the deployment preflight in this order:
|
| 210 |
|
| 211 |
```powershell
|
| 212 |
python -m modal run modal_app.py::check_remote_imports
|
|
@@ -215,74 +167,23 @@ python -m modal run modal_app.py::smoke_test_llamacpp_image
|
|
| 215 |
python -m modal run modal_app.py::run_analysis_endpoint_check
|
| 216 |
```
|
| 217 |
|
| 218 |
-
|
| 219 |
|
| 220 |
```powershell
|
| 221 |
-
python -m modal run modal_app.py::run_runtime_preflight
|
| 222 |
-
```
|
| 223 |
-
|
| 224 |
-
The smoke test returns a JSON object. Treat `"ok": true` as evidence that
|
| 225 |
-
llama.cpp accepted image input with the selected GGUF and `mmproj`. The analysis
|
| 226 |
-
endpoint has a bounded generation path and a local coercion fallback because the
|
| 227 |
-
reasoning model can emit verbose `<think>` text before JSON.
|
| 228 |
-
|
| 229 |
-
## Hugging Face Space Configuration
|
| 230 |
-
|
| 231 |
-
Current Space:
|
| 232 |
-
|
| 233 |
-
- Hub repo: https://huggingface.co/spaces/jasondo111/Snap2Sim
|
| 234 |
-
- App host: https://jasondo111-snap2sim.hf.space
|
| 235 |
-
- Visibility: private as of the latest deployment
|
| 236 |
-
- Runtime: `RUNNING` on `cpu-basic`
|
| 237 |
-
|
| 238 |
-
The Space is configured as a Gradio SDK app with these variables:
|
| 239 |
-
|
| 240 |
-
```text
|
| 241 |
-
INFERENCE_BACKEND=modal
|
| 242 |
-
MODAL_ANALYZE_URL=<stored in Hugging Face Space variables>
|
| 243 |
-
MODAL_GENERATE_URL=<optional compatibility value>
|
| 244 |
-
INFERENCE_TIMEOUT_SECONDS=300
|
| 245 |
```
|
| 246 |
|
| 247 |
-
|
| 248 |
-
|
| 249 |
-
|
| 250 |
-
|
| 251 |
-
|
| 252 |
-
|
| 253 |
-
|
| 254 |
-
|
| 255 |
-
|
| 256 |
-
|
| 257 |
-
``
|
| 258 |
-
hf auth login --force
|
| 259 |
-
hf repos create jasondo111/Snap2Sim --type space --space-sdk gradio --exist-ok --env-file .env.example
|
| 260 |
-
hf upload jasondo111/Snap2Sim . . --repo-type space --commit-message "Deploy Snap2Sim demo"
|
| 261 |
-
```
|
| 262 |
-
|
| 263 |
-
The `.hfignore` file excludes local credentials, caches, downloaded model
|
| 264 |
-
weights, and Playwright artifacts from uploads.
|
| 265 |
-
|
| 266 |
-
For an authenticated API check against this private Space, use `handle_file`
|
| 267 |
-
with `gradio_client`; plain string file paths are rejected by the Gradio 6 image
|
| 268 |
-
input schema.
|
| 269 |
-
|
| 270 |
-
## GitHub to Hugging Face Sync
|
| 271 |
-
|
| 272 |
-
GitHub is the source of truth for this project:
|
| 273 |
-
|
| 274 |
-
- GitHub repo: https://github.com/Bigstonks1/Snap2Sim
|
| 275 |
-
- Hugging Face Space: configured separately as the sync target
|
| 276 |
-
|
| 277 |
-
The workflow at `.github/workflows/sync_to_hf.yml` syncs pushes to the `main`
|
| 278 |
-
branch into `jasondo111/Snap2Sim`. It requires a GitHub Actions secret named
|
| 279 |
-
`HF_TOKEN` with write access to that Space.
|
| 280 |
-
|
| 281 |
-
Do not edit files directly in the Hugging Face Space. Those changes are not the
|
| 282 |
-
source of truth and will be overwritten by the next GitHub-to-Hugging Face sync.
|
| 283 |
-
Configure runtime values such as `MODAL_ANALYZE_URL`, `MODAL_GENERATE_URL`, and
|
| 284 |
-
`SNAP2SIM_API_TOKEN` through Hugging Face Space variables/secrets instead of
|
| 285 |
-
committing them.
|
| 286 |
|
| 287 |
## Acknowledgements
|
| 288 |
|
|
|
|
| 5 |
license: mit
|
| 6 |
colorFrom: blue
|
| 7 |
colorTo: yellow
|
| 8 |
+
short_description: Photograph a mystery part and get an animated mechanical cutaway.
|
| 9 |
+
models:
|
| 10 |
+
- unsloth/NVIDIA-Nemotron-3-Nano-Omni-30B-A3B-Reasoning-GGUF
|
| 11 |
+
tags:
|
| 12 |
+
- build-small-hackathon
|
| 13 |
+
- backyard-ai
|
| 14 |
+
- llama-champion
|
| 15 |
+
- nvidia-nemotron-quest
|
| 16 |
+
- modal-award
|
| 17 |
+
- openai-codex
|
| 18 |
+
- off-brand
|
| 19 |
+
- best-demo
|
| 20 |
+
- sharing-is-caring
|
| 21 |
+
- llama-cpp
|
| 22 |
+
- gguf
|
| 23 |
+
- gradio
|
| 24 |
+
- three-js
|
| 25 |
---
|
| 26 |
|
| 27 |
+
# Snap2Sim: Inside the Machine
|
| 28 |
|
| 29 |
You find a small metal cylinder at a flea market. What is it? How does it work
|
| 30 |
inside?
|
| 31 |
|
| 32 |
+
Snap2Sim turns a photo of a hardware component into a narrated mechanical
|
| 33 |
+
cutaway. Drop in a picture of a valve, lock, pump, gear train, fan, latch, or
|
| 34 |
+
other mystery part; the app identifies the likely mechanism, names the internal
|
| 35 |
+
pieces, and renders a moving 3D field-manual view that explains how the object
|
| 36 |
+
works.
|
| 37 |
|
| 38 |
+
The goal is not just recognition. It is the moment a curious maker gets to see
|
| 39 |
+
the hidden motion: the plunger sliding, the spring loading, the rotor spinning,
|
| 40 |
+
the pawl catching, the port opening.
|
|
|
|
|
|
|
| 41 |
|
| 42 |
+
## Demo
|
| 43 |
|
| 44 |
+
- Hugging Face Space: https://huggingface.co/spaces/jasondo111/Snap2Sim
|
| 45 |
+
- App host: https://jasondo111-snap2sim.hf.space
|
| 46 |
+
- Source of truth: https://github.com/Bigstonks1/Snap2Sim
|
|
|
|
| 47 |
|
| 48 |
+
The Space is private until final submission approval. The app is built as a
|
| 49 |
+
Gradio Space with a custom `gradio.Server` shell and can be made public from the
|
| 50 |
+
same repository when ready.
|
|
|
|
| 51 |
|
| 52 |
+
## Hackathon Fit
|
|
|
|
|
|
|
| 53 |
|
| 54 |
+
Snap2Sim is a Backyard AI project for people who learn by taking things apart:
|
| 55 |
+
garage tinkerers, repair hobbyists, thrift-store scavengers, robotics students,
|
| 56 |
+
and anyone holding an unknown part and wondering what is going on inside.
|
| 57 |
|
| 58 |
+
The README metadata tags request consideration for:
|
| 59 |
+
|
| 60 |
+
| Track or badge | Why it applies |
|
| 61 |
+
|---|---|
|
| 62 |
+
| Backyard AI | Explains real-world objects from a user's own photo. |
|
| 63 |
+
| Llama Champion | Runs the primary model through llama.cpp with GGUF assets. |
|
| 64 |
+
| NVIDIA Nemotron Quest | Uses NVIDIA Nemotron 3 Nano Omni as the vision-reasoning model. |
|
| 65 |
+
| Modal Award | GPU inference is deployed through Modal with cached model assets. |
|
| 66 |
+
| OpenAI Codex | Built with Codex assistance and Codex-attributed commits in the linked GitHub repo. |
|
| 67 |
+
| Off-Brand | The experience is a branded maker tool, not a generic chatbot or image demo. |
|
| 68 |
+
| Best Demo / Sharing is Caring | Targeted once the public demo video and social post links are added to this README. |
|
| 69 |
+
| Off the Grid | Not claimed while inference runs on Modal. |
|
| 70 |
+
|
| 71 |
+
## How It Works
|
| 72 |
+
|
| 73 |
+
1. The browser accepts a component photo and sends it to the Space's same-origin
|
| 74 |
+
`/analyze_image` API.
|
| 75 |
+
2. The Space calls a secured Modal endpoint running
|
| 76 |
+
`unsloth/NVIDIA-Nemotron-3-Nano-Omni-30B-A3B-Reasoning-GGUF` through
|
| 77 |
+
llama.cpp and `llama-mtmd-cli`.
|
| 78 |
+
3. Nemotron returns a structured mechanism JSON payload: component name,
|
| 79 |
+
confidence, internal parts, geometry hints, motion types, annotations, and a
|
| 80 |
+
plain-language explanation.
|
| 81 |
+
4. `/generate_scene` validates the analysis and chooses the render mode.
|
| 82 |
+
5. The browser renders the result with trusted deterministic Three.js code. If a
|
| 83 |
+
confident 3D reconstruction is not justified, it falls back to annotated
|
| 84 |
+
callouts on the original photo instead of pretending.
|
| 85 |
+
|
| 86 |
+
The model describes the mechanism. The browser owns the rendering. That keeps
|
| 87 |
+
the demo reliable and avoids injecting model-authored HTML or JavaScript into
|
| 88 |
+
the page.
|
| 89 |
+
|
| 90 |
+
## Tech Stack
|
| 91 |
+
|
| 92 |
+
- Frontend: plain `index.html`, CSS, and JavaScript served directly by
|
| 93 |
+
`gradio.Server`.
|
| 94 |
+
- Rendering: deterministic Three.js primitives, labels, OrbitControls, camera
|
| 95 |
+
fitting, and an annotated-photo fallback.
|
| 96 |
+
- Model runtime: NVIDIA Nemotron 3 Nano Omni 30B-A3B GGUF, `UD-Q4_K_M`, with
|
| 97 |
+
`mmproj-F16.gguf` through llama.cpp.
|
| 98 |
+
- GPU service: Modal, with weights cached in a Modal Volume and bearer-token
|
| 99 |
+
protection between the Space and Modal.
|
| 100 |
+
- Space runtime: Hugging Face `cpu-basic`; expensive inference happens on Modal.
|
| 101 |
+
- Contract: JSON schema validation in `snap2sim/schema.py` before anything is
|
| 102 |
+
rendered.
|
| 103 |
+
|
| 104 |
+
The primary model is approximately 30B total parameters with roughly 3B active
|
| 105 |
+
per MoE token, keeping the single-model pipeline under the hackathon's 32B model
|
| 106 |
+
budget.
|
| 107 |
+
|
| 108 |
+
## What Makes It Different
|
| 109 |
+
|
| 110 |
+
Most image demos stop at a label. Snap2Sim tries to answer the maker's next
|
| 111 |
+
question: "what would I see if I cut this open?"
|
| 112 |
+
|
| 113 |
+
- It produces a mechanism explanation, not just a class name.
|
| 114 |
+
- It maps internal parts into a moving technical cutaway.
|
| 115 |
+
- It shows uncertainty honestly with a confidence threshold and a photo
|
| 116 |
+
annotation fallback.
|
| 117 |
+
- It uses a custom public-facing interface instead of a default chat or form UI.
|
| 118 |
+
- It keeps secrets and GPU endpoints server-side while the client calls only
|
| 119 |
+
same-origin APIs.
|
| 120 |
+
|
| 121 |
+
## Current Verification
|
| 122 |
+
|
| 123 |
+
Recent local and deployed checks confirmed:
|
| 124 |
+
|
| 125 |
+
- The private Space serves the trusted `index.html` shell.
|
| 126 |
+
- `/analyze_image` returns validated mechanism payloads through the secured
|
| 127 |
+
Modal bearer-token flow.
|
| 128 |
+
- `/generate_scene` returns a deterministic Three.js scene descriptor, not raw
|
| 129 |
+
HTML.
|
| 130 |
+
- The renderer handles six-part scenes, axial orientation from geometry sizes,
|
| 131 |
+
geometry-aware motion-axis fallback, fitted camera framing, orbit controls,
|
| 132 |
+
reset view, mobile layout, and annotated-photo fallbacks.
|
| 133 |
+
- Unauthenticated Modal requests return `401 Unauthorized`.
|
| 134 |
|
| 135 |
+
## Run Locally
|
| 136 |
|
| 137 |
```powershell
|
| 138 |
+
pip install -r requirements.txt
|
| 139 |
+
python app.py
|
| 140 |
```
|
| 141 |
|
| 142 |
+
For UI-only development, set:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 143 |
|
| 144 |
+
```powershell
|
| 145 |
+
$env:INFERENCE_BACKEND="local"
|
| 146 |
+
python app.py
|
| 147 |
+
```
|
|
|
|
|
|
|
| 148 |
|
| 149 |
+
For the real demo path, configure the public variables and private token as
|
| 150 |
+
described in `.env.example` and `SECURITY.md`. Do not commit tokens, Modal
|
| 151 |
+
endpoint URLs, uploaded user photos, or downloaded model weights.
|
| 152 |
|
| 153 |
+
## Runtime Preflight
|
|
|
|
|
|
|
| 154 |
|
| 155 |
+
Check the selected GGUF and projector metadata before spending GPU time:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 156 |
|
| 157 |
```powershell
|
| 158 |
+
python scripts/verify_runtime_assets.py
|
| 159 |
```
|
| 160 |
|
| 161 |
+
Useful Modal checks:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 162 |
|
| 163 |
```powershell
|
| 164 |
python -m modal run modal_app.py::check_remote_imports
|
|
|
|
| 167 |
python -m modal run modal_app.py::run_analysis_endpoint_check
|
| 168 |
```
|
| 169 |
|
| 170 |
+
On Windows, use UTF-8 output when running Modal commands:
|
| 171 |
|
| 172 |
```powershell
|
| 173 |
+
$env:PYTHONIOENCODING="utf-8"; python -m modal run modal_app.py::run_runtime_preflight
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 174 |
```
|
| 175 |
|
| 176 |
+
## Repository Map
|
| 177 |
+
|
| 178 |
+
- `app.py` - Gradio Server app, same-origin APIs, image validation, rate limits.
|
| 179 |
+
- `index.html` - upload flow, analysis panel, Three.js renderer, fallback UI.
|
| 180 |
+
- `modal_app.py` - Modal deployment, llama.cpp runtime, diagnostics.
|
| 181 |
+
- `docs/` - implementation notes, review passes, and representative fixtures.
|
| 182 |
+
- `snap2sim/backend.py` - backend selection and authenticated Modal client.
|
| 183 |
+
- `snap2sim/prompts.py` - vision prompt contract.
|
| 184 |
+
- `snap2sim/schema.py` - mechanism JSON schema and sample payload.
|
| 185 |
+
- `scripts/verify_runtime_assets.py` - Hugging Face model metadata preflight.
|
| 186 |
+
- `SECURITY.md` - public/private handling rules for deployment.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 187 |
|
| 188 |
## Acknowledgements
|
| 189 |
|
docs/README.md
ADDED
|
@@ -0,0 +1,30 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Project Notes
|
| 2 |
+
|
| 3 |
+
This folder keeps implementation notes, review passes, and fixtures out of the
|
| 4 |
+
repository root while preserving the context that led to the current demo.
|
| 5 |
+
|
| 6 |
+
## Features
|
| 7 |
+
|
| 8 |
+
- `features/confidence-threshold.md` - confidence threshold control and
|
| 9 |
+
server-side render-mode selection.
|
| 10 |
+
- `features/annotated-photo-fallback.md` - contained photo fallback with
|
| 11 |
+
markers, leader lines, and annotation boxes.
|
| 12 |
+
- `features/renderer-vocabulary.md` - expanded deterministic primitive and
|
| 13 |
+
motion vocabulary.
|
| 14 |
+
- `features/vision-prompt-roles.md` - split system/user prompt contract for
|
| 15 |
+
llama.cpp analysis.
|
| 16 |
+
|
| 17 |
+
## Reviews
|
| 18 |
+
|
| 19 |
+
- `reviews/security-hardening.md` - Space and Modal security pass.
|
| 20 |
+
- `reviews/interaction-and-fallback-review.md` - camera controls, source-photo
|
| 21 |
+
preview, and fallback strategy.
|
| 22 |
+
- `reviews/scene-geometry-review.md` - primitive orientation, sizing, camera
|
| 23 |
+
fitting, and Y alignment.
|
| 24 |
+
- `reviews/motion-axis-bias.md` - prompt and renderer defaults for reducing
|
| 25 |
+
Y-axis motion bias.
|
| 26 |
+
|
| 27 |
+
## Fixtures
|
| 28 |
+
|
| 29 |
+
- `fixtures/mini-fan-analysis.json` - representative model output used to verify
|
| 30 |
+
deterministic scene geometry.
|
docs/features/annotated-photo-fallback.md
ADDED
|
@@ -0,0 +1,284 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Annotated-Photo Fallback: Image Sizing and Annotation Accuracy
|
| 2 |
+
|
| 3 |
+
Review date: 2026-06-14. Scope: one user report — when the **annotated-photo
|
| 4 |
+
fallback** triggers (instead of the 3D cutaway), the uploaded image is "too
|
| 5 |
+
large to display," and the annotation placement should be confirmed accurate.
|
| 6 |
+
|
| 7 |
+
This document is findings + next steps for Codex to implement. It does **not**
|
| 8 |
+
authorize any deployment or Hugging Face changes on its own; follow the normal
|
| 9 |
+
GitHub → HF sync and verification flow in `AGENTS.md`. All file/line references
|
| 10 |
+
are to `index.html` unless noted.
|
| 11 |
+
|
| 12 |
+
---
|
| 13 |
+
|
| 14 |
+
## TL;DR
|
| 15 |
+
|
| 16 |
+
1. **The image-sizing bug is real and confirmed in a browser.** The
|
| 17 |
+
annotated-photo image renders at its **intrinsic aspect height at full panel
|
| 18 |
+
width**, ignoring the viewport height, so tall photos overflow the viewport
|
| 19 |
+
by ~2x. `object-fit: contain` never engages because the box it should fit
|
| 20 |
+
inside is itself oversized.
|
| 21 |
+
2. **Root cause:** nested `display: grid; place-items: center` containers
|
| 22 |
+
(`.annotated-stage` → `.annotation-frame`) with auto-sized rows. A child's
|
| 23 |
+
`height: 100%` resolves against an *auto* (content-sized) grid row, not the
|
| 24 |
+
container's pixel height, so it collapses back to the image's natural size.
|
| 25 |
+
3. **The annotation accuracy is mostly broken *as a side effect* of the sizing
|
| 26 |
+
bug** — labels are positioned against the oversized, overflowing image box,
|
| 27 |
+
so any callout in the lower half of a tall photo lands below the visible
|
| 28 |
+
viewport and is clipped. Fixing the sizing fixes most of the misplacement.
|
| 29 |
+
A few annotation-accuracy improvements remain that are independent of sizing
|
| 30 |
+
(no marker dot at the point, edge-clamp detaches labels, unused `box`).
|
| 31 |
+
|
| 32 |
+
---
|
| 33 |
+
|
| 34 |
+
## How this was verified (not just code inspection)
|
| 35 |
+
|
| 36 |
+
I reproduced the exact `buildAnnotatedPhoto` DOM (`index.html:996-1076`) and the
|
| 37 |
+
exact CSS rules (`.fallback-stage`/`.annotated-stage`/`.annotation-frame`/
|
| 38 |
+
`.annotation-image`, `index.html:466-511`) in a standalone page, loaded a
|
| 39 |
+
**3000 × 4000 portrait** test image into a **900 × 600** viewport pane, and
|
| 40 |
+
measured the rendered geometry in a real browser (Chromium via Playwright):
|
| 41 |
+
|
| 42 |
+
| Scenario | Image rendered (W×H) | Fits 600px-tall pane? |
|
| 43 |
+
| --- | --- | --- |
|
| 44 |
+
| **Current code (baseline)** | **864 × 1151** | ❌ overflows (image bottom at 1209px) |
|
| 45 |
+
| Fix: pin image absolutely to padded box | 864 × 498 | ✅ contained (top 58 → bottom 556) |
|
| 46 |
+
| Fix: definite `minmax(0,1fr)` tracks at **both** grid levels | 864 × 498 | ✅ contained |
|
| 47 |
+
|
| 48 |
+
`864 = 900 − 18px×2` (panel width minus horizontal padding); `1151 ≈ 864 ×
|
| 49 |
+
(4000/3000)` — i.e. the image is laid out at panel width and natural aspect,
|
| 50 |
+
with **zero** vertical constraint. That is the "too large to display" symptom.
|
| 51 |
+
(The repro page was temporary and has been removed; re-create it if you want to
|
| 52 |
+
re-measure.)
|
| 53 |
+
|
| 54 |
+
---
|
| 55 |
+
|
| 56 |
+
## Root cause (precise)
|
| 57 |
+
|
| 58 |
+
The fallback markup is three nested boxes:
|
| 59 |
+
|
| 60 |
+
```
|
| 61 |
+
#viewport (absolute, inset:0) <- definite size, good
|
| 62 |
+
└─ .fallback-stage.annotated-stage index.html:1011-1012
|
| 63 |
+
position:absolute; inset:0 (definite height ✔)
|
| 64 |
+
display:grid; place-items:center; padding:58/18/44 index.html:490-495
|
| 65 |
+
└─ .annotation-frame index.html:1013-1014
|
| 66 |
+
width:100%; height:100% index.html:497-503
|
| 67 |
+
display:grid; place-items:center <-- second nested grid
|
| 68 |
+
├─ img.annotation-image index.html:1015-1018
|
| 69 |
+
│ width:100%; height:100%; object-fit:contain index.html:505-511
|
| 70 |
+
└─ .label-layer (absolute, inset:0)
|
| 71 |
+
```
|
| 72 |
+
|
| 73 |
+
Two compounding CSS facts:
|
| 74 |
+
|
| 75 |
+
- **`place-items: center` sets `align-items: center`, not `stretch`.** Combined
|
| 76 |
+
with the default `grid-auto-rows: auto`, the single grid row is **content
|
| 77 |
+
sized**, not stretched to the container's definite height.
|
| 78 |
+
- **A percentage `height` resolves against the grid *area* (the track), and an
|
| 79 |
+
`auto` track is *indefinite*.** So `height: 100%` on a child of a
|
| 80 |
+
`place-items: center` grid computes as `auto`.
|
| 81 |
+
|
| 82 |
+
This happens **twice**:
|
| 83 |
+
|
| 84 |
+
1. `.annotation-frame { height: 100% }` inside `.annotated-stage` →
|
| 85 |
+
the frame's track is auto → frame height becomes auto.
|
| 86 |
+
2. `img.annotation-image { height: 100% }` inside `.annotation-frame` →
|
| 87 |
+
the image's track is auto → image height becomes auto → the `<img>` uses its
|
| 88 |
+
intrinsic aspect ratio at the available width (864px) → 1151px tall.
|
| 89 |
+
|
| 90 |
+
Measured proof that the bug is the **inner** grid too: even after forcing
|
| 91 |
+
`.annotation-frame` to a correct definite 498px box, the image still rendered
|
| 92 |
+
1151px, because its `height:100%` resolves against `.annotation-frame`'s *inner
|
| 93 |
+
auto row*, not the frame's element box. Both grid levels must be fixed (or the
|
| 94 |
+
nested-grid sizing dropped entirely).
|
| 95 |
+
|
| 96 |
+
> Note: `.source-image` in the side panel (`index.html:384-390`) does **not**
|
| 97 |
+
> have this bug because it has a fixed `height: 180px` — a definite height, so
|
| 98 |
+
> `object-fit: contain` works there. The viewport fallback wants to fill
|
| 99 |
+
> available space rather than a fixed height, which is why it hit the trap.
|
| 100 |
+
|
| 101 |
+
---
|
| 102 |
+
|
| 103 |
+
## Fix options (both verified to contain the image at 864 × 498)
|
| 104 |
+
|
| 105 |
+
Pick one. **Option A is recommended** — fewest moving parts, no dependence on
|
| 106 |
+
fragile nested-grid percentage resolution.
|
| 107 |
+
|
| 108 |
+
### Option A (recommended): pin the image to the padded content box
|
| 109 |
+
|
| 110 |
+
Stop relying on `height: 100%` chaining through two grids. Make the image fill a
|
| 111 |
+
single absolutely-positioned box and let `object-fit: contain` do the framing.
|
| 112 |
+
|
| 113 |
+
```css
|
| 114 |
+
/* .annotated-stage can keep its background; drop the grid centering */
|
| 115 |
+
.annotation-frame {
|
| 116 |
+
position: absolute;
|
| 117 |
+
inset: 58px 18px 44px; /* same as the old .annotated-stage padding */
|
| 118 |
+
/* remove width/height:100% + display:grid + place-items:center */
|
| 119 |
+
}
|
| 120 |
+
.annotation-image {
|
| 121 |
+
position: absolute;
|
| 122 |
+
inset: 0;
|
| 123 |
+
width: 100%;
|
| 124 |
+
height: 100%;
|
| 125 |
+
object-fit: contain; /* now fits a definite box -> letterboxed, contained */
|
| 126 |
+
}
|
| 127 |
+
```
|
| 128 |
+
|
| 129 |
+
`.label-layer` already is `position:absolute; inset:0` (`index.html:466-471`),
|
| 130 |
+
so it keeps lining up with the frame. The `58/18/44` padding currently lives on
|
| 131 |
+
`.annotated-stage` (`index.html:493`); move that inset onto `.annotation-frame`
|
| 132 |
+
(or keep the padding on the stage and set `.annotation-frame { position:absolute;
|
| 133 |
+
inset:0 }`). Either way the frame becomes a **definite** box and the image is
|
| 134 |
+
contained.
|
| 135 |
+
|
| 136 |
+
### Option B: keep the grids but make every track definite
|
| 137 |
+
|
| 138 |
+
If you prefer to keep `display:grid` centering, the breakage is the auto rows —
|
| 139 |
+
give **both** grids definite tracks and let the items stretch:
|
| 140 |
+
|
| 141 |
+
```css
|
| 142 |
+
.annotated-stage {
|
| 143 |
+
display: grid;
|
| 144 |
+
grid-template-columns: minmax(0, 1fr);
|
| 145 |
+
grid-template-rows: minmax(0, 1fr);
|
| 146 |
+
place-items: stretch; /* not center: stretch the frame to fill */
|
| 147 |
+
padding: 58px 18px 44px;
|
| 148 |
+
}
|
| 149 |
+
.annotation-frame {
|
| 150 |
+
display: grid;
|
| 151 |
+
grid-template-columns: minmax(0, 1fr);
|
| 152 |
+
grid-template-rows: minmax(0, 1fr);
|
| 153 |
+
min-width: 0;
|
| 154 |
+
min-height: 0;
|
| 155 |
+
width: auto; /* track sizing handles it; 100% no longer needed */
|
| 156 |
+
height: auto;
|
| 157 |
+
}
|
| 158 |
+
.annotation-image { width: 100%; height: 100%; object-fit: contain; }
|
| 159 |
+
```
|
| 160 |
+
|
| 161 |
+
This also measured 864 × 498, contained. It is more brittle (relies on readers
|
| 162 |
+
understanding the `minmax(0,1fr)` + `min-height:0` idiom), hence Option A is
|
| 163 |
+
preferred.
|
| 164 |
+
|
| 165 |
+
> Whichever option: this is **CSS-only**. No change to `buildAnnotatedPhoto`'s
|
| 166 |
+
> JS, the schema, the backend, or `app.py` is required for the sizing fix.
|
| 167 |
+
|
| 168 |
+
---
|
| 169 |
+
|
| 170 |
+
## Annotation accuracy
|
| 171 |
+
|
| 172 |
+
### A) Most misplacement is a *symptom* of the sizing bug — fixing sizing fixes it
|
| 173 |
+
|
| 174 |
+
`updatePhotoLabels` (`index.html:1031-1062`) positions each callout from:
|
| 175 |
+
|
| 176 |
+
- `containedImageRect(image)` (`index.html:1096-1118`) — computes the
|
| 177 |
+
letterboxed sub-rect of the image from `image.naturalWidth/Height` and the
|
| 178 |
+
`<img>` bounding box, then
|
| 179 |
+
- `label.left = imageRect.left − layerRect.left + point[0] * imageRect.width`
|
| 180 |
+
(and the `point[1]` equivalent for top), clamped to the layer.
|
| 181 |
+
|
| 182 |
+
This math is **correct** *only when the `<img>` box matches the visible image
|
| 183 |
+
area*. With the current sizing bug the `<img>` box is 864 × 1151 and overflows
|
| 184 |
+
the viewport, so:
|
| 185 |
+
|
| 186 |
+
- `imageRect.width/height` come from the oversized box, so a `point[1] = 0.8`
|
| 187 |
+
callout is placed at ~0.8 × 1151 ≈ 920px down — **far below** the 600px
|
| 188 |
+
viewport, which is `overflow: hidden` (`.viewport-pane`, `index.html:105`), so
|
| 189 |
+
the label is clipped and the user never sees it.
|
| 190 |
+
- The clamp uses `layerRect.height` (also 1151), so the "keep on screen" clamp
|
| 191 |
+
(`index.html:1044-1047`) clamps to the wrong, oversized bounds.
|
| 192 |
+
|
| 193 |
+
**Once the image is contained (Option A/B), `containedImageRect` returns the
|
| 194 |
+
real letterboxed rect and the existing placement math is accurate.** So the
|
| 195 |
+
sizing fix is also the primary annotation-accuracy fix. Re-verify after fixing.
|
| 196 |
+
|
| 197 |
+
### B) Independent annotation-accuracy improvements (do after the sizing fix)
|
| 198 |
+
|
| 199 |
+
**User decision (2026-06-14): in scope — implement sizing fix *and* this
|
| 200 |
+
annotation-accuracy polish.** Do items #1–#3 below; #4 (1px border) and #5
|
| 201 |
+
(coercion clamp) are optional robustness items, implement if cheap.
|
| 202 |
+
|
| 203 |
+
1. **No marker at the actual point.** The callout box is centered *on* the point
|
| 204 |
+
via `.scene-label { transform: translate(-50%, -50%) }` (`index.html:476`),
|
| 205 |
+
so the text occludes the component instead of pointing at it, and an
|
| 206 |
+
edge-clamped label has nothing tying it back to the real location. Recommend
|
| 207 |
+
drawing a small dot/crosshair at the exact `point` (a 1–2px element, not
|
| 208 |
+
transformed) and offsetting the text label, optionally with a short leader
|
| 209 |
+
line. Numbered prefixes already exist (`(index + 1) + ". "`,
|
| 210 |
+
`index.html:1051`) but there's no matching number on the photo, so the number
|
| 211 |
+
is currently meaningless spatially.
|
| 212 |
+
|
| 213 |
+
2. **Edge clamp silently detaches the label from its point.** `Math.min(width −
|
| 214 |
+
72, Math.max(72, …))` (`index.html:1040-1047`) keeps the label on screen but,
|
| 215 |
+
with no leader/marker (see #1), a clamped label no longer indicates which
|
| 216 |
+
component it describes. The marker dot from #1 is the fix — keep the *dot* at
|
| 217 |
+
the true (clamped-to-image) point and only clamp the *text*.
|
| 218 |
+
|
| 219 |
+
3. **`annotation.box` is parsed and validated but never drawn.** The schema and
|
| 220 |
+
coercion carry an optional normalized `box: [x, y, w, h]`
|
| 221 |
+
(`snap2sim/schema.py:197-202`, `snap2sim/model_io.py:325-327`), but
|
| 222 |
+
`buildAnnotatedPhoto` ignores it. When present, drawing the bounding box over
|
| 223 |
+
the contained image would be a much stronger, more accurate annotation than a
|
| 224 |
+
single point. Optional enhancement.
|
| 225 |
+
|
| 226 |
+
4. **1px border offset.** `.annotation-image` has a `1px` border
|
| 227 |
+
(`index.html:509`) and `box-sizing: border-box` is global (`index.html:38-40`),
|
| 228 |
+
so `object-fit: contain` fits the *content* box (inside the border) while
|
| 229 |
+
`containedImageRect` measures the *border* box — a 1px placement error.
|
| 230 |
+
Negligible; mention only. If you want it exact, account for the border or use
|
| 231 |
+
`outline` instead of `border`.
|
| 232 |
+
|
| 233 |
+
5. **Coercion rejects (drops) out-of-range points instead of clamping.**
|
| 234 |
+
`_unit_number_list` returns `None` if any coordinate is outside `[0,1]`
|
| 235 |
+
(`snap2sim/model_io.py:311-315`), so a point like `[1.02, 0.5]` discards the
|
| 236 |
+
*entire* annotation and the part may then fall through to "unavailable". The
|
| 237 |
+
browser already `clamp01`s at render (`index.html:1120-1122`), so consider
|
| 238 |
+
clamping (not rejecting) server-side for resilience. Minor robustness item;
|
| 239 |
+
the prompt already asks for `[0,1]` (`snap2sim/prompts.py:31-34`).
|
| 240 |
+
|
| 241 |
+
The point-coordinate contract itself is consistent end-to-end: normalized
|
| 242 |
+
`[0,1]`, **origin top-left**, asserted in the prompt (`snap2sim/prompts.py:31-34`)
|
| 243 |
+
and consumed as `point[0]→x*width`, `point[1]→y*height` from the image's
|
| 244 |
+
top-left (`index.html:1040-1047`). No change needed there.
|
| 245 |
+
|
| 246 |
+
---
|
| 247 |
+
|
| 248 |
+
## Security / invariants to preserve
|
| 249 |
+
|
| 250 |
+
- This is a client-side CSS/JS change to an existing render path; keep the
|
| 251 |
+
"no model-authored HTML" rule from `AGENTS.md`/`SECURITY.md`. `label`/`note`
|
| 252 |
+
must stay `textContent`, never `innerHTML` (already correct,
|
| 253 |
+
`index.html:1051-1058`). A new marker-dot/box element must be built with
|
| 254 |
+
`document.createElement` + style, not injected markup.
|
| 255 |
+
- Keep using the **local** `currentPreviewUrl` object URL for the photo
|
| 256 |
+
(`index.html:1018`, `index.html:1134-1139`); do not round-trip the image back
|
| 257 |
+
from the server for display.
|
| 258 |
+
- No change to rate limiting, upload caps, or decompression-bomb guards in
|
| 259 |
+
`app.py` is required.
|
| 260 |
+
|
| 261 |
+
---
|
| 262 |
+
|
| 263 |
+
## Suggested implementation order for Codex
|
| 264 |
+
|
| 265 |
+
1. **Sizing fix first (Option A).** CSS-only in `index.html`. This is the bug
|
| 266 |
+
the user reported and it also corrects most annotation placement.
|
| 267 |
+
2. **Re-verify in a real browser** with both a **tall (portrait)** and a **wide
|
| 268 |
+
(landscape)** photo that triggers the annotate fallback: the whole image is
|
| 269 |
+
visible and contained, no viewport overflow/scroll, and callouts land on the
|
| 270 |
+
correct components within the image (especially lower-half points that were
|
| 271 |
+
previously clipped). Check the ≤860px stacked layout (`index.html:555-581`).
|
| 272 |
+
3. **Then** the annotation-accuracy items (in scope per the user): marker dot at
|
| 273 |
+
the true point + offset/leader label (#1, #2), and render `annotation.box`
|
| 274 |
+
when present (#3). Items #4/#5 optional.
|
| 275 |
+
4. Run the standard local checks per `AGENTS.md` (schema/parser + FastAPI
|
| 276 |
+
`TestClient` for `/`, `/analyze_image`, `/generate_scene`), then the normal
|
| 277 |
+
PR → GitHub Actions HF sync → authenticated private-Space verification.
|
| 278 |
+
|
| 279 |
+
## Resolved scope
|
| 280 |
+
|
| 281 |
+
- **Sizing fix + annotation-accuracy polish are both in scope**
|
| 282 |
+
(user decision, 2026-06-14): the CSS sizing fix, marker dots at the true
|
| 283 |
+
point, offset/leader labels, and rendering `annotation.box` when the model
|
| 284 |
+
provides it.
|
FEATURE.md → docs/features/confidence-threshold.md
RENAMED
|
@@ -1,4 +1,4 @@
|
|
| 1 |
-
#
|
| 2 |
|
| 3 |
Status: **Implemented and synced to the private Space, June 14, 2026.**
|
| 4 |
Supersedes the prior implemented version of this file (commit `a0540e9`,
|
|
@@ -179,7 +179,8 @@ accessible (don't regress the existing `aria-live` label).
|
|
| 179 |
only while a request is in flight, and re-enabled afterward.
|
| 180 |
- **Accessibility/layout (regression):** slider keyboard-operable, `#thresholdValue`
|
| 181 |
announced, does not block canvas OrbitControls (the pointer-events bug fixed in
|
| 182 |
-
the
|
|
|
|
| 183 |
- `INFERENCE_BACKEND=local` sample mode still renders the example analysis with
|
| 184 |
the slider present and the threshold honored at generation.
|
| 185 |
- Existing local checks pass: schema/parser checks and FastAPI `TestClient` for
|
|
|
|
| 1 |
+
# Confidence Threshold: Apply Only at Analysis and Generation
|
| 2 |
|
| 3 |
Status: **Implemented and synced to the private Space, June 14, 2026.**
|
| 4 |
Supersedes the prior implemented version of this file (commit `a0540e9`,
|
|
|
|
| 179 |
only while a request is in flight, and re-enabled afterward.
|
| 180 |
- **Accessibility/layout (regression):** slider keyboard-operable, `#thresholdValue`
|
| 181 |
announced, does not block canvas OrbitControls (the pointer-events bug fixed in
|
| 182 |
+
the `docs/reviews/interaction-and-fallback-review.md` pass), and no mobile
|
| 183 |
+
horizontal overflow.
|
| 184 |
- `INFERENCE_BACKEND=local` sample mode still renders the example analysis with
|
| 185 |
the slider present and the threshold honored at generation.
|
| 186 |
- Existing local checks pass: schema/parser checks and FastAPI `TestClient` for
|
docs/features/renderer-vocabulary.md
ADDED
|
@@ -0,0 +1,314 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Renderer Vocabulary: Richer Geometry, Motion, and Part Counts
|
| 2 |
+
|
| 3 |
+
Review date: 2026-06-14. Reviewer: Claude (Opus 4.8), for OpenAI Codex to
|
| 4 |
+
implement. Scope: one user goal — the vision prompt should be **more concise and
|
| 5 |
+
descriptive**, and the pipeline should give the model **more freedom over the
|
| 6 |
+
built-in Three.js geometry/motion vocabulary** and a **larger range of parts**.
|
| 7 |
+
|
| 8 |
+
This document is findings + next steps. It does **not** authorize any deployment
|
| 9 |
+
or Hugging Face changes on its own; follow the normal GitHub → HF sync and
|
| 10 |
+
verification flow in `AGENTS.md`. File/line references are to the repo state at
|
| 11 |
+
review time.
|
| 12 |
+
|
| 13 |
+
User decisions captured for this spec (2026-06-14):
|
| 14 |
+
|
| 15 |
+
- **Shapes:** add the *high-value set* — `cone`, `capsule`, `torus`, and a
|
| 16 |
+
`spring` (helix) builder — on top of the existing 5.
|
| 17 |
+
- **Motions:** add `screw` (helical: rotate + translate together), `orbit`
|
| 18 |
+
(revolve around a pivot point), and `pulse` (scale breathing) on top of the
|
| 19 |
+
existing 4.
|
| 20 |
+
- **Parts:** raise the usable cap to **6** (the renderer already allows 6).
|
| 21 |
+
|
| 22 |
+
---
|
| 23 |
+
|
| 24 |
+
## TL;DR — the one architectural fact that drives everything
|
| 25 |
+
|
| 26 |
+
**The prompt cannot grant freedom the renderer doesn't already have.** Because of
|
| 27 |
+
the hard security invariant — *no model-authored HTML; render deterministically
|
| 28 |
+
from validated JSON* (`AGENTS.md` Runtime Notes, `SECURITY.md` Agent Guidance,
|
| 29 |
+
`docs/reviews/security-hardening.md` §3) — the only things that can appear on
|
| 30 |
+
screen are shapes/motions
|
| 31 |
+
that the **browser renderer** explicitly builds in `buildPartMesh`
|
| 32 |
+
(`index.html:1141`) and `applyMotion` (`index.html:1195`).
|
| 33 |
+
|
| 34 |
+
So today the vocabulary is capped **identically in four places**, and any
|
| 35 |
+
expansion must change all four *in lockstep* (renderer is the source of truth):
|
| 36 |
+
|
| 37 |
+
| Layer | Shapes | Motions | Parts cap |
|
| 38 |
+
| --- | --- | --- | --- |
|
| 39 |
+
| Prompt (`snap2sim/prompts.py:26-27`, `:32`) | box, cylinder, sphere, gear, rod | rotate, translate, oscillate, static | "2 to 4" |
|
| 40 |
+
| Schema enum (`snap2sim/schema.py:136-139`, `:165-169`, `:214-215`) | same 5 | same 4 | no max |
|
| 41 |
+
| Coercion (`snap2sim/model_io.py:166`, `:169`, `:117`) | same 5 (else→box) | same 4 (else→static) | `parts[:4]` |
|
| 42 |
+
| Renderer (`index.html:842`, `:1145-1156`, `:1202-1210`, `:929`) | same 5 (else→box) | same 4 | `slice(0,6)` |
|
| 43 |
+
|
| 44 |
+
Two consequences:
|
| 45 |
+
1. **Parts is already inconsistent**: renderer renders 6, but the prompt asks for
|
| 46 |
+
≤4 and coercion truncates to 4. Raising the usable count to 6 is mostly
|
| 47 |
+
removing an artificial cap (see §3).
|
| 48 |
+
2. **Adding shapes/motions is a 4-layer change**, not a prompt edit. The order
|
| 49 |
+
that avoids a broken intermediate state is: **renderer → schema → coercion →
|
| 50 |
+
prompt** (build the capability, allow it, coerce toward it, then ask for it).
|
| 51 |
+
|
| 52 |
+
---
|
| 53 |
+
|
| 54 |
+
## Finding 1 — The prompt is verbose and under-describes the vocabulary it already has
|
| 55 |
+
|
| 56 |
+
`build_vision_prompt` (`snap2sim/prompts.py:13-56`) is ~40 lines and embeds a
|
| 57 |
+
full JSON skeleton, but:
|
| 58 |
+
|
| 59 |
+
- It lists shapes/motions as **bare enums** (`prompts.py:26-27`) with **zero
|
| 60 |
+
guidance on when to use each**. The model gets `box, cylinder, sphere, gear,
|
| 61 |
+
rod` and must guess mapping. In practice the example payload
|
| 62 |
+
(`EXAMPLE_ANALYSIS`, `schema.py:11-110`) only ever uses box/cylinder/gear, so
|
| 63 |
+
`sphere` and `rod` are effectively invisible to the model. The vocabulary is
|
| 64 |
+
underused before we even expand it.
|
| 65 |
+
- It repeats constraints (e.g. "compact", "physically plausible", "short" appear
|
| 66 |
+
multiple times across `:15-18`, `:31`, and the example) and spends many lines
|
| 67 |
+
re-stating field rules already enforced by `schema.py` / coerced by
|
| 68 |
+
`model_io.py`.
|
| 69 |
+
- `VISION_SYSTEM_PROMPT` (`prompts.py:5-10`) is terse and doesn't frame the
|
| 70 |
+
model's strengths (it's a *Reasoning* GGUF per `AGENTS.md`) or the "annotated
|
| 71 |
+
cutaway" product goal.
|
| 72 |
+
|
| 73 |
+
**Net:** the prompt is long on rules and short on *descriptive guidance*, which
|
| 74 |
+
is the opposite of what helps a reasoning model pick good primitives.
|
| 75 |
+
|
| 76 |
+
**A cheap win exists independent of any renderer change:** add a terse
|
| 77 |
+
"shape → use for" and "motion → use for" guide so the model exploits the
|
| 78 |
+
existing 5/4 vocabulary. That guide then grows naturally when we add the new
|
| 79 |
+
primitives.
|
| 80 |
+
|
| 81 |
+
---
|
| 82 |
+
|
| 83 |
+
## Finding 2 — High-value Three.js primitives are unused; the cutaway aesthetic wants them
|
| 84 |
+
|
| 85 |
+
The renderer hand-builds geometry in `buildPartMesh` (`index.html:1141-1167`)
|
| 86 |
+
and `gearGeometry` (`index.html:1169-1187`). Three.js ships many more primitives
|
| 87 |
+
that map cleanly onto real mechanism elements. The user picked the **high-value
|
| 88 |
+
set**:
|
| 89 |
+
|
| 90 |
+
| New shape | Three.js primitive | Real mechanism elements it covers |
|
| 91 |
+
| --- | --- | --- |
|
| 92 |
+
| `cone` | `ConeGeometry(radius, height, seg)` | valve cones, drill/screw tips, springs' seats, nozzles, pawl points |
|
| 93 |
+
| `capsule` | `CapsuleGeometry(radius, length, …)` | pistons, dowel pins, rollers, bearings, plungers, shafts with rounded ends |
|
| 94 |
+
| `torus` | `TorusGeometry(radius, tube, …)` | o-rings, retaining/snap rings, coils, washers (thick), seals |
|
| 95 |
+
| `spring` | custom helix via `TubeGeometry` + `CatmullRomCurve3` (like `gearGeometry` is custom) | compression/extension springs, coils, helical elements — *very* common and currently impossible to depict |
|
| 96 |
+
|
| 97 |
+
> Note on a latent inconsistency to clean up while here: the current `cylinder`
|
| 98 |
+
> branch (`index.html:1146`) already secretly supports a **cone/taper** by using
|
| 99 |
+
> `size[0]/2` as top radius and `size[2]/2` as bottom radius — but `size` is
|
| 100 |
+
> documented everywhere else as `[x, y, z]` extents, and nothing tells the model
|
| 101 |
+
> this. Introducing an explicit `cone` shape (with its own `size` semantics)
|
| 102 |
+
> removes the need for that undocumented trick. Decide whether `cylinder` should
|
| 103 |
+
> revert to a true cylinder (`radiusTop == radiusBottom`) once `cone` exists;
|
| 104 |
+
> recommended yes, to keep `size` semantics consistent.
|
| 105 |
+
|
| 106 |
+
### `size` semantics for the new shapes (must be documented in the prompt)
|
| 107 |
+
|
| 108 |
+
Keep the existing convention: `size: [x, y, z]` are bounding extents, and the
|
| 109 |
+
builder derives radii/lengths from them so the model never has to send
|
| 110 |
+
`radius`/`height` (which the prompt already forbids, `prompts.py:28-30`). Proposed:
|
| 111 |
+
|
| 112 |
+
- `cone`: base diameter from `max(x, z)`, height from `y`. Apex +y.
|
| 113 |
+
- `capsule`: diameter from `max(x, z)`, total length from `y` (long axis +y;
|
| 114 |
+
reuse the `rod` pattern if a horizontal default reads better — match `rod`'s
|
| 115 |
+
`rotateX` convention at `index.html:1151` for consistency).
|
| 116 |
+
- `torus`: outer diameter from `max(x, z)`, tube thickness from `y` (clamp tube
|
| 117 |
+
to a fraction of radius so it stays a ring, not a sphere).
|
| 118 |
+
- `spring`: outer diameter from `max(x, z)`, length/height from `y`; expose
|
| 119 |
+
`coils` (int, like `teeth` for gear, `schema.py:158`) and an optional
|
| 120 |
+
`wire` thickness with sane defaults.
|
| 121 |
+
|
| 122 |
+
All four must also be reachable through the same `color`, `rotation`, and
|
| 123 |
+
`position` handling that already wraps `buildPartMesh`
|
| 124 |
+
(`index.html:929-940`, color at `index.html:1158`/`colorFor` `:1228`).
|
| 125 |
+
|
| 126 |
+
---
|
| 127 |
+
|
| 128 |
+
## Finding 3 — Parts cap is artificially low and inconsistent
|
| 129 |
+
|
| 130 |
+
- Prompt says **"Use 2 to 4 parts"** (`prompts.py:32`).
|
| 131 |
+
- Coercion truncates with **`parts[:4]`** (`model_io.py:117`).
|
| 132 |
+
- Renderer already maps **`slice(0, 6)`** (`index.html:929`) and labels project
|
| 133 |
+
per mesh (`updateLabels`, `index.html:1213`).
|
| 134 |
+
- Schema imposes **no max** on `parts` (`schema.py:123-125`).
|
| 135 |
+
|
| 136 |
+
**Decision: usable cap = 6.** This is mostly *removing* a cap:
|
| 137 |
+
|
| 138 |
+
- Prompt: change "Use 2 to 4 parts" → "Use **2 to 6** parts; prefer the fewest
|
| 139 |
+
that explain the mechanism" (`prompts.py:32`).
|
| 140 |
+
- Coercion: `parts[:4]` → `parts[:6]` (`model_io.py:117`).
|
| 141 |
+
- Renderer: already 6 — no change, but double-check label legibility / overlap at
|
| 142 |
+
6 parts (`updateLabels` clamps at `index.html:1221-1222`).
|
| 143 |
+
- Schema: optionally add `"maxItems": 6` to the `parts` array
|
| 144 |
+
(`schema.py:123`) so over-long payloads fail validation predictably instead of
|
| 145 |
+
being silently truncated only on the coercion path (note: `model_io.py` runs
|
| 146 |
+
only on the Modal path, per `docs/reviews/interaction-and-fallback-review.md`;
|
| 147 |
+
the Space `validate_analysis` path
|
| 148 |
+
does **not** truncate, so a schema `maxItems` keeps both paths consistent).
|
| 149 |
+
|
| 150 |
+
---
|
| 151 |
+
|
| 152 |
+
## Finding 4 — Motions are hand-coded, not "built-in"; expand the custom set
|
| 153 |
+
|
| 154 |
+
`applyMotion` (`index.html:1195-1211`) implements all motion by hand: `rotate`
|
| 155 |
+
(continuous spin on `axis`), `oscillate` (sinusoidal rotation), `translate`
|
| 156 |
+
(sinusoidal slide along `axis` within `range`), `static`. Three.js has no
|
| 157 |
+
"motion" concept of its own, so "more built-in motions" = **add more primitives
|
| 158 |
+
to `applyMotion`**. User picked:
|
| 159 |
+
|
| 160 |
+
| New motion | Behavior | Mechanism examples | Params (extend schema) |
|
| 161 |
+
| --- | --- | --- | --- |
|
| 162 |
+
| `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` |
|
| 163 |
+
| `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]` |
|
| 164 |
+
| `pulse` | sinusoidal **scale** breathing | diaphragms, bladders, pumps, bellows, valves opening/closing | `speed`, `phase`, `amplitude` |
|
| 165 |
+
|
| 166 |
+
Implementation notes for `applyMotion`:
|
| 167 |
+
|
| 168 |
+
- It already resets to base each frame (`mesh.position.copy(basePosition)`,
|
| 169 |
+
`mesh.rotation.copy(baseRotation)`, `index.html:1200-1201`) — good; `screw` and
|
| 170 |
+
`orbit` can compose translate+rotate cleanly on top of that.
|
| 171 |
+
- `orbit` needs the pivot in world space; store it on `mesh.userData` at build
|
| 172 |
+
time like `basePosition` (`index.html:936`). Offset = position − pivot, rotate
|
| 173 |
+
the offset by `axis`, re-add pivot.
|
| 174 |
+
- `pulse` should multiply the **reveal** scale (`revealMesh`, `index.html:1189`)
|
| 175 |
+
rather than fight it — guard so the staggered reveal still plays first.
|
| 176 |
+
- Keep the existing param defaults pattern (`speed||1`, `phase||0`,
|
| 177 |
+
`amplitude||0.25`, `range||[-0.25,0.25]`, `index.html:1197-1207`).
|
| 178 |
+
|
| 179 |
+
Schema additions: extend the `motion` enum (`schema.py:165-169`, `_MOTIONS`
|
| 180 |
+
`:215`) and add optional `pitch` (number) and `pivot` (3-number list) to the
|
| 181 |
+
`motion` properties (`schema.py:162-186`), validated with the existing
|
| 182 |
+
`_require_number_list` helpers (`schema.py:326-329`). Coercion: extend the
|
| 183 |
+
allowed set (`model_io.py:169`) and carry the new params through (mirror
|
| 184 |
+
`_axis_vector`, `model_io.py:173-175`, `:290`).
|
| 185 |
+
|
| 186 |
+
---
|
| 187 |
+
|
| 188 |
+
## Finding 5 — The renderability gate and validators must learn the new vocabulary too
|
| 189 |
+
|
| 190 |
+
Three guards currently encode the old vocabulary and will silently reject or
|
| 191 |
+
downgrade the new shapes if missed:
|
| 192 |
+
|
| 193 |
+
1. **Browser renderability check** (`index.html:840-844`): the
|
| 194 |
+
`["box","cylinder","sphere","gear","rod"].includes(geometry.shape)` test
|
| 195 |
+
decides whether a part counts as renderable (feeds `render_mode` /
|
| 196 |
+
annotate-fallback selection, `schema.py:select_render_mode`). Add the new
|
| 197 |
+
shapes here or a part with `shape:"spring"` will be treated as non-renderable
|
| 198 |
+
and fall back to annotate-only.
|
| 199 |
+
2. **Schema validator** (`schema.py:_SHAPES` `:214`, `_MOTIONS` `:215`, enums
|
| 200 |
+
`:136-139`/`:165-169`) — extend both sets/enums.
|
| 201 |
+
3. **Coercion fallbacks** (`model_io.py:166-167`, `:169-170`) — extend the
|
| 202 |
+
allowed sets so a valid new shape isn't rewritten to `box`/`static`.
|
| 203 |
+
|
| 204 |
+
Also update `EXAMPLE_ANALYSIS` (`schema.py:11-110`) to exercise at least a couple
|
| 205 |
+
of the new primitives (e.g. a `spring` and a `capsule`) so the in-prompt example
|
| 206 |
+
actually demonstrates the wider vocabulary — the example is the single biggest
|
| 207 |
+
lever on what the model emits.
|
| 208 |
+
|
| 209 |
+
---
|
| 210 |
+
|
| 211 |
+
## Proposed prompt rewrite (concise + descriptive)
|
| 212 |
+
|
| 213 |
+
Goal: **shorter scaffolding, richer guidance.** Two concrete changes.
|
| 214 |
+
|
| 215 |
+
### A) `VISION_SYSTEM_PROMPT` (`prompts.py:5-10`) — frame the task and the model
|
| 216 |
+
|
| 217 |
+
Keep it ~4 lines but add: this is a *reasoning* model building an **annotated
|
| 218 |
+
technical cutaway**; reason briefly, then emit one JSON object; prefer the
|
| 219 |
+
*simplest set of primitives* that truthfully explains the mechanism; lower
|
| 220 |
+
confidence + annotate when unsure (already the policy — keep it).
|
| 221 |
+
|
| 222 |
+
### B) `build_vision_prompt` (`prompts.py:13-56`) — trim rules, add a vocabulary guide
|
| 223 |
+
|
| 224 |
+
- **Cut redundancy**: state "final answer = one JSON object, no markdown" once;
|
| 225 |
+
drop repeated "compact/short/plausible" restatements.
|
| 226 |
+
- **Replace bare enums with a terse guide** the model can act on, e.g.:
|
| 227 |
+
|
| 228 |
+
```
|
| 229 |
+
Shapes (pick the closest; size = [x,y,z] extents):
|
| 230 |
+
box plates, housings, blocks, levers, selectors
|
| 231 |
+
cylinder shafts, sleeves, bushings, drums, pins
|
| 232 |
+
cone valve cones, tips, nozzles, tapers
|
| 233 |
+
capsule pistons, rollers, dowel pins, plungers, bearings
|
| 234 |
+
sphere balls, detents, ball bearings, nodes
|
| 235 |
+
rod links, tie rods, thin axles, connecting rods
|
| 236 |
+
gear toothed wheels (set teeth); ratchets, cogs
|
| 237 |
+
torus o-rings, snap/retaining rings, seals, coils (single)
|
| 238 |
+
spring helical springs, coils (set coils)
|
| 239 |
+
|
| 240 |
+
Motions (axis is a numeric vector like [0,1,0]):
|
| 241 |
+
static fixed structure / housing
|
| 242 |
+
rotate continuous spin (speed)
|
| 243 |
+
oscillate sinusoidal twist (amplitude, speed)
|
| 244 |
+
translate slide along axis (range [min,max])
|
| 245 |
+
screw spin + advance along axis together (pitch) — screws, drills
|
| 246 |
+
orbit revolve around a pivot point (pivot [x,y,z]) — planetary/cranks
|
| 247 |
+
pulse scale breathing (amplitude) — diaphragms, pumps
|
| 248 |
+
```
|
| 249 |
+
|
| 250 |
+
- **Update the part-count line** to "Use 2 to 6 parts; prefer the fewest that
|
| 251 |
+
explain the mechanism."
|
| 252 |
+
- **Keep** the hard rules that protect parsing/rendering: numeric `size`/
|
| 253 |
+
`position`/`axis` vectors only, no `radius`/`height`/string-axis
|
| 254 |
+
(`prompts.py:28-30`), normalized `[0,1]` top-left annotation coords
|
| 255 |
+
(`prompts.py:31-34`), and "reason first, final answer is one JSON object"
|
| 256 |
+
(`prompts.py:14-17`, required by the `<think>`-stripping parser,
|
| 257 |
+
`docs/reviews/interaction-and-fallback-review.md`
|
| 258 |
+
Q3 / `model_io.py`).
|
| 259 |
+
- **Keep the example** but shrink it to a 2–3 part skeleton that now includes a
|
| 260 |
+
new primitive, and rely on `EXAMPLE_ANALYSIS` (the schema sample) to show the
|
| 261 |
+
full shape.
|
| 262 |
+
|
| 263 |
+
---
|
| 264 |
+
|
| 265 |
+
## Security / invariants to preserve (do not regress)
|
| 266 |
+
|
| 267 |
+
- **Deterministic rendering only.** All new shapes/motions are built in
|
| 268 |
+
`buildPartMesh`/`applyMotion` from validated JSON. **Never** inject
|
| 269 |
+
model-authored HTML/JS/markup (`AGENTS.md`, `SECURITY.md`,
|
| 270 |
+
`docs/reviews/security-hardening.md` §3).
|
| 271 |
+
`label`/`note` stay `textContent` (`index.html:1223`).
|
| 272 |
+
- **Validate before render.** New enum values must be added to `schema.py` so
|
| 273 |
+
`validate_analysis` (`schema.py:263`) still runs before scene generation; do
|
| 274 |
+
not loosen validation to "accept anything."
|
| 275 |
+
- **Coercion stays conservative.** Unknown shapes/motions must still fall back to
|
| 276 |
+
`box`/`static` (`model_io.py:166-170`), never crash.
|
| 277 |
+
- No change required to rate limiting, upload caps, decompression-bomb guards
|
| 278 |
+
(`app.py`), Modal bearer auth, or the GitHub→HF sync.
|
| 279 |
+
|
| 280 |
+
---
|
| 281 |
+
|
| 282 |
+
## Suggested implementation order for Codex
|
| 283 |
+
|
| 284 |
+
1. **Renderer first** (`index.html`): add `cone`, `capsule`, `torus`, and a
|
| 285 |
+
`spring` (helix) builder to `buildPartMesh`/a new helper next to
|
| 286 |
+
`gearGeometry`; add `screw`, `orbit`, `pulse` to `applyMotion`
|
| 287 |
+
(store `pivot` on `userData`). Update the renderability gate at
|
| 288 |
+
`index.html:840-844`. Verify each renders before touching Python.
|
| 289 |
+
2. **Schema** (`snap2sim/schema.py`): extend shape/motion enums + `_SHAPES`/
|
| 290 |
+
`_MOTIONS`; add `pitch`/`pivot`/`coils` properties; optional `parts.maxItems:
|
| 291 |
+
6`; update `EXAMPLE_ANALYSIS` to use a couple of new primitives.
|
| 292 |
+
3. **Coercion** (`snap2sim/model_io.py`): extend allowed shape/motion sets;
|
| 293 |
+
carry new params; change `parts[:4]` → `parts[:6]`.
|
| 294 |
+
4. **Prompt** (`snap2sim/prompts.py`): apply the concise rewrite (system + user),
|
| 295 |
+
the shape/motion guide, and "2 to 6 parts".
|
| 296 |
+
5. **Local verification** (per `AGENTS.md`): schema/parser checks, FastAPI
|
| 297 |
+
`TestClient` for `/`, `/analyze_image`, `/generate_scene`; and a **real
|
| 298 |
+
browser** check that each new shape and motion renders and that 5–6-part
|
| 299 |
+
scenes don't have unreadable overlapping labels.
|
| 300 |
+
6. **Optional Modal re-measure**
|
| 301 |
+
(`docs/reviews/interaction-and-fallback-review.md` Q3 tuning note): the richer prompt
|
| 302 |
+
may change token usage; confirm `run_analysis_endpoint_check` still returns
|
| 303 |
+
valid JSON within the current `4096`/`8192`/`300s` budgets before deploy.
|
| 304 |
+
7. Standard PR → GitHub Actions HF sync → authenticated private-Space
|
| 305 |
+
verification. Keep the Space **private** unless the user explicitly approves
|
| 306 |
+
going public (`AGENTS.md`, `SECURITY.md`).
|
| 307 |
+
|
| 308 |
+
## Open items to confirm with the user (only if they come up during build)
|
| 309 |
+
|
| 310 |
+
- Whether `cylinder` should revert to a true (untapered) cylinder once explicit
|
| 311 |
+
`cone` exists (recommended yes — see §2 note).
|
| 312 |
+
- Whether `spring`/`torus` need extra params (`coils`, `wire`/`tube` thickness)
|
| 313 |
+
exposed to the model or just sensible fixed defaults (recommend defaults first,
|
| 314 |
+
expose only if visual quality needs it).
|
docs/features/vision-prompt-roles.md
ADDED
|
@@ -0,0 +1,169 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Vision Prompt Roles: Split System and User Prompt
|
| 2 |
+
|
| 3 |
+
## Question being answered
|
| 4 |
+
|
| 5 |
+
> Is it better to split the **analysis** portion and the **generation** portion
|
| 6 |
+
> into different system prompts and user prompts? Would this help the model
|
| 7 |
+
> interpret the input and produce better output?
|
| 8 |
+
|
| 9 |
+
Short answer: **Yes, split — but split by chat *role* inside the single
|
| 10 |
+
analysis call, not into two model calls.** The "generation" step is not a model
|
| 11 |
+
step in this codebase, so the only useful split is system-vs-user within the one
|
| 12 |
+
vision call. Wiring that up is a small, safe change that aligns the prompt with
|
| 13 |
+
how the instruct/reasoning model was trained.
|
| 14 |
+
|
| 15 |
+
---
|
| 16 |
+
|
| 17 |
+
## Findings
|
| 18 |
+
|
| 19 |
+
### Finding 1 — There is only one model step; "generation" is deterministic
|
| 20 |
+
|
| 21 |
+
The pipeline is:
|
| 22 |
+
|
| 23 |
+
```
|
| 24 |
+
image -> [MODEL] vision analysis -> validated JSON -> [NO MODEL] Three.js scene
|
| 25 |
+
```
|
| 26 |
+
|
| 27 |
+
- Scene rendering is deterministic browser-side Three.js built from the
|
| 28 |
+
validated analysis JSON (`AGENTS.md:34-35`, `AGENTS.md:196-198`).
|
| 29 |
+
- `InferenceClient.generate_scene` (`snap2sim/backend.py:47-51`) does **no**
|
| 30 |
+
model inference — it validates the analysis and picks a `render_mode`. Model-
|
| 31 |
+
authored scene HTML/JS was intentionally removed (`SECURITY.md:96-97`).
|
| 32 |
+
|
| 33 |
+
**Implication:** "split analysis and generation into different prompts" cannot
|
| 34 |
+
mean *two model calls*. There is no generation model call to separate, and
|
| 35 |
+
adding a second round-trip would double an already slow path (~35s observed,
|
| 36 |
+
`300s` timeout at `modal_app.py:392`) for zero quality gain, since the scene is
|
| 37 |
+
deterministic. **Do not introduce a second inference step.**
|
| 38 |
+
|
| 39 |
+
### Finding 2 — The system prompt is currently dead code
|
| 40 |
+
|
| 41 |
+
- `VISION_SYSTEM_PROMPT` (`snap2sim/prompts.py:5-10`) is defined but **never
|
| 42 |
+
imported or used**. `modal_app.py` imports only `build_vision_prompt`
|
| 43 |
+
(`modal_app.py:20`).
|
| 44 |
+
- The real call, `run_llamacpp_prompt` (`modal_app.py:291-334`), passes the
|
| 45 |
+
prompt through a single `-p` flag (`modal_app.py:308-309`). There is **no**
|
| 46 |
+
system message.
|
| 47 |
+
- Net effect: the model receives one large *user* turn that mixes four different
|
| 48 |
+
concerns — role framing, the analytical task, the full output schema, and
|
| 49 |
+
field-by-field formatting rules (`build_vision_prompt`,
|
| 50 |
+
`snap2sim/prompts.py:12-70`).
|
| 51 |
+
|
| 52 |
+
So today there is effectively **no** system/user separation, and the one
|
| 53 |
+
"system" string we wrote is doing nothing.
|
| 54 |
+
|
| 55 |
+
### Finding 3 — The runtime supports a real role split
|
| 56 |
+
|
| 57 |
+
- `llama-mtmd-cli` accepts a separate system message via `-sys` alongside the
|
| 58 |
+
user prompt `-p`, and applies the model's chat template. (Confirmed against
|
| 59 |
+
llama.cpp `mtmd-cli` usage.)
|
| 60 |
+
- The model is `unsloth/NVIDIA-Nemotron-3-Nano-...-Reasoning-GGUF`
|
| 61 |
+
(`AGENTS.md:20`), an instruct/reasoning model. Such models are trained to
|
| 62 |
+
treat the **system** turn as persistent role + constraints and the **user**
|
| 63 |
+
turn as the immediate request. Collapsing everything into the user turn is
|
| 64 |
+
off-distribution and dilutes the per-request ask.
|
| 65 |
+
|
| 66 |
+
### Why a role split should help interpretation and output
|
| 67 |
+
|
| 68 |
+
1. **Aligns with training.** Stable behavior/output-contract belongs in system;
|
| 69 |
+
the moment-to-moment ask belongs in user. The model already expects this
|
| 70 |
+
shape.
|
| 71 |
+
2. **Sharpens the ask.** The user turn becomes short and image-focused
|
| 72 |
+
("Analyze the component in this photo …") instead of being buried under ~40
|
| 73 |
+
lines of schema rules.
|
| 74 |
+
3. **Separates invariants from the request.** The schema, shape/motion
|
| 75 |
+
vocabulary, and hard rules are constant across every image; they read as
|
| 76 |
+
*policy* in system, not as part of *this* request.
|
| 77 |
+
4. **Stops wasting the role channel.** We already wrote a system prompt; right
|
| 78 |
+
now it is ignored.
|
| 79 |
+
|
| 80 |
+
This is a low-risk change: it does not touch the schema, validator, coercion
|
| 81 |
+
(`snap2sim/schema.py`, `snap2sim/model_io.py`), or the deterministic renderer.
|
| 82 |
+
|
| 83 |
+
---
|
| 84 |
+
|
| 85 |
+
## Recommendation
|
| 86 |
+
|
| 87 |
+
**Adopt a system/user role split within the single vision call.** Concretely:
|
| 88 |
+
|
| 89 |
+
- **System message** = the invariant output contract:
|
| 90 |
+
- role + reasoning frame (it is a reasoning model; brief reasoning then JSON),
|
| 91 |
+
- the "emit exactly one JSON object, no markdown" rule,
|
| 92 |
+
- the JSON skeleton,
|
| 93 |
+
- the shape -> use-for and motion -> use-for vocabulary guide,
|
| 94 |
+
- the hard field rules (`size: [x,y,z]`, numeric axis vectors, 2–6 parts, no
|
| 95 |
+
`radius/height/length/width/depth`).
|
| 96 |
+
- **User message** = only the per-image ask: "Analyze the hardware component in
|
| 97 |
+
this photo as a cutaway mechanism and return the analysis JSON." (The image is
|
| 98 |
+
attached via `--image`.)
|
| 99 |
+
|
| 100 |
+
Keep it one inference call. Keep temperature, token, context, and timeout
|
| 101 |
+
budgets as-is (`modal_app.py:388-394`) — this is a prompt-structure change, not
|
| 102 |
+
a budget change.
|
| 103 |
+
|
| 104 |
+
---
|
| 105 |
+
|
| 106 |
+
## Next steps (for Codex to implement)
|
| 107 |
+
|
| 108 |
+
1. **`snap2sim/prompts.py`**
|
| 109 |
+
- Repurpose `VISION_SYSTEM_PROMPT` to hold the full invariant contract
|
| 110 |
+
(role + JSON-only rule + skeleton + shape/motion vocabulary guide + hard
|
| 111 |
+
field rules) currently living inside `build_vision_prompt`.
|
| 112 |
+
- Reduce `build_vision_prompt()` to the short per-image ask only. Consider
|
| 113 |
+
renaming it `build_vision_user_prompt()` (keep a back-compat alias if any
|
| 114 |
+
diagnostic entrypoint imports the old name).
|
| 115 |
+
- Optionally add `build_vision_messages()` returning `(system, user)` so call
|
| 116 |
+
sites have one source of truth.
|
| 117 |
+
|
| 118 |
+
2. **`snap2sim/prompts.py` (smoke test parity, optional)**
|
| 119 |
+
- The smoke-test prompt is a separate inline string
|
| 120 |
+
(`modal_app.py:211-234`). Leave functionally as-is, but it can reuse the
|
| 121 |
+
same system message for realism. Not required for the demo.
|
| 122 |
+
|
| 123 |
+
3. **`modal_app.py`**
|
| 124 |
+
- In `run_llamacpp_prompt` (`:291-334`), add an optional `system_prompt:
|
| 125 |
+
str | None = None` parameter and append `-sys <system_prompt>` to `cmd`
|
| 126 |
+
when provided. Verify the exact flag name against the deployed
|
| 127 |
+
`llama-mtmd-cli` build before relying on it (`-sys`); if the installed
|
| 128 |
+
build does not accept it, fall back to prepending the system text to `-p`
|
| 129 |
+
so behavior never regresses.
|
| 130 |
+
- In `analyze_image_llamacpp_payload` (`:385-400`), pass the new system
|
| 131 |
+
message + the short user prompt.
|
| 132 |
+
- Update the diagnostic entrypoints that build the prompt
|
| 133 |
+
(`run_analysis_raw_check` near `:489`, `run_analysis_endpoint_check`,
|
| 134 |
+
remote check near `:512`) to use the same two-part prompt so diagnostics
|
| 135 |
+
match production.
|
| 136 |
+
|
| 137 |
+
4. **Verify before/after (no regressions, measured improvement)**
|
| 138 |
+
- Local: schema/parser checks + FastAPI `TestClient` for `/`,
|
| 139 |
+
`/analyze_image`, `/generate_scene` still pass.
|
| 140 |
+
- Modal: run `run_analysis_raw_check` and `run_analysis_endpoint_check`
|
| 141 |
+
against the synthetic target image; confirm strict JSON still parses and
|
| 142 |
+
latency is comparable. Compare parse success / field quality vs. the
|
| 143 |
+
current single-`-p` prompt on a few representative real photos before
|
| 144 |
+
committing the prompt change.
|
| 145 |
+
- Confirm the flag actually took effect (e.g. inspect that `-sys` is honored,
|
| 146 |
+
not silently ignored) before trusting the split.
|
| 147 |
+
|
| 148 |
+
5. **Then deploy** following the normal path (Modal deploy of
|
| 149 |
+
`analyze_image_llamacpp`, GitHub `main` push, GitHub->HF sync, authenticated
|
| 150 |
+
private-Space verification), per the existing workflow in `AGENTS.md`.
|
| 151 |
+
|
| 152 |
+
---
|
| 153 |
+
|
| 154 |
+
## Explicitly NOT recommended
|
| 155 |
+
|
| 156 |
+
- **Two separate model calls** (an "analysis" call feeding a "generation"
|
| 157 |
+
call). There is no model generation step; the scene is deterministic. This
|
| 158 |
+
would only add latency and a failure mode.
|
| 159 |
+
- **Re-introducing model-authored scene HTML/JS.** Prohibited by
|
| 160 |
+
`SECURITY.md:96-97` and `AGENTS.md:35`.
|
| 161 |
+
|
| 162 |
+
## Open question for the user
|
| 163 |
+
|
| 164 |
+
The recommendation assumes you meant "should the prompt be *structured* into
|
| 165 |
+
system vs user roles" (yes), not "should the app make two separate model
|
| 166 |
+
requests" (no — there is no model generation step). If you specifically wanted
|
| 167 |
+
the model to also generate the scene/animation (re-adding a model generation
|
| 168 |
+
call), that conflicts with the current deterministic-renderer security decision
|
| 169 |
+
and should be discussed separately before implementing.
|
docs/fixtures/mini-fan-analysis.json
ADDED
|
@@ -0,0 +1,105 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
{
|
| 2 |
+
"component": "mini fan",
|
| 3 |
+
"confidence": 0.95,
|
| 4 |
+
"summary": "A compact desk fan with a motor-driven rotating blade assembly, wire cage, and plastic base.",
|
| 5 |
+
"trigger": "power switch",
|
| 6 |
+
"motion_sequence": [
|
| 7 |
+
"power on",
|
| 8 |
+
"blade rotation"
|
| 9 |
+
],
|
| 10 |
+
"parts": [
|
| 11 |
+
{
|
| 12 |
+
"id": "rotor",
|
| 13 |
+
"name": "blade assembly",
|
| 14 |
+
"role": "rotating element",
|
| 15 |
+
"geometry": {
|
| 16 |
+
"shape": "cylinder",
|
| 17 |
+
"size": [
|
| 18 |
+
0.12,
|
| 19 |
+
0.12,
|
| 20 |
+
0.35
|
| 21 |
+
],
|
| 22 |
+
"position": [
|
| 23 |
+
0,
|
| 24 |
+
0,
|
| 25 |
+
0
|
| 26 |
+
]
|
| 27 |
+
},
|
| 28 |
+
"motion": {
|
| 29 |
+
"type": "rotate",
|
| 30 |
+
"axis": [
|
| 31 |
+
0,
|
| 32 |
+
0,
|
| 33 |
+
1
|
| 34 |
+
],
|
| 35 |
+
"speed": 2
|
| 36 |
+
},
|
| 37 |
+
"annotation": {
|
| 38 |
+
"point": [
|
| 39 |
+
0.5,
|
| 40 |
+
0.5
|
| 41 |
+
],
|
| 42 |
+
"label": "hub",
|
| 43 |
+
"note": "central rotating hub"
|
| 44 |
+
}
|
| 45 |
+
},
|
| 46 |
+
{
|
| 47 |
+
"id": "cage",
|
| 48 |
+
"name": "protective cage",
|
| 49 |
+
"role": "static enclosure",
|
| 50 |
+
"geometry": {
|
| 51 |
+
"shape": "torus",
|
| 52 |
+
"size": [
|
| 53 |
+
0.55,
|
| 54 |
+
0.55,
|
| 55 |
+
0.04
|
| 56 |
+
],
|
| 57 |
+
"position": [
|
| 58 |
+
0,
|
| 59 |
+
0,
|
| 60 |
+
0
|
| 61 |
+
]
|
| 62 |
+
},
|
| 63 |
+
"motion": {
|
| 64 |
+
"type": "static"
|
| 65 |
+
},
|
| 66 |
+
"annotation": {
|
| 67 |
+
"point": [
|
| 68 |
+
0.5,
|
| 69 |
+
0.5
|
| 70 |
+
],
|
| 71 |
+
"label": "cage",
|
| 72 |
+
"note": "wire mesh surrounding blades"
|
| 73 |
+
}
|
| 74 |
+
},
|
| 75 |
+
{
|
| 76 |
+
"id": "base",
|
| 77 |
+
"name": "stand",
|
| 78 |
+
"role": "support",
|
| 79 |
+
"geometry": {
|
| 80 |
+
"shape": "box",
|
| 81 |
+
"size": [
|
| 82 |
+
0.45,
|
| 83 |
+
0.25,
|
| 84 |
+
0.2
|
| 85 |
+
],
|
| 86 |
+
"position": [
|
| 87 |
+
0,
|
| 88 |
+
-0.15,
|
| 89 |
+
0
|
| 90 |
+
]
|
| 91 |
+
},
|
| 92 |
+
"motion": {
|
| 93 |
+
"type": "static"
|
| 94 |
+
},
|
| 95 |
+
"annotation": {
|
| 96 |
+
"point": [
|
| 97 |
+
0.5,
|
| 98 |
+
0.85
|
| 99 |
+
],
|
| 100 |
+
"label": "base",
|
| 101 |
+
"note": "green plastic base"
|
| 102 |
+
}
|
| 103 |
+
}
|
| 104 |
+
]
|
| 105 |
+
}
|
docs/reviews/interaction-and-fallback-review.md
ADDED
|
@@ -0,0 +1,325 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Interaction and Fallback Review: Camera Control and Fallback Strategy
|
| 2 |
+
|
| 3 |
+
Review date: 2026-06-14. Scope: two questions raised by the user —
|
| 4 |
+
(1) is the cutaway camera movable, and (2) what should the analysis/generation
|
| 5 |
+
fallback be. This document is findings + next steps for Codex to implement.
|
| 6 |
+
It does **not** authorize any deployment or Hugging Face changes on its own;
|
| 7 |
+
follow the normal GitHub→HF sync and verification flow in `AGENTS.md`.
|
| 8 |
+
|
| 9 |
+
---
|
| 10 |
+
|
| 11 |
+
## Question 1 — Is the camera movable?
|
| 12 |
+
|
| 13 |
+
**Update (2026-06-14): the user reports orbit does NOT work in practice.** The
|
| 14 |
+
`OrbitControls` code *is* present, but a layout bug stops it from receiving input.
|
| 15 |
+
Codex must double-check the camera end-to-end (see root-cause + checklist below),
|
| 16 |
+
not assume it works because the control is instantiated.
|
| 17 |
+
|
| 18 |
+
The control is wired up in `index.html`:
|
| 19 |
+
- `OrbitControls` is imported and merged onto `window.THREE`
|
| 20 |
+
(`index.html:19`, `index.html:21`).
|
| 21 |
+
- It is instantiated per scene with damping in `buildDeterministicScene`
|
| 22 |
+
(`index.html:630-636`): `enableDamping = true`, `target.set(0, 0.1, 0)`.
|
| 23 |
+
- `controls.update()` runs every frame in `animate()` (`index.html:690`), which
|
| 24 |
+
is required for damping to work, and HTML labels reproject each frame via
|
| 25 |
+
`updateLabels` (`index.html:692`, `index.html:797-810`), so callouts track the
|
| 26 |
+
camera as it moves.
|
| 27 |
+
|
| 28 |
+
### Root-cause (prime suspect): the label layer eats pointer events
|
| 29 |
+
|
| 30 |
+
`OrbitControls` attaches its listeners to `renderer.domElement` (the canvas),
|
| 31 |
+
but a sibling overlay covers that canvas and swallows the mouse/touch events:
|
| 32 |
+
|
| 33 |
+
- In `buildDeterministicScene`, `stage.append(mount, labelLayer)`
|
| 34 |
+
(`index.html:617`) appends `labelLayer` **after** `mount`, so it paints on top.
|
| 35 |
+
- `.fallback-stage`, `.scene-mount`, and `.label-layer` all use
|
| 36 |
+
`position: absolute; inset: 0` with no `z-index` (`index.html:357-362`), so
|
| 37 |
+
`.label-layer` covers the entire viewport, directly over the canvas.
|
| 38 |
+
- `.scene-label` has `pointer-events: none` (`index.html:374`) **but the
|
| 39 |
+
`.label-layer` container does not.** A plain `<div>` defaults to
|
| 40 |
+
`pointer-events: auto`, so the layer intercepts every `mousedown`, `wheel`,
|
| 41 |
+
and touch before it can reach the canvas. OrbitControls therefore never sees a
|
| 42 |
+
drag or scroll → orbit/zoom/pan appear dead.
|
| 43 |
+
|
| 44 |
+
**Fix:** add `pointer-events: none` to `.label-layer` (keep it on `.scene-label`,
|
| 45 |
+
which is already correct). The labels are non-interactive, so this is safe.
|
| 46 |
+
|
| 47 |
+
### Camera verification checklist for Codex
|
| 48 |
+
|
| 49 |
+
Do not close this out on code inspection alone — verify in a real browser:
|
| 50 |
+
|
| 51 |
+
1. Apply the `.label-layer { pointer-events: none; }` fix and confirm
|
| 52 |
+
left-drag orbits, wheel zooms, right-drag pans.
|
| 53 |
+
2. Confirm `window.THREE.OrbitControls` is actually defined at the moment
|
| 54 |
+
`buildDeterministicScene` runs (the `type="module"` import at
|
| 55 |
+
`index.html:17-22` is deferred; if a payload could render before that module
|
| 56 |
+
executes, `controls` would be `null` and the camera would be locked —
|
| 57 |
+
`index.html:630-632` already guards with a null check, so a missing import
|
| 58 |
+
fails silently to a static camera).
|
| 59 |
+
3. Confirm no other overlay intercepts events after load: `.drop-zone`
|
| 60 |
+
(`index.html:93-117`) must be `hidden` once a scene renders
|
| 61 |
+
(`resetScene` does this at `index.html:569`), and `.viewport-pane::after`
|
| 62 |
+
/`.scan-line` are already `pointer-events: none` (`index.html:84`,
|
| 63 |
+
`index.html:129`).
|
| 64 |
+
4. Verify on touch / the ≤860px layout (`index.html:388-404`).
|
| 65 |
+
|
| 66 |
+
Once orbit is confirmed working, apply the polish below. **Recommended polish
|
| 67 |
+
(low risk, no backend change):**
|
| 68 |
+
|
| 69 |
+
- **Distance clamps.** Set `controls.minDistance` / `controls.maxDistance`
|
| 70 |
+
(and optionally `maxPolarAngle`) so users can't zoom inside a mesh or fly
|
| 71 |
+
under the grid and lose the model.
|
| 72 |
+
- **Reset-view control.** Add a small "Reset view" tool button next to
|
| 73 |
+
`Play`/`Load` (`index.html:411-414`) that restores `camera.position` and
|
| 74 |
+
`controls.target` to the defaults (`camera.position.set(5.4, 3.9, 6.2)`,
|
| 75 |
+
target `(0, 0.1, 0)`). Store the defaults once so reset is exact.
|
| 76 |
+
- **Affordance hint.** Briefly show a "Drag to orbit · scroll to zoom" hint on
|
| 77 |
+
first cutaway, or a persistent muted caption in the viewport. Nothing today
|
| 78 |
+
tells the user the scene is interactive.
|
| 79 |
+
- **Touch / mobile.** `OrbitControls` supports touch by default, but verify on
|
| 80 |
+
the ≤860px layout (`index.html:388-404`) that one-finger orbit / two-finger
|
| 81 |
+
zoom don't fight page scroll; set `controls.touches` and/or
|
| 82 |
+
`renderer.domElement.style.touchAction = "none"` if they do.
|
| 83 |
+
- **Accessibility gap.** `#viewport` is `aria-hidden="true"` (`index.html:415`).
|
| 84 |
+
That's defensible for a canvas, but the interactive camera is then invisible
|
| 85 |
+
to assistive tech. Pair the visual hint with the existing screen-reader status
|
| 86 |
+
region, and ensure the textual readout panel remains the accessible source of
|
| 87 |
+
truth for part names/roles.
|
| 88 |
+
|
| 89 |
+
**Bottom line:** do not "add" a movable camera — it exists. Implement the polish
|
| 90 |
+
items above so the existing capability is obvious and bounded.
|
| 91 |
+
|
| 92 |
+
---
|
| 93 |
+
|
| 94 |
+
## Question 2 — What should the fallback be?
|
| 95 |
+
|
| 96 |
+
**User decision (2026-06-14): the fallback should annotate the uploaded photo**
|
| 97 |
+
— labeled callouts on each visible component plus a short description — instead
|
| 98 |
+
of always showing a fixed/generic 3D scene.
|
| 99 |
+
|
| 100 |
+
### Current behavior (the "fixed value" today)
|
| 101 |
+
|
| 102 |
+
There are two distinct fallback paths, both of which ignore or only loosely
|
| 103 |
+
relate to the actual photo:
|
| 104 |
+
|
| 105 |
+
1. **Local / demo mode** (`INFERENCE_BACKEND=local`, which is what the HF Space
|
| 106 |
+
currently runs). `InferenceClient.analyze_image` returns the **fixed**
|
| 107 |
+
`EXAMPLE_ANALYSIS` ratchet-wrench payload for *every* image
|
| 108 |
+
(`snap2sim/backend.py:36-45`, `snap2sim/schema.py:10-89`). So in the live
|
| 109 |
+
demo, uploading a photo of anything renders the same ratchet cutaway. This is
|
| 110 |
+
the misleading "fixed value" the user is reacting to.
|
| 111 |
+
2. **Modal real-model mode.** `analyze_image_llamacpp_payload` runs the model,
|
| 112 |
+
then on parse failure calls `coerce_analysis_response`
|
| 113 |
+
(`modal_app.py:380-394`). Worst case that bottoms out in `_generic_analysis`
|
| 114 |
+
— a generic housing/rotor/guide 3D scene (`snap2sim/model_io.py:46`,
|
| 115 |
+
`snap2sim/model_io.py:170-204`). Still a fixed-ish 3D primitive not tied to
|
| 116 |
+
the image.
|
| 117 |
+
|
| 118 |
+
Note: `snap2sim/model_io.py` (parse/coerce) is **only** used by `modal_app.py`.
|
| 119 |
+
The Space-side `app.py` → `backend.py` path does not call it, so coercion never
|
| 120 |
+
runs in the local placeholder path. Keep this in mind when wiring the new
|
| 121 |
+
fallback so it actually triggers on the deployed Space.
|
| 122 |
+
|
| 123 |
+
### Target behavior — annotated-photo fallback
|
| 124 |
+
|
| 125 |
+
When a confident, per-image 3D mechanism can't be produced, render the user's
|
| 126 |
+
**own photo** with numbered callouts on each identified component and a text
|
| 127 |
+
description of what it is / how it likely works. This is honest about
|
| 128 |
+
uncertainty and never shows an unrelated mechanism.
|
| 129 |
+
|
| 130 |
+
Decision logic the browser should follow (in `index.html`):
|
| 131 |
+
1. **Full success** — valid analysis with parts *and* usable primitive geometry
|
| 132 |
+
→ render the interactive 3D cutaway (current path, with the camera polish
|
| 133 |
+
from Q1).
|
| 134 |
+
2. **Annotated-photo fallback** — model identified components and gave 2D image
|
| 135 |
+
locations, but 3D geometry/motion is missing or low-confidence → overlay
|
| 136 |
+
callouts on the source photo + descriptions. **This is the new default
|
| 137 |
+
fallback.**
|
| 138 |
+
3. **Last resort** — nothing usable at all → keep a clearly-labeled, honest
|
| 139 |
+
"couldn't analyze" state (retry affordance already exists at
|
| 140 |
+
`index.html:427`, `index.html:530-531`). Do **not** silently fall back to the
|
| 141 |
+
fixed ratchet scene.
|
| 142 |
+
|
| 143 |
+
### Why the schema must change first
|
| 144 |
+
|
| 145 |
+
The annotated-photo view needs **2D image coordinates per component**, which the
|
| 146 |
+
current contract does not carry. The analysis schema only has 3D
|
| 147 |
+
`geometry.position` (`snap2sim/schema.py:92-169`) — that's world-space for
|
| 148 |
+
Three.js, not pixel space on the photo. Implement in this order:
|
| 149 |
+
|
| 150 |
+
1. **Extend the schema** (`snap2sim/schema.py`): add an optional per-part
|
| 151 |
+
`annotation` object, e.g.
|
| 152 |
+
`{"point": [x, y], "box": [x, y, w, h]?, "label": str, "note": str}` with
|
| 153 |
+
`point`/`box` in **normalized [0,1] image coordinates**. Make it optional so
|
| 154 |
+
existing 3D-only payloads still validate. Add validation helpers mirroring the
|
| 155 |
+
existing `_require_number_list` style, and bound coordinates to [0,1].
|
| 156 |
+
2. **Update the prompt** (`snap2sim/prompts.py`): instruct the model to include
|
| 157 |
+
`annotation.point` (and `note`) per part when it can locate the component in
|
| 158 |
+
the image, in normalized coordinates with origin at top-left. Keep the
|
| 159 |
+
"JSON only, no markdown, no `<think>`" constraints already there.
|
| 160 |
+
3. **Update parsing/coercion** (`snap2sim/model_io.py`): carry `annotation`
|
| 161 |
+
through `_coerce_part`; treat missing/invalid annotations as simply absent
|
| 162 |
+
(don't fail the whole part). Decide the fallback trigger: e.g. if a part has a
|
| 163 |
+
valid `annotation` but no usable `geometry`, mark the payload as
|
| 164 |
+
"annotate-only" so the browser picks path 2. A top-level `render_mode` hint
|
| 165 |
+
(`"three"` vs `"annotate"`) returned by `generate_scene` is the cleanest
|
| 166 |
+
signal — `generate_scene` already returns `{"renderer": ..., "analysis": ...}`
|
| 167 |
+
(`snap2sim/backend.py:47-49`), so extend that envelope rather than inventing a
|
| 168 |
+
new channel.
|
| 169 |
+
4. **Browser render path** (`index.html`): add a `buildAnnotatedPhoto(analysis)`
|
| 170 |
+
alongside `buildDeterministicScene` (`index.html:597`). Reuse the existing
|
| 171 |
+
source-photo capture — the upload is already shown via `showSourcePreview`
|
| 172 |
+
/ `currentPreviewUrl` (`index.html:718-723`) — draw the photo into the
|
| 173 |
+
viewport and position absolutely-placed callouts using the normalized
|
| 174 |
+
`annotation.point` scaled to the rendered image rect. Reuse the existing
|
| 175 |
+
`.scene-label` styling (`index.html:364-375`) for visual consistency. `Play`
|
| 176 |
+
should be disabled/hidden in this mode (no animation).
|
| 177 |
+
5. **Demo honesty (local mode):** decide what the placeholder backend does. The
|
| 178 |
+
fixed ratchet for-every-image (`backend.py:45`) should not remain the live
|
| 179 |
+
demo experience. Options for Codex to confirm with the user before shipping:
|
| 180 |
+
either (a) point the Space at the Modal model so real analysis runs, or
|
| 181 |
+
(b) keep a placeholder but clearly label it "DEMO — sample mechanism, not
|
| 182 |
+
your photo" so it isn't mistaken for real analysis.
|
| 183 |
+
|
| 184 |
+
### Validation / security notes (keep the security-hardening pass intact)
|
| 185 |
+
|
| 186 |
+
- Normalized [0,1] annotation coords are safe to render as positioned DOM, but
|
| 187 |
+
still **clamp** them client-side (like `updateLabels` already clamps,
|
| 188 |
+
`index.html:805-806`) and treat `label`/`note` as text (`textContent`, never
|
| 189 |
+
`innerHTML`) to preserve the "no model-authored HTML" rule from `AGENTS.md`
|
| 190 |
+
and `docs/reviews/security-hardening.md`.
|
| 191 |
+
- The photo for annotation is the **local** `currentPreviewUrl` object URL; do
|
| 192 |
+
not round-trip the image back from the server for display.
|
| 193 |
+
- No change to rate limiting, upload caps, or decompression-bomb handling in
|
| 194 |
+
`app.py` is required for this work.
|
| 195 |
+
|
| 196 |
+
---
|
| 197 |
+
|
| 198 |
+
## Question 3 — Let the model think longer / use more token context
|
| 199 |
+
|
| 200 |
+
**User request (2026-06-14): allow the AI model to think for longer and use more
|
| 201 |
+
token context when it needs to.** The selected model is a *Reasoning* GGUF
|
| 202 |
+
(`unsloth/NVIDIA-Nemotron-3-Nano-Omni-30B-A3B-Reasoning-GGUF`, per `AGENTS.md`),
|
| 203 |
+
so giving it room to reason should improve analysis quality on hard photos.
|
| 204 |
+
|
| 205 |
+
Current constraints that limit thinking:
|
| 206 |
+
- **The prompt forbids reasoning.** `build_vision_prompt` tells the model "Do not
|
| 207 |
+
include a reasoning trace or `<think>` tags" (`snap2sim/prompts.py:15-17`), and
|
| 208 |
+
`VISION_SYSTEM_PROMPT` (`snap2sim/prompts.py:5-9`) is terse. This actively
|
| 209 |
+
suppresses the reasoning the model was trained for.
|
| 210 |
+
- **Generation budget is small.** `analyze_image_llamacpp_payload` calls with
|
| 211 |
+
`max_tokens=1536` (`modal_app.py:386`); `run_llamacpp_prompt` default is `1024`
|
| 212 |
+
(`modal_app.py:292`) and passes it as `-n` (`modal_app.py:307-308`). If the
|
| 213 |
+
model thinks first, 1536 tokens can be exhausted before the JSON is emitted.
|
| 214 |
+
- **No explicit context window.** The `llama-mtmd-cli` command sets `-n` (output)
|
| 215 |
+
but never `-c` / `--ctx-size` (`modal_app.py:299-311`), so it falls back to the
|
| 216 |
+
build default context size. A long `<think>` block plus the multimodal image
|
| 217 |
+
tokens can overflow the default window and truncate output.
|
| 218 |
+
- **Timeout.** `timeout_seconds=180` at the call site (`modal_app.py:387`) and as
|
| 219 |
+
the default (`modal_app.py:293`); longer reasoning needs more wall-clock budget.
|
| 220 |
+
|
| 221 |
+
### Changes to implement (Codex)
|
| 222 |
+
|
| 223 |
+
1. **Allow reasoning in the prompts** (`snap2sim/prompts.py`): remove the "do not
|
| 224 |
+
include a reasoning trace or `<think>` tags" instruction and instead permit
|
| 225 |
+
the model to reason first, then emit the final JSON. Keep the hard requirement
|
| 226 |
+
that the **final answer is a single JSON object** so parsing still works.
|
| 227 |
+
2. **Strip the reasoning before parsing** (`snap2sim/model_io.py`): add explicit
|
| 228 |
+
handling to drop any `<think>...</think>` block (and any prose before the JSON)
|
| 229 |
+
prior to `parse_analysis_response`. The existing `_json_object_candidates`
|
| 230 |
+
scanner (`snap2sim/model_io.py:53-62`) already searches for balanced `{...}`
|
| 231 |
+
objects anywhere in the text and tries each candidate, so it is largely robust
|
| 232 |
+
to a leading reasoning trace — but a `<think>` block can itself contain `{`
|
| 233 |
+
characters that produce false candidates, so strip it first for reliability.
|
| 234 |
+
Prefer the **last** complete top-level JSON object when reasoning precedes the
|
| 235 |
+
answer.
|
| 236 |
+
3. **Raise the generation budget and context window** (`modal_app.py`):
|
| 237 |
+
- Increase `max_tokens` for analysis (e.g. 1536 → ~3072–4096) at
|
| 238 |
+
`modal_app.py:386` and/or the `run_llamacpp_prompt` default
|
| 239 |
+
(`modal_app.py:292`).
|
| 240 |
+
- Add an explicit `-c` / `--ctx-size` flag to the `llama-mtmd-cli` command
|
| 241 |
+
(`modal_app.py:299-311`) sized for image tokens + reasoning + JSON
|
| 242 |
+
(e.g. 8192; confirm against the model card's supported context and Modal GPU
|
| 243 |
+
memory). Make it a parameter of `run_llamacpp_prompt` with a sensible
|
| 244 |
+
default so the smoke test and analysis path can differ.
|
| 245 |
+
- Raise `timeout_seconds` (`modal_app.py:387`, default `modal_app.py:293`) to
|
| 246 |
+
match the larger budget; the existing `TimeoutExpired` handling
|
| 247 |
+
(`modal_app.py:317-325`) already surfaces partial output.
|
| 248 |
+
4. **Keep latency acceptable for the demo.** More thinking = slower responses.
|
| 249 |
+
The browser already shows cold-start language ("WAKING THE WORKSHOP...",
|
| 250 |
+
`index.html:539`); confirm the front-end `INFERENCE_TIMEOUT_SECONDS`
|
| 251 |
+
(`snap2sim/backend.py:23`, default 180) and any HF Space/Modal request
|
| 252 |
+
timeouts are raised in lockstep so longer reasoning doesn't get cut off by an
|
| 253 |
+
intermediate layer.
|
| 254 |
+
5. **Verify** with `run_analysis_endpoint_check` (`modal_app.py:412`) and the
|
| 255 |
+
smoke test (`modal_app.py:289`+) that the larger budgets still return valid
|
| 256 |
+
JSON, then re-run the standard endpoint verification before deploy.
|
| 257 |
+
|
| 258 |
+
> Tuning note: pick the smallest token/context/timeout values that reliably yield
|
| 259 |
+
> valid JSON on representative photos — over-provisioning context wastes GPU
|
| 260 |
+
> memory and inflates demo latency. Treat the numbers above as starting points to
|
| 261 |
+
> measure, not fixed targets.
|
| 262 |
+
|
| 263 |
+
---
|
| 264 |
+
|
| 265 |
+
## Question 4 — Source photo preview is sized/cropped incorrectly
|
| 266 |
+
|
| 267 |
+
**User report (2026-06-14): the uploaded image is not sized properly in the
|
| 268 |
+
"Source Photo" card.** The preview crops the photo instead of showing it whole.
|
| 269 |
+
|
| 270 |
+
Cause in `index.html`:
|
| 271 |
+
- `.source-image` uses `width: 100%; max-height: 150px; object-fit: cover`
|
| 272 |
+
(`index.html:275-281`). `object-fit: cover` **fills and crops** the box, so any
|
| 273 |
+
portrait or non-matching-aspect photo gets its top/bottom (or sides) clipped —
|
| 274 |
+
the user can't see the full component they uploaded.
|
| 275 |
+
- There is no fixed/defined preview area, only a `max-height`. With `width: 100%`
|
| 276 |
+
and no explicit height, the rendered height is aspect-driven and clamped at
|
| 277 |
+
150px, so wide images shrink while tall images crop — inconsistent framing.
|
| 278 |
+
- The element is `<img id="sourceImage" class="source-image">`
|
| 279 |
+
(`index.html:432`), populated by `showSourcePreview` via an object URL
|
| 280 |
+
(`index.html:718-723`); the markup/JS are fine, this is purely a CSS sizing
|
| 281 |
+
choice.
|
| 282 |
+
|
| 283 |
+
### Changes to implement (Codex)
|
| 284 |
+
|
| 285 |
+
1. **Show the whole photo, not a crop.** Switch `.source-image` to
|
| 286 |
+
`object-fit: contain` (`index.html:277`) so the entire image is visible,
|
| 287 |
+
letterboxed within the card. The card already has a `--bg` backdrop
|
| 288 |
+
(`index.html:280`) to fill any letterbox gaps.
|
| 289 |
+
2. **Define a stable preview area.** Give the image (or a wrapping container) a
|
| 290 |
+
consistent box — e.g. a fixed `height`/`aspect-ratio` plus
|
| 291 |
+
`width: 100%`, with `object-fit: contain` — so portrait and landscape uploads
|
| 292 |
+
frame consistently instead of the height jumping per image.
|
| 293 |
+
3. **Consider raising the cap on larger screens.** `max-height: 150px` is small;
|
| 294 |
+
confirm the chosen size reads well in the right-hand panel
|
| 295 |
+
(`.source-card`, `index.html:264-273`) and on the ≤860px stacked layout
|
| 296 |
+
(`index.html:388-404`).
|
| 297 |
+
4. **Verify** with both a tall (portrait) and wide (landscape) photo that the
|
| 298 |
+
full image is visible and the card doesn't shift layout awkwardly.
|
| 299 |
+
|
| 300 |
+
---
|
| 301 |
+
|
| 302 |
+
## Suggested implementation order for Codex
|
| 303 |
+
|
| 304 |
+
1. **Fix the orbit bug first** (`.label-layer { pointer-events: none; }`) and run
|
| 305 |
+
the camera verification checklist in a real browser — this is the regression
|
| 306 |
+
the user actually hit.
|
| 307 |
+
2. Q1 camera polish (self-contained, `index.html` only): distance clamps, reset
|
| 308 |
+
button, hint, mobile/touch check.
|
| 309 |
+
3. Q4 source-photo preview sizing (`index.html` CSS only): `object-fit: contain`
|
| 310 |
+
+ stable preview box; quick standalone fix.
|
| 311 |
+
4. Schema + prompt + parsing changes for `annotation` and a `render_mode` signal.
|
| 312 |
+
5. `buildAnnotatedPhoto` render path + decision logic in `index.html`.
|
| 313 |
+
6. Q3 model thinking: allow reasoning, strip `<think>`, raise tokens/context/
|
| 314 |
+
timeouts, keep timeouts aligned across layers.
|
| 315 |
+
7. Resolve the local-demo honesty question (real model vs. labeled placeholder).
|
| 316 |
+
8. Local verification with FastAPI `TestClient` (per `AGENTS.md`), then the
|
| 317 |
+
standard PR → GitHub Actions HF sync → authenticated Space verification.
|
| 318 |
+
|
| 319 |
+
## Open items to confirm with the user
|
| 320 |
+
|
| 321 |
+
- Local-demo behavior (real Modal model vs. clearly-labeled placeholder).
|
| 322 |
+
- Whether the annotated-photo fallback should also offer a manual toggle to the
|
| 323 |
+
generic 3D cutaway, or replace it entirely (current decision: replace).
|
| 324 |
+
- Final token/context/timeout values for Q3 once Codex has measured latency vs.
|
| 325 |
+
JSON-validity on representative photos.
|
docs/reviews/motion-axis-bias.md
ADDED
|
@@ -0,0 +1,147 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Review: Motion Axis Bias Toward [0, 1, 0]
|
| 2 |
+
|
| 3 |
+
Date: 2026-06-15
|
| 4 |
+
Scope: Does the current prompt (and surrounding pipeline) bias part motion
|
| 5 |
+
toward the world Y axis `[0, 1, 0]`?
|
| 6 |
+
|
| 7 |
+
## Short answer
|
| 8 |
+
|
| 9 |
+
Yes. The system prompt nudges the model toward `[0, 1, 0]`, and two
|
| 10 |
+
non-prompt layers (the renderer default and the coercion fallback) push the
|
| 11 |
+
*rendered* result toward Y even harder. The net effect is that uncertain or
|
| 12 |
+
under-specified parts tend to spin/oscillate/slide about world Y regardless of
|
| 13 |
+
their actual geometry orientation.
|
| 14 |
+
|
| 15 |
+
## Where the bias comes from
|
| 16 |
+
|
| 17 |
+
### 1. Prompt anchoring (`snap2sim/prompts.py`, `VISION_SYSTEM_PROMPT`)
|
| 18 |
+
|
| 19 |
+
`[0, 1, 0]` is the *only* concrete axis literal the model ever sees, and it
|
| 20 |
+
appears twice with no counterexample:
|
| 21 |
+
|
| 22 |
+
- Line 29: `Motions, with axis as a numeric vector like [0, 1, 0]:`
|
| 23 |
+
- Line 40: `Every motion axis must be a numeric vector such as [0, 1, 0], never a string like x, y, or z.`
|
| 24 |
+
|
| 25 |
+
The compact example payload (lines 52–68) uses a `pulse` motion, which has no
|
| 26 |
+
`axis` field, so it adds no balancing example. With a single repeated exemplar
|
| 27 |
+
and no contrast cases, an LLM under uncertainty defaults to the most available
|
| 28 |
+
pattern — here, `[0, 1, 0]`. This is textbook few-shot anchoring.
|
| 29 |
+
|
| 30 |
+
Note: `EXAMPLE_ANALYSIS` in `snap2sim/schema.py` is *not* injected into the
|
| 31 |
+
prompt (the prompt is self-contained), so it does not directly steer the model.
|
| 32 |
+
But it reflects the same lean: of its three parts that carry an axis, two are
|
| 33 |
+
`[0, 1, 0]` (`ratchet_gear` rotate, `pawl` oscillate) and one is `[1, 0, 0]`
|
| 34 |
+
(`selector_pin` translate). It is also the local sample-mode render, so the
|
| 35 |
+
Y-lean is visible there too.
|
| 36 |
+
|
| 37 |
+
### 2. Renderer hard default (`index.html`)
|
| 38 |
+
|
| 39 |
+
`applyMotion`, line 1418:
|
| 40 |
+
|
| 41 |
+
```js
|
| 42 |
+
const axis = new THREE.Vector3(...(motion.axis || [0, 1, 0])).normalize();
|
| 43 |
+
```
|
| 44 |
+
|
| 45 |
+
If the model omits `axis` entirely (valid — `axis` is optional in the schema
|
| 46 |
+
and coercion never injects one), the renderer animates about Y. So "model is
|
| 47 |
+
biased to emit `[0, 1, 0]`" and "model omits axis" produce the *same* on-screen
|
| 48 |
+
result: Y-axis motion. This makes the bias look even stronger than the model's
|
| 49 |
+
raw output.
|
| 50 |
+
|
| 51 |
+
### 3. Coercion fallback (`snap2sim/model_io.py`)
|
| 52 |
+
|
| 53 |
+
- `_axis_vector` (lines 297–309) returns `None` when no usable axis is present;
|
| 54 |
+
`_coerce_part` only adds `axis` if truthy (lines 173–175). So omitted axes
|
| 55 |
+
stay omitted → renderer default (Y) applies.
|
| 56 |
+
- `_generic_analysis` (line 232) hard-codes `"axis": [0, 1, 0]` for its rotor
|
| 57 |
+
part. Any fully-degraded response that falls back to the generic payload is
|
| 58 |
+
pure Y.
|
| 59 |
+
|
| 60 |
+
## Why this is a real visual problem, not just cosmetics
|
| 61 |
+
|
| 62 |
+
Geometry orientation and motion axis are decided **independently**, and they can
|
| 63 |
+
disagree:
|
| 64 |
+
|
| 65 |
+
- Geometry: `axisProfile` + `orientYAxisGeometry` / `orientZAxisGeometry`
|
| 66 |
+
(`index.html` lines 1319–1367) bake the primitive's long axis into the
|
| 67 |
+
geometry buffer using the **odd-one-out `size` dimension**. A cylinder with
|
| 68 |
+
`size [3, 0.5, 3]`... e.g. `size [2.0, 0.4, 0.4]` lays the shaft along world X.
|
| 69 |
+
- Motion: `applyMotion` calls `mesh.rotateOnAxis(axis, …)` with the raw motion
|
| 70 |
+
axis (object space, and with no `baseRotation` that equals world space).
|
| 71 |
+
|
| 72 |
+
So a shaft whose geometry was oriented along **X** but whose motion axis is the
|
| 73 |
+
biased **`[0, 1, 0]`** will spin about Y — i.e. it sweeps end-over-end instead
|
| 74 |
+
of rotating about its own length. The size-driven geometry inference added in
|
| 75 |
+
`docs/reviews/scene-geometry-review.md` made geometry orientation smart, but
|
| 76 |
+
motion axis was left to the model/default, so the two can visibly contradict.
|
| 77 |
+
|
| 78 |
+
## Conclusion
|
| 79 |
+
|
| 80 |
+
The prompt is a genuine contributor to the `[0, 1, 0]` lean, but fixing only the
|
| 81 |
+
prompt will not fully resolve the on-screen behavior because the renderer
|
| 82 |
+
default and generic fallback also resolve to Y. Address the prompt *and* make
|
| 83 |
+
motion axis follow the geometry's inferred primitive axis when the model is
|
| 84 |
+
silent or low-signal.
|
| 85 |
+
|
| 86 |
+
## Next steps (for Codex to implement)
|
| 87 |
+
|
| 88 |
+
1. **De-anchor the prompt** (`snap2sim/prompts.py`):
|
| 89 |
+
- In the motions section, show axis examples across all three world axes,
|
| 90 |
+
e.g. spin about a shaft's long axis `[1, 0, 0]` / `[0, 0, 1]` as well as
|
| 91 |
+
`[0, 1, 0]`, so no single axis is the lone exemplar.
|
| 92 |
+
- Add one sentence: the motion axis should match the part's real axis of
|
| 93 |
+
rotation/translation (usually the geometry's long/odd-one-out dimension),
|
| 94 |
+
not default to vertical.
|
| 95 |
+
- Optionally add a second `parts` exemplar in the compact shape that carries
|
| 96 |
+
an explicit non-`[0,1,0]` axis, to balance the `pulse` example.
|
| 97 |
+
|
| 98 |
+
2. **Make the renderer default geometry-aware** (`index.html`):
|
| 99 |
+
- When `motion.axis` is absent, derive the default from the same
|
| 100 |
+
`axisProfile` result used to orient the geometry (map the inferred
|
| 101 |
+
`profile.axis` 0/1/2 to `[1,0,0]`/`[0,1,0]`/`[0,0,1]`) instead of the hard
|
| 102 |
+
`[0, 1, 0]`. This aligns spin with the part's long axis for free.
|
| 103 |
+
- Keep `[0, 1, 0]` only as the final fallback when no geometry/profile is
|
| 104 |
+
available.
|
| 105 |
+
|
| 106 |
+
3. **Align the coercion fallback** (`snap2sim/model_io.py`):
|
| 107 |
+
- Consider deriving `_generic_analysis` rotor axis from its own geometry, or
|
| 108 |
+
at minimum document that the generic part is intentionally Y.
|
| 109 |
+
- Decide whether `_coerce_part` should inject a geometry-derived axis when the
|
| 110 |
+
model omits one (vs. leaving it to the renderer). Prefer leaving it to the
|
| 111 |
+
renderer so there is a single source of truth (option 2).
|
| 112 |
+
|
| 113 |
+
4. **Verification after changes:**
|
| 114 |
+
- `EXAMPLE_ANALYSIS` and `docs/fixtures/mini-fan-analysis.json` render with
|
| 115 |
+
each part rotating about a sensible axis (shafts about their length).
|
| 116 |
+
- Schema/parser/coercion checks still pass; FastAPI `TestClient` smoke for
|
| 117 |
+
`/`, `/analyze_image`, `/generate_scene`.
|
| 118 |
+
- Re-run a representative Modal analysis and inspect the distribution of
|
| 119 |
+
emitted motion axes — confirm it is no longer almost entirely `[0, 1, 0]`.
|
| 120 |
+
|
| 121 |
+
## Implementation status
|
| 122 |
+
|
| 123 |
+
Completed locally on 2026-06-15:
|
| 124 |
+
|
| 125 |
+
- Prompt examples now show X, Y, and Z axes and explicitly tell the model to
|
| 126 |
+
match motion axes to the real rotation or translation axis.
|
| 127 |
+
- The deterministic renderer stores the geometry-inferred axis on each mesh
|
| 128 |
+
and uses it when `motion.axis` is omitted, keeping `[0, 1, 0]` only as the
|
| 129 |
+
final no-profile fallback.
|
| 130 |
+
- `_generic_analysis` no longer hard-codes the rotor motion axis; its disk-like
|
| 131 |
+
geometry lets the renderer derive the Y axis from size.
|
| 132 |
+
- Local schema/parser/coercion checks, FastAPI smoke checks, and Playwright
|
| 133 |
+
renderer probes passed.
|
| 134 |
+
- Modal dev `run_analysis_endpoint_check` returned a rotating `dial` part with
|
| 135 |
+
`axis: [1, 0, 0]`, confirming the prompt no longer collapses the synthetic
|
| 136 |
+
check to `[0, 1, 0]`.
|
| 137 |
+
- Stable Modal deployment completed after the prompt/renderer update, and an
|
| 138 |
+
unauthenticated deployed `runtime_probe` request returned `401 Unauthorized`.
|
| 139 |
+
|
| 140 |
+
## Files referenced
|
| 141 |
+
|
| 142 |
+
- `snap2sim/prompts.py` — `VISION_SYSTEM_PROMPT` (lines 29, 40), example payload.
|
| 143 |
+
- `index.html` — `applyMotion` (line 1418), `axisProfile` (1319–1344),
|
| 144 |
+
`orientYAxisGeometry` / `orientZAxisGeometry` (1354–1367), `buildPartMesh`.
|
| 145 |
+
- `snap2sim/model_io.py` — `_axis_vector` (297–309), `_coerce_part` (145–204),
|
| 146 |
+
`_generic_analysis` (~232).
|
| 147 |
+
- `snap2sim/schema.py` — `EXAMPLE_ANALYSIS` axes, motion schema.
|
docs/reviews/scene-geometry-review.md
ADDED
|
@@ -0,0 +1,212 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Scene Geometry Review: Rotation, Sizing, and Camera Y-Alignment
|
| 2 |
+
|
| 3 |
+
Reviewer pass on the deterministic Three.js renderer in `index.html`, validated
|
| 4 |
+
against `docs/fixtures/mini-fan-analysis.json` (a "mini fan": rotor cylinder,
|
| 5 |
+
cage torus, base box), `snap2sim/schema.py`, and `snap2sim/prompts.py`.
|
| 6 |
+
|
| 7 |
+
Scope: the three reported symptoms — **rotations are off**, **sizing is off**, and
|
| 8 |
+
**the object is not aligned with the camera in Y** so the user cannot see all the
|
| 9 |
+
detail. All three reproduce from the current code and have concrete root causes
|
| 10 |
+
below. This document is for Codex to implement; it does not change any code.
|
| 11 |
+
|
| 12 |
+
---
|
| 13 |
+
|
| 14 |
+
## TL;DR
|
| 15 |
+
|
| 16 |
+
All three symptoms originate in two places:
|
| 17 |
+
|
| 18 |
+
1. `buildPartMesh()` (`index.html:1214`) builds every axial primitive with a
|
| 19 |
+
**hardcoded long axis** (mostly Y) and derives radius from `max(x, z)`,
|
| 20 |
+
ignoring how the model actually distributes the `[x, y, z]` extents. This
|
| 21 |
+
breaks both proportions (sizing) and apparent spin axis (rotation).
|
| 22 |
+
2. `buildDeterministicScene()` (`index.html:890`) uses a **fixed camera** at
|
| 23 |
+
`(5.4, 3.9, 6.2)` looking at `(0, 0.1, 0)` with no bounding-box fit and no
|
| 24 |
+
recentering. Small models render as a dot; the assembly's vertical center is
|
| 25 |
+
wherever the model put it, not where the camera looks.
|
| 26 |
+
|
| 27 |
+
The agreed direction (see **Decisions**): infer orientation from the `size`
|
| 28 |
+
vector ("odd-one-out" axis), size the radial cross-section from the remaining
|
| 29 |
+
pair, and apply any explicit `geometry.rotation` additively on top.
|
| 30 |
+
|
| 31 |
+
---
|
| 32 |
+
|
| 33 |
+
## Root Cause 1 — Rotations are off
|
| 34 |
+
|
| 35 |
+
`buildPartMesh()` hardcodes the primitive's symmetry axis instead of deriving it
|
| 36 |
+
from the `size` extents:
|
| 37 |
+
|
| 38 |
+
- `index.html:1221` cylinder → `CylinderGeometry(r, r, y, ...)`: axis is **Y**,
|
| 39 |
+
height is always `y`.
|
| 40 |
+
- `index.html:1224` cone → axis **Y**, height `y`.
|
| 41 |
+
- `index.html:1228` capsule → axis **Y**, length `y`.
|
| 42 |
+
- `index.html:1231` rod → built along Y then `rotateX(PI/2)` → axis **Z**
|
| 43 |
+
(inconsistent with cylinder).
|
| 44 |
+
- `index.html:1236` torus → `rotateX(PI/2)` → ring lies flat in the XZ plane
|
| 45 |
+
(symmetry axis **Y**), always horizontal.
|
| 46 |
+
- `index.html:1258` gear → extruded along Z, then `rotateX(PI/2)` → axis **Y**.
|
| 47 |
+
- `index.html:1278` spring → built along **Y**.
|
| 48 |
+
|
| 49 |
+
Because the geometry axis is fixed but the model places the real long/thin axis
|
| 50 |
+
on whatever dimension is natural, the motion then *looks* wrong even though
|
| 51 |
+
`applyMotion()` (`index.html:1303`) is correct.
|
| 52 |
+
|
| 53 |
+
**Concrete failure (fan rotor, `docs/fixtures/mini-fan-analysis.json`):**
|
| 54 |
+
`geometry.size = [0.12, 0.12, 0.35]`, `motion.axis = [0, 0, 1]`. The intent is a
|
| 55 |
+
thin shaft whose long axis is **Z**, spinning about Z. The code builds a disk of
|
| 56 |
+
radius `max(0.12, 0.35)/2 = 0.175` and height `0.12` oriented along **Y**, then
|
| 57 |
+
spins it about world Z — so a fat short puck tumbles face-over instead of a hub
|
| 58 |
+
spinning in place.
|
| 59 |
+
|
| 60 |
+
**Concrete failure (fan cage):** `size = [0.55, 0.55, 0.04]` — the thin
|
| 61 |
+
dimension is Z, so the guard ring should stand **upright facing the camera**
|
| 62 |
+
(ring in the XY plane). The forced `rotateX(PI/2)` lays it **flat/horizontal**
|
| 63 |
+
in the XZ plane instead.
|
| 64 |
+
|
| 65 |
+
Note `applyMotion` uses `mesh.rotateOnAxis` in mesh-local space and the explicit
|
| 66 |
+
model rotation is applied as `mesh.rotation.fromArray` (`index.html:962`). So if
|
| 67 |
+
auto-orientation is baked into the *geometry* (via `geometry.rotate*`) rather than
|
| 68 |
+
the mesh, motion axes stay in world space (the fan's `[0,0,1]` keeps spinning
|
| 69 |
+
about world Z) and explicit `rotation` remains additive on the mesh. This is the
|
| 70 |
+
clean separation to preserve.
|
| 71 |
+
|
| 72 |
+
## Root Cause 2 — Sizing is off
|
| 73 |
+
|
| 74 |
+
Two independent problems:
|
| 75 |
+
|
| 76 |
+
**2a. Radius is taken from `max(x, z)` instead of the cross-section pair.**
|
| 77 |
+
`index.html:1222` (`radius = Math.max(x, z) / 2`) and the analogous lines for
|
| 78 |
+
cone/sphere/gear/torus. `size` is documented as full extents
|
| 79 |
+
(`schema.py` / `prompts.py:18`), so a circular primitive has two ~equal extents
|
| 80 |
+
(the diameter) and one different (the length). Using `max` picks the length when
|
| 81 |
+
the long axis is X or Z, ballooning the radius and collapsing the length. Fan
|
| 82 |
+
rotor: radius becomes `0.175` (should be `0.06`), height becomes `0.12` (should
|
| 83 |
+
be `0.35`).
|
| 84 |
+
|
| 85 |
+
**2b. No scene normalization or camera fit.** The camera, FOV, and OrbitControls
|
| 86 |
+
distances are constants (`index.html:924`, `index.html:934-935`), but model
|
| 87 |
+
scales vary by an order of magnitude:
|
| 88 |
+
- `docs/fixtures/mini-fan-analysis.json` fan: extents `0.04`–`0.55`
|
| 89 |
+
(sub-unit).
|
| 90 |
+
- `schema.py` `EXAMPLE_ANALYSIS`: extents `0.18`–`3.2` (unit-scale).
|
| 91 |
+
|
| 92 |
+
At distance ~9 with a 42° FOV the view frames roughly 6–8 units. The fan
|
| 93 |
+
(max extent ~0.55) renders as a speck dead center; a larger model can overflow
|
| 94 |
+
or clip on the near plane. There is no `Box3`/bounding-sphere computation
|
| 95 |
+
anywhere in `buildDeterministicScene()`.
|
| 96 |
+
|
| 97 |
+
## Root Cause 3 — Object not aligned with the camera in Y
|
| 98 |
+
|
| 99 |
+
The camera target is the constant `(0, 0.1, 0)` (`index.html:926`) and the grid
|
| 100 |
+
sits at `y = 0` (`index.html:953`). Part positions come straight from the model
|
| 101 |
+
(`index.html:961`) with arbitrary Y offsets and no recentering. The assembly's
|
| 102 |
+
vertical center is therefore wherever the model happened to place it, not where
|
| 103 |
+
the camera looks — so the object floats high or low and detail is cut off or
|
| 104 |
+
off-center. The fix for this is the same bounding-box work as 2b: recenter the
|
| 105 |
+
assembly (especially in Y) onto the camera target, then fit the distance.
|
| 106 |
+
|
| 107 |
+
---
|
| 108 |
+
|
| 109 |
+
## Recommended fixes / next steps (for Codex)
|
| 110 |
+
|
| 111 |
+
Order matters; 1–3 are the substantive changes.
|
| 112 |
+
|
| 113 |
+
### 1. Derive primitive orientation and radius from the `size` vector
|
| 114 |
+
Rewrite `buildPartMesh()` (`index.html:1214`) for the axial/disk shapes:
|
| 115 |
+
|
| 116 |
+
- Compute the **odd-one-out axis**: the size dimension that differs most from the
|
| 117 |
+
other two. For cylinder/cone/capsule/rod/spring this is the elongated axis;
|
| 118 |
+
for torus/gear it is the thin (normal) axis. Same selector, both cases:
|
| 119 |
+
- `[0.12, 0.12, 0.35]` → pair `(x, y)=0.12`, odd `z` → **axis Z**, diameter 0.12.
|
| 120 |
+
- `[3.2, 0.55, 3.2]` → pair `(x, z)=3.2`, odd `y` → **axis Y**, diameter 3.2.
|
| 121 |
+
- `[0.55, 0.55, 0.04]` (torus) → odd `z` → ring stands in XY, faces camera.
|
| 122 |
+
- Cross-section **radius = mean of the two non-axis extents / 2** (not `max`).
|
| 123 |
+
- Build each primitive in its native Three.js orientation (axis = Y) with
|
| 124 |
+
`height/length = extent along the detected axis`, then rotate the
|
| 125 |
+
**BufferGeometry** so its Y maps onto the detected axis (skip when axis is Y).
|
| 126 |
+
Keep using `geometry.rotate*` so motion axes stay in world space.
|
| 127 |
+
- Fallback when all three extents are distinct (no clear pair): for cylinder-like
|
| 128 |
+
shapes use the **max** extent as the axis; for torus/gear use the **min**
|
| 129 |
+
extent. Otherwise default to Y (current behavior). Document the fallback.
|
| 130 |
+
- Remove the now-redundant fixed `rotateX(PI/2)` on rod (`index.html:1233`),
|
| 131 |
+
torus (`index.html:1240`), and gear (`index.html:1274`) — orientation comes
|
| 132 |
+
from the detected axis instead.
|
| 133 |
+
- Leave the existing `mesh.rotation.fromArray(geometry.rotation)`
|
| 134 |
+
(`index.html:962`) untouched: explicit model rotation now layers additively on
|
| 135 |
+
top of the auto-orientation. Confirm `sphere` (no axis) and `box` (exact
|
| 136 |
+
extents) are unaffected.
|
| 137 |
+
|
| 138 |
+
### 2. Recenter the assembly and fit the camera to its bounding box
|
| 139 |
+
In `buildDeterministicScene()` (`index.html:890`), after the meshes are created
|
| 140 |
+
and **before** the reveal animation shrinks them:
|
| 141 |
+
|
| 142 |
+
- Put the part meshes in a `THREE.Group` (or compute over the mesh list) and
|
| 143 |
+
build a `THREE.Box3`. Measure at **full scale** — currently `mesh.scale` is set
|
| 144 |
+
to `0.001` for the reveal at `index.html:972`, so either measure before that
|
| 145 |
+
line or derive the box from geometry bounding boxes. Do not measure the grid.
|
| 146 |
+
- Recenter so the assembly center sits at the origin (translate group by
|
| 147 |
+
`-center`, or store the offset). This is what aligns the object in **Y** with
|
| 148 |
+
the camera target — the reported issue 3.
|
| 149 |
+
- Compute the bounding **sphere** radius and set the camera distance from FOV and
|
| 150 |
+
aspect: `distance = (radius / sin(fov/2)) * margin`, taking the max of the
|
| 151 |
+
vertical and horizontal (aspect-corrected) fit so wide models do not clip.
|
| 152 |
+
Keep the existing pleasing 3/4 viewing direction — normalize
|
| 153 |
+
`(5.4, 3.9, 6.2)` and scale it by the computed distance.
|
| 154 |
+
- Set `controls.target` to the recentered center (origin) and derive
|
| 155 |
+
`minDistance`/`maxDistance` from `radius` (e.g. `radius*0.6` .. `radius*6`)
|
| 156 |
+
instead of the hardcoded `2.4`/`12` (`index.html:934-935`). Recompute
|
| 157 |
+
`defaultCameraPosition`/`defaultTarget` so **Reset view** (`index.html:982`)
|
| 158 |
+
still lands on the fitted framing.
|
| 159 |
+
- Move the grid (`index.html:953`) to the bottom of the recentered box
|
| 160 |
+
(`y = box.min.y`) so it reads as a ground plane instead of bisecting the part.
|
| 161 |
+
- Re-run the fit inside `resize()` (`index.html:996`) so aspect changes (mobile,
|
| 162 |
+
window resize) keep the object framed.
|
| 163 |
+
|
| 164 |
+
### 3. Verify motion still reads correctly after re-orientation
|
| 165 |
+
With orientation in geometry and motion axes in world space, confirm against
|
| 166 |
+
`docs/fixtures/mini-fan-analysis.json`:
|
| 167 |
+
- Rotor spins about Z **in place** as a slim hub (not a tumbling puck).
|
| 168 |
+
- Cage stands upright facing the camera and stays static.
|
| 169 |
+
- Base sits beneath, assembly vertically centered in the viewport.
|
| 170 |
+
Spot-check `schema.py` `EXAMPLE_ANALYSIS` (gear about Y, oscillating pawl,
|
| 171 |
+
translating selector pin, pulsing spring, flat retaining ring) for regressions.
|
| 172 |
+
|
| 173 |
+
### 4. Optional / follow-up
|
| 174 |
+
- The prompt (`prompts.py:18`) and schema example mix conventions (the example
|
| 175 |
+
gives equal `x,z` **plus** an explicit `rotation`; the fan gives neither and
|
| 176 |
+
relies on the long axis being in `size`). After the renderer auto-orients,
|
| 177 |
+
tighten the prompt so the model is told the renderer infers axis from `size`
|
| 178 |
+
and that `rotation` is only a fine-tuning offset — reduces conflicting/double
|
| 179 |
+
rotations from the model.
|
| 180 |
+
- Consider clamping or warning when a part's extents are wildly out of range
|
| 181 |
+
relative to the rest of the assembly, so one bad number can't blow up the fit.
|
| 182 |
+
|
| 183 |
+
---
|
| 184 |
+
|
| 185 |
+
## Decisions (confirmed with the user)
|
| 186 |
+
|
| 187 |
+
- **Orientation:** auto-orient axial/disk primitives from the `size` vector using
|
| 188 |
+
the odd-one-out axis; radius from the remaining pair; apply explicit
|
| 189 |
+
`geometry.rotation` additively on top. (Not the "trust explicit rotation only"
|
| 190 |
+
approach — the fan output carries no rotation and must still render correctly.)
|
| 191 |
+
|
| 192 |
+
## Open considerations (flag if hit during implementation)
|
| 193 |
+
|
| 194 |
+
- True ambiguous case (all three extents distinct, e.g. an elliptical cross
|
| 195 |
+
section): the spec uses the max/min fallback above. Three.js cylinders are
|
| 196 |
+
circular, so a non-circular cross section can only approximate — note it rather
|
| 197 |
+
than over-engineer.
|
| 198 |
+
- Whether to normalize the whole scene to a fixed target size instead of fitting
|
| 199 |
+
the camera. Camera-fit (recommended above) preserves real relative scale
|
| 200 |
+
between parts; global normalization would flatten it. Implement camera-fit
|
| 201 |
+
unless a reason to do otherwise surfaces.
|
| 202 |
+
|
| 203 |
+
## Verification checklist
|
| 204 |
+
|
| 205 |
+
- `python -c "import snap2sim.schema, snap2sim.prompts"` and existing schema/parser
|
| 206 |
+
checks still pass (no Python contract change expected).
|
| 207 |
+
- FastAPI `TestClient` for `/`, `/analyze_image`, `/generate_scene` unchanged.
|
| 208 |
+
- Browser: load `docs/fixtures/mini-fan-analysis.json` through the deterministic path and confirm
|
| 209 |
+
fan rotor proportions/axis, upright cage, vertical centering, and a sensible
|
| 210 |
+
frame at both sub-unit (fan) and unit-scale (`EXAMPLE_ANALYSIS`) inputs.
|
| 211 |
+
- Resize / mobile: object stays centered and framed; OrbitControls still orbit,
|
| 212 |
+
pan, zoom; **Reset view** returns to the fitted framing.
|
docs/reviews/security-hardening.md
ADDED
|
@@ -0,0 +1,290 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Security Hardening Review
|
| 2 |
+
|
| 3 |
+
Review date: 2026-06-14. Reviewer: Claude (Opus 4.8), for OpenAI Codex to implement.
|
| 4 |
+
|
| 5 |
+
This review answers three questions:
|
| 6 |
+
|
| 7 |
+
1. Are the endpoints secure?
|
| 8 |
+
2. Is the UI/UX good?
|
| 9 |
+
3. Should we prioritize Three.js or A-Frame for animation rendering?
|
| 10 |
+
|
| 11 |
+
Two product decisions were confirmed with the owner and shape the priorities below:
|
| 12 |
+
|
| 13 |
+
- **The Space is going public soon.** Findings that are only mitigated by the
|
| 14 |
+
Space being private are therefore treated as blockers, not nice-to-haves.
|
| 15 |
+
- **Screen-only viewing; no VR/AR goal.** WebXR is not required, which removes
|
| 16 |
+
A-Frame's main structural advantage.
|
| 17 |
+
|
| 18 |
+
File references use `path:line` from the state of the repo at review time.
|
| 19 |
+
|
| 20 |
+
---
|
| 21 |
+
|
| 22 |
+
## 1. Endpoint Security
|
| 23 |
+
|
| 24 |
+
### Architecture summary
|
| 25 |
+
|
| 26 |
+
- `app.py` is the public Hugging Face Space (`gradio.Server`). It serves
|
| 27 |
+
`index.html` at `/` and exposes `POST /analyze_image` and
|
| 28 |
+
`POST /generate_scene`. The browser calls these **same-origin**, and the
|
| 29 |
+
server attaches the Modal bearer token when it forwards to Modal. This split
|
| 30 |
+
(token stays server-side) is correct and matches `SECURITY.md`.
|
| 31 |
+
- `modal_app.py` exposes the GPU/inference web endpoints, each guarded by a
|
| 32 |
+
bearer token via `require_authorization`.
|
| 33 |
+
|
| 34 |
+
The Modal layer is in good shape. The Space layer is the exposure.
|
| 35 |
+
|
| 36 |
+
### HIGH — The Space's own `/analyze_image` and `/generate_scene` are unauthenticated (`app.py:64`, `app.py:74`)
|
| 37 |
+
|
| 38 |
+
These endpoints have no auth of their own. Today they are gated only by the
|
| 39 |
+
Space being private. **Once the Space is public, anyone on the internet can POST
|
| 40 |
+
to them.**
|
| 41 |
+
|
| 42 |
+
- With `INFERENCE_BACKEND=local` (current default), they return placeholder data
|
| 43 |
+
— low impact but a free compute/bandwidth sink (base64 decode + PIL decode +
|
| 44 |
+
scene build per request).
|
| 45 |
+
- With `INFERENCE_BACKEND=modal`, every public request is proxied to Modal
|
| 46 |
+
**using the server's own bearer token**, turning the Space into an open proxy
|
| 47 |
+
that spends GPU credits on behalf of any anonymous caller. The Modal bearer
|
| 48 |
+
token protects Modal from direct callers, but the Space is a trusted caller,
|
| 49 |
+
so the token does not help here. This directly contradicts the stated intent
|
| 50 |
+
in `SECURITY.md` that the token is "the real protection against
|
| 51 |
+
credit-spending spam."
|
| 52 |
+
|
| 53 |
+
**Recommendation (before going public):**
|
| 54 |
+
|
| 55 |
+
- Add abuse controls at the Space layer that do not require leaking a secret to
|
| 56 |
+
the browser. Options, roughly in order of preference:
|
| 57 |
+
1. **Server-side rate limiting / quota** per client IP and a global ceiling
|
| 58 |
+
(e.g. `slowapi`/`limits`, or a small in-process token bucket). This is the
|
| 59 |
+
most important mitigation since the Space is intentionally token-free on
|
| 60 |
+
the client side.
|
| 61 |
+
2. A short-lived, server-issued session/CSRF-style token handed to the page on
|
| 62 |
+
`GET /` and required on the POST endpoints, to stop trivial scripted abuse
|
| 63 |
+
(not a strong control, but raises the bar).
|
| 64 |
+
3. Hard caps: max upload size, max requests/min, and a circuit breaker that
|
| 65 |
+
falls back to `local` placeholder mode if a Modal spend threshold is hit.
|
| 66 |
+
- Consider HF Space-level protections too (HF supports gating), but do not rely
|
| 67 |
+
on them as the only layer once public.
|
| 68 |
+
|
| 69 |
+
### HIGH — Untrusted scene HTML is injected via `innerHTML` (`index.html:545`)
|
| 70 |
+
|
| 71 |
+
`renderAframe()` does `viewport.innerHTML = sceneHtml;` where `sceneHtml` comes
|
| 72 |
+
from `/generate_scene`. In the deterministic path the HTML is built server-side
|
| 73 |
+
with `html.escape` (`snap2sim/aframe_scene.py:14-16`, safe). But the
|
| 74 |
+
model-generation path (`generate_scene_llamacpp` in `modal_app.py:462`) returns
|
| 75 |
+
**raw model output**, extracted by `parse_scene_response`
|
| 76 |
+
(`snap2sim/model_io.py:49`) which does *no sanitization* — it slices everything
|
| 77 |
+
between `<a-scene>` and `</a-scene>` verbatim.
|
| 78 |
+
|
| 79 |
+
Because the scene is derived from a **user-uploaded image**, an attacker can
|
| 80 |
+
prompt-inject the vision/scene model (text in the photo, adversarial content)
|
| 81 |
+
into emitting markup such as `<a-entity onloaded="...">` or
|
| 82 |
+
`<img src=x onerror=...>`. `innerHTML` will not execute injected `<script>`
|
| 83 |
+
tags, but it *does* run inline event-handler attributes and A-Frame can execute
|
| 84 |
+
component JS — so this is a realistic stored/reflected XSS vector that becomes
|
| 85 |
+
internet-reachable when the Space is public.
|
| 86 |
+
|
| 87 |
+
The validation in `validate_analysis` protects the *JSON analysis* path but does
|
| 88 |
+
nothing for the free-form HTML scene path.
|
| 89 |
+
|
| 90 |
+
**Recommendation:** Do not inject free-form model HTML into the DOM. See §3 —
|
| 91 |
+
the cleanest fix is to make scene rendering deterministic from the validated
|
| 92 |
+
JSON (which is already escaped/validated) and stop returning model-authored
|
| 93 |
+
HTML. If any model-authored HTML is ever rendered, sanitize it with an
|
| 94 |
+
allowlist (e.g. DOMPurify with an A-Frame-aware tag/attribute allowlist) before
|
| 95 |
+
`innerHTML`, and strip all `on*` attributes.
|
| 96 |
+
|
| 97 |
+
### MEDIUM — No upload size limit or decompression-bomb hardening (`app.py:88`, `modal_app.py:327`)
|
| 98 |
+
|
| 99 |
+
`_decode_image` and `write_payload_image` do `base64.b64decode(...)` then
|
| 100 |
+
`Image.open(BytesIO(...))` with no cap on the base64 string length or decoded
|
| 101 |
+
pixel count. A large or crafted image (decompression bomb) can exhaust memory on
|
| 102 |
+
`cpu-basic`. PIL's default `DecompressionBombWarning` only warns; it does not
|
| 103 |
+
block at typical sizes.
|
| 104 |
+
|
| 105 |
+
**Recommendation:**
|
| 106 |
+
|
| 107 |
+
- Reject requests whose base64 body exceeds a sane limit (e.g. ~8–12 MB) before
|
| 108 |
+
decoding.
|
| 109 |
+
- Set `Image.MAX_IMAGE_PIXELS` to an explicit ceiling and catch
|
| 110 |
+
`Image.DecompressionBombError`.
|
| 111 |
+
- Wrap decode failures and return a clean `400` instead of a 500/stack trace.
|
| 112 |
+
|
| 113 |
+
### MEDIUM — Fixed temp file paths cause cross-request collisions (`modal_app.py:199`, `modal_app.py:342`)
|
| 114 |
+
|
| 115 |
+
Both the smoke test and `write_payload_image` write to fixed paths
|
| 116 |
+
(`/tmp/snap2sim-request-image.jpg`). Concurrent requests on the same container
|
| 117 |
+
overwrite each other's input image, so one user could be analyzed against
|
| 118 |
+
another user's photo. This is both a correctness and a privacy concern.
|
| 119 |
+
|
| 120 |
+
**Recommendation:** Use `tempfile.NamedTemporaryFile`/`mkstemp` (unique per
|
| 121 |
+
request) and clean up in a `finally`.
|
| 122 |
+
|
| 123 |
+
### LOW — Third-party scripts loaded without Subresource Integrity (`index.html:7-13`)
|
| 124 |
+
|
| 125 |
+
A-Frame (`aframe.io`), the Gradio client (`jsdelivr`), and fonts
|
| 126 |
+
(`bunny.net`) are loaded from CDNs with no `integrity`/SRI hashes. A CDN
|
| 127 |
+
compromise would run arbitrary JS in the (soon public) page.
|
| 128 |
+
|
| 129 |
+
**Recommendation:** Pin versions and add SRI hashes where the CDN supports it.
|
| 130 |
+
Note: if A-Frame is dropped (see §3) and Three.js is bundled/self-hosted, this
|
| 131 |
+
surface shrinks substantially.
|
| 132 |
+
|
| 133 |
+
### LOW — Unused third-party dependency loaded (`index.html:10-13`)
|
| 134 |
+
|
| 135 |
+
The Gradio JS client is imported into `window.snap2simGradioClient` but never
|
| 136 |
+
used; all calls go through a plain `fetch` (`postJson`, `index.html:765`).
|
| 137 |
+
Remove the import to cut an unnecessary CDN dependency and load.
|
| 138 |
+
|
| 139 |
+
### Positives (keep these)
|
| 140 |
+
|
| 141 |
+
- Modal bearer check uses constant-time comparison
|
| 142 |
+
(`token_secrets.compare_digest`, `modal_app.py:113`) and returns `503` when
|
| 143 |
+
the token is unconfigured, `401` on mismatch (`modal_app.py:105-115`).
|
| 144 |
+
Unauthenticated `401` was verified per AGENTS.md.
|
| 145 |
+
- Token never reaches the browser; server-side attaches it
|
| 146 |
+
(`snap2sim/backend.py:62-68`). Matches `SECURITY.md` guidance.
|
| 147 |
+
- The deterministic scene builders escape text with `html.escape`
|
| 148 |
+
(`snap2sim/aframe_scene.py:14-16`) and the analysis readout uses `textContent`
|
| 149 |
+
rather than `innerHTML` (`index.html:513-536`) — no XSS on that path.
|
| 150 |
+
- GitHub→HF workflow uses least-privilege `permissions: contents: read` and
|
| 151 |
+
keeps `HF_TOKEN` in secrets (`.github/workflows/sync_to_hf.yml:19-21`).
|
| 152 |
+
- Schema validation (`snap2sim/schema.py:176`) runs before scene generation.
|
| 153 |
+
|
| 154 |
+
---
|
| 155 |
+
|
| 156 |
+
## 2. UI/UX
|
| 157 |
+
|
| 158 |
+
The visual direction is strong and cohesive: a "technical cutaway / field
|
| 159 |
+
manual" aesthetic (amber `#E8A33D` + cyan `#5FD4D0` on dark, blueprint grid,
|
| 160 |
+
Chakra Petch / Fira Code). The two-pane layout (viewport + readout) is clear and
|
| 161 |
+
the loading choreography (progress bar, scan-line reveal, staggered mesh
|
| 162 |
+
reveal, cold-start "WAKING THE WORKSHOP" message after 6.5 s) is a genuinely
|
| 163 |
+
nice touch for HF cold starts. Drag-and-drop plus a Load button, and a
|
| 164 |
+
play/pause control, cover the core interactions. The fallback chain
|
| 165 |
+
(A-Frame → deterministic Three.js → "3D runtime unavailable") is thoughtful.
|
| 166 |
+
|
| 167 |
+
### Issues, roughly by impact
|
| 168 |
+
|
| 169 |
+
**Accessibility (MEDIUM — matters more once public):**
|
| 170 |
+
|
| 171 |
+
- Status messages (`index.html:392`) are not announced to screen readers. Add
|
| 172 |
+
`aria-live="polite"` (and `role="status"`) so analysis progress/errors are
|
| 173 |
+
read out.
|
| 174 |
+
- `#viewport` injected 3D content has no text alternative. Provide an
|
| 175 |
+
`aria-label`/visually-hidden description, or rely on the readout panel as the
|
| 176 |
+
accessible representation and mark the viewport `aria-hidden`.
|
| 177 |
+
- Low-contrast text: `--text-muted #6B7280` on the dark panel is below WCAG AA
|
| 178 |
+
for body text (`trigger`, raw JSON, drop-zone hint). Nudge it lighter.
|
| 179 |
+
- The file input is triggered via a `<label>` drop zone (`index.html:382`); make
|
| 180 |
+
sure keyboard focus + Enter works to open the picker, and the Load button is
|
| 181 |
+
already focusable (good).
|
| 182 |
+
|
| 183 |
+
**Functional gaps:**
|
| 184 |
+
|
| 185 |
+
- **No preview of the uploaded photo.** Users lose their reference image once
|
| 186 |
+
the cutaway renders. Show a thumbnail of the source photo in the panel — it
|
| 187 |
+
also reinforces the "this came from *my* object" story for a demo.
|
| 188 |
+
- **`playButton` initial state is confusing** (`index.html:379`): it reads
|
| 189 |
+
"Pause" while disabled, before any scene exists. Start as disabled "Play" or
|
| 190 |
+
hide until a scene is ready.
|
| 191 |
+
- **Error UX is thin:** errors render as uppercased status text only
|
| 192 |
+
(`index.html:495`). Add a visible retry affordance and keep the drop zone
|
| 193 |
+
reachable so the user can try another photo without hunting for the Load
|
| 194 |
+
button.
|
| 195 |
+
- **No client-side file validation:** `accept="image/*"` only. Reject
|
| 196 |
+
non-images and oversized files before base64 inflation (~33% size increase)
|
| 197 |
+
and surface a friendly message. (Pairs with the server-side size cap in §1.)
|
| 198 |
+
- **Mobile:** the layout has a breakpoint at 860 px (good), but A-Frame
|
| 199 |
+
`look-controls` can capture touch/scroll. If Three.js becomes the primary
|
| 200 |
+
renderer (see §3), use OrbitControls tuned for touch and ensure the page still
|
| 201 |
+
scrolls. Also consider `capture="environment"` to offer "take a photo" on
|
| 202 |
+
mobile.
|
| 203 |
+
- **No confidence visualization:** confidence is shown as a number
|
| 204 |
+
(`index.html:516`); a small bar/gauge would read faster and fit the
|
| 205 |
+
instrument aesthetic.
|
| 206 |
+
- **First-run onboarding** is just "Drop component photo." A one-line example
|
| 207 |
+
("try a lock, a gear, a ratchet…") lowers the cold-start barrier for judges.
|
| 208 |
+
|
| 209 |
+
**Verdict:** UI/UX is above-average for a hackathon and the aesthetic is a real
|
| 210 |
+
asset. The gaps are accessibility, an uploaded-image preview, and error/retry
|
| 211 |
+
polish — none are large, and they raise the demo from "looks great" to "feels
|
| 212 |
+
finished."
|
| 213 |
+
|
| 214 |
+
---
|
| 215 |
+
|
| 216 |
+
## 3. Three.js vs A-Frame for Rendering
|
| 217 |
+
|
| 218 |
+
**Recommendation: consolidate on Three.js as the primary (and only) renderer,
|
| 219 |
+
driven deterministically from the validated JSON analysis. Drop the A-Frame
|
| 220 |
+
dependency and stop returning model-authored scene HTML.**
|
| 221 |
+
|
| 222 |
+
### Reasoning
|
| 223 |
+
|
| 224 |
+
1. **No VR/AR goal (confirmed).** A-Frame's defining advantage is declarative
|
| 225 |
+
WebXR. Screen-only viewing removes the main reason to carry it. Without VR,
|
| 226 |
+
A-Frame is mostly a ~1.2 MB declarative wrapper over Three.js.
|
| 227 |
+
|
| 228 |
+
2. **The Three.js path is already the more capable implementation.** The
|
| 229 |
+
"fallback" in `index.html:565-755` has OrbitControls, DOM-projected part
|
| 230 |
+
labels, staggered reveal, gear extrusion, fog/lighting, and motion handling.
|
| 231 |
+
It is richer than the A-Frame output (`snap2sim/aframe_scene.py`). You'd be
|
| 232 |
+
promoting your best renderer, not rewriting one.
|
| 233 |
+
|
| 234 |
+
3. **True "cutaway" cross-sections need clipping planes.** The product is an
|
| 235 |
+
*annotated technical cutaway*. Three.js exposes
|
| 236 |
+
`renderer.localClippingEnabled` + `material.clippingPlanes` natively, which is
|
| 237 |
+
the clean way to slice a housing and reveal internals. A-Frame has no
|
| 238 |
+
first-class clipping-plane support. If "cutaway" is to be more than parts
|
| 239 |
+
floating in space, Three.js is required anyway.
|
| 240 |
+
|
| 241 |
+
4. **It removes the §1 HIGH XSS vector.** Rendering deterministically from
|
| 242 |
+
validated JSON means no `innerHTML` of untrusted model HTML. The model
|
| 243 |
+
already produces the structured JSON (`snap2sim/schema.py`), which is
|
| 244 |
+
validated and safe; let a deterministic Three.js builder turn that JSON into
|
| 245 |
+
geometry. This is both safer and more *reliable* than asking an LLM to emit
|
| 246 |
+
well-formed scene markup.
|
| 247 |
+
|
| 248 |
+
5. **Smaller, self-hostable footprint.** Three.js can be pinned/bundled and
|
| 249 |
+
served same-origin, shrinking the CDN/SRI surface from §1.
|
| 250 |
+
|
| 251 |
+
### What this means concretely for implementation
|
| 252 |
+
|
| 253 |
+
- **Keep the model's job as the JSON `analysis` contract** (it already is,
|
| 254 |
+
`snap2sim/schema.py`). The model should *not* author HTML/JS scenes.
|
| 255 |
+
- **Make `generate_scene` deterministic** from that JSON. The browser already
|
| 256 |
+
has `buildDeterministicScene()` — promote it from fallback to the main path.
|
| 257 |
+
You can drop the server returning scene HTML entirely, or keep the server
|
| 258 |
+
returning the validated analysis and let the browser build the scene.
|
| 259 |
+
- **Remove A-Frame** (`index.html:9`, the `<a-scene>` injection in
|
| 260 |
+
`renderAframe`, and the A-Frame branch of `generate_scene_llamacpp`) once the
|
| 261 |
+
Three.js path is the default. Delete `snap2sim/aframe_scene.py` and the
|
| 262 |
+
A-Frame prompt template after migration, or keep them only if you want a
|
| 263 |
+
server-rendered static preview.
|
| 264 |
+
- **If you keep any model-authored HTML path**, it must be sanitized
|
| 265 |
+
(DOMPurify, strip `on*`) per §1.
|
| 266 |
+
|
| 267 |
+
This also simplifies the mental model: one renderer, one safe data contract,
|
| 268 |
+
no dual A-Frame/Three.js maintenance.
|
| 269 |
+
|
| 270 |
+
---
|
| 271 |
+
|
| 272 |
+
## Suggested Implementation Order (for Codex)
|
| 273 |
+
|
| 274 |
+
Blockers before the Space goes public:
|
| 275 |
+
|
| 276 |
+
1. Add server-side rate limiting / quota + max upload size to `app.py`
|
| 277 |
+
endpoints (§1 HIGH).
|
| 278 |
+
2. Eliminate the untrusted-HTML `innerHTML` path: move to deterministic Three.js
|
| 279 |
+
rendering from validated JSON, or sanitize (§1 HIGH + §3).
|
| 280 |
+
3. Add decompression-bomb guards and unique temp files (§1 MEDIUM).
|
| 281 |
+
|
| 282 |
+
Then:
|
| 283 |
+
|
| 284 |
+
4. Consolidate rendering on Three.js; remove A-Frame and the dead Gradio client
|
| 285 |
+
import; add SRI/self-hosting (§3, §1 LOW).
|
| 286 |
+
5. UI/UX polish: uploaded-image preview, `aria-live` status, contrast fix,
|
| 287 |
+
error/retry affordance, play-button initial state (§2).
|
| 288 |
+
|
| 289 |
+
Out of scope / leave as-is: Modal bearer auth (already correct), the
|
| 290 |
+
GitHub→HF sync workflow, the schema contract.
|
index.html
CHANGED
|
@@ -1268,37 +1268,45 @@
|
|
| 1268 |
const y = Math.max(0.05, Number(size[1]) || 1);
|
| 1269 |
const z = Math.max(0.05, Number(size[2]) || 1);
|
| 1270 |
let geometry;
|
|
|
|
| 1271 |
if (geometryData.shape === "cylinder") {
|
| 1272 |
const profile = axisProfile([x, y, z], "max");
|
|
|
|
| 1273 |
geometry = new THREE.CylinderGeometry(profile.radius, profile.radius, profile.length, 48);
|
| 1274 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1275 |
} else if (geometryData.shape === "cone") {
|
| 1276 |
const profile = axisProfile([x, y, z], "max");
|
|
|
|
| 1277 |
geometry = new THREE.ConeGeometry(profile.radius, profile.length, 48);
|
| 1278 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1279 |
} else if (geometryData.shape === "sphere") {
|
| 1280 |
geometry = new THREE.SphereGeometry(Math.max(x, y, z) / 2, 36, 18);
|
| 1281 |
} else if (geometryData.shape === "capsule") {
|
| 1282 |
const profile = axisProfile([x, y, z], "max");
|
|
|
|
| 1283 |
const radius = Math.min(profile.radius, profile.length / 2);
|
| 1284 |
geometry = new THREE.CapsuleGeometry(radius, Math.max(0.05, profile.length - radius * 2), 8, 24);
|
| 1285 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1286 |
} else if (geometryData.shape === "rod") {
|
| 1287 |
const profile = axisProfile([x, y, z], "max");
|
|
|
|
| 1288 |
geometry = new THREE.CylinderGeometry(profile.radius, profile.radius, profile.length, 24);
|
| 1289 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1290 |
} else if (geometryData.shape === "gear") {
|
| 1291 |
const profile = axisProfile([x, y, z], "min");
|
|
|
|
| 1292 |
geometry = gearGeometry(profile.radius, profile.length, geometryData.teeth || 18);
|
| 1293 |
orientZAxisGeometry(geometry, profile.axis);
|
| 1294 |
} else if (geometryData.shape === "torus") {
|
| 1295 |
const profile = axisProfile([x, y, z], "min");
|
|
|
|
| 1296 |
const outerRadius = profile.radius;
|
| 1297 |
const tube = Math.min(profile.length / 2, outerRadius * 0.36);
|
| 1298 |
geometry = new THREE.TorusGeometry(Math.max(0.05, outerRadius - tube), Math.max(0.025, tube), 16, 64);
|
| 1299 |
orientZAxisGeometry(geometry, profile.axis);
|
| 1300 |
} else if (geometryData.shape === "spring") {
|
| 1301 |
const profile = axisProfile([x, y, z], "max");
|
|
|
|
| 1302 |
geometry = springGeometry(profile.radius, profile.length, geometryData.coils || 5, geometryData.wire);
|
| 1303 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1304 |
} else {
|
|
@@ -1313,7 +1321,9 @@
|
|
| 1313 |
emissive: color,
|
| 1314 |
emissiveIntensity: 0.04
|
| 1315 |
});
|
| 1316 |
-
|
|
|
|
|
|
|
| 1317 |
}
|
| 1318 |
|
| 1319 |
function axisProfile(extents, fallback) {
|
|
@@ -1343,6 +1353,12 @@
|
|
| 1343 |
};
|
| 1344 |
}
|
| 1345 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1346 |
function dimensionsAreClose(left, right) {
|
| 1347 |
return relativeDifference(left, right) <= 0.2;
|
| 1348 |
}
|
|
@@ -1415,7 +1431,8 @@
|
|
| 1415 |
const motion = mesh.userData.part.motion || { type: "static" };
|
| 1416 |
const speed = Number(motion.speed || 1);
|
| 1417 |
const phase = Number(motion.phase || 0);
|
| 1418 |
-
const
|
|
|
|
| 1419 |
mesh.position.copy(mesh.userData.basePosition);
|
| 1420 |
mesh.rotation.copy(mesh.userData.baseRotation);
|
| 1421 |
if (motion.type === "rotate") {
|
|
|
|
| 1268 |
const y = Math.max(0.05, Number(size[1]) || 1);
|
| 1269 |
const z = Math.max(0.05, Number(size[2]) || 1);
|
| 1270 |
let geometry;
|
| 1271 |
+
let inferredMotionAxis = [0, 1, 0];
|
| 1272 |
if (geometryData.shape === "cylinder") {
|
| 1273 |
const profile = axisProfile([x, y, z], "max");
|
| 1274 |
+
inferredMotionAxis = axisVector(profile.axis);
|
| 1275 |
geometry = new THREE.CylinderGeometry(profile.radius, profile.radius, profile.length, 48);
|
| 1276 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1277 |
} else if (geometryData.shape === "cone") {
|
| 1278 |
const profile = axisProfile([x, y, z], "max");
|
| 1279 |
+
inferredMotionAxis = axisVector(profile.axis);
|
| 1280 |
geometry = new THREE.ConeGeometry(profile.radius, profile.length, 48);
|
| 1281 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1282 |
} else if (geometryData.shape === "sphere") {
|
| 1283 |
geometry = new THREE.SphereGeometry(Math.max(x, y, z) / 2, 36, 18);
|
| 1284 |
} else if (geometryData.shape === "capsule") {
|
| 1285 |
const profile = axisProfile([x, y, z], "max");
|
| 1286 |
+
inferredMotionAxis = axisVector(profile.axis);
|
| 1287 |
const radius = Math.min(profile.radius, profile.length / 2);
|
| 1288 |
geometry = new THREE.CapsuleGeometry(radius, Math.max(0.05, profile.length - radius * 2), 8, 24);
|
| 1289 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1290 |
} else if (geometryData.shape === "rod") {
|
| 1291 |
const profile = axisProfile([x, y, z], "max");
|
| 1292 |
+
inferredMotionAxis = axisVector(profile.axis);
|
| 1293 |
geometry = new THREE.CylinderGeometry(profile.radius, profile.radius, profile.length, 24);
|
| 1294 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1295 |
} else if (geometryData.shape === "gear") {
|
| 1296 |
const profile = axisProfile([x, y, z], "min");
|
| 1297 |
+
inferredMotionAxis = axisVector(profile.axis);
|
| 1298 |
geometry = gearGeometry(profile.radius, profile.length, geometryData.teeth || 18);
|
| 1299 |
orientZAxisGeometry(geometry, profile.axis);
|
| 1300 |
} else if (geometryData.shape === "torus") {
|
| 1301 |
const profile = axisProfile([x, y, z], "min");
|
| 1302 |
+
inferredMotionAxis = axisVector(profile.axis);
|
| 1303 |
const outerRadius = profile.radius;
|
| 1304 |
const tube = Math.min(profile.length / 2, outerRadius * 0.36);
|
| 1305 |
geometry = new THREE.TorusGeometry(Math.max(0.05, outerRadius - tube), Math.max(0.025, tube), 16, 64);
|
| 1306 |
orientZAxisGeometry(geometry, profile.axis);
|
| 1307 |
} else if (geometryData.shape === "spring") {
|
| 1308 |
const profile = axisProfile([x, y, z], "max");
|
| 1309 |
+
inferredMotionAxis = axisVector(profile.axis);
|
| 1310 |
geometry = springGeometry(profile.radius, profile.length, geometryData.coils || 5, geometryData.wire);
|
| 1311 |
orientYAxisGeometry(geometry, profile.axis);
|
| 1312 |
} else {
|
|
|
|
| 1321 |
emissive: color,
|
| 1322 |
emissiveIntensity: 0.04
|
| 1323 |
});
|
| 1324 |
+
const mesh = new THREE.Mesh(geometry, material);
|
| 1325 |
+
mesh.userData.inferredMotionAxis = inferredMotionAxis;
|
| 1326 |
+
return mesh;
|
| 1327 |
}
|
| 1328 |
|
| 1329 |
function axisProfile(extents, fallback) {
|
|
|
|
| 1353 |
};
|
| 1354 |
}
|
| 1355 |
|
| 1356 |
+
function axisVector(axis) {
|
| 1357 |
+
if (axis === 0) return [1, 0, 0];
|
| 1358 |
+
if (axis === 2) return [0, 0, 1];
|
| 1359 |
+
return [0, 1, 0];
|
| 1360 |
+
}
|
| 1361 |
+
|
| 1362 |
function dimensionsAreClose(left, right) {
|
| 1363 |
return relativeDifference(left, right) <= 0.2;
|
| 1364 |
}
|
|
|
|
| 1431 |
const motion = mesh.userData.part.motion || { type: "static" };
|
| 1432 |
const speed = Number(motion.speed || 1);
|
| 1433 |
const phase = Number(motion.phase || 0);
|
| 1434 |
+
const axisValues = motion.axis || mesh.userData.inferredMotionAxis || [0, 1, 0];
|
| 1435 |
+
const axis = new THREE.Vector3(...axisValues).normalize();
|
| 1436 |
mesh.position.copy(mesh.userData.basePosition);
|
| 1437 |
mesh.rotation.copy(mesh.userData.baseRotation);
|
| 1438 |
if (motion.type === "rotate") {
|
snap2sim/model_io.py
CHANGED
|
@@ -229,7 +229,7 @@ def _generic_analysis(component: str) -> dict[str, Any]:
|
|
| 229 |
"name": "central rotor",
|
| 230 |
"role": "transfers motion through the assembly",
|
| 231 |
"geometry": {"shape": "cylinder", "size": [0.9, 0.35, 0.9], "position": [0, 0.18, 0]},
|
| 232 |
-
"motion": {"type": "rotate", "
|
| 233 |
},
|
| 234 |
{
|
| 235 |
"id": "guide",
|
|
|
|
| 229 |
"name": "central rotor",
|
| 230 |
"role": "transfers motion through the assembly",
|
| 231 |
"geometry": {"shape": "cylinder", "size": [0.9, 0.35, 0.9], "position": [0, 0.18, 0]},
|
| 232 |
+
"motion": {"type": "rotate", "speed": 0.55},
|
| 233 |
},
|
| 234 |
{
|
| 235 |
"id": "guide",
|
snap2sim/prompts.py
CHANGED
|
@@ -26,7 +26,8 @@ Shapes, with size always [x, y, z] extents:
|
|
| 26 |
- torus: o-rings, snap rings, seals, washers, single coils
|
| 27 |
- spring: helical springs and coils; set coils when useful
|
| 28 |
|
| 29 |
-
Motions, with axis as a numeric vector
|
|
|
|
| 30 |
- static: fixed structure or housing
|
| 31 |
- rotate: continuous spin; use speed
|
| 32 |
- oscillate: sinusoidal twist; use amplitude and speed
|
|
@@ -37,7 +38,10 @@ Motions, with axis as a numeric vector like [0, 1, 0]:
|
|
| 37 |
|
| 38 |
Every geometry must use size: [x, y, z] and position: [x, y, z]. Do not use
|
| 39 |
radius, height, length, width, or depth fields. Every motion axis must be a
|
| 40 |
-
numeric vector such as [
|
|
|
|
|
|
|
|
|
|
| 41 |
For axial or disk-like shapes, express the main axis or disk normal through the
|
| 42 |
size vector: one dimension should differ from the other two. The renderer uses
|
| 43 |
that odd-one-out dimension for the primitive axis and treats rotation as a
|
|
@@ -63,6 +67,13 @@ Use this compact shape:
|
|
| 63 |
"geometry": {"shape": "spring", "size": [0.4, 1.2, 0.4], "position": [0, 0, 0], "coils": 6},
|
| 64 |
"motion": {"type": "pulse", "speed": 2, "amplitude": 0.08},
|
| 65 |
"annotation": {"point": [0.5, 0.5], "label": "visible label", "note": "short visible clue"}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 66 |
}
|
| 67 |
]
|
| 68 |
}
|
|
|
|
| 26 |
- torus: o-rings, snap rings, seals, washers, single coils
|
| 27 |
- spring: helical springs and coils; set coils when useful
|
| 28 |
|
| 29 |
+
Motions, with axis as a numeric vector when direction matters. Use [1, 0, 0],
|
| 30 |
+
[0, 1, 0], or [0, 0, 1] for world X/Y/Z directions:
|
| 31 |
- static: fixed structure or housing
|
| 32 |
- rotate: continuous spin; use speed
|
| 33 |
- oscillate: sinusoidal twist; use amplitude and speed
|
|
|
|
| 38 |
|
| 39 |
Every geometry must use size: [x, y, z] and position: [x, y, z]. Do not use
|
| 40 |
radius, height, length, width, or depth fields. Every motion axis must be a
|
| 41 |
+
numeric vector such as [1, 0, 0], [0, 1, 0], or [0, 0, 1], never a string like
|
| 42 |
+
x, y, or z. Match the motion axis to the real axis of rotation or translation;
|
| 43 |
+
for shafts, pins, disks, and springs this is usually the geometry's
|
| 44 |
+
odd-one-out size dimension, not automatically vertical.
|
| 45 |
For axial or disk-like shapes, express the main axis or disk normal through the
|
| 46 |
size vector: one dimension should differ from the other two. The renderer uses
|
| 47 |
that odd-one-out dimension for the primitive axis and treats rotation as a
|
|
|
|
| 67 |
"geometry": {"shape": "spring", "size": [0.4, 1.2, 0.4], "position": [0, 0, 0], "coils": 6},
|
| 68 |
"motion": {"type": "pulse", "speed": 2, "amplitude": 0.08},
|
| 69 |
"annotation": {"point": [0.5, 0.5], "label": "visible label", "note": "short visible clue"}
|
| 70 |
+
},
|
| 71 |
+
{
|
| 72 |
+
"id": "shaft",
|
| 73 |
+
"name": "drive shaft",
|
| 74 |
+
"role": "rotates along its length",
|
| 75 |
+
"geometry": {"shape": "cylinder", "size": [1.2, 0.25, 0.25], "position": [0, 0.45, 0]},
|
| 76 |
+
"motion": {"type": "rotate", "axis": [1, 0, 0], "speed": 1.4}
|
| 77 |
}
|
| 78 |
]
|
| 79 |
}
|