./mfc.sh context: what to re-read to check a diff

[sbryngelson]

Summary

On long tasks, re-reading drifts: the agent works from memory of the code and misses details exploration does not surface. Three kinds recur: Fypp-generated copies of a changed line (#1823's c_sum_Yi_Phi was private in one HLLC copy and shared in the other), module variables set as a side effect elsewhere (dir_idx, is1), and every call site a macro change affects (#2018). ./mfc.sh context prints that set for a diff.

./mfc.sh context              # what to re-read for this branch (diff against the merge base with origin/master)
./mfc.sh context --base HEAD  # only uncommitted changes

Report sections:

• changed routines, in full up to 300 lines, otherwise the GPU kernel holding the change; "line N is emitted K times (Fypp loop): check every copy" when a changed line is generated more than once
• changed macros (#:def): call sites, direct or through wrapper macros, filtered to those passing the arguments the change touches
• callers and callees
• module variables the changed code uses that other routines assign at run time
• changed lines in no routine (module declarations, code this build configuration leaves out), new, deleted and untracked files, and a warning when the diff does not match the source it is mapped onto

How

context_expand.py runs Fypp itself with cmake/Fypp.cmake's flags (a test fails if they drift) and the includes the build generates (get_generated_files, Case.get_fpp), into a cache keyed by content under build/context. No build or compiler is needed. context.py maps diff lines through Fypp's # N "file" markers, mapping continuation lines to their statement and counting copies as separate appearances, so one macro expansion is not reported as several.

Claude review

The review workflow gets the report as extra reading (.claude-review/context.md). The step runs this checkout's tool on a worktree of the PR's merge base (--root, --expand base), so the diff's old-side line numbers point at the code they came from, and no PR code runs in the privileged job. If it fails the review still runs, with a warning in the log.

The re-read rule in .claude/CLAUDE.md now points at the command.

Testing

• 28 unit tests in toolchain/mfc/test_context.py; ./mfc.sh precheck passes (796 toolchain tests).
• On real diffs:

diff	output	what it shows
one-line edit in HLLC's #:for HYPO body	743 lines	"line 998 is emitted 6 times"; the kernel holding it (routine is 1598 lines); dir_idx, is1, isx set by s_populate_riemann_states_variables_buffers, dir_idx by s_initialize_cbc
#2018 (OMP_PARALLEL_LOOP)	15 lines	21 call sites passing reduction/copy/default, including all six loops #2018 changed (420 sites unfiltered)
#1952 (dt abort)	134 lines	s_compute_dt, its caller and callees, q_cons_ts set by s_tvd_rk and s_save_data
#1953 via the CI path (merge-base worktree)	636 lines	the three per-target s_assign_default_values_to_user_inputs kept apart, plus declaration changes listed as not in a routine

• 7 s cold, 2-3 s warm on a 128-core node (Fypp runs 16 files at a time).

Limits

• Matching is by name with regexes: generic interfaces and functions not named f_* are missed as callers.
• Macro call sites are found by name (Fypp markers record where a macro is defined, not where it expands).
• Cantera-generated thermochem modules are not expanded.
• With --expand base, a routine the PR appends is attributed to the routine before it.
• The cache under build/context is not pruned.

[github-actions]

Claude Code Review

Head SHA: dd61ce7

Files changed:

• 7
• .claude/CLAUDE.md
• .github/workflows/claude-code-review.yml
• toolchain/main.py
• toolchain/mfc/cli/commands.py
• toolchain/mfc/context.py
• toolchain/mfc/context_expand.py
• toolchain/mfc/test_context.py

Findings:

• medium toolchain/mfc/context_expand.py: _write_includes does sys.path.insert(0, repo/"toolchain") and then imports mfc.case and mfc.params.generators.fortran_gen. By then mfc is already in sys.modules (it is imported via from mfc import state and by main.py), so those submodules load from the running checkout, not from --root. In the new CI step the running toolchain is the base-branch tip and --root is the older PR merge base. The generated parameter includes and case.fpp can therefore differ from the merge-base .fpp sources, which gives spurious "could not expand" errors or expansions that don't match the source. The error is swallowed by continue-on-error, so the review would just get a thin or empty context.md. Suggested fix: document that generated includes always come from the running toolchain, or run the generators in a subprocess with cwd=repo. Confidence: moderate, from reading the import order only.
• medium toolchain/mfc/context.py: build_index matches USE and DECL one physical line at a time, but the Fypp flags (--no-folding, nocontlines) keep & continuation lines as separate lines. A use m, only: a, & line yields an only-list containing "&", and names on the following line are dropped. A module-variable declaration split across lines loses its continuation names. Both feed module_vars and owners in _globals, so the "set elsewhere at run time" section can silently omit variables. Suggested fix: join & continuations (as _joined already does for macros) before matching in build_index. Confidence: moderate; I did not check how often MFC splits use or declaration lines.
• medium toolchain/mfc/context.py: in parse_diff, line.startswith("--- ") and "+++ " are tested before the hunk-body branch. A removed line whose text starts with -- or an added line starting with ++ is read as a file header, which resets cur/old_path and drops the rest of that file's hunks. This is unlikely in Fortran sources but possible. Suggested fix: ignore headers while still inside a hunk, tracking the remaining old/new line counts from the @@ header. Confidence: low-moderate.