Gate the reaction family choice on reaction direction and the radical before the label tie-break

[calvinp0]

Stacked on #979, which is stacked on #978. Read those first — #978 makes the wider family set reachable, #979 makes the resulting order deterministic, and this PR decides which of several matching families wins.

The problem

• More than one reaction family can match the same reaction. When that happens, product_dicts[0] wins.
• Make reaction family determination deterministic and honour a pinned family #979 made that order deterministic — but deterministic is not correct. The winner is currently decided by the family label's sort order, which has nothing to do with chemistry.
• That order is not neutral. RMG's naming puts concerted/pericyclic families early in the alphabet and radical-addition families late, so a radical reaction that also matches a concerted template loses to it by default.
• The choice is not cosmetic: the family drives the recipe used for reactive-bond identification, and it selects which TS-guess adapters run at all.

What this PR does

New prioritize_family_product_dicts, applied inside get_reaction_family_products before the existing label-sort tie-break:

1. Direction gate — if any match was found in the forward direction, drop the ones found in reverse. A reverse match's atom-label map is built against the flipped reaction, so its indices address the products; pairing it with a forward recipe yields bonds that do not exist in the reactant.
2. Radical gate — if the reactants carry unpaired electrons, prefer families whose recipe actually gains or loses a radical. A family with no radical bookkeeping is a closed-shell template that matched only because the radical sat out the graph transformation. Applied only if it leaves at least one candidate.
3. Tie-break — fewest bonds formed/broken, then fewest bonds changed, then the existing deterministic order from Make reaction family determination deterministic and honour a pinned family #979.

Both gates only ever choose among families that already matched, so they cannot invent a mechanism — only pick between valid ones.

Why this order

• The direction gate is first because it is a validity filter, not a preference: a reverse-discovered entry's label map is in the wrong index space whatever its chemistry, so it must not be weighed against a chemical criterion.
• The radical gate is second because it discriminates among entries that are all valid — it is a chemical preference, and it is applied only when it neither empties nor fails to narrow the candidate set.
• The recipe bond-change counts are last before the label sort because they are the weakest signal: they prefer the simpler elementary step when nothing stronger separates two families. Both counts were needed — formed/broken alone leaves H_Abstraction (2, 0) tied with families that differ only in changed.
• Bracketing: each of the three steps was removed in turn and a distinct test fails; and the agreement figure below falls if any is dropped. Nothing here is a tunable threshold — there is no magic number to pick, only the sequence.

Contract change worth flagging

• get_reaction_family_products previously returned every match; it now returns a filtered and ordered list, and the docstring says so. Measured on C=C[CH]CCC + CC=CCCC >> C=CC(CCC)C(C)[CH]CCC under 'all': 4 product dicts before (1 forward R_Addition_MultipleBond + 3 reverse Retroene), 1 after.
• This applies regardless of discover_own_reverse_rxns_in_reverse. linear.py is the one caller that passes that flag as True and it has dedicated handling for discovered_in_reverse; after this PR it no longer receives reverse-discovered entries whenever a forward match co-exists. That is intended — those entries are the ones whose label maps produce non-existent bonds — but it is a real change to a caller that opted in, and reviewers of linear.py should know.
• R_Addition_MultipleBond / Retroene is the single largest ambiguous combination in the benchmark pool (118 of the 281 ambiguous reactions), so the direction gate, not the radical gate, is the highest-volume change here.

Why we think it is right

The benchmark data records which family each reaction was originally generated from, which gives an independent check over the 281 ambiguous reactions:

	agrees with the recorded family
before	122 / 281
after	227 / 281

• Of the 130 reactions whose family changes, 110 move toward the recorded family, 5 away.
• That figure understates the improvement: 13 of the 15 changes that match no recorded family are the hex-5-enyl radical-clock class, where the recorded family is the wrong mechanism — Intra_RH_Add_Endocyclic reproduces the product connectivity by shifting a hydrogen with the radical as a spectator, which is not the reaction anyone means.

The 5 that move away are Ketoenol → intra_H_migration, and they are correct

This is the one result a reviewer should not have to re-derive, so the reasoning is here in full:

• Ketoenol/groups.py contains no u1 atom anywhere in its group tree. It is a closed-shell tautomerisation template, trained on closed-shell reactants, and it needs a separate Ketone_To_Enol family for the reverse direction. All 5 of these reactants are delocalised doublets with the spin density on the H-acceptor carbon. A template with no radical atom matching a radical reaction is precisely the failure mode the radical gate exists to catch.
• 4 of the 5 produce byte-identical formed and broken bonds either way — the two families differ only in changed, so the TS is the same and only the label moves.
• The 5th is symmetry-degenerate. For 2-hydroxyallyl radical ([CH2]C(=C)O), C0 and C2 are equivalent allyl termini, so H→C0 and H→C2 are the same 1,3-shift under a mirror. There the gate measurably wins: Ketoenol's atom map scrambles the four CH₂ hydrogens across both termini, giving formed 5 / broken 5, which no single imaginary mode can satisfy, against a clean 1 / 1 for intra_H_migration.
• The recorded labels are internally inconsistent for this pair: of the 8 Ketoenol/intra_H_migration ambiguities the record labels 5 as Ketoenol and 3 as intra_H_migration, while 6 of the 8 give identical bonds. The record tracks which Lewis structure RMG enumerated first, not a chemical distinction.
• intra_H_migration additionally gains AutoTST, which is not registered for Ketoenol.

Chemistry review: passed, ship as written. Two non-blocking follow-ups it raised, neither introduced by this PR: get_number_of_atoms_in_reaction_zone drops 4→3 under intra_H_migration, and intra_H_migration's empty changed list will interact with the NMD family-recipe work when the two meet.

Behaviour worth knowing

• Disproportionation → H_Abstraction moves 4 reactions from no TS adapters at all to heuristics/autotst/crest.
• Intra_2+2_cycloaddition_Cd → Intra_R_Add_* gains kinbot.
• Genuinely ambiguous pairs are left alone — Intra_R_Add_Endo/Exocyclic, Intra_RH_Add_Endo/Exocyclic, Cl_/H_Abstraction, H_Abstraction/Substitution_O all tie on recipe counts and still fall through to the deterministic order, as intended.
• Known limitation, deliberate: the radical gate tests multiplicity > 1, so a singlet biradical does not trigger it.

The direction gate runs before the recipes, and it outranks the reverse-discovery opt-in

Two things that an earlier revision of this PR got wrong, both fixed here.

The gates were silently abandoned by one unreadable groups.py. prioritize_family_product_dicts read every candidate family's recipe first and, on FileNotFoundError/KeyError/ValueError/InvalidAdjacencyListError, returned the list untouched at logger.debug. A correctness precondition that one unparsable file disables at debug level is not one. The salient asymmetry is that the direction gate needs no recipe at all — only discovered_in_reverse. So it now runs first, and an unreadable recipe costs only the radical gate and the bond-change ordering, which genuinely do need the recipes; that degradation is logged at warning. Running the gate first also means the recipes of the matches it drops are never read.

linear.py was asking for reverse-discovered matches it cannot use. arc/job/adapters/ts/linear.py:1738 passed discover_own_reverse_rxns_in_reverse=True into its wider family-set rescue. That flag is now inert — and not only in the obvious case. It admits only own_reverse reverse matches, and "own reverse" means the same template matches the forward direction too, so a forward match always co-exists and the gate always fires. Measured on the base: H_Abstraction CH4 + OH goes 4 → 6 dicts with the flag, intra_H_migration on [CH2]CCC goes 4 → 10; with the gate, both stay at 4 either way.

The gate is nevertheless right and the call site was wrong, because a reverse-discovered r_label_map indexes the flipped reaction:

CH4 + OH >> CH3 + H2O, reactant symbols [C,H,H,H,H,O,H], product symbols [C,H,H,H,O,H,H]
  path 3 [forward]  BREAK=[(0, 4)]  -> in reactant graph: True   in product graph: False
  path 4 [REVERSE]  BREAK=[(4, 5)]  -> in reactant graph: False  in product graph: True
  path 5 [REVERSE]  BREAK=[(4, 6)]  -> in reactant graph: False  in product graph: True

Every reverse-discovered BREAK_BOND pair is absent from the reactant graph and present in the product graph — those are the H₂O O–H bonds. Every consumer reads that map as reactant indices: get_expected_changing_bonds() at six sites in linear.py, and map_rxn, which calls find_all_breaking_bonds(r_direction=True) and then cuts the reactants with the result. arc/mapping/ contains no reference to discovered_in_reverse anywhere.

linear.py's three discovered_in_reverse sites do not close that gap:

• :246 — a _PathContext dataclass field. Plumbing.
• :1676 — a docstring claiming the split/cross classification is direction-agnostic. That claim is about a different axis: given indices already in the unimolecular species' space, graph membership tells you which bonds to stretch without knowing the direction. It says nothing about index space, and the in uni_bond_set test it describes is exactly what makes wrong-space indices quiet — they usually fail it and the path is dropped with no message. Corrected here rather than deleted.
• :3035 — _strategy_ring_scission uses the flag as a strategy trigger (discovered_in_reverse and bb and not fb), then consumes ctx.bb[0] as a reactant-space bond without translating it.

So the flag requested matches the adapter has no way to interpret, and the call now omits it. This matches feature_nmd_family_recipe, which returns None for reverse-discovered matches rather than emit wrong-index-space bonds, on the grounds that translating a reverse label map needs the very atom-mapping machinery the recipe path exists to bypass.

_strategy_ring_scission is unaffected: the gate drops reverse matches only when a forward match exists, and a reverse-only reaction — which is the ring-scission case — keeps all of them.

Checks

Rebased onto #979's head 9c3ca6aa, which itself now sits on origin/main at 44a6b112; the whole stack is 0 commits behind main. This PR's own contribution is byte-identical across that rebase — the added and removed lines of git diff <base> <head> are identical before and after; no hunk header, no content line, and no commit message character changed. The rebase auto-merged arc/job/adapters/ts/linear.py, where main's TS-guess identity change (869a1e2e) and this PR's removal of the discover_own_reverse_rxns_in_reverse kwarg both survive.

• Make the RMG family set configurable via settings, resolved in get_all_families #978's ordering guarantee still holds: its three mutations (reversed tiers / sorted across tiers / set dedup) each still fail its ordering test and Make reaction family determination deterministic and honour a pinned family #979's determinism test.
• Make reaction family determination deterministic and honour a pinned family #979's determinism still holds: get_all_families() is sha256-identical across PYTHONHASHSEED 0, 1, 2, 3, 7, 13.
• 16 tests in TestFamilyChoiceGates, each mutation-proved — removing either gate or the count tie-break fails a distinct test. test_a_radical_cyclization_is_not_labeled_as_a_cycloaddition exercises the real path end to end on pentadienyl cyclisation, where Intra_2+2_cycloaddition_Cd sorts ahead of Intra_R_Add_Exocyclic and the gate still resolves to the radical addition.
• Of the 5 added here, 3 fail on the previous revision: the direction gate surviving an unreadable recipe, the warning level, and not reading a dropped match's recipe. The other two pin the decision end to end — allyl + propene keeps only the forward R_Addition_MultipleBond over three reverse Retroene matches without the caller opting in, and CH4 + OH still drops both reverse matches with the caller opting in.
• linear_test.py and arc/mapping/: 215 passed.
• Full-suite parity against origin/main (44a6b112), re-run on the current head 4aed863b: 5 failed / 2761 passed / 36 skipped on the branch vs 5 failed / 2730 passed / 36 skipped on main — the same 5 on both, all arc/job/adapters/torch_ani_test.py, an environment issue. 0 added, 0 removed. (The branch passes 31 more tests because the stack adds that many.) Under -n 6 the branch additionally trips scheduler_test.py::test_initialize_output_dict; that test is order-dependent and fails identically on origin/main when run in isolation, so it is a pre-existing xdist worker-distribution artefact, not a regression — main at -n 8 trips a different pair (conformers_test, ts_test) for the same reason. (test_arc_families_path passes here; it fails only when the worktree path lacks the string ARC.)

Reuse before writing

Searched for an existing home for the direction check before touching it: discovered_in_reverse across the repo (only arc/common.py's dict comparison, linear.py's four sites, and family.py itself), and arc/mapping/ for any existing reverse-aware label-map translation — there is none, which is the finding that decided this. No new helper was added; the fix is a reordering, a log level, and a removed keyword argument.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.04%. Comparing base (e5a8386) to head (6d13f96).

Additional details and impacted files

@@                         Coverage Diff                         @@
##           fix_family_determinism_and_pinning     #982   +/-   ##
===================================================================
  Coverage                               64.03%   64.04%           
===================================================================
  Files                                     119      119           
  Lines                                   39510    39539   +29     
  Branches                                10260    10266    +6     
===================================================================
+ Hits                                    25300    25322   +22     
- Misses                                  11233    11241    +8     
+ Partials                                 2977     2976    -1     

Flag	Coverage Δ
functionaltests	64.04% <ø> (+<0.01%)	⬆️
unittests	64.04% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.