fix: kekulize aromatic ions (-9986) via charge-center relaxation

[fbaensch-beilstein]

Summary

Fixes Error -9986 (Cannot process aromatic bonds) for aromatic ions supplied with MOL aromatic (type-4) bonds:

• cyclopropenyl cation (C₃H₃⁺)
• cyclopentadienyl anion (C₅H₅⁻)
• tropylium / cycloheptatrienyl cation (C₇H₇⁺)

Refs #154, #82.

Root cause

InChI has no "aromatic" bond order; it kekulizes type-4 bonds into alternating single/double bonds via a balanced-network search (BnsAdjustFlowBondsRad). The kekulizer requires every aromatic ring atom to take exactly one localized double bond — a perfect-matching constraint that is only satisfiable for an even number of participating atoms. In an aromatic ion the charge-bearing ring atom contributes to the π system via an empty orbital (cation) or lone pair (anion), not a double bond, so no perfect matching exists and the conversion fails with BNS_ALTBOND_ERR.

Approach

The fix lives in the BNS layer (mark_alt_bonds_and_taut_groups, ichi_bns.c), not at MOL-parse time. On the first kekulization's BNS_ALTBOND_ERR, it relaxes the 2-coordinate charged aromatic ring centers — chem_bonds_valence-- paired with num_H++ (valence- and charge-neutral; moves one valence unit from "double bond" to implicit H) — then rebuilds the network (ReInitBnStruct) and retries the conversion once. If it still fails, it errors exactly as before.

Why this placement: a parse-time per-atom valence correction was prototyped first and rejected — it acts before InChI's charge resolution and mis-fires (it regressed N-oxides and aromatic radicals). The deferral approach is inherently non-regressing: any structure that already kekulizes succeeds on the first pass and never reaches the retry, so neutral and net-zero (e.g. N-oxide) aromatics are untouched.

Guard: charge != 0 && num_H == 0 && valence == 2 && chem_bonds_valence > valence && nBondsValToMetal == 0.

Expected outputs

• InChI=1S/C3H3/c1-2-3-1/h1-3H/q+1
• InChI=1S/C5H5/c1-2-4-5-3-1/h1-5H/q-1
• InChI=1S/C7H7/c1-2-4-6-7-5-3-1/h1-7H/q+1

Scope / known limitations (tracked as strict xfail)

This is the charged-ion increment, not the full "rework aromaticity detection" of #154:

• Cyclopentadienyl radical (C₅H₅•) — a neutral odd ring; fails earlier in the pre-existing FIX_AROM_RADICAL path. Pre-existing failure (verified failing on the base commit too), not a regression. Out of scope here.
• Ferrocene — its Cp carbons are metal-bonded (excluded by the nBondsValToMetal guard). Investigation showed that even forcing −1 onto the rings is insufficient: after metal disconnection the ring relaxation fires but the kekulization retry still fails for a second, independent reason in the disconnection path. Needs the organometallic/aromaticity rework.

Both are marked xfail(strict=True) so they flip loudly if a future change fixes them.

Tests

INCHI-1-TEST/tests/test_executable/test_aromatic_ions.py:

• the three ion InChIs assert success;
• pyridinium asserts unchanged output (guards against over-relaxation of N⁺, which legitimately keeps its ring double bond);
• cyclopentadienyl radical and ferrocene are strict xfail.

Error checks read result.log (the file-mode CLI writes diagnostics to the log file, not stderr).

Verification

• Builds cleanly for both targets (CLI + libinchi).
• C++ unit tests: 15/15.
• Full executable pytest: no new failures.
• Regression suite (config_ci, 4190 structures): 0 unexpected diffs; the neutral corpus is byte-identical and two structures that an earlier prototype regressed (an N-oxide and a phenyl radical) are unaffected.

Draft notes for reviewers

• The fix is deliberately charge-only and 2-coordinate-only; radicals are left to the existing FIX_AROM_RADICAL mechanism / the rework.
• Single retry → guaranteed termination; over-relaxed inputs still fail the post-check honestly rather than producing a wrong structure.

[github-actions]

Unit Test Coverage Report

Coverage Regression Summary

Metric	PR (%)	Base (%)	Difference (pp)
Lines	18.6	18.6	+0.00
Branches	12.3	12.3	+0.00

Find details on the base coverage at https://iupac-inchi.github.io/InChI/coverage/index.html

Find details on this PR's coverage by downloading coverage-reports-28797613916 and opening html/index.html

[schatzsc]

@fbaensch-beilstein What is the effect of this on the representation in the "internal chemical object"?!?

You need to make sure that the result is independent of which atom in the odd-membered ring initially carried the charge in the molfile, as otherwise you will "hallucinate" isomers where there are none.

For example, let's assume you have the methyl cyclopentadienyl anion [C5H4CH3]- then the resulting InChI string has to be the same for and independent of whether the (methyl-atom carrying) C1 atom had been assigned the -1 charge or C2/C5 or C3/H4.

Same also applies to all other odd-membered rings.

[fbaensch-beilstein]

@schatzsc That's why this is just a draft, the solution is not sufficient so far.

[fbaensch-beilstein]

Superseded by #248, which takes a different approach to the same -9986 failure: instead of relaxing the charge center, it restores the original flexible type-4 bonds and rebuilds the balanced network, so charge position no longer matters and metal-bonded rings resolve in place. The test_aromatic_ions.py cases from this branch are carried over there.

Closing in favour of #248. Branch fix/aromatic-ion-valence is kept for reference.