fix(provider-openai): forward raw MCP tool schema, stop rebuilding #90

2026-05-17T02:54:06-04:00

jasoncouture commented

2026-05-17 02:54:06 -04:00

(Migrated from github.com)

Summary

ToolDescriptor now carries the source's raw JSON schema as a JsonElement alongside the parsed ToolParameter list.
ModelContextProtocolClient.MapTool clones the MCP JsonElement (so it survives the McpClient JsonDocument disposal) and stores it on the descriptor.
OpenAiLanguageModel serializes that JsonElement verbatim into the function definition's parameters slot, falling back to a built schema only when none was advertised.
Parsed Parameters stay around for caller-side introspection (required-ness, top-level type names).

Why

Rebuilding the schema from ToolParameter drops fields like items on array properties. OpenAI itself accepts that, but strict downstream validators reject it — observed today with Google AI Studio via OpenRouter:

GenerateContentRequest.tools[0].function_declarations[2].parameters.properties[indices].items: missing field.

Forwarding the raw MCP schema preserves whatever the tool advertised.

Test plan

dotnet build clean (pre-existing [Obsolete] warnings only)
dotnet test — 519/519 pass
Manual: agent on Gemini-via-OpenRouter completes a tool-calling turn without the INVALID_ARGUMENT 400

🤖 Generated with Claude Code

## Summary - `ToolDescriptor` now carries the source's raw JSON schema as a `JsonElement` alongside the parsed `ToolParameter` list. - `ModelContextProtocolClient.MapTool` clones the MCP `JsonElement` (so it survives the `McpClient` `JsonDocument` disposal) and stores it on the descriptor. - `OpenAiLanguageModel` serializes that `JsonElement` verbatim into the function definition's `parameters` slot, falling back to a built schema only when none was advertised. - Parsed `Parameters` stay around for caller-side introspection (required-ness, top-level type names). ## Why Rebuilding the schema from `ToolParameter` drops fields like `items` on array properties. OpenAI itself accepts that, but strict downstream validators reject it — observed today with Google AI Studio via OpenRouter: ``` GenerateContentRequest.tools[0].function_declarations[2].parameters.properties[indices].items: missing field. ``` Forwarding the raw MCP schema preserves whatever the tool advertised. ## Test plan - [x] `dotnet build` clean (pre-existing `[Obsolete]` warnings only) - [x] `dotnet test` — 519/519 pass - [x] Manual: agent on Gemini-via-OpenRouter completes a tool-calling turn without the `INVALID_ARGUMENT` 400 🤖 Generated with [Claude Code](https://claude.com/claude-code)

github-actions[bot] commented

2026-05-17 02:55:50 -04:00

(Migrated from github.com)

Package	Line Rate	Branch Rate	Complexity	Health
LlamaShears.Core.Eventing	91%	84%	53	✔
LlamaShears.Core.Abstractions	49%	34%	365	❌
LlamaShears.Api	29%	30%	461	❌
LlamaShears.Provider.Ollama	44%	28%	188	❌
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
LlamaShears.Core	45%	41%	1386	❌
LlamaShears.Provider.Onnx.Embeddings	37%	38%	72	❌
LlamaShears.Provider.OpenAI	66%	65%	229	➖
LlamaShears.Api.Web	1%	1%	428	❌
LlamaShears.Hosting	33%	21%	27	❌
LlamaShears.Plugins	0%	100%	1	❌
LlamaShears.Core.Eventing	89%	75%	53	✔
LlamaShears	52%	36%	25	➖
LlamaShears.Plugins.Host	34%	24%	36	❌
LlamaShears.Core.Abstractions	33%	14%	365	❌
LlamaShears.Api	7%	1%	461	❌
LlamaShears.Provider.Ollama	3%	1%	188	❌
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
StrangeSoft.Plugins.Host	20%	21%	87	❌
LlamaShears.Core	40%	27%	1386	❌
LlamaShears.Provider.Onnx.Embeddings	3%	0%	72	❌
LlamaShears.Provider.OpenAI	2%	0%	229	❌
LlamaShears.Api.Web	21%	10%	428	❌
LlamaShears.Hosting	26%	8%	27	❌
LlamaShears.Plugins	0%	100%	1	❌
LlamaShears.Core.Eventing	89%	75%	53	✔
LlamaShears	52%	36%	25	➖
LlamaShears.Plugins.Host	34%	24%	36	❌
LlamaShears.Core.Abstractions	33%	14%	365	❌
LlamaShears.IntegrationTests	85%	73%	72	✔
LlamaShears.Api	9%	3%	461	❌
LlamaShears.Provider.Ollama	3%	1%	188	❌
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
StrangeSoft.Plugins.Host	20%	21%	87	❌
LlamaShears.Core	41%	28%	1386	❌
LlamaShears.Provider.Onnx.Embeddings	3%	0%	72	❌
LlamaShears.Provider.OpenAI	2%	0%	229	❌
LlamaShears.Api.Web	31%	17%	428	❌
LlamaShears.Hosting	26%	8%	27	❌
LlamaShears.Analyzers.CodeFixes	85%	69%	60	✔
LlamaShears.Analyzers	88%	76%	199	✔
Summary	46% (10709 / 31373)	36% (2319 / 9482)	10259	❌

Package | Line Rate | Branch Rate | Complexity | Health -------- | --------- | ----------- | ---------- | ------ LlamaShears.Core.Eventing | 91% | 84% | 53 | ✔ LlamaShears.Core.Abstractions | 49% | 34% | 365 | ❌ LlamaShears.Api | 29% | 30% | 461 | ❌ LlamaShears.Provider.Ollama | 44% | 28% | 188 | ❌ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ LlamaShears.Core | 45% | 41% | 1386 | ❌ LlamaShears.Provider.Onnx.Embeddings | 37% | 38% | 72 | ❌ LlamaShears.Provider.OpenAI | 66% | 65% | 229 | ➖ LlamaShears.Api.Web | 1% | 1% | 428 | ❌ LlamaShears.Hosting | 33% | 21% | 27 | ❌ LlamaShears.Plugins | 0% | 100% | 1 | ❌ LlamaShears.Core.Eventing | 89% | 75% | 53 | ✔ LlamaShears | 52% | 36% | 25 | ➖ LlamaShears.Plugins.Host | 34% | 24% | 36 | ❌ LlamaShears.Core.Abstractions | 33% | 14% | 365 | ❌ LlamaShears.Api | 7% | 1% | 461 | ❌ LlamaShears.Provider.Ollama | 3% | 1% | 188 | ❌ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ StrangeSoft.Plugins.Host | 20% | 21% | 87 | ❌ LlamaShears.Core | 40% | 27% | 1386 | ❌ LlamaShears.Provider.Onnx.Embeddings | 3% | 0% | 72 | ❌ LlamaShears.Provider.OpenAI | 2% | 0% | 229 | ❌ LlamaShears.Api.Web | 21% | 10% | 428 | ❌ LlamaShears.Hosting | 26% | 8% | 27 | ❌ LlamaShears.Plugins | 0% | 100% | 1 | ❌ LlamaShears.Core.Eventing | 89% | 75% | 53 | ✔ LlamaShears | 52% | 36% | 25 | ➖ LlamaShears.Plugins.Host | 34% | 24% | 36 | ❌ LlamaShears.Core.Abstractions | 33% | 14% | 365 | ❌ LlamaShears.IntegrationTests | 85% | 73% | 72 | ✔ LlamaShears.Api | 9% | 3% | 461 | ❌ LlamaShears.Provider.Ollama | 3% | 1% | 188 | ❌ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ StrangeSoft.Plugins.Host | 20% | 21% | 87 | ❌ LlamaShears.Core | 41% | 28% | 1386 | ❌ LlamaShears.Provider.Onnx.Embeddings | 3% | 0% | 72 | ❌ LlamaShears.Provider.OpenAI | 2% | 0% | 229 | ❌ LlamaShears.Api.Web | 31% | 17% | 428 | ❌ LlamaShears.Hosting | 26% | 8% | 27 | ❌ LlamaShears.Analyzers.CodeFixes | 85% | 69% | 60 | ✔ LlamaShears.Analyzers | 88% | 76% | 199 | ✔ **Summary** | **46%** (10709 / 31373) | **36%** (2319 / 9482) | **10259** | ❌

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-05-17 02:57:01 -04:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR preserves tool parameter JSON Schemas end-to-end (especially for MCP-provided tools) by carrying and forwarding the raw schema instead of reconstructing it from the parsed parameter list, avoiding loss of required JSON Schema fields (e.g., items for arrays) that strict validators reject.

Changes:

Extend ToolDescriptor to include the raw parameters JSON Schema (JsonElement Schema) alongside the parsed ToolParameter list.
Update MCP tool mapping to clone and store the raw schema so it survives document disposal.
Update OpenAI tool serialization to forward the raw schema verbatim (fallback to rebuilt schema only when none is advertised), and update API docs accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptor.cs	Adds `Schema: JsonElement` to `ToolDescriptor` to retain the raw JSON Schema.
src/LlamaShears.Core/Tools/ModelContextProtocol/ModelContextProtocolClient.cs	Clones MCP-provided schema and stores it on `ToolDescriptor`.
src/LlamaShears.Provider.OpenAI/OpenAILanguageModel.cs	Serializes `ToolDescriptor.Schema` into OpenAI tool `parameters`, with fallback schema generation.
docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/ToolDescriptor.md	Documents the new `Schema` property and updated semantics for `Parameters`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This PR preserves tool parameter JSON Schemas end-to-end (especially for MCP-provided tools) by carrying and forwarding the raw schema instead of reconstructing it from the parsed parameter list, avoiding loss of required JSON Schema fields (e.g., `items` for arrays) that strict validators reject. **Changes:** - Extend `ToolDescriptor` to include the raw parameters JSON Schema (`JsonElement Schema`) alongside the parsed `ToolParameter` list. - Update MCP tool mapping to clone and store the raw schema so it survives document disposal. - Update OpenAI tool serialization to forward the raw schema verbatim (fallback to rebuilt schema only when none is advertised), and update API docs accordingly. ### Reviewed changes Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments. | File | Description | | ---- | ----------- | | src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptor.cs | Adds `Schema: JsonElement` to `ToolDescriptor` to retain the raw JSON Schema. | | src/LlamaShears.Core/Tools/ModelContextProtocol/ModelContextProtocolClient.cs | Clones MCP-provided schema and stores it on `ToolDescriptor`. | | src/LlamaShears.Provider.OpenAI/OpenAILanguageModel.cs | Serializes `ToolDescriptor.Schema` into OpenAI tool `parameters`, with fallback schema generation. | | docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/ToolDescriptor.md | Documents the new `Schema` property and updated semantics for `Parameters`. | --- 💡 <a href="/jasoncouture/llama-shears/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/ToolDescriptor.md

					
				@ -12,2 +12,3 @@

				- `Parameters` — Parameter schema in declaration order.

				- `Parameters` — Parsed parameter list in declaration order; lossy projection of `Schema` kept for caller-side introspection (required-ness, top-level type names).

				- `Schema` — Raw JSON Schema for the tool's parameters as the source advertised it. Providers should forward this verbatim instead of rebuilding from `Parameters`, since rebuilding drops fields strict validators (Gemini, structured-output) require. JsonValueKind.`Undefined` means the source advertised no schema.

Markdown formatting looks off here: JsonValueKind.Undefined`` renders with only Undefined in code ticks. Consider changing to a single code span like JsonValueKind.Undefined for consistency/readability.

Markdown formatting looks off here: `JsonValueKind.`Undefined`` renders with only `Undefined` in code ticks. Consider changing to a single code span like `JsonValueKind.Undefined` for consistency/readability.

src/LlamaShears.Provider.OpenAI/OpenAILanguageModel.cs

					
				@ -407,0 +418,4 @@

				        return new JsonObject

				        {

				            ["type"] = "function",

				            ["function"] = new JsonObject

ToOpenAiTool only falls back when Schema.ValueKind == Undefined. If a caller provides a non-object schema (e.g., Null), JsonNode.Parse(descriptor.Schema.GetRawText()) can return null or a non-object node, and the request will serialize "parameters": null / invalid shape. Consider treating any non-Object Schema value kind (or a null parse result) as “no schema” and using the fallback builder.

`ToOpenAiTool` only falls back when `Schema.ValueKind == Undefined`. If a caller provides a non-object schema (e.g., `Null`), `JsonNode.Parse(descriptor.Schema.GetRawText())` can return `null` or a non-object node, and the request will serialize `"parameters": null` / invalid shape. Consider treating any non-`Object` `Schema` value kind (or a `null` parse result) as “no schema” and using the fallback builder.

New behavior forwards an advertised raw JSON schema into the OpenAI tool definition (with a fallback when absent), but the OpenAI provider tests don’t currently assert the emitted tools[].function.parameters shape. Adding a unit test covering both the raw-schema path (including array items) and the fallback path would help prevent regressions.

New behavior forwards an advertised raw JSON schema into the OpenAI tool definition (with a fallback when absent), but the OpenAI provider tests don’t currently assert the emitted `tools[].function.parameters` shape. Adding a unit test covering both the raw-schema path (including array `items`) and the fallback path would help prevent regressions.

src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptor.cs

This adds a new parameter to the primary constructor of a public record in a packable src/public API. Even with a default value, this changes the constructor signature and will break binary compatibility for consumers compiled against the previous ToolDescriptor(string,string,ImmutableArray<ToolParameter>) (MissingMethodException at runtime). Consider adding an explicit overload with the old signature that delegates to the new one (or otherwise ensure a major-version bump / documented breaking change).

This adds a new parameter to the primary constructor of a public record in a packable `src/public` API. Even with a default value, this changes the constructor signature and will break binary compatibility for consumers compiled against the previous `ToolDescriptor(string,string,ImmutableArray<ToolParameter>)` (MissingMethodException at runtime). Consider adding an explicit overload with the old signature that delegates to the new one (or otherwise ensure a major-version bump / documented breaking change).

Sign in to join this conversation.

No reviewers