feat(agents): support custom providers and secret store values#12901
Open
mikeldking wants to merge 10 commits intomainfrom
Open
feat(agents): support custom providers and secret store values#12901mikeldking wants to merge 10 commits intomainfrom
mikeldking wants to merge 10 commits intomainfrom
Conversation
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
mikeldking
changed the title
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
CodeQL flagged the TYPE_CHECKING import as unused because the annotation was a string literal. With `from __future__ import annotations` in effect the quotes are unnecessary, and removing them exposes the bare-name reference to static analysis.
ContextualTool is defined before the back-import in registry.py, so a plain top-level import works without a circular-import workaround.
Restore the Args/Returns block dropped in the chat refactor and reword the summary so it describes the behavior rather than the MCP transport.
Captures lessons from PR #12901 review: don't strip Args/Returns blocks during refactors, and describe behavior abstractly on public APIs rather than coupling docstrings to a specific transport (e.g. MCP).
- Map the agents/ package in the __init__ docstring so future consumers can find the right module without reading every file. - Add module + class docstrings to chat_params, model_factory, and tools/registry, including a short "adding a new contextual tool" recipe. - Document the secret-vs-env precedence in _resolve_secret_or_env and the params/raises contract for build_chat_model. - Comment the assert_never + raise pattern so future readers don't strip one of the two as dead code (it satisfies CodeQL flow analysis). - Re-export ContextualTool from agents.tools so callers don't need to reach into registry. - Remove the unused instance-level MintlifyDocsClient.is_backend_tool duplicate; only the module-level helper is referenced.
Lift transport concerns out of the agents domain layer:
- New phoenix.server.agents.exceptions module with an AgentError
hierarchy (Provider{NotFound,Config,Credentials,Unsupported,
Dependency}Error). Each subclass carries a status_code so the REST
router maps them to HTTP responses without inspecting types.
- model_factory no longer imports fastapi or strawberry.relay. It
raises domain exceptions; the chat router translates them to
HTTPException at the boundary.
- GlobalID decoding moves from build_chat_model into a Pydantic
validator on CustomProviderChatSearchParams.provider_id, so the
factory works on the decoded integer node ID.
- _resolve_secret_or_env now wraps the BadRequest raised by
playground_clients._resolve_secrets into a ProviderConfigError. This
closes a latent bug where a corrupt stored secret produced a 500
instead of the intended 400.
Tests updated to assert on the new domain exceptions, plus a new
regression test that exercises the previously-broken decrypt-failure-
during-secret-lookup path through build_chat_model.
- Use the existing from_global_id_with_expected_type helper from phoenix.server.api.types.node in the chat_params validator instead of hand-rolling the relay-id-to-int decode. Adds free type validation that the GlobalID corresponds to a GenerativeModelCustomProvider. - Add an explicit int annotation to ProviderNotFoundError.status_code so subclass overrides match the base class's signature. - Extract the duplicated "Azure identity package not installed." string into a module-level constant. - Batch the AWS Bedrock secret lookup into a single DB query via a new _resolve_secrets_or_env helper (previously three sequential queries for AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN). _resolve_secret_or_env now delegates to the batched helper.
dosubot
Bot
added
the
size:XXL
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates the agents chat flow to support both secret-backed built-in providers and custom generative model providers.
It also refactors the chat implementation so the router is a thin HTTP layer and the chat-domain code lives under
src/phoenix/server/agents/.Closes #12899.