Skip to content

Restructure pydantic_ai utils into LlamaStack model and provider classes#2025

Open
Jazzcort wants to merge 1 commit into
lightspeed-core:mainfrom
Jazzcort:restructure-pydantic-ai-utils-function
Open

Restructure pydantic_ai utils into LlamaStack model and provider classes#2025
Jazzcort wants to merge 1 commit into
lightspeed-core:mainfrom
Jazzcort:restructure-pydantic-ai-utils-function

Conversation

@Jazzcort

@Jazzcort Jazzcort commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

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:

  • 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.

Note that TestFilteredResponseStream, TestPrepareConversationContinuation, TestRequest, and TestRequestStream in tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py are 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

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

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

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

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

    • Expanded Llama Stack model compatibility to support both server-mode and in-process library-mode clients.
    • Added a public model factory to standardize how model settings are derived and applied for agent and question-validation flows.
  • Bug Fixes

    • Improved handling and remapping of model parameters (including token limits, temperature, headers, and prior-response IDs).
    • Fixed streamed tool-call event ordering and delta sequencing to preserve the correct tool update flow.
  • Tests

    • Added/updated unit coverage for client wiring, parameter conversion, and streaming/event behavior.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Jazzcort, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4837bb5e-023a-4210-b4c4-7b9c7ffcc818

📥 Commits

Reviewing files that changed from the base of the PR and between 8624366 and 71bc46a.

📒 Files selected for processing (8)
  • src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py
  • src/pydantic_ai_lightspeed/llamastack/_model.py
  • src/pydantic_ai_lightspeed/llamastack/_provider.py
  • src/utils/pydantic_ai.py
  • tests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
  • tests/unit/utils/test_pydantic_ai.py

Walkthrough

Adds Llama Stack client-based factory methods, moves response-parameter conversion into the model module, and updates build_agent and QuestionValidity to use the new construction path.

Changes

LlamaStack factory methods and call-site migration

Layer / File(s) Summary
Provider client factory
src/pydantic_ai_lightspeed/llamastack/_provider.py, tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
Adds LlamaStackProvider.from_llama_stack_client(...) for library-mode and server-mode async clients, including base URL normalization, API key handling, transport reuse, and matching unit coverage.
Model settings conversion and factory
src/pydantic_ai_lightspeed/llamastack/_model.py, tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
Adds _LLS_RESPONSES_EXTRA_FIELDS, converts ResponsesApiParams into OpenAIResponsesModelSettings, and adds LlamaStackResponsesModel.from_llama_stack_client(...) with coverage for settings mapping, request/stream behavior, and response stream filtering.
Call-site migration
src/utils/pydantic_ai.py, src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py, tests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.py, tests/unit/utils/test_pydantic_ai.py
build_agent and QuestionValidity.__post_init__ now construct LlamaStackResponsesModel through the new factory, and the old private helper and migrated test imports are removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • asimurka
  • tisnik
  • jrobertboos
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactor: moving pydantic_ai LlamaStack utilities into model and provider classes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 8733dfe and c787adf.

📒 Files selected for processing (7)
  • src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py
  • src/pydantic_ai_lightspeed/llamastack/_model.py
  • src/pydantic_ai_lightspeed/llamastack/_provider.py
  • src/utils/pydantic_ai.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
  • tests/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

View job details

##[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

View job details

##[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

View job details

##[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

View job details

##[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

View job details

##[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: Use from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for 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
Use async def for 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 @abstractmethod decorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes

Files:

  • src/pydantic_ai_lightspeed/llamastack/_provider.py
  • src/utils/pydantic_ai.py
  • src/pydantic_ai_lightspeed/llamastack/_model.py
  • src/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
Use pytest.mark.asyncio marker for async tests

Files:

  • tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
  • tests/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.py
  • src/utils/pydantic_ai.py
  • src/pydantic_ai_lightspeed/llamastack/_model.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
  • src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py
  • tests/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]

Comment thread src/pydantic_ai_lightspeed/llamastack/_model.py
Comment on lines +389 to +404
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
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Suggested change
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.

Comment thread src/pydantic_ai_lightspeed/llamastack/_provider.py
Comment thread tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
@Jazzcort

Copy link
Copy Markdown
Contributor Author

Will fix the failing CI later. Please hold on to review this one.

@Jazzcort Jazzcort force-pushed the restructure-pydantic-ai-utils-function branch 3 times, most recently from 9c6da47 to 3a15fcb Compare June 29, 2026 23:14

@asimurka asimurka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please rebase and fix CI

@Jazzcort Jazzcort force-pushed the restructure-pydantic-ai-utils-function branch from 3a15fcb to a0a9154 Compare June 30, 2026 13:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py (1)

715-733: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fix the malformed MCP replay expectation.

Line 733 still expects an extra }. The replayed delta should be the concatenation of delta1 and delta2, 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

📥 Commits

Reviewing files that changed from the base of the PR and between 3bd0762 and a0a9154.

📒 Files selected for processing (8)
  • src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py
  • src/pydantic_ai_lightspeed/llamastack/_model.py
  • src/pydantic_ai_lightspeed/llamastack/_provider.py
  • src/utils/pydantic_ai.py
  • tests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
  • tests/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
Use pytest.mark.asyncio marker for async tests

Files:

  • tests/unit/pydantic_ai_lightspeed/llamastack/test_provider.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
  • tests/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: Use from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for 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
Use async def for 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 @abstractmethod decorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes

Files:

  • src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py
  • src/pydantic_ai_lightspeed/llamastack/_provider.py
  • src/pydantic_ai_lightspeed/llamastack/_model.py
  • src/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.py
  • src/pydantic_ai_lightspeed/capabilities/question_validity/_capability.py
  • src/pydantic_ai_lightspeed/llamastack/_provider.py
  • src/pydantic_ai_lightspeed/llamastack/_model.py
  • tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py
  • tests/unit/pydantic_ai_lightspeed/capabilities/question_validity/test_capability.py
  • src/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 value

Use the repo-standard Parameters: docstring header.

This factory's docstring still uses Args:. Based on learnings, this repo requires the Parameters: section header. As per coding guidelines, functions must "include descriptive docstrings" following Google conventions with Parameters/Returns sections.

Sources: Coding guidelines, Learnings


71-80: LGTM!

src/pydantic_ai_lightspeed/llamastack/_model.py (3)

69-72: 📐 Maintainability & Code Quality | 💤 Low value

Both new docstrings need the repo's sectioned Parameters: format.

_model_settings_from_responses_params has only a one-line docstring (missing Parameters/Returns), and from_llama_stack_client (lines 372-384) uses Args: instead of Parameters:. As per coding guidelines, all functions need descriptive docstrings following Google conventions; based on learnings the repo uses Parameters:.

Also applies to: 372-384

Sources: Coding guidelines, Learnings


389-394: 🩺 Stability & Availability | ⚡ Quick win

Validate mutual 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 allocates a new httpx.AsyncClient, so an invalid call leaks resources before the ValueError is 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

@Jazzcort Jazzcort force-pushed the restructure-pydantic-ai-utils-function branch 4 times, most recently from 8624366 to 33380ad Compare June 30, 2026 14:41
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.
@Jazzcort Jazzcort force-pushed the restructure-pydantic-ai-utils-function branch from 33380ad to 71bc46a Compare June 30, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants