fix: precision-correctness audit findings (F6/F11/F2/F1)#74
Conversation
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).
… 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>
…ct 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>
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
Audit R2 — PR2/5: precision correctness (F6/F11/F2/F1)
F6 — CLI batch dropped precision
cli/main._run_calcran the accelerator and serialized mpf outside anyprecision_guard, truncating high-precision results to ambientmp.dps. Now wraps compute + serialization inprecision_guard(job.precision); mpf serialized viamp.nstr(value, job.precision).F11 — workspace common/latex config captured but never restored
mpmath precision, uncertainty/display digits, scientific flag, LaTeX input digits/group size/dcolumn/output path — plus caption text and TeX engine — were saved but silently reset to defaults on reload.
_restore_common_confignow restores all of them.F2 — MCMC gaussian log-probability
Extracted
_gaussian_log_probabilityand use it for both weighted/unweighted paths.F1 — covariance negative-variance guard
_error_from_variancemaps negative/NaN/Inf variance tomp.nanand flagshas_negative_variancein the covariance warning.Test plan
mp.dpsrestore leak fixed too); common/latex round-trip incl. caption+engine (RED without the restore fix); MCMC log-prob + covariance-guard tests.mypy --strictcore-layer gate;ruffclean.🤖 Generated with Claude Code