Found by an integration review of the five cache/filter/observe merges. This is new leakage created by #42 × #43 composing — neither PR's own review could see it, because #42's stats sink and #43's observe mode landed independently.
Problem
cmdfilter calls c.FilterStats with no Mode check (components/offload/cmdfilter.go:105 and the FilterAct call site). So an observe-only run — zero enforced requests, sync_enforced: 0 — still reports real filter activity:
"cmdfilter_families": {"tests": {"acts": 1, "saved_tokens": 1787}},
"cmdfilter_filters": {"pytest": {"acts": 1, "saved_tokens": 1787}},
"cmdfilter_selector_misses": [ … ]Those tokens were never saved — nothing was forwarded.
Why this one matters more than the known shared fields
#43's review accepted two deliberately-shared fields: cg_added_ms_avg (a real measurement, and its reading ~0 is observe's headline result) and llm_calls/llm_*_tokens (real money actually spent). Both are labelled via observe_llm_notice, and relabelling real spend as potential_* would have been a worse lie.
These three fields are different: they have no mode label and no potential_* counterpart, so a consumer cannot tell an enforced saving from a hypothetical one. That directly violates #31's stated requirement — "observe/hypothetical metrics must be namespace-separated and can never be aggregated into enforced savings" — and #43's own claim that enforced aggregates are zero by construction in observe mode.
A mis-labelled hypothetical is worse than no number, because it silently inflates the product's headline claim. This is the correctness risk #31 named as its primary one.
Desired behavior
Either:
- gate the
FilterStats sink on Ctx.Mode != ModeObserve, so the enforced fields stay honestly zero; or
- route observe-mode filter activity into
potential_*-namespaced counterparts, matching how the rest of the observe payload is separated.
The second is more useful (per-filter projections are exactly what an evaluating user wants), but the first is correct and cheap. Either beats the status quo.
Relevant code
components/offload/cmdfilter.go:105 and its FilterAct call site.components/component.go — Ctx.Mode, FilterStats FilterStatsSink.metrics/metrics.go — cmdfilter_families / cmdfilter_filters / cmdfilter_selector_misses, and the potential_* / projected_* block for the pattern to follow.proxy/modes.go — where observe short-circuits.
Testing plan
- Run a request in observe mode through a pipeline containing
cmdfilter and assert all three fields are absent or zero (or populated only under potential_*).
- Assert the same request in sync mode does populate them, so the test proves the gate rather than a broken sink.
- Extend the existing observe assertion that enforced aggregates are zero to cover every field added since — the gap here was that the assertion enumerated a fixed list rather than being exhaustive.
Acceptance criteria
Found by an integration review of the five cache/filter/observe merges. This is new leakage created by #42 × #43 composing — neither PR's own review could see it, because #42's stats sink and #43's observe mode landed independently.
Problem
cmdfiltercallsc.FilterStatswith noModecheck (components/offload/cmdfilter.go:105and theFilterActcall site). So an observe-only run — zero enforced requests,sync_enforced: 0— still reports real filter activity:Those tokens were never saved — nothing was forwarded.
Why this one matters more than the known shared fields
#43's review accepted two deliberately-shared fields:
cg_added_ms_avg(a real measurement, and its reading ~0 is observe's headline result) andllm_calls/llm_*_tokens(real money actually spent). Both are labelled viaobserve_llm_notice, and relabelling real spend aspotential_*would have been a worse lie.These three fields are different: they have no mode label and no
potential_*counterpart, so a consumer cannot tell an enforced saving from a hypothetical one. That directly violates #31's stated requirement — "observe/hypothetical metrics must be namespace-separated and can never be aggregated into enforced savings" — and #43's own claim that enforced aggregates are zero by construction in observe mode.A mis-labelled hypothetical is worse than no number, because it silently inflates the product's headline claim. This is the correctness risk #31 named as its primary one.
Desired behavior
Either:
FilterStatssink onCtx.Mode != ModeObserve, so the enforced fields stay honestly zero; orpotential_*-namespaced counterparts, matching how the rest of the observe payload is separated.The second is more useful (per-filter projections are exactly what an evaluating user wants), but the first is correct and cheap. Either beats the status quo.
Relevant code
components/offload/cmdfilter.go:105and itsFilterActcall site.components/component.go—Ctx.Mode,FilterStats FilterStatsSink.metrics/metrics.go—cmdfilter_families/cmdfilter_filters/cmdfilter_selector_misses, and thepotential_*/projected_*block for the pattern to follow.proxy/modes.go— where observe short-circuits.Testing plan
cmdfilterand assert all three fields are absent or zero (or populated only underpotential_*).Acceptance criteria
/statsfield is populated by an observe-mode run, or it is explicitly labelled as hypothetical./statsstays backward compatible — fields added, never renamed or removed (deploy/harbor/*.pyparses it).