IFC-2025: Fix artifact diff migrated kind#9273
Conversation
github-actions
Bot
added
the
group/backend
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 4 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. This PR modifies the core artifact diff engine and a Cypher query to handle node migrations by using UUID matching, which warrants human review to ensure no unintended side effects on global diffing behavior.
Re-trigger cubic
| @@ -0,0 +1 @@ | |||
| ArtifactDiffCalculator no longer produces false-positive diff entries for artifacts whose target node was re-keyed by a node kind migration (schema name or namespace change). | |||
There was a problem hiding this comment.
users aren't going to know what ArtifactDiffCalculator is
| } | ||
|
|
||
|
|
||
| async def test_calculate_artifact_diff_migrated_kind( |
There was a problem hiding this comment.
if you combine this test with the one below into a class or single test case, then the migration only has to run one time instead of two
| (target_artifact:%(artifact_kind)s)-[drel1:IS_RELATED]-(drel_node:Relationship)-[drel2:IS_RELATED]-(definition_node) | ||
| WHERE trel_node.name = $target_rel_identifier | ||
| WHERE tn_target.uuid = target_node.uuid | ||
| AND trel_node.name = $target_rel_identifier |
There was a problem hiding this comment.
much smaller change than I would have expected this to be. very cool
| assert artifact_diffs[0].item_previous.checksum == "aabbccdd11223344556677889900aabb" | ||
|
|
||
|
|
||
| async def test_calculate_artifact_diff_main_side_migration( |
There was a problem hiding this comment.
would be worth checking the with_genuine_change case for the main branch like the tests above too
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would auto-approve. This PR fixes a targeted bug in artifact diff calculation after a kind migration by updating the Cypher query to match target nodes by UUID and adding a guard to skip diffs when storage is unchanged; the changes are minimal, well-tested, and do not affect critical infrastructure or production data.
Re-trigger cubic
2 participants
Why
ArtifactDiffQueryanchors its target-branch artifact lookup on the Neo4j node identity oftarget_node. AfterNodeKindUpdateMigrationruns, two physical Neo4j nodes share the same UUID — the original (soft-deleted on the migration branch) and the new-kind duplicate (active). The query starts from the new-kind node, which has noIS_RELATEDedges on the target branch, so the OPTIONAL MATCH returns NULL and the artifact appears asADDED.Closes #7490
How to test
Impact & rollout
WHERE tn_target.uuid = target_node.uuid) relies on the existinguuidindex on:Node; no regression expected.Checklist
backend/changelog/7490.fixed.md)Summary by cubic
Fixes artifact diffs for nodes re-keyed by a kind migration so we don’t show false ADDED/UPDATED entries. Aligns with IFC-2025 by matching migrated targets by UUID and only reporting real content changes.
tn_target.uuid = target_node.uuid) to find the correct artifact across migrated nodes.storage_idandchecksum).Written for commit 5d1b2ec. Summary will update on new commits. Review in cubic