V13: Merge Behavior Documentation Discrepancies

[Yuri05]

This is the result of the comparison of the current V13 documentation of merge/overwrite behavior vs. the implementation in OSPSuite.Core (V13)

Full report

https://github.com/Open-Systems-Pharmacology/OSPSuite.Core/blob/copilot/research-merge-behavior-documentation/merge-behavior-findings.md

Summary Table

#	Claim / Topic	File & Lines	Status	Notes
1	MergeBehavior enum — two values, module-level, Overwrite default	Module.cs 18–34, 56	✅ Confirmed	Exactly two values; default = Overwrite
2	analyzeBuilderMerges dispatch, tryExtendContainers, 5 type-specific methods	SimulationBuilder.cs 96–135, 143–151, 164/172/224/261/266	✅ Confirmed	All present; reactions call tryExtendContainers twice (idempotent)
3a	Spatial Structure — MoleculeProperties always extended (even in Overwrite)	SpatialStructureMerger.cs 81–93, 144	✅ Confirmed	Hard-coded special case for MOLECULE_PROPERTIES name
3b	Spatial Structure — parameters replaced by name in Extend	ContainerMergeTask.cs 62	✅ Confirmed	AddOrReplaceInContainer for all leaf entities
3c	Spatial Structure — container Mode overwritten	ContainerMergeTask.cs 67	✅ Confirmed	targetContainer.Mode = containerToMerge.Mode
3d	Spatial Structure — tags behavior	ContainerMergeTask.cs 68	⚠️ Nuanced	Tags are additive (source tags appended, existing not removed)
4a	Molecules — QuantityType NOT changed in Extend (doc claim)	SimulationBuilder.cs 274	❌ Discrepancy	Code does set target.QuantityType = incoming.QuantityType
4b	Molecules — IsFloating behavior	SimulationBuilder.cs 272	❌ Discrepancy (if doc claims unchanged)	Code does set target.IsFloating = incoming.IsFloating
4c	Molecules — calculation methods replaced	SimulationBuilder.cs 277–278	✅ Confirmed	Cleared then replaced wholesale
4d	Molecules — parameters merged by name	SimulationBuilder.cs 125 → ContainerMergeTask.cs 62	✅ Confirmed	Via tryExtendContainers + AddOrReplaceInContainer
4e	Molecules — distributed parameter type handling	ContainerMergeTask.cs 46	✅ Confirmed	Treated as leaf entity; replaced wholesale
4f	Molecules — active transports	ContainerMergeTask.cs 50–59	✅ Confirmed	TransporterMoleculeContainer (a Container) recursively merged
5a	Reactions — formula (kinetic equation) replaced	SimulationBuilder.cs 176	✅ Confirmed	Unconditional assignment
5b	Reactions — educts/products upserted by molecule name	SimulationBuilder.cs 198–222	✅ Confirmed	Remove-existing-then-add; stoichiometry cloned
5c	Reactions — modifiers additive	SimulationBuilder.cs 183–188	✅ Confirmed	HashSet union; no removals
5d	Reactions — ContainerCriteria replaced	SimulationBuilder.cs 194–195	✅ Confirmed	Replaced only if incoming non-null
5e	Reactions — CreateProcessRateParameter/Persistable	SimulationBuilder.cs 177–178	✅ Confirmed	Both set from incoming
6a	Passive Transports — formula replaced	SimulationBuilder.cs 233	✅ Confirmed
6b	Passive Transports — source/target criteria	SimulationBuilder.cs 228–229, 236–240	⚠️ Nuanced	Operator replaced; conditions appended (additive)
6c	Passive Transports — molecule list (ForAll/include/exclude)	SimulationBuilder.cs 242–259	✅ Confirmed	ForAll taken from source; both lists upserted
7a	Observers — ObserverBuilder extends Entity (NOT Container)	ObserverBuilder.cs 12	✅ Confirmed	Critical — changes Extend semantics
7b	Observers — formula updated in Extend	SimulationBuilder.cs 261–264	❌ Discrepancy	Formula is NOT updated; mergeObservers only calls mergeMoleculeLists and ObserverBuilder is not a Container
7c	Observers — ContainerCriteria updated in Extend	SimulationBuilder.cs 261–264	❌ Discrepancy	ContainerCriteria is NOT updated; same reason as 7b
7d	Observers — molecule list behavior	SimulationBuilder.cs 262–263	✅ Confirmed	ForAll/include/exclude all handled via mergeMoleculeLists
8a	Events — event tree (parameters, assignments) merged	SimulationBuilder.cs 125 → ContainerMergeTask.cs	✅ Confirmed	EventGroupBuilder is a Container; tree merged recursively
8b	Events — EventGroupType replaced	SimulationBuilder.cs 167	✅ Confirmed
8c	Events — SourceCriteria behavior	SimulationBuilder.cs 169, 236–240	⚠️ Nuanced	Operator replaced; conditions appended (additive)
9	Parameter Values — order Individual vs Expression Profiles vs PV BBs	QuantityValuesUpdater.cs 62–68	⚠️ Nuanced	Actual order: ExpressionProfiles → Individual → PV BBs; code comment confirms Individual can overwrite EP (for aging)
10	Initial Conditions — Molecules BB → Expression Profiles → IC BBs	SimulationBuilder.cs 310–322; MoleculeBuilderToMoleculeAmountMapper.cs 91–111	✅ Confirmed	Three-stage override chain; last-writer-wins cache
11	Neighborhood merge — Extend vs Overwrite	NeighborhoodCollectionToContainerMapper.cs 80–101	✅ Confirmed	Extend: container-merge + update neighbor refs; Overwrite: full replace

---

Key Discrepancies Found

1. QuantityType (item 4a, SimulationBuilder.cs line 274): Code unconditionally sets target.QuantityType = incoming.QuantityType in Extend mode. If documentation states this is NOT modified in Extend, the code contradicts it.

2. IsFloating (item 4b, line 272): Same — unconditionally overwritten in Extend mode.

3. Observer Formula & ContainerCriteria (items 7b/7c): Because ObserverBuilder inherits from Entity (not Container), tryExtendContainers is a no-op for observers, and mergeObservers only updates the molecule list. Formula and ContainerCriteria are left unchanged from the base builder in Extend mode.

4. Parameter Value application order (item 9): The actual execution order is ExpressionProfiles → Individual → PV BBs — meaning Individual values override Expression Profile values for the same parameter (not the other way around). Depending on the documentation's phrasing, this may represent a subtle ordering discrepancy.

5. DescriptorCriteria accumulation (items 6b, 8c): In mergeDescriptorCriteria, conditions are appended (never removed) from the incoming builder. The operator is replaced, but conditions accumulate across merges. This could lead to unintended criteria expansion if not documented as additive.

[PavelBal]

3d - manual tests show: tags are extended in Merge behavior Extend, but overwritten (and old are removed) in Merge behavior Overwrite.

4a - Issue created Open-Systems-Pharmacology/OSPSuite.Core#2852
4b - Documentation sais: - Stationary: the property is always overwritten. All good.
6b - True for Extend only. In Overwrite, a passive transport is completely replaced by name
7b - confirmed bug Open-Systems-Pharmacology/OSPSuite.Core#2848
7c - did not check, but probably a bug
8c - could not check Events merge behavior because of this bug Open-Systems-Pharmacology/OSPSuite.Core#2850