Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/offload/agentdiet.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func (d *AgentDiet) Offload(req *bschemas.BifrostChatRequest, rep *components.Re
if !schema.Rewritable(*msg) {
continue
}
if fk, saved, ok := reapplyFrozen(c, d.Name(), msg); ok {
if fk, saved, ok := reapplyFrozen(c, rep, d.Name(), msg); ok {
rep.TokensBefore += saved // best-effort; the pipeline recomputes exactly
keys = append(keys, fk...)
changed++
Expand Down
2 changes: 1 addition & 1 deletion components/offload/cmdfilter.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (f *Cmdfilter) Offload(req *schemas.BifrostChatRequest, rep *components.Rep
// reapplyFrozen also re-Puts the stashed original for every marker in the
// replacement, so the expand loop keeps working across turns, and it declines
// content the agent has expanded (kept-verbatim).
if fk, _, ok := reapplyFrozen(c, f.Name(), m); ok {
if fk, _, ok := reapplyFrozen(c, rep, f.Name(), m); ok {
changed++
keys = append(keys, fk...)
continue
Expand Down
2 changes: 1 addition & 1 deletion components/offload/collapse.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (cl *Collapse) Offload(req *schemas.BifrostChatRequest, rep *components.Rep
// on purpose — with max_frac set, CtxWindow can resolve differently mid-session
// (model swap, refreshed modelinfo), and a threshold that drifts above this output
// would otherwise flip it collapsed→full inside the cached prefix.
if fk, _, ok := reapplyFrozen(c, cl.Name(), m); ok {
if fk, _, ok := reapplyFrozen(c, rep, cl.Name(), m); ok {
changed++
keys = append(keys, fk...)
continue
Expand Down
2 changes: 1 addition & 1 deletion components/offload/failed_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (fr *FailedRun) Offload(req *schemas.BifrostChatRequest, rep *components.Re
// Reapply a previously-frozen collapse on EVERY turn (cache-stable), regardless
// of the tail boundary — the agent re-sends the original, so we must re-collapse
// it to the same bytes or it reverts to full and churns the cache.
if fk, _, ok := reapplyFrozen(c, fr.Name(), m); ok {
if fk, _, ok := reapplyFrozen(c, rep, fr.Name(), m); ok {
changed++
keys = append(keys, fk...)
continue
Expand Down
5 changes: 3 additions & 2 deletions components/offload/freeze_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,11 @@ func TestFrozenCountersMove(t *testing.T) {
st := store.NewMemory(store.Options{})
c := &components.Ctx{Session: "sCount", Store: st}
msg := tool("some tool output")
reapplyFrozen(c, "mask", &msg) // miss: nothing frozen yet
var rep components.Report
reapplyFrozen(c, &rep, "mask", &msg) // miss: nothing frozen yet
freeze(c, "mask", "some tool output", "short")
msg2 := tool("some tool output")
reapplyFrozen(c, "mask", &msg2) // hit
reapplyFrozen(c, &rep, "mask", &msg2) // hit
h1, m1 := FrozenStats()
if h1 <= h0 || m1 <= m0 {
t.Fatalf("hits/misses must both advance: %d->%d, %d->%d", h0, h1, m0, m1)
Expand Down
2 changes: 1 addition & 1 deletion components/offload/mask.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (m *Mask) Offload(req *bschemas.BifrostChatRequest, rep *components.Report,
// the tail boundary: the agent re-sends the original, so we must re-mask it to the
// same bytes or it reverts full→masked→full and churns the provider KV cache. This
// also skips kept-verbatim content (see reapplyFrozen).
if fk, _, ok := reapplyFrozen(c, m.Name(), msg); ok {
if fk, _, ok := reapplyFrozen(c, rep, m.Name(), msg); ok {
changed++
keys = append(keys, fk...)
continue
Expand Down
2 changes: 1 addition & 1 deletion components/offload/readlifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (rl *ReadLifecycle) Offload(req *bschemas.BifrostChatRequest, rep *componen
// Replay a frozen decision on EVERY turn, at any depth: the agent re-sends the
// original each turn, so not re-offloading it would flip the message
// offloaded→full→offloaded and churn the provider's KV cache.
if fk, _, ok := reapplyFrozen(c, rl.Name(), msg); ok {
if fk, _, ok := reapplyFrozen(c, rep, rl.Name(), msg); ok {
changed++
keys = append(keys, fk...)
continue
Expand Down
2 changes: 1 addition & 1 deletion components/offload/skeleton.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (s *Skeleton) Offload(req *schemas.BifrostChatRequest, rep *components.Repo
// Replay a frozen decision at ANY depth: the agent re-sends the original every
// turn, so not re-eliding it would flip the message skeleton→full→skeleton and
// churn the provider's KV cache. Same contract as mask/failed_run/readlifecycle.
if fk, _, ok := reapplyFrozen(c, s.Name(), m); ok {
if fk, _, ok := reapplyFrozen(c, rep, s.Name(), m); ok {
emitted++
keys = append(keys, fk...)
continue
Expand Down
22 changes: 21 additions & 1 deletion components/offload/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func frozenLost(c *components.Ctx, key string) bool {
// exists and still shrinks it. It also refreshes the expand originals for any markers
// in the replacement (the agent re-sent the full original as m's content), so
// restoration keeps working across turns. Returns the marker keys + whether it acted.
func reapplyFrozen(c *components.Ctx, comp string, m *bschemas.ChatMessage) ([]string, int, bool) {
func reapplyFrozen(c *components.Ctx, rep *components.Report, comp string, m *bschemas.ChatMessage) ([]string, int, bool) {
content := schema.MessageText(*m)
if isKeptVerbatim(c, contentKey(content)) {
return nil, 0, false // agent expanded this; replaying the collapse would loop
Expand All @@ -217,6 +217,26 @@ func reapplyFrozen(c *components.Ctx, comp string, m *bschemas.ChatMessage) ([]s
return nil, 0, false
}
keys := expand.ParseMarkers(rs)
if len(keys) == 0 {
// A REPLAY WITH NO MARKERS IS A DEGRADED-MODE REPLAY, and it has to say so.
//
// Under marker_mode summary/off nothing was stashed, so the frozen replacement carries no
// <<cg:HASH>> to parse and this returns no cache keys — while still shrinking the message.
// components/pipeline.go reverts exactly that combination ("dropped content without
// stashing a cache_key") unless rep.Irreversible says the loss was chosen.
//
// The turn that MADE the decision sets it, through commitMark's non-full branch. Every
// later turn replays it through here and did not, so from turn 2 onward a summary-mode
// offloader had its whole component reverted and the transcript sent verbatim — a
// full-suffix cache write at ~11.5x the read price, on every turn, for the rest of the
// session. Measured on a two-turn mask fixture: turn 1 Irreversible=true, turn 2
// Irreversible=false with the message rewritten and no keys returned.
//
// The blanket flag cannot mask a FULL-mode bug: every freeze() site is downstream of a
// tryMark/commitMark pair, so a full-mode frozen replacement always carries a marker and
// len(keys) == 0 implies a degraded mode.
rep.Irreversible = true
}
for _, k := range keys {
c.Store.Put(k, []byte(content)) // refresh the stashed original for expand
}
Expand Down
86 changes: 86 additions & 0 deletions components/offload/summarymodereplay_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package offload

import (
"context"
"strings"
"testing"

bschemas "github.com/maximhq/bifrost/core/schemas"
"github.com/rossoctl/context-guru/components"
"github.com/rossoctl/context-guru/schema"
"github.com/rossoctl/context-guru/store"
)

// THE DEFECT
//
// Under marker_mode summary/off an offloader takes a deliberate LOSSY drop: nothing is stashed and
// no <<cg:HASH>> is written, so the component returns no cache keys. components/pipeline.go treats
// that combination — the request shrank, no cache keys, not Skipped — as a contract violation and
// REVERTS the component, unless rep.Irreversible says the loss was chosen.
//
// commitMark's non-full branch sets that flag, so the turn that TAKES the decision is fine. Every
// later turn replays the frozen decision through reapplyFrozen, which never set it. So from turn 2
// onward, for the whole session:
//
// turn 1: keys=[] Irreversible=true -> kept
// turn 2: keys=[] Irreversible=false -> REVERTED, transcript sent verbatim
//
// A revert is not merely a lost saving. Earlier turns sent the reduced bytes, so sending the
// original re-writes the provider's whole cached suffix at ~11.5x the read price — and it happens
// on every turn, for every message the component had reduced. The component reports itself as
// working: it acts, it computes a replacement, and the pipeline throws it away afterwards.
//
// Found while auditing replay paths for a related review; the defect is independent of that work.
func TestASummaryModeReplayIsNotRevertedFromTurnTwoOnward(t *testing.T) {
body := strings.Repeat("a line of log output that goes on for a while\n", 60)
st := store.NewMemory(store.Options{})
c, err := newMask([]byte("keep_recent: 0\nmin_tokens: 20\nmarker_mode: summary\n"))
if err != nil {
t.Fatal(err)
}
comp := c.(components.Offload)
ctx := &components.Ctx{Ctx: context.Background(), Session: "s", Store: st, MaxCachedIdx: -1}
req := func() *bschemas.BifrostChatRequest {
m := bschemas.ChatMessage{Role: bschemas.ChatMessageRoleTool}
schema.SetMessageText(&m, body)
return &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{m}}
}

// Turn 1 takes the decision. This turn was never broken — commitMark sets the flag — and it is
// asserted so a fixture that stopped reaching summary mode fails here rather than passing below.
r1 := components.Report{Kind: "offload"}
if _, err := comp.Offload(req(), &r1, ctx); err != nil {
t.Fatal(err)
}
if !r1.Irreversible || len(r1.CacheKeys) != 0 {
t.Fatalf("turn 1: Irreversible=%v keys=%v, want true/none — the fixture is not in a "+
"degraded marker mode, so turn 2 is not the case under test",
r1.Irreversible, r1.CacheKeys)
}

// Turn 2 replays it, through reapplyFrozen.
req2 := req()
r2 := components.Report{Kind: "offload"}
if _, err := comp.Offload(req2, &r2, ctx); err != nil {
t.Fatal(err)
}
if got := schema.MessageText(req2.Input[0]); got == body {
t.Fatalf("turn 2 did not replay the frozen decision, so the revert precondition is not " +
"reachable and this assertion would pass vacuously")
}
if r2.Skipped {
t.Fatal("turn 2 reported Skipped, so the pipeline would not revert it and this test " +
"proves nothing")
}
if len(r2.CacheKeys) != 0 {
t.Fatalf("turn 2 returned cache keys (%v); in summary mode nothing is stashed, so the "+
"fixture is not exercising the degraded path", r2.CacheKeys)
}
if !r2.Irreversible {
t.Error("a replayed summary-mode decision rewrote the message, returned NO cache key and " +
"did not set rep.Irreversible. That is exactly what components/pipeline.go reverts as " +
"\"offload dropped content without stashing a cache_key\", so from turn 2 onward the " +
"whole component is discarded and the transcript goes upstream verbatim — flipping " +
"content earlier turns had already sent reduced, at ~11.5x the read price, every turn")
}
}
Loading