feat(inference): add llmisvc_model_provider_resolver by jland-redhat · Pull Request #699 · praxis-proxy/ai

jland-redhat · 2026-08-10T20:38:00Z

Summary

Add llmisvc_model_provider_resolver, porting only the LLMISvc / KServe BBR body-rewrite path from IPP’s model-provider-resolver.
Prefer a configurable model header (default X-Model, aligned with model_to_header), fall back to body "model", and when the value is a publisher ID (publishers/.../models/<name>) rewrite the body "model" to <name> only.
Leave the routing header untouched so KServe can still route on the publisher ID; stash the original ID in llmisvc_model_provider_resolver.publisher_id for metering.
Includes unit tests, example config, integration coverage, and generated filter docs.

Does not port ExternalModel / ExternalProvider resolution, weighted provider selection, Host rewrite, api-format detection, or credential handling.

Sister PR (merge after this)

praxis-extproc: fix ExtProc body-mutation path to set content-length to the mutated body length (avoids Envoy mismatch_between_content_length_and_the_length_of_the_mutated_body / 500).
Link: fix(extproc): sync content-length on body mutate + rustls CryptoProvider opendatahub-io/praxis-extproc#23

Without the ExtProc follow-up, body rewrites that change length will fail in Envoy BUFFERED + header SEND mode even though this filter’s rewrite is correct.

Test plan

Unit tests for rewrite / header preference / body fallback / non-publisher passthrough
Example config + integration tests
Validated on local cluster with publisher-ID model request; upstream returned a completion:

{
  "id": "chatcmpl-efb19481-6952-5be9-9572-ebfb8aa9070b",
  "model": "demo/sim-stream",
  "object": "chat.completion",
  "choices": [
    {
      "finish_reason": "stop",
      "message": {
        "role": "assistant",
        "content": "I am fine, how are you today? ..."
      }
    }
  ]
}

praxis-bot-app · 2026-08-10T20:38:14Z

Missing Signed-off-by: 5113bb3. All commits require sign-off (via git commit --signoff).

praxis-bot-app · 2026-08-10T20:45:16Z

AI tool authorship detected:

d11ef9e: Co-authored-by: Cursor <cursoragent@cursor.com>

Sorry, this project does not accept commits authored by tools as valid.
Commits need to be authored by and signed-off by the human(s) responsible for the PR, with their name and contact.

Signed-off-by: jland <jland@redhat.com>

praxis-bot-app · 2026-08-10T21:14:47Z

Unsigned commits: d03e60a. Please sign your commits.

aslakknutsen · 2026-08-10T22:32:21Z

+        return Some(from_header);
+    }
+
+    obj.get("model")


Why this if model_to_header ?

model_to_header puts the model in the header.

But the model serving needs the header to be a "canonical id" publishers//models/<MODEL_NAME>. So the way BBR works in 3.5 is that we have the user put this value in the body "models" value and we move it into the header and replace it with the real <MODEL_NAME>

This resolver is that second part.

External Models do something similar but replacement will happen based on the ExternalModel CRD.

I mean, this seems to "if it's not in the header as provided by model_to_header", it will fall back to getting the value from where model_to_header would/should have gotten the value.

Hmm maybe we can sync on this so I can better understand but I would think that is ok right?

It is faster to get this from the header and we fallback to the model if it is not there seems reasonable. But I can just remove the fallback if we think that makes more sense.

Agreed after sync — dropped the body fallback. This filter now assumes model_to_header (or an equivalent) already set the routing header. If the header is absent/empty, we no-op and leave the body alone.

aslakknutsen · 2026-08-10T23:09:35Z

+            return Ok(FilterAction::Continue);
+        }
+
+        obj.insert("model".to_owned(), serde_json::Value::String(short_name.to_owned()));


Is there ever a chance that "model" won't be in the request body on a valid request?

Not if it is following the OpenAI spec I don't believe, and it has to follow that spec if it is a vLLM model which should be the only thing that this filter would catch.

Ok. I'm hinting at we could pretty easy do a StringBuffer splice to inject the new model value to avoid the full DOM deserialize and serialize of the complete body. (handling missing model fields is a little bit more tricky, but a replace in a buffer is easy)

What you are pointing at: today we serde_json::from_slice the whole request body into a DOM, mutate the "model" field, then re-serialize with replace_json_body. That is correct but pays a full parse/serialize of the JSON for a single string field change.

A StringBuffer-style splice would instead find the existing "model":"..." bytes in the buffered body and overwrite/replace just that string in place (or with a small rebuild of the surrounding bytes), avoiding a full DOM round-trip. That is attractive for hot paths, especially once we know "model" is always present on OpenAI/vLLM chat completions which is all this should be targeting currently.

For this PR I am keeping the DOM path: we already share replace_json_body with other filters, and with the header-required + no-invent-model guards the edge cases are simpler. Happy to follow up with a targeted byte/string splice later if profiling shows the parse/serialize cost matters here.

Backtick CamelCase identifiers for doc_markdown, shrink tests under too_many_lines, and set metrics_route when building against praxis main. Signed-off-by: jland <jland@redhat.com>

praxis-bot

praxis-bot review

Clean filter implementation with good test coverage, proper config validation, and correct use of replace_json_body. One edge case around body mutation when the "model" field is absent.

Findings: 1 medium

praxis-bot · 2026-08-11T12:38:11Z

+            return Ok(FilterAction::Continue);
+        }
+
+        obj.insert("model".to_owned(), serde_json::Value::String(short_name.to_owned()));


[Medium] When resolve_model_name obtains the model from the header (not the body), the body might not contain a "model" key at all. In that case obj.insert(...) silently adds a "model" field the caller never sent, which could surprise backends that do not expect one (e.g. non-completions endpoints that happen to share a pipeline).

Guard the insert so it only rewrites an existing field:

if !obj.contains_key("model") { return Ok(FilterAction::Continue); } obj.insert("model".to_owned(), serde_json::Value::String(short_name.to_owned()));

Add a unit test: header carries a publisher ID, body is {"messages":[]} (no "model"), assert the body is unchanged after the filter runs.

Fixed in the latest push: we only rewrite when the body already has a "model" key; otherwise no-op (no invented field). Also added a unit test for header publisher ID + body without "model".

praxis-bot

praxis-bot review (round 2)

Fix commit is clean: backtick formatting, CI metrics_route field, and test loop refactor all look correct. One nit below. The prior Medium (body mutation when "model" absent) remains open.

Findings: 1 nit

praxis-bot · 2026-08-12T18:50:32Z

+        assert_eq!(
+            llmisvc_short_model_name("publishers/ns/models/a/b"),
+            Some("a/b"),
+            "SplitN keeps remainder after first /models/"


[Nit] Assertion message says SplitN but the implementation (line 279) uses split_once. They are semantically similar, but the message should match the actual method for accuracy.

Fixed — assertion message now says split_once.

praxis-bot

praxis-bot review (round 3)

Clean implementation with good separation of concerns. Config validation (deny_unknown_fields, empty-header rejection, validate_max_body_bytes), the header-vs-body resolution chain, and the split_once-based publisher-ID parsing are all correct. Integration tests exercise the core rewrite, routing-header preservation, and non-publisher passthrough end-to-end. The register.rs test refactor to a loop is a nice cleanup.

The prior Medium from round 1 (body mutation when "model" absent -- obj.insert(...) adds a field the caller never sent) remains the only actionable concern and is not yet addressed.

Findings: 0 new (prior Medium still open)

Resolve metrics_route test-utils conflict (now unconditional on 0.5.2) and keep both inference_fallback and llmisvc example mods. Signed-off-by: jland <jland@redhat.com>

Assume model_to_header already set the routing header; no-op when it is missing. Do not invent a body model field, and rustfmt the llmisvc tests. Signed-off-by: jland <jland@redhat.com>

Signed-off-by: jland <jland@redhat.com>

jland-redhat requested review from a team and leseb August 10, 2026 20:38

jland-redhat mentioned this pull request Aug 10, 2026

fix(extproc): sync content-length on body mutate + rustls CryptoProvider opendatahub-io/praxis-extproc#23

Open

3 tasks

Jaland force-pushed the llmisvc_model_provider_resolver branch from 5113bb3 to d11ef9e Compare August 10, 2026 20:45

Adding nnew llmisvc_model_provider_resolver

b0d0e9a

Signed-off-by: jland <jland@redhat.com>

Jaland force-pushed the llmisvc_model_provider_resolver branch from d11ef9e to b0d0e9a Compare August 10, 2026 20:46

jland-redhat changed the title ~~Adding nnew llmisvc_model_provider_resolver~~ feat(inference): add llmisvc_model_provider_resolver Aug 10, 2026

Jaland force-pushed the llmisvc_model_provider_resolver branch from b53a455 to d03e60a Compare August 10, 2026 21:14

aslakknutsen reviewed Aug 10, 2026

View reviewed changes

fix(llmisvc): clippy docs and praxis-main CI

8f9c832

Backtick CamelCase identifiers for doc_markdown, shrink tests under too_many_lines, and set metrics_route when building against praxis main. Signed-off-by: jland <jland@redhat.com>

Jaland force-pushed the llmisvc_model_provider_resolver branch from d03e60a to 8f9c832 Compare August 11, 2026 00:45

praxis-bot reviewed Aug 11, 2026

View reviewed changes

jland-redhat mentioned this pull request Aug 11, 2026

Do Not Merge: Praxis PoC opendatahub-io/models-as-a-service#1360

Open

praxis-bot reviewed Aug 12, 2026

View reviewed changes

praxis-bot reviewed Aug 13, 2026

View reviewed changes

jland-redhat added 3 commits August 14, 2026 12:00

Merge upstream/main into llmisvc_model_provider_resolver

a0cae0a

Resolve metrics_route test-utils conflict (now unconditional on 0.5.2) and keep both inference_fallback and llmisvc example mods. Signed-off-by: jland <jland@redhat.com>

fix(llmisvc): require model header, no body fallback

8656cdd

Assume model_to_header already set the routing header; no-op when it is missing. Do not invent a body model field, and rustfmt the llmisvc tests. Signed-off-by: jland <jland@redhat.com>

Cleaning up some documentation

b6c729b

Signed-off-by: jland <jland@redhat.com>

Jaland force-pushed the llmisvc_model_provider_resolver branch from a1d79f9 to b6c729b Compare August 14, 2026 16:50

Uh oh!

Conversation

jland-redhat commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Sister PR (merge after this)

Test plan

Uh oh!

praxis-bot-app Bot commented Aug 10, 2026

Uh oh!

praxis-bot-app Bot commented Aug 10, 2026

Uh oh!

praxis-bot-app Bot commented Aug 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praxis-bot left a comment

Choose a reason for hiding this comment

praxis-bot review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praxis-bot left a comment

Choose a reason for hiding this comment

praxis-bot review (round 2)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praxis-bot left a comment

Choose a reason for hiding this comment

praxis-bot review (round 3)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jland-redhat commented Aug 10, 2026 •

edited

Loading