Skip to content

fix(fit): strip @ in compare --exclude-chains; mh-summary clean break#422

Merged
vsbuffalo merged 1 commit into
mainfrom
fix/mh-clean-break-and-exclude-at
Jul 10, 2026
Merged

fix(fit): strip @ in compare --exclude-chains; mh-summary clean break#422
vsbuffalo merged 1 commit into
mainfrom
fix/mh-clean-break-and-exclude-at

Conversation

@vsbuffalo

Copy link
Copy Markdown
Owner

Two convergence-tooling fixes garki hit on 75eb8def. Both verified against the installed code (they are real bugs, not a stale install).

Q1 — compare --exclude-chains @fit:ids never matched

parse_cohort_exclude split the token but kept the @ sigil as part of the fit name, so @ctl_rm.toml:4 was compared against ctl_rm.toml and always missed ("no compared fit named '@ctl_rm.toml'"). The --help-documented @fit:ids form was unusable; only the bare fit:ids form worked.

Fix: strip the optional @ before matching, so @fit:ids and fit:ids bind the same fit. The prior unit tests passed vacuously — the model names were literally @a/@b, so no strip was ever exercised; retargeted them to real names and assert both forms resolve identically.

Q2 — mh-summary rename is a clean break (no back-compat)

The pmmh_summary.jsonmh_summary.json rename (#415) is a deliberate breaking change. read_convergence was inconsistently still falling back across sibling summary names, while PmmhStageResult::load was not. Removed the fallback so both read only the method's own <algorithm>_summary.json.

Consequence (documented breaking change): a pre-rename mh fit (only pmmh_summary.json on disk) is NotAssessed and must be re-run — it is never silently read under the old name. Fits produced by a build ≥ #415 are unaffected.

Tests

  • parse_cohort_exclude_forms / _rejects_bad_forms — real names; both @fit:ids and fit:ids forms, error messages name the stripped fit.
  • read_convergence_clean_break_no_pmmh_fallback — an old mh fit (pmmh_summary.json only) is NotAssessed.
  • read_convergence_finds_mh_summary_for_mh_method — mh reads its own mh_summary.json.

Installed locally at this commit ahead of merge to unblock the garki agent.

@vsbuffalo
vsbuffalo force-pushed the fix/mh-clean-break-and-exclude-at branch from 03157ea to 937d6a9 Compare July 10, 2026 21:43
…an break

Q1. `compare --exclude-chains fit:ids` matches the fit name VERBATIM against
the compared fits — a run-store-handle fit is named `@a`, a path-named one
`ctl_rm.toml`; the `@` is part of a handle name, not a sigil. Following the
`--help` (whose example is the handle name `@a`), a user can prepend a
spurious `@` to a path-named fit and get a bare "no compared fit named
'@ctl_rm.toml'". Now, when dropping a leading `@` would match, the error
names the fix, and the `--help` states the name is matched verbatim.

Q2. The pmmh_summary.json -> mh_summary.json rename (#415) is a clean break.
read_convergence no longer falls back across sibling summary names — it
reads only the method's own <algorithm>_summary.json, matching
PmmhStageResult::load. A pre-rename mh fit (only pmmh_summary.json on disk)
is NotAssessed and must be re-run, never read under the old name.
@vsbuffalo
vsbuffalo force-pushed the fix/mh-clean-break-and-exclude-at branch from 937d6a9 to ca4df22 Compare July 10, 2026 22:10
@vsbuffalo
vsbuffalo merged commit 0f40fb4 into main Jul 10, 2026
2 checks passed
@vsbuffalo
vsbuffalo deleted the fix/mh-clean-break-and-exclude-at branch July 10, 2026 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant