Skip the TS normal mode displacement analysis when the ESS reports no…

[calvinp0]

Sits on #967. Reading order: #967 → #968 → #970. Both change the head of analyze_ts_normal_mode_displacement(), but the causes are independent — #967 is about coordinate frames, this one is about ESSs that report no modes at all. Either can be reverted without resurrecting the other's bug.

What breaks

• A single frequency job run with most of ARC's ESSs kills the entire run with an uncaught TypeError.
• analyze_ts_normal_mode_displacement() guarded the parse with except NotImplementedError — but no parser adapter ever raises it. Only 3 of ARC's 9 concrete adapters return real data (Gaussian, xtb, YAML); the other 6 (CFour, Molpro, Orca, Psi4, QChem, TeraChem) return the sentinel (None, None).
• (None, None) is not None, so the guard was dead code, execution fell through to normal_mode_disp[0], and subscripting None raised TypeError.
• Neither check_ts() call site in arc/scheduler.py is inside a try, so the exception propagates out of the scheduler to ARC.py::main(). One Orca frequency job was enough to end the run.
• The same defect sits at a second site, arc/job/trsh.py::trsh_negative_freq, which unpacked the sentinel and then evaluated len(normal_modes_disp) — TypeError: object of type 'NoneType' has no len(), reproduced against freq/orca_neg_freq_ts.out, freq/orca_example_freq.log, freq/orca6_example.out, freq/CH2O_freq_molpro.out, freq/C2H6_freq_QChem.out and freq/CH2O_freq_terachem.dat. It is reachable for any non-TS species: Scheduler.check_freq_job() and parse_composite_geo() both call troubleshoot_negative_freq() when a frequency job converges with an imaginary frequency and trsh_ess_jobs is on, and nothing on that path catches it either. Frequencies parse for those files while displacements do not (orca_neg_freq_ts.out reports 15 frequencies, minimum −1271.62 cm⁻¹), so the troubleshooter is entered on exactly the files whose displacements are missing.

What the fix does

• Adds get_normal_mode_displacement(), which centralizes the parse and returns the frequencies and displacements, or None — with a warning naming the ESS and the file — whenever no normal mode displacements can be obtained. It handles both the (None, None) sentinel and an empty displacement array.
• Applies that helper at both sites. trsh_negative_freq() now reaches its existing "Could not troubleshoot negative frequency" branch instead of raising, leaving the species to the ordinary ESS troubleshooter. The helper takes a log file path rather than a JobAdapter because trsh_negative_freq() receives a path, and it returns the frequencies alongside the displacements because that caller needs both.
• The helper lives in arc/parser/parser.py, beside parse_normal_mode_displacement() whose sentinel it interprets and beside determine_ess() which it calls. Both call sites already import that module on main — arc/checks/nmd.py as from arc.parser import parser, arc/job/trsh.py as from arc.parser.parser import ... — so one shared implementation is reached from both at module level, with no function-level import and no new import edge. Defining it in arc/checks/nmd.py instead would have made arc/job/trsh.py import arc.checks.nmd, an edge main does not have and one that closes a cycle, since nmd.py's own imports reach arc.job.trsh back through arc/__init__.py's eager package imports (CodeQL py/unsafe-cyclic-import).
• Treats "this ESS does not report normal modes" as analysis not performed (None), not TS rejected (False). An ESS that cannot report modes is telling us nothing about the TS, so it must not count as evidence against it.

How it was verified

• Import graph, measured not asserted. Every module's top-level imports were walked on origin/main and on this branch (following if TYPE_CHECKING and try/except blocks, excluding function bodies). The production import graph is identical — the only difference anywhere is in the test module arc/checks/nmd_test.py, which gains arc.checks.ts, arc.scheduler and arc.species.vectors. arc.parser.parser reaches neither arc.checks nor arc.job.trsh at module level, in either direction.
• Import order, both ways, in fresh interpreters run from an isolated directory (import arc.job.trsh first, import arc.checks.nmd first, import arc.parser.parser first): the helper resolves from arc.parser.parser in all three, arc/checks/nmd.py sees it, it returns None for orca_neg_freq_ts.out and a (15,) / (15, 7, 3) pair for TS_CH4_OH.log, and trsh_negative_freq() returns four empty lists for the Orca file and 2 conformers for the Gaussian one.
• 2 new tests in parser_test.py for the helper's own contract: the frequencies and displacements are returned unchanged for a Gaussian, a YAML and an xtb log; None — not the (None, None) sentinel — comes back for the six unreadable fixtures and for freq/yml_no_freqs.yml.
• 4 new tests in nmd_test.py at the analysis level: the analysis is skipped for an unsupported ESS and for a log with no normal modes; it still reaches a bool verdict for an xtb log; and a scheduler-level test that the TS is not discarded when its ESS reports no displacements — the behaviour that actually matters downstream.
• 1 test in trsh_test.py, over the six fixtures above, asserting trsh_negative_freq() returns its four empty lists rather than raising. Proved failing before the fix with the TypeError quoted above.
• arc/checks/ + arc/parser/parser_test.py + arc/job/trsh_test.py pass in full: 112 tests.

Still not addressed

• arc/checks/ts.py::get_rxn_zone_atom_indices() passes the same sentinel into get_rms_from_normal_mode_disp(). It is left alone because it needs a decision about what an empty reaction zone should mean for the caller, not a guard — unlike the two sites here, which both already had a "give up cleanly" branch that the crash was jumping over. It can now reuse the helper when it is fixed.

What I searched for

• Every parser adapter implementing parse_normal_mode_displacement, to establish what they actually return on the unsupported path — this is what showed the NotImplementedError guard could never fire.
• Whether an existing helper already normalized "parsed or not" for this parser call before adding one; whether any "is this ESS supported for this parse method" predicate or registry of adapter capabilities exists. None does — settings['supported_ess'] lists the programs ARC can run, not what each parser adapter can read.
• Every other caller of parse_normal_mode_displacement in the tree, which is how the trsh.py site was found; ts.py is the only remaining one.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (fix_nmd_freq_frame_geometry@4644b85). Learn more about missing BASE report.

Additional details and impacted files

@@                      Coverage Diff                       @@
##             fix_nmd_freq_frame_geometry     #968   +/-   ##
==============================================================
  Coverage                               ?   64.58%           
==============================================================
  Files                                  ?      119           
  Lines                                  ?    39661           
  Branches                               ?    10279           
==============================================================
  Hits                                   ?    25616           
  Misses                                 ?    11059           
  Partials                               ?     2986           

Flag	Coverage Δ
functionaltests	64.58% <ø> (?)
unittests	64.58% <ø> (?)

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]

Subsumed by #970. This branch's tip d1b67ea is a strict ancestor of #970 (verified with git merge-base --is-ancestor), so reviewing #970 already reviews this — the three NMD PRs were a linear stack in content while all three declared main as their base.

#970 is now restructured to three commits with no file touched twice, and this PR's content is split across the two it belongs in: arc/parser/parser.py (the dead except NotImplementedError, the (None, None) sentinel six shipped-fixture programs actually return, why not make_parser/raise_error, and the import-graph argument for homing the helper there) and arc/job/trsh.py (the len(None) TypeError, the six fixtures, and reachability via check_freq_job/parse_composite_geo for any non-TS species). The None vs False distinction — 'could not check' rather than 'checked and failed', so switch_ts() does not discard a converged TS — is preserved in the arc/checks/nmd.py commit.