From e76ca534879487b1c5e08338080cf42b88616057 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 03:16:18 +0000 Subject: [PATCH 1/7] feat(cmdfilter): port rtk's filter coverage (3 -> 23) and fix three DSL loss-typing bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmdfilter shipped a 3-filter starter set against rtk's 63. The DSL engine was already a faithful port, so the gap was coverage — but rtk's selectors are not portable: it matches a shell COMMAND string, and a proxy only ever sees the OUTPUT. Every ported filter's selector is rewritten to an output-shape signature, and a guardrail test asserts each filter's own test input actually routes to that filter, so the rewrite is verified rather than hoped for. Engine fixes (each with a test): - truncate_lines_at never recorded loss: it ran before `loss` was initialized, so a real intra-line cut reported LossNone and no recovery hint was emitted. It now types as LossWhole (an intra-line cut is non-contiguous by nature). - truncate_lines_at cut silently. A mid-line cut with no marker reads as corrupted output to a model; it now appends "...", sized to fit inside the cap. - recoveryHint collapsed LossTail and LossWhole into one hint, making a cheap partial recovery look like an expensive whole-blob one. They are now distinct: a tail cut names the cut point. 23 filters ported with converted inline tests: pytest, npm-install, make, gradle, xcodebuild, gcc, swift-build, dotnet-build, turbo, nx, terraform-plan, terraform-init, pulumi, liquibase, ssh, ping, rsync, bundle-install, poetry-install, composer-install, uv-sync, brew-install, quarto-render. Filters whose output is indistinguishable are merged (terraform/tofu, the five pulumi subcommands). rtk's ~24 width-cap-only filters, 11 blank-line-only linter filters, spring-boot, filter_stderr, its command-detection rules and its native Rust filters are deliberately not ported; docs/components/cmdfilter.md records why. Every success-collapse rule carries an `unless` guard (rtk ships 9 of 11 unguarded) plus an explicit negative test: a build with a warning AND a success marker must not collapse. In a proxy the agent cannot re-run the command to discover what a bare collapse swallowed. Also: shared `cap` budget classes by signal density instead of 23 hand-picked max_lines; a 500-byte size floor (rtk's MIN_TEE_SIZE); load-time guardrails (duplicate names rejected, inline tests executed at load); a selector-miss ledger so the next filter to write is data; per-family cmdfilter metrics in /stats (additive fields only — the harness parsers keep working). Filter definitions and the DSL pipeline are adapted from rtk (Apache-2.0); a THIRD-PARTY-NOTICES entry records the notice and the modifications made. Closes #29 Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- THIRD-PARTY-NOTICES | 54 ++ components/component.go | 16 + components/dsl/dsl.go | 134 ++++- components/dsl/dsl_loss_test.go | 135 +++++ components/offload/cmdfilter.go | 93 +-- components/offload/cmdfilter_filters.go | 739 ++++++++++++++++++++++++ components/offload/cmdfilter_test.go | 214 +++++++ components/pipeline.go | 7 + docs/components.md | 4 +- docs/components/cmdfilter.md | 144 ++++- docs/components/dsl.md | 88 ++- docs/how-to/choose-a-preset.md | 4 +- docs/how-to/custom-dsl-filter.md | 57 +- docs/results/components.md | 5 +- metrics/metrics.go | 128 ++++ metrics/metrics_test.go | 58 ++ 16 files changed, 1782 insertions(+), 98 deletions(-) create mode 100644 THIRD-PARTY-NOTICES create mode 100644 components/dsl/dsl_loss_test.go create mode 100644 components/offload/cmdfilter_filters.go create mode 100644 components/offload/cmdfilter_test.go diff --git a/THIRD-PARTY-NOTICES b/THIRD-PARTY-NOTICES new file mode 100644 index 0000000..41854d3 --- /dev/null +++ b/THIRD-PARTY-NOTICES @@ -0,0 +1,54 @@ +THIRD-PARTY NOTICES +=================== + +context-guru includes third-party material. This file records the notices those +works require. It covers material adapted INTO this source tree; Go module +dependencies carry their own licenses in the module cache and in go.sum. + +------------------------------------------------------------------------------- +rtk (Rust Token Killer) +------------------------------------------------------------------------------- + +Project: rtk +Homepage: https://github.com/rtk-ai/rtk +License: Apache License, Version 2.0 +Notice: Copyright 2024 rtk-ai and rtk-ai Labs + +Portions of this software are adapted from rtk, with modifications: + + - components/dsl (DSL filter pipeline). The declarative filter engine — the + eight-stage pipeline (strip_ansi, replace, match_output+unless, strip/keep + lines, truncate_lines_at, head/tail, max_lines, on_empty), the field names, + the Lossiness typing, and the shared truncation cap classes (CAP_ERRORS, + CAP_WARNINGS, CAP_LIST, CAP_INVENTORY) — is adapted from rtk's TOML filter + DSL (src/core/toml_filter.rs, src/core/truncate.rs) and re-implemented in Go. + + - components/offload (cmdfilter filter definitions and test corpora). The + shipped filter set is adapted from rtk's src/filters/*.toml, including its + inline test cases. + +Modifications made: + + - Re-implemented in Go, from Rust. + - Filter documents are YAML with a schema_version, not TOML. + - Every filter's selector is rewritten. rtk matches a shell COMMAND string + (match_command); context-guru is a proxy and never sees the command, so each + filter matches an output-shape signature against the first non-empty line of + the tool output instead. rtk's command regexes are not portable as written. + - Filters whose outputs are indistinguishable from the output alone are merged + (terraform/tofu plan; terraform/tofu init; the pulumi subcommands). + - Every success-collapse (match_output) rule carries an `unless` guard; most of + rtk's do not. A proxy's agent cannot re-run a command to discover a warning + that a bare collapse swallowed. + - Per-filter line budgets are replaced by shared `cap` budget classes, applying + rtk's truncate.rs cap idea to the filter definitions (rtk's own TOML filters + hand-pick a max_lines each). + - truncate_lines_at records intra-line loss and marks the cut with an ellipsis. + - A `buildlog` cap class, a `family` field (per-family metrics), a `priority` + field (selector ordering), load-time guardrails (duplicate names rejected, + inline tests executed at load), and a selector-miss ledger were added. + - rtk's filter_stderr, its command-detection rules, and its native Rust + (non-DSL) filters are not ported — a proxy has no analogue for them. + +The Apache-2.0 license text is available at http://www.apache.org/licenses/LICENSE-2.0 +and, since context-guru is itself Apache-2.0 licensed, in this repository's LICENSE. diff --git a/components/component.go b/components/component.go index 34abf17..39500f4 100644 --- a/components/component.go +++ b/components/component.go @@ -123,6 +123,22 @@ type Ctx struct { // -1 = unknown/first turn/cache off ⇒ no tail restriction. Only meaningful when // CacheAware is true. MaxCachedIdx int + // FilterStats receives cmdfilter's per-filter ledger (which command families pay + // off, and which output shapes matched nothing). nil = not recording. + FilterStats FilterStatsSink +} + +// FilterStatsSink records cmdfilter's per-filter/per-family ledger. metrics.Aggregator +// implements it; the pipeline depends only on this interface. Implementations must be +// safe for concurrent use. +type FilterStatsSink interface { + // FilterAct notes one applied filter: its family (builds/tests/iac/pkg/net/...), + // its name, the content key (so a compaction re-sent verbatim next turn is counted + // once), and the tokens saved. + FilterAct(family, filter, contentKey string, saved int) + // FilterMiss notes a selector that matched no filter — the ledger that says which + // filter is worth writing next (after rtk's parse_failures table). + FilterMiss(selector string) } // TailOnly reports whether a supersession/age-based offloader may mutate the message diff --git a/components/dsl/dsl.go b/components/dsl/dsl.go index 94feca2..e484c2a 100644 --- a/components/dsl/dsl.go +++ b/components/dsl/dsl.go @@ -36,10 +36,42 @@ const ( LossWhole // non-contiguous / whole-blob loss; only full retrieval recovers ) +// Caps are the shared line budgets a filter picks by SIGNAL DENSITY (`cap: errors`) +// instead of hand-picking a max_lines per filter. The first four names and values +// are rtk's (src/core/truncate.rs); `buildlog` is ours — a build/plan transcript is +// mostly noise but the line that matters can sit anywhere in it, so it gets a +// deliberately generous budget. One map tunes the whole filter set. +var Caps = map[string]int{ + "errors": 20, // most actionable, shown the most + "warnings": 10, // lower signal density than errors + "list": 20, // flat lists (packages, services): one line per item + "inventory": 50, // exhaustive lookups (installed packages, file listings) + "buildlog": 80, // full build/plan transcripts: verbose, signal is positional +} + +// ReducedCap is rtk's `reduced` deviation helper: a cap lowered for a more verbose +// data class, underflow-safe — a deviation can never empty the budget. +func ReducedCap(cap, by int) int { + if by > 0 && by < cap { + return cap - by + } + return cap +} + // Def is a raw filter definition (from YAML). All fields except Match are optional. type Def struct { - Description string `yaml:"description"` - Match string `yaml:"match"` // regex against the selector key + Description string `yaml:"description"` + Match string `yaml:"match"` // regex against the selector key + // Family groups filters for per-family metrics (builds, tests, iac, pkg, net, ...). + Family string `yaml:"family"` + // Priority orders matching: higher first, then by name. Absent (0) = today's + // behavior (name order). Use it to put a specific filter ahead of a generic one, + // which matters more here than in rtk because we match on output shape. + Priority int `yaml:"priority"` + // Cap selects a shared budget class from Caps instead of a literal MaxLines; + // CapReduce lowers it for an extra-verbose variant. MaxLines wins if both are set. + Cap string `yaml:"cap"` + CapReduce int `yaml:"cap_reduce"` StripANSI bool `yaml:"strip_ansi"` Replace []ReplaceRule `yaml:"replace"` MatchOutput []MatchRule `yaml:"match_output"` @@ -111,6 +143,18 @@ func Compile(name string, d Def) (*Compiled, error) { if len(d.StripLinesMatching) > 0 && len(d.KeepLinesMatching) > 0 { return nil, fmt.Errorf("dsl: filter %q sets both strip_lines_matching and keep_lines_matching", name) } + if d.Cap != "" { + base, ok := Caps[d.Cap] + if !ok { + return nil, fmt.Errorf("dsl: filter %q unknown cap %q", name, d.Cap) + } + if d.MaxLines == nil { // an explicit max_lines still wins + n := ReducedCap(base, d.CapReduce) + d.MaxLines = &n + } + } else if d.CapReduce != 0 { + return nil, fmt.Errorf("dsl: filter %q sets cap_reduce without cap", name) + } m, err := regexp.Compile(d.Match) if err != nil { return nil, fmt.Errorf("dsl: filter %q match: %w", name, err) @@ -189,20 +233,26 @@ func Apply(c *Compiled, input string) (string, Lossiness) { } else if len(c.keepLines) > 0 { lines = filterLines(lines, c.keepLines, true) } - // 5. truncate_lines_at (unicode-safe per-line cap) + // 5. truncate_lines_at (unicode-safe per-line cap). An intra-line cut is a REAL + // loss and is non-contiguous by nature (every long line loses its own tail), so + // it types as LossWhole; and it appends an ellipsis, because a silent mid-line + // cut reads as corrupted output to a model (both after rtk). + loss := LossNone if c.def.TruncateLinesAt != nil { n := *c.def.TruncateLinesAt for i, l := range lines { - if r := []rune(l); len(r) > n { - lines[i] = string(r[:n]) + if t := truncateRunes(l, n); t != l { + lines[i] = t + loss = LossWhole } } } - loss := LossNone // 6. head/tail if c.def.HeadLines != nil || c.def.TailLines != nil { - lines, loss = headTail(lines, c.def.HeadLines, c.def.TailLines) + var htLoss Lossiness + lines, htLoss = headTail(lines, c.def.HeadLines, c.def.TailLines) + loss = worseLoss(loss, htLoss) } // 7. max_lines (absolute cap, counts the omission marker) if c.def.MaxLines != nil && len(lines) > *c.def.MaxLines { @@ -212,7 +262,7 @@ func Apply(c *Compiled, input string) (string, Lossiness) { if loss == LossNone { loss = LossTail } else { - loss = LossWhole + loss = LossWhole // already lossy above: the cap is no longer a clean tail cut } } out := strings.Join(lines, "\n") @@ -223,6 +273,28 @@ func Apply(c *Compiled, input string) (string, Lossiness) { return out, loss } +// truncateRunes caps a line at n runes, marking the cut with an ellipsis that fits +// INSIDE the budget (so the result is never longer than n). Ported from rtk's +// utils::truncate. +func truncateRunes(s string, n int) string { + r := []rune(s) + if len(r) <= n { + return s + } + if n < 3 { + return "..." + } + return string(r[:n-3]) + "..." +} + +// worseLoss returns the more severe of two loss kinds. +func worseLoss(a, b Lossiness) Lossiness { + if b > a { + return b + } + return a +} + func filterLines(lines []string, res []*regexp.Regexp, keep bool) []string { out := lines[:0:0] for _, l := range lines { @@ -272,11 +344,27 @@ func headTail(lines []string, head, tail *int) ([]string, Lossiness) { return lines, LossNone } -// Registry holds compiled filters, matched first-by-sorted-name for determinism. -type Registry struct{ filters []*Compiled } +// Family is the filter's command family, used for per-family metrics. "" when unset. +func (c *Compiled) Family() string { + if c.def.Family == "" { + return "other" + } + return c.def.Family +} + +// Registry holds compiled filters, matched by descending priority then by name for +// determinism (specific-before-generic without relying on alphabetical luck). +type Registry struct { + filters []*Compiled + names map[string]struct{} +} // Load parses a YAML filter document and appends its filters to the registry. -// schema_version must be 1. Filters are stored sorted by name. +// schema_version must be 1. Duplicate filter names are rejected (a silently +// shadowed filter is a debugging trap — rtk's build.rs rejects them too) and any +// inline tests the document carries must pass, so a broken filter fails loudly at +// load instead of quietly mangling output. (Requiring a test to EXIST is enforced +// for the shipped builtins by a unit test, not here — user configs stay free.) func (r *Registry) Load(b []byte) error { var f File dec := yaml.NewDecoder(strings.NewReader(string(b))) @@ -292,13 +380,31 @@ func (r *Registry) Load(b []byte) error { names = append(names, n) } sort.Strings(names) + if r.names == nil { + r.names = map[string]struct{}{} + } for _, n := range names { + if _, dup := r.names[n]; dup { + return fmt.Errorf("dsl: duplicate filter name %q", n) + } c, err := Compile(n, f.Filters[n]) if err != nil { return err } + for _, tc := range f.Tests[n] { + if got, _ := Apply(c, tc.Input); !sameText(got, tc.Expected) { + return fmt.Errorf("dsl: filter %q test %q failed: got %q want %q", n, tc.Name, got, tc.Expected) + } + } + r.names[n] = struct{}{} r.filters = append(r.filters, c) } + sort.SliceStable(r.filters, func(i, j int) bool { + if r.filters[i].def.Priority != r.filters[j].def.Priority { + return r.filters[i].def.Priority > r.filters[j].def.Priority + } + return r.filters[i].Name < r.filters[j].Name + }) return nil } @@ -312,6 +418,10 @@ func (r *Registry) Match(key string) *Compiled { return nil } +func sameText(got, want string) bool { + return strings.TrimRight(got, "\n") == strings.TrimRight(want, "\n") +} + // Len reports how many filters are loaded. func (r *Registry) Len() int { return len(r.filters) } @@ -329,7 +439,7 @@ func RunTests(b []byte) (failures []string, err error) { } for _, tc := range f.Tests[name] { got, _ := Apply(c, tc.Input) - if strings.TrimRight(got, "\n") != strings.TrimRight(tc.Expected, "\n") { + if !sameText(got, tc.Expected) { failures = append(failures, name+"/"+tc.Name) } } diff --git a/components/dsl/dsl_loss_test.go b/components/dsl/dsl_loss_test.go new file mode 100644 index 0000000..6297b62 --- /dev/null +++ b/components/dsl/dsl_loss_test.go @@ -0,0 +1,135 @@ +package dsl + +import ( + "strings" + "testing" +) + +func load(t *testing.T, doc string) *Registry { + t.Helper() + var r Registry + if err := r.Load([]byte(doc)); err != nil { + t.Fatal(err) + } + return &r +} + +// truncate_lines_at used to run BEFORE `loss` was initialized, so a real intra-line +// cut reported LossNone and cmdfilter emitted no recovery hint for it. +func TestTruncateLinesAtReportsLoss(t *testing.T) { + r := load(t, "schema_version: 1\nfilters:\n f:\n match: .\n truncate_lines_at: 10\n") + c := r.Match("x") + + out, loss := Apply(c, "short\n"+strings.Repeat("y", 40)) + if loss != LossWhole { + t.Fatalf("intra-line cut must report LossWhole, got %d (out=%q)", loss, out) + } + // and no cut at all must stay lossless + if _, loss := Apply(c, "short\nalso ok"); loss != LossNone { + t.Fatalf("no cut must be LossNone, got %d", loss) + } +} + +// A silent mid-line cut reads as corrupted output to a model; the cut is marked, +// and the marker fits INSIDE the budget so the line never grows. +func TestTruncateLinesAtEllipsis(t *testing.T) { + r := load(t, "schema_version: 1\nfilters:\n f:\n match: .\n truncate_lines_at: 10\n") + out, _ := Apply(r.Match("x"), strings.Repeat("y", 40)) + if want := strings.Repeat("y", 7) + "..."; out != want { + t.Fatalf("want %q, got %q", want, out) + } + if len([]rune(out)) > 10 { + t.Fatalf("truncation must respect the cap, got %d runes", len([]rune(out))) + } +} + +// An intra-line cut plus a line cap is not a clean tail drop — it must not be +// reported as the cheap-recovery case. +func TestTruncatePlusMaxLinesIsWhole(t *testing.T) { + r := load(t, "schema_version: 1\nfilters:\n f:\n match: .\n truncate_lines_at: 5\n max_lines: 2\n") + _, loss := Apply(r.Match("x"), strings.Repeat("abcdefgh\n", 8)) + if loss != LossWhole { + t.Fatalf("truncate + max_lines must be LossWhole, got %d", loss) + } +} + +func TestCapClassResolvesToMaxLines(t *testing.T) { + r := load(t, "schema_version: 1\nfilters:\n f:\n match: .\n cap: warnings\n") + out, loss := Apply(r.Match("x"), strings.Repeat("line\n", 30)) + if got := len(strings.Split(out, "\n")); got != Caps["warnings"]+1 { // +1 omission marker + t.Fatalf("cap: warnings should cap at %d lines (+marker), got %d", Caps["warnings"], got) + } + if loss != LossTail { + t.Fatalf("a clean cap is a tail drop, got %d", loss) + } +} + +func TestCapReduceAndValidation(t *testing.T) { + r := load(t, "schema_version: 1\nfilters:\n f:\n match: .\n cap: list\n cap_reduce: 15\n") + out, _ := Apply(r.Match("x"), strings.Repeat("line\n", 30)) + if got := len(strings.Split(out, "\n")); got != 6 { // 20-15=5, +1 marker + t.Fatalf("cap_reduce wrong: got %d lines", got) + } + if ReducedCap(10, 20) != 10 || ReducedCap(10, 0) != 10 || ReducedCap(10, 4) != 6 { + t.Fatal("ReducedCap must be underflow-safe and a no-op at by<=0") + } + var bad Registry + if err := bad.Load([]byte("schema_version: 1\nfilters:\n f:\n match: .\n cap: nope\n")); err == nil { + t.Fatal("unknown cap class must be rejected at load") + } + var bad2 Registry + if err := bad2.Load([]byte("schema_version: 1\nfilters:\n f:\n match: .\n cap_reduce: 3\n")); err == nil { + t.Fatal("cap_reduce without cap must be rejected at load") + } +} + +func TestDuplicateFilterNameRejected(t *testing.T) { + doc := "schema_version: 1\nfilters:\n f:\n match: .\n" + var r Registry + if err := r.Load([]byte(doc)); err != nil { + t.Fatal(err) + } + if err := r.Load([]byte(doc)); err == nil { + t.Fatal("a second filter with the same name must be rejected, not silently shadowed") + } +} + +func TestFailingInlineTestRejectedAtLoad(t *testing.T) { + doc := ` +schema_version: 1 +filters: + f: + match: . + strip_lines_matching: ['noise'] +tests: + f: + - name: wrong-expectation + input: "keep\nnoise\n" + expected: "noise" +` + var r Registry + if err := r.Load([]byte(doc)); err == nil { + t.Fatal("a filter whose own inline test fails must not load") + } +} + +func TestPriorityBeatsNameOrder(t *testing.T) { + doc := ` +schema_version: 1 +filters: + aaa-generic: + match: '.' + on_empty: generic + zzz-specific: + match: 'SPECIFIC' + priority: 5 + on_empty: specific +` + r := load(t, doc) + if got := r.Match("SPECIFIC output"); got == nil || got.Name != "zzz-specific" { + t.Fatalf("priority must beat name order, matched %v", got) + } + if got := r.Match("anything else"); got == nil || got.Name != "aaa-generic" { + t.Fatalf("generic filter should still catch the rest, matched %v", got) + } +} diff --git a/components/offload/cmdfilter.go b/components/offload/cmdfilter.go index 289bf2f..45874a1 100644 --- a/components/offload/cmdfilter.go +++ b/components/offload/cmdfilter.go @@ -6,6 +6,7 @@ package offload import ( "crypto/sha256" "encoding/hex" + "strconv" "strings" "github.com/maximhq/bifrost/core/schemas" @@ -23,16 +24,24 @@ func init() { components.Register("cmdfilter", newCmdfilter) } // recover it. Filters match on the tool output's first non-empty line (the // proxy-world stand-in for rtk's shell command). type Cmdfilter struct { - reg *dsl.Registry - mode markerMode + reg *dsl.Registry + mode markerMode + minSize int } type cmdfilterConfig struct { Filters []string `yaml:"filters"` // inline filter YAML documents DisableBuiltins bool `yaml:"disable_builtins"` // skip the bundled starter filters MarkerMode string `yaml:"marker_mode"` // full (default) | summary | off + MinSize *int `yaml:"min_size"` // byte floor below which filtering isn't worth a marker } +// defaultMinSize is rtk's MIN_TEE_SIZE: below it the recovery marker routinely +// costs more tokens than the filter saves, so we don't bother. The +// marker-inclusive never-worse check would catch those anyway; this just skips the +// work (and the stash) instead of doing it and throwing it away. +const defaultMinSize = 500 + func newCmdfilter(raw []byte) (components.Component, error) { var cfg cmdfilterConfig if len(raw) > 0 { @@ -51,7 +60,11 @@ func newCmdfilter(raw []byte) (components.Component, error) { return nil, err } } - return &Cmdfilter{reg: reg, mode: parseMarkerMode(cfg.MarkerMode)}, nil + minSize := defaultMinSize + if cfg.MinSize != nil { + minSize = *cfg.MinSize + } + return &Cmdfilter{reg: reg, mode: parseMarkerMode(cfg.MarkerMode), minSize: minSize}, nil } func (Cmdfilter) Name() string { return "cmdfilter" } @@ -77,8 +90,17 @@ func (f *Cmdfilter) Offload(req *schemas.BifrostChatRequest, rep *components.Rep continue // marker-bearing (a filter rule could drop the marker line and orphan // the stash) or expanded by the agent — leave it verbatim } - filt := f.reg.Match(selectorKey(content)) + if len(content) < f.minSize { + continue // below the size floor the marker often costs more than the saving + } + key := selectorKey(content) + filt := f.reg.Match(key) if filt == nil { + if c.FilterStats != nil { + // The miss ledger: it turns "which filter to write next" into data + // instead of guesswork (after rtk's parse_failures table). + c.FilterStats.FilterMiss(key) + } continue } out, loss := dsl.Apply(filt, content) @@ -90,13 +112,13 @@ func (f *Cmdfilter) Offload(req *schemas.BifrostChatRequest, rep *components.Rep // still bail. Compare the FULL rewritten text (token included) against the // original — the marker costs tokens too, so filtering that barely wins can // still make the message larger (rtk never_worse, at the message level). - key := hashKey(content) + stashKey := hashKey(content) // degrade full→off when the store can't persist (no unresolvable marker). mode := effectiveMode(c, f.mode) var token string switch mode { case markerFull: - token = expand.Marker(key) + recoveryHint(loss) + token = expand.Marker(stashKey) + recoveryHint(loss, len(strings.Split(out, "\n"))) case markerSummary: token = expand.SummaryMarker } // off: no token @@ -104,17 +126,21 @@ func (f *Cmdfilter) Offload(req *schemas.BifrostChatRequest, rep *components.Rep if token != "" { newText += "\n" + token } - if schema.TextTokens(newText) >= schema.TextTokens(content) { + before, after := schema.TextTokens(content), schema.TextTokens(newText) + if after >= before { continue } if mode == markerFull { - c.Store.Put(key, []byte(content)) - recordOwner(c, key) // scope GET /expand retrieval to this session - keys = append(keys, key) + c.Store.Put(stashKey, []byte(content)) + recordOwner(c, stashKey) // scope GET /expand retrieval to this session + keys = append(keys, stashKey) } else { rep.Irreversible = true } schema.SetMessageText(m, newText) + if c.FilterStats != nil { + c.FilterStats.FilterAct(filt.Family(), filt.Name, stashKey, before-after) + } changed++ } if changed == 0 { @@ -134,46 +160,23 @@ func selectorKey(content string) string { return "" } -func recoveryHint(loss dsl.Lossiness) string { - if loss == dsl.LossNone { +// recoveryHint types the hint by WHAT was lost. A clean contiguous tail cut is +// cheaply recoverable — the agent can re-read from the cut point instead of pulling +// the whole blob back — so it says so (rtk emits a partial-recovery hint for the +// same case). Collapsing both kinds into one hint made every loss look like a +// whole-blob loss and pushed the agent toward the expensive recovery. +func recoveryHint(loss dsl.Lossiness, kept int) string { + switch loss { + case dsl.LossTail: + return " [truncated after line " + strconv.Itoa(kept) + "; rest via " + expand.ToolName + "]" + case dsl.LossWhole: + return " [full output: call " + expand.ToolName + "]" + default: return "" } - return " [full output: call " + expand.ToolName + "]" } func hashKey(s string) string { h := sha256.Sum256([]byte(s)) return hex.EncodeToString(h[:])[:16] } - -// builtinFilters is a small starter set adapted from rtk built-ins. Users add -// more via the cmdfilter `filters:` config with no recompile. -const builtinFilters = ` -schema_version: 1 -filters: - pytest: - description: keep failures + summary, drop passing noise - match: "(pytest|=+ test session starts)" - strip_lines_matching: - - "^\\s*$" - - " PASSED" - - "^\\.+$" - max_lines: 80 - on_empty: "pytest: all passed" - npm-install: - description: collapse npm/yarn install chatter - match: "^(npm|yarn|added|removed) " - strip_lines_matching: - - "^npm warn" - - "^\\s*$" - max_lines: 40 - on_empty: "install: ok" - make: - description: drop make directory chatter - match: "^(make|gcc|cc|clang) " - strip_lines_matching: - - "^make\\[\\d+\\]:" - - "^\\s*$" - max_lines: 60 - on_empty: "make: ok" -` diff --git a/components/offload/cmdfilter_filters.go b/components/offload/cmdfilter_filters.go new file mode 100644 index 0000000..b884cba --- /dev/null +++ b/components/offload/cmdfilter_filters.go @@ -0,0 +1,739 @@ +package offload + +// builtinFilters is the shipped filter set. It is adapted from rtk's TOML filter +// definitions (github.com/rtk-ai/rtk, Apache-2.0 — see THIRD-PARTY-NOTICES) with +// two systematic modifications: +// +// 1. Selectors are rewritten. rtk matches a SHELL COMMAND (`^terraform\s+plan`); +// a proxy never sees the command, so every `match` here is an OUTPUT-SHAPE +// signature tested against the first non-empty, trimmed line of the tool +// output. rtk's command regexes would never fire. +// 2. Where two tools' output is INDISTINGUISHABLE from the output alone, +// rtk's per-command filters are merged into one (terraform/tofu plan; +// terraform/tofu init; the five pulumi subcommands). Splitting them would +// only make the shared selector ambiguous and the ordering arbitrary. +// +// Plus: every success-collapse (`match_output`) rule carries an `unless` guard — +// rtk ships 9 of 11 unguarded, and in a proxy the agent cannot re-run the command +// to discover the warning that got swallowed. And line budgets come from the +// shared `cap` classes (dsl.Caps) rather than 25 hand-picked max_lines. +// +// Every filter ships inline tests; they run at load time (dsl.Registry.Load) and +// TestBuiltinFiltersSelfCheck asserts each test's input actually routes to its own +// filter — the check that makes a selector rewrite verifiable instead of hopeful. +// +// YAML strings are SINGLE-quoted so regex backslashes stay literal. +const builtinFilters = ` +schema_version: 1 +filters: + + pytest: + description: keep failures + summary, drop passing noise + family: tests + priority: 10 + match: '(pytest|=+ test session starts)' + strip_lines_matching: + - '^\s*$' + - ' PASSED' + - '^\.+$' + - '^=+ test session starts =+$' + - '^platform \S+ -- Python' + - '^cachedir:' + - '^rootdir:' + - '^plugins:' + cap: buildlog + on_empty: 'pytest: all passed' + + npm-install: + description: collapse npm/yarn/pnpm install chatter + family: pkg + match: '^(npm |yarn |pnpm |added \d|removed \d|up to date)' + strip_lines_matching: + - '^npm warn' + - '^\s*$' + - '^\s*[-|\\/] ' + match_output: + - pattern: 'up to date' + message: 'npm: up to date' + unless: 'error|ERR!|deprecat|[1-9]\d* (\w+ )*severity' + cap: list + on_empty: 'install: ok' + + make: + description: drop make directory chatter and no-op notices + family: builds + match: '^(make(\[\d+\])?:|(gcc|g\+\+|cc|clang) )' + strip_lines_matching: + - '^make(\[\d+\])?: (Entering|Leaving) directory' + - '^make(\[\d+\])?: Nothing to be done' + - '^Nothing to be done' + - '^\s*$' + cap: buildlog + on_empty: 'make: ok' + + gradle: + description: strip Gradle progress and no-op tasks, keep tasks and errors + family: builds + match: '^(> (Task|Configuring project|Resolving dependencies|Transform )|Starting a Gradle Daemon|BUILD (SUCCESSFUL|FAILED))' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^> Configuring project' + - '^> Resolving dependencies' + - '^> Transform ' + - '^Download(ing)?\s+http' + - '^\s*<-+>\s*$' + - '^> Task :.*UP-TO-DATE$' + - '^> Task :.*NO-SOURCE$' + - '^> Task :.*FROM-CACHE$' + - '^Starting a Gradle Daemon' + - '^Daemon will be stopped' + truncate_lines_at: 200 + cap: buildlog + on_empty: 'gradle: ok' + + xcodebuild: + description: strip xcodebuild build phases and tool invocations, keep diagnostics + family: builds + match: '^(note: Using new build system|CompileC |CompileSwift |Ld |CodeSign |PhaseScriptExecution |\*\* BUILD)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^CompileC\s' + - '^CompileSwift\s' + - '^Ld\s' + - '^CreateBuildDirectory\s' + - '^MkDir\s' + - '^ProcessInfoPlistFile\s' + - '^CopySwiftLibs\s' + - '^CodeSign\s' + - '^Signing Identity:' + - '^RegisterWithLaunchServices' + - '^Validate\s' + - '^ProcessProductPackaging' + - '^Touch\s' + - '^LinkStoryboards' + - '^CompileStoryboard' + - '^CompileAssetCatalog' + - '^GenerateDSYMFile' + - '^PhaseScriptExecution' + - '^PBXCp\s' + - '^SetMode\s' + - '^SetOwnerAndGroup\s' + - '^Ditto\s' + - '^CpResource\s' + - '^CpHeader\s' + - '^\s+cd\s+/' + - '^\s+export\s' + - '^\s+/Applications/Xcode' + - '^\s+/usr/bin/' + - '^\s+builtin-' + - '^note: Using new build system' + cap: buildlog + on_empty: 'xcodebuild: ok' + + gcc: + description: strip include traces and diagnostic counters, keep every error and warning + family: builds + match: '^(In file included from|/usr/bin/ld:|collect2: error:|\S+:\d+:(\d+:)? (error|warning|note):)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^\s+\|\s*$' + - '^In file included from' + - '^\s+from\s' + - '^\d+ warnings? generated' + - '^\d+ errors? generated' + cap: buildlog + on_empty: 'gcc: ok' + + swift-build: + description: strip Compiling/Linking noise, collapse a clean build + family: builds + match: '^(Compiling |Linking |Building for |Build complete!)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^Compiling ' + - '^Linking ' + match_output: + - pattern: 'Build complete!' + message: 'ok (build complete)' + unless: 'warning:|error:|failed|Failed' + cap: buildlog + + dotnet-build: + description: strip MSBuild banners, collapse a clean build + family: builds + match: '^(Microsoft \(R\) Build Engine|MSBuild version)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^Microsoft \(R\)' + - '^Copyright \(C\)' + - '^ Determining projects' + match_output: + # dotnet writes "0 Error(s)" even on success, so the guard names the + # diagnostic FORM (error CS1002 / warning CS0168), not the word. + - pattern: '0 Warning\(s\)\n\s+0 Error\(s\)' + message: 'ok (build succeeded)' + unless: '(error|warning) [A-Z]+\d|Build FAILED' + cap: buildlog + + turbo: + description: strip Turborepo cache status noise, keep task results + family: builds + match: '^(cache (hit|miss|bypass)|\d+ packages in scope|> [^ ]+:[^ ]+$)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^\s*cache (hit|miss|bypass)' + - '^\s*\d+ packages in scope' + - '^\s*Tasks:\s+\d+' + - '^\s*Duration:\s+' + - '^\s*Remote caching (enabled|disabled)' + truncate_lines_at: 200 + cap: buildlog + on_empty: 'turbo: ok' + + nx: + description: strip Nx task-graph banners, keep task output + family: builds + match: '^(> +NX +|Nx \(powered by)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^\s*>\s*NX\s+Running target' + - '^\s*>\s*NX\s+Nx read the output' + - '^\s*>\s*NX\s+View logs' + - '^———————' + - '^\s+Nx \(powered by' + truncate_lines_at: 200 + cap: buildlog + + terraform-plan: + description: strip state-refresh and unchanged-resource noise from a terraform/tofu plan + family: iac + match: '^(Acquiring state lock|Releasing state lock|Refreshing state|(Terraform|OpenTofu) (will perform|used the selected providers)|No changes\.)' + strip_ansi: true + strip_lines_matching: + - '^Refreshing state' + - '^\s*#.*unchanged' + - '^\s*$' + - '^Acquiring state lock' + - '^Releasing state lock' + cap: buildlog + on_empty: 'plan: no changes detected' + + terraform-init: + description: strip provider download spam from a terraform/tofu init + family: iac + match: '^Initializing (the backend|provider plugins|modules)' + strip_ansi: true + strip_lines_matching: + - '^- Downloading' + - '^- Installing' + - '^- Using previously-installed' + - '^\s*$' + - '^Initializing provider' + - '^Initializing the backend' + - '^Initializing modules' + cap: list + on_empty: 'init: ok' + + pulumi: + description: strip pulumi banners, permalinks and per-resource progress rows + family: iac + match: '^(Previewing (update|refresh|destroy)|Updating \(|Refreshing \(|Destroying \(|No stacks found|Please choose a stack|Current stack is)' + strip_ansi: true + match_output: + - pattern: 'No stacks found' + message: 'pulumi stack: empty' + unless: 'error|Error' + strip_lines_matching: + - '^\s*$' + - '^Previewing (update|refresh|destroy)' + - '^Updating \(' + - '^Refreshing \(' + - '^Destroying \(' + - '^@ (Previewing (update|refresh|destroy)|Updating|Refreshing|Destroying)' + - '^View in Browser' + - '^View Live:' + - '^Duration:' + - '^Permalink:' + - '^\s*Type\s+Name\s+' + - '^Loading policy packs' + - '^More information at:' + - '^Use \x60pulumi ' + - '^Please choose a stack' + - '^Current stack outputs \(0\):' + - '^\s+No output values currently' + - '^The resources in the stack have been deleted' + - '^If you want to remove the stack completely' + - '^\s+\+\s+.*\bcreating\s+\(' + - '^\s+~\s+.*\bupdating\s+\(' + - '^\s+-\s+.*\bdeleting\s+\(' + - '^\s+.*\brefreshing\s+\(' + - '^\s+pulumi:pulumi:Stack\s+\S+\s+running\s*$' + - '^\s{4,}at\s+\S+\s*\(' + - '^\s{4,}at\s+/' + - '^\s+at\s+processTicksAndRejections' + - '^\s+promise:\s+Promise' + - '^\s+\[Circular' + - '^\s* (Fetching|Downloading)|Warning: .+ is already installed)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^==> Downloading' + - '^==> Pouring' + - '^Already downloaded:' + - '^###' + - '^==> Fetching' + match_output: + - pattern: 'already installed' + message: 'ok (already installed)' + unless: 'Error|error:|failed' + cap: list + + quarto-render: + description: strip quarto render progress, collapse a successful render + family: builds + match: '^processing file: ' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^\s*processing file:' + - '^\s*\d+/\d+\s' + - '^\s*running' + - '^\s*Rendering' + - '^pandoc ' + - '^ Validating' + - '^ Resolving' + match_output: + - pattern: 'Output created:' + message: 'ok (output created)' + unless: 'ERROR|error:|Error|WARNING' + cap: list + +tests: + + pytest: + - name: all-green collapses + input: "===== test session starts =====\ntests/a.py::t1 PASSED\ntests/a.py::t2 PASSED\n" + expected: 'pytest: all passed' + - name: failure kept + input: "===== test session starts =====\ntests/a.py::t1 PASSED\nFAILED tests/a.py::t2 - AssertionError\n1 failed, 1 passed in 0.1s\n" + expected: "FAILED tests/a.py::t2 - AssertionError\n1 failed, 1 passed in 0.1s" + + npm-install: + - name: up to date collapses + input: "up to date, audited 240 packages in 1s\nfound 0 vulnerabilities\n" + expected: 'npm: up to date' + - name: vulnerabilities not swallowed + input: "up to date, audited 240 packages in 1s\n3 moderate severity vulnerabilities\n" + expected: "up to date, audited 240 packages in 1s\n3 moderate severity vulnerabilities" + - name: warn lines stripped + input: "npm warn deprecated q@1.5.1\nadded 12 packages in 3s\n" + expected: 'added 12 packages in 3s' + + make: + - name: strips entering/leaving lines + input: "make[1]: Entering directory '/home/user'\ngcc -O2 foo.c\nmake[1]: Leaving directory '/home/user'\n" + expected: 'gcc -O2 foo.c' + - name: strips blank lines + input: "gcc -O2 foo.c\n\ngcc -O2 bar.c\n" + expected: "gcc -O2 foo.c\ngcc -O2 bar.c" + - name: nothing to be done collapses + input: "make[1]: Entering directory '/home/user'\nmake[1]: Nothing to be done for 'all'.\nmake[1]: Leaving directory '/home/user'\n" + expected: 'make: ok' + - name: error kept + input: "make[1]: Entering directory '/home/user'\nfoo.c:3:1: error: expected declaration\nmake[1]: *** [Makefile:4: foo.o] Error 1\n" + expected: "foo.c:3:1: error: expected declaration\nmake[1]: *** [Makefile:4: foo.o] Error 1" + + gradle: + - name: strips UP-TO-DATE tasks, keeps build result + input: "> Configuring project :app\n> Task :app:compileJava UP-TO-DATE\n> Task :app:compileKotlin UP-TO-DATE\n> Task :app:test\n\n3 tests completed, 1 failed\n\nBUILD FAILED in 12s" + expected: "> Task :app:test\n3 tests completed, 1 failed\nBUILD FAILED in 12s" + - name: clean build preserved + input: "BUILD SUCCESSFUL in 8s\n7 actionable tasks: 7 executed" + expected: "BUILD SUCCESSFUL in 8s\n7 actionable tasks: 7 executed" + - name: empty after stripping + input: "> Configuring project :app\n" + expected: 'gradle: ok' + + xcodebuild: + - name: strips build phases, keeps errors and summary + input: "note: Using new build system\nCompileSwift normal arm64 /d/App/ViewController.swift\n cd /d/App\n /Applications/Xcode.app/Contents/Developer/usr/bin/swift-frontend -c\nLd /d/Build/App normal arm64\n cd /d/App\nCodeSign /d/Build/App.app\n builtin-codesign --force --sign\n\n/d/App/ViewController.swift:42:9: error: use of unresolved identifier 'foo'\n/d/App/Model.swift:18:5: warning: variable 'x' was never used\n\n** BUILD FAILED **\n" + expected: "/d/App/ViewController.swift:42:9: error: use of unresolved identifier 'foo'\n/d/App/Model.swift:18:5: warning: variable 'x' was never used\n** BUILD FAILED **" + - name: clean build success + input: "note: Using new build system\nCompileSwift normal arm64 /d/App/Main.swift\n cd /d/App\nLd /d/Build/App normal arm64\nCodeSign /d/Build/App.app\n builtin-codesign --force --sign\n\n** BUILD SUCCEEDED **\n" + expected: '** BUILD SUCCEEDED **' + - name: test results kept + input: "note: Using new build system\nCompileSwift normal arm64 /d/AppTests/Tests.swift\n cd /d/App\nTest Case '-[AppTests testExample]' passed (0.001 seconds).\nTest Case '-[AppTests testFailing]' failed (0.002 seconds).\nExecuted 2 tests, with 1 failure in 0.003 seconds\n" + expected: "Test Case '-[AppTests testExample]' passed (0.001 seconds).\nTest Case '-[AppTests testFailing]' failed (0.002 seconds).\nExecuted 2 tests, with 1 failure in 0.003 seconds" + + gcc: + - name: strips include chain, keeps errors and warnings + input: "In file included from /usr/include/stdio.h:42:\n from main.c:1:\nmain.c:10:5: error: use of undeclared identifier 'foo'\n foo();\n ^\nmain.c:15:12: warning: unused variable 'x' [-Wunused-variable]\n int x = 42;\n ^\n2 warnings generated.\n1 error generated.\n" + expected: "main.c:10:5: error: use of undeclared identifier 'foo'\n foo();\n ^\nmain.c:15:12: warning: unused variable 'x' [-Wunused-variable]\n int x = 42;\n ^" + - name: linker error kept + input: "/usr/bin/ld: /tmp/main.o: undefined reference to 'missing_func'\ncollect2: error: ld returned 1 exit status\n" + expected: "/usr/bin/ld: /tmp/main.o: undefined reference to 'missing_func'\ncollect2: error: ld returned 1 exit status" + + swift-build: + - name: successful build collapses + input: "Build complete! (4.21s)\n" + expected: 'ok (build complete)' + - name: build errors pass through after stripping noise + input: "Compiling MyApp MyApp.swift\n/h/Sources/MyApp/main.swift:5:1: error: use of unresolved identifier 'foo'\nfoo()\n^~~\nLinking MyApp\nerror: build had 1 command failure\n" + expected: "/h/Sources/MyApp/main.swift:5:1: error: use of unresolved identifier 'foo'\nfoo()\n^~~\nerror: build had 1 command failure" + - name: warnings not swallowed when Build complete present + input: "Compiling MyApp MyFile.swift\n/path/to/MyFile.swift:42:10: warning: unused variable 'x'\nBuild complete! (with warnings)\n" + expected: "/path/to/MyFile.swift:42:10: warning: unused variable 'x'\nBuild complete! (with warnings)" + + dotnet-build: + - name: successful build collapses + input: "Microsoft (R) Build Engine version 17.8.3\nCopyright (C) Microsoft Corporation. All rights reserved.\n\n Determining projects to restore...\n MyApp -> /h/MyApp/bin/Debug/net8.0/MyApp.dll\n\nBuild succeeded.\n 0 Warning(s)\n 0 Error(s)\n\nTime Elapsed 00:00:02.34\n" + expected: 'ok (build succeeded)' + - name: build with warnings not collapsed + input: "Microsoft (R) Build Engine version 17.8.3\nCopyright (C) Microsoft Corporation. All rights reserved.\n\n MyApp -> /h/MyApp/bin/Debug/net8.0/MyApp.dll\n\nBuild succeeded.\n 3 Warning(s)\n 0 Error(s)\n\nTime Elapsed 00:00:01.87\n" + expected: " MyApp -> /h/MyApp/bin/Debug/net8.0/MyApp.dll\nBuild succeeded.\n 3 Warning(s)\n 0 Error(s)\nTime Elapsed 00:00:01.87" + - name: zero-count warning line does not swallow a real diagnostic + input: "Microsoft (R) Build Engine version 17.8.3\nsrc/Program.cs(9,5): warning CS0168: variable declared but never used\nBuild succeeded.\n 0 Warning(s)\n 0 Error(s)\n" + expected: "src/Program.cs(9,5): warning CS0168: variable declared but never used\nBuild succeeded.\n 0 Warning(s)\n 0 Error(s)" + - name: build errors pass through + input: "Microsoft (R) Build Engine version 17.8.3\nCopyright (C) Microsoft Corporation. All rights reserved.\n\n Determining projects to restore...\nsrc/Program.cs(10,5): error CS1002: ; expected [/h/MyApp/MyApp.csproj]\n\nBuild FAILED.\n 0 Warning(s)\n 1 Error(s)\n" + expected: "src/Program.cs(10,5): error CS1002: ; expected [/h/MyApp/MyApp.csproj]\nBuild FAILED.\n 0 Warning(s)\n 1 Error(s)" + + turbo: + - name: strips cache noise, keeps task output + input: " cache hit, replaying logs abc123\n cache miss, executing abc456\n\n3 packages in scope\n\n> myapp:build\n\nCompiled successfully.\n\nTasks: 2 successful, 2 total (1 cached)\nDuration: 3.2s" + expected: "> myapp:build\nCompiled successfully." + - name: preserves error output + input: "> myapp:lint\n\nError: src/index.ts(5,1): error TS2304\n\nTasks: 0 successful, 1 total\nDuration: 1.1s" + expected: "> myapp:lint\nError: src/index.ts(5,1): error TS2304" + - name: empty after stripping + input: " cache hit, replaying logs abc\n\n" + expected: 'turbo: ok' + + nx: + - name: strips Nx noise, keeps build output + input: "\n > NX Running target build for project myapp\n\n———————————————————————————————————————\nCompiled successfully.\nOutput: dist/apps/myapp\n\n > NX View logs at /tmp/.nx/runs/abc123\n\n Nx (powered by computation caching)\n" + expected: "Compiled successfully.\nOutput: dist/apps/myapp" + - name: preserves error output + input: " > NX Running target build for project myapp\n\nERROR: Cannot find module '@myapp/shared'\nFailed at step: build\n\n > NX View logs at /tmp/.nx/runs/abc\n" + expected: "ERROR: Cannot find module '@myapp/shared'\nFailed at step: build" + + terraform-plan: + - name: strips refresh and lock noise + input: "Acquiring state lock. This may take a few moments...\nRefreshing state... [id=vpc-abc]\nRefreshing state... [id=sg-123]\nReleasing state lock. This may take a few moments...\n\nTerraform will perform the following actions:\n\n # aws_instance.web will be created\n + resource \"aws_instance\" \"web\" {}\n\nPlan: 1 to add, 0 to change, 0 to destroy.\n" + expected: "Terraform will perform the following actions:\n # aws_instance.web will be created\n + resource \"aws_instance\" \"web\" {}\nPlan: 1 to add, 0 to change, 0 to destroy." + - name: opentofu plan also handled + input: "Acquiring state lock. This may take a few moments...\nRefreshing state... [id=vpc-abc123]\nReleasing state lock. This may take a few moments...\n\nOpenTofu will perform the following actions:\n\n # aws_instance.web will be created\n\nPlan: 1 to add, 0 to change, 0 to destroy.\n" + expected: "OpenTofu will perform the following actions:\n # aws_instance.web will be created\nPlan: 1 to add, 0 to change, 0 to destroy." + - name: no-changes result preserved + input: "Refreshing state... [id=vpc-abc]\nNo changes. Your infrastructure matches the configuration.\n" + expected: 'No changes. Your infrastructure matches the configuration.' + - name: on_empty when all noise stripped + input: "Refreshing state... [id=vpc-abc]\nAcquiring state lock. This may take a few moments...\nReleasing state lock. This may take a few moments...\n" + expected: 'plan: no changes detected' + - name: unchanged-resource comments dropped, changes kept + input: "Refreshing state... [id=vpc-abc]\nTerraform will perform the following actions:\n # aws_s3_bucket.a will be created\n # (12 unchanged attributes hidden)\n # (3 unchanged blocks hidden)\nPlan: 1 to add, 0 to change, 0 to destroy.\n" + expected: "Terraform will perform the following actions:\n # aws_s3_bucket.a will be created\nPlan: 1 to add, 0 to change, 0 to destroy." + + terraform-init: + - name: strips downloading/installing lines + input: "Initializing the backend...\nInitializing provider plugins...\n- Downloading hashicorp/aws 5.0.0...\n- Installing hashicorp/aws 5.0.0...\n- Using previously-installed hashicorp/random 3.5.1\n\nOpenTofu has been successfully initialized!\n" + expected: 'OpenTofu has been successfully initialized!' + - name: on_empty when all noise stripped + input: "Initializing the backend...\nInitializing provider plugins...\n- Using previously-installed hashicorp/aws 5.0.0\n\n" + expected: 'init: ok' + - name: init error kept + input: "Initializing the backend...\nError: Failed to get existing workspaces: S3 bucket does not exist.\n" + expected: 'Error: Failed to get existing workspaces: S3 bucket does not exist.' + + pulumi: + - name: preview strips header, url and duration noise + input: "Previewing update (dev)\n\nView in Browser (Ctrl+O): https://app.pulumi.com/org/p/dev/previews/abc\n\n Type Name Plan\n + pulumi:pulumi:Stack my-proj-dev create\n + └─ aws:s3:Bucket my-bucket create\n\nResources:\n + 2 to create\n\nDuration: 3s\n" + expected: " + pulumi:pulumi:Stack my-proj-dev create\n + └─ aws:s3:Bucket my-bucket create\nResources:\n + 2 to create" + - name: up strips header and url banner + input: "Updating (dev)\n\nView in Browser (Ctrl+O): https://app.pulumi.com/org/p/dev/updates/42\n\n Type Name Status\n + pulumi:pulumi:Stack my-proj-dev created\n\nOutputs:\n bucket_name: \"my-bucket-abc123\"\n\nResources:\n + 2 created\n\nDuration: 15s\n" + expected: " + pulumi:pulumi:Stack my-proj-dev created\nOutputs:\n bucket_name: \"my-bucket-abc123\"\nResources:\n + 2 created" + - name: refresh passes drift rows + input: "Refreshing (dev)\n\nView in Browser (Ctrl+O): https://app.pulumi.com/org/p/dev/updates/44\n\n Type Name Status\n ~ aws:s3:Bucket my-bucket refreshed\n\nResources:\n ~ 1 updated\n\nDuration: 4s\n" + expected: " ~ aws:s3:Bucket my-bucket refreshed\nResources:\n ~ 1 updated" + - name: destroy strips header and url banner + input: "Destroying (dev)\n\nView in Browser (Ctrl+O): https://app.pulumi.com/org/p/dev/updates/43\n\n Type Name Status\n - pulumi:pulumi:Stack my-proj-dev deleted\n\nResources:\n - 2 deleted\n\nDuration: 7s\n" + expected: " - pulumi:pulumi:Stack my-proj-dev deleted\nResources:\n - 2 deleted" + - name: on_empty when only noise lines present + input: "Previewing update (dev)\n\nView in Browser (Ctrl+O): https://app.pulumi.com/org/p/dev/previews/abc\n\nDuration: 1s\n" + expected: 'pulumi: no changes' + - name: no stacks collapses + input: "No stacks found in the current workspace.\n" + expected: 'pulumi stack: empty' + - name: stack identity kept, prompt noise stripped + input: "Please choose a stack, or create a new one:\nCurrent stack is dev:\n Managed by my-user\n Owner: my-org\n\nCurrent stack resources (3):\n TYPE NAME\n pulumi:pulumi:Stack my-proj-dev\n" + expected: "Current stack is dev:\n Managed by my-user\n Owner: my-org\nCurrent stack resources (3):\n TYPE NAME\n pulumi:pulumi:Stack my-proj-dev" + - name: js stack frames pruned, error message kept + input: "Updating (dev)\n\n error: Error: connect ECONNREFUSED 127.0.0.1:443\n at TCPConnectWrap.afterConnect (node:net:1595:16)\n at /home/u/node_modules/@pulumi/runtime/invoke.js:120:23\n at processTicksAndRejections (node:internal/process:95:5)\n\nDuration: 2s\n" + expected: ' error: Error: connect ECONNREFUSED 127.0.0.1:443' + + liquibase: + - name: strip ascii banner and info logs + input: "####################################################\n## _ _ _ _ ##\n####################################################\nStarting Liquibase at 10:12:11 (version 4.29.1)\nLiquibase Version: 4.29.1\nLiquibase Open Source 4.29.1 by Liquibase\nINFO [liquibase.integration] Starting command\nINFO [liquibase.core] Reading resource db/changelog.xml\nINFO [liquibase.core] Parsing db/changelog.xml\nRunning Changeset: filepath::id::author\nChangeset filepath::id::author ran successfully\n" + expected: "Liquibase Version: 4.29.1\nRunning Changeset: filepath::id::author\nChangeset filepath::id::author ran successfully" + - name: strip jar inventory, keep version line + input: "####################################################\n## _ _ _ _ ##\n####################################################\nStarting Liquibase at 13:45:24 using Java 17.0.15\nLiquibase Home: /opt/liquibase\nJava Home /usr/lib/jvm/jdk-17 (Version 17.0.15)\nLibraries:\n - internal/lib/commons-io.jar: Apache Commons IO 2.17.0\n - internal/lib/picocli.jar: picocli 4.7.6\n\nLiquibase Version: 4.30.0\nLiquibase Open Source 4.30.0 by Liquibase\n" + expected: 'Liquibase Version: 4.30.0' + - name: keep status and error lines + input: "####################################################\n## _ _ _ _ ##\n####################################################\nStarting Liquibase at 10:00:00 (version 4.30.0)\nLiquibase Version: 4.30.0\nLiquibase Open Source 4.30.0 by Liquibase\nHR@jdbc:oracle:thin:@localhost:1523:XE is up to date\nLiquibase command 'status' was executed successfully.\n" + expected: "Liquibase Version: 4.30.0\nHR@jdbc:oracle:thin:@localhost:1523:XE is up to date\nLiquibase command 'status' was executed successfully." + + ssh: + - name: strips connection banners, keeps command output + input: "Warning: Permanently added '192.168.1.10' (ED25519) to the list of known hosts.\n\ntotal 32\ndrwxr-xr-x 4 user user 4096 Mar 10 12:00 app\n-rw-r--r-- 1 user user 1234 Mar 10 11:00 config.yaml\n\nConnection to 192.168.1.10 closed.\n" + expected: "total 32\ndrwxr-xr-x 4 user user 4096 Mar 10 12:00 app\n-rw-r--r-- 1 user user 1234 Mar 10 11:00 config.yaml" + - name: verbose debug lines stripped + input: "debug1: Connecting to host.example.com port 22.\ndebug1: Connection established.\nAuthenticated to host.example.com ([1.2.3.4]:22).\nuptime: 12:00:00 up 42 days, load average: 0.10, 0.15, 0.12\nConnection to host.example.com closed.\n" + expected: 'uptime: 12:00:00 up 42 days, load average: 0.10, 0.15, 0.12' + - name: remote failure kept + input: "debug1: Connecting to host.example.com port 22.\nPermission denied (publickey).\n" + expected: 'Permission denied (publickey).' + + ping: + - name: success keeps summary only + input: "PING example.com (93.184.216.34): 56 data bytes\n64 bytes from 93.184.216.34: icmp_seq=0 ttl=56 time=14.2 ms\n64 bytes from 93.184.216.34: icmp_seq=1 ttl=56 time=13.8 ms\n64 bytes from 93.184.216.34: icmp_seq=2 ttl=56 time=14.1 ms\n64 bytes from 93.184.216.34: icmp_seq=3 ttl=56 time=13.9 ms\n\n--- example.com ping statistics ---\n4 packets transmitted, 4 packets received, 0.0% packet loss\nround-trip min/avg/max/stddev = 13.8/14.0/14.2/0.2 ms\n" + expected: "--- example.com ping statistics ---\n4 packets transmitted, 4 packets received, 0.0% packet loss\nround-trip min/avg/max/stddev = 13.8/14.0/14.2/0.2 ms" + - name: windows format keeps stats block only + input: "Pinging 192.0.2.1 with 32 bytes of data:\nReply from 192.0.2.1: bytes=32 time=14ms TTL=56\nReply from 192.0.2.1: bytes=32 time=13ms TTL=56\nReply from 192.0.2.1: bytes=32 time=14ms TTL=56\nReply from 192.0.2.1: bytes=32 time=13ms TTL=56\n\nPing statistics for 192.0.2.1:\n Packets: Sent = 4, Received = 4, Lost = 0 (0% loss),\nApproximate round trip times in milli-seconds:\n Minimum = 13ms, Maximum = 14ms, Average = 13ms\n" + expected: "Ping statistics for 192.0.2.1:\n Packets: Sent = 4, Received = 4, Lost = 0 (0% loss),\nApproximate round trip times in milli-seconds:\n Minimum = 13ms, Maximum = 14ms, Average = 13ms" + - name: unreachable host passes error through + input: "PING unreachable.example.com (192.0.2.1): 56 data bytes\nRequest timeout for icmp_seq 0\nRequest timeout for icmp_seq 1\n\n--- unreachable.example.com ping statistics ---\n2 packets transmitted, 0 packets received, 100.0% packet loss\n" + expected: "Request timeout for icmp_seq 0\nRequest timeout for icmp_seq 1\n--- unreachable.example.com ping statistics ---\n2 packets transmitted, 0 packets received, 100.0% packet loss" + + rsync: + - name: successful sync collapses + input: "sending incremental file list\n./\nfile1.txt\nfile2.txt\n\nsent 1,234 bytes received 42 bytes 2,552.00 bytes/sec\ntotal size is 98,765 speedup is 77.31\n" + expected: 'ok (synced)' + - name: error lines pass through + input: "sending incremental file list\nrsync: [Receiver] mkdir \"/remote/path\" failed: Permission denied (13)\nrsync error: error in file system (code 11) at receiver.c(741)\n" + expected: "rsync: [Receiver] mkdir \"/remote/path\" failed: Permission denied (13)\nrsync error: error in file system (code 11) at receiver.c(741)" + - name: errors not swallowed when total size present + input: "rsync: [sender] error\nerror in rsync protocol data stream (code 12)\nsent 100 bytes received 200 bytes 60.00 bytes/sec\ntotal size is 1000 speedup is 3.33\n" + expected: "rsync: [sender] error\nerror in rsync protocol data stream (code 12)\ntotal size is 1000 speedup is 3.33" + + bundle-install: + - name: all cached collapses + input: "Using bundler 2.5.6\nUsing rake 13.1.0\nUsing ast 2.4.2\nUsing minitest 5.22.2\nBundle complete! 85 Gemfile dependencies, 200 gems now installed.\nUse 'bundle info [gemname]' to see where a bundled gem is installed.\n" + expected: 'ok bundle: complete' + - name: mixed install collapses + input: "Fetching gem metadata from https://rubygems.org/.........\nResolving dependencies...\nUsing rake 13.1.0\nFetching rspec 3.13.0\nInstalling rspec 3.13.0\nBundle complete! 85 Gemfile dependencies, 202 gems now installed.\n" + expected: 'ok bundle: complete' + - name: update output collapses + input: "Fetching gem metadata from https://rubygems.org/.........\nResolving dependencies...\nUsing rake 13.1.0\nInstalling rspec 3.14.0 (was 3.13.0)\nBundle updated!\n" + expected: 'ok bundle: updated' + - name: conflict not swallowed by Bundle complete + input: "Fetching gem metadata from https://rubygems.org/.........\nwarning: rack 3.0 conflicts with rails 6.1\nBundle complete! 5 Gemfile dependencies, 9 gems now installed.\n" + expected: "warning: rack 3.0 conflicts with rails 6.1\nBundle complete! 5 Gemfile dependencies, 9 gems now installed." + + poetry-install: + - name: up to date collapses + input: "Installing dependencies from lock file\n\nNo dependencies to install or update\n" + expected: 'ok (up to date)' + - name: bullet syntax collapses + input: "• Installing requests (2.31.0)\n• Installing certifi (2023.11.17)\n\nNo changes.\n" + expected: 'ok (up to date)' + - name: install strips download lines + input: "Installing dependencies from lock file\n\n - Downloading requests-2.31.0-py3-none-any.whl (62.6 kB)\n - Installing certifi (2023.11.17)\n - Installing charset-normalizer (3.3.2)\n - Installing requests (2.31.0)\n\nWriting lock file\n" + expected: "Installing dependencies from lock file\nWriting lock file" + - name: solver error not swallowed + input: "Installing dependencies from lock file\nSolverProblemError: version solving failed\nNo changes.\n" + expected: "Installing dependencies from lock file\nSolverProblemError: version solving failed\nNo changes." + + composer-install: + - name: nothing to do collapses + input: "Loading composer repositories with package information\nUpdating dependencies\nLock file operations: 0 installs, 0 updates, 0 removals\nNothing to install, update or remove\nGenerating autoload files\n" + expected: 'ok (up to date)' + - name: install strips download lines + input: "Loading composer repositories with package information\nUpdating dependencies\n - Downloading symfony/console (v6.4.0)\n - Installing symfony/console (v6.4.0): Extracting archive\n - Downloading psr/log (3.0.0)\nWriting lock file\nGenerating autoload files\n" + expected: "Writing lock file\nGenerating autoload files" + - name: abandoned package warning not swallowed + input: "Loading composer repositories with package information\nWarning: Package foo/bar is abandoned, use baz/qux instead.\nNothing to install, update or remove\n" + expected: "Warning: Package foo/bar is abandoned, use baz/qux instead.\nNothing to install, update or remove" + + uv-sync: + - name: audited packages collapses + input: "Resolved 42 packages in 123ms\nAudited 42 packages in 0.05ms\n" + expected: 'ok (up to date)' + - name: install strips download and cached lines + input: " Downloading requests-2.31.0-py3-none-any.whl (62.6 kB)\n Using cached certifi-2023.11.17-py3-none-any.whl (162 kB)\n Preparing packages...\nInstalled 5 packages in 23ms\n + certifi==2023.11.17\n + requests==2.31.0\n" + expected: "Installed 5 packages in 23ms\n + certifi==2023.11.17\n + requests==2.31.0" + - name: failure not swallowed by Audited + input: "Resolved 42 packages in 123ms\nwarning: 'pytest' was not found in the lockfile\nAudited 42 packages in 0.05ms\n" + expected: "Resolved 42 packages in 123ms\nwarning: 'pytest' was not found in the lockfile\nAudited 42 packages in 0.05ms" + + brew-install: + - name: already installed collapses + input: "Warning: jq 1.7.1 is already installed and up-to-date.\nTo reinstall 1.7.1, run:\n brew reinstall jq\n" + expected: 'ok (already installed)' + - name: install strips download lines + input: "==> Fetching jq\n==> Downloading https://ghcr.io/v2/homebrew/core/jq/blobs/sha256:abc\n######################################################################## 100.0%\n==> Pouring jq-1.7.1.arm64_sonoma.bottle.tar.gz\n==> Summary\n/opt/homebrew/Cellar/jq/1.7.1: 18 files, 1.2MB\n" + expected: "==> Summary\n/opt/homebrew/Cellar/jq/1.7.1: 18 files, 1.2MB" + - name: error not swallowed by already installed + input: "Warning: jq 1.7.1 is already installed and up-to-date.\nError: Could not link jq: permission denied\n" + expected: "Warning: jq 1.7.1 is already installed and up-to-date.\nError: Could not link jq: permission denied" + + quarto-render: + - name: success collapses + input: "processing file: index.qmd\n Validating schema\n Resolving resources\npandoc to html5\nOutput created: _site/index.html\n" + expected: 'ok (output created)' + - name: error passes through + input: "processing file: broken.qmd\n Validating schema\nERROR: Render failed\n\ncaused by:\n syntax error at line 10\n" + expected: "ERROR: Render failed\ncaused by:\n syntax error at line 10" + - name: warning not swallowed by Output created + input: "processing file: index.qmd\nWARNING: unable to resolve crossref @fig-1\nOutput created: _site/index.html\n" + expected: "WARNING: unable to resolve crossref @fig-1\nOutput created: _site/index.html" +` diff --git a/components/offload/cmdfilter_test.go b/components/offload/cmdfilter_test.go new file mode 100644 index 0000000..8b15484 --- /dev/null +++ b/components/offload/cmdfilter_test.go @@ -0,0 +1,214 @@ +package offload + +import ( + "context" + "strings" + "testing" + + "github.com/maximhq/bifrost/core/schemas" + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/components/dsl" + "github.com/rossoctl/context-guru/expand" + "github.com/rossoctl/context-guru/schema" + "github.com/rossoctl/context-guru/store" + "gopkg.in/yaml.v3" +) + +func builtinDoc(t *testing.T) dsl.File { + t.Helper() + var f dsl.File + if err := yaml.Unmarshal([]byte(builtinFilters), &f); err != nil { + t.Fatal(err) + } + return f +} + +// The builtins load (which also RUNS every inline test — see dsl.Registry.Load). +func TestBuiltinFiltersLoad(t *testing.T) { + var r dsl.Registry + if err := r.Load([]byte(builtinFilters)); err != nil { + t.Fatal(err) + } + if r.Len() < 20 { + t.Fatalf("expected the full ported set, got %d filters", r.Len()) + } +} + +// Guardrail (rtk's build.rs): every shipped filter must carry at least one test, +// and every test's input must actually ROUTE to its own filter. The second half is +// the check that makes the selector rewrite verifiable — an rtk `match_command` +// regex copied over would compile fine and simply never fire. +func TestEveryBuiltinFilterHasTestsAndRoutes(t *testing.T) { + f := builtinDoc(t) + var r dsl.Registry + if err := r.Load([]byte(builtinFilters)); err != nil { + t.Fatal(err) + } + for name := range f.Filters { + cases := f.Tests[name] + if len(cases) == 0 { + t.Errorf("filter %q ships no tests", name) + continue + } + for _, tc := range cases { + if strings.TrimSpace(tc.Input) == "" { + continue // an empty-input case has no selector to route + } + got := r.Match(selectorKey(tc.Input)) + if got == nil { + t.Errorf("filter %q test %q: selector %q matches NO filter", name, tc.Name, selectorKey(tc.Input)) + continue + } + if got.Name != name { + t.Errorf("filter %q test %q: selector %q routes to %q instead", name, tc.Name, selectorKey(tc.Input), got.Name) + } + } + } +} + +// Every success-collapse rule must carry an unless guard: in a proxy the agent +// cannot re-run the command to find the warning a bare collapse swallowed. +func TestEveryMatchOutputRuleIsGuarded(t *testing.T) { + for name, def := range builtinDoc(t).Filters { + for i, rule := range def.MatchOutput { + if rule.Unless == "" { + t.Errorf("filter %q match_output[%d] (%q) has no unless guard", name, i, rule.Pattern) + } + } + } +} + +// Every filter must declare a family, else its savings land in "other" and the +// per-family attribution in /stats is useless. +func TestEveryBuiltinFilterHasFamily(t *testing.T) { + for name, def := range builtinDoc(t).Filters { + if def.Family == "" { + t.Errorf("filter %q has no family", name) + } + } +} + +// A tail cut is recoverable more cheaply than a whole-blob loss, and the hint must +// say so rather than pushing the agent to the expensive path for both. +func TestRecoveryHintDistinguishesTailFromWhole(t *testing.T) { + if got := recoveryHint(dsl.LossNone, 3); got != "" { + t.Fatalf("no loss must emit no hint, got %q", got) + } + tail, whole := recoveryHint(dsl.LossTail, 42), recoveryHint(dsl.LossWhole, 42) + if tail == whole { + t.Fatal("LossTail and LossWhole must not share one hint") + } + if !strings.Contains(tail, "42") || !strings.Contains(tail, "truncated") { + t.Fatalf("tail hint should name the cut point: %q", tail) + } + if !strings.Contains(whole, expand.ToolName) { + t.Fatalf("whole-blob hint must name the expand tool: %q", whole) + } +} + +func cmdToolMsg(s string) schemas.ChatMessage { + c := s + return schemas.ChatMessage{Role: schemas.ChatMessageRoleTool, Content: &schemas.ChatMessageContent{ContentStr: &c}} +} + +func newFilterComp(t *testing.T, cfg string) components.Offload { + t.Helper() + comp, err := newCmdfilter([]byte(cfg)) + if err != nil { + t.Fatal(err) + } + return comp.(components.Offload) +} + +func runFilter(t *testing.T, f components.Offload, text string) (string, *components.Report) { + t.Helper() + req := &schemas.BifrostChatRequest{Provider: schemas.Anthropic, Input: []schemas.ChatMessage{cmdToolMsg(text)}} + c := &components.Ctx{Ctx: context.Background(), Session: "s", Store: store.NewMemory(store.Options{}), MaxCachedIdx: -1} + rep := &components.Report{} + if _, err := f.Offload(req, rep, c); err != nil { + t.Fatal(err) + } + return schema.MessageText(req.Input[0]), rep +} + +// Below the size floor the marker routinely costs more than the filter saves, so +// nothing is touched at all (no stash, no marker). +func TestSizeFloorSkipsSmallOutputs(t *testing.T) { + small := "make[1]: Entering directory '/x'\ngcc -O2 foo.c\nmake[1]: Leaving directory '/x'\n" + if len(small) >= defaultMinSize { + t.Fatalf("fixture is not below the floor (%d bytes)", len(small)) + } + out, rep := runFilter(t, newFilterComp(t, ""), small) + if out != small || !rep.Skipped { + t.Fatalf("output below the size floor must be untouched: %q skipped=%v", out, rep.Skipped) + } + out2, _ := runFilter(t, newFilterComp(t, "min_size: 1\n"), small) + if strings.Contains(out2, "Entering directory") { + t.Fatalf("with min_size: 1 the chatter should be stripped: %q", out2) + } +} + +func planFixture() string { + var b strings.Builder + b.WriteString("Acquiring state lock. This may take a few moments...\n") + for i := 0; i < 40; i++ { + b.WriteString("Refreshing state... [id=subnet-0000000" + string(rune('a'+i%26)) + "]\n") + } + b.WriteString("\nTerraform will perform the following actions:\n\n # aws_instance.web will be created\n # (14 unchanged attributes hidden)\n\nPlan: 1 to add, 0 to change, 0 to destroy.\n") + return b.String() +} + +// Compression floors rtk asserts for its own equivalents (terraform-plan, make). +func TestCompressionFloors(t *testing.T) { + var r dsl.Registry + if err := r.Load([]byte(builtinFilters)); err != nil { + t.Fatal(err) + } + var mk strings.Builder + for i := 0; i < 30; i++ { + mk.WriteString("make[1]: Entering directory '/src/pkg'\ncc -c -O2 file.c\nmake[1]: Leaving directory '/src/pkg'\n") + } + for _, tc := range []struct{ name, input string }{ + {"terraform-plan", planFixture()}, + {"make", mk.String()}, + } { + c := r.Match(selectorKey(tc.input)) + if c == nil || c.Name != tc.name { + t.Fatalf("%s fixture routed to %v", tc.name, c) + } + out, _ := dsl.Apply(c, tc.input) + saved := 1 - float64(schema.TextTokens(out))/float64(schema.TextTokens(tc.input)) + if saved < 0.60 { + t.Errorf("%s saved only %.1f%% (floor 60%%)", tc.name, saved*100) + } + } +} + +// The per-family ledger is what lets /stats say which families pay off. +type fakeStats struct { + acts []string + misses []string +} + +func (f *fakeStats) FilterAct(family, filter, key string, saved int) { + f.acts = append(f.acts, family+"/"+filter) +} +func (f *fakeStats) FilterMiss(sel string) { f.misses = append(f.misses, sel) } + +func TestFamilyMetricsAndSelectorMisses(t *testing.T) { + fs := &fakeStats{} + req := &schemas.BifrostChatRequest{Provider: schemas.Anthropic, Input: []schemas.ChatMessage{ + cmdToolMsg(planFixture()), + cmdToolMsg("Totally unrecognized first line\n" + strings.Repeat("filler line to clear the size floor\n", 30)), + }} + c := &components.Ctx{Ctx: context.Background(), Session: "s", Store: store.NewMemory(store.Options{}), MaxCachedIdx: -1, FilterStats: fs} + if _, err := newFilterComp(t, "").Offload(req, &components.Report{}, c); err != nil { + t.Fatal(err) + } + if len(fs.acts) != 1 || fs.acts[0] != "iac/terraform-plan" { + t.Fatalf("expected one iac/terraform-plan act, got %v", fs.acts) + } + if len(fs.misses) != 1 || fs.misses[0] != "Totally unrecognized first line" { + t.Fatalf("expected the unmatched selector to be logged, got %v", fs.misses) + } +} diff --git a/components/pipeline.go b/components/pipeline.go index 7a1786d..1c3357c 100644 --- a/components/pipeline.go +++ b/components/pipeline.go @@ -30,6 +30,13 @@ func NewPipeline(comps []Component, e Emitter) *Pipeline { // are rolled back, so the returned request is never worse than the input. func (p *Pipeline) Run(req *schemas.BifrostChatRequest, c *Ctx) *RunReport { rr := &RunReport{Session: c.Session, TokensBefore: schema.MessagesTokens(req)} + // Hand cmdfilter its per-family ledger sink when the emitter implements one, so no + // host has to thread a second field through every Ctx construction site. + if c.FilterStats == nil { + if s, ok := p.emitter.(FilterStatsSink); ok { + c.FilterStats = s + } + } if c.Bypass { rr.TokensAfter = rr.TokensBefore return rr diff --git a/docs/components.md b/docs/components.md index 7867f7d..85106e5 100644 --- a/docs/components.md +++ b/docs/components.md @@ -179,8 +179,8 @@ after: [superseded by a later run] <> [full output: …] [run 2] ### `cmdfilter` Shrinks tool output with **declarative DSL filters** (see below). Matches a filter on the output's first non-empty line, applies its 8-stage pipeline, stashes the original, and appends a recovery -hint only when the filter was actually lossy. Ships builtin `pytest` / `npm-install` / `make` -filters. +hint only when the filter was actually lossy. Ships 23 filters (test runners, build tools, package +managers, IaC plans, verbose network clients) — see [cmdfilter](components/cmdfilter.md). ``` before: pytest … 100 lines of PASSED + warnings + 1 failure diff --git a/docs/components/cmdfilter.md b/docs/components/cmdfilter.md index 1742525..c5df646 100644 --- a/docs/components/cmdfilter.md +++ b/docs/components/cmdfilter.md @@ -8,34 +8,160 @@ `cmdfilter` shrinks tool output with **declarative DSL filters** (see [The DSL filter engine](dsl.md)). It matches a filter on the output's first non-empty line, applies its 8-stage pipeline, stashes the original, and appends a recovery hint only when the filter was -actually lossy. It ships builtin `pytest` / `npm-install` / `make` filters, and is `Enabled` only -when ≥1 filter is loaded. +actually lossy. It is `Enabled` only when ≥1 filter is loaded. + +Deterministic filtering costs nothing — no LLM call, ~0 latency — and it is cache-safe: it acts on +the newest tool output, in the mutable tail. + +### Selectors match OUTPUT, not commands + +The shipped filters are adapted from [rtk](https://github.com/rtk-ai/rtk) (Apache-2.0 — see +`THIRD-PARTY-NOTICES`), with one systematic rewrite. rtk is a shell hook: it matches a **command +string** (`^terraform\s+plan`). A proxy never sees the command — it sees the *result*. So every +filter here matches an **output-shape signature** against the output's first non-empty line +(`^Refreshing state`, `^> Task :`, `^==> Downloading`). rtk's command regexes are not portable as +written; copied over they would compile fine and never fire. + +That is also the structural advantage: rtk's hook only sees Bash calls, so an agent's built-in +`Read`/`Grep`/`Glob` tools are invisible to it. A proxy sees every tool result regardless of origin. + +A `TestEveryBuiltinFilterHasTestsAndRoutes` guardrail asserts every filter's own test input actually +routes to that filter, so a selector rewrite is verified rather than hoped for. + +### Size floor + +Below `min_size` bytes (default **500**, rtk's `MIN_TEE_SIZE`) `cmdfilter` doesn't filter at all — +the recovery marker routinely costs more tokens than the saving. The marker-inclusive never-worse +check would reject those rewrites anyway; the floor skips the work and the stash instead. + +## The shipped filter set + +23 filters. Compression is measured on each filter's own fixtures (its inline tests), summed: + +| filter | family | preserves | drops | saved | +|---|---|---|---|--:| +| `pytest` | tests | failures, summary line | `PASSED` lines, progress dots, session header | 57% | +| `npm-install` | pkg | added/removed counts, vulnerability counts | `npm warn`, spinner lines | 46% | +| `make` | builds | compiler lines, errors | `Entering/Leaving directory`, `Nothing to be done` | 56% | +| `gradle` | builds | executed tasks, test results, BUILD result | `UP-TO-DATE`/`NO-SOURCE`/`FROM-CACHE` tasks, daemon + download chatter | 49% | +| `xcodebuild` | builds | errors, warnings, test results, BUILD result | 31 build-phase and tool-invocation patterns | 62% | +| `gcc` | builds | **every** error and warning, with its source context | include-chain traces, `N warnings generated` counters | 27% | +| `swift-build` | builds | diagnostics; collapses a clean build to `ok` | `Compiling`/`Linking` lines | 29% | +| `dotnet-build` | builds | diagnostics; collapses a clean build to `ok` | MSBuild banner, restore chatter | 54% | +| `turbo` | builds | task output, errors | cache hit/miss/bypass, scope + duration lines | 66% | +| `nx` | builds | task output, errors | `> NX Running target`, log links, rule bars | 75% | +| `terraform-plan` | iac | planned changes, `Plan:` line, no-change result | `Refreshing state`, state locks, `# (N unchanged …)` | 53% | +| `terraform-init` | iac | the initialization result, errors | provider download/install lines | 72% | +| `pulumi` | iac | resource rows, outputs, resource counts, error messages | banners, permalinks, per-resource progress rows, JS stack frames | 59% | +| `liquibase` | builds | version, changeset status, errors | ASCII banner, jar inventory, INFO chatter | 76% | +| `ssh` | net | the remote command's output | `debug1:` flood, host-key and connection banners | 52% | +| `ping` | net | the statistics block, timeouts | per-packet replies | 62% | +| `rsync` | net | errors; collapses a clean sync to `ok` | file list, byte counters | 48% | +| `bundle-install` | pkg | installs, conflicts; collapses a complete bundle | `Using ` lines, metadata fetch | 81% | +| `poetry-install` | pkg | lock writes, solver errors; collapses an up-to-date lock | download/install lines, virtualenv chatter | 70% | +| `composer-install` | pkg | lock writes, warnings; collapses a no-op install | download/install lines | 75% | +| `uv-sync` | pkg | the installed-package list; collapses an audited-only sync | download/cache lines | 51% | +| `brew-install` | pkg | the install summary; collapses an already-installed formula | download/pour/progress lines | 59% | +| `quarto-render` | builds | errors, warnings; collapses a successful render | per-file processing and pandoc lines | 54% | + +`terraform-plan` and `make` additionally assert a **≥60% floor** on a realistic large fixture +(`TestCompressionFloors`), matching the floors rtk asserts for its equivalents. + +### Every success-collapse carries an `unless` guard + +Nine of rtk's eleven `match_output` success-collapse rules are unguarded — a build that emits a +warning *and* a success marker collapses to `ok` and the warning is gone. rtk learned this itself +(its `swift-build` test is named "warnings not swallowed when Build complete present"). In a proxy +the stakes are higher: the agent cannot re-run the command to find out. So every collapse rule here +carries an `unless`, plus an explicit negative test proving a warning + success marker does **not** +collapse. `TestEveryMatchOutputRuleIsGuarded` fails the build if one is added without a guard. + +`dotnet-build`'s guard is worth noting: dotnet prints `0 Error(s)` on success, so a guard on the +word "error" would never let it collapse. It guards on the diagnostic *form* instead +(`error CS1002` / `warning CS0168`). + +### Line budgets: shared `cap` classes + +Filters select a budget by **signal density** (`cap: errors`) rather than each hand-picking a +`max_lines`, so the whole set is tunable from one map (`dsl.Caps`). See +[the DSL engine](dsl.md#line-budgets-cap-classes). + +## What is deliberately NOT ported + +rtk ships 63 DSL filters and ~50 native Rust ones. 23 filters are ported. The rest is excluded on +purpose: + +- **The ~24 `truncate_lines_at`-only filters** (`df`, `ps`, `du`, `jq`, `jira`, `markdownlint`, + `yamllint`, `stat`, `gcloud`, `helm`, `iptables`, `skopeo`, `yadm`, `hadolint`, …). Their whole + "filter" is "strip blank lines + cap line width" — whole-blob lossy for a modest, unmeasured + saving. If that effect is wanted, one generic width-cap filter beats 24 files. +- **The blank-line-only linter filters** (`shellcheck`, `systemctl-status`, `sops`, `fail2ban`, + `basedpyright`, `ty`, `oxlint`, `biome`, `mix-format`, `tofu-fmt`, `tofu-validate`). Same reason; + the useful part is their `on_empty` collapse, which one or two generic linter filters can carry. +- **`spring-boot`** — a `keep_lines_matching` allowlist over unbounded application logs. Too easy + to drop the one line that mattered, and nothing recovers it but a full expand. +- **`filter_stderr`** — no proxy analogue; by the time output reaches a proxy the streams are + already merged. +- **rtk's 86 command-detection rules** — entirely about rewriting shell commands to `rtk `. + Irrelevant to a proxy. (Its per-tool `savings_pct` figures are hand-estimated, not measured, so + they are not reused as expected-gain data either.) +- **rtk's ~50 native Rust filters** (43,850 lines: `--format json` → parse → re-render for cargo, + rubocop, golangci, ruff, phpstan; TRX/binlog parsing; git diffstat compression). Its + highest-compression technique, but inexpressible in the DSL and mostly unreachable from a proxy + that cannot inject `--format json` into a command it never sees. The output-side half — a JSON or + TRX blob that *arrives* as a tool result and could be re-rendered — remains open. + +## Observability + +Savings are attributed **per command family** and per filter in `/stats`, cumulative and unique +(deduped by content key, since the agent re-sends history verbatim every turn): + +```json +"cmdfilter_families": { "iac": {"acts": 3, "saved_tokens": 640, "saved_tokens_unique": 340} }, +"cmdfilter_filters": { "terraform-plan": {"acts": 2, "saved_tokens": 600, "saved_tokens_unique": 300} }, +"cmdfilter_selector_misses": [ {"selector": "Some unrecognized first line", "count": 7} ] +``` + +`cmdfilter_selector_misses` is the **ledger of output shapes that matched no filter**, ranked by +frequency (after rtk's `parse_failures` table). It makes "which filter to write next" data instead +of guesswork. The ledger is bounded at 200 distinct selectors. ## Before → After ``` -before: pytest … 100 lines of PASSED + warnings + 1 failure -after: <> [full output: …] +before: terraform plan … 40 "Refreshing state" lines + state locks + unchanged-attribute comments +after: Terraform will perform the following actions: + # aws_instance.web will be created + Plan: 1 to add, 0 to change, 0 to destroy. + <> [full output: call context_guru_expand] ``` ## Lossiness Lossy but reversible — the original is stashed and recovered via `context_guru_expand` / -`GET /expand`. A recovery hint is appended only when the filter actually dropped content. +`GET /expand`. A recovery hint is appended only when the filter actually dropped content, and it is +**typed by what was lost**: a clean contiguous tail cut names the cut point (cheap partial recovery), +a whole-blob loss points at the expand tool. See [the DSL engine](dsl.md#lossiness). ## Configuration | Key | Default | Meaning | |---|---|---| | `filters` | `[]` | Inline filter YAML docs, added with no recompile. | -| `disable_builtins` | `false` | Disable the shipped `pytest` / `npm-install` / `make` filters. | +| `disable_builtins` | `false` | Disable the shipped filter set and run only your own. | +| `marker_mode` | `full` | `full` (stash + resolvable marker) / `summary` / `off`. | +| `min_size` | `500` | Byte floor; smaller outputs are left alone. | ## When it shines -Noisy but structured command/log output (test runners, package managers, build tools). +Noisy but structured command output: build tools, test runners, package managers, IaC plans, verbose +network clients. ## When it's inert -Output whose first line matches no filter, or where filtering doesn't shrink it. +Output whose first line matches no filter (logged as a selector miss), output under `min_size`, or +filtering that doesn't shrink the message once the marker is counted. -See also: [Components overview](../components.md) · [Choose a preset](../how-to/choose-a-preset.md) +See also: [Components overview](../components.md) · [The DSL filter engine](dsl.md) · +[Write a custom DSL filter](../how-to/custom-dsl-filter.md) · +[Choose a preset](../how-to/choose-a-preset.md) diff --git a/docs/components/dsl.md b/docs/components/dsl.md index 4e6b22a..f556ae1 100644 --- a/docs/components/dsl.md +++ b/docs/components/dsl.md @@ -5,11 +5,11 @@ ## How it works -`components/dsl` is a declarative, user-extensible text-filter engine (adapted from rtk), wrapped -by [`cmdfilter`](cmdfilter.md). Filters are authored in YAML (no recompile), matched -first-by-sorted-name, and each runs a fixed **8-stage** pipeline. Because filters drop lines they -are lossy, which is why the wrapping `cmdfilter` component is an Offload (it stashes the original -first). +`components/dsl` is a declarative, user-extensible text-filter engine (adapted from rtk — see +`THIRD-PARTY-NOTICES`), wrapped by [`cmdfilter`](cmdfilter.md). Filters are authored in YAML (no +recompile), matched by descending `priority` then by name, and each runs a fixed **8-stage** +pipeline. Because filters drop lines they are lossy, which is why the wrapping `cmdfilter` component +is an Offload (it stashes the original first). ```mermaid flowchart LR @@ -23,22 +23,79 @@ flowchart LR All optional except `match`: - `match` — regex vs the selector (= first non-empty line) +- `family` — command family for per-family metrics (`builds` / `tests` / `iac` / `pkg` / `net` / …) +- `priority` — match order; higher first, then by name. Absent (`0`) is today's name ordering. - `strip_ansi` - `replace` — chained `pattern`→`replacement`, `$1` backrefs - `match_output` — whole-blob short-circuit: `pattern`/`message`/`unless` - `strip_lines_matching` **xor** `keep_lines_matching` - `truncate_lines_at` — per-line char cap - `head_lines` / `tail_lines` +- `cap` / `cap_reduce` — a shared line budget (see below); an explicit `max_lines` wins - `max_lines` — absolute cap with omission marker - `on_empty` — replacement when output is blank +### `priority`, and why order matters more here + +`cmdfilter` matches on the *shape of the output*, not on a command, so a generic pattern can shadow a +specific one in a way rtk's command matching never had to deal with. `priority` makes +specific-before-generic explicit instead of dependent on alphabetical luck. Absent, ordering is by +name — exactly the previous behavior. + +### Line budgets: `cap` classes + +Instead of every filter hand-picking a `max_lines`, a filter selects a budget by **signal density**. +The class names and the first four values are rtk's (`src/core/truncate.rs`); `buildlog` is ours. + +| `cap` | lines | for | +|---|--:|---| +| `errors` | 20 | error lists — most actionable, shown the most | +| `warnings` | 10 | warnings and test failures — lower signal density | +| `list` | 20 | flat lists (packages, services): one line per item | +| `inventory` | 50 | exhaustive lookups (installed packages, file listings) | +| `buildlog` | 80 | full build/plan transcripts — verbose, and the signal can sit anywhere | + +`cap_reduce: N` lowers the chosen cap for an extra-verbose variant, underflow-safe (a deviation can +never empty the budget — rtk's `reduced` helper). Unknown cap names, and a `cap_reduce` without a +`cap`, are rejected at load. rtk's own TOML filters don't use its cap classes; applying them to the +definitions makes the whole set tunable from one map. + ## Lossiness -`Lossiness` is reported back to `cmdfilter` (it drives whether a recovery hint is appended): +`Lossiness` is reported back to `cmdfilter`, which uses it to pick the recovery hint: + +- **None** — nothing dropped / reversible reformat → no hint +- **Tail** — a clean contiguous tail dropped → the hint names the cut point, because re-reading from + there is cheaper than pulling the whole blob back +- **Whole** — non-contiguous or whole-blob loss → the hint points at the expand tool + +!!! note "Loss-typing fixes" + Three bugs made a real loss invisible or misleading. All fixed: + + - **`truncate_lines_at` never recorded loss.** It ran before `loss` was initialized, so an + intra-line cut reported `None` and `cmdfilter` emitted no recovery hint for a real loss. An + intra-line cut is non-contiguous by nature (every long line loses its own tail), so it now + types as **Whole**. + - **`truncate_lines_at` cut silently.** A mid-line cut with no marker reads as corrupted output + to a model. It now appends `...`, sized to fit *inside* the cap so the line never grows. + - **The recovery hint collapsed `Tail` and `Whole`.** Both got the same "call + `context_guru_expand`" text, making a cheap partial recovery look like an expensive one. They + are now distinct. + +## Load-time guardrails + +Mirroring rtk's `build.rs`, a filter document fails **at load**, not at first use, when: + +- two filters share a name (a silently shadowed filter is a debugging trap); +- a `match` / `replace` / `match_output` / `strip_lines_matching` regex doesn't compile; +- `strip_lines_matching` and `keep_lines_matching` are both set; +- `cap` names an unknown class, or `cap_reduce` is set without `cap`; +- **any inline test in the document fails** — so a broken filter fails loudly instead of quietly + mangling output. -- **None** — nothing dropped / reversible reformat -- **Tail** — a clean contiguous tail dropped -- **Whole** — non-contiguous or whole-blob loss +The shipped filter set must additionally have ≥1 test *per filter*, and each test's input must route +to its own filter (`TestEveryBuiltinFilterHasTestsAndRoutes`). User-supplied documents aren't +required to ship tests, but any they do ship must pass. ## Example @@ -47,18 +104,21 @@ schema_version: 1 filters: pytest: description: keep failures + summary, drop passing noise + family: tests + priority: 10 match: "(pytest|=+ test session starts)" strip_lines_matching: ["^\\s*$", " PASSED", "^\\.+$"] - max_lines: 80 + cap: buildlog on_empty: "pytest: all passed" -tests: # inline; run via dsl.RunTests (a `verify` command) +tests: # inline; run at load, and via dsl.RunTests pytest: - name: all-green input: "pytest\n....\n" expected: "pytest: all passed" ``` -Documents load with `schema_version: 1` and strict unknown-field rejection. Inline `tests` -(input → expected) run via `dsl.RunTests`, so a filter ships with its own regression check. +Documents load with `schema_version: 1` and strict unknown-field rejection. -See also: [Components overview](../components.md) · [Choose a preset](../how-to/choose-a-preset.md) +See also: [Components overview](../components.md) · [cmdfilter](cmdfilter.md) · +[Write a custom DSL filter](../how-to/custom-dsl-filter.md) · +[Choose a preset](../how-to/choose-a-preset.md) diff --git a/docs/how-to/choose-a-preset.md b/docs/how-to/choose-a-preset.md index 3c03de0..89adf45 100644 --- a/docs/how-to/choose-a-preset.md +++ b/docs/how-to/choose-a-preset.md @@ -47,8 +47,8 @@ test/build runs (`failed_run`), and DSL command-log filtering (`cmdfilter`). - **Fits:** general agent traffic; the safe everyday choice. - **Caveat:** `cmdfilter` only fires when ≥1 filter is loaded and the output's first line matches - one. Its builtins cover pytest / npm-install / make; author more with a - [custom DSL filter](custom-dsl-filter.md). + one. It ships 23 filters covering test runners, build tools, package managers, IaC plans and + verbose network clients; author more with a [custom DSL filter](custom-dsl-filter.md). ### `aggressive` — `[format, dedup, failed_run, cmdfilter, smartcrush, extract, cacheinject]` `balanced` plus JSON-array crushing (`smartcrush`) and query-relevance projection (`extract`). diff --git a/docs/how-to/custom-dsl-filter.md b/docs/how-to/custom-dsl-filter.md index b9106b4..cbe9902 100644 --- a/docs/how-to/custom-dsl-filter.md +++ b/docs/how-to/custom-dsl-filter.md @@ -2,7 +2,7 @@ `components/dsl` is a declarative, user-extensible text-filter engine (adapted from rtk), wrapped by the [`cmdfilter`](../components/cmdfilter.md) component. Filters are authored in YAML — **no -recompile** — matched first-by-sorted-name, and each runs a fixed 8-stage pipeline. Because filters +recompile** — matched by descending `priority` then by name, and each runs a fixed 8-stage pipeline. Because filters drop lines they are lossy, which is why the wrapping `cmdfilter` is an [Offload](../components.md#offload-lossy-reversible): it stashes the original first and appends a `<>` recovery hint only when the filter was actually lossy. @@ -19,8 +19,9 @@ flowchart LR ``` The engine reports `Lossiness` back to `cmdfilter`, which drives the recovery hint: `None` (nothing -dropped / reversible reformat), `Tail` (a clean contiguous tail dropped), `Whole` (non-contiguous or -whole-blob loss). +dropped / reversible reformat, no hint), `Tail` (a clean contiguous tail dropped — the hint names the +cut point, since re-reading from there is cheaper than a full expand), `Whole` (non-contiguous or +whole-blob loss — the hint points at the expand tool). ## Filter fields @@ -29,19 +30,33 @@ All optional except `match`. | Field | Purpose | |---|---| | `match` | regex tested against the **selector** (the output's first non-empty line) — decides if this filter applies | +| `family` | command family for the per-family `/stats` ledger: `builds` / `tests` / `iac` / `pkg` / `net` / … | +| `priority` | match order — higher first, then by name. Use it to put a specific filter ahead of a generic one. | | `strip_ansi` | strip ANSI escape codes | | `replace` | chained `pattern` → `replacement` substitutions, `$1` backrefs | | `match_output` | whole-blob short-circuit: `pattern` / `message` / `unless` | | `strip_lines_matching` **xor** `keep_lines_matching` | drop, or keep-only, lines matching these regexes (mutually exclusive) | | `truncate_lines_at` | per-line character cap | | `head_lines` / `tail_lines` | keep the first / last N lines | -| `max_lines` | absolute line cap with an omission marker | +| `cap` / `cap_reduce` | a **shared** line budget by signal density — `errors` 20, `warnings` 10, `list` 20, `inventory` 50, `buildlog` 80; `cap_reduce: N` lowers it, underflow-safe. Prefer this to a hand-picked `max_lines`. | +| `max_lines` | absolute line cap with an omission marker (wins over `cap` if both are set) | | `on_empty` | replacement text when the output ends up blank | !!! warning "strip xor keep" `strip_lines_matching` and `keep_lines_matching` are mutually exclusive — set one or the other, never both. +!!! danger "Guard every `match_output` collapse with `unless`" + A `match_output` rule replaces the **whole** output with one message. Without an `unless`, a + build that emits a warning *and* a success marker collapses to `ok` and the warning is gone — + and in a proxy the agent cannot re-run the command to find it. Every shipped collapse rule + carries `unless: 'error|warning|failed|…'` plus a negative test proving the co-occurring case + does **not** collapse. Do the same in your own filters. + + Watch for guards that can never fire: `dotnet build` prints `0 Error(s)` on success, so guarding + on the word "error" would block every collapse. Guard on the diagnostic *form* + (`(error|warning) [A-Z]+\d`) instead. + ## A full example (pytest) Documents load with `schema_version: 1` and strict unknown-field rejection. @@ -64,9 +79,25 @@ tests: # inline; run via dsl.RunTests (a `verify` command) ### Ship tests with the filter -Inline `tests` (input → expected) run via `dsl.RunTests`, so a filter ships with its own regression -check. Above, the `all-green` case proves that a passing run collapses to the `on_empty` message — -if a future edit breaks that, the test fails. +Inline `tests` (input → expected) **run at load time** as well as via `dsl.RunTests`, so a filter +that doesn't do what its tests say never loads at all. Above, the `all-green` case proves a passing +run collapses to the `on_empty` message — if a future edit breaks that, the load fails. + +### Write the selector against a real sample + +This is the step that most often goes wrong. `match` is tested against the output's **first +non-empty line**, not against a command. An `rtk`-style command regex (`^terraform\s+plan`) compiles +fine and never fires. Paste a real sample of the output, take its first non-empty line, and write the +regex against *that* (`^Refreshing state`, `^> Task :`, `^==> Downloading`). + +Selectors that match nothing are logged: `/stats` exposes `cmdfilter_selector_misses`, the frequency- +ranked list of output shapes no filter claimed. That is the backlog of filters worth writing. + +### The load-time guardrails + +A document is rejected at load — not at first use — if two filters share a name, a regex doesn't +compile, `strip` and `keep` are both set, `cap` names an unknown class, `cap_reduce` appears without +`cap`, or any inline test fails. ## Load it into `cmdfilter` @@ -85,14 +116,16 @@ components: strip_lines_matching: ["^\\s*$", " PASSED", "^\\.+$"] max_lines: 80 on_empty: "pytest: all passed" - disable_builtins: false # keep the builtin pytest / npm-install / make filters too + disable_builtins: false # keep the 23 shipped filters too + min_size: 500 # byte floor: below it the marker costs more than the saving ``` - `cmdfilter` is `Enabled` only when ≥1 filter is loaded. -- It ships builtin `pytest` / `npm-install` / `make` filters; set `disable_builtins: true` to run - only your own. -- The output's first non-empty line is the selector each filter's `match` is tested against, sorted - by filter name. +- It ships [23 filters](../components/cmdfilter.md#the-shipped-filter-set); set + `disable_builtins: true` to run only your own. +- The output's first non-empty line is the selector each filter's `match` is tested against, in + descending `priority` then name order. +- Outputs smaller than `min_size` (default 500 bytes) are skipped entirely. !!! tip Filtering that doesn't shrink the output, or output whose first line matches no filter, is a diff --git a/docs/results/components.md b/docs/results/components.md index 52bf94c..27bce95 100644 --- a/docs/results/components.md +++ b/docs/results/components.md @@ -55,8 +55,9 @@ Run: **0 acts** (cache-aware), but still scans every run-like output (~6.7 s tot costliest *deterministic* detection). ### 4. `cmdfilter` (Offload) -Declarative DSL filters keyed on a command output's first line (builtin `pytest`, -`npm-install`, `make`): strip blank/`PASSED`/progress lines, cap length, keep failures. +Declarative DSL filters keyed on a command output's first line (23 shipped, e.g. `pytest`, +`make`, `gradle`, `terraform-plan`, `pulumi`): strip blank/`PASSED`/progress lines, cap length, +keep failures. > **Real example** (pytest session): `1140 → 1068 tok` — passing/blank noise stripped, > failures + warnings kept verbatim. Run: 3 acts. diff --git a/metrics/metrics.go b/metrics/metrics.go index 3263a13..6f3ae37 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -31,6 +31,24 @@ func (t Tee) Run(r components.RunReport) { } } +// FilterAct / FilterMiss forward cmdfilter's ledger to whichever tee'd emitters +// record it (so a Tee still satisfies components.FilterStatsSink). +func (t Tee) FilterAct(family, filter, contentKey string, saved int) { + for _, e := range t { + if s, ok := e.(components.FilterStatsSink); ok { + s.FilterAct(family, filter, contentKey, saved) + } + } +} + +func (t Tee) FilterMiss(selector string) { + for _, e := range t { + if s, ok := e.(components.FilterStatsSink); ok { + s.FilterMiss(selector) + } + } +} + // Slog logs each component and run in the GenAI semantic-convention vocabulary. type Slog struct{ L *slog.Logger } @@ -93,6 +111,66 @@ type Aggregator struct { sseTTFBMsBuf float64 sseStreamed int64 sseBuffered int64 + // cmdfilter's per-family / per-filter ledger, plus the selector-miss ledger that + // makes the next filter to write data instead of guesswork. + filterFam map[string]*filterStat + filterName map[string]*filterStat + filterMiss map[string]int64 +} + +// filterStat is one cmdfilter family's or filter's ledger. SavedUnique dedups by +// content key exactly as compStat does — the agent re-sends history verbatim every +// turn, so the cumulative figure double-counts the same compaction. +type filterStat struct { + Acts int64 `json:"acts"` + Saved int64 `json:"saved_tokens"` + SavedUnique int64 `json:"saved_tokens_unique"` + + seenKeys map[string]struct{} // content keys already counted (not serialized) +} + +// maxMissKeys bounds the selector-miss ledger; output shapes are unbounded in +// principle. Once full we only keep counting selectors already tracked. +// ponytail: fixed cap, no eviction — first-seen wins. Swap for a count-min sketch if +// the ledger ever gets dominated by whatever arrived first. +const maxMissKeys = 200 + +// FilterAct implements components.FilterStatsSink: one applied cmdfilter filter. +func (a *Aggregator) FilterAct(family, filter, contentKey string, saved int) { + a.mu.Lock() + defer a.mu.Unlock() + if a.filterFam == nil { + a.filterFam, a.filterName = map[string]*filterStat{}, map[string]*filterStat{} + } + bump(a.filterFam, family, contentKey, saved) + bump(a.filterName, filter, contentKey, saved) +} + +func bump(m map[string]*filterStat, key, contentKey string, saved int) { + fs := m[key] + if fs == nil { + fs = &filterStat{seenKeys: map[string]struct{}{}} + m[key] = fs + } + fs.Acts++ + fs.Saved += int64(saved) + if _, seen := fs.seenKeys[contentKey]; !seen { + fs.seenKeys[contentKey] = struct{}{} + fs.SavedUnique += int64(saved) + } +} + +// FilterMiss implements components.FilterStatsSink: a selector that matched nothing. +func (a *Aggregator) FilterMiss(selector string) { + a.mu.Lock() + defer a.mu.Unlock() + if a.filterMiss == nil { + a.filterMiss = map[string]int64{} + } + if _, known := a.filterMiss[selector]; !known && len(a.filterMiss) >= maxMissKeys { + return + } + a.filterMiss[selector]++ } type compStat struct { @@ -258,6 +336,18 @@ type Snapshot struct { // requests", not as a latency to compare against sse_ttfb_ms_avg. SSETTFBMsAvgBuf float64 `json:"sse_ttfb_ms_avg_buffered"` SSEBufferedPct float64 `json:"sse_buffered_pct"` + // cmdfilter attribution: which command FAMILIES pay off (builds/tests/iac/pkg/net), + // which individual filters fire, and which output shapes matched no filter (the + // backlog of filters worth writing). Additive fields — nothing above is renamed. + CmdfilterFamilies map[string]filterStat `json:"cmdfilter_families,omitempty"` + CmdfilterFilters map[string]filterStat `json:"cmdfilter_filters,omitempty"` + CmdfilterMisses []SelectorMiss `json:"cmdfilter_selector_misses,omitempty"` +} + +// SelectorMiss is one output shape that matched no filter, with how often it appeared. +type SelectorMiss struct { + Selector string `json:"selector"` + Count int64 `json:"count"` } // Snapshot returns a point-in-time copy of the rollups. @@ -314,5 +404,43 @@ func (a *Aggregator) Snapshot() Snapshot { AddedLatencyMsAvg: addedAvg, UpstreamMsAvg: upAvg, UpstreamMsAvgBypassed: upAvgByp, SSEStreamed: a.sseStreamed, SSEBuffered: a.sseBuffered, SSETTFBMsAvg: ttfb, SSETTFBMsAvgBuf: ttfbBuf, SSEBufferedPct: bufPct, + CmdfilterFamilies: copyFilterStats(a.filterFam), + CmdfilterFilters: copyFilterStats(a.filterName), + CmdfilterMisses: topMisses(a.filterMiss, 20), + } +} + +func copyFilterStats(src map[string]*filterStat) map[string]filterStat { + if len(src) == 0 { + return nil + } + out := make(map[string]filterStat, len(src)) + for k, v := range src { + fs := *v + fs.seenKeys = nil // don't serialize the working set + out[k] = fs + } + return out +} + +// topMisses returns the n most frequent unmatched selectors, descending (ties by +// selector, so the output is deterministic). +func topMisses(src map[string]int64, n int) []SelectorMiss { + if len(src) == 0 { + return nil + } + out := make([]SelectorMiss, 0, len(src)) + for s, c := range src { + out = append(out, SelectorMiss{Selector: s, Count: c}) + } + sort.Slice(out, func(i, j int) bool { + if out[i].Count != out[j].Count { + return out[i].Count > out[j].Count + } + return out[i].Selector < out[j].Selector + }) + if len(out) > n { + out = out[:n] } + return out } diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index 0f4c661..083ed23 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -1,6 +1,8 @@ package metrics import ( + "encoding/json" + "strconv" "testing" "github.com/rossoctl/context-guru/components" @@ -113,3 +115,59 @@ func TestMutatedZeroSavingsNotPassthrough(t *testing.T) { t.Fatalf("cacheinject should record a mutation, got %+v", s.Components["cacheinject"]) } } + +// The cmdfilter ledger: per-family and per-filter attribution, unique-vs-cumulative +// dedup, and the bounded selector-miss ledger. +func TestFilterStatsLedger(t *testing.T) { + a := NewAggregator() + a.FilterAct("iac", "terraform-plan", "k1", 300) + a.FilterAct("iac", "terraform-plan", "k1", 300) // same compaction re-sent next turn + a.FilterAct("iac", "terraform-init", "k2", 40) + a.FilterAct("builds", "make", "k3", 100) + a.FilterMiss("Totally unknown shape") + a.FilterMiss("Totally unknown shape") + a.FilterMiss("Another shape") + + s := a.Snapshot() + if got := s.CmdfilterFamilies["iac"]; got.Acts != 3 || got.Saved != 640 || got.SavedUnique != 340 { + t.Fatalf("iac family ledger wrong: %+v", got) + } + if got := s.CmdfilterFilters["make"]; got.Acts != 1 || got.SavedUnique != 100 { + t.Fatalf("per-filter ledger wrong: %+v", got) + } + if len(s.CmdfilterMisses) != 2 || s.CmdfilterMisses[0].Selector != "Totally unknown shape" || s.CmdfilterMisses[0].Count != 2 { + t.Fatalf("misses should be ranked by frequency: %+v", s.CmdfilterMisses) + } + // the ledger is bounded: unknown selectors past the cap are dropped, not appended + for i := 0; i < maxMissKeys*2; i++ { + a.FilterMiss("shape-" + strconv.Itoa(i)) + } + if n := len(a.filterMiss); n > maxMissKeys { + t.Fatalf("miss ledger grew past its cap: %d", n) + } +} + +// /stats must stay backward compatible: the fields deploy/harbor/*.py parses are +// still present, and the new cmdfilter fields are additive-and-omitted-when-empty. +func TestSnapshotStaysBackwardCompatible(t *testing.T) { + b, err := json.Marshal(NewAggregator().Snapshot()) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + t.Fatal(err) + } + for _, k := range []string{"requests", "tokens_before", "tokens_after", "saved_tokens", + "savings_pct", "wasted_tokens", "bounces", "adjusted_saved", "components", + "llm_calls", "cg_added_ms_avg", "upstream_ms_avg"} { + if _, ok := m[k]; !ok { + t.Errorf("/stats lost the %q field the harness parses", k) + } + } + for _, k := range []string{"cmdfilter_families", "cmdfilter_filters", "cmdfilter_selector_misses"} { + if _, ok := m[k]; ok { + t.Errorf("%q should be omitted when empty", k) + } + } +} From a3a5c7254919c47966f00f498969af35bded62d3 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 04:07:57 +0000 Subject: [PATCH 2/7] feat(cmdfilter): add apt filter and widen gcc's selector from miss-ledger data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaying the shipped selectors over a recorded Terminal-Bench tool-output dump showed which output shapes matched nothing. Two dominated: - apt/dpkg install boilerplate: 584 outputs, ~1.0 MB, the largest reachable output family on that benchmark. rtk has no apt filter at all. New `apt` filter: 76% compression on its fixtures, pure boilerplate collapses to one line, and every E:/W:/N:, dpkg error and prompt survives. - ": In function 'main':" — gcc's diagnostic HEADER line, 108 outputs. rtk's gcc filter matches the command, so its patterns never had to name this shape; ported as written it missed the most common way gcc output starts. The IaC and mobile-build filters fired zero times on that dump. They stay (they are correct, tested, and free when inert) but the docs now say plainly that a filter set's value comes from the workload, not its size. TestAptKeepsProblems is kept as a permanent regression check: it caught a '^debconf: ' strip rule that swallowed "debconf: unable to initialize frontend" along with the harmless delaying notice. Narrowed to the delaying notice only. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- components/offload/cmdfilter_apt_test.go | 44 ++++++++++++++++++++++ components/offload/cmdfilter_filters.go | 47 +++++++++++++++++++++++- docs/components/cmdfilter.md | 33 +++++++++++++++-- docs/how-to/custom-dsl-filter.md | 13 ++++++- 4 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 components/offload/cmdfilter_apt_test.go diff --git a/components/offload/cmdfilter_apt_test.go b/components/offload/cmdfilter_apt_test.go new file mode 100644 index 0000000..35949f8 --- /dev/null +++ b/components/offload/cmdfilter_apt_test.go @@ -0,0 +1,44 @@ +package offload + +import ( + "strings" + "testing" + + "github.com/rossoctl/context-guru/components/dsl" +) + +// apt output that carries a real problem must survive the filter. This test earned +// its place: it caught a '^debconf: ' strip rule that swallowed +// "debconf: unable to initialize frontend" along with the harmless delaying notice. +// Any new strip rule on a high-volume filter should be run past a list like this. +func TestAptKeepsProblems(t *testing.T) { + var r dsl.Registry + if err := r.Load([]byte(builtinFilters)); err != nil { + t.Fatal(err) + } + boiler := strings.Repeat("Setting up libfoo:amd64 (1.2.3-1) ...\n", 40) + for _, keep := range []string{ + "E: Unable to locate package nope", + "E: Package 'foo' has no installation candidate", + "dpkg: dependency problems prevent configuration of bar:", + "W: Possible missing firmware /lib/firmware/x.bin", + "Errors were encountered while processing:", + "debconf: unable to initialize frontend: Dialog", + "N: Ignoring file 'x.list.bak' in directory", + "Do you want to continue? [Y/n]", + } { + in := boiler + keep + "\n" + boiler + c := r.Match(selectorKey(in)) + if c == nil || c.Name != "apt" { + t.Fatalf("routed to %v", c) + } + out, _ := dsl.Apply(c, in) + if !strings.Contains(out, keep) { + t.Errorf("apt filter DROPPED %q -> %q", keep, out) + } + } + // and the pure-boilerplate case collapses hard + if out, _ := dsl.Apply(r.Match(selectorKey(boiler)), boiler); out != "apt: install ok" { + t.Fatalf("pure boilerplate should collapse, got %q", out) + } +} diff --git a/components/offload/cmdfilter_filters.go b/components/offload/cmdfilter_filters.go index b884cba..ba1b745 100644 --- a/components/offload/cmdfilter_filters.go +++ b/components/offload/cmdfilter_filters.go @@ -135,7 +135,7 @@ filters: gcc: description: strip include traces and diagnostic counters, keep every error and warning family: builds - match: '^(In file included from|/usr/bin/ld:|collect2: error:|\S+:\d+:(\d+:)? (error|warning|note):)' + match: '^(In file included from|/usr/bin/ld:|collect2: error:|\S+: In function|\S+:\d+:(\d+:)? (error|warning|note):)' strip_ansi: true strip_lines_matching: - '^\s*$' @@ -420,6 +420,36 @@ filters: unless: 'error|Error|warning|failed' cap: list + apt: + description: collapse apt/dpkg install boilerplate, keep errors and configuration prompts + family: pkg + match: '^(Get:\d+ http|Selecting previously unselected package|Preparing to unpack |Unpacking |Setting up |Processing triggers for |Reading package lists|Building dependency tree)' + strip_ansi: true + strip_lines_matching: + - '^\s*$' + - '^Get:\d+ http' + - '^Selecting previously unselected package' + - '^Preparing to unpack ' + - '^Unpacking ' + - '^Setting up ' + - '^Processing triggers for ' + - '^Reading package lists' + - '^Building dependency tree' + - '^Reading state information' + - '^\(Reading database \.\.\.' + - '^Fetched \d' + - '^Need to get \d' + - '^After this operation' + # NOT '^debconf: ' — that swallows real diagnostics like + # "debconf: unable to initialize frontend". Only the delaying notice is noise. + - '^debconf: delaying package configuration' + - '^update-alternatives: ' + - '^Created symlink ' + - '^Creating config file ' + - '^\d+ upgraded, \d+ newly installed' + cap: list + on_empty: 'apt: install ok' + brew-install: description: strip brew download/pour chatter, collapse an already-installed formula family: pkg @@ -523,6 +553,10 @@ tests: input: "/usr/bin/ld: /tmp/main.o: undefined reference to 'missing_func'\ncollect2: error: ld returned 1 exit status\n" expected: "/usr/bin/ld: /tmp/main.o: undefined reference to 'missing_func'\ncollect2: error: ld returned 1 exit status" + - name: In-function header routes and diagnostics survive + input: "/tmp/spherepeak.c: In function 'main':\n/tmp/spherepeak.c:12:9: warning: unused variable 'r' [-Wunused-variable]\n 12 | int r = 0;\n | ^\n1 warning generated.\n" + expected: "/tmp/spherepeak.c: In function 'main':\n/tmp/spherepeak.c:12:9: warning: unused variable 'r' [-Wunused-variable]\n 12 | int r = 0;\n | ^" + swift-build: - name: successful build collapses input: "Build complete! (4.21s)\n" @@ -715,6 +749,17 @@ tests: input: "Resolved 42 packages in 123ms\nwarning: 'pytest' was not found in the lockfile\nAudited 42 packages in 0.05ms\n" expected: "Resolved 42 packages in 123ms\nwarning: 'pytest' was not found in the lockfile\nAudited 42 packages in 0.05ms" + apt: + - name: pure install boilerplate collapses + input: "Setting up libx11-data (2:1.8.7-1build1) ...\nSetting up perl-modules-5.38 (5.38.2-3.2ubuntu0.3) ...\nSetting up git (1:2.43.0-1ubuntu7.3) ...\nProcessing triggers for libc-bin (2.39-0ubuntu8.6) ...\n" + expected: 'apt: install ok' + - name: errors and prompts kept + input: "Reading package lists...\nBuilding dependency tree...\nGet:1 http://archive.ubuntu.com/ubuntu noble/main amd64 libfoo amd64 1.0 [12 kB]\nE: Unable to locate package libnope\nSetting up libfoo (1.0) ...\n" + expected: 'E: Unable to locate package libnope' + - name: dpkg failure kept + input: "Preparing to unpack .../libbar_2.0_amd64.deb ...\nUnpacking libbar (2.0) ...\ndpkg: error processing archive libbar_2.0_amd64.deb (--unpack):\n trying to overwrite '/usr/lib/libbar.so', which is also in package libbaz\nSetting up libfoo (1.0) ...\n" + expected: "dpkg: error processing archive libbar_2.0_amd64.deb (--unpack):\n trying to overwrite '/usr/lib/libbar.so', which is also in package libbaz" + brew-install: - name: already installed collapses input: "Warning: jq 1.7.1 is already installed and up-to-date.\nTo reinstall 1.7.1, run:\n brew reinstall jq\n" diff --git a/docs/components/cmdfilter.md b/docs/components/cmdfilter.md index c5df646..67118be 100644 --- a/docs/components/cmdfilter.md +++ b/docs/components/cmdfilter.md @@ -36,7 +36,7 @@ check would reject those rewrites anyway; the floor skips the work and the stash ## The shipped filter set -23 filters. Compression is measured on each filter's own fixtures (its inline tests), summed: +24 filters. Compression is measured on each filter's own fixtures (its inline tests), summed: | filter | family | preserves | drops | saved | |---|---|---|---|--:| @@ -45,7 +45,7 @@ check would reject those rewrites anyway; the floor skips the work and the stash | `make` | builds | compiler lines, errors | `Entering/Leaving directory`, `Nothing to be done` | 56% | | `gradle` | builds | executed tasks, test results, BUILD result | `UP-TO-DATE`/`NO-SOURCE`/`FROM-CACHE` tasks, daemon + download chatter | 49% | | `xcodebuild` | builds | errors, warnings, test results, BUILD result | 31 build-phase and tool-invocation patterns | 62% | -| `gcc` | builds | **every** error and warning, with its source context | include-chain traces, `N warnings generated` counters | 27% | +| `gcc` | builds | **every** error and warning, with its source context | include-chain traces, `N warnings generated` counters | 21% | | `swift-build` | builds | diagnostics; collapses a clean build to `ok` | `Compiling`/`Linking` lines | 29% | | `dotnet-build` | builds | diagnostics; collapses a clean build to `ok` | MSBuild banner, restore chatter | 54% | | `turbo` | builds | task output, errors | cache hit/miss/bypass, scope + duration lines | 66% | @@ -61,12 +61,38 @@ check would reject those rewrites anyway; the floor skips the work and the stash | `poetry-install` | pkg | lock writes, solver errors; collapses an up-to-date lock | download/install lines, virtualenv chatter | 70% | | `composer-install` | pkg | lock writes, warnings; collapses a no-op install | download/install lines | 75% | | `uv-sync` | pkg | the installed-package list; collapses an audited-only sync | download/cache lines | 51% | +| `apt` | pkg | `E:`/`W:`/`N:` lines, dpkg errors, prompts | `Setting up`/`Unpacking`/`Get:`/trigger boilerplate | 76% | | `brew-install` | pkg | the install summary; collapses an already-installed formula | download/pour/progress lines | 59% | | `quarto-render` | builds | errors, warnings; collapses a successful render | per-file processing and pandoc lines | 54% | `terraform-plan` and `make` additionally assert a **≥60% floor** on a realistic large fixture (`TestCompressionFloors`), matching the floors rtk asserts for its equivalents. +### Two filters came from measurement, not from rtk + +`apt` and `gcc`'s widened selector are not ports — they came from replaying the shipped +selectors over a recorded Terminal-Bench tool-output dump and counting what matched nothing. Two +shapes dominated the misses: + +- **apt/dpkg install boilerplate** — 584 outputs, ~1.0 MB, the single largest reachable family on + that benchmark. rtk has no apt filter at all. 76% compression, and pure boilerplate collapses to + one line. +- **`: In function 'main':`** — gcc's diagnostic *header* line, 108 outputs. rtk's `gcc` filter + matches the command, so its patterns never had to name this shape; ported as-is, the filter would + miss the most common way gcc output starts. + +Meanwhile the IaC and mobile-build filters (`pulumi`, `terraform-plan`, `xcodebuild`, `gradle`) fired +**zero** times on that dump. They are kept — they are correct, tested and cost nothing when inert — +but the honest reading is that a filter set's value is decided by the workload, not by its size. The +`cmdfilter_selector_misses` ledger exists so that stays measurable rather than assumed. + +### A cautionary note on strip rules + +The `apt` filter originally stripped `^debconf: `, which also swallowed +`debconf: unable to initialize frontend` — a real diagnostic. `TestAptKeepsProblems` catches that +class of mistake by asserting a list of must-survive lines against a wall of boilerplate. Any new +strip rule on a high-volume filter should be run past a list like it. + ### Every success-collapse carries an `unless` guard Nine of rtk's eleven `match_output` success-collapse rules are unguarded — a build that emits a @@ -88,7 +114,8 @@ Filters select a budget by **signal density** (`cap: errors`) rather than each h ## What is deliberately NOT ported -rtk ships 63 DSL filters and ~50 native Rust ones. 23 filters are ported. The rest is excluded on +rtk ships 63 DSL filters and ~50 native Rust ones. 22 are ported (plus 2 written from measurement). +The rest is excluded on purpose: - **The ~24 `truncate_lines_at`-only filters** (`df`, `ps`, `du`, `jq`, `jira`, `markdownlint`, diff --git a/docs/how-to/custom-dsl-filter.md b/docs/how-to/custom-dsl-filter.md index cbe9902..16a2007 100644 --- a/docs/how-to/custom-dsl-filter.md +++ b/docs/how-to/custom-dsl-filter.md @@ -91,7 +91,16 @@ fine and never fires. Paste a real sample of the output, take its first non-empt regex against *that* (`^Refreshing state`, `^> Task :`, `^==> Downloading`). Selectors that match nothing are logged: `/stats` exposes `cmdfilter_selector_misses`, the frequency- -ranked list of output shapes no filter claimed. That is the backlog of filters worth writing. +ranked list of output shapes no filter claimed. That is the backlog of filters worth writing — the +shipped `apt` filter and `gcc`'s widened selector both came out of reading it. + +### Be conservative with strip rules + +A strip rule matching more than you meant is silent: the line is gone and the output still looks +plausible. `^debconf: ` looks like install noise and also matches +`debconf: unable to initialize frontend`. Write the rule as narrowly as the noise allows, and pair a +high-volume filter with a test that asserts a list of must-survive lines against a wall of +boilerplate (see `TestAptKeepsProblems`). ### The load-time guardrails @@ -121,7 +130,7 @@ components: ``` - `cmdfilter` is `Enabled` only when ≥1 filter is loaded. -- It ships [23 filters](../components/cmdfilter.md#the-shipped-filter-set); set +- It ships [24 filters](../components/cmdfilter.md#the-shipped-filter-set); set `disable_builtins: true` to run only your own. - The output's first non-empty line is the selector each filter's `match` is tested against, in descending `priority` then name order. From f14c7b17103bb0372d4a637d43994c48eb841f87 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 04:12:27 +0000 Subject: [PATCH 3/7] refactor(dsl): inline the single-use loss-severity helper Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- components/dsl/dsl.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/components/dsl/dsl.go b/components/dsl/dsl.go index e484c2a..1db0965 100644 --- a/components/dsl/dsl.go +++ b/components/dsl/dsl.go @@ -252,7 +252,9 @@ func Apply(c *Compiled, input string) (string, Lossiness) { if c.def.HeadLines != nil || c.def.TailLines != nil { var htLoss Lossiness lines, htLoss = headTail(lines, c.def.HeadLines, c.def.TailLines) - loss = worseLoss(loss, htLoss) + if htLoss > loss { // keep the more severe of an intra-line cut and a line drop + loss = htLoss + } } // 7. max_lines (absolute cap, counts the omission marker) if c.def.MaxLines != nil && len(lines) > *c.def.MaxLines { @@ -287,14 +289,6 @@ func truncateRunes(s string, n int) string { return string(r[:n-3]) + "..." } -// worseLoss returns the more severe of two loss kinds. -func worseLoss(a, b Lossiness) Lossiness { - if b > a { - return b - } - return a -} - func filterLines(lines []string, res []*regexp.Regexp, keep bool) []string { out := lines[:0:0] for _, l := range lines { From 2ca4644cf84f65cb03e4e6a4663e1bad29616e87 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 04:27:00 +0000 Subject: [PATCH 4/7] test(cmdfilter): realistic pytest fixture asserting no diagnostic is dropped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pytest is the filter that actually fires in practice — the only one to match in a recorded Terminal-Bench tool-output dump. Its inline tests were small synthetic cases, so add a full session fixture (failures, errors, skips, xfails, tracebacks, short summary) and assert every line the agent acts on survives while only passing noise is stripped. 35% saved with loss=None, so no recovery marker is needed. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- components/offload/cmdfilter_pytest_test.go | 70 +++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 components/offload/cmdfilter_pytest_test.go diff --git a/components/offload/cmdfilter_pytest_test.go b/components/offload/cmdfilter_pytest_test.go new file mode 100644 index 0000000..e17a142 --- /dev/null +++ b/components/offload/cmdfilter_pytest_test.go @@ -0,0 +1,70 @@ +package offload + +import ( + "strings" + "testing" + + "github.com/rossoctl/context-guru/components/dsl" +) + +// pytest is the filter that fires most in practice (the only one to match in a recorded +// Terminal-Bench dump), so it gets a realistic end-to-end fixture: every diagnostic the +// agent acts on must survive, and only passing noise may go. +func TestPytestRealistic(t *testing.T) { + var r dsl.Registry + if err := r.Load([]byte(builtinFilters)); err != nil { + t.Fatal(err) + } + in := `============================= test session starts ============================== +platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.2.0 +cachedir: .pytest_cache +rootdir: /testbed +plugins: cov-4.1.0, xdist-3.3.1 +collected 214 items + +tests/test_a.py::test_one PASSED [ 1%] +tests/test_a.py::test_two PASSED [ 2%] +tests/test_b.py::test_three FAILED [ 3%] +tests/test_b.py::test_four PASSED [ 4%] +tests/test_c.py::test_five ERROR [ 5%] +tests/test_c.py::test_six SKIPPED (needs network) [ 6%] +tests/test_c.py::test_seven XFAIL [ 7%] + +=================================== FAILURES =================================== +_________________________________ test_three ___________________________________ + + def test_three(): +> assert compute(2) == 5 +E assert 4 == 5 + +tests/test_b.py:12: AssertionError +==================================== ERRORS ==================================== +_______________________ ERROR at setup of test_five ____________________________ +E fixture 'db' not found +=========================== short test summary info ============================ +FAILED tests/test_b.py::test_three - assert 4 == 5 +ERROR tests/test_c.py::test_five +============= 1 failed, 3 passed, 1 skipped, 1 xfailed, 1 error in 2.14s ======= +` + c := r.Match(selectorKey(in)) + if c == nil || c.Name != "pytest" { + t.Fatalf("routed to %v", c) + } + out, loss := dsl.Apply(c, in) + // Nothing was capped or cut mid-line, so no recovery marker is warranted at all. + if loss != dsl.LossNone { + t.Errorf("stripping known-noise lines should not report a cut, got loss=%d", loss) + } + for _, must := range []string{ + "FAILED tests/test_b.py::test_three", "assert 4 == 5", "tests/test_b.py:12", + "ERROR tests/test_c.py::test_five", "fixture 'db' not found", + "1 failed, 3 passed", "SKIPPED", "XFAIL", "collected 214 items", + } { + if !strings.Contains(out, must) { + t.Errorf("pytest filter DROPPED %q", must) + } + } + if strings.Contains(out, "test_one PASSED") { + t.Error("passing noise should be gone") + } +} From 10172c120eda31f5e06ce287afb934b63e1fc3e5 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 05:34:40 +0000 Subject: [PATCH 5/7] fix(cmdfilter): match a multi-line selector, and key selectors on tool identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaying captured proxy traffic through the real registry exposed two defects that no unit test would have caught, because both are about what the filters MISS. 1. The selector was one line, so a filter's reach depended on the agent's output framing rather than on the tool that produced the output. 112 pytest runs (311 KB) matched nothing because the harness prepends its own preamble ("Exit code 1", "Internet access disabled") or the report opens with a bare "ERROR path::test", so pytest's session banner was never line 1. selectorKey now spans the first 6 non-empty lines and match regexes compile with (?m) so ^/$ anchor per line. Measured on real SWE traffic: claimed output 13 -> 124 of 520 (2.5% -> 23.8%). Kept at 6 lines deliberately — a whole-blob scan would let a generic pattern match some incidental line deep inside unrelated output. That widening made generic filters shadow specific ones, exactly the ordering hazard the multi-line selector introduces, and the routing guardrail test caught every case. Resolved with priority: tool-identity banners at 20, gcc at -10 (its selector is the generic "some compiler said something" shape that also occurs inside make/swift/dotnet output, so it is the last resort, not the first match). poetry's "- Installing x (ver)" was also tightened — bare "- Installing" is how terraform init reports providers. 2. swift-build's '^Compiling ' claimed CYTHON output in a live run and stripped its "Compiling x.pyx because it changed" lines. A generic verb is not a tool signature. Now requires Swift identity (a .swift file or a Swift build phase), with a regression test asserting Cython and cargo output are not claimed. The miss ledger keys on the first line only: the selector is multi-line, and keying a bounded ledger on multi-line blobs would make nearly every entry unique and spend the cap on noise instead of ranking real shapes. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- components/dsl/dsl.go | 6 +++- components/offload/cmdfilter.go | 39 ++++++++++++++++++++---- components/offload/cmdfilter_apt_test.go | 19 ++++++++++++ components/offload/cmdfilter_filters.go | 36 +++++++++++++++++++--- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/components/dsl/dsl.go b/components/dsl/dsl.go index 1db0965..56025fb 100644 --- a/components/dsl/dsl.go +++ b/components/dsl/dsl.go @@ -155,7 +155,11 @@ func Compile(name string, d Def) (*Compiled, error) { } else if d.CapReduce != 0 { return nil, fmt.Errorf("dsl: filter %q sets cap_reduce without cap", name) } - m, err := regexp.Compile(d.Match) + // The selector spans a few leading lines, not one, so `^`/`$` in a match regex must + // mean "start/end of A line" rather than "of the whole selector". Without (?m) a + // filter anchored at ^ only matches output whose very FIRST line is its signature, + // which is exactly the output-framing dependence a multi-line selector removes. + m, err := regexp.Compile("(?m)" + d.Match) if err != nil { return nil, fmt.Errorf("dsl: filter %q match: %w", name, err) } diff --git a/components/offload/cmdfilter.go b/components/offload/cmdfilter.go index 45874a1..c328065 100644 --- a/components/offload/cmdfilter.go +++ b/components/offload/cmdfilter.go @@ -98,8 +98,11 @@ func (f *Cmdfilter) Offload(req *schemas.BifrostChatRequest, rep *components.Rep if filt == nil { if c.FilterStats != nil { // The miss ledger: it turns "which filter to write next" into data - // instead of guesswork (after rtk's parse_failures table). - c.FilterStats.FilterMiss(key) + // instead of guesswork (after rtk's parse_failures table). Log only the + // FIRST line — the selector is multi-line, and keying the bounded ledger + // on whole multi-line blobs would make almost every entry unique and + // exhaust the cap on noise instead of ranking real shapes. + c.FilterStats.FilterMiss(firstLine(key)) } continue } @@ -149,15 +152,39 @@ func (f *Cmdfilter) Offload(req *schemas.BifrostChatRequest, rep *components.Rep return keys, nil } -// selectorKey is the string a filter's match regex is tested against: the first -// non-empty, trimmed line of the tool output. +// selectorHeadLines is how many leading non-empty lines a filter's match regex is +// tested against. It is NOT 1, and that matters: measuring real agent traffic showed +// 112 pytest outputs (311 KB) matching nothing because the harness prepends its own +// preamble ("Exit code 1", "Internet access disabled") or the run opens with a bare +// "ERROR path::test" line, so pytest's "=== test session starts ===" banner is never +// the FIRST line. A one-line selector makes a filter's reach depend on the agent's +// output framing rather than on the tool that produced it. +// +// Kept small on purpose: a whole-blob scan would let a generic pattern match on some +// incidental line deep inside unrelated output, which is the opposite failure. +const selectorHeadLines = 6 + +// firstLine returns the leading line of a (possibly multi-line) selector key. +func firstLine(key string) string { + if i := strings.IndexByte(key, '\n'); i >= 0 { + return key[:i] + } + return key +} + +// selectorKey is the string a filter's match regex is tested against: the first few +// non-empty, trimmed lines of the tool output, newline-joined. func selectorKey(content string) string { + var head []string for _, line := range strings.Split(content, "\n") { if s := strings.TrimSpace(line); s != "" { - return s + head = append(head, s) + if len(head) == selectorHeadLines { + break + } } } - return "" + return strings.Join(head, "\n") } // recoveryHint types the hint by WHAT was lost. A clean contiguous tail cut is diff --git a/components/offload/cmdfilter_apt_test.go b/components/offload/cmdfilter_apt_test.go index 35949f8..d4171e5 100644 --- a/components/offload/cmdfilter_apt_test.go +++ b/components/offload/cmdfilter_apt_test.go @@ -42,3 +42,22 @@ func TestAptKeepsProblems(t *testing.T) { t.Fatalf("pure boilerplate should collapse, got %q", out) } } + +// Selectors must key on TOOL IDENTITY, not on a generic verb. Found in production: +// swift-build's bare '^Compiling ' claimed CYTHON output and stripped its +// "Compiling x.pyx because it changed" lines. These are real outputs from other tools +// that a tool-specific filter must not claim. +func TestSelectorsDoNotClaimForeignOutput(t *testing.T) { + var r dsl.Registry + if err := r.Load([]byte(builtinFilters)); err != nil { + t.Fatal(err) + } + for _, tc := range []struct{ name, out string }{ + {"cython", "Compiling pkg/mod.pyx because it changed.\n[1/4] Cythonizing pkg/mod.pyx\n[2/4] Cythonizing pkg/other.pyx\nrunning build_ext\n"}, + {"cargo", "Compiling serde v1.0.197\nCompiling libc v0.2.153\n Finished dev [unoptimized] target(s) in 4.21s\n"}, + } { + if c := r.Match(selectorKey(tc.out)); c != nil && c.Name == "swift-build" { + t.Errorf("%s output must not be claimed by swift-build", tc.name) + } + } +} diff --git a/components/offload/cmdfilter_filters.go b/components/offload/cmdfilter_filters.go index ba1b745..989be51 100644 --- a/components/offload/cmdfilter_filters.go +++ b/components/offload/cmdfilter_filters.go @@ -31,7 +31,7 @@ filters: description: keep failures + summary, drop passing noise family: tests priority: 10 - match: '(pytest|=+ test session starts)' + match: '(^=+ test session starts|^=+ (FAILURES|ERRORS|short test summary info)|^(FAILED|ERROR) \S+::|^\d+ (passed|failed|error)|^collected \d+ items)' strip_lines_matching: - '^\s*$' - ' PASSED' @@ -47,6 +47,7 @@ filters: npm-install: description: collapse npm/yarn/pnpm install chatter family: pkg + priority: 20 match: '^(npm |yarn |pnpm |added \d|removed \d|up to date)' strip_lines_matching: - '^npm warn' @@ -62,6 +63,7 @@ filters: make: description: drop make directory chatter and no-op notices family: builds + priority: 20 match: '^(make(\[\d+\])?:|(gcc|g\+\+|cc|clang) )' strip_lines_matching: - '^make(\[\d+\])?: (Entering|Leaving) directory' @@ -74,6 +76,7 @@ filters: gradle: description: strip Gradle progress and no-op tasks, keep tasks and errors family: builds + priority: 20 match: '^(> (Task|Configuring project|Resolving dependencies|Transform )|Starting a Gradle Daemon|BUILD (SUCCESSFUL|FAILED))' strip_ansi: true strip_lines_matching: @@ -95,6 +98,7 @@ filters: xcodebuild: description: strip xcodebuild build phases and tool invocations, keep diagnostics family: builds + priority: 20 match: '^(note: Using new build system|CompileC |CompileSwift |Ld |CodeSign |PhaseScriptExecution |\*\* BUILD)' strip_ansi: true strip_lines_matching: @@ -135,6 +139,10 @@ filters: gcc: description: strip include traces and diagnostic counters, keep every error and warning family: builds + # Lowest priority ON PURPOSE: this selector is a generic compiler-diagnostic shape + # that also occurs inside make / swift / dotnet output. It is the fallback for + # "some compiler said something" when no tool-specific filter claimed the output. + priority: -10 match: '^(In file included from|/usr/bin/ld:|collect2: error:|\S+: In function|\S+:\d+:(\d+:)? (error|warning|note):)' strip_ansi: true strip_lines_matching: @@ -150,12 +158,13 @@ filters: swift-build: description: strip Compiling/Linking noise, collapse a clean build family: builds - match: '^(Compiling |Linking |Building for |Build complete!)' + priority: 20 + match: '^(Compiling \S+ \S+\.swift|Building for (debugging|production)|Build complete!|\S+\.swift:\d+:\d+: (error|warning):)' strip_ansi: true strip_lines_matching: - '^\s*$' - - '^Compiling ' - - '^Linking ' + - '^Compiling \S+ \S+\.swift' + - '^Linking \S+$' match_output: - pattern: 'Build complete!' message: 'ok (build complete)' @@ -165,6 +174,7 @@ filters: dotnet-build: description: strip MSBuild banners, collapse a clean build family: builds + priority: 20 match: '^(Microsoft \(R\) Build Engine|MSBuild version)' strip_ansi: true strip_lines_matching: @@ -183,6 +193,7 @@ filters: turbo: description: strip Turborepo cache status noise, keep task results family: builds + priority: 20 match: '^(cache (hit|miss|bypass)|\d+ packages in scope|> [^ ]+:[^ ]+$)' strip_ansi: true strip_lines_matching: @@ -199,6 +210,7 @@ filters: nx: description: strip Nx task-graph banners, keep task output family: builds + priority: 20 match: '^(> +NX +|Nx \(powered by)' strip_ansi: true strip_lines_matching: @@ -214,6 +226,7 @@ filters: terraform-plan: description: strip state-refresh and unchanged-resource noise from a terraform/tofu plan family: iac + priority: 20 match: '^(Acquiring state lock|Releasing state lock|Refreshing state|(Terraform|OpenTofu) (will perform|used the selected providers)|No changes\.)' strip_ansi: true strip_lines_matching: @@ -228,6 +241,7 @@ filters: terraform-init: description: strip provider download spam from a terraform/tofu init family: iac + priority: 20 match: '^Initializing (the backend|provider plugins|modules)' strip_ansi: true strip_lines_matching: @@ -244,6 +258,7 @@ filters: pulumi: description: strip pulumi banners, permalinks and per-resource progress rows family: iac + priority: 20 match: '^(Previewing (update|refresh|destroy)|Updating \(|Refreshing \(|Destroying \(|No stacks found|Please choose a stack|Current stack is)' strip_ansi: true match_output: @@ -287,6 +302,7 @@ filters: liquibase: description: strip the ASCII banner, jar inventory and INFO chatter family: builds + priority: 20 match: '^(#{10,}$|Starting Liquibase at|Liquibase (Community|Open Source) )' strip_ansi: true strip_lines_matching: @@ -308,6 +324,7 @@ filters: ssh: description: strip ssh connection banners and debug1 flood, keep the remote output family: net + priority: 20 match: '^(Warning: Permanently added|debug1:|OpenSSH_|Pseudo-terminal)' strip_ansi: true strip_lines_matching: @@ -324,6 +341,7 @@ filters: ping: description: drop per-packet replies, keep the statistics block family: net + priority: 20 match: '^(PING |Pinging )' strip_ansi: true strip_lines_matching: @@ -337,6 +355,7 @@ filters: rsync: description: strip the file list and byte counters, collapse a clean sync family: net + priority: 20 match: '^(sending incremental file list|rsync: |rsync error:)' strip_ansi: true strip_lines_matching: @@ -352,6 +371,7 @@ filters: bundle-install: description: strip cached-gem "Using" lines, collapse a complete bundle family: pkg + priority: 20 match: '^(Fetching gem metadata|Resolving dependencies\.|Using \S+ \d)' strip_ansi: true strip_lines_matching: @@ -371,7 +391,8 @@ filters: poetry-install: description: strip poetry download/install chatter, collapse an up-to-date lock family: pkg - match: '^(Installing dependencies from lock file|Creating virtualenv|Using virtualenv|[•-] Installing |[•-] Downloading )' + priority: 20 + match: '^(Installing dependencies from lock file|Creating virtualenv|Using virtualenv|\s*[•-] (Installing|Downloading) \S+ \()' strip_ansi: true strip_lines_matching: - '^\s*$' @@ -390,6 +411,7 @@ filters: composer-install: description: strip composer download/install chatter, collapse a no-op install family: pkg + priority: 20 match: '^Loading composer repositories' strip_ansi: true strip_lines_matching: @@ -407,6 +429,7 @@ filters: uv-sync: description: strip uv download/cache chatter, collapse an audited-only sync family: pkg + priority: 20 match: '^(Resolved \d+ packages|Downloading \S+\.whl|Using cached \S+\.whl|Audited \d+ package)' strip_ansi: true strip_lines_matching: @@ -423,6 +446,7 @@ filters: apt: description: collapse apt/dpkg install boilerplate, keep errors and configuration prompts family: pkg + priority: 20 match: '^(Get:\d+ http|Selecting previously unselected package|Preparing to unpack |Unpacking |Setting up |Processing triggers for |Reading package lists|Building dependency tree)' strip_ansi: true strip_lines_matching: @@ -453,6 +477,7 @@ filters: brew-install: description: strip brew download/pour chatter, collapse an already-installed formula family: pkg + priority: 20 match: '^(==> (Fetching|Downloading)|Warning: .+ is already installed)' strip_ansi: true strip_lines_matching: @@ -471,6 +496,7 @@ filters: quarto-render: description: strip quarto render progress, collapse a successful render family: builds + priority: 20 match: '^processing file: ' strip_ansi: true strip_lines_matching: From 58d4857d49818354972460fafb5845150321cc9a Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 05:37:50 +0000 Subject: [PATCH 6/7] docs(cmdfilter): document the multi-line selector, priority ordering and the tool-identity rule Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- docs/components/cmdfilter.md | 33 +++++++++++++++++++++++++++++++- docs/components/dsl.md | 16 +++++++++++----- docs/how-to/custom-dsl-filter.md | 20 ++++++++++++++----- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/docs/components/cmdfilter.md b/docs/components/cmdfilter.md index 67118be..61afd84 100644 --- a/docs/components/cmdfilter.md +++ b/docs/components/cmdfilter.md @@ -18,10 +18,20 @@ the newest tool output, in the mutable tail. The shipped filters are adapted from [rtk](https://github.com/rtk-ai/rtk) (Apache-2.0 — see `THIRD-PARTY-NOTICES`), with one systematic rewrite. rtk is a shell hook: it matches a **command string** (`^terraform\s+plan`). A proxy never sees the command — it sees the *result*. So every -filter here matches an **output-shape signature** against the output's first non-empty line +filter here matches an **output-shape signature** against the output's first few non-empty lines (`^Refreshing state`, `^> Task :`, `^==> Downloading`). rtk's command regexes are not portable as written; copied over they would compile fine and never fire. +The selector spans the first **6** non-empty lines, not one, and that detail is load-bearing. +Measured on real agent traffic, a one-line selector missed 112 pytest runs (311 KB) outright: the +harness prepends its own preamble (`Exit code 1`, `Internet access disabled`) or the report opens +with a bare `ERROR path::test`, so pytest's session banner is never line 1. A one-line selector ties +a filter's reach to the agent's *output framing* rather than to the tool that produced the output. +Widening it took claimed output from 13 to 124 of 520 eligible outputs (2.5% → **23.8%**). It stays +at 6 lines on purpose: a whole-blob scan would let a generic pattern match some incidental line deep +inside unrelated output, which is the opposite failure. Match regexes compile with `(?m)`, so `^` +and `$` anchor per line. + That is also the structural advantage: rtk's hook only sees Bash calls, so an agent's built-in `Read`/`Grep`/`Glob` tools are invisible to it. A proxy sees every tool result regardless of origin. @@ -106,6 +116,27 @@ collapse. `TestEveryMatchOutputRuleIsGuarded` fails the build if one is added wi word "error" would never let it collapse. It guards on the diagnostic *form* instead (`error CS1002` / `warning CS0168`). +### Ordering: `priority`, because a wider selector shadows + +A multi-line selector lets a generic filter claim output a specific one should own. That is a real +hazard, and `TestEveryBuiltinFilterHasTestsAndRoutes` catches it: it asserts every filter's own test +input routes to *that* filter. Resolved with explicit `priority`: + +- **20** — tool-identity banners (`make`, `apt`, `terraform-*`, `pulumi`, the package managers …). + Unambiguous, so they win. +- **10** — `pytest`, matched on its report shapes. +- **-10** — `gcc`. Its selector is the *generic* `file:line:col: error:` diagnostic shape, which also + occurs inside make, swift and dotnet output. It is the deliberate last resort for "some compiler + said something" when no tool-specific filter claimed the output. + +### A selector must key on tool IDENTITY, not a generic verb + +Found in a live run: `swift-build`'s `^Compiling ` claimed **Cython** output and stripped its +`Compiling x.pyx because it changed` lines. Cython, cargo and others all print `Compiling`. The +selector now requires Swift identity (a `.swift` file or a Swift build phase), and +`TestSelectorsDoNotClaimForeignOutput` asserts Cython and cargo output are not claimed. Prefer a +signature no other tool emits over a verb that many do. + ### Line budgets: shared `cap` classes Filters select a budget by **signal density** (`cap: errors`) rather than each hand-picking a diff --git a/docs/components/dsl.md b/docs/components/dsl.md index f556ae1..8fb5aa9 100644 --- a/docs/components/dsl.md +++ b/docs/components/dsl.md @@ -22,7 +22,8 @@ flowchart LR All optional except `match`: -- `match` — regex vs the selector (= first non-empty line) +- `match` — regex vs the selector (= the first few non-empty lines, `(?m)` applied so `^`/`$` + anchor per line) - `family` — command family for per-family metrics (`builds` / `tests` / `iac` / `pkg` / `net` / …) - `priority` — match order; higher first, then by name. Absent (`0`) is today's name ordering. - `strip_ansi` @@ -37,10 +38,15 @@ All optional except `match`: ### `priority`, and why order matters more here -`cmdfilter` matches on the *shape of the output*, not on a command, so a generic pattern can shadow a -specific one in a way rtk's command matching never had to deal with. `priority` makes -specific-before-generic explicit instead of dependent on alphabetical luck. Absent, ordering is by -name — exactly the previous behavior. +`cmdfilter` matches on the *shape of the output*, not on a command, and against several leading lines +rather than one — so a generic pattern can shadow a specific one in a way rtk's command matching +never had to deal with. This is not hypothetical: widening the selector made `gcc` start claiming +`make`, `swift-build` and `dotnet-build` output, because a bare `file:line: error:` line appears +inside all of them. `priority` makes specific-before-generic explicit instead of dependent on +alphabetical luck. Absent, ordering is by name — exactly the previous behavior. + +Rule of thumb: a filter whose selector is a *tool banner* can be high priority; one whose selector is +a *generic diagnostic shape* must be low, so it only catches what nothing else claimed. ### Line budgets: `cap` classes diff --git a/docs/how-to/custom-dsl-filter.md b/docs/how-to/custom-dsl-filter.md index 16a2007..41e4d6d 100644 --- a/docs/how-to/custom-dsl-filter.md +++ b/docs/how-to/custom-dsl-filter.md @@ -29,7 +29,7 @@ All optional except `match`. | Field | Purpose | |---|---| -| `match` | regex tested against the **selector** (the output's first non-empty line) — decides if this filter applies | +| `match` | regex tested against the **selector** (the output's first few non-empty lines; `(?m)` is applied, so `^`/`$` anchor per line) — decides if this filter applies | | `family` | command family for the per-family `/stats` ledger: `builds` / `tests` / `iac` / `pkg` / `net` / … | | `priority` | match order — higher first, then by name. Use it to put a specific filter ahead of a generic one. | | `strip_ansi` | strip ANSI escape codes | @@ -85,10 +85,20 @@ run collapses to the `on_empty` message — if a future edit breaks that, the lo ### Write the selector against a real sample -This is the step that most often goes wrong. `match` is tested against the output's **first -non-empty line**, not against a command. An `rtk`-style command regex (`^terraform\s+plan`) compiles -fine and never fires. Paste a real sample of the output, take its first non-empty line, and write the -regex against *that* (`^Refreshing state`, `^> Task :`, `^==> Downloading`). +This is the step that most often goes wrong. `match` is tested against the output's **first few +non-empty lines**, not against a command. An `rtk`-style command regex (`^terraform\s+plan`) compiles +fine and never fires. Paste a real sample of the output and write the regex against a line that +actually appears near its top (`^Refreshing state`, `^> Task :`, `^==> Downloading`). + +Two traps, both found by measuring real traffic rather than by reasoning: + +- **Don't assume your signature is line 1.** Agent harnesses prepend their own preamble + (`Exit code 1`, `Internet access disabled`), so the selector deliberately spans several lines. Do + not write a filter that only works when its banner leads. +- **Key on tool identity, not a generic verb.** `^Compiling ` looks like a Swift signature and is + also what Cython and cargo print — a filter anchored on it will strip other tools' output. Prefer a + signature no other tool emits (`^Compiling \S+ \S+\.swift`), and give a filter whose selector is + unavoidably generic a **negative** `priority` so it only catches leftovers. Selectors that match nothing are logged: `/stats` exposes `cmdfilter_selector_misses`, the frequency- ranked list of output shapes no filter claimed. That is the backlog of filters worth writing — the From d50b44de9788ee02162fdfda16941214c6a33bda Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 05:56:26 +0000 Subject: [PATCH 7/7] test(cmdfilter): assert never_worse holds for every shipped filter The marker costs tokens too, so a filter that barely wins can still grow a message. A wider selector routes more output to more filters, so sweep every filter's own test inputs through the component and assert no message grows. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- .../offload/cmdfilter_neverworse_test.go | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 components/offload/cmdfilter_neverworse_test.go diff --git a/components/offload/cmdfilter_neverworse_test.go b/components/offload/cmdfilter_neverworse_test.go new file mode 100644 index 0000000..1e290b6 --- /dev/null +++ b/components/offload/cmdfilter_neverworse_test.go @@ -0,0 +1,44 @@ +package offload + +import ( + "context" + "strings" + "testing" + + "github.com/maximhq/bifrost/core/schemas" + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/components/dsl" + "github.com/rossoctl/context-guru/schema" + "github.com/rossoctl/context-guru/store" + "gopkg.in/yaml.v3" +) + +// never_worse at the message level: for EVERY shipped filter, running its own test +// inputs through the component must never make a message larger. The marker costs +// tokens too, so a filter that barely wins can still grow the message — which is why +// the guard compares the marker-INCLUSIVE rewrite. A wider selector routes more +// output to more filters, so this is worth asserting across the whole set. +func TestNeverWorseAcrossEveryBuiltin(t *testing.T) { + var f dsl.File + if err := yaml.Unmarshal([]byte(builtinFilters), &f); err != nil { + t.Fatal(err) + } + comp := newFilterComp(t, "min_size: 1\n") // ignore the floor so every case is exercised + for name, cases := range f.Tests { + for _, tc := range cases { + if strings.TrimSpace(tc.Input) == "" { + continue + } + req := &schemas.BifrostChatRequest{Provider: schemas.Anthropic, Input: []schemas.ChatMessage{cmdToolMsg(tc.Input)}} + c := &components.Ctx{Ctx: context.Background(), Session: "s", Store: store.NewMemory(store.Options{}), MaxCachedIdx: -1} + if _, err := comp.Offload(req, &components.Report{}, c); err != nil { + t.Fatal(err) + } + before := schema.TextTokens(tc.Input) + after := schema.TextTokens(schema.MessageText(req.Input[0])) + if after > before { + t.Errorf("%s/%s GREW the message: %d -> %d tokens", name, tc.Name, before, after) + } + } + } +}