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

[github-actions]

Summary

CompileToolResponseFilter and CompileToolResponseFilterWithVars in internal/middleware/jqschema.go share ~20 lines of near-identical cache-check / parse / compile / cache-store logic. Any future change to this pipeline (error messages, metrics, cache invalidation) must be applied twice.

Part of duplicate code analysis: #8551

Duplication Details

Pattern: Repeated cache-check / parse / compile / store pipeline

• Severity: Medium
• Occurrences: 2 instances (~20 identical structural lines each)
• Locations:
  • internal/middleware/jqschema.go lines 255–283 (CompileToolResponseFilter)
  • internal/middleware/jqschema.go lines 298–330 (CompileToolResponseFilterWithVars)

Duplicated structural skeleton (annotated):

// --- CompileToolResponseFilter ---
func CompileToolResponseFilter(filter string) (*gojq.Code, error) {
    if cached, ok := filterCodeCache.Load(filter); ok {           // cache check
        code, ok := cached.(*gojq.Code)
        if !ok { return nil, fmt.Errorf("internal error: ...") }  // type assert guard
        logMiddleware.Printf("... cache hit ...")
        return code, nil
    }
    logMiddleware.Printf("... parsing ...")
    query, err := gojq.Parse(filter)                              // parse
    if err != nil { return nil, fmt.Errorf("...") }
    code, err := gojq.Compile(query, secureCompileOpts...)        // compile (DIFFERS)
    if err != nil { return nil, fmt.Errorf("...") }
    filterCodeCache.Store(filter, code)                           // store (cache key DIFFERS)
    logMiddleware.Printf("... compiled and cached ...")
    return code, nil
}

// --- CompileToolResponseFilterWithVars ---
func CompileToolResponseFilterWithVars(filter string, varNames []string) (*gojq.Code, error) {
    cacheKey := toolResponseFilterVarsCacheKey{...}               // extra: struct key
    if cached, ok := filterCodeCache.Load(cacheKey); ok {         // identical pattern
        code, ok := cached.(*gojq.Code)
        if !ok { return nil, fmt.Errorf("internal error: ...") }  // identical guard
        logMiddleware.Printf("... cache hit ...")
        return code, nil
    }
    logMiddleware.Printf("... parsing ...")
    query, err := gojq.Parse(filter)                              // identical parse
    if err != nil { return nil, fmt.Errorf("...") }
    code, err := gojq.Compile(query,                              // compile (uses varNames)
        compileOptsWithVariables(varNames)...)
    if err != nil { return nil, fmt.Errorf("...") }
    filterCodeCache.Store(cacheKey, code)                         // store (struct key)
    logMiddleware.Printf("... compiled and cached ...")
    return code, nil
}

Differences (only 3):

1. Cache key type: string vs toolResponseFilterVarsCacheKey
2. Compile opts: secureCompileOpts... vs compileOptsWithVariables(varNames)...
3. Log messages append , vars=%v in the WithVars variant

Impact Analysis

• Maintainability: Any change to the cache-check/parse/compile/store pattern (e.g., adding telemetry, changing error messages, switching from sync.Map) must be applied to both functions identically
• Bug Risk: Low at present, but a future inconsistent edit (e.g., adding cache eviction in only one function) could introduce subtle behavioral differences
• Code Bloat: ~20 lines of structural duplication in a 861-line file

Refactoring Recommendations

Option 1 (Preferred): Extract a private generic helper

Extract a private compileFilterInternal[K comparable] function that accepts a pre-computed cache key, the raw filter string, and the compiler options. Both public functions become thin wrappers:

func CompileToolResponseFilter(filter string) (*gojq.Code, error) {
    return compileFilterInternal(filter, filter, secureCompileOpts, "")
}

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

• Estimated effort: ~1 hour
• Benefits: single point of change for cache semantics, error messages, and future metrics

Option 2: Accept duplication with cross-reference comments

Add a comment to each function noting the structural duplication and pointing to the other function. Lower effort but leaves the duplication in place.

Implementation Checklist

• Review duplication findings
• Decide on Option 1 (extract helper) or Option 2 (document duplication)
• Implement changes (if Option 1)
• Run make agent-finished to verify build, lint, and tests pass
• Verify no behavioral change to either public function

Parent Issue

See parent analysis report: #8551
Related to #8551

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Duplicate Code Detector · 385.7 AIC · ⊞ 7.7K · ◷

• expires on Jul 10, 2026, 3:46 AM UTC