LCORE-3141: models group schema export ability - #2263
Conversation
WalkthroughAdds ChangesModel group schema dumping
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ArgumentParser
participant lightspeed_stack.main
participant models_dumper
CLI->>ArgumentParser: Parse --dump-models-group
ArgumentParser-->>lightspeed_stack.main: Provide selected model group
lightspeed_stack.main->>models_dumper: Dump group schemas
models_dumper-->>lightspeed_stack.main: Return result or exception
lightspeed_stack.main-->>CLI: Log outcome and exit
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
a277fb4 to
86783a5
Compare
86783a5 to
516dfb9
Compare
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/lightspeed_stack.py`:
- Around line 227-237: Update the main() documentation in
src/lightspeed_stack.py to describe --dump-models-group and state that dump
failures exit with status 1. Update the README.md usage synopsis at lines
853-855 to include [-gr DUMP_MODELS_GROUP].
- Around line 75-82: Make the argument parser reject simultaneous use of
dump_models and dump_models_group instead of allowing main() to silently
prioritize dump_models. Place both options in a mutually exclusive argument
group, or add equivalent validation before main() dispatch, while preserving
each mode’s existing standalone behavior.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: de0f36b0-18e1-4ae4-b109-972239d00887
📒 Files selected for processing (2)
README.mdsrc/lightspeed_stack.py
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: integration_tests (3.12)
- GitHub Check: ruff
- GitHub Check: unit_tests (3.13)
- GitHub Check: Pylinter
- GitHub Check: radon
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
README.mdsrc/lightspeed_stack.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/lightspeed_stack.py
🧠 Learnings (3)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/lightspeed_stack.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/lightspeed_stack.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/lightspeed_stack.py
🔇 Additional comments (1)
src/lightspeed_stack.py (1)
30-32: LGTM!
| parser.add_argument( | ||
| "-gr", | ||
| "--dump-models-group", | ||
| dest="dump_models_group", | ||
| help="dump schemas for selected models group into OpenAPI-compatible file and quit", | ||
| action="store", | ||
| default=None, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject conflicting dump modes instead of silently ignoring the group selector.
The parser accepts -m -gr foo, but main() handles args.dump_models first at Line 218, dumps all models, and returns without honoring --dump-models-group. Put the dump modes in one mutually exclusive group or reject conflicting flags before dispatch.
🤖 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 `@src/lightspeed_stack.py` around lines 75 - 82, Make the argument parser
reject simultaneous use of dump_models and dump_models_group instead of allowing
main() to silently prioritize dump_models. Place both options in a mutually
exclusive argument group, or add equivalent validation before main() dispatch,
while preserving each mode’s existing standalone behavior.
| # -gr or --dump-models-group CLI parameter is used to dump schema for | ||
| # selected models into a JSON file that is compatible with OpenAPI schema | ||
| # specification | ||
| if args.dump_models_group is not None: | ||
| try: | ||
| models_dumper.dump_models_group(args.dump_models_group) | ||
| logger.info("Schema for group %s of models dumped", args.dump_models_group) | ||
| except Exception as e: | ||
| logger.error("Failed to dump schema for models: %s", e) | ||
| raise SystemExit(1) from e | ||
| return |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the model-group dump documentation consistent.
The new option is implemented, but the function docstring and README usage synopsis do not fully expose the CLI contract.
src/lightspeed_stack.py#L227-L237: document--dump-models-groupand its status-1 failure behavior inmain().README.md#L863-L864: add[-gr DUMP_MODELS_GROUP]to the usage synopsis at README.md Lines 853-855.
📍 Affects 2 files
src/lightspeed_stack.py#L227-L237(this comment)README.md#L863-L864
🤖 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 `@src/lightspeed_stack.py` around lines 227 - 237, Update the main()
documentation in src/lightspeed_stack.py to describe --dump-models-group and
state that dump failures exit with status 1. Update the README.md usage synopsis
at lines 853-855 to include [-gr DUMP_MODELS_GROUP].
Source: Coding guidelines
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="lightspeed-core/lightspeed-stack"
body=$(cat <<'EOF'
## Summary
Keep the `--dump-models-group` CLI documentation aligned with its implementation.
## Required changes
- Update `src/lightspeed_stack.py`'s `main()` docstring to describe `--dump-models-group` and state that failures while dumping exit with status 1.
- Update the `README.md` CLI usage synopsis to include `[-gr DUMP_MODELS_GROUP]`.
## Rationale
The option is implemented, but its public usage and failure contract are not fully documented.
## Affected areas
- `src/lightspeed_stack.py`
- `README.md`
## Acceptance criteria
- `main()` documentation lists the `--dump-models-group` behavior.
- The documented `Raises`/failure behavior covers model-group dump failures and status 1.
- The README usage synopsis contains `[-gr DUMP_MODELS_GROUP]`.
## Backlinks
- Source pull request: https://github.com/lightspeed-core/lightspeed-stack/pull/2263
- Source review comment: https://github.com/lightspeed-core/lightspeed-stack/pull/2263#discussion_r3660714065
Requested by: `@tisnik`
EOF
)
gh issue create \
--repo "$repo" \
--title "Document --dump-models-group CLI contract" \
--body "$body" \
--assignee tisnikLength of output: 1422
@tisnik, created and assigned the follow-up issue: #2265 — Document --dump-models-group CLI contract.
🧠 Learnings used
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
1 participant
Description
LCORE-3141: models group schema export ability
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit
New Features
--dump-models-groupcommand-line option to export OpenAPI-compatible schemas for selected model groups.Documentation