Fixes #2891 Axis Settings shows an empty Dimension for merged dimensions

[rwmcintosh]

Problem

In the single-axis Axis Settings dialog, the Dimension field rendered blank for an axis whose dimension is a merged dimension (e.g. a Concentration axis with unit µmol/l), even though the Unit was populated and the axis-options grid showed the dimension correctly.

Root cause

getMergedDimensions inserted the axis's own dimension instance first (keyed by DisplayName), then the loop overwrote that same key with a freshly-minted merged-dimension instance (IDimensionFactory.MergedDimensionFor is not cached — it returns a new object on every call). Since Dimension/MergedDimensionFor compare by reference, the combo binder could no longer match the axis's instance to any list item, so it rendered blank. Only mergeable dimensions (e.g. molar↔mass Concentration) hit this; non-mergeable dimensions return the same factory-owned instance each call and were unaffected.

Fix

Move the defaultDimension insertion to after the loop in getMergedDimensions, so the axis's actual instance wins the DisplayName key and reference-matches in the editor. No behavior change for non-mergeable dimensions.

Test

Added a regression spec that builds a dimension factory with merging information and asserts the editor list contains the exact merged-dimension instance held by the axis (fails without the fix, passes with it).

Verified manually in PK-Sim: opening Axis Settings on a concentration axis of a time profile now shows the populated Dimension combo.

Fixes #2891

Summary by CodeRabbit

• Bug Fixes
  • Fixed dimension selection in editors to reliably return and select the correct dimension instance when dealing with merged dimensions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d79c763c-bf83-4266-b390-5b28de090e4f

📥 Commits

Reviewing files that changed from the base of the PR and between 6933cfe and 38f148f.

📒 Files selected for processing (2)

• src/OSPSuite.Core/Domain/UnitSystem/DimensionFactoryExtensions.cs
• tests/OSPSuite.Core.Tests/Domain/DimensionFactoryExtensionSpecs.cs

---

📝 Walkthrough

Walkthrough

getMergedDimensions is reordered so the loop over dimensionFactory.Dimensions runs first and defaultDimension is inserted into the cache afterward, preventing the loop from overwriting the axis's own instance. Explanatory comments are added, and a new BDD spec verifies that AllDimensionsForEditors returns the exact dimension instance held by the axis.

Changes

Merged Dimension Reference Fix

Layer / File(s)

Summary

Cache insertion order fix and regression test src/OSPSuite.Core/Domain/UnitSystem/DimensionFactoryExtensions.cs, tests/OSPSuite.Core.Tests/Domain/DimensionFactoryExtensionSpecs.cs

getMergedDimensions now iterates dimensionFactory.Dimensions before inserting defaultDimension into the cache, ensuring the axis's own instance wins on DisplayName key collision. New comments document that MergedDimensionFor creates a new instance per call and that editors compare by reference. The new spec When_getting_all_dimensions_for_editors_for_an_axis_holding_a_merged_dimension constructs a merged-dimension scenario and asserts AllDimensionsForEditors contains the exact same instance held by the axis.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A list of dimensions, a cache, and a bug,
The last one inserted would earn a warm hug.
The loop ran too late, the reference was lost,
The combo showed nothing — such a terrible cost!
Now default goes last, wins the key by design,
And the editor sees Concentration just fine! ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly references the linked issue #2891 and accurately captures the fix—that merged dimensions now display correctly in Axis Settings by reordering the dimension cache population.
Linked Issues check	✅ Passed	The code changes fully implement the suggested fix: reordering getMergedDimensions to insert the defaultDimension after the loop, preserving reference equality for editor selection. The regression test validates the exact instance is selectable.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #2891: the core fix in DimensionFactoryExtensions.cs and a regression test in DimensionFactoryExtensionSpecs.cs. No unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2891-empty-dimension-for-merged-dimensions

---

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 and usage tips.}