Skip to content

Improve gojq usage: ValueError handling, WithVariables, filter cache, inferSchema decoupling, any alias#7680

Merged
lpcox merged 3 commits into
mainfrom
copilot/go-fan-gojq-module-review
Jun 17, 2026
Merged

Improve gojq usage: ValueError handling, WithVariables, filter cache, inferSchema decoupling, any alias#7680
lpcox merged 3 commits into
mainfrom
copilot/go-fan-gojq-module-review

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Addresses all five recommendations from the gojq module review (#7670): better runtime error diagnostics, injection-safe variable binding, compile-time filter caching, decoupled schema logic, and idiomatic Go 1.25 types.

Error handling (runJqCode)

  • Added gojq.ValueError check between HaltError and the generic fallback — intentional error() builtin calls get a clean message; all unexported type errors (null access, wrong-type builtins) get a "check filter handles null/missing fields" hint
  • Note: gojq.TypeError from the issue report does not exist in the public API; gojq.ValueError is the correct interface

Variable injection (CompileToolResponseFilterWithVars)

New function using gojq.WithVariables for per-call context without string interpolation:

code, _ := CompileToolResponseFilterWithVars(
    `if $serverID == "github" then .items[:20] else . end`,
    []string{"$serverID"},
)
iter := code.RunWithContext(ctx, data, serverID)

validateToolResponseFiltersWithVars added in config/validation.go to mirror the same compile options for fail-fast startup validation.

Filter compilation cache (filterCodeCache sync.Map)

  • CompileToolResponseFilter checks the cache before parsing; stores on miss
  • Cache is unbounded but bounded in practice by the number of distinct filter strings in config
  • Uses checked type assertion (cached.(*gojq.Code)) to avoid potential panics on unexpected cache contents

inferSchema decoupled from gojq

  • Moved to internal/middleware/schema.go — imports only encoding/json and reflect, no gojq dependency
  • Enables direct calls and independent testing without the gojq import

interface{}any

All internal and public signatures in jqschema.go updated to any (idiomatic Go 1.18+, project targets Go 1.25).

GitHub Advanced Security started work on behalf of lpcox June 17, 2026 14:01 View session
GitHub Advanced Security finished work on behalf of lpcox June 17, 2026 14:03
…ter cache, inferSchema decoupling, any type alias
GitHub Advanced Security started work on behalf of lpcox June 17, 2026 14:17 View session
GitHub Advanced Security finished work on behalf of lpcox June 17, 2026 14:19
Copilot AI changed the title [WIP] Review gojq module for Go integration Improve gojq usage: ValueError handling, WithVariables, filter cache, inferSchema decoupling, any alias Jun 17, 2026
GitHub Advanced Security started work on behalf of lpcox June 17, 2026 14:19 View session
Copilot finished work on behalf of lpcox June 17, 2026 14:20
Copilot AI requested a review from lpcox June 17, 2026 14:20
GitHub Advanced Security finished work on behalf of lpcox June 17, 2026 14:21
@lpcox lpcox marked this pull request as ready for review June 17, 2026 14:51
Copilot AI review requested due to automatic review settings June 17, 2026 14:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refines the gateway’s jq (gojq) integration in internal/middleware by improving runtime error diagnostics, adding an injection-safe compilation path for variable-bound filters, caching compiled filter bytecode, and separating schema inference into a gojq-independent helper. It also updates middleware-facing APIs to use Go’s any alias consistently.

Changes:

  • Added gojq.ValueError handling in runJqCode to distinguish intentional error(...) failures from runtime type errors and include a null-safety hint for the latter.
  • Introduced CompileToolResponseFilterWithVars (uses gojq.WithVariables) and added a sync.Map cache for compiled non-parameterized filters.
  • Moved inferSchema into a new internal/middleware/schema.go to decouple schema inference from gojq imports, and updated interface{}any across the middleware surface.
Show a summary per file
File Description
internal/middleware/schema.go New gojq-independent inferSchema implementation for schema inference.
internal/middleware/jqschema.go Adds filter compilation cache, ValueError vs runtime-type error handling, WithVariables compilation helper, and any type updates.
internal/middleware/jqschema_coverage_test.go Adds coverage for ValueError/type-error paths, WithVars compilation, and cache hit behavior.
internal/config/validation.go Adds validateToolResponseFiltersWithVars helper to mirror WithVariables compile options for startup validation.
internal/config/validation_test.go Tests the new validateToolResponseFiltersWithVars validation helper.

Copilot's findings

Tip

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

  • Files reviewed: 5/5 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 4896661 into main Jun 17, 2026
40 checks passed
@lpcox lpcox deleted the copilot/go-fan-gojq-module-review branch June 17, 2026 14:58
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.

3 participants