fix: precision-correctness audit findings (F6/F11/F2/F1) (#74)

* fix: precision-correctness audit findings F6/F11/F2/F1

F6 (cli/main.py) — CLI batch `calc` serialized the extrapolation limit with bare
str(mpf) outside any precision_guard, truncating a requested high-precision (e.g.
50-digit) result to the ambient mp.dps (~15). Wrap compute + serialization in
precision_guard(job.precision) and render via mp.nstr at that precision.

F11 (app_desktop/workspace_controller.py) — workspace save persisted config.common
(mpmath_precision, uncertainty/display digits, scientific) and config.latex (input
digits, group size, dcolumn) but restore never read them back, silently resetting
compute-affecting precision to defaults on reload. Add _restore_common_config,
called from _restore_workspace_contents.

F2 (datalab_core/mcmc_refine.py) — the Gaussian log-probability divided the
already sigma-weighted residual sum (Σ rᵢ²/σᵢ²) by rmse² again, double-scaling the
likelihood and corrupting credible-interval WIDTHS for any weighted fit. Extract
_gaussian_log_probability: -0.5·residuals_sq when weighted (weights already 1/σ²),
keep the /rmse² plug-in variance only in the unweighted branch.

F1 (fitting/hp_fitter.py) — _compute_covariance took mp.sqrt of a covariance
diagonal guarded only for NaN, so a negative diagonal (finite-precision indefinite
inverse) yielded a complex mpc that crashed combine_error_components. Add
_error_from_variance (negative/NaN/Inf → NaN) and flag negative variances in the
covariance warning, so the fit degrades gracefully instead of crashing.

Each fix is TDD'd (pure-function unit tests + workspace round-trip + CLI calc
high-precision). Neighborhood regression green (218 tests).

* fix(workspace): restore LaTeX caption + engine on reload; stop mp.dps leak in test

Review follow-ups on the F11 precision/round-trip fix:

- workspace_controller._restore_common_config captured config.latex.caption and
  config.latex.engine at save (lines 1125-1126) but never read them back, so a
  saved caption reset to empty and the TeX engine reverted to the "tectonic"
  default on reload — the same "captured but never restored" class F11 set out
  to fix, just incomplete for these two fields. Restore both, and extend the
  round-trip test to set and assert them (it previously only covered the
  precision/digit fields, so the gap was uncaught).

- test_cli_batch set mp.dps = 15 to simulate ambient CLI precision but never
  restored it; since mp.dps is process-global that leaked into later tests.
  Wrap the mutation in try/finally.

ruff clean; 148 workspace/cli tests pass. Round-trip test is RED without the
restore fix (caption not preserved), GREEN with it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(mcmc): index likelihood_weights via the None-check so mypy --strict narrows

The F2 refactor hoisted the None-check into a `weighted` boolean and then indexed
likelihood_weights[index] guarded by `if weighted`. mypy --strict cannot use a
separate local flag to narrow `Sequence[Any] | None`, so it reported
"Value of type 'Sequence[Any] | None' is not indexable", failing the core-layer
strict gate (test_mypy_strict_zero_errors_on_full_core_layer) — main passed it.
Index against the direct `if likelihood_weights is not None` instead (mypy
narrows it); keep the `weighted` flag for the _gaussian_log_probability call.

Behavior is identical (weighted is True exactly when likelihood_weights is not
None). mypy strict gate + MCMC log-prob tests pass; ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: fanghao <fanghaoaa@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: 3 people

app_desktop/workspace_controller.py

@@ -730,6 +730,36 @@ def _restore_root_config(window: Any, config: Any, *, restore_constants: bool =
     _restore_display_units_controls(window, "root", units, include_constants=True)
 
 
+def _set_checked_if(window: Any, attr: str, value: Any) -> None:
+    widget = getattr(window, attr, None)
+    if widget is not None and value is not None and hasattr(widget, "setChecked"):
+        widget.setChecked(bool(value))
+
+
+def _restore_common_config(window: Any, common: Any, latex: Any) -> None:
+    """Restore the mode-independent common + latex settings. These are captured
+    at save (config.common / config.latex) but were never read back, silently
+    resetting compute-affecting precision and display options to defaults on
+    reload (audit F11). mpmath_precision in particular changes the numerical
+    result, so it must survive a round-trip."""
+    if isinstance(common, dict):
+        _set_value(getattr(window, "mpmath_precision_spin", None), common.get("mpmath_precision"))
+        _set_value(getattr(window, "uncertainty_digits_spin", None), common.get("uncertainty_digits"))
+        _set_value(getattr(window, "display_digits_spin", None), common.get("display_digits"))
+        _set_checked_if(window, "generate_latex_checkbox", common.get("generate_latex"))
+        _set_checked_if(window, "generate_plots_checkbox", common.get("generate_plots"))
+        _set_checked_if(window, "verbose_checkbox", common.get("verbose"))
+        _set_checked_if(window, "scientific_checkbox", common.get("display_scientific"))
+    if isinstance(latex, dict):
+        _set_value(getattr(window, "latex_input_precision_spin", None), latex.get("input_digits"))
+        _set_value(getattr(window, "latex_group_size_spin", None), latex.get("group_size"))
+        _set_checked_if(window, "dcolumn_checkbox", latex.get("use_dcolumn"))
+        _set_checked_if(window, "caption_checkbox", latex.get("use_caption"))
+        _set_text(getattr(window, "output_file_edit", None), str(latex.get("output_path") or ""))
+        _set_text(getattr(window, "caption_edit", None), str(latex.get("caption") or ""))
+        _set_combo_data(getattr(window, "latex_engine_combo", None), str(latex.get("engine") or "tectonic"))
+
+
 def _restore_extrapolation_config(window: Any, config: Any) -> None:
     if not isinstance(config, dict):
         return
@@ -1910,6 +1940,7 @@ def _restore_workspace_contents(window: Any, manifest: dict[str, Any], attachmen
     _restore_data_section(window, unified_constants, constants=True)
 
     config = workspace.get("config") or {}
+    _restore_common_config(window, config.get("common"), config.get("latex"))
     _restore_extrapolation_config(window, config.get("extrapolation"))
     _restore_error_config(window, config.get("error"))
     _restore_statistics_config(window, config.get("statistics"))

cli/main.py

@@ -158,29 +158,42 @@ def _run_calc(job: BatchJob) -> dict[str, object]:
     method hard-defaults to ``wynn_epsilon``; future iterations can
     expose ``method`` via the YAML schema.
     """
+    from mpmath import mp
+
     from extrapolation_methods.accelerators import (
         SequenceAcceleratorConfig,
         apply_sequence_accelerator,
     )
+    from shared.precision import precision_guard
 
     xs, ys = _read_xy_csv(job.data_path)
     if len(ys) < 3:
         raise ValueError(
             f"Job {job.name!r}: calc needs at least 3 points, got {len(ys)}"
         )
-    config = SequenceAcceleratorConfig(precision=job.precision)
-    result = apply_sequence_accelerator("wynn_epsilon", ys, config)
+
+    def _serialize_mpf(value: object) -> object:
+        # str(mpf) emits only ambient-mp.dps digits, silently truncating a
+        # high-precision result; nstr at the requested precision preserves it.
+        return mp.nstr(value, job.precision) if hasattr(value, "_mpf_") else value
+
+    # Wrap compute AND serialization in the precision guard so the accelerator
+    # runs at, and the limit/metadata are rendered at, the requested precision
+    # regardless of the ambient mp.dps the CLI process happens to be at.
+    with precision_guard(job.precision):
+        config = SequenceAcceleratorConfig(precision=job.precision)
+        result = apply_sequence_accelerator("wynn_epsilon", ys, config)
+        limit_text = _serialize_mpf(result.value)
+        metadata = {k: _serialize_mpf(v) for k, v in (result.metadata or {}).items()}
+
     return {
         "job_name": job.name,
         "operation": "calc",
         "method": "wynn_epsilon",
         "data_path": str(job.data_path),
         "precision": job.precision,
-        "limit": str(result.value),
-        "metadata": {
-            k: (str(v) if hasattr(v, "_mpf_") else v)
-            for k, v in (result.metadata or {}).items()
-        },
+        "limit": limit_text,
+        "metadata": metadata,
     }
 
 

datalab_core/mcmc_refine.py

@@ -84,6 +84,25 @@ def _evaluate_prediction(evaluator: Any, params: Mapping[str, mp.mpf], observati
     raise TypeError("MCMC evaluator does not accept supported argument shapes")
 
 
+def _gaussian_log_probability(residuals_sq: float, rmse: float, *, weighted: bool) -> float:
+    """Gaussian log-likelihood (up to an additive constant) for MCMC.
+
+    ``residuals_sq`` is Σ wᵢ·rᵢ². When the fit is WEIGHTED the weights are already
+    1/σᵢ² (see ``likelihood_weights_from_series``), so ``residuals_sq`` is the
+    correct χ² = Σ rᵢ²/σᵢ² and the log-likelihood is simply ``-0.5·residuals_sq``.
+    Dividing again by rmse² (the old code path) double-scaled the likelihood,
+    acting as a constant "temperature" that rescaled every posterior variance and
+    made the reported credible intervals wrong whenever σ/weights were supplied.
+
+    When UNWEIGHTED (weights all 1, data noise unknown), rmse² is the plug-in
+    variance estimate, so the Gaussian exponent is ``-0.5·Σrᵢ²/rmse²`` — keep the
+    division there.
+    """
+    if weighted:
+        return -0.5 * residuals_sq
+    return -0.5 * residuals_sq / (rmse**2)
+
+
 def _estimate_rmse(targets: Sequence[Any], fit_result: Any) -> float:
     """Estimate a strictly positive RMSE for MCMC proposal scaling."""
     residuals = fit_result.details.get("residuals") if fit_result.details else None
@@ -141,6 +160,8 @@ def refine_fit_with_mcmc(
     initial_guess = [float(fit_result.params[name]) for name in param_names]  # float-bridge: emcee walker seeds
     rmse = _estimate_rmse(targets, fit_result)
 
+    weighted = likelihood_weights is not None
+
     def _log_probability(theta: Sequence[object]) -> float:
         if not param_names or rmse <= 0:
             return float("-inf")  # float-bridge: emcee log-prob sentinel
@@ -153,11 +174,13 @@ def _log_probability(theta: Sequence[object]) -> float:
                 if not math.isfinite(pred):
                     return float("-inf")  # float-bridge: emcee log-prob sentinel
                 residual = float(target) - pred  # float-bridge: emcee log-prob
+                # Index against the direct None-check (not the hoisted `weighted`
+                # flag) so mypy --strict narrows Sequence | None to indexable.
                 weight = float(likelihood_weights[index]) if likelihood_weights is not None else 1.0  # float-bridge: emcee log-prob
                 residuals_sq += weight * (residual**2)
                 if not math.isfinite(residuals_sq):
                     return float("-inf")  # float-bridge: emcee log-prob sentinel
-            return -0.5 * residuals_sq / (rmse**2)
+            return _gaussian_log_probability(residuals_sq, rmse, weighted=weighted)
         except (TypeError, ValueError, ArithmeticError, OverflowError, KeyError):
             return float("-inf")  # float-bridge: emcee log-prob sentinel
 

fitting/hp_fitter.py

@@ -293,6 +293,21 @@ def _compute_statistics(
     )
 
 
+def _error_from_variance(variance: mp.mpf) -> mp.mpf:
+    """Parameter standard error = sqrt(variance), degrading to NaN for a
+    non-finite or NEGATIVE variance. A negative covariance diagonal (finite-
+    precision indefiniteness of the inverse) would otherwise make mp.sqrt return
+    a complex mpc that later crashes combine_error_components with
+    'cannot create mpf from mpc' — degrade to NaN so the fit surfaces a
+    covariance warning instead of a hard failure (audit finding F1)."""
+    try:
+        if mp.isnan(variance) or mp.isinf(variance) or variance < 0:
+            return mp.nan
+        return mp.sqrt(variance)
+    except (TypeError, ValueError):
+        return mp.nan
+
+
 def _compute_covariance(
     model: ModelSpecification,
     params: dict[str, mp.mpf],
@@ -343,12 +358,17 @@ def _compute_covariance(
     noise = chi2 / dof if dof > 0 else mp.nan
     covariance = [[inv[i, j] * noise for j in range(k)] for i in range(k)]
     errors = {
-        name: mp.sqrt(covariance[idx][idx])
-        if not mp.isnan(covariance[idx][idx])
-        else mp.nan
+        name: _error_from_variance(covariance[idx][idx])
         for idx, name in enumerate(free_params)
     }
-    if any(mp.isnan(val) or mp.isinf(val) for row in covariance for val in row):
+    # Flag ill-conditioning: non-finite cells OR a negative variance on the
+    # diagonal. A negative diagonal (finite-precision indefiniteness of the
+    # inverse) is neither NaN nor Inf, so it would otherwise pass silently and
+    # yield a complex sqrt that crashes downstream error combination.
+    has_negative_variance = any(covariance[i][i] < 0 for i in range(k))
+    if has_negative_variance or any(
+        mp.isnan(val) or mp.isinf(val) for row in covariance for val in row
+    ):
         cov_warning = "协方差矩阵病态或奇异，参数不确定度可能不可靠。 / Covariance matrix is ill-conditioned or singular; parameter uncertainties may be unreliable."
     return covariance, errors, cov_warning
 

tests/test_cli_batch.py

@@ -57,6 +57,47 @@ def _batch_config(_data_csv: Path, tmp_path: Path) -> Path:
     return path
 
 
+def test_run_calc_preserves_requested_high_precision_in_limit(tmp_path: Path):
+    """CLI batch `calc` must serialize the extrapolated limit at the requested
+    precision, not truncate it to the ambient mp.dps via bare str(mpf) outside a
+    precision_guard (audit finding F6)."""
+    from mpmath import mp
+
+    from cli.batch_config import BatchJob
+    from cli.main import _run_calc
+
+    # A geometric series 1 + 1/2 + 1/4 + ... whose Wynn-eps limit is exactly 2,
+    # but we assert the SIGNIFICANT-DIGIT COUNT of the serialized limit, which is
+    # what truncation to dps=15 would cap. Use a slowly-varying convergent whose
+    # accelerated value carries many digits: partial sums of sum 6/k^2 -> pi^2.
+    data = tmp_path / "seq.csv"
+    rows = ["n,y"]
+    total = mp.mpf(0)
+    with mp.workdps(80):
+        for n in range(1, 40):
+            total += mp.mpf(6) / mp.mpf(n) ** 2
+            rows.append(f"{n},{mp.nstr(total, 60)}")
+    data.write_text("\n".join(rows), encoding="utf-8")
+
+    # mp.dps is process-global; set a low ambient precision to simulate the CLI
+    # before the per-job guard, then restore it so this mutation can't leak into
+    # later tests (the per-job precision_guard is exactly what we're asserting).
+    original_dps = mp.dps
+    try:
+        mp.dps = 15  # simulate the ambient CLI precision before per-job guard
+        job = BatchJob(
+            name="hp-calc", operation="calc", data_path=data,
+            output_dir=tmp_path, model=None, precision=50,
+        )
+        result = _run_calc(job)
+    finally:
+        mp.dps = original_dps
+
+    limit = str(result["limit"])
+    digits = len(limit.replace(".", "").replace("-", "").lstrip("0"))
+    assert digits > 20, f"limit truncated to ambient dps: {limit!r} ({digits} digits)"
+
+
 def test_load_batch_config_round_trips(_batch_config: Path):
     """Parse a batch YAML into a BatchConfig dataclass."""
     from cli.batch_config import load_batch_config

tests/test_hp_fitter_covariance_guard.py

@@ -0,0 +1,40 @@
+"""Covariance error guard against a negative variance diagonal (audit F1).
+
+A near-rank-deficient inverse can have a NEGATIVE diagonal (finite-precision
+indefiniteness). sqrt of that is a complex mpc, which passes an isnan-only guard
+and then crashes combine_error_components ('cannot create mpf from mpc'). The
+error must degrade to NaN so the fit surfaces a covariance warning instead.
+"""
+
+from __future__ import annotations
+
+from mpmath import mp
+
+from fitting.hp_fitter import _error_from_variance, combine_error_components
+
+
+def test_error_from_negative_variance_is_nan_not_complex():
+    result = _error_from_variance(mp.mpf("-1e-5"))
+    assert mp.isnan(result)
+    # Must NOT be a complex mpc (the pre-fix behavior).
+    assert not isinstance(result, mp.mpc)
+
+
+def test_error_from_valid_variance_is_sqrt():
+    assert _error_from_variance(mp.mpf("4")) == mp.mpf("2")
+
+
+def test_error_from_nan_and_inf_variance_is_nan():
+    assert mp.isnan(_error_from_variance(mp.nan))
+    assert mp.isnan(_error_from_variance(mp.inf))
+
+
+def test_nan_error_does_not_crash_combine_error_components():
+    """The NaN degradation flows through error combination without the
+    'cannot create mpf from mpc' TypeError the negative-variance path caused."""
+    stat = _error_from_variance(mp.mpf("-1e-5"))  # NaN, not mpc
+    stat_map, _sys_map, total_map = combine_error_components(
+        {"a": mp.mpf("1")}, {"a": stat}, {}
+    )
+    assert mp.isnan(stat_map["a"])
+    assert mp.isnan(total_map["a"])

tests/test_mcmc_log_probability.py

@@ -0,0 +1,35 @@
+"""MCMC Gaussian log-probability scaling (audit finding F2).
+
+The weighted branch must NOT divide the already-weighted chi-square (Σ rᵢ²/σᵢ²)
+by rmse² again — that double-scaling corrupted the posterior width and the
+reported credible intervals whenever sigma/weights were supplied. The unweighted
+branch keeps the /rmse² plug-in variance. This pure function needs no emcee.
+"""
+
+from __future__ import annotations
+
+from datalab_core.mcmc_refine import _gaussian_log_probability
+
+
+def test_weighted_log_probability_is_not_divided_by_rmse_squared():
+    residuals_sq = 4.0  # already Σ rᵢ²/σᵢ² when weighted
+    rmse = 0.01  # small rmse would blow up the exponent if wrongly divided
+    lp = _gaussian_log_probability(residuals_sq, rmse, weighted=True)
+    # Correct weighted Gaussian: -0.5 * chi-square, independent of rmse.
+    assert lp == -0.5 * residuals_sq
+    # And crucially NOT the double-scaled value.
+    assert lp != -0.5 * residuals_sq / (rmse**2)
+
+
+def test_weighted_log_probability_ignores_rmse():
+    # Same chi-square with wildly different rmse must give the SAME log-prob.
+    assert _gaussian_log_probability(4.0, 0.01, weighted=True) == _gaussian_log_probability(
+        4.0, 1000.0, weighted=True
+    )
+
+
+def test_unweighted_log_probability_keeps_rmse_normalization():
+    residuals_sq = 4.0  # Σ rᵢ² (weights all 1)
+    rmse = 2.0
+    lp = _gaussian_log_probability(residuals_sq, rmse, weighted=False)
+    assert lp == -0.5 * residuals_sq / (rmse**2)

tests/test_workspace_controller.py

@@ -206,6 +206,43 @@ def test_workspace_controller_captures_manual_state(qtbot) -> None:
     assert bundle.attachments["attachments/plots/plot-001.png"] == PNG_1X1
 
 
+def test_workspace_round_trips_common_and_latex_precision_settings(qtbot) -> None:
+    """The common (mpmath_precision, uncertainty/display digits, scientific) and
+    latex (input digits, group size) settings are saved but were never restored,
+    silently resetting compute-affecting precision to defaults on reload
+    (audit finding F11)."""
+    from app_desktop.window import ExtrapolationWindow
+    from app_desktop.workspace_controller import capture_workspace, restore_workspace
+
+    source = ExtrapolationWindow()
+    qtbot.addWidget(source)
+    source.mpmath_precision_spin.setValue(50)
+    source.uncertainty_digits_spin.setValue(3)
+    source.display_digits_spin.setValue(25)
+    source.scientific_checkbox.setChecked(True)
+    source.latex_input_precision_spin.setValue(40)
+    source.latex_group_size_spin.setValue(5)
+    # caption text + TeX engine are also captured at save; they must round-trip
+    # too, or the F11 "captured but never restored" fix is incomplete.
+    source.caption_edit.setText("Table 1: extrapolated limits")
+    source.latex_engine_combo.setCurrentText("pdflatex")
+
+    bundle = capture_workspace(source, title="precision settings")
+
+    target = ExtrapolationWindow()
+    qtbot.addWidget(target)
+    restore_workspace(target, bundle.manifest, bundle.attachments)
+
+    assert target.mpmath_precision_spin.value() == 50
+    assert target.uncertainty_digits_spin.value() == 3
+    assert target.display_digits_spin.value() == 25
+    assert target.scientific_checkbox.isChecked() is True
+    assert target.latex_input_precision_spin.value() == 40
+    assert target.latex_group_size_spin.value() == 5
+    assert target.caption_edit.text() == "Table 1: extrapolated limits"
+    assert target.latex_engine_combo.currentText() == "pdflatex"
+
+
 def test_workspace_preserves_raw_constants_text_view_draft(qtbot) -> None:
     from app_desktop.window import ExtrapolationWindow
     from app_desktop.workspace_controller import capture_workspace, restore_workspace