I've refactored core components for robustness, configuration, and er… #10
Conversation
…ror handling. Key changes include: - Improved FileConfig.can_handle_media logic with tests. - Standardized API key management and ModelConfig usage in model_clients.py. - Implemented consistent custom error handling (ModelClientError hierarchy) across all model clients. - Optimized video file handling to use processed (smaller) data/frames, reducing memory usage. - Replaced fragile HTML string manipulation in arbiter_v4.py with BeautifulSoup, adding tests. - Made video/image processing parameters in file_handler.py more configurable and efficient. Further unit tests for model_clients.py (API key, ModelConfig, error handling mocks) are pending.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors core components for improved robustness, configuration management, and error handling across the codebase. Key improvements include enhanced file handling logic, standardized model configuration, and replacement of fragile HTML parsing with BeautifulSoup.
- Enhanced
FileConfig.can_handle_medialogic with comprehensive provider mapping and model support checking - Added configurable video processing parameters and optimized memory usage by using processed video data instead of raw files
- Replaced brittle HTML string manipulation in
arbiter_v4.pywith robust BeautifulSoup parsing
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_file_handler.py | Comprehensive test coverage for FileConfig.can_handle_media with edge cases and provider validation |
| tests/test_arbiter_v4.py | Updated mock data and added BeautifulSoup-based HTML modification tests |
| src/file_handler.py | Enhanced media handling logic, video processing optimization, and configurable parameters |
| src/configdataclasses.py | Extended ModelConfig with additional parameters and validation |
| src/arbiter_v4.py | Replaced HTML string manipulation with BeautifulSoup for robust parsing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
| ) | ||
| }) | ||
| except FileIOError as e: |
There was a problem hiding this comment.
FileIOError is not a standard Python exception. This should be IOError or OSError to properly catch file I/O errors.
| "type": "base64", | ||
| "mime_type": metadata.mime_type, | ||
| "data": file_data, | ||
| "data": file_data, # This is file_data from with open(metadata.path, "rb") |
There was a problem hiding this comment.
The comment references file_data but this variable is not defined in the visible scope of this method. This suggests either missing variable declaration or incorrect comment.
|
|
||
| # For other types like text, if they were previously read into file_data, handle here | ||
| # This part is simplified, assuming 'file_data' would be the raw bytes for text/code | ||
| elif metadata.type not in ["image", "video"] and file_data: # file_data is raw bytes here |
There was a problem hiding this comment.
The variable file_data is referenced but not defined in the visible scope of this method. This will cause a NameError at runtime.
…ror handling.
Key changes include:
Further unit tests for model_clients.py (API key, ModelConfig, error handling mocks) are pending.