feat(factorio-cycle-calculator): Improve Codegen Workflow Via Mise Task#122
feat(factorio-cycle-calculator): Improve Codegen Workflow Via Mise Task#122
Conversation
95a6a06 to
0da07b3
Compare
There was a problem hiding this comment.
Pull request overview
This pull request improves the code generation workflow for the factorio-cycle-calculator by introducing a mise task for schema-driven model generation and adding a script entry point for easier application launching.
Changes:
- Added mise task
schema:generatethat fetches the Factorio data-raw JSON schema from a remote URL and generates Python dataclasses using datamodel-code-generator - Introduced a
[build-system]configuration and script entry point (factorio-cycle-calculator) for running the Streamlit app - Updated generated models to use modern Python 3.12 syntax (StrEnum, type statements, pipe unions)
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .mise.toml | Enabled experimental monorepo support and registered factorio-cycle-calculator as a config root |
| src/private/app/factorio-cycle-calculator/.mise.toml | Added schema:generate task for pulling schema from URL and regenerating models |
| src/private/app/factorio-cycle-calculator/pyproject.toml | Added build-system, script entry point, and datamodel-codegen configuration |
| src/private/app/factorio-cycle-calculator/src/factorio_cycle_calculator/main.py | New entry point for launching Streamlit app via script command |
| src/private/app/factorio-cycle-calculator/src/factorio_cycle_calculator/app.py | Simplified import path by removing sys.path manipulation |
| src/private/app/factorio-cycle-calculator/src/factorio_cycle_calculator/generated/data_raw_models.py | Regenerated models with modern Python syntax and updated timestamp |
| src/private/app/factorio-cycle-calculator/README.md | Updated documentation with new script entry point and schema generation instructions |
| uv.lock | Added debug and http extras for datamodel-code-generator, changed package source from virtual to editable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/private/app/factorio-cycle-calculator/src/factorio_cycle_calculator/__main__.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95a6a0602e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
0da07b3 to
0986a73
Compare
0986a73 to
aaa47d5
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/private/app/factorio-cycle-calculator/src/factorio_cycle_calculator/app.py:70
- The raised
GeneratedModelsImportErrorultimately tells users to “Regenerate with datamodel-codegen”, but the README indicates generation is driven via the mise task. To keep the guidance accurate and actionable, update the error messaging (either thereasonhere or the exception’s message template) to point tomise //src/private/app/factorio-cycle-calculator:schema:generate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --group dev \ | ||
| ruff check \ | ||
| --fix \ | ||
| --exit-zero \ |
There was a problem hiding this comment.
Using ruff check --exit-zero will make the generation task succeed even if Ruff still reports unfixable issues in the generated file, which can mask real regressions in the codegen output. Prefer removing --exit-zero and instead adjusting per-file ignores (or a targeted --select/--ignore) so the task fails when new, unexpected Ruff issues appear.
| --exit-zero \ |
| def main(argv: list[str] | None = None) -> None: | ||
| """Launch the Streamlit application.""" | ||
| module = import_module("factorio_cycle_calculator.app") | ||
| app_path = Path(module.__file__).resolve() | ||
| new_argv = ["streamlit", "run", str(app_path)] | ||
| if argv is not None: | ||
| new_argv.extend(argv) | ||
|
|
There was a problem hiding this comment.
The console script entry point ignores user-provided CLI args when argv is left as None. Since main() is invoked by console_scripts without arguments, flags like --server.port passed to factorio-cycle-calculator will be dropped. Consider defaulting argv to sys.argv[1:] so options are forwarded to streamlit run.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aaa47d54ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --group dev \ | ||
| datamodel-codegen \ | ||
| --url https://raw.githubusercontent.com/jacquev6/factorio-data-raw-json-schema/refs/heads/main/factorio-data-raw-json-schema.full.json \ | ||
| --output src/factorio_cycle_calculator/generated/data_raw_models.py \ |
There was a problem hiding this comment.
Resolve schema task paths against project directory
Running the documented command mise //src/private/app/factorio-cycle-calculator:schema:generate from the repo root executes this task with no task dir, but the generated-file path here is relative (src/factorio_cycle_calculator/...) and therefore points outside the package directory; this makes codegen/ruff operate on the wrong location (or fail if the path does not exist), so the intended src/private/app/factorio-cycle-calculator/src/factorio_cycle_calculator/generated/data_raw_models.py is not reliably updated.
Useful? React with 👍 / 👎.
| if argv is not None: | ||
| new_argv.extend(argv) |
There was a problem hiding this comment.
Forward CLI arguments to Streamlit entrypoint
The console script entry point invokes main() without parameters, and this code only appends extra args when argv is explicitly passed, so user-supplied flags on factorio-cycle-calculator (for example --help or --server.port) are silently dropped and never reach Streamlit; defaulting to sys.argv[1:] is needed to preserve CLI behavior.
Useful? React with 👍 / 👎.
No description provided.