AUDIT F11 follow-up: support solvent in geometry_opt with required final single point

quantui/app_formatters.py

@@ -856,6 +856,21 @@ def format_past_result(data: dict[str, Any], result_dir: Optional[Path] = None)
     )
     _conv = "Yes" if data.get("converged") else "No (treat results with caution)"
     _cc = _converged_color(bool(data.get("converged")))
+    # AUDIT F07/F08/F15 (code review follow-up) — data["converged"] already
+    # folds in CC-amplitude (post-HF), TD-root (tddft), or Hessian
+    # (frequency) convergence on top of the reference SCF's own status,
+    # exactly like the live cards (format_result/format_freq_result/
+    # format_tddft_result). A bare "SCF converged" label here would blame
+    # the SCF for a CC/TD/Hessian-only failure whose reference SCF was
+    # fine — mirror each live formatter's label choice.
+    if ct == "frequency":
+        _conv_label = "Converged"
+    elif ct == "tddft":
+        _conv_label = "Converged"
+    elif data.get("cc_converged") is not None:
+        _conv_label = "Converged"
+    else:
+        _conv_label = "SCF converged"
     _gap = (
         f"{data['homo_lumo_gap_ev']:.4f} eV"
         if data.get("homo_lumo_gap_ev") is not None
@@ -877,7 +892,7 @@ def format_past_result(data: dict[str, Any], result_dir: Optional[Path] = None)
                 _gap,
                 _theme.css.TEXT_HEADING,
             ),
-            ("SCF converged", _conv, _cc),
+            (_conv_label, _conv, _cc),
             (
                 "SCF iterations",
                 (

quantui/backends/slurm_ingest.py

@@ -79,6 +79,8 @@ def _basic_result(payload: dict[str, Any], record: JobRecord) -> SimpleNamespace
         ccsd_correlation_hartree=payload.get("ccsd_correlation_hartree"),
         ccsd_t_correction_hartree=payload.get("ccsd_t_correction_hartree"),
         cc_converged=payload.get("cc_converged"),
+        td_converged=payload.get("td_converged"),
+        n_converged_states=payload.get("n_converged_states"),
         dispersion_applied=payload.get("dispersion_applied"),
         solvent=payload.get("solvent"),
         gpu_used=bool(payload.get("gpu_used", False)),

quantui/backends/worker.py

@@ -49,16 +49,23 @@
 
 _SUPPORTED_CALC_TYPES = frozenset(CALC_TYPES)
 
-# AUDIT F11 — the science APIs behind these calc types (run_freq_calc,
-# run_tddft_calc, run_nmr_calc, run_pes_scan, optimize_geometry) don't
-# accept a solvent argument at all, so request.solvent used to be silently
-# dropped rather than either applied or rejected. Only these two runners
-# actually thread request.solvent through to a PCM-capable API
-# (session_calc.run_in_session / reorganization_energy.run_reorganization_
-# energy, both real gas+PCM or PCM-single-point implementations — see each
-# runner below and reorganization_energy's own docstring for the
-# gas-phase-optimization + PCM-single-point approximation it documents).
-_SOLVENT_SUPPORTED_CALC_TYPES = frozenset({"single_point", "reorganization_energy"})
+# AUDIT F11 (additional concern, code review) — the science APIs behind
+# most of these calc types (run_freq_calc, run_tddft_calc, run_nmr_calc,
+# run_pes_scan, optimize_geometry) don't accept a solvent argument at all,
+# so request.solvent used to be silently dropped rather than either applied
+# or rejected. These three runners actually thread request.solvent through
+# to a PCM-capable API: session_calc.run_in_session for "single_point";
+# reorganization_energy.run_reorganization_energy for
+# "reorganization_energy" (its own docstring documents the gas-phase-
+# optimization + PCM-single-point approximation); and _run_geometry_opt for
+# "geometry_opt", which mirrors the interactive app's approximation —
+# optimize gas-phase, then run a required solvated single point on the
+# final geometry (see app.py's _run_required_final_single_point) — so the
+# app_runflow.py UI, which enables the solvent checkbox for these same
+# three calc types, is never lying about what a submitted job will do.
+_SOLVENT_SUPPORTED_CALC_TYPES = frozenset(
+    {"single_point", "geometry_opt", "reorganization_energy"}
+)
 
 
 def _write_progress(
@@ -358,7 +365,7 @@ def _run_geometry_opt(
             "attempt's checkpoint.",
         )
 
-    return optimize_geometry(
+    result = optimize_geometry(
         molecule=molecule,
         method=request.method,
         basis=request.basis,
@@ -371,6 +378,56 @@ def _run_geometry_opt(
         resume=resumable,
     )
 
+    # AUDIT F11 (additional concern) — mirror app.py's interactive
+    # "Geometry Opt" + solvent handling: optimize_geometry has no solvent
+    # argument, so a solvated result here means gas-phase optimization
+    # followed by a required PCM single point on the final geometry, whose
+    # energy/convergence replace the optimizer's last-step values. Without
+    # this, request.solvent for geometry_opt would either be rejected
+    # outright (AUDIT F11) or, if permitted, silently ignored.
+    if request.solvent:
+        _write_progress(
+            staging_dir, "running", "Running required solvated single point", 90.0
+        )
+        _append_log(
+            staging_dir,
+            "\n-- Required single-point (after geometry optimisation) "
+            "on optimized geometry --------------------------------",
+        )
+        from quantui.session_calc import run_in_session
+
+        sp_result = run_in_session(
+            molecule=result.molecule,
+            method=request.method,
+            basis=request.basis,
+            progress_stream=log_stream,
+            solvent=request.solvent,
+            scf_rescue=scf_rescue,
+        )
+        if not bool(getattr(sp_result, "converged", False)):
+            raise RuntimeError(
+                "Required post-optimization single-point did not converge."
+            )
+        _append_log(
+            staging_dir,
+            "Required single-point converged on optimized geometry.",
+        )
+        # ``energy_hartree`` is a read-only property derived from
+        # ``energies_hartree[-1]`` (there is no ``homo_lumo_gap_ev`` field on
+        # ``OptimizationResult`` either) — updating the last trajectory
+        # energy is how app.py's interactive path folds the solvated result
+        # in too, so both surfaces agree on what "the" final energy is.
+        sp_energy = getattr(sp_result, "energy_hartree", None)
+        if (
+            isinstance(getattr(result, "energies_hartree", None), list)
+            and result.energies_hartree
+            and isinstance(sp_energy, (int, float))
+        ):
+            result.energies_hartree[-1] = float(sp_energy)
+        result.converged = bool(result.converged) and bool(sp_result.converged)
+
+    return result
+
 
 def _run_frequency(
     request: CalculationRequest, staging_dir: Path, log_stream

quantui/backends/worker_payload.py

@@ -196,6 +196,17 @@ def tddft_result_payload(result) -> Dict[str, Any]:
         "scf_variant": getattr(result, "scf_variant", "") or None,
         # AUDIT F12 — was never serialized, though TDDFTResult carries it.
         "density_fit": bool(getattr(result, "density_fit", False)),
+        # AUDIT F08 (code review follow-up) — per-root convergence detail
+        # never left the worker process; a SLURM-submitted TDDFT run's
+        # History card could show only the folded "converged" bool, never
+        # the per-root tally format_tddft_result shows for an interactive
+        # run.
+        "td_converged": (
+            [bool(c) for c in result.td_converged]
+            if getattr(result, "td_converged", None) is not None
+            else None
+        ),
+        "n_converged_states": getattr(result, "n_converged_states", None),
         "spectra": {
             "uv_vis": {
                 "excitation_energies_ev": list(result.excitation_energies_ev),

quantui/orbital_visualization.py

@@ -592,20 +592,45 @@ def infer_charge_and_spin(
         spin = int(np.sum(np.isclose(occ, 1.0)))
         n_electrons = float(occ.sum())
 
+    # Code review — the ``pyscf.gto`` import and the ``load_ecp`` table
+    # lookup used to run once per atom instead of once per unique element,
+    # even though every atom of the same element gets the same answer. For
+    # a large cube-export molecule (the interactive "Generate" path this
+    # feeds) that's needless repeated work: hoist the import out of the
+    # loop and cache the per-element core-electron count.
+    _ecp_core_electrons: dict[str, int] = {}
+    _gto: Any = None
+    if basis:
+        try:
+            import pyscf.gto as _gto_mod
+
+            _gto = _gto_mod
+        except Exception:
+            # Match the pre-refactor behavior: any failure to even import
+            # pyscf.gto (not just a load_ecp lookup failure) falls back to
+            # the all-electron count rather than raising out of this
+            # function — infer_charge_and_spin has no PySCF hard dependency
+            # otherwise, and a caller (e.g. cube-export's Generate path)
+            # must not crash just because ECP data couldn't be resolved.
+            _gto = None
+
+    def _core_electrons_for(sym: str) -> int:
+        if not basis or _gto is None:
+            return 0
+        if sym in _ecp_core_electrons:
+            return _ecp_core_electrons[sym]
+        try:
+            _ecp_data = _gto.basis.load_ecp(basis, sym)
+            core_electrons = int(_ecp_data[0]) if _ecp_data else 0
+        except Exception:
+            core_electrons = 0
+        _ecp_core_electrons[sym] = core_electrons
+        return core_electrons
+
     nuclear_charge = 0
     for sym, _pos in mol_atom:
         z = ATOMIC_NUMBERS.get(sym, 0)
-        core_electrons = 0
-        if basis:
-            try:
-                from pyscf import gto as _gto
-
-                _ecp_data = _gto.basis.load_ecp(basis, sym)
-                if _ecp_data:
-                    core_electrons = int(_ecp_data[0])
-            except Exception:
-                core_electrons = 0
-        nuclear_charge += z - core_electrons
+        nuclear_charge += z - _core_electrons_for(sym)
 
     charge = int(round(nuclear_charge - n_electrons))
     return charge, spin

quantui/results_storage.py

@@ -177,6 +177,16 @@ def _opt_str_list(x: object) -> Optional[list]:
         return None
 
 
+def _opt_bool_list(x: object) -> Optional[list]:
+    """Coerce an optional iterable to a JSON-safe list of bools."""
+    if x is None:
+        return None
+    try:
+        return [bool(v) for v in x]  # type: ignore[union-attr, attr-defined]
+    except TypeError:
+        return None
+
+
 def _reorg_channels_payload(result) -> Optional[list]:
     """Serialise ReorgChannelResult objects, or None for other calc types.
 
@@ -352,6 +362,15 @@ def save_result(
         "ccsd_t_correction_hartree": _opt_float(
             getattr(result, "ccsd_t_correction_hartree", None)
         ),
+        # AUDIT F07/F08 (code review follow-up) — cc_converged/td_converged
+        # were only ever attributes on the in-memory result object, never
+        # persisted here, so a reloaded History card had no way to tell "no
+        # post-HF/TD-DFT correlation ran" (None) apart from "it ran and
+        # converged/didn't" (True/False) even though data["converged"]
+        # already folds one of these in. See format_past_result.
+        "cc_converged": getattr(result, "cc_converged", None),
+        "td_converged": _opt_bool_list(getattr(result, "td_converged", None)),
+        "n_converged_states": _opt_int(getattr(result, "n_converged_states", None)),
         # Persisted so the saved-result card matches the live card
         # (formatter-parity fix). Additive — absent on older results, where the
         # history card falls back exactly as before (CPU / no dipole / no

quantui/tddft_calc.py

@@ -63,12 +63,22 @@ class TDDFTResult:
         energy_hartree: Ground-state SCF energy in Hartrees.
         homo_lumo_gap_ev: HOMO-LUMO gap in eV from the ground-state SCF,
             or ``None``.
-        converged: ``True`` only when BOTH the ground-state SCF converged
-            AND (if excited states were requested and the solve ran) every
-            requested TD root converged (AUDIT F08) — an SCF-only flag is
-            not overall success for a calculation whose deliverable is the
-            excited states. See ``td_converged``/``n_converged_states`` for
-            the per-root detail this folds together.
+        converged: ``True`` when the ground-state SCF converged, the TD
+            solve ran and produced states, and — whenever per-root status is
+            available — at least one requested root actually converged
+            (AUDIT F08; relaxed in code review from "every root" to "at
+            least one root", since it is routine for a Davidson solve's
+            higher/harder roots to miss the default iteration budget while
+            the lower, physically relevant ones are fine). ``False`` only
+            for a solve that raised, returned no states, or (when
+            ``td_converged`` is known) converged *none* of them — an
+            SCF-only flag is not overall success for a calculation whose
+            deliverable is the excited states. A PySCF build that doesn't
+            expose ``td.converged`` at all (``td_converged is None``) is
+            treated as "no per-root information available", not as
+            "unconverged" — it does not by itself flip this to ``False``.
+            See ``td_converged``/``n_converged_states`` for the per-root
+            detail this folds together.
         n_iterations: Number of ground-state SCF macro-iterations.
         method: DFT functional or HF method used.
         basis: Basis set.
@@ -367,16 +377,30 @@ def _run_tddft_calc_body(
             except Exception:  # noqa: BLE001 — cleanup (stream may be closed)
                 pass
 
-    # AUDIT F08 — overall success requires every requested root to have
-    # actually converged, not just the ground-state SCF. A TD-DFT run whose
-    # entire purpose is the excited states is not "converged" if the
-    # Davidson solve raised before producing any roots, or if it returned
-    # roots that never converged.
+    # AUDIT F08 — overall success requires the TD solve to have actually
+    # produced converged roots, not just a ground-state SCF. A TD-DFT run
+    # whose entire purpose is the excited states is not "converged" if the
+    # Davidson solve raised before producing any roots, or if none of the
+    # roots it returned ever converged.
+    #
+    # Code review (2026-09): requiring *every* requested root to converge
+    # was too strict — a Davidson solve for nstates > a few routinely leaves
+    # the higher/harder roots short of the default iteration budget while
+    # the lower ones (usually what a UV-Vis analysis actually cares about)
+    # are fine, so a perfectly normal multi-state run was always flagged
+    # "treat with caution". Relaxed to "at least one requested root
+    # converged" — still catches the original bug this audit fixed (a
+    # solve where every root came back unconverged), and n_converged_states
+    # below still reports the exact per-root tally for anyone who wants it.
+    #
+    # Separately, when the installed PySCF doesn't expose ``td.converged``
+    # at all, td_converged stays None — that is "no information", not
+    # "unconverged", and must not by itself force converged=False (it used
+    # to, forcing every TD-DFT/TDHF result on such a build to be flagged).
     converged = (
         scf_converged
         and excitation_energies_ev != []
-        and td_converged is not None
-        and all(td_converged)
+        and (td_converged is None or any(td_converged))
     )
 
     return TDDFTResult(