From dd79a55f6e2202f5edaf78282806eb0ee37cfb21 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 13:35:16 +0000 Subject: [PATCH 1/2] perf(middleware): bypass gojq interpreter for schema inference in applyJqSchema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walk_schema function registered in init() is a thin wrapper around the native Go inferSchema function. Going through the gojq interpreter adds per-call overhead: context-with-timeout allocation, gojq iterator setup, and interpreter dispatch — none of which provides value for a pure Go recursive walk. Replace the runJqCode call in applyJqSchema with a direct inferSchema call: - Eliminates context.WithTimeout allocation on every large-payload schema walk - Removes gojq iterator setup and Next() dispatch overhead - Preserves context cancellation: ctx.Err() is checked on entry The gojq compilation in init() is retained as a startup canary that validates the jqSchemaFilter expression is parseable, and TestInferSchema_MatchesJqOutput continues to verify parity between the direct path and the jq reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/middleware/jqschema.go | 55 +++++++++------------- internal/middleware/jqschema_bench_test.go | 2 +- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/internal/middleware/jqschema.go b/internal/middleware/jqschema.go index 294f879b7..937cae8d2 100644 --- a/internal/middleware/jqschema.go +++ b/internal/middleware/jqschema.go @@ -103,15 +103,13 @@ type toolResponseFilterVarsCacheKey struct { // share a single authoritative definition of the $ENV-disabled gojq compile options. var secureCompileOpts = jqutil.SecureCompileOpts -// init compiles the jq schema filter at startup for better performance and validation. -// Following gojq best practices: compile once, run many times. +// init compiles the jq schema filter at startup as a canary check. // -// The walk_schema function is registered as a native Go implementation via -// gojq.WithFunction so that the recursive schema walk runs entirely in Go, -// avoiding jq interpreter overhead for deeply-nested payloads. -// -// This provides fail-fast behavior - if the jq query is invalid, the application -// will fail at startup rather than at runtime during a tool call. +// applyJqSchema no longer runs the gojq interpreter at request time — it calls +// inferSchema directly for lower latency. This init() serves two purposes: +// 1. Startup validation: panics early if the jqSchemaFilter expression is invalid. +// 2. Parity canary: TestInferSchema_MatchesJqOutput exercises both paths to detect +// any future divergence between inferSchema and the jq reference implementation. func init() { query, err := gojq.Parse(jqSchemaFilter) if err != nil { @@ -129,46 +127,37 @@ func init() { panic(fmt.Sprintf("built-in jq schema filter failed to compile: %v", jqSchemaCompileErr)) } - logger.LogInfo("startup", "jq schema filter compiled successfully - native Go walk_schema, array limit: 2^29 elements, timeout: %v", DefaultJqTimeout) + logger.LogInfo("startup", "jq schema filter compiled successfully - native Go walk_schema (direct-call fast path active), array limit: 2^29 elements") } // queryIDBytes is the number of random bytes used to generate a query ID. // The resulting hex string has length 2*queryIDBytes (32 characters). const queryIDBytes = 16 -// applyJqSchema applies the jq schema transformation to JSON data -// Uses pre-compiled query code for better performance (3-10x faster than parsing on each request) +// applyJqSchema applies the jq schema transformation to JSON data. // -// Accepts a context for timeout and cancellation support. If the context does not have a deadline, -// a default timeout of DefaultJqTimeout (5 seconds) is enforced to prevent hangs from: -// - Malformed jq queries -// - Extremely large or deeply nested payloads -// - Infinite loops in query logic +// The schema walk is implemented as a native Go recursive function (inferSchema), +// which is called directly here instead of going through the gojq interpreter. +// This eliminates the per-call overhead of gojq iterator setup, context-with-timeout +// allocation, and interpreter dispatch — significant savings on the large-payload path +// where this function is called for every tool response that exceeds the size threshold. // -// Returns the schema as an any object (not a JSON string) +// Context cancellation is respected: if ctx is already done on entry, an error is returned +// immediately. This preserves the observable behaviour of the previous gojq-based +// implementation for callers that supply a pre-cancelled context. // -// Error handling: -// - Returns compilation errors if init() failed -// - Returns context.DeadlineExceeded if query times out -// - Returns enhanced gojq type error messages when available -// - Properly handles gojq.HaltError for clean halt conditions +// Returns the schema as an any object (not a JSON string). func applyJqSchema(ctx context.Context, jsonData any) (any, error) { - // Check if compilation succeeded at init time - if jqSchemaCompileErr != nil { - return nil, fmt.Errorf("jq schema filter not compiled (check startup logs): %w", jqSchemaCompileErr) + // Check context before doing any work, mirroring the behaviour of the former + // gojq-based path which checked ctx before running the iterator. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("jq query execution failed: %w", err) } logMiddleware.Printf("applyJqSchema: starting schema inference, dataType=%T", jsonData) - v, err := runJqCode(ctx, jqSchemaCode, jsonData, "jq schema filter", runJqCodeOptions{ - ExecutionPrefix: "jq query", - LogDefaultTimeout: true, - }) - if err != nil { - return nil, err - } + v := inferSchema(jsonData) - // Return the schema object directly (no JSON marshaling needed here) logMiddleware.Printf("applyJqSchema: schema inference completed, resultType=%T", v) return v, nil } diff --git a/internal/middleware/jqschema_bench_test.go b/internal/middleware/jqschema_bench_test.go index df5170d77..0a1597fcb 100644 --- a/internal/middleware/jqschema_bench_test.go +++ b/internal/middleware/jqschema_bench_test.go @@ -10,7 +10,7 @@ import ( ) // BenchmarkApplyJqSchema_CompiledCode benchmarks the current implementation -// that uses pre-compiled query code (the optimized version) +// that calls inferSchema directly (bypassing the gojq interpreter for schema walks) func BenchmarkApplyJqSchema_CompiledCode(b *testing.B) { tests := []struct { name string From c043a4386dc179f0751c0345ab48087ab00f4b8e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:38:55 +0000 Subject: [PATCH 2/2] fix(middleware): fix CI failure and address review feedback - Remove obsolete 'returns_compilation_error_when_init_failed' subtest: applyJqSchema no longer checks jqSchemaCode/jqSchemaCompileErr since init() panics on compile failure, making the simulated state unreachable. - Fix TestInferSchema_MatchesJqOutput to truly exercise the jq path by running jqSchemaCode.RunWithContext directly, making it a genuine canary that detects divergence between inferSchema and the jq implementation. - Fix misleading error message 'jq query execution failed' -> 'schema inference failed' since no jq query is executed in applyJqSchema. --- internal/middleware/jqschema.go | 2 +- internal/middleware/jqschema_test.go | 40 +++++++++------------------- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/internal/middleware/jqschema.go b/internal/middleware/jqschema.go index 937cae8d2..886fb3066 100644 --- a/internal/middleware/jqschema.go +++ b/internal/middleware/jqschema.go @@ -151,7 +151,7 @@ func applyJqSchema(ctx context.Context, jsonData any) (any, error) { // Check context before doing any work, mirroring the behaviour of the former // gojq-based path which checked ctx before running the iterator. if err := ctx.Err(); err != nil { - return nil, fmt.Errorf("jq query execution failed: %w", err) + return nil, fmt.Errorf("schema inference failed: %w", err) } logMiddleware.Printf("applyJqSchema: starting schema inference, dataType=%T", jsonData) diff --git a/internal/middleware/jqschema_test.go b/internal/middleware/jqschema_test.go index f2a4e1958..fe12a9299 100644 --- a/internal/middleware/jqschema_test.go +++ b/internal/middleware/jqschema_test.go @@ -947,28 +947,6 @@ func TestApplyJqSchema_TimeoutBehavior(t *testing.T) { assert.Equal(t, "number", schema["level"], "Level should have number type") assert.Contains(t, schema, "child", "Should contain child field") }) - - t.Run("returns compilation error when init failed", func(t *testing.T) { - // Save the current compiled code and error - originalCode := jqSchemaCode - originalErr := jqSchemaCompileErr - - // Simulate compilation failure - jqSchemaCode = nil - jqSchemaCompileErr = assert.AnError - - // Restore after test - defer func() { - jqSchemaCode = originalCode - jqSchemaCompileErr = originalErr - }() - - input := map[string]interface{}{"test": "data"} - _, err := applyJqSchema(context.Background(), input) - - require.Error(t, err, "Should return error when compilation failed") - assert.ErrorContains(t, err, "not compiled", "Error should mention compilation failure") - }) } // TestApplyJqSchema_ContextTimeout tests timeout behavior with various context configurations @@ -1687,10 +1665,14 @@ func TestInferSchema(t *testing.T) { } // TestInferSchema_MatchesJqOutput verifies that inferSchema (called directly) produces -// the same output as applyJqSchema (which invokes inferSchema via the gojq runtime). +// the same output as the compiled jq expression via jqSchemaCode.RunWithContext. // This validates the gojq.WithFunction wiring: that the compiled jq code correctly // dispatches to the native Go implementation for all supported input shapes. +// It acts as a canary to detect any future divergence between inferSchema and the +// jq reference implementation. func TestInferSchema_MatchesJqOutput(t *testing.T) { + require.NotNil(t, jqSchemaCode, "jq schema compiled code must not be nil") + inputs := []interface{}{ map[string]interface{}{"name": "test", "count": 42}, map[string]interface{}{"user": map[string]interface{}{"id": 123, "active": true}}, @@ -1702,18 +1684,22 @@ func TestInferSchema_MatchesJqOutput(t *testing.T) { for _, input := range inputs { inputJSON, _ := json.Marshal(input) t.Run(string(inputJSON), func(t *testing.T) { - jqResult, err := applyJqSchema(context.Background(), input) - require.NoError(t, err, "applyJqSchema must not error") + // Exercise the compiled jq expression path directly. + iter := jqSchemaCode.RunWithContext(context.Background(), input) + jqRaw, ok := iter.Next() + require.True(t, ok, "jq walk_schema must produce at least one output") + _, isErr := jqRaw.(error) + require.False(t, isErr, "jq walk_schema must not produce an error: %v", jqRaw) goResult := inferSchema(input) - jqJSON, err := json.Marshal(jqResult) + jqJSON, err := json.Marshal(jqRaw) require.NoError(t, err) goJSON, err := json.Marshal(goResult) require.NoError(t, err) assert.JSONEq(t, string(jqJSON), string(goJSON), - "inferSchema output must match applyJqSchema output") + "inferSchema output must match jq walk_schema output") }) } }