Skip to content

Fix .silol.series generation#1591

Open
wilfonba wants to merge 3 commits into
MFlowCode:masterfrom
wilfonba:IOFix
Open

Fix .silol.series generation#1591
wilfonba wants to merge 3 commits into
MFlowCode:masterfrom
wilfonba:IOFix

Conversation

@wilfonba

Copy link
Copy Markdown
Contributor

This PR moves the generation of the .silo.series file before the failure message in MFC.sh. If post process failed, then the silo series file wouldn't be generated. I'm not sure how this got reordered, because I thought I'd done the correct ordering when I first merged this. Git blame shows that I just did it wrong though.

@wilfonba wilfonba requested a review from sbryngelson as a code owner June 13, 2026 17:15
Copilot AI review requested due to automatic review settings June 13, 2026 17:15

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

Note

Copilot was unable to run its full agentic suite in this review.

This PR fixes the ordering of .silo.series generation so it happens before the post-process failure path in MFC.sh, ensuring the series file is still produced when post-processing fails.

Changes:

  • Move the generate_silo_series.py invocation earlier in the post_process target block.
  • Ensure .silo.series generation occurs before the $code == 22 early-exit error message.

Comment thread toolchain/templates/include/helpers.mako
@github-actions

github-actions Bot commented Jun 13, 2026

Copy link
Copy Markdown

Claude Code Review

Head SHA: 454d7f6

Files changed:

  • 1
  • toolchain/templates/include/helpers.mako

Findings:

generate_silo_series.py now runs unconditionally even when post_process has failed

In run_epilogue, the diff moves the generate_silo_series.py invocation from after the error-exit checks to before them:

+% if target.name == 'post_process':
+    python3 "${MFC_ROOT_DIR}/toolchain/templates/include/generate_silo_series.py" '${os.path.dirname(input)}'
+% endif
+
     if [ $code -eq 22 ]; then
         ...
     fi

     if [ $code -ne 0 ]; then
         exit 1
     fi

-% if target.name == 'post_process':
-    python3 "${MFC_ROOT_DIR}/toolchain/templates/include/generate_silo_series.py" '${os.path.dirname(input)}'
-% endif

Because code=$? is captured from post_process before this block, the error-exit logic ($code -ne 0) still correctly terminates on failure. However, the silo-series script is now invoked before that check, so it will run even when post_process exited non-zero (including the case-file-error code 22). Previously it only ran on success.

Running generate_silo_series.py over incomplete or corrupt post-process output is likely to produce a broken silo series and may mask the real error by emitting spurious output before the failure message.

@sbryngelson sbryngelson marked this pull request as draft June 15, 2026 03:38
@sbryngelson sbryngelson marked this pull request as ready for review June 15, 2026 23:26
@sbryngelson

Copy link
Copy Markdown
Member

@wilfonba, what do you think of the Claude review above?

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