Staging seedback by seedback · Pull Request #7 · seedback/LayerSafe

seedback · 2026-04-05T11:41:47Z

No description provided.

Copilot

Pull request overview

This PR refactors tray/cutout generation to use Build123d joints for positioning and introduces caching for repeated tray/cutout geometry generation.

Changes:

Refactors cutout_generator and cutout-position calculators to produce Parts/Compounds with RigidJoints for assembly-style placement.
Updates base_tray_generator to return additional hinge-pin geometry metadata and reworks the tray geometry construction accordingly.
Updates full_tray_generator to connect cutouts to computed position joints, apply boolean subtraction per cutout, and cache generated cutouts.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`Trays/functions/old_cutout_generator.py`	Adds an “old” cutout generator implementation (currently appears unused).
`Trays/functions/full_tray_generator.py`	Switches to joint-based placement, adds cutout caching, and changes the function return shape.
`Trays/functions/cutout_generator.py`	Rebuilds cutout geometry around joints (`Track`, `LipShaper`, `Center`) and adds new parameters (hinge pin height, taper angle).
`Trays/functions/calculate_cutout_positions/calculate_linear_cutout_positions.py`	Returns a Part with joints instead of a list of dict positions; assigns joints for front/back lines.
`Trays/functions/calculate_cutout_positions/calculate_alternating_cutout_positions.py`	Returns a Part with joints instead of a list of dict positions.
`Trays/functions/base_tray_generator.py`	Major rewrite of tray construction using joints; returns `(tray_compound, hinge_pin_height)`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-05T11:46:11Z

    hinge_lock_offset=0.5,
    hinge_lock_depth=8.3,
-    edge_offsets = [],
-    edge_adjusts = [],
+    edge_offsets=[],
+    edge_adjusts=[],


edge_offsets / edge_adjusts are mutable defaults. These lists are shared across calls and can cause state leakage if they’re ever mutated. Prefer None defaults and initialize to [] inside the function.

Copilot · 2026-04-05T11:46:12Z

  max_diameter = max(diameters)
  if max_diameter <= -usable_area['min']['y'] or not is_double_tray:
-    positions = calculate_linear_cutout_positions(
+    print("linear")
+    return calculate_linear_cutout_positions(
        usable_area, diameters, edge_offsets, tolerance, is_double_tray)
  else:
-    positions = calculate_alternating_cutout_positions(
+    print("alternating")
+    return calculate_alternating_cutout_positions(


Unconditional print("linear") / print("alternating") will spam stdout for library/CLI use and makes program output harder to consume. Consider removing these or gating behind __main__/a debug flag, or using a logger with configurable level.

Copilot · 2026-04-05T11:46:12Z

@@ -54,17 +55,16 @@ def calculate_cutout_positions(
 ):
  if len(diameters) == 0:
    return []


calculate_cutout_positions returns [] when there are no diameters, but returns a Build123d Part (with .joints) otherwise. generate_full_tray later assumes .joints exists, so calling generate_full_tray([]) will fail. Keep the return type consistent (e.g., always return a Part with zero joints, or have generate_full_tray short-circuit when diameters is empty).

Suggested change

return []

return Part()

Copilot · 2026-04-05T11:46:12Z

+  for i, position in enumerate(positions.joints):
+    # Create cache key from cutout parameters
+    cutout_key = (
+        diameters[i],
        tolerance,
        flap_depth,
        hinge_diameter,
        flap_center_gap,
        safety_margin[1],
        edge_adjusts[i],
-        edge_offsets[i],
+        hinge_pin_height,
+        12.5,
        epsilon
-    ))
-
-    # Rotate 180 degrees if flipped (for top edge circles)
-    if position['flipped']:
-      cutout = cutout.rotate(Axis.Z, 180)
-
-    cutout = cutout.translate((position['x'], position['y'], floor_thickness))
+    )


The loop iterates over positions.joints but sizes each cutout using diameters[i]. For double-sided linear placement, joint order does not necessarily match the original diameters order (the algorithm alternates picking from left/right), so this can generate the wrong cutout size at a given joint/position. Consider naming joints with the original diameter index (e.g., line_one_indices[i]) or returning per-position metadata so the correct diameter is used.

Copilot · 2026-04-05T11:46:12Z

-    tray_compound -= cutouts
-
-  return tray_compound, cutouts_list
+  return tray_compound, cutouts_list, positions


generate_full_tray now returns three values (tray_compound, cutouts_list, positions), but existing callers (e.g., Trays/tray_generator.py expects two values) will break with a tuple-unpack error. Either keep the return signature backward-compatible (e.g., optional third value) or update all call sites in this PR.

Copilot · 2026-04-05T11:46:13Z

-    for i, pos in enumerate(y_positions):
+    for i, pos in enumerate(back_positions):
      pos['y'] = usable_area['max']['y'] - (pos['diameter']) / 2
      if edge_offsets and i < len(edge_offsets):


Same edge-offset indexing issue as the front loop: i < len(edge_offsets) doesn’t guarantee line_two_indices[i] is in range, so edge_offsets[line_two_indices[i]] can raise IndexError. Guard using line_two_indices[i] < len(edge_offsets) instead.

Suggested change

if edge_offsets and i < len(edge_offsets):

if edge_offsets and line_two_indices[i] < len(edge_offsets):

Copilot · 2026-04-05T11:46:13Z

-  normal_base.part = normal_base.part.translate((0,0,-epsilon))
+  with BuildPart() as track_builder:
+    cross_section = section(base_builder.part, Plane.XZ)
+    extrude(cross_section, total_radius - flap_depth + cutout_edge_spacing - flap_middle_gap + epsilon)


track_builder always extrudes the slide/track by total_radius - flap_depth + .... When total_radius is smaller than the flap geometry, this amount becomes negative; the old implementation guarded this with a conditional. If a negative length isn’t intended here, reintroduce the conditional (or clamp to 0) to avoid invalid/reversed geometry for small diameters.

Suggested change

extrude(cross_section, total_radius - flap_depth + cutout_edge_spacing - flap_middle_gap + epsilon)

track_extrude_amount = max(

0,

total_radius - flap_depth + cutout_edge_spacing - flap_middle_gap + epsilon

)

extrude(cross_section, track_extrude_amount)

Copilot · 2026-04-05T11:46:13Z

+    revolve_axis = Axis(origin=(0, total_radius + hinge_diameter/2 - lip_offset - tolerance, 0), direction=(0, 0, 1))
+    with BuildSketch(Plane.YZ) as revolve_sketch:
+      Circle(hinge_diameter/2)
    revolve(axis=revolve_axis)
+    y_adjust = math.sqrt((hinge_diameter/2)**2 - hinge_pin_height**2)


y_adjust = sqrt((hinge_diameter/2)^2 - hinge_pin_height^2) will raise a math-domain error if abs(hinge_pin_height) > hinge_diameter/2 (e.g., from unusual floor_thickness/hinge_height inputs). Consider validating inputs and raising a clearer ValueError, or clamping to a safe range.

Suggested change

revolve_axis = Axis(origin=(0, total_radius + hinge_diameter/2 - lip_offset - tolerance, 0), direction=(0, 0, 1))

with BuildSketch(Plane.YZ) as revolve_sketch:

Circle(hinge_diameter/2)

revolve(axis=revolve_axis)

y_adjust = math.sqrt((hinge_diameter/2)**2 - hinge_pin_height**2)

hinge_radius = hinge_diameter / 2

revolve_axis = Axis(origin=(0, total_radius + hinge_radius - lip_offset - tolerance, 0), direction=(0, 0, 1))

with BuildSketch(Plane.YZ) as revolve_sketch:

Circle(hinge_radius)

revolve(axis=revolve_axis)

if abs(hinge_pin_height) > hinge_radius:

raise ValueError(

f"Invalid hinge geometry: abs(hinge_pin_height) ({abs(hinge_pin_height)}) "

f"must be less than or equal to hinge_diameter/2 ({hinge_radius})."

)

y_adjust = math.sqrt(hinge_radius**2 - hinge_pin_height**2)

Copilot · 2026-04-05T11:46:13Z

+import copy
+


copy is imported but not used in this file. If it’s not needed, please remove it to avoid unused-import lint issues.

Suggested change

import copy

Copilot · 2026-04-05T11:46:14Z

+# %%
+from build123d import *
+from ocp_vscode import *
+import math
+import copy
+
+# %%
+
+
+def generate_cutout(
+    base_diameter,
+    tolerance=0.55,
+    flap_depth=11.8,
+    hinge_diameter=27.5,
+    flap_center_gap=0.2,
+    cutout_edge_spacing=.8,
+    floor_thickness=.8,
+    lip_offset=0,
+    epsilon=0.001
+):


This file appears to be unused in the repository (no references found) and duplicates cutout_generator.py under an “old_” name. Keeping unused generator code in the main functions package makes maintenance harder and can confuse future refactors; consider moving it to an examples/ or archive/ area, or removing it if it’s no longer needed.

seedback added 7 commits April 2, 2026 01:07

Refactor base_tray_generator

f682ebb

Adjust cutout placement to account for floor_thickness

c3650d6

minor change to export path

e5c5661

Merge branch 'improved-tray-generator' into staging-seedback

77f24e2

Delete old_base_tray_generator.py

af44e8a

improve cutout generator and linear positioning

83cdb0a

Update alternating positioning logic

5b49cd0

Copilot AI review requested due to automatic review settings April 5, 2026 11:41

Copilot started reviewing on behalf of seedback April 5, 2026 11:42 View session

Copilot AI reviewed Apr 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Staging seedback#7

Staging seedback#7
seedback wants to merge 7 commits into
mainfrom
staging-seedback

seedback commented Apr 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if edge_offsets and i < len(edge_offsets):
	if edge_offsets and line_two_indices[i] < len(edge_offsets):

-    revolve_axis = Axis(origin=(0, total_radius + hinge_diameter/2 - lip_offset - tolerance, 0), direction=(0, 0, 1))
-    with BuildSketch(Plane.YZ) as revolve_sketch:
-      Circle(hinge_diameter/2)
-    revolve(axis=revolve_axis)
-    y_adjust = math.sqrt((hinge_diameter/2)**2 - hinge_pin_height**2)
+    hinge_radius = hinge_diameter / 2
+    revolve_axis = Axis(origin=(0, total_radius + hinge_radius - lip_offset - tolerance, 0), direction=(0, 0, 1))
+    with BuildSketch(Plane.YZ) as revolve_sketch:
+      Circle(hinge_radius)
+    revolve(axis=revolve_axis)
+    if abs(hinge_pin_height) > hinge_radius:
+      raise ValueError(
+        f"Invalid hinge geometry: abs(hinge_pin_height) ({abs(hinge_pin_height)}) "
+        f"must be less than or equal to hinge_diameter/2 ({hinge_radius})."
+      )
+    y_adjust = math.sqrt(hinge_radius**2 - hinge_pin_height**2)

Conversation

seedback commented Apr 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants