Conversation
…-stt-new' into feature/unified-api-stt-new merge upstream
…ssistant_id handling
📝 WalkthroughWalkthroughThis PR introduces comprehensive LLM call tracking with database persistence, adds Google AI provider support for speech-to-text, implements partial configuration version updates with field inheritance, adds multimodal input handling (text and audio), and expands the provider ecosystem with new CRUD operations and validation logic. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Route
participant Service as LLM Service
participant JobSvc as Job Executor
participant Provider as LLM Provider
participant InputRes as Input Resolver
participant DB as Database
Client->>API: POST /llm/complete
API->>Service: execute_llm_job(request)
Service->>DB: create_llm_call(request metadata)
DB-->>Service: llm_call_id
Service->>InputRes: resolve_input(query_input)
InputRes-->>Service: resolved_input (text/file path)
Service->>Provider: execute(config, resolved_input)
Provider-->>Service: LLMCallResponse, provider_response_id
Service->>DB: update_llm_call_response(response data)
DB-->>Service: updated LlmCall
Service-->>API: completion result
API-->>Client: response with conversation_id
sequenceDiagram
actor User
participant API as Config API
participant CRUD as Config CRUD
participant DB as Database
User->>API: POST /config/{config_id}/versions (partial update)
API->>CRUD: create_from_partial_or_raise(ConfigVersionCreatePartial)
CRUD->>DB: get latest version
DB-->>CRUD: latest ConfigVersion
CRUD->>CRUD: deep_merge(latest.blob, partial.blob)
CRUD->>CRUD: validate_config_type_unchanged(merged)
CRUD->>CRUD: validate_immutable_fields(provider)
CRUD->>DB: create new ConfigVersion (incremented)
DB-->>CRUD: new version id
CRUD-->>API: ConfigVersionPublic
API-->>User: 201 Created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
backend/app/cli/bench/commands.py (3)
134-134: 🛠️ Refactor suggestion | 🟠 MajorAdd missing return type hint.
The function returns a filename string but is missing the return type annotation. As per coding guidelines, all function return values should have type hints.
✨ Proposed fix to add return type hint
-def output_csv(items: List[BenchItem]): +def output_csv(items: List[BenchItem]) -> str:
169-175: 🛠️ Refactor suggestion | 🟠 MajorUse
Callablefrom typing instead ofcallable.The
build_payloadparameter uses the lowercasecallablebuiltin. For proper type hinting, useCallablefrom thetypingmodule to enable better type checking and IDE support. As per coding guidelines, all function parameters should have proper type hints.🔧 Proposed fix to use proper Callable type hint
def send_benchmark_request( prompt: str, i: int, total: int, endpoint: str, - build_payload: callable, + build_payload: Callable[[str], dict], ) -> BenchItem:Note: Ensure
Callableis imported at the top:from typing import List, Protocol, Callable
269-269: 🛠️ Refactor suggestion | 🟠 MajorAdd missing return type hint.
The function is missing a return type annotation. As per coding guidelines, all function return values should have type hints.
✨ Proposed fix to add return type hint
-def print_statistics(stats: dict): +def print_statistics(stats: dict) -> None:backend/app/alembic/versions/043_extend_collection_table_for_provider_.py (1)
29-81:⚠️ Potential issue | 🟡 MinorAdd return type hints to Alembic migration entry points.
upgrade()anddowngrade()functions must declare-> Noneper the type-hints guideline that applies to all Python files.Suggested fix
-def upgrade(): +def upgrade() -> None: -def downgrade(): +def downgrade() -> None:backend/app/api/routes/config/version.py (1)
25-130:⚠️ Potential issue | 🟡 MinorAdd explicit APIResponse return types on endpoints.
Return types are missing on the route handlers.
🛠️ Suggested fix
def create_version( config_id: UUID, version_create: ConfigVersionCreatePartial, current_user: AuthContextDep, session: SessionDep, -): +) -> APIResponse[ConfigVersionPublic]: @@ def list_versions( config_id: UUID, current_user: AuthContextDep, session: SessionDep, skip: int = Query(0, ge=0, description="Number of records to skip"), limit: int = Query(100, ge=1, le=100, description="Maximum records to return"), -): +) -> APIResponse[list[ConfigVersionItems]]: @@ def get_version( config_id: UUID, current_user: AuthContextDep, session: SessionDep, version_number: int = Path( ..., ge=1, description="The version number of the config" ), -): +) -> APIResponse[ConfigVersionPublic]: @@ def delete_version( config_id: UUID, current_user: AuthContextDep, session: SessionDep, version_number: int = Path( ..., ge=1, description="The version number of the config" ), -): +) -> APIResponse[Message]:As per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code.backend/app/services/llm/providers/oai.py (1)
22-31:⚠️ Potential issue | 🟡 MinorAdd explicit return type for init.
🛠️ Suggested fix
- def __init__(self, client: OpenAI): + def __init__(self, client: OpenAI) -> None:As per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code.backend/app/tests/api/routes/configs/test_version.py (1)
17-55:⚠️ Potential issue | 🟡 MinorAdd explicit
-> Nonereturn types on test functions.Several tests in this file are missing return type annotations. Please apply across the file.
🛠️ Suggested fix (apply pattern throughout)
def test_create_version_success( db: Session, client: TestClient, user_api_key: TestAuthContext, -) -> None: +) -> None: @@ def test_create_version_nonexistent_config( db: Session, client: TestClient, user_api_key: TestAuthContext, -) -> None: +) -> None:As per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code.backend/app/tests/services/llm/providers/test_openai.py (1)
5-60:⚠️ Potential issue | 🟡 MinorAdd parameter and return type hints for fixtures/tests.
Many fixtures and test methods are missing annotations. Please add them consistently across the file.
🛠️ Suggested fix (apply pattern throughout)
-from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, patch +from typing import Self @@ - def mock_client(self): + def mock_client(self: Self) -> MagicMock: """Create a mock OpenAI client.""" return MagicMock() @@ - def provider(self, mock_client): + def provider(self: Self, mock_client: MagicMock) -> OpenAIProvider: """Create an OpenAIProvider instance with mock client.""" return OpenAIProvider(client=mock_client) @@ - def completion_config(self): + def completion_config(self: Self) -> NativeCompletionConfig: """Create a basic completion config.""" return NativeCompletionConfig( @@ - def query_params(self): + def query_params(self: Self) -> QueryParams: """Create basic query parameters.""" return QueryParams(input="Test query") @@ - def test_execute_success_without_conversation( - self, provider, mock_client, completion_config, query_params - ): + def test_execute_success_without_conversation( + self: Self, + provider: OpenAIProvider, + mock_client: MagicMock, + completion_config: NativeCompletionConfig, + query_params: QueryParams, + ) -> None:As per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code.backend/app/tests/services/llm/test_mappers.py (1)
23-496:⚠️ Potential issue | 🟡 MinorAdd return type hints to test methods.
All 27 test methods in this file lack explicit return type annotations (
-> None), which violates the typing guideline. Add return type hints to each test method.Example
- def test_basic_model_mapping(self): + def test_basic_model_mapping(self) -> None:Per coding guideline
**/*.py: Always add type hints to all function parameters and return values in Python code.
🤖 Fix all issues with AI agents
In `@backend/app/alembic/versions/042_add_llm_call_table.py`:
- Around line 20-185: The Alembic entry points upgrade() and downgrade() lack
explicit return type hints; update their signatures to include return type None
(e.g., def upgrade() -> None: and def downgrade() -> None:) so both functions
have explicit return type annotations per the project's typing guidelines;
ensure no runtime behavior changes and keep doc/comments intact.
In `@backend/app/alembic/versions/044_remove_enum_checks_llm_call_provider.py`:
- Around line 20-41: The migration functions upgrade() and downgrade() lack
explicit type hints; update both function signatures to include return type
annotations (e.g., def upgrade() -> None: and def downgrade() -> None:) in this
file and apply the same change consistently across all migration files in
backend/app/alembic/versions/ (all 44 migration functions) so every migration
function has an explicit return type of None.
In `@backend/app/crud/config/version.py`:
- Around line 192-209: The current _validate_immutable_fields only enforces
immutability of the completion.type but allows completion.provider to be changed
on partial updates; update _validate_immutable_fields to also extract
existing_completion.get("provider") and merged_completion.get("provider") and
raise an HTTPException (400) with a clear message if they differ, ensuring
provider cannot be switched during create_from_partial_or_raise-based partial
updates (or explicitly document otherwise); reference the existing function
_validate_immutable_fields and the variables existing_completion,
merged_completion, existing_type/merged_type when making the change.
In `@backend/app/crud/evaluations/core.py`:
- Around line 445-450: The access to config.completion.params assumes it's a
dict and uses .get, which will raise AttributeError if params is None or not a
mapping; update the code in the block that reads config.completion.params
(around where model is fetched for eval_run.id) to first validate the type
(e.g., params = config.completion.params; if not isinstance(params, Mapping):
raise ValueError(f"Config for evaluation {eval_run.id} has invalid 'params'
(expected mapping)"); then use model = params.get("model") and keep the existing
ValueError if model is missing; import Mapping from collections.abc if needed.
In `@backend/app/models/llm/request.py`:
- Around line 4-11: The file imports Field and SQLModel twice which causes
duplicate-import (F811); remove the redundant import by keeping a single import
of Field and SQLModel and consolidating any additional names from the second
import (e.g., Index, text) into that one statement so that only one "from
sqlmodel import Field, SQLModel, Index, text" remains; update import lines in
request.py where Field and SQLModel are referenced to use the single
consolidated import.
- Around line 469-473: The updated_at Field only sets a default on insert and
won't change on updates; modify the updated_at declaration (the Field named
updated_at) to include an on-update behavior by adding SQLAlchemy's onupdate
(e.g., via sa_column_kwargs add "onupdate": now) or replace sa_column_kwargs
with an explicit sa_column using Column(..., default=now, onupdate=now,
nullable=False, comment=...) so the timestamp is refreshed on record
modifications.
In `@backend/app/services/llm/input_resolver.py`:
- Around line 103-125: The current implementation writes response.content into a
temp file causing potential OOM; modify the audio download in the
resolve_audio_url flow to use requests.get(..., stream=True) and write
response.iter_content(chunk_size=8192) to the NamedTemporaryFile while tracking
cumulative bytes, enforcing a configurable max_size (use a constant or passed-in
param), aborting, deleting the partial temp file and returning an error if
exceeded (also check Content-Length header up front to fail fast if > max_size),
and keep using get_file_extension to determine suffix and logger.info to log
temp_path on success.
In `@backend/app/services/llm/jobs.py`:
- Line 205: Remove the unused variable user_sent_config_provider from the
affected scope in jobs.py: locate the assignment "user_sent_config_provider =
\"\"" and delete it (and any dead/commented references) so the function/class no
longer contains an unused local causing ruff F841; ensure no other code relies
on that symbol before removal.
- Around line 287-313: The code calls cleanup_temp_file(resolved_input) based on
the equality check resolved_input != request.query.input which can mistakenly
delete non-temp text when resolved_input is a string; change the cleanup logic
to only remove temp files for audio inputs by having resolve_input return an
explicit cleanup flag (e.g., resolved_input, resolve_error, needs_cleanup) or by
returning a distinct AudioInput type, then in jobs.py use that flag/type to
guard cleanup (update the call sites around resolve_input, and replace the
equality check with the new needs_cleanup or isinstance check) so
cleanup_temp_file is invoked only for actual temporary audio files.
In `@backend/app/services/llm/mappers.py`:
- Around line 101-102: The google mapper currently always sets
google_params["model"] = kaapi_params.get("model"), producing {"model": None}
when missing; change it to match the OpenAI mapper behavior by first reading
model = kaapi_params.get("model") (or reuse the existing model variable if
present) and only assign google_params["model"] when model is truthy/non-None,
so downstream consumers don't receive a null model value; look for the google
mapping block that references google_params and kaapi_params and apply this
conditional assignment.
In `@backend/app/services/llm/providers/gai.py`:
- Around line 114-116: The log call in GoogleAIProvider._execute_stt uses an
unnecessary f-string with no placeholders; replace logger.warning(f"...") with a
normal string logger.warning("...") for the message
"[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros"
to satisfy Ruff F541 and remove the redundant f-string syntax.
In `@backend/app/services/llm/providers/registry.py`:
- Around line 86-94: Update the logger.error call to prefix the message with the
current function name in square brackets; specifically, change the statement
that logs initialization failures for provider_type (where
provider_class.create_client(...) is called) to include a function-name prefix
(e.g., logger.error(f"[<function_name>] Failed to initialize {provider_type}
client: {e}", exc_info=True)), keeping the rest of the message and exc_info=True
intact; ensure you update the log invocation near provider_class, create_client
and provider_type so all registry initialization errors are consistently
prefixed.
In `@backend/app/tests/api/routes/configs/test_version.py`:
- Around line 734-738: The test function
test_create_config_with_kaapi_provider_success currently declares a db fixture
parameter that isn't used; either remove the db parameter or rename it to _db to
silence the linter (Ruff). Update the function signature in the test to drop db
if not needed, or change db to _db (e.g., def
test_create_config_with_kaapi_provider_success(_db, client, user_api_key):) so
the unused fixture is explicitly ignored.
In `@backend/app/tests/crud/test_llm.py`:
- Around line 13-46: The test_job fixture lacks a return type hint and the Job
model isn't imported; update the top imports to include Job from app.models and
add a return type annotation to the test_job fixture signature (def test_job(db:
Session) -> Job:) so the fixture explicitly returns a Job instance created via
JobCrud.create(job_type=JobType.LLM_API, trace_id="test-llm-trace"); ensure Job
is referenced in the import list alongside Project and Organization and adjust
any typing imports if needed.
In `@backend/app/tests/utils/test_data.py`:
- Around line 321-374: create_test_version currently builds a new ConfigBlob
with hard-coded params for all completion types, causing validation errors for
stt/tts; update the logic that constructs config_blob (reference ConfigBlob,
NativeCompletionConfig, KaapiCompletionConfig, completion_config and
config_type) to preserve the existing completion_config["params"] by default and
only override or inject defaults when config_type == "text" (or when a specific
param is missing), and for non-text types provide type-specific defaults or keep
originals (e.g., ensure stt includes "instructions" and tts includes "voice" and
"language") so validators won't fail.
🧹 Nitpick comments (14)
backend/app/cli/bench/commands.py (1)
125-125: Add function name prefix to log message.The log message should be prefixed with the function name in square brackets per coding guidelines:
logger.info(f"[function_name] Message").📝 Proposed fix to add function prefix
- logging.warning(f"No pricing found for model '{model}'. Returning cost = 0.") + logging.warning(f"[estimate_cost] No pricing found for model '{model}'. Returning cost = 0.")backend/app/services/doctransform/zerox_transformer.py (2)
17-18: Consider adding return type hint to__init__method.The
__init__method is missing a return type hint. As per coding guidelines, all functions should have type hints for parameters and return values.✨ Proposed addition
- def __init__(self, model: str = "gpt-4o"): + def __init__(self, model: str = "gpt-4o") -> None: self.model = model
21-21: Consider adding consistent function name prefixes to log messages.Some log messages are missing the function name prefix, while line 68 correctly uses
[ZeroxTransformer.transform]. For consistency and easier log filtering, consider adding the prefix to all log messages in this method. As per coding guidelines, log messages should be prefixed with the function name in square brackets.🔧 Proposed changes for consistency
- logger.info(f"ZeroxTransformer Started: (model={self.model})") + logger.info(f"[transform] ZeroxTransformer Started: (model={self.model})")logger.error( - f"ZeroxTransformer timed out for {input_path} (model={self.model})" + f"[transform] ZeroxTransformer timed out for {input_path} (model={self.model})" )logger.error( - f"ZeroxTransformer failed for {input_path}: {e}\n" + f"[transform] ZeroxTransformer failed for {input_path}: {e}\n" "This may be due to a missing Poppler installation or a corrupt PDF file.", exc_info=True, )Also applies to: 37-39, 44-48
backend/app/tests/crud/config/test_config.py (1)
19-31: Consider moving fixture construction to a test factory helper.The
example_config_blobfixture still constructsConfigBlobinline; please align with the factory pattern used acrossbackend/app/tests/by moving this setup into a factory helper (e.g., inapp.tests.utils.test_data).As per coding guidelines: Use factory pattern for test fixtures in
backend/app/tests/.backend/app/services/llm/input_resolver.py (1)
66-84: Validate base64 payloads and cap decoded size.Decoding unbounded input risks memory abuse;
b64decodealso accepts non‑base64 characters unlessvalidate=True.🔧 Suggested fix (validate + size cap)
+MAX_AUDIO_BYTES = 25 * 1024 * 1024 # consider moving to settings ... - audio_bytes = base64.b64decode(data) + audio_bytes = base64.b64decode(data, validate=True) + if len(audio_bytes) > MAX_AUDIO_BYTES: + return "", "Audio payload exceeds max size"backend/app/tests/crud/config/test_version.py (1)
17-28: Convertexample_config_blobto a factory fixture and add return types.Suggested refactor
+from typing import Callable @@ `@pytest.fixture` -def example_config_blob(): - return ConfigBlob( - completion=NativeCompletionConfig( - provider="openai-native", - type="text", - params={ - "model": "gpt-4", - "temperature": 0.8, - "max_tokens": 1500, - }, - ) - ) +def example_config_blob() -> Callable[[], ConfigBlob]: + def _factory() -> ConfigBlob: + return ConfigBlob( + completion=NativeCompletionConfig( + provider="openai-native", + type="text", + params={ + "model": "gpt-4", + "temperature": 0.8, + "max_tokens": 1500, + }, + ) + ) + return _factoryAs per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code;backend/app/tests/**/*.py: Use factory pattern for test fixtures in backend/app/tests/.backend/app/services/llm/providers/registry.py (1)
68-72: Remove duplicated credential_provider assignment.The same assignment appears twice; keeping a single copy improves clarity.
♻️ Suggested refactor
- # e.g., "openai-native" → "openai", "claude-native" → "claude" - credential_provider = provider_type.replace("-native", "")backend/app/crud/config/version.py (1)
113-116: Align log prefixes with the function name.New logs use
[ConfigVersionCrud.*]instead of the function name (e.g.,[create_from_partial_or_raise],[_validate_config_type_unchanged]). Please update to match the convention.
As per coding guidelines: Prefix all log messages with the function name in square brackets:logger.info(f"[function_name] Message ...").Also applies to: 136-139, 145-147, 324-337
backend/app/tests/services/llm/test_jobs.py (2)
36-885: Add type hints on fixtures/helpers/test methods.Fixtures and tests here omit return annotations (and in some helpers, parameter types), which conflicts with the typing requirement. Please add return types (and parameter hints where needed) throughout.
✍️ Example annotations
- `@pytest.fixture` - def llm_call_request(self): + `@pytest.fixture` + def llm_call_request(self) -> LLMCallRequest: @@ - def test_start_job_success(self, db: Session, llm_call_request: LLMCallRequest): + def test_start_job_success( + self, db: Session, llm_call_request: LLMCallRequest + ) -> None:As per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code.
36-279: Prefer factory-style fixtures for mutable inputs.Fixtures like
request_datareturn mutable objects that tests modify. A factory fixture keeps mutation isolated and aligns with the test-fixture guideline.
As per coding guidelines:backend/app/tests/**/*.py: Use factory pattern for test fixtures inbackend/app/tests/.backend/app/tests/services/llm/providers/test_gai.py (2)
52-249: Add return type hints to fixtures/tests.Fixtures and test methods are missing explicit return annotations; please add
-> None/return types to comply with typing guidelines.✍️ Example
- `@pytest.fixture` - def mock_client(self): + `@pytest.fixture` + def mock_client(self) -> MagicMock: @@ - def test_stt_success_with_auto_language( - self, provider, mock_client, stt_config, query_params - ): + def test_stt_success_with_auto_language( + self, provider, mock_client, stt_config, query_params + ) -> None:As per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code.
52-81: Use factory-style fixtures for configurability.Fixtures like
stt_configandquery_paramsreturn concrete objects; consider factory fixtures so each test can customize safely while following the fixture guideline.
As per coding guidelines:backend/app/tests/**/*.py: Use factory pattern for test fixtures inbackend/app/tests/.backend/app/services/llm/providers/gai.py (1)
22-29: Add return type annotation to__init__.Please add
-> Noneto comply with the type-hint requirement.✍️ Example
- def __init__(self, client: genai.Client): + def __init__(self, client: genai.Client) -> None:As per coding guidelines:
**/*.py: Always add type hints to all function parameters and return values in Python code.backend/app/models/llm/request.py (1)
63-63: Consider using modern union syntaxX | Yfor type aliases.Ruff suggests using
X | Yinstead ofUnion[X, Y]for Python 3.10+ type annotations. While functional, updating to modern syntax improves consistency.♻️ Proposed refactor
-KaapiLLMParams = Union[TextLLMParams, STTLLMParams, TTSLLMParams] +KaapiLLMParams = TextLLMParams | STTLLMParams | TTSLLMParams-QueryInput = Annotated[ - Union[TextInput, AudioBase64Input, AudioUrlInput], - Field(discriminator="type"), -] +QueryInput = Annotated[ + TextInput | AudioBase64Input | AudioUrlInput, + Field(discriminator="type"), +]Also applies to: 87-90
| def upgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.create_table( | ||
| "llm_call", | ||
| sa.Column( | ||
| "id", | ||
| sa.Uuid(), | ||
| nullable=False, | ||
| comment="Unique identifier for the LLM call record", | ||
| ), | ||
| sa.Column( | ||
| "job_id", | ||
| sa.Uuid(), | ||
| nullable=False, | ||
| comment="Reference to the parent job (status tracked in job table)", | ||
| ), | ||
| sa.Column( | ||
| "project_id", | ||
| sa.Integer(), | ||
| nullable=False, | ||
| comment="Reference to the project this LLM call belongs to", | ||
| ), | ||
| sa.Column( | ||
| "organization_id", | ||
| sa.Integer(), | ||
| nullable=False, | ||
| comment="Reference to the organization this LLM call belongs to", | ||
| ), | ||
| sa.Column( | ||
| "input", | ||
| sqlmodel.sql.sqltypes.AutoString(), | ||
| nullable=False, | ||
| comment="User input - text string, binary data, or file path for multimodal", | ||
| ), | ||
| sa.Column( | ||
| "input_type", | ||
| sa.String(), | ||
| nullable=False, | ||
| comment="Input type: text, audio, image", | ||
| ), | ||
| sa.Column( | ||
| "output_type", | ||
| sa.String(), | ||
| nullable=True, | ||
| comment="Expected output type: text, audio, image", | ||
| ), | ||
| sa.Column( | ||
| "provider", | ||
| sa.String(), | ||
| nullable=False, | ||
| comment="AI provider: openai, google, anthropic", | ||
| ), | ||
| sa.Column( | ||
| "model", | ||
| sqlmodel.sql.sqltypes.AutoString(), | ||
| nullable=False, | ||
| comment="Specific model used e.g. 'gpt-4o', 'gemini-2.5-pro'", | ||
| ), | ||
| sa.Column( | ||
| "provider_response_id", | ||
| sqlmodel.sql.sqltypes.AutoString(), | ||
| nullable=True, | ||
| comment="Original response ID from the provider (e.g., OpenAI's response ID)", | ||
| ), | ||
| sa.Column( | ||
| "content", | ||
| postgresql.JSONB(astext_type=sa.Text()), | ||
| nullable=True, | ||
| comment="Response content: {text: '...'}, {audio_bytes: '...'}, or {image: '...'}", | ||
| ), | ||
| sa.Column( | ||
| "usage", | ||
| postgresql.JSONB(astext_type=sa.Text()), | ||
| nullable=True, | ||
| comment="Token usage: {input_tokens, output_tokens, reasoning_tokens}", | ||
| ), | ||
| sa.Column( | ||
| "conversation_id", | ||
| sqlmodel.sql.sqltypes.AutoString(), | ||
| nullable=True, | ||
| comment="Identifier linking this response to its conversation thread", | ||
| ), | ||
| sa.Column( | ||
| "auto_create", | ||
| sa.Boolean(), | ||
| nullable=True, | ||
| comment="Whether to auto-create conversation if conversation_id doesn't exist (OpenAI specific)", | ||
| ), | ||
| sa.Column( | ||
| "config", | ||
| postgresql.JSONB(astext_type=sa.Text()), | ||
| nullable=True, | ||
| comment="Configuration: {config_id, config_version} for stored config OR {config_blob} for ad-hoc config", | ||
| ), | ||
| sa.Column( | ||
| "created_at", | ||
| sa.DateTime(), | ||
| nullable=False, | ||
| comment="Timestamp when the LLM call was created", | ||
| ), | ||
| sa.Column( | ||
| "updated_at", | ||
| sa.DateTime(), | ||
| nullable=False, | ||
| comment="Timestamp when the LLM call was last updated", | ||
| ), | ||
| sa.Column( | ||
| "deleted_at", | ||
| sa.DateTime(), | ||
| nullable=True, | ||
| comment="Timestamp when the record was soft-deleted", | ||
| ), | ||
| sa.ForeignKeyConstraint(["job_id"], ["job.id"], ondelete="CASCADE"), | ||
| sa.ForeignKeyConstraint( | ||
| ["organization_id"], ["organization.id"], ondelete="CASCADE" | ||
| ), | ||
| sa.ForeignKeyConstraint(["project_id"], ["project.id"], ondelete="CASCADE"), | ||
| sa.PrimaryKeyConstraint("id"), | ||
| ) | ||
| op.create_index( | ||
| "idx_llm_call_conversation_id", | ||
| "llm_call", | ||
| ["conversation_id"], | ||
| unique=False, | ||
| postgresql_where=sa.text("conversation_id IS NOT NULL AND deleted_at IS NULL"), | ||
| ) | ||
| op.create_index( | ||
| "idx_llm_call_job_id", | ||
| "llm_call", | ||
| ["job_id"], | ||
| unique=False, | ||
| postgresql_where=sa.text("deleted_at IS NULL"), | ||
| ) | ||
| op.alter_column( | ||
| "collection", | ||
| "llm_service_name", | ||
| existing_type=sa.VARCHAR(), | ||
| comment="Name of the LLM service", | ||
| existing_comment="Name of the LLM service provider", | ||
| existing_nullable=False, | ||
| ) | ||
| # ### end Alembic commands ### | ||
|
|
||
|
|
||
| def downgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.alter_column( | ||
| "collection", | ||
| "llm_service_name", | ||
| existing_type=sa.VARCHAR(), | ||
| comment="Name of the LLM service provider", | ||
| existing_comment="Name of the LLM service", | ||
| existing_nullable=False, | ||
| ) | ||
| op.drop_index( | ||
| "idx_llm_call_job_id", | ||
| table_name="llm_call", | ||
| postgresql_where=sa.text("deleted_at IS NULL"), | ||
| ) | ||
| op.drop_index( | ||
| "idx_llm_call_conversation_id", | ||
| table_name="llm_call", | ||
| postgresql_where=sa.text("conversation_id IS NOT NULL AND deleted_at IS NULL"), | ||
| ) | ||
| op.drop_table("llm_call") | ||
| # ### end Alembic commands ### |
There was a problem hiding this comment.
Add return type hints for Alembic entry points.
Both Alembic entry points are missing explicit return types.
🛠️ Suggested fix
-def upgrade():
+def upgrade() -> None:
@@
-def downgrade():
+def downgrade() -> None:As per coding guidelines: **/*.py: Always add type hints to all function parameters and return values in Python code.
🤖 Prompt for AI Agents
In `@backend/app/alembic/versions/042_add_llm_call_table.py` around lines 20 -
185, The Alembic entry points upgrade() and downgrade() lack explicit return
type hints; update their signatures to include return type None (e.g., def
upgrade() -> None: and def downgrade() -> None:) so both functions have explicit
return type annotations per the project's typing guidelines; ensure no runtime
behavior changes and keep doc/comments intact.
| def upgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.alter_column( | ||
| "llm_call", | ||
| "provider", | ||
| existing_type=sa.VARCHAR(), | ||
| comment="AI provider as sent by user (e.g openai, -native, google)", | ||
| existing_comment="AI provider: openai, google, anthropic", | ||
| existing_nullable=False, | ||
| ) | ||
| # ### end Alembic commands ### | ||
|
|
||
|
|
||
| def downgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.alter_column( | ||
| "llm_call", | ||
| "provider", | ||
| existing_type=sa.VARCHAR(), | ||
| comment="AI provider: openai, google, anthropic", | ||
| existing_comment="AI provider as sent by user (e.g openai, -native, google)", | ||
| existing_nullable=False, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "044_remove_enum_checks_llm_call_provider.py" -type fRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 149
🏁 Script executed:
# Check if the alembic/versions directory exists and list migration files
ls -la backend/app/alembic/versions/ 2>/dev/null | head -20Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 1587
🏁 Script executed:
# Check content of the specific file if it exists
if [ -f "backend/app/alembic/versions/044_remove_enum_checks_llm_call_provider.py" ]; then
cat -n "backend/app/alembic/versions/044_remove_enum_checks_llm_call_provider.py"
fiRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 1486
🏁 Script executed:
# Look at a few other migration files to understand the pattern
fd "^[0-9]+_" backend/app/alembic/versions/ --type f | head -5 | xargs -I {} bash -c 'echo "=== {} ===" && head -50 {}'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 7251
🏁 Script executed:
# Search all migration files to see if any have type hints on upgrade/downgrade functions
rg "def upgrade\(\)" backend/app/alembic/versions/ -A 1 | head -40Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 2902
🏁 Script executed:
# Check if there's any Alembic configuration or documentation about type hints
fd "alembic.ini|env.py|script.py.mako" backend/app/alembic/ -type fRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 245
🏁 Script executed:
# Search for any existing type hints in any migration files
rg "def (upgrade|downgrade)\(\)" backend/app/alembic/versions/ --countRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 3124
🏁 Script executed:
# Get total count of migration files and check if any use type hints
find backend/app/alembic/versions -name "*.py" -type f ! -name ".keep" -exec grep -l "def upgrade" {} \; | wc -lRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 77
🏁 Script executed:
# Check for any migration files that actually DO have type hints
rg "def (upgrade|downgrade)\(\)" backend/app/alembic/versions/ -A 0 | grep -E "\->\s*(None|Any)" | wc -lRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 76
Add explicit return type hints to migration functions.
Suggested fix
-def upgrade():
+def upgrade() -> None:
@@
-def downgrade():
+def downgrade() -> None:Per coding guideline **/*.py: Always add type hints to all function parameters and return values in Python code.
Note: This applies to all 44 migration functions in backend/app/alembic/versions/ and should be systematically addressed across all migration files.
🤖 Prompt for AI Agents
In `@backend/app/alembic/versions/044_remove_enum_checks_llm_call_provider.py`
around lines 20 - 41, The migration functions upgrade() and downgrade() lack
explicit type hints; update both function signatures to include return type
annotations (e.g., def upgrade() -> None: and def downgrade() -> None:) in this
file and apply the same change consistently across all migration files in
backend/app/alembic/versions/ (all 44 migration functions) so every migration
function has an explicit return type of None.
| def _validate_immutable_fields( | ||
| self, existing: dict[str, Any], merged: dict[str, Any] | ||
| ) -> None: | ||
| """ | ||
| Validate that immutable fields (type) haven't been changed. | ||
| Provider and model can change between versions. | ||
| """ | ||
| existing_completion = existing.get("completion", {}) | ||
| merged_completion = merged.get("completion", {}) | ||
|
|
||
| existing_type = existing_completion.get("type") | ||
| merged_type = merged_completion.get("type") | ||
|
|
||
| if existing_type != merged_type: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"Cannot change config type from '{existing_type}' to '{merged_type}'. Type is immutable.", | ||
| ) |
There was a problem hiding this comment.
Prevent provider changes in partial updates.
create_from_partial_or_raise says provider is inherited, but _validate_immutable_fields only checks type. This allows provider switches via partial updates, which can produce incompatible configs/credentials. Enforce provider immutability (or explicitly document that it’s allowed).
🔒 Suggested fix
existing_completion = existing.get("completion", {})
merged_completion = merged.get("completion", {})
+ existing_provider = existing_completion.get("provider")
+ merged_provider = merged_completion.get("provider")
existing_type = existing_completion.get("type")
merged_type = merged_completion.get("type")
+ if existing_provider != merged_provider:
+ raise HTTPException(
+ status_code=400,
+ detail=(
+ f"Cannot change provider from '{existing_provider}' to "
+ f"'{merged_provider}'. Provider is immutable."
+ ),
+ )
+
if existing_type != merged_type:
raise HTTPException(
status_code=400,
detail=f"Cannot change config type from '{existing_type}' to '{merged_type}'. Type is immutable.",
)🤖 Prompt for AI Agents
In `@backend/app/crud/config/version.py` around lines 192 - 209, The current
_validate_immutable_fields only enforces immutability of the completion.type but
allows completion.provider to be changed on partial updates; update
_validate_immutable_fields to also extract existing_completion.get("provider")
and merged_completion.get("provider") and raise an HTTPException (400) with a
clear message if they differ, ensuring provider cannot be switched during
create_from_partial_or_raise-based partial updates (or explicitly document
otherwise); reference the existing function _validate_immutable_fields and the
variables existing_completion, merged_completion, existing_type/merged_type when
making the change.
| # params is a dict, not a Pydantic model, so use dict access | ||
| model = config.completion.params.get("model") | ||
| if not model: | ||
| raise ValueError( | ||
| f"Config for evaluation {eval_run.id} does not contain a 'model' parameter" | ||
| ) |
There was a problem hiding this comment.
Guard against non-dict params to avoid AttributeError.
If config.completion.params is None or not a mapping, .get will throw an AttributeError instead of the intended ValueError. Consider validating type first for a deterministic error path.
Suggested fix
- # params is a dict, not a Pydantic model, so use dict access
- model = config.completion.params.get("model")
+ # params is a dict, not a Pydantic model, so use dict access
+ params = config.completion.params
+ if not isinstance(params, dict):
+ raise ValueError(
+ f"Config for evaluation {eval_run.id} has invalid params; expected dict"
+ )
+ model = params.get("model")
if not model:
raise ValueError(
f"Config for evaluation {eval_run.id} does not contain a 'model' parameter"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # params is a dict, not a Pydantic model, so use dict access | |
| model = config.completion.params.get("model") | |
| if not model: | |
| raise ValueError( | |
| f"Config for evaluation {eval_run.id} does not contain a 'model' parameter" | |
| ) | |
| # params is a dict, not a Pydantic model, so use dict access | |
| params = config.completion.params | |
| if not isinstance(params, dict): | |
| raise ValueError( | |
| f"Config for evaluation {eval_run.id} has invalid params; expected dict" | |
| ) | |
| model = params.get("model") | |
| if not model: | |
| raise ValueError( | |
| f"Config for evaluation {eval_run.id} does not contain a 'model' parameter" | |
| ) |
🤖 Prompt for AI Agents
In `@backend/app/crud/evaluations/core.py` around lines 445 - 450, The access to
config.completion.params assumes it's a dict and uses .get, which will raise
AttributeError if params is None or not a mapping; update the code in the block
that reads config.completion.params (around where model is fetched for
eval_run.id) to first validate the type (e.g., params =
config.completion.params; if not isinstance(params, Mapping): raise
ValueError(f"Config for evaluation {eval_run.id} has invalid 'params' (expected
mapping)"); then use model = params.get("model") and keep the existing
ValueError if model is missing; import Mapping from collections.abc if needed.
| from sqlmodel import Field, SQLModel | ||
| from pydantic import Discriminator, model_validator, HttpUrl | ||
| from datetime import datetime | ||
| from app.core.util import now | ||
|
|
||
| import sqlalchemy as sa | ||
| from sqlalchemy.dialects.postgresql import JSONB | ||
| from sqlmodel import Field, SQLModel, Index, text |
There was a problem hiding this comment.
Fix duplicate imports of Field and SQLModel.
Static analysis correctly identifies that Field and SQLModel are imported twice - first from sqlmodel on line 4, then again on line 11. This shadows the first import and triggers F811 errors.
🔧 Proposed fix
from uuid import UUID, uuid4
from sqlmodel import Field, SQLModel
from pydantic import Discriminator, model_validator, HttpUrl
from datetime import datetime
from app.core.util import now
import sqlalchemy as sa
from sqlalchemy.dialects.postgresql import JSONB
-from sqlmodel import Field, SQLModel, Index, text
+from sqlmodel import Index, text🧰 Tools
🪛 Ruff (0.14.14)
[error] 11-11: Redefinition of unused Field from line 4: Field redefined here
Remove definition: Field
(F811)
[error] 11-11: Redefinition of unused SQLModel from line 4: SQLModel redefined here
Remove definition: SQLModel
(F811)
🤖 Prompt for AI Agents
In `@backend/app/models/llm/request.py` around lines 4 - 11, The file imports
Field and SQLModel twice which causes duplicate-import (F811); remove the
redundant import by keeping a single import of Field and SQLModel and
consolidating any additional names from the second import (e.g., Index, text)
into that one statement so that only one "from sqlmodel import Field, SQLModel,
Index, text" remains; update import lines in request.py where Field and SQLModel
are referenced to use the single consolidated import.
| logger.warning( | ||
| f"[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n backend/app/services/llm/providers/gai.py | sed -n '110,120p'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 659
Drop redundant f-string prefix.
Ruff F541: the f-string has no placeholders.
- logger.warning(
- f"[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros"
- )
+ logger.warning(
+ "[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.warning( | |
| f"[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros" | |
| ) | |
| logger.warning( | |
| "[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros" | |
| ) |
🧰 Tools
🪛 Ruff (0.14.14)
[error] 115-115: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In `@backend/app/services/llm/providers/gai.py` around lines 114 - 116, The log
call in GoogleAIProvider._execute_stt uses an unnecessary f-string with no
placeholders; replace logger.warning(f"...") with a normal string
logger.warning("...") for the message "[GoogleAIProvider._execute_stt] Response
missing usage_metadata, using zeros" to satisfy Ruff F541 and remove the
redundant f-string syntax.
| try: | ||
| client = provider_class.create_client(credentials=credentials) | ||
| return provider_class(client=client) | ||
| except ValueError: | ||
| # Re-raise ValueError for credential/configuration errors | ||
| raise | ||
| except Exception as e: | ||
| logger.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True) | ||
| raise RuntimeError(f"Could not connect to {provider_type} services.") |
There was a problem hiding this comment.
Prefix registry error logs with the function name.
The error log should include the function-name prefix.
🛠️ Suggested fix
- logger.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True)
+ logger.error(
+ f"[get_llm_provider] Failed to initialize {provider_type} client: {e}",
+ exc_info=True,
+ )As per coding guidelines: Prefix all log messages with the function name in square brackets: logger.info(f"[function_name] Message {mask_string(sensitive_value)}").
🤖 Prompt for AI Agents
In `@backend/app/services/llm/providers/registry.py` around lines 86 - 94, Update
the logger.error call to prefix the message with the current function name in
square brackets; specifically, change the statement that logs initialization
failures for provider_type (where provider_class.create_client(...) is called)
to include a function-name prefix (e.g., logger.error(f"[<function_name>] Failed
to initialize {provider_type} client: {e}", exc_info=True)), keeping the rest of
the message and exc_info=True intact; ensure you update the log invocation near
provider_class, create_client and provider_type so all registry initialization
errors are consistently prefixed.
| def test_create_config_with_kaapi_provider_success( | ||
| db: Session, | ||
| client: TestClient, | ||
| user_api_key: TestAuthContext, | ||
| ) -> None: |
There was a problem hiding this comment.
Address the unused db fixture argument.
Ruff reports db as unused in this test. Rename it to _db or remove it.
🛠️ Suggested fix
def test_create_config_with_kaapi_provider_success(
- db: Session,
+ _db: Session,
client: TestClient,
user_api_key: TestAuthContext,
) -> None:🧰 Tools
🪛 Ruff (0.14.14)
[warning] 735-735: Unused function argument: db
(ARG001)
🤖 Prompt for AI Agents
In `@backend/app/tests/api/routes/configs/test_version.py` around lines 734 - 738,
The test function test_create_config_with_kaapi_provider_success currently
declares a db fixture parameter that isn't used; either remove the db parameter
or rename it to _db to silence the linter (Ruff). Update the function signature
in the test to drop db if not needed, or change db to _db (e.g., def
test_create_config_with_kaapi_provider_success(_db, client, user_api_key):) so
the unused fixture is explicitly ignored.
| from app.models import JobType, Project, Organization | ||
| from app.models.llm import ( | ||
| ConfigBlob, | ||
| LLMCallRequest, | ||
| LlmCall, | ||
| QueryParams, | ||
| ) | ||
| from app.models.llm.request import ( | ||
| KaapiCompletionConfig, | ||
| LLMCallConfig, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def test_project(db: Session) -> Project: | ||
| """Get the first available test project.""" | ||
| project = db.exec(select(Project).limit(1)).first() | ||
| assert project is not None, "No test project found in seed data" | ||
| return project | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def test_organization(db: Session, test_project: Project) -> Organization: | ||
| """Get the organization for the test project.""" | ||
| org = db.get(Organization, test_project.organization_id) | ||
| assert org is not None, "No organization found for test project" | ||
| return org | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def test_job(db: Session): | ||
| """Create a test job for LLM call tests.""" | ||
| crud = JobCrud(db) | ||
| return crud.create(job_type=JobType.LLM_API, trace_id="test-llm-trace") |
There was a problem hiding this comment.
Add return type hint for the test_job fixture (and import Job).
🛠️ Suggested fix
-from app.models import JobType, Project, Organization
+from app.models import Job, JobType, Project, Organization
@@
-def test_job(db: Session):
+def test_job(db: Session) -> Job:
"""Create a test job for LLM call tests."""
crud = JobCrud(db)
return crud.create(job_type=JobType.LLM_API, trace_id="test-llm-trace")As per coding guidelines: **/*.py: Always add type hints to all function parameters and return values in Python code.
🤖 Prompt for AI Agents
In `@backend/app/tests/crud/test_llm.py` around lines 13 - 46, The test_job
fixture lacks a return type hint and the Job model isn't imported; update the
top imports to include Job from app.models and add a return type annotation to
the test_job fixture signature (def test_job(db: Session) -> Job:) so the
fixture explicitly returns a Job instance created via
JobCrud.create(job_type=JobType.LLM_API, trace_id="test-llm-trace"); ensure Job
is referenced in the import list alongside Project and Organization and adjust
any typing imports if needed.
| if config_blob is None: | ||
| config_blob = ConfigBlob( | ||
| completion=NativeCompletionConfig( | ||
| provider="openai-native", | ||
| params={ | ||
| "model": "gpt-4", | ||
| "temperature": 0.8, | ||
| "max_tokens": 1500, | ||
| }, | ||
| # Fetch the latest version to maintain type consistency | ||
| from sqlmodel import select, and_ | ||
| from app.models import ConfigVersion | ||
|
|
||
| stmt = ( | ||
| select(ConfigVersion) | ||
| .where( | ||
| and_( | ||
| ConfigVersion.config_id == config_id, | ||
| ConfigVersion.deleted_at.is_(None), | ||
| ) | ||
| ) | ||
| .order_by(ConfigVersion.version.desc()) | ||
| .limit(1) | ||
| ) | ||
| latest_version = db.exec(stmt).first() | ||
|
|
||
| if latest_version: | ||
| # Extract the type and provider from the latest version | ||
| completion_config = latest_version.config_blob.get("completion", {}) | ||
| config_type = completion_config.get("type") | ||
| provider = completion_config.get("provider", "openai-native") | ||
|
|
||
| # Create a new config_blob maintaining the same type and provider | ||
| if provider in ["openai-native", "google-native"]: | ||
| config_blob = ConfigBlob( | ||
| completion=NativeCompletionConfig( | ||
| provider=provider, | ||
| type=config_type, | ||
| params={ | ||
| "model": completion_config.get("params", {}).get( | ||
| "model", "gpt-4" | ||
| ), | ||
| "temperature": 0.8, | ||
| "max_tokens": 1500, | ||
| }, | ||
| ) | ||
| ) | ||
| else: | ||
| # For Kaapi providers (openai, google) | ||
| config_blob = ConfigBlob( | ||
| completion=KaapiCompletionConfig( | ||
| provider=provider, | ||
| type=config_type, | ||
| params={ | ||
| "model": completion_config.get("params", {}).get( | ||
| "model", "gpt-4" | ||
| ), | ||
| "temperature": 0.8, | ||
| }, | ||
| ) | ||
| ) | ||
| else: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the create_test_version function definition
rg -n "def create_test_version" -A 20 --type pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 1590
🏁 Script executed:
# Find all call sites of create_test_version
rg -n "create_test_version\(" -B 2 -A 2 --type pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 8105
🏁 Script executed:
# Look for config type definitions (stt, tts, text)
rg -n '"stt"|"tts"|"text"' -B 1 -A 1 --type py | head -100Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 6248
🏁 Script executed:
# Check for ConfigBlob, NativeCompletionConfig, KaapiCompletionConfig schema definitions
rg -n "class ConfigBlob|class NativeCompletionConfig|class KaapiCompletionConfig" -A 10 --type pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 2409
🏁 Script executed:
# Look for param validation or schema definitions
rg -n "params.*schema|schema.*params|Field.*params" -B 2 -A 2 --type py | head -80Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 511
🏁 Script executed:
# Get the complete create_test_version function
sed -n '306,400p' backend/app/tests/utils/test_data.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 3460
🏁 Script executed:
# Check if there are any tests creating stt or tts configs
rg -n 'create_test_config.*stt|create_test_config.*tts' -B 5 -A 5 --type pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 57
🏁 Script executed:
# Look for the stt config usage in test_mappers.py to understand if it ever calls create_test_version without explicit config_blob
sed -n '455,475p' backend/app/tests/services/llm/test_mappers.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 875
🏁 Script executed:
# Check if create_test_config creates different types
rg -n 'def create_test_config' -A 50 --type py | head -80Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 3876
🏁 Script executed:
# Look at the validation logic for ConfigBlob
rg -n 'validate|pydantic|validator' backend/app/models/llm/request.py -B 2 -A 2Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 1231
🏁 Script executed:
# Find the param schema definitions for stt and tts
rg -n 'class TextLLMParams|class STTLLMParams|class TTSLLMParams' -A 20 --type pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 3871
🏁 Script executed:
# Check if create_test_version is ever called after creating an stt/tts config
rg -B 10 'create_test_version' backend/app/tests/services/llm/test_mappers.py | grep -A 10 'type.*stt\|type.*tts'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 57
🏁 Script executed:
# Let's look at the test that creates stt config more closely
sed -n '455,490p' backend/app/tests/services/llm/test_mappers.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 1402
🏁 Script executed:
# Search for any tests that create stt/tts configs and then use create_test_version implicitly
rg -n 'type="stt"|type="tts"' -A 20 --type py | grep -B 20 'create_test_version'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 57
🏁 Script executed:
# Search for create_test_version calls that don't provide config_blob explicitly
# combined with configs that might be stt/tts
rg -n 'create_test_version\(' backend/app/tests --type py -A 5 | grep -v 'config_blob'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 9407
🏁 Script executed:
# Check if STTLLMParams and TTSLLMParams validation would fail with missing required fields
# Look at the validator logic more closely
sed -n '185,200p' backend/app/models/llm/request.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 513
🏁 Script executed:
# Check if there are any integration tests that actually test non-text config types
rg -n 'type.*=.*["\x27](stt|tts)["\x27]' backend/app/tests/crud --type py -B 5 -A 10Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 2031
🏁 Script executed:
# Let's check if stt_config_blob or tts_config_blob are used in tests that call create_test_version
rg -B 5 -A 15 'stt_config_blob|tts_config_blob' backend/app/tests/crud/test_llm.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 2582
🏁 Script executed:
# Check if there are any test configs created with stt or tts that later call create_test_version
rg -n 'create_test_config.*stt|create_test_config.*tts|create_test_config.*type.*["\x27](stt|tts)' --type pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 57
🏁 Script executed:
# Let me look at the full test in test_llm.py to see if create_test_version is ever called
sed -n '71,150p' backend/app/tests/crud/test_llm.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 2267
🏁 Script executed:
# Check create_test_version in context - what happens when config_type is stt
# Let's test the validation by checking if KaapiCompletionConfig with stt type accepts the hard-coded params
python3 << 'EOF'
# Simulating what happens in create_test_version
# For stt type with Kaapi provider, it tries:
# params={"model": "...", "temperature": 0.8}
# But STTLLMParams requires "instructions" field
print("STTLLMParams requirements:")
print("- model (required)")
print("- instructions (required)")
print("- input_language (optional)")
print("- output_language (optional)")
print("- response_format (optional)")
print("- temperature (optional, default 0.2)")
print()
print("create_test_version hard-codes for Kaapi:")
print("- model: extracted from old config")
print("- temperature: 0.8")
print()
print("MISSING: instructions (REQUIRED)")
print("Validation will fail: ValidationError")
EOFRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 439
🏁 Script executed:
# Verify that no tests currently create stt/tts configs and then call create_test_version implicitly
# by checking all create_test_config calls with specific config_blob parameter
rg -n 'create_test_config' backend/app/tests/crud/config/test_version.py -A 3 | head -50Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 1470
🏁 Script executed:
# Check if there's ANY path where create_test_config could be called with stt/tts without explicit config_blob
rg -n 'create_test_config\(' backend/app/tests --type py -A 2 | grep -c 'type.*stt\|type.*tts'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 76
🏁 Script executed:
# Final check: verify that the `@model_validator` will indeed raise on missing required fields
python3 << 'EOF'
from pydantic import BaseModel, Field
from typing import Literal
# Simulate STTLLMParams
class STTLLMParams(BaseModel):
model: str
instructions: str
input_language: str | None = None
# Simulate what happens when create_test_version tries to create stt config with incomplete params
try:
params = STTLLMParams.model_validate({
"model": "whisper-1",
"temperature": 0.8 # This field doesn't exist in STTLLMParams
})
except Exception as e:
print(f"Validation Error Type: {type(e).__name__}")
print(f"Error: {str(e)[:200]}")
print("\nResult: VALIDATION WILL FAIL - missing required 'instructions'")
EOFRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 385
create_test_version hard-codes text params for all config types, causing validation failures for stt/tts.
When config_blob is not provided and the latest version is stt or tts, the new params will be incomplete: stt requires instructions but only gets {"model": "...", "temperature": 0.8}; tts requires voice and language. Validation via @model_validator in KaapiCompletionConfig will fail. Preserve existing params unless config_type == "text", or use type-specific defaults.
🔧 Suggested fix
- config_type = completion_config.get("type")
- provider = completion_config.get("provider", "openai-native")
+ config_type = completion_config.get("type", "text")
+ provider = completion_config.get("provider", "openai-native")
+ params = completion_config.get("params", {}) or {}
- # Create a new config_blob maintaining the same type and provider
- if provider in ["openai-native", "google-native"]:
+ if config_type == "text":
+ params = {
+ "model": params.get("model", "gpt-4"),
+ "temperature": params.get("temperature", 0.8),
+ **(
+ {"max_tokens": params.get("max_tokens", 1500)}
+ if provider in ["openai-native", "google-native"]
+ else {}
+ ),
+ }
+
+ if provider in ["openai-native", "google-native"]:
config_blob = ConfigBlob(
completion=NativeCompletionConfig(
provider=provider,
type=config_type,
- params={
- "model": completion_config.get("params", {}).get(
- "model", "gpt-4"
- ),
- "temperature": 0.8,
- "max_tokens": 1500,
- },
+ params=params,
)
)
else:
# For Kaapi providers (openai, google)
config_blob = ConfigBlob(
completion=KaapiCompletionConfig(
provider=provider,
type=config_type,
- params={
- "model": completion_config.get("params", {}).get(
- "model", "gpt-4"
- ),
- "temperature": 0.8,
- },
+ params=params,
)
)🤖 Prompt for AI Agents
In `@backend/app/tests/utils/test_data.py` around lines 321 - 374,
create_test_version currently builds a new ConfigBlob with hard-coded params for
all completion types, causing validation errors for stt/tts; update the logic
that constructs config_blob (reference ConfigBlob, NativeCompletionConfig,
KaapiCompletionConfig, completion_config and config_type) to preserve the
existing completion_config["params"] by default and only override or inject
defaults when config_type == "text" (or when a specific param is missing), and
for non-text types provide type-specific defaults or keep originals (e.g.,
ensure stt includes "instructions" and tts includes "voice" and "language") so
validators won't fail.
3 participants
Summary
Target issue is #PLEASE_TYPE_ISSUE_NUMBER
Explain the motivation for making this change. What existing problem does the pull request solve?
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit
New Features
Chores