Skip to content

refactor(provider): generate tool schemas from input structs#151

Merged
omarluq merged 3 commits into
mainfrom
feat/jsonschema-validation
Jun 21, 2026
Merged

refactor(provider): generate tool schemas from input structs#151
omarluq merged 3 commits into
mainfrom
feat/jsonschema-validation

Conversation

@omarluq

@omarluq omarluq commented Jun 21, 2026

Copy link
Copy Markdown
Owner

No description provided.

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: faf04e1b-38a6-4cdb-bc78-8b19103f21e0

📥 Commits

Reviewing files that changed from the base of the PR and between eb7d00f and 483f10b.

📒 Files selected for processing (4)
  • internal/extension/manager_loader.go
  • internal/extension/manager_test.go
  • internal/tool/input_validation.go
  • internal/tool/input_validation_internal_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/tool/input_validation_internal_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Enforced valid AST tool mode values via JSON-schema enums.
    • Standardized tool metadata so schemas are always present (using an explicit empty schema when unspecified).
  • Refactor
    • Reworked built-in tool schema generation to be reflection-based, with cached reuse and consistent field descriptions.
    • Updated provider request payloads (OpenAI/Anthropic) to use the shared cached tool-schema approach.
  • Chores
    • Updated dependency set to improve JSON-schema handling.
  • Tests
    • Updated schema assertions and expanded coverage for generated built-in tool metadata.

Walkthrough

Introduces tool.Schema, an immutable JSON schema wrapper type, replacing map[string]any throughout the codebase. Migrates tool, LLM, and extension domain models to use this type. Relocates extension tool adapter logic from tool/extension.go to assistant/extension_tool.go. Adds schema caching infrastructure to HTTPCompletionClient. Refactors tool schema generation in tool_schema.go to use jsonschema.Reflector with per-field comment lookup and caching. Updates all built-in tool definitions to return EmptySchema() and refactors payload builders in Anthropic, OpenAI Chat, and OpenAI Responses modules to use cached schemas. Updates go.mod with invopop/jsonschema and transitive dependencies.

Changes

Schema type refactoring and reflection-based generation

Layer / File(s) Summary
Schema type definition and core operations
internal/tool/schema.go, internal/tool/schema_test.go
New immutable Schema type wraps json.RawMessage with defensive copying. Provides EmptySchema(), SchemaFromMap(map), SchemaFromRaw([]byte), MustSchemaFromMap(map) constructors. Methods IsEmpty(), RawMessage(), ToMap(), MustToMap() for introspection. MarshalJSON serializes empty as null, non-empty as raw JSON. Error wrapping via oops.
Domain model schema type adoption
internal/tool/types.go, internal/llm/tools.go, internal/extension/types.go
tool.Definition, llm.ToolDefinition, and extension.Tool structs update Schema/InputSchema fields from map[string]any to tool.Schema type. All three files add tool package import and reorder fields accordingly.
Built-in tool schema initialization
internal/tool/{bash,edit,find,grep,ls,read,write,ast}.go
All tool Definition() methods now return Schema: EmptySchema() instead of nil. ASTInput.Mode gains jsonschema struct tag enumerating valid modes (outline, symbols, query, node, tree).
Input validation and schema parsing
internal/tool/input_validation.go, internal/tool/input_validation_internal_test.go
schemaRequiredArguments rewritten to accept tool.Schema, check emptiness, unmarshal raw JSON into {Required []string} struct. Test cases updated to construct schemas via MustSchemaFromMap() and EmptySchema().
Extension tool adapter relocation
internal/assistant/extension_tool.go, internal/assistant/extension_tool_internal_test.go
Extension tool adapter moved from deleted tool/extension.go to new assistant/extension_tool.go. Defines extensionToolRunner interface (package-local), extensionToolExecutor implementation, and registerExtensionTools registry wiring function. Adapts extension results to tool.TextResult and wraps execution errors with oops. Test moved to assistant package.
Extension loader schema conversion
internal/extension/manager_loader.go, internal/extension/manager_test.go
luaOptionalSchema returns tool.Schema using EmptySchema() and MustSchemaFromMap(). luaRegisterTool validates conversion and rejects invalid schemas via Lua argument error. Tests update expected schema comparisons and add TestManager_RegisterToolRejectsInvalidInputSchema.
HTTPCompletionClient caching infrastructure
internal/provider/client.go
Adds toolSchemas builtinToolSchemaCache field to HTTPCompletionClient. NewHTTPCompletionClient initializes cache and configures HTTP client with timeout and HTTP/2 transport. Removes unused jsonStringType constant.
Reflection-based tool schema generation and caching
internal/provider/tool_schema.go, go.mod
builtinToolSchemaCache caches reflected schemas from tool.*Input types using jsonschema.Reflector with LookupComment. builtinToolNames() provides ordered tool list. builtinToolSchema() generates schemas; toolSchemaFieldComments() centralizes per-field descriptions. ResponseTools, OpenAIChatTools, OpenAIChatToolsFromDefinitions use the cache. go.mod adds invopop/jsonschema v0.14.0, tmaxmax/go-sse v0.11.0, and transitive indirect deps.
Payload builders refactored for caching
internal/provider/{anthropic,openai_chat,openai_responses}.go
advanceAnthropicLoop and advanceOpenAIChatLoop call client methods (client.anthropicPayload, client.openAIChatPayload) instead of package functions. completeResponsesLoop calls client.responsesPayload. Cache-backed methods route tool schema generation through builtinToolSchemaCache for consistency. Tool definitions derived from requestToolDefinitions(request).
LLM conversion and registry updates
internal/assistant/{llm_conversion.go,tool_registry.go}
llmToolDefinitionFromTool returns EmptySchema() for nil definitions and assigns schema directly without cloning. tool_registry.go uses extensionToolRunner and registerExtensionTools.
Provider test refactoring
internal/provider/{anthropic_internal_test.go,openai_chat_payload_internal_test.go,openai_responses_internal_test.go,provider_payload_hook_internal_test.go,provider_tool_calls_internal_test.go,tool_schema_internal_test.go,tool_registry_internal_test.go}
Payload function calls updated to new signatures (single-argument forms). HTTP clients initialized with toolSchemas: newBuiltinToolSchemaCache(). Schema literals converted to tool.MustSchemaFromMap(map). Schema access now uses MustToMap(). New test TestBuiltinToolSchemaUsesGeneratedStructMetadata validates reflected metadata.
LLM and assistant test updates
internal/assistant/{tool_schema_internal_test.go,llm_conversion_behavior_internal_test.go}
Schemas constructed via MustSchemaFromMap() and EmptySchema(). Enum comparisons use assert.JSONEq with marshaled values. Schema cloning tests use MustToMap() for mutation/verification.

Sequence Diagram

sequenceDiagram
  participant RequestLoop
  participant HTTPClient
  participant builtinToolSchemaCache
  participant builtinToolSchema
  participant jsonschema.Reflector
  participant PayloadBuilder
  
  RequestLoop->>HTTPClient: openAIChatPayload(request)
  HTTPClient->>builtinToolSchemaCache: openAIChatPayload(request, messages)
  builtinToolSchemaCache->>builtinToolSchemaCache: for each tool definition
  builtinToolSchemaCache->>builtinToolSchema: tool name
  builtinToolSchema->>jsonschema.Reflector: Reflect(tool.*Input type)
  jsonschema.Reflector-->>builtinToolSchema: schema map
  builtinToolSchema-->>builtinToolSchemaCache: cached schema
  builtinToolSchemaCache->>PayloadBuilder: OpenAI format tools
  PayloadBuilder-->>HTTPClient: payload with tools
  HTTPClient-->>RequestLoop: complete payload
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • omarluq/librecode#43: Related to extension tool adapter relocation and how extension tool schemas are surfaced through the unified registry.
  • omarluq/librecode#94: Introduced the ast tool with ASTInput.Mode field, directly affected by the jsonschema enum tag addition and schema type refactoring.
  • omarluq/librecode#100: Both PRs modify internal/provider/tool_schema.go to standardize and refactor ToolParameterSchema and related tool-schema generation logic.

Poem

🐇 Behold! The schema sheds its map disguise,
A Schema type now holds the prize.
With Reflector's gaze on tool-input types,
Generated fields shine bright with jsonschema's might.
The cache keeps schemas swift and tight! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No description was provided by the author, making it impossible to verify relevance to the changeset. Add a pull request description explaining the motivation, approach, and key changes made in this refactoring.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main refactoring: generating tool schemas from input structs instead of hardcoded definitions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/jsonschema-validation

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/provider/tool_schema.go`:
- Around line 151-169: The builtinToolSchemaForName function regenerates JSON
schemas through reflection on every call, causing unnecessary CPU and allocation
overhead. Create a package-level cache map to store the computed schemas for
each tool (indexed by tool name), and populate this cache once at package
initialization rather than on each lookup. Modify builtinToolSchemaForName to
simply return the precomputed schema from the cache map instead of calling
builtinToolSchema which performs reflection and JSON operations.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 52990dce-90f8-48eb-9b69-e9d1f64a54dd

📥 Commits

Reviewing files that changed from the base of the PR and between 7c06647 and 3748c2e.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • go.mod
  • internal/assistant/tool_schema_internal_test.go
  • internal/provider/client.go
  • internal/provider/tool_schema.go
  • internal/provider/tool_schema_internal_test.go
  • internal/tool/ast.go
💤 Files with no reviewable changes (1)
  • internal/provider/client.go

Comment thread internal/provider/tool_schema.go Outdated
@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.48896% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.41%. Comparing base (7c06647) to head (483f10b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/provider/tool_schema.go 86.04% 16 Missing and 8 partials ⚠️
internal/tool/schema.go 69.76% 7 Missing and 6 partials ⚠️
internal/provider/anthropic.go 50.00% 7 Missing ⚠️
internal/extension/manager_loader.go 92.85% 1 Missing ⚠️
internal/tool/input_validation.go 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
- Coverage   80.52%   80.41%   -0.12%     
==========================================
  Files         279      280       +1     
  Lines       21879    21978      +99     
==========================================
+ Hits        17619    17674      +55     
- Misses       3047     3077      +30     
- Partials     1213     1227      +14     
Flag Coverage Δ
unittests 80.41% <85.48%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@omarluq omarluq force-pushed the feat/jsonschema-validation branch from 3748c2e to eb7d00f Compare June 21, 2026 03:38

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/extension/manager_loader.go`:
- Around line 268-275: The luaOptionalSchema function uses
tool.MustSchemaFromMap which panics on conversion failures, but since the input
comes from untrusted Lua extension tables, malformed values should be handled
gracefully. Replace the call to tool.MustSchemaFromMap with a version that
returns both the schema and an error (such as tool.SchemaFromMap if it exists),
check for errors, and return tool.EmptySchema() when the conversion fails
instead of allowing a panic to occur.

In `@internal/tool/input_validation.go`:
- Around line 66-73: The unmarshal operation for extracting required fields in
the document struct fails entirely when the JSON contains mixed types or invalid
entries, causing valid required fields to be lost. Modify the logic after the
json.Unmarshal call to handle mixed-type arrays gracefully by filtering and
preserving only valid string entries from the required array instead of
returning nil and false when any non-string values are encountered. Keep the
permissive filtering approach so that partial valid entries in the required
field are preserved and blank or invalid names are filtered out appropriately.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b30a5b6-3f1b-448f-b5af-3499479cca38

📥 Commits

Reviewing files that changed from the base of the PR and between 3748c2e and eb7d00f.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (39)
  • go.mod
  • internal/assistant/extension_tool.go
  • internal/assistant/extension_tool_internal_test.go
  • internal/assistant/llm_conversion.go
  • internal/assistant/llm_conversion_behavior_internal_test.go
  • internal/assistant/tool_registry.go
  • internal/assistant/tool_registry_test.go
  • internal/assistant/tool_schema_internal_test.go
  • internal/extension/manager_loader.go
  • internal/extension/manager_test.go
  • internal/extension/types.go
  • internal/llm/tools.go
  • internal/provider/anthropic.go
  • internal/provider/anthropic_internal_test.go
  • internal/provider/anthropic_tools_internal_test.go
  • internal/provider/client.go
  • internal/provider/openai_chat.go
  • internal/provider/openai_chat_payload_internal_test.go
  • internal/provider/openai_responses.go
  • internal/provider/openai_responses_internal_test.go
  • internal/provider/provider_payload_hook_internal_test.go
  • internal/provider/provider_tool_calls_internal_test.go
  • internal/provider/tool_registry_internal_test.go
  • internal/provider/tool_schema.go
  • internal/provider/tool_schema_internal_test.go
  • internal/tool/ast.go
  • internal/tool/bash.go
  • internal/tool/edit.go
  • internal/tool/extension.go
  • internal/tool/find.go
  • internal/tool/grep.go
  • internal/tool/input_validation.go
  • internal/tool/input_validation_internal_test.go
  • internal/tool/ls.go
  • internal/tool/read.go
  • internal/tool/schema.go
  • internal/tool/schema_test.go
  • internal/tool/types.go
  • internal/tool/write.go
💤 Files with no reviewable changes (1)
  • internal/tool/extension.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/provider/tool_schema_internal_test.go
  • go.mod

Comment thread internal/extension/manager_loader.go Outdated
Comment thread internal/tool/input_validation.go Outdated
@omarluq omarluq merged commit 7e46350 into main Jun 21, 2026
15 checks passed
@omarluq omarluq deleted the feat/jsonschema-validation branch June 21, 2026 04:55
@sonarqubecloud

Copy link
Copy Markdown

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