fix(DataMapper): Preserve sibling variable after choice

[pvillant]

Resolves: #3452

Screen.Recording.2026-07-31.at.10.49.11.mov

Summary by CodeRabbit

• Bug Fixes

  • Preserved variable items when cleaning up stale mappings for a document.
  • Prevented valid mapping content from being removed during stale-mapping cleanup.

• Tests

  • Added regression coverage to verify variable items remain intact.

[coderabbitai]

📝 Walkthrough

Walkthrough

removeStaleMappingsForDocument() now preserves VariableItem children during target-document cleanup. A regression test verifies that an inserted variable and the mapping child count remain unchanged.

Changes

Mapping cleanup

Layer / File(s)	Summary
Preserve VariableItem children packages/ui/src/services/mapping/mapping.service.ts, packages/ui/src/services/mapping/mapping.service.test.ts	Stale-mapping cleanup retains VariableItem nodes. A regression test verifies variable and child-count preservation.

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

Possibly related PRs

• KaotoIO/kaoto#3205: Modifies MappingService handling of VariableItem children and stale-mapping cleanup.
• KaotoIO/kaoto#3421: Extends VariableItem support in MappingService.
• KaotoIO/kaoto#3553: Modifies mapping.service.ts and mapping selector handling.

Suggested reviewers: igarashitm, mmelko, tplevko

Poem

A rabbit found a variable small,
Safe beside the mapping wall.
Cleanup came with pruning teeth,
But left that node and siblings beneath.
The tree stayed whole, from root to leaf.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The code preserves VariableItem children and adds a regression test for issue #3452.
Out of Scope Changes check	✅ Passed	All changes directly support preserving variables during stale-mapping cleanup.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes preserving a sibling variable after a choice selection, which is the primary change in the 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[pvillant]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

packages/ui/src/services/mapping/mapping.service.test.ts (1)

668-670: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that cleanup retains the original VariableItem instance.

The current find() check passes if cleanup replaces variable with a new item that has the same name. Assert reference identity so the test detects loss of VariableItem state.

Proposed assertion

-      const retained = parentItem.children.find((c) => c instanceof VariableItem && c.name === 'myVar');
-      expect(retained).toBeDefined();
+      expect(parentItem.children).toContain(variable);
+      expect(variable.parent).toBe(parentItem);

🤖 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 `@packages/ui/src/services/mapping/mapping.service.test.ts` around lines 668 -
670, Update the cleanup assertion in the test around parentItem.children to
verify that the retained entry is the original VariableItem instance, not merely
an item with the same name; capture or reuse the pre-cleanup variable reference
and assert reference identity while preserving the existing children-length
check.

🤖 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 `@packages/ui/src/services/mapping/mapping.service.test.ts`:
- Around line 668-670: Update the cleanup assertion in the test around
parentItem.children to verify that the retained entry is the original
VariableItem instance, not merely an item with the same name; capture or reuse
the pre-cleanup variable reference and assert reference identity while
preserving the existing children-length check.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 599809fc-8170-4363-8a99-4e18ad25d189

📥 Commits

Reviewing files that changed from the base of the PR and between dab84fd and 484c0c2.

📒 Files selected for processing (2)

• packages/ui/src/services/mapping/mapping.service.test.ts
• packages/ui/src/services/mapping/mapping.service.ts

[igarashitm]

Fix LGTM 👍

However the screencast reveals a bug, when choice selection is made to dataValue after variable creation, the dataValue node gets an expand button at left despite of being string leaf field. Could you either:

• Fix it together if it's trivial
• File an issue otherwise

[igarashitm]

Also, this assertion improvement sounds nice to have and low hanging - #3594 (review)

[igarashitm]

Verified this "expand button on leaf field" issue reproduces without variable creation #3594 (review)

Interestingly, if I reproduce it after creating a variable, then remove the variable, the button disappears.