Skip to content

feat(llm): opt-in strict function tools for OpenAI-compatible providers - #402

Open
plombeer31 wants to merge 3 commits into
mainfrom
feat/strict-tools-provider-flag
Open

feat(llm): opt-in strict function tools for OpenAI-compatible providers#402
plombeer31 wants to merge 3 commits into
mainfrom
feat/strict-tools-provider-flag

Conversation

@plombeer31

Copy link
Copy Markdown
Collaborator

The report

Discord #feedback-and-bugs, thegreatteacher, 2026-09-10:

I was testing mercury 2.5 from inception labs with atomic-agent. Unfortunately, the model makes a lot of tool calling mistakes without "strict": true option. Is it possible to set "strict": true through config file or do you need to add provider specific patch into atomic-agent?

Maintainer's reply:

I believe we'll need a patch to let user configure model specific parameters.

Why extraBody could not already do it

extraBody merges at the top level of the chat-completion body. OpenAI's strict mode is not a top-level field — it is tools[].function.strict, one flag per tool. And tools is in RESERVED_BODY_KEYS (src/llm/provider/openai/openai-build-body.ts:10), re-applied over the merge specifically so a passthrough can never drop the tool contract. So an operator could put strict anywhere in extraBody and it would be discarded. There was no config path to it at all.

The ToolsSupportLevel union already has a "strict" member (llm-provider.ts:35), userModels[].supportsTools accepts it (llm-config.ts:392, config-schema.ts:954), and it survives a config round-trip — but nothing consumes it. The only reads anywhere are entry.supportsTools === "none" for a display tag (format-model-details.ts:42, model-search.ts:63). This PR does not wire that level; it stays dormant. See the note on #395 below.

The knob

strictTools on the provider entry — config v63, additive, off by default:

"llm": { "providers": [{
  "id": "mercury", "kind": "openai-compatible",
  "baseUrl": "https://api.inceptionlabs.ai/v1",
  "defaultChatModel": "mercury", "strictTools": true
}] }

Parsed alongside supportsTools / maxOutputTokens in llm-config.ts, carried on LlmProviderConfigEntry, and handed to OpenAiProvider by all five OpenAI-shaped factories (openai-compatible, qwen-openai-compatible, openrouter, aimlapi, gemini — every one of them extends OpenAiProvider). It reaches buildOpenAiChatBody as a sixth argument, exactly the seam extraBody and maxOutputTokens already use, and the transform runs before the extraBody merge so the reserved-key restore keeps the transformed array. Absent, the request body is byte-identical to today.

The transform (openai-strict-tools.ts)

Getting this wrong is worse than the bug: a schema strict mode rejects 400s the whole request, not the one tool. Applied recursively to every object schema:

  • additionalProperties: false;
  • every key of properties present in required — an optional parameter is widened to include "null" (and null added to its enum members where it has an enum), never omitted from required;
  • keyword allowlist, not a blocklist.

Kept verbatim: type, enum, description, title, $ref (+ $defs, recursed). Recursed: properties, items, anyOf. oneOf is renamed to anyOf — interchangeable for a constrained decoder, and anyOf is the spelling strict mode documents. Recomputed: required, additionalProperties.

What I stripped, and why. I scanned every keyword present at a schema position across all 85 bundled descriptors (default-tool-args-schemas.ts + github-tool-args-schemas.ts + the inline reply/finish schemas). The full set in use is type, properties, required, additionalProperties, enum, items, anyOf, minItems, maxItems, minimum — plus minLength on reply.text. So the only things actually stripped from this repo's own schemas are minItems, maxItems, minimum, minLength (fusion.delegate, vision.describe, reply). That is safe for the reason default-tool-args-schemas.ts states in its own header: these schemas guard the shape, and the runtime validators — not the provider — do the value-range checks. An empty reply.text is still rejected by validateBatch and routed to the repair prompt. Dropping a bound loosens the schema; it never invalidates a call the agent would otherwise have accepted.

The allowlist also covers what an MCP server's inputSchema may carry and this repo does not (pattern, format, default, examples, const, uniqueItems, multipleOf, exclusive*, $schema, …): those are dropped too, so a new keyword degrades to "ignored" rather than "sent and 400'd".

Refused rather than mangled. A tools array may mix strict and non-strict functions, so anything that cannot be expressed is emitted with strict: false and its schema untouched:

refused why
os.http.request headers is additionalProperties: {type:"string"} and body's object branch is a bare {type:"object"} — both free-form maps carrying the actual payload
mcp.prompt.get arguments is a server-defined string map, same shape
any descriptor with no argsJsonSchema descriptorToJsonSchema's fallback is {type:"object", properties:{}, additionalProperties:true}

That is the answer to the additionalProperties: true + empty properties question: closing it would leave a schema admitting only {}, i.e. silently deleting the tool's arguments — so the tool travels unconstrained instead. An object that declares properties and additionalProperties: true (os.fs.archive.extract.limits, the sole case) is closed: the three declared keys are its whole documented contract, so forbidding extras loses nothing a caller was entitled to send. Also refused: allOf/not/if/then/else, tuple items, an array with no items, a non-object root, a schema with nothing constraining it.

Result: 80 of 82 emitted tools are strict, the two above are not.

The response side (the part that is not just schema shuffling)

Making optionals nullable-and-required changes what the model sends: "userName": null where it used to omit the key. Several tools branch on presence, not value — os.git.init (args.userName !== undefined), memory.profile.set (rawArgs.pinned, rawArgs.keywords) — and would take the "caller asked for this" branch with nothing to put in it. So on a provider that opted in, OpenAiProvider wraps its tool-call adapter to drop top-level null arguments, leaving the parsed call identical to what those tools saw before. Nested nulls are left alone: that is data the model meant to send. Nothing changes for a provider without the flag.

Tests

src/llm/provider/openai/openai-strict-tools.test.ts (110 tests):

  • The conformance sweep that matters: every descriptor in DEFAULT_TOOL_DESCRIPTORS goes through descriptorsToOpenAiTools (the same adapter a real turn uses) and then the transform, and each result is checked against every strict rule recursively — keyword allowlist, additionalProperties: false, required ⊇ and ⊆ properties, array items, anyOf branches, $defs. One it() per tool, so a failure names the tool. A new tool with an unsupported keyword fails here instead of as a field 400.
  • The refusal list is pinned (["os__http__request", "mcp__prompt__get"]), so a new free-form-map tool is a decision, not a surprise.
  • Table-driven unit cases: optional → nullable + required, optional enum gets null in its members, optional anyOf gains a null branch, nested objects, arrays of objects, the strip list, oneOfanyOf, zero-argument tool, and six refusal shapes.
  • Idempotence, over both the fixtures and the whole catalog: f(f(x)) deep-equals f(x). Non-mutation of the input schema.
  • The null-argument drop, top-level and nested.

openai-build-body.test.ts (+4): flag absent ⇒ JSON.stringify(body) identical to the pre-flag call including key order; flag on ⇒ tools rewritten and marked; the transformed array survives an extraBody that tries to replace tools; no tools key when the request carries none.

llm-config.test.ts (+4): strictTools round-trips true/false, absent by default, rejects non-booleans. 62 added to ACCEPTED_CONFIG_VERSIONS with the v63 bump.

npm run lint                                  # tsc --noEmit, clean
npx vitest run src/llm/provider src/config    # 60 files, 936 tests, all pass

Proof the tests are not vacuous

Stash the src change, keep the tests. Reverting the six modified src files and removing openai-strict-tools.ts: 5 tests fail —

  • openai-strict-tools.test.ts fails to load at all (the module is gone);
  • llm-configround-trips the opt-in flag, round-trips an explicit opt-out, rejects anything that is not a boolean;
  • openai-build-bodyrewrites the tools and marks them strict when the flag is on, keeps the strict tools over an extraBody that tries to replace them.

The two control cases keep passing, which is their job: leaves the body byte-identical when the flag is absent and sends no tools key at all when the request carries none.

Five targeted mutations of the restored transform, each reverted after:

mutation result
required kept as the input's instead of every key 66 of 110 fail
keyword allowlist removed (nothing stripped) 6 fail — the schemas carrying minItems/maxItems/minimum/minLength
free-form-object refusal removed 2 fail — the pinned refusal list and the fallback case
withoutTopLevelNullArgs made a no-op 2 fail
adapter wrapper made a pass-through 1 fails

Overlap with #395

While this was being written, #395 landed on the same Discord report from another session, taking the other design: it wires the dormant supportsTools: "strict" catalog level per model, threaded through bootstrapagent-loopstep-executor → both directions of the adapter. It is not merged, and the two collide (both touch openai-tool-call-adapter.ts and llm-config.ts). They are alternatives — please take one. The differences worth weighing:

  • Granularity. feat(llm): honour supportsTools: "strict" on the OpenAI tools payload #395 is per model (userModels[].supportsTools: "strict"), which is the more correct unit and makes the dead enum member live. This is per provider entry, which is one flag on the entry the operator already edits and needs no plumbing through the agent loop — the whole change lives at the buildOpenAiChatBody seam, so it does not touch bootstrap.ts, agent-loop.ts or step-executor.ts at all.
  • Coverage. feat(llm): honour supportsTools: "strict" on the OpenAI tools payload #395 converts 77 of 82 and refuses on any bound keyword. This converts 80 of 82 because it strips bounds instead of refusing on them — fusion.delegate, vision.describe and reply become strict, and reply is the tool models botch most.
  • Both handle the null-argument consequence the same way and both refuse the free-form-map tools.

A merged design (per-model level as the source of truth, this transform's strip-not-refuse behaviour, one adapter wrapper) is straightforward if a maintainer wants it; say the word and I will fold them.

Not covered / could not verify

  • No live provider was called. Every assertion here is about the bytes we emit. Nobody has run this against mercury-2.5, against OpenAI, or against any other endpoint. The reporter is the natural first tester. The least universal shape emitted is the anyOf: [..., {type:"null"}] widening for an optional union; if a provider rejects it, the fix is a tighter allowlist, not a looser one.
  • The dormant supportsTools: "strict" level is still dormant after this PR. Deliberate — wiring it is feat(llm): honour supportsTools: "strict" on the OpenAI tools payload #395's design, and doing both would be two config surfaces for one behaviour.
  • No TUI surface. strictTools is hand-edited in config.json; the Providers tab does not offer it.
  • The refusal list is behaviour, not policy. A new default tool with a free-form map silently joins it; the pinned test is what turns that into a decision.
  • MCP inputSchemas are handled generically, not tested against real servers. Anything the allowlist does not cover is refused, so the failure mode is "that tool is not strict", never a 400 — but I have not run this against a live MCP server's schemas.
  • Nested optionals in a converted third-party schema arrive as explicit null and reach the tool that way; the null-drop is top-level only, by design.
  • Nothing here touches the grammar/local path — llama-server ignores tools.
  • Full suite not run (the repo's known ~10 flaky failures); the targeted src/llm/provider src/config sweep is green at 936/936.

Some models call tools reliably only when the provider constrains
decoding to the tool's schema — OpenAI's strict mode. Until now there
was no way to ask for it: `strict` is a field on each tool
(`tools[].function.strict`), and `tools` sits in `RESERVED_BODY_KEYS`
and is re-applied on top of the `extraBody` merge, so the vendor
passthrough could never reach it. The dormant `supportsTools: "strict"`
catalog level did not help either — nothing has ever read it.

Adds `strictTools` to the provider entry (config v63, off by default,
byte-identical request body when absent) and a schema transform that
rewrites each tool into the subset strict mode accepts: objects closed,
every property listed in `required` with optional ones widened to
nullable, and the value-range keywords the runtime validators already
enforce stripped. Tools whose arguments are a free-form map cannot be
expressed strictly and are sent unconstrained rather than silently
losing their payload.

Nullable optionals change the response too — a strict model sends
`"userName": null` where it used to omit the key, and several tools
branch on presence rather than value. So on a provider that opted in,
the tool-call adapter is wrapped to drop top-level null arguments,
leaving the parsed call identical to what those tools saw before.

A conformance test walks every descriptor the agent registers through
the adapter and the transform and checks every strict rule recursively,
so a new tool with an unsupported keyword fails here rather than as a
400 that kills the whole request.

Reported on Discord by thegreatteacher (2026-09-10) against Inception
Labs' Mercury 2.5.
Strict decoding and parallel function calls do not compose. OpenAI's
own guidance is explicit: "Structured Outputs is not compatible with
parallel function calls — when a parallel function call is generated,
it may not match supplied schemas. Set `parallel_tool_calls: false`."

`buildOpenAiChatBody` defaulted `parallel_tool_calls` to `true`
(`agent.maxParallelToolCalls` is 8 out of the box), so a provider that
opted into `strictTools` was emitting `strict: true` on 80 tools and
still getting best-effort adherence — the exact symptom the flag
exists to cure. Under the flag the wire now asks for one call per
response; the executor's own batching is untouched, and a provider
without the flag keeps today's value verbatim.

Also corrects two claims in the transform's header that do not hold:

  * `os.git.init` is cited as a tool an explicit null would break, but
    its `optionalString` maps null to undefined before the presence
    check. `memory.profile.set` is the real case — `parseSetOptions`
    gates on `!== undefined` and then demands a boolean, so a null
    turns a good call into a validation error. Cited that instead.
  * "no schema this transform converts turns a nested optional into a
    null the caller did not choose" is false: five do
    (`os.fs.archive.extract.limits.*`, `fusion.delegate.tasks[].*`).
    All five readers happen to treat null as their default, so nothing
    is broken today; the comment now names them, and names the other
    place the drop does not reach — step-executor's two content
    recovery paths build a batch from the grammar parser and use the
    adapter for `nameUnescape` alone.
Mutation-testing the branch found four edits that removed the feature
end to end and left all 936 tests green: dropping `this.strictTools`
from either `buildOpenAiChatBody` call site in `OpenAiProvider`, not
wrapping the tool-call adapter, and deleting `strictTools:
entry.strictTools` from all five OpenAI-shaped factories. The
transform, the body builder and the config parser were each well
covered; the wiring between them was not covered at all.

Adds the two observable ends. `openai-provider.test.ts` asserts the
posted body — streaming and not — carries `function.strict` with the
rewritten schema and `parallel_tool_calls: false`, is byte-identical
with the flag absent or false, and that the wrapped adapter drops a
top-level null (`memory.profile.set.pinned`) while the unwrapped one
keeps it. `cloud-passthroughs.test.ts` gets the `strictTools` row
alongside `maxOutputTokens` and `extraBody`, which is what that file
exists for.

Each of the four mutations now fails at least one test.
plombeer31 added a commit that referenced this pull request Sep 10, 2026
OpenAI documents Structured Outputs as not compatible with parallel
function calls — a parallel call generated under strict mode "may not
match supplied schemas" — and says to send `parallel_tool_calls: false`.
This branch marked every convertible tool `strict: true` and left the
flag at its default `true`, so an operator who turned the level on still
got best-effort adherence: the exact symptom the feature exists to cure.

Found by the parallel work on PR #402, which fixed it at the body
builder off its own provider flag. Implemented here off the emitted
tools array instead, because strict is granted per tool: an adapter can
ignore the option and a descriptor set can convert nothing, and neither
should silently lose parallel calls for a request that is not
constrained at all. `hasStrictFunctionTools` is the one predicate, read
in two places that therefore cannot disagree — `buildLlmStreamParams`
makes the decision, so the CompletionRequest is honest about it, and
`buildOpenAiChatBody` is the floor under every other caller of the body
builder.

The executor's own `maxParallelToolCalls` batching is untouched: a model
that emits several calls anyway is planned and run exactly as before.
plombeer31 added a commit that referenced this pull request Sep 10, 2026
76 of the 82 emitted functions carried `strict: true`. Three of the six
refusals were over bounds the strict compiler ignores anyway — `reply`
(`minLength: 1`), `vision.describe` (`maxItems`), `fusion.delegate`
(`minItems`/`maxItems`/`minimum`) — so the refusal bought nothing: the
tool came out unconstrained AND unbounded instead of constrained and
unbounded, and `reply` is the one tool most worth constraining.

So a value-range keyword is now stripped rather than refused over. It is
safe for the reason `default-tool-args-schemas.ts` gives in its own
header: these schemas guard shape, and every bound they carry is
re-checked by the tool's own parser, which is what rejects a bad call
today — checked one by one for the three recovered (the batch
validator's non-empty `text` rule, `maxImagesPerCall`,
`parseDelegateArgs`). `const` is deliberately not on the list: it pins a
value the way a one-member `enum` does, so dropping it would widen what
the tool accepts, and it stays a refusal.

The fourth, `os.fs.archive.extract`, was refused over a nested `limits`
that declares properties AND says `additionalProperties: true`. An
object in that shape is now closed: the author wrote both halves, the
declared keys are the whole documented contract, the model is shown
nothing else, and `parseLimits` reads exactly those three keys. An
ABSENT `additionalProperties` still refuses — same semantics, but it is
what pydantic/FastMCP emit for every model, so refusing there keeps this
rule to schemas somebody actually typed `true` into. A zero-property
object refuses either way, so the open-object fallback is still never
published as a zero-argument tool.

Coverage measured over the real DEFAULT_TOOL_DESCRIPTORS: 80 of 82
emitted functions strict, refusing only `os.http.request` and
`mcp.prompt.get`, where the map IS the payload. Both counts are now
pinned — the registry pin alone would have missed `reply`, whose schema
the adapter hand-writes.

Nothing about the null-drop bookkeeping moves: `strictWidenedProperties`
reads the original schema's `properties` and `required` and nothing
else, and a bound says nothing about either. Pinned as its own test.
The coverage idea came from the parallel work on PR #402.
plombeer31 added a commit that referenced this pull request Sep 10, 2026
Two v63s and two strict-tools paths met here. The slot-count change took
v63 first, so #402's provider flag is renumbered v64. In the request
builder the two mechanisms are one: the provider flag transforms the
tool array, and the parallel_tool_calls floor is keyed to the array that
actually goes on the wire — which covers both the flag and a caller that
marked tools strict itself. Both test suites kept.
Sign up for free to 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.

1 participant