feat(ai-proxy): pure helpers for the post-override effective request#13370
Open
janiussyafiq wants to merge 7 commits into
Open
feat(ai-proxy): pure helpers for the post-override effective request#13370janiussyafiq wants to merge 7 commits into
janiussyafiq wants to merge 7 commits into
Conversation
Move the three-step instance-override application (options flat overwrite, override.llm_options capability hook, override.request_body deep merge) out of the inline block in ai-providers/base.lua build_request and into a new pure helper in apisix/plugins/ai-proxy/base.lua. build_request calls the helper at the same point the inline code lived (post-converter), so the body sent upstream is unchanged. extra_opts no longer carries the four override-derived fields; it passes the picked ai_instance through and the helper reads from it directly. Zero behavior change. Motivation: ai-cache (planned follow-up plugin) needs to compute its cache key from the post-override effective body without going through build_request, which performs the upstream HTTP call, signing, and keepalive.
dosubot
Bot
added
size:M
5 tasks
…elpers Two pure helpers on top of apply_instance_overrides (introduced in the preceding refactor), both in apisix/plugins/ai-proxy/base.lua: - effective_model(ctx) returns ai_instance.options.model when the operator forces a model on the instance, falling back to ctx.var.request_llm_model (the client-supplied model that detect_request_type mirrors). - effective_request_for_cache(ctx) returns the request body as it would be sent upstream: reads the parsed body, resolves the target protocol from ctx.ai_client_protocol against the provider's capabilities (so peer plugins running in access phase before before_proxy can still get the post-override view), and applies apply_instance_overrides. A small internal resolve_target_protocol helper mirrors the routing logic in before_proxy so callers don't have to wait for ctx.ai_target_protocol to be populated. These helpers exist for ai-cache (planned follow-up) to compute a cache key over the effective body without invoking build_request (which would make the upstream HTTP call). The signatures are pure and ctx-driven. Test: t/plugin/ai-proxy-request-body-override.t TEST 17 drives a real request through ai-proxy with options + override.request_body, then uses serverless-post-function (priority -2000, runs after ai-proxy access at 1040) to invoke both helpers and log their output. Asserts both the upstream-received body AND the helper outputs reflect the same post-override view.
dosubot
Bot
added
size:L
janiussyafiq
changed the title
effective_model duplicates information already present on the body that effective_request_for_cache returns (ai_instance.options.model is written onto the body during apply_instance_overrides step 1). Callers that need the model can read it off the effective body. A cheap ctx-only model lookup can be added later if a concrete consumer needs it without parsing the body. Updates TEST 17 to drop the EFFECTIVE_MODEL assertion; the EFFECTIVE_BODY assertions still prove the helper produces the same body the upstream receives.
The cache key produced via effective_request_for_cache should reflect what would actually be sent upstream. Previously the helper only applied apply_instance_overrides, so if a converter was in the chain (e.g. anthropic-messages client routed to an openai-chat provider) the helper returned the pre-converter body while build_request sent the converted body - the cache key would diverge from the upstream request shape. Now the helper: 1. Reads the request body 2. Resolves (target_protocol, converter) via resolve_target_protocol 3. Applies the converter when present 4. Applies apply_instance_overrides resolve_target_protocol's return signature widens from `target_protocol` to `(target_protocol, converter)`; the fast-path (ctx.ai_target_protocol already set) returns ctx.ai_converter alongside. Tests: - TEST 17 (no-converter path) reformatted - the inline serverless-post- function was a single 297-char line; broken into a readable multi-line body to match the style used elsewhere in the file. - TEST 18 added covering the converter path: anthropic-messages client to an openai provider. Asserts EFFECTIVE_BODY contains max_completion_tokens (post-converter rename of max_tokens) and temperature 0.42 (post-override), but NOT the original max_tokens field - proving the converter ran inside the helper. Drive-by: comment on apply_instance_overrides shortened from 11 lines to 5 (precedence rules + "mutates in place"). Other two helpers keep their longer docs.
The b796b9d refactor changed build_request to read overrides from opts.ai_instance, but the ai-request-rewrite sidecar caller was missed and kept passing the now-dead opts.model_options. Result: conf.options silently stopped propagating to the LLM sidecar request body. Fix: pass ai_instance = conf. conf has the same .options / .override shape apply_instance_overrides reads; the override.llm_options / request_body branches are no-ops since the rewrite schema only defines override.endpoint. t/plugin/ai-request-rewrite2.t TEST 1, which validates extra_option in the LLM-stub request body, now passes (was failing with status 400 "LLM service returned error status: 400" once httpbin is reachable).
Previously only checked status=200 plus the EFFECTIVE_BODY error_log regex. The cache-key correctness contract requires the helper's output to match what build_request actually sends upstream — but with no upstream-side assertion, the test would have passed even if the helper diverged from build_request as long as the helper's own log contained the expected fields. Decode body.content[1].text (the openai-chat body echoed by the /v1/chat/completions stub, surfaced through the converter's response transform) and assert max_completion_tokens=10, temperature=0.42, max_tokens=nil. Combined with the existing EFFECTIVE_BODY regex on the same fields, this pins down helper == upstream for the converter's distinctive markers. Mirrors TEST 17's structure.
janiussyafiq
force-pushed
the
refactor/ai-proxy-apply-instance-overrides
branch
from
2cd96b3 to
1d8a9a2
Compare
There was a problem hiding this comment.
Pull request overview
This PR extracts AI proxy request-body override handling into reusable helpers so future plugins can compute the effective upstream request body before proxying.
Changes:
- Adds
apply_instance_overridesandeffective_request_for_cachehelpers inai-proxybase logic. - Updates provider request building to delegate override application to the new helper.
- Fixes
ai-request-rewriteto pass the AI instance/config through the updated provider request contract. - Adds regression tests for effective post-conversion/post-override request bodies.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
apisix/plugins/ai-proxy/base.lua |
Adds effective request-body helper and routes override application through shared logic. |
apisix/plugins/ai-providers/base.lua |
Delegates request override application to ai-proxy.base. |
apisix/plugins/ai-request-rewrite.lua |
Passes plugin config as ai_instance for provider request construction. |
t/plugin/ai-proxy-request-body-override.t |
Adds tests covering effective request body after overrides and conversion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+199
to
+200
| return _M.apply_instance_overrides( | ||
| request_body, ai_instance, ai_provider, target_protocol) |
Comment on lines
+185
to
+191
| end | ||
| local ok, ai_provider = pcall(require, | ||
| "apisix.plugins.ai-providers." .. ai_instance.provider) | ||
| if not ok then | ||
| return nil, "failed to load provider: " .. tostring(ai_instance.provider) | ||
| end | ||
| local target_protocol, converter = resolve_target_protocol(ctx, ai_provider) |
| core.table.try_read_attr(ai_instance, "override", "request_body"), | ||
| request_body_force_override = | ||
| core.table.try_read_attr(ai_instance, "override", "request_body_force_override"), | ||
| ai_instance = ai_instance, |
| endpoint = core.table.try_read_attr(conf, "override", "endpoint"), | ||
| auth = conf.auth, | ||
| model_options = conf.options, | ||
| ai_instance = conf, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Setup for the upcoming
ai-cacheplugin. ai-cache needs the request body as it would be sent upstream — post-converter, post-override — to compute a cache key. Today that view only exists insidebuild_request, which isn't callable in isolation (HTTP, signing, auth). This PR extracts it as a pure helper.effective_request_for_cache(ctx)inapisix/plugins/ai-proxy/base.luareturns the post-converter, post-override body. The inline override block inbuild_requestis extracted to a pureapply_instance_overridesand called at the same call site — zero behavior change.Two wrinkles worth noting:
build_request. Without the converter step, the helper would return a pre-converter body and ai-cache would hash a different shape than what hits upstream.resolve_target_protocolmirrors the routing inbefore_proxyso ai-cache (priority 1035, access phase) can compute the view beforebefore_proxypopulatesctx.ai_target_protocol.The
ai-request-rewritechange is a regression fix surfaced in review: the refactor changedbuild_request'sextra_optscontract but missed the sidecar caller, which kept passing a now-dead field. Passai_instance = confinstead.Which issue(s) this PR fixes:
N/A — new internal API surface for the upcoming
ai-cacheplugin.Behavior change
None for
ai-proxy/ai-proxy-multi. Theai-request-rewritechange restoresconf.optionspropagation that the refactor inadvertently broke.Checklist