Skip to content

Deduplicate jq filter compile/cache pipeline in jqschema without changing public behavior - #8565

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-fix-jqschema
Jul 3, 2026
Merged

Deduplicate jq filter compile/cache pipeline in jqschema without changing public behavior#8565
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-fix-jqschema

Conversation

Copilot AI commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

CompileToolResponseFilter and CompileToolResponseFilterWithVars duplicated the same cache-check → parse → compile → cache-store flow, forcing parallel maintenance of error/log/cache behavior. This refactor consolidates that pipeline into one internal path while preserving the two intentional behavioral differences (cache key shape and compile options).

  • Refactor: shared internal compile path

    • Introduced compileToolResponseFilterInternal[K comparable] to centralize:
      • cache lookup/type guard
      • jq parse/compile error wrapping
      • cache store
      • common logging lifecycle
    • Kept wrappers thin and explicit for each public API.
  • Preserved API-specific behavior

    • CompileToolResponseFilter still uses:
      • cache key: raw filter string
      • compile options: secureCompileOpts
    • CompileToolResponseFilterWithVars still uses:
      • cache key: toolResponseFilterVarsCacheKey{filter, varNamesKey}
      • compile options: compileOptsWithVariables(varNames)
      • var-aware log context
  • Coverage update for cache-key separation

    • Added a focused test asserting that compiling the same filter through plain and vars-enabled APIs does not reuse the same cached *gojq.Code.
func CompileToolResponseFilter(filter string) (*gojq.Code, error) {
	return compileToolResponseFilterInternal(
		filter, filter, secureCompileOpts, "CompileToolResponseFilter", "",
	)
}

func CompileToolResponseFilterWithVars(filter string, varNames []string) (*gojq.Code, error) {
	cacheKey := toolResponseFilterVarsCacheKey{
		filter:      filter,
		varNamesKey: buildVarNamesCacheKey(varNames),
	}
	return compileToolResponseFilterInternal(
		cacheKey, filter, compileOptsWithVariables(varNames),
		"CompileToolResponseFilterWithVars", fmt.Sprintf(", vars=%v", varNames),
	)
}

Copilot AI changed the title [WIP] Refactor duplicate code in jqschema.go Deduplicate jq filter compile/cache pipeline in jqschema without changing public behavior Jul 3, 2026
Copilot finished work on behalf of lpcox July 3, 2026 12:41
Copilot AI requested a review from lpcox July 3, 2026 12:41
@lpcox
lpcox marked this pull request as ready for review July 3, 2026 12:43
Copilot AI review requested due to automatic review settings July 3, 2026 12:43

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 refactors the jq tool-response filter compilation logic in internal/middleware/jqschema.go by consolidating the shared cache lookup → parse → compile → cache store pipeline into a single internal helper, while keeping the two public entry points’ intentional differences (cache-key shape and compile options) intact.

Changes:

  • Introduced a shared internal compile function (compileToolResponseFilterInternal) to deduplicate cache/compile flow and logging.
  • Kept CompileToolResponseFilter and CompileToolResponseFilterWithVars as thin wrappers to preserve API-specific behavior.
  • Added a coverage test ensuring the plain and var-enabled APIs do not share cached compiled code.
Show a summary per file
File Description
internal/middleware/jqschema.go Deduplicates the filter compile/cache pipeline into a shared internal helper while preserving wrapper-level behavior differences.
internal/middleware/jqschema_coverage_test.go Adds a focused test ensuring cache-key separation between plain vs var-enabled compilation paths.

Review details

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment on lines +286 to +290
filter,
compileOptsWithVariables(varNames),
"CompileToolResponseFilterWithVars",
fmt.Sprintf(", vars=%v", varNames),
)
code, ok := cached.(*gojq.Code)
if !ok {
// Should never happen; the cache only stores *gojq.Code values.
return nil, fmt.Errorf("internal error: unexpected cached value type for filter (len=%d)", len(filter))
@lpcox
lpcox merged commit 9049382 into main Jul 3, 2026
39 checks passed
@lpcox
lpcox deleted the copilot/duplicate-code-fix-jqschema branch July 3, 2026 14:52
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.

[duplicate-code] Duplicate Code Pattern: jq filter compile pipeline duplicated in jqschema.go

3 participants