Skip to content

LCORE-1880: Split requests into multiple files#1684

Merged
tisnik merged 1 commit into
lightspeed-core:mainfrom
asimurka:move_error_responses
May 6, 2026
Merged

LCORE-1880: Split requests into multiple files#1684
tisnik merged 1 commit into
lightspeed-core:mainfrom
asimurka:move_error_responses

Conversation

@asimurka
Copy link
Copy Markdown
Contributor

@asimurka asimurka commented May 5, 2026

Description

Splits requests into multiple separate files under src/models/api/requests or replaces shared internal models under src/models/common. Refines model documentations, updates AGENTS.md to reflect new project structure.

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

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: Cursor

Related Tickets & Documents

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

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Refactor

    • Reorganized internal model structure for improved maintainability, moving API request and shared models to dedicated directories without affecting functionality.
  • Documentation

    • Refined OpenAPI schema descriptions to enhance clarity and consistency.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

Walkthrough

This PR reorganizes REST API request models from a monolithic src/models/requests.py file into a modular domain-driven structure under src/models/api/requests/ with submodules (catalog, conversations, feedback, MCP servers, prompts, query, responses, vector stores), extracts shared types (Attachment, SolrVectorSearchRequest, FeedbackCategory, HealthStatus) to src/models/common/, and updates import paths throughout the codebase.

Changes

Model Reorganization & Type Extraction

Layer / File(s) Summary
Shared Type Extraction
src/models/common/feedback.py, src/models/common/health.py, src/models/common/query.py, src/models/common/__init__.py
New shared models extracted: FeedbackCategory enum, HealthStatus enum, Attachment and SolrVectorSearchRequest with legacy coercion support. Module re-exports updated to include new shared types.
Request Model Organization
src/models/api/requests/__init__.py, src/models/api/requests/catalog.py, src/models/api/requests/conversations.py, src/models/api/requests/feedback.py, src/models/api/requests/mcp_servers.py, src/models/api/requests/prompts.py, src/models/api/requests/query.py, src/models/api/requests/responses_openai.py, src/models/api/requests/vector_stores.py
New domain-organized request models with field validators, Pydantic configurations (extra="forbid"), and JSON schema examples. Aggregator __init__.py re-exports curated public types via __all__.
Monolithic Source Removal
src/models/requests.py
Deleted 1303-line module containing original request models, enums, and validators.
Application Import Wiring
src/app/endpoints/*, src/models/context.py, src/utils/*
Updated ~15 endpoint files, 3 utility modules, and 1 context module to import request types from models.api.requests and shared types from models.common.* instead of models.requests.
Test Import Wiring
tests/integration/endpoints/*, tests/unit/app/endpoints/*, tests/unit/models/requests/*, tests/unit/utils/*
Updated ~45 test files to import request types from new models.api.requests location and shared types from models.common.*; simplified test in test_query_request.py by removing logger mocking.
Documentation & Metadata
AGENTS.md, src/models/README.md, tests/unit/models/requests/README.md, tests/unit/models/requests/__init__.py, docs/openapi.json
Updated project structure documentation, model module README, test documentation, and OpenAPI schema descriptions (removed embedded code examples, normalized formatting).

Sequence Diagram

Sequence diagrams are not applicable to this change, as it is primarily a refactoring of code organization and module structure without introducing new functional interactions or control flow patterns between components.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 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 'LCORE-1880: Split requests into multiple files' clearly and concisely summarizes the main change: reorganizing request models from a monolithic file into multiple organized files under a new module structure.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/models/common/health.py (1)

19-39: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

ProviderHealthStatus.status is str despite the co-located HealthStatus enum; docstring/examples reference a non-existent "unhealthy" value.

Two related problems:

  1. Type: status: str gives up the validation and IDE-completion benefits of HealthStatus. Typing it as HealthStatus (once the casing fix above is applied) enforces the closed set of values.
  2. Docs/examples mismatch: The class docstring (line 24) and field examples (line 33) both list 'unhealthy', which is not a member of HealthStatus. The closest valid values are HEALTHY = "healthy" and ERROR = "error".
🛡️ Proposed fix
-    status: str = Field(
+    status: HealthStatus = Field(
         description="The health status",
-        examples=["ok", "unhealthy", "not_implemented"],
+        examples=["ok", "error", "not_implemented"],
     )

And update the docstring attribute line:

-        status: The health status ('ok', 'unhealthy', 'not_implemented').
+        status: The health status ('ok', 'error', 'not_implemented', 'healthy', 'unknown').
🤖 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/models/common/health.py` around lines 19 - 39, Change the
ProviderHealthStatus.status field to use the HealthStatus enum instead of str
(i.e., status: HealthStatus) so Pydantic validates the closed set; update the
class docstring and the Field examples to remove the invalid 'unhealthy' value
and use the actual enum values (e.g., "healthy" and "error" or whatever
HealthStatus defines), and add an import for HealthStatus if not already
present.
🤖 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 `@docs/openapi.json`:
- Line 11465: Regenerate docs/openapi.json from the canonical schema/models
using the project's documented generator command prefixed with "uv run" (e.g.,
run the repo's openapi/doc generation task via "uv run
<generate-openapi-command>"), replace the committed drifted docs/openapi.json
with the newly generated file, commit the updated artifact, and push; repeat the
same regeneration/commit for the other out-of-date generated entries referenced
in the review so the generated docs remain synchronized with the source models.

In `@src/models/api/requests/catalog.py`:
- Around line 8-20: The class docstring for ModelFilter is incorrect: update the
docstring so it no longer says "Required model type" and instead describes
model_type as an optional filter (e.g., "Optional model type filter to select
models such as 'llm', 'embeddings'"); ensure the Attributes section matches the
field definition model_type: Optional[str] and the existing Field description,
and keep model_config = {"extra": "forbid"} and the symbol names ModelFilter and
model_type unchanged.

In `@src/models/api/requests/feedback.py`:
- Around line 170-173: The presence validation currently treats whitespace-only
strings as non-empty because it checks self.user_feedback == ""; update the
check in the conditional that references self.sentiment, self.user_feedback and
self.categories to treat whitespace-only feedback as empty by trimming before
emptiness check (e.g., use self.user_feedback.strip() or a helper like
cleaned_feedback = (self.user_feedback or "").strip() and test that for
equality/emptiness) so that `"   "` does not satisfy the “at least one feedback
form” rule; adjust the conditional that uses self.user_feedback accordingly.

In `@src/models/api/requests/mcp_servers.py`:
- Line 5: Replace usages of Optional[int] combined with Field(gt=0) in this
Pydantic model with Optional[PositiveInt] and update the imports accordingly:
add PositiveInt to the pydantic import line (replace or augment "from pydantic
import BaseModel, Field, field_validator" to include PositiveInt) and remove the
redundant gt=0 constraint from those Field(...) calls; specifically update each
field currently typed as Optional[int] + Field(gt=0) (the occurrences around the
later model fields) to Optional[PositiveInt] so validation uses the annotated
type instead of manual gt checks.

In `@src/models/api/requests/responses_openai.py`:
- Around line 157-169: The mutual-exclusivity check and the SUID validator treat
empty string as missing; update them to use explicit None checks so empty string
is considered provided and validated. Change the mutual-exclusivity condition to
check "self.conversation is not None" and "self.previous_response_id is not
None" (the block that raises ValueError when both are present) and change the
`@field_validator` method check_suid to "if value is not None and not
suid.check_suid(value)" so empty string fails SUID validation rather than being
skipped.

In `@src/models/api/requests/vector_stores.py`:
- Line 3: Replace the "from typing import Any, Optional, Self" import so that
Self is imported from typing_extensions (use "from typing import Any, Optional"
plus "from typing_extensions import Self") in
src/models/api/requests/vector_stores.py and the sibling request files query.py,
feedback.py, and responses_openai.py; update the import lines that reference
Self (and any annotations using Self) to use typing_extensions.Self to satisfy
Ruff UP035 and the project coding guidelines.

In `@src/models/common/health.py`:
- Around line 9-16: The HealthStatus enum has an inconsistent value: change
HealthStatus.ERROR from "Error" to lowercase "error" so it matches the other
enum members; update the enum definition in class HealthStatus (the ERROR
member) and any tests or comparisons that rely on HealthStatus.ERROR.value to
ensure they expect the lowercase string.

In `@src/models/common/query.py`:
- Around line 13-59: Change Attachment's model_config to use Pydantic v2's typed
ConfigDict like SolrVectorSearchRequest for consistency: import ConfigDict from
pydantic and replace the raw dict assigned to Attachment.model_config with a
ConfigDict(...) containing the same keys ("extra" and "json_schema_extra" with
the examples). Keep the contents identical but use ConfigDict to match the style
used by SolrVectorSearchRequest.

In `@src/models/README.md`:
- Around line 15-25: Update the README headings to satisfy MD022 by inserting a
single blank line between each heading and its following description for all
sections; specifically edit the headings "## [api/](api/)", "##
[common/](common/)", "## [database/](database/)", "## [rlsapi/](rlsapi/)" and
the pre-existing top sections that follow the same pattern so each heading
(e.g., the lines containing those exact strings) is immediately followed by one
blank line before the descriptive text.

In `@tests/unit/models/requests/test_query_request.py`:
- Around line 95-109: Test currently only checks that QueryRequest accepts
"text/plain"; add cases to cover the validator's other branches: assert that
"application/json" (MEDIA_TYPE_JSON) is accepted by constructing QueryRequest
(class QueryRequest) with media_type="application/json" and asserting
qr.media_type, and assert that an invalid media_type (e.g., "image/png") raises
ValueError from the model-validator validate_media_type in
src/models/api/requests/query.py by using pytest.raises(ValueError) when
instantiating QueryRequest with that value; reference QueryRequest and the
constants MEDIA_TYPE_JSON and MEDIA_TYPE_TEXT in the test names/comments so it's
clear which branch is covered.

---

Outside diff comments:
In `@src/models/common/health.py`:
- Around line 19-39: Change the ProviderHealthStatus.status field to use the
HealthStatus enum instead of str (i.e., status: HealthStatus) so Pydantic
validates the closed set; update the class docstring and the Field examples to
remove the invalid 'unhealthy' value and use the actual enum values (e.g.,
"healthy" and "error" or whatever HealthStatus defines), and add an import for
HealthStatus if not already present.
🪄 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: da100b50-94de-486d-be81-ed767c3d939e

📥 Commits

Reviewing files that changed from the base of the PR and between 59c8eca and 066674f.

📒 Files selected for processing (71)
  • AGENTS.md
  • docs/openapi.json
  • src/app/endpoints/a2a.py
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/feedback.py
  • src/app/endpoints/health.py
  • src/app/endpoints/mcp_servers.py
  • src/app/endpoints/models.py
  • src/app/endpoints/prompts.py
  • src/app/endpoints/query.py
  • src/app/endpoints/responses.py
  • src/app/endpoints/stream_interrupt.py
  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/vector_stores.py
  • src/models/README.md
  • src/models/api/requests/__init__.py
  • src/models/api/requests/catalog.py
  • src/models/api/requests/conversations.py
  • src/models/api/requests/feedback.py
  • src/models/api/requests/mcp_servers.py
  • src/models/api/requests/prompts.py
  • src/models/api/requests/query.py
  • src/models/api/requests/responses_openai.py
  • src/models/api/requests/vector_stores.py
  • src/models/common/__init__.py
  • src/models/common/feedback_category.py
  • src/models/common/health.py
  • src/models/common/query.py
  • src/models/context.py
  • src/models/requests.py
  • src/utils/query.py
  • src/utils/responses.py
  • src/utils/shields.py
  • src/utils/transcripts.py
  • src/utils/vector_search.py
  • tests/integration/endpoints/test_conversations_v1_integration.py
  • tests/integration/endpoints/test_conversations_v2_integration.py
  • tests/integration/endpoints/test_health_integration.py
  • tests/integration/endpoints/test_model_list.py
  • tests/integration/endpoints/test_query_byok_integration.py
  • tests/integration/endpoints/test_query_integration.py
  • tests/integration/endpoints/test_stream_interrupt_integration.py
  • tests/integration/endpoints/test_streaming_query_byok_integration.py
  • tests/integration/endpoints/test_streaming_query_integration.py
  • tests/unit/app/endpoints/test_conversations.py
  • tests/unit/app/endpoints/test_conversations_v2.py
  • tests/unit/app/endpoints/test_feedback.py
  • tests/unit/app/endpoints/test_health.py
  • tests/unit/app/endpoints/test_mcp_servers.py
  • tests/unit/app/endpoints/test_models.py
  • tests/unit/app/endpoints/test_prompts.py
  • tests/unit/app/endpoints/test_query.py
  • tests/unit/app/endpoints/test_responses.py
  • tests/unit/app/endpoints/test_responses_splunk.py
  • tests/unit/app/endpoints/test_stream_interrupt.py
  • tests/unit/app/endpoints/test_streaming_query.py
  • tests/unit/app/endpoints/test_vector_stores.py
  • tests/unit/models/requests/README.md
  • tests/unit/models/requests/__init__.py
  • tests/unit/models/requests/test_attachment.py
  • tests/unit/models/requests/test_feedback_request.py
  • tests/unit/models/requests/test_feedback_status_update_request.py
  • tests/unit/models/requests/test_query_request.py
  • tests/unit/models/requests/test_responses_request.py
  • tests/unit/models/requests/test_vector_store_requests.py
  • tests/unit/utils/test_prompts.py
  • tests/unit/utils/test_query.py
  • tests/unit/utils/test_responses.py
  • tests/unit/utils/test_transcripts.py
  • tests/unit/utils/test_vector_search.py
💤 Files with no reviewable changes (1)
  • src/models/requests.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). (9)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 1
  • 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
🧰 Additional context used
📓 Path-based instructions (5)
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use pytest for all unit and integration tests, not unittest

Use pytest-mock for AsyncMock objects in tests

Use marker pytest.mark.asyncio for async tests

Unit tests require 60% coverage, integration tests require 10% coverage

Files:

  • tests/integration/endpoints/test_stream_interrupt_integration.py
  • tests/unit/models/requests/test_feedback_status_update_request.py
  • tests/unit/models/requests/test_feedback_request.py
  • tests/unit/models/requests/test_attachment.py
  • tests/unit/models/requests/__init__.py
  • tests/unit/app/endpoints/test_responses.py
  • tests/unit/utils/test_responses.py
  • tests/integration/endpoints/test_query_byok_integration.py
  • tests/unit/app/endpoints/test_conversations_v2.py
  • tests/unit/app/endpoints/test_query.py
  • tests/unit/app/endpoints/test_conversations.py
  • tests/integration/endpoints/test_streaming_query_byok_integration.py
  • tests/unit/models/requests/test_query_request.py
  • tests/integration/endpoints/test_streaming_query_integration.py
  • tests/unit/app/endpoints/test_feedback.py
  • tests/integration/endpoints/test_model_list.py
  • tests/unit/utils/test_vector_search.py
  • tests/unit/app/endpoints/test_stream_interrupt.py
  • tests/unit/app/endpoints/test_prompts.py
  • tests/unit/app/endpoints/test_mcp_servers.py
  • tests/integration/endpoints/test_conversations_v1_integration.py
  • tests/unit/utils/test_transcripts.py
  • tests/unit/models/requests/test_vector_store_requests.py
  • tests/unit/utils/test_query.py
  • tests/integration/endpoints/test_query_integration.py
  • tests/unit/models/requests/test_responses_request.py
  • tests/integration/endpoints/test_conversations_v2_integration.py
  • tests/unit/app/endpoints/test_models.py
  • tests/unit/app/endpoints/test_streaming_query.py
  • tests/unit/utils/test_prompts.py
  • tests/unit/app/endpoints/test_responses_splunk.py
  • tests/unit/app/endpoints/test_vector_stores.py
  • tests/unit/app/endpoints/test_health.py
  • tests/integration/endpoints/test_health_integration.py
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use absolute imports for internal modules (e.g., from authentication import get_auth_dependency)

Use from fastapi import APIRouter, HTTPException, Request, status, Depends for FastAPI dependencies

Use from llama_stack_client import AsyncLlamaStackClient for Llama Stack imports

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

Use Final[type] as type hint for all constants

Use @field_validator and @model_validator for custom validation in Pydantic models

Use type hints Optional[FilePath], PositiveInt, SecretStr in Pydantic models

All functions require docstrings with brief descriptions following Google Python docstring conventions

Use complete type annotations for function parameters and return types, including typing_extensions.Self, modern str | int syntax, and Optional[Type]

Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)

Avoid in-place parameter modification anti-patterns; return new data structures instead

Use async def for I/O operations and external API calls

Handle APIConnectionError from Llama Stack in error handling

Use standard log levels: logger.debug() for detailed diagnostic information, logger.info() for general information, logger.warning() for unexpected events, logger.error() for serious problems

All classes require descriptive docstrings explaining purpose

Use PascalCase with descriptive names and standard suffixes: Configuration for config classes, Error/Exception for custom exceptions, Resolver for strategy pattern, Interface for abstract base classes

Pydantic configuration models must extend ConfigurationBase, data models must extend BaseModel

Abstract classes must use ABC with @abstractmethod decorators

Use complete type annotations for all class attributes; avoid using Any

Fo...

Files:

  • src/models/api/requests/conversations.py
  • src/models/api/requests/__init__.py
  • src/app/endpoints/mcp_servers.py
  • src/models/common/__init__.py
  • src/app/endpoints/vector_stores.py
  • src/models/common/feedback_category.py
  • src/app/endpoints/a2a.py
  • src/models/context.py
  • src/models/api/requests/catalog.py
  • src/app/endpoints/feedback.py
  • src/models/common/query.py
  • src/models/common/health.py
  • src/models/api/requests/prompts.py
  • src/app/endpoints/stream_interrupt.py
  • src/app/endpoints/prompts.py
  • src/utils/vector_search.py
  • src/models/api/requests/feedback.py
  • src/app/endpoints/query.py
  • src/models/api/requests/responses_openai.py
  • src/models/api/requests/mcp_servers.py
  • src/models/api/requests/vector_stores.py
  • src/app/endpoints/conversations_v2.py
  • src/models/api/requests/query.py
  • src/app/endpoints/responses.py
  • src/utils/responses.py
  • src/app/endpoints/conversations_v1.py
  • src/utils/query.py
  • src/utils/shields.py
  • src/app/endpoints/health.py
  • src/app/endpoints/models.py
  • src/app/endpoints/streaming_query.py
  • src/utils/transcripts.py
AGENTS.md

📄 CodeRabbit inference engine (CLAUDE.md)

Document agent implementations and their configurations in AGENTS.md

Files:

  • AGENTS.md
src/**/__init__.py

📄 CodeRabbit inference engine (AGENTS.md)

Package __init__.py files must contain brief package descriptions

Files:

  • src/models/api/requests/__init__.py
  • src/models/common/__init__.py
src/app/endpoints/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use FastAPI HTTPException with appropriate status codes for API endpoint error handling

Files:

  • src/app/endpoints/mcp_servers.py
  • src/app/endpoints/vector_stores.py
  • src/app/endpoints/a2a.py
  • src/app/endpoints/feedback.py
  • src/app/endpoints/stream_interrupt.py
  • src/app/endpoints/prompts.py
  • src/app/endpoints/query.py
  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/responses.py
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/health.py
  • src/app/endpoints/models.py
  • src/app/endpoints/streaming_query.py
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Always check `pyproject.toml` for existing dependencies before adding new ones
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Always verify current library versions in `pyproject.toml` rather than assuming versions
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Define E2E test steps in `tests/e2e/features/steps/` and maintain test list in `tests/e2e/test_list.txt`
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Run `uv run make format` to auto-format code with black + ruff before completion
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Run `uv run make verify` to run all linters (black, pylint, pyright, ruff, docstyle, check-types) before completion
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Create unit tests for new code before completion
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Use `uv run` prefix for all commands and `uv sync --group dev --group llslibdev` for dependencies
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: PR titles MUST start with a JIRA issue key prefix from the allowed list: `LCORE-`, `RSPEED-`, `MGTM-`, `OLS-`, `RHDHPAI-`, `LEADS-`
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Follow existing code patterns in the module you're modifying
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Never commit secrets/keys; use environment variables for sensitive data
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T11:16:13.327Z
Learning: Check `pyproject.toml` for Python versions and required dependencies before development
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/api/requests/conversations.py
  • src/models/api/requests/__init__.py
  • src/models/common/__init__.py
  • src/models/common/feedback_category.py
  • src/models/context.py
  • src/models/api/requests/catalog.py
  • src/models/common/query.py
  • src/models/common/health.py
  • src/models/api/requests/prompts.py
  • src/models/api/requests/feedback.py
  • src/models/api/requests/responses_openai.py
  • src/models/api/requests/mcp_servers.py
  • src/models/api/requests/vector_stores.py
  • src/models/api/requests/query.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.

Applied to files:

  • src/models/api/requests/conversations.py
  • src/models/api/requests/__init__.py
  • src/models/common/__init__.py
  • src/models/common/feedback_category.py
  • src/models/context.py
  • src/models/api/requests/catalog.py
  • src/models/common/query.py
  • src/models/common/health.py
  • src/models/api/requests/prompts.py
  • src/models/api/requests/feedback.py
  • src/models/api/requests/responses_openai.py
  • src/models/api/requests/mcp_servers.py
  • src/models/api/requests/vector_stores.py
  • src/models/api/requests/query.py
📚 Learning: 2026-03-02T15:57:10.746Z
Learnt from: asamal4
Repo: lightspeed-core/lightspeed-stack PR: 1250
File: AGENTS.md:11-12
Timestamp: 2026-03-02T15:57:10.746Z
Learning: In AGENTS.md, Linting Tools section should list actual tool names (e.g., mypy, pylint, pyright, ruff, black) instead of make target names (e.g., check-types). Ensure the section documents the individual tools that perform linting, not the Makefile targets that invoke them, so readers understand which tools are used and how to configure/run them directly.

Applied to files:

  • AGENTS.md
📚 Learning: 2026-04-06T20:18:07.852Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.

Applied to files:

  • src/app/endpoints/mcp_servers.py
  • src/app/endpoints/vector_stores.py
  • src/app/endpoints/a2a.py
  • src/app/endpoints/feedback.py
  • src/app/endpoints/stream_interrupt.py
  • src/app/endpoints/prompts.py
  • src/app/endpoints/query.py
  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/responses.py
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/health.py
  • src/app/endpoints/models.py
  • src/app/endpoints/streaming_query.py
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.

Applied to files:

  • src/app/endpoints/query.py
📚 Learning: 2026-02-23T14:56:59.186Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1198
File: src/utils/responses.py:184-192
Timestamp: 2026-02-23T14:56:59.186Z
Learning: In the lightspeed-stack codebase (lightspeed-core/lightspeed-stack), do not enforce de-duplication of duplicate client.models.list() calls in model selection flows (e.g., in src/utils/responses.py prepare_responses_params). These calls are considered relatively cheap and removing duplicates could add unnecessary complexity to the flow. Apply this guideline specifically to this file/context unless similar performance characteristics and design decisions are documented elsewhere.

Applied to files:

  • src/utils/responses.py
🪛 GitHub Actions: OpenAPI (Spectral)
docs/openapi.json

[error] 1-1: CI check failed: docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json (diff -u /tmp/openapi-generated.json vs docs/openapi.json failed).

🪛 markdownlint-cli2 (0.22.1)
src/models/README.md

[warning] 15-15: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 24-24: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🔇 Additional comments (62)
tests/unit/models/requests/__init__.py (1)

1-1: Docstring update is consistent with the new model layout.

Looks good and aligned with the refactor from requests.py to models.api.requests.

tests/unit/models/requests/README.md (1)

4-4: README entry correctly reflects the new request-model module path.

This keeps docs in sync with the refactor.

AGENTS.md (1)

34-38: LGTM — project structure accurately reflects the new layout.

The updated tree correctly documents the split into api/requests/, api/responses/, and common/.

src/models/common/feedback_category.py (1)

1-19: LGTM — clean, well-documented enum.

Enum values are consistent (all lowercase), class and module docstrings are present, and the inline comments provide useful context for each category.

src/models/api/requests/conversations.py (1)

1-22: LGTM — well-structured request model with proper validation constraints.

Field constraints (min_length=1, max_length=1000) and extra="forbid" are all correctly applied.

src/models/api/requests/prompts.py (1)

1-86: LGTM — both request models are well-structured with appropriate validation.

min_length, gt=0, and extra="forbid" constraints are correctly applied; OpenAPI examples are provided via json_schema_extra.

src/models/common/query.py (1)

89-109: LGTM — backward-compatibility validator is correct and well-documented.

The three-branch guard (None/non-dict → pass-through, "filters"/"mode" present → new format, otherwise → legacy wrap) correctly handles all input shapes, and the deprecation warning message is precise and actionable.

src/utils/vector_search.py (1)

21-21: Import relocation looks correct.

The move to from models.common.query import SolrVectorSearchRequest is consistent with the model split and keeps internal imports absolute.

src/models/common/__init__.py (1)

1-51: models.common export surface is coherent after the split.

The new exports and __all__ updates are consistent with the refactor and keep shared types centrally accessible.

src/app/endpoints/health.py (1)

29-29: Shared health model import is correctly wired.

Using HealthStatus/ProviderHealthStatus from models.common matches the new shared-model structure.

tests/unit/app/endpoints/test_health.py (1)

17-17: Unit test import update is correct.

Switching HealthStatus to models.common keeps tests aligned with the endpoint refactor.

tests/integration/endpoints/test_health_integration.py (1)

17-17: Integration test import aligns with shared enum location.

src/models/api/requests/responses_openai.py (1)

109-141: Body-size guard placement and error messaging look good.

Pre-validation size checks are clear and enforce the configured cap consistently.

tests/unit/app/endpoints/test_models.py (1)

14-14: Import path migration is correct.

This keeps the test aligned with the new request-model package surface.

tests/unit/models/requests/test_responses_request.py (1)

9-9: Updated import is consistent with the new model layout.

tests/integration/endpoints/test_conversations_v2_integration.py (1)

22-22: Import update is correct for the refactored request modules.

src/app/endpoints/stream_interrupt.py (1)

10-10: Request import swap is clean and consistent with endpoint typing.

tests/unit/utils/test_responses.py (1)

62-62: QueryRequest import path update looks good.

tests/integration/endpoints/test_conversations_v1_integration.py (1)

23-23: Import migration is correct and keeps v1 tests aligned with the new package exports.

tests/unit/app/endpoints/test_mcp_servers.py (1)

17-17: Import path update is correct for the refactored request model structure.

tests/unit/app/endpoints/test_responses.py (1)

29-29: Import migration is correct and aligned with the request-model split.

This update keeps the test wired to the new API request model package without changing test behavior.

tests/unit/utils/test_transcripts.py (1)

8-8: Good import-path update for QueryRequest.

This keeps transcripts tests aligned with the new request model module layout.

tests/unit/app/endpoints/test_feedback.py (1)

21-21: Import relocation looks right for feedback request models.

Both request types now correctly target the new API request package.

tests/unit/models/requests/test_vector_store_requests.py (1)

6-6: Vector store request imports are updated correctly.

The tests now reference the new canonical request-model namespace.

tests/unit/app/endpoints/test_vector_stores.py (1)

26-30: This consolidated import move is correct.

VectorStoreCreateRequest, VectorStoreFileCreateRequest, and VectorStoreUpdateRequest are all sourced from the new API requests package as expected.

tests/integration/endpoints/test_streaming_query_byok_integration.py (1)

19-19: Import update is aligned for BYOK streaming integration tests.

Using models.api.requests.QueryRequest here is consistent with the rest of the refactor.

tests/unit/app/endpoints/test_responses_splunk.py (1)

25-25: ResponsesRequest import path migration is correct.

This keeps the Splunk response tests synchronized with the new API request model organization.

src/utils/responses.py (1)

94-94: LGTM!

Import correctly updated to the new models.api.requests namespace, consistent with the PR-wide migration.

tests/unit/models/requests/test_feedback_request.py (1)

6-6: LGTM!

Import correctly migrated from the legacy models.requests to models.api.requests.

tests/unit/models/requests/test_attachment.py (1)

3-3: LGTM!

Attachment correctly re-pointed to its new canonical home in models.common.query.

tests/integration/endpoints/test_stream_interrupt_integration.py (1)

10-10: LGTM!

Import correctly updated to models.api.requests; test logic and lifecycle assertions are unaffected.

tests/integration/endpoints/test_model_list.py (1)

15-15: LGTM!

ModelFilter now correctly imported from models.api.requests; all test scenarios are unaffected.

tests/unit/app/endpoints/test_conversations_v2.py (1)

23-23: LGTM!

ConversationUpdateRequest correctly re-pointed to models.api.requests; no test logic affected.

tests/unit/models/requests/test_feedback_status_update_request.py (1)

3-3: LGTM!

Import correctly migrated to models.api.requests.

tests/unit/utils/test_prompts.py (1)

9-10: LGTM!

QueryRequest correctly sourced from the new models.api.requests namespace; all prompt-related test cases are unaffected.

src/app/endpoints/feedback.py (1)

16-16: Import migration is correct and consistent.

Using models.api.requests here aligns with the request-model split and keeps endpoint typing unchanged.

tests/unit/app/endpoints/test_prompts.py (1)

20-20: Test import update looks good.

The request model source now matches the new API request package used by production code.

src/app/endpoints/conversations_v1.py (1)

19-19: Good import source update for conversation request model.

This keeps the endpoint aligned with the new request model module layout.

tests/integration/endpoints/test_query_byok_integration.py (1)

18-18: Integration test import migration is consistent.

QueryRequest now comes from the same package structure used across the refactor.

tests/unit/app/endpoints/test_stream_interrupt.py (1)

10-10: Looks good — request model import was migrated correctly.

No functional behavior is affected by this test-side update.

src/app/endpoints/vector_stores.py (1)

25-29: Request model imports are correctly re-pointed.

All vector store request types are now sourced from the new API request package as expected.

tests/unit/app/endpoints/test_conversations.py (1)

22-22: Conversation update request import update is correct.

The unit tests now reference the same request-model package as the endpoint code.

src/app/endpoints/a2a.py (1)

47-47: A2A request model import migration is clean.

QueryRequest is now sourced from the consolidated API request package consistently.

tests/unit/utils/test_vector_search.py (1)

9-9: Import path update looks correct.

This aligns with the request/common model split and is consistent with usage in this test module.

tests/integration/endpoints/test_query_integration.py (1)

21-23: Request/attachment imports are correctly migrated.

The new import sources match the refactored model package boundaries without changing test behavior.

tests/integration/endpoints/test_streaming_query_integration.py (1)

15-16: Streaming integration imports are aligned with the new model layout.

No functional risk here; the updated modules are the correct targets for these types.

src/app/endpoints/query.py (1)

27-27: Endpoint request type import is correctly updated.

This keeps /query bound to the new API request-model surface.

src/app/endpoints/mcp_servers.py (1)

14-14: MCP registration request import migration is correct.

The handler remains correctly typed against the new API request module.

tests/unit/app/endpoints/test_streaming_query.py (1)

67-70: Unit test imports are migrated consistently.

The updated paths reflect the new request/common model separation without altering behavior.

tests/unit/utils/test_query.py (1)

18-25: utils.query unit tests now reference the right model modules.

Good alignment with the new API/common request model structure.

src/app/endpoints/models.py (1)

15-15: ModelFilter import update is appropriate.

The /models endpoint remains correctly typed against the refactored catalog request model.

src/utils/transcripts.py (1)

19-19: LGTM – import path correctly updated.

tests/unit/app/endpoints/test_query.py (1)

15-18: LGTM – imports correctly updated to new module locations.

src/models/context.py (1)

7-7: LGTM – QueryRequest correctly sourced from new location.

tests/unit/models/requests/test_query_request.py (1)

5-6: LGTM – imports correctly updated.

src/app/endpoints/streaming_query.py (1)

63-63: LGTM – QueryRequest import correctly updated to models.api.requests.

src/utils/shields.py (1)

21-21: LGTM – QueryRequest import correctly updated.

src/app/endpoints/conversations_v2.py (1)

11-11: LGTM – ConversationUpdateRequest import correctly updated.

src/app/endpoints/prompts.py (1)

16-16: LGTM – PromptCreateRequest and PromptUpdateRequest imports correctly updated.

src/app/endpoints/responses.py (1)

41-41: LGTM — import re-pointed correctly.

ResponsesRequest is properly exported from models.api.requests.__init__ (confirmed in src/models/api/requests/__init__.py, line 9 / __all__ line 27) and defined in src/models/api/requests/responses_openai.py.

src/utils/query.py (1)

28-28: LGTM — both imports re-pointed correctly.

QueryRequest is re-exported from models.api.requests and Attachment is defined in models.common.query; both usages in prepare_input, store_query_results, and validate_attachments_metadata are unaffected.

Also applies to: 39-39

src/models/api/requests/__init__.py (1)

1-32: LGTM — aggregation module is well-structured.

All submodule imports resolve to the new domain-organized files, __all__ is exhaustive, and the FeedbackCategory convenience re-export keeps callers from needing a second import for constructing FeedbackRequest.

Comment thread docs/openapi.json
Comment thread src/models/api/requests/catalog.py
Comment thread src/models/api/requests/feedback.py
Comment thread src/models/api/requests/mcp_servers.py
Comment thread src/models/api/requests/responses_openai.py
Comment thread src/models/api/requests/vector_stores.py
Comment thread src/models/common/health.py
Comment thread src/models/common/query.py
Comment thread src/models/README.md
Comment thread tests/unit/models/requests/test_query_request.py
@asimurka asimurka force-pushed the move_error_responses branch from 066674f to 2c45fa3 Compare May 5, 2026 11:25
HEALTHY = "healthy"
UNKNOWN = "unknown"


Copy link
Copy Markdown
Contributor Author

@asimurka asimurka May 5, 2026

Choose a reason for hiding this comment

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

Replaced out of endpoint to src/models/common/health.py

@asimurka asimurka force-pushed the move_error_responses branch from 2c45fa3 to 651c12b Compare May 5, 2026 12:35
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (8)
src/models/README.md (1)

15-25: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

MD022: Headings still missing blank lines below them.

The four new headings (lines 15, 18, 21, 24) are each immediately followed by description text without a blank line, which still triggers markdownlint MD022. This was flagged in a previous review and marked addressed, but the current code does not reflect the fix.

📄 Proposed fix
-## [api/](api/)
-Models for API request and response bodies.
+## [api/](api/)
+
+Models for API request and response bodies.

-## [common/](common/)
-Shared models and types used across endpoints.
+## [common/](common/)
+
+Shared models and types used across endpoints.

-## [database/](database/)
-SQLAlchemy ORM models.
+## [database/](database/)
+
+SQLAlchemy ORM models.

-## [rlsapi/](rlsapi/)
-Models for the rlsapi v1 API surface.
+## [rlsapi/](rlsapi/)
+
+Models for the rlsapi v1 API surface.
🤖 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/models/README.md` around lines 15 - 25, The four level-2 headings "##
[api/](api/)", "## [common/](common/)", "## [database/](database/)", and "##
[rlsapi/](rlsapi/)" in README.md lack the required blank line after each heading
which triggers markdownlint MD022; fix this by inserting a single empty line
immediately after each of those heading lines so each heading is followed by a
blank line before its descriptive text, then run markdownlint to verify the
MD022 warning is resolved.
src/models/api/requests/catalog.py (1)

12-12: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Misleading docstring: "Required model type" should be "Optional model type filter".

The Attributes section still says "Required model type" while the field is Optional[str] defaulting to None. This was flagged in a prior review and remains unresolved.

📄 Proposed fix
-        model_type: Required model type, such as 'llm', 'embeddings' etc.
+        model_type: Optional filter to narrow results to models of this type, such as 'llm', 'embeddings' etc.
🤖 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/models/api/requests/catalog.py` at line 12, Update the misleading
docstring for the model_type attribute in src/models/api/requests/catalog.py:
change "Required model type" to "Optional model type filter" and clarify it is
Optional[str] defaulting to None (e.g., "Optional model type filter, such as
'llm' or 'embeddings'; defaults to None"). Ensure the description next to the
model_type field and any Attributes section consistently reflect that it's an
optional filter rather than required.
src/models/api/requests/vector_stores.py (1)

3-3: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Self must be imported from typing_extensions, not typing.

The coding guidelines explicitly require typing_extensions.Self, and the project has Ruff rule UP035 enabled. This was flagged in a prior review and remains unresolved.

📄 Proposed fix
-from typing import Any, Optional, Self
+from typing import Any, Optional
+from typing_extensions import Self
🤖 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/models/api/requests/vector_stores.py` at line 3, The import is using Self
from the stdlib typing which violates the UP035 rule; change the import
statement so that Self is imported from typing_extensions instead of typing
(e.g., replace "from typing import Any, Optional, Self" with importing Any and
Optional from typing and Self from typing_extensions), ensuring the symbol Self
used in this module (e.g., any class or function type hints referencing Self)
now references typing_extensions.Self.
src/models/common/health.py (1)

9-16: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

ERROR = "Error" still has inconsistent casing.

Every other member ("ok", "not_implemented", "healthy", "unknown") is lowercase. The capital "E" will silently mismatch any code that compares .value to a lowercase string literal.

🐛 Proposed fix
-    ERROR = "Error"
+    ERROR = "error"
🤖 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/models/common/health.py` around lines 9 - 16, The HealthStatus enum has
inconsistent casing: the member ERROR in class HealthStatus currently uses
"Error" which should be lowercase; update HealthStatus.ERROR to use "error" to
match the other members and avoid silent mismatches when comparing
HealthStatus.value to lowercase strings (check usages of HealthStatus.ERROR if
any to ensure comparisons remain valid).
tests/unit/models/requests/test_query_request.py (1)

95-109: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

test_validate_media_type is still missing the rejection and application/json branches.

Only text/plain acceptance is covered. The validate_media_type validator raises ValueError for unsupported types, and application/json is also a valid value — both branches remain untested.

🛡️ Proposed additions
+    def test_validate_media_type_json(self) -> None:
+        """Test that application/json is also an accepted media_type."""
+        qr = QueryRequest(
+            query="Tell me about Kubernetes",
+            media_type="application/json",
+        )  # pyright: ignore[reportCallIssue]
+        assert qr.media_type == "application/json"
+
+    def test_validate_media_type_invalid_raises(self) -> None:
+        """Test that an unsupported media_type raises ValueError."""
+        with pytest.raises(ValueError, match="media_type must be either"):
+            QueryRequest(
+                query="Tell me about Kubernetes",
+                media_type="text/html",
+            )  # pyright: ignore[reportCallIssue]
🤖 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/models/requests/test_query_request.py` around lines 95 - 109, The
test test_validate_media_type only checks that QueryRequest stores "text/plain";
add two more assertions: one that QueryRequest(media_type="application/json") is
accepted and stores media_type "application/json", and one that an unsupported
media_type (e.g., "image/png") raises ValueError from the validate_media_type
validator when constructing QueryRequest; use pytest.raises(ValueError) to
assert the rejection and direct attribute checks for the successful
application/json case, referencing the QueryRequest class and its
validate_media_type validator.
src/models/api/requests/feedback.py (1)

170-179: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Whitespace-only user_feedback still bypasses the "at least one feedback signal" guard.

" " is neither None nor "", so (self.user_feedback is None or self.user_feedback == "") evaluates to False, allowing a blank-whitespace-only string to satisfy the presence check.

Proposed fix
         if (
             self.sentiment is None
-            and (self.user_feedback is None or self.user_feedback == "")
+            and not (self.user_feedback or "").strip()
             and self.categories is None
         ):
🤖 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/models/api/requests/feedback.py` around lines 170 - 179, The presence
check currently treats whitespace-only user_feedback as valid; update the guard
in the method that returns self to treat strings containing only whitespace as
empty by checking user_feedback.strip() (e.g., consider user_feedback is None or
user_feedback.strip() == "" or use a truthy check like not (self.user_feedback
and self.user_feedback.strip())); ensure the conditional that combines
sentiment, user_feedback, and categories uses this trimmed check so
whitespace-only user_feedback no longer bypasses the validation for attributes
sentiment, user_feedback, and categories.
src/models/api/requests/responses_openai.py (1)

157-170: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

conversation empty-string bypass in mutual-exclusivity check and SUID validator still unresolved.

Both conditions treat "" as "not provided" because they use truthiness rather than is not None, silently skipping validation:

  • Line 157: if self.conversation and self.previous_response_id — a client sending conversation="" alongside previous_response_id passes the mutual-exclusivity guard.
  • Line 168: if value and not suid.check_suid(value)conversation="" skips SUID validation entirely.
Proposed fix
-        if self.conversation and self.previous_response_id:
+        if self.conversation is not None and self.previous_response_id is not None:
             raise ValueError(
                 "`conversation` and `previous_response_id` are mutually exclusive. "
                 "Only one can be provided at a time."
             )
-        if value and not suid.check_suid(value):
+        if value is not None and not suid.check_suid(value):
             raise ValueError(f"Improper conversation ID '{value}'")
🤖 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/models/api/requests/responses_openai.py` around lines 157 - 170, The
mutual-exclusivity and SUID validation incorrectly use truthiness so an empty
string ("") bypasses checks; update the guard in the method that checks
`self.conversation` and `self.previous_response_id` to treat an empty string as
provided (e.g., use explicit checks like `self.conversation is not None and
self.conversation != ""`), and update the `check_suid` field validator (the
`@field_validator` method named `check_suid`) to only skip validation for None but
validate and reject empty-string and non-SUID values (e.g., `if value is not
None and value != "" and not suid.check_suid(value): raise ValueError(...)`).
src/models/common/query.py (1)

38-59: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Attachment.model_config still uses a raw dict while SolrVectorSearchRequest in the same file uses ConfigDict.

This inconsistency was raised in a previous review and remains unresolved.

♻️ Proposed fix
-    model_config = {
-        "extra": "forbid",
-        "json_schema_extra": {
-            "examples": [
-                ...
-            ]
-        },
-    }
+    model_config = ConfigDict(
+        extra="forbid",
+        json_schema_extra={
+            "examples": [
+                ...
+            ]
+        },
+    )
🤖 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/models/common/query.py` around lines 38 - 59, Attachment.model_config is
using a plain dict while SolrVectorSearchRequest uses pydantic's ConfigDict;
replace the raw dict with a typed ConfigDict for Attachment.model_config, import
ConfigDict from pydantic (or reuse the existing import pattern used by
SolrVectorSearchRequest), and assign model_config as a ClassVar[ConfigDict]
containing the same keys ("extra": "forbid", "json_schema_extra": {...}) to
match the other model's style and ensure consistency.
🤖 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/app/endpoints/models.py`:
- Line 15: Replace the direct submodule import with the package-level re-export:
change the import of ModelFilter in endpoints/models.py from
models.api.requests.catalog to using the package import (ModelFilter from
models.api.requests) so it matches tests and other modules; update the import
statement that references ModelFilter accordingly to use the package-level
symbol exported by models.api.requests.

In `@src/models/api/requests/mcp_servers.py`:
- Around line 145-154: The code validates header values by using stripped =
header_value.strip() but returns the original value dict unchanged, so inputs
like " client " remain unnormalized; update the loop in the validator to store
the normalized value back into the dict (e.g., value[header_name] = stripped) or
build and return a new dict of stripped values after checking allowed (use the
existing symbols header_name, header_value, stripped, value, allowed) so the
function returns normalized header strings rather than the original untrimmed
ones.

In `@src/models/api/requests/query.py`:
- Line 3: Replace the use of typing.Self with typing_extensions.Self in the
import statement so validators in Pydantic models use the portable,
Pydantic-recommended Self; update the line that currently imports "Self" (e.g.,
"from typing import Optional, Self") to import Self from typing_extensions
instead, ensuring all references to Self in model methods/validators (such as
any Pydantic model classes or validator signatures in this file) remain
unchanged.
- Around line 81-91: The fields no_tools and generate_topic_summary are typed as
Optional[bool] which allows callers to send null and makes downstream truthiness
checks unreliable; change their annotations from Optional[bool] to plain bool
and keep the existing default values (no_tools=False,
generate_topic_summary=True) so Pydantic will reject nulls and preserve intended
boolean semantics for the no_tools and generate_topic_summary fields defined
with Field(...).

---

Duplicate comments:
In `@src/models/api/requests/catalog.py`:
- Line 12: Update the misleading docstring for the model_type attribute in
src/models/api/requests/catalog.py: change "Required model type" to "Optional
model type filter" and clarify it is Optional[str] defaulting to None (e.g.,
"Optional model type filter, such as 'llm' or 'embeddings'; defaults to None").
Ensure the description next to the model_type field and any Attributes section
consistently reflect that it's an optional filter rather than required.

In `@src/models/api/requests/feedback.py`:
- Around line 170-179: The presence check currently treats whitespace-only
user_feedback as valid; update the guard in the method that returns self to
treat strings containing only whitespace as empty by checking
user_feedback.strip() (e.g., consider user_feedback is None or
user_feedback.strip() == "" or use a truthy check like not (self.user_feedback
and self.user_feedback.strip())); ensure the conditional that combines
sentiment, user_feedback, and categories uses this trimmed check so
whitespace-only user_feedback no longer bypasses the validation for attributes
sentiment, user_feedback, and categories.

In `@src/models/api/requests/responses_openai.py`:
- Around line 157-170: The mutual-exclusivity and SUID validation incorrectly
use truthiness so an empty string ("") bypasses checks; update the guard in the
method that checks `self.conversation` and `self.previous_response_id` to treat
an empty string as provided (e.g., use explicit checks like `self.conversation
is not None and self.conversation != ""`), and update the `check_suid` field
validator (the `@field_validator` method named `check_suid`) to only skip
validation for None but validate and reject empty-string and non-SUID values
(e.g., `if value is not None and value != "" and not suid.check_suid(value):
raise ValueError(...)`).

In `@src/models/api/requests/vector_stores.py`:
- Line 3: The import is using Self from the stdlib typing which violates the
UP035 rule; change the import statement so that Self is imported from
typing_extensions instead of typing (e.g., replace "from typing import Any,
Optional, Self" with importing Any and Optional from typing and Self from
typing_extensions), ensuring the symbol Self used in this module (e.g., any
class or function type hints referencing Self) now references
typing_extensions.Self.

In `@src/models/common/health.py`:
- Around line 9-16: The HealthStatus enum has inconsistent casing: the member
ERROR in class HealthStatus currently uses "Error" which should be lowercase;
update HealthStatus.ERROR to use "error" to match the other members and avoid
silent mismatches when comparing HealthStatus.value to lowercase strings (check
usages of HealthStatus.ERROR if any to ensure comparisons remain valid).

In `@src/models/common/query.py`:
- Around line 38-59: Attachment.model_config is using a plain dict while
SolrVectorSearchRequest uses pydantic's ConfigDict; replace the raw dict with a
typed ConfigDict for Attachment.model_config, import ConfigDict from pydantic
(or reuse the existing import pattern used by SolrVectorSearchRequest), and
assign model_config as a ClassVar[ConfigDict] containing the same keys ("extra":
"forbid", "json_schema_extra": {...}) to match the other model's style and
ensure consistency.

In `@src/models/README.md`:
- Around line 15-25: The four level-2 headings "## [api/](api/)", "##
[common/](common/)", "## [database/](database/)", and "## [rlsapi/](rlsapi/)" in
README.md lack the required blank line after each heading which triggers
markdownlint MD022; fix this by inserting a single empty line immediately after
each of those heading lines so each heading is followed by a blank line before
its descriptive text, then run markdownlint to verify the MD022 warning is
resolved.

In `@tests/unit/models/requests/test_query_request.py`:
- Around line 95-109: The test test_validate_media_type only checks that
QueryRequest stores "text/plain"; add two more assertions: one that
QueryRequest(media_type="application/json") is accepted and stores media_type
"application/json", and one that an unsupported media_type (e.g., "image/png")
raises ValueError from the validate_media_type validator when constructing
QueryRequest; use pytest.raises(ValueError) to assert the rejection and direct
attribute checks for the successful application/json case, referencing the
QueryRequest class and its validate_media_type validator.
🪄 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: 299316b8-10d0-4861-95ef-03749c488eec

📥 Commits

Reviewing files that changed from the base of the PR and between 066674f and 651c12b.

📒 Files selected for processing (71)
  • AGENTS.md
  • docs/openapi.json
  • src/app/endpoints/a2a.py
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/feedback.py
  • src/app/endpoints/health.py
  • src/app/endpoints/mcp_servers.py
  • src/app/endpoints/models.py
  • src/app/endpoints/prompts.py
  • src/app/endpoints/query.py
  • src/app/endpoints/responses.py
  • src/app/endpoints/stream_interrupt.py
  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/vector_stores.py
  • src/models/README.md
  • src/models/api/requests/__init__.py
  • src/models/api/requests/catalog.py
  • src/models/api/requests/conversations.py
  • src/models/api/requests/feedback.py
  • src/models/api/requests/mcp_servers.py
  • src/models/api/requests/prompts.py
  • src/models/api/requests/query.py
  • src/models/api/requests/responses_openai.py
  • src/models/api/requests/vector_stores.py
  • src/models/common/__init__.py
  • src/models/common/feedback.py
  • src/models/common/health.py
  • src/models/common/query.py
  • src/models/context.py
  • src/models/requests.py
  • src/utils/query.py
  • src/utils/responses.py
  • src/utils/shields.py
  • src/utils/transcripts.py
  • src/utils/vector_search.py
  • tests/integration/endpoints/test_conversations_v1_integration.py
  • tests/integration/endpoints/test_conversations_v2_integration.py
  • tests/integration/endpoints/test_health_integration.py
  • tests/integration/endpoints/test_model_list.py
  • tests/integration/endpoints/test_query_byok_integration.py
  • tests/integration/endpoints/test_query_integration.py
  • tests/integration/endpoints/test_stream_interrupt_integration.py
  • tests/integration/endpoints/test_streaming_query_byok_integration.py
  • tests/integration/endpoints/test_streaming_query_integration.py
  • tests/unit/app/endpoints/test_conversations.py
  • tests/unit/app/endpoints/test_conversations_v2.py
  • tests/unit/app/endpoints/test_feedback.py
  • tests/unit/app/endpoints/test_health.py
  • tests/unit/app/endpoints/test_mcp_servers.py
  • tests/unit/app/endpoints/test_models.py
  • tests/unit/app/endpoints/test_prompts.py
  • tests/unit/app/endpoints/test_query.py
  • tests/unit/app/endpoints/test_responses.py
  • tests/unit/app/endpoints/test_responses_splunk.py
  • tests/unit/app/endpoints/test_stream_interrupt.py
  • tests/unit/app/endpoints/test_streaming_query.py
  • tests/unit/app/endpoints/test_vector_stores.py
  • tests/unit/models/requests/README.md
  • tests/unit/models/requests/__init__.py
  • tests/unit/models/requests/test_attachment.py
  • tests/unit/models/requests/test_feedback_request.py
  • tests/unit/models/requests/test_feedback_status_update_request.py
  • tests/unit/models/requests/test_query_request.py
  • tests/unit/models/requests/test_responses_request.py
  • tests/unit/models/requests/test_vector_store_requests.py
  • tests/unit/utils/test_prompts.py
  • tests/unit/utils/test_query.py
  • tests/unit/utils/test_responses.py
  • tests/unit/utils/test_transcripts.py
  • tests/unit/utils/test_vector_search.py
💤 Files with no reviewable changes (1)
  • src/models/requests.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use absolute imports for internal modules: from authentication import get_auth_dependency

Use from llama_stack_client import AsyncLlamaStackClient for Llama Stack integration

All modules must start with descriptive docstrings explaining their purpose

Use logger = get_logger(__name__) from log.py for module logging

Use Final[type] as type hint for all constants

Define type aliases at module level for clarity

Use @field_validator and @model_validator for custom validation in Pydantic models

All functions must have complete docstrings with brief descriptions

Use complete type annotations for all function parameters and return types

Use typing_extensions.Self for model validators in Pydantic models

Use modern union syntax str | int instead of Union[str, int] for type annotations

Use Optional[Type] for optional parameters in type annotations

Use snake_case with descriptive, action-oriented function names (get_, validate_, check_ patterns)

Avoid in-place parameter modification anti-patterns; return new data structures instead

Use async def for functions that perform I/O operations and external API calls

Handle APIConnectionError from Llama Stack in functions that interact with Llama Stack

Use standard log levels with clear purposes: debug (diagnostic), info (execution), warning (unexpected), error (serious problems)

All classes must have descriptive docstrings explaining their purpose

Use PascalCase for class names with descriptive names and standard suffixes (Configuration, Error/Exception, Resolver, Interface)

Pydantic configuration models must extend ConfigurationBase, data models must extend BaseModel

Use ABC (Abstract Base Class) for interfaces with @abstractmethod decorators

Use complete type annotations for all class attributes; avoid using Any type

Follow Google Python docstring conventions (https://google.github.io/styleguide/pyguide.html) with Parameters, Returns, Raises, and Attributes sections

Files:

  • src/app/endpoints/feedback.py
  • src/utils/responses.py
  • src/models/common/feedback.py
  • src/models/api/requests/__init__.py
  • src/utils/vector_search.py
  • src/models/common/health.py
  • src/app/endpoints/stream_interrupt.py
  • src/models/api/requests/vector_stores.py
  • src/app/endpoints/a2a.py
  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/vector_stores.py
  • src/models/api/requests/mcp_servers.py
  • src/models/common/__init__.py
  • src/app/endpoints/query.py
  • src/app/endpoints/models.py
  • src/models/api/requests/prompts.py
  • src/models/common/query.py
  • src/models/api/requests/conversations.py
  • src/models/api/requests/catalog.py
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/responses.py
  • src/utils/shields.py
  • src/utils/transcripts.py
  • src/app/endpoints/mcp_servers.py
  • src/models/api/requests/query.py
  • src/utils/query.py
  • src/app/endpoints/prompts.py
  • src/models/context.py
  • src/models/api/requests/feedback.py
  • src/app/endpoints/conversations_v2.py
  • src/models/api/requests/responses_openai.py
  • src/app/endpoints/health.py
src/app/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use FastAPI dependencies: from fastapi import APIRouter, HTTPException, Request, status, Depends

Use FastAPI HTTPException with appropriate status codes for API endpoint error handling

Files:

  • src/app/endpoints/feedback.py
  • src/app/endpoints/stream_interrupt.py
  • src/app/endpoints/a2a.py
  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/vector_stores.py
  • src/app/endpoints/query.py
  • src/app/endpoints/models.py
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/responses.py
  • src/app/endpoints/mcp_servers.py
  • src/app/endpoints/prompts.py
  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/health.py
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use pytest for all unit and integration tests; do not use unittest

Use pytest-mock with AsyncMock objects for mocking in unit tests

Use pytest.mark.asyncio marker for async test functions

Files:

  • tests/unit/models/requests/test_feedback_request.py
  • tests/unit/models/requests/test_attachment.py
  • tests/integration/endpoints/test_stream_interrupt_integration.py
  • tests/unit/models/requests/__init__.py
  • tests/unit/utils/test_prompts.py
  • tests/unit/app/endpoints/test_feedback.py
  • tests/unit/app/endpoints/test_conversations_v2.py
  • tests/unit/app/endpoints/test_mcp_servers.py
  • tests/unit/app/endpoints/test_responses.py
  • tests/unit/utils/test_responses.py
  • tests/unit/app/endpoints/test_streaming_query.py
  • tests/integration/endpoints/test_conversations_v2_integration.py
  • tests/integration/endpoints/test_conversations_v1_integration.py
  • tests/unit/app/endpoints/test_prompts.py
  • tests/unit/models/requests/test_feedback_status_update_request.py
  • tests/unit/models/requests/test_query_request.py
  • tests/integration/endpoints/test_streaming_query_byok_integration.py
  • tests/unit/models/requests/test_vector_store_requests.py
  • tests/unit/utils/test_transcripts.py
  • tests/unit/utils/test_query.py
  • tests/unit/app/endpoints/test_models.py
  • tests/unit/app/endpoints/test_query.py
  • tests/integration/endpoints/test_health_integration.py
  • tests/unit/utils/test_vector_search.py
  • tests/unit/app/endpoints/test_conversations.py
  • tests/unit/models/requests/test_responses_request.py
  • tests/integration/endpoints/test_streaming_query_integration.py
  • tests/unit/app/endpoints/test_stream_interrupt.py
  • tests/integration/endpoints/test_query_byok_integration.py
  • tests/integration/endpoints/test_model_list.py
  • tests/unit/app/endpoints/test_vector_stores.py
  • tests/integration/endpoints/test_query_integration.py
  • tests/unit/app/endpoints/test_health.py
  • tests/unit/app/endpoints/test_responses_splunk.py
src/**/__init__.py

📄 CodeRabbit inference engine (AGENTS.md)

Package __init__.py files must contain brief package descriptions

Files:

  • src/models/api/requests/__init__.py
  • src/models/common/__init__.py
AGENTS.md

📄 CodeRabbit inference engine (CLAUDE.md)

Document agent implementations and their configurations in AGENTS.md

Files:

  • AGENTS.md
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Use `uv run` prefix for all commands (format, verify, testing, etc.)
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Unit tests require 60% code coverage; integration tests require 10% coverage
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Run `uv run make format` to auto-format code with black and ruff before completion
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Run `uv run make verify` to run all linters (black, pylint, pyright, ruff, docstyle, check-types) before completion
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: PR titles MUST start with JIRA issue key prefix (LCORE-, RSPEED-, MGTM-, OLS-, RHDHPAI-, LEADS-)
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Never commit secrets/keys; use environment variables for sensitive data
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Use bandit for security issue detection
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Use `uv sync --group dev --group llslibdev` for setting up development dependencies
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Follow existing code patterns in the module you're modifying
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack

Timestamp: 2026-05-05T12:39:25.013Z
Learning: Create unit tests covering new functionality before completion
📚 Learning: 2026-04-06T20:18:07.852Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.

Applied to files:

  • src/app/endpoints/feedback.py
  • src/app/endpoints/stream_interrupt.py
  • src/app/endpoints/a2a.py
  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/vector_stores.py
  • src/app/endpoints/query.py
  • src/app/endpoints/models.py
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/responses.py
  • src/app/endpoints/mcp_servers.py
  • src/app/endpoints/prompts.py
  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/health.py
📚 Learning: 2026-02-23T14:56:59.186Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1198
File: src/utils/responses.py:184-192
Timestamp: 2026-02-23T14:56:59.186Z
Learning: In the lightspeed-stack codebase (lightspeed-core/lightspeed-stack), do not enforce de-duplication of duplicate client.models.list() calls in model selection flows (e.g., in src/utils/responses.py prepare_responses_params). These calls are considered relatively cheap and removing duplicates could add unnecessary complexity to the flow. Apply this guideline specifically to this file/context unless similar performance characteristics and design decisions are documented elsewhere.

Applied to files:

  • src/utils/responses.py
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/common/feedback.py
  • src/models/api/requests/__init__.py
  • src/models/common/health.py
  • src/models/api/requests/vector_stores.py
  • src/models/api/requests/mcp_servers.py
  • src/models/common/__init__.py
  • src/models/api/requests/prompts.py
  • src/models/common/query.py
  • src/models/api/requests/conversations.py
  • src/models/api/requests/catalog.py
  • src/models/api/requests/query.py
  • src/models/context.py
  • src/models/api/requests/feedback.py
  • src/models/api/requests/responses_openai.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.

Applied to files:

  • src/models/common/feedback.py
  • src/models/api/requests/__init__.py
  • src/models/common/health.py
  • src/models/api/requests/vector_stores.py
  • src/models/api/requests/mcp_servers.py
  • src/models/common/__init__.py
  • src/models/api/requests/prompts.py
  • src/models/common/query.py
  • src/models/api/requests/conversations.py
  • src/models/api/requests/catalog.py
  • src/models/api/requests/query.py
  • src/models/context.py
  • src/models/api/requests/feedback.py
  • src/models/api/requests/responses_openai.py
📚 Learning: 2026-03-02T15:57:10.746Z
Learnt from: asamal4
Repo: lightspeed-core/lightspeed-stack PR: 1250
File: AGENTS.md:11-12
Timestamp: 2026-03-02T15:57:10.746Z
Learning: In AGENTS.md, Linting Tools section should list actual tool names (e.g., mypy, pylint, pyright, ruff, black) instead of make target names (e.g., check-types). Ensure the section documents the individual tools that perform linting, not the Makefile targets that invoke them, so readers understand which tools are used and how to configure/run them directly.

Applied to files:

  • AGENTS.md
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.

Applied to files:

  • src/app/endpoints/query.py
🔇 Additional comments (52)
tests/unit/models/requests/__init__.py (1)

1-1: LGTM!

Docstring correctly reflects the relocated path models.api.requests.

AGENTS.md (1)

34-38: LGTM!

Project structure tree accurately mirrors the new src/models/api/requests/ and src/models/common/ layout introduced by this PR.

tests/unit/models/requests/README.md (1)

1-24: LGTM!

README correctly reflects the relocated model path and includes all current test file entries.

src/models/common/feedback.py (1)

1-19: LGTM!

Clean enum definition with appropriate docstrings and well-defined categories.

src/models/api/requests/prompts.py (1)

1-86: LGTM!

Both request models are well-structured with appropriate field constraints, docstrings, and extra="forbid" configuration.

src/app/endpoints/a2a.py (1)

47-47: Import path update looks correct and consistent with the refactor.
No concerns with this change.

tests/integration/endpoints/test_streaming_query_byok_integration.py (1)

19-19: Good import migration for QueryRequest in integration tests.
This aligns with the new model module layout.

tests/unit/models/requests/test_vector_store_requests.py (1)

6-6: LGTM — request model imports are updated correctly.

tests/unit/app/endpoints/test_mcp_servers.py (1)

17-17: Import update is correct for MCP server request model relocation.

tests/integration/endpoints/test_stream_interrupt_integration.py (1)

10-10: Looks good — StreamingInterruptRequest import now points to the new package.

tests/unit/utils/test_transcripts.py (1)

8-8: Approved — QueryRequest import matches the new models.api.requests location.

tests/unit/app/endpoints/test_feedback.py (1)

21-21: Import migration for feedback request models is correct.

tests/unit/app/endpoints/test_conversations.py (1)

22-22: Looks good — ConversationUpdateRequest import is updated correctly.

tests/unit/models/requests/test_feedback_status_update_request.py (1)

3-3: Import path update is correct and consistent with the refactor.

The test now targets the reorganized request-model package without changing behavior.

tests/unit/app/endpoints/test_responses.py (1)

29-29: ResponsesRequest import migration looks good.

This keeps unit tests aligned with the new models.api.requests package layout.

src/utils/vector_search.py (1)

21-21: Shared-model import update is correct.

Using models.common.query.SolrVectorSearchRequest fits the new shared-model split and keeps this utility aligned with the refactor.

tests/integration/endpoints/test_conversations_v1_integration.py (1)

23-23: Import path migration is clean and consistent.

The integration tests now reference ConversationUpdateRequest from the new API request package as expected.

tests/unit/models/requests/test_feedback_request.py (1)

6-7: Feedback model imports are correctly split across API/common modules.

This matches the new model organization while preserving existing test intent.

tests/integration/endpoints/test_model_list.py (1)

15-15: ModelFilter import update looks good.

The test now points at the new request-model namespace with no behavioral regression risk in this change.

src/app/endpoints/stream_interrupt.py (1)

10-10: Request import update is correct.

Switching to models.api.requests.StreamingInterruptRequest is consistent with the model split and keeps endpoint typing aligned.

tests/unit/app/endpoints/test_conversations_v2.py (1)

23-23: ConversationUpdateRequest import change is good.

This keeps the unit tests synchronized with the new API request-model package structure.

tests/unit/app/endpoints/test_vector_stores.py (1)

26-30: Import path update looks correct.

The three vector store request model imports are consistently redirected to models.api.requests, matching the source change in src/app/endpoints/vector_stores.py.

src/app/endpoints/feedback.py (1)

16-16: Import path update looks correct.

FeedbackRequest and FeedbackStatusUpdateRequest are now sourced from models.api.requests, matching the new module structure.

tests/unit/app/endpoints/test_prompts.py (1)

20-20: Import path update looks correct.

PromptCreateRequest and PromptUpdateRequest are now sourced from models.api.requests, consistent with the endpoint source changes.

tests/unit/utils/test_prompts.py (1)

9-9: Import path update looks correct.

QueryRequest is now sourced from models.api.requests, aligning with the consolidated API request model structure.

tests/integration/endpoints/test_query_byok_integration.py (1)

18-18: Import path update looks correct.

QueryRequest is now sourced from models.api.requests, consistent with the rest of the BYOK test suite.

src/app/endpoints/conversations_v1.py (1)

19-19: Import path update looks correct.

ConversationUpdateRequest is now sourced from models.api.requests, matching the new conversations request submodule.

tests/integration/endpoints/test_query_integration.py (1)

21-23: Split import pattern is correct and architecturally sound.

QueryRequest from models.api.requests and Attachment from models.common.query properly reflect the separation of API-layer request models from shared domain types. The Attachment class is correctly defined in src/models/common/query.py as a Pydantic BaseModel and is properly exported for use in tests.

src/app/endpoints/vector_stores.py (1)

25-29: Import path updates are correct.

All three request models (VectorStoreCreateRequest, VectorStoreFileCreateRequest, VectorStoreUpdateRequest) are properly defined in src/models/api/requests/vector_stores.py and correctly re-exported through src/models/api/requests/__init__.py. No stale models.requests imports remain in the codebase.

tests/unit/app/endpoints/test_stream_interrupt.py (1)

10-10: LGTM – import path updated correctly.

StreamingInterruptRequest now resolves from the new models.api.requests package, consistent with the rest of the migration.

tests/unit/utils/test_vector_search.py (1)

9-9: LGTM – SolrVectorSearchRequest correctly moved to models.common.query.

tests/integration/endpoints/test_health_integration.py (1)

17-17: LGTM – HealthStatus correctly sourced from the shared models.common package.

tests/unit/utils/test_query.py (1)

18-25: LGTM – both QueryRequest and Attachment import paths updated correctly.

tests/unit/app/endpoints/test_streaming_query.py (1)

67-70: LGTM – import paths aligned with the new module structure.

tests/unit/app/endpoints/test_health.py (1)

17-17: LGTM – HealthStatus and ProviderHealthStatus both correctly sourced from models.common.

tests/integration/endpoints/test_streaming_query_integration.py (1)

15-16: LGTM – import paths updated consistently with the unit test counterparts.

src/app/endpoints/query.py (1)

27-27: ⚡ Quick win

New module structure is properly wired and migration is complete.

No stale models.requests imports remain in the codebase. The models/api/requests/__init__.py package correctly re-exports QueryRequest, and models/common/__init__.py properly re-exports HealthStatus, ProviderHealthStatus, Attachment, and SolrVectorSearchRequest. All symbols are in their expected locations.

src/utils/shields.py (1)

21-21: LGTM — import path updated correctly.

QueryRequest now resolves from the new models.api.requests package, consistent with the PR-wide migration.

src/models/context.py (1)

7-7: LGTM — import path updated correctly.

tests/unit/app/endpoints/test_query.py (1)

15-19: LGTM — both import paths updated correctly.

QueryRequest from models.api.requests and Attachment from models.common.query align with the PR-wide restructuring.

src/app/endpoints/streaming_query.py (1)

63-63: LGTM — import path updated correctly.

src/utils/transcripts.py (1)

19-19: LGTM — import path updated correctly.

src/models/api/requests/conversations.py (1)

1-22: LGTM — well-constrained model with appropriate validation.

ConversationUpdateRequest follows all the established patterns: BaseModel, Google-style Attributes docstring, field-level constraints, and extra="forbid" to reject unknown keys.

src/models/common/__init__.py (1)

1-51: LGTM — exports are consistent with the new common submodule structure.

All three new re-exports (FeedbackCategory, HealthStatus/ProviderHealthStatus, Attachment/SolrVectorSearchRequest) have matching import statements and __all__ entries.

src/app/endpoints/conversations_v2.py (1)

11-11: LGTM — import path correctly updated to the new module location.

src/app/endpoints/mcp_servers.py (1)

14-14: Import migration verified — no stale references remain.

The change at line 14 correctly uses the new absolute import path from models.api.requests import MCPServerRegistrationRequest. A search of the codebase confirms no remaining references to the old models.requests path exist, indicating the migration is complete.

docs/openapi.json (1)

11465-11465: LGTM! Documentation improvements are comprehensive and well-structured.

The OpenAPI schema descriptions have been properly regenerated with refined documentation for all affected request models. Each description follows a consistent format, clearly documents attributes with appropriate detail about optionality and defaults, and aligns with the PR's objective of improving model documentation after the request model reorganization.

The backtick formatting improvement in SolrVectorSearchRequest and the legacy compatibility note demonstrate attention to detail.

Also applies to: 12590-12590, 12967-12967, 13037-13037, 14000-14000, 16453-16453, 16687-16687, 17100-17100, 19294-19294

src/app/endpoints/prompts.py (1)

16-16: LGTM — import re-pointed to models.api.requests.

src/utils/query.py (1)

28-39: LGTM — both import paths correctly re-pointed to models.api.requests and models.common.query.

src/app/endpoints/health.py (1)

29-29: LGTMHealthStatus and ProviderHealthStatus correctly consolidated into models.common.

src/app/endpoints/responses.py (1)

41-41: LGTMResponsesRequest import correctly re-pointed to models.api.requests.

src/models/api/requests/__init__.py (1)

1-30: LGTM — clean aggregator module; all 13 exports are present in both imports and __all__.

Comment thread src/app/endpoints/models.py
Comment thread src/models/api/requests/mcp_servers.py
Comment thread src/models/api/requests/query.py
Comment thread src/models/api/requests/query.py
Copy link
Copy Markdown
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

LGTM

@tisnik tisnik merged commit f1946cf into lightspeed-core:main May 6, 2026
31 checks passed
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