fix: preserve explicit H/D/T polymer crossing-bond end groups

[nnuk]

Summary

• Supports explicit H/D/T atoms as polymer leaving groups / crossing-bond endpoints.
• Prevents polymer-boundary H/D/T from being removed during terminal-H processing when /Polymers is enabled.
• Fixes the polymer representation for the structure reported in Polymers with explicit H atoms as leaving group are not supported #252.
• Unit test added.

Previous behavior

The structure from #252 was rejected with:

Error 9002: Hydrogen as polymer end group is not supported

New behaviour

The new behaviour is the expected result

InChI=1B/C2H6O2/c3-1-2-4-5/h3H,1-2H2/z101-1-2,4(5-4,3-1)

Type of change

• feat — new feature (minor bump)
• fix — bug fix (patch bump)
• perf — performance improvement (patch bump)
• BREAKING CHANGE — incompatible change (! in title or footer; major bump)
• refactor / docs / chore / build / ci / test (no release)

Checklist

• PR title follows Conventional Commits
• Builds cleanly for both targets (./INCHI-1-TEST/build_with_cmake.sh all)
• Compiles without new warnings on GCC/Clang/MSVC where applicable
• Code follows .editorconfig (4-space indent, LF, no trailing whitespace)
• Unit tests pass (ctest in the full_build test dir)
• CLI tests pass (pytest INCHI-1-TEST/tests/test_executable)
• New behavior is covered by tests
• Public API / behavior changes are documented (Doxygen in headers + README/docs if applicable)

Related issues

Closes #252

[github-actions]

Unit Test Coverage Report

Coverage Regression Summary

Metric	PR (%)	Base (%)	Difference (pp)
Lines	23.3	21.8	+1.50
Branches	15.4	14.4	+1.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-32359601905 and opening html/index.html

[fbaensch-beilstein]

Review

Verdict: merge with fixes. No correctness, memory-safety, or compatibility defect found. Verified by building both targets, ctest (19/19, also under Debug+ASAN), the pytest CLI suite (15 passed / 3 skipped / 10 xfailed), and a differential run over the 2190-structure InChI_TestSet_ext.sdf — output is byte-identical to dev under default options, -Polymers, and -Polymers -NPZz -FoldCRU.

What's solid

• Root-cause fix in the right place. The mask is threaded into the shared remove_terminal_HDT (strutil.c:3745) with NULL as the "no polymer" default, rather than special-casing the polymer call path. All five call sites (ichinorm.c:5808,5836,7187, ichimake.c:3896, inchi_dll_a2.c:1254), both Create_INChI callers (runichi.c:1977, ichirvr1.c:5326) and the single Normalization_step caller (inchi_dll_a2.c:791) were updated — nothing missed, no demo/e_* copy affected.
• Index reconstruction is correct. BuildPolymerCrossingBondEndpointMask (runichi2.c:360-422) walks orig_inp_data->at filtering on .component, exactly matching ExtractConnectedComponent (strutil.c:5481-5516), so local_idx cannot exceed the allocated num_at. out_at is a straight memcpy of inp_at (ichimake.c:3862) with no reordering in between, so mask indices still line up at the point of use. Confirmed on an 11-atom, 2-component structure with the polymer as component 2.
• Memory handling is sound. Ownership sits on INP_ATOM_DATA and is released in FreeInpAtomData (mol2atom.c:1186-1193), which CreateInpAtomData calls first, so the per-component loop cannot leak. ASAN run of test_polymers reports no leak and no OOB. D and T endpoints work (/i5+1, /i5+2), and the mixed case (extra explicit H removed, endpoint H kept) is correct.

Should fix

1. Commented-out validation left in place — mol2atom.c:1569-1576, runichi3.c:1838-1854. Both rejection checks are wrapped in /** ... */ with an "Addressed" note instead of being deleted. Git history is the record; please delete the blocks. Related: error code 9002 is now only reachable via the unrelated message at runichi3.c:1564 ("Single polymer unit may not be RAN/ALT/BLO") — worth a comment or a renumber.

2. Test coverage is a single happy path — test_polymers.cpp:33-59 is exactly the issue structure. Three gaps:

   • No multi-component case. The component-local index reconstruction is the only genuinely subtle part of this change and nothing exercises it; someone "simplifying" the loop to use the global index would pass CI.
   • No D/T case. They work, but nothing locks it in.
   • No negative case. remove_terminal_HDT sits on the path of every structure; there is no test asserting a non-polymer structure with explicit H is unaffected.

   The multi-component one is the important one.

3. Commit messages don't follow Conventional Commits — 877f753 and af75176 carry no fix: / docs: prefix. These feed release-please; either amend or ensure the squash-merge title is conventional.

4. Unrelated whitespace churn bundled into the functional commit. Of the 192 changed lines in runichi2.c, roughly 85 are the new function and the GetOneComponent hook; the rest is trailing-whitespace/reindent noise. Same in inchi_dll_a2.c:341-378, plus a no-op at runichi3.c:1574 (u->nb != 2 ) → u->nb != 2)), removed debug comments at runichi.c:330-345, and a reworded comment at runichi.c:380-384. All .editorconfig-aligned and harmless, but it triples the review surface and destroys git blame on those regions — please split into a separate style: commit.

Minor

5. runichi2.c:353-356 — Doxygen @param tags name types (INP_ATOM_DATA, ORIG_ATOM_DATA, INPUT_PARMS) rather than parameter names (inp_cur_data, orig_inp_data, ip). Doxygen will warn and drop them, and docs are auto-published from dev.
6. runichi2.c:404-411 — the mask marks every crossing-bond endpoint, including heavy atoms and star atoms, whose bits remove_terminal_HDT can never consult (the check sits inside the k < kMax H/D/T branch). Harmless but obscures intent; an element test when setting the bit would document it and skip the scan in the common case.
7. test_polymers.cpp:26-30 — ASSERT_NE(poutput->szInChI, nullptr) returns before FreeINCHI(poutput), so a failing assertion produces a gtest failure plus an ASAN leak report.
8. mol2atom.c:1571 — typo "Adressed". Also @nnuk author markers in shipped source (runichi2.c:340, strutil.c:3836) — attribution belongs in git, though there is precedent in these files.
9. CHANGELOG.md:10-12 — accurate but incomplete: it doesn't state the user-visible consequence, that the retained H becomes a numbered atom in the /c layer rather than an implicit hydrogen, and that structures previously rejected with "Hydrogen as polymer end group is not supported" now succeed.

Beyond the diff

• No polymer regression coverage exists. InChI_TestSet_ext.sdf contains no M STY records at all, so the byte-identical differential run above proves only that non-polymer structures are untouched — it says nothing about polymers. Before release, a polymer SDF should land in the regression config; right now one unit test is the entire safety net for this feature.
• Behavioural note worth documenting (follows from the accepted design, not a defect): a retained endpoint H is no longer treated as a hydrogen by normalization — it cannot be a mobile H and does not participate in tautomer perception or charge migration on its neighbour. For an -OH or -COOH polymer end drawn with an explicit H, mobile-H perception therefore differs from the same end drawn with an implicit H. Inherent to the approach, but should be stated somewhere user-facing.
• Untested interaction, low confidence: ichinorm.c:5808/5836/7187 (Underivatize, Ring2Chain) pass NULL, so those transforms would strip an endpoint H while blist still uses pre-removal numbering. -Polymers -Underivatize and -Polymers -Ring2Chain on the #252 structure give the correct result, but only because neither transform fires on that input. Worth either a guard or a deliberate "not supported in combination" note.

The InChI2Struct round-trip behaviour for polymer H-caps was treated as out of scope for this PR and is not covered above.

[fbaensch-beilstein]

Line-anchored follow-ups to the review above — same points, placed where they apply.

[fbaensch-beilstein]

Approving. The review findings that mattered are addressed:

• Commented-out validation removed from mol2atom.c and runichi3.c; 9002's remaining single use is documented at runichi3.c:1564.
• Doxygen @param tags corrected.
• Mask now skips non-H/D/T endpoints and short-circuits entirely when a component holds no explicit H (runichi2.c:378-392).
• Test helper no longer leaks on assertion failure.
• CHANGELOG states the /c-layer consequence.

Verified at 69b1981: syntax-clean for both TARGET_EXE_STANDALONE and TARGET_API_LIB across all touched translation units, with no new warning classes against dev under -Wall -Wextra. Behaviour checked by hand — the #252 structure, its D (/i5+1) and T (/i5+2) variants, a multi-component structure with the polymer as component 2 (endpoint H retained, the other component's explicit H correctly removed), and the negative cases (no polymer markup / no -Polymers, both unchanged 1S output). The merge of dev at 6fb414e is a clean auto-merge — its diff is byte-identical to 8e8c33f..7c246aa, no PR file touched.

Non-blocking, worth a follow-up:

• The test suite is still the single happy-path case. The multi-component, D/T and negative cases all pass but nothing locks them in, and the component-local index reconstruction is the one genuinely subtle part of this change.
• The regression dataset contains no M STY records at all, so nothing exercises the polymer path at dataset level.
• ichinorm.c:5808/5836/7187 (Underivatize, Ring2Chain) pass NULL for the mask; neither transform fires on the #252 input, so the combination is untested rather than known-good.

Please make sure the squash-merge title carries the fix: prefix — none of the five commits have one.