fix(LiteLlm): recognize assistant- prefix as valid OpenAI file ID#5758
fix(LiteLlm): recognize assistant- prefix as valid OpenAI file ID#5758nileshpatil6 wants to merge 6 commits into
Conversation
|
Response from ADK Triaging Agent Hello @nileshpatil6, thank you for creating this PR to fix the Azure OpenAI assistant prefix issue! To help our reviewers process this contribution more efficiently and maintain code quality, could you please address the following items from our Contribution Guidelines:
These details will help ensure we prevent regressions and streamline the review process. Thank you again for your contribution! |
|
Hi @nileshpatil6 , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Can you please fix the failing unit tests before we can proceed with the review. |
|
hey @rohityan thanks for the review. added unit tests in tests/unittests/models/test_litellm.py covering both the new assistant- prefix recognition and the redact helper. all 10 new tests pass locally along with the rest of test_litellm.py (268 passed, 1 skipped). the two failing checks on the run dont seem related to this PR:
happy to look into those if you want but they seem like flakes from main. let me know if anything else needed |
|
Hi @rohityan, thanks for keeping this synced with main. Quick check in from my side. The unit tests you asked for are in tests/unittests/models/test_litellm.py, covering the assistant- prefix case along with the existing ones, and all the CI checks are green right now. Is there anything else you want me to change, or is it good to go? Happy to make any tweaks if needed. |
Merge #5758 Azure OpenAI files uploaded with `purpose="assistants"` receive IDs with an `assistant-` prefix (e.g. `assistant-abc123`). `_looks_like_openai_file_id` only checked for the `file-` prefix, causing Azure PDF attachments to be silently dropped from requests instead of being routed through the Responses API content block. Fix: extend the `startswith` check to include `"assistant-"`. Also updated `_redact_file_uri_for_log` to return `assistant-<redacted>` for `assistant-` prefixed IDs, consistent with how `file-` IDs are handled. Fixes #5664 ## Testing plan Added unit tests in `tests/unittests/models/test_litellm.py`: - `test_looks_like_openai_file_id` (parametrized) covers: - `file-abc123` -> True (existing behavior) - `assistant-abc123` -> True (new behavior) - `https://example.com/file.pdf` -> False - `not-a-file-id` -> False - empty string -> False - `FILE-abc123` -> False (case sensitive) - `test_redact_file_uri_for_log_openai_prefixes` (parametrized): - `file-abc123` -> `file-<redacted>` - `assistant-abc123` -> `assistant-<redacted>` (new behavior) - `test_redact_file_uri_for_log_uses_display_name_when_provided` confirms `display_name` short-circuits redaction. - `test_redact_file_uri_for_log_http_url_keeps_scheme_and_tail` confirms HTTP URLs still redact host but preserve scheme and filename. Run locally with: ``` pytest tests/unittests/models/test_litellm.py -k "looks_like_openai_file_id or redact_file_uri_for_log" ``` Co-authored-by: George Weale <gweale@google.com> COPYBARA_INTEGRATE_REVIEW=#5758 from nileshpatil6:fix/lite-llm-assistant-file-id-prefix 6b2afd7 PiperOrigin-RevId: 933898777
|
Thank you @nileshpatil6 for your contribution! 🎉 Your changes have been successfully imported and merged via Copybara in commit 796964a. Closing this PR as the changes are now in the main branch. |
Merge #5758 Original PR by @nileshpatil6 (Nilesh Patil <128893479+nileshpatil6@users.noreply.github.com>) Azure OpenAI files receive IDs with an `assistant-` prefix (e.g. `assistant-abc123`) or `file-` prefix. When `_looks_like_openai_file_id` recognizes an `assistant-` prefixed ID (e.g., `assistant-abc123`) or `file-` prefixed ID, `_redact_file_uri_for_log` defaulted to logging both as `file - <redacted>`. Fix: refactor `_redact_file_uri_for_log` to dynamically extract file ID prefixes (`file-`, `assistant-`). This ensures compatibility with both current prefixes and future OpenAI/Azure file ID types. Fixes #5664 Co-authored-by: Yi Liu <yiliuly@google.com> PiperOrigin-RevId: 933931477
Azure OpenAI files uploaded with
purpose="assistants"receive IDs with anassistant-prefix (e.g.assistant-abc123)._looks_like_openai_file_idonly checked for thefile-prefix, causing Azure PDF attachments to be silently dropped from requests instead of being routed through the Responses API content block.Fix: extend the
startswithcheck to include"assistant-".Also updated
_redact_file_uri_for_logto returnassistant-<redacted>forassistant-prefixed IDs, consistent with howfile-IDs are handled.Fixes #5664
Testing plan
Added unit tests in
tests/unittests/models/test_litellm.py:test_looks_like_openai_file_id(parametrized) covers:file-abc123-> True (existing behavior)assistant-abc123-> True (new behavior)https://example.com/file.pdf-> Falsenot-a-file-id-> FalseFILE-abc123-> False (case sensitive)test_redact_file_uri_for_log_openai_prefixes(parametrized):file-abc123->file-<redacted>assistant-abc123->assistant-<redacted>(new behavior)test_redact_file_uri_for_log_uses_display_name_when_providedconfirmsdisplay_nameshort-circuits redaction.test_redact_file_uri_for_log_http_url_keeps_scheme_and_tailconfirms HTTP URLs still redact host but preserve scheme and filename.Run locally with: