AlinsRan commented on code in PR #13676: URL: https://github.com/apache/apisix/pull/13676#discussion_r3575243417
########## apisix/plugins/ai-proxy/embedding.lua: ########## @@ -0,0 +1,139 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- + +-- Batch embedding client for the semantic balancer. Routes the request through +-- the shared ai-provider layer (apisix.plugins.ai-providers.*) so endpoint +-- resolution, authentication and HTTP transport match the chat path, instead of +-- re-implementing them here. One call embeds a list of texts (OpenAI-compatible +-- /embeddings: input is an array), returning a vector per text. Keeps semantic +-- routing free of a vector database. +local core = require("apisix.core") +local ipairs = ipairs +local type = type +local tostring = tostring +local require = require +local pcall = pcall + +local _M = {} + +local EMBEDDINGS_PROTOCOL = "openai-embeddings" + + +-- Resolve a provider's embeddings endpoint path from its capabilities. +local function embeddings_path(ai_provider, provider_name) + local cap = ai_provider.capabilities and ai_provider.capabilities[EMBEDDINGS_PROTOCOL] + if not cap then + return nil, "provider " .. tostring(provider_name) .. " does not support embeddings" + end + local path = cap.path + if type(path) == "function" then + path = path() + end + return path +end + + +-- Fetch embeddings for `texts` in a single batch request. +-- Returns an array of vectors index-aligned with `texts`, or nil + err. +function _M.fetch(conf, texts) + if not texts or #texts == 0 then + return {} + end + + if not conf.model or conf.model == "" then + return nil, "embedding model is not configured" + end + + local ok, ai_provider = pcall(require, "apisix.plugins.ai-providers." .. conf.provider) + if not ok then + return nil, "failed to load ai-provider: " .. tostring(conf.provider) + end + + local target_path, perr = embeddings_path(ai_provider, conf.provider) + if not target_path then + return nil, perr + end + + -- The embedding call is a self-contained sidecar request; use a throwaway + -- ctx so it never reads or mutates the in-flight request's ai_* fields. + -- ai_request_body_changed = true tells build_request to send our + -- { model, input } body instead of reusing the client's request body. + local ctx = { ai_request_body_changed = true } + local extra_opts = { + endpoint = conf.endpoint, + auth = conf.auth, + target_path = target_path, + -- This call carries its own credentials; never forward the client's + -- headers (Authorization, Cookie, ...) to the embedding provider. + skip_client_headers = true, + } + local req_conf = { + ssl_verify = conf.ssl_verify ~= false, + timeout = conf.timeout or 10000, + keepalive = true, + } + + local status, raw_body, err = ai_provider:request(ctx, req_conf, + { model = conf.model, input = texts }, extra_opts) + -- The upstream body never reaches a log line or a returned error. It is + -- untrusted third-party content that may echo the prompt back, be arbitrarily + -- large, or forge log lines with newlines. Report the status and a bounded, + -- sanitized summary instead. + if status ~= 200 then + if err then + return nil, "embedding request failed: " .. err + end + return nil, "embedding endpoint returned status " .. tostring(status) + end + + -- A 200 body that decodes to a scalar, boolean or null must not be indexed + -- (`data.data` on a number raises). `derr` is cjson's own parse error, which + -- is bounded and carries no body content. + local data, derr = core.json.decode(raw_body) + if type(data) ~= "table" or type(data.data) ~= "table" then + return nil, "invalid embedding response: " .. (derr or "unexpected shape") + end + + local vectors = {} + for i, item in ipairs(data.data) do + -- OpenAI returns an `index`; fall back to positional order. + local idx = (type(item.index) == "number") and (item.index + 1) or i + local emb = item.embedding + if type(emb) ~= "table" or #emb == 0 then + return nil, "invalid embedding vector at index " .. (idx - 1) + end Review Comment: Confirmed and fixed in bbb4318 — `resty -e` reproduces it: a 200 body of `{"data":[1,2,3]}` makes `item.index` raise `attempt to index a number value`, which escapes `pick_semantic_instance` as a 500. Same class as the scalar top-level body, one level deeper. ```lua for i, item in ipairs(data.data) do if type(item) ~= "table" then return nil, "invalid embedding entry at index " .. (i - 1) end ``` Added a routing test whose mock returns exactly that payload and asserts the request falls back to the catchall instead of 500ing. ########## apisix/plugins/ai-proxy-multi.lua: ########## @@ -598,9 +641,174 @@ local function pick_target(ctx, conf, ups_tab) end +local function extract_last_user_message() + local body = core.request.get_json_request_body_table() + if not body or type(body.messages) ~= "table" then + return nil + end + for i = #body.messages, 1, -1 do + local m = body.messages[i] + if type(m) == "table" and m.role == "user" then + local content = m.content + if type(content) == "string" then + return content + elseif type(content) == "table" then + -- multimodal content: concatenate the text parts so routing + -- still works for {type=text|image_url,...} arrays. + local parts = {} + for _, p in ipairs(content) do + if type(p) == "table" and p.type == "text" + and type(p.text) == "string" then + parts[#parts + 1] = p.text + end + end + if #parts > 0 then + return table_concat(parts, " ") + end + end + end + end + return nil +end + + +-- Embed every instance's examples in one batch and group the normalized +-- reference vectors by instance name. Raises on embedding failure so the +-- lrucache below does not cache a bad result. +local function build_instance_vectors(conf) + local texts = {} + local owners = {} + for _, inst in ipairs(conf.instances) do + if inst.examples then + for _, ex in ipairs(inst.examples) do + texts[#texts + 1] = ex + owners[#texts] = inst.name + end + end + end Review Comment: Good catch — fixed in bbb4318. It is not just cosmetic: with `examples` on the catchall it gets reference vectors and enters `ranked`, so it can outrank a legitimate instance (both above threshold, catchall scores higher → catchall wins, where without examples the other instance would have been picked). It also contradicted the documented contract that the catchall carries no similarity score. Went with the schema-validation option rather than silently skipping the examples, so the mistake surfaces at config time instead of quietly doing nothing: ``` instance 'fallback': `catchall` instance must not configure `examples`; it is the fallback target and does not take part in ranking ``` Schema description and docs (en/zh) updated to match, plus a negative schema test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
