fix(cli): wizard CSS_PATH broken after cli.py package split#1129
Open
scoobyluu wants to merge 2 commits into
Open
fix(cli): wizard CSS_PATH broken after cli.py package split#1129scoobyluu wants to merge 2 commits into
scoobyluu wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated three wizard CSS path declarations to resolve ChangesWizard CSS path relocation
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
massgen/tests/test_cli_wizard_css_paths.py (1)
27-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd Google-style docstrings to the new functions.
The helper uses a free-form docstring, and both new tests have none. Please convert these to Google-style so this module follows the repo standard. As per coding guidelines,
**/*.py:For new or changed functions, include Google-style docstrings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@massgen/tests/test_cli_wizard_css_paths.py` around lines 27 - 65, The new helper and tests in test_cli_wizard_css_paths.py use non-standard or missing docstrings; update _resolve_css_path, test_entrypoint_declares_at_least_one_css_path, and test_every_wizard_css_path_resolves_to_an_existing_file to use Google-style docstrings consistent with the repo’s Python style. Keep the docstrings concise, use the standard summary plus Args/Returns as applicable, and replace the free-form helper docstring with the Google-style format.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@massgen/tests/test_cli_wizard_css_paths.py`:
- Around line 42-65: The CSS path tests only verify that at least one wizard
CSS_PATH exists, so a missing or rewritten wizard can slip through. Update the
tests in test_cli_wizard_css_paths.py to explicitly assert all three expected
wizard CSS_PATH declarations are present, using the existing _CSS_PATH_PATTERN
and _ENTRYPOINT_PATH checks, before validating that each resolved path exists.
If helpful, key the assertion off the specific wizard classes or the expected
match count so coverage stays aligned with the three wizard flows.
---
Nitpick comments:
In `@massgen/tests/test_cli_wizard_css_paths.py`:
- Around line 27-65: The new helper and tests in test_cli_wizard_css_paths.py
use non-standard or missing docstrings; update _resolve_css_path,
test_entrypoint_declares_at_least_one_css_path, and
test_every_wizard_css_path_resolves_to_an_existing_file to use Google-style
docstrings consistent with the repo’s Python style. Keep the docstrings concise,
use the standard summary plus Args/Returns as applicable, and replace the
free-form helper docstring with the Google-style format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e25944a-07c5-4b18-9401-12a7c247befa
📒 Files selected for processing (2)
massgen/cli/entrypoint.pymassgen/tests/test_cli_wizard_css_paths.py
scoobyluu
pushed a commit
to scoobyluu/MassGen
that referenced
this pull request
Jun 26, 2026
Address CodeRabbit feedback on PR massgen#1129: - Tighten the regression test to assert exactly the three expected wizard CSS_PATH declarations (setup, quickstart, first-run). The prior "at least one" assertion would have silently masked an accidental wizard removal. - Add Google-style docstrings to _resolve_css_path and both test functions so this module satisfies the repo's docstring coverage standard.
…e split The setup, quickstart, and first-run wizards in massgen/cli/entrypoint.py declared CSS_PATH as `Path(__file__).parent / "frontend" / ...`. That worked when the code lived in massgen/cli.py, but after commit 68fcd74 split it into the massgen/cli/ package, __file__.parent resolves to massgen/cli/, so the path pointed at the nonexistent massgen/cli/frontend/.... `massgen --setup` (and the quickstart/first-run flows) crashed on launch with `StylesheetError: unable to read CSS file ... textual_themes/dark.tcss`. Use `.parent.parent` so the path resolves to massgen/frontend/displays/ textual_themes/dark.tcss, matching the actual theme location. Added a regression test (test_cli_wizard_css_paths.py) that parses every CSS_PATH literal in entrypoint.py and asserts the resolved path exists, so a future module relocation surfaces the breakage before users do.
Address CodeRabbit feedback on PR massgen#1129: - Tighten the regression test to assert exactly the three expected wizard CSS_PATH declarations (setup, quickstart, first-run). The prior "at least one" assertion would have silently masked an accidental wizard removal. - Add Google-style docstrings to _resolve_css_path and both test functions so this module satisfies the repo's docstring coverage standard.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
massgen/tests/test_cli_wizard_css_paths.py (1)
63-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScope this assertion to the three wizard classes.
Lines 63-68 currently count every matching
CSS_PATHliteral inentrypoint.py, not just_QuickstartWizardApp,SetupWizardApp, and_FirstRunSetupApp. A future non-wizard app with the same literal shape would fail this regression even though the wizard contract is still intact. Parsing those three class assignments explicitly would keep the test aligned with what this PR is actually protecting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@massgen/tests/test_cli_wizard_css_paths.py` around lines 63 - 68, The test in test_cli_wizard_css_paths.py is counting every CSS_PATH literal in entrypoint.py instead of only the three wizard classes. Update the assertion logic to explicitly scope the search to _QuickstartWizardApp, SetupWizardApp, and _FirstRunSetupApp, using those class assignments as the source of truth. Keep the existing _ENTRYPOINT_PATH and _CSS_PATH_PATTERN usage, but narrow the parsing so unrelated future apps do not affect the wizard CSS_PATH count.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@massgen/tests/test_cli_wizard_css_paths.py`:
- Around line 63-68: The test in test_cli_wizard_css_paths.py is counting every
CSS_PATH literal in entrypoint.py instead of only the three wizard classes.
Update the assertion logic to explicitly scope the search to
_QuickstartWizardApp, SetupWizardApp, and _FirstRunSetupApp, using those class
assignments as the source of truth. Keep the existing _ENTRYPOINT_PATH and
_CSS_PATH_PATTERN usage, but narrow the parsing so unrelated future apps do not
affect the wizard CSS_PATH count.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b06a88b5-281a-4b5c-9cae-c25cf472a6b5
📒 Files selected for processing (1)
massgen/tests/test_cli_wizard_css_paths.py
scoobyluu
force-pushed
the
fix/cli-wizard-css-path
branch
from
b9101a8 to
a0cc1f4
Compare
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
1 participant
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
massgen --setup,--quickstart, and the first-run wizard crash on launch withStylesheetError: unable to read CSS file ... textual_themes/dark.tcss. After commit 68fcd74 movedmassgen/cli.pyinto themassgen/cli/package,Path(__file__).parentinentrypoint.pynow resolves tomassgen/cli/, so the wizardCSS_PATHdefinitions point at the nonexistentmassgen/cli/frontend/displays/textual_themes/dark.tcss.CSS_PATHdeclarations inmassgen/cli/entrypoint.py(lines 2245, 2366, 2806) from.parentto.parent.parentso they resolve back tomassgen/frontend/displays/textual_themes/dark.tcss.massgen/tests/test_cli_wizard_css_paths.py: parses everyCSS_PATH = Path(__file__).parent... / "*.tcss"literal inentrypoint.pyand asserts each resolves to a real file, so a future module relocation surfaces the breakage in CI instead of via user crash.Test plan
uv run pytest massgen/tests/test_cli_wizard_css_paths.py -v— 2 passedtest_every_wizard_css_path_resolves_to_an_existing_filefail with the three missingmassgen/cli/frontend/...pathsuv run massgen --setupshould reach the wizard UI instead of raisingStylesheetErrorSummary by CodeRabbit
Bug Fixes
Tests
.tcssfile exists on disk, preventing future regressions from layout/path changes.