-
Notifications
You must be signed in to change notification settings - Fork 0
feat(cli): add --output-dir option to run and resume commands #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add -o/--output-dir option to both `run` and `resume` CLI commands, allowing users to specify where workflow outputs are stored. The Workflow class already supported output_dir, this change exposes it via the CLI interface. Closes #35 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @PSthelyBlog, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the command-line interface by adding a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully adds the --output-dir option to the run and resume CLI commands, allowing users to specify a custom output directory for workflow artifacts. The implementation is clean and correctly integrates with the existing Workflow class. I've identified one potential issue regarding the behavior of the resume command when the --output-dir flag is not provided, which may lead to unexpected behavior. See my detailed comment below.
| def resume( | ||
| state_id: str = typer.Option(None, "--id", help="Specific workflow ID to resume"), | ||
| output_dir: Path | None = typer.Option( | ||
| None, "--output-dir", "-o", help="Directory for workflow outputs (overrides saved setting)" | ||
| ), | ||
| ) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a logic issue when resuming a workflow without the --output-dir flag. The Workflow class does not use the output_dir saved in the workflow's state. Instead, it falls back to a default directory. This means that resuming a workflow will cause its output to go to a different directory than the original one, unless --output-dir is explicitly provided again. This is likely not the intended behavior and could lead to artifacts being saved in an unexpected location.
To address this, the Workflow constructor in src/game_workflow/orchestrator/workflow.py should be modified to check for and use the output_dir from the loaded state if no override is given. Here's a suggested change:
# In Workflow.__init__ in src/game_workflow/orchestrator/workflow.py
if output_dir is not None:
self.output_dir = output_dir
elif state is not None and (saved_dir := state.metadata.get("output_dir")):
self.output_dir = Path(saved_dir)
else:
self.output_dir = settings.workflow.state_dir / self.state.idWhile the fix is outside the files in this PR, it's important for the correctness of this new feature.
Summary
-o/--output-diroption to theruncommand-o/--output-diroption to theresumecommandChanges
The
Workflowclass already supported anoutput_dirparameter, but it wasn't exposed via the CLI. Users can now control where workflow outputs (design docs, game builds, publish artifacts) are stored.Usage
Test plan
--helpoutput shows new option for both commandsCloses #35
🤖 Generated with Claude Code