Skip to content

Optimize jq schema middleware scalar handling and de-risk gojq comment drift#4211

Merged
lpcox merged 2 commits intomainfrom
copilot/go-fan-review-itchyny-gojq
Apr 20, 2026
Merged

Optimize jq schema middleware scalar handling and de-risk gojq comment drift#4211
lpcox merged 2 commits intomainfrom
copilot/go-fan-review-itchyny-gojq

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026

The jq schema middleware already used gojq well, but had a few low-cost gaps from the module review: scalar payloads still took a JSON round-trip in the large-payload path, comments were version-pinned, and iterator consumption behavior could be clearer. This PR makes those focused improvements without changing middleware contracts.

  • Schema extraction path: native scalar passthrough

    • Extend native passthrough in WrapToolHandler from {map, slice} to also include JSON scalar types: string, float64, bool.
    • Keep JSON unmarshal fallback for non-native/non-JSON-compatible inputs.
  • Comment cleanup for long-term accuracy

    • Remove version-specific wording tied to a single gojq release in error-handling comments.
    • Rephrase to generic “enhanced gojq type error messages when available.”
  • Iterator behavior documentation

    • Clarify in applyJqSchema why only one Next() call is consumed and why no drain is required (synchronous iterator, no background goroutines).
  • Focused regression coverage

    • Add a targeted test validating large scalar payloads still produce PayloadMetadata and correct schema types (string, number, boolean).
switch data.(type) {
case map[string]interface{}, []interface{}, string, float64, bool:
    jsonData = data
default:
    if err := json.Unmarshal(payloadJSON, &jsonData); err != nil {
        return fmt.Errorf("failed to unmarshal for schema: %w", err)
    }
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build952800513/b513/launcher.test /tmp/go-build952800513/b513/launcher.test -test.testlogfile=/tmp/go-build952800513/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build952800513/b476/vet.cfg g_.a -I x_amd64/vet --gdwarf-5 grpcsync -o x_amd64/vet -o .cfg 9579546/b287/ 64/pkg/tool/linu-nilfunc -p os/exec -lang=go1.25 64/pkg/tool/linu-buildtags (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build952800513/b495/config.test /tmp/go-build952800513/b495/config.test -test.testlogfile=/tmp/go-build952800513/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build952800513/b337/vet.cfg @v1.1.3/cpu/arm/go1.25.8 9579546/b151/ x_amd64/vet --gdwarf-5 nal/encoding/tag-atomic -o x_amd64/vet 9579�� g_.a -trimpath x_amd64/vet -p 64 -lang=go1.25 x_amd64/vet (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build952800513/b513/launcher.test /tmp/go-build952800513/b513/launcher.test -test.testlogfile=/tmp/go-build952800513/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build952800513/b476/vet.cfg g_.a -I x_amd64/vet --gdwarf-5 grpcsync -o x_amd64/vet -o .cfg 9579546/b287/ 64/pkg/tool/linu-nilfunc -p os/exec -lang=go1.25 64/pkg/tool/linu-buildtags (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build952800513/b513/launcher.test /tmp/go-build952800513/b513/launcher.test -test.testlogfile=/tmp/go-build952800513/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build952800513/b476/vet.cfg g_.a -I x_amd64/vet --gdwarf-5 grpcsync -o x_amd64/vet -o .cfg 9579546/b287/ 64/pkg/tool/linu-nilfunc -p os/exec -lang=go1.25 64/pkg/tool/linu-buildtags (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build952800513/b522/mcp.test /tmp/go-build952800513/b522/mcp.test -test.testlogfile=/tmp/go-build952800513/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s 9579�� .cfg -I x_amd64/vet us.pb.go g/grpc/grpclog/i/usr/bin/runc -o x_amd64/vet .cfg�� 9579546/b458/_pkg_.a -plugin-opt=/usr/libexec/gcc/x86-ifaceassert x_amd64/vet -plugin-opt=-pasbash contextprotocol//usr/bin/runc -plugin-opt=-pas--version x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI linked an issue Apr 20, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Review gojq module for jq schema middleware Optimize jq schema middleware scalar handling and de-risk gojq comment drift Apr 20, 2026
Copilot AI requested a review from lpcox April 20, 2026 15:41
@lpcox lpcox marked this pull request as ready for review April 20, 2026 16:09
Copilot AI review requested due to automatic review settings April 20, 2026 16:09
@lpcox lpcox merged commit 2559092 into main Apr 20, 2026
27 checks passed
@lpcox lpcox deleted the copilot/go-fan-review-itchyny-gojq branch April 20, 2026 16:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 improves the jq schema middleware’s large-payload schema extraction path by avoiding an unnecessary JSON unmarshal round-trip for native scalar payloads, while also making gojq-related comments less version-pinned and adding a focused regression test.

Changes:

  • Extend the “native JSON-compatible passthrough” in WrapToolHandler to include scalar types (string, float64, bool) in addition to maps/slices.
  • Reword gojq-related comments to avoid pinning behavior to a specific gojq version and clarify iterator consumption rationale.
  • Add a regression test ensuring large scalar payloads return PayloadMetadata with the expected scalar schema type.
Show a summary per file
File Description
internal/middleware/jqschema.go Extends schema generation’s native passthrough to scalar types and updates gojq/iterator comments for long-term accuracy.
internal/middleware/jqschema_test.go Adds coverage for large scalar payloads to validate metadata + correct schema typing.

Copilot's findings

Tip

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

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

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.

[go-fan] Go Module Review: itchyny/gojq

3 participants