Skip to content

Bugfix: Apply transformations to correct mesh when duplicated - #740

Merged
lawsie merged 1 commit into
flipcomputing:mainfrom
lawsie:bugfix-duplicate-cascade
Aug 6, 2026
Merged

Bugfix: Apply transformations to correct mesh when duplicated#740
lawsie merged 1 commit into
flipcomputing:mainfrom
lawsie:bugfix-duplicate-cascade

Conversation

@lawsie

@lawsie lawsie commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixed a bug where if you duplicated a mesh which had >1 transformation applied (e.g. resize and rotate), the copy would have two transform blocks applied but the second would target the original mesh.

Previous behaviour:
image

AI usage

Claude Sonnet 5 found the bug and fixed. I identified it during manual testing.

Summary by CodeRabbit

  • Bug Fixes
    • Improved variable handling across connected block sequences.
    • Ensured variable retargeting consistently respects creation filters throughout nested blocks.
    • Fixed adoption of isolated default variables when blocks are linked in sequence.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Variable descendant traversal now follows each connected next chain. Retargeting preserves the createdIds filter. Isolated-default-variable adoption recursively processes sequential blocks within input subtrees.

Changes

Variable traversal

Layer / File(s) Summary
Traverse sequential descendants
blocks/blocks.js
Variable retargeting now scans complete next chains and preserves the createdIds restriction. Variable adoption now includes sequential siblings and their input subtrees.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: tracygardner

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the bugfix for applying transformations to the correct duplicated mesh, which matches the pull request objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
blocks/blocks.js (1)

755-768: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression test for sequential descendant traversal.

The supplied test in tests/blocks.test.js at Lines 120-156 does not cover a variable field in an input-connected getNextBlock() sibling or its nested inputs. Add a test that duplicates a block with two transformation blocks and verifies that both fields use the new variable ID.

As per coding guidelines, include API tests in the implementation plan and run automated tests where possible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@blocks/blocks.js` around lines 755 - 768, Add a regression test in the
existing blocks duplication test area covering sequential descendants reached
through an input connection: build two transformation blocks with a variable
field, including nested input descendants, duplicate the root, and assert both
copied fields reference the new variable ID rather than the original. Include
the API test in the implementation plan and run the relevant automated tests.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@blocks/blocks.js`:
- Around line 755-768: Add a regression test in the existing blocks duplication
test area covering sequential descendants reached through an input connection:
build two transformation blocks with a variable field, including nested input
descendants, duplicate the root, and assert both copied fields reference the new
variable ID rather than the original. Include the API test in the implementation
plan and run the relevant automated tests.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 287e5ffd-d1e8-4daa-b9cd-9af60c5b8cbc

📥 Commits

Reviewing files that changed from the base of the PR and between 8989d7f and e856ea5.

📒 Files selected for processing (1)
  • blocks/blocks.js

@lawsie
lawsie merged commit 8b8c9f3 into flipcomputing:main Aug 6, 2026
3 checks passed
@lawsie
lawsie deleted the bugfix-duplicate-cascade branch August 6, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant