fix(template): filter dispatch, shadowing, and arg order

[zeroedin]

Summary

Closes #199. Closes #200.

#199 — Built-in filters produce wrong output

• findRE and replaceRE had input/pattern args swapped for Liquid convention. Added wrapper functions in RegisterBuiltinFilters that swap to (text, pattern) for Liquid while preserving the Go function signature (pattern, text) for unit tests.
• Removed contains, findRE, replaceRE from knownLiquidFilters so they route through the plugin_filter bridge — fixes contains returning truthy string instead of boolean in conditionals.

#200 — Plugin filter shadowing

• AddFilter now marks filters as dynamic when overriding an existing registration (detected via f.funcs[name] existence check). This ensures the override routes through plugin_filter bridge instead of liquidgo built-in dispatch.
• First registration of known filters (e.g., upcase without prior RegisterBuiltinFilters) uses liquidgo native dispatch.

Test plan

• All 129 template tests pass
• All 54 integration tests pass
• Full suite green

Generated with Claude Code

[zeroedin]

PR #207 Review — Filter Dispatch, Shadowing, and Arg Order (#199, #200)

Scope: liquid.go (+13/-9), filters.go (+16/-2). Fixes three issues: filter shadowing generalization, findRE/replaceRE arg order, and contains dispatch.

Issue #200: General Filter Shadowing

The fix:

func (e *liquidEngine) AddFilter(name string, fn FilterFunc) error {
    if _, exists := e.filters.funcs[name]; exists {
        // Already registered → override → mark as dynamic
        e.dynamicFilters[name] = true
    } else if !knownLiquidFilters[name] {
        // Novel filter → mark as dynamic
        e.dynamicFilters[name] = true
    }
    e.filters.funcs[name] = fn
    return nil
}

When AddFilter is called for a filter that's already in funcs (e.g., upcase registered by RegisterBuiltinFilters), it adds the name to dynamicFilters. This triggers the rewriteFilterToPlugin pre-processing during Parse, which rewrites {{ x | upcase }} to {{ x | plugin_filter: "upcase" }}. The PluginFilter bridge method on alloyFilterBridge then dispatches to the overriding function.

This is the general solution — no need to add per-filter bridge methods like Reverse() was done in PR #153. Any filter can be shadowed by calling AddFilter after RegisterBuiltinFilters. The Reverse bridge method from PR #153 is now redundant (though not harmful).

Key detail: e.filters.funcs[name] = fn is moved AFTER the existence check. Previously it was first, so the check would always find it "existing" on the first registration too. Now the check happens before the store — correct.

Issue #199: findRE and replaceRE Arg Order

The problem: Liquid convention is {{ text | findRE: pattern }} — input is the text, first arg is the pattern. But the Go FindRE function signature was FindRE(pattern, text) — args swapped.

The fix: Wrapper functions in filters.go that swap args:

"findRE": func(input interface{}, args ...interface{}) interface{} {
    // Liquid: input=text, args[0]=pattern
    // Go:     input=pattern, args[0]=text
    return FindRE(args[0], input)
},
"replaceRE": func(input interface{}, args ...interface{}) interface{} {
    // Liquid: input=text, args[0]=pattern, args[1]=replacement
    return ReplaceRE(args[0], input, args[1])
},

Clean — the wrapper adapts the Liquid calling convention to the Go function signature without changing the underlying FindRE/ReplaceRE implementations.

Guard clauses return empty/passthrough values when required args are missing — no panics on index-out-of-bounds.

contains, findRE, replaceRE Removed from knownLiquidFilters

These three filters are removed from the known list:

// Before:
"slugify": true, "contains": true, "group_by": true,
"findRE": true, "replaceRE": true, "json": true,

// After:
"slugify": true, "group_by": true,
"json": true, ...

This means they're now treated as "novel" filters and go through the PluginFilter bridge via template pre-processing. This is correct for findRE/replaceRE — they need the wrapper functions (arg swapping) which are registered via RegisterBuiltinFilters → AddFilter. The PluginFilter bridge ensures those wrappers are called, not liquidgo's reflection dispatch (which would try to find an exported method named FindRE on the bridge struct).

For contains — removing it from knownLiquidFilters means it routes through PluginFilter instead of the exported Contains method on alloyFilterBridge. This may change behavior if the bridge method had different logic than the registered FilterFunc. Worth verifying that the contains filter still works correctly through the pipeline test from PR #204.

Verdict

Clean fix that solves the general shadowing problem without per-filter bridge methods. The arg-swapping wrappers for findRE/replaceRE are correct and well-documented. Removing contains, findRE, replaceRE from knownLiquidFilters forces them through the PluginFilter bridge where the correct wrapper functions are used. No blocking issues.

[Copilot]

Pull request overview

Fixes Liquid filter dispatch so built-in and plugin-registered filters produce correct Liquid behavior (argument ordering and “last loaded wins” overriding).

Changes:

• Updates Liquid filter registration/dispatch to route certain filters through the plugin_filter bridge and mark overridden filters as dynamic.
• Wraps findRE / replaceRE built-ins to match Liquid argument ordering while keeping underlying Go helpers unchanged.
• Removes contains, findRE, replaceRE from knownLiquidFilters so they are rewritten to plugin_filter for correct behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
internal/template/liquid.go	Adjusts known-filter routing and AddFilter dynamic marking to ensure plugin overrides dispatch through plugin_filter.
internal/template/filters.go	Adds wrapper functions for findRE/replaceRE to swap Liquid argument order at registration time.

---

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