Add geometry-loop + Move advisories to validation

addin/tools/validate_design.py

@@ -335,6 +335,37 @@ def _check_replication(design, bodies, exclude_prefixes, min_group=2):
     return {"advisory": True, "groups": findings}
 
 
+def _check_moves(design):
+    """ADVISORY (never affects pass/fail): Move/rotate features in the timeline.
+
+    A Move that *orients a part to an angled reference* (the common case — splay,
+    tilt) bakes a non-parametric transform and can mask sizing bugs that only show
+    once the part is in place. Prefer building the part in position: sketch its
+    outline against the reference, or — for a compound angle — loft between two
+    offset cross-sections (parametric, no Move). Legit uses remain (a cosmetic
+    roll about a part's own axis, aligning to a measured orientation); suppress
+    those by putting `_norep` in the Move feature's name.
+    """
+    moves = []
+    try:
+        tl = design.timeline
+        for i in range(tl.count):
+            try:
+                feat = tl.item(i).entity
+                ot = getattr(feat, "objectType", "") or ""
+                if "MoveFeature" not in ot:
+                    continue
+                nm = getattr(feat, "name", "") or ""
+                if "_norep" in nm:
+                    continue
+                moves.append(nm or "(unnamed Move)")
+            except Exception:
+                pass
+    except Exception:
+        pass
+    return {"advisory": True, "count": len(moves), "names": moves}
+
+
 def handler(exclude_prefixes: list = None, min_contact_cm2: float = None) -> dict:
     """Run all structural validation checks on the current design."""
 
@@ -389,6 +420,15 @@ def handler(exclude_prefixes: list = None, min_contact_cm2: float = None) -> dic
         except Exception as re:
             replication = {"advisory": True, "error": str(re), "groups": []}
 
+        # Move advisory — Move/rotate features that likely orient a part to an
+        # angled reference. ADVISORY ONLY: never changes `passed`. Prefer building
+        # in place (or loft for compound angles); suppress legit rolls with _norep.
+        moves = None
+        try:
+            moves = _check_moves(design)
+        except Exception as me:
+            moves = {"advisory": True, "error": str(me), "names": []}
+
         import json
         result = {
             "passed": passed,
@@ -399,6 +439,8 @@ def handler(exclude_prefixes: list = None, min_contact_cm2: float = None) -> dic
             result["deps"] = deps_result
         if replication is not None and replication.get("groups"):
             result["replication"] = replication
+        if moves is not None and moves.get("names"):
+            result["moves"] = moves
 
         parts = []
         if connectivity["connected"] and not connectivity["weakConnections"]:
@@ -427,6 +469,13 @@ def handler(exclude_prefixes: list = None, min_contact_cm2: float = None) -> dic
                 f"replication ADVISORY ({len(g)} group(s) built independently — "
                 f"{ex}) [does not affect pass/fail]")
 
+        if moves is not None and moves.get("names"):
+            parts.append(
+                f"move ADVISORY ({moves['count']} Move feature(s): "
+                f"{', '.join(moves['names'][:4])} — if orienting to an angled "
+                f"reference, prefer build-in-place/loft; name a legit roll "
+                f"`_norep`) [does not affect pass/fail]")
+
         status = "PASSED" if passed else "FAILED"
         msg = f"{status}: {', '.join(parts)}"
 
@@ -470,9 +519,20 @@ def handler(exclude_prefixes: list = None, min_contact_cm2: float = None) -> dic
    a redo; the recommended response is to refactor the named group to one
    template + Mirror/Pattern (a sub-agent can do this so the main build's context
    stays lean).
+5. **Move advisory** (ADVISORY — never affects pass/fail): Move/rotate features
+   in the timeline. A Move that orients a part to an angled reference bakes a
+   non-parametric transform and can hide sizing bugs until the part is in place —
+   prefer building the part in position (sketch its outline against the reference;
+   for a compound angle, loft between two offset cross-sections). Legit uses
+   (a cosmetic roll about a part's own axis, aligning to a measured face) are
+   suppressed by putting `_norep` in the Move feature's name.
+
+Note: the geometry-loop advisory (a Python loop that creates geometry instead of
+a Rectangular Pattern/Mirror feature) is emitted by the dependency-tree check and
+appears in the deps output.
 
 Returns a single pass/fail result. Fails if disconnected, has weak
-connections, or has interference (NOT for the replication advisory).
+connections, or has interference (NOT for the replication/move/loop advisories).
 Run after EVERY phase."""
 
 tool = Tool.create_simple(

docs/angled-construction.md

@@ -36,7 +36,21 @@ Build compound splay in two steps:
 
 2. **Secondary splay (along width)** — Apply a Move feature that rotates the leg body around the X axis by `splay_w`. This avoids compound-angle sketch math and keeps each splay axis independent.
 
-**Why not a single compound-angle sketch?** The sketch would need trigonometric expressions for foreshortened dimensions, and the extrude direction wouldn't align with any principal axis. Two-step splay is simpler, more readable, and each angle is independently adjustable.
+**Why not a single compound-angle sketch?** A *single* compound-angle sketch would need trigonometric expressions for foreshortened dimensions, and the extrude direction wouldn't align with any principal axis.
+
+### Parametric Alternative: Loft Between Two Sections (no Move)
+
+The trapezoid + Move above bakes the second angle as a non-parametric transform. To keep compound splay **fully parametric and Move-free**, use **two sketches + a Loft** instead:
+
+1. Sketch the leg's cross-section (a plain, true-size rectangle) on an XY plane at the **top** (seat) height.
+2. Sketch the same rectangle on an XY plane at the **bottom** (floor), its center offset by `splay_shift` in X **and** `splay_shift_w` in Y.
+3. **Loft** between the two profiles.
+
+Both sections are axis-aligned rectangles at true size — **no foreshortening trig** — and the loft produces the doubly-leaning leg (an oblique prism with planar faces and horizontal top/bottom). The offsets are ordinary `tan()`-based parameter expressions, so the leg recomputes when a splay angle changes.
+
+**Why a Loft and not an Extrude?** Fusion's Extrude only goes *normal to the profile plane* (the taper angle adds draft, it does not redirect the extrusion axis) — there is no off-axis extrude. Two separated profiles can therefore only be bridged by a Loft (profile→profile) or a Sweep along an angled path. A Loft between world-aligned rectangles also gives the geometry you actually want: **horizontal top/bottom faces** (flat on the floor and into the seat) with the sides leaning — whereas tilting a sketch plane for a normal extrude would yield faces *perpendicular to the leg axis* (angled cuts, wrong for floor/seat contact).
+
+Note that **single-axis splay needs neither** — the trapezoid/parallelogram sketch (lean *in* the sketch) extruded normally in the perpendicular direction already captures it. Reach for Loft (or, as the legacy option, the Move below) only for the *second* axis. `validate_design` emits a (non-failing) **move advisory** when a Move is present, to nudge toward this in-place alternative; if a Move is genuinely the right tool (a cosmetic roll about a part's own axis, or aligning to a measured orientation), name the feature with a `_norep` substring to suppress it.
 
 ### Trapezoid Sketch (Primary Splay)
 
@@ -464,7 +478,7 @@ matrix = [
 
 The Move feature's matrix is baked at script time — it doesn't update when parameters change. If `splay_w` changes in Change Parameters, the Move angle stays the same.
 
-**Mitigation:** For furniture models, splay angles are rarely adjusted after the initial design. If parametric splay is required, use a component-level rotation (via occurrence transform) instead of a Move feature, but this adds complexity to cross-component CUT operations.
+**Mitigation:** For furniture models, splay angles are rarely adjusted after the initial design. If parametric splay IS required, prefer building the part in place — for compound splay, the **two-section Loft** above (parametric, no Move); see "Parametric Alternative: Loft Between Two Sections." (A component-level rotation via occurrence transform also stays parametric but complicates cross-component CUTs.) A Move also *hides* sizing bugs until the part lands in position, so after any Move, run `check_interference` and sanity-check the part's dimensions in its final spot.
 
 ### Re-Find Bodies After Move
 

helpers/sp/deps.py

@@ -218,6 +218,68 @@ def _check_sketch_anchoring(comp, comp_name, is_root_comp, issues):
         # else: only spline interiors are free → legitimate sculpted profile.
 
 
+def _geometry_loop_findings(script_path):
+    """ADVISORY: scan the tracked build script for Python ``for``/``while`` loops
+    that CREATE geometry (sketches, extrudes, combines, cyl_/box_ helpers) — the
+    non-parametric replication anti-pattern. The right way is a Rectangular
+    Pattern or Mirror feature (one parametric feature that recomputes when the
+    count changes), not N independent features from a loop.
+
+    This is the script-source complement to validate_design's body-congruence
+    replication advisory: that one only sees congruent BODIES, so a loop of
+    identical CUTS (dog holes, mortise/hole arrays — which remove voids, not add
+    bodies) is invisible to it. The AST sees both.
+
+    Returns a list of {line, kind, calls}. Best-effort; never raises. A loop that
+    builds a list and then patterns/mirrors it (calls a *pattern*/*mirror* helper)
+    is NOT flagged.
+    """
+    import ast
+    import os
+    try:
+        if not script_path or not os.path.exists(script_path):
+            return []
+        with open(script_path, "r") as f:
+            tree = ast.parse(f.read())
+    except Exception:
+        return []
+
+    # call-name fragments (lower-cased, dotted) that create per-instance geometry
+    GEO = ("sketches.add", "extrudefeatures", "revolvefeatures", "loftfeatures",
+           "sweepfeatures", "ext_new", "ext_op", "ext_new_sym", "combine",
+           "cyl_", "box_", "addbytwopoints", "addbycenterradius", "sketchcircles",
+           "sketchlines", "sketcharcs")
+    # if the loop itself patterns/mirrors, it's already the right approach
+    OK = ("pattern", "mirror")
+
+    def _dotted(fn):
+        if isinstance(fn, ast.Attribute):
+            base = _dotted(fn.value)
+            return (base + "." + fn.attr) if base else fn.attr
+        if isinstance(fn, ast.Name):
+            return fn.id
+        return None
+
+    findings = []
+    for node in ast.walk(tree):
+        if not isinstance(node, (ast.For, ast.While)):
+            continue
+        calls = []
+        for n in ast.walk(node):
+            if isinstance(n, ast.Call):
+                d = _dotted(n.func)
+                if d:
+                    calls.append(d.lower())
+        if any(o in c for c in calls for o in OK):
+            continue
+        hits = sorted({g for c in calls for g in GEO if g in c})
+        if hits:
+            findings.append({"line": node.lineno,
+                             "kind": "for" if isinstance(node, ast.For) else "while",
+                             "calls": hits})
+    return findings
+
+
 def validate_deps(ctx, metadata_path=None):
     """Validate dependency tree from model.json.
 
@@ -235,13 +297,25 @@ def validate_deps(ctx, metadata_path=None):
     import os
     import re
 
+    # Resolve the tracked build script up-front — used both to locate model.json
+    # (when no metadata_path was passed) AND for the geometry-loop advisory below,
+    # which must run even when an explicit metadata_path was given.
+    script_path = None
+    try:
+        from server.document_tracker import DocumentTracker
+        script_path = DocumentTracker._script_path
+    except Exception:
+        pass
+
+    # Geometry-loop advisory (NOTE — never affects pass/fail): a Python loop that
+    # creates geometry should be a Rectangular Pattern or Mirror feature instead.
+    for fnd in _geometry_loop_findings(script_path):
+        print(f"  NOTE  geometry built in a `{fnd['kind']}` loop at line "
+              f"{fnd['line']} ({', '.join(fnd['calls'])}) — use a Rectangular "
+              f"Pattern or Mirror feature so the count stays parametric, not a "
+              f"Python loop. (advisory; does not affect pass/fail)")
+
     if metadata_path is None:
-        script_path = None
-        try:
-            from server.document_tracker import DocumentTracker
-            script_path = DocumentTracker._script_path
-        except Exception:
-            pass
         if script_path:
             script_dir = os.path.dirname(script_path)
             stem = os.path.splitext(os.path.basename(script_path))[0]

tests/test_deps_loops.py

@@ -0,0 +1,85 @@
+"""Offline test of deps.py geometry-loop advisory. Pure Python — no Fusion.
+
+Run: python3 tests/test_deps_loops.py   (also pytest-discoverable)
+
+Exercises _geometry_loop_findings: a Python for/while loop that CREATES geometry
+should be flagged (use a Rectangular Pattern / Mirror instead); a loop that
+patterns/mirrors, or does no geometry, should not be.
+"""
+import os, sys, types, tempfile, importlib.util
+
+for m in ("adsk", "adsk.core", "adsk.fusion"):
+    sys.modules[m] = types.ModuleType(m)
+
+_DEPS = os.path.normpath(os.path.join(
+    os.path.dirname(os.path.abspath(__file__)), "..", "helpers", "sp", "deps.py"))
+spec = importlib.util.spec_from_file_location("deps", _DEPS)
+deps = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(deps)
+
+
+def _find(src):
+    with tempfile.NamedTemporaryFile("w", suffix=".py", delete=False) as f:
+        f.write(src); path = f.name
+    try:
+        return deps._geometry_loop_findings(path)
+    finally:
+        os.unlink(path)
+
+
+CASES = [
+    # (name, source, expect_flagged)
+    ("for-loop of CUTs (dog holes)", """
+def run(ctx):
+    for j in range(6):
+        sp.ext_op(top_c, prof, "t", CUT, top_front, "Dog_%d" % j, flip=False)
+""", True),
+    ("while-loop creating sketches", """
+def run(ctx):
+    j = 0
+    while j < n:
+        sk = top_c.sketches.add(pl)
+        j += 1
+""", True),
+    ("for-loop of cyl_ helper", """
+def run(ctx):
+    for k in range(4):
+        cyl_y(cx, z, ya, dia, ylen, "Hole_%d" % k)
+""", True),
+    ("loop that patterns — NOT flagged", """
+def run(ctx):
+    bodies = []
+    for k in range(n):
+        bodies.append(make_template(k))
+    sp.body_pattern(comp, seed, axis, "n", "sp", "Pat")
+""", False),
+    ("loop with no geometry — NOT flagged", """
+def run(ctx):
+    for b in comp.bRepBodies:
+        b.name = classify(b)
+""", False),
+    ("syntax error / missing — no crash", "def run(ctx): this is not python", False),
+]
+
+
+def test_geometry_loop_findings():
+    ok = True
+    for name, src, expect in CASES:
+        flagged = len(_find(src)) > 0
+        passed = (flagged == expect)
+        ok = ok and passed
+        print(f"  {'PASS' if passed else 'FAIL'}  {name} "
+              f"(flagged={flagged}, expect={expect})")
+    assert deps._geometry_loop_findings(None) == []      # None path → no crash
+    assert deps._geometry_loop_findings("/no/such.py") == []
+    assert ok, "geometry-loop detector cases failed"
+
+
+if __name__ == "__main__":
+    try:
+        test_geometry_loop_findings()
+        print("\nall geometry-loop cases passed")
+        sys.exit(0)
+    except AssertionError as e:
+        print(f"\nFAILED: {e}")
+        sys.exit(1)