WIP: Add fallback support with new configuration, error handling, and metadata#20
WIP: Add fallback support with new configuration, error handling, and metadata#20chonknick wants to merge 1 commit into
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
catsu-docs | 22c4109 | Commit Preview URL Branch Preview URL |
Dec 17 2025, 11:43 PM |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if model_dims is not None and not model_info.supports_dimensions: | ||
| # Skip this model if it doesn't support requested dimensions | ||
| if self.verbose: | ||
| print(f"[catsu] Skipping {model}: doesn't support custom dimensions") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
The problem is the possible logging of sensitive information if, by mistake or malicious design, the model field in the chain dictionaries contains sensitive data (such as an API key). The best fix is to avoid logging this value directly. There are two main approaches:
- Sanitize logging: Only log safe, known, non-sensitive values; for example, you might only log model names after validating them.
- Remove or redact sensitive fields: Never log the actual value unless it is guaranteed (via whitelist or validation) not to be sensitive.
For this code, the least intrusive, most robust fix is to avoid logging the actual value of model, and instead indicate just the action (e.g., "Skipping model (name redacted): doesn't support custom dimensions"), or, if validation/whitelisting is possible, log only an explicit safe identifier.
Changes needed:
- In
src/catsu/client.py, in the log at line 510, replaceprint(f"[catsu] Skipping {model}: doesn't support custom dimensions")with a version that does not include the possibly-tainted variable. For example:print("[catsu] Skipping a model: doesn't support custom dimensions")or, preferably, log a sanitized model name (if you can be sure you have one; otherwise, just do not log it). - No imports or additional dependencies are needed.
| @@ -507,7 +507,7 @@ | ||
| if model_dims is not None and not model_info.supports_dimensions: | ||
| # Skip this model if it doesn't support requested dimensions | ||
| if self.verbose: | ||
| print(f"[catsu] Skipping {model}: doesn't support custom dimensions") | ||
| print("[catsu] Skipping a model due to unsupported custom dimensions") | ||
| continue | ||
|
|
||
| model_input_type = input_type |
| ) | ||
|
|
||
| if self.verbose and model != primary_model: | ||
| print(f"[catsu] Fallback succeeded with {model}") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
The best way to fix this problem is to ensure that no potentially sensitive data is logged, even indirectly. In this case, the best fix is to avoid printing the full value of model. Instead, log only a safe representation of the model identifier (for example, by showing its index in the chain, a truncated, or obfuscated version, or just a generic success message without interpolating external input). This should be done by editing line 539 in the _embed_with_fallbacks method (and symmetrically in any similar logging in corresponding async methods or related functions).
Steps:
- In the relevant block, replace
print(f"[catsu] Fallback succeeded with {model}")with a variant that does not include the model value, or which only logs a known-safe representation. - No new imports are required for simple redaction or generic messages.
- Since only the block shown is to be edited, avoid any changes to other files or regions.
| @@ -536,7 +536,7 @@ | ||
| ) | ||
|
|
||
| if self.verbose and model != primary_model: | ||
| print(f"[catsu] Fallback succeeded with {model}") | ||
| print("[catsu] Fallback succeeded with secondary model.") | ||
|
|
||
| return response | ||
|
|
| except Exception as e: | ||
| errors[model] = e | ||
| if self.verbose: | ||
| print(f"[catsu] Cycle {cycle}: {model} failed: {e}") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix this problem, the code should ensure sensitive information (such as api_key and anything that could encode it) is never logged directly. This involves:
- Avoid logging the entire config or model string if it may contain sensitive data.
- When logging, only include non-sensitive identifiers (e.g., model names, cycle numbers), and explicitly avoid interpolating the entire config or exception message if they are not guaranteed to be free of secrets.
- Modify the log statement at line 546 to output only safe, non-sensitive details. Instead of including
model(which may be tainted) ore(the full exception, which may embed sensitive text), only log, for example, the position in the fallback list and the error type, or a sanitized error message. - The same principle applies to any similar surrounding log statements.
Required changes:
- In
src/catsu/client.py, refactor the print statement at line 546 to avoid directly logging user-tainted values. Only print the error type and, if useful, a safe identifier for the model, or redact as appropriate. - Add a redaction helper if needed, unless simply omitting the field is best.
| @@ -543,7 +543,7 @@ | ||
| except Exception as e: | ||
| errors[model] = e | ||
| if self.verbose: | ||
| print(f"[catsu] Cycle {cycle}: {model} failed: {e}") | ||
| print(f"[catsu] Cycle {cycle}: model fallback failed with error: {type(e).__name__}") | ||
|
|
||
| # Check if this error should trigger fallback | ||
| if not self._is_fallback_error(e): |
| model_dims = model_kwargs.get("dimensions") | ||
| if model_dims is not None and not model_info.supports_dimensions: | ||
| if self.verbose: | ||
| print(f"[catsu] Skipping {model}: doesn't support custom dimensions") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix this problem, we should ensure that no sensitive information or tainted data (such as api_key or model possibly containing sensitive info) is ever logged in clear text. The robust approach is to sanitize any values interpolated into logs and, where possible, log only safe, non-sensitive identifiers. For these logs, it is best to log only a whitelisted set of values or explicitly redact any value that could contain sensitive data.
- Specifically, replace logging of the raw
modelvariable with a redacted or validated safe identifier. One safe strategy is to log only the model's name after parsing, or, if not possible, use a constant string or placeholder such as "[REDACTED]" or "[MODEL_ID]" instead. - If there's a function for extracting just the safe model name (
model_namefromself._parse_model_string(model)), use that for logging. - All such logs in the function should be similarly sanitized.
- Only changes within the shown snippet in
src/catsu/client.pyare permitted; external helpers or changing callsites are not.
| @@ -626,7 +626,12 @@ | ||
| model_dims = model_kwargs.get("dimensions") | ||
| if model_dims is not None and not model_info.supports_dimensions: | ||
| if self.verbose: | ||
| print(f"[catsu] Skipping {model}: doesn't support custom dimensions") | ||
| # Safely log only the sanitized model identifier, never user-supplied data | ||
| try: | ||
| provider_id, safe_model_id = self._parse_model_string(model) | ||
| print(f"[catsu] Skipping model ({provider_id}/{safe_model_id}): doesn't support custom dimensions") | ||
| except Exception: | ||
| print("[catsu] Skipping an unsafe or invalid model identifier: doesn't support custom dimensions") | ||
| continue | ||
|
|
||
| model_input_type = input_type | ||
| @@ -654,14 +659,24 @@ | ||
| ) | ||
|
|
||
| if self.verbose and model != primary_model: | ||
| print(f"[catsu] Fallback succeeded with {model}") | ||
| # Safely log only the sanitized model identifier | ||
| try: | ||
| provider_id, safe_model_id = self._parse_model_string(model) | ||
| print(f"[catsu] Fallback succeeded with model ({provider_id}/{safe_model_id})") | ||
| except Exception: | ||
| print("[catsu] Fallback succeeded with an unsafe or invalid model identifier") | ||
|
|
||
| return response | ||
|
|
||
| except Exception as e: | ||
| errors[model] = e | ||
| if self.verbose: | ||
| print(f"[catsu] Cycle {cycle}: {model} failed: {e}") | ||
| # Safely log only the sanitized model identifier | ||
| try: | ||
| provider_id, safe_model_id = self._parse_model_string(model) | ||
| print(f"[catsu] Cycle {cycle}: model ({provider_id}/{safe_model_id}) failed: {e}") | ||
| except Exception: | ||
| print(f"[catsu] Cycle {cycle}: unsafe or invalid model identifier failed: {e}") | ||
|
|
||
| if not self._is_fallback_error(e): | ||
| raise |
| ) | ||
|
|
||
| if self.verbose and model != primary_model: | ||
| print(f"[catsu] Fallback succeeded with {model}") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix the problem, ensure that the log message at line 657 does not reveal sensitive data. This includes not printing the raw model string if there is any chance it could contain secrets. Instead, sanitize or redact it before logging, or log only safe, non-sensitive identifiers/metadata. The safest fix is to avoid logging the raw model identifier entirely or to log only the provider and a redacted identifier if required for debugging (but never the API key or any secret value). The required code change is limited to the logging line(s) at or near line 657 in src/catsu/client.py. No extra imports are needed; we can refactor the log statement to show only generic metadata or an explicit [REDACTED] marker.
| @@ -654,7 +654,8 @@ | ||
| ) | ||
|
|
||
| if self.verbose and model != primary_model: | ||
| print(f"[catsu] Fallback succeeded with {model}") | ||
| # Do not log potentially sensitive model identifiers or secrets. | ||
| print("[catsu] Fallback succeeded with alternative model.") | ||
|
|
||
| return response | ||
|
|
| except Exception as e: | ||
| errors[model] = e | ||
| if self.verbose: | ||
| print(f"[catsu] Cycle {cycle}: {model} failed: {e}") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix this problem, we should ensure that no sensitive information, such as API keys or passwords contained in the config dictionary, is ever printed to logs. Specifically, when logging model (which could be tainted), we should avoid logging full config details or any data that could come from sensitive input sources. The best way to do this is to only print the model name, after extracting it explicitly, and ensure that it is not the API key.
We must update the logging at line 664 and any similar log lines to ensure that only safe, non-sensitive details are logged. If additional info about the error is needed, the exception class name (type(e).__name__) can be logged, but not the potentially sensitive message (e).
- Update any print/log lines logging potentially tainted values (e.g.,
config,model,e) so that only non-sensitive data is logged. - Optionally, we can explicitly redact or mask values when necessary.
- No external library imports are needed, as only string construction and filtering are required.
- Changes are limited to src/catsu/client.py, specifically the lines that contain print/log statements in the
if self.verbose:and similar error blocks in_aembed_with_fallbacks(and any related areas that match the same pattern, if present).
| @@ -661,7 +661,7 @@ | ||
| except Exception as e: | ||
| errors[model] = e | ||
| if self.verbose: | ||
| print(f"[catsu] Cycle {cycle}: {model} failed: {e}") | ||
| print(f"[catsu] Cycle {cycle}: Model '{model}' failed with error type: {type(e).__name__}") | ||
|
|
||
| if not self._is_fallback_error(e): | ||
| raise |
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive fallback support for embedding models, allowing automatic failover when primary models encounter transient errors. The implementation adds new exception types for configuration and exhaustion scenarios, extends the EmbedResponse model to track fallback metadata, and includes extensive test coverage for the fallback logic.
Key Changes:
- New exception classes (
ConfigurationError,FallbackExhaustedError) for distinguishing between configuration issues and exhausted fallback attempts - Enhanced
EmbedResponsemodel with fallback metadata fields (requested_model,fallback_used,fallback_reason) - Client-level and request-level fallback configuration with dimension compatibility filtering and round-robin retry logic
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_client.py |
Comprehensive test suite covering fallback normalization, credential validation, dimension filtering, error classification, client configuration, new exception types, and response metadata |
src/catsu/utils/errors.py |
New ConfigurationError and FallbackExhaustedError exception classes with detailed attributes and messaging |
src/catsu/models.py |
Extended EmbedResponse with fallback metadata fields and updated __repr__ to include fallback information |
src/catsu/client.py |
Core fallback implementation including helper methods, orchestration logic, and updated public API with fallback parameters |
src/catsu/__init__.py |
Export new exception classes at package level for public API access |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| error = FallbackExhaustedError( | ||
| primary_model="voyage-3", | ||
| fallbacks=["model-a", "model-b"], | ||
| errors={"voyage-3": RateLimitError("Rate limited")}, |
There was a problem hiding this comment.
The RateLimitError constructor requires a provider parameter, but none is provided here. This will cause the test to fail with a TypeError.
| errors={"voyage-3": RateLimitError("Rate limited")}, | |
| errors={"voyage-3": RateLimitError("Rate limited", provider="test")}, |
| def __init__( | ||
| self, | ||
| primary_model: str, | ||
| fallbacks: List[str], |
There was a problem hiding this comment.
The parameter documentation describes 'fallbacks' as 'List of fallback model names that were tried', but the type hint is List[str]. Consider clarifying in the docstring that these are model name strings (e.g., 'voyage-3' or 'provider:model').
This pull request adds robust support for fallback models and error handling in the
catsupackage. It introduces new exception types for configuration and fallback failures, extends theEmbedResponsemodel to include fallback metadata, and adds comprehensive tests for fallback normalization, credential validation, dimension compatibility, error classification, and the new error types.Fallback and Error Handling Enhancements:
ConfigurationErrorandFallbackExhaustedErrortocatsu.utils.errors, allowing the client to distinguish between configuration issues and cases where all fallback models have failed.src/catsu/__init__.pyto export the new exceptions, making them available at the package level.EmbedResponse Model Improvements:
EmbedResponsemodel with new fields:requested_model,fallback_used, andfallback_reason, providing detailed metadata when a fallback is triggered. The__repr__method now includes fallback information when relevant. [1] [2] [3]Testing and Validation of Fallback Logic:
tests/test_client.pycovering:ConfigurationErrorwhen API keys are missing.FallbackExhaustedErrorandConfigurationErrorclasses.EmbedResponsefallback metadata and representation. [1] [2]Minor Internal Improvements:
catsu.utils.errorsto includeList, supporting the new exception classes.