feat: add visual joint markers to all templates#4
Open
gaius-codius wants to merge 1 commit intomainfrom
Open
Conversation
Add colored visual markers showing joint positions in 3D models to help woodworkers identify where cuts need to be made. Infrastructure (base.py): - Add JointMarker dataclass with position, dimensions, and color - Add JOINT_COLORS dict with 7 joint types - Add show_joints parameter (default True) to BaseTemplate - Add _create_joint_marker_ruby() for Ruby code generation - Add _get_joint_markers() hook and _generate_markers_ruby() method Template implementations: - bookshelf: dado markers on side panels for shelf positions - box: finger_joint corners + rabbet bottom edges - table: mortise_tenon on leg faces for apron/stretcher connections - cabinet: dado for shelves + rabbet for back panel - workbench: mortise_tenon on 4 or 6 legs - desk: dado markers for rail connections on leg panels - picture_frame: miter corners (purple) + rabbet for glass (blue) - shelf_bracket: bracket markers where shelf meets supports - tray: rabbet corners + bottom edge markers Joint colors: dado=red, rabbet=blue, finger_joint=orange, miter=purple, mortise_tenon=green, butt=gray, bracket=yellow
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found across 11 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/sketchup_mcp/templates/desk.py">
<violation number="1" location="src/sketchup_mcp/templates/desk.py:173">
P2: Color reference is hardcoded as 'red' but `self.joinery` is dynamic. If joinery is set to something other than 'dado' (e.g., 'mortise_tenon'), the notes will incorrectly say 'red markers' when the actual markers would be a different color (green for mortise_tenon). Consider deriving the color name dynamically from the joint type.</violation>
</file>
<file name="src/sketchup_mcp/templates/workbench.py">
<violation number="1" location="src/sketchup_mcp/templates/workbench.py:241">
P3: Unused instance variable `_stretcher_z` is computed but never read. Either remove this dead code or implement stretcher joint markers (workbenches have lower stretchers with mortise-tenon joints that could also have markers).</violation>
</file>
<file name="src/sketchup_mcp/templates/box.py">
<violation number="1" location="src/sketchup_mcp/templates/box.py:245">
P2: Hardcoded color '(orange markers)' is misleading when `self.joinery` is not 'finger_joint'. Since joinery is configurable, consider removing the hardcoded color or deriving it dynamically from the joint type.</violation>
</file>
<file name="src/sketchup_mcp/templates/tray.py">
<violation number="1" location="src/sketchup_mcp/templates/tray.py:142">
P2: Incomplete bottom rabbet markers implementation - only front edge marker is created but the comment and stored `_interior_depth` variable suggest all 4 bottom edges should have markers. The `_interior_depth` field is stored but never used, and comparing with `box.py` shows this pattern typically includes multiple edge markers.</violation>
</file>
<file name="src/sketchup_mcp/templates/cabinet.py">
<violation number="1" location="src/sketchup_mcp/templates/cabinet.py:209">
P2: Hardcoded "(red markers)" doesn't reflect actual marker color when joinery differs from "dado". The marker color depends on `self.joinery`, so if a user specifies a different joint type (e.g., rabbet=blue), this note becomes misleading.</violation>
</file>
<file name="src/sketchup_mcp/templates/bookshelf.py">
<violation number="1" location="src/sketchup_mcp/templates/bookshelf.py:152">
P2: Hardcoded `(red markers)` in notes will be incorrect if joinery type is changed from the default 'dado'. Consider either removing the color reference or dynamically looking up the color from `JOINT_COLORS`.</violation>
</file>
<file name="src/sketchup_mcp/templates/base.py">
<violation number="1" location="src/sketchup_mcp/templates/base.py:244">
P2: Incomplete sanitization for Ruby variable name. The replacement only handles spaces and hyphens, but other characters like parentheses, dots, or special symbols would create invalid Ruby variable names. Use a regex to filter to only alphanumeric and underscore characters.</violation>
<violation number="2" location="src/sketchup_mcp/templates/base.py:250">
P2: Potential Ruby syntax error if `marker.name` contains double quotes. The name is interpolated directly into a Ruby string literal without escaping special characters.</violation>
</file>
<file name="src/sketchup_mcp/templates/picture_frame.py">
<violation number="1" location="src/sketchup_mcp/templates/picture_frame.py:164">
P2: Missing left and right rabbet markers. Picture frames have rabbet cuts along all 4 inner edges, but only top and bottom rabbet markers are implemented. Woodworkers won't see where to cut the rabbet on the side rails.</violation>
</file>
<file name="src/sketchup_mcp/templates/shelf_bracket.py">
<violation number="1" location="src/sketchup_mcp/templates/shelf_bracket.py:219">
P2: Hardcoded color '(yellow markers)' in notes may be incorrect if joinery type is overridden. The marker color is dynamically determined by `self.joinery`, but the notes always say yellow. Consider removing the color reference or making it dynamic.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| quantity=2, | ||
| material=self.material, | ||
| notes="Left and right leg panels", | ||
| notes=f"Left and right leg panels, {self.joinery} joints (red markers)", |
There was a problem hiding this comment.
P2: Color reference is hardcoded as 'red' but self.joinery is dynamic. If joinery is set to something other than 'dado' (e.g., 'mortise_tenon'), the notes will incorrectly say 'red markers' when the actual markers would be a different color (green for mortise_tenon). Consider deriving the color name dynamically from the joint type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/desk.py, line 173:
<comment>Color reference is hardcoded as 'red' but `self.joinery` is dynamic. If joinery is set to something other than 'dado' (e.g., 'mortise_tenon'), the notes will incorrectly say 'red markers' when the actual markers would be a different color (green for mortise_tenon). Consider deriving the color name dynamically from the joint type.</comment>
<file context>
@@ -114,7 +170,7 @@ def generate(self) -> TemplateResult:
quantity=2,
material=self.material,
- notes="Left and right leg panels",
+ notes=f"Left and right leg panels, {self.joinery} joints (red markers)",
),
LumberPiece(
</file context>
| quantity=2, | ||
| material=self.material, | ||
| notes=f"Left and right sides, {self.joinery}", | ||
| notes=f"Left and right sides, {self.joinery} (orange markers)", |
There was a problem hiding this comment.
P2: Hardcoded color '(orange markers)' is misleading when self.joinery is not 'finger_joint'. Since joinery is configurable, consider removing the hardcoded color or deriving it dynamically from the joint type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/box.py, line 245:
<comment>Hardcoded color '(orange markers)' is misleading when `self.joinery` is not 'finger_joint'. Since joinery is configurable, consider removing the hardcoded color or deriving it dynamically from the joint type.</comment>
<file context>
@@ -105,7 +242,7 @@ def generate(self) -> TemplateResult:
quantity=2,
material=self.material,
- notes=f"Left and right sides, {self.joinery}",
+ notes=f"Left and right sides, {self.joinery} (orange markers)",
),
LumberPiece(
</file context>
Suggested change
| notes=f"Left and right sides, {self.joinery} (orange markers)", | |
| notes=f"Left and right sides, {self.joinery} joints", |
| ) | ||
| ) | ||
|
|
||
| # Bottom rabbet markers |
There was a problem hiding this comment.
P2: Incomplete bottom rabbet markers implementation - only front edge marker is created but the comment and stored _interior_depth variable suggest all 4 bottom edges should have markers. The _interior_depth field is stored but never used, and comparing with box.py shows this pattern typically includes multiple edge markers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/tray.py, line 142:
<comment>Incomplete bottom rabbet markers implementation - only front edge marker is created but the comment and stored `_interior_depth` variable suggest all 4 bottom edges should have markers. The `_interior_depth` field is stored but never used, and comparing with `box.py` shows this pattern typically includes multiple edge markers.</comment>
<file context>
@@ -63,6 +63,99 @@ def __init__(
+ )
+ )
+
+ # Bottom rabbet markers
+ # Front edge
+ markers.append(
</file context>
| quantity=2, | ||
| material=self.material, | ||
| notes=f"Left and right sides, {self.joinery} for shelves", | ||
| notes=f"Left and right sides, {self.joinery} for shelves (red markers)", |
There was a problem hiding this comment.
P2: Hardcoded "(red markers)" doesn't reflect actual marker color when joinery differs from "dado". The marker color depends on self.joinery, so if a user specifies a different joint type (e.g., rabbet=blue), this note becomes misleading.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/cabinet.py, line 209:
<comment>Hardcoded "(red markers)" doesn't reflect actual marker color when joinery differs from "dado". The marker color depends on `self.joinery`, so if a user specifies a different joint type (e.g., rabbet=blue), this note becomes misleading.</comment>
<file context>
@@ -113,7 +206,7 @@ def generate(self) -> TemplateResult:
quantity=2,
material=self.material,
- notes=f"Left and right sides, {self.joinery} for shelves",
+ notes=f"Left and right sides, {self.joinery} for shelves (red markers)",
),
LumberPiece(
</file context>
Suggested change
| notes=f"Left and right sides, {self.joinery} for shelves (red markers)", | |
| notes=f"Left and right sides, {self.joinery} for shelves", |
| quantity=self.shelves, | ||
| material=self.material, | ||
| notes=f"Fixed shelves, {self.joinery} joints", | ||
| notes=f"Fixed shelves, {self.joinery} joints (red markers)", |
There was a problem hiding this comment.
P2: Hardcoded (red markers) in notes will be incorrect if joinery type is changed from the default 'dado'. Consider either removing the color reference or dynamically looking up the color from JOINT_COLORS.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/bookshelf.py, line 152:
<comment>Hardcoded `(red markers)` in notes will be incorrect if joinery type is changed from the default 'dado'. Consider either removing the color reference or dynamically looking up the color from `JOINT_COLORS`.</comment>
<file context>
@@ -100,7 +149,7 @@ def generate(self) -> TemplateResult:
quantity=self.shelves,
material=self.material,
- notes=f"Fixed shelves, {self.joinery} joints",
+ notes=f"Fixed shelves, {self.joinery} joints (red markers)",
),
LumberPiece(
</file context>
Suggested change
| notes=f"Fixed shelves, {self.joinery} joints (red markers)", | |
| notes=f"Fixed shelves, {self.joinery} joints", |
| return f''' | ||
| # Create joint marker: {marker.name} | ||
| marker_group = model.active_entities.add_group | ||
| marker_group.name = "Joint: {marker.name}" |
There was a problem hiding this comment.
P2: Potential Ruby syntax error if marker.name contains double quotes. The name is interpolated directly into a Ruby string literal without escaping special characters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/base.py, line 250:
<comment>Potential Ruby syntax error if `marker.name` contains double quotes. The name is interpolated directly into a Ruby string literal without escaping special characters.</comment>
<file context>
@@ -187,6 +227,81 @@ def _apply_material_ruby(self, group_name: str, color: str) -> str:
+ return f'''
+# Create joint marker: {marker.name}
+marker_group = model.active_entities.add_group
+marker_group.name = "Joint: {marker.name}"
+marker_ents = marker_group.entities
+
</file context>
| Ruby code string to create the marker | ||
| """ | ||
| # Sanitize name for Ruby variable | ||
| var_name = marker.name.lower().replace(" ", "_").replace("-", "_") |
There was a problem hiding this comment.
P2: Incomplete sanitization for Ruby variable name. The replacement only handles spaces and hyphens, but other characters like parentheses, dots, or special symbols would create invalid Ruby variable names. Use a regex to filter to only alphanumeric and underscore characters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/base.py, line 244:
<comment>Incomplete sanitization for Ruby variable name. The replacement only handles spaces and hyphens, but other characters like parentheses, dots, or special symbols would create invalid Ruby variable names. Use a regex to filter to only alphanumeric and underscore characters.</comment>
<file context>
@@ -187,6 +227,81 @@ def _apply_material_ruby(self, group_name: str, color: str) -> str:
+ Ruby code string to create the marker
+ """
+ # Sanitize name for Ruby variable
+ var_name = marker.name.lower().replace(" ", "_").replace("-", "_")
+ mat_name = f"Joint_{marker.joint_type}"
+
</file context>
| ) | ||
| ) | ||
|
|
||
| return markers |
There was a problem hiding this comment.
P2: Missing left and right rabbet markers. Picture frames have rabbet cuts along all 4 inner edges, but only top and bottom rabbet markers are implemented. Woodworkers won't see where to cut the rabbet on the side rails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/picture_frame.py, line 164:
<comment>Missing left and right rabbet markers. Picture frames have rabbet cuts along all 4 inner edges, but only top and bottom rabbet markers are implemented. Woodworkers won't see where to cut the rabbet on the side rails.</comment>
<file context>
@@ -58,6 +58,111 @@ def __init__(
+ )
+ )
+
+ return markers
+
def generate(self) -> TemplateResult:
</file context>
| quantity=1, | ||
| material=self.material, | ||
| notes="Sits on bracket horizontals", | ||
| notes="Sits on bracket horizontals (yellow markers)", |
There was a problem hiding this comment.
P2: Hardcoded color '(yellow markers)' in notes may be incorrect if joinery type is overridden. The marker color is dynamically determined by self.joinery, but the notes always say yellow. Consider removing the color reference or making it dynamic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/shelf_bracket.py, line 219:
<comment>Hardcoded color '(yellow markers)' in notes may be incorrect if joinery type is overridden. The marker color is dynamically determined by `self.joinery`, but the notes always say yellow. Consider removing the color reference or making it dynamic.</comment>
<file context>
@@ -172,7 +216,7 @@ def generate(self) -> TemplateResult:
quantity=1,
material=self.material,
- notes="Sits on bracket horizontals",
+ notes="Sits on bracket horizontals (yellow markers)",
)
)
</file context>
Suggested change
| notes="Sits on bracket horizontals (yellow markers)", | |
| notes="Sits on bracket horizontals (see joint markers)", |
| self._leg_x_positions = leg_x_positions | ||
| self._leg_y_positions = leg_y_positions | ||
| self._apron_z = leg_height - apron_height | ||
| self._stretcher_z = shelf_z - apron_height |
There was a problem hiding this comment.
P3: Unused instance variable _stretcher_z is computed but never read. Either remove this dead code or implement stretcher joint markers (workbenches have lower stretchers with mortise-tenon joints that could also have markers).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/sketchup_mcp/templates/workbench.py, line 241:
<comment>Unused instance variable `_stretcher_z` is computed but never read. Either remove this dead code or implement stretcher joint markers (workbenches have lower stretchers with mortise-tenon joints that could also have markers).</comment>
<file context>
@@ -114,6 +232,14 @@ def generate(self) -> TemplateResult:
+ self._leg_x_positions = leg_x_positions
+ self._leg_y_positions = leg_y_positions
+ self._apron_z = leg_height - apron_height
+ self._stretcher_z = shelf_z - apron_height
+
# Build cut list
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add colored visual markers showing joint positions in 3D models to help woodworkers identify where cuts need to be made.
Infrastructure (base.py):
Template implementations:
Joint colors: dado=red, rabbet=blue, finger_joint=orange, miter=purple, mortise_tenon=green, butt=gray, bracket=yellow
Summary by cubic
Adds colored joint markers to all templates to show exact joint positions in the 3D model, improving cut and assembly planning. Introduces a shared JointMarker API and Ruby generation, with a show_joints flag enabled by default.
New Features
Migration
Written for commit eb63b03. Summary will update on new commits.