Seed the flipped reaction with a family so the reverse-discovery retry can map it

[kfir4444]

Problem

7885a456 ("Map reactions in the direction their family was discovered in") correctly recognizes a
reaction whose every template was discovered in reverse and routes it to the flip retry:

if rxn.product_dicts and all(product_dict.get('discovered_in_reverse', False)
                             for product_dict in rxn.product_dicts):
    return map_reaction(rxn, backend=backend, flip=True)

The detection is right — such a template's products are isomorphic to the reaction's reactants, and
both get_template_product_order and reorder_p_label_map compare template products against
rxn.p_species, so a forward attempt is doomed.

But the flip retry it hands off to cannot map them:

if flip:
    raw_map = try_mapping(rxn.flip_reaction())

ARCReaction.flip_reaction deletes family from the reaction dict (reset_keys), so the flipped copy
re-derives its product dictionaries lazily with the default family set. A reaction whose family was
only matched by a broader set — the usual situation for a template discovered in reverse — comes back with
zero product dictionaries and no family, and the retry has nothing to map with. The branch above returns
unconditionally, so the forward attempt is not tried either.

Concretely, for C2H4F2 <=> CH2CHF + HF:

forward   family=XY_Addition_MultipleBond  discovered_in_reverse=True
          template products ['CC(F)F'] -> isomorphic to the *reactant*, not the products
flipped   product_dicts (default set) = 0   -> family None

Measured over the 452 reactions of testing/errs_no_li.yml, 43 have a family that is discovered only in
reverse and is not its own reverse. On main, 6 of those 43 map.

Fix

Give the flipped reaction a usable, forward-discovered template before mapping it. New
prepare_flipped_reaction widens the family search when the lazy default finds nothing usable, prefers a
template that describes the flipped reaction forward, and seeds family / family_own_reverse /
product_dicts onto the copy.

     if flip:
-        raw_map = try_mapping(rxn.flip_reaction())
+        raw_map = try_mapping(prepare_flipped_reaction(rxn))

The detection block from 7885a456 is unchanged; this only makes the path it delegates to work. Nothing
else in the mapping pipeline is touched, and the forward path is byte-identical.

Test

Two new tests in arc/mapping/driver_test.py:

• test_prepare_flipped_reaction_seeds_a_forward_template — the flipped reaction gets a family, its
  template is forward-discovered, and its template products are isomorphic to its own products.
• test_map_reaction_with_a_reverse_discovered_template — C2H5Cl <=> C2H4 + HCl
  (XY_Addition_MultipleBond, discovered in reverse) maps to a valid permutation.

Before (this branch's tests against main's arc/mapping/driver.py):

E       NameError: name 'prepare_flipped_reaction' is not defined
arc/mapping/driver_test.py:873: NameError
>       self.assertIsNotNone(atom_map)
E       AssertionError: unexpectedly None
arc/mapping/driver_test.py:899: AssertionError
2 failed in 4.40s

After:

2 passed in 4.89s

Full suites: arc/mapping/ and arc/reaction/ → 133 passed.

Corpus effect

Same script over all 452 reactions of testing/errs_no_li.yml, map_reaction with
rmg_family_set='all', 45 s per reaction:

	mapped	of the 43 reverse-discovered
main (c4c2db9)	294 / 452	6 / 43
this branch	331 / 452	43 / 43

All 43 reactions whose template is discovered only in reverse now map, and no reaction that mapped before
stops mapping. The families recovered are XY_Addition_MultipleBond (32), plus
halocarbene_recombination and R_Addition_MultipleBond cases — HX eliminations such as
C2H5Cl <=> C2H4 + HCl and C2H3F3 <=> CH2CF2 + HF, which RMG matches only as additions.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.75%. Comparing base (09e9ede) to head (71a8239).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1023      +/-   ##
==========================================
- Coverage   64.76%   64.75%   -0.02%     
==========================================
  Files         119      119              
  Lines       40039    40061      +22     
  Branches    10350    10352       +2     
==========================================
+ Hits        25932    25942      +10     
- Misses      11129    11141      +12     
  Partials     2978     2978              

Flag	Coverage Δ
functionaltests	64.75% <ø> (-0.02%)	⬇️
unittests	64.75% <ø> (-0.02%)	⬇️

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.

[calvinp0]

flipped.family is taken from product_dicts[0], but the whole list is assigned and get_reaction_family_products concatenates matches across all 99 families with no grouping. If two families amtch, map_rxn's pdi retry pairs family A's recipe with family B's label map then reads product_dicts[pdi]['r_label_map']).

Both use *1/*2/*3, so it succeeds against the wrong atoms and check_atom_map_and_return only checks it's a permutation.

[kfir4444]

Confirmed, and thank you — this is a genuine defect, not a nit. prepare_flipped_reaction assigns the whole list but takes family from product_dicts[0], so map_rxn's pdi retry can pair one family's recipe with another's r_label_map, and since both use *1/*2/*3 it succeeds against the wrong atoms exactly as you describe.

Measured over the 452-reaction corpus: 7 of 335 flipped reactions carry product_dicts spanning more than one family — e.g. C10H11 <=> C10H11-2 matching Intra_Diels_alder_monocyclic + Intra_R_Add_Endocyclic + Intra_R_Add_Exocyclic.
restrict_product_dicts_to_family() from #978 is precisely the right fix. Two options: I add an interim single-family restriction here so this can land independently, or I rebase onto #978 and call your method. I'd prefer the latter — one implementation, not two. Your call on ordering.

[calvinp0]

Could we log these at debug? get_reaction_family_products logs its onw skips that way so it'd match the neighbourhoo and right now a failed widening is indistinguishable from "this reaction has no family" until it surfaces as a generic could not be atom mapped much later.

[kfir4444]

Agreed — a failed widening should be distinguishable from "no family at all". Moving to debug to match get_reaction_family_products.

[calvinp0]

not any(not x) is all(x), empty list included:

    if all(pd.get('discovered_in_reverse') for pd in product_dicts):

[kfir4444]

Correct, and equivalent on the empty list too. Simplifying to all(...).

[calvinp0]

Worth asserting the label? assertEqual(flipped.family, 'XY_Addition_MultipleBond') would catch the 'all' search settling on a different family, which assertIsNotNone can't distinguish from success.

[kfir4444]

Agreed, assertIsNotNone is too weak — it can't tell success from 'all' settling on a different family. Adding assertEqual(flipped.family, 'XY_Addition_MultipleBond').

[kfir4444]

@calvinp0 — all four addressed in fb9ca52, thank you. The first one was a real defect, and I measured it before fixing: over the 452-reaction corpus, 7 of 335 flipped reactions carry product_dicts spanning more than one family, e.g.

C10H11 <=> C10H11-2   Intra_Diels_alder_monocyclic + Intra_R_Add_Endocyclic + Intra_R_Add_Exocyclic
C9H9-11 <=> C9H9-12   Intra_2+2_cycloaddition_Cd  + Intra_R_Add_Exocyclic
C10H9  <=> C10H9-2    Intra_ene_reaction          + intra_H_migration

I've restricted to the chosen family inline, with a TODO to swap in restrict_product_dicts_to_family() once #978 lands — one implementation rather than two.

---

Reading #978 properly: these two are converging on the same defect from opposite ends, and I think they should be sequenced rather than raced.

Same root cause. Your §4 finding — "arc/mapping/ contains no reference to discovered_in_reverse at all", and every consumer reads a reverse map's indices as reactant indices — is exactly what this PR fixes on the mapping side. Your direction gate drops reverse matches when a forward match exists; this handles the reverse-only case, which your gate deliberately leaves alone ("a reverse-only reaction keeps all of them"). They compose. Corpus effect here: 6/43 → 43/43 on affected reactions, 294 → 331 of 452 overall, almost entirely XY_Addition_MultipleBond HX eliminations that RMG matches only as additions.

Your determinism fix shows up in my data. I hit C10H9 <=> C10H9-2 reporting intra_H_migration on one sweep and Intra_ene_reaction on another and wrote it off as unstable ordering. It is the third row in the table above — genuinely two families matching — and your recommended.py ordering plus the PYTHONHASHSEED work is the fix. Independent corroboration from a different workload.

The OverflowError too, probably. My sweeps hit exactly one OverflowError across 452 reactions. Given your make_bond_changes charge-separating analysis that is likely the same one, though I have not checked the traceback — happy to confirm and tell you which reaction if it is useful evidence for your guard.

Convergent insight worth unifying. You use fewest formed/broken/changed bonds as a family tie-break, and your Ketoenol case notes the scrambled map gives 5/5 against 1/1. In #1024 I use the same signal as a map validity filter — Diels-Alder of butadiene with ethene gives 2 changed bonds for the correct map and 10–14 for scrambled ones, each of which otherwise opens a spurious reaction channel. Same observation, two applications; probably one helper.

Proposed order: #978 first — it is larger and more foundational — then #1023 rebases onto it and drops the inline restriction in favour of your method, then #1024. Worth flagging that your direction gate sits directly upstream of #1024: enumerate_atom_maps sweeps every product_dict, so your gate reduces what I enumerate (good — fewer scrambled maps reaching the validity filter) but shifts its corpus numbers, which I will re-measure once #978 is in. Happy to do the rebasing on both.