LCORE-889: fixed issues in config models #1063
Conversation
WalkthroughThese changes modify unit test files across configuration, request, and response model tests. Primary modifications include adding pyright type-checking directives to suppress reportCallIssue warnings, reformatting constructor calls into multi-line structures, converting string file paths to Path objects in customization tests, and making explicit test assertions with optional parameters. No production code is modified. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
🤖 Fix all issues with AI agents
In `@tests/unit/models/responses/test_rag_chunk.py`:
- Line 11: The RAGChunk Pydantic model uses Pydantic v1-style Field positional
defaults causing type-checker ignores; update the RAGChunk field definitions in
the RAGChunk model (in src/utils/types.py) to use explicit default= for optional
fields (e.g., change Field(None, ...) to Field(default=None, ...)) for both
source and score (and any other optional Field uses) so the fields are correctly
recognized as optional by Pydantic v2 and you can remove the pyright ignore
comments in the related tests.
🧹 Nitpick comments (2)
tests/unit/models/requests/test_feedback_request.py (1)
196-198: Consider using consistent pyright ignore format.The comment
# pyright: ignore Should be listdiffers from the format used elsewhere in this PR (# pyright: ignore[reportCallIssue]). For consistency, consider specifying the error code.Suggested format
- categories="invalid_type", # pyright: ignore Should be list, not string + categories="invalid_type", # pyright: ignore[reportArgumentType] - Should be list, not stringtests/unit/models/config/test_llama_stack_configuration.py (1)
32-40: Consider using# type: ignore[call-arg]for consistency with other test files.The
# pyright: ignore[reportCallIssue]comments are added to suppress type checker warnings. However, other test files in this PR (e.g.,test_successful_responses.py) use# type: ignore[call-arg]which is more portable across type checkers (mypy, pyright, etc.).
| def test_constructor_with_content_only(self) -> None: | ||
| """Test RAGChunk constructor with content only.""" | ||
| chunk = RAGChunk(content="Sample content") | ||
| chunk = RAGChunk(content="Sample content") # pyright: ignore[reportCallIssue] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check Pydantic version
find . -name "*.toml" -o -name "*requirements*.txt" | head -20 | xargs rg "pydantic" -lRepository: lightspeed-core/lightspeed-stack
Length of output: 107
🏁 Script executed:
# First, find where RAGChunk is defined
fd -t f "*.py" | xargs rg -l "class RAGChunk"Repository: lightspeed-core/lightspeed-stack
Length of output: 395
🏁 Script executed:
# Examine the test file to understand the pattern
wc -l tests/unit/models/responses/test_rag_chunk.pyRepository: lightspeed-core/lightspeed-stack
Length of output: 125
🏁 Script executed:
# Check Pydantic version
rg "pydantic" requirements.hashes.wheel.txt | head -5Repository: lightspeed-core/lightspeed-stack
Length of output: 146
🏁 Script executed:
# Examine RAGChunk model definition
cat -n src/utils/types.py | head -100Repository: lightspeed-core/lightspeed-stack
Length of output: 4008
🏁 Script executed:
# Examine the test file around the specified lines
cat -n tests/unit/models/responses/test_rag_chunk.pyRepository: lightspeed-core/lightspeed-stack
Length of output: 5522
🏁 Script executed:
# Find RAGChunk class definition
rg -n "class RAGChunk" src/utils/types.py -A 20Repository: lightspeed-core/lightspeed-stack
Length of output: 962
🏁 Script executed:
# Also check if there are other files defining RAGChunk
fd -type f -name "*.py" | xargs rg -l "class RAGChunk"Repository: lightspeed-core/lightspeed-stack
Length of output: 282
🏁 Script executed:
# Check if there are other Pydantic models in the codebase using proper v2 syntax
rg "Field\(default" src/ --type py -A 1 | head -20Repository: lightspeed-core/lightspeed-stack
Length of output: 373
🏁 Script executed:
# Check the learnings file mentioned to understand expected patterns
cat -n CLAUDE.md 2>/dev/null || echo "CLAUDE.md not found"Repository: lightspeed-core/lightspeed-stack
Length of output: 10041
🏁 Script executed:
# Verify if Field(None, ...) works at runtime in Pydantic v2 by checking other models
rg "Field\(None" src/ --type pyRepository: lightspeed-core/lightspeed-stack
Length of output: 376
🏁 Script executed:
# Verify by checking Pydantic documentation and seeing if Field(None, ...) works in v2
# Let's check if there are any tests that might show runtime behavior
rg -n "RAGChunk" tests/ -B 2 -A 2 | head -40Repository: lightspeed-core/lightspeed-stack
Length of output: 3324
🏁 Script executed:
# Check if any of the constructed chunks are actually used/asserted properly
rg -n "assert.*score.*None" tests/unit/models/responses/test_rag_chunk.pyRepository: lightspeed-core/lightspeed-stack
Length of output: 189
Update RAGChunk model to use proper Pydantic v2 syntax for optional fields.
The systematic pyright: ignore[reportCallIssue] comments across multiple tests indicate the RAGChunk model uses outdated Pydantic v1 Field syntax. In Pydantic v2, the first positional argument to Field() is not the default value.
The model definition in src/utils/types.py (lines 142-143) currently uses:
source: Optional[str] = Field(None, description="Source document or URL")
score: Optional[float] = Field(None, description="Relevance score")This should be updated to:
source: Optional[str] = Field(default=None, description="Source document or URL")
score: Optional[float] = Field(default=None, description="Relevance score")This is why tests that provide all three parameters (lines 96-98, 116-118) don't require ignore comments—they satisfy the incorrectly-inferred required parameters. Fix the model definition rather than suppressing type-checker warnings in tests.
Also applies to: 42-42, 49-51, 59-61, 65-67, 71-73, 78-78, 110-110
🤖 Prompt for AI Agents
In `@tests/unit/models/responses/test_rag_chunk.py` at line 11, The RAGChunk
Pydantic model uses Pydantic v1-style Field positional defaults causing
type-checker ignores; update the RAGChunk field definitions in the RAGChunk
model (in src/utils/types.py) to use explicit default= for optional fields
(e.g., change Field(None, ...) to Field(default=None, ...)) for both source and
score (and any other optional Field uses) so the fields are correctly recognized
as optional by Pydantic v2 and you can remove the pyright ignore comments in the
related tests.
1 participant
Description
LCORE-889: fixed issues in config models
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.