Skip to content

feat(cmdfilter): port rtk's filter coverage (3 -> 24) and fix three DSL loss-typing bugs - #37

Closed
OsherElhadad wants to merge 7 commits into
mainfrom
feat/i29-cmdfilter
Closed

feat(cmdfilter): port rtk's filter coverage (3 -> 24) and fix three DSL loss-typing bugs#37
OsherElhadad wants to merge 7 commits into
mainfrom
feat/i29-cmdfilter

Conversation

@OsherElhadad

@OsherElhadad OsherElhadad commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Closes #29

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 the port is not a copy, and the interesting work turned out to be
in the two things measurement contradicted.

The three engine bugs, fixed with tests

  1. truncate_lines_at never recorded loss. It ran before loss was initialized (dsl.go:192-200
    vs :202), so a real intra-line cut reported LossNone and cmdfilter emitted no recovery
    hint for a real loss
    . Now types as LossWhole — an intra-line cut is non-contiguous by nature,
    since every long line loses its own tail.
  2. truncate_lines_at cut silently. A mid-line cut with no marker reads as corrupted output to a
    model. Now appends ..., sized to fit inside the cap so the line never grows.
  3. recoveryHint collapsed LossTail and LossWhole. Both got the same "call
    context_guru_expand" text, making a cheap partial recovery look like an expensive whole-blob
    one and pushing the agent to the costly path. Now distinct: a tail cut names the cut point.

The port: 24 filters, every selector rewritten

rtk matches a shell command (^terraform\s+plan); a proxy only ever sees the output. Every
selector is rewritten to an output-shape signature. Copied verbatim, rtk's regexes compile fine and
never fire — so TestEveryBuiltinFilterHasTestsAndRoutes asserts each filter's own test input
actually routes to that filter. That test is what makes the rewrite verifiable instead of hopeful,
and it caught every shadowing regression below.

Ported (22): 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).

Tier 3 not ported, with reasons recorded in docs/components/cmdfilter.md: the ~24
width-cap-only filters, the 11 blank-line-only linter filters, spring-boot, filter_stderr, rtk's
86 command-detection rules, and its ~50 native Rust filters.

Every success-collapse rule carries an unless guard (rtk ships 9 of 11 unguarded), each with 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 find what a bare collapse swallowed.
TestEveryMatchOutputRuleIsGuarded fails the build if one is added without a guard. dotnet-build
needed care: 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 (error CS1002).

Also: shared cap budget classes by signal density instead of 24 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 bounded selector-miss ledger; per-family metrics in /stats.

Two things measurement contradicted

I replayed the shipped selectors over recorded proxy traffic before trusting the plan. Both findings
changed the code.

1. The issue's Tier-1 priorities did not survive contact with the data. On a Terminal-Bench dump,
pulumi, terraform-plan, xcodebuild and gradle fired zero times. What dominated were two
shapes rtk has no filter for: apt/dpkg boilerplate (584 outputs, ~1.0 MB — the largest reachable
family) and gcc's <file>: In function 'main': header. I added both; apt gets 76% compression and
went on to carry ~73% of all savings in the live run. The IaC filters stay — correct, tested, free
when inert — but the docs now say plainly that a filter set's value comes from the workload, not its
size. That is what the miss ledger is for.

2. The one-line selector was a real bug in my own port. 112 pytest runs (311 KB) matched nothing
because the agent 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. selectorKey now spans the first 6 non-empty lines, with (?m) so ^/$
anchor per line:

workload claimed before claimed after
SWE-bench captured traffic (520 eligible outputs) 13 (2.5%) 124 (23.8%)
Terminal-Bench dump (223 eligible outputs) 140 (62.8%) 172 (77.1%)

Widening it made generic filters shadow specific ones — exactly the ordering hazard #29 flagged — and
the routing guardrail caught every case. Resolved with explicit priority: tool 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).

A false positive caught in a live run: swift-build's ^Compiling claimed Cython output and
stripped its Compiling x.pyx because it changed lines — it fired 80× on a C build task with almost
no unique saving, visible directly in the new per-filter ledger. A generic verb is not a tool
signature. Fixed to require Swift identity, with a regression test asserting Cython and cargo output
are not claimed.

Benchmarks

Terminal-Bench, build-heavy tasks, off baseline vs codesmart, matched per task. Reward and
steps are the numbers that matter here
— this is the failure mode that sank rtk on Terminal-Bench.

task reward off → cs steps off → cs norm cost off → cs
build-pmars 1.0 → 1.0 32 → 34 $0.461 → $0.503
build-cython-ext 0.0 → 1.0 70 → 59 $1.177 → $1.213
compile-compcert 1.0 → 0.0 55 → 58 $1.602 → $1.128
aggregate 2/3 = 2/3 52.3 → 50.3 (−3.8%) $3.240 → $2.843 (−12.3%)

Cache-hit 95.97% → 97.43%; cache-write 393k → 239k (−39%) — the lever that actually mattered
here. Total including context-guru's own LLM cost: $2.862 (−11.7%). cmdfilter: 128 acts / 153 runs,
0 reverts, 0 expand bounces, ~2 ms/request.

Reward is net neutral (2 = 2), not improved — and I am claiming neither flip. Both went in
opposite directions, and in each case the losing trial was an exception: true with agent_wall_s
pinned exactly at its budget
(1800.0 s and 4800.0 s). Both flips are wall-clock timeouts, not
information loss.
The cython "win" and the compcert "loss" are the same artifact.

The one real concern, and it is not the rtk failure mode. compcert's codesmart trial burned +17%
agent wall (4089 s → 4800 s) and hit its budget. Steps went down overall and aggregate reward held,
so this is not the "agent lost information → more steps" spiral that sank rtk. It is added latency:
cg_added_ms_avg 5 ms → 145 ms, upstream 12.9 s → 15.6 s/req — dominated by extract_llm
(17.8 s across 2 calls, 0 tokens saved)
, not cmdfilter. On a task already near its wall clock that
was enough to push it over. That belongs to #28, which independently concluded extract_llm is
net-negative on caching backends; it is not a reason to back off these filters.

Live per-family ledger (the new /stats fields):

cmdfilter: 128 acts, 0 reverted
  pkg    (apt)          — the largest reachable family, ~73% of live savings
  tests  (pytest)
  builds (swift-build ← the Cython false positive, now fixed)

Honest caveats, stated plainly:

  • The SWE-bench control arm is unusable and I am not reporting it as a result. Another agent's
    proxy already held the port I had been assigned, so my first attempt was measuring through
    someone else's config; after moving to a verified-free port, 2 of 3 codesmart trials aborted with
    API Error: ConnectionRefused from proxy restarts. One clean trial is not a control. What it did
    give me is the capture file that exposed the selector bug — the more valuable outcome.
  • n=1 per task. All three rewards carry single-trial noise; with two of the three decided by a
    timeout, the reward column should be read as "no regression detected", not as a measurement.
  • This ran the PRE-FIX binary. The selector widening (2.5% → 23.8% match rate on SWE traffic) and
    the Cython false-positive fix landed after this run, so the merged version should produce more
    output than these numbers show.
  • Unique savings are small in absolute terms (~3.6k tokens). On a ~96% cached agent this is a
    rounding error against the bill, consistent with the existing finding that cost tracks steps, not
    unique tokens. The case for cmdfilter is that it is free — no LLM, ~2 ms, cache-safe — not that
    it moves the bill on its own.
  • The TB arm ran the pre-fix binary, so its cmdfilter numbers understate the merged version
    (the selector fix and the Cython fix both landed after it started).

Licensing

THIRD-PARTY-NOTICES (new) names rtk, github.com/rtk-ai/rtk, Apache-2.0, the
Copyright 2024 rtk-ai and rtk-ai Labs notice, and states that filter definitions and the DSL
pipeline are "adapted from, with modifications" — enumerating them. Apache-2.0 §4 applies to the
filter TOMLs as copyrightable expression; the existing prose credit in dsl.go is good practice but
not sufficient for a bulk port.

Verification

CGO_ENABLED=1 go build -tags cg_skeleton ./... · go test -tags cg_skeleton ./... ·
go test -race · make lint · gofmt -l — all clean. /stats additions are additive only, with
a test asserting the keys deploy/harbor/*.py parses still exist.

Docs: docs/components/cmdfilter.md (full filter table, what each preserves, the Tier-3 rationale,
the two measurement findings), docs/components/dsl.md (loss typing, cap classes, priority),
docs/how-to/custom-dsl-filter.md (both selector traps), THIRD-PARTY-NOTICES.

@OsherElhadad

Copy link
Copy Markdown
Collaborator Author

Orchestrator verification — the benchmark reading is correct, and the caveats are real

I re-derived everything from /tmp/i29-runs/ row files rather than taking the table on trust.

Both reward flips confirmed as wall-clock timeouts, not information loss

This is the load-bearing claim, and it checks out exactly as stated:

task reward steps exception agent_wall_s
build-cython-ext 0.0 → 1.0 70 → 59 True → False 1800.0 → 1386.1
compile-compcert 1.0 → 0.0 55 → 58 False → True 4089.4 → 4800.0
build-pmars 1.0 → 1.0 32 → 34 False → False 776.7 → 747.4

Each losing trial is an exception: true with agent_wall_s pinned exactly at its budget (1800.0 and 4800.0). Declining to claim either flip is the right call — they are the same artifact pointing opposite directions, and treating the cython "win" as a gain while explaining away the compcert "loss" would have been the tempting and wrong move.

Aggregate figures reproduce

off codesmart
billed cost $3.240 $2.843 (−12.3%)
cache-read 9,379,052 9,183,987
cache-write 393,402 239,210 (−39.2%)
cache-hit 95.97% 97.43%

Cache-write −39% is the result that matters on this benchmark, and it is the lever the two-benchmark study identified as decisive on long-horizon contexts. Worth noting it lands in the same direction and similar magnitude as #35's async arms (−45% SWE / −39% TB), from a completely different mechanism.

The SWE control is worse than "unusable" — confirm it is excluded

astropy-12907  reward 0.0 steps 2  exception True   →  None (exception True)
astropy-14365  reward 0.0 steps 2  exception True   →  1.0 (34 steps)
astropy-8707   reward 0.0 steps 2  exception True   →  None (exception True)

The off arm is $0.000 total, zero cache-read, all three trials 2-step aborts. There is nothing there. The one codesmart trial that ran has no counterpart. Correctly reported as not-a-result; I confirmed no delta is quoted from it anywhere in the PR body.

On the extract_llm attribution

The claim that compcert's +17% wall came from extract_llm (17.8 s across 2 calls, 0 tokens saved) rather than cmdfilter is consistent with two independent findings: #34 concluded the component is net-negative on caching backends and recommends disabling it there, and #35 measured it making zero calls on Terminal-Bench. Three separate workstreams converging on the same component is worth more than any one of them.

That said, this does mean the merged codesmart preset would still carry the latency that pushed compcert over its budget. That is a preset-composition decision for the merge, not a defect in this PR — but it should not be lost: shipping #37 and #34 together, and acting on #34's recommendation, is what actually removes the observed harm.

What to fix before merge

The caveats are all stated in the PR body, which I've verified. The one thing I'd add: make it explicit that this run used the pre-fix binary, so the selector widening (2.5% → 23.8% SWE match rate) and the Cython false-positive fix are not reflected — the merged version should produce more output than these numbers, and a future reader comparing against a re-run needs to know that.

Verdict from my side: reward is net neutral with no regression detected, the cost and cache-write improvements are real, and the methodology disclosure is honest. The apt filter carrying ~73% of savings makes it the highest-stakes one — I've asked the independent reviewer to hunt for more cases of the ^debconf: class of over-broad pattern that your own safety test already caught once.

OsherElhadad pushed a commit that referenced this pull request Aug 10, 2026
…tion scope

Three high-severity review defects, plus the shipping decision.

H3 (worst): /stats credited the WHOLE pipeline's savings to extract_llm. It passed
snap.SavedTokens — every component's savings — valued against extract_llm's cost
alone, so on a preset like codesmart the component displayed as comfortably
positive while its own arithmetic proves it negative. It inverted the conclusion in
the single field an operator reads.

This also invalidated our own benchmark table: the previously reported "+$0.0287
net win" on non-caching traffic was produced BY the bug. With honest attribution
the best result on these captures is break-even, not profit. Docs corrected.

ExtractSnapshot now takes a per-saved-token RATE and applies it internally to the
component's own GrossSavedTokens, so passing a pipeline-wide total is no longer
expressible. Regression test fails if anyone re-wires it.

H1: markSeenContent fired after the gate ALLOWED a call, so first sight
reclassified itself as recurring and collected a 50% valuation bump (6 expected
reuses vs 4) it had not earned — the gate over-firing, opposite in direction to the
two pessimistic priors fixed earlier. It is now test-and-set on OBSERVATION,
returning the prior value; a suppressed candidate still counts as seen, which is
correct since recurrence is a property of the content, not of what we spent. The
flag-setting path now has tests; previously only the pre-computed bool did.

H2: the exploration budget was per-PROCESS. The tracker lives on a Pipeline held
for proxy lifetime, so the first session spent the whole budget and every later
session inherited an unrevisable prior — the self-justifying-prior failure at
process scope. Now per-session (budget 3 -> 2). The learned ratio is also shrunk
toward the prior and capped: minRatioSampleTokens is one medium output, so a raw
n=1 mean could drop the cached break-even from ~30,500 to ~7,000 permanently.

PR #37: exploration spends wall clock as well as money, and an agent on a task
deadline feels the former more (#37 measured 17.8s over 2 calls saving 0 tokens).
Speculative calls now stop once observed mean latency reaches 6s.

SHIPPING DECISION, in code rather than prose: extract_llm is disabled by default on
prompt-caching backends (allow_on_caching_backend: true overrides). Every caching
workload measured is net-negative even with a correct gate — break-even ~30,500
tokens/output against a largest-observed 2,053 — so the gate could reduce the loss
but never eliminate it. codesmart is the proxy default; shipping a component our own
numbers say loses money, guarded only by a doc note, is not a defensible default.
On caching traffic it now makes zero calls and costs nothing.

Medium items taken:
- Floor dropped from the cache fingerprint except in "auto" mode. It is derived from
  context pressure, so it rotated the key as the window filled and discarded the
  cross-session reuse the key exists to capture; strategyOrder reads it only on the
  "auto" branch, so elsewhere it cannot change the result.
- PromptVersion is now DERIVED from a hash of the prompt constants. A manual constant
  only works while every future editor remembers to bump it, and one omission serves
  extractions produced under rules that no longer exist, with no symptom.
  semanticsVersion remains as the manual hatch for validation-gate changes.
- Cross-session reuse gated on RECOVERABILITY rather than restricted to deletion-only:
  a cached result can be a lossy rewrite steered by another session's goal, which is
  acceptable only while expand can recover the original. With marker_mode summary/off
  (or a non-persisting store) the global cache falls back to same-session reuse, on
  both the read and write side. Verified deletion-only results are always global.

Gates: build, test -tags cg_skeleton, -race -count=5, gofmt -l (clean), go vet.

Refs #28

Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
Osher-Elhadad added 7 commits August 10, 2026 09:04
…SL loss-typing bugs

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 <Osher.Elhadad@ibm.com>
…dger data

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.
- "<file>: 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 <Osher.Elhadad@ibm.com>
Assisted-By: Claude Opus 5
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
…dropped

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 <Osher.Elhadad@ibm.com>
…l identity

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 <Osher.Elhadad@ibm.com>
…and the tool-identity rule

Assisted-By: Claude Opus 5
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
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 <Osher.Elhadad@ibm.com>
@OsherElhadad

Copy link
Copy Markdown
Collaborator Author

Superseded by #42 — same 7 commits rebased onto main after #33/#36/#40 merged. Could not rebase in place because branch protection blocks force-push to this branch.

The components/component.go conflict needed a manual merge, not the keep-both resolution that works for metrics/metrics.go: #36 adds an ExistingBreakpoints field to Ctx, while this branch adds a FilterStats field and a FilterStatsSink interface immediately after the struct. Concatenating the two sides drops the struct field into the interface body and yields invalid Go. Resolved with both fields inside Ctx and the interface separate; full gates re-run green.

@github-project-automation github-project-automation Bot moved this from New/ToDo to Done in Rossoctl Issue Prioritization Aug 10, 2026
OsherElhadad added a commit that referenced this pull request Aug 10, 2026
… verdict (#28, rebased) (#51)

* perf(extract_llm): gate LLM calls on expected value, cache results globally

extract_llm is the only component that spends money to save money, and on
Terminal-Bench it lost: 271 calls, $3.26, ~1,592s of added latency against
~197,548 unique tokens saved — ~8x underwater once those tokens are priced at
the rate they would actually have been billed. ~93% of its realized value came
from the replay cache, not from the LLM.

The cause is arithmetic. A request to a caching backend is ~99.95% cached, so a
token removed from a cached region saves the cache-read rate ($0.30/MTok), not
the fresh-input rate ($3/MTok) — a 10x haircut. Break-even for one ~$0.012 call
is therefore ~12,700 tokens of output under caching versus ~1,270 without it,
and most tool outputs are nowhere near that. The component already carried this
insight as a comment on skip_file_reads; this turns it into an actual gate that
applies to every candidate.

Economic gate (on by default): call the LLM only when
  expected_saving = removed x (1 + expected replays) x per-token value
exceeds the observed mean cost of a call. The per-token value comes from
Ctx.CacheAware, the compression ratio is learned from accepted results (repeated
misses drive it to zero and shut the gate), and the call cost is computed from
real token usage times real model pricing (CHEAP_MODEL_PRICE_*), never a
hard-coded constant. Every decision records a reason.

Global result cache: an extraction is a context-free derived result, so re-key it
on sha256(content + prompt version + model + config fingerprint) with no session
prefix — 82 of 103 unique contents recurred across sessions, and the old key
threw that reuse away. A version, model, or config change misses rather than
serving a stale extraction. Old entries are inert once; a session-scoped entry is
still honored as a migration read. (Contrast #27's xdedup index, session-scoped on
purpose: that makes a conversational reference, this caches a derived result.)

Derived triggering: no per-workload threshold. Context pressure plus growth rate
replace min_tokens, which stays honored when set explicitly. A merely growing
context no longer fires on every step.

Prompt-cache the preamble, with the measurement that matters: the ~1,463-token
invariant contract now goes in a stable system block with cache_control (a leading
system message on OpenAI, which has no explicit breakpoints). Measured against the
gateway, this is INERT on claude-haiku-4-5, whose minimum cacheable prefix is 4096
tokens — a sub-minimum breakpoint is silently ignored, write=0 read=0. It caches on
claude-sonnet-5 (minimum 1024). Shipped because it is free and correct where it
wins, but /stats exposes prompt_cache_read_tokens so nobody infers a cache win
from placement.

Reusing the agent's cached prefix (part B) prototyped and REJECTED, with numbers:
it works mechanically (a 103,019-token prefix read from cache, no write, no
invalidation), but costs $0.034 at 103k, $0.153 at 500k and $0.513 at 1.7M tokens
— 8.5x to 128x a dedicated cheap-model call, and ~$2.04/turn at 4 concurrent
calls. It also risks a cache-write on the agent's own prefix (11.5x a read) and
couples the compaction model to the agent model.

Metrics: /stats gains an extract block with calls, calls avoided by cache, calls
suppressed by the gate, prompt-cache behavior, cost, gross value and NET value
after cost — the honest headline, previously impossible to see because cost was
reported in a field separate from savings. Purely additive; every existing key
keeps its name so deploy/harbor/*.py keeps parsing.

Tests cover the cached-system-block shape on both backends, cross-session cache
reuse, a version bump missing rather than serving stale, gate suppression when
cache-aware, permission on a non-caching backend and for recurring content, the
cost model against known tokens times known price, the documented break-even
sizes, and that the trigger does not fire every step on a growing context.

Refs #28

Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>

* fix(extract_llm): price calls by size and allow bounded exploration

Benchmarking the gate on real captures found two deadlocks of the same shape: a
pessimistic estimate suppressing every call, so nothing was ever observed, so the
estimate could never correct itself. A gate that cannot revise its own prior is
an off switch, not a gate.

1. Flat per-call cost. The gate priced every call at the ~$0.012 Terminal-Bench
   average — roughly 5x the true cost on a workload with small outputs — and
   suppressed everything. Observed cost was $0.0024/call, and calls that were
   ~2.2x profitable were declined. Cost is now analytic and size-aware (preamble
   1,463 tok + shown content + overhead, at real rates), reconciled with the
   observed mean once real calls exist, so it is right on the first call with no
   observations at all.

2. Pessimistic compression ratio. The default was 0.45; measured on real captures
   an accepted extraction removes only 31-254 tokens per call on 400-2,000-token
   outputs — a real ratio near 0.12. Corrected, with the direction of conservatism
   made explicit: for a SPENDING gate, conservative means UNDER-estimating the
   saving. But a pessimistic prior on a workload below break-even then suppressed
   everything and forwent a genuine +$0.0094 net, so the tracker now gets a bounded
   budget of 3 exploratory calls to learn whether this workload actually compresses.

Also fixes /compact hard-coding the context window as unknown, which silently
disabled every fraction-based trigger AND the new pressure-based triggering on
that endpoint — so offline replay/eval measured a different component than ships.

Measured after these fixes (replay of real captures, aws/claude-haiku-4-5;
forced = pre-#28 behavior, gated = new default):

  Terminal-Bench, non-caching:  net +$0.0287 vs +$0.0091 (3.2x), 17,286 vs 6,664
                                tokens saved, 4,654 vs 10,287 ms avg latency
  SWE-bench, non-caching:       13 calls -> 2, net -$0.0571 -> -$0.0016 (-97% waste)
  Both, caching backend:        loss reduced and latency ~30% lower, but still
                                NEGATIVE — the gate cannot make the component pay
                                when saved tokens bill at the cache-read rate

The honest verdict, now documented: on a non-caching backend extract_llm earns its
place; on a caching backend it does not, even after #28. Docs recommend codesafe
(no LLM pass) or dropping extract_llm from codesmart for caching traffic.

Break-even sizes are pinned by test so a drift in these figures fails CI rather
than silently invalidating the documented verdict.

Refs #28

Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>

* fix(extract_llm): correct savings attribution, seen-flag, and exploration scope

Three high-severity review defects, plus the shipping decision.

H3 (worst): /stats credited the WHOLE pipeline's savings to extract_llm. It passed
snap.SavedTokens — every component's savings — valued against extract_llm's cost
alone, so on a preset like codesmart the component displayed as comfortably
positive while its own arithmetic proves it negative. It inverted the conclusion in
the single field an operator reads.

This also invalidated our own benchmark table: the previously reported "+$0.0287
net win" on non-caching traffic was produced BY the bug. With honest attribution
the best result on these captures is break-even, not profit. Docs corrected.

ExtractSnapshot now takes a per-saved-token RATE and applies it internally to the
component's own GrossSavedTokens, so passing a pipeline-wide total is no longer
expressible. Regression test fails if anyone re-wires it.

H1: markSeenContent fired after the gate ALLOWED a call, so first sight
reclassified itself as recurring and collected a 50% valuation bump (6 expected
reuses vs 4) it had not earned — the gate over-firing, opposite in direction to the
two pessimistic priors fixed earlier. It is now test-and-set on OBSERVATION,
returning the prior value; a suppressed candidate still counts as seen, which is
correct since recurrence is a property of the content, not of what we spent. The
flag-setting path now has tests; previously only the pre-computed bool did.

H2: the exploration budget was per-PROCESS. The tracker lives on a Pipeline held
for proxy lifetime, so the first session spent the whole budget and every later
session inherited an unrevisable prior — the self-justifying-prior failure at
process scope. Now per-session (budget 3 -> 2). The learned ratio is also shrunk
toward the prior and capped: minRatioSampleTokens is one medium output, so a raw
n=1 mean could drop the cached break-even from ~30,500 to ~7,000 permanently.

PR #37: exploration spends wall clock as well as money, and an agent on a task
deadline feels the former more (#37 measured 17.8s over 2 calls saving 0 tokens).
Speculative calls now stop once observed mean latency reaches 6s.

SHIPPING DECISION, in code rather than prose: extract_llm is disabled by default on
prompt-caching backends (allow_on_caching_backend: true overrides). Every caching
workload measured is net-negative even with a correct gate — break-even ~30,500
tokens/output against a largest-observed 2,053 — so the gate could reduce the loss
but never eliminate it. codesmart is the proxy default; shipping a component our own
numbers say loses money, guarded only by a doc note, is not a defensible default.
On caching traffic it now makes zero calls and costs nothing.

Medium items taken:
- Floor dropped from the cache fingerprint except in "auto" mode. It is derived from
  context pressure, so it rotated the key as the window filled and discarded the
  cross-session reuse the key exists to capture; strategyOrder reads it only on the
  "auto" branch, so elsewhere it cannot change the result.
- PromptVersion is now DERIVED from a hash of the prompt constants. A manual constant
  only works while every future editor remembers to bump it, and one omission serves
  extractions produced under rules that no longer exist, with no symptom.
  semanticsVersion remains as the manual hatch for validation-gate changes.
- Cross-session reuse gated on RECOVERABILITY rather than restricted to deletion-only:
  a cached result can be a lossy rewrite steered by another session's goal, which is
  acceptable only while expand can recover the original. With marker_mode summary/off
  (or a non-persisting store) the global cache falls back to same-session reuse, on
  both the read and write side. Verified deletion-only results are always global.

Gates: build, test -tags cg_skeleton, -race -count=5, gofmt -l (clean), go vet.

Refs #28

Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>

* fix(extract_llm): evaluate the tail gate before the global result lookup

Only the SAME-SESSION replay may bypass the tail gate. That session already sent
the compacted bytes, so the provider's cached prefix holds the compacted form and
replaying is byte-identical at any depth. A cross-session global hit has no such
guarantee: the receiving session never compacted that content, so its cached
prefix holds the ORIGINAL, and splicing another session's result at depth mutates
already-cached content and forces a suffix re-write at 11.5x the read price --
exactly the harm #40 removed repairLostResult to prevent.

Restore the ordering: getResult, then the tail gate, then getResultGlobal only for
messages the gate permits. A global hit is then frozen into the session so later
turns replay it from the depth-safe path. TestGlobalCacheHitIsNotSplicedAtDepth
fails if the ordering is flattened again; name that invariant at the call site.

Also adapt to #40's unified result-cache key and document why the global namespace
must keep the pair together too: splitting projected text and summary across two
global keys would re-create the half-a-decision bug cross-session, where
independent TTLs let a hit on one and a miss on the other emit projected text with
the summary segment silently gone.

Add TestNoDefaultConfigRunsExtractLLMOnCachingBackend, which drives the bare
defaults and the codesmart preset end to end with an output far above the cached
break-even, so the economics alone would permit the call. It fails if
allow_on_caching_backend or the allowCached wiring is ever lost in a rebase.

Update the measured verdict: the 197,548 saved tokens sit in the cached prefix, so
at cache-read price they are worth $0.0395 against $3.26 and 1,592,467 ms -- 82x
underwater, not the 8x in the improvement plan, which priced them as fresh input.

Assisted-By: Claude Opus 5
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>

* docs(extract_llm): state the 82x cache-read verdict and name the rate used

The improvement plan's ~8x figure implicitly priced the 197,548 saved tokens as
fresh input. They sat in the cached prefix, so they bill at the cache-read rate:
$0.0395 against $3.26 and 1,592,467 ms of blocking time -- 82x underwater. A
later Terminal-Bench arm that excluded the component entirely re-derived this
independently. Name which cache-read rate each figure uses, since the gate reasons
at $0.30/MTok (55x) while the issue quoted $0.20/MTok (82x); the gate's is the
more generous of the two, so the shipped decline is the conservative one.

Assisted-By: Claude Opus 5
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>

---------

Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
Co-authored-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

feat(cmdfilter): port rtk's filter coverage (3 -> ~25 filters) and fix three DSL loss-typing bugs

2 participants