LCORE-2478: improved docstring in schema_dumper#1860
Conversation
WalkthroughThe ChangesDocstring Clarification
🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.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/utils/schema_dumper.py`:
- Around line 17-21: The docstring claims full recursive traversal, but the
implementation only recurses into nested dict values (and handles the specific
anyOf branch) and skips generic list-contained dicts; update the function that
contains this docstring so lists are traversed too: when encountering a list
value, iterate its items and for any item that is a dict call the same
compatibility-fix routine (preserving the existing anyOf special-case and the
dict-recursion behavior), or alternatively narrow the docstring wording to
remove "recursively" if you prefer not to change logic; focus changes around the
anyOf handling and the dict-recursion code paths so list-contained dicts get
processed the same way as plain nested dicts.
- Around line 23-31: The docstring for the function in
src/utils/schema_dumper.py that takes parameter original (dict[str, Any]) and
returns dict[str, Any] uses NumPy-style sections; update it to Google docstring
style by replacing the NumPy "Parameters:" block with an "Args:" section
(document original: dict[str, Any] — what it is and produced-by Pydantic),
convert the "Returns:" block to a single-line Google "Returns:" entry describing
the returned dict[str, Any], remove the underline blocks and trailing colons,
and add a "Raises:" section if the function may raise exceptions; keep wording
concise and follow Google conventions for parameter and return typing.
🪄 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
Run ID: cf1697b6-b89c-419a-98f3-7bd7643e054f
📒 Files selected for processing (1)
src/utils/schema_dumper.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Pyright
- GitHub Check: unit_tests (3.13)
- GitHub Check: unit_tests (3.12)
- GitHub Check: radon
- GitHub Check: Pylinter
- GitHub Check: build-pr
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: spectral
- GitHub Check: bandit
- GitHub Check: mypy
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
Llama Stack imports: Usefrom llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Useasync deffor I/O operations and external API calls
Use standard log levels with clear purposes:debug()for diagnostic info,info()for program execution,warning()for unexpected events,error()for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes:Configuration,Error/Exception,Resolver,Interface
Abstract classes must use ABC with@abstractmethoddecorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes
Files:
src/utils/schema_dumper.py
| Recursively walks the input mapping and applies compatibility fixes: | ||
| - converts patterns like `anyOf: [{ "type": X }, { "type": "null" }]` into | ||
| a single `"type": X` with `"nullable": True` (optional types) | ||
| - rewrites `"exclusiveMinimum"` to `"minimum"` | ||
| - otherwise preserves entries |
There was a problem hiding this comment.
Docstring overstates recursive coverage.
The text says the function recursively applies transformations across the schema, but the implementation only recurses into nested dict values and does not recurse through generic list elements (except the specific anyOf branch). Please narrow the wording to match actual behavior or extend implementation to recurse into list-contained dicts.
Also applies to: 30-31
🤖 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/utils/schema_dumper.py` around lines 17 - 21, The docstring claims full
recursive traversal, but the implementation only recurses into nested dict
values (and handles the specific anyOf branch) and skips generic list-contained
dicts; update the function that contains this docstring so lists are traversed
too: when encountering a list value, iterate its items and for any item that is
a dict call the same compatibility-fix routine (preserving the existing anyOf
special-case and the dict-recursion behavior), or alternatively narrow the
docstring wording to remove "recursively" if you prefer not to change logic;
focus changes around the anyOf handling and the dict-recursion code paths so
list-contained dicts get processed the same way as plain nested dicts.
| Parameters: | ||
| ---------- | ||
| original (dict): The original schema dictionary to transform. | ||
| original (dict[str, Any]): Input schema dictionary that is produced by | ||
| Pydantic. | ||
|
|
||
| Returns: | ||
| ------- | ||
| dict: A new dictionary with OpenAPI-compatible transformations applied. | ||
| dict[str, Any]: A new schema dictionary with all OpenAPI-compatible | ||
| transformations applied. |
There was a problem hiding this comment.
Docstring format does not follow repository-required Google style.
This function docstring still uses NumPy-style section formatting (Parameters: + underline blocks) instead of the required Google style sections.
As per coding guidelines, “Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes.”
🤖 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/utils/schema_dumper.py` around lines 23 - 31, The docstring for the
function in src/utils/schema_dumper.py that takes parameter original (dict[str,
Any]) and returns dict[str, Any] uses NumPy-style sections; update it to Google
docstring style by replacing the NumPy "Parameters:" block with an "Args:"
section (document original: dict[str, Any] — what it is and produced-by
Pydantic), convert the "Returns:" block to a single-line Google "Returns:" entry
describing the returned dict[str, Any], remove the underline blocks and trailing
colons, and add a "Raises:" section if the function may raise exceptions; keep
wording concise and follow Google conventions for parameter and return typing.
Source: Coding guidelines
1 participant
Description
LCORE-2478: improved docstring in
schema_dumperType of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit