refactor(tr): Phase 1 - add ONLY clauses to USE TRCOMM (4 files)#4
Merged
Conversation
…trrslt Phase 1 of the TR module refactoring (per docs/superpowers/specs/2026-04-17-tr-refactoring-design.md). Each USE TRCOMM (no ONLY clause) in the four target files now lists only the symbols actually referenced in that scope. Behavior is invariant: direct pre/post regression comparison on tr_m0904 shows metrics match within 1e-10 tolerance. (The baseline regression check already shows pre-existing environment-dependent drift on develop independent of this change; this PR does not address that.) Files modified (other modules untouched): - tr/trloop.f90: tr_loop (1 USE site) - tr/trfout.f90: tr_fout (1 USE site) - tr/trparm.f90: tr_parm + tr_nlin (2 USE sites; tr_nlin lists full namelist) - tr/trrslt.f90: TRGLOB + TRPRNT + tr_setup_kv (3 USE sites)
Owner
Author
|
@cursor review |
- TRGLOB: remove duplicate RKEV - tr_setup_kv: reduce ONLY list to KVT/KVRT only (the rest were false-positives from string literals like 'ANS0(1)', not actual symbol refs) Pre/post regression comparison still matches within 1e-10 on tr_m0904.
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d5b268d. Configure here.
3 tasks
5 tasks
k-yoshimi
added a commit
that referenced
this pull request
Apr 28, 2026
Bugbot's second pass on PR #178 surfaced 4 issues that survived the first round of LOW fixes (e1e5a37). All four addressed in this commit, with targeted regression tests: #1 (LOW) PipelineResult.to_dict() now returns defensive copies of inner scalars dicts and coupling_applied lists. Previously, MCP- side mutation of the JSON payload would propagate into the originating PipelineStep. New test_to_dict_returns_defensive_copies asserts the contract. #2 (LOW) _validate_steps now accepts list-of-2-element steps in addition to tuple-of-2-element. JSON-decoded MCP payloads deserialise tuples as lists, so the API now round-trips through to_dict() without manual coercion. New test_run_pipeline_accepts_list_shaped_steps regression guard. #3 (LOW) set_param now explicitly rejects bool inputs with TotPipelineCouplingError instead of silently coercing to 1.0/0.0 via the numeric branch (Python's bool is a subclass of int). The previous test_set_param_records_bool_as_float (which documented the silent-coercion behavior) is replaced with test_set_param_rejects_bool that asserts the explicit rejection. #4 (MED) ExceptionGroup compatibility shim for Python 3.10. The close() multi-failure path uses builtin ExceptionGroup (3.11+); tot_mcp's pyproject.toml declares requires-python=">=3.10". The shim probes for the builtin first, falls back to the exceptiongroup PyPI backport if available, and degrades gracefully to raising errors[0] only when neither is reachable (same as pre-fix behavior). CI runs 3.11+ so the existing ExceptionGroup test still passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
k-yoshimi
added a commit
that referenced
this pull request
Apr 28, 2026
* docs(spec): add L-7a cross-module coupling design Design for incremental Python-side scalar coupling between fp and tr in the tot orchestrator. Establishes TotPipeline class skeleton in a new file (python/totlib/pipeline.py) without touching the legacy Tot wrapper. Covers fp -> tr driven current (RJT volume integral -> PNBCD scalar) only; profile/coordinate transforms deferred to L-7b via BPSD. Two Codex review passes: 9 findings (HIGH 2 / MED 5 / LOW 1) all addressed inline. Pre-flight Research R1 (singleton coexistence + finalize/init state-leak) / R2 (rjt_volint extraction) / R3 (tr param name confirmation) gated before implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(plan): add L-7a cross-module coupling implementation plan Bite-sized TDD task list executing the L-7a design spec (0a12bd0) across 4 phases / 16 tasks / 3 sub-PRs: - Phase 0: Pre-flight research R1/R2/R3 (singleton coexistence, rjt_volint extraction, tr driven-current param confirmation) - Phase 1 (PR 1): TotPipeline class + errors + mock unit tests - Phase 2 (PR 2): compute_rjt_volint helper + COUPLING_RULES population + fp -> tr 1e-10 equivalence test - Phase 3 (PR 3): run_pipeline MCP tool + docs Each PR phase ends with the CLAUDE.md pre-push gate (code-reviewer + codex:codex-rescue + REVIEW_OK marker + --forked --timeout=120 --timeout-method=signal pytest). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(spec): record L-7a R1 (singleton coexistence) outcome R1-a (concurrent live) and R1-b (re-init state leak) verified per the L-7a design spec. Outcome and fallback decision recorded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(spec): record L-7a R2 (rjt_volint extraction) outcome R2 PASS. compute_rjt_volint(state, *, R0, a) verified against fp internal aggregate rtotalIP at 1e-10 (sanity case NRMAX=10, RR=3, RA=1, default fixture: helper -2.769528e-13 MA == fp stdout -2.7695E-13 MA). FpState shape characterized: top-level dataclass attrs [nrmax, nsamax, npmax, nthmax, ntg2, timefp, RNT/RWT/RTT/RJT/RPCT/RPWT]; **no `.scalars` dict** (asymmetric to TrState — R1 concern #3 confirmed). FpState exposes no grid info (RG/DV/VOLR/RA/RR absent), so helper takes R0, a as kwargs and assumes uniform rho-grid in [0,1] (matches fp/fpprep.f90:97-99). RJT units: [MA/m^2] (matches RJS in fp/fpcale.f90:290 where RJ_P = RJS*1.D6 converts to A/m^2). Helper applies *1e6 internally and returns total I in [A]. fplib unchanged — helper lives in totlib/pipeline.py (Phase 2 Task 2.1). Probe stdout (truncated): FpState attributes: RJT/RNT/RPCT/RPWT/RTT/RWT (list[list[float]]), npmax=50, nrmax=1, nsamax=1, ntg2=2, nthmax=50, timefp=0.01. Grid candidates RG/DV/VOLP/RM/RMAX/DR/RA/RR/RB: ALL ABSENT. Scalar candidates AJT/AJT_driven/.../scalars: ALL ABSENT. Sanity check (NRMAX=10): fp stdout: total plasma current [MA] -2.7695E-13 helper: compute_rjt_volint = -2.769528e-07 A = -2.769528e-13 MA (1e-10 match) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(spec): record L-7a R3 (tr driven current param) outcome PARTIAL PASS: PNBCD is NOT in tr_param_registry. Registered scalar candidates are PICCD, PECCD, PLHCD (dimensionless current-drive factors) and PLHTOT (LH input power [MW]). Selected PLHCD as L-7a skeleton target with documented unit mismatch (Amperes -> dimensionless factor); rigorous physical conversion deferred to L-7b. Picked a = tr.RA for compute_rjt_volint(state, R0=tr.RR, a=tr.RA) because fp's radial mesh is RA-normalized (fpcale.f90:34, 56). Active-drive fixture for Phase 2.3 equivalence test: fp.set_param('E0', 0.001) # induction E-field [V/m] yields ~0.946 MA (vs ~1e-13 MA for default), well above the 1e-10 equivalence-test floor. MODEL_WAVE+PABS_LH path failed with rc=3 and needs additional setup; E0 is the minimum-cost active-drive trigger. Open issue surfaced: tr lacks a registered scalar that semantically accepts an external driven current in MA. L-7b should add such a param (or expose PNBCD via the registry). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(l7a): R4 decision gate — accept skeleton coupling, update spec/plan R3 confirmed PNBCD is not registered in tr_param_registry.f90; only PLHCD/PICCD/PECCD (dimensionless multipliers) accept set_param. Per user decision (option A), L-7a proceeds as API-plumbing skeleton with PLHCD as the dst_param; physical fidelity (proper EXTERNAL_DRIVEN_I scalar in tr) is moved to L-7b follow-ups. Spec updates: - §2 table: PNBCD -> PLHCD with skeleton-coupling caveat - §3 non-goals: explicit "physical fidelity is L-7b scope" - §5.2 CouplingRule: src_state_key now (state, params) 2-arg callable - §5.3 PipelineResult + new _state_to_scalars adapter (handles FpState which lacks the .scalars dict per R1/R2) - §5.4 TotPipeline: self._params tracking for callable rules - §6.2 run_pipeline: uses _state_to_scalars + records dst_param injections back into _params - §12 follow-ups: tr EXTERNAL_DRIVEN_I_MA addition + fplib .scalars property consideration Plan updates: - Task 1.4: __init__ adds self._params; set_param records into it - Task 1.5: _extract_source 2-arg callable + _state_to_scalars helper + 5 new tests (callable+params, set_param tracking, adapter) - Task 2.1: compute_rjt_volint(state, *, R0, a) signature with the R2-derived implementation pasted in - Task 2.2: COUPLING_RULES uses lambda wrapper pulling tr:RR/tr:RA from params; dst_param=PLHCD; documents skeleton nature - Task 2.3: equivalence test uses E0=0.001 active-drive fixture (R3), PLHCD instead of PNBCD, _state_to_scalars adapter, plus a sanity guard that the integral magnitude is non-trivial (>= 1e3 A) - Test count expectations updated throughout Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(totlib): add TotPipeline error hierarchy Five new exception classes for the L-7a pipeline: - TotPipelineError (base, extends TotlibError) - TotPipelineUnknownModuleError, TotPipelineCouplingError, TotPipelineLifecycleError - TotPipelineRunError carrying partial_result + failed_step_index + failed_module attributes for debugging mid-pipeline failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(totlib): add CouplingRule/PipelineStep/PipelineResult dataclasses Pure data carriers for the run_pipeline orchestrator. CouplingRule is frozen (immutable) so the registry can be hashable in the future. PipelineResult.last(module) returns the most recent matching step, to_dict() is the MCP serialization format. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(totlib): match typing convention from surrounding modules Code-quality reviewer flagged divergence: pipeline.py used PEP 585 (dict[...], list[...]) while totlib/state.py and sibling state.py files use Dict[...], List[...] from typing. Match the surrounding codebase. Also addresses two reviewer nits: explicit "frozen=True rationale" docstring on CouplingRule, and explicit shallow-ref caveat docstring on PipelineResult.to_dict. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(totlib): add module registry + lazy import helpers _MODULE_REGISTRY maps short names (fp/tr/eq/wr/wrx/ti) to (package, wrapper_class, base_error_class) triples. _import_wrapper / _import_module_error keep the lazy-load contract from spec section 5.5: only modules used in the current pipeline get their lib*.so / errors.py loaded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(totlib): correct ti wrapper class name + add parametrized registry tests Code-quality reviewer flagged a coverage gap: only fp was exercised by the registry-resolves-class tests. Adding parametrized tests across all 6 modules immediately surfaced a real bug: the ti wrapper class is `TiLib` (capital L), not `Tilib` as the registry claimed. The sibling `TilibError` IS lowercase, so ti is the only entry where the wrapper-class and error-class casing diverge — easy to miss. Fixed registry entry; added a comment flagging the asymmetry. Spec and plan updated to match. Now 31/31 pipeline tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(totlib): add TotPipeline lifecycle (init/set_param/close) Implements the lifecycle subset of TotPipeline: - __init__ creates empty _modules / _params / _closed=False; opens nothing. - _ensure_module lazy-instantiates a wrapper on first reference, gated by the closed flag. - set_param routes strings to set_param_str, numerics to set_param, and records the value into _params after the wrapper accepts it (so failed wrapper validation keeps _params consistent). - close is idempotent and finalizes all modules even when one raises; the first exception is re-raised so the root cause is visible. - Context manager support via __enter__/__exit__. run_pipeline / _extract_source / COUPLING_RULES are deferred to Task 1.5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(totlib): guard set_param_str when module wrapper lacks the method Code-quality reviewer flagged that wrlib.Wrlib, wrxlib.Wrxlib, and tilib.TiLib do not expose set_param_str (only fp/tr/eq do). Routing a string value through TotPipeline.set_param previously raised a bare AttributeError leaking from getattr. Now raises a typed TotPipelineCouplingError with a clear message identifying the module. Added a regression test using a mock wrapper without set_param_str. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(totlib): implement TotPipeline.run_pipeline orchestrator - _state_to_scalars adapter (handles fp's missing .scalars dict + the standard .scalars-bearing modules uniformly). - COUPLING_RULES registry (empty at this commit; ('fp','tr') is filled in Phase 2 after R2/R3 outcomes are baked into pipeline.py). - _validate_steps performs side-effect-free pre-flight checks. - _extract_source resolves callable src_state_key with (state, params) signature; string keys go through _state_to_scalars. - run_pipeline iterates steps, applies coupling rules between adjacent pairs, and wraps per-module errors as TotPipelineRunError with the partial PipelineResult attached for debugging. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(totlib): export TotPipeline + dataclasses at package level Backwards-compatible: legacy 'from totlib import Tot' / 'TotState' imports continue to work. New 'from totlib import TotPipeline, CouplingRule, PipelineStep, PipelineResult' is added for L-7a users. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(totlib): broaden run_pipeline catch + cover Codex review gaps Codex independent review (2026-04-28) flagged 1 MED + 2 LOW gaps the prior reviewer missed: MED1: run_pipeline's except clause was (base_err, TotPipelineCouplingError), which let TypeError/ValueError from wrong kwargs escape without TotPipelineRunError wrapping — so partial_result was lost. Broadened to `except Exception`. KeyboardInterrupt and SystemExit still propagate (they are BaseException, not Exception). The lazy `_import_module_error(name)` call is kept for forward-compat debugging clarity (annotated noqa). LOW2: test_set_param_records_into_params_dict only exercised the numeric path of set_param. Added a string-value entry so the string path's _params bookkeeping is also verified. LOW3: test_run_pipeline_partial_failure_carries_partial_result did not assert on coupling_applied. Added an empty-list assertion (regression guard against silently dropping coupling metadata). Plus a new test test_run_pipeline_typeerror_from_bad_kwargs_is_wrapped that exercises the broadened catch path directly. 61 tests pass under --forked --timeout=120 --timeout-method=signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(totlib): address PR #178 Bugbot LOW findings Three independent fixes surfaced by the Cursor Bugbot pass on PR #178: 1. close() no longer silently drops secondary finalize errors (pipeline.py:259-269). When more than one module's close() raises, the caller now sees an ExceptionGroup carrying every collected error instead of just errors[0]. Single-failure path is unchanged (the original exception type is preserved). Tested via the new test_close_raises_exception_group_when_multiple_modules_fail. 2. set_param's _params bookkeeping no longer diverges from the value the wrapper actually received (pipeline.py:235-249). Numeric inputs are coerced via float() at the boundary, but _params used to store the original (e.g. True or int 2). Coupling rules reading self._params now see the same float the underlying module saw. Existing test_set_param_records_into_params_dict updated for the int → float coercion; new test_set_param_records_bool_as_float pins the bool path explicitly. 3. run_pipeline no longer calls _import_module_error per step (pipeline.py:329-336). The call had become dead after the broad "except Exception" catch in commit 941f353 — base_err was F841'd noqa'd, the comment misleadingly claimed "lazy-import semantics," and the call still triggered an .errors module import per step. _validate_steps already gates on _MODULE_REGISTRY membership pre-execution, and _ensure_module raises on bad names, so removing the dead call doesn't reduce safety. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(totlib): address PR #178 remaining Bugbot findings (4 items) Bugbot's second pass on PR #178 surfaced 4 issues that survived the first round of LOW fixes (e1e5a37). All four addressed in this commit, with targeted regression tests: #1 (LOW) PipelineResult.to_dict() now returns defensive copies of inner scalars dicts and coupling_applied lists. Previously, MCP- side mutation of the JSON payload would propagate into the originating PipelineStep. New test_to_dict_returns_defensive_copies asserts the contract. #2 (LOW) _validate_steps now accepts list-of-2-element steps in addition to tuple-of-2-element. JSON-decoded MCP payloads deserialise tuples as lists, so the API now round-trips through to_dict() without manual coercion. New test_run_pipeline_accepts_list_shaped_steps regression guard. #3 (LOW) set_param now explicitly rejects bool inputs with TotPipelineCouplingError instead of silently coercing to 1.0/0.0 via the numeric branch (Python's bool is a subclass of int). The previous test_set_param_records_bool_as_float (which documented the silent-coercion behavior) is replaced with test_set_param_rejects_bool that asserts the explicit rejection. #4 (MED) ExceptionGroup compatibility shim for Python 3.10. The close() multi-failure path uses builtin ExceptionGroup (3.11+); tot_mcp's pyproject.toml declares requires-python=">=3.10". The shim probes for the builtin first, falls back to the exceptiongroup PyPI backport if available, and degrades gracefully to raising errors[0] only when neither is reachable (same as pre-fix behavior). CI runs 3.11+ so the existing ExceptionGroup test still passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
k-yoshimi
added a commit
that referenced
this pull request
Apr 28, 2026
…ative links The intersphinx URL for the `portal` mapping is a placeholder (`https://task-docs.example.com/portal/<lang>/`), so production HTML resolves cross-project {ref} targets to dead URLs. Each module's design/build/index pages now use a Markdown relative link to the actual source file, matching the convention used elsewhere in the docs (cross-doc links via relative paths). The label definitions in `portal/<lang>/common/architecture.md` are unchanged. Files touched: 20 (ja 10 + en 10), 30 occurrences total (handoff counted 29; one additional occurrence in ti/ja/build.md was a two-line {ref} spanning lines 19-20, handled with perl -0777 slurp mode). Per the morning handoff §残作業 #3 and the evening handoff §中優先 #4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
k-yoshimi
added a commit
that referenced
this pull request
Apr 28, 2026
…nks (#182) * docs(sphinx): replace dead {ref}`portal:common-architecture` with relative links The intersphinx URL for the `portal` mapping is a placeholder (`https://task-docs.example.com/portal/<lang>/`), so production HTML resolves cross-project {ref} targets to dead URLs. Each module's design/build/index pages now use a Markdown relative link to the actual source file, matching the convention used elsewhere in the docs (cross-doc links via relative paths). The label definitions in `portal/<lang>/common/architecture.md` are unchanged. Files touched: 20 (ja 10 + en 10), 30 occurrences total (handoff counted 29; one additional occurrence in ti/ja/build.md was a two-line {ref} spanning lines 19-20, handled with perl -0777 slurp mode). Per the morning handoff §残作業 #3 and the evening handoff §中優先 #4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(sphinx): suppress myst.xref_missing for cross-Sphinx-project links The previous commit replaced dead `{ref}\`portal:common-architecture\`` references with relative Markdown links to the portal source files (e.g. `[text](../../../portal/<lang>/common/architecture.md)`). MyST classifies these as document cross-references and validates them against `env.all_docs` for the current Sphinx project, but the portal target lives in a sibling Sphinx project — so MyST emits `myst.xref_missing` warnings that `-W` then promotes to errors. Suppress that specific warning class via `conf_common.py`. The link text is correct in source, the deployed unified site serves all projects under one tree, so the relative `.md` path resolves to the deployed `.html` at runtime. Verified: all 14 module builds (tr/eq/ti/fp/wr/wrx/tot × en/ja) + 2 portal builds (en + ja) complete clean, no new warnings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(sphinx): narrow myst.xref_missing suppression to module conf files Reviewer concern (codex MED): the previous global suppression in conf_common.py covered every Sphinx project that imports the common config — including portal — and would silently mask future intra- project link typos (e.g. [foo](typo.md) within the same project). Move the suppression out of conf_common.py and into each of the 14 module conf.py files (tr/eq/ti/fp/wr/wrx/tot × en/ja). Each module defines an idempotent extension that appends "myst.xref_missing" to whatever conf_common.py already provided. Portal builds no longer carry the suppression, so legitimate xref typos there will still surface as build failures under -W. A TODO comment in each conf.py points to the eventual revert path (real intersphinx_mapping URL once portal is deployed). Verified: with portal pre-built, all 14 module builds report "ビルド 成功". The cross-Sphinx-project Markdown links from this PR's commit c14ea93 still suppress correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
k-yoshimi
added a commit
that referenced
this pull request
Apr 29, 2026
* ci+tot: Layer 1 (1e-10 equivalence) baseline generation in CI Closes the third L-6 layer: tot_demo2014_short Layer 1 equivalence test now runs in CI at 1e-10 tolerance. This is the first time any Layer 1 case has executed on any platform. Changes: - tot/Makefile: extend the tot_static_stubs.o gate (introduced for Layer 2 in #185) to the standalone `tot` Fortran binary's link line. Same `ifeq ($(strip $(GFLIBS)),)` condition: production-graphics builds keep the real graphics chain, CI / no-graphics builds get the no-op stubs and link cleanly without libgsp / libg3d. - .github/workflows/python-tests.yml: add a "Generate Layer 1 eqdata baselines" step between the Layer 2/4 C ABI checks and pytest. It builds the graphics-free tot binary and runs the demo2014 short case, which writes test_run/test_output/tot_demo2014_short/ eqdata.demo2014 (~45 KB). pytest's test_equivalence.py then auto- detects the file and lifts its skipUnless guard, running the 1e-10 numerical comparison against the already-committed test_run/baselines/tot_demo2014_short/metrics.json baseline. - python/totlib/tests/fixtures/tot_ht6m_params.py: update the eq:PP0 comment. PP0 IS registered in eq_param_registry.f90 (line 118), so the original "not yet in eq_param_registry" claim was stale. Adding PP0 to SCALARS still does NOT lift the rc=3 CALCULATION_FAILED in the Python pipeline's tot_run for this case — there is a deeper fixture-vs-driver divergence. Document and skip baseline generation for tot_ht6m_short until the divergence is investigated separately. Pytest delta: 204 passed / 3 skipped -> 205 passed / 2 skipped on this branch. The new pass is test_tot_demo2014_short; the remaining skips are tr_mcp test_setup_cli (unrelated, expected) and test_tot_ht6m_short (gated behind the missing eqdata-HT6M baseline, follow-up). Local verification (macOS, GFLIBS= empty): - `make -C tot tot` -> graphics-free binary builds cleanly - `./tot/tot < test_run/inputs/tot_demo2014_short.in` produces eqdata.demo2014 in cwd - `pytest --forked --timeout=240 ... test_equivalence.py` -> test_tot_demo2014_short PASSED, test_tot_ht6m_short SKIPPED Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci+fixture: tighten Layer 1 baseline step + clarify ht6m PP0 status Code-review feedback on 45f8414: - (in-house MED #3) Use `test -s` instead of `test -f` so a silently- exiting tot run that produces a 0-byte / truncated eqdata fails the step explicitly instead of slipping through as a confusing pytest failure later. - (in-house cosmetic) Step name singularised: only one case (demo2014) is generated today, so call it "baseline (demo2014)" rather than the misleading plural. - (in-house cosmetic) Switch the step shell to `set -eo pipefail` to match the existing Layer 2/4 step style and so a pipeline mid-step failure surfaces cleanly. - (codex MED + in-house MED #4) Reconcile the in-fixture PP0 status to a single authoritative source: rewrite both the SCALARS comment and the UNREGISTERED_KEYS comment so they agree (PP0 IS registered; it is intentionally absent from the dict because adding it does not lift the deeper ht6m rc=3 divergence). The previous file held two contradictory statements about PP0's registry state. No behaviour change: pytest still 205 passed / 2 skipped on this branch, demo2014 PASSES at 1e-10, ht6m SKIP'd via the missing- eqdata-HT6M guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of the TR module refactoring per
docs/superpowers/specs/2026-04-17-tr-refactoring-design.mdsection 5.Each
USE TRCOMM(no ONLY clause) in the four target files now lists only the symbols actually referenced in that scope.Files modified
tr/trloop.f90—tr_loop(1 USE site, 21 symbols)tr/trfout.f90—tr_fout(1 USE site, ~40 symbols)tr/trparm.f90—tr_parm+tr_nlin(2 USE sites;tr_nlinlists full namelist)tr/trrslt.f90—TRGLOB+TRPRNT+tr_setup_kv(3 USE sites)No other files modified. No semantic changes.
Behavior invariance
Verified by direct pre/post regression comparison on
tr_m0904:OK: metrics match within tol=1e-10✅Note: The Phase 0 regression baseline (
test_run/baselines/) was generated in a different build environment and shows pre-existing drift from develop's HEAD (~1e-5 to 1e-3) independent of this PR. That baseline mismatch and a pre-existing libc abort at exit time are out of scope for this PR.Test plan
gfortran -O3 -m64 -std=legacycompile cleantr2binary built successfullymetrics.jsondirect comparison passes within 1e-10Note
Low Risk
Low risk refactor limited to
USE TRCOMMimport lists; main risk is accidental omission of a needed symbol causing compile/runtime issues, but behavior should be unchanged.Overview
Refactors four TR Fortran sources to replace broad
USE TRCOMMstatements with explicitONLYsymbol lists in key routines (tr_fout,tr_loop,tr_parm/tr_nlin,TRGLOB/TRPRNT/tr_setup_kv).This narrows module dependencies and makes each routine’s TRCOMM usage explicit, without intended semantic changes.
Reviewed by Cursor Bugbot for commit d5b268d. Bugbot is set up for automated code reviews on this repo. Configure here.