Skip to content

feature-skip-preprocess-on-restart - #1761

Open
danieljvickers wants to merge 3 commits into
MFlowCode:masterfrom
danieljvickers:feature-skip-preprocess-on-restart
Open

feature-skip-preprocess-on-restart#1761
danieljvickers wants to merge 3 commits into
MFlowCode:masterfrom
danieljvickers:feature-skip-preprocess-on-restart

Conversation

@danieljvickers

Copy link
Copy Markdown
Member

I have deleted valuable restart data one-to-many times because I forget to call -t simulation on restart. This PR makes the adjustment that pre_process must be explicitly called on restart. This still allows one to restart cases with preprocess if they desire to modify patches, but prevents the deletion of all data from the directory because one forgets to set a target.


Acknowledgement

  • I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

PR template credit: junegunn

…d on restart. This is a preventative measure against deleting all of your restart data.
@github-actions

Copy link
Copy Markdown

Claude Code Review

Head SHA: 2835916

Files changed:

  • 4
  • docs/documentation/case.md
  • toolchain/mfc/args.py
  • toolchain/mfc/lint_docs.py
  • toolchain/mfc/run/run.py

Findings:

  • args["targets_explicit"] = any(tok in ("-t", "--targets") for tok in cli_argv) (toolchain/mfc/args.py:92) only matches tokens that are exactly -t or --targets. It misses argparse's other valid forms for the same flag: --targets=simulation (equals syntax), an unambiguous abbreviation like --target (argparse allow_abbrev is on by default), or an attached short form like -tsimulation. If a user runs a restart with ./mfc.sh run case.py --targets=pre_process simulation (or any of these other forms) and t_step_start > 0, targets_explicit is computed as False, so run() (toolchain/mfc/run/run.py:164-166) silently drops pre_process from the target list even though the user explicitly requested it — the opposite of the intended "only auto-skip when the user didn't ask for it" behavior, and it happens silently (just a yellow console note, easy to miss), regenerating stale/no pre_process output instead of what was explicitly asked for.

@sbryngelson

Copy link
Copy Markdown
Member

lol. check ai comments when you have a chance

sbryngelson
sbryngelson previously approved these changes Aug 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents default preprocessing from overwriting restart data.

Changes:

  • Detects explicitly requested run targets.
  • Skips preprocessing for restart runs by default.
  • Documents the behavior and updates documentation linting.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
toolchain/mfc/run/run.py Adds restart-aware preprocessing exclusion.
toolchain/mfc/args.py Records explicit target selection.
toolchain/mfc/lint_docs.py Allows the documented target name.
docs/documentation/case.md Documents restart behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread toolchain/mfc/args.py
Comment thread toolchain/mfc/run/run.py Outdated
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.67%. Comparing base (e2f0e26) to head (c3a27fe).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1761   +/-   ##
=======================================
  Coverage   61.67%   61.67%           
=======================================
  Files          84       84           
  Lines       21619    21619           
  Branches     3196     3196           
=======================================
  Hits        13334    13334           
  Misses       6093     6093           
  Partials     2192     2192           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants