Fix fp-stability log preservation and add local/inline reporting#1537
Merged
Conversation
fp-stability-logs/ was created but never written to: all logs went to a tempdir deleted in _run_case's finally block, so the directory was empty after every run, locally and in CI (the artifact upload shipped nothing). The markdown report was also only emitted to GITHUB_STEP_SUMMARY, so local runs got no report at all. - preserve each case's text artifacts (*.log, *.out, *.inp, cancel_gen.txt) into fp-stability-logs/<case>/ before the work dir is removed, mirroring the run-dir layout; bulky .dat field data is skipped - always write the report to fp-stability-logs/summary.md and print its path; still appended to GITHUB_STEP_SUMMARY when set
… console The file:line sites were already collected and reported in summary.md and GitHub annotations, but the console only printed counts. Print the top 5 cancellation sites (ranked by digits lost, fypp-expansion ambiguity marked) and float-max overflow sites inline, eliding the rest to summary.md.
For each cancellation and float-max site shown in the report, embed a line-numbered excerpt of the source (marked line plus 3 lines of context) as a nested fenced code block, so the offending code is readable without opening the file. Unresolvable sources degrade to no snippet.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the FP-stability suite’s log/report handling so artifacts aren’t lost when the per-case scratch directory is removed, and enhances reporting to make error sources easier to locate (console file:line output + summary.md snippets).
Changes:
- Preserve per-case text artifacts by copying them into
fp-stability-logs/<case>/before removing the scratchwork_dir. - Always write a local markdown report to
fp-stability-logs/summary.md(and still append toGITHUB_STEP_SUMMARYin CI). - Add inline source excerpts for cancellation / float-max sites and add unit tests covering the new behaviors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| toolchain/mfc/fp_stability.py | Adds log preservation and improves console reporting for top-ranked cancellation/overflow sites. |
| toolchain/mfc/fp_stability_report.py | Writes summary.md locally and embeds source snippets under reported sites. |
| toolchain/mfc/fp_stability_metrics.py | Adds _source_snippet() helper to produce line-numbered excerpts with context. |
| toolchain/mfc/test_fp_stability.py | Adds unit tests for snippet extraction, local report writing, and log preservation behavior. |
The cells/cell-steps feasibility guard protects against accidentally launching hours of Verrou runs, but offered no escape hatch for users who knowingly want a larger grid or more time steps. --force runs the case anyway, printing the expected cost multiple and a reminder to trim passes (-N 1, --no-*). Guard errors now mention the flag.
…back) - _preserve_logs runs in _run_case's finally block; an OSError there (disk full, dest collision) would replace the case's real outcome and, being uncatchable by the suite's MFCException handler, abort the whole run. Make it best-effort with a printed warning, matching the adjacent ignore_errors rmtree. - _emit_github_summary creates log_dir if missing instead of raising FileNotFoundError at the very end of a run.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1537 +/- ##
==========================================
+ Coverage 60.63% 60.80% +0.16%
==========================================
Files 73 73
Lines 20219 20199 -20
Branches 2937 2932 -5
==========================================
+ Hits 12259 12281 +22
+ Misses 5972 5932 -40
+ Partials 1988 1986 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Description
created
fp-stability-logs/and printed its path, but nothing ever wrote into it: all logs went to a tempdir deleted in_run_case'sfinallyblock. The directory was empty after every run — locally and in CI, where the artifact upload offp-stability-logs/has been shipping nothing. The markdown report was also emitted only toGITHUB_STEP_SUMMARY, so local runs got no report at all.This PR fixes both and improves error-source visibility:
*.log,*.out,*.inp,cancel_gen.txt) are copied intofp-stability-logs/<case>/(mirroring the run-dir layout) before the work dir is removed, including on failure paths. Bulky.datfield data is skipped.fp-stability-logs/summary.md(path printed at the end); still appended toGITHUB_STEP_SUMMARYwhen set.file:linedirectly to the console instead of just counts.summary.mdembeds a line-numbered excerpt (marked line ± 3 lines of context) under each cancellation/float-max site as a nestedf90code block, so the offending code is readable without opening the file.Type of change
Testing
toolchain/mfc/test_fp_stability.py(log preservation keeps text artifacts / skips field data; localsummary.mdwritten without CI env; snippet marks the right line with correct context and degrades to empty for unresolvable sources). All 20 pass.fp-stability-logs/now containspre.log, per-runverrou.log/sim.out,.inpfiles, andsummary.mdwith ranked cancellation sites and source snippets../mfc.sh precheckpasses (via pre-commit hook on each commit).Checklist