Model Config: relax model validation to warning#902
Conversation
…instead of raising errors
📝 WalkthroughWalkthroughModel validation in validate_blob_model_or_raise now logs warnings for missing ModelConfig rows and unsupported TTS voices and continues processing; tests are updated to expect warnings and allow validation to proceed instead of asserting HTTP 400 errors. ChangesModel validation: fail-fast to warning-and-continue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
| f"Voice '{voice}' is not supported for provider='{provider}' " | ||
| f"model='{model_name}'. Allowed: {allowed_voices}" | ||
| ), | ||
| logger.warning( |
There was a problem hiding this comment.
@Prajna1999 Voice name validation is also being skipped now, only logging a warning instead of failing the request. Is this behavior okay?
Since if the voice is not allowed then provider itself will fail it.
There was a problem hiding this comment.
Okay lets keep this behaviour. Helpful for 11labs that allows passing custom voice ids and user generated ids should be allowed at Kaapi.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Prajna1999
left a comment
Prajna1999
left a comment
There was a problem hiding this comment.
I think modality specific checks i.e text, stt, tts and their models should be mandatory, let's keep in mind to add that in the next task.
| f"Voice '{voice}' is not supported for provider='{provider}' " | ||
| f"model='{model_name}'. Allowed: {allowed_voices}" | ||
| ), | ||
| logger.warning( |
There was a problem hiding this comment.
Okay lets keep this behaviour. Helpful for 11labs that allows passing custom voice ids and user generated ids should be allowed at Kaapi.
| f"type='{completion_type}'. Allowed: {allowed}" | ||
| ), | ||
| logger.warning( | ||
| f"[validate_blob_model_or_raise] Model '{model_name}' not found for provider='{provider}'." |
There was a problem hiding this comment.
Can add another line like Ignoring validation checks to provider, non-blocking.. Something that says although kaapi does not yet support this model, but passing thru as long as there are no typos/deprecated model in the model field.
There was a problem hiding this comment.
yeah I added it .. but this is just the log, will helpful for devs only. not for the client .. for client can we send the message in webhook callback ?
{
"success": true,
"data": {
"job_id": "1170d050-2901-4511-97d4-40784d338405",
"status": "SUCCESS",
"llm_response": {
"response": {
"provider_response_id": "WiIdaoSoFfmfjuMPicKmqAQ",
"conversation_id": null,
"provider": "google",
"model": "gemini-2.5-pro",
"output": {
"type": "text",
"content": {
"format": "text",
"value": "Output"
}
}
},
"usage": {
"input_tokens": 12,
"output_tokens": 120,
"total_tokens": 1359,
"reasoning_tokens": 1227
},
"provider_raw_response": null
},
"error_message": null
},
"error": null,
"errors": null,
"metadata": {"message": "Kaapi does not yet support this model, but will forward as long as the `model` field has no typos and the model is not deprecated by the provider"}
}
here message can be made proper.
There was a problem hiding this comment.
Will keep it as log.error for now to indicate that the model is not supported by Kaapi.
| f"model='{model_name}'. Allowed: {allowed_voices}" | ||
| ), | ||
| logger.warning( | ||
| f"[validate_blob_model_or_raise] Voice '{voice}' is not supported for provider='{provider}' " |
There was a problem hiding this comment.
same as above, a bit more explanatory logs. Else could be misleading in the logs.
There was a problem hiding this comment.
@Prajna1999 The modality checks and completion_type is already implemented in new pr where the completion_type I had set as an array ['text', 'stt']
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/app/crud/model_config.py`:
- Around line 126-129: The log message in validate_blob_model_or_raise
concatenates two string literals without a separating space, causing
"...provider='{provider}'.Kaapi..." in logs; fix by ensuring a space between the
sentences (e.g., merge into a single f-string or add a trailing space at the end
of the first string or leading space at the start of the second) in the
logger.warning call so the message reads "...provider='{provider}'. Kaapi does
not yet support...".
🪄 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: CHILL
Plan: Pro
Run ID: 40660dee-7473-4ade-9111-eabe3c3b25a8
📒 Files selected for processing (1)
backend/app/crud/model_config.py
| logger.warning( | ||
| f"[validate_blob_model_or_raise] Model '{model_name}' not found for provider='{provider}'." | ||
| "Kaapi does not yet support this model, but will forward as long as the `model` field has no typos and the model is not deprecated by the provider" | ||
| ) |
There was a problem hiding this comment.
Missing whitespace between concatenated strings.
The two string literals are implicitly concatenated, but there's no space or newline between the period at the end of line 127 and "Kaapi" at the start of line 128. The resulting log message will read ...provider='{provider}'.Kaapi does not....
🔧 Proposed fix
if model_row is None:
logger.warning(
f"[validate_blob_model_or_raise] Model '{model_name}' not found for provider='{provider}'. "
- "Kaapi does not yet support this model, but will forward as long as the `model` field has no typos and the model is not deprecated by the provider"
+ "Kaapi does not yet support this model, but will forward as long as the `model` field has no typos and the model is not deprecated by the provider."
)🤖 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 `@backend/app/crud/model_config.py` around lines 126 - 129, The log message in
validate_blob_model_or_raise concatenates two string literals without a
separating space, causing "...provider='{provider}'.Kaapi..." in logs; fix by
ensuring a space between the sentences (e.g., merge into a single f-string or
add a trailing space at the end of the first string or leading space at the
start of the second) in the logger.warning call so the message reads
"...provider='{provider}'. Kaapi does not yet support...".
3 participants
Target issue is #901
Summary
validate_blob_model_or_raiseno longer blocks requests when a model is missing frommodel_configor has a mismatched completion type. It logs a warning and lets the call through. Provider API stays the source of truth;model_configis now metadata, not a gatekeeper.What changed
logger.warning, continue.Nonemodel_row.modelparam still returns400.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.Summary by CodeRabbit
Bug Fixes