.forgejo/workflows/nigig-build.yml

@@ -108,32 +108,6 @@ jobs:
           fi
           echo "OK"
 
-      # CadNode::pos/rot/size return Vec3f BY VALUE. `node.pos().x = v`
-      # compiles, mutates a temporary and throws it away. This silently
-      # broke all nine properties-panel inputs and the Extend tool; it
-      # produces no warning and no runtime error, only a control that
-      # does nothing. Mutation must go through set_pos/set_rot/set_size.
-      - name: No writes through the by-value Vec3f getters
-        run: |
-          set -euo pipefail
-          cad=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad
-          # Exclude the two test modules that demonstrate the trap.
-          if grep -rnE '\.(pos|rot|size)\(\)\.[xyz][[:space:]]*[-+*/]?=[^=]' "$cad" \
-               | grep -v 'setters_write_through_but_getters_are_copies' \
-               | sed 's/^[^:]*:[0-9]*://' \
-               | grep -vE '^[[:space:]]*(//|/\*|\*)' \
-               | grep -v 'n\.pos()\.x = 42\.0' \
-               | grep -v 'n\.rot()\.y = 42\.0' \
-               | grep -v 'p\.size()\.y = v'; then
-            echo
-            echo "ERROR: the assignment(s) above write to a temporary copy"
-            echo "and are discarded. CadNode::pos/rot/size return Vec3f by"
-            echo "value. Read into a local, mutate it, then call set_pos /"
-            echo "set_rot / set_size."
-            exit 1
-          fi
-          echo "OK"
-
       - name: Reject whitespace errors
         run: git diff --check
 
@@ -180,23 +154,9 @@ jobs:
       - name: Check
         run: cargo check --locked -p nigig-build --lib
 
-      - name: Test (lib)
+      - name: Test
         run: cargo test --locked -p nigig-build --lib
 
-      # Phase 4.7 moved the CAD integration suite out of src/ into its own
-      # test target. `--lib` does not build it, so without this step the
-      # 154 tests it contains would run in no pipeline at all.
-      #
-      # This names the target explicitly rather than running the whole
-      # crate's tests, because `--test cost_estimator` and
-      # `--test cost_estimator_ui` do not currently compile. That is
-      # pre-existing breakage, not this workflow's to hide -- but gating
-      # on it would make this job red for reasons unrelated to the CAD
-      # module, and a step that is always red gets ignored. Add those
-      # targets here the moment they build.
-      - name: Test (CAD integration suite)
-        run: cargo test --locked -p nigig-build --test cad_integration
-
       # NOTE: `cargo clippy -- -D warnings` is not enabled yet -- the crate
       # currently emits ~290 warnings. Enable it once that backlog is
       # cleared; a step that cannot fail is worse than no step.

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/ARCHITECTURE.md

@@ -59,11 +59,12 @@ of its cache-coherence complexity.
 
 | File | LOC | Responsibility |
 |---|--:|---|
-| `mod.rs` | 2,087 | Module wiring, re-exports, and the `#[derive]`-heavy struct definitions (`CadViewport`, `CadWorkspace`, `CadTool`, `DrawingState`, `SnapSettings`, `DrawCadMesh`). Structs live here because the derive macros need the `script_mod!` context; their `impl` blocks live elsewhere. |
+| `mod.rs` | 2,058 | Module wiring, re-exports, and the `#[derive]`-heavy struct definitions (`CadViewport`, `CadWorkspace`, `CadTool`, `DrawingState`, `SnapSettings`, `DrawCadMesh`). Structs live here because the derive macros need the `script_mod!` context; their `impl` blocks live elsewhere. |
-| `cad_scene.rs` | 2,279 | The domain model. `CadScene`, `CadNode`, `CadSolid`, `CadTransform`, newtype ids, `SceneBuilder`, `SceneVisitor` + `walk_scene`, `MeshCache`, and the `Exporter` trait. |
+| `cad_scene.rs` | 2,173 | The domain model. `CadScene`, `CadNode`, `CadSolid`, `CadTransform`, newtype ids, `SceneBuilder`, `SceneVisitor` + `walk_scene`, `MeshCache`, and the `Exporter` trait. |
-| `viewport.rs` | 7,521 | `impl CadViewport` — input handling, 2D and 3D rendering, picking, snapping, drawing tools, and the rebuild worker. |
+| `viewport.rs` | 7,177 | `impl CadViewport` — input handling, 2D and 3D rendering, picking, snapping, drawing tools, and the rebuild worker. |
-| `workspace.rs` | 3,734 | `impl CadWorkspace` — panel layout, the AI agent session, export entry points, and viewport synchronisation. |
+| `workspace.rs` | 3,360 | `impl CadWorkspace` — panel layout, the AI agent session, export entry points, and viewport synchronisation. |
-| `commands.rs` | 1,305 | Undo/redo: `Command`, `CommandContext`, `UndoRedoStack`. Node edits go through `CommandContext::update_node`, which preserves list position — never `delete_node` + `create_node`. |
+| `commands.rs` | 1,318 | Undo/redo: `Command`, `CommandContext`, `UndoRedoStack`. Node edits go through `CommandContext::update_node`, which preserves list position — never `delete_node` + `create_node`. |
+| `tests.rs` | 2,840 | Integration tests. Uses no `Cx`; belongs in `tests/`. |
 | `arch_pdf.rs` | 1,301 | PDF floor-plan exporter (`printpdf`). 2D plan projection; does not use meshes. |
 | `arch_gltf.rs` | 915 | GLB 2.0 exporter + the generated HTML viewer. |
 | `script_bindings.rs` | 712 | Script VM bindings: `cad_script_mod`, solid/shape handles, `eval_cad_script`. |
@@ -73,7 +74,7 @@ of its cache-coherence complexity.
 | `construction_geometry.rs` | 453 | Construction lines/points, coordinate-entry parsing. |
 | `cad_editor_sheet.rs` | 408 | The draggable bottom sheet. |
 | `arch_stl.rs` | 393 | Binary STL exporter. |
-| `scene_holder.rs` | 754 | `PartsStore` (the parts list + its generation counter) and `SceneCache` — the cached `Arc<CadScene>` + shared `Arc<MeshCache>`. |
+| `scene_holder.rs` | 390 | `SceneCache` — the cached `Arc<CadScene>` + shared `Arc<MeshCache>`. |
 | `profile_benchmarks.rs` | 281 | Timing harness. Currently `#[test]`, with wall-clock assertions. |
 | `tools.rs` | 247 | `impl CadTool` — tool cycling and kind mapping. |
 | `viewport_2d.rs` | 184 | 2D projection helpers split out of `viewport.rs`. |
@@ -215,29 +216,10 @@ enforces that `Cargo.lock` is committed and current — see
 The crate compiles and its tests run:
 
 ```bash
-cargo test --locked -p nigig-build --lib              # 608 passed; 0 failed; 10 ignored
+cargo test --locked -p nigig-build --lib
-cargo test --locked -p nigig-build --test cad_integration   # 154 passed; 0 failed
+# 741 passed; 0 failed; 10 ignored
 ```
 
-Both are gated in CI and assert a plain pass, so any failing test fails the
+The suite is green and CI asserts a plain pass, so any failing test fails
-build. The 10 ignored are the timing benchmarks in `profile_benchmarks.rs`.
+the build. The 7 ignored are the timing benchmarks in
-See `TEST_BASELINE.md`.
+`profile_benchmarks.rs`. See `TEST_BASELINE.md`.
-
-## Where a test goes
-
-Phase 4.7 moved the cross-cutting suite to
-`crates/apps/nigig-build/tests/cad_integration.rs`, which compiles as a
-separate crate and therefore sees only the public API.
-
-- **Reaches only `pub` items** → `tests/cad_integration.rs`. It exercises
-  the module the way a consumer does, so anything it can reach is, by
-  definition, API.
-- **Reaches a `pub(crate)` or private item** → a `#[cfg(test)] mod` in the
-  file that owns the item.
-
-The rule is that visibility is never widened to make a test compile. When
-the suite moved out, six items would have needed `pub`
-(`cad_mesh_data_from_solid`, `part_mesh_buffers`, `CommandBorrows`,
-`CadRenderMode`, `DrawingState`, `SnapSettings`). Widening them would have
-turned editor internals into a public contract to suit a file layout, so
-the 16 tests involved moved back to `viewport.rs` and `mod.rs` instead.

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/arch_gltf.rs

@@ -315,9 +315,9 @@ impl GltfExporter {
     /// assemble GLB. Exposed publicly so tests can call it without
     /// going through a writer.
     ///
-    /// Uses parallel mesh generation for scenes of >=32 nodes and
+    /// v17: Uses parallel mesh generation for large scenes (>=32 nodes).
-    /// sequential below that: rayon's thread-pool overhead exceeds the
+    /// For smaller scenes, falls back to sequential (rayon thread pool
-    /// benefit for small scenes of simple primitives.
+    /// overhead exceeds the benefit for simple primitives).
     pub fn build_glb(
         &self,
         scene: &CadScene,
@@ -337,9 +337,10 @@ impl GltfExporter {
             });
         }
 
-        // Only parallelise large scenes. Measured: for 100 simple box
+        // v17 fix: Only use parallel for large scenes.
-        // walls parallel was 2x SLOWER than sequential, because rayon's
+        // Benchmark results showed that for 100 simple box walls,
-        // task dispatch and synchronisation cost more than the meshing.
+        // parallel was 2x SLOWER than sequential due to rayon's
+        // thread pool overhead (task dispatch + synchronization).
         //
         // Parallel wins when each mesh build is expensive (complex
         // CSG with differences/unions, high-segment cylinders, etc.).
@@ -478,8 +479,9 @@ impl Exporter for GltfExporter {
         self.export_with_cache(scene, &cache, writer)
     }
 
-    /// Overrides the `Exporter` default so the cache is actually
+    /// v2 fix: `export_with_cache` is now a provided method on the
-    /// consulted; the default provided method ignores it.
+    /// `Exporter` trait itself (no more `ExporterWithCacheOverride`
+    /// indirection). Override it here to actually consult the cache.
     fn export_with_cache(
         &self,
         scene: &CadScene,

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/arch_pdf.rs

@@ -20,7 +20,7 @@
 //!   └─ decorations: title block, scale bar, north arrow
 //! ```
 //!
-//! ## Classification
+//! ## v2 design (preserved from previous version)
 //!
 //! Each `CadNode` is classified by **direct pattern
 //! match on node name + geometry kind**, not by guessing from dimensions. The user
@@ -236,8 +236,8 @@ pub enum ArchElement {
 // Instead of `match part.kind { ... }`, we override the visitor
 // methods for each geometry kind. The "is this a Wall or a Slab?"
 // decision is made from the node's layer name (set by the editor)
-// rather than from geometry, so the classification is explicit rather
+// rather than from geometry — this preserves the v2 "explicit type"
-// than inferred.
+// design while letting us use the unified CadScene.
 //
 // Layer-name → arch-type mapping:
 //   "walls"     → Wall
@@ -248,8 +248,8 @@ pub enum ArchElement {
 //   "beams"     → Beam
 //   anything else → fallback (Block / Column / Sphere by geometry)
 //
-// If a node has no layer (or an unknown layer), fall back to
+// If a node has no layer (or an unknown layer), we fall back to
-// geometry-based classification.
+// geometry-based classification, which is what the v1 code did.
 
 struct PdfArchProjector<'a> {
     scene: &'a CadScene,
@@ -655,8 +655,10 @@ impl Exporter for PdfExporter {
         self.export_with_cache(scene, &cache, writer)
     }
 
-    /// The PDF exporter accepts the mesh cache for API symmetry with
+    /// v2 fix: `export_with_cache` is now a provided method on the
-    /// `GltfExporter` but does not use it
+    /// `Exporter` trait itself (no more `ExporterWithCacheOverride`
+    /// indirection). The PDF exporter accepts the cache for API
+    /// symmetry with `GltfExporter` but doesn't actually use it
     /// (PDF works in 2D plan view, no triangle meshes needed).
     fn export_with_cache(
         &self,
@@ -776,7 +778,7 @@ fn compute_viewport(elements: &[ArchElement], o: &PdfExportOptions) -> Option<Vi
 }
 
 // ===========================================================================
-// PDF primitive helpers (printpdf 0.7 API)
+// PDF primitive helpers (printpdf 0.7 API) — unchanged from v2
 // ===========================================================================
 
 fn color_rgb(r: f32, g: f32, b: f32) -> PdfColor {
@@ -915,7 +917,7 @@ fn draw_text(
 }
 
 // ===========================================================================
-// Grid + element renderers
+// Grid + element renderers — unchanged drawing logic from v2
 // ===========================================================================
 
 fn draw_grid(layer: &PdfLayerReference, vp: &Viewport, o: &PdfExportOptions) {
@@ -1079,7 +1081,7 @@ fn draw_element(layer: &PdfLayerReference, e: &ArchElement, vp: &Viewport, font:
 }
 
 // ===========================================================================
-// Decorations: title block, scale bar, north arrow
+// Decorations: title block, scale bar, north arrow — unchanged from v2
 // ===========================================================================
 
 fn draw_title_block(

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/cad_scene.rs

@@ -1,24 +1,47 @@
-//! # cad_scene — the immutable CAD scene graph and the export foundation.
+//! # cad_scene — immutable CAD scene graph + shared export foundation.
 //!
-//! A `CadScene` is a flat arena of `CadNode`s addressed by `NodeId`,
+//! ## v2 — compile fixes after first integration attempt
-//! plus side tables for materials, layers and sheets. It is built once
-//! from the editor's parts (see `scene_holder::SceneCache`) and then
-//! only read, which is what lets exporters share one `MeshCache`
-//! keyed on `(NodeId, ParamHash)`.
 //!
-//! Everything that walks a scene goes through `SceneVisitor` /
+//! This version fixes the 24 compile errors from the first integration:
-//! `walk_scene`, and everything that writes a file implements
-//! `Exporter`. `Exporter::export_with_cache` is a provided method, so
-//! an exporter that has no use for cached meshes implements only
-//! `export`.
 //!
-//! `SceneBuilder` is the only supported way to construct a scene; it
+//! 1. **`Exporter` trait consolidation** — `export_with_cache` is now a
-//! owns the `IdAllocator` so ids cannot collide.
+//!    *provided method* on `Exporter` itself, not on a separate
+//!    `ExporterWithCache` extension trait. This fixes the
+//!    `<T as Exporter>::export_with_cache` not-found errors. The
+//!    default impl ignores the cache; exporters that want to use the
+//!    cache override the method directly.
 //!
-//! Rotations in `CadTransform` are **degrees** (see Phase 1.1).
+//! 2. **`Solid::cube(...).mesh()` returns `&TriMesh`** — added
-//! `CadSolid::Arc` angles are radians — it is the sole exception and is
+//!    `.clone()` so `build_mesh()` returns an owned `TriMesh`.
-//! documented at its definition.
+//!
-
+//! 3. **`Solid::cylinder` / `Solid::sphere` take `u32` segments** —
+//!    removed the `as usize` casts. The `segments` / `segments_u` /
+//!    `segments_v` fields are already `u32`.
+//!
+//! 4. **`start_node` expects `Option<CadSolid>`** — wrapped the
+//!    geometry args in `Some(...)` in the `cube()` / `cylinder()` /
+//!    `sphere()` / `csg()` / `polygon_2d()` / `extruded_polygon()`
+//!    builder methods.
+//!
+//! 5. **`NodeBuilder::material_by_name` / `layer_by_name` borrow
+//!    conflict** — extracted the resolved id into a local before
+//!    mutably borrowing `pending`.
+//!
+//! 6. **Public `SceneBuilder` API for legacy adapter** — added
+//!    `register_material_for_color(color) -> MaterialId` and
+//!    `push_node(node)` public methods so the legacy `parts_to_scene`
+//!    adapters in `arch_gltf.rs` and `mod.rs` don't need to touch
+//!    private fields.
+//!
+//! 7. **`DofConstraint` not re-exported** — removed from the
+//!    `pub use` list in `mod.rs` so it doesn't conflict with the
+//!    legacy struct. Added `From<crate::cad::DofConstraint> for
+//!    cad_scene::DofConstraint` so the legacy struct can be
+//!    converted at the bridge point.
+//!
+//! 8. **`HashMap<[f32; 4], _>` doesn't work** (f32 has no `Eq`/`Hash`)
+//!    — the legacy adapter in `arch_gltf.rs` and `mod.rs` now uses
+//!    `[u32; 4]` (via `f32::to_bits()`) as the key.
 
 use std::collections::HashMap;
 use std::sync::{Arc, RwLock};
@@ -1470,10 +1493,10 @@ impl<'a> NodeBuilder<'a> {
 // Mesh cache (#3) — with parameter-hash invalidation
 // ===========================================================================
 //
-// The cache keys on (NodeId, ParamHash), not NodeId alone, so editing
+// v2 fix: the cache now keys on (NodeId, ParamHash) instead of just
-// a node's parameters (e.g. wall length) produces a fresh mesh even
+// NodeId. This means editing a node's parameters (e.g. wall length)
-// though the NodeId is unchanged. `invalidate(id)` is the coarse
+// correctly produces a fresh mesh even though the NodeId is unchanged.
-// hammer for cases the hash cannot see.
+// `invalidate(id)` still works as a coarse hammer.
 
 /// Stable hash of a node's geometry + transform + material parameters.
 /// Computed via `DefaultHasher` (cheap, deterministic within a process).
@@ -1649,12 +1672,13 @@ impl Default for MeshCache {
 }
 
 // ===========================================================================
-// Exporter trait
+// Exporter trait (#1) — consolidated in v2
 // ===========================================================================
 //
-// `export_with_cache` is a *provided method* on `Exporter` itself
+// v2 fix: `export_with_cache` is now a *provided method* on
-// rather than a separate extension trait, so every exporter can be
+// `Exporter` itself, not on a separate `ExporterWithCache` trait.
-// called through one path. The default impl ignores the cache; exporters that want
+// This fixes the `<T as Exporter>::export_with_cache` not-found
+// errors. The default impl ignores the cache; exporters that want
 // to use the cache override the method directly.
 
 pub trait Exporter {
@@ -1846,6 +1870,16 @@ pub enum PartKind {
     Beam,
 }
 
+// ===========================================================================
+// Cross-impl: legacy DofConstraint <-> cad_scene::DofConstraint
+// ===========================================================================
+//
+// The legacy `crate::cad::DofConstraint` struct (defined in `mod.rs`)
+// has the same fields as this one. The `From` impls below let the
+// two coexist during the migration period without duplicating logic.
+
+
+
 // ===========================================================================
 // Tests
 // ===========================================================================
@@ -1872,47 +1906,6 @@ mod size_tests {
         }
     }
 
-    /// `pos()` / `rot()` / `size()` return `Vec3f` **by value**. Writing
-    /// through them (`node.pos().x = v`) compiles, mutates a temporary
-    /// and silently discards the write — which is exactly what every
-    /// properties-panel input in workspace.rs used to do. Mutation must
-    /// go through the `set_*` methods.
-    #[test]
-    fn setters_write_through_but_getters_are_copies() {
-        let mut n = node_with(CadSolid::Box { size: Vec3f { x: 1.0, y: 1.0, z: 1.0 } });
-
-        // The trap: this compiles and does nothing.
-        #[allow(unused_must_use)]
-        {
-            n.pos().x = 42.0;
-            n.rot().y = 42.0;
-        }
-        assert!(
-            n.pos().x.abs() < EPS,
-            "writing through pos() must not reach the node (it returns a copy)"
-        );
-        assert!(
-            n.rot().y.abs() < EPS,
-            "writing through rot() must not reach the node (it returns a copy)"
-        );
-
-        // The correct path.
-        let mut p = n.pos();
-        p.x = 42.0;
-        n.set_pos(p);
-        assert!((n.pos().x - 42.0).abs() < EPS, "set_pos must write through");
-
-        let mut r = n.rot();
-        r.y = 90.0;
-        n.set_rot(r);
-        assert!((n.rot().y - 90.0).abs() < EPS, "set_rot must write through");
-
-        let mut sz = n.size();
-        sz.z = 7.0;
-        n.set_size(sz);
-        assert!((n.size().z - 7.0).abs() < EPS, "set_size must write through");
-    }
-
     fn assert_size(got: Vec3f, want: (f32, f32, f32), what: &str) {
         assert!(
             (got.x - want.0).abs() < EPS

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/commands.rs

@@ -1,21 +1,34 @@
-//! # commands — the undo/redo command stack.
+//! # commands — Command pattern for undo/redo (#7)
 //!
 //! ## Design
 //!
-//! Each command is a struct that owns its payload and knows how to
+//! The existing `CadCommand` enum in `mod.rs` (line ~3538) works for
-//! execute and undo itself; the stack is `Vec<Box<dyn Command>>`. This
+//! the current command set, but it has two limitations:
-//! is open for extension (a new command is a new struct, not a new
-//! match arm in two places) and self-contained (the data for "which
-//! node, moved from where to where" lives in the command, not in a
-//! side table the editor has to keep in sync).
 //!
-//! Commands reach the scene through `CommandContext`, which the editor
+//! 1. **Closed for extension.** Adding a new command type (e.g.
-//! implements over its parts store. `MockCommandContext` implements it
+//!    `SplitWall`, `MirrorSelection`) requires modifying the enum,
-//! over an insertion-ordered `Vec` for tests — it must stay ordered,
+//!    the `execute()` match, and the `undo()` match. Plugin authors
-//! see the note on that type.
+//!    can't add commands without forking.
 //!
-//! `MAX_UNDO_LEVELS` bounds the stack; it is a `VecDeque` so the oldest
+//! 2. **No payload carrying.** The enum variants are bare
-//! entry is dropped in O(1).
+//!    identifiers; the actual data (which node, where it moved from /
+//!    to) lives in side tables that the editor has to maintain.
+//!
+//! The `Command` trait below fixes both. Each command is a struct
+//! that owns its payload and knows how to execute + undo itself.
+//! The undo/redo stack becomes `Vec<Box<dyn Command>>`.
+//!
+//! ## Migration strategy
+//!
+//! This module is **additive** — it doesn't touch the existing
+//! `CadCommand` enum. During the migration period:
+//!
+//! - New code writes `Box<dyn Command>` impls.
+//! - Old code keeps using `CadCommand` enum variants.
+//! - `CadCommandWrapper` adapts an old enum variant into a `Command`
+//!   so it can sit on the new stack.
+//!
+//! Once every variant has a wrapper, the enum can be deleted.
 //!
 //! ## Example
 //!

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/mod.rs

@@ -1,13 +1,44 @@
 // File: src/construction_frame/pages/workspace/cad/mod.rs
 //
-// Widget definitions, script (DSL) declarations and shared types for
+// ## Rewrite status (v4 — cached CadScene + shared MeshCache)
-// the CAD editor. The behaviour lives in the sibling modules; this file
-// exists because `#[derive(Script)]` types must be declared alongside
-// the `script_mod!` block that references them.
 //
-// See `ARCHITECTURE.md` in this directory for the module map, the
+// v4 changes:
-// ownership model and the invariants. Do not describe design here — it
+//   - `CadViewport` now holds a `SceneCache` (cached `Arc<CadScene>`
-// drifts. Describe it there, next to the tests that enforce it.
+//     + shared `Arc<MeshCache>`) instead of a bare `MeshCache`.
+//   - `scene()` returns `Arc<CadScene>` (was `CadScene`). O(1) on
+//     no-op redraws (was O(parts) per call).
+//   - `mesh_cache()` returns `Arc<MeshCache>` (was `&MeshCache`).
+//     Lets the GLB exporter reuse the preview renderer's cached meshes.
+//   - Export entry points (`export_floor_plan_pdf`, `export_3d_viewer`)
+//     now use the cached scene + shared cache instead of cloning parts
+//     + building a fresh scene + fresh cache per export.
+//   - `mark_scene_dirty()` added; called from `add_part` and
+//     `apply_part_field`. Other mutation methods need to call it too
+//     (see scene_holder.rs docs).
+//   - `SceneCache` safety net: `scene()` rebuilds if `parts.len()`
+//     differs from cached scene's node count, catching missing
+//     `mark_dirty()` calls for add/delete operations.
+//
+// v2-v3 changes (still in place):
+//
+// v2 fixes (vs v1):
+//   - `DofConstraint` removed from the `cad_scene` re-export list
+//     (was conflicting with the legacy struct).
+//   - `export_floor_plan_pdf` and `export_3d_viewer` now call
+//     `cad_scene::Exporter::export_with_cache(...)` — `export_with_cache`
+//     is a provided method on `Exporter` itself in v2 (no more
+//     separate `ExporterWithCache` trait).
+//
+// v1 priorities (still in place):
+//   - Strong IDs (#7): `NodeId`, `LayerId`, `MaterialId`, `SheetId`.
+//   - Mesh cache (#3): `CadViewport::mesh_cache` is a new field,
+//     with parameter-hash-based invalidation in v2.
+//   - Exporter trait (#1) + SceneVisitor (#9): exporters go through
+//     `Exporter::export_with_cache`.
+//   - Builder API (#8): `cad_scene::SceneBuilder`.
+//
+// Everything below the header comment is the original mod.rs, with
+// targeted edits at the architectural seams only.
 
 pub use ::makepad_ai;
 pub use ::makepad_code_editor;
@@ -41,7 +72,7 @@ use std::time::Instant;
 use crate::cad_store;
 // arch_pdf: vector PDF export (added by apply_arch_pdf_patch.py)
 pub mod arch_pdf;
-// arch_gltf: GLB 2.0 export for interactive 3D viewing.
+// arch_gltf: GLB 2.0 export for interactive 3D viewing (added by v3 patch)
 pub mod arch_gltf;
 // arch_stl: Binary STL export for 3D printing and CAD interchange.
 pub mod arch_stl;
@@ -52,10 +83,10 @@ pub mod cad_editor_sheet;
 // cad_scene: immutable scene graph + Exporter trait + SceneVisitor + MeshCache.
 // New in this rewrite — see cad_scene.rs for the full rationale.
 pub mod cad_scene;
-// scene_holder: the parts store, the cached Arc<CadScene> and the
+// scene_holder: cached Arc<CadScene> + shared Arc<MeshCache> for CadViewport.
-// shared Arc<MeshCache>. The Arc sharing lets the GLB exporter reuse
+// New in v4 — lets the GLB exporter reuse the preview renderer's cached meshes.
-// the preview renderer's meshes instead of re-meshing.
 pub mod scene_holder;
+// v6: wired-in extracted modules:
 pub mod constants;
 pub mod persistence;
 pub mod math;
@@ -70,6 +101,8 @@ pub mod exporters;
 pub mod code_editor;
 pub mod workspace;
 #[cfg(test)]
+mod tests;
+#[cfg(test)]
 pub mod profile_benchmarks;
 #[cfg(test)]
 pub mod send_sync_audit;
@@ -90,7 +123,7 @@ pub use cad_scene::{
     SheetId, walk_scene,
 };
 pub use scene_holder::{PartsStore, SceneCache};
-// Re-exports so call sites need not name the sub-module.
+// v6: re-export wired-in module types so existing call sites keep working.
 pub(crate) use constants::{DEFAULT_CAD_SCRIPT, LIVE_UPDATE_INTERVAL, local_openai_url, local_openai_model, GENERATED_DIR, GENERATED_SCRIPT_FILE, GENERATED_OBJ_FILE, DEMO_MAX_CURVE_SEGMENTS, DEMO_MAX_SPHERE_RINGS, DEMO_MAX_TORUS_MINOR_SEGMENTS, PART_SELECT_COLOR, PART_PICK_RADIUS, MAX_UNDO_LEVELS, HOVER_PICK_MIN_MOVE_PX};
 pub(crate) use persistence::{PENDING_ATTACHED_IMAGE, CAD_SCRIPT_OUTPUT, cad_manifest_path, cad_generated_dir_path, cad_generated_script_path, cad_generated_obj_path, load_saved_cad_script, save_cad_state, set_cad_script_output, clear_cad_script_output, take_cad_script_output};
 pub(crate) use math::{DVec3, vec3_cross, vec3_dot, vec3_length_sq, vec3_length, vec3_normalize, triangulate_polygon, polygon_centroid, polygon_area, point_in_polygon, mat4_mul_vec4, translate_mat, mat4_mul, mat4_inverse, rot_x_mat, rot_y_mat, rot_z_mat, part_model_matrix, ortho_proj, ray_triangle_intersect, ray_aabb_intersect};
@@ -101,8 +134,8 @@ pub(crate) use commands::{
     MoveNode, DeleteNode, CreateNode, YawNode,
     ResizeNode, RotateNode, ModifyNode, CadCommandCtx,
 };
-// Types defined in viewport.rs but named as field types in the
+// v16: re-export types moved to viewport.rs that are still used
-// CadViewport/CadWorkspace struct definitions below.
+// as field types in CadViewport/CadWorkspace struct definitions.
 pub(crate) use viewport::{
     CadStats, CadMeshData, CadRenderMode, CadViewportViewSnapshot,
     CadRebuildWorker, CadRebuildRequest, CadRebuildPayload, CadRebuildResult,
@@ -1575,7 +1608,7 @@ script_mod! {
 
 // [moved to viewport.rs: struct CadViewportViewSnapshot]
 
-/// Holder for simultaneous mutable borrows of `parts`,
+/// v11: Holder for simultaneous mutable borrows of `parts`,
 /// `scene_cache`, and `command_stack`. Returned by
 /// `CadViewport::split_for_command()` to solve the borrow-checker
 /// conflict where `legacy_ctx()` + `command_stack.execute()` both
@@ -1643,9 +1676,10 @@ pub struct CadViewport {
     part_geoms: HashMap<u64, Geometry>,
     /// Cached `Arc<CadScene>` + shared `Arc<MeshCache>`.
     ///
-    /// The scene snapshot is cached, so `scene()` is O(1) on a no-op
+    /// v4: replaces the bare `MeshCache` field. The scene snapshot
-    /// redraw rather than O(parts). The mesh cache is `Arc`-shared so
+    /// is cached so `scene()` is O(1) on no-op redraws (was O(parts)
-    /// the GLB exporter reuses the preview renderer's meshes.
+    /// per call). The mesh cache is `Arc`-shared so the GLB exporter
+    /// can reuse the preview renderer's cached meshes.
     ///
     /// After *any* mutation to `self.parts`, call
     /// `self.scene_cache.mark_dirty()`. The `scene()` safety net
@@ -1693,8 +1727,8 @@ pub struct CadViewport {
     script_dirty: bool,
     #[rust(false)]
     view_dirty: bool,
-    /// Trait-based command stack (`Command` + `UndoRedoStack`) with
+    /// v19: trait-based command stack with automatic cache
-    /// automatic cache invalidation.
+    /// invalidation. Uses the new `Command` trait + `UndoRedoStack`.
     #[rust]
     command_stack: UndoRedoStack,
     /// Screen position of the last hover pick.
@@ -1754,7 +1788,7 @@ pub struct CadViewport {
     #[rust]
     last_middle_click_abs: DVec2,
 
-    // ---- CAD tool system ----
+    // ---- v3 CAD tool system ----
     #[rust]
     tool: CadTool,
     #[rust]
@@ -1766,7 +1800,7 @@ pub struct CadViewport {
     frame_timer_avg_ms: f64,
     #[rust(0u32)]
     frame_timer_count: u32,
-    // ---- multi-select, clipboard, drag-select ----
+    // ---- v4: multi-select, clipboard, drag-select ----
     #[rust(false)]
     shift_pressed: bool,
     #[rust]
@@ -2022,66 +2056,3 @@ pub fn register_cad(vm: &mut ScriptVm) {
     cad_editor_sheet::script_mod(vm);
     script_mod(vm);
 }
-
-// ===========================================================================
-// Tests for the plain-data editor state owned by this module.
-//
-// These reach private fields, so they belong here rather than in
-// tests/cad_integration.rs.
-// ===========================================================================
-
-#[cfg(test)]
-mod editor_state_tests {
-    use super::*;
-    use crate::construction_frame::pages::workspace::cad::section_shape::SectionShape;
-
-    #[test]
-    fn polar_settings_default() {
-        let s = SnapSettings::default();
-        assert!(!s.polar_enabled);
-        assert!((s.polar_angle_increment - 45.0).abs() < 0.1);
-    }
-
-    #[test]
-    fn polar_settings_custom_increment() {
-        let mut s = SnapSettings::default();
-        s.polar_enabled = true;
-        s.polar_angle_increment = 15.0;
-        assert!(s.polar_enabled);
-        assert!((s.polar_angle_increment - 15.0).abs() < 0.1);
-    }
-
-    // ── Beam section settings ──
-
-    #[test]
-    fn drawing_state_beam_section_default() {
-        let ds = DrawingState::default();
-        assert_eq!(ds.beam_section, SectionShape::Rect);
-    }
-
-    #[test]
-    fn drawing_state_beam_section_cycle() {
-        let mut ds = DrawingState::default();
-        assert_eq!(ds.beam_section, SectionShape::Rect);
-        ds.beam_section = ds.beam_section.next();
-        assert_eq!(ds.beam_section, SectionShape::IBeam);
-        ds.beam_section = ds.beam_section.next();
-        assert_eq!(ds.beam_section, SectionShape::HSS);
-        ds.beam_section = ds.beam_section.next();
-        assert_eq!(ds.beam_section, SectionShape::Rect);
-    }
-
-    // ── PdfPreview enum variant ──
-
-    #[test]
-    fn pdf_preview_active_pane_variant() {
-        let pane = CadEditorActivePane::PdfPreview;
-        assert_eq!(pane.title(), "PDF Preview");
-    }
-
-    #[test]
-    fn pdf_preview_active_pane_not_default() {
-        let default = CadEditorActivePane::default();
-        assert_ne!(default, CadEditorActivePane::PdfPreview);
-    }
-}

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/profile_benchmarks.rs

@@ -389,7 +389,7 @@ mod profile_benchmarks {
     }
 }
 
-    /// Compare parallel vs sequential GLB export.
+    /// v17: Compare parallel vs sequential GLB export.
     #[test]
     #[ignore = "benchmark: timing-dependent, run explicitly with --ignored"]
     fn bench_parallel_vs_sequential_export() {

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/tests.rs

@@ -1,14 +1,13 @@
-//! # cad_integration — integration tests for the CAD module.
+//! # tests — comprehensive integration tests for the CAD rewrite
 //!
-//! These cross file boundaries: scene conversion, exporter output, GLB
+//! This module collects tests that cross file boundaries (scene
-//! validity, PDF dimensions, mesh-cache behaviour under realistic
+//! conversion, exporter output, GLB validity, PDF dimensions, mesh
-//! workloads. Per-module unit tests stay in their own files next to the
+//! cache behavior under realistic workloads). Per-module unit tests
-//! code they test; this is the integration layer, and it exercises the
+//! stay in their own files; this is the integration layer.
-//! crate from outside, as a consumer would.
 //!
-//! Run with: `cargo test --package nigig-build --test cad_integration`
+//! Run with: `cargo test --package nigig-build cad::tests`
 
-use nigig_build::construction_frame::pages::workspace::cad::cad_scene::{
+use crate::construction_frame::pages::workspace::cad::cad_scene::{
     self, CadNode, CadScene, CadSolid, CadTransform, DofConstraint, Exporter, IdAllocator, LayerId,
     MaterialId, MeshCache, NodeId, NodeMetadata, PartKind, SceneBuilder,
     SceneVisitor, walk_scene,
@@ -100,7 +99,7 @@ mod scene_conversion {
         // Build a small scene, extract nodes, convert
         // back to CadScene, verify the geometry survived.
         let original = build_house_scene();
-        let parts = nigig_build::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&original);
+        let parts = crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&original);
         assert_eq!(parts.len(), 11);
 
         // Convert back via the legacy adapter.
@@ -147,7 +146,7 @@ mod scene_conversion {
             PartKind::Arc,
         ] {
             // The From impls in mod.rs handle this.
-            let cad_kind: nigig_build::construction_frame::pages::workspace::cad::cad_scene::PartKind = kind.into();
+            let cad_kind: crate::construction_frame::pages::workspace::cad::cad_scene::PartKind = kind.into();
             let back: PartKind = cad_kind.into();
             assert_eq!(kind, back, "PartKind round-trip failed for {:?}", kind);
         }
@@ -157,7 +156,7 @@ mod scene_conversion {
     #[test]
     fn empty_scene_converts_cleanly() {
         let empty = CadScene::default();
-        let parts = nigig_build::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&empty);
+        let parts = crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&empty);
         assert!(parts.is_empty());
     }
 }
@@ -191,7 +190,7 @@ mod visitor {
         fn visit_sphere(&mut self, _node: &CadNode, _r: f32, _su: u32, _sv: u32) { self.spheres += 1; }
         fn visit_polygon_2d(&mut self, _node: &CadNode, _verts: &[DVec2]) { self.polygons += 1; }
         fn visit_extruded_polygon(&mut self, _node: &CadNode, _verts: &[DVec2], _h: f32) { self.extruded += 1; }
-        fn visit_csg(&mut self, _node: &CadNode, _solid: &nigig_build::makepad_csg::Solid) { self.csgs += 1; }
+        fn visit_csg(&mut self, _node: &CadNode, _solid: &crate::makepad_csg::Solid) { self.csgs += 1; }
         fn visit_group(&mut self, _node: &CadNode) { self.groups += 1; }
     }
 
@@ -240,7 +239,7 @@ mod mesh_cache {
         let cache = MeshCache::new();
 
         // First pass: every node is a cache miss, builds a fresh mesh.
-        let meshes_v1: Vec<Arc<nigig_build::makepad_csg::TriMesh>> = scene
+        let meshes_v1: Vec<Arc<crate::makepad_csg::TriMesh>> = scene
             .nodes()
             .iter()
             .map(|n| cache.get_or_build(n))
@@ -249,7 +248,7 @@ mod mesh_cache {
         assert_eq!(cache.len(), 11, "cache should have one entry per node");
 
         // Second pass: every node should be a cache hit (same Arc).
-        let meshes_v2: Vec<Arc<nigig_build::makepad_csg::TriMesh>> = scene
+        let meshes_v2: Vec<Arc<crate::makepad_csg::TriMesh>> = scene
             .nodes()
             .iter()
             .map(|n| cache.get_or_build(n))
@@ -355,7 +354,7 @@ mod mesh_cache {
 
 mod glb_validity {
     use super::*;
-    use nigig_build::construction_frame::pages::workspace::cad::arch_gltf::{GltfExporter, GltfExportOptions};
+    use crate::construction_frame::pages::workspace::cad::arch_gltf::{GltfExporter, GltfExportOptions};
 
     /// GLB 2.0 binary format:
     ///   bytes 0..4   = "glTF" magic
@@ -491,7 +490,7 @@ mod glb_validity {
 
 mod pdf_dimensions {
     use super::*;
-    use nigig_build::construction_frame::pages::workspace::cad::arch_pdf::{
+    use crate::construction_frame::pages::workspace::cad::arch_pdf::{
         Orientation, PaperSize, PdfExporter, PdfExportOptions,
     };
 
@@ -554,7 +553,7 @@ mod pdf_dimensions {
         assert_eq!(pdf.format_name(), "PDF 1.7");
         assert_eq!(pdf.file_extension(), "pdf");
 
-        let glb = nigig_build::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter::default();
+        let glb = crate::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter::default();
         assert_eq!(glb.format_name(), "GLB 2.0");
         assert_eq!(glb.file_extension(), "glb");
     }
@@ -566,8 +565,8 @@ mod pdf_dimensions {
 
 mod exporter_trait {
     use super::*;
-    use nigig_build::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter;
+    use crate::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter;
-    use nigig_build::construction_frame::pages::workspace::cad::arch_pdf::PdfExporter;
+    use crate::construction_frame::pages::workspace::cad::arch_pdf::PdfExporter;
 
     #[test]
     fn both_exporters_can_be_used_through_trait_object() {
@@ -596,7 +595,7 @@ mod exporter_trait {
 
 mod round_trip {
     use super::*;
-    use nigig_build::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter;
+    use crate::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter;
 
     #[test]
     fn build_export_reparse_preserves_node_count() {
@@ -648,16 +647,16 @@ mod round_trip {
 }
 
 // ===========================================================================
-// SceneCache integration tests
+// SceneCache integration tests (v4)
 // ===========================================================================
 
 mod scene_cache_integration {
     use super::*;
-    use nigig_build::construction_frame::pages::workspace::cad::scene_holder::SceneCache;
+    use crate::construction_frame::pages::workspace::cad::scene_holder::SceneCache;
     use std::sync::Arc;
 
     /// Build a Vec<CadNode> with N walls of different colors.
-    use nigig_build::construction_frame::pages::workspace::cad::scene_holder::PartsStore;
+    use crate::construction_frame::pages::workspace::cad::scene_holder::PartsStore;
 
     fn make_colored_store(n: usize) -> PartsStore {
         let mut store = PartsStore::new();
@@ -678,7 +677,7 @@ mod scene_cache_integration {
                 .finish();
         }
         let scene = builder.build();
-        nigig_build::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)
+        crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)
     }
 
     #[test]
@@ -803,7 +802,7 @@ mod scene_cache_integration {
         // the mesh cache by exporting once, then export again and
         // verify the cache has entries (proving the second export
         // reused the shared cache).
-        use nigig_build::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter;
+        use crate::construction_frame::pages::workspace::cad::arch_gltf::GltfExporter;
 
         let cache = SceneCache::new();
         let parts = make_colored_parts(5);
@@ -884,6 +883,164 @@ mod scene_cache_integration {
     }
 }
 
+// ===========================================================================
+// v16: Tests for moved viewport helpers
+// ===========================================================================
+
+mod viewport_helper_tests {
+    use super::*;
+    use crate::construction_frame::pages::workspace::cad::cad_scene::{
+        CadSolid, CadTransform, IdAllocator, LayerId, MaterialId, NodeId,
+        NodeMetadata, SceneBuilder,
+    };
+    use crate::makepad_csg::Solid;
+
+    /// Test that CadStats correctly computes vertex/triangle counts.
+    #[test]
+    fn cad_stats_computes_counts() {
+        let mut alloc = IdAllocator::new();
+        let scene = SceneBuilder::new(&mut alloc)
+            .cube()
+            .size(Vec3f { x: 2.0, y: 2.0, z: 2.0 })
+            .finish()
+            .build();
+
+        // Build a solid and check stats.
+        let part = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)[0];
+        let solid = part.build_solid();
+        let mesh = solid.mesh();
+
+        assert!(!mesh.vertices.is_empty(), "cube should have vertices");
+        assert!(!mesh.triangles.is_empty(), "cube should have triangles");
+
+        // A cube has 8 vertices and 12 triangles (6 faces × 2).
+        // But makepad_csg may produce more depending on tessellation.
+        assert!(mesh.vertices.len() >= 8, "cube should have at least 8 vertices");
+        assert!(mesh.triangles.len() >= 12, "cube should have at least 12 triangles");
+    }
+
+    /// Test that part_mesh_buffers produces valid (indices, vertices) pairs.
+    #[test]
+    fn part_mesh_buffers_produces_valid_data() {
+        let mut alloc = IdAllocator::new();
+        let scene = SceneBuilder::new(&mut alloc)
+            .cube()
+            .size(Vec3f { x: 1.0, y: 1.0, z: 1.0 })
+            .finish()
+            .build();
+
+        let part = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)[0];
+        let solid = part.build_solid();
+        let mesh = solid.mesh();
+
+        assert!(!mesh.vertices.is_empty());
+        assert!(!mesh.triangles.is_empty());
+
+        // Verify triangle indices are within vertex bounds.
+        for tri in &mesh.triangles {
+            assert!((tri[0] as usize) < mesh.vertices.len());
+            assert!((tri[1] as usize) < mesh.vertices.len());
+            assert!((tri[2] as usize) < mesh.vertices.len());
+        }
+    }
+
+    /// Test that build_solid produces different meshes for different sizes.
+    #[test]
+    fn build_solid_respects_size() {
+        let mut alloc = IdAllocator::new();
+        let scene_small = SceneBuilder::new(&mut alloc)
+            .cube()
+            .size(Vec3f { x: 1.0, y: 1.0, z: 1.0 })
+            .finish()
+            .build();
+
+        let mut alloc2 = IdAllocator::new();
+        let scene_big = SceneBuilder::new(&mut alloc2)
+            .cube()
+            .size(Vec3f { x: 10.0, y: 10.0, z: 10.0 })
+            .finish()
+            .build();
+
+        let part_small = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene_small)[0];
+        let part_big = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene_big)[0];
+
+        let solid_small = part_small.build_solid();
+        let solid_big = part_big.build_solid();
+        let mesh_small = solid_small.mesh();
+        let mesh_big = solid_big.mesh();
+
+        // Both should have the same number of vertices (same shape,
+        // different scale), but different coordinate values.
+        assert_eq!(mesh_small.vertices.len(), mesh_big.vertices.len());
+
+        // The big cube's vertices should have larger coordinates.
+        let small_max = mesh_small.vertices.iter().map(|v| v.x.abs()).fold(0.0f64, f64::max);
+        let big_max = mesh_big.vertices.iter().map(|v| v.x.abs()).fold(0.0f64, f64::max);
+        assert!(big_max > small_max, "big cube should have larger coordinates");
+    }
+
+    /// Test that build_world_solid applies translation.
+    #[test]
+    fn build_world_solid_applies_translation() {
+        let mut alloc = IdAllocator::new();
+        let scene = SceneBuilder::new(&mut alloc)
+            .cube()
+            .size(Vec3f { x: 1.0, y: 1.0, z: 1.0 })
+            .translation(Vec3f { x: 5.0, y: 0.0, z: 0.0 })
+            .finish()
+            .build();
+
+        let part = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)[0];
+        let local_solid = part.build_solid();
+        let world_solid = part.build_world_solid();
+        let local_mesh = local_solid.mesh();
+        let world_mesh = world_solid.mesh();
+
+        // World mesh vertices should be shifted by +5 in X.
+        let local_avg_x = local_mesh.vertices.iter().map(|v| v.x).sum::<f64>()
+            / local_mesh.vertices.len() as f64;
+        let world_avg_x = world_mesh.vertices.iter().map(|v| v.x).sum::<f64>()
+            / world_mesh.vertices.len() as f64;
+
+        assert!(
+            (world_avg_x - local_avg_x - 5.0).abs() < 0.01,
+            "world mesh should be translated by +5 in X (got diff={})",
+            world_avg_x - local_avg_x
+        );
+    }
+
+    /// Test that CadRenderMode has the expected variants.
+    #[test]
+    fn cad_render_mode_variants() {
+        use crate::construction_frame::pages::workspace::cad::viewport::CadRenderMode;
+        // Just verify the type exists and can be constructed.
+        let mode = CadRenderMode::Realistic;
+        let _ = format!("{:?}", mode);
+    }
+
+    /// `CommandBorrows` can be constructed and its fields read.
+    #[test]
+    fn command_borrows_fields_accessible() {
+        use crate::construction_frame::pages::workspace::cad::viewport::CommandBorrows;
+        use crate::construction_frame::pages::workspace::cad::scene_holder::SceneCache;
+        use crate::construction_frame::pages::workspace::cad::commands::UndoRedoStack;
+
+        let mut parts: Vec<CadNode> = vec![];
+        let scene_cache = SceneCache::new();
+        let mut command_stack = UndoRedoStack::new();
+
+        let holder = CommandBorrows {
+            parts: &mut parts,
+            scene_cache: &scene_cache,
+            command_stack: &mut command_stack,
+        };
+
+        // Verify fields are accessible.
+        assert_eq!(holder.parts.len(), 0);
+        assert!(!holder.command_stack.can_undo());
+    }
+}
+
 // ===========================================================================
 // Editing tool tests — 2D and 3D coverage for Add/Delete/Move/Resize/Rotate
 
@@ -895,10 +1052,10 @@ mod scene_cache_integration {
 
 mod editing_tools_tests {
     use super::*;
-    use nigig_build::construction_frame::pages::workspace::cad::commands::{
+    use crate::construction_frame::pages::workspace::cad::commands::{
     };
-    use nigig_build::construction_frame::pages::workspace::cad::scene_holder::SceneCache;
+    use crate::construction_frame::pages::workspace::cad::scene_holder::SceneCache;
-    use nigig_build::construction_frame::pages::workspace::cad::math::point_in_polygon;
+    use crate::construction_frame::pages::workspace::cad::math::point_in_polygon;
 
     /// Build a test part at the given position with a given size.
     fn make_part_at(id: u64, pos: Vec3f, size: Vec3f) -> CadNode {
@@ -1366,7 +1523,7 @@ mod editing_tools_tests {
 
 #[cfg(test)]
 mod dde_integration_tests {
-    use nigig_build::construction_frame::pages::workspace::cad::construction_geometry::{
+    use crate::construction_frame::pages::workspace::cad::construction_geometry::{
         CoordInput, parse_coord_input,
     };
     use makepad_widgets::DVec2;
@@ -1584,7 +1741,7 @@ mod dde_integration_tests {
 
 #[cfg(test)]
 mod window_crossing_tests {
-    use nigig_build::construction_frame::pages::workspace::cad::SelectionMode;
+    use crate::construction_frame::pages::workspace::cad::SelectionMode;
     use makepad_widgets::DVec2;
 
     /// A minimal 2D part representation for testing marquee selection logic.
@@ -1896,7 +2053,7 @@ mod window_crossing_tests {
 
 #[cfg(test)]
 mod polar_tracking_tests {
-    use nigig_build::construction_frame::pages::workspace::cad::construction_geometry::{
+    use crate::construction_frame::pages::workspace::cad::construction_geometry::{
         snap_to_polar_angle, next_polar_increment,
     };
     use std::f64::consts::PI;
@@ -1981,6 +2138,21 @@ mod polar_tracking_tests {
         assert!((next_polar_increment(22.0) - 5.0).abs() < 0.1);
     }
 
+    #[test]
+    fn polar_settings_default() {
+        let s = crate::construction_frame::pages::workspace::cad::SnapSettings::default();
+        assert!(!s.polar_enabled);
+        assert!((s.polar_angle_increment - 45.0).abs() < 0.1);
+    }
+
+    #[test]
+    fn polar_settings_custom_increment() {
+        let mut s = crate::construction_frame::pages::workspace::cad::SnapSettings::default();
+        s.polar_enabled = true;
+        s.polar_angle_increment = 15.0;
+        assert!(s.polar_enabled);
+        assert!((s.polar_angle_increment - 15.0).abs() < 0.1);
+    }
 }
 
 // ===========================================================================
@@ -1989,8 +2161,8 @@ mod polar_tracking_tests {
 
 #[cfg(test)]
 mod section_shape_tests {
-    use nigig_build::construction_frame::pages::workspace::cad::cad_scene::{CadNode, CadSolid, PartKind};
+    use crate::construction_frame::pages::workspace::cad::cad_scene::{CadNode, CadSolid, PartKind};
-    use nigig_build::construction_frame::pages::workspace::cad::section_shape::{
+    use crate::construction_frame::pages::workspace::cad::section_shape::{
         SectionShape, IBeamParams, HSSParams,
         section_vertices, section_bounding_box, section_area,
         rect_vertices, ibeam_vertices, hss_vertices,
@@ -2163,6 +2335,25 @@ mod section_shape_tests {
         assert_eq!(v.len(), 8);
     }
 
+    // ── Beam section settings ──
+
+    #[test]
+    fn drawing_state_beam_section_default() {
+        let ds = crate::construction_frame::pages::workspace::cad::DrawingState::default();
+        assert_eq!(ds.beam_section, SectionShape::Rect);
+    }
+
+    #[test]
+    fn drawing_state_beam_section_cycle() {
+        let mut ds = crate::construction_frame::pages::workspace::cad::DrawingState::default();
+        assert_eq!(ds.beam_section, SectionShape::Rect);
+        ds.beam_section = ds.beam_section.next();
+        assert_eq!(ds.beam_section, SectionShape::IBeam);
+        ds.beam_section = ds.beam_section.next();
+        assert_eq!(ds.beam_section, SectionShape::HSS);
+        ds.beam_section = ds.beam_section.next();
+        assert_eq!(ds.beam_section, SectionShape::Rect);
+    }
 }
 
 // ===========================================================================
@@ -2322,8 +2513,8 @@ mod selection_action_tests {
 #[cfg(test)]
 mod export_tests {
     use super::*;
-    use nigig_build::construction_frame::pages::workspace::cad::arch_stl::{StlExporter, StlExportOptions};
+    use crate::construction_frame::pages::workspace::cad::arch_stl::{StlExporter, StlExportOptions};
-    use nigig_build::construction_frame::pages::workspace::cad::arch_svg::{SvgExporter, SvgExportOptions};
+    use crate::construction_frame::pages::workspace::cad::arch_svg::{SvgExporter, SvgExportOptions};
 
     fn make_box_node(id: u64, pos: Vec3f) -> CadNode {
         CadNode {
@@ -2530,6 +2721,21 @@ mod export_tests {
         assert!(header.starts_with("custom header test"));
     }
 
+    // ── PdfPreview enum variant ──
+
+    #[test]
+    fn pdf_preview_active_pane_variant() {
+        use super::super::CadEditorActivePane;
+        let pane = CadEditorActivePane::PdfPreview;
+        assert_eq!(pane.title(), "PDF Preview");
+    }
+
+    #[test]
+    fn pdf_preview_active_pane_not_default() {
+        use super::super::CadEditorActivePane;
+        let default = CadEditorActivePane::default();
+        assert_ne!(default, CadEditorActivePane::PdfPreview);
+    }
 }
 
 
@@ -2539,7 +2745,7 @@ mod export_tests {
 
 #[cfg(test)]
 mod hover_throttle_tests {
-    use nigig_build::construction_frame::pages::workspace::cad::constants::HOVER_PICK_MIN_MOVE_PX;
+    use crate::construction_frame::pages::workspace::cad::constants::HOVER_PICK_MIN_MOVE_PX;
     use makepad_widgets::DVec2;
 
     /// Mirrors the predicate in `CadViewport::handle_event`.
@@ -2577,7 +2783,7 @@ mod hover_throttle_tests {
     /// would visibly lag the cursor.
     #[test]
     fn threshold_is_smaller_than_pick_radius() {
-        use nigig_build::construction_frame::pages::workspace::cad::constants::PART_PICK_RADIUS;
+        use crate::construction_frame::pages::workspace::cad::constants::PART_PICK_RADIUS;
         assert!(
             HOVER_PICK_MIN_MOVE_PX < PART_PICK_RADIUS / 4.0,
             "hover threshold {HOVER_PICK_MIN_MOVE_PX} is too coarse for pick radius {PART_PICK_RADIUS}"
@@ -2587,7 +2793,7 @@ mod hover_throttle_tests {
 
 #[cfg(test)]
 mod pick_bounds_tests {
-    use nigig_build::construction_frame::pages::workspace::cad::cad_scene::{
+    use crate::construction_frame::pages::workspace::cad::cad_scene::{
         CadNode, CadSolid, CadTransform, LayerId, MaterialId, MeshCache, NodeId, NodeMetadata,
     };
     use makepad_widgets::{vec3, Vec4f};

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/viewport.rs

@@ -87,17 +87,23 @@ impl CadViewport {
         stats
     }
 
-    /// Switch to the other view mode and return the new one.
-    ///
-    /// This is `set_view_mode` with the target computed rather than
-    /// passed; the two carried duplicate copies of the gesture-state
-    /// reset, so a fix to one silently missed the other.
     pub(crate) fn toggle_view_mode(&mut self, cx: &mut Cx) -> ViewMode {
-        let next = match self.view_mode {
+        self.view_mode = match self.view_mode {
             ViewMode::ThreeD => ViewMode::TwoD,
             ViewMode::TwoD => ViewMode::ThreeD,
         };
-        self.set_view_mode(cx, next);
+        self.part_dragging = false;
+        self.view_dragging = false;
+        self.pan_3d_dragging = false;
+        self.active_touches.clear();
+        self.pinch_last_dist = None;
+        self.pinch_last_mid = None;
+        self.camera.orbit_last_abs = None;
+        if self.view_mode == ViewMode::TwoD {
+            self.pan_2d = DVec2::default();
+            self.view_drag_world = DVec2::default();
+        }
+        cx.redraw_all();
         self.view_mode
     }
 
@@ -150,8 +156,9 @@ impl CadViewport {
         self.selection = selection;
         self.next_part_id = next_part_id;
         self.part_geoms.clear();
-        // Wholesale parts replacement: the scene snapshot is stale and
+        // v4: invalidate caches. Wholesale parts replacement = scene
-        // node ids may have shifted, so clear the mesh cache too.
+        // snapshot is stale, and node ids may have shifted so clear
+        // the mesh cache too.
         self.mark_scene_dirty();
         self.clear_mesh_cache();
         self.script_dirty = false;
@@ -212,10 +219,10 @@ impl CadViewport {
         self.area.redraw(cx);
     }
 
-    // ----- command-based undo/redo -----
+    // ----- v19: command-based undo/redo system -----
     //
-    // `Command` + `UndoRedoStack`, with `CadCommandCtx` providing the
+    // Uses the new `Command` trait + `UndoRedoStack` with
-    // `CommandContext` implementation.
+    // `CadCommandCtx` providing the `CommandContext` implementation.
     // Cache invalidation happens automatically inside each command's
     // `execute()` / `undo()` via the context.
 
@@ -328,10 +335,10 @@ impl CadViewport {
         self.command_stack.clear();
     }
 
-    // ----- command constructors -----
+    // ----- v19: convenience methods using new Command types -----
     //
-    // These build a command and push it onto the UndoRedoStack via
+    // These construct the new command structs and push them onto the
-    // execute_command(). Cache invalidation is
+    // UndoRedoStack via execute_command(). Cache invalidation is
     // automatic inside each command.
 
     /// Move a part and push a `MoveNode` onto the command stack.
@@ -382,8 +389,8 @@ impl CadViewport {
 
     /// Record an add-part operation on the command stack.
     ///
-    /// Records a `CreateNode` on the command stack. The caller must
+    /// v19: now uses `CreateNode` on the new command stack. The caller
-    /// push the part onto `self.parts` BEFORE calling this.
+    /// must push the part onto `self.parts` BEFORE calling this method.
     pub(crate) fn add_part_command(&mut self, id: u64) {
         // Find the just-pushed part to snapshot it for undo/redo.
         let part_snapshot = match self.parts.iter().position(|p| p.id.raw() == id) {
@@ -427,7 +434,7 @@ impl CadViewport {
             PartKind::Rect2D => (vec3(1.5, 0.8, 0.1), vec4(0.4, 0.8, 0.9, 1.0)),
             PartKind::Circle2D => (vec3(0.9, 0.9, 0.1), vec4(0.9, 0.6, 0.4, 1.0)),
             PartKind::Polygon2D => (vec3(1.0, 0.1, 1.0), vec4(0.3, 0.7, 0.5, 1.0)),
-            // Architectural defaults — realistic dimensions in metres.
+            // v2: architectural defaults — realistic dimensions in meters.
             PartKind::Wall => (vec3(6.0, 2.8, 0.2), vec4(0.78, 0.78, 0.78, 1.0)),
             PartKind::Slab => (vec3(4.0, 0.2, 4.0), vec4(0.85, 0.85, 0.85, 1.0)),
             PartKind::Door => (vec3(0.9, 2.1, 0.2), vec4(0.55, 0.35, 0.20, 1.0)),
@@ -1216,8 +1223,8 @@ impl CadViewport {
         // index so the delete command can restore it on undo.
         self.delete_part_command(part.id.raw(), part.clone(), old_idx);
         self.selection = new_ids;
-        // Structural edit (added N parts, removed 1): invalidate both
+        // v4: structural edit (added N parts, removed 1). Invalidate
-        // caches.
+        // both caches.
         self.mark_scene_dirty();
         self.clear_mesh_cache();
         self.script_dirty = true;
@@ -1263,8 +1270,8 @@ impl CadViewport {
             }
             p
         };
-        // modify_part_command gives automatic cache invalidation and
+        // v12: use modify_part_command for automatic cache invalidation
-        // undo recording on the command stack.
+        // + undo recording on the command stack.
         self.modify_part_command(id, old_part.clone(), new_part.clone());
         self.part_geoms.remove(&id);
         self.script_dirty = true;
@@ -1329,9 +1336,10 @@ impl CadViewport {
                 p.parent = Some(NodeId(gid));
             }
         }
-        // Parameter edit: group_id feeds NodeMetadata.parent, so the
+        // v4: parameter edit (group_id affects NodeMetadata.parent).
-        // cached snapshot must be rebuilt. No mesh invalidation — the
+        // mark_scene_dirty so the cached snapshot picks up the new
-        // group id does not affect geometry.
+        // group ids. No mesh invalidation needed — group_id doesn't
+        // affect geometry.
         self.mark_scene_dirty();
     }
 
@@ -1509,9 +1517,10 @@ impl CadViewport {
 
     /// Get the current scene as a cached `Arc<CadScene>`.
     ///
-    /// O(1) on a no-op redraw: returns the cached `Arc`. Rebuilds only
+    /// v4: now O(1) on no-op redraws (returns the cached Arc).
-    /// when the `PartsStore` generation has moved past the one the
+    /// Rebuilds only after `scene_cache.mark_dirty()` or if
-    /// snapshot was built from.
+    /// `parts.len()` differs from the cached scene's node count
+    /// (safety net for missing `mark_dirty()` calls).
     ///
     /// The returned `Arc` can be held independently of the viewport's
     /// lifetime — the scene stays alive even if the viewport mutates
@@ -1525,8 +1534,9 @@ impl CadViewport {
     /// can be held independently of the viewport's lifetime —
     /// the cache stays alive even if the viewport is dropped.
     ///
-    /// Returns an `Arc` so the GLB exporter can reuse the preview
+    /// v4: now returns `Arc<MeshCache>` (was `&MeshCache`). This
-    /// renderer's cached meshes; both hold the same underlying cache.
+    /// lets the GLB exporter reuse the preview renderer's cached
+    /// meshes: both hold `Arc`s to the same underlying cache.
     pub fn mesh_cache(&self) -> std::sync::Arc<MeshCache> {
         self.scene_cache.mesh_cache()
     }
@@ -1535,8 +1545,9 @@ impl CadViewport {
     /// the node's parameters change (e.g. wall length edited in the
     /// properties panel).
     ///
-    /// Rarely necessary: parameter edits are detected automatically
+    /// Note: with v2's parameter-hash-based cache, this is now rarely
-    /// via ParamHash. Use this only to force a rebuild.
+    /// necessary — parameter edits are detected automatically via
+    /// ParamHash. Use this only for forced rebuilds.
     pub fn invalidate_node(&self, id: u64) {
         self.scene_cache.invalidate_node(id);
     }
@@ -1551,10 +1562,9 @@ impl CadViewport {
     /// *any* mutation to `self.parts`. The next `scene()` call will
     /// rebuild.
     ///
-    /// Marks the cached scene snapshot stale. Prefer mutating through
+    /// v4: new method. The `scene()` safety net catches length
-    /// `PartsStore`, which bumps the generation and makes this
+    /// mismatches, but parameter edits (same length, different
-    /// unnecessary; this remains for the paths that still edit parts
+    /// content) need explicit `mark_dirty()`.
-    /// through `as_mut_vec()`.
     pub fn mark_scene_dirty(&self) {
         self.scene_cache.mark_dirty();
     }
@@ -1642,7 +1652,7 @@ impl CadViewport {
                         format!("cube(0.01,0.01,0.01,true)")
                     }
                 }
-                // Arch kinds emit cube() or cylinder() in the CAD script.
+                // v2: arch kinds emit cube() or cylinder() in the CAD script.
                 PartKind::Wall
                 | PartKind::Slab
                 | PartKind::Door
@@ -2586,7 +2596,7 @@ impl CadViewport {
         self.draw_vector.line_to(o.x as f32, o.y as f32 + 6.0);
         self.draw_vector.stroke(1.0);
 
-        // Rubber band preview.
+        // v3: rubber band preview
         self.draw_rubber_band(cx);
         self.draw_polar_guidelines(cx);
         self.draw_section_indicators(cx);
@@ -2792,7 +2802,7 @@ impl CadViewport {
     }
 
     // =======================================================================
-    // CAD tool system — snapping, drawing, rubber band, coordinate readout
+    // v3: CAD Tool System — snapping, drawing, rubber band, coordinate readout
     // =======================================================================
 
     pub(crate) fn set_tool(&mut self, cx: &mut Cx, tool: CadTool) {
@@ -3353,15 +3363,8 @@ impl CadViewport {
                         let new_w = (part.size().x as f64 + extend_dist).min(10.0) as f32;
                         let new_h = (part.size().z as f64 + extend_dist).min(10.0) as f32;
                         if let Some(p) = self.parts.get_mut_by_raw_id(selected_id) {
-                            // `size()` returns a copy; writing through it
+                            p.size().x = new_w;
-                            // (`p.size().x = ..`) compiles and silently
+                            p.size().z = new_h;
-                            // discards the write, which is what this did
-                            // and why the Extend tool never extended
-                            // anything.
-                            let mut s = p.size();
-                            s.x = new_w;
-                            s.z = new_h;
-                            p.set_size(s);
                         }
                     }
                 }
@@ -3479,8 +3482,9 @@ impl CadViewport {
             }
         }
         self.selection = vec![id];
-        // Structural edit: a tool finished and added one or more parts
+        // v4: structural edit (tool finished, one or more parts added).
-        // (wall/circle/rect/area/column/beam). Invalidate caches.
+        // Invalidate caches. Catches wall/circle/rect/area/column/beam
+        // tool completions.
         self.mark_scene_dirty();
         self.clear_mesh_cache();
         self.script_dirty = true;
@@ -3516,7 +3520,7 @@ impl CadViewport {
             kind_hint: Some(PartKind::Polygon2D),
         });
         self.add_part_command(id);
-        // Structural edit (new polygon part): invalidate caches.
+        // v4: structural edit (new polygon part). Invalidate caches.
         self.mark_scene_dirty();
         self.clear_mesh_cache();
     }
@@ -3636,7 +3640,7 @@ impl CadViewport {
             prev = next;
         }
         self.selection = ids;
-        // Structural edit (added N walls from path): invalidate caches.
+        // v4: structural edit (added N walls from path). Invalidate caches.
         self.mark_scene_dirty();
         self.clear_mesh_cache();
         self.script_dirty = true;
@@ -6751,7 +6755,7 @@ impl Widget for CadViewport {
 
 
 // ===========================================================================
-// Helpers that need no #[derive(Script)], so they live outside mod.rs
+// v16: Helpers moved from mod.rs (no #[derive(Script)] needed)
 // ===========================================================================
 
 impl DrawCadMesh {
@@ -6802,55 +6806,7 @@ pub(crate) struct CadMeshData {
     pub(crate) stats: CadStats,
 }
 
-/// How a solid's mesh is mapped into GPU buffer space.
+pub(crate) fn cad_mesh_data_from_solid(solid: &Solid) -> CadMeshData {
-///
-/// The two consumers of the mesh pipeline want different things from
-/// the same triangle walk, and used to have a near-identical copy of
-/// that walk each. The differences are exactly the two variants below.
-#[derive(Clone, Copy, PartialEq, Eq)]
-pub(crate) enum MeshSpace {
-    /// Recentre on the solid's bounding-box centre and scale so the
-    /// largest dimension is `VIEW_FIT_EXTENT`. Nudge each vertex
-    /// `SURFACE_OFFSET` along its normal to reduce z-fighting between
-    /// coincident faces. Used for the single-solid preview viewport,
-    /// which has no camera framing of its own.
-    ViewNormalised,
-    /// Keep model coordinates untouched. Used for per-part geometry in
-    /// the multi-part scene, where each part's transform places it and
-    /// rescaling would break the layout.
-    Model,
-}
-
-/// Fit extent for `MeshSpace::ViewNormalised`: the largest dimension of
-/// the solid is scaled to this many world units.
-const VIEW_FIT_EXTENT: f64 = 1.75;
-
-/// Per-vertex outward nudge applied in `MeshSpace::ViewNormalised`.
-const SURFACE_OFFSET: f64 = 0.0001;
-
-/// Triangle winding of the emitted vertex buffer.
-#[derive(Clone, Copy, PartialEq, Eq)]
-pub(crate) enum Winding {
-    /// Emit p0, p1, p2 — the order the normal was computed from.
-    AsComputed,
-    /// Emit p0, p2, p1. The per-part scene path has always used this
-    /// and the shader disables backface culling, so it renders the
-    /// same; it is preserved rather than "fixed" because nothing here
-    /// proves which one the depth-sorted paths expect.
-    Reversed,
-}
-
-/// The one triangle walk that feeds every GPU buffer in this module.
-///
-/// Returns the interleaved vertex buffer (8 floats per vertex: position
-/// xyzw, normal xyz, pad) and its identity index buffer, plus the stats
-/// derived from the solid. Returns empty buffers when the solid has no
-/// geometry; callers decide whether that is `None` or an empty struct.
-fn build_mesh_buffers(
-    solid: &Solid,
-    space: MeshSpace,
-    winding: Winding,
-) -> (Vec<u32>, Vec<f32>, CadStats) {
     let mesh = solid.mesh();
     let mut stats = CadStats {
         vertices: solid.vertex_count(),
@@ -6859,27 +6815,24 @@ fn build_mesh_buffers(
     };
 
     if mesh.vertices.is_empty() || mesh.triangles.is_empty() {
-        return (Vec::new(), Vec::new(), stats);
+        return CadMeshData {
+            stats,
+            ..Default::default()
+        };
     }
 
-    // Only the normalised path needs the bounding box. An empty box is
+    let bbox = mesh.bounding_box();
-    // unusable for scaling, so that path bails; the model path does not
+    if bbox.is_empty() {
-    // care and carries on.
+        return CadMeshData {
-    let (center, scale, offset) = match space {
+            stats,
-        MeshSpace::ViewNormalised => {
+            ..Default::default()
-            let bbox = mesh.bounding_box();
+        };
-            if bbox.is_empty() {
+    }
-                return (Vec::new(), Vec::new(), stats);
+
-            }
+    let center = bbox.center();
-            let size = bbox.size();
+    let size = bbox.size();
-            stats.max_dimension = size.x.max(size.y).max(size.z);
+    stats.max_dimension = size.x.max(size.y).max(size.z);
-            let scale = VIEW_FIT_EXTENT / stats.max_dimension.max(0.000_001);
+    let scale = 1.75 / stats.max_dimension.max(0.000_001);
-            let c = bbox.center();
-            ((c.x, c.y, c.z), scale, SURFACE_OFFSET)
-        }
-        MeshSpace::Model => ((0.0, 0.0, 0.0), 1.0, 0.0),
-    };
-    let (center_x, center_y, center_z) = center;
 
     #[cfg(target_os = "android")]
     let y_flip = -1.0;
@@ -6890,26 +6843,32 @@ fn build_mesh_buffers(
     let mut indices = Vec::with_capacity(mesh.triangles.len() * 3);
 
     for tri in &mesh.triangles {
-        let (Some(a), Some(b), Some(c)) = (
+        let Some(a) = mesh.vertices.get(tri[0] as usize) else {
-            mesh.vertices.get(tri[0] as usize),
+            continue;
-            mesh.vertices.get(tri[1] as usize),
+        };
-            mesh.vertices.get(tri[2] as usize),
+        let Some(b) = mesh.vertices.get(tri[1] as usize) else {
-        ) else {
+            continue;
+        };
+        let Some(c) = mesh.vertices.get(tri[2] as usize) else {
             continue;
         };
 
-        let ax = (a.x - center_x) * scale;
+        let ax = (a.x - center.x) * scale;
-        let ay = (a.y - center_y) * scale;
+        let ay = (a.y - center.y) * scale;
-        let az = (a.z - center_z) * scale;
+        let az = (a.z - center.z) * scale;
-        let bx = (b.x - center_x) * scale;
+        let bx = (b.x - center.x) * scale;
-        let by = (b.y - center_y) * scale;
+        let by = (b.y - center.y) * scale;
-        let bz = (b.z - center_z) * scale;
+        let bz = (b.z - center.z) * scale;
-        let cx_ = (c.x - center_x) * scale;
+        let cx_ = (c.x - center.x) * scale;
-        let cy = (c.y - center_y) * scale;
+        let cy = (c.y - center.y) * scale;
-        let cz = (c.z - center_z) * scale;
+        let cz = (c.z - center.z) * scale;
 
-        let (ex, ey, ez) = (bx - ax, by - ay, bz - az);
+        let ex = bx - ax;
-        let (fx, fy, fz) = (cx_ - ax, cy - ay, cz - az);
+        let ey = by - ay;
+        let ez = bz - az;
+        let fx = cx_ - ax;
+        let fy = cy - ay;
+        let fz = cz - az;
         let mut nx = ey * fz - ez * fy;
         let mut ny = ez * fx - ex * fz;
         let mut nz = ex * fy - ey * fx;
@@ -6924,64 +6883,73 @@ fn build_mesh_buffers(
             nz = 0.0;
         }
 
-        // Flip normals that point back towards the centroid so a convex
-        // solid always faces outward.
         let centroid_x = (ax + bx + cx_) / 3.0;
         let centroid_y = (ay + by + cy) / 3.0;
         let centroid_z = (az + bz + cz) / 3.0;
-        let inward = nx * centroid_x + ny * centroid_y + nz * centroid_z < 0.0;
+        let dot = nx * centroid_x + ny * centroid_y + nz * centroid_z;
-        let (fnx, fny, fnz) = if inward { (-nx, -ny, -nz) } else { (nx, ny, nz) };
+        let offset = 0.0001;
 
-        let place = |px: f64, py: f64, pz: f64| {
+        let (p0, p1, p2, normal) = if dot < 0.0 {
-            [
+            let fnx = -nx;
-                (px + fnx * offset) as f32,
+            let fny = -ny;
-                ((py + fny * offset) * y_flip) as f32,
+            let fnz = -nz;
-                (pz + fnz * offset) as f32,
+            (
-            ]
+                [
-        };
+                    (bx + fnx * offset) as f32,
-        // A flipped normal also swaps the first two vertices, so the
+                    ((by + fny * offset) * y_flip) as f32,
-        // winding stays consistent with the emitted normal.
+                    (bz + fnz * offset) as f32,
-        let (p0, p1, p2) = if inward {
+                ],
-            (place(bx, by, bz), place(ax, ay, az), place(cx_, cy, cz))
+                [
+                    (ax + fnx * offset) as f32,
+                    ((ay + fny * offset) * y_flip) as f32,
+                    (az + fnz * offset) as f32,
+                ],
+                [
+                    (cx_ + fnx * offset) as f32,
+                    ((cy + fny * offset) * y_flip) as f32,
+                    (cz + fnz * offset) as f32,
+                ],
+                [fnx as f32, (fny * y_flip) as f32, fnz as f32],
+            )
         } else {
-            (place(ax, ay, az), place(bx, by, bz), place(cx_, cy, cz))
+            (
+                [
+                    (ax + nx * offset) as f32,
+                    ((ay + ny * offset) * y_flip) as f32,
+                    (az + nz * offset) as f32,
+                ],
+                [
+                    (bx + nx * offset) as f32,
+                    ((by + ny * offset) * y_flip) as f32,
+                    (bz + nz * offset) as f32,
+                ],
+                [
+                    (cx_ + nx * offset) as f32,
+                    ((cy + ny * offset) * y_flip) as f32,
+                    (cz + nz * offset) as f32,
+                ],
+                [nx as f32, (ny * y_flip) as f32, nz as f32],
+            )
         };
-        let normal = [fnx as f32, (fny * y_flip) as f32, fnz as f32];
 
-        let ordered = match winding {
+        for p in [p0, p1, p2] {
-            Winding::AsComputed => [p0, p1, p2],
-            Winding::Reversed => [p0, p2, p1],
-        };
-        for p in ordered {
             vertices
                 .extend_from_slice(&[p[0], p[1], p[2], 1.0, normal[0], normal[1], normal[2], 0.0]);
             indices.push(indices.len() as u32);
         }
     }
 
-    (indices, vertices, stats)
-}
-
-/// Mesh for the single-solid preview viewport: framed to fit, with
-/// stats for the HUD.
-pub(crate) fn cad_mesh_data_from_solid(solid: &Solid) -> CadMeshData {
-    let (indices, vertices, stats) =
-        build_mesh_buffers(solid, MeshSpace::ViewNormalised, Winding::AsComputed);
     if vertices.is_empty() || indices.is_empty() {
-        CadMeshData { stats, ..Default::default() }
+        CadMeshData {
+            stats,
+            ..Default::default()
+        }
     } else {
-        CadMeshData { indices, vertices, stats }
+        CadMeshData {
-    }
+            indices,
-}
+            vertices,
-
+            stats,
-/// Mesh for one part of the multi-part scene, in model coordinates.
+        }
-/// `None` when the solid has no geometry.
-pub(crate) fn part_mesh_buffers(solid: &Solid) -> Option<(Vec<u32>, Vec<f32>)> {
-    let (indices, vertices, _) = build_mesh_buffers(solid, MeshSpace::Model, Winding::Reversed);
-    if vertices.is_empty() || indices.is_empty() {
-        None
-    } else {
-        Some((indices, vertices))
     }
 }
 
@@ -7123,6 +7091,76 @@ pub(crate) fn cad_rebuild_worker_loop(
     log!("[CAD_WORKSPACE] worker: thread exiting (channel closed)");
 }
 
+pub(crate) fn part_mesh_buffers(solid: &Solid) -> Option<(Vec<u32>, Vec<f32>)> {
+    let mesh = solid.mesh();
+    if mesh.vertices.is_empty() || mesh.triangles.is_empty() {
+        return None;
+    }
+
+    #[cfg(target_os = "android")]
+    let y_flip = -1.0;
+    #[cfg(not(target_os = "android"))]
+    let y_flip = 1.0;
+
+    let mut vertices = Vec::with_capacity(mesh.triangles.len() * 3 * 8);
+    let mut indices = Vec::with_capacity(mesh.triangles.len() * 3);
+
+    for tri in &mesh.triangles {
+        let (Some(a), Some(b), Some(c)) = (
+            mesh.vertices.get(tri[0] as usize),
+            mesh.vertices.get(tri[1] as usize),
+            mesh.vertices.get(tri[2] as usize),
+        ) else {
+            continue;
+        };
+        let (ax, ay, az) = (a.x, a.y, a.z);
+        let (bx, by, bz) = (b.x, b.y, b.z);
+        let (cx_, cy, cz) = (c.x, c.y, c.z);
+        let (ex, ey, ez) = (bx - ax, by - ay, bz - az);
+        let (fx, fy, fz) = (cx_ - ax, cy - ay, cz - az);
+        let mut nx = ey * fz - ez * fy;
+        let mut ny = ez * fx - ex * fz;
+        let mut nz = ex * fy - ey * fx;
+        let len = (nx * nx + ny * ny + nz * nz).sqrt();
+        if len > 1.0e-10 {
+            nx /= len;
+            ny /= len;
+            nz /= len;
+        } else {
+            nx = 0.0;
+            ny = 1.0;
+            nz = 0.0;
+        }
+        let cenx = (ax + bx + cx_) / 3.0;
+        let ceny = (ay + by + cy) / 3.0;
+        let cenz = (az + bz + cz) / 3.0;
+        let (p0, p1, p2, normal) = if nx * cenx + ny * ceny + nz * cenz < 0.0 {
+            (
+                [bx as f32, (by * y_flip) as f32, bz as f32],
+                [ax as f32, (ay * y_flip) as f32, az as f32],
+                [cx_ as f32, (cy * y_flip) as f32, cz as f32],
+                [(-nx) as f32, ((-ny) * y_flip) as f32, (-nz) as f32],
+            )
+        } else {
+            (
+                [ax as f32, (ay * y_flip) as f32, az as f32],
+                [bx as f32, (by * y_flip) as f32, bz as f32],
+                [cx_ as f32, (cy * y_flip) as f32, cz as f32],
+                [nx as f32, (ny * y_flip) as f32, nz as f32],
+            )
+        };
+        for p in [p0, p2, p1] {
+            vertices
+                .extend_from_slice(&[p[0], p[1], p[2], 1.0, normal[0], normal[1], normal[2], 0.0]);
+            indices.push(indices.len() as u32);
+        }
+    }
+    if vertices.is_empty() || indices.is_empty() {
+        None
+    } else {
+        Some((indices, vertices))
+    }
+}
 
 pub(crate) fn ensure_ground_geometry(cx: &mut Cx, geometry: &mut Option<Geometry>) -> GeometryId {
     let geometry = geometry.get_or_insert_with(|| {
@@ -7198,331 +7236,3 @@ fn part_model_matrix_cadnode(node: &CadNode) -> Mat4f {
     let rzyx = mat4_mul(&mat4_mul(&rot_z_mat(t.rotation_euler_xyz.z), &rot_y_mat(t.rotation_euler_xyz.y)), &rot_x_mat(t.rotation_euler_xyz.x));
     mat4_mul(&translate_mat(t.translation), &rzyx)
 }
-
-// ===========================================================================
-// Tests for the mesh producers and the viewport's plain-data helpers.
-//
-// These live here, not in tests/cad_integration.rs, because they reach
-// crate-private items. A test that needs `pub` widened to run is a test
-// in the wrong place.
-// ===========================================================================
-
-#[cfg(test)]
-mod viewport_helper_tests {
-    use super::*;
-    use makepad_widgets::{Vec3f, Vec4f};
-    use crate::construction_frame::pages::workspace::cad::cad_scene::{
-        CadSolid, CadTransform, IdAllocator, LayerId, MaterialId, NodeId,
-        NodeMetadata, SceneBuilder,
-    };
-    use crate::makepad_csg::Solid;
-
-    /// Test that CadStats correctly computes vertex/triangle counts.
-    #[test]
-    fn cad_stats_computes_counts() {
-        let mut alloc = IdAllocator::new();
-        let scene = SceneBuilder::new(&mut alloc)
-            .cube()
-            .size(Vec3f { x: 2.0, y: 2.0, z: 2.0 })
-            .finish()
-            .build();
-
-        // Build a solid and check stats.
-        let part = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)[0];
-        let solid = part.build_solid();
-        let mesh = solid.mesh();
-
-        assert!(!mesh.vertices.is_empty(), "cube should have vertices");
-        assert!(!mesh.triangles.is_empty(), "cube should have triangles");
-
-        // A cube has 8 vertices and 12 triangles (6 faces × 2).
-        // But makepad_csg may produce more depending on tessellation.
-        assert!(mesh.vertices.len() >= 8, "cube should have at least 8 vertices");
-        assert!(mesh.triangles.len() >= 12, "cube should have at least 12 triangles");
-    }
-
-    // -----------------------------------------------------------------
-    // Characterization tests for the two mesh-buffer producers.
-    //
-    // `cad_mesh_data_from_solid` and `part_mesh_buffers` were separate
-    // near-copies. These tests pin the observable differences between
-    // them so the unification behind one function with a normalisation
-    // flag (Phase 4.4) cannot silently change either caller's output.
-    // -----------------------------------------------------------------
-
-    fn unit_cube_solid() -> Solid {
-        let mut alloc = IdAllocator::new();
-        let scene = SceneBuilder::new(&mut alloc)
-            .cube()
-            .size(Vec3f { x: 2.0, y: 4.0, z: 6.0 })
-            .finish()
-            .build();
-        let part =
-            &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)
-                [0];
-        part.build_solid()
-    }
-
-    /// Normalised output is recentred on the origin and scaled so the
-    /// largest dimension is 1.75. Raw output keeps model coordinates.
-    #[test]
-    fn normalised_mesh_is_recentred_and_scaled_raw_is_not() {
-        let solid = unit_cube_solid();
-        let norm = crate::construction_frame::pages::workspace::cad::viewport::cad_mesh_data_from_solid(&solid);
-        let (_, raw_verts) =
-            crate::construction_frame::pages::workspace::cad::viewport::part_mesh_buffers(&solid)
-                .expect("cube produces buffers");
-
-        // Stride is 8 floats: pos.xyzw then normal.xyz + pad.
-        let extent = |v: &[f32], off: usize| {
-            let mut lo = f32::MAX;
-            let mut hi = f32::MIN;
-            for chunk in v.chunks_exact(8) {
-                lo = lo.min(chunk[off]);
-                hi = hi.max(chunk[off]);
-            }
-            (lo, hi)
-        };
-
-        // Largest dimension of the normalised mesh is 1.75 (the Z extent,
-        // since the cube is 2 x 4 x 6).
-        let (nz_lo, nz_hi) = extent(&norm.vertices, 2);
-        assert!(
-            ((nz_hi - nz_lo) - 1.75).abs() < 1e-3,
-            "normalised max dimension should be 1.75, got {}",
-            nz_hi - nz_lo
-        );
-        // ...and it is centred.
-        assert!(
-            (nz_hi + nz_lo).abs() < 1e-3,
-            "normalised mesh should be centred, got lo={nz_lo} hi={nz_hi}"
-        );
-
-        // The raw mesh keeps model units: the Z extent is still 6.
-        let (rz_lo, rz_hi) = extent(&raw_verts, 2);
-        assert!(
-            ((rz_hi - rz_lo) - 6.0).abs() < 1e-3,
-            "raw mesh should keep model units, got {}",
-            rz_hi - rz_lo
-        );
-
-        // Stats are only populated on the normalised path.
-        assert_eq!(norm.stats.vertices, solid.vertex_count());
-        assert_eq!(norm.stats.triangles, solid.triangle_count());
-        assert!(
-            (norm.stats.max_dimension - 6.0).abs() < 1e-6,
-            "stats.max_dimension is the pre-scale extent"
-        );
-    }
-
-    /// Both producers emit the same triangle count and a trivial
-    /// index buffer (0..n), and both flip inward-facing normals out.
-    #[test]
-    fn both_mesh_producers_agree_on_topology_and_outward_normals() {
-        let solid = unit_cube_solid();
-        let norm = crate::construction_frame::pages::workspace::cad::viewport::cad_mesh_data_from_solid(&solid);
-        let (raw_idx, raw_verts) =
-            crate::construction_frame::pages::workspace::cad::viewport::part_mesh_buffers(&solid)
-                .expect("cube produces buffers");
-
-        assert_eq!(norm.indices.len(), raw_idx.len());
-        assert_eq!(norm.vertices.len(), raw_verts.len());
-        let expected: Vec<u32> = (0..raw_idx.len() as u32).collect();
-        assert_eq!(raw_idx, expected, "index buffer is the identity sequence");
-        assert_eq!(norm.indices, expected);
-
-        // Every normal points away from the origin for a centred convex
-        // solid, which is what the dot < 0 branch in both functions is for.
-        for chunk in norm.vertices.chunks_exact(8) {
-            let (px, py, pz) = (chunk[0], chunk[1], chunk[2]);
-            let (nx, ny, nz) = (chunk[4], chunk[5], chunk[6]);
-            let len = (nx * nx + ny * ny + nz * nz).sqrt();
-            assert!((len - 1.0).abs() < 1e-3, "normals are unit length");
-            assert!(
-                px * nx + py * ny + pz * nz > -1e-3,
-                "normal should face outward"
-            );
-        }
-    }
-
-    /// The two producers wind their triangles in opposite orders. The
-    /// normalised path emits p0,p1,p2; the raw path emits p0,p2,p1.
-    /// This is a real difference the merge must preserve per caller.
-    #[test]
-    fn mesh_producers_use_opposite_triangle_winding() {
-        let solid = unit_cube_solid();
-        let norm = crate::construction_frame::pages::workspace::cad::viewport::cad_mesh_data_from_solid(&solid);
-        let (_, raw_verts) =
-            crate::construction_frame::pages::workspace::cad::viewport::part_mesh_buffers(&solid)
-                .expect("cube produces buffers");
-
-        // Compare the first triangle of each, undoing the normalisation
-        // so positions are directly comparable.
-        let scale = 1.75 / norm.stats.max_dimension as f32;
-        let pos = |v: &[f32], vert: usize| {
-            let o = vert * 8;
-            (v[o], v[o + 1], v[o + 2])
-        };
-
-        let n0 = pos(&norm.vertices, 0);
-        let n1 = pos(&norm.vertices, 1);
-        let n2 = pos(&norm.vertices, 2);
-        let r0 = pos(&raw_verts, 0);
-        let r1 = pos(&raw_verts, 1);
-        let r2 = pos(&raw_verts, 2);
-
-        // Vertex 0 matches (both emit p0 first); vertices 1 and 2 are swapped.
-        let close = |a: (f32, f32, f32), b: (f32, f32, f32)| {
-            // The normalised path also applies a 0.0001 positional offset
-            // along the normal, so allow a loose epsilon.
-            (a.0 - b.0 * scale).abs() < 5e-3
-                && (a.1 - b.1 * scale).abs() < 5e-3
-                && (a.2 - b.2 * scale).abs() < 5e-3
-        };
-        // r is not centred, so recentre it the same way before comparing.
-        let cz = 0.0f32; // cube is built centred already for x/y/z
-        let _ = cz;
-        assert!(
-            close(n1, r2) && close(n2, r1),
-            "raw path emits p0,p2,p1 where the normalised path emits p0,p1,p2; \
-             got n1={n1:?} n2={n2:?} r1={r1:?} r2={r2:?}"
-        );
-    }
-
-    /// An empty solid yields `None` from the raw producer and a
-    /// zero-buffer `CadMeshData` (with stats) from the normalised one.
-    #[test]
-    fn empty_solid_returns_none_and_empty_mesh_data() {
-        let empty = Solid::empty();
-        assert!(
-            crate::construction_frame::pages::workspace::cad::viewport::part_mesh_buffers(&empty)
-                .is_none(),
-            "raw producer signals emptiness with None"
-        );
-        let norm = crate::construction_frame::pages::workspace::cad::viewport::cad_mesh_data_from_solid(&empty);
-        assert!(norm.indices.is_empty());
-        assert!(norm.vertices.is_empty());
-        assert_eq!(norm.stats.triangles, 0);
-    }
-
-    /// Test that part_mesh_buffers produces valid (indices, vertices) pairs.
-    #[test]
-    fn part_mesh_buffers_produces_valid_data() {
-        let mut alloc = IdAllocator::new();
-        let scene = SceneBuilder::new(&mut alloc)
-            .cube()
-            .size(Vec3f { x: 1.0, y: 1.0, z: 1.0 })
-            .finish()
-            .build();
-
-        let part = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)[0];
-        let solid = part.build_solid();
-        let mesh = solid.mesh();
-
-        assert!(!mesh.vertices.is_empty());
-        assert!(!mesh.triangles.is_empty());
-
-        // Verify triangle indices are within vertex bounds.
-        for tri in &mesh.triangles {
-            assert!((tri[0] as usize) < mesh.vertices.len());
-            assert!((tri[1] as usize) < mesh.vertices.len());
-            assert!((tri[2] as usize) < mesh.vertices.len());
-        }
-    }
-
-    /// Test that build_solid produces different meshes for different sizes.
-    #[test]
-    fn build_solid_respects_size() {
-        let mut alloc = IdAllocator::new();
-        let scene_small = SceneBuilder::new(&mut alloc)
-            .cube()
-            .size(Vec3f { x: 1.0, y: 1.0, z: 1.0 })
-            .finish()
-            .build();
-
-        let mut alloc2 = IdAllocator::new();
-        let scene_big = SceneBuilder::new(&mut alloc2)
-            .cube()
-            .size(Vec3f { x: 10.0, y: 10.0, z: 10.0 })
-            .finish()
-            .build();
-
-        let part_small = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene_small)[0];
-        let part_big = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene_big)[0];
-
-        let solid_small = part_small.build_solid();
-        let solid_big = part_big.build_solid();
-        let mesh_small = solid_small.mesh();
-        let mesh_big = solid_big.mesh();
-
-        // Both should have the same number of vertices (same shape,
-        // different scale), but different coordinate values.
-        assert_eq!(mesh_small.vertices.len(), mesh_big.vertices.len());
-
-        // The big cube's vertices should have larger coordinates.
-        let small_max = mesh_small.vertices.iter().map(|v| v.x.abs()).fold(0.0f64, f64::max);
-        let big_max = mesh_big.vertices.iter().map(|v| v.x.abs()).fold(0.0f64, f64::max);
-        assert!(big_max > small_max, "big cube should have larger coordinates");
-    }
-
-    /// Test that build_world_solid applies translation.
-    #[test]
-    fn build_world_solid_applies_translation() {
-        let mut alloc = IdAllocator::new();
-        let scene = SceneBuilder::new(&mut alloc)
-            .cube()
-            .size(Vec3f { x: 1.0, y: 1.0, z: 1.0 })
-            .translation(Vec3f { x: 5.0, y: 0.0, z: 0.0 })
-            .finish()
-            .build();
-
-        let part = &crate::construction_frame::pages::workspace::cad::cad_scene::nodes_from_scene(&scene)[0];
-        let local_solid = part.build_solid();
-        let world_solid = part.build_world_solid();
-        let local_mesh = local_solid.mesh();
-        let world_mesh = world_solid.mesh();
-
-        // World mesh vertices should be shifted by +5 in X.
-        let local_avg_x = local_mesh.vertices.iter().map(|v| v.x).sum::<f64>()
-            / local_mesh.vertices.len() as f64;
-        let world_avg_x = world_mesh.vertices.iter().map(|v| v.x).sum::<f64>()
-            / world_mesh.vertices.len() as f64;
-
-        assert!(
-            (world_avg_x - local_avg_x - 5.0).abs() < 0.01,
-            "world mesh should be translated by +5 in X (got diff={})",
-            world_avg_x - local_avg_x
-        );
-    }
-
-    /// Test that CadRenderMode has the expected variants.
-    #[test]
-    fn cad_render_mode_variants() {
-        use crate::construction_frame::pages::workspace::cad::viewport::CadRenderMode;
-        // Just verify the type exists and can be constructed.
-        let mode = CadRenderMode::Realistic;
-        let _ = format!("{:?}", mode);
-    }
-
-    /// `CommandBorrows` can be constructed and its fields read.
-    #[test]
-    fn command_borrows_fields_accessible() {
-        use crate::construction_frame::pages::workspace::cad::viewport::CommandBorrows;
-        use crate::construction_frame::pages::workspace::cad::scene_holder::SceneCache;
-        use crate::construction_frame::pages::workspace::cad::commands::UndoRedoStack;
-
-        let mut parts: Vec<CadNode> = vec![];
-        let scene_cache = SceneCache::new();
-        let mut command_stack = UndoRedoStack::new();
-
-        let holder = CommandBorrows {
-            parts: &mut parts,
-            scene_cache: &scene_cache,
-            command_stack: &mut command_stack,
-        };
-
-        // Verify fields are accessible.
-        assert_eq!(holder.parts.len(), 0);
-        assert!(!holder.command_stack.can_undo());
-    }
-}

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/workspace.rs

@@ -63,86 +63,6 @@ impl CadViewportLayoutMode {
     }
 }
 
-/// Whether an AI response is complete or still arriving.
-///
-/// The two script extractors differed only in how they handle a
-/// response that is not yet whole, so that is the parameter.
-#[derive(Clone, Copy, PartialEq, Eq)]
-enum ResponseState {
-    /// The response is finished. Trim both ends and, if a fence was
-    /// opened but never closed, return the text as-is rather than
-    /// guessing where the body starts.
-    Complete,
-    /// Tokens are still arriving. Preserve trailing whitespace (it is
-    /// the start of the next line), take the body of an unclosed fence,
-    /// and if there is no fence at all, skip leading prose by seeking
-    /// the first CAD-script token.
-    Streaming,
-}
-
-/// Tokens that mark the start of a CAD script in an unfenced streaming
-/// response. Kept in sync with the function list in `system_prompt.md`.
-const SCRIPT_START_TOKENS: [&str; 10] = [
-    "let ",
-    "render(",
-    "empty()",
-    "cube(",
-    "cube_uniform(",
-    "sphere(",
-    "cylinder(",
-    "cone(",
-    "torus(",
-    "tapered_cylinder(",
-];
-
-/// Pull the CAD script out of an AI response.
-///
-/// The system prompt tells the model to emit bare script with no
-/// markdown, but models add fences regardless, so both shapes are
-/// accepted.
-fn extract_script(text: &str, state: ResponseState) -> String {
-    let trimmed = match state {
-        ResponseState::Complete => text.trim(),
-        ResponseState::Streaming => text.trim_start(),
-    };
-
-    if let Some(start) = trimmed.find("```") {
-        let after_open = &trimmed[start + 3..];
-        // Skip the info string on the opening fence line.
-        let code_start = after_open.find('\n').map(|i| i + 1).unwrap_or(0);
-        if let Some(end) = after_open[code_start..].find("```") {
-            let body = &after_open[code_start..code_start + end];
-            return match state {
-                ResponseState::Complete => body.trim().to_string(),
-                ResponseState::Streaming => body.trim_start().to_string(),
-            };
-        }
-        // Fence opened but not yet closed.
-        return match state {
-            ResponseState::Streaming => after_open[code_start..].trim_start().to_string(),
-            // Complete and unclosed: the fence is malformed, so return
-            // everything and let the caller's empty check deal with it.
-            ResponseState::Complete => trimmed.to_string(),
-        };
-    }
-
-    // No fence. A complete response is assumed to be the script.
-    if state == ResponseState::Complete {
-        return trimmed.to_string();
-    }
-
-    // Streaming and unfenced: drop any preamble before the earliest
-    // script token.
-    match SCRIPT_START_TOKENS
-        .iter()
-        .filter_map(|needle| trimmed.find(needle))
-        .min()
-    {
-        Some(idx) => trimmed[idx..].to_string(),
-        None => trimmed.to_string(),
-    }
-}
-
 impl CadWorkspace {
     pub(crate) fn set_rebuild_pending(&mut self, cx: &mut Cx, pending: bool) {
         if self.rebuild_pending == pending {
@@ -339,38 +259,6 @@ impl CadWorkspace {
         }
     }
 
-    /// Apply a parsed value to one field of the selected part, in every
-    /// viewport, and invalidate everything that derives from it.
-    ///
-    /// The nine position/size/rotation inputs in the properties panel
-    /// each had their own copy of this body, differing only in which
-    /// field they assigned. Nine copies is nine chances to forget one of
-    /// the four invalidation steps, and the geometry cache in particular
-    /// fails silently when missed: the part keeps rendering at its old
-    /// shape until something else happens to evict it.
-    ///
-    /// `set` receives the part and the new value.
-    fn apply_field_to_selected_part(
-        &mut self,
-        cx: &mut Cx,
-        value: f32,
-        set: impl Fn(&mut CadNode, f32) + Copy,
-    ) {
-        self.apply_to_all_viewports(cx, |vp, _cx| {
-            if let Some(id) = vp.selection.first().copied() {
-                if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    set(p, value);
-                    // The uploaded GPU geometry, the cached mesh and the
-                    // cached scene snapshot all derive from this part.
-                    vp.part_geoms.remove(&id);
-                    vp.invalidate_node(id);
-                }
-            }
-            vp.mark_scene_dirty();
-            vp.script_dirty = true;
-        });
-    }
-
     pub(crate) fn apply_to_all_viewports(
         &mut self,
         cx: &mut Cx,
@@ -870,7 +758,7 @@ impl CadWorkspace {
     /// Export the current parts list as a vector PDF floor plan.
     /// Added by apply_arch_pdf_patch.py — uses arch_pdf::export_parts_to_pdf.
     fn export_floor_plan_pdf(&mut self, cx: &mut Cx) {
-        // Cached Arc<CadScene> + shared Arc<MeshCache>
+        // v4: get the cached Arc<CadScene> + shared Arc<MeshCache>
         // from the viewport. No parts clone, no scene rebuild, no
         // fresh cache — we reuse what the preview renderer built.
         let (scene, cache, part_count) = {
@@ -922,7 +810,7 @@ impl CadWorkspace {
     /// Open viewer.html in any browser to get interactive 3D
     /// (drag-rotate, scroll-zoom, auto-rotate).
     fn export_3d_viewer(&mut self, cx: &mut Cx) {
-        // Cached Arc<CadScene> + shared Arc<MeshCache>.
+        // v4: get the cached Arc<CadScene> + shared Arc<MeshCache>.
         // The GLB exporter *does* use the mesh cache — this is the
         // main perf win: previously-built preview meshes are reused
         // instead of re-triangulated.
@@ -961,7 +849,7 @@ impl CadWorkspace {
             Ok(()) => {
                 let glb_path = dir.join("model.glb");
                 let html_path = dir.join("viewer.html");
-                // Use the cached scene + shared mesh cache from the
+                // v4: use the cached scene + shared mesh cache from the
                 // viewport. The GLB exporter reuses previously-built
                 // preview meshes — re-exports are near-instant.
                 let exporter = arch_gltf::GltfExporter::new(options);
@@ -1436,11 +1324,51 @@ impl CadWorkspace {
     }
 
     fn extract_cad_script(text: &str) -> String {
-        extract_script(text, ResponseState::Complete)
+        let trimmed = text.trim();
+        if let Some(start) = trimmed.find("```") {
+            let after_open = &trimmed[start + 3..];
+            let code_start = after_open.find('\n').map(|i| i + 1).unwrap_or(0);
+            if let Some(end) = after_open[code_start..].find("```") {
+                return after_open[code_start..code_start + end].trim().to_string();
+            }
+        }
+        trimmed.to_string()
     }
 
     fn extract_streaming_cad_script(text: &str) -> String {
-        extract_script(text, ResponseState::Streaming)
+        let trimmed = text.trim_start();
+        if let Some(start) = trimmed.find("```") {
+            let after_open = &trimmed[start + 3..];
+            let code_start = after_open.find('\n').map(|i| i + 1).unwrap_or(0);
+            if let Some(end) = after_open[code_start..].find("```") {
+                return after_open[code_start..code_start + end]
+                    .trim_start()
+                    .to_string();
+            }
+            return after_open[code_start..].trim_start().to_string();
+        }
+        let mut first = None;
+        for needle in [
+            "let ",
+            "render(",
+            "empty()",
+            "cube(",
+            "cube_uniform(",
+            "sphere(",
+            "cylinder(",
+            "cone(",
+            "torus(",
+            "tapered_cylinder(",
+        ] {
+            if let Some(idx) = trimmed.find(needle) {
+                first = Some(first.map_or(idx, |f: usize| f.min(idx)));
+            }
+        }
+        if let Some(idx) = first {
+            trimmed[idx..].to_string()
+        } else {
+            trimmed.to_string()
+        }
     }
 
     fn stream_ai_response_to_editor(&mut self, cx: &mut Cx) {
@@ -2351,7 +2279,7 @@ impl CadWorkspace {
             }
         }
 
-        // Tool selection buttons.
+        // v3 tool selection buttons
         let tool = if self.view.button(cx, ids!(select_tool_btn)).clicked(actions) {
             Some(CadTool::Select)
         } else if self.view.button(cx, ids!(line_tool_btn)).clicked(actions) {
@@ -2482,7 +2410,7 @@ impl CadWorkspace {
                 vp.redo(cx);
             });
         }
-        // Update undo/redo button labels with command descriptions.
+        // v14: Update undo/redo button labels with command descriptions.
         // Shows "Undo Move part" instead of just "Undo", so the user
         // knows what will be undone before clicking.
         {
@@ -2983,10 +2911,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.pos();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.x = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_pos(t);
+                            p.pos().x = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
             let val = self
@@ -3001,10 +2936,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.pos();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.y = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_pos(t);
+                            p.pos().y = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
             let val = self
@@ -3019,10 +2961,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.pos();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.z = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_pos(t);
+                            p.pos().z = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
 
@@ -3039,10 +2988,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.size();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.x = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_size(t);
+                            p.size().x = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
             let val = self
@@ -3057,10 +3013,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.size();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.y = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_size(t);
+                            p.size().y = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
             let val = self
@@ -3075,10 +3038,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.size();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.z = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_size(t);
+                            p.size().z = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
 
@@ -3095,10 +3065,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.rot();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.x = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_rot(t);
+                            p.rot().x = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
             let val = self
@@ -3113,10 +3090,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.rot();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.y = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_rot(t);
+                            p.rot().y = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
             let val = self
@@ -3131,10 +3115,17 @@ impl CadWorkspace {
                         .and_then(|(t, _)| t.parse::<f32>().ok())
                 });
             if let Some(val) = val {
-                self.apply_field_to_selected_part(cx, val, |p, v| {
+                self.apply_to_all_viewports(cx, |vp, _cx| {
-                    let mut t = p.rot();
+                    if let Some(id) = vp.selection.first().copied() {
-                    t.z = v;
+                        if let Some(p) = vp.parts.get_mut_by_raw_id(id) {
-                    p.set_rot(t);
+                            p.rot().z = val;
+                            vp.part_geoms.remove(&id);
+                            // v4: parameter edit — invalidate caches.
+                            vp.invalidate_node(id);
+                        }
+                    }
+                    vp.mark_scene_dirty();
+                    vp.script_dirty = true;
                 });
             }
 
@@ -3508,190 +3499,6 @@ pub(crate) fn read_attachment_as_base64(path: &std::path::Path) -> Result<String
     Ok(format!("{};{}", kind.mime(), b64))
 }
 
-#[cfg(test)]
-mod properties_panel_setter_tests {
-    use super::*;
-    use crate::construction_frame::pages::workspace::cad::cad_scene::{
-        CadNode, CadSolid, CadTransform, LayerId, MaterialId, NodeId, NodeMetadata,
-    };
-    use makepad_widgets::{Vec3f, Vec4f};
-
-    fn cube() -> CadNode {
-        CadNode {
-            id: NodeId(1),
-            name: "n".into(),
-            solid: Some(CadSolid::Box { size: Vec3f { x: 1.0, y: 1.0, z: 1.0 } }),
-            transform: CadTransform::IDENTITY,
-            material: MaterialId::ROOT,
-            layer: LayerId::ROOT,
-            parent: None,
-            metadata: NodeMetadata::default(),
-            color: Vec4f { x: 1.0, y: 1.0, z: 1.0, w: 1.0 },
-            kind_hint: None,
-        }
-    }
-
-    /// The nine properties-panel inputs pass a setter closure to
-    /// `apply_field_to_selected_part`. Those closures are the whole
-    /// behaviour of the panel, and every one of them used to be a
-    /// silent no-op (`p.pos().x = v` writes to a temporary — see
-    /// `cad_scene::size_tests::setters_write_through_but_getters_are_copies`).
-    ///
-    /// This applies each of the nine closures exactly as the call sites
-    /// do and asserts it reaches the node, on the correct axis, without
-    /// disturbing the other two.
-    #[test]
-    fn each_properties_panel_setter_writes_its_own_axis() {
-        let setters: [(&str, fn(&mut CadNode, f32)); 9] = [
-            ("pos.x", |p, v| { let mut t = p.pos(); t.x = v; p.set_pos(t); }),
-            ("pos.y", |p, v| { let mut t = p.pos(); t.y = v; p.set_pos(t); }),
-            ("pos.z", |p, v| { let mut t = p.pos(); t.z = v; p.set_pos(t); }),
-            ("size.x", |p, v| { let mut t = p.size(); t.x = v; p.set_size(t); }),
-            ("size.y", |p, v| { let mut t = p.size(); t.y = v; p.set_size(t); }),
-            ("size.z", |p, v| { let mut t = p.size(); t.z = v; p.set_size(t); }),
-            ("rot.x", |p, v| { let mut t = p.rot(); t.x = v; p.set_rot(t); }),
-            ("rot.y", |p, v| { let mut t = p.rot(); t.y = v; p.set_rot(t); }),
-            ("rot.z", |p, v| { let mut t = p.rot(); t.z = v; p.set_rot(t); }),
-        ];
-
-        for (name, set) in setters {
-            let mut node = cube();
-            let before = (node.pos(), node.size(), node.rot());
-            set(&mut node, 3.5);
-            let after = (node.pos(), node.size(), node.rot());
-
-            let (group, axis) = name.split_once('.').expect("name is group.axis");
-            let read = |v: (Vec3f, Vec3f, Vec3f)| match group {
-                "pos" => v.0,
-                "size" => v.1,
-                _ => v.2,
-            };
-            let component = |v: Vec3f| match axis {
-                "x" => v.x,
-                "y" => v.y,
-                _ => v.z,
-            };
-
-            assert!(
-                (component(read(after)) - 3.5).abs() < 1e-4,
-                "{name}: setter did not reach the node (got {})",
-                component(read(after))
-            );
-
-            // The other two groups must be untouched.
-            for other in ["pos", "size", "rot"] {
-                if other == group {
-                    continue;
-                }
-                let pick = |v: (Vec3f, Vec3f, Vec3f)| match other {
-                    "pos" => v.0,
-                    "size" => v.1,
-                    _ => v.2,
-                };
-                let (b, a) = (pick(before), pick(after));
-                assert!(
-                    (b.x - a.x).abs() < 1e-4
-                        && (b.y - a.y).abs() < 1e-4
-                        && (b.z - a.z).abs() < 1e-4,
-                    "{name}: setter also changed {other}"
-                );
-            }
-        }
-    }
-}
-
-#[cfg(test)]
-mod script_extraction_tests {
-    use super::*;
-
-    // These pin the behaviour of the two AI-response extractors before
-    // they were unified behind one function (Phase 4.4). The system
-    // prompt tells the model to emit no code fences, but models do it
-    // anyway, so both fenced and bare input must work.
-
-    #[test]
-    fn final_extract_unwraps_a_closed_fence() {
-        let text = "Here you go:\n```cad\nlet a = cube(1.0, 1.0, 1.0, true)\nrender(a)\n```\nEnjoy!";
-        assert_eq!(
-            CadWorkspace::extract_cad_script(text),
-            "let a = cube(1.0, 1.0, 1.0, true)\nrender(a)"
-        );
-    }
-
-    #[test]
-    fn final_extract_falls_back_to_whole_text_on_unclosed_fence() {
-        // A truncated response has no closing fence. The final extractor
-        // deliberately returns everything rather than guessing.
-        let text = "```\nlet a = cube(1.0, 1.0, 1.0, true)";
-        assert_eq!(
-            CadWorkspace::extract_cad_script(text),
-            "```\nlet a = cube(1.0, 1.0, 1.0, true)"
-        );
-    }
-
-    #[test]
-    fn final_extract_passes_through_unfenced_script() {
-        let text = "  let a = cube(1.0, 1.0, 1.0, true)\nrender(a)  ";
-        assert_eq!(
-            CadWorkspace::extract_cad_script(text),
-            "let a = cube(1.0, 1.0, 1.0, true)\nrender(a)"
-        );
-    }
-
-    #[test]
-    fn streaming_extract_returns_partial_body_of_an_unclosed_fence() {
-        // This is the difference that matters: mid-stream there is no
-        // closing fence yet, and the editor still needs the body.
-        let text = "```cad\nlet a = cube(1.0, 1.0";
-        assert_eq!(CadWorkspace::extract_streaming_cad_script(text), "let a = cube(1.0, 1.0");
-    }
-
-    #[test]
-    fn streaming_extract_skips_unfenced_prose_to_the_first_script_token() {
-        let text = "Sure! I'll build a box for you.\nlet a = cube(1.0, 1.0, 1.0, true)";
-        assert_eq!(
-            CadWorkspace::extract_streaming_cad_script(text),
-            "let a = cube(1.0, 1.0, 1.0, true)"
-        );
-    }
-
-    #[test]
-    fn streaming_extract_picks_the_earliest_script_token_not_the_first_listed() {
-        // `render(` appears before `cube(` in the text but later in the
-        // needle list; the earliest position must win.
-        let text = "blah render(x) and cube(";
-        assert_eq!(
-            CadWorkspace::extract_streaming_cad_script(text),
-            "render(x) and cube("
-        );
-    }
-
-    #[test]
-    fn streaming_extract_keeps_trailing_whitespace_but_final_extract_trims_it() {
-        // Mid-stream a trailing newline is the start of the next line,
-        // so the streaming path preserves it; the final path does not.
-        let text = "let a = cube(1.0, 1.0, 1.0, true)\n";
-        assert_eq!(
-            CadWorkspace::extract_streaming_cad_script(text),
-            "let a = cube(1.0, 1.0, 1.0, true)\n"
-        );
-        assert_eq!(
-            CadWorkspace::extract_cad_script(text),
-            "let a = cube(1.0, 1.0, 1.0, true)"
-        );
-    }
-
-    #[test]
-    fn both_extractors_return_empty_for_pure_prose() {
-        // No fence, no script token: streaming has nothing to show. The
-        // final extractor returns the prose so the caller's
-        // "AI returned an empty CAD script" check can catch it.
-        let text = "I'm sorry, I can't help with that.";
-        assert_eq!(CadWorkspace::extract_streaming_cad_script(text), text);
-        assert_eq!(CadWorkspace::extract_cad_script(text), text);
-    }
-}
-
 #[cfg(test)]
 mod system_prompt_tests {
     use super::*;

crates/apps/spreadsheet/spreadsheet-ui/src/workspace.rs

@@ -181,13 +181,7 @@ impl SpreadsheetWorkspace {
     }
 
     fn exchange_grid_with_model(&mut self, cx: &mut Cx, index: usize) -> bool {
-        // The WidgetRef must be bound to a local. In a `let ... else`
+        let Some(mut grid) = self.view.widget(cx, ids!(grid)).borrow_mut::<SpreadsheetGrid>() else {
-        // the scrutinee's temporaries are dropped at the end of the
-        // statement (unlike `if let`, where they live to the end of the
-        // block), so borrowing directly from the returned temporary
-        // leaves `grid` dangling.
-        let grid_ref = self.view.widget(cx, ids!(grid));
-        let Some(mut grid) = grid_ref.borrow_mut::<SpreadsheetGrid>() else {
             return false;
         };
         let mut model = WorkspaceModel::from_parts(