Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis change refactors the CLI infrastructure to centralize Typer configuration, introduces new schema navigation and validation modules, and adds JSON schema generation capabilities alongside improved config validation with template support and VSCode integration. Changes
Sequence DiagramssequenceDiagram
participant User
participant CLI as schema command
participant Navigator as schema_navigation
participant Output
User->>CLI: schema train [section] [--keywords] [--flat]
CLI->>Navigator: filter_schema(schema, section)
Navigator->>Navigator: resolve $refs and variants
Navigator-->>CLI: (node, error?)
alt Section found
CLI->>Navigator: print_keywords(schema, section)
Navigator->>Navigator: aggregate properties & variants
Navigator-->>CLI: keyword list
CLI->>Output: print formatted keywords
else Section not found
CLI->>Output: print error message
end
User->>CLI: schema md --keywords
Note over CLI,Output: Similar flow for md configuration
sequenceDiagram
participant User
participant CLI as validate command
participant Validator as validation logic
participant Template as template handling
participant Output
User->>CLI: validate train-config config.yaml [--template]
CLI->>Validator: _validate_config(TrainConfig, path, config, label)
Validator->>Validator: load and parse config YAML
alt Template requested
Validator->>Template: apply _TEMPLATE_DATA_DEFAULTS
Template-->>Validator: config with defaults
end
Validator->>Validator: pydantic validation
alt Validation succeeds
Validator->>Output: print success
else Validation fails
Validator->>Validator: _format_errors(ValidationError)
Validator->>Output: print formatted errors
Validator->>CLI: exit(1)
end
sequenceDiagram
participant User
participant CLI as schema vscode
participant Generator as schema handler
participant FileIO as file system
User->>CLI: schema vscode
CLI->>Generator: _handle_schema(TrainConfig, "train", ...)
Generator->>Generator: generate JSON schema for config
Generator->>FileIO: write to .vscode/settings.json
FileIO-->>Generator: file written
CLI->>Generator: _handle_schema(MDConfig, "md", ...)
Generator->>Generator: generate JSON schema for config
Generator->>FileIO: append/update .vscode/settings.json
FileIO-->>Generator: file written
CLI->>Output: print success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apax/cli/apax_app.py (2)
219-223: Placeholder paths use/tmpwhich triggers security lint warnings.These paths are validation placeholders and never used for actual file operations, so the security risk is minimal. However, using clearly fictional paths would silence the linter and make intent clearer.
♻️ Alternative placeholder paths
_TEMPLATE_DATA_DEFAULTS = { - "directory": "/tmp/apax_validate", - "experiment": "placeholder", - "data_path": "/tmp/apax_validate/placeholder.extxyz", + "directory": "PLACEHOLDER_DIRECTORY", + "experiment": "PLACEHOLDER_EXPERIMENT", + "data_path": "PLACEHOLDER_DATA_PATH", }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apax/cli/apax_app.py` around lines 219 - 223, The placeholder defaults in _TEMPLATE_DATA_DEFAULTS use /tmp which triggers security linters; update the dict (symbol: _TEMPLATE_DATA_DEFAULTS in apax_app.py) to use clearly fictional/nonexistent paths (e.g. "/nonexistent/apax_validate" or "/placeholder/path/apax_validate") for "directory" and "data_path" while keeping "experiment": "placeholder" unchanged so the values remain clearly fake and never used for real file operations.
206-216: Consider suppressing implicit exception chaining.When raising
typer.Exitinside anexceptblock, Python implicitly chains the exceptions. Since the validation error is already formatted and printed, addingfrom Nonewould produce cleaner output without the "During handling of the above exception" message.♻️ Suggested improvement
except ValidationError as e: print(f"{e.error_count()} validation errors for config") print(_format_errors(e)) console.print("Configuration Invalid!", style="red3") - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apax/cli/apax_app.py` around lines 206 - 216, The except block in _validate_config currently raises typer.Exit while handling a ValidationError, which causes implicit exception chaining and extra traceback text; modify the raise to suppress chaining by re-raising typer.Exit from None (i.e., use "raise typer.Exit(code=1) from None") so the printed/ formatted ValidationError remains the only error output and no "During handling of the above exception" appears; update the raise in the except block that catches ValidationError in _validate_config accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apax/cli/apax_app.py`:
- Around line 219-223: The placeholder defaults in _TEMPLATE_DATA_DEFAULTS use
/tmp which triggers security linters; update the dict (symbol:
_TEMPLATE_DATA_DEFAULTS in apax_app.py) to use clearly fictional/nonexistent
paths (e.g. "/nonexistent/apax_validate" or "/placeholder/path/apax_validate")
for "directory" and "data_path" while keeping "experiment": "placeholder"
unchanged so the values remain clearly fake and never used for real file
operations.
- Around line 206-216: The except block in _validate_config currently raises
typer.Exit while handling a ValidationError, which causes implicit exception
chaining and extra traceback text; modify the raise to suppress chaining by
re-raising typer.Exit from None (i.e., use "raise typer.Exit(code=1) from None")
so the printed/ formatted ValidationError remains the only error output and no
"During handling of the above exception" appears; update the raise in the except
block that catches ValidationError in _validate_config accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41b99637-97eb-457b-baa1-880e3904937b
📒 Files selected for processing (5)
apax/cli/apax_app.pyapax/cli/templates/train_config_full.yamlapax/cli/templates/train_config_minimal.yamlapax/config/flat_schema.pyapax/config/schema_navigation.py
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.
Adds some subcommands to apax schema that allow navigating the individual sections of the apax config files. Convenient for agents to understand the nested parameters in the trianing and MD configs.
Summary by CodeRabbit
New Features
Configuration Changes