Feature/square hex and oval cutouts

[layersafe]

I unleashed my free Fable 5 credits onto the codbase and it threw up quite a few suggestions.
Ive since spent the rest of the day going through and getting it to implement the suggestions.

I started with establishing tests for current code generation to ensure there is no loss in the STLS as it rewrote a large portion of the codebase.

I appreciate that this is your codebase and this is a large sweeping changes so feel free to ignore and or reject the changes, but ive sp far test printed some hex trays and it works very well.

I have Oval Tray lined up next.

Tray Generator — Architecture & Code Review

A. Architecture changes needed before adding Hex/Oval (the main ask)

A1. Introduce a CutoutShape abstraction (strategy + registry)

Today shape support is a string scattered across three files: the CLI choices=["circle", "square"] (tray_generator.py:146), the dispatch cutout_fn = generate_square_cutout if cutout_shape == 'square' else generate_cutout (full_tray_generator.py:169), and the layout override cutout_shape == 'square' (full_tray_generator.py:161). Adding hex/oval means touching all three plus each layout file.

Instead define one class/protocol per shape:

class CutoutShape(Protocol):
    name: str                                  # registry key, CLI choice
    supports_alternating: bool                 # square/hex: False (or shape-aware math)
    def footprint(self, size, tolerance) -> (width_x, depth_y)   # bounding extent for layout
    def min_center_distance(self, other, gap) -> float           # replaces circle tangency math
    def build(self, size, params) -> Compound                    # the 3D cutout solid

SHAPES = {"circle": CircleShape(), "square": SquareShape(), ...}

The CLI derives choices from SHAPES.keys(), the orchestrator calls shape.build(...), and layout code consumes footprint() — nobody string-compares shape names anymore.

A2. Stop calling everything diameter

Layout position dicts carry 'diameter' (calculate_linear_cutout_positions.py:132, alternating too), and the CLI positional arg is diameters. For a square it's a side; for a hex it's ambiguous (across-flats vs across-corners — these differ by ~15%, which matters at 0.1mm fit tolerance); an oval needs two numbers.

Rename to size/sizes internally, and make layout consume footprint_x / footprint_y per item rather than a single scalar. This is the single most invasive rename, so do it before adding shapes, not after.

A3. Decide the CLI input format for two-dimensional shapes (oval) and hex orientation

The current nargs="+", type=float positional list can't express an oval.

Recommendation:

• Keep floats working for symmetric shapes, and accept WxD tokens (e.g. 40x60) parsed by a custom type= function
• Add --hex-orientation {flat,point} or fold orientation into the shape name (hex-flat)
• Also decide now whether shape is global-per-tray (current) or per-cutout (e.g. circle:40 square:25) — the registry design in A1 supports either, but the CLI grammar should be chosen once

A4. Make layout shape-aware instead of hard-excluding shapes

The alternating layout's nesting math is pure circle tangency (_side_from_hyp, radius_i + radius_next in Trays/functions/calculate_cutout_positions/calculate_alternating_cutout_positions.py:129). Squares are currently forced to linear (full_tray_generator.py:158-161).

With A1's min_center_distance(other, gap), alternating layout works for any shape that implements it (circle: sum of radii; square/hex: conservative circumscribed-circle radius, or exact support-function math later). Minimum viable: keep the linear-only restriction but drive it from shape.supports_alternating instead of a string check.

A5. Unify the two cutout generators' shared structure

generate_cutout and generate_square_cutout duplicate the pattern "tapered extrusion + slide-path extrusion toward the flap + flattener intersection", but with different magic numbers: circle uses extrude(amount=6, taper=12.5) (cutout_generator.py:25), square uses amount=5, taper=5 (cutout_generator.py:134).

If the taper difference isn't a deliberate design decision, extract shared constants; either way, factor the common "profile sketch → taper → slide path → flatten" pipeline so a new shape only supplies the 2D profile plus its lip/edge treatment.

Note also generate_square_cutout accepts lip_offset and floor_thickness but ignores both silently — with A1, shapes that don't support a feature should say so (warning or error), because a user passing --edge-offsets with squares currently gets no fit change and no explanation.

A6. Replace the 24-positional-argument call chain with a config object

tray_generator.py:211-240 passes ~24 positional args to generate_full_tray, which forwards ~19 positionally to generate_base_tray.

This has already produced a latent unit bug: the value flows from hinge_pin_radius (tray_generator) into a parameter named hinge_pin_diameter (base_tray_generator.py:17), which is then used as a radius in Cylinder(hinge_pin_diameter + ...).

One @dataclass TrayConfig (with the defaults living in exactly one place) eliminates the whole class of error and makes adding shape params cheap.

Related: defaults currently diverge between layers — hinge_lock_radius is 3.5 in tray_generator but 2 in the two function signatures; square tolerance default 0.6 vs circle 0.55; floor_thickness README says 0.4, code says 0.8.

A7. Fix parameter repurposing in the cutout call

full_tray_generator.py:171-182 passes floor_thickness=edge_adjusts[i] and cutout_edge_spacing=safety_margin[1] — the names on the receiving side don't mean what they say, and the actual floor_thickness is used two lines later for the Z translate.

Also generate_full_tray's own cutout_edge_spacing=.4 parameter is dead (always overridden). Rename the cutout parameters to what they really are (edge_adjust, edge_margin_y, …) as part of A1's build() signature.

A8. Fix or remove the base-tray cache

base_tray_storage (full_tray_generator.py:18) is keyed only on ((width, depth), is_double_tray) — any other geometry change (rail height, flap depth, hinge params) silently returns a stale tray if the module is reused in-process (batch generation, notebook, future GUI).

Either key on all inputs (easy once A6's config is a frozen dataclass) or drop the cache; a CLI run builds the base tray once anyway.

---

B. Correctness bugs (independent of the refactor)

• B1. Alternating validator's boundary check is dead code. In calculate_alternating_cutout_positions.py:69-80, an out-of-bounds position just breaks the loop without setting has_error — boundary violations are silently accepted, and cutouts can be placed intersecting the rails/flap edges.

• B2. Overlap check only tests consecutive pairs (lines 84-94). In alternating layouts, positions i and i+2 sit on the same side and can overlap without i/i+1 doing so. Check same-side neighbors too (or all pairs; n is tiny).

• B3. Edge-offset sign inconsistency in alternating layout. First position: y = min + d/2 - edge_offsets[0] (line 42); subsequent non-flipped positions: y = min + d/2 + edge_offsets[i] (line 54). Same side, opposite sign — one is wrong. (Linear layout uses + on the near line, - on the far line, i.e. "inward"; match that convention.)

• B4. Single-diameter alternating path works by accident and its demo crashes. Line 19 uses loop leftover diameter (happens to equal diameters[0]) and edge_offsets[0] (IndexErrors if unpadded). The file's own __main__ block calls the function with two args missing → TypeError.

• B5. {RESET} printed literally in error messages. In tray_generator.py:271-293, the multi-line implicit string concatenations only have f on the first fragment, so {RESET} is emitted as literal text and the terminal stays red. Also: the handler print-then-raises, so exit(1) is unreachable for those branches (user gets a duplicate traceback), and matching exceptions by substring ("math domain error", "keyerror: 'flipped'") is brittle — raise typed exceptions (LayoutError, etc.) at the source instead. Note _side_from_hyp already raises a friendly ValueError, so the "math domain error" branch may be mostly stale.

• B6. Output directory depends on CWD. tray_generator.py:250 does os.makedirs("output") — running from the repo root writes to ./output (that stray directory already exists in the repo), contradicting the README's Trays/output/. Anchor to the script: Path(__file__).parent / "output".

• B7. Bare except: around show() (tray_generator.py:244-247) swallows KeyboardInterrupt/SystemExit. Use except Exception: — or better, only call show() when a viewer is actually requested (see C2).

• B8. Linear-layout edge-offset guard checks the wrong length. calculate_linear_cutout_positions.py:85-86 guards i < len(edge_offsets) but indexes edge_offsets[line_one_indices[i]] — safe only because the caller pre-pads; a direct call with a short list IndexErrors. Guard on line_one_indices[i] < len(edge_offsets) or make padding the callee's job.

• B9. Tolerance is inconsistently applied between the two layouts. Linear adds tolerance between items; alternating computes full_diameters and then never uses it — nesting math runs on raw diameters, so alternating trays get ~0.55mm less clearance than linear ones. Decide once (probably: layout always works on toleranced footprints) and delete the dead full_diameters loops in both files.

---

C. Code quality / hygiene

• C1. Dead code and unused imports. math, copy unused in tray_generator.py:4-5; math unused in linear positions; unused e = and normal = copy.deepcopy(...) in cutout_generator.py:30-33; big commented-out show(...) blocks; leftover debug prints print("usable_area", ...) in full_tray_generator.py:153-157 (route through logging or a --verbose flag).

• C2. from ocp_vscode import * at module import time everywhere. Every module (including pure-math ones' siblings) hard-requires a VS Code viewer package just to run the CLI. Import it lazily inside the __main__/preview paths only. Same for from build123d import * star-imports — switch to explicit imports (Circle, BuildPart, …); star-imports from two libraries into the same namespace is how shadowing bugs happen.

• C3. Package structure / dual-import hack. The if __name__ == "__main__": absolute-vs-relative import blocks (full_tray_generator.py:7-16) should go away: add a pyproject.toml, make Trays (or a renamed layersafe) a proper package, run modules via python -m. Also add requirements.txt/dependencies (build123d, ocp-vscode) — currently only the README mentions them.

• C4. Typos and naming. base_heigth → base_height (everywhere); hinge_pin_radius/hinge_pin_diameter mismatch (see A6); calculate_line_positions's results are called x_positions/y_positions in the caller but both are rows (front/back), not axes — rename front_row/back_row.

• C5. Mutable default arguments. diameters=[], edge_offsets=[], edge_adjusts=[] in generate_full_tray (full_tray_generator.py:75-95). Use None + normalize. Currently benign, classic footgun.

• C6. Magic numbers in geometry. extrude(amount=6, taper=12.5), Cylinder(base_diameter, 6), 2 - floor_thickness, boxes of height 5/8, 0.4 - epsilon chamfers, hinge_top_offset = floor_thickness - 0.4. Name them as module constants or config fields with a comment on what each controls physically — essential once four shapes must produce matching wall/lip behavior.

• C7. No type hints, no docstrings on the core API. The layout functions and generate_full_tray take/return dicts with implicit keys ('x', 'y', 'diameter', 'flipped'). At minimum add a Position dataclass/TypedDict — this pairs naturally with A2.

• C8. The two chamfer try/excepts in base_tray_generator silently skip on failure (base_tray_generator.py:116-148). Fine as a policy, but log a warning so a user knows their print lost a functional chamfer (the hinge-rotation one is arguably not cosmetic — the comment says it "allows hinge to rotate back").

---

D. Testing, docs, repo

• D1. No tests at all. The layout modules are pure math with zero CAD dependency — perfect unit-test targets, and they're exactly what you'll be changing for new shapes. Before the refactor, pin current behavior with tests for: linear fit/overflow, alternating nesting + the B1–B3 bugs, edge-offset padding, tolerance handling, usable-area math. Add one slow "smoke" test that builds a small tray end-to-end and asserts the export succeeds and cutout count matches. This is the safety net that makes everything in section A cheap.

• D2. README drift. Options table is missing --cutout-shape and --min-cutout-spacing; defaults differ from code (floor_thickness 0.4 vs 0.8); it references Trays/tray_generator.ipynb which doesn't exist; auto-filenames don't encode shape (a 40mm square tray exports as tray_1x40.0mm — indistinguishable from a circle tray; include the shape in the name).

• D3. Repo hygiene. venv/, path/to/venv/ (an accidental literal-path venv — delete it), __pycache__/, .DS_Store, and generated Trays/output/*.stl|*.step are all committed. Add/extend .gitignore and git rm -r --cached them. Also the stray root-level output/ dir (symptom of B6).