Skip to content

Consolidate report and result boundaries - #33

Merged
dhsohn merged 1 commit into
mainfrom
fix/report-boundary-consolidation
Jul 25, 2026
Merged

Consolidate report and result boundaries#33
dhsohn merged 1 commit into
mainfrom
fix/report-boundary-consolidation

Conversation

@dhsohn

@dhsohn dhsohn commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Motivation

I found that report generation still maintained two pass-through layers, built both structured and plain-text methods even though only the structured form was rendered, and split each fit outcome into parallel success and failure maps before joining them again. These seams added synchronization and typing costs without representing separate supported behavior.

Changes

  • Merged the HTML renderer into the canonical report module, removing the one-to-one facade and callback boundary.
  • Kept structured methods sections as the single report path and removed the unused plain-text builder.
  • Preserved complete FitResult objects in one dataset/model map for both successful and failed fits.
  • Centralized model display labels and removed fixed display-name callbacks from the report pipeline.
  • Moved information-criterion orchestration into stats.py and removed the single-function fit_criteria.py module.
  • Replaced loose species/result types and equilibrium tuple/bool inference with concrete types.
  • Updated tests and the model-selection implementation reference for the consolidated paths.

The CLI, public fit_models return fields, numerical solvers, model-selection rules, and report/artifact contracts are unchanged.

Verification

  • python -m pytest -q -W error — 174 passed, no warnings.
  • python -m ruff check . — passed.
  • python -m mypy --ignore-missing-imports nmr_bind_fit — passed for 13 source files.
  • Full python -m mypy nmr_bind_fit now reports only the 7 existing missing-stub errors for pandas/SciPy; the 22 project-code errors are gone.
  • Ran the same synthetic 1:1 CLI analysis from main and this branch. Artifact inventories matched; HTML was identical after normalizing its generated timestamp, PNG and CSV files were byte-identical, and rendered PDFs were identical.

@dhsohn
dhsohn marked this pull request as ready for review July 25, 2026 06:54
@dhsohn
dhsohn merged commit 1965fa5 into main Jul 25, 2026
6 checks passed
@dhsohn
dhsohn deleted the fix/report-boundary-consolidation branch July 25, 2026 06:54
@dhsohn dhsohn added the enhancement New feature or request label Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant