feat: pass extra body fields from Lua to HTTP request#262
Conversation
Add serde(flatten) to Args so Lua can pass arbitrary extra fields (e.g. OpenRouter's `models` array or `provider` routing options) through to the HTTP request body. OpenAI-compatible providers merge them; others ignore.
|
|
|
|
📝 WalkthroughWalkthroughThe pull request adds extensible field-merging capabilities to LLM providers via a new configurable merge utility module, extends the Prompt structure with extra fields for provider customization, updates API documentation with changelog reorganization and floating-point behavior guidance, refines logging output formats, and improves type annotations across Lua bindings and test infrastructure. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@modules/implementation/src/llm/ctx.rs`:
- Around line 99-101: The long method call on
`zelf.exec_prompt_in_provider(&args.prompt, &args.model, &args.provider,
args.format, &args.extra).await` exceeds the line-length limit; split the
arguments across multiple lines or assign some arguments to short local
variables before the call so the chain fits formatting. Locate the call in the
`ctx.rs` function where `zelf` invokes `exec_prompt_in_provider` and reformat it
into a multi-line method chain (one argument per line or grouped logically) or
introduce temporaries for `args.prompt`, `args.model`, `args.provider`,
`args.format`, `args.extra` and then call `exec_prompt_in_provider(...) .await`
to satisfy `cargo fmt`.
In `@modules/implementation/src/llm/providers.rs`:
- Around line 106-112: merge_extra currently unconditionally overwrites existing
JSON keys which allows extras to replace core fields like model, messages,
stream, temperature, etc.; change merge_extra (function name: merge_extra) to
only add keys that do not already exist by using the map entry API (e.g.,
obj.entry(k.clone()).or_insert(v.clone())) so extras are purely additive and
cannot overwrite existing request fields or bypass safety controls.
- Around line 134-141: The line calling self.exec_prompt_json_as_text in
function exec_prompt_json exceeds the project's line-length limit; reformat that
call so it wraps arguments across multiple lines (e.g., place each parameter
ctx, prompt, model, extra on its own line and align the .await on the next line)
to satisfy rustfmt—update the expression using the exec_prompt_json_as_text
method and the let res = ... .await assignment so rustfmt no longer flags the
line.
🧹 Nitpick comments (1)
modules/implementation/src/llm/ctx.rs (1)
77-85:#[serde(flatten)]silently swallows typos in known field names.If Lua sends
{ provder = "x", ... }(typo), it won't fail deserialization — the misspelled key lands inextraand the actualproviderfield triggers a missing-field error only if it's required. But for optional-like scenarios or debugging, this can be confusing. Consider adding a log/trace whenextrais non-empty so unexpected keys are visible during development.This is an acceptable trade-off for extensibility, but worth documenting.
|
I am taking this one from here, thanks for the PR |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@modules/install/lib/genvm-lua/lib-web.lua`:
- Line 4: The WebRequestPayload type alias omits the standard "PUT" HTTP method;
update the type alias WebRequestPayload to include "PUT" in the method union
(or, if PUT is intentionally disallowed, add an inline comment on the
WebRequestPayload alias explaining why PUT is excluded) so the method list
correctly reflects allowed methods or documents the intentional restriction.
🧹 Nitpick comments (3)
modules/implementation/src/llm/merge.rs (2)
69-75:merge_extrais a trivial wrapper overmerge_values.Currently these are identical. If the intent is to provide a stable public API for the "merge extra fields into request" use case, consider adding a doc comment clarifying why both exist. Otherwise, callers could just use
merge_valuesdirectly.
97-104:Nonestrategy silently replaces on type mismatch (e.g., object→scalar or scalar→object).The catch-all arm on line 102 means that if the source is a different JSON type than the target (e.g., source is an array and target is an object, or vice versa), the target is silently replaced. This is fine for scalar→scalar, but type-changing replacements could mask configuration errors. Consider whether a warning or error for object↔non-object mismatches would be appropriate.
doc/website/src/api/index.rst (1)
14-17: Documentation changes appear unrelated to PR objectives.The PR objectives describe adding extra body fields support from Lua to HTTP requests (Rust FFI boundary changes). However, these documentation updates add
boot_process,floating_point, and reorganize the changelog structure, which don't appear to relate to the core feature described in the PR objectives. Consider moving these documentation changes to a separate PR to maintain focused, single-purpose changes.
|
|
||
| ---@alias WebRenderPayload { url: string, mode: "text" | "html" | "screenshot", wait_after_loaded: number } | ||
| ---@alias WebRequestPayload { url: string, method: string, headers: table<string, string>, body: string?, sign: boolean? } | ||
| ---@alias WebRequestPayload { url: string, method: "GET" | "POST" | "HEAD" | "DELETE" | "OPTIONS" | "PATCH", headers: table<string, string>, body: string?, sign: boolean? } |
There was a problem hiding this comment.
PUT is missing from the HTTP method union.
The type lists "GET" | "POST" | "HEAD" | "DELETE" | "OPTIONS" | "PATCH" but omits "PUT", which is a standard and widely-used HTTP method. If this is intentional (i.e., PUT requests are deliberately disallowed), please add a comment explaining why; otherwise, add it to the union.
Proposed fix
----@alias WebRequestPayload { url: string, method: "GET" | "POST" | "HEAD" | "DELETE" | "OPTIONS" | "PATCH", headers: table<string, string>, body: string?, sign: boolean? }
+---@alias WebRequestPayload { url: string, method: "GET" | "POST" | "PUT" | "HEAD" | "DELETE" | "OPTIONS" | "PATCH", headers: table<string, string>, body: string?, sign: boolean? }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ---@alias WebRequestPayload { url: string, method: "GET" | "POST" | "HEAD" | "DELETE" | "OPTIONS" | "PATCH", headers: table<string, string>, body: string?, sign: boolean? } | |
| ---@alias WebRequestPayload { url: string, method: "GET" | "POST" | "PUT" | "HEAD" | "DELETE" | "OPTIONS" | "PATCH", headers: table<string, string>, body: string?, sign: boolean? } |
🤖 Prompt for AI Agents
In `@modules/install/lib/genvm-lua/lib-web.lua` at line 4, The WebRequestPayload
type alias omits the standard "PUT" HTTP method; update the type alias
WebRequestPayload to include "PUT" in the method union (or, if PUT is
intentionally disallowed, add an inline comment on the WebRequestPayload alias
explaining why PUT is excluded) so the method list correctly reflects allowed
methods or documents the intentional restriction.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
3 participants
Summary
#[serde(flatten)]to theArgsstruct inexec_prompt_in_providerso Lua can pass arbitrary extra fields through to the HTTP request bodyExtraBodytype alias andmerge_extrahelper for merging extra fields into JSON request bodiesextraparameter through theProvidertrait methodsOpenAICompatibleprovider merges extra fields into the HTTP body (enables OpenRouter'smodelsarray andproviderrouting options)Gemini,OLlama,Anthropic) accept but ignore the parameterMotivation
OpenRouter supports native fallback via two mechanisms:
"models": ["model-a", "model-b"]— cross-model fallback"provider": {"allow_fallbacks": true}— same-model, cross-provider routingCurrently the Lua→Rust FFI boundary (
Argsstruct) only accepts{provider, model, prompt, format}, so Lua can't pass these OpenRouter-specific fields through. This change makes the boundary extensible without requiring Rust changes for each new provider feature.Usage (Lua side)
After this change,
llm.luacan pass extra fields:Test plan
&serde_json::Map::new()for the new parameter)Summary by CodeRabbit
Release Notes
New Features
Documentation
Bug Fixes
Tests