fix(accountant): Harden stats cache key, state file and panics (#916 #917 #918 #920)

cmd/context-guru-proxy/dashemitter_waitcause_test.go

@@ -1,7 +1,9 @@
 package main
 
 import (
+	"context"
 	"fmt"
+	"github.com/rossoctl/context-guru/internal/modelinfo"
 	"path/filepath"
 	"testing"
 	"time"
@@ -117,3 +119,19 @@ func waitForRow(t *testing.T, rec *dash.Recorder) (*dash.Page, error) {
 		time.Sleep(5 * time.Millisecond)
 	}
 }
+
+// A panic while a ping row is built must not end the process: the keeper calls KeepAlivePing from its own goroutine
+// (issue #918). A nil Pricer inside the interface value panics in Price.
+type panicPricer struct{ modelinfo.Pricer }
+
+func (panicPricer) Price(context.Context, string) (modelinfo.Price, bool) { panic("price lookup") }
+
+func TestDashEmitterKeepAlivePingSurvivesAPanic(t *testing.T) {
+	rec, err := dash.NewRecorder(dash.Options{DBPath: filepath.Join(t.TempDir(), "dash.db"), BatchSize: 1, FlushInterval: time.Millisecond})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer rec.Close()
+	em := &dashEmitter{rec: rec, prices: panicPricer{}}
+	em.KeepAlivePing(components.KeepAliveReport{Tenant: "t1", Session: "s1", Model: "m", Status: 200, CacheRead: 10, TS: time.Now().UnixMilli()})
+}

cmd/context-guru-proxy/main.go

@@ -973,6 +973,13 @@ func (d *dashEmitter) KeepAlivePing(r components.KeepAliveReport) {
 	if d.rec == nil {
 		return
 	}
+	// The keeper calls this from its own goroutine. A panic here (a price lookup, the row build) would end the
+	// whole process, so log it and carry on without this row (issue #918).
+	defer func() {
+		if p := recover(); p != nil {
+			slog.Error("context-guru: keep-alive ping row panicked; the row is dropped", "panic", p)
+		}
+	}()
 	var price modelinfo.Price
 	priced := false
 	if d.prices != nil && r.Model != "" {

dash/api.go

@@ -531,21 +531,11 @@ func (a *API) refreshJSON(c *jsonCache, key string, compute func(db *DB) ([]byte
 // cacheKey scopes a cache entry to the actual caller, not just the URL: a manager and a
 // tenant hitting the same query string must never share a cached body.
 //
-// since/until are rounded to the cache TTL bucket rather than compared verbatim. The UI's
-// relative ranges ("last 24h") re-stamp since/until with Date.now() on every single call —
-// so the raw query string is different every time and this cache would otherwise never hit
-// for the single most common view on the page.
+// since and until stay exact in the key. An earlier version rounded them down to the TTL bucket,
+// so a narrower window got the wider window's cached totals (issue #916). The UI keeps its relative
+// ranges cheap by rounding `now` itself (resolveTime in ui/app.js), so one repaint burst shares a key.
 func cacheKey(p Principal, r *http.Request) string {
-	q := r.URL.Query()
-	bucket := dashCacheTTL.Milliseconds()
-	for _, param := range []string{"since", "until"} {
-		if v := q.Get(param); v != "" {
-			if ms, err := strconv.ParseInt(v, 10, 64); err == nil {
-				q.Set(param, strconv.FormatInt(ms/bucket*bucket, 10))
-			}
-		}
-	}
-	return p.TenantID + "\x00" + strconv.FormatBool(p.Manager) + "\x00" + q.Encode()
+	return p.TenantID + "\x00" + strconv.FormatBool(p.Manager) + "\x00" + r.URL.Query().Encode()
 }
 
 // NewAPI builds the HTTP surface for a recorder. Malformed CIDRs are skipped with

dash/componentscache_test.go

@@ -101,3 +101,19 @@ func TestComponentsCached(t *testing.T) {
 		t.Errorf("past dashCacheStale: got %d component rows, want 3 (recomputed)", len(rows))
 	}
 }
+
+// A narrower window must not share a key with a wider one (issue #916). The key used to round
+// since/until down to 30 s, so since=T+1s and since=T hit one cached body.
+func TestCacheKeyKeepsSinceAndUntilExact(t *testing.T) {
+	p := Principal{Manager: true}
+	k := func(u string) string { return cacheKey(p, httptest.NewRequest(http.MethodGet, u, nil)) }
+	if k("/api/stats?since=1700000001000") == k("/api/stats?since=1700000000000") {
+		t.Error("since 1 s apart gives one key; a narrower window would get the wider window's totals")
+	}
+	if k("/api/stats?since=1&until=1700000001000") == k("/api/stats?since=1&until=1700000000000") {
+		t.Error("until 1 s apart gives one key")
+	}
+	if k("/api/stats?since=5") != k("/api/stats?since=5") {
+		t.Error("the same query must give the same key")
+	}
+}

dash/firstrequest_test.go

@@ -0,0 +1,35 @@
+package dash
+
+import (
+	"testing"
+
+	"github.com/rossoctl/context-guru/internal/modelinfo"
+)
+
+// A first request of a session with a realistic cold write books its cost when the price is known
+// (issue #920). The reported $0 row had no price: the proxy's price map had not loaded yet, so Price
+// was called with accountingComplete true and p.Zero() true. tokens_before=49 plays no part.
+func TestFirstRequestBooksCostWhenThePriceIsKnown(t *testing.T) {
+	p := modelinfo.Price{Input: 3e-06, Output: 1.5e-05, CacheRead: 3e-07, CacheWrite: 3.75e-06}
+	for _, tokensBefore := range []int{49, 29650} {
+		e := &Event{Model: "claude-sonnet-4-5", Status: 200, TokensBefore: tokensBefore, CacheWrite: 29650, OutputTokens: 50}
+		e.PriceMode(p, true)
+		if e.TokenAccounting != AccountingComplete {
+			t.Errorf("tokens_before=%d: token_accounting = %q, want complete", tokensBefore, e.TokenAccounting)
+		}
+		want := 29650*3.75e-06 + 50*1.5e-05
+		if e.CostUSD < want*0.999 || e.CostUSD > want*1.001 {
+			t.Errorf("tokens_before=%d: cost = %v, want %v", tokensBefore, e.CostUSD, want)
+		}
+	}
+}
+
+// Without a price the row is partial at $0. That is the honest answer for an unknown price. The fix for
+// #920 is to make the price known before the first request, not to guess one here.
+func TestFirstRequestWithoutAPriceIsPartial(t *testing.T) {
+	e := &Event{Model: "claude-sonnet-4-5", Status: 200, TokensBefore: 49, CacheWrite: 29650}
+	e.PriceMode(modelinfo.Price{}, true)
+	if e.TokenAccounting != AccountingPartial || e.CostUSD != 0 {
+		t.Errorf("got %q cost %v, want partial and 0", e.TokenAccounting, e.CostUSD)
+	}
+}

dash/ui/app.js

@@ -274,7 +274,9 @@ function resolveTime(v, nowMs) {
   if (v === 'now') return nowMs;
   if (typeof v === 'string') {
     const m = /^now-(\d+)([smhdw])$/.exec(v);
-    if (m) return nowMs - Number(m[1]) * RANGE_UNIT_MS[m[2]];
+    // Round `now` down to 30 s so a relative range gives the same since for 30 s and the server cache can hit.
+    // The server keeps since exact (issue #916), so this rounding is the UI's own choice.
+    if (m) return Math.floor(nowMs / 30000) * 30000 - Number(m[1]) * RANGE_UNIT_MS[m[2]];
     const n = Number(v);
     return Number.isFinite(n) && n > 0 ? n : 0;
   }

docs/dashboard.md

@@ -167,6 +167,10 @@ Limits:
 - The library keeps each thread's state in memory. The proxy saves it to `<dashboard-db>.accountant-state.json`
   every minute and at a clean shutdown, and restores it at start, so a restart does not drop the active
   changes of a live session. The file holds hashes, sizes and ids, no message text, and is mode 0600.
+  The file carries a proxy version (`cgStateVersion`) around the library snapshot. A file with another version is
+  ignored, because the hash of a body changed between versions. The save syncs the file before the rename, skips the
+  write when nothing changed, and the start removes temp files a crash left. The hash leaves out every `cache_control`
+  mark, nested ones too.
   A file older than one hour is ignored (no cache entry lives longer), and so is a missing or unreadable
   one: the proxy then starts empty, as before. A crash (kill -9) loses up to one minute of state. A
   request that follows a lost state is priced as a first request and carries the estimate flag. The

docs/data-dependence.md

@@ -226,7 +226,7 @@ One row for each registered component. The test `components/all/datadep_test.go`
 | Thread tracker | metadata | 🟡 Works, metadata | 🟡 Works, metadata | 🟡 Works, metadata | It tells the main agent from its subagents. It uses hashes to check if a new request continues an earlier one. |
 | Session tracker | metadata | 🟡 Works, metadata | 🟡 Works, metadata | 🟡 Works, metadata | It needs a stable label for each conversation. When the client sends none, it hashes the start of the conversation. |
 | Cache tracker | metadata | 🟡 Works, metadata | 🟡 Works, metadata | 🟡 Works, metadata | It must know how much of the conversation the provider already cached, so components never edit that part. After a restart it asks the dashboard (when on) for the time each thread's cached prefix was last refreshed — by its own last request or a keep-alive ping that read the cache, under either session id the conversation uses (the row stores the content-derived alias id, a hash like the session id itself); timestamps and ids, metadata only — and treats a thread refreshed inside the request's cache lifetime as cached, not as new, until that thread's first request. Without the dashboard — and for a tenant running observe mode, whose rows carry no session id — it uses the proxy's start time instead. |
-| Accountant savings (per-component figures from the accountant library) | metadata | ❌ Off. It needs the dashboard, because the figure is attached to a dashboard event. | 🟡 Works, metadata | 🟡 Works, metadata | To price a saving it must know which part of the conversation a rewrite changed and whether the provider had that part cached. It reads each request once and keeps only a hash and a size for each part, plus the cache breakpoints. The text is dropped as soon as it is hashed, and the hashes are held in memory, one set per conversation thread, plus each thread's last set for the sibling check (SharedWarm). So a restart does not lose them, it also saves them to one file beside the dashboard database (`<dashboard-db>.accountant-state.json`, readable by its owner only), every minute and when it stops, and loads them again when it starts. The file holds hashes, sizes, positions, and the ids and names the state needs (session and thread ids, tool and component names, model names, times). It holds no message text. It is ignored when it is more than one hour old. It is not written when there is no dashboard. It reads the tools and system sections the same way, as names, hashes and sizes, to report the envelope changes. The result it writes to the dashboard is dollar amounts for each component, with their split by tier. No level changes what it does. |
+| Accountant savings (per-component figures from the accountant library) | metadata | ❌ Off. It needs the dashboard, because the figure is attached to a dashboard event. | 🟡 Works, metadata | 🟡 Works, metadata | To price a saving it must know which part of the conversation a rewrite changed and whether the provider had that part cached. It reads each request once and keeps only a hash and a size for each part, plus the cache breakpoints. The text is dropped as soon as it is hashed, and the hashes are held in memory, one set per conversation thread, plus each thread's last set for the sibling check (SharedWarm). So a restart does not lose them, it also saves them to one file beside the dashboard database (`<dashboard-db>.accountant-state.json`, readable by its owner only), every minute and when it stops, and loads them again when it starts. The file holds hashes, sizes, positions, and the ids and names the state needs (session and thread ids, tool and component names, model names, times). It holds no message text. It also holds one proxy version number, and it is ignored when that number differs or when it is more than one hour old. It is not written when there is no dashboard. It reads the tools and system sections the same way, as names, hashes and sizes, to report the envelope changes. The result it writes to the dashboard is dollar amounts for each component, with their split by tier. No level changes what it does. |
 | Wait-cause and lifecycle signals | metadata | ❌ Off (it needs the dashboard) | 🟡 Works, metadata | 🟡 Works, metadata | To tell why the agent paused, it reads the request once. It keeps only counts and ids. Since #857 it also reads the **tool name** of the last tool call, to tell that an agent has finished (`done`). The keep-alive then sends no pings for that thread. A tool name is metadata, and the rule works at every storage level. Since #858 the keep-alive also uses the **wait cause** (from the stop reason, the tool names and the open-task counts), whether the thread id starts with `a:`, the token count of the prefix and the time of day, to pick the ping count, to ignore a strategy's time windows for a wait that is not for a person, and to skip the sweep question during a tool wait. It also keeps one flag per thread: "has a tool call been seen". All of it is metadata, and it works at every storage level. |
 | Costly settings check (`/context-guru:save-more settings`, `GET /api/adapt/settings`) | metadata | ❌ Off (the cost needs the dashboard). The plugin still finds the settings and says "cost not measured yet". | 🟡 Works, metadata | 🟡 Works, metadata | It counts and prices dashboard rows by request shape: model name, tool count, `max_tokens` and the stream flag. One check also uses the upstream status (400) of a Haiku row in the same session. It reads no request text and keeps nothing new. The plugin reads only the named keys of the user's own settings files (the betas setting, the recap setting, `ANTHROPIC_SMALL_FAST_MODEL` and `ANTHROPIC_DEFAULT_HAIKU_MODEL`) on the user's machine, and sends none of them to the proxy (#615). |
 | Beep (`beep.sh`, the `beep` plugin option) | none | 🟡 Works, no data | 🟡 Works, no data | 🟡 Works, no data | It plays a sound when Claude finishes or waits for you. It reads no stored data and never reads the hook payload. It sends nothing to the proxy or anywhere else. The level changes nothing. |

proxy/accountant_ping.go

@@ -1,6 +1,7 @@
 package proxy
 
 import (
+	"log/slog"
 	"sync"
 	"time"
 
@@ -101,15 +102,30 @@ func pingStart(r components.KeepAliveReport) time.Time {
 // is already booked on its own row as ManualSavedUSD, from the user's estimate. Storing the facts too
 // would let the next real request credit the same write again as a library rescue, and the total would
 // count it twice (context-guru#768 B3: $0.0200 booked, about $0.0096 true).
-func PingSavings(r components.KeepAliveReport, st *PingStore) (accountant.Result, bool) {
+//
+// A panic in the library is recovered and logged, and the ping is reported as not converted. The keeper
+// runs this in its own goroutine, so an unrecovered panic would end the process (issue #918).
+func PingSavings(r components.KeepAliveReport, st *PingStore) (res accountant.Result, ok bool) {
+	defer func() {
+		if p := recover(); p != nil {
+			slog.Error("accountant: ping pricing panicked; the ping is recorded with no saving", "panic", p)
+			res, ok = accountant.Result{}, false
+		}
+	}()
 	if r.Window.IsZero() || r.Status < 200 || r.Status > 299 ||
 		r.FreshInput+r.CacheRead+r.CacheWrite+r.CacheWrite1h+r.Output == 0 {
 		return accountant.Result{}, false
 	}
-	ops, facts := accountant.PingOps(pingFromReport(r), accountant.Billing{Mode: accountant.BillingAPI})
-	res := accountant.ComputeStandalone(ops)
+	ops, facts := pingOps(pingFromReport(r), accountant.Billing{Mode: accountant.BillingAPI})
+	res = accountantStandalone(ops)
 	if !r.Manual {
 		st.put(accPingKey(r.Session, accThreadID(r.Thread)), facts)
 	}
 	return res, true
 }
+
+// pingOps and accountantStandalone are the two library calls PingSavings makes. Tests replace them to make them panic.
+var (
+	pingOps              = accountant.PingOps
+	accountantStandalone = accountant.ComputeStandalone
+)

proxy/accountant_ping_test.go

@@ -370,3 +370,26 @@ func TestTheEntryTTLFollowsTheBilledTier(t *testing.T) {
 		t.Errorf("wrote nothing, gap 20m: phase = %q, want hot (no tier billed: the asked 1h stands)", got)
 	}
 }
+
+// A panic in the library while a ping is priced is recovered: the keeper's goroutine must live (issue #918).
+func TestPingSavingsRecoversFromALibraryPanic(t *testing.T) {
+	origOps, origStd := pingOps, accountantStandalone
+	defer func() { pingOps, accountantStandalone = origOps, origStd }()
+	pings := NewPingStore()
+
+	pingOps = func(accountant.Ping, accountant.Billing) (accountant.RequestOps, accountant.PingFacts) {
+		panic("boom in PingOps")
+	}
+	if _, ok := PingSavings(okReport(), pings); ok {
+		t.Error("a panicking PingOps reported a figure")
+	}
+	pingOps = origOps
+	accountantStandalone = func(accountant.RequestOps) accountant.Result { panic("boom in ComputeStandalone") }
+	if _, ok := PingSavings(okReport(), pings); ok {
+		t.Error("a panicking ComputeStandalone reported a figure")
+	}
+	accountantStandalone = origStd
+	if _, ok := PingSavings(okReport(), pings); !ok {
+		t.Error("after the panics a normal ping was not priced")
+	}
+}

proxy/accountant_savings.go

@@ -17,6 +17,8 @@ type savingsTracker struct {
 	clients sync.Map // *accountant.Request -> clientHash
 	// onEnd, when set, is called with each record after EndRequest. Tests only.
 	onEnd func(*accountant.Request)
+	// written is the last state file's bytes, for save to skip an unchanged write.
+	written lastSaved
 }
 
 // clientHash is what begin built from the client body, kept until end so a request that was forwarded unchanged does

proxy/accountant_savings_body.go

@@ -50,32 +50,69 @@ func accBodies(raw []byte) accountant.Body {
 	return accountant.Body{Start: built.Start, Pos: built.Pos}
 }
 
-// withoutCacheMarks is one element's JSON without any cache_control: the element's own, and the ones on
-// its content blocks. An element with none is returned as it is, byte for byte, so the common case costs
-// no allocation. The result is only hashed. It is never sent anywhere.
+// withoutCacheMarks is one element's JSON without any cache_control: the element's own, the ones on its content
+// blocks, and the ones nested deeper, such as a mark inside a tool_result block's own content array (issue #917).
+// An element with none is returned as it is, byte for byte, so the common case costs no allocation. The result is
+// only hashed. It is never sent anywhere.
 func withoutCacheMarks(raw string) []byte {
 	if !strings.Contains(raw, "cache_control") {
 		return []byte(raw)
 	}
 	out := raw
-	if gjson.Get(out, "cache_control").Exists() {
-		if next, err := sjson.Delete(out, "cache_control"); err == nil {
-			out = next
+	// Each delete removes one mark and the walk starts again, so the paths stay right after the edit. A body has
+	// at most four marks on the wire, so the loop is short.
+	for n := 0; n < 64; n++ {
+		path := findCacheMark(gjson.Parse(out), "")
+		if path == "" {
+			break
 		}
+		next, err := sjson.Delete(out, path)
+		if err != nil {
+			break
+		}
+		out = next
+	}
+	return []byte(out)
+}
+
+// findCacheMark returns the gjson path of the first cache_control key under v, or "" when there is none. It looks
+// at objects and arrays only, and does not look inside a string.
+func findCacheMark(v gjson.Result, prefix string) string {
+	var found string
+	join := func(k string) string {
+		if prefix == "" {
+			return k
+		}
+		return prefix + "." + k
 	}
-	if content := gjson.Get(out, "content"); content.IsArray() {
+	switch {
+	case v.IsObject():
+		v.ForEach(func(k, c gjson.Result) bool {
+			if k.String() == "cache_control" {
+				found = join(strings.NewReplacer(".", "\\.", "*", "\\*", "?", "\\?").Replace(k.String()))
+				return false
+			}
+			if c.IsObject() || c.IsArray() {
+				ek := strings.NewReplacer(".", "\\.", "*", "\\*", "?", "\\?", "|", "\\|", "#", "\\#", "@", "\\@").Replace(k.String())
+				if found = findCacheMark(c, join(ek)); found != "" {
+					return false
+				}
+			}
+			return true
+		})
+	case v.IsArray():
 		i := 0
-		content.ForEach(func(_, blk gjson.Result) bool {
-			if blk.Get("cache_control").Exists() {
-				if next, err := sjson.Delete(out, "content."+strconv.Itoa(i)+".cache_control"); err == nil {
-					out = next
+		v.ForEach(func(_, c gjson.Result) bool {
+			if c.IsObject() || c.IsArray() {
+				if found = findCacheMark(c, join(strconv.Itoa(i))); found != "" {
+					return false
 				}
 			}
 			i++
 			return true
 		})
 	}
-	return []byte(out)
+	return found
 }
 
 // accBreakpoints lists the cache_control marks, as positions in the same space as accBodies, one per