Align CSG result naming with variable-based IDs and collision-safe blockKey handling#506
Conversation
Deploying flockdev with
|
| Latest commit: |
151a054
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://46d8fe0c.flockdev.pages.dev |
| Branch Preview URL: | https://codex-update-csg-naming-to-u.flockdev.pages.dev |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 19 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds deterministic mesh identity handling for CSG: generators emit Changes
Sequence Diagram(s)sequenceDiagram
participant Generator
participant CSG_API
participant Scene
Generator->>CSG_API: call CSG op with meshId `${resultVar}__${block.id}`
CSG_API->>CSG_API: resolveCsgModelIdentity(requestedModelId) -> {modelId, blockKey}
CSG_API->>Scene: prepareMeshes(..., modelId, blockKey)
Scene-->>CSG_API: prepared meshes
CSG_API->>CSG_API: perform CSG operation -> result mesh
CSG_API->>Scene: add mesh with unique modelId (append uid if collision)
CSG_API->>Scene: flock.applyResultMeshProperties(result, blockKey)
Scene-->>Generator: result reference
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
generators/generators-transform.js (1)
575-580:⚠️ Potential issue | 🟡 MinorAlign
hull_meshesID pattern with other CSG blocks.The
merge_meshes,subtract_meshes, andintersection_meshesblocks all use${resultVar}__${block.id}, buthull_meshesuses"hull" + "_" + generateUniqueId(). This inconsistency meansresolveCsgModelIdentitywill parse blockKey differently for hull IDs—the entire ID becomes blockKey instead of extracting the block ID from the__separator.Change
hull_meshesto use the same pattern:- const meshId = "hull" + "_" + generateUniqueId(); + const meshId = `${resultVar}__${block.id}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generators/generators-transform.js` around lines 575 - 580, The hull_meshes case is creating meshId with "hull_"+generateUniqueId(), which breaks resolveCsgModelIdentity's expected `${resultVar}__${block.id}` pattern; update the code where meshId is set (and where meshMap and meshBlockIdMap are assigned) to use the same pattern as other CSG blocks (e.g., meshId = `${resultVar}__${block.id}`), leaving the subsequent createHull call (createHull("${meshId}", ${meshList})) and the meshMap/meshBlockIdMap assignments intact so identity parsing via resolveCsgModelIdentity works consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/csg.js`:
- Around line 919-924: createHull is calling resolveCsgModelIdentity but the
hull generator ("hull" + "_" + generateUniqueId()) uses a single underscore so
resolveCsgModelIdentity sets blockKey to the whole string (e.g., "hull_abc123")
instead of the raw block ID like other CSG ops; update the hull_meshes generator
to emit IDs using the same split pattern as other CSG items (e.g.,
"hull__<blockId>" or whatever delimiter resolveCsgModelIdentity expects) so
blockKey becomes the block ID, or alternatively change createHull to
normalize/extract the block ID before calling flock.prepareMeshes (referencing
createHull, resolveCsgModelIdentity, hull_meshes generator, and
flock.prepareMeshes).
---
Outside diff comments:
In `@generators/generators-transform.js`:
- Around line 575-580: The hull_meshes case is creating meshId with
"hull_"+generateUniqueId(), which breaks resolveCsgModelIdentity's expected
`${resultVar}__${block.id}` pattern; update the code where meshId is set (and
where meshMap and meshBlockIdMap are assigned) to use the same pattern as other
CSG blocks (e.g., meshId = `${resultVar}__${block.id}`), leaving the subsequent
createHull call (createHull("${meshId}", ${meshList})) and the
meshMap/meshBlockIdMap assignments intact so identity parsing via
resolveCsgModelIdentity works consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f8041ab-5ef2-4664-bf8f-5e840198bc3c
📒 Files selected for processing (2)
api/csg.jsgenerators/generators-transform.js
| const { modelId: resolvedModelId, blockKey } = | ||
| resolveCsgModelIdentity(modelId); | ||
| modelId = resolvedModelId; | ||
|
|
||
| return flock | ||
| .prepareMeshes(modelId, meshList, blockId) | ||
| .prepareMeshes(modelId, meshList, blockKey) |
There was a problem hiding this comment.
createHull uses resolveCsgModelIdentity but generator still uses old pattern.
This function will parse an ID like hull_abc123 (from the generator's "hull" + "_" + generateUniqueId()). Since there's no __ in that pattern, resolveCsgModelIdentity will set blockKey = requestedModelId (the full string), which differs from the other CSG operations where blockKey is just the block ID.
This asymmetry may cause issues with block ownership metadata. Either update hull_meshes generator to use the new pattern, or document why hull is intentionally different.
Also applies to: 1003-1004
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/csg.js` around lines 919 - 924, createHull is calling
resolveCsgModelIdentity but the hull generator ("hull" + "_" +
generateUniqueId()) uses a single underscore so resolveCsgModelIdentity sets
blockKey to the whole string (e.g., "hull_abc123") instead of the raw block ID
like other CSG ops; update the hull_meshes generator to emit IDs using the same
split pattern as other CSG items (e.g., "hull__<blockId>" or whatever delimiter
resolveCsgModelIdentity expects) so blockKey becomes the block ID, or
alternatively change createHull to normalize/extract the block ID before calling
flock.prepareMeshes (referencing createHull, resolveCsgModelIdentity,
hull_meshes generator, and flock.prepareMeshes).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
generators/generators-transform.js (1)
505-510:⚠️ Potential issue | 🟠 MajorUse
lastIndexOf("__")instead ofindexOf("__")to correctly parse meshIds when variable names contain double underscores.The meshId format
${resultVar}__${block.id}creates a parsing ambiguity inresolveCsgModelIdentity()at line 264 ofapi/csg.js. Since Blockly allows any valid JavaScript identifier (including those with__), a variable namedmy__meshwould create meshIdmy__mesh__abc123. The current code usingindexOf("__")finds the first occurrence and incorrectly parses it asmodelId = "my"andblockKey = "mesh__abc123", when it should parse the last__separator to getmodelId = "my__mesh"andblockKey = "abc123".Fix line 264 in
api/csg.js:- const separatorIndex = requestedModelId.indexOf("__"); + const separatorIndex = requestedModelId.lastIndexOf("__");This affects all four CSG transform blocks:
merge_meshes(line 505),subtract_meshes(line 531),intersection_meshes(line 553), andhull_meshes(line 575) in generators-transform.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generators/generators-transform.js` around lines 505 - 510, The meshId generation uses `${resultVar}__${block.id}` (symbols: meshId, meshMap, meshBlockIdMap, mergeMeshes) but parsing in resolveCsgModelIdentity() (api/csg.js) uses indexOf("__") which breaks when resultVar contains "__"; change the parser to use lastIndexOf("__") instead of indexOf("__") so the separator is taken as the final "__" and modelId and blockKey are split correctly; ensure the same fix is applied conceptually to any CSG transform parsing logic associated with merge_meshes, subtract_meshes, intersection_meshes, and hull_meshes so generated ids like `my__mesh__abc123` yield modelId="my__mesh" and blockKey="abc123".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@generators/generators-transform.js`:
- Around line 505-510: The meshId generation uses `${resultVar}__${block.id}`
(symbols: meshId, meshMap, meshBlockIdMap, mergeMeshes) but parsing in
resolveCsgModelIdentity() (api/csg.js) uses indexOf("__") which breaks when
resultVar contains "__"; change the parser to use lastIndexOf("__") instead of
indexOf("__") so the separator is taken as the final "__" and modelId and
blockKey are split correctly; ensure the same fix is applied conceptually to any
CSG transform parsing logic associated with merge_meshes, subtract_meshes,
intersection_meshes, and hull_meshes so generated ids like `my__mesh__abc123`
yield modelId="my__mesh" and blockKey="abc123".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5df57c07-df37-4f9c-950c-b79b0df1ea70
📒 Files selected for processing (1)
generators/generators-transform.js
1 participant
Motivation
name__blockId) so scene names are readable and predictable.blockKey) is derived from the variable-based ID rather than a hardcoded runtime prefix.Description
generators/generators-transform.js(merge_meshes,subtract_meshes,intersection_meshes) to emit IDs as${resultVar}__${block.id}and preservedmeshMap/meshBlockIdMapwiring and generated call shapes (e.g. `await mergeMeshes(Codex Task
Summary by CodeRabbit
Bug Fixes
Refactor