feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E) - #312

Merged
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton
May 17, 2026
Merged

feat(provider-vertex): add aisix-provider-vertex skeleton crate (D5, #302 §3 Phase E)#312
moonming merged 3 commits into
mainfrom
feat/vertex-bridge-skeleton

Conversation

@moonming

Copy link
Copy Markdown
Member

Summary

Phase E / D5 of api7/AISIX-Cloud#302 — scaffolds the Google Vertex AI family bridge so a `provider_key` row with `adapter: "vertex"` resolves to a real bridge in the Hub.

Skeleton PR: the crate exists, compiles, is registered in the workspace + Hub. The actual GCP OAuth + per-publisher HTTP dispatch is tracked as D5.1–D5.5 follow-ups (see crate-level docs in `lib.rs`).

What lands

  • New crate `crates/aisix-provider-vertex/` (workspace member, registered).
  • `VertexBridge` struct implementing `aisix_gateway::Bridge`:
  • `VertexPublisher` enum with `from_upstream_id()` and `url_segment()` helpers — implements the LiteLLM `vertex_ai/` multi-publisher single-entry pattern. A single `google-vertex` catalog provider dispatches by model prefix at runtime:
    • `gemini-*` → `publishers/google`
    • `claude-*` → `publishers/anthropic`
    • `llama-*` → `publishers/meta`
    • `mistral-` / `codestral-` → `publishers/mistralai`
    • `jamba-*` → `publishers/ai21`
  • `build_hub()` registers it via `Hub::register_family(Adapter::Vertex, ...)`. The legacy `Provider`-keyed path is unchanged.

Why multi-publisher single entry (not per-publisher provider)

Diverging from LiteLLM would force every customer to register a separate provider_key per publisher even though the GCP credential is the same — exactly the operator pain `google-vertex` solves. The publisher is encoded in the upstream model id (e.g. `claude-3-5-sonnet@20241022`), so the bridge can dispatch internally without exposing the split to cp-api / dashboard.

Tests (10 passing)

  • Publisher prefix resolution for gemini / claude / llama / mistral / codestral / jamba
  • Case-insensitive matching on the model name
  • Unknown prefix returns `None` (drives the publisher-resolution guard)
  • `url_segment()` matches Vertex API path conventions — including the trap that Mistral's Vertex tag is `mistralai`, not `mistral` (pinned to catch a typo regression in a future dispatch PR)
  • `chat()` surfaces clear "not yet implemented + issue refactor(server): inline DeepSeek/Google bridge factories + delete wrapper crates (Phase A) #302" error
  • `chat()` with unknown publisher prefix errors before dispatch (proves the dispatch-time guard)
  • `bridge.name() == "vertex"` (metrics label stability — would silently break customer dashboards on rename)
  • `wire.rs` reserved query params (`alt`, `key`, `access_token`) — covered for the eventual default-headers / default-body-fields override guard (a misconfigured override block must never redirect SSE streaming to JSON or hijack auth)

References (per CLAUDE.md §7)

Out of scope — D5 follow-up tracker

TaskDescription
D5.1google-cloud-auth / yup-oauth2 bearer-token acquisition (service-account JSON key, ADC, metadata server)
D5.2Gemini publisher dispatch (`:streamGenerateContent`)
D5.3Anthropic-on-Vertex dispatch (`:streamRawPredict` — different wire shape from canonical Anthropic Messages)
D5.4Llama / Mistral / AI21 publisher dispatch
D5.5`BridgeContext.deadline` + Retry-After plumbing

Test plan

  • `cargo build --workspace` clean
  • `cargo test --workspace --lib` passes (818 + 10 new = 828 total)
  • `cargo clippy --workspace --all-targets -- -D warnings` clean
  • `cargo fmt --check --all` clean
  • Independent audit per CLAUDE.md §8 (spawned after this PR is open)

Related

…Phase E / D5)
Wave 5 D5 — scaffolds the Google Vertex AI family bridge so a
`provider_key` row with `adapter: "vertex"` resolves to a real
bridge in the Hub instead of falling through to the legacy
fallback. The actual GCP OAuth + per-publisher HTTP dispatch is
tracked as D5.1-D5.5 follow-ups; this PR ships the:
- new `aisix-provider-vertex` crate registered in the workspace
- `VertexBridge` struct implementing `aisix_gateway::Bridge` with
a clear `BridgeError::Config` returning the tracking issue #
on every chat / chat_stream call (skeleton contract — an
operator who reaches this path knows it's intentional WIP, not
silently broken)
- `VertexPublisher` enum (Google / Anthropic / Meta / Mistral /
Ai21) with `from_upstream_id()` and `url_segment()` helpers per
the LiteLLM `vertex_ai/` multi-publisher single-entry pattern —
a single `google-vertex` catalog provider dispatches by model
prefix at runtime inside the bridge instead of forcing
customers to register one provider_key per publisher even
though the GCP credential is shared
- Hub registration via `hub.register_family(Adapter::Vertex, ...)`
in `build_hub()` — the new two-tier dispatch wire (D1) can now
see Vertex; the legacy `Provider`-keyed path is unchanged
Tests (10 passing):
- publisher prefix resolution for gemini / claude / llama /
mistral / codestral / jamba
- case-insensitive matching on the model name
- unknown prefix returns None (drives the publisher-resolution
guard in chat / chat_stream)
- url_segment matches Vertex API path conventions (Mistral's
Vertex publisher tag is `mistralai`, not `mistral` — pinned
to catch a typo regression)
- chat() surfaces clear "not yet implemented + issue #302" error
- chat() with unknown publisher prefix errors before dispatch
- bridge.name() == "vertex" (metrics label stability)
- wire.rs reserved query params (`alt`, `key`, `access_token`)
covered for the eventual default-headers / default-body-fields
override guard
References (per CLAUDE.md §7):
- LiteLLM vertex_ai/ design — single prefix, internal publisher
dispatch: https://github.com/BerriAI/litellm/tree/main/litellm/llms/vertex_ai
- Vertex AI REST API — https://cloud.google.com/vertex-ai/docs/reference/rest
- Vertex publishers index — https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models
Out of scope, tracked under #302 Phase E follow-ups:
- D5.1 google-cloud-auth / yup-oauth2 bearer token acquisition
(service account JSON key, ADC, metadata server)
- D5.2 Gemini publisher dispatch (`:streamGenerateContent`)
- D5.3 Anthropic-on-Vertex dispatch (`:streamRawPredict`)
- D5.4 Llama / Mistral / AI21 publisher dispatch
- D5.5 BridgeContext.deadline + Retry-After plumbing
CopilotAI review requested due to automatic review settings May 17, 2026 05:11
@coderabbitai

coderabbitaiBot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@moonming has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 42923edd-0876-468f-9062-d80a0ffa7c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7edfd and acf779e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/aisix-provider-vertex/Cargo.toml
  • crates/aisix-provider-vertex/src/bridge.rs
  • crates/aisix-provider-vertex/src/lib.rs
  • crates/aisix-provider-vertex/src/wire.rs
  • crates/aisix-server/Cargo.toml
  • crates/aisix-server/src/main.rs

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… LOW-2
HIGH-1 — Meta.url_segment() pointed at non-existent path. LiteLLM's
vertex_llm_base.py:277 routes Llama through the OpenAPI shim at
`endpoints/openapi/chat/completions`, NOT `publishers/meta/...`.
Returning the latter would 404 on every Llama-on-Vertex request once
dispatch lands.
Fix: change url_segment() return to Option<&'static str> and return
None for Meta. The D5.4 follow-up will build the OpenAPI-shim URL
separately via its own code path rather than synthesizing the wrong
publishers/meta/ form.
HIGH-2 — `llama-` prefix didn't match real Vertex Llama model ids.
LiteLLM vertex_ai_partner_models/main.py:33-34 recognizes Meta via
`META_PREFIX = "meta/"` and the bare-`llama` family. Real Vertex
model ids are `meta/llama-3.3-70b-instruct-maas` and
`llama3-405b-instruct-maas` (note: `llama3-` has a digit immediately
after `llama`, not a hyphen). The `lower.starts_with("llama-")`
matcher rejected both.
Fix: match `lower.starts_with("meta/") || lower.starts_with("llama")`.
Tests updated with the real wire forms.
MEDIUM-1 — Doc comment claimed "follows the LiteLLM vertex_ai/
convention" without acknowledging the gap. LiteLLM also routes
deepseek-ai/*, qwen*, openai/gpt-oss-*, minimaxai/*, moonshotai/*,
zai-org/*. We don't handle any of these today; the operator gets a
clear "publisher unknown" error if they try.
Fix: amended the doc comment to enumerate which prefixes ARE handled
and which are known-missing-for-follow-up. The "publisher unknown"
contract is still correct — just documented honestly now.
MEDIUM-2 — Cutover risk between cp-api enabling `google-vertex` on
the catalog and D5.2 (Gemini dispatch) shipping. Today cp-api's
`isSupportedProvider` gate blocks `provider: "google-vertex"` at
createProviderKey, so the skeleton bridge is unreachable in
production. The moment Phase B loosens that gate, every chat through
a google-vertex PK lands on this NOT-IMPLEMENTED skeleton.
Fix: explicit CUTOVER CAUTION comment in `build_hub()` documenting
the strict order — D5.2 must merge before the cp-api catalog flip.
LOW-2 — `let _ = wire::reserved_query_params()` code smell. Kept
as-is: tests exercise the function directly, but the explicit
reachability in chat() makes the future dispatch PR's diff cleaner
(no "function added but unused" warning when the override apply
path is wired in). Justified inline.
@moonming

Copy link
Copy Markdown
MemberAuthor

Audit response — 2 HIGH + 2 MEDIUM + 1 LOW

Fixed in `50cd6d9`

  • HIGH-1 — `Meta.url_segment()` pointed at non-existent path (`publishers/meta` doesn't exist on Vertex; Llama uses the OpenAPI shim per LiteLLM `vertex_llm_base.py:277`): ✅ changed return to `Option<&'static str>` and return `None` for Meta. D5.4 (Meta dispatch) will build the `endpoints/openapi/chat/completions` URL separately via its own code path. New test `publisher_url_segment_meta_is_none` pins this.

  • HIGH-2 — `llama-` prefix didn't match real Vertex model ids: ✅ now matches `meta/` prefix OR bare `llama` family (per LiteLLM `vertex_ai_partner_models/main.py:33-34`). Tests use real wire forms: `meta/llama-3.3-70b-instruct-maas` and `llama3-405b-instruct-maas`. The audit fixture `llama-3-70b-instruct-maas` was synthetic and never occurred on real Vertex deployments.

  • MEDIUM-1 — Doc comment overclaimed LiteLLM compatibility: ✅ doc comment now enumerates which prefixes ARE handled (gemini / claude / meta+llama / mistral+codestral / jamba) and lists known-missing-for-follow-up (`deepseek-ai/`, `qwen`, `openai/gpt-oss-`, `minimaxai/`, `moonshotai/`, `zai-org/`). Operators hitting unsupported prefixes today get a clean "publisher unknown" error — that contract is correct, just documented honestly now.

  • MEDIUM-2 — Cutover risk (cp-api flip vs D5.2 ship order): ✅ added explicit `CUTOVER CAUTION` comment in `build_hub()` documenting the strict order — D5.2 (Gemini dispatch) must merge before cp-api loosens `isSupportedProvider`.

Kept as justified (LOW-1, LOW-2)

  • LOW-1 — `BridgeError::Config` semantically wrong for "not implemented": kept as-is. Adding a new `BridgeError::Unimplemented` variant ripples through every Bridge impl, the proxy error mapping, the OpenAI-shaped error envelope translator, and the metrics labels. The error message starts with "vertex bridge is not yet implemented" so operators triaging the log line know what happened. Will revisit if/when we need the variant for other reasons.

  • LOW-2 — `let _ = wire::reserved_query_params()` code smell: kept as-is. The explicit reachability call in `chat()` makes the future dispatch PR's diff cleaner — no "function added but unused" warning when the override apply path is wired in. The function is also exercised by direct unit tests in `wire.rs`.

Test results

```
cargo build --workspace # clean
cargo test --workspace --lib # all green (838+ tests including 12 vertex)
cargo clippy --workspace --all-targets # clean (-D warnings)
cargo fmt --check --all # clean
```

All HIGH and MEDIUM either fixed or explicitly justified per CLAUDE.md §8 merge gate.

…el.model_name)
D6 audit on PR #313 surfaced a wire-shape bug that applies equally
to D5's skeleton: chat() and chat_stream() resolved the publisher
from req.model (customer-typed display name) instead of from
ctx.model.model_name (operator-pinned upstream id).
Once dispatch lands in D5.2 (Gemini publisher), the URL builder
would have produced `.../publishers/google/models/<customer-facing-
name>:streamGenerateContent` and 404 on every Vertex request. The
skeleton tests previously pinned the wrong contract (passed
`req.model="gemini-1.5-pro"` so the matcher accidentally got the
right input), so a future dispatch PR would have inherited the
broken wire.
Fix:
- Add upstream_model(ctx) helper mirroring OpenAiBridge.
- chat() / chat_stream() resolve from ctx.model.model_name.
- req.model is now _req and explicitly ignored.
- sample_model() renamed to sample_model_with(model_name) so tests
can pin "display_name differs from model_name" by construction.
- New regression test chat_ignores_req_model_and_uses_ctx_model_name
sets req.model="gpt-4o" (would fail publisher resolution if it
were the source of truth) and asserts the not-implemented stub
fires — proving model_name was the actual input.
- New defense test chat_with_missing_model_name_errors_before_dispatch
ensures Option<String> = None on Model.model_name surfaces a
clear error rather than panicking.
D6 (#313) already fixed; D7 (#314) was authored after D6's audit
landed so it already has the fix.
CopilotAI review requested due to automatic review settings May 17, 2026 05:43
@moonming

Copy link
Copy Markdown
MemberAuthor

Backport: D6 audit HIGH-1 (`req.model` → `ctx.model.model_name`)

D6's audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton — the initial D5 audit missed it.

`chat()` / `chat_stream()` resolved the publisher from `req.model` (customer-typed display name in `/v1/chat/completions`) instead of `ctx.model.model_name` (operator-pinned upstream id). Once D5.2 lands, the URL builder would have produced `.../publishers/google/models/:streamGenerateContent` and 404 on every Vertex request.

Fixed in `acf779e`

  • Added `upstream_model(ctx)` helper mirroring OpenAiBridge.
  • `chat()` / `chat_stream()` now resolve from `ctx.model.model_name`. `req.model` is `_req` and explicitly ignored.
  • `sample_model_with(model_name)` test helper makes the display-name-vs-upstream-id distinction explicit.
  • New `chat_ignores_req_model_and_uses_ctx_model_name` regression test pins the contract — sets `req.model="gpt-4o"` (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires.
  • New `chat_with_missing_model_name_errors_before_dispatch` defense test for the `Option` None case.

Test results

```
cargo test -p aisix-provider-vertex --lib # 13 passed (was 12; 1 new defense test)
cargo clippy --workspace --all-targets # clean
cargo fmt --check --all # clean
```

D6 (#313) already fixed; D7 (#314) was authored after D6's audit so it already had the fix.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@moonming
moonming merged commit d3c2806 into mainMay 17, 2026
11 of 12 checks passed
@moonming
moonming deleted the feat/vertex-bridge-skeleton branch May 17, 2026 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@moonming