Skip to content

bug: Warn about Azure OpenAI completions file-incompatibility#4048

Open
dsfaccini wants to merge 22 commits intomainfrom
review-azure-file-support
Open

bug: Warn about Azure OpenAI completions file-incompatibility#4048
dsfaccini wants to merge 22 commits intomainfrom
review-azure-file-support

Conversation

@dsfaccini
Copy link
Copy Markdown
Collaborator

@dsfaccini dsfaccini commented Jan 20, 2026

Pre-Review Checklist

  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • No breaking changes in accordance with the version policy.
  • Linting and type checking pass per make format and make typecheck.
  • PR title is fit for the release changelog.

Pre-Merge Checklist

  • New tests for any fix or new behavior, maintaining 100% coverage.
  • Updated documentation for new features and behaviors, including docstrings for API docs.

@dsfaccini dsfaccini added bug Report that something isn't working, or PR implementing a fix media azure labels Jan 20, 2026
docs/input.md Outdated
Comment on lines +121 to +122
A model API may be unable to download a file (e.g., because of crawling or access restrictions) even if it supports file URLs. For example, [`GoogleModel`][pydantic_ai.models.google.GoogleModel] on Vertex AI limits YouTube video URLs to one URL per request. In such cases, you can instruct Pydantic AI to download the file content locally and send that instead of the URL by setting `force_download` on the URL object:
??? warning "`DocumentUrl` and `BinaryContent` documents are not supported when using `AzureProvider` with `OpenAIChatModel`."
Use [`OpenAIResponsesModel`][pydantic_ai.models.openai.OpenAIResponsesModel] with [`AzureProvider`][pydantic_ai.providers.azure.AzureProvider] instead:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need an example here, but I want to make sure the Azure doc (currently section on the OpenAI page) explains (in a generic way) that it can be used with the Responses API as well, and that that's actually recommend over OpenAIChatModel. Can you add it there instead please?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for sure, TODO

Copy link
Copy Markdown
Collaborator Author

@dsfaccini dsfaccini Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to azure, I'm thinking we should simply replace the default example with responses and leave the chat one in a second tab?
Edit: also can I remove those trailing ... from examples?
image

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 20, 2026

Docs Preview

commit: f882fe8
Preview URL: https://9307b50e-pydantic-ai-previews.pydantic.workers.dev

docs/input.md Outdated
| [`BedrockConverseModel`][pydantic_ai.models.bedrock.BedrockConverseModel] | S3 URLs (`s3://`) | `ImageUrl`, `DocumentUrl`, `VideoUrl` | `AudioUrl` |

A model API may be unable to download a file (e.g., because of crawling or access restrictions) even if it supports file URLs. For example, [`GoogleModel`][pydantic_ai.models.google.GoogleModel] on Vertex AI limits YouTube video URLs to one URL per request. In such cases, you can instruct Pydantic AI to download the file content locally and send that instead of the URL by setting `force_download` on the URL object:
??? warning "`DocumentUrl` and `BinaryContent` documents are not supported when using `AzureProvider` with `OpenAIChatModel`."
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather add a Notes column to the table, like we have on the builtin tools doc, where we can state on the OpenAIChatModel row that Azure doesn't support it, and any other notes for provider-specific support (I believe there are some profile flags for specific providers that enable file types that are not supported by OpenAI itself)

Copy link
Copy Markdown
Collaborator Author

@dsfaccini dsfaccini Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at my screen I'm not liking the idea of a fifth column
image

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding a fifth column, what about mentioning the Azure case under "Unsupported", and then linking to the section that explains the responses workaround?

docs/input.md Outdated
Comment on lines +121 to +122
A model API may be unable to download a file (e.g., because of crawling or access restrictions) even if it supports file URLs. For example, [`GoogleModel`][pydantic_ai.models.google.GoogleModel] on Vertex AI limits YouTube video URLs to one URL per request. In such cases, you can instruct Pydantic AI to download the file content locally and send that instead of the URL by setting `force_download` on the URL object:
??? warning "`DocumentUrl` and `BinaryContent` documents are not supported when using `AzureProvider` with `OpenAIChatModel`."
Use [`OpenAIResponsesModel`][pydantic_ai.models.openai.OpenAIResponsesModel] with [`AzureProvider`][pydantic_ai.providers.azure.AzureProvider] instead:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need an example here, but I want to make sure the Azure doc (currently section on the OpenAI page) explains (in a generic way) that it can be used with the Responses API as well, and that that's actually recommend over OpenAIChatModel. Can you add it there instead please?

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@dsfaccini dsfaccini force-pushed the review-azure-file-support branch from a01c883 to 63d7e6d Compare January 26, 2026 15:48
)


async def test_yaml_document_as_binary_content_input(allow_model_requests: None, openai_api_key: str):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we need these new tests?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The YAML VCR tests (test_yaml_document_as_binary_content_input, test_x_yaml_document_as_binary_content_input, test_yaml_document_url_input) cover the YAML text-like media type handling added in this PR (the new application/yaml and application/x-yaml branches in _is_text_like_media_type).

test_is_text_like_media_type is a unit test for branch coverage of _is_text_like_media_type.

test_video_url_not_supported replaces a pragma: no cover that was on the VideoUrl raise — now that branch is tested directly.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests cover YAML media-type edge cases I discovered while investigating how Azure handles different file types. They validate pre-existing behavior that was untested, ensuring the content-type classification works correctly for YAML variants (application/x-yaml, text/yaml, etc). Happy to split them out if you prefer, but they're directly related to the file handling work in this PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK that makes sense, happy to have them here then

@pydantic pydantic deleted a comment from greptile-apps bot Feb 5, 2026
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

- Rename `openai_chat_supports_file_input` → `openai_chat_supports_document_input`
- Rename `_raise_file_input_not_supported_error` → `_raise_document_input_not_supported_error`
- Update docstrings and comments to say "document" instead of "file"
- Remove duplicated paragraph in docs/models/openai.md
- Replace warning box with footnote in docs/input.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the size: S Small PR (≤100 weighted lines) label Feb 15, 2026
dsfaccini and others added 4 commits February 15, 2026 09:48
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test was failing in CI because SSRF protection resolves
raw.githubusercontent.com to an IP, causing VCR host matcher to fail.
Adding disable_ssrf_protection_for_vcr fixture fixes this, consistent
with all other URL-downloading VCR tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Covers previously uncovered lines 1233 and 1259 in openai.py:
- Azure + DocumentUrl → Azure-specific error (L1233 + L1255)
- Generic provider + DocumentUrl → generic error (L1233 + L1259)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added size: M Medium PR (101-500 weighted lines) and removed size: S Small PR (≤100 weighted lines) labels Feb 15, 2026
github-actions[bot]

This comment was marked as resolved.

github-actions[bot]

This comment was marked as resolved.

github-actions[bot]

This comment was marked as resolved.

github-actions[bot]

This comment was marked as resolved.

)


async def test_yaml_document_as_binary_content_input(allow_model_requests: None, openai_api_key: str):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As DouweM already noted, these YAML tests (test_yaml_document_as_binary_content_input, test_x_yaml_document_as_binary_content_input, test_yaml_document_url_input) test pre-existing YAML media type handling that's already on main — they don't cover any new behavior introduced by this PR. They also bring along ~1100 lines of cassette recordings. Unless there's a specific reason to add them here (e.g. they were missing and this is a good opportunity), they'd be better suited for a separate PR to keep this one focused on the Azure document input limitation.

github-actions[bot]

This comment was marked as resolved.

… version

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 0 new potential issues.

View 6 additional findings in Devin Review.

Open in Devin Review

)


async def test_yaml_document_as_binary_content_input(allow_model_requests: None, openai_api_key: str):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK that makes sense, happy to have them here then

@github-actions
Copy link
Copy Markdown
Contributor

This PR is stale, and will be closed in 7 days if no reply is received.

…upport

# Conflicts:
#	pydantic_ai_slim/pydantic_ai/models/openai.py
#	tests/models/test_openai.py
#	tests/test_examples.py
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Potential bypass of document support check when openai_chat_supports_file_urls=True

In _map_document_url_item at pydantic_ai_slim/pydantic_ai/models/openai.py:1245, the first branch checks not item.force_download and profile.openai_chat_supports_file_urls and returns a File content part directly WITHOUT checking openai_chat_supports_document_input. If a provider were configured with openai_chat_supports_file_urls=True AND openai_chat_supports_document_input=False, the document support check would be bypassed. Currently no provider has this combination (only OpenRouter sets openai_chat_supports_file_urls=True, and it supports documents), so this is not a practical issue today. But it's a latent inconsistency that could matter if a new provider is added with this combination.

(Refers to line 1245)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the openai_chat_supports_file_urls flag inherently implies the provider supports documents, so I don't think we need to explicitly handle the combination

for future reference, if we found these inconsistencies, having a single dataclass that uses property to validate combinations would be a better approach than checking multiple flags, since adding a check branch here would bloat this out further

dsfaccini and others added 2 commits March 19, 2026 13:43
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 0 new potential issues.

View 7 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 0 new potential issues.

View 7 additional findings in Devin Review.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

azure bug Report that something isn't working, or PR implementing a fix media size: M Medium PR (101-500 weighted lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AzureOpenAI models don't support files

2 participants