Restructure pydantic_ai utils into LlamaStack model and provider classes#2025
Restructure pydantic_ai utils into LlamaStack model and provider classes#2025Jazzcort wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughAdds Llama Stack client-based factory methods, moves response-parameter conversion into the model module, and updates ChangesLlamaStack factory methods and call-site migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/pydantic_ai_lightspeed/llamastack/_model.py`:
- Around line 69-72: Update the docstrings for
`_model_settings_from_responses_params` and `from_llama_stack_client` to match
the repo’s required Google-style sectioned format.
`_model_settings_from_responses_params` currently lacks the required section
breakdown, and `from_llama_stack_client` should replace `Args:` with
`Parameters:` while also including the appropriate `Returns:` and `Raises:`
sections if applicable. Keep the docstrings descriptive and aligned with the
existing convention used in `_model.py`.
- Around line 389-404: Move the mutual-exclusivity validation for
responses_params and model_settings ahead of
LlamaStackProvider.from_llama_stack_client(client) in the
LlamaStackResponsesModel construction path, so the ValueError is raised before
any provider/client resources are created. Keep the existing logic in the same
method that builds LlamaStackResponsesModel, but check the two inputs first and
only instantiate the provider after the validation passes.
In `@src/pydantic_ai_lightspeed/llamastack/_provider.py`:
- Around line 57-68: The docstring on the LlamaStackProvider factory still uses
the non-standard Args header; update it to the repo-standard Parameters section.
Keep the existing description for the client parameter and ensure the function
docstring in LlamaStackProvider.from_client follows the project’s Google-style
convention with Parameters and Returns sections.
In `@tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py`:
- Around line 330-348: The MCP replay test in
test_replays_mcp_buffered_deltas_with_suffixed_id is asserting a malformed
combined delta string; update the expected replayed.delta value to match the
actual concatenation of delta1 and delta2 produced by _FilteredResponseStream,
so the test reflects the correct MCP buffered replay output rather than an extra
closing brace.
🪄 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: 7ba5e735-ee16-4bdd-99d6-6a9bf0dfef54
📒 Files selected for processing (7)
src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.pysrc/pydantic_ai_lightspeed/llamastack/_model.pysrc/pydantic_ai_lightspeed/llamastack/_provider.pysrc/utils/pydantic_ai.pytests/unit/pydantic_ai_lightspeed/llamastack/test_model.pytests/unit/pydantic_ai_lightspeed/llamastack/test_provider.pytests/unit/utils/test_pydantic_ai.py
💤 Files with no reviewable changes (1)
- tests/unit/utils/test_pydantic_ai.py
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: unit_tests (3.12)
- GitHub Check: integration_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: Pylinter
- GitHub Check: integration_tests (3.13)
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: E2E Tests for Lightspeed Evaluation job
⚠️ CI failures not shown inline (5)
GitHub Actions: Type checks / mypy: Restructure pydantic_ai utils into LlamaStack model and provider classes
Conclusion: failure
##[group]Run uv run mypy --explicit-package-bases --disallow-untyped-calls --disallow-untyped-defs --disallow-incomplete-defs --ignore-missing-imports --disable-error-code attr-defined src/
�[36;1muv run mypy --explicit-package-bases --disallow-untyped-calls --disallow-untyped-defs --disallow-incomplete-defs --ignore-missing-imports --disable-error-code attr-defined src/�[0m
shell: /usr/bin/bash -e {0}
env:
UV_PYTHON: 3.12
VIRTUAL_ENV: /home/runner/work/lightspeed-stack/lightspeed-stack/.venv
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
src/pydantic_ai_lightspeed/llamastack/_model.py:398: error: Incompatible types in assignment (expression has type "ModelSettings", variable has type "OpenAIResponsesModelSettings") [assignment]
Found 1 error in 1 file (checked 198 source files)
##[error]Process completed with exit code 1.
GitHub Actions: PR Title Checker / check: Restructure pydantic_ai utils into LlamaStack model and provider classes
Conclusion: failure
##[group]Run thehanimo/pr-title-checker@v1.4.3
with:
GITHUB_***REDACTED***
pass_on_octokit_error: false
configuration_path: .github/pr-title-checker-config.json
##[endgroup]
(node:2224) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Using config file .github/pr-title-checker-config.json from repo lightspeed-core/lightspeed-stack [ref: 8733dfe89799f448c3661a4c283a959dc44f67f7]
(node:2224) [DEP0169] DeprecationWarning: `url.parse()` behavior is not standardized and prone to errors that have security implications. Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.
Creating label (title needs formatting)...
Label (title needs formatting) already created.
Adding label (title needs formatting) to PR...
HttpError: Resource not accessible by integration
##[error]Failed to add label (title needs formatting) to PR
GitHub Actions: PR Title Checker / 0_check.txt: Restructure pydantic_ai utils into LlamaStack model and provider classes
Conclusion: failure
##[group]Run thehanimo/pr-title-checker@v1.4.3
with:
GITHUB_***REDACTED***
pass_on_octokit_error: false
configuration_path: .github/pr-title-checker-config.json
##[endgroup]
(node:2224) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Using config file .github/pr-title-checker-config.json from repo lightspeed-core/lightspeed-stack [ref: 8733dfe89799f448c3661a4c283a959dc44f67f7]
(node:2224) [DEP0169] DeprecationWarning: `url.parse()` behavior is not standardized and prone to errors that have security implications. Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.
Creating label (title needs formatting)...
Label (title needs formatting) already created.
Adding label (title needs formatting) to PR...
HttpError: Resource not accessible by integration
##[error]Failed to add label (title needs formatting) to PR
GitHub Actions: OpenAPI (Spectral) / spectral: Restructure pydantic_ai utils into LlamaStack model and provider classes
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json"�[0m
GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt: Restructure pydantic_ai utils into LlamaStack model and provider classes
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json"�[0m
🧰 Additional context used
📓 Path-based instructions (2)
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/pydantic_ai_lightspeed/llamastack/_provider.pysrc/utils/pydantic_ai.pysrc/pydantic_ai_lightspeed/llamastack/_model.pysrc/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/unit/pydantic_ai_lightspeed/llamastack/test_model.pytests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
🧠 Learnings (1)
📚 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/pydantic_ai_lightspeed/llamastack/_provider.pysrc/utils/pydantic_ai.pysrc/pydantic_ai_lightspeed/llamastack/_model.pytests/unit/pydantic_ai_lightspeed/llamastack/test_model.pysrc/pydantic_ai_lightspeed/capabilities/question_validity/_capability.pytests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
🪛 ast-grep (0.44.0)
tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
[warning] 167-167: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 179-179: Do not make http calls without encryption
Context: "http://my-server:8321"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 192-192: Do not make http calls without encryption
Context: "http://my-server:8321/"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 204-204: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 217-217: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 228-228: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🪛 GitHub Actions: Type checks / 0_mypy.txt
src/pydantic_ai_lightspeed/llamastack/_model.py
[error] 398-398: mypy error: Incompatible types in assignment (expression has type "ModelSettings", variable has type "OpenAIResponsesModelSettings"). [assignment]
[error] 1-1: Command failed: uv run mypy --explicit-package-bases --disallow-untyped-calls --disallow-untyped-defs --disallow-incomplete-defs --ignore-missing-imports --disable-error-code attr-defined src/ (exit code 1).
🪛 GitHub Actions: Type checks / mypy
src/pydantic_ai_lightspeed/llamastack/_model.py
[error] 398-398: mypy failed: Incompatible types in assignment (expression has type "ModelSettings", variable has type "OpenAIResponsesModelSettings"). [assignment]
| provider = LlamaStackProvider.from_llama_stack_client(client) | ||
|
|
||
| if responses_params is not None and model_settings is not None: | ||
| raise ValueError( | ||
| "You can only pass either ResponsesApiParams or ModelSetting not both." | ||
| ) | ||
| elif responses_params is not None: | ||
| _settings = _model_settings_from_responses_params(responses_params) | ||
| elif model_settings is not None: | ||
| _settings = model_settings | ||
| else: | ||
| _settings = None | ||
|
|
||
| return LlamaStackResponsesModel( | ||
| model_name, provider=provider, profile=profile, settings=_settings | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate exclusivity before constructing the provider.
LlamaStackProvider.from_llama_stack_client(client) runs before the responses_params/model_settings check. On the library-client path, provider construction creates a new httpx.AsyncClient, so the error path allocates resources before immediately raising ValueError.
Suggested fix
- provider = LlamaStackProvider.from_llama_stack_client(client)
-
if responses_params is not None and model_settings is not None:
raise ValueError(
"You can only pass either ResponsesApiParams or ModelSetting not both."
)
elif responses_params is not None:
_settings = _model_settings_from_responses_params(responses_params)
elif model_settings is not None:
_settings = model_settings
else:
_settings = None
+
+ provider = LlamaStackProvider.from_llama_stack_client(client)
return LlamaStackResponsesModel(
model_name, provider=provider, profile=profile, settings=_settings
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| provider = LlamaStackProvider.from_llama_stack_client(client) | |
| if responses_params is not None and model_settings is not None: | |
| raise ValueError( | |
| "You can only pass either ResponsesApiParams or ModelSetting not both." | |
| ) | |
| elif responses_params is not None: | |
| _settings = _model_settings_from_responses_params(responses_params) | |
| elif model_settings is not None: | |
| _settings = model_settings | |
| else: | |
| _settings = None | |
| return LlamaStackResponsesModel( | |
| model_name, provider=provider, profile=profile, settings=_settings | |
| ) | |
| if responses_params is not None and model_settings is not None: | |
| raise ValueError( | |
| "You can only pass either ResponsesApiParams or ModelSetting not both." | |
| ) | |
| elif responses_params is not None: | |
| _settings = _model_settings_from_responses_params(responses_params) | |
| elif model_settings is not None: | |
| _settings = model_settings | |
| else: | |
| _settings = None | |
| provider = LlamaStackProvider.from_llama_stack_client(client) | |
| return LlamaStackResponsesModel( | |
| model_name, provider=provider, profile=profile, settings=_settings | |
| ) |
🧰 Tools
🪛 GitHub Actions: Type checks / 0_mypy.txt
[error] 398-398: mypy error: Incompatible types in assignment (expression has type "ModelSettings", variable has type "OpenAIResponsesModelSettings"). [assignment]
🪛 GitHub Actions: Type checks / mypy
[error] 398-398: mypy failed: Incompatible types in assignment (expression has type "ModelSettings", variable has type "OpenAIResponsesModelSettings"). [assignment]
🤖 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/pydantic_ai_lightspeed/llamastack/_model.py` around lines 389 - 404, Move
the mutual-exclusivity validation for responses_params and model_settings ahead
of LlamaStackProvider.from_llama_stack_client(client) in the
LlamaStackResponsesModel construction path, so the ValueError is raised before
any provider/client resources are created. Keep the existing logic in the same
method that builds LlamaStackResponsesModel, but check the two inputs first and
only instantiate the provider after the validation passes.
|
Will fix the failing CI later. Please hold on to review this one. |
9c6da47 to
3a15fcb
Compare
asimurka
left a comment
There was a problem hiding this comment.
LGTM, please rebase and fix CI
3a15fcb to
a0a9154
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py (1)
715-733: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the malformed MCP replay expectation.
Line 733 still expects an extra
}. The replayed delta should be the concatenation ofdelta1anddelta2, so this assertion currently locks in invalid output and will fail against correct behavior.Suggested fix
- assert replayed.delta == '{"arg":"v"}}' + assert replayed.delta == '{"arg":"v"}'🤖 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 `@tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py` around lines 715 - 733, The MCP replay test in test_replays_mcp_buffered_deltas_with_suffixed_id has a malformed expected delta string: it currently asserts an extra closing brace in the replayed ResponseFunctionCallArgumentsDeltaEvent. Update the assertion to match the actual concatenation of delta1 and delta2 produced by _FilteredResponseStream, and keep the item_id check for the "-call" suffix unchanged.
🤖 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.
Duplicate comments:
In `@tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py`:
- Around line 715-733: The MCP replay test in
test_replays_mcp_buffered_deltas_with_suffixed_id has a malformed expected delta
string: it currently asserts an extra closing brace in the replayed
ResponseFunctionCallArgumentsDeltaEvent. Update the assertion to match the
actual concatenation of delta1 and delta2 produced by _FilteredResponseStream,
and keep the item_id check for the "-call" suffix unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c095c7f-ebba-42fa-ba5d-c65e424624e2
📒 Files selected for processing (8)
src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.pysrc/pydantic_ai_lightspeed/llamastack/_model.pysrc/pydantic_ai_lightspeed/llamastack/_provider.pysrc/utils/pydantic_ai.pytests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.pytests/unit/pydantic_ai_lightspeed/llamastack/test_model.pytests/unit/pydantic_ai_lightspeed/llamastack/test_provider.pytests/unit/utils/test_pydantic_ai.py
💤 Files with no reviewable changes (1)
- tests/unit/utils/test_pydantic_ai.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.pytests/unit/pydantic_ai_lightspeed/llamastack/test_model.pytests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.py
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/pydantic_ai_lightspeed/capabilities/question_validity/_capability.pysrc/pydantic_ai_lightspeed/llamastack/_provider.pysrc/pydantic_ai_lightspeed/llamastack/_model.pysrc/utils/pydantic_ai.py
🧠 Learnings (1)
📚 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:
tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.pysrc/pydantic_ai_lightspeed/capabilities/question_validity/_capability.pysrc/pydantic_ai_lightspeed/llamastack/_provider.pysrc/pydantic_ai_lightspeed/llamastack/_model.pytests/unit/pydantic_ai_lightspeed/llamastack/test_model.pytests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.pysrc/utils/pydantic_ai.py
🪛 ast-grep (0.44.0)
tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
[warning] 167-167: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 179-179: Do not make http calls without encryption
Context: "http://my-server:8321"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 192-192: Do not make http calls without encryption
Context: "http://my-server:8321/"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 204-204: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 217-217: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 228-228: Do not make http calls without encryption
Context: "http://my-server:8321/v1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🔇 Additional comments (10)
src/pydantic_ai_lightspeed/llamastack/_provider.py (2)
65-69: 📐 Maintainability & Code Quality | 💤 Low valueUse the repo-standard
Parameters:docstring header.This factory's docstring still uses
Args:. Based on learnings, this repo requires theParameters:section header. As per coding guidelines, functions must "include descriptive docstrings" following Google conventions withParameters/Returnssections.Sources: Coding guidelines, Learnings
71-80: LGTM!src/pydantic_ai_lightspeed/llamastack/_model.py (3)
69-72: 📐 Maintainability & Code Quality | 💤 Low valueBoth new docstrings need the repo's sectioned
Parameters:format.
_model_settings_from_responses_paramshas only a one-line docstring (missingParameters/Returns), andfrom_llama_stack_client(lines 372-384) usesArgs:instead ofParameters:. As per coding guidelines, all functions need descriptive docstrings following Google conventions; based on learnings the repo usesParameters:.Also applies to: 372-384
Sources: Coding guidelines, Learnings
389-394: 🩺 Stability & Availability | ⚡ Quick winValidate mutual exclusivity before constructing the provider.
LlamaStackProvider.from_llama_stack_client(client)runs before theresponses_params/model_settingscheck. On the library-client path, provider construction allocates a newhttpx.AsyncClient, so an invalid call leaks resources before theValueErroris raised. Move the validation ahead of provider construction.Suggested fix
- provider = LlamaStackProvider.from_llama_stack_client(client) - if responses_params is not None and model_settings is not None: raise ValueError( "You can only pass either ResponsesApiParams or ModelSetting not both." ) _settings: OpenAIResponsesModelSettings | ModelSettings | None = None if responses_params is not None: _settings = _model_settings_from_responses_params(responses_params) elif model_settings is not None: _settings = model_settings + provider = LlamaStackProvider.from_llama_stack_client(client) + return LlamaStackResponsesModel( model_name, provider=provider, profile=profile, settings=_settings )
52-91: LGTM!src/utils/pydantic_ai.py (1)
76-85: LGTM!src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py (1)
78-84: LGTM!tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py (1)
1-714: LGTM!Also applies to: 735-762
tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py (1)
7-8: LGTM!Also applies to: 148-238
tests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.py (1)
9-9: LGTM!Also applies to: 27-28, 109-137, 152-154, 214-216
8624366 to
33380ad
Compare
Move llama_stack_provider_from_client and _model_settings_from_responses_params out of utils/pydantic_ai.py into their owning classes as static factory methods: - LlamaStackProvider.from_llama_stack_client() in _provider.py - LlamaStackResponsesModel.from_llama_stack_client() in _model.py - _model_settings_from_responses_params and _LLS_RESPONSES_EXTRA_FIELDS into _model.py Update callers (QuestionValidity, build_agent) to use the new factory methods and relocate corresponding tests to test_model.py and test_provider.py.
33380ad to
71bc46a
Compare
Description
Move llama_stack_provider_from_client and _model_settings_from_responses_params out of utils/pydantic_ai.py into their owning classes as static factory methods:
Update callers (QuestionValidity, build_agent) to use the new factory methods and relocate corresponding tests to test_model.py and test_provider.py.
Note that
TestFilteredResponseStream,TestPrepareConversationContinuation,TestRequest, andTestRequestStreamintests/unit/pydantic_ai_lightspeed/llamastack/test_model.pyare out-of-scope but it's good to have the unit test cover the entire module instead of just partial of it. If we want the PR to only cover the restructure related part, I can remove these test cases later. I'm fine with either ways.Type of change
Tools used to create PR
Test cases are drafted by claude-opus-4.6 and refined by me.
Related Tickets & Documents
Related discussion: here
Checklist before requesting a review
Testing
Everything related to pydantic agent should work exactly like it used to since this patch is just to restructure the utility functions under the hood which should not affect the overall behavior.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests