refactor(workflow): replace env-var stage handoff with workflow context#186
Open
liamnwhite1 wants to merge 11 commits into
Open
refactor(workflow): replace env-var stage handoff with workflow context#186liamnwhite1 wants to merge 11 commits into
liamnwhite1 wants to merge 11 commits into
Conversation
46e8c4b to
2696c7f
Compare
gknapp1
reviewed
Jun 3, 2026
gknapp1
left a comment
Collaborator
There was a problem hiding this comment.
First pass through reading the code, this seems like a reasonable approach. That said, I do need to spend some more time reading through it in detail and testing functionality before approving.
Collaborator
There was a problem hiding this comment.
This further reinforces my experience from #185 on the decisions file. The important parts of this decision are included in changes to ARCHITECTURE.md, so I'm not seeing a strong benefit for this more verbose file. @liamnwhite1 is there a reason to keep this that I'm missing?
That said, some of this rationale could be in the PR description to assist reviewer(s).
Remove the workflow-step communication that previously relied on mutable MYNA_* environment variables between Myna workflow stages. Changes: - add explicit `WorkflowContext` support for input file, current step, previous step, step class, and step index - update `MynaApp` to consume explicit workflow context first while preserving legacy environment-variable fallback for direct script invocation - run component configure/execute/postprocess stages in-process instead of launching Python wrapper subprocesses - preserve stage CLI argument behavior by temporarily patching `sys.argv` during in-process stage calls - update `myna run` and `myna sync` to pass workflow state directly to components instead of mutating `MYNA_*` variables - replace affected app/database direct env reads with context-backed accessors or app instance attributes - add regression tests for context propagation, stage argument forwarding, and absence of workflow env mutation Closes #125 Verification: - `uv run pytest` - 55 passed, 1 skipped, 11 deselected
Document the workflow context architecture and application guidance in the current root-docs layout. Extend the docs harness to require architecture documentation for sensitive changes and enforce PR template guidance/headings. Co-authored-by: White, Liam <whiteln@ornl.gov>
2696c7f to
e163152
Compare
- fixes error in input file with meshing settings - adds flexibility in solidification data concatenation from additivefoam to handle headers (different versions of additivefoam have different behaviors with outputting headers for decomposed ExacA files)
- forgot to remove old code for solidification data reconstruction, so caused conflict
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
Replace workflow-stage communication through mutable
MYNA_*environment variables with explicitWorkflowContextpropagation, then document the architecture decision and add harness guardrails for future docs-sensitive changes.Related issues
Details
This refactor adds
WorkflowContextto carry the input file, current step, previous step, step class, and step index through workflow execution.myna runandmyna syncnow pass workflow state directly into component stages, and stage execution runs in-process while preserving CLI argument behavior by temporarily patchingsys.argv.Affected apps, databases, and metadata helpers now read workflow state through context-backed accessors or app instance attributes instead of directly depending on mutable environment variables. Legacy environment-variable fallback remains in place for direct script invocation.
The PR also updates architecture and developer documentation, adds an ADR for the workflow context decision, expands application documentation, and strengthens the docs harness so future PRs consider architecture documentation and complete the repository PR template.
Impact
Behavioral impact is medium because workflow stage execution and state propagation are changed across core workflow, component, app, and database paths. Risk is mitigated by preserving direct-invocation fallback behavior and adding regression coverage for context propagation, stage argument forwarding, and avoiding workflow environment mutation.
Maintainability improves by making workflow state explicit, testable, and less dependent on process-global environment state.
Testing strategy
Validated with:
uv run pytestpython3 scripts/check_docs_harness.pyuv run python scripts/check_docs_harness.pypython3 -m py_compile scripts/check_docs_harness.pygit diff --checkThe full test run reported 55 passed, 1 skipped, and 11 deselected.
Checklist
ARCHITECTURE.md, developer docs, and/or ADRs where needed, or explained why not.