fix(v2.0): Codex R16 — SmilesParser adoption + missing tests + bond-prop doc; 282/282

R16 reviewed Track B Cluster B; verdict NEEDS-FIXES with 3 [BUG]s.
All addressed before v2.0.0-rc3 tag.

# C-B1 [BUG]: SmilesParser adoption order

Pre-fix: smilesParser.C inserted empty Molecule into System first,
then tried to single-atom-adopt bonded orphan atoms one by one.
K0's canAdopt() rejects every such call (partners still in orphan).
Net countAtoms()=0 for every SMILES input.

Fix: build the Molecule off-system, insert atoms into it (no adopt
triggered), THEN insert populated Molecule into System →
adoptSubtree handles batched bonded migration. SmilesParser_test
WILL_FAIL quarantine removed.

# C-B4 [BUG]: 3 CORE_ONLY tests silently dropped

FragmentDB_test, DefaultProcessors_test, NormalizeNamesProcessor_test
were listed only in BALL_KERNEL_TESTS, not BALL_STRUCTURE_TESTS. The
CB-3 dedupe removed them from BALL_CORE_KERNEL_TESTS expecting
BALL_STRUCTURE_TESTS to cover them — but it didn't. CORE_ONLY
silently dropped 3 tests.

Fix: added to BALL_STRUCTURE_TESTS in
test/cmake/BALLTestExecutables.cmake.

# C-B7 [BUG] → DOCUMENTED: Bond PropertyManager bag not in JSON

K0.6 JSON serialises BondRecord (a, b, order, type, flags) but not
the Bond *handle's* PropertyManager bag. Affected: MMFF94SBMB,
MMFF94RBL, VIRTUAL__BOND, HBondProcessor annotations.

Most BALL workloads don't persist bond properties (set ephemerally
during force-field setup, recomputed on load). Shipping rc3 with
the gap documented is acceptable.

V21-BOND-PROPERTY-JSON filed as v2.1 backlog. Implementation needs
bond_back_ptr_[i] exposure in writer + Bond handle lookup in reader.

# Documentation

.planning/RELEASE-NOTES-v2.0.md: new 'Bond PropertyManager bag'
known-gaps subsection.

.planning/v2.x/K0-CODEX-REVIEW-ROUND16.md: full R16 review record.

# Verification

- CORE_ONLY ctest: 282/282 PASS (was 279/279 at Cluster B close;
  +3 from C-B4 restoration).
- 0 WILL_FAIL, 0 disabled.
- MoleculeStore_test 100-run stress: 0/100.
- Heap-System repro: exit 0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: okohlbacher

.planning/RELEASE-NOTES-v2.0.md

@@ -360,6 +360,20 @@ build-time concept.
 - **Tighter perf gates** — JSON I/O test gates (30s save / 30s load)
   are tripwires, not regression-pins. v2.1 should calibrate against
   CI machine variance.
+- **Bond PropertyManager bag — not in K0.6 JSON** (Codex R16 C-B7,
+  2026-05-19): the K0.6 JSON persistence path serialises the
+  per-atom + per-molecule + per-system PropertyManager bags, plus
+  the BondRecord `(a, b, order, type, flags)` tuple. The Bond
+  *handle's* `PropertyManager` bag (used by MMFF94 for `MMFF94SBMB`,
+  `MMFF94RBL`, MMFF94 bond-type discriminator; `VIRTUAL__BOND`
+  marker; HBondProcessor annotations) is NOT serialised. Most BALL
+  workloads don't use bond properties — they're set ephemerally
+  during force-field setup and computed fresh on every load. v2.1
+  backlog item: V21-BOND-PROPERTY-JSON. Implementation requires
+  exposing bond_back_ptr_[i] in the writer and finding the Bond
+  handle for property restore in the reader (Bond handle isn't
+  stored in the store; it's heap-owned by the AtomContainer
+  hierarchy).
 
 ## Things deferred to v2.1
 

.planning/v2.x/K0-CODEX-REVIEW-ROUND16.md

@@ -0,0 +1,100 @@
+# Track B Cluster B — Codex CLI Round 16 (2026-05-19)
+
+**Status:** Complete. Verdict: **NEEDS-FIXES → 2/3 [BUG]s fixed,
+1 documented → GO for v2.0.0-rc3**.
+
+**Reviewer:** Codex CLI 0.128.0 (`codex exec`).
+**Subject:** Track B Cluster B (commits `2d6a820e3` + `73a640a80`) —
+QSAR + STRUCTURE restore + MOLMEC AMBER/MMFF94 + SCORING
+COMPONENTS/FUNCTIONS + SOLVATION + DOCKING + remaining FORMAT.
+Cluster B brought CORE_ONLY ctest from 174/174 (rc2) to 279/279 (+105).
+
+---
+
+## Verdict (Codex, verbatim)
+
+> Final: NEEDS-FIXES
+>
+> Minimum rc3 blockers: fix SmilesParser adoption order, restore the
+> three missing CORE_ONLY tests into `BALL_STRUCTURE_TESTS`, and either
+> implement bond-property JSON persistence or explicitly downgrade
+> the K0.6 persistence claim before tagging.
+
+## Probe answers
+
+| # | Probe | Verdict | Status |
+|---|---|---|---|
+| C-B1 | SmilesParser_test failure root cause | [BUG] — K0 adoption-order break in public parser, not a harmless WILL_FAIL | **Fixed** (build off-system, insert populated) |
+| C-B2 | scoringComponent.C un-ifdef'd back-transform | OK with caveat | — |
+| C-B3 | scoringFunction.C un-ifdef'd SideChainOptimizer | OK | — |
+| C-B4 | Test dedupe silently lost 3 CORE_ONLY tests | [BUG] — FragmentDB/DefaultProcessors/NormalizeNamesProcessor missing | **Fixed** (added to BALL_STRUCTURE_TESTS) |
+| C-B5 | AssignBondOrderProcessor_test2 quarantine gate | OK | — |
+| C-B6 | Lock ordering / thread-safety regressions | OK; residual Bond::setOrder data race documented | — |
+| C-B7 | Bond-property JSON persistence | [BUG] — MMFF94/HBond bond annotations don't round-trip | **Documented** (v2.1 backlog V21-BOND-PROPERTY-JSON) |
+| C-B8 | sizeof(Atom)/sizeof(Bond) unchanged | OK — Sizeof_test still pins 360 B / 288 B | — |
+| C-B9 | Ship rc3 with SmilesParser WILL_FAIL? | [BUG] No — fix before tagging | **Fixed** via C-B1 |
+
+## Fixes applied this commit
+
+### C-B1 fix — SmilesParser adoption order
+
+`source/STRUCTURE/smilesParser.C` line 158-163:
+
+Pre-fix:
+```cpp
+Molecule* molecule = new Molecule;
+system_.insert(*molecule);                     // empty molecule into system FIRST
+for (atom : all_atoms_) molecule->insert(*atom); // single-atom adopt rejected
+                                                 //  by canAdopt — partners in orphan
+```
+
+Post-fix:
+```cpp
+Molecule* molecule = new Molecule;
+for (atom : all_atoms_) molecule->insert(*atom); // off-system: no adopt
+system_.insert(*molecule);                       // triggers adoptSubtree
+                                                 //  which migrates bonded atoms
+                                                 //  in a single batched pass
+```
+
+SmilesParser_test WILL_FAIL quarantine removed in test/CMakeLists.txt.
+
+### C-B4 fix — Restore 3 CORE_ONLY tests into BALL_STRUCTURE_TESTS
+
+`test/cmake/BALLTestExecutables.cmake`: added FragmentDB_test,
+DefaultProcessors_test, NormalizeNamesProcessor_test to
+BALL_STRUCTURE_TESTS. They had been listed only in BALL_KERNEL_TESTS,
+so when the CB-3 dedupe removed them from BALL_CORE_KERNEL_TESTS the
+CORE_ONLY build silently dropped them. Net: +3 tests.
+
+### C-B7 — Bond-property JSON gap documented (v2.1)
+
+`.planning/RELEASE-NOTES-v2.0.md`: new "Known gaps" subsection
+explicitly stating that Bond's `PropertyManager` bag is NOT
+serialised in K0.6 JSON. Affected: MMFF94 bond-type properties
+(MMFF94SBMB, MMFF94RBL), VIRTUAL__BOND markers, HBondProcessor
+annotations. Most workloads don't persist bond properties (they're
+ephemeral during force-field setup), so shipping rc3 with the gap
+documented is acceptable.
+
+V21-BOND-PROPERTY-JSON filed as v2.1 backlog item. Implementation
+needs: writer exposes `bond_back_ptr_[i]` to find Bond handle,
+serialises its PropertyManager bag inline with the bond record;
+reader finds the Bond handle after restore (Bond is heap-owned by
+the AtomContainer hierarchy, not in the store).
+
+## Verification
+
+- CORE_ONLY ctest: **282/282 PASS** (was 279 at Cluster B close;
+  +3 from C-B4 restoration; +0 from C-B1 since SmilesParser_test
+  was already counted but WILL_FAIL'd; net +3).
+- MoleculeStore_test 100-run stress: 0/100 fails.
+- Heap-System repro: exit 0.
+- 0 WILL_FAIL tests in CORE_ONLY mode. 0 disabled tests.
+
+## v2.0.0-rc3 readiness
+
+All R16 blocker items are either fixed (C-B1, C-B4) or explicitly
+documented with v2.1 backlog (C-B7). Cluster B closure is clean.
+
+**Verdict: GO for v2.0.0-rc3 tag.**

source/STRUCTURE/smilesParser.C

@@ -154,14 +154,23 @@ namespace BALL
 		// fill up empty valences with hydrogens
 		addMissingHydrogens();
 
-		// Transfer all atoms into a new molecule.
+		// 2026-05-19 (Codex R16 C-B1 fix): build the molecule
+		// OFF-system first, then insert the populated molecule INTO
+		// the system. The AtomContainer::insert(AtomContainer&) path
+		// triggers System::adoptSubtree which handles bonded multi-
+		// atom migration correctly. Pre-fix, this code inserted an
+		// empty molecule into the system first and then tried to
+		// single-Atom-adopt each bonded orphan atom one by one;
+		// K0's canAdopt() rejects every such call because each
+		// atom's bond partners are still in the orphan store. Net
+		// result was countAtoms()=0 on the parsed System.
 		Molecule* molecule = new Molecule;
-		system_.insert(*molecule);
 		for (Position i = 0; i < all_atoms_.size(); i++)
 		{
-			molecule->insert(*all_atoms_[i]);
+			molecule->insert(*all_atoms_[i]);   // off-system insert: no adopt
 		}
-		
+		system_.insert(*molecule);              // triggers adoptSubtree
+
 		// Clean up the pointers to these atoms.
 		all_atoms_.clear();
 	}

test/CMakeLists.txt

@@ -269,19 +269,13 @@ ENDIF()
 #   Previously WILL_FAIL'd in full builds. CB-3 brought rotamerLibrary.C
 #   into CORE_ONLY too, and re-tested — passes in both modes.
 #
-# SmilesParser_test — QUARANTINED (2026-05-19, TEST-SMILES-PARSER)
-#   getSystem().countAtoms() returns 0 instead of 5 for all SMILES
-#   inputs (line 76, 114). The SMILES parser produces no atoms —
-#   likely a v2.0 kernel interaction (Atom auto-adopt path from
-#   temporary parser-internal containers differs from v1.x).
-#   Files: source/STRUCTURE/smilesParser.C + smilesParserParser.C.
-#   SmartsParser_test PASSES with shared parser infrastructure, so
-#   the issue is SMILES-specific. Filed as v2.0.x backlog
-#   (TEST-SMILES-PARSER).
-SET_TESTS_PROPERTIES (
-	SmilesParser_test
-	PROPERTIES WILL_FAIL TRUE
-)
+# SmilesParser_test — QUARANTINE LIFTED (Codex R16 C-B1, 2026-05-19)
+# Root cause: smilesParser.C built atoms in orphan store with bonds,
+# inserted empty molecule into system FIRST, then tried to adopt
+# bonded orphan atoms one-by-one — K0's canAdopt() rejected each
+# (partners still in orphan). Fix: build molecule off-system, insert
+# atoms into it, THEN insert populated molecule into system →
+# adoptSubtree handles the bonded multi-atom migration correctly.
 
 IF (BALL_HAS_VIEW)
 	FOREACH(test ${VIEW_TESTS})

test/cmake/BALLTestExecutables.cmake

@@ -262,6 +262,13 @@ SET(BALL_STRUCTURE_TESTS
 	AtomBijection_test
 	BinaryFingerprintMethods_test
 	BindingPocketProcessor_test
+	# Cluster B step 3 (Codex R16 C-B4 fix): moved here from
+	# BALL_KERNEL_TESTS — the 3 STRUCTURE tests had been listed only
+	# in KERNEL group, so when CB-3 dedupe removed them from
+	# BALL_CORE_KERNEL_TESTS the CORE_ONLY build silently dropped them.
+	DefaultProcessors_test
+	FragmentDB_test
+	NormalizeNamesProcessor_test
 	ConnectedComponentsProcessor_test
 	DisulfidBondProcessor_test
 	Enumerator_test