From 7a21db00983745cd3c933b70efddab2279e6e4b4 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Thu, 11 Jun 2026 23:47:21 +0200 Subject: [PATCH 01/12] indexer, mcp: resolve unprefixed paths against the lone tracked repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-repo mode indexes nodes and file paths without a repo prefix while registering the repo's metadata under its real prefix, so RepoRoot(""), MultiIndexer.ResolveFilePath, and the MCP path resolvers refused every node the single-repo indexer minted: get_symbol_source errored with 'could not resolve repo root (repo_prefix="")', smart_context silently dropped its embedded source, and bare-relative read paths were rejected — and because savings recording sits behind those source reads, the token-savings ledger never saw an observation on single-repo daemons. With exactly one tracked repo the empty prefix is unambiguous: resolve it to the lone repo's root. Two or more tracked repos keep the ambiguity miss/error. --- internal/indexer/multi.go | 31 ++++- .../indexer/multi_singlerepo_resolve_test.go | 107 ++++++++++++++++ internal/mcp/tools_fileops.go | 12 +- internal/mcp/tools_fileops_singlerepo_test.go | 115 ++++++++++++++++++ 4 files changed, 262 insertions(+), 3 deletions(-) create mode 100644 internal/indexer/multi_singlerepo_resolve_test.go create mode 100644 internal/mcp/tools_fileops_singlerepo_test.go diff --git a/internal/indexer/multi.go b/internal/indexer/multi.go index 392b0443..e6a2dbae 100644 --- a/internal/indexer/multi.go +++ b/internal/indexer/multi.go @@ -1677,6 +1677,11 @@ func (mi *MultiIndexer) ResolveFilePath(prefixedPath string) string { } } if bestPrefix == "" { + // Single-repo mode mints unprefixed graph paths; resolve them + // against the lone registered repo instead of failing. + if meta := mi.loneRepoLocked(); meta != nil && meta.RootPath != "" { + return filepath.Join(meta.RootPath, prefixedPath) + } return "" } return filepath.Join(bestRoot, strings.TrimPrefix(prefixedPath, bestPrefix+"/")) @@ -1699,12 +1704,22 @@ func (mi *MultiIndexer) RepoPrefixes() []string { // RepoRoot returns the local filesystem root for the given repo prefix. // ok is true only when the prefix is registered AND meta.RootPath is non-empty. // Caller is responsible for joining repo-relative file paths against the root. +// +// The empty prefix resolves to the lone registered repo when exactly one is +// tracked: single-repo mode indexes nodes without a repo prefix (see +// indexSingleRepo) while registering its metadata under the repo's real +// prefix, so every node the single-repo indexer mints carries RepoPrefix="" +// — refusing the empty prefix would orphan all of them. With two or more +// repos the empty prefix is ambiguous and stays a miss. func (mi *MultiIndexer) RepoRoot(repoPrefix string) (string, bool) { + mi.mu.RLock() + defer mi.mu.RUnlock() if repoPrefix == "" { + if meta := mi.loneRepoLocked(); meta != nil && meta.RootPath != "" { + return meta.RootPath, true + } return "", false } - mi.mu.RLock() - defer mi.mu.RUnlock() meta, ok := mi.repos[repoPrefix] if !ok || meta == nil || meta.RootPath == "" { return "", false @@ -1712,6 +1727,18 @@ func (mi *MultiIndexer) RepoRoot(repoPrefix string) (string, bool) { return meta.RootPath, true } +// loneRepoLocked returns the metadata of the only registered repo when +// exactly one repo is tracked, else nil. Caller must hold mi.mu. +func (mi *MultiIndexer) loneRepoLocked() *RepoMetadata { + if len(mi.repos) != 1 { + return nil + } + for _, meta := range mi.repos { + return meta + } + return nil +} + // LinkedWorktreeRoots returns the on-disk roots of every tracked linked // git worktree that shares its .git common directory with the checkout // at mainRepoPath — i.e. the worktree siblings of that main repo. The diff --git a/internal/indexer/multi_singlerepo_resolve_test.go b/internal/indexer/multi_singlerepo_resolve_test.go new file mode 100644 index 00000000..b51dd5ff --- /dev/null +++ b/internal/indexer/multi_singlerepo_resolve_test.go @@ -0,0 +1,107 @@ +package indexer + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/search" +) + +func indexSingleRepoForTest(t *testing.T) (*MultiIndexer, string) { + t.Helper() + dir := setupRepoDir(t, "myrepo") + + tmpCfg := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{ + Repos: []config.RepoEntry{{Path: dir, Name: "myrepo"}}, + } + gc.SetConfigPath(tmpCfg) + require.NoError(t, gc.Save()) + + cm, err := config.NewConfigManager(tmpCfg) + require.NoError(t, err) + + g := graph.New() + mi := NewMultiIndexer(g, newTestRegistry(), search.NewBM25(), cm, zap.NewNop()) + _, err = mi.IndexAll() + require.NoError(t, err) + return mi, dir +} + +func indexTwoReposForTest(t *testing.T) (*MultiIndexer, string, string) { + t.Helper() + repoA := setupRepoDir(t, "repo-a") + repoB := setupRepoDir(t, "repo-b") + + tmpCfg := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{ + Repos: []config.RepoEntry{ + {Path: repoA, Name: "repo-a"}, + {Path: repoB, Name: "repo-b"}, + }, + } + gc.SetConfigPath(tmpCfg) + require.NoError(t, gc.Save()) + + cm, err := config.NewConfigManager(tmpCfg) + require.NoError(t, err) + + g := graph.New() + mi := NewMultiIndexer(g, newTestRegistry(), search.NewBM25(), cm, zap.NewNop()) + _, err = mi.IndexAll() + require.NoError(t, err) + return mi, repoA, repoB +} + +// Single-repo mode indexes nodes and file paths without a repo prefix +// while registering the repo's metadata under its real prefix. The empty +// prefix must therefore resolve to the lone repo — otherwise every node +// the single-repo indexer mints is unresolvable (no source reads, no +// savings recording, no editing by graph path). +func TestMultiIndexer_RepoRoot_EmptyPrefixResolvesLoneRepo(t *testing.T) { + mi, dir := indexSingleRepoForTest(t) + + root, ok := mi.RepoRoot("") + require.True(t, ok, "empty prefix must resolve when exactly one repo is tracked") + assert.Equal(t, dir, root) + + // The real prefix keeps working. + root, ok = mi.RepoRoot("myrepo") + require.True(t, ok) + assert.Equal(t, dir, root) + + // Unknown prefixes still miss. + _, ok = mi.RepoRoot("nope") + assert.False(t, ok) +} + +func TestMultiIndexer_RepoRoot_EmptyPrefixAmbiguousInMultiRepo(t *testing.T) { + mi, _, _ := indexTwoReposForTest(t) + + _, ok := mi.RepoRoot("") + assert.False(t, ok, "empty prefix is ambiguous with two tracked repos") +} + +func TestMultiIndexer_ResolveFilePath_UnprefixedSingleRepo(t *testing.T) { + mi, dir := indexSingleRepoForTest(t) + + // Unprefixed path (the form single-repo nodes carry) anchors to the + // lone repo root. + assert.Equal(t, filepath.Join(dir, "main.go"), mi.ResolveFilePath("main.go")) + + // The prefixed form keeps working too. + assert.Equal(t, filepath.Join(dir, "main.go"), mi.ResolveFilePath("myrepo/main.go")) +} + +func TestMultiIndexer_ResolveFilePath_UnprefixedMultiRepoStaysEmpty(t *testing.T) { + mi, _, _ := indexTwoReposForTest(t) + + assert.Empty(t, mi.ResolveFilePath("main.go"), + "bare path with two tracked repos is ambiguous and must not resolve") +} diff --git a/internal/mcp/tools_fileops.go b/internal/mcp/tools_fileops.go index 8265e411..3f77b943 100644 --- a/internal/mcp/tools_fileops.go +++ b/internal/mcp/tools_fileops.go @@ -62,9 +62,19 @@ func (s *Server) resolveFilePath(rawPath string) (absPath, relPath string, err e if s.multiIndexer != nil { // Multi-repo mode requires a repo-prefixed path. Bare-relative // paths are ambiguous; refuse rather than fall through to the - // daemon process CWD. + // daemon process CWD. With exactly one tracked repo there is no + // ambiguity — single-repo mode indexes unprefixed paths, so a + // bare-relative path anchors to the lone repo's root. prefix := matchedRepoPrefix(s.multiIndexer, rawPath) if prefix == "" { + if root, ok := s.multiIndexer.RepoRoot(""); ok { + abs := filepath.Clean(filepath.Join(root, rawPath)) + if !pathContainedIn(abs, root) { + return "", "", fmt.Errorf("%w: %q resolves to %q, outside repo root %q", errPathEscape, rawPath, abs, root) + } + abs = worktreeRootedPath(abs, root, s.multiIndexer) + return abs, rawPath, nil + } prefixes := s.multiIndexer.RepoPrefixes() return "", "", fmt.Errorf("%w: path %q does not start with a known repo prefix; expected one of: %s/, or an absolute path", errPathUnresolved, rawPath, strings.Join(prefixes, "/, ")) diff --git a/internal/mcp/tools_fileops_singlerepo_test.go b/internal/mcp/tools_fileops_singlerepo_test.go new file mode 100644 index 00000000..3d05aea8 --- /dev/null +++ b/internal/mcp/tools_fileops_singlerepo_test.go @@ -0,0 +1,115 @@ +package mcp + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/indexer" + "github.com/zzet/gortex/internal/parser" + "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/query" + "github.com/zzet/gortex/internal/search" +) + +// newSingleRepoServer indexes one repo through the MultiIndexer — the +// daemon/serverstack shape where multiIndexer is always non-nil — and +// returns the server plus the repo root. Nodes carry RepoPrefix="" +// (single-repo mode indexes without prefixing). +func newSingleRepoServer(t *testing.T) (*Server, *graph.Graph, string) { + t.Helper() + dir := setupMiniRepo(t, "myrepo") + + tmpCfg := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{ + Repos: []config.RepoEntry{{Path: dir, Name: "myrepo"}}, + } + gc.SetConfigPath(tmpCfg) + require.NoError(t, gc.Save()) + cm, err := config.NewConfigManager(tmpCfg) + require.NoError(t, err) + + reg := parser.NewRegistry() + reg.Register(languages.NewGoExtractor()) + g := graph.New() + mi := indexer.NewMultiIndexer(g, reg, search.NewBM25(), cm, zap.NewNop()) + _, err = mi.IndexAll() + require.NoError(t, err) + + srv := NewServer(query.NewEngine(g), g, nil, nil, zap.NewNop(), nil, MultiRepoOptions{ + ConfigManager: cm, + MultiIndexer: mi, + }) + return srv, g, dir +} + +// A single tracked repo mints unprefixed nodes; resolving their source +// path must anchor to the lone repo root rather than erroring with +// `could not resolve repo root (repo_prefix="")`. This is the gate in +// front of every source read — and of savings recording. +func TestResolveNodePath_SingleRepoUnprefixedNode(t *testing.T) { + srv, g, dir := newSingleRepoServer(t) + + node := g.GetNode("main.go::Hello") + require.NotNil(t, node, "single-repo node IDs are unprefixed") + require.Empty(t, node.RepoPrefix) + + abs, err := srv.resolveNodePath(node) + require.NoError(t, err) + assert.Equal(t, filepath.Join(dir, "main.go"), abs) +} + +func TestResolveFilePath_SingleRepoBareRelative(t *testing.T) { + srv, _, dir := newSingleRepoServer(t) + + abs, rel, err := srv.resolveFilePath("main.go") + require.NoError(t, err) + assert.Equal(t, filepath.Join(dir, "main.go"), abs) + assert.Equal(t, "main.go", rel) + + // Containment still enforced: escaping the lone root is refused. + _, _, err = srv.resolveFilePath("../escape.go") + require.Error(t, err) + + // The prefixed form keeps working. + abs, _, err = srv.resolveFilePath("myrepo/main.go") + require.NoError(t, err) + assert.Equal(t, filepath.Join(dir, "main.go"), abs) +} + +func TestResolveFilePath_MultiRepoBareRelativeStillAmbiguous(t *testing.T) { + repoA := setupMiniRepo(t, "repo-a") + repoB := setupMiniRepo(t, "repo-b") + + tmpCfg := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{ + Repos: []config.RepoEntry{ + {Path: repoA, Name: "repo-a"}, + {Path: repoB, Name: "repo-b"}, + }, + } + gc.SetConfigPath(tmpCfg) + require.NoError(t, gc.Save()) + cm, err := config.NewConfigManager(tmpCfg) + require.NoError(t, err) + + reg := parser.NewRegistry() + reg.Register(languages.NewGoExtractor()) + g := graph.New() + mi := indexer.NewMultiIndexer(g, reg, search.NewBM25(), cm, zap.NewNop()) + _, err = mi.IndexAll() + require.NoError(t, err) + + srv := NewServer(query.NewEngine(g), g, nil, nil, zap.NewNop(), nil, MultiRepoOptions{ + ConfigManager: cm, + MultiIndexer: mi, + }) + + _, _, err = srv.resolveFilePath("main.go") + require.Error(t, err, "bare-relative path with two tracked repos stays ambiguous") +} From fa61b854a4e5a5aab516006a6c755097ae90a26b Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Thu, 11 Jun 2026 23:56:04 +0200 Subject: [PATCH 02/12] mcp: record token savings on the read-family tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The savings ledger only saw get_symbol_source, batch_symbols (include_source:true), and smart_context's embedded sources — while read_file, get_file_summary, and get_editing_context, the tools agents actually lean on as Read replacements, recorded nothing. A session served entirely by those tools left the savings dashboard empty no matter how much it saved. Each of the three now books a server-side observation: read_file with exact counts of the content it returned vs the original bytes (an uncompressed read books returned == baseline, counting the call without inventing savings), and the summary-shaped tools against the on-disk size of the file their response stands in for, using the payload itself to calibrate the chars-per-token ratio. A synthesized attribution node carries the repo prefix and language so per-repo/per-language buckets stay correct, including the lone-repo fallback for single-repo mode. --- internal/mcp/tools_coding.go | 11 ++++++ internal/mcp/tools_core.go | 14 +++++++ internal/mcp/tools_fileops.go | 48 +++++++++++++++++++++++ internal/mcp/tools_savings_record_test.go | 43 ++++++++++++++++++++ 4 files changed, 116 insertions(+) create mode 100644 internal/mcp/tools_savings_record_test.go diff --git a/internal/mcp/tools_coding.go b/internal/mcp/tools_coding.go index 5ac8f445..1e7f03b9 100644 --- a/internal/mcp/tools_coding.go +++ b/internal/mcp/tools_coding.go @@ -3,6 +3,7 @@ package mcp import ( "bufio" "context" + "encoding/json" "fmt" "os" "path/filepath" @@ -532,6 +533,16 @@ func (s *Server) handleGetEditingContext(ctx context.Context, req mcp.CallToolRe return notModifiedResult(etag), nil } + // Server-side accounting only — an editing-context bundle stands in + // for reading the whole file before editing it. + ctxLang := "" + if fileNodeForScope != nil { + ctxLang = fileNodeForScope.Language + } + if payload, merr := json.Marshal(out); merr == nil { + s.recordFileBaselineSavings(ctx, "get_editing_context", fp, ctxLang, string(payload)+sourceCompressed) + } + // Omission notes: flag vendored/generated provenance and body // compression so the model doesn't over-trust the payload. omissions := pathOmissions(fp) diff --git a/internal/mcp/tools_core.go b/internal/mcp/tools_core.go index 9541e076..04914293 100644 --- a/internal/mcp/tools_core.go +++ b/internal/mcp/tools_core.go @@ -1872,6 +1872,20 @@ func (s *Server) handleGetFileSummary(ctx context.Context, req mcp.CallToolReque return mcp.NewToolResultError("no symbols found for file: " + fp), nil } + // Server-side accounting only — a file summary stands in for + // reading the whole file. The compact rendering doubles as the + // payload sample for every output format, which makes the recorded + // `returned` an upper bound for gcx and a fair count for + // compact/json. + summaryLang := "" + for _, n := range sg.Nodes { + if n != nil && n.Language != "" { + summaryLang = n.Language + break + } + } + s.recordFileBaselineSavings(ctx, "get_file_summary", fp, summaryLang, compactSubGraph(sg)) + if isCompact(req) { return mcp.NewToolResultText(compactSubGraph(sg)), nil } diff --git a/internal/mcp/tools_fileops.go b/internal/mcp/tools_fileops.go index 3f77b943..eead8ef9 100644 --- a/internal/mcp/tools_fileops.go +++ b/internal/mcp/tools_fileops.go @@ -14,6 +14,7 @@ import ( "github.com/zzet/gortex/internal/elide" "github.com/zzet/gortex/internal/graph" "github.com/zzet/gortex/internal/indexer" + "github.com/zzet/gortex/internal/tokens" ) // errPathUnresolved is returned when a relative path cannot be anchored to any @@ -339,6 +340,43 @@ func (s *Server) resolveGraphPath(graphPath string) (string, error) { return "", fmt.Errorf("%w: path=%q", errPathUnresolved, graphPath) } +// fileAttributionNode synthesizes a node carrying just the repo prefix +// and language of a file — enough for tokenStats.record to attribute an +// observation to the right per-repo / per-language bucket when no real +// symbol node is in hand. +func (s *Server) fileAttributionNode(relPath, language string) *graph.Node { + prefix := "" + if s.multiIndexer != nil { + prefix = matchedRepoPrefix(s.multiIndexer, relPath) + if prefix == "" { + if prefixes := s.multiIndexer.RepoPrefixes(); len(prefixes) == 1 { + prefix = prefixes[0] + } + } + } + return &graph.Node{RepoPrefix: prefix, Language: language, FilePath: relPath} +} + +// recordFileBaselineSavings books a savings observation for a tool whose +// response stands in for reading a whole file. payload is the response +// content actually produced — used both for the returned-token count and +// as the chars-per-token calibration sample — and the baseline is the +// on-disk byte size of the file the agent would otherwise have read. +// Best-effort accounting: files that don't resolve or stat book nothing. +func (s *Server) recordFileBaselineSavings(ctx context.Context, tool, relPath, language, payload string) { + abs, err := s.resolveGraphPath(relPath) + if err != nil { + return + } + info, err := os.Stat(abs) + if err != nil || info.IsDir() { + return + } + returned := tokens.CachedCountInt64(payload) + fullFile := int64(tokens.EstimateFromSample(int(info.Size()), payload)) + s.tokenStatsFor(ctx).record(s.fileAttributionNode(relPath, language), tool, returned, fullFile) +} + // repoRelative converts an absolute path to a repo-prefixed or root-relative // string if it falls under any indexed repo, otherwise returns the absolute // path unchanged. @@ -722,6 +760,16 @@ func (s *Server) handleReadFile(ctx context.Context, req mcp.CallToolRequest) (* } } + // Server-side accounting only — read_file is the heaviest source + // fetch and must show up in the savings ledger even when nothing + // was saved (an uncompressed read returns the whole file, so + // returned == baseline and only the call is counted). + if !isBinary { + returned := tokens.CachedCountInt64(string(content)) + fullFile := int64(tokens.EstimateFromSample(originalBytes, string(content))) + s.tokenStatsFor(ctx).record(s.fileAttributionNode(relPath, language), "read_file", returned, fullFile) + } + // Record the access for frecency credit on any node defined in // this file. read_file is a heavy access (full file), so we // credit every defined symbol — keeps the "agent is working in diff --git a/internal/mcp/tools_savings_record_test.go b/internal/mcp/tools_savings_record_test.go new file mode 100644 index 00000000..421ce225 --- /dev/null +++ b/internal/mcp/tools_savings_record_test.go @@ -0,0 +1,43 @@ +package mcp + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestReadFamilyToolsRecordSavings pins the savings recording surface on a +// single-repo server (the issue-67 shape: one tracked repo, unprefixed +// nodes). Every read-family tool — read_file, get_file_summary, +// get_editing_context — and the original get_symbol_source must book an +// observation; before the lone-repo resolution fix none of them could, +// because the record sites sit behind resolveNodePath/resolveFilePath. +func TestReadFamilyToolsRecordSavings(t *testing.T) { + srv, _, _ := newSingleRepoServer(t) + ctx := context.Background() + + calls := func() int64 { + return srv.tokenStats.snapshot()["calls_counted"].(int64) + } + require.Equal(t, int64(0), calls()) + + res := callToolByName(t, srv, ctx, "read_file", map[string]any{"path": "main.go"}) + require.False(t, res.IsError, "read_file must succeed on a bare-relative path in single-repo mode") + require.Equal(t, int64(1), calls(), "read_file must record a savings observation") + + res = callToolByName(t, srv, ctx, "get_file_summary", map[string]any{"path": "main.go"}) + require.False(t, res.IsError) + require.Equal(t, int64(2), calls(), "get_file_summary must record a savings observation") + + res = callToolByName(t, srv, ctx, "get_editing_context", map[string]any{"path": "main.go"}) + require.False(t, res.IsError) + require.Equal(t, int64(3), calls(), "get_editing_context must record a savings observation") + + res = callToolByName(t, srv, ctx, "get_symbol_source", map[string]any{"id": "main.go::Hello"}) + require.False(t, res.IsError, "get_symbol_source must resolve unprefixed single-repo nodes") + require.Equal(t, int64(4), calls(), "get_symbol_source must record a savings observation") + + snap := srv.tokenStats.snapshot() + require.Greater(t, snap["tokens_returned"].(int64), int64(0)) +} From f05aa9a90770eea7fa1f63e8c949b64258f2aa77 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 00:20:58 +0200 Subject: [PATCH 03/12] savings: move the ledger into the sidecar database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flat-file ledger (savings.json cumulative totals + savings.jsonl event log under the cache dir) was only durable on a lucky schedule: the cumulative file flushed every 20 observations, on a 5-minute ticker gated on pending work, or on graceful shutdown — and MCP clients SIGKILL their stdio servers, so light sessions never reached disk at all. Every write error was silently discarded on top. The ledger now lives in the machine-global SQLite sidecar (~/.gortex/sidecar.sqlite, shared with notes/memories/scopes): savings_events one row per call (now carrying the session id), savings_totals running aggregates per bucket, savings_meta the first/last stamps. Each observation is a single transaction — durable at the call, safe across concurrent writer processes via WAL, no flush machinery left to miss (the periodic flusher and its wiring are gone; FlushSavings stays as a no-op for the shutdown chains). Flat files import once on open — totals, per-repo/per-language buckets, and the event history (a lone .jsonl without its cumulative file rebuilds totals from events) — then rename to *.bak behind a migration mark. The savings/gain CLIs and serverstack derive the ledger from the same sidecar path the side-stores use, --cache-dir now relocating both the ledger and the legacy files it imports. A fresh ledger reports a zero FirstSeen instead of seeding time.Now(), so nothing claims to have been tracking before anything was recorded. --- cmd/gortex/daemon.go | 7 - cmd/gortex/gain.go | 21 +- cmd/gortex/gain_test.go | 32 +- cmd/gortex/mcp.go | 18 +- cmd/gortex/savings.go | 58 +- cmd/gortex/savings_test.go | 4 +- internal/mcp/server.go | 36 +- internal/mcp/session_ctx.go | 5 +- internal/persistence/sidecar_savings.go | 265 ++++++++ internal/persistence/sidecar_savings_test.go | 145 +++++ internal/persistence/sidecar_sqlite.go | 25 + internal/savings/events.go | 94 +-- internal/savings/events_test.go | 143 ++--- internal/savings/store.go | 614 ++++++++----------- internal/savings/store_test.go | 476 +++++++------- internal/serverstack/shared_server.go | 30 +- 16 files changed, 1176 insertions(+), 797 deletions(-) create mode 100644 internal/persistence/sidecar_savings.go create mode 100644 internal/persistence/sidecar_savings_test.go diff --git a/cmd/gortex/daemon.go b/cmd/gortex/daemon.go index 7c6e08ed..f5bdd892 100644 --- a/cmd/gortex/daemon.go +++ b/cmd/gortex/daemon.go @@ -453,13 +453,6 @@ func runDaemonStart(cmd *cobra.Command, _ []string) error { } defer stopSnapshotter() - // Periodic savings flush — 5 minute interval. Bounds on-crash data - // loss for the savings counters even when the call rate is too low - // to trip the every-N-observations flush. No-op when persistence - // isn't wired (e.g. cache dir unavailable). - stopSavingsFlush := state.mcpServer.StartPeriodicSavingsFlush(5 * time.Minute) - defer stopSavingsFlush() - // Periodic reconciliation — the "janitor". Walks each tracked repo // and runs IncrementalReindex to evict files deleted offline and // re-index files whose mtime changed. Insurance against gaps in diff --git a/cmd/gortex/gain.go b/cmd/gortex/gain.go index ecc80fb8..754f4b3b 100644 --- a/cmd/gortex/gain.go +++ b/cmd/gortex/gain.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/cobra" + "github.com/zzet/gortex/internal/persistence" "github.com/zzet/gortex/internal/savings" ) @@ -321,25 +322,27 @@ func (h *gainHistory) toJSON() map[string]any { } } -// loadHistory loads the cumulative savings store, restricted to the -// since-window via the JSONL event log. Returns an error only on -// hard I/O failures; missing files / empty stores produce a populated -// gainHistory with Calls=0 so the caller can decide whether to render. +// loadHistory loads the cumulative savings ledger, restricted to the +// since-window via the event history. Returns an error only on hard +// I/O failures; an empty ledger produces a populated gainHistory with +// Calls=0 so the caller can decide whether to render. func loadHistory(cacheDir string, since time.Duration) (*gainHistory, error) { - path := savings.DefaultPath() + path := savings.DefaultDBPath() + legacyJSON := savings.DefaultPath() if cacheDir != "" { - path = filepath.Join(cacheDir, "savings.json") + path = persistence.DefaultSidecarPath(cacheDir) + legacyJSON = filepath.Join(cacheDir, "savings.json") } store, err := savings.Open(path) if err != nil { return nil, err } + _ = store.ImportLegacy(legacyJSON) snap := store.Snapshot() - eventsPath := savings.EventsPathFor(path) if since <= 0 { // --since 0 → entire-history view; just use the cumulative - // totals. No JSONL scan needed. + // totals. No event scan needed. return &gainHistory{ Path: path, Since: since, @@ -351,7 +354,7 @@ func loadHistory(cacheDir string, since time.Duration) (*gainHistory, error) { } cutoff := time.Now().UTC().Add(-since) - events, err := savings.LoadEvents(eventsPath, cutoff) + events, err := store.EventsSince(cutoff) if err != nil { return nil, err } diff --git a/cmd/gortex/gain_test.go b/cmd/gortex/gain_test.go index 91ddbcc6..f6610014 100644 --- a/cmd/gortex/gain_test.go +++ b/cmd/gortex/gain_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/zzet/gortex/internal/persistence" "github.com/zzet/gortex/internal/savings" ) @@ -142,25 +143,38 @@ func TestLoadHistory_EmptyStore(t *testing.T) { } func TestLoadHistory_SinceZeroUsesCumulative(t *testing.T) { - // Populate a store with a known total, then call loadHistory with + // Populate a ledger with a known total, then call loadHistory with // since=0. The result must come from the cumulative snapshot, not - // a JSONL scan. + // an event scan. dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - store, err := savings.Open(path) + store, err := savings.Open(persistence.DefaultSidecarPath(dir)) if err != nil { t.Fatal(err) } - store.AddObservation("/r", "go", "get_symbol_source", 50, 500) - if err := store.Flush(); err != nil { - t.Fatal(err) - } + store.AddObservation(savings.Observation{Repo: "/r", Language: "go", Tool: "get_symbol_source", Returned: 50, Saved: 500}) h, err := loadHistory(dir, 0) if err != nil { t.Fatal(err) } if h.Calls != 1 || h.Saved != 500 { - t.Errorf("since=0 should reflect cumulative store, got %+v", h) + t.Errorf("since=0 should reflect cumulative ledger, got %+v", h) + } +} + +func TestLoadHistory_WindowFiltersEvents(t *testing.T) { + dir := t.TempDir() + store, err := savings.Open(persistence.DefaultSidecarPath(dir)) + if err != nil { + t.Fatal(err) + } + store.AddObservation(savings.Observation{Repo: "/r", Language: "go", Tool: "read_file", Returned: 10, Saved: 90}) + + h, err := loadHistory(dir, 24*time.Hour) + if err != nil { + t.Fatal(err) + } + if h.Calls != 1 || h.Saved != 90 { + t.Errorf("fresh event should fall inside a 24h window, got %+v", h) } } diff --git a/cmd/gortex/mcp.go b/cmd/gortex/mcp.go index 59cff25e..be00f9cc 100644 --- a/cmd/gortex/mcp.go +++ b/cmd/gortex/mcp.go @@ -15,7 +15,6 @@ import ( "github.com/zzet/gortex/internal/llm/conversationlog" "github.com/zzet/gortex/internal/persistence" "github.com/zzet/gortex/internal/platform" - "github.com/zzet/gortex/internal/savings" "github.com/zzet/gortex/internal/server" "github.com/zzet/gortex/internal/server/hub" "github.com/zzet/gortex/internal/serverstack" @@ -140,9 +139,13 @@ func runMCP(cmd *cobra.Command, args []string) error { if sideStoreCacheDir == "" { sideStoreCacheDir = platform.CacheDir() } - savingsPath := savings.DefaultPath() + // The savings ledger rides in the same sidecar database the side + // stores use (serverstack derives it from NotesDir). A custom + // --cache-dir also relocates the flat-file era's savings.json the + // one-shot legacy import looks for. + savingsLegacyJSON := "" if mcpCacheDir != "" { - savingsPath = filepath.Join(mcpCacheDir, "savings.json") + savingsLegacyJSON = filepath.Join(mcpCacheDir, "savings.json") } ss, err := serverstack.NewSharedServer(serverstack.SharedServerConfig{ @@ -166,8 +169,8 @@ func runMCP(cmd *cobra.Command, args []string) error { FeedbackRepo: mcpIndex, NotebookPath: mcpIndex, }, - SavingsPath: savingsPath, - SavingsRepo: mcpIndex, + SavingsLegacyJSON: savingsLegacyJSON, + SavingsRepo: mcpIndex, }) if err != nil { return fmt.Errorf("build server stack: %w", err) @@ -204,11 +207,6 @@ func runMCP(cmd *cobra.Command, args []string) error { } } - // Periodic savings flush. NewSharedServer flushes on Close (deferred - // above); this guards against a crash losing accumulated totals. - stopSavingsFlush := srv.StartPeriodicSavingsFlush(5 * time.Minute) - defer stopSavingsFlush() - fmt.Fprintf(os.Stderr, "[gortex] MCP server ready (transport: %s)\n", mcpTransport) // Start server HTTP API if requested. diff --git a/cmd/gortex/savings.go b/cmd/gortex/savings.go index 6ccbf015..f735b6c4 100644 --- a/cmd/gortex/savings.go +++ b/cmd/gortex/savings.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" + "github.com/zzet/gortex/internal/persistence" "github.com/zzet/gortex/internal/progress" "github.com/zzet/gortex/internal/savings" "github.com/zzet/gortex/internal/tui" @@ -58,25 +59,26 @@ func init() { } func runSavings(_ *cobra.Command, _ []string) error { - path := savings.DefaultPath() + dbPath := savings.DefaultDBPath() + legacyJSON := savings.DefaultPath() if savingsCacheDir != "" { - path = filepath.Join(savingsCacheDir, "savings.json") + dbPath = persistence.DefaultSidecarPath(savingsCacheDir) + legacyJSON = filepath.Join(savingsCacheDir, "savings.json") } - eventsPath := savings.EventsPathFor(path) - store, err := savings.Open(path) + store, err := savings.Open(dbPath) if err != nil { - return fmt.Errorf("open savings store: %w", err) + return fmt.Errorf("open savings ledger: %w", err) + } + if ierr := store.ImportLegacy(legacyJSON); ierr != nil { + fmt.Fprintf(os.Stderr, "[gortex savings] legacy import failed: %v\n", ierr) } if savingsReset { if err := store.Reset(); err != nil { return fmt.Errorf("reset: %w", err) } - fmt.Fprintf(os.Stderr, "[gortex savings] reset cumulative totals at %s\n", path) - if eventsPath != "" { - fmt.Fprintf(os.Stderr, "[gortex savings] removed event log at %s\n", eventsPath) - } + fmt.Fprintf(os.Stderr, "[gortex savings] reset cumulative totals + event history at %s\n", dbPath) return nil } @@ -86,26 +88,23 @@ func runSavings(_ *cobra.Command, _ []string) error { if savingsUTC { loc = time.UTC } - buckets, err := savings.BuildDashboard(eventsPath, snap.Totals, time.Now(), loc) + events, err := store.EventsSince(time.Time{}) if err != nil { - // Don't fail the whole command on event-log read errors — fall - // back to a dashboard with empty Today/7d buckets. - fmt.Fprintf(os.Stderr, "[gortex savings] event log read failed: %v\n", err) - buckets = []savings.Bucket{ - {Label: "Today"}, - {Label: "Last 7 days"}, - {Label: "All time", Totals: snap.Totals}, - } + // Don't fail the whole command on event read errors — fall back + // to a dashboard with empty Today/7d buckets. + fmt.Fprintf(os.Stderr, "[gortex savings] event history read failed: %v\n", err) + events = nil } + buckets := savings.BuildDashboard(events, snap.Totals, time.Now(), loc) if savingsJSON { - return emitSavingsJSON(snap, buckets, path, eventsPath) + return emitSavingsJSON(snap, buckets, dbPath) } - emitSavingsDashboard(snap, buckets, path, eventsPath) + emitSavingsDashboard(snap, buckets, dbPath) return nil } -func emitSavingsJSON(snap savings.File, buckets []savings.Bucket, path, eventsPath string) error { +func emitSavingsJSON(snap savings.File, buckets []savings.Bucket, path string) error { bucketJSON := make([]map[string]any, 0, len(buckets)) for _, b := range buckets { entry := map[string]any{ @@ -133,15 +132,18 @@ func emitSavingsJSON(snap savings.File, buckets []savings.Bucket, path, eventsPa out := map[string]any{ "path": path, - "events_path": eventsPath, - "first_seen": snap.FirstSeen.Format(time.RFC3339), - "last_updated": snap.LastUpdated.Format(time.RFC3339), "tokens_saved": snap.Totals.TokensSaved, "tokens_returned": snap.Totals.TokensReturned, "calls_counted": snap.Totals.CallsCounted, "cost_avoided_usd": savings.CostAvoidedAll(snap.Totals.TokensSaved), "buckets": bucketJSON, } + if !snap.FirstSeen.IsZero() { + out["first_seen"] = snap.FirstSeen.Format(time.RFC3339) + } + if !snap.LastUpdated.IsZero() { + out["last_updated"] = snap.LastUpdated.Format(time.RFC3339) + } if len(snap.PerRepo) > 0 { out["per_repo"] = snap.PerRepo } @@ -161,7 +163,7 @@ func emitSavingsJSON(snap savings.File, buckets []savings.Bucket, path, eventsPa // On a TTY we wrap the header in a styled banner + stat-strip card; on a // non-TTY (output piped into grep / a file) we preserve the bare text // header so script parsers keep matching. -func emitSavingsDashboard(snap savings.File, buckets []savings.Bucket, path, eventsPath string) { +func emitSavingsDashboard(snap savings.File, buckets []savings.Bucket, path string) { tty := progress.IsTTY(os.Stdout) && !noProgress if tty { @@ -173,9 +175,6 @@ func emitSavingsDashboard(snap savings.File, buckets []savings.Bucket, path, eve fmt.Println(banner) fmt.Println() fmt.Println(" " + progress.Row("store", path, 14)) - if eventsPath != "" { - fmt.Println(" " + progress.Row("event log", eventsPath, 14)) - } if !snap.FirstSeen.IsZero() { fmt.Println(" " + progress.Row("tracking since", snap.FirstSeen.Format("2006-01-02 15:04"), 14)) } @@ -186,9 +185,6 @@ func emitSavingsDashboard(snap savings.File, buckets []savings.Bucket, path, eve fmt.Println("Gortex Token Savings") fmt.Println("====================") fmt.Printf("Store: %s\n", path) - if eventsPath != "" { - fmt.Printf("Event log: %s\n", eventsPath) - } if !snap.FirstSeen.IsZero() { fmt.Printf("Tracking since: %s\n", snap.FirstSeen.Format("2006-01-02 15:04")) } diff --git a/cmd/gortex/savings_test.go b/cmd/gortex/savings_test.go index d74f4084..f83403f6 100644 --- a/cmd/gortex/savings_test.go +++ b/cmd/gortex/savings_test.go @@ -156,7 +156,7 @@ func TestEmitSavingsDashboard_RendersThreeBuckets(t *testing.T) { savingsVerbose = oldVerbose savingsBarCells = oldCells }() - emitSavingsDashboard(snap, buckets, "/tmp/savings.json", "/tmp/savings.jsonl") + emitSavingsDashboard(snap, buckets, "/tmp/sidecar.sqlite") }) for _, want := range []string{ @@ -187,7 +187,7 @@ func TestEmitSavingsDashboard_EmptyTotals(t *testing.T) { {Label: "All time"}, } out := captureStdout(t, func() { - emitSavingsDashboard(snap, buckets, "/tmp/savings.json", "") + emitSavingsDashboard(snap, buckets, "/tmp/sidecar.sqlite") }) if !strings.Contains(out, "No source-reading tool calls recorded yet") { t.Errorf("empty totals should show the no-data hint, got:\n%s", out) diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 71bc5da2..c29790b2 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -543,6 +543,7 @@ type tokenStats struct { persistent *savings.Store parent *tokenStats // process-wide aggregate (nil for the root) repoPath string // forwarded to savings for per-repo aggregation + sessionID string // rides on persisted events; "" for the shared default } // record adds a single savings observation. node is the symbol whose @@ -563,6 +564,7 @@ func (ts *tokenStats) record(node *graph.Node, tool string, returned, fullFile i store := ts.persistent parent := ts.parent fallbackRepo := ts.repoPath + sessionID := ts.sessionID ts.mu.Unlock() // Fan out to the process-wide aggregate so graph_stats called @@ -591,11 +593,18 @@ func (ts *tokenStats) record(node *graph.Node, tool string, returned, fullFile i language = node.Language } - // Forward to the persistent store outside our lock — its own mutex guards - // concurrent writers, and flushing to disk shouldn't block new record() - // calls on the hot path. + // Forward to the persistent store outside our lock — its own + // synchronization guards concurrent writers, and the ledger write + // shouldn't block new record() calls on the hot path. if store != nil { - store.AddObservation(repo, language, tool, returned, saved) + store.AddObservation(savings.Observation{ + Repo: repo, + Language: language, + Tool: tool, + SessionID: sessionID, + Returned: returned, + Saved: saved, + }) } } @@ -1529,8 +1538,9 @@ func (s *Server) tokenStatsFor(ctx context.Context) *tokenStats { return s.sessions.get(id).tokenStats } -// FlushSavings forces any buffered savings observations to disk. Called on -// server shutdown to minimize data loss on unclean exits. +// FlushSavings is kept for shutdown-path compatibility. The sidecar-backed +// ledger commits every observation as it is recorded, so there is nothing +// buffered to write. func (s *Server) FlushSavings() error { store := s.savingsStore() if store == nil { @@ -1539,20 +1549,6 @@ func (s *Server) FlushSavings() error { return store.Flush() } -// StartPeriodicSavingsFlush starts a background ticker that flushes the -// savings store every interval if there are pending observations. Returns -// a stop function for clean shutdown. No-op when persistence isn't wired. -// -// This bounds on-crash data loss to roughly `interval` worth of observations -// even when the call rate is too low to trip the every-N-observations flush. -func (s *Server) StartPeriodicSavingsFlush(interval time.Duration) func() { - store := s.savingsStore() - if store == nil { - return func() {} - } - return store.StartPeriodicFlush(interval) -} - // savingsStore extracts the persistent savings store via tokenStats. Returns // nil when persistence isn't initialized. func (s *Server) savingsStore() *savings.Store { diff --git a/internal/mcp/session_ctx.go b/internal/mcp/session_ctx.go index b1dd6de3..e4172290 100644 --- a/internal/mcp/session_ctx.go +++ b/internal/mcp/session_ctx.go @@ -96,13 +96,14 @@ type sessionLocal struct { // is shared. parent, when non-nil, is the process-wide tokenStats // aggregate; every per-session record() call also bumps it so the // shared default reflects daemon-wide live activity. -func newSessionLocal(persistent *savings.Store, repoPath string, parent *tokenStats) *sessionLocal { +func newSessionLocal(id string, persistent *savings.Store, repoPath string, parent *tokenStats) *sessionLocal { return &sessionLocal{ session: newSessionState(), tokenStats: &tokenStats{ persistent: persistent, repoPath: repoPath, parent: parent, + sessionID: id, }, } } @@ -155,7 +156,7 @@ func (m *sessionMap) get(id string) *sessionLocal { defer m.mu.Unlock() sl, ok := m.sessions[id] if !ok { - sl = newSessionLocal(m.persistent, m.repoPath, m.parent) + sl = newSessionLocal(id, m.persistent, m.repoPath, m.parent) m.sessions[id] = sl } return sl diff --git a/internal/persistence/sidecar_savings.go b/internal/persistence/sidecar_savings.go new file mode 100644 index 00000000..ba734cd0 --- /dev/null +++ b/internal/persistence/sidecar_savings.go @@ -0,0 +1,265 @@ +package persistence + +import ( + "database/sql" + "fmt" + "time" +) + +// Token-savings ledger tables. The savings ledger is machine-global — +// unlike notes/memories it carries no repo_key partition; per-repo +// attribution rides on the bucket key of savings_totals and the repo +// column of savings_events instead. +// +// Three tables, one job each: +// - savings_events: one row per recorded source-reading tool call. +// Durable at the call (single INSERT inside the observation tx), so +// a SIGKILLed server loses nothing — the property the flat-file +// ledger's batched flush could not give. +// - savings_totals: running aggregates keyed by bucket ('' top-line, +// 'repo:', 'lang:'), updated transactionally with the +// event insert so reads are point lookups instead of full scans. +// - savings_meta: first_seen / last_updated unix-nano stamps. + +// SavingsEvent is one recorded source-reading observation. +type SavingsEvent struct { + TS time.Time + SessionID string + Tool string + Repo string + Language string + Returned int64 + Saved int64 +} + +// SavingsTotalsRow is the aggregate for one savings_totals bucket. +type SavingsTotalsRow struct { + Saved int64 + Returned int64 + Calls int64 +} + +const savingsLegacyMigrationKind = "savings_files" + +// AddSavingsObservation books one observation: the event row, the +// affected totals buckets, and the meta stamps, in a single transaction. +func (s *SidecarStore) AddSavingsObservation(ev SavingsEvent) error { + if s == nil { + return nil + } + s.writeMu.Lock() + defer s.writeMu.Unlock() + + tx, err := s.db.Begin() + if err != nil { + return fmt.Errorf("persistence: savings tx: %w", err) + } + defer func() { _ = tx.Rollback() }() + + ts := ev.TS + if ts.IsZero() { + ts = time.Now() + } + tsN := ts.UTC().UnixNano() + + if _, err := tx.Exec( + `INSERT INTO savings_events (ts, session_id, tool, repo, language, returned, saved) VALUES (?,?,?,?,?,?,?)`, + tsN, ev.SessionID, ev.Tool, ev.Repo, ev.Language, ev.Returned, ev.Saved, + ); err != nil { + return fmt.Errorf("persistence: savings event: %w", err) + } + + buckets := []string{""} + if ev.Repo != "" { + buckets = append(buckets, "repo:"+ev.Repo) + } + if ev.Language != "" { + buckets = append(buckets, "lang:"+ev.Language) + } + for _, bucket := range buckets { + if err := upsertSavingsBucket(tx, bucket, ev.Saved, ev.Returned, 1); err != nil { + return err + } + } + + if _, err := tx.Exec( + `INSERT INTO savings_meta (key, value) VALUES ('first_seen', ?) ON CONFLICT(key) DO NOTHING`, tsN, + ); err != nil { + return fmt.Errorf("persistence: savings meta: %w", err) + } + if _, err := tx.Exec( + `INSERT INTO savings_meta (key, value) VALUES ('last_updated', ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value`, tsN, + ); err != nil { + return fmt.Errorf("persistence: savings meta: %w", err) + } + + return tx.Commit() +} + +func upsertSavingsBucket(tx *sql.Tx, bucket string, saved, returned, calls int64) error { + _, err := tx.Exec( + `INSERT INTO savings_totals (bucket, saved, returned, calls) VALUES (?,?,?,?) + ON CONFLICT(bucket) DO UPDATE SET + saved = savings_totals.saved + excluded.saved, + returned = savings_totals.returned + excluded.returned, + calls = savings_totals.calls + excluded.calls`, + bucket, saved, returned, calls, + ) + if err != nil { + return fmt.Errorf("persistence: savings totals: %w", err) + } + return nil +} + +// SavingsTotals returns every totals bucket plus the first_seen / +// last_updated stamps. Buckets map keys are '' (top-line), +// 'repo:', and 'lang:'. The zero time means "never". +func (s *SidecarStore) SavingsTotals() (map[string]SavingsTotalsRow, time.Time, time.Time, error) { + if s == nil { + return map[string]SavingsTotalsRow{}, time.Time{}, time.Time{}, nil + } + rows, err := s.db.Query(`SELECT bucket, saved, returned, calls FROM savings_totals`) + if err != nil { + return nil, time.Time{}, time.Time{}, fmt.Errorf("persistence: savings totals: %w", err) + } + defer func() { _ = rows.Close() }() + + out := make(map[string]SavingsTotalsRow) + for rows.Next() { + var bucket string + var r SavingsTotalsRow + if err := rows.Scan(&bucket, &r.Saved, &r.Returned, &r.Calls); err != nil { + return nil, time.Time{}, time.Time{}, err + } + out[bucket] = r + } + if err := rows.Err(); err != nil { + return nil, time.Time{}, time.Time{}, err + } + + return out, s.savingsMetaTime("first_seen"), s.savingsMetaTime("last_updated"), nil +} + +func (s *SidecarStore) savingsMetaTime(key string) time.Time { + var n int64 + row := s.db.QueryRow(`SELECT value FROM savings_meta WHERE key = ?`, key) + if err := row.Scan(&n); err != nil || n == 0 { + return time.Time{} + } + return time.Unix(0, n).UTC() +} + +// SavingsEventsSince returns events with ts >= since, oldest first. +// since=zero returns everything. +func (s *SidecarStore) SavingsEventsSince(since time.Time) ([]SavingsEvent, error) { + if s == nil { + return nil, nil + } + rows, err := s.db.Query( + `SELECT ts, session_id, tool, repo, language, returned, saved + FROM savings_events WHERE ts >= ? ORDER BY ts, id`, + unixOrZero(since), + ) + if err != nil { + return nil, fmt.Errorf("persistence: savings events: %w", err) + } + defer func() { _ = rows.Close() }() + + var out []SavingsEvent + for rows.Next() { + var ev SavingsEvent + var tsN int64 + if err := rows.Scan(&tsN, &ev.SessionID, &ev.Tool, &ev.Repo, &ev.Language, &ev.Returned, &ev.Saved); err != nil { + return nil, err + } + ev.TS = time.Unix(0, tsN).UTC() + out = append(out, ev) + } + return out, rows.Err() +} + +// SavingsLegacyImportDone reports whether the one-shot flat-file +// (savings.json + savings.jsonl) import has already run. +func (s *SidecarStore) SavingsLegacyImportDone() bool { + if s == nil { + return true + } + return s.migrationDone("", savingsLegacyMigrationKind) +} + +// ImportLegacySavings seeds the ledger from the flat-file era: bucket +// totals from the cumulative savings.json and event rows from the +// savings.jsonl log. Idempotent — guarded by a migration mark, which is +// set even for an empty import so the file probing never repeats. The +// caller owns reading (and afterwards renaming) the legacy files. +func (s *SidecarStore) ImportLegacySavings(buckets map[string]SavingsTotalsRow, firstSeen, lastUpdated time.Time, events []SavingsEvent) error { + if s == nil || s.migrationDone("", savingsLegacyMigrationKind) { + return nil + } + s.writeMu.Lock() + defer s.writeMu.Unlock() + + tx, err := s.db.Begin() + if err != nil { + return fmt.Errorf("persistence: savings import tx: %w", err) + } + defer func() { _ = tx.Rollback() }() + + for bucket, r := range buckets { + if err := upsertSavingsBucket(tx, bucket, r.Saved, r.Returned, r.Calls); err != nil { + return err + } + } + for _, ev := range events { + if _, err := tx.Exec( + `INSERT INTO savings_events (ts, session_id, tool, repo, language, returned, saved) VALUES (?,?,?,?,?,?,?)`, + unixOrZero(ev.TS), ev.SessionID, ev.Tool, ev.Repo, ev.Language, ev.Returned, ev.Saved, + ); err != nil { + return fmt.Errorf("persistence: savings import event: %w", err) + } + } + if !firstSeen.IsZero() { + if _, err := tx.Exec( + `INSERT INTO savings_meta (key, value) VALUES ('first_seen', ?) + ON CONFLICT(key) DO UPDATE SET value = MIN(savings_meta.value, excluded.value)`, + unixOrZero(firstSeen), + ); err != nil { + return fmt.Errorf("persistence: savings import meta: %w", err) + } + } + if !lastUpdated.IsZero() { + if _, err := tx.Exec( + `INSERT INTO savings_meta (key, value) VALUES ('last_updated', ?) + ON CONFLICT(key) DO UPDATE SET value = MAX(savings_meta.value, excluded.value)`, + unixOrZero(lastUpdated), + ); err != nil { + return fmt.Errorf("persistence: savings import meta: %w", err) + } + } + if err := tx.Commit(); err != nil { + return err + } + s.markMigrated("", savingsLegacyMigrationKind) + return nil +} + +// ResetSavings wipes the savings ledger (events, totals, meta). The +// legacy-import migration mark survives so renamed flat files are not +// re-imported after a reset. +func (s *SidecarStore) ResetSavings() error { + if s == nil { + return nil + } + s.writeMu.Lock() + defer s.writeMu.Unlock() + for _, stmt := range []string{ + `DELETE FROM savings_events`, + `DELETE FROM savings_totals`, + `DELETE FROM savings_meta`, + } { + if _, err := s.db.Exec(stmt); err != nil { + return fmt.Errorf("persistence: savings reset: %w", err) + } + } + return nil +} diff --git a/internal/persistence/sidecar_savings_test.go b/internal/persistence/sidecar_savings_test.go new file mode 100644 index 00000000..63950110 --- /dev/null +++ b/internal/persistence/sidecar_savings_test.go @@ -0,0 +1,145 @@ +package persistence + +import ( + "path/filepath" + "testing" + "time" +) + +func openTestSidecar(t *testing.T) (*SidecarStore, string) { + t.Helper() + path := filepath.Join(t.TempDir(), "sidecar.sqlite") + sc, err := OpenSidecar(path) + if err != nil { + t.Fatal(err) + } + return sc, path +} + +// The durability contract behind the savings ledger: an observation +// survives a full close + reopen of the database — no flush step exists +// to forget. +func TestSavings_DurableAcrossReopen(t *testing.T) { + sc, path := openTestSidecar(t) + + ev := SavingsEvent{ + TS: time.Date(2026, 6, 1, 10, 0, 0, 0, time.UTC), + SessionID: "sess-1", + Tool: "get_symbol_source", + Repo: "repo-a", + Language: "go", + Returned: 23, + Saved: 77, + } + if err := sc.AddSavingsObservation(ev); err != nil { + t.Fatal(err) + } + if err := sc.Close(); err != nil { + t.Fatal(err) + } + + sc2, err := OpenSidecar(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = sc2.Close() }() + + buckets, firstSeen, lastUpdated, err := sc2.SavingsTotals() + if err != nil { + t.Fatal(err) + } + top := buckets[""] + if top.Calls != 1 || top.Saved != 77 || top.Returned != 23 { + t.Errorf("top-line bucket = %+v, want calls=1 saved=77 returned=23", top) + } + if r := buckets["repo:repo-a"]; r.Calls != 1 { + t.Errorf("repo bucket = %+v, want calls=1", r) + } + if l := buckets["lang:go"]; l.Calls != 1 { + t.Errorf("lang bucket = %+v, want calls=1", l) + } + if !firstSeen.Equal(ev.TS) || !lastUpdated.Equal(ev.TS) { + t.Errorf("meta stamps = (%v, %v), want both %v", firstSeen, lastUpdated, ev.TS) + } + + evs, err := sc2.SavingsEventsSince(time.Time{}) + if err != nil { + t.Fatal(err) + } + if len(evs) != 1 || evs[0].SessionID != "sess-1" || !evs[0].TS.Equal(ev.TS) { + t.Errorf("reloaded events = %+v", evs) + } +} + +func TestSavings_MetaStampsMinMax(t *testing.T) { + sc, _ := openTestSidecar(t) + defer func() { _ = sc.Close() }() + + t1 := time.Date(2026, 6, 1, 10, 0, 0, 0, time.UTC) + t2 := t1.Add(time.Hour) + if err := sc.AddSavingsObservation(SavingsEvent{TS: t1, Tool: "a"}); err != nil { + t.Fatal(err) + } + if err := sc.AddSavingsObservation(SavingsEvent{TS: t2, Tool: "b"}); err != nil { + t.Fatal(err) + } + + _, firstSeen, lastUpdated, err := sc.SavingsTotals() + if err != nil { + t.Fatal(err) + } + if !firstSeen.Equal(t1) { + t.Errorf("first_seen = %v, want %v (first observation wins)", firstSeen, t1) + } + if !lastUpdated.Equal(t2) { + t.Errorf("last_updated = %v, want %v (latest observation wins)", lastUpdated, t2) + } +} + +func TestSavings_ResetClearsButKeepsImportMark(t *testing.T) { + sc, _ := openTestSidecar(t) + defer func() { _ = sc.Close() }() + + if err := sc.ImportLegacySavings( + map[string]SavingsTotalsRow{"": {Saved: 100, Returned: 10, Calls: 1}}, + time.Now().UTC(), time.Now().UTC(), nil, + ); err != nil { + t.Fatal(err) + } + if !sc.SavingsLegacyImportDone() { + t.Fatal("import mark must be set after ImportLegacySavings") + } + if err := sc.ResetSavings(); err != nil { + t.Fatal(err) + } + buckets, firstSeen, _, err := sc.SavingsTotals() + if err != nil { + t.Fatal(err) + } + if len(buckets) != 0 || !firstSeen.IsZero() { + t.Errorf("reset must clear totals + meta, got buckets=%v firstSeen=%v", buckets, firstSeen) + } + if !sc.SavingsLegacyImportDone() { + t.Error("reset must NOT clear the legacy-import mark (renamed files would re-import)") + } +} + +func TestSavings_ImportIsIdempotent(t *testing.T) { + sc, _ := openTestSidecar(t) + defer func() { _ = sc.Close() }() + + rows := map[string]SavingsTotalsRow{"": {Saved: 100, Returned: 10, Calls: 1}} + if err := sc.ImportLegacySavings(rows, time.Time{}, time.Time{}, nil); err != nil { + t.Fatal(err) + } + if err := sc.ImportLegacySavings(rows, time.Time{}, time.Time{}, nil); err != nil { + t.Fatal(err) + } + buckets, _, _, err := sc.SavingsTotals() + if err != nil { + t.Fatal(err) + } + if got := buckets[""].Calls; got != 1 { + t.Errorf("calls after double import = %d, want 1", got) + } +} diff --git a/internal/persistence/sidecar_sqlite.go b/internal/persistence/sidecar_sqlite.go index 2aabd828..21e176a1 100644 --- a/internal/persistence/sidecar_sqlite.go +++ b/internal/persistence/sidecar_sqlite.go @@ -137,6 +137,31 @@ CREATE TABLE IF NOT EXISTS suppressions ( PRIMARY KEY (repo_key, identity_key) ) WITHOUT ROWID; CREATE INDEX IF NOT EXISTS idx_supp_updated ON suppressions (repo_key, last_hit DESC); + +CREATE TABLE IF NOT EXISTS savings_events ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + ts INTEGER NOT NULL DEFAULT 0, + session_id TEXT NOT NULL DEFAULT '', + tool TEXT NOT NULL DEFAULT '', + repo TEXT NOT NULL DEFAULT '', + language TEXT NOT NULL DEFAULT '', + returned INTEGER NOT NULL DEFAULT 0, + saved INTEGER NOT NULL DEFAULT 0 +); +CREATE INDEX IF NOT EXISTS idx_savings_events_ts ON savings_events (ts); +CREATE INDEX IF NOT EXISTS idx_savings_events_tool ON savings_events (tool, ts); + +CREATE TABLE IF NOT EXISTS savings_totals ( + bucket TEXT NOT NULL PRIMARY KEY, + saved INTEGER NOT NULL DEFAULT 0, + returned INTEGER NOT NULL DEFAULT 0, + calls INTEGER NOT NULL DEFAULT 0 +) WITHOUT ROWID; + +CREATE TABLE IF NOT EXISTS savings_meta ( + key TEXT NOT NULL PRIMARY KEY, + value INTEGER NOT NULL DEFAULT 0 +) WITHOUT ROWID; ` // DefaultSidecarPath is the canonical location of the side-store DB: diff --git a/internal/savings/events.go b/internal/savings/events.go index 01ac7e8f..227fec5c 100644 --- a/internal/savings/events.go +++ b/internal/savings/events.go @@ -7,59 +7,33 @@ import ( "fmt" "io" "os" - "path/filepath" "sort" "strings" "time" ) -// Event is a single source-reading observation written to the JSONL log. -// One line per call; the dashboard reads the tail of the file to compute -// the Today / Last 7 days buckets the cumulative totals can't reconstruct -// on their own. +// Event is a single source-reading observation. The dashboard reads the +// event history (Store.EventsSince) to compute the Today / Last 7 days +// buckets the cumulative totals can't reconstruct on their own. // -// Fields use compact JSON keys to keep the on-disk log small — the log can -// grow to thousands of entries over a few weeks of heavy use. +// Fields use compact JSON keys — they are also the line schema of the +// flat-file era's savings.jsonl log, which LoadEvents still parses for +// the one-shot legacy import. type Event struct { - TS time.Time `json:"ts"` - Repo string `json:"repo,omitempty"` - Language string `json:"lang,omitempty"` - Tool string `json:"tool,omitempty"` - Returned int64 `json:"returned"` - Saved int64 `json:"saved"` + TS time.Time `json:"ts"` + SessionID string `json:"session,omitempty"` + Repo string `json:"repo,omitempty"` + Language string `json:"lang,omitempty"` + Tool string `json:"tool,omitempty"` + Returned int64 `json:"returned"` + Saved int64 `json:"saved"` } -// appendEvent serializes ev as a single JSON line and appends it to path, -// creating the file (and parent dir) when missing. O_APPEND makes the -// write atomic for sane line sizes on POSIX, so multiple writers don't -// corrupt each other's lines. -func appendEvent(path string, ev Event) error { - if path == "" { - return nil - } - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - return err - } - f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) - if err != nil { - return err - } - defer func() { _ = f.Close() }() - - data, err := json.Marshal(ev) - if err != nil { - return err - } - data = append(data, '\n') - _, err = f.Write(data) - return err -} - -// LoadEvents reads the JSONL log at path and returns events with ts >= -// since. since=zero returns everything. Returned events are in file -// order (oldest first). Malformed lines are skipped silently — they +// LoadEvents reads a flat-file era JSONL log at path and returns events +// with ts >= since. since=zero returns everything. Returned events are in +// file order (oldest first). Malformed lines are skipped silently — they // only happen when a previous gortex process crashed mid-write and the -// dashboard should keep working anyway. +// legacy import should keep working anyway. func LoadEvents(path string, since time.Time) ([]Event, error) { if path == "" { return nil, nil @@ -194,40 +168,26 @@ func FilterDay(events []Event, day time.Time, loc *time.Location) []Event { } // BuildDashboard returns the three canonical buckets (Today / Last 7 days / -// All time) using `now` as the reference clock and `loc` as the calendar -// for the "Today" boundary. Pass eventsPath="" or a missing file to skip -// reading the event log — All time then still works via storeAllTime. -func BuildDashboard(eventsPath string, storeAllTime Totals, now time.Time, loc *time.Location) ([]Bucket, error) { +// All time) from the full event history (oldest first), using `now` as the +// reference clock and `loc` as the calendar for the "Today" boundary. +// storeAllTime supplies the All-time totals — it can exceed what the events +// reconstruct when history predates the event log (flat-file era imports). +func BuildDashboard(events []Event, storeAllTime Totals, now time.Time, loc *time.Location) []Bucket { if loc == nil { loc = time.Local } - // Lower bound for events we care about — anything older than 7 days - // is irrelevant to Today/7d, and All time comes from storeAllTime. - weekStart := now.Add(-7 * 24 * time.Hour) - events, err := LoadEvents(eventsPath, weekStart) - if err != nil { - return nil, err - } + weekEvents := FilterSince(events, now.Add(-7*24*time.Hour)) - todayEvents := FilterDay(events, now, loc) + todayEvents := FilterDay(weekEvents, now, loc) todayTotals, todayPerTool := AggregateByTool(todayEvents) - weekTotals, weekPerTool := AggregateByTool(events) - - // All time per-tool requires a full scan — only do it when the log - // is reasonably small. The dashboard skips per-tool for All time when - // the log doesn't cover the full history (start_tracking < first - // event line) since the breakdown would be misleading. - allEvents, err := LoadEvents(eventsPath, time.Time{}) - if err != nil { - return nil, err - } - _, allPerTool := AggregateByTool(allEvents) + weekTotals, weekPerTool := AggregateByTool(weekEvents) + _, allPerTool := AggregateByTool(events) return []Bucket{ {Label: "Today", Totals: todayTotals, PerTool: todayPerTool}, {Label: "Last 7 days", Totals: weekTotals, PerTool: weekPerTool}, {Label: "All time", Totals: storeAllTime, PerTool: allPerTool}, - }, nil + } } // SavingsPercent returns the percentage of "full file size" tokens that diff --git a/internal/savings/events_test.go b/internal/savings/events_test.go index 913dcbf8..5ab1bfcd 100644 --- a/internal/savings/events_test.go +++ b/internal/savings/events_test.go @@ -1,6 +1,7 @@ package savings import ( + "encoding/json" "os" "path/filepath" "strings" @@ -9,6 +10,24 @@ import ( "time" ) +// appendLegacyEventLine writes one JSONL line the way the flat-file era +// did — test fixture for the legacy reader. +func appendLegacyEventLine(t *testing.T, path string, ev Event) { + t.Helper() + data, err := json.Marshal(ev) + if err != nil { + t.Fatal(err) + } + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + if _, err := f.Write(append(data, '\n')); err != nil { + t.Fatal(err) + } +} + func TestEventsPathFor(t *testing.T) { cases := []struct { in string @@ -27,37 +46,27 @@ func TestEventsPathFor(t *testing.T) { } } -func TestAddObservation_AppendsEventLine(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - - s, err := Open(path) +func TestAddObservation_RecordsEvent(t *testing.T) { + s, err := Open(filepath.Join(t.TempDir(), "sidecar.sqlite")) if err != nil { t.Fatal(err) } - s.AddObservation("/repo-a", "go", "get_symbol_source", 100, 900) - s.AddObservation("/repo-b", "ts", "batch_symbols", 50, 50) + s.AddObservation(Observation{Repo: "/repo-a", Language: "go", Tool: "get_symbol_source", SessionID: "sess-1", Returned: 100, Saved: 900}) + s.AddObservation(Observation{Repo: "/repo-b", Language: "ts", Tool: "batch_symbols", Returned: 50, Saved: 50}) - logPath := EventsPathFor(path) - data, err := os.ReadFile(logPath) - if err != nil { - t.Fatalf("expected event log at %s: %v", logPath, err) - } - lines := strings.Split(strings.TrimRight(string(data), "\n"), "\n") - if got, want := len(lines), 2; got != want { - t.Fatalf("event log line count = %d, want %d\n%s", got, want, data) - } - - evs, err := LoadEvents(logPath, time.Time{}) + evs, err := s.EventsSince(time.Time{}) if err != nil { t.Fatal(err) } if got, want := len(evs), 2; got != want { - t.Fatalf("LoadEvents = %d, want %d", got, want) + t.Fatalf("EventsSince = %d, want %d", got, want) } if evs[0].Tool != "get_symbol_source" || evs[0].Saved != 900 || evs[0].Repo != "/repo-a" { t.Errorf("event 0 = %+v", evs[0]) } + if evs[0].SessionID != "sess-1" { + t.Errorf("event 0 session = %q, want sess-1", evs[0].SessionID) + } if evs[1].Tool != "batch_symbols" || evs[1].Returned != 50 || evs[1].Language != "ts" { t.Errorf("event 1 = %+v", evs[1]) } @@ -73,9 +82,7 @@ func TestLoadEvents_FiltersSince(t *testing.T) { now.Add(-24 * time.Hour), now.Add(-1 * time.Hour), } { - if err := appendEvent(path, Event{TS: ts, Tool: "t", Saved: int64(i + 1) * 10}); err != nil { - t.Fatal(err) - } + appendLegacyEventLine(t, path, Event{TS: ts, Tool: "t", Saved: int64(i+1) * 10}) } // since = now - 25h should drop the 48h-old one. @@ -188,29 +195,23 @@ func TestFilterDay_LocalAndUTC(t *testing.T) { } func TestBuildDashboard_BucketsByWindow(t *testing.T) { - dir := t.TempDir() - logPath := filepath.Join(dir, "savings.jsonl") - now := time.Date(2026, 5, 18, 12, 0, 0, 0, time.UTC) // 3 today events, 2 within 7d but not today, 1 outside 7d. - mustAppend := func(ts time.Time, tool string, saved int64) { - if err := appendEvent(logPath, Event{TS: ts, Tool: tool, Saved: saved, Returned: 10}); err != nil { - t.Fatal(err) - } + mk := func(ts time.Time, tool string, saved int64) Event { + return Event{TS: ts, Tool: tool, Saved: saved, Returned: 10} + } + events := []Event{ + mk(now.Add(-30*24*time.Hour), "old_tool", 9999), // outside 7d + mk(now.Add(-5*24*time.Hour), "smart_context", 300), + mk(now.Add(-3*24*time.Hour), "smart_context", 200), + mk(now.Add(-3*time.Hour), "get_symbol_source", 150), + mk(now.Add(-2*time.Hour), "get_symbol_source", 100), + mk(now.Add(-30*time.Minute), "batch_symbols", 50), } - mustAppend(now.Add(-2*time.Hour), "get_symbol_source", 100) - mustAppend(now.Add(-3*time.Hour), "get_symbol_source", 150) - mustAppend(now.Add(-30*time.Minute), "batch_symbols", 50) - mustAppend(now.Add(-3*24*time.Hour), "smart_context", 200) - mustAppend(now.Add(-5*24*time.Hour), "smart_context", 300) - mustAppend(now.Add(-30*24*time.Hour), "old_tool", 9999) // outside 7d all := Totals{TokensSaved: 9999 + 300 + 200 + 50 + 150 + 100, TokensReturned: 60, CallsCounted: 6} - buckets, err := BuildDashboard(logPath, all, now, time.UTC) - if err != nil { - t.Fatal(err) - } + buckets := BuildDashboard(events, all, now, time.UTC) if len(buckets) != 3 { t.Fatalf("buckets = %d, want 3", len(buckets)) } @@ -242,13 +243,13 @@ func TestBuildDashboard_BucketsByWindow(t *testing.T) { t.Errorf("buckets[2].Label = %q, want All time", all2.Label) } if all2.Totals != all { - t.Errorf("All time totals = %+v, want %+v (from store, not log)", all2.Totals, all) + t.Errorf("All time totals = %+v, want %+v (from store, not events)", all2.Totals, all) } - // All time per-tool comes from a full log scan, so the 30-day-old + // All time per-tool covers the full history, so the 30-day-old // event is included. gotTools := map[string]bool{} - for _, t := range all2.PerTool { - gotTools[t.Tool] = true + for _, tt := range all2.PerTool { + gotTools[tt.Tool] = true } if !gotTools["old_tool"] { t.Errorf("All-time per-tool missing the >7d event (got %v)", gotTools) @@ -309,38 +310,8 @@ func TestBarString_DefaultsCellsTo16(t *testing.T) { } } -func TestReset_RemovesEventLog(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - - s, err := Open(path) - if err != nil { - t.Fatal(err) - } - s.AddObservation("/r", "go", "get_symbol_source", 10, 100) - _ = s.Flush() - - logPath := EventsPathFor(path) - if _, err := os.Stat(logPath); err != nil { - t.Fatalf("expected event log at %s before reset: %v", logPath, err) - } - - if err := s.Reset(); err != nil { - t.Fatalf("Reset: %v", err) - } - if _, err := os.Stat(logPath); !os.IsNotExist(err) { - t.Errorf("event log should be removed after Reset, got err=%v", err) - } - if _, err := os.Stat(path); !os.IsNotExist(err) { - t.Errorf("cumulative file should be removed after Reset, got err=%v", err) - } -} - -func TestAddObservation_EventLogIsConcurrentSafe(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - - s, err := Open(path) +func TestEvents_ConcurrentWritersAllRecorded(t *testing.T) { + s, err := Open(filepath.Join(t.TempDir(), "sidecar.sqlite")) if err != nil { t.Fatal(err) } @@ -351,29 +322,35 @@ func TestAddObservation_EventLogIsConcurrentSafe(t *testing.T) { for range writers { wg.Go(func() { for range per { - s.AddObservation("/r", "go", "tool", 1, 10) + s.AddObservation(Observation{Repo: "/r", Language: "go", Tool: "tool", Returned: 1, Saved: 10}) } }) } wg.Wait() - evs, err := LoadEvents(EventsPathFor(path), time.Time{}) + evs, err := s.EventsSince(time.Time{}) if err != nil { t.Fatal(err) } if got, want := len(evs), writers*per; got != want { - t.Errorf("LoadEvents = %d, want %d (line interleaving lost events)", got, want) + t.Errorf("EventsSince = %d, want %d (concurrent observation lost)", got, want) } } -func TestEventLog_DisabledForInMemoryStore(t *testing.T) { +func TestEventHistory_InMemoryStoreStaysInMemory(t *testing.T) { s, err := Open("") if err != nil { t.Fatal(err) } - s.AddObservation("/r", "go", "tool", 1, 10) - // No path → no events file should be written anywhere. - if got := s.eventsPath; got != "" { - t.Errorf("in-memory store should have empty eventsPath, got %q", got) + s.AddObservation(Observation{Repo: "/r", Language: "go", Tool: "tool", Returned: 1, Saved: 10}) + if s.sc != nil { + t.Fatal("empty-path store must not open a sidecar") + } + evs, err := s.EventsSince(time.Time{}) + if err != nil { + t.Fatal(err) + } + if len(evs) != 1 { + t.Errorf("in-memory event history = %d, want 1", len(evs)) } } diff --git a/internal/savings/store.go b/internal/savings/store.go index a1380f83..5c3301f3 100644 --- a/internal/savings/store.go +++ b/internal/savings/store.go @@ -3,12 +3,14 @@ // server's tokenStats, so over time the numbers become a credible narrative: // "Gortex saved N tokens / $X at model rate this month". // -// Storage format: a single JSON file under the user cache dir (or the -// configured cache dir). Atomic writes via temp-file + rename, with an -// advisory file lock on a sidecar `.lock` file so multiple gortex processes -// (e.g. a daemon and a parallel `gortex mcp`) can write to the same -// savings file without clobbering each other's deltas. Falls back to an -// in-memory-only store when the path isn't writable. +// Storage: the machine-global SQLite sidecar (~/.gortex/sidecar.sqlite — +// the same database that holds notes, memories, scopes, and notebooks). +// Every observation is one transaction (event row + totals upsert), so the +// ledger is durable at the call: a SIGKILLed server loses nothing, and +// multiple gortex processes write the same database safely through SQLite's +// WAL + busy-timeout. The flat-file era (savings.json cumulative totals + +// savings.jsonl event log under the cache dir) is imported once on open and +// the legacy files renamed to *.bak. package savings import ( @@ -20,17 +22,14 @@ import ( "sync" "time" - "github.com/gofrs/flock" - + "github.com/zzet/gortex/internal/persistence" "github.com/zzet/gortex/internal/platform" ) -const ( - // schemaVersion lets future changes migrate or reject incompatible files. - schemaVersion = 1 - // flushEvery buffers this many observations before writing to disk. - flushEvery = 20 -) +// schemaVersion is the snapshot-shape version, kept for the JSON surface +// (graph_stats cumulative_savings, `gortex savings --json`) and for +// reading flat-file-era ledgers during legacy import. +const schemaVersion = 1 // Totals is the cumulative record for a single scope (top-level or per-repo). type Totals struct { @@ -39,9 +38,9 @@ type Totals struct { CallsCounted int64 `json:"calls_counted"` } -// File is the on-disk schema. Older files without `per_language` load -// cleanly — JSON unmarshal leaves the missing field as a nil map and -// the next write upgrades it. +// File is the snapshot shape callers consume (graph_stats, the CLI) and +// the on-disk schema of the flat-file era — still parsed by the one-shot +// legacy import. type File struct { Version int `json:"version"` FirstSeen time.Time `json:"first_seen"` @@ -51,44 +50,46 @@ type File struct { PerLanguage map[string]*Totals `json:"per_language,omitempty"` } -// Store holds the cumulative savings state and flushes to disk periodically. -// All operations are safe for concurrent use. When path is empty the store -// still tracks in-memory but never writes to disk. +// Observation is one source-reading tool call to book. +type Observation struct { + Repo string + Language string + Tool string + SessionID string + Returned int64 + Saved int64 +} + +// Store is the token-savings ledger. All operations are safe for +// concurrent use. When opened with an empty path the store tracks +// in-memory only — the behaviour test fixtures and the eval servers +// rely on — and never touches disk. // -// Concurrency model: in-process callers serialize through s.mu. Cross-process -// safety is achieved on flush by acquiring an advisory flock on a sidecar -// lock file, re-reading the on-disk totals, merging this process's pending -// delta, and writing back. A second process that flushed in between just -// shifts our baseline up; nothing is lost. +// Write errors against the sidecar are intentionally not propagated to +// record() callers (accounting must never fail a tool call), but unlike +// the flat-file era there is no batching: an observation either commits +// durably or is dropped whole. type Store struct { mu sync.Mutex - path string - file File - delta Totals // cumulative this-process contributions since last successful flush - perDelta map[string]*Totals // per-repo deltas, same semantics as delta - langDelta map[string]*Totals // per-language deltas - pending int // observations since last flush - - // eventsPath is the JSONL event log derived from `path`. Each - // AddObservation appends one line so the `gortex savings` dashboard - // can compute Today / Last 7 days windows the cumulative file can't. - // Empty when the store is in-memory only or when disabled. - eventsPath string - eventsDisabled bool + sc *persistence.SidecarStore + mem File // in-memory accumulation when sc == nil + memEvents []Event // in-memory event log when sc == nil +} - // stop signals the optional periodic flusher to exit. Nil when no - // flusher is running. - stopOnce sync.Once - stopCh chan struct{} +// DefaultDBPath returns the canonical savings ledger location: the +// machine-global sidecar database under the Gortex data dir. +func DefaultDBPath() string { + return persistence.DefaultSidecarPath(platform.DataDir()) } -// DefaultPath returns the canonical savings.json location under the -// Gortex cache dir. +// DefaultPath returns the flat-file era's savings.json location under the +// Gortex cache dir. The live ledger no longer writes it — the path is the +// default source for the one-shot legacy import (see Store.ImportLegacy). // // An absolute $XDG_CACHE_HOME is honoured; otherwise the location stays // under os.UserCacheDir() — the historical default for this store, kept // so an existing savings file is not orphaned. Returns an empty string -// (i.e. "disable persistence") when no cache dir can be resolved. +// when no cache dir can be resolved. func DefaultPath() string { if v := os.Getenv("XDG_CACHE_HOME"); v == "" || !filepath.IsAbs(v) { if base, err := os.UserCacheDir(); err != nil || base == "" { @@ -98,8 +99,8 @@ func DefaultPath() string { return filepath.Join(platform.OSCacheDir(), "savings.json") } -// DefaultEventsPath returns the canonical savings.jsonl event-log path next -// to DefaultPath. Empty when the cache dir is unavailable. +// DefaultEventsPath returns the flat-file era's savings.jsonl event-log +// path next to DefaultPath. Empty when the cache dir is unavailable. func DefaultEventsPath() string { p := DefaultPath() if p == "" { @@ -109,8 +110,8 @@ func DefaultEventsPath() string { } // EventsPathFor returns the JSONL event-log path that corresponds to a -// cumulative savings JSON path — `/savings.jsonl` alongside the JSON -// file. Empty when storePath is empty. +// flat-file cumulative savings JSON path — `/savings.jsonl` alongside +// the JSON file. Empty when storePath is empty. func EventsPathFor(storePath string) string { if storePath == "" { return "" @@ -125,95 +126,66 @@ func EventsPathFor(storePath string) string { return filepath.Join(dir, stem+".jsonl") } -// Open loads savings from path, or returns an empty Store when the file -// doesn't exist yet. Corrupt or incompatible files are backed up to -// `.corrupt-` and replaced with a fresh state so a bad write can't -// permanently break metrics. -func Open(path string) (*Store, error) { - s := &Store{ - path: path, - eventsPath: EventsPathFor(path), - perDelta: make(map[string]*Totals), - langDelta: make(map[string]*Totals), - } - s.file.Version = schemaVersion - s.file.FirstSeen = time.Now().UTC() - s.file.PerRepo = make(map[string]*Totals) - s.file.PerLanguage = make(map[string]*Totals) - - if path == "" { +// Open opens the savings ledger inside the sidecar database at dbPath +// (creating tables as needed). An empty dbPath yields an in-memory-only +// store. The sidecar handle is process-shared: opening the same path the +// notes/memories managers use reuses their connection. +func Open(dbPath string) (*Store, error) { + s := &Store{} + s.mem = emptyFile() + if dbPath == "" { return s, nil } - - loaded, err := readFile(path) + sc, err := persistence.OpenSidecar(dbPath) if err != nil { - return s, err - } - if loaded != nil { - s.file = *loaded - // Older files have no per_language section; fill in so callers - // don't need a nil check. - if s.file.PerLanguage == nil { - s.file.PerLanguage = make(map[string]*Totals) - } + return nil, fmt.Errorf("open savings ledger: %w", err) } + s.sc = sc return s, nil } -// readFile loads the savings file at path. Returns (nil, nil) when the file -// doesn't exist or is corrupt (in which case it's renamed to a .corrupt-N -// sidecar so future opens get a clean slate). -func readFile(path string) (*File, error) { - data, err := os.ReadFile(path) - if errors.Is(err, os.ErrNotExist) { - return nil, nil - } - if err != nil { - return nil, fmt.Errorf("read savings: %w", err) - } - - var loaded File - if jerr := json.Unmarshal(data, &loaded); jerr != nil || loaded.Version != schemaVersion { - backup := fmt.Sprintf("%s.corrupt-%d", path, time.Now().Unix()) - _ = os.Rename(path, backup) - return nil, nil - } - if loaded.PerRepo == nil { - loaded.PerRepo = make(map[string]*Totals) - } - if loaded.PerLanguage == nil { - loaded.PerLanguage = make(map[string]*Totals) +func emptyFile() File { + return File{ + Version: schemaVersion, + PerRepo: make(map[string]*Totals), + PerLanguage: make(map[string]*Totals), } - return &loaded, nil } -// AddObservation increments the store by one source-reading tool call. -// repoPath and language, when non-empty, also aggregate under per-repo -// and per-language buckets respectively. tool, when non-empty, is recorded -// in the JSONL event log so the dashboard can build a per-tool breakdown -// in --verbose mode. Writes to disk every flushEvery observations. -func (s *Store) AddObservation(repoPath, language, tool string, returned, saved int64) { +// AddObservation books one source-reading tool call. Durable immediately +// when the store is sidecar-backed; in-memory otherwise. +func (s *Store) AddObservation(o Observation) { if s == nil { return } + if o.Saved < 0 { + o.Saved = 0 + } now := time.Now().UTC() - s.mu.Lock() - - if saved < 0 { - saved = 0 + if s.sc != nil { + _ = s.sc.AddSavingsObservation(persistence.SavingsEvent{ + TS: now, + SessionID: o.SessionID, + Tool: o.Tool, + Repo: o.Repo, + Language: o.Language, + Returned: o.Returned, + Saved: o.Saved, + }) + return } - s.file.Totals.TokensSaved += saved - s.file.Totals.TokensReturned += returned - s.file.Totals.CallsCounted++ - s.file.LastUpdated = now - - s.delta.TokensSaved += saved - s.delta.TokensReturned += returned - s.delta.CallsCounted++ - - addBucket := func(bucket map[string]*Totals, deltaBucket map[string]*Totals, key string) { + s.mu.Lock() + defer s.mu.Unlock() + if s.mem.FirstSeen.IsZero() { + s.mem.FirstSeen = now + } + s.mem.LastUpdated = now + s.mem.Totals.TokensSaved += o.Saved + s.mem.Totals.TokensReturned += o.Returned + s.mem.Totals.CallsCounted++ + addBucket := func(bucket map[string]*Totals, key string) { if key == "" { return } @@ -222,254 +194,227 @@ func (s *Store) AddObservation(repoPath, language, tool string, returned, saved t = &Totals{} bucket[key] = t } - t.TokensSaved += saved - t.TokensReturned += returned + t.TokensSaved += o.Saved + t.TokensReturned += o.Returned t.CallsCounted++ - - dt := deltaBucket[key] - if dt == nil { - dt = &Totals{} - deltaBucket[key] = dt - } - dt.TokensSaved += saved - dt.TokensReturned += returned - dt.CallsCounted++ - } - addBucket(s.file.PerRepo, s.perDelta, repoPath) - addBucket(s.file.PerLanguage, s.langDelta, language) - - s.pending++ - flushCumulative := s.pending >= flushEvery - eventsPath := s.eventsPath - eventsDisabled := s.eventsDisabled - s.mu.Unlock() - - // Best-effort JSONL append outside the store mutex — the file - // handle is opened with O_APPEND so concurrent writers from this - // or other gortex processes interleave at line boundaries. - if !eventsDisabled && eventsPath != "" { - _ = appendEvent(eventsPath, Event{ - TS: now, - Repo: repoPath, - Language: language, - Tool: tool, - Returned: returned, - Saved: saved, - }) - } - - if flushCumulative { - s.mu.Lock() - _ = s.flushLocked() - s.mu.Unlock() } + addBucket(s.mem.PerRepo, o.Repo) + addBucket(s.mem.PerLanguage, o.Language) + s.memEvents = append(s.memEvents, Event{ + TS: now, + SessionID: o.SessionID, + Repo: o.Repo, + Language: o.Language, + Tool: o.Tool, + Returned: o.Returned, + Saved: o.Saved, + }) } -// Snapshot returns a deep copy of the current totals (safe for reads outside -// the mutex). Used by graph_stats and the CLI. +// Snapshot returns the current cumulative totals. Sidecar-backed stores +// read the live aggregates, so the snapshot reflects every writer process, +// not just this one. FirstSeen stays the zero time until something has +// actually been recorded — callers must not present it as "tracking since" +// when it is zero. func (s *Store) Snapshot() File { if s == nil { - return File{Version: schemaVersion, PerRepo: map[string]*Totals{}} + return emptyFile() } - s.mu.Lock() - defer s.mu.Unlock() - - cp := s.file - cp.PerRepo = make(map[string]*Totals, len(s.file.PerRepo)) - for k, v := range s.file.PerRepo { - t := *v - cp.PerRepo[k] = &t + if s.sc == nil { + s.mu.Lock() + defer s.mu.Unlock() + cp := s.mem + cp.PerRepo = copyTotalsMap(s.mem.PerRepo) + cp.PerLanguage = copyTotalsMap(s.mem.PerLanguage) + return cp } - return cp -} -// Flush writes pending observations to disk. Safe to call when no path is -// configured (no-op). -func (s *Store) Flush() error { - if s == nil { - return nil + buckets, firstSeen, lastUpdated, err := s.sc.SavingsTotals() + if err != nil { + return emptyFile() + } + out := emptyFile() + out.FirstSeen = firstSeen + out.LastUpdated = lastUpdated + for bucket, r := range buckets { + t := &Totals{TokensSaved: r.Saved, TokensReturned: r.Returned, CallsCounted: r.Calls} + switch { + case bucket == "": + out.Totals = *t + case len(bucket) > 5 && bucket[:5] == "repo:": + out.PerRepo[bucket[5:]] = t + case len(bucket) > 5 && bucket[:5] == "lang:": + out.PerLanguage[bucket[5:]] = t + } } - s.mu.Lock() - defer s.mu.Unlock() - return s.flushLocked() + return out } -// Pending reports whether the store has unflushed observations. Lets a -// background ticker skip the flock+IO when nothing has happened. -func (s *Store) Pending() bool { +// EventsSince returns the per-call events with TS >= since, oldest first. +// since=zero returns the full event history. +func (s *Store) EventsSince(since time.Time) ([]Event, error) { if s == nil { - return false - } - s.mu.Lock() - defer s.mu.Unlock() - return s.pending > 0 -} - -// StartPeriodicFlush kicks off a goroutine that flushes the store every -// interval if there are pending observations. Returns a stop function the -// caller should invoke at shutdown to terminate the ticker. Calling -// StartPeriodicFlush more than once on the same Store is a no-op for the -// extra calls (returns a no-op stopper). -// -// The point of the periodic flusher is to bound on-crash data loss to -// roughly `interval` worth of observations even when the call rate is too -// low to trip the every-N-observations flush. -func (s *Store) StartPeriodicFlush(interval time.Duration) func() { - if s == nil || interval <= 0 { - return func() {} + return nil, nil } - - s.mu.Lock() - if s.stopCh != nil { - s.mu.Unlock() - return func() {} + if s.sc == nil { + s.mu.Lock() + defer s.mu.Unlock() + return FilterSince(s.memEvents, since), nil } - stop := make(chan struct{}) - s.stopCh = stop - s.mu.Unlock() - - go func() { - t := time.NewTicker(interval) - defer t.Stop() - for { - select { - case <-stop: - return - case <-t.C: - if s.Pending() { - _ = s.Flush() - } - } - } - }() - - return func() { - s.stopOnce.Do(func() { close(stop) }) + rows, err := s.sc.SavingsEventsSince(since) + if err != nil { + return nil, err + } + out := make([]Event, 0, len(rows)) + for _, r := range rows { + out = append(out, Event{ + TS: r.TS, + SessionID: r.SessionID, + Repo: r.Repo, + Language: r.Language, + Tool: r.Tool, + Returned: r.Returned, + Saved: r.Saved, + }) } + return out, nil } -// Reset wipes all cumulative data and removes the persisted file. Used by -// `gortex savings --reset`. +// Flush is a no-op kept for call-site compatibility: every observation +// commits durably as it is recorded, so there is nothing to flush. +func (s *Store) Flush() error { return nil } + +// Reset wipes all cumulative data and events. Used by +// `gortex savings --reset`. The legacy-import mark survives, so flat +// files already imported (and renamed *.bak) are not re-imported. func (s *Store) Reset() error { if s == nil { return nil } s.mu.Lock() - defer s.mu.Unlock() - - s.file = File{ - Version: schemaVersion, - FirstSeen: time.Now().UTC(), - PerRepo: make(map[string]*Totals), - PerLanguage: make(map[string]*Totals), - } - s.delta = Totals{} - s.perDelta = make(map[string]*Totals) - s.langDelta = make(map[string]*Totals) - s.pending = 0 - - if s.path == "" { + s.mem = emptyFile() + s.memEvents = nil + s.mu.Unlock() + if s.sc == nil { return nil } - if err := os.Remove(s.path); err != nil && !errors.Is(err, os.ErrNotExist) { - return err - } - if s.eventsPath != "" { - if err := os.Remove(s.eventsPath); err != nil && !errors.Is(err, os.ErrNotExist) { - return err - } - } - return nil + return s.sc.ResetSavings() } -// flushLocked must be called with s.mu held. -// -// The flush is cross-process safe: an advisory flock on `.lock` -// serializes with other gortex processes writing the same file. Inside the -// critical section we re-read the on-disk file, add this process's pending -// deltas to it, and write back. That way two parallel writers each get -// their contributions persisted instead of last-flusher-wins. -func (s *Store) flushLocked() error { - if s.path == "" { - s.pending = 0 +// ImportLegacy imports the flat-file era's ledger — the cumulative +// savings.json at jsonPath and its sibling savings.jsonl — into the +// sidecar, then renames both files to *.bak. Idempotent: a migration +// mark guarantees the import runs once per sidecar, including when +// there was nothing to import. In-memory stores skip the import. +func (s *Store) ImportLegacy(jsonPath string) error { + if s == nil || s.sc == nil || jsonPath == "" { return nil } - if err := os.MkdirAll(filepath.Dir(s.path), 0o755); err != nil { - return err - } - - lockPath := s.path + ".lock" - fl := flock.New(lockPath) - if err := fl.Lock(); err != nil { - return fmt.Errorf("acquire savings lock: %w", err) + if s.sc.SavingsLegacyImportDone() { + return nil } - defer func() { _ = fl.Unlock() }() - // Re-read whatever's on disk now — another process may have flushed - // since we last loaded. Merge our deltas onto that baseline. - merged, err := readFile(s.path) + legacy, err := readLegacyFile(jsonPath) if err != nil { return err } - if merged == nil { - // File missing (or was just backed up as corrupt). Start fresh - // from our in-memory baseline — s.file already includes both - // any value loaded at Open time and everything observed in this - // process, so don't re-add the delta on top. Deep-copy maps so - // the merged copy doesn't alias s.file's maps. - fresh := s.file - fresh.PerRepo = copyTotalsMap(s.file.PerRepo) - fresh.PerLanguage = copyTotalsMap(s.file.PerLanguage) - merged = &fresh - } else { - mergeTotals(&merged.Totals, &s.delta) - if merged.PerRepo == nil { - merged.PerRepo = make(map[string]*Totals) + eventsPath := EventsPathFor(jsonPath) + events, _ := LoadEvents(eventsPath, time.Time{}) + + buckets := make(map[string]persistence.SavingsTotalsRow) + var firstSeen, lastUpdated time.Time + switch { + case legacy != nil: + buckets[""] = totalsRow(legacy.Totals) + for k, v := range legacy.PerRepo { + buckets["repo:"+k] = totalsRow(*v) } - if merged.PerLanguage == nil { - merged.PerLanguage = make(map[string]*Totals) + for k, v := range legacy.PerLanguage { + buckets["lang:"+k] = totalsRow(*v) } - mergeBucketDeltas(merged.PerRepo, s.perDelta) - mergeBucketDeltas(merged.PerLanguage, s.langDelta) - if merged.FirstSeen.IsZero() || s.file.FirstSeen.Before(merged.FirstSeen) { - merged.FirstSeen = s.file.FirstSeen + firstSeen, lastUpdated = legacy.FirstSeen, legacy.LastUpdated + case len(events) > 0: + // Event log without a cumulative file (e.g. the cumulative + // flush never ran before the process died): rebuild the + // totals the file would have carried. + for _, ev := range events { + bump := func(bucket string) { + r := buckets[bucket] + r.Saved += ev.Saved + r.Returned += ev.Returned + r.Calls++ + buckets[bucket] = r + } + bump("") + if ev.Repo != "" { + bump("repo:" + ev.Repo) + } + if ev.Language != "" { + bump("lang:" + ev.Language) + } } - merged.LastUpdated = time.Now().UTC() - merged.Version = schemaVersion + firstSeen = events[0].TS + lastUpdated = events[len(events)-1].TS + } + + pevents := make([]persistence.SavingsEvent, 0, len(events)) + for _, ev := range events { + pevents = append(pevents, persistence.SavingsEvent{ + TS: ev.TS, + SessionID: ev.SessionID, + Tool: ev.Tool, + Repo: ev.Repo, + Language: ev.Language, + Returned: ev.Returned, + Saved: ev.Saved, + }) } - - // Atomic write: temp file in the same dir, then rename. - tmp, err := os.CreateTemp(filepath.Dir(s.path), ".savings-*.tmp") - if err != nil { + if err := s.sc.ImportLegacySavings(buckets, firstSeen, lastUpdated, pevents); err != nil { return err } - tmpName := tmp.Name() + renameLegacySavings(jsonPath) + renameLegacySavings(eventsPath) + return nil +} - enc := json.NewEncoder(tmp) - enc.SetIndent("", " ") - if err := enc.Encode(merged); err != nil { - _ = tmp.Close() - _ = os.Remove(tmpName) - return err - } - if err := tmp.Close(); err != nil { - _ = os.Remove(tmpName) - return err +func totalsRow(t Totals) persistence.SavingsTotalsRow { + return persistence.SavingsTotalsRow{Saved: t.TokensSaved, Returned: t.TokensReturned, Calls: t.CallsCounted} +} + +// renameLegacySavings moves an already-imported flat file aside to +// .bak. Best-effort — never deletes; a missing file is fine. +func renameLegacySavings(path string) { + if path == "" { + return } - if err := os.Rename(tmpName, s.path); err != nil { - _ = os.Remove(tmpName) - return err + if _, err := os.Stat(path); err != nil { + return } + _ = os.Rename(path, path+".bak") +} - // Adopt the merged view as our authoritative in-memory state and - // clear the delta — anything new arriving after this point will be - // what we commit on the next flush. - s.file = *merged - s.delta = Totals{} - s.perDelta = make(map[string]*Totals) - s.langDelta = make(map[string]*Totals) - s.pending = 0 - return nil +// readLegacyFile loads a flat-file era savings.json. Returns (nil, nil) +// when the file doesn't exist; corrupt or version-mismatched files are +// skipped the same way (the import has nothing trustworthy to carry over). +func readLegacyFile(path string) (*File, error) { + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("read legacy savings: %w", err) + } + var loaded File + if jerr := json.Unmarshal(data, &loaded); jerr != nil || loaded.Version != schemaVersion { + return nil, nil + } + if loaded.PerRepo == nil { + loaded.PerRepo = make(map[string]*Totals) + } + if loaded.PerLanguage == nil { + loaded.PerLanguage = make(map[string]*Totals) + } + return &loaded, nil } // copyTotalsMap returns a deep copy of a name → Totals map. @@ -484,22 +429,3 @@ func copyTotalsMap(src map[string]*Totals) map[string]*Totals { } return dst } - -// mergeBucketDeltas folds the per-process delta map into the merged map -// (which represents the on-disk baseline + this process's contributions). -func mergeBucketDeltas(merged, deltas map[string]*Totals) { - for k, dt := range deltas { - t := merged[k] - if t == nil { - t = &Totals{} - merged[k] = t - } - mergeTotals(t, dt) - } -} - -func mergeTotals(dst, src *Totals) { - dst.TokensSaved += src.TokensSaved - dst.TokensReturned += src.TokensReturned - dst.CallsCounted += src.CallsCounted -} diff --git a/internal/savings/store_test.go b/internal/savings/store_test.go index 105ed651..8805bd46 100644 --- a/internal/savings/store_test.go +++ b/internal/savings/store_test.go @@ -10,25 +10,28 @@ import ( "time" ) +// testLedgerPath returns a fresh sidecar DB path. Each test gets its own +// file so the process-shared sidecar handle cache can't leak state +// between tests. +func testLedgerPath(t *testing.T) string { + t.Helper() + return filepath.Join(t.TempDir(), "sidecar.sqlite") +} + func TestAddObservation_PerLanguageBucket(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") + path := testLedgerPath(t) s, err := Open(path) if err != nil { t.Fatal(err) } - s.AddObservation("/repo-a", "go", "get_symbol_source", 100, 200) - s.AddObservation("/repo-a", "go", "get_symbol_source", 50, 80) - s.AddObservation("/repo-b", "typescript", "batch_symbols", 30, 70) + s.AddObservation(Observation{Repo: "/repo-a", Language: "go", Tool: "get_symbol_source", Returned: 100, Saved: 200}) + s.AddObservation(Observation{Repo: "/repo-a", Language: "go", Tool: "get_symbol_source", Returned: 50, Saved: 80}) + s.AddObservation(Observation{Repo: "/repo-b", Language: "typescript", Tool: "batch_symbols", Returned: 30, Saved: 70}) // Empty language is allowed (e.g. record() called with a nil node); // it should accumulate in the totals but not in any per-language bucket. - s.AddObservation("/repo-c", "", "smart_context", 10, 20) - - if err := s.Flush(); err != nil { - t.Fatal(err) - } + s.AddObservation(Observation{Repo: "/repo-c", Tool: "smart_context", Returned: 10, Saved: 20}) reopened, err := Open(path) if err != nil { @@ -48,123 +51,42 @@ func TestAddObservation_PerLanguageBucket(t *testing.T) { if ts := snap.PerLanguage["typescript"]; ts == nil || ts.CallsCounted != 1 || ts.TokensSaved != 70 { t.Errorf("typescript bucket = %+v, want calls=1 saved=70", ts) } + if len(snap.PerRepo) != 3 { + t.Errorf("PerRepo size = %d, want 3", len(snap.PerRepo)) + } } -func TestStartPeriodicFlush_FlushesPendingObservations(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") +// The headline property of the sidecar-backed ledger: an observation is +// durable the moment it is recorded. No flush, no ticker, no graceful +// shutdown required — the failure mode that left the flat-file ledger +// permanently empty under SIGKILLing MCP clients. +func TestAddObservation_DurableImmediately(t *testing.T) { + path := testLedgerPath(t) s, err := Open(path) if err != nil { t.Fatal(err) } + s.AddObservation(Observation{Repo: "/r", Language: "go", Tool: "read_file", Returned: 10, Saved: 90}) - stop := s.StartPeriodicFlush(50 * time.Millisecond) - defer stop() - - s.AddObservation("/some/repo", "", "test", 100, 200) - if got := s.Snapshot().Totals.CallsCounted; got != 1 { - t.Fatalf("CallsCounted before flush = %d, want 1", got) - } - - // Wait for the ticker to fire and write to disk. - deadline := time.Now().Add(2 * time.Second) - for time.Now().Before(deadline) { - if _, err := os.Stat(path); err == nil { - break - } - time.Sleep(20 * time.Millisecond) - } if _, err := os.Stat(path); err != nil { - t.Fatalf("expected savings file to exist after periodic flush: %v", err) - } - - // Re-open in a fresh store and confirm the observation persisted. - s2, err := Open(path) - if err != nil { - t.Fatal(err) - } - if got := s2.Snapshot().Totals.CallsCounted; got != 1 { - t.Errorf("re-opened CallsCounted = %d, want 1", got) - } -} - -func TestStartPeriodicFlush_StopsCleanly(t *testing.T) { - s, err := Open("") - if err != nil { - t.Fatal(err) - } - stop := s.StartPeriodicFlush(10 * time.Millisecond) - stop() - stop() // idempotent — should not panic on double-close - - // Calling StartPeriodicFlush again after stop is currently a no-op - // (see comment on StartPeriodicFlush). This test pins that behavior. - stop2 := s.StartPeriodicFlush(10 * time.Millisecond) - stop2() -} - -func TestFlush_MergesConcurrentProcessDeltas(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - - // Simulate two independent processes writing to the same savings - // file. Both Open the empty file (baseline = 0), each adds its own - // observations, then both Flush. Without merge-on-flush the second - // flusher overwrites the first; with it, both contributions land. - a, err := Open(path) - if err != nil { - t.Fatal(err) + t.Fatalf("ledger DB must exist immediately after the first observation: %v", err) } - b, err := Open(path) - if err != nil { - t.Fatal(err) - } - - a.AddObservation("/repo-a", "", "test", 50, 150) - a.AddObservation("/repo-a", "", "test", 50, 150) - b.AddObservation("/repo-b", "", "test", 30, 70) - - if err := a.Flush(); err != nil { - t.Fatalf("a.Flush: %v", err) - } - if err := b.Flush(); err != nil { - t.Fatalf("b.Flush: %v", err) + snap := s.Snapshot() + if snap.Totals.CallsCounted != 1 || snap.Totals.TokensSaved != 90 { + t.Errorf("snapshot = %+v, want calls=1 saved=90 with no flush", snap.Totals) } - - // Re-open and verify merged totals = a's contribution + b's. - final, err := Open(path) + evs, err := s.EventsSince(time.Time{}) if err != nil { t.Fatal(err) } - snap := final.Snapshot() - - if got, want := snap.Totals.CallsCounted, int64(3); got != want { - t.Errorf("merged CallsCounted = %d, want %d", got, want) - } - if got, want := snap.Totals.TokensSaved, int64(150+150+70); got != want { - t.Errorf("merged TokensSaved = %d, want %d", got, want) - } - if got, want := snap.Totals.TokensReturned, int64(50+50+30); got != want { - t.Errorf("merged TokensReturned = %d, want %d", got, want) - } - if len(snap.PerRepo) != 2 { - t.Errorf("merged PerRepo size = %d, want 2", len(snap.PerRepo)) - } - if r := snap.PerRepo["/repo-a"]; r == nil || r.CallsCounted != 2 { - t.Errorf("repo-a calls = %v, want 2", r) - } - if r := snap.PerRepo["/repo-b"]; r == nil || r.CallsCounted != 1 { - t.Errorf("repo-b calls = %v, want 1", r) + if len(evs) != 1 || evs[0].Tool != "read_file" { + t.Errorf("events = %+v, want one read_file event", evs) } } -func TestFlush_FlockBlocksConcurrentWriters(t *testing.T) { - // Verify the flock is actually held: a second goroutine flushing - // shouldn't see partial state. We hammer two stores in parallel and - // expect the final on-disk total to equal the sum of contributions. - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") +func TestConcurrentWriters_SameLedger(t *testing.T) { + path := testLedgerPath(t) const perStore = 200 stores := make([]*Store, 4) @@ -182,77 +104,65 @@ func TestFlush_FlockBlocksConcurrentWriters(t *testing.T) { go func(s *Store, repo string) { defer wg.Done() for j := 0; j < perStore; j++ { - s.AddObservation(repo, "", "test", 1, 10) + s.AddObservation(Observation{Repo: repo, Tool: "test", Returned: 1, Saved: 10}) } - _ = s.Flush() }(s, "/repo-"+string(rune('a'+i))) } wg.Wait() - final, err := Open(path) - if err != nil { - t.Fatal(err) - } - snap := final.Snapshot() + snap := stores[0].Snapshot() wantCalls := int64(len(stores) * perStore) if got := snap.Totals.CallsCounted; got != wantCalls { - t.Errorf("merged CallsCounted = %d, want %d (delta lost across processes)", got, wantCalls) + t.Errorf("CallsCounted = %d, want %d (observation lost across writers)", got, wantCalls) + } + if got, want := snap.Totals.TokensSaved, wantCalls*10; got != want { + t.Errorf("TokensSaved = %d, want %d", got, want) + } + evs, err := stores[0].EventsSince(time.Time{}) + if err != nil { + t.Fatal(err) } - wantSaved := wantCalls * 10 - if got := snap.Totals.TokensSaved; got != wantSaved { - t.Errorf("merged TokensSaved = %d, want %d", got, wantSaved) + if got := int64(len(evs)); got != wantCalls { + t.Errorf("events = %d, want %d", got, wantCalls) } } -func TestOpen_MissingFile_ReturnsEmpty(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - - s, err := Open(path) +// A fresh ledger reports nothing — including a zero FirstSeen. The +// flat-file store seeded FirstSeen=now at Open, which made the dashboard +// print "tracking since " on a machine that +// had never recorded anything. +func TestOpen_FreshLedger_EmptySnapshot(t *testing.T) { + s, err := Open(testLedgerPath(t)) if err != nil { t.Fatalf("Open: %v", err) } snap := s.Snapshot() if snap.Totals.CallsCounted != 0 { - t.Errorf("new store has CallsCounted=%d, want 0", snap.Totals.CallsCounted) + t.Errorf("new ledger has CallsCounted=%d, want 0", snap.Totals.CallsCounted) + } + if !snap.FirstSeen.IsZero() { + t.Errorf("new ledger FirstSeen = %v, want zero time (nothing recorded yet)", snap.FirstSeen) } if snap.Version != schemaVersion { - t.Errorf("new store version=%d, want %d", snap.Version, schemaVersion) + t.Errorf("new ledger version=%d, want %d", snap.Version, schemaVersion) } } -func TestAddObservation_AccumulatesAndPersists(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - - s, err := Open(path) +func TestObservation_StampsFirstAndLastSeen(t *testing.T) { + s, err := Open(testLedgerPath(t)) if err != nil { t.Fatal(err) } - for range flushEvery + 5 { - s.AddObservation("/some/repo", "", "test", 100, 900) - } - if err := s.Flush(); err != nil { - t.Fatalf("Flush: %v", err) - } + before := time.Now().UTC().Add(-time.Second) + s.AddObservation(Observation{Tool: "test", Returned: 1, Saved: 1}) + after := time.Now().UTC().Add(time.Second) - // Re-open and verify totals survived the write. - s2, err := Open(path) - if err != nil { - t.Fatal(err) - } - snap := s2.Snapshot() - if got, want := snap.Totals.CallsCounted, int64(flushEvery+5); got != want { - t.Errorf("CallsCounted = %d, want %d", got, want) - } - if got, want := snap.Totals.TokensSaved, int64((flushEvery+5)*900); got != want { - t.Errorf("TokensSaved = %d, want %d", got, want) - } - if got, want := snap.Totals.TokensReturned, int64((flushEvery+5)*100); got != want { - t.Errorf("TokensReturned = %d, want %d", got, want) + snap := s.Snapshot() + if snap.FirstSeen.Before(before) || snap.FirstSeen.After(after) { + t.Errorf("FirstSeen = %v, want within [%v, %v]", snap.FirstSeen, before, after) } - if len(snap.PerRepo) != 1 { - t.Errorf("PerRepo size = %d, want 1", len(snap.PerRepo)) + if snap.LastUpdated.Before(before) || snap.LastUpdated.After(after) { + t.Errorf("LastUpdated = %v, want within [%v, %v]", snap.LastUpdated, before, after) } } @@ -270,7 +180,7 @@ func TestAddObservation_ConcurrentSafe(t *testing.T) { go func() { defer wg.Done() for range per { - s.AddObservation("", "", "test", 10, 100) + s.AddObservation(Observation{Tool: "test", Returned: 10, Saved: 100}) expectedSaved.Add(100) } }() @@ -286,92 +196,231 @@ func TestAddObservation_ConcurrentSafe(t *testing.T) { } } -func TestOpen_CorruptFile_IsBackedUp(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - if err := os.WriteFile(path, []byte("{not json"), 0o644); err != nil { +func TestOpen_EmptyPath_InMemoryOnly(t *testing.T) { + s, err := Open("") + if err != nil { t.Fatal(err) } - s, err := Open(path) - if err != nil { - t.Fatalf("Open should recover from corrupt file, got error: %v", err) + s.AddObservation(Observation{Repo: "r", Tool: "test", Returned: 10, Saved: 100}) + if err := s.Flush(); err != nil { + t.Errorf("Flush on in-memory store should no-op, got: %v", err) } snap := s.Snapshot() - if snap.Totals.CallsCounted != 0 { - t.Errorf("corrupt recovery should start fresh, got CallsCounted=%d", snap.Totals.CallsCounted) + if snap.Totals.CallsCounted != 1 { + t.Errorf("in-memory store should track, got CallsCounted=%d", snap.Totals.CallsCounted) + } + evs, err := s.EventsSince(time.Time{}) + if err != nil { + t.Fatal(err) } - // Backup should exist. - matches, _ := filepath.Glob(path + ".corrupt-*") - if len(matches) == 0 { - t.Errorf("expected a .corrupt-* backup file in %s", dir) + if len(evs) != 1 { + t.Errorf("in-memory store should keep events, got %d", len(evs)) } } -func TestReset_ClearsStateAndRemovesFile(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") +func TestReset_ClearsLedger(t *testing.T) { + path := testLedgerPath(t) s, _ := Open(path) - s.AddObservation("/r", "", "test", 50, 500) - _ = s.Flush() + s.AddObservation(Observation{Repo: "/r", Tool: "test", Returned: 50, Saved: 500}) - if _, err := os.Stat(path); err != nil { - t.Fatalf("expected file to exist after flush: %v", err) - } if err := s.Reset(); err != nil { t.Fatalf("Reset: %v", err) } - if _, err := os.Stat(path); !os.IsNotExist(err) { - t.Errorf("expected savings.json removed after reset, got %v", err) - } snap := s.Snapshot() if snap.Totals.CallsCounted != 0 { - t.Errorf("in-memory state should be cleared after reset, got CallsCounted=%d", snap.Totals.CallsCounted) + t.Errorf("totals should be cleared after reset, got CallsCounted=%d", snap.Totals.CallsCounted) } -} - -func TestOpen_EmptyPath_InMemoryOnly(t *testing.T) { - s, err := Open("") + if !snap.FirstSeen.IsZero() { + t.Errorf("FirstSeen should be cleared after reset, got %v", snap.FirstSeen) + } + evs, err := s.EventsSince(time.Time{}) if err != nil { t.Fatal(err) } - s.AddObservation("r", "", "test", 10, 100) - if err := s.Flush(); err != nil { - t.Errorf("Flush on in-memory store should no-op, got: %v", err) - } - snap := s.Snapshot() - if snap.Totals.CallsCounted != 1 { - t.Errorf("in-memory store should track, got CallsCounted=%d", snap.Totals.CallsCounted) + if len(evs) != 0 { + t.Errorf("events should be cleared after reset, got %d", len(evs)) } } -func TestFile_Schema(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "savings.json") - - s, _ := Open(path) - s.AddObservation("/repo-a", "", "test", 10, 100) - _ = s.Flush() +// Snapshot keeps the JSON shape graph_stats and `gortex savings --json` +// expose — the surface contract of cumulative_savings. +func TestSnapshot_JSONShape(t *testing.T) { + s, _ := Open(testLedgerPath(t)) + s.AddObservation(Observation{Repo: "/repo-a", Language: "go", Tool: "test", Returned: 10, Saved: 100}) - data, err := os.ReadFile(path) + data, err := json.Marshal(s.Snapshot()) if err != nil { t.Fatal(err) } var parsed map[string]any if err := json.Unmarshal(data, &parsed); err != nil { - t.Fatalf("written file is not JSON: %v\n%s", err, data) + t.Fatal(err) } - for _, key := range []string{"version", "first_seen", "last_updated", "totals", "per_repo"} { + for _, key := range []string{"version", "first_seen", "last_updated", "totals", "per_repo", "per_language"} { if _, ok := parsed[key]; !ok { - t.Errorf("missing top-level key %q in persisted file", key) + t.Errorf("missing key %q in snapshot JSON", key) } } } -// TestDefaultPath_HonorsXDGCacheHome verifies the savings file path is -// routed through the XDG resolver: an absolute $XDG_CACHE_HOME -// relocates it to /gortex/savings.json, so the savings -// store is consistent with the rest of Gortex's cache layout. +func TestEventsSince_Filters(t *testing.T) { + s, _ := Open(testLedgerPath(t)) + s.AddObservation(Observation{Tool: "a", Returned: 1, Saved: 1}) + time.Sleep(5 * time.Millisecond) + cutoff := time.Now().UTC() + time.Sleep(5 * time.Millisecond) + s.AddObservation(Observation{Tool: "b", Returned: 1, Saved: 1}) + + evs, err := s.EventsSince(cutoff) + if err != nil { + t.Fatal(err) + } + if len(evs) != 1 || evs[0].Tool != "b" { + t.Errorf("EventsSince(cutoff) = %+v, want only [b]", evs) + } + all, err := s.EventsSince(time.Time{}) + if err != nil { + t.Fatal(err) + } + if len(all) != 2 { + t.Errorf("EventsSince(zero) = %d, want 2", len(all)) + } +} + +func TestImportLegacy_FullFlatFiles(t *testing.T) { + legacyDir := t.TempDir() + jsonPath := filepath.Join(legacyDir, "savings.json") + jsonlPath := filepath.Join(legacyDir, "savings.jsonl") + + firstSeen := time.Date(2026, 5, 1, 8, 0, 0, 0, time.UTC) + lastUpdated := time.Date(2026, 5, 20, 9, 0, 0, 0, time.UTC) + legacy := File{ + Version: schemaVersion, + FirstSeen: firstSeen, + LastUpdated: lastUpdated, + Totals: Totals{TokensSaved: 1000, TokensReturned: 100, CallsCounted: 10}, + PerRepo: map[string]*Totals{"repo-a": {TokensSaved: 1000, TokensReturned: 100, CallsCounted: 10}}, + PerLanguage: map[string]*Totals{"go": {TokensSaved: 1000, TokensReturned: 100, CallsCounted: 10}}, + } + data, _ := json.Marshal(legacy) + if err := os.WriteFile(jsonPath, data, 0o644); err != nil { + t.Fatal(err) + } + line, _ := json.Marshal(Event{TS: lastUpdated, Repo: "repo-a", Language: "go", Tool: "get_symbol_source", Returned: 23, Saved: 77}) + if err := os.WriteFile(jsonlPath, append(line, '\n'), 0o644); err != nil { + t.Fatal(err) + } + + s, err := Open(testLedgerPath(t)) + if err != nil { + t.Fatal(err) + } + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatalf("ImportLegacy: %v", err) + } + + snap := s.Snapshot() + if snap.Totals.CallsCounted != 10 || snap.Totals.TokensSaved != 1000 { + t.Errorf("imported totals = %+v, want calls=10 saved=1000", snap.Totals) + } + if r := snap.PerRepo["repo-a"]; r == nil || r.CallsCounted != 10 { + t.Errorf("imported repo bucket = %+v", r) + } + if !snap.FirstSeen.Equal(firstSeen) { + t.Errorf("FirstSeen = %v, want %v (carried from legacy file)", snap.FirstSeen, firstSeen) + } + evs, _ := s.EventsSince(time.Time{}) + if len(evs) != 1 || evs[0].Tool != "get_symbol_source" || evs[0].Saved != 77 { + t.Errorf("imported events = %+v", evs) + } + + // Legacy files renamed aside, originals gone. + if _, err := os.Stat(jsonPath); !os.IsNotExist(err) { + t.Errorf("legacy savings.json should be renamed after import, stat err=%v", err) + } + if _, err := os.Stat(jsonPath + ".bak"); err != nil { + t.Errorf("expected savings.json.bak, stat err=%v", err) + } + if _, err := os.Stat(jsonlPath + ".bak"); err != nil { + t.Errorf("expected savings.jsonl.bak, stat err=%v", err) + } + + // Idempotent: a second import (e.g. another entry point racing the + // first) must not double-count. + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatalf("second ImportLegacy: %v", err) + } + if got := s.Snapshot().Totals.CallsCounted; got != 10 { + t.Errorf("totals after second import = %d, want 10 (no double count)", got) + } +} + +// A jsonl without its cumulative file (the flat-file flush never ran +// before the process died — the common SIGKILL case) still imports: +// totals are rebuilt from the events. +func TestImportLegacy_EventsOnlyRebuildsTotals(t *testing.T) { + legacyDir := t.TempDir() + jsonPath := filepath.Join(legacyDir, "savings.json") + jsonlPath := filepath.Join(legacyDir, "savings.jsonl") + + ts := time.Date(2026, 6, 1, 10, 0, 0, 0, time.UTC) + var lines []byte + for i := 0; i < 3; i++ { + line, _ := json.Marshal(Event{TS: ts.Add(time.Duration(i) * time.Minute), Repo: "r", Language: "go", Tool: "smart_context", Returned: 10, Saved: 30}) + lines = append(lines, line...) + lines = append(lines, '\n') + } + if err := os.WriteFile(jsonlPath, lines, 0o644); err != nil { + t.Fatal(err) + } + + s, _ := Open(testLedgerPath(t)) + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatalf("ImportLegacy: %v", err) + } + snap := s.Snapshot() + if snap.Totals.CallsCounted != 3 || snap.Totals.TokensSaved != 90 { + t.Errorf("rebuilt totals = %+v, want calls=3 saved=90", snap.Totals) + } + if r := snap.PerRepo["r"]; r == nil || r.CallsCounted != 3 { + t.Errorf("rebuilt repo bucket = %+v", r) + } + if !snap.FirstSeen.Equal(ts) { + t.Errorf("FirstSeen = %v, want first event ts %v", snap.FirstSeen, ts) + } +} + +// With nothing to import the mark is still set, so legacy files that +// appear later (e.g. restored from a backup) are not silently merged +// into a ledger that has moved on. +func TestImportLegacy_NothingToImportMarksDone(t *testing.T) { + legacyDir := t.TempDir() + jsonPath := filepath.Join(legacyDir, "savings.json") + + s, _ := Open(testLedgerPath(t)) + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatalf("ImportLegacy on missing files: %v", err) + } + + // A legacy file materializing afterwards is ignored. + legacy := File{Version: schemaVersion, Totals: Totals{TokensSaved: 5000, CallsCounted: 50}} + data, _ := json.Marshal(legacy) + if err := os.WriteFile(jsonPath, data, 0o644); err != nil { + t.Fatal(err) + } + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatal(err) + } + if got := s.Snapshot().Totals.CallsCounted; got != 0 { + t.Errorf("late-appearing legacy file must not import, got calls=%d", got) + } +} + +// TestDefaultPath_HonorsXDGCacheHome verifies the legacy flat-file path is +// routed through the XDG resolver: an absolute $XDG_CACHE_HOME relocates +// it to /gortex/savings.json, so the legacy import looks +// where the flat-file era actually wrote. func TestDefaultPath_HonorsXDGCacheHome(t *testing.T) { xdg := t.TempDir() t.Setenv("XDG_CACHE_HOME", xdg) @@ -387,3 +436,16 @@ func TestDefaultPath_HonorsXDGCacheHome(t *testing.T) { t.Fatalf("DefaultEventsPath() with XDG_CACHE_HOME = %s, want %s", got, wantEvents) } } + +// TestDefaultDBPath_HonorsXDGDataHome verifies the ledger DB follows the +// data-dir resolver — the same sidecar.sqlite the notes/memories +// managers share. +func TestDefaultDBPath_HonorsXDGDataHome(t *testing.T) { + xdg := t.TempDir() + t.Setenv("XDG_DATA_HOME", xdg) + + want := filepath.Join(xdg, "gortex", "sidecar.sqlite") + if got := DefaultDBPath(); got != want { + t.Fatalf("DefaultDBPath() with XDG_DATA_HOME = %s, want %s", got, want) + } +} diff --git a/internal/serverstack/shared_server.go b/internal/serverstack/shared_server.go index 8f6a61a0..811c896c 100644 --- a/internal/serverstack/shared_server.go +++ b/internal/serverstack/shared_server.go @@ -19,6 +19,7 @@ import ( gortexmcp "github.com/zzet/gortex/internal/mcp" "github.com/zzet/gortex/internal/parser" "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/persistence" "github.com/zzet/gortex/internal/query" "github.com/zzet/gortex/internal/savings" "github.com/zzet/gortex/internal/semantic" @@ -86,11 +87,17 @@ type SharedServerConfig struct { // SemanticMode selects the goanalysis provider mode: "callgraph" // builds the call graph; anything else (default) type-checks. SemanticMode string - // SavingsPath overrides the token-savings store path; empty uses the - // default (~/.gortex/cache/savings.json). SavingsRepo scopes the - // accumulated totals (empty = workspace-global). - SavingsPath string - SavingsRepo string + // SavingsPath overrides the token-savings ledger database; empty + // derives /sidecar.sqlite (the same sidecar the + // notes/memories managers share), falling back to the machine + // default under the data dir. SavingsRepo scopes the accumulated + // totals (empty = workspace-global). SavingsLegacyJSON names the + // flat-file era's cumulative savings.json to import once — its + // sibling .jsonl event log rides along; empty uses the historical + // default location under the cache dir. + SavingsPath string + SavingsLegacyJSON string + SavingsRepo string } // SideStores configures where the agent-authored knowledge stores @@ -455,9 +462,20 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { savingsPath := cfg.SavingsPath if savingsPath == "" { - savingsPath = savings.DefaultPath() + if sideCfg.NotesDir != "" { + savingsPath = persistence.DefaultSidecarPath(sideCfg.NotesDir) + } else { + savingsPath = savings.DefaultDBPath() + } } if savingsStore, err := savings.Open(savingsPath); err == nil { + legacyJSON := cfg.SavingsLegacyJSON + if legacyJSON == "" { + legacyJSON = savings.DefaultPath() + } + if ierr := savingsStore.ImportLegacy(legacyJSON); ierr != nil { + logger.Warn("serverstack: legacy savings import failed", zap.Error(ierr)) + } srv.InitSavings(savingsStore, cfg.SavingsRepo) s.cleanup = append(s.cleanup, func() { _ = srv.FlushSavings() }) } else { From dab9304c16cafa9a422d02a62c5cbe9a7ae40371 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 00:22:57 +0200 Subject: [PATCH 04/12] savings: honest dashboard text for the ledger era MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The empty state hinted at three tools when six record, the help text still described the flat files the ledger replaced, and --cache-dir's description pointed at savings.json. The empty-state test now also pins that a never-used ledger prints no 'tracking since' line — the zero FirstSeen stays hidden instead of being passed off as a start date. --- cmd/gortex/gain.go | 8 ++++---- cmd/gortex/savings.go | 28 ++++++++++++++++------------ cmd/gortex/savings_test.go | 5 +++++ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/cmd/gortex/gain.go b/cmd/gortex/gain.go index 754f4b3b..94df475e 100644 --- a/cmd/gortex/gain.go +++ b/cmd/gortex/gain.go @@ -51,8 +51,8 @@ Default behavior: 1. Find the most recent gortex bench tokens output (auto-discovery under bench/results/, then a transparent re-run when none). 2. Render a USD-per-model card scaled to --responses-per-day. - 3. Append a short "Your history" section from ~/.gortex/cache/savings.json - when --since's window has any tracked calls. + 3. Append a short "Your history" section from the savings ledger + (~/.gortex/sidecar.sqlite) when --since's window has any tracked calls. Flags: --bench-result PATH specific bench tokens JSON to use (skip discovery) @@ -61,7 +61,7 @@ Flags: --since DURATION history window (e.g. 24h, 7d; default 7d) --json emit machine-readable JSON --no-history skip the cumulative-history section - --cache-dir DIR override savings cache (savings.json lives here)`, + --cache-dir DIR override the ledger directory (its sidecar.sqlite)`, RunE: runGain, } @@ -72,7 +72,7 @@ func init() { gainCmd.Flags().DurationVar(&gainSince, "since", 7*24*time.Hour, "history window for the cumulative-savings section (e.g. 24h, 7d)") gainCmd.Flags().BoolVar(&gainJSON, "json", false, "emit machine-readable JSON") gainCmd.Flags().BoolVar(&gainNoHistory, "no-history", false, "skip the cumulative-history section") - gainCmd.Flags().StringVar(&gainCacheDir, "cache-dir", "", "override graph cache directory (savings.json lives here)") + gainCmd.Flags().StringVar(&gainCacheDir, "cache-dir", "", "override the ledger directory (its sidecar.sqlite holds the savings ledger)") rootCmd.AddCommand(gainCmd) } diff --git a/cmd/gortex/savings.go b/cmd/gortex/savings.go index f735b6c4..18e5b41b 100644 --- a/cmd/gortex/savings.go +++ b/cmd/gortex/savings.go @@ -35,23 +35,26 @@ sessions: Today, Last 7 days, and All time. Each bucket shows a 16-cell saved/total bar, percentage saved, raw token counts, and the USD value of the tokens avoided (priced against popular models). -Savings accumulate every time a source-reading MCP tool (get_symbol_source, -batch_symbols, smart_context) returns a symbol or compressed view instead of -a full-file read. Cumulative totals live at ~/.gortex/cache/savings.json and -per-call events at the sibling ~/.gortex/cache/savings.jsonl — Today / 7-day -buckets come from the JSONL log, All time from the cumulative file. +Savings accumulate every time a source-reading MCP tool — read_file, +get_file_summary, get_editing_context, get_symbol_source, batch_symbols, +smart_context — returns a summary, symbol, or compressed view that stands +in for a full-file read. The ledger lives in the machine-global sidecar +database (~/.gortex/sidecar.sqlite); flat-file ledgers from older +releases (savings.json / savings.jsonl under the cache dir) are imported +once and renamed *.bak. -Override the cache dir with --cache-dir, override pricing by exporting -GORTEX_MODEL_PRICING_JSON, and pass --verbose for a per-tool breakdown -inside each bucket.`, +Override the ledger location with --cache-dir (uses that directory's +sidecar.sqlite and imports its legacy files), override pricing by +exporting GORTEX_MODEL_PRICING_JSON, and pass --verbose for a per-tool +breakdown inside each bucket.`, RunE: runSavings, } func init() { savingsCmd.Flags().StringVar(&savingsModel, "model", "", "highlight one model in USD output (default: show all)") savingsCmd.Flags().BoolVar(&savingsJSON, "json", false, "emit machine-readable JSON instead of the dashboard") - savingsCmd.Flags().BoolVar(&savingsReset, "reset", false, "wipe cumulative totals + event log and exit") - savingsCmd.Flags().StringVar(&savingsCacheDir, "cache-dir", "", "override graph cache directory (savings.json + savings.jsonl live here)") + savingsCmd.Flags().BoolVar(&savingsReset, "reset", false, "wipe cumulative totals + event history and exit") + savingsCmd.Flags().StringVar(&savingsCacheDir, "cache-dir", "", "override the ledger directory (its sidecar.sqlite holds the savings ledger)") savingsCmd.Flags().BoolVarP(&savingsVerbose, "verbose", "v", false, "include per-tool breakdown for each bucket") savingsCmd.Flags().IntVar(&savingsBarCells, "bar-width", 16, "number of cells in each bar (default 16, matching semble)") savingsCmd.Flags().BoolVar(&savingsUTC, "utc", false, "bucket Today by UTC calendar (default: local time)") @@ -199,13 +202,14 @@ func emitSavingsDashboard(snap savings.File, buckets []savings.Bucket, path stri headline, headlineModel := pickHeadlineCost(costs, savingsModel) fmt.Println() if snap.Totals.CallsCounted == 0 { + hint := "savings record when the agent reads code through gortex (read_file, get_file_summary, get_editing_context, get_symbol_source, smart_context, …)" if tty { fmt.Println(" " + progress.StyleHint.Render("◌ no source-reading tool calls recorded yet")) - fmt.Println(" " + progress.Caption("run `gortex mcp` and use get_symbol_source / batch_symbols / smart_context")) + fmt.Println(" " + progress.Caption(hint)) fmt.Println() } else { fmt.Println("No source-reading tool calls recorded yet.") - fmt.Println("Run `gortex mcp` and use get_symbol_source / batch_symbols / smart_context.") + fmt.Println("Savings record when the agent reads code through gortex (read_file, get_file_summary, get_editing_context, get_symbol_source, smart_context, ...).") } return } diff --git a/cmd/gortex/savings_test.go b/cmd/gortex/savings_test.go index f83403f6..544fb832 100644 --- a/cmd/gortex/savings_test.go +++ b/cmd/gortex/savings_test.go @@ -196,4 +196,9 @@ func TestEmitSavingsDashboard_EmptyTotals(t *testing.T) { if strings.Contains(out, "█") { t.Errorf("empty totals should not render bars, got:\n%s", out) } + // An empty ledger has never tracked anything — the dashboard must not + // claim a "tracking since" moment (the zero FirstSeen stays hidden). + if strings.Contains(out, "Tracking since") { + t.Errorf("empty ledger must not print a tracking-since line, got:\n%s", out) + } } From 10344e8657ae6f250e45ccde875cfbd96d016679 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 00:26:30 +0200 Subject: [PATCH 05/12] docs: savings ledger lives in the sidecar database Actualise the savings docs for the sidecar-backed ledger: where it lives, the transactional durability story, the legacy flat-file import, and the real recording surface (the read family records too, and the per-call value is server-side accounting, not a response field). --- docs/landing-pages/per-tool-savings.md | 2 +- docs/onboarding.md | 2 +- docs/savings.md | 6 +++--- scripts/landing/per-tool-savings.sh | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/landing-pages/per-tool-savings.md b/docs/landing-pages/per-tool-savings.md index b5d88a35..a79276a0 100644 --- a/docs/landing-pages/per-tool-savings.md +++ b/docs/landing-pages/per-tool-savings.md @@ -2,7 +2,7 @@ **Last regenerated**: 2026-05-18T22:20:29Z · Source: `gortex savings --verbose --json` against the operator's cumulative store -(`~/.gortex/cache/savings.json` + `~/.gortex/cache/savings.jsonl`). +(the `~/.gortex/sidecar.sqlite` savings ledger). ## Headline diff --git a/docs/onboarding.md b/docs/onboarding.md index 489bcd9b..0dd7d959 100644 --- a/docs/onboarding.md +++ b/docs/onboarding.md @@ -241,7 +241,7 @@ On macOS the unit lands at `~/Library/LaunchAgents/com.zzet.gortex.plist`; on Li - Every tracked repo gets its own fsnotify watcher so edits on disk flow into the graph live; no manual reload needed. `gortex track` attaches a watcher as part of the track operation; `gortex untrack` detaches it before evicting nodes. - Graph state is snapshotted to `~/.gortex/cache/daemon.gob.gz` on shutdown and every 10 minutes. Daemon restarts load it back and re-index only changed files. - Opening Claude Code in an untracked directory returns a structured `repo_not_tracked` error on every tool call. The agent surfaces it; you run `gortex track .` to include it. -- Per-session state is isolated by a handshake-assigned session ID — two Claude Code windows see their own recent-activity and token-savings counters, not a merged view. Cumulative savings in `~/.gortex/cache/savings.json` are still shared. +- Per-session state is isolated by a handshake-assigned session ID — two Claude Code windows see their own recent-activity and token-savings counters, not a merged view. Cumulative savings in the sidecar ledger (`~/.gortex/sidecar.sqlite`) are still shared. ### Fallback rules diff --git a/docs/savings.md b/docs/savings.md index 442b8a2e..295a722a 100644 --- a/docs/savings.md +++ b/docs/savings.md @@ -2,9 +2,9 @@ Gortex tracks how many tokens it saves compared to naive file reads — per-call, per-session, and cumulative across restarts: -- **Per-call:** `get_symbol_source` and other source-reading tools include a `tokens_saved` field in the response, showing the difference between reading the full file vs the targeted symbol. +- **Per-call:** every source-reading tool — `read_file`, `get_file_summary`, `get_editing_context`, `get_symbol_source`, `batch_symbols` (with `include_source`), `smart_context` — books an observation server-side: tokens actually returned vs the full-file read the response stands in for. The per-call value is deliberately not echoed in responses (agents don't act on it and it would burn tokens on every reply); it lands in the ledger. - **Session-level:** `graph_stats` returns a `token_savings` object with `calls_counted`, `tokens_returned`, `tokens_saved`, `efficiency_ratio`. -- **Cumulative (cross-session):** `graph_stats` also returns `cumulative_savings` when persistence is wired — includes `first_seen`, `last_updated`, and `cost_avoided_usd` per model (Claude Opus/Sonnet/Haiku, GPT-4o, GPT-4o-mini). Backed by `~/.gortex/cache/savings.json` (top-line totals + per-repo + per-language) and a sibling `~/.gortex/cache/savings.jsonl` event log (one line per call) used to render the windowed buckets and the per-tool breakdown. +- **Cumulative (cross-session):** `graph_stats` also returns `cumulative_savings` when persistence is wired — includes `first_seen`, `last_updated`, and `cost_avoided_usd` per model (Claude Opus/Sonnet/Haiku, GPT-4o, GPT-4o-mini). Backed by the machine-global sidecar database (`~/.gortex/sidecar.sqlite` — the same file that holds notes/memories): `savings_totals` carries top-line + per-repo + per-language aggregates and `savings_events` one session-tagged row per call, powering the windowed buckets and the per-tool breakdown. Each observation commits transactionally, so the ledger survives SIGKILLed MCP servers and concurrent writer processes. Flat-file ledgers from older releases (`~/.gortex/cache/savings.json` + `savings.jsonl`) are imported once on first open and renamed `*.bak`. `gortex savings` renders a three-bucket dashboard: @@ -34,7 +34,7 @@ gortex savings --utc # Machine-readable output (mirrors the dashboard structure: buckets[].per_tool, cost_avoided_usd, etc.) gortex savings --json -# Wipe cumulative totals and the JSONL event log +# Wipe cumulative totals and the event history gortex savings --reset # Override pricing (JSON array of {model, usd_per_m_input}) diff --git a/scripts/landing/per-tool-savings.sh b/scripts/landing/per-tool-savings.sh index 9b08319c..523295c3 100755 --- a/scripts/landing/per-tool-savings.sh +++ b/scripts/landing/per-tool-savings.sh @@ -42,7 +42,7 @@ cat > "$OUT" < Date: Fri, 12 Jun 2026 00:26:30 +0200 Subject: [PATCH 06/12] persistence: check and set the savings import mark inside the import tx The legacy-import guard checked migration_marks before taking the write path, so two processes racing the first start (daemon + CLI) could both pass the check and double-seed the ledger. The mark is now re-checked and written inside the import transaction: the loser either sees the winner's mark or aborts on the write conflict. --- internal/persistence/sidecar_savings.go | 29 ++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/internal/persistence/sidecar_savings.go b/internal/persistence/sidecar_savings.go index ba734cd0..7c72b0b9 100644 --- a/internal/persistence/sidecar_savings.go +++ b/internal/persistence/sidecar_savings.go @@ -191,7 +191,11 @@ func (s *SidecarStore) SavingsLegacyImportDone() bool { // totals from the cumulative savings.json and event rows from the // savings.jsonl log. Idempotent — guarded by a migration mark, which is // set even for an empty import so the file probing never repeats. The -// caller owns reading (and afterwards renaming) the legacy files. +// mark is checked and written inside the import transaction, so two +// processes racing the first start (daemon + CLI) cannot both seed the +// ledger: the loser either sees the winner's mark or aborts on the +// write conflict. The caller owns reading (and afterwards renaming) +// the legacy files. func (s *SidecarStore) ImportLegacySavings(buckets map[string]SavingsTotalsRow, firstSeen, lastUpdated time.Time, events []SavingsEvent) error { if s == nil || s.migrationDone("", savingsLegacyMigrationKind) { return nil @@ -205,6 +209,19 @@ func (s *SidecarStore) ImportLegacySavings(buckets map[string]SavingsTotalsRow, } defer func() { _ = tx.Rollback() }() + // Re-check under the transaction — the cheap pre-check above races + // with other writers (in-process via writeMu it cannot, but another + // process opening the same database can). + var marked int + if err := tx.QueryRow( + `SELECT COUNT(1) FROM migration_marks WHERE repo_key = '' AND kind = ?`, savingsLegacyMigrationKind, + ).Scan(&marked); err != nil { + return fmt.Errorf("persistence: savings import mark check: %w", err) + } + if marked > 0 { + return nil + } + for bucket, r := range buckets { if err := upsertSavingsBucket(tx, bucket, r.Saved, r.Returned, r.Calls); err != nil { return err @@ -236,11 +253,13 @@ func (s *SidecarStore) ImportLegacySavings(buckets map[string]SavingsTotalsRow, return fmt.Errorf("persistence: savings import meta: %w", err) } } - if err := tx.Commit(); err != nil { - return err + if _, err := tx.Exec( + `INSERT OR REPLACE INTO migration_marks (repo_key, kind, done_at) VALUES ('', ?, ?)`, + savingsLegacyMigrationKind, time.Now().UTC().UnixNano(), + ); err != nil { + return fmt.Errorf("persistence: savings import mark: %w", err) } - s.markMigrated("", savingsLegacyMigrationKind) - return nil + return tx.Commit() } // ResetSavings wipes the savings ledger (events, totals, meta). The From 51b4cfc34d14e538f64c9a6c6b3b545502761e9b Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 00:28:39 +0200 Subject: [PATCH 07/12] serverstack: the savings ledger always defaults machine-global MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deriving the ledger from the side-store dir split it by entry point: the embedded server's side stores default to the cache dir, so its ledger landed in /sidecar.sqlite while the savings CLI reads ~/.gortex/sidecar.sqlite — the writer/reader divergence the flat files had. Every entry point now defaults to the machine-global sidecar; an explicit --cache-dir still relocates both the ledger and the legacy files for isolation. --- cmd/gortex/mcp.go | 11 +++++++---- internal/serverstack/shared_server.go | 11 +++++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/cmd/gortex/mcp.go b/cmd/gortex/mcp.go index be00f9cc..e5206120 100644 --- a/cmd/gortex/mcp.go +++ b/cmd/gortex/mcp.go @@ -139,12 +139,14 @@ func runMCP(cmd *cobra.Command, args []string) error { if sideStoreCacheDir == "" { sideStoreCacheDir = platform.CacheDir() } - // The savings ledger rides in the same sidecar database the side - // stores use (serverstack derives it from NotesDir). A custom - // --cache-dir also relocates the flat-file era's savings.json the - // one-shot legacy import looks for. + // The savings ledger defaults to the machine-global sidecar the + // `gortex savings` CLI reads. A custom --cache-dir relocates both + // the ledger and the flat-file era's savings.json the one-shot + // legacy import looks for — the isolation tests rely on. + savingsDBPath := "" savingsLegacyJSON := "" if mcpCacheDir != "" { + savingsDBPath = persistence.DefaultSidecarPath(mcpCacheDir) savingsLegacyJSON = filepath.Join(mcpCacheDir, "savings.json") } @@ -169,6 +171,7 @@ func runMCP(cmd *cobra.Command, args []string) error { FeedbackRepo: mcpIndex, NotebookPath: mcpIndex, }, + SavingsPath: savingsDBPath, SavingsLegacyJSON: savingsLegacyJSON, SavingsRepo: mcpIndex, }) diff --git a/internal/serverstack/shared_server.go b/internal/serverstack/shared_server.go index 811c896c..c7d1d266 100644 --- a/internal/serverstack/shared_server.go +++ b/internal/serverstack/shared_server.go @@ -19,7 +19,6 @@ import ( gortexmcp "github.com/zzet/gortex/internal/mcp" "github.com/zzet/gortex/internal/parser" "github.com/zzet/gortex/internal/parser/languages" - "github.com/zzet/gortex/internal/persistence" "github.com/zzet/gortex/internal/query" "github.com/zzet/gortex/internal/savings" "github.com/zzet/gortex/internal/semantic" @@ -460,13 +459,13 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { srv.InitCombo(sideCfg.FeedbackDir, sideCfg.FeedbackRepo, gortexmcp.ModeAI) srv.InitFrecency(sideCfg.FeedbackDir, sideCfg.FeedbackRepo, gortexmcp.ModeAI) + // The savings ledger is machine-global: every entry point defaults to + // the same sidecar database the `gortex savings` CLI reads. Deriving + // it from a per-mode side-store dir would split the ledger between + // writer and reader — the failure mode the flat files had. savingsPath := cfg.SavingsPath if savingsPath == "" { - if sideCfg.NotesDir != "" { - savingsPath = persistence.DefaultSidecarPath(sideCfg.NotesDir) - } else { - savingsPath = savings.DefaultDBPath() - } + savingsPath = savings.DefaultDBPath() } if savingsStore, err := savings.Open(savingsPath); err == nil { legacyJSON := cfg.SavingsLegacyJSON From 1b02b4a8078b002dd9a50037f55d093e0875610f Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 01:05:29 +0200 Subject: [PATCH 08/12] indexer, mcp: provenance-guard the lone-repo fallback; survive repo-count transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review of the lone-repo fallback found it trusted whichever repo happened to be the only one tracked. Three hardening changes: RepoMetadata carries Unprefixed — stamped where single-repo mode mints unprefixed nodes — and the empty-prefix fallback honours only a lone repo that actually did. A 1→2→1 track/untrack sequence leaves a prefixed lone survivor, and stale unprefixed node IDs now keep failing closed instead of resolving (and writing) into the wrong checkout. Untracking a single-mode repo evicts its nodes file-by-file: they never enter the byRepo bucket EvictRepo walks, so untrack used to remove nothing and the next lone repo inherited them. Tracking a second repo into a live single-repo daemon re-mints the first repo's nodes under its real prefix (index first, evict the unprefixed originals after — crash leaves both forms resolvable, never neither), so its symbols stay reachable when the fallback disarms. Plus a collision guard in both path resolvers: a lone repo whose directory name matches one of its own top-level directories (repo "api" containing api/handlers.go) resolves the raw on-disk path instead of being hijacked by the prefix-strip join. --- internal/indexer/multi.go | 136 +++++++++++++++++++++- internal/indexer/multi_transition_test.go | 128 ++++++++++++++++++++ internal/mcp/tools_fileops.go | 13 +++ 3 files changed, 274 insertions(+), 3 deletions(-) create mode 100644 internal/indexer/multi_transition_test.go diff --git a/internal/indexer/multi.go b/internal/indexer/multi.go index e6a2dbae..ec0fa8d2 100644 --- a/internal/indexer/multi.go +++ b/internal/indexer/multi.go @@ -41,6 +41,16 @@ type RepoMetadata struct { // needs this remembered flag to know a vanished root was a // worktree and may be garbage-collected. IsWorktree bool + // Unprefixed records that this repo was indexed in single-repo + // mode: its nodes carry RepoPrefix="" and raw relative file paths. + // The empty-prefix resolution fallback (RepoRoot, ResolveFilePath) + // honours only a lone repo that actually minted unprefixed nodes — + // without the provenance check, stale unprefixed nodes surviving a + // track/untrack transition would resolve against whatever repo + // happens to be the lone one. Eviction also branches on it: + // unprefixed nodes are invisible to the byRepo bucket EvictRepo + // walks, so UntrackRepo must evict them file-by-file. + Unprefixed bool } // MultiIndexer orchestrates indexing across multiple repositories. @@ -667,6 +677,7 @@ func (mi *MultiIndexer) indexSingleRepo(entry config.RepoEntry) (map[string]*Ind ParseErrors: result.Errors, FileMtimes: idx.FileMtimes(), IsWorktree: ResolveWorktree(absPath).IsWorktree, + Unprefixed: true, } mi.indexers[prefix] = idx mi.mu.Unlock() @@ -674,6 +685,78 @@ func (mi *MultiIndexer) indexSingleRepo(entry config.RepoEntry) (map[string]*Ind return map[string]*IndexResult{prefix: result}, nil } +// migrateLoneUnprefixedRepoCtx re-mints the formerly-lone repo's nodes +// with its real prefix the moment a second repo joins. Without it, the +// first repo's unprefixed nodes become unreachable (the empty-prefix +// fallback disarms at two repos) until a cold reindex. Ordering is +// crash-safe: the prefixed re-index lands first, the stale unprefixed +// nodes are evicted after — an interruption leaves both ID forms +// resolvable rather than neither. +func (mi *MultiIndexer) migrateLoneUnprefixedRepoCtx(ctx context.Context) { + mi.mu.RLock() + var oldPrefix string + var oldMeta *RepoMetadata + if len(mi.repos) == 1 { + for p, m := range mi.repos { + if m != nil && m.Unprefixed && m.RootPath != "" { + oldPrefix, oldMeta = p, m + } + } + } + mi.mu.RUnlock() + if oldMeta == nil { + return + } + + cfg := mi.configMgr.GetRepoConfig(oldPrefix) + idx := mi.newPerRepoIndexer(cfg.Index) + idx.SetRepoPrefix(oldPrefix) + entry := config.RepoEntry{Path: oldMeta.RootPath, Name: oldPrefix} + if mi.configMgr != nil { + for _, e := range mi.configMgr.Global().Repos { + if e.Path == oldMeta.RootPath { + entry = e + break + } + } + } + idx.SetWorkspaceID(resolveWorkspaceID(&entry, cfg, oldPrefix)) + idx.SetProjectID(resolveProjectID(&entry, cfg, oldPrefix)) + + result, err := idx.IndexCtx(ctx, oldMeta.RootPath) + if err != nil { + mi.logger.Warn("re-prefixing lone repo failed; its unprefixed nodes stay until a reindex", + zap.String("prefix", oldPrefix), zap.Error(err)) + return + } + + // The prefixed nodes are live; now drop the unprefixed originals. + // They are invisible to EvictRepo (no byRepo bucket entry), so evict + // per file — unprefixed paths cannot collide with prefixed ones. + for path := range oldMeta.FileMtimes { + mi.graph.EvictFile(path) + } + + mi.mu.Lock() + mi.repos[oldPrefix] = &RepoMetadata{ + RepoPrefix: oldPrefix, + RootPath: oldMeta.RootPath, + Identity: oldMeta.Identity, + LastIndexTime: time.Now(), + FileCount: result.FileCount, + NodeCount: result.NodeCount, + EdgeCount: result.EdgeCount, + ParseErrors: result.Errors, + FileMtimes: idx.FileMtimes(), + IsWorktree: oldMeta.IsWorktree, + } + mi.indexers[oldPrefix] = idx + mi.mu.Unlock() + + mi.logger.Info("re-minted lone repo with its prefix for multi-repo mode", + zap.String("prefix", oldPrefix), zap.Int("nodes", result.NodeCount)) +} + // readGoModModule reads the module path from a go.mod file. func readGoModModule(repoPath string) string { data, err := os.ReadFile(filepath.Join(repoPath, "go.mod")) @@ -1110,6 +1193,13 @@ func (mi *MultiIndexer) TrackRepoCtx(ctx context.Context, entry config.RepoEntry } willBeMultiRepo := len(mi.repos)+1 >= 2 || totalConfigured >= 2 + // A second repo joining a live single-repo daemon flips the graph + // into prefixed-ID mode, but the first repo's nodes were minted + // unprefixed — re-mint them before they become unreachable. + if willBeMultiRepo { + mi.migrateLoneUnprefixedRepoCtx(ctx) + } + idx := mi.newPerRepoIndexer(cfg.Index) if willBeMultiRepo { idx.SetRepoPrefix(prefix) @@ -1143,6 +1233,7 @@ func (mi *MultiIndexer) TrackRepoCtx(ctx context.Context, entry config.RepoEntry ParseErrors: result.Errors, FileMtimes: idx.FileMtimes(), IsWorktree: ResolveWorktree(absPath).IsWorktree, + Unprefixed: !willBeMultiRepo, } mi.indexers[prefix] = idx mi.mu.Unlock() @@ -1226,6 +1317,13 @@ func (mi *MultiIndexer) ReconcileRepoCtx(ctx context.Context, entry config.RepoE } willBeMultiRepo := len(mi.repos)+1 >= 2 || totalConfigured >= 2 + // Same transition guard as TrackRepoCtx: an already-reconciled + // lone repo with unprefixed nodes must be re-minted before this + // second repo flips the graph into prefixed-ID mode. + if willBeMultiRepo { + mi.migrateLoneUnprefixedRepoCtx(ctx) + } + idx := mi.newPerRepoIndexer(cfg.Index) if willBeMultiRepo { idx.SetRepoPrefix(prefix) @@ -1287,6 +1385,7 @@ func (mi *MultiIndexer) ReconcileRepoCtx(ctx context.Context, entry config.RepoE ParseErrors: result.Errors, FileMtimes: idx.FileMtimes(), IsWorktree: ResolveWorktree(absPath).IsWorktree, + Unprefixed: !willBeMultiRepo, } mi.indexers[prefix] = idx mi.mu.Unlock() @@ -1399,7 +1498,20 @@ func (mi *MultiIndexer) UntrackRepo(repoPrefix string) (int, int) { delete(mi.indexers, repoPrefix) mi.mu.Unlock() - nodesRemoved, edgesRemoved := mi.graph.EvictRepo(repoPrefix) + var nodesRemoved, edgesRemoved int + if meta.Unprefixed { + // Single-repo-mode nodes carry RepoPrefix="" and never enter the + // byRepo bucket EvictRepo walks — evict them file-by-file off the + // recorded file set instead, or they linger in the graph and a + // later lone repo would mis-resolve them. + for path := range meta.FileMtimes { + n, e := mi.graph.EvictFile(path) + nodesRemoved += n + edgesRemoved += e + } + } else { + nodesRemoved, edgesRemoved = mi.graph.EvictRepo(repoPrefix) + } // Remove from global config. if meta.RootPath != "" { @@ -1684,6 +1796,17 @@ func (mi *MultiIndexer) ResolveFilePath(prefixedPath string) string { } return "" } + // Collision guard for the lone unprefixed repo: its graph paths are + // raw relative paths, so one whose first segment happens to equal + // the repo's own prefix (repo "api" containing api/handlers.go) + // would be hijacked by the prefix-strip join. Prefer the raw join + // when that file actually exists on disk. + if meta := mi.loneRepoLocked(); meta != nil && meta.RootPath != "" { + raw := filepath.Join(meta.RootPath, prefixedPath) + if _, err := os.Stat(raw); err == nil { + return raw + } + } return filepath.Join(bestRoot, strings.TrimPrefix(prefixedPath, bestPrefix+"/")) } @@ -1728,13 +1851,20 @@ func (mi *MultiIndexer) RepoRoot(repoPrefix string) (string, bool) { } // loneRepoLocked returns the metadata of the only registered repo when -// exactly one repo is tracked, else nil. Caller must hold mi.mu. +// exactly one repo is tracked AND that repo was indexed unprefixed +// (single-repo mode). The provenance check matters: after a 1→2→1 +// track/untrack sequence the lone survivor can be a prefixed repo, and +// stale unprefixed nodes from the departed repo must keep failing +// closed instead of resolving against the wrong checkout. Caller must +// hold mi.mu. func (mi *MultiIndexer) loneRepoLocked() *RepoMetadata { if len(mi.repos) != 1 { return nil } for _, meta := range mi.repos { - return meta + if meta != nil && meta.Unprefixed { + return meta + } } return nil } diff --git a/internal/indexer/multi_transition_test.go b/internal/indexer/multi_transition_test.go new file mode 100644 index 00000000..9d5319e3 --- /dev/null +++ b/internal/indexer/multi_transition_test.go @@ -0,0 +1,128 @@ +package indexer + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/search" +) + +// Untracking a single-mode repo must actually evict its nodes. They +// carry RepoPrefix="" and never enter the byRepo bucket EvictRepo +// walks, so before the file-set eviction they lingered forever — and +// the lone-repo fallback would then resolve them against whichever +// repo was tracked next. +func TestUntrackRepo_SingleModeEvictsUnprefixedNodes(t *testing.T) { + dir := setupRepoDir(t, "repo-a") + + cm := newTestConfigManager(t) + g := graph.New() + mi := NewMultiIndexer(g, newTestRegistry(), search.NewBM25(), cm, zap.NewNop()) + + _, err := mi.TrackRepoCtx(context.Background(), config.RepoEntry{Path: dir, Name: "repo-a"}) + require.NoError(t, err) + require.NotNil(t, g.GetNode("main.go::Hello"), "single-mode node is unprefixed") + + nodes, _ := mi.UntrackRepo("repo-a") + assert.Greater(t, nodes, 0, "untrack must evict the unprefixed nodes") + assert.Nil(t, g.GetNode("main.go::Hello"), "stale unprefixed node must not survive untrack") + + // A different repo tracked afterwards (single mode again) must not + // inherit any of repo-a's identity. + dirB := setupRepoDir(t, "repo-b") + _, err = mi.TrackRepoCtx(context.Background(), config.RepoEntry{Path: dirB, Name: "repo-b"}) + require.NoError(t, err) + root, ok := mi.RepoRoot("") + require.True(t, ok) + assert.Equal(t, dirB, root) +} + +// Tracking a second repo into a live single-repo daemon re-mints the +// first repo's nodes with its prefix, so they stay reachable when the +// empty-prefix fallback disarms. +func TestTrackRepo_SecondRepoRemintsLoneUnprefixedRepo(t *testing.T) { + dirA := setupRepoDir(t, "repo-a") + dirB := setupRepoDir(t, "repo-b") + + cm := newTestConfigManager(t) + g := graph.New() + mi := NewMultiIndexer(g, newTestRegistry(), search.NewBM25(), cm, zap.NewNop()) + + _, err := mi.TrackRepoCtx(context.Background(), config.RepoEntry{Path: dirA, Name: "repo-a"}) + require.NoError(t, err) + require.NotNil(t, g.GetNode("main.go::Hello"), "lone repo indexes unprefixed") + + _, err = mi.TrackRepoCtx(context.Background(), config.RepoEntry{Path: dirB, Name: "repo-b"}) + require.NoError(t, err) + + // repo-a's nodes are re-minted under its prefix; the unprefixed + // originals are gone. + assert.Nil(t, g.GetNode("main.go::Hello"), "unprefixed originals must be evicted on transition") + require.NotNil(t, g.GetNode("repo-a/main.go::Hello"), "repo-a re-minted with prefix") + require.NotNil(t, g.GetNode("repo-b/main.go::Hello"), "repo-b indexed with prefix") + + // With two repos the empty-prefix fallback stays ambiguous. + _, ok := mi.RepoRoot("") + assert.False(t, ok) + + // Both repos resolve through their prefixes. + root, ok := mi.RepoRoot("repo-a") + require.True(t, ok) + assert.Equal(t, dirA, root) +} + +// After a 1→2→1 sequence the lone survivor is a *prefixed* repo; the +// empty-prefix fallback must keep failing closed for it so any stale +// unprefixed ID cannot resolve against the wrong checkout. +func TestRepoRoot_EmptyPrefixFailsClosedForLonePrefixedRepo(t *testing.T) { + dirA := setupRepoDir(t, "repo-a") + dirB := setupRepoDir(t, "repo-b") + + cm := newTestConfigManager(t) + g := graph.New() + mi := NewMultiIndexer(g, newTestRegistry(), search.NewBM25(), cm, zap.NewNop()) + + ctx := context.Background() + _, err := mi.TrackRepoCtx(ctx, config.RepoEntry{Path: dirA, Name: "repo-a"}) + require.NoError(t, err) + _, err = mi.TrackRepoCtx(ctx, config.RepoEntry{Path: dirB, Name: "repo-b"}) + require.NoError(t, err) + mi.UntrackRepo("repo-a") + + // repo-b is the lone survivor but was minted prefixed: the empty + // prefix must not resolve to it. + _, ok := mi.RepoRoot("") + assert.False(t, ok, "lone prefixed repo must not satisfy the empty-prefix fallback") + assert.Empty(t, mi.ResolveFilePath("main.go")) +} + +// A repo whose directory name collides with one of its own top-level +// directories: single-repo graph paths are raw relative paths, so the +// prefix-matching join must not hijack them. +func TestResolveFilePath_LoneRepoOwnPrefixCollision(t *testing.T) { + base := t.TempDir() + dir := filepath.Join(base, "api") + require.NoError(t, os.MkdirAll(filepath.Join(dir, "api"), 0o755)) + writeFile(t, filepath.Join(dir, "api", "handlers.go"), "package api\n\nfunc Handle() {}\n") + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc Hello() {}\n") + + cm := newTestConfigManager(t) + g := graph.New() + mi := NewMultiIndexer(g, newTestRegistry(), search.NewBM25(), cm, zap.NewNop()) + _, err := mi.TrackRepoCtx(context.Background(), config.RepoEntry{Path: dir, Name: "api"}) + require.NoError(t, err) + + // The graph path "api/handlers.go" names /api/handlers.go — + // not /handlers.go via prefix stripping. + assert.Equal(t, filepath.Join(dir, "api", "handlers.go"), mi.ResolveFilePath("api/handlers.go")) + // Plain unprefixed paths still anchor to the root. + assert.Equal(t, filepath.Join(dir, "main.go"), mi.ResolveFilePath("main.go")) +} diff --git a/internal/mcp/tools_fileops.go b/internal/mcp/tools_fileops.go index eead8ef9..1edb0586 100644 --- a/internal/mcp/tools_fileops.go +++ b/internal/mcp/tools_fileops.go @@ -84,6 +84,19 @@ func (s *Server) resolveFilePath(rawPath string) (absPath, relPath string, err e if !ok { return "", "", fmt.Errorf("%w: repo prefix %q has no root path", errPathUnresolved, prefix) } + // Collision guard for the lone unprefixed repo: its indexed + // paths are raw relative paths, so one whose first segment + // equals the repo's own prefix (repo "api" containing + // api/handlers.go) would be hijacked by the prefix-strip join. + // Prefer the raw join when that file actually exists. + if loneRoot, lok := s.multiIndexer.RepoRoot(""); lok && loneRoot == root { + raw := filepath.Clean(filepath.Join(loneRoot, rawPath)) + if pathContainedIn(raw, loneRoot) { + if _, err := os.Stat(raw); err == nil { + return worktreeRootedPath(raw, loneRoot, s.multiIndexer), rawPath, nil + } + } + } abs := filepath.Clean(filepath.Join(root, strings.TrimPrefix(rawPath, prefix+"/"))) if !pathContainedIn(abs, root) { return "", "", fmt.Errorf("%w: %q resolves to %q, outside repo root %q", errPathEscape, rawPath, abs, root) From a514d3a453f87ff6d400c2b60a8f7d961291942d Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 01:11:42 +0200 Subject: [PATCH 09/12] mcp: truthful savings accounting on conditional fetches; cached calibration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three accounting defects from the review. Conditional fetches that hit the etag (if_none_match) booked full-content savings while shipping a 47-byte not_modified stub — a polling client minted unbounded fake savings; read_file and get_file_summary now record after the conditional-fetch return, like get_editing_context always did, which also skips tokenizing the content on the cheap turnaround. EstimateFromSample ran the uncached tokenizer over the whole sample on every call (~200ms per MB on the read_file hot path, even when the line above had just cache-counted identical bytes); it now goes through the disk cache, and an uncompressed read_file skips the second pass entirely (returned == baseline by construction). Attribution: get_file_summary records the payload of the format it actually returns instead of pricing every format off the compact rendering (which also double-rendered the compact path); out-of-repo absolute reads stay unattributed instead of polluting the lone repo's buckets; and symbol-tool events in single-repo mode now land in the same per-repo bucket as the read family via the lone-prefix attribution. --- internal/mcp/tools_coding.go | 6 +-- internal/mcp/tools_core.go | 36 ++++++++++------- internal/mcp/tools_fileops.go | 49 ++++++++++++++++++----- internal/mcp/tools_savings_record_test.go | 48 ++++++++++++++++++++++ internal/tokens/tokens.go | 6 ++- 5 files changed, 116 insertions(+), 29 deletions(-) diff --git a/internal/mcp/tools_coding.go b/internal/mcp/tools_coding.go index 1e7f03b9..811dfd0a 100644 --- a/internal/mcp/tools_coding.go +++ b/internal/mcp/tools_coding.go @@ -838,7 +838,7 @@ func (s *Server) handleGetSymbolSource(ctx context.Context, req mcp.CallToolRequ // response). Aggregated stats remain available via the `savings` tool. returned := tokens.CachedCountInt64(source) fullFile := int64(tokens.EstimateFromSample(totalFileChars, source)) - s.tokenStatsFor(ctx).record(node, "get_symbol_source", returned, fullFile) + s.tokenStatsFor(ctx).record(s.savingsAttributionNode(node), "get_symbol_source", returned, fullFile) result := map[string]any{ "id": node.ID, @@ -1058,7 +1058,7 @@ func (s *Server) handleBatchSymbols(ctx context.Context, req mcp.CallToolRequest entry["from_line"] = fromLine returned := tokens.CachedCountInt64(source) fullFile := int64(tokens.EstimateFromSample(totalFileChars, source)) - s.tokenStatsFor(ctx).record(node, "batch_symbols", returned, fullFile) + s.tokenStatsFor(ctx).record(s.savingsAttributionNode(node), "batch_symbols", returned, fullFile) } } } @@ -1921,7 +1921,7 @@ func (s *Server) handleSmartContext(ctx context.Context, req mcp.CallToolRequest sourcesEmbedded++ returned := tokens.CachedCountInt64(source) fullFile := int64(tokens.EstimateFromSample(totalFileChars, source)) - s.tokenStatsFor(ctx).record(sym, "smart_context", returned, fullFile) + s.tokenStatsFor(ctx).record(s.savingsAttributionNode(sym), "smart_context", returned, fullFile) } } } diff --git a/internal/mcp/tools_core.go b/internal/mcp/tools_core.go index 04914293..f5ab5751 100644 --- a/internal/mcp/tools_core.go +++ b/internal/mcp/tools_core.go @@ -1872,11 +1872,22 @@ func (s *Server) handleGetFileSummary(ctx context.Context, req mcp.CallToolReque return mcp.NewToolResultError("no symbols found for file: " + fp), nil } + // ETag conditional fetch — checked before any savings accounting so + // a not_modified turnaround books nothing (a polling client would + // otherwise mint fake savings on every poll). Use the structural + // SubGraph hash — json.Marshal'ing the whole SubGraph + Meta on + // every call was the dominant cost on large files (~2 ms / call on + // a 500-symbol file). + etag := etagSubGraph(sg) + if ifNoneMatch := req.GetString("if_none_match", ""); ifNoneMatch != "" && ifNoneMatch == etag { + return notModifiedResult(etag), nil + } + // Server-side accounting only — a file summary stands in for - // reading the whole file. The compact rendering doubles as the - // payload sample for every output format, which makes the recorded - // `returned` an upper bound for gcx and a fair count for - // compact/json. + // reading the whole file. The payload sample tracks the format + // actually returned (the compact text for compact/gcx, the + // marshaled result for json/toon) so `returned` reflects what was + // shipped. summaryLang := "" for _, n := range sg.Nodes { if n != nil && n.Language != "" { @@ -1884,21 +1895,15 @@ func (s *Server) handleGetFileSummary(ctx context.Context, req mcp.CallToolReque break } } - s.recordFileBaselineSavings(ctx, "get_file_summary", fp, summaryLang, compactSubGraph(sg)) if isCompact(req) { - return mcp.NewToolResultText(compactSubGraph(sg)), nil - } - - // ETag conditional fetch. Use the structural SubGraph hash — - // json.Marshal'ing the whole SubGraph + Meta on every call was the - // dominant cost on large files (~2 ms / call on a 500-symbol file). - etag := etagSubGraph(sg) - if ifNoneMatch := req.GetString("if_none_match", ""); ifNoneMatch != "" && ifNoneMatch == etag { - return notModifiedResult(etag), nil + payload := compactSubGraph(sg) + s.recordFileBaselineSavings(ctx, "get_file_summary", fp, summaryLang, payload) + return mcp.NewToolResultText(payload), nil } if s.isGCX(ctx, req) { + s.recordFileBaselineSavings(ctx, "get_file_summary", fp, summaryLang, compactSubGraph(sg)) return s.gcxResponseWithBudget(req)(encodeFileSummary(sg, etag)) } @@ -1911,6 +1916,9 @@ func (s *Server) handleGetFileSummary(ctx context.Context, req mcp.CallToolReque "truncated": sg.Truncated, "etag": etag, } + if payload, merr := json.Marshal(result); merr == nil { + s.recordFileBaselineSavings(ctx, "get_file_summary", fp, summaryLang, string(payload)) + } if s.isTOON(ctx, req) { return returnTOON(result) } diff --git a/internal/mcp/tools_fileops.go b/internal/mcp/tools_fileops.go index 1edb0586..97b0f05c 100644 --- a/internal/mcp/tools_fileops.go +++ b/internal/mcp/tools_fileops.go @@ -361,7 +361,11 @@ func (s *Server) fileAttributionNode(relPath, language string) *graph.Node { prefix := "" if s.multiIndexer != nil { prefix = matchedRepoPrefix(s.multiIndexer, relPath) - if prefix == "" { + // Lone-repo attribution applies only to repo-relative paths — + // an absolute path that matched no prefix points outside every + // tracked repo and must stay unattributed rather than polluting + // the lone repo's buckets. + if prefix == "" && !filepath.IsAbs(relPath) { if prefixes := s.multiIndexer.RepoPrefixes(); len(prefixes) == 1 { prefix = prefixes[0] } @@ -370,6 +374,23 @@ func (s *Server) fileAttributionNode(relPath, language string) *graph.Node { return &graph.Node{RepoPrefix: prefix, Language: language, FilePath: relPath} } +// savingsAttributionNode fills the per-repo attribution for symbol +// nodes minted in single-repo mode (RepoPrefix="") so symbol-tool +// events land in the same per-repo bucket the read-family tools use. +// The graph node itself is never mutated. +func (s *Server) savingsAttributionNode(node *graph.Node) *graph.Node { + if node == nil || node.RepoPrefix != "" || s.multiIndexer == nil { + return node + } + prefixes := s.multiIndexer.RepoPrefixes() + if len(prefixes) != 1 { + return node + } + cp := *node + cp.RepoPrefix = prefixes[0] + return &cp +} + // recordFileBaselineSavings books a savings observation for a tool whose // response stands in for reading a whole file. payload is the response // content actually produced — used both for the returned-token count and @@ -773,16 +794,6 @@ func (s *Server) handleReadFile(ctx context.Context, req mcp.CallToolRequest) (* } } - // Server-side accounting only — read_file is the heaviest source - // fetch and must show up in the savings ledger even when nothing - // was saved (an uncompressed read returns the whole file, so - // returned == baseline and only the call is counted). - if !isBinary { - returned := tokens.CachedCountInt64(string(content)) - fullFile := int64(tokens.EstimateFromSample(originalBytes, string(content))) - s.tokenStatsFor(ctx).record(s.fileAttributionNode(relPath, language), "read_file", returned, fullFile) - } - // Record the access for frecency credit on any node defined in // this file. read_file is a heavy access (full file), so we // credit every defined symbol — keeps the "agent is working in @@ -840,6 +851,22 @@ func (s *Server) handleReadFile(ctx context.Context, req mcp.CallToolRequest) (* } result["etag"] = etag + // Server-side accounting only — read_file is the heaviest source + // fetch and must show up in the savings ledger even when nothing + // was saved (an uncompressed read returns the whole file, so + // returned == baseline and only the call is counted). Recorded + // after the conditional-fetch return so a not_modified turnaround + // books nothing and skips the tokenization entirely. + if !isBinary { + contentStr := string(content) + returned := tokens.CachedCountInt64(contentStr) + fullFile := returned + if bodiesElided || salienceTruncated { + fullFile = int64(tokens.EstimateFromSample(originalBytes, contentStr)) + } + s.tokenStatsFor(ctx).record(s.fileAttributionNode(relPath, language), "read_file", returned, fullFile) + } + if s.isTOON(ctx, req) { return returnTOON(result) } diff --git a/internal/mcp/tools_savings_record_test.go b/internal/mcp/tools_savings_record_test.go index 421ce225..66f9a762 100644 --- a/internal/mcp/tools_savings_record_test.go +++ b/internal/mcp/tools_savings_record_test.go @@ -2,9 +2,13 @@ package mcp import ( "context" + "encoding/json" "testing" + mcplib "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/savings" ) // TestReadFamilyToolsRecordSavings pins the savings recording surface on a @@ -17,18 +21,33 @@ func TestReadFamilyToolsRecordSavings(t *testing.T) { srv, _, _ := newSingleRepoServer(t) ctx := context.Background() + store, err := savings.Open("") + require.NoError(t, err) + srv.InitSavings(store, "") + calls := func() int64 { return srv.tokenStats.snapshot()["calls_counted"].(int64) } require.Equal(t, int64(0), calls()) + etagOf := func(raw string) string { + var parsed struct { + ETag string `json:"etag"` + } + require.NoError(t, json.Unmarshal([]byte(raw), &parsed)) + require.NotEmpty(t, parsed.ETag) + return parsed.ETag + } + res := callToolByName(t, srv, ctx, "read_file", map[string]any{"path": "main.go"}) require.False(t, res.IsError, "read_file must succeed on a bare-relative path in single-repo mode") require.Equal(t, int64(1), calls(), "read_file must record a savings observation") + readEtag := etagOf(textOfResult(t, res)) res = callToolByName(t, srv, ctx, "get_file_summary", map[string]any{"path": "main.go"}) require.False(t, res.IsError) require.Equal(t, int64(2), calls(), "get_file_summary must record a savings observation") + summaryEtag := etagOf(textOfResult(t, res)) res = callToolByName(t, srv, ctx, "get_editing_context", map[string]any{"path": "main.go"}) require.False(t, res.IsError) @@ -38,6 +57,35 @@ func TestReadFamilyToolsRecordSavings(t *testing.T) { require.False(t, res.IsError, "get_symbol_source must resolve unprefixed single-repo nodes") require.Equal(t, int64(4), calls(), "get_symbol_source must record a savings observation") + // Conditional fetches that hit the etag transfer ~nothing and must + // book nothing — a polling client would otherwise mint fake savings + // on every poll. + res = callToolByName(t, srv, ctx, "read_file", map[string]any{"path": "main.go", "if_none_match": readEtag}) + require.False(t, res.IsError) + require.Equal(t, int64(4), calls(), "not-modified read_file must not record") + + res = callToolByName(t, srv, ctx, "get_file_summary", map[string]any{"path": "main.go", "if_none_match": summaryEtag}) + require.False(t, res.IsError) + require.Equal(t, int64(4), calls(), "not-modified get_file_summary must not record") + snap := srv.tokenStats.snapshot() require.Greater(t, snap["tokens_returned"].(int64), int64(0)) + + // Single-repo events attribute to the lone repo's prefix — the same + // bucket key multi-repo mode would use — for every recording tool. + ledger := store.Snapshot() + require.NotNil(t, ledger.PerRepo["myrepo"], "events must land in the lone repo's per-repo bucket, got %v", ledger.PerRepo) + require.Equal(t, int64(4), ledger.PerRepo["myrepo"].CallsCounted) +} + +// textOfResult extracts the first text content of a tool result. +func textOfResult(t *testing.T, res *mcplib.CallToolResult) string { + t.Helper() + for _, c := range res.Content { + if tc, ok := c.(mcplib.TextContent); ok { + return tc.Text + } + } + t.Fatal("tool result has no text content") + return "" } diff --git a/internal/tokens/tokens.go b/internal/tokens/tokens.go index a5e0d108..0a2e342d 100644 --- a/internal/tokens/tokens.go +++ b/internal/tokens/tokens.go @@ -65,7 +65,11 @@ func EstimateFromSample(totalChars int, sample string) int { if sample == "" { return totalChars / charsPerTokenFallback } - sampleTokens := Count(sample) + // CachedCount, not Count: callers calibrate with a payload they have + // typically just counted — the disk cache turns this second pass + // over the same bytes into a hash lookup instead of a full BPE run + // (~200ms per MB). + sampleTokens := CachedCount(sample) if sampleTokens == 0 || len(sample) == 0 { return totalChars / charsPerTokenFallback } From d5e9786f1dd2c8b9bc170e6dfa96b88d03ec42f6 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 01:12:58 +0200 Subject: [PATCH 10/12] tokens: age-sweep the disk token-count cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content-addressed cache grew one inode per unique payload forever — and the read-family instrumentation now feeds it every file version. Every 64th write prunes the shard it just wrote of entries idle longer than 30 days; read hits refresh the entry mtime so the TTL approximates LRU for content still flowing through the counters. Best-effort by construction: a swept entry is just a future miss. --- internal/tokens/cache.go | 45 +++++++++++++++++- internal/tokens/cache_sweep_test.go | 72 +++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 internal/tokens/cache_sweep_test.go diff --git a/internal/tokens/cache.go b/internal/tokens/cache.go index c5c2adf7..bec45f42 100644 --- a/internal/tokens/cache.go +++ b/internal/tokens/cache.go @@ -18,6 +18,8 @@ import ( "strconv" "strings" "sync" + "sync/atomic" + "time" "github.com/zzet/gortex/internal/platform" ) @@ -31,6 +33,17 @@ const tokenCacheFormat = "1" // than the tokenization it would save. const minCacheBytes = 2048 +// sweepEvery / sweepMaxAge bound the cache's growth: every sweepEvery-th +// write prunes the shard directory just written of entries older than +// sweepMaxAge. Entries are one content version each and never reused +// once the content changes, so without a sweep the cache grows one +// inode per unique payload forever. Read hits refresh the entry's +// mtime, so the TTL approximates LRU for content that is still live. +const ( + sweepEvery = 64 + sweepMaxAge = 30 * 24 * time.Hour +) + // DiskCache is a content-addressed token-count cache backed by a // directory tree. It is safe for concurrent use — entries are written // atomically (temp + rename) and a torn or absent entry simply falls @@ -38,6 +51,7 @@ const minCacheBytes = 2048 type DiskCache struct { dir string revision string + writes atomic.Uint64 } // DefaultTokenCacheDir returns the default cache location: @@ -97,7 +111,8 @@ func (c *DiskCache) pathFor(key string) string { } func (c *DiskCache) read(key string) (int, bool) { - data, err := os.ReadFile(c.pathFor(key)) + path := c.pathFor(key) + data, err := os.ReadFile(path) if err != nil { return 0, false } @@ -105,6 +120,10 @@ func (c *DiskCache) read(key string) (int, bool) { if err != nil || n < 0 { return 0, false } + // Refresh the entry so the age sweep approximates LRU: content that + // is still being counted stays; content that stopped flowing ages out. + now := time.Now() + _ = os.Chtimes(path, now, now) return n, true } @@ -129,6 +148,30 @@ func (c *DiskCache) write(key string, n int) { } if err := os.Rename(tmpName, path); err != nil { _ = os.Remove(tmpName) + return + } + if c.writes.Add(1)%sweepEvery == 0 { + c.sweepShard(filepath.Dir(path)) + } +} + +// sweepShard removes entries in one shard directory whose mtime is +// older than sweepMaxAge. Best-effort and concurrent-safe by +// construction: a deleted entry is just a future cache miss. +func (c *DiskCache) sweepShard(dir string) { + entries, err := os.ReadDir(dir) + if err != nil { + return + } + cutoff := time.Now().Add(-sweepMaxAge) + for _, e := range entries { + info, ierr := e.Info() + if ierr != nil || info.IsDir() { + continue + } + if info.ModTime().Before(cutoff) { + _ = os.Remove(filepath.Join(dir, e.Name())) + } } } diff --git a/internal/tokens/cache_sweep_test.go b/internal/tokens/cache_sweep_test.go new file mode 100644 index 00000000..5d4b10d7 --- /dev/null +++ b/internal/tokens/cache_sweep_test.go @@ -0,0 +1,72 @@ +package tokens + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// The age sweep removes stale entries from a shard and keeps fresh +// ones; read hits refresh an entry's mtime so live content survives. +func TestSweepShard_RemovesStaleKeepsFresh(t *testing.T) { + dir := t.TempDir() + c := NewDiskCache(dir) + + shard := filepath.Join(dir, "ab") + if err := os.MkdirAll(shard, 0o755); err != nil { + t.Fatal(err) + } + stale := filepath.Join(shard, "stale-entry") + fresh := filepath.Join(shard, "fresh-entry") + for _, p := range []string{stale, fresh} { + if err := os.WriteFile(p, []byte("42"), 0o644); err != nil { + t.Fatal(err) + } + } + old := time.Now().Add(-2 * sweepMaxAge) + if err := os.Chtimes(stale, old, old); err != nil { + t.Fatal(err) + } + + c.sweepShard(shard) + + if _, err := os.Stat(stale); !os.IsNotExist(err) { + t.Errorf("stale entry should be swept, stat err=%v", err) + } + if _, err := os.Stat(fresh); err != nil { + t.Errorf("fresh entry must survive the sweep: %v", err) + } +} + +// A cache hit refreshes the entry's mtime, so the TTL behaves like LRU +// for content that is still flowing through the counters. +func TestCacheRead_RefreshesMtime(t *testing.T) { + dir := t.TempDir() + c := NewDiskCache(dir) + + content := make([]byte, minCacheBytes+1) + for i := range content { + content[i] = 'a' + } + s := string(content) + want := c.Count(s) // miss → write + + key := c.key(s) + path := c.pathFor(key) + old := time.Now().Add(-2 * sweepMaxAge) + if err := os.Chtimes(path, old, old); err != nil { + t.Fatal(err) + } + + if got := c.Count(s); got != want { + t.Fatalf("cached count = %d, want %d", got, want) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.ModTime().Before(time.Now().Add(-time.Hour)) { + t.Errorf("read hit must refresh the entry mtime, got %v", info.ModTime()) + } +} From 762a42d93c375b830fac32e1d69256a25a03c7ab Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 01:21:24 +0200 Subject: [PATCH 11/12] savings: harden the ledger against the review's failure modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Crash class: a legacy savings.json carrying JSON null bucket values panicked the import on every server start — readLegacyFile now drops nil entries at the single choke point. A hard event-log read error aborts the import without marking or renaming, so a truncated read is retried next open instead of becoming a permanent loss. Consistency class: ResetSavings runs its three DELETEs in one transaction (a concurrent observation either survives whole or is wiped whole); the import floors totals per bucket and field at what the events reconstruct, so a flush-lagged cumulative file can't leave 'Last 7 days' above 'All time'; the already-imported path self-heals lingering flat files left by a crash between commit and rename. Diagnosability class: a failing ledger no longer mimics a fresh install — Snapshot returns its read error (the CLI and graph_stats surface it), dropped writes warn once on stderr and ride on the snapshot as dropped_observations, and rename failures propagate. Scale class: the dashboard now reads one week of events plus a SQL per-tool aggregate (SavingsToolTotals) instead of materializing the full event history per invocation, and Store gains Close so tests and one-shot callers release the process-cached handle. --- cmd/gortex/gain.go | 23 +-- cmd/gortex/gain_test.go | 11 +- cmd/gortex/savings.go | 16 +- cmd/gortex/savings_test.go | 8 +- internal/mcp/server.go | 11 +- internal/mcp/tools_savings_record_test.go | 3 +- internal/persistence/sidecar_savings.go | 52 +++++- internal/savings/events.go | 12 +- internal/savings/events_test.go | 9 +- internal/savings/store.go | 205 +++++++++++++++++----- internal/savings/store_test.go | 166 ++++++++++++++++-- 11 files changed, 419 insertions(+), 97 deletions(-) diff --git a/cmd/gortex/gain.go b/cmd/gortex/gain.go index 94df475e..0f516fc1 100644 --- a/cmd/gortex/gain.go +++ b/cmd/gortex/gain.go @@ -7,10 +7,10 @@ // // gain vs savings: // - savings — rear-looking: what HAPPENED across past MCP calls -// (your actual cumulative store + JSONL event log) +// (your actual cumulative store + JSONL event log) // - gain — forward-looking: what gortex SAVES on a typical -// call (benchmark-derived, works on fresh installs -// with no history) +// call (benchmark-derived, works on fresh installs +// with no history) package main import ( @@ -303,12 +303,12 @@ func renderGainProjection(w interface{ Write([]byte) (int, error) }, rows []toke // constrained to the --since window. Zero-population when no calls // fell inside the window. type gainHistory struct { - Path string - Since time.Duration - Calls int64 - Saved int64 - Returned int64 - Costs map[string]float64 + Path string + Since time.Duration + Calls int64 + Saved int64 + Returned int64 + Costs map[string]float64 } func (h *gainHistory) toJSON() map[string]any { @@ -338,7 +338,10 @@ func loadHistory(cacheDir string, since time.Duration) (*gainHistory, error) { return nil, err } _ = store.ImportLegacy(legacyJSON) - snap := store.Snapshot() + snap, serr := store.Snapshot() + if serr != nil { + fmt.Fprintf(os.Stderr, "[gortex gain] savings totals read failed: %v\n", serr) + } if since <= 0 { // --since 0 → entire-history view; just use the cumulative diff --git a/cmd/gortex/gain_test.go b/cmd/gortex/gain_test.go index f6610014..f9e76f61 100644 --- a/cmd/gortex/gain_test.go +++ b/cmd/gortex/gain_test.go @@ -15,11 +15,11 @@ import ( func TestHumanDuration(t *testing.T) { cases := map[time.Duration]string{ - 24 * time.Hour: "24h", - 7 * 24 * time.Hour: "7d", - 30 * 24 * time.Hour: "30d", - 2 * time.Hour: "2h", - 90 * time.Minute: "1h30m0s", + 24 * time.Hour: "24h", + 7 * 24 * time.Hour: "7d", + 30 * 24 * time.Hour: "30d", + 2 * time.Hour: "2h", + 90 * time.Minute: "1h30m0s", } for in, want := range cases { if got := humanDuration(in); got != want { @@ -240,4 +240,3 @@ func TestGainCmd_Registered(t *testing.T) { t.Errorf("rootCmd missing `gain`; have %v", subs) } } - diff --git a/cmd/gortex/savings.go b/cmd/gortex/savings.go index 18e5b41b..3854e478 100644 --- a/cmd/gortex/savings.go +++ b/cmd/gortex/savings.go @@ -85,20 +85,30 @@ func runSavings(_ *cobra.Command, _ []string) error { return nil } - snap := store.Snapshot() + snap, serr := store.Snapshot() + if serr != nil { + // Surface it: an unreadable ledger must not masquerade as a + // fresh install's "nothing recorded yet" empty state. + fmt.Fprintf(os.Stderr, "[gortex savings] totals read failed: %v\n", serr) + } loc := time.Local if savingsUTC { loc = time.UTC } - events, err := store.EventsSince(time.Time{}) + now := time.Now() + events, err := store.EventsSince(now.Add(-7 * 24 * time.Hour)) if err != nil { // Don't fail the whole command on event read errors — fall back // to a dashboard with empty Today/7d buckets. fmt.Fprintf(os.Stderr, "[gortex savings] event history read failed: %v\n", err) events = nil } - buckets := savings.BuildDashboard(events, snap.Totals, time.Now(), loc) + allPerTool, err := store.ToolTotals(time.Time{}) + if err != nil { + fmt.Fprintf(os.Stderr, "[gortex savings] per-tool aggregate failed: %v\n", err) + } + buckets := savings.BuildDashboard(events, snap.Totals, allPerTool, now, loc) if savingsJSON { return emitSavingsJSON(snap, buckets, dbPath) diff --git a/cmd/gortex/savings_test.go b/cmd/gortex/savings_test.go index 544fb832..fe5ae3c4 100644 --- a/cmd/gortex/savings_test.go +++ b/cmd/gortex/savings_test.go @@ -41,8 +41,8 @@ func captureStdout(t *testing.T, fn func()) string { func TestPickHeadlineCost_RespectsModelFlag(t *testing.T) { costs := map[string]float64{ - "claude-opus-4": 0.50, - "claude-sonnet-4": 0.10, + "claude-opus-4": 0.50, + "claude-sonnet-4": 0.10, "claude-haiku-4.5": 0.03, } val, name := pickHeadlineCost(costs, "claude-sonnet-4") @@ -165,8 +165,8 @@ func TestEmitSavingsDashboard_RendersThreeBuckets(t *testing.T) { "Today", "Last 7 days", "All time", - "█", // at least one filled bar cell - "░", // at least one empty bar cell + "█", // at least one filled bar cell + "░", // at least one empty bar cell "get_symbol_source", // verbose per-tool table "smart_context", "Cost avoided per model (all time):", diff --git a/internal/mcp/server.go b/internal/mcp/server.go index c29790b2..7a63727c 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -1575,9 +1575,12 @@ func (s *Server) cumulativeSavingsSnapshot() map[string]any { return nil } - snap := store.Snapshot() + snap, err := store.Snapshot() + if err != nil && s.logger != nil { + s.logger.Warn("cumulative savings snapshot failed", zap.Error(err)) + } costs := savings.CostAvoidedAll(snap.Totals.TokensSaved) - return map[string]any{ + out := map[string]any{ "first_seen": snap.FirstSeen.Format(time.RFC3339), "last_updated": snap.LastUpdated.Format(time.RFC3339), "tokens_saved": snap.Totals.TokensSaved, @@ -1585,6 +1588,10 @@ func (s *Server) cumulativeSavingsSnapshot() map[string]any { "calls_counted": snap.Totals.CallsCounted, "cost_avoided_usd": costs, } + if snap.DroppedObservations > 0 { + out["dropped_observations"] = snap.DroppedObservations + } + return out } // ExportContext generates a portable context briefing for the given task. diff --git a/internal/mcp/tools_savings_record_test.go b/internal/mcp/tools_savings_record_test.go index 66f9a762..86a300da 100644 --- a/internal/mcp/tools_savings_record_test.go +++ b/internal/mcp/tools_savings_record_test.go @@ -73,7 +73,8 @@ func TestReadFamilyToolsRecordSavings(t *testing.T) { // Single-repo events attribute to the lone repo's prefix — the same // bucket key multi-repo mode would use — for every recording tool. - ledger := store.Snapshot() + ledger, lerr := store.Snapshot() + require.NoError(t, lerr) require.NotNil(t, ledger.PerRepo["myrepo"], "events must land in the lone repo's per-repo bucket, got %v", ledger.PerRepo) require.Equal(t, int64(4), ledger.PerRepo["myrepo"].CallsCounted) } diff --git a/internal/persistence/sidecar_savings.go b/internal/persistence/sidecar_savings.go index 7c72b0b9..5ea3120b 100644 --- a/internal/persistence/sidecar_savings.go +++ b/internal/persistence/sidecar_savings.go @@ -112,7 +112,7 @@ func upsertSavingsBucket(tx *sql.Tx, bucket string, saved, returned, calls int64 } // SavingsTotals returns every totals bucket plus the first_seen / -// last_updated stamps. Buckets map keys are '' (top-line), +// last_updated stamps. Buckets map keys are ” (top-line), // 'repo:', and 'lang:'. The zero time means "never". func (s *SidecarStore) SavingsTotals() (map[string]SavingsTotalsRow, time.Time, time.Time, error) { if s == nil { @@ -262,7 +262,46 @@ func (s *SidecarStore) ImportLegacySavings(buckets map[string]SavingsTotalsRow, return tx.Commit() } -// ResetSavings wipes the savings ledger (events, totals, meta). The +// SavingsToolTotals aggregates events per tool over ts >= since +// (zero = all time), tokens-saved descending — the dashboard breakdown +// without materializing the event history into Go. +func (s *SidecarStore) SavingsToolTotals(since time.Time) ([]SavingsToolRow, error) { + if s == nil { + return nil, nil + } + rows, err := s.db.Query( + `SELECT tool, SUM(saved), SUM(returned), COUNT(1) + FROM savings_events WHERE ts >= ? + GROUP BY tool ORDER BY SUM(saved) DESC, tool`, + unixOrZero(since), + ) + if err != nil { + return nil, fmt.Errorf("persistence: savings tool totals: %w", err) + } + defer func() { _ = rows.Close() }() + + var out []SavingsToolRow + for rows.Next() { + var r SavingsToolRow + if err := rows.Scan(&r.Tool, &r.Saved, &r.Returned, &r.Calls); err != nil { + return nil, err + } + out = append(out, r) + } + return out, rows.Err() +} + +// SavingsToolRow is one per-tool aggregate row. +type SavingsToolRow struct { + Tool string + Saved int64 + Returned int64 + Calls int64 +} + +// ResetSavings wipes the savings ledger (events, totals, meta) in one +// transaction, so a concurrent writer's observation either survives +// whole or is wiped whole — never an orphan event without totals. The // legacy-import migration mark survives so renamed flat files are not // re-imported after a reset. func (s *SidecarStore) ResetSavings() error { @@ -271,14 +310,19 @@ func (s *SidecarStore) ResetSavings() error { } s.writeMu.Lock() defer s.writeMu.Unlock() + tx, err := s.db.Begin() + if err != nil { + return fmt.Errorf("persistence: savings reset tx: %w", err) + } + defer func() { _ = tx.Rollback() }() for _, stmt := range []string{ `DELETE FROM savings_events`, `DELETE FROM savings_totals`, `DELETE FROM savings_meta`, } { - if _, err := s.db.Exec(stmt); err != nil { + if _, err := tx.Exec(stmt); err != nil { return fmt.Errorf("persistence: savings reset: %w", err) } } - return nil + return tx.Commit() } diff --git a/internal/savings/events.go b/internal/savings/events.go index 227fec5c..f0d2d06a 100644 --- a/internal/savings/events.go +++ b/internal/savings/events.go @@ -168,20 +168,20 @@ func FilterDay(events []Event, day time.Time, loc *time.Location) []Event { } // BuildDashboard returns the three canonical buckets (Today / Last 7 days / -// All time) from the full event history (oldest first), using `now` as the +// All time) from the last week's events (oldest first), using `now` as the // reference clock and `loc` as the calendar for the "Today" boundary. -// storeAllTime supplies the All-time totals — it can exceed what the events -// reconstruct when history predates the event log (flat-file era imports). -func BuildDashboard(events []Event, storeAllTime Totals, now time.Time, loc *time.Location) []Bucket { +// storeAllTime supplies the All-time totals and allPerTool its per-tool +// breakdown — both come from the ledger's aggregates, so callers never +// materialize the full event history just to render the dashboard. +func BuildDashboard(weekEvents []Event, storeAllTime Totals, allPerTool []ToolTotal, now time.Time, loc *time.Location) []Bucket { if loc == nil { loc = time.Local } - weekEvents := FilterSince(events, now.Add(-7*24*time.Hour)) + weekEvents = FilterSince(weekEvents, now.Add(-7*24*time.Hour)) todayEvents := FilterDay(weekEvents, now, loc) todayTotals, todayPerTool := AggregateByTool(todayEvents) weekTotals, weekPerTool := AggregateByTool(weekEvents) - _, allPerTool := AggregateByTool(events) return []Bucket{ {Label: "Today", Totals: todayTotals, PerTool: todayPerTool}, diff --git a/internal/savings/events_test.go b/internal/savings/events_test.go index 5ab1bfcd..e4a0eb83 100644 --- a/internal/savings/events_test.go +++ b/internal/savings/events_test.go @@ -48,6 +48,9 @@ func TestEventsPathFor(t *testing.T) { func TestAddObservation_RecordsEvent(t *testing.T) { s, err := Open(filepath.Join(t.TempDir(), "sidecar.sqlite")) + if err == nil { + t.Cleanup(func() { _ = s.Close() }) + } if err != nil { t.Fatal(err) } @@ -211,7 +214,8 @@ func TestBuildDashboard_BucketsByWindow(t *testing.T) { all := Totals{TokensSaved: 9999 + 300 + 200 + 50 + 150 + 100, TokensReturned: 60, CallsCounted: 6} - buckets := BuildDashboard(events, all, now, time.UTC) + _, allPerTool := AggregateByTool(events) + buckets := BuildDashboard(events, all, allPerTool, now, time.UTC) if len(buckets) != 3 { t.Fatalf("buckets = %d, want 3", len(buckets)) } @@ -312,6 +316,9 @@ func TestBarString_DefaultsCellsTo16(t *testing.T) { func TestEvents_ConcurrentWritersAllRecorded(t *testing.T) { s, err := Open(filepath.Join(t.TempDir(), "sidecar.sqlite")) + if err == nil { + t.Cleanup(func() { _ = s.Close() }) + } if err != nil { t.Fatal(err) } diff --git a/internal/savings/store.go b/internal/savings/store.go index 5c3301f3..0154449a 100644 --- a/internal/savings/store.go +++ b/internal/savings/store.go @@ -20,6 +20,7 @@ import ( "os" "path/filepath" "sync" + "sync/atomic" "time" "github.com/zzet/gortex/internal/persistence" @@ -48,6 +49,11 @@ type File struct { Totals Totals `json:"totals"` PerRepo map[string]*Totals `json:"per_repo,omitempty"` PerLanguage map[string]*Totals `json:"per_language,omitempty"` + // DroppedObservations counts ledger writes this process discarded + // (accounting must never fail a tool call, but drops must not be + // invisible — a persistently failing ledger looks exactly like + // "nothing recorded" otherwise). + DroppedObservations int64 `json:"dropped_observations,omitempty"` } // Observation is one source-reading tool call to book. @@ -74,6 +80,13 @@ type Store struct { sc *persistence.SidecarStore mem File // in-memory accumulation when sc == nil memEvents []Event // in-memory event log when sc == nil + + // dropped counts observations the sidecar refused (disk full, + // permissions, lock timeout). warnOnce emits a single stderr line + // the first time it happens so the failure is diagnosable without + // failing any tool call. + dropped atomic.Int64 + warnOnce sync.Once } // DefaultDBPath returns the canonical savings ledger location: the @@ -164,7 +177,7 @@ func (s *Store) AddObservation(o Observation) { now := time.Now().UTC() if s.sc != nil { - _ = s.sc.AddSavingsObservation(persistence.SavingsEvent{ + err := s.sc.AddSavingsObservation(persistence.SavingsEvent{ TS: now, SessionID: o.SessionID, Tool: o.Tool, @@ -173,6 +186,12 @@ func (s *Store) AddObservation(o Observation) { Returned: o.Returned, Saved: o.Saved, }) + if err != nil { + s.dropped.Add(1) + s.warnOnce.Do(func() { + fmt.Fprintf(os.Stderr, "gortex: savings ledger write failed, observations will be dropped: %v\n", err) + }) + } return } @@ -216,9 +235,13 @@ func (s *Store) AddObservation(o Observation) { // not just this one. FirstSeen stays the zero time until something has // actually been recorded — callers must not present it as "tracking since" // when it is zero. -func (s *Store) Snapshot() File { +// +// On a read error the returned File is empty and the error is non-nil — +// callers that render the empty state must distinguish "nothing recorded" +// from "ledger unreadable". +func (s *Store) Snapshot() (File, error) { if s == nil { - return emptyFile() + return emptyFile(), nil } if s.sc == nil { s.mu.Lock() @@ -226,16 +249,18 @@ func (s *Store) Snapshot() File { cp := s.mem cp.PerRepo = copyTotalsMap(s.mem.PerRepo) cp.PerLanguage = copyTotalsMap(s.mem.PerLanguage) - return cp + cp.DroppedObservations = s.dropped.Load() + return cp, nil } buckets, firstSeen, lastUpdated, err := s.sc.SavingsTotals() if err != nil { - return emptyFile() + return emptyFile(), fmt.Errorf("savings totals read: %w", err) } out := emptyFile() out.FirstSeen = firstSeen out.LastUpdated = lastUpdated + out.DroppedObservations = s.dropped.Load() for bucket, r := range buckets { t := &Totals{TokensSaved: r.Saved, TokensReturned: r.Returned, CallsCounted: r.Calls} switch { @@ -247,7 +272,48 @@ func (s *Store) Snapshot() File { out.PerLanguage[bucket[5:]] = t } } - return out + return out, nil +} + +// ToolTotals returns the per-tool aggregate over events with TS >= since +// (zero = all time), sorted by tokens saved descending. Sidecar-backed +// stores aggregate in SQL so the dashboard never materializes the full +// event history just to render a breakdown. +func (s *Store) ToolTotals(since time.Time) ([]ToolTotal, error) { + if s == nil { + return nil, nil + } + if s.sc == nil { + s.mu.Lock() + events := FilterSince(s.memEvents, since) + s.mu.Unlock() + _, per := AggregateByTool(events) + return per, nil + } + rows, err := s.sc.SavingsToolTotals(since) + if err != nil { + return nil, err + } + out := make([]ToolTotal, 0, len(rows)) + for _, r := range rows { + out = append(out, ToolTotal{Tool: r.Tool, Totals: Totals{ + TokensSaved: r.Saved, + TokensReturned: r.Returned, + CallsCounted: r.Calls, + }}) + } + return out, nil +} + +// Close releases the underlying sidecar handle (dropping it from the +// process-wide cache). Safe on nil and in-memory stores. The daemon +// keeps its store open for the process lifetime; tests and one-shot +// CLI paths should Close when done. +func (s *Store) Close() error { + if s == nil || s.sc == nil { + return nil + } + return s.sc.Close() } // EventsSince returns the per-call events with TS >= since, oldest first. @@ -306,11 +372,24 @@ func (s *Store) Reset() error { // sidecar, then renames both files to *.bak. Idempotent: a migration // mark guarantees the import runs once per sidecar, including when // there was nothing to import. In-memory stores skip the import. +// +// The cumulative file was flush-batched while the event log appended +// eagerly, so a SIGKILL-era ledger can carry fewer totals than its own +// events; totals are floored per bucket and per field at what the +// events reconstruct, or "Last 7 days" could exceed "All time". func (s *Store) ImportLegacy(jsonPath string) error { if s == nil || s.sc == nil || jsonPath == "" { return nil } + eventsPath := EventsPathFor(jsonPath) if s.sc.SavingsLegacyImportDone() { + // Self-heal lingering files: a crash between the import commit + // and the renames — or an old-version daemon recreating the + // files after migration — leaves live-looking flat files no + // future open would ever touch. Their content is intentionally + // not imported (the mark owns that decision); sweep them aside. + _ = renameLegacySavings(jsonPath) + _ = renameLegacySavings(eventsPath) return nil } @@ -318,43 +397,61 @@ func (s *Store) ImportLegacy(jsonPath string) error { if err != nil { return err } - eventsPath := EventsPathFor(jsonPath) - events, _ := LoadEvents(eventsPath, time.Time{}) + // A hard read error must abort without marking or renaming so the + // import retries next open — importing a truncated event log and + // renaming the original away would make the loss permanent. + events, err := LoadEvents(eventsPath, time.Time{}) + if err != nil { + return fmt.Errorf("read legacy savings events: %w", err) + } + + // Rebuild totals from the events unconditionally... + rebuilt := make(map[string]persistence.SavingsTotalsRow) + for _, ev := range events { + bump := func(bucket string) { + r := rebuilt[bucket] + r.Saved += ev.Saved + r.Returned += ev.Returned + r.Calls++ + rebuilt[bucket] = r + } + bump("") + if ev.Repo != "" { + bump("repo:" + ev.Repo) + } + if ev.Language != "" { + bump("lang:" + ev.Language) + } + } - buckets := make(map[string]persistence.SavingsTotalsRow) + // ...then overlay the cumulative file's (possibly larger, possibly + // flush-lagged) numbers, taking the per-field max of the two views. + buckets := rebuilt var firstSeen, lastUpdated time.Time - switch { - case legacy != nil: - buckets[""] = totalsRow(legacy.Totals) + if legacy != nil { + overlay := func(bucket string, t Totals) { + r := buckets[bucket] + r.Saved = max(r.Saved, t.TokensSaved) + r.Returned = max(r.Returned, t.TokensReturned) + r.Calls = max(r.Calls, t.CallsCounted) + buckets[bucket] = r + } + overlay("", legacy.Totals) for k, v := range legacy.PerRepo { - buckets["repo:"+k] = totalsRow(*v) + overlay("repo:"+k, *v) } for k, v := range legacy.PerLanguage { - buckets["lang:"+k] = totalsRow(*v) + overlay("lang:"+k, *v) } firstSeen, lastUpdated = legacy.FirstSeen, legacy.LastUpdated - case len(events) > 0: - // Event log without a cumulative file (e.g. the cumulative - // flush never ran before the process died): rebuild the - // totals the file would have carried. - for _, ev := range events { - bump := func(bucket string) { - r := buckets[bucket] - r.Saved += ev.Saved - r.Returned += ev.Returned - r.Calls++ - buckets[bucket] = r - } - bump("") - if ev.Repo != "" { - bump("repo:" + ev.Repo) - } - if ev.Language != "" { - bump("lang:" + ev.Language) - } + } + if len(events) > 0 { + if firstSeen.IsZero() || events[0].TS.Before(firstSeen) { + firstSeen = events[0].TS + } + if last := events[len(events)-1].TS; last.After(lastUpdated) { + lastUpdated = last } - firstSeen = events[0].TS - lastUpdated = events[len(events)-1].TS } pevents := make([]persistence.SavingsEvent, 0, len(events)) @@ -372,25 +469,27 @@ func (s *Store) ImportLegacy(jsonPath string) error { if err := s.sc.ImportLegacySavings(buckets, firstSeen, lastUpdated, pevents); err != nil { return err } - renameLegacySavings(jsonPath) - renameLegacySavings(eventsPath) - return nil -} - -func totalsRow(t Totals) persistence.SavingsTotalsRow { - return persistence.SavingsTotalsRow{Saved: t.TokensSaved, Returned: t.TokensReturned, Calls: t.CallsCounted} + return errors.Join( + renameLegacySavings(jsonPath), + renameLegacySavings(eventsPath), + ) } // renameLegacySavings moves an already-imported flat file aside to -// .bak. Best-effort — never deletes; a missing file is fine. -func renameLegacySavings(path string) { +// .bak. Never deletes; a missing file is fine. A rename failure +// is reported so the caller can surface it — silently leaving the file +// in place makes it look live when it no longer is. +func renameLegacySavings(path string) error { if path == "" { - return + return nil } if _, err := os.Stat(path); err != nil { - return + return nil } - _ = os.Rename(path, path+".bak") + if err := os.Rename(path, path+".bak"); err != nil { + return fmt.Errorf("rename legacy savings file: %w", err) + } + return nil } // readLegacyFile loads a flat-file era savings.json. Returns (nil, nil) @@ -414,6 +513,18 @@ func readLegacyFile(path string) (*File, error) { if loaded.PerLanguage == nil { loaded.PerLanguage = make(map[string]*Totals) } + // JSON null map values unmarshal to nil pointers — drop them here, + // the single choke point, so no consumer dereferences one. + for k, v := range loaded.PerRepo { + if v == nil { + delete(loaded.PerRepo, k) + } + } + for k, v := range loaded.PerLanguage { + if v == nil { + delete(loaded.PerLanguage, k) + } + } return &loaded, nil } diff --git a/internal/savings/store_test.go b/internal/savings/store_test.go index 8805bd46..80b57045 100644 --- a/internal/savings/store_test.go +++ b/internal/savings/store_test.go @@ -18,10 +18,32 @@ func testLedgerPath(t *testing.T) string { return filepath.Join(t.TempDir(), "sidecar.sqlite") } +// mustSnapshot unwraps Snapshot for tests that expect a healthy ledger. +func mustSnapshot(t *testing.T, s *Store) File { + t.Helper() + snap, err := s.Snapshot() + if err != nil { + t.Fatalf("Snapshot: %v", err) + } + return snap +} + +// closeOnCleanup releases the sidecar handle when the test ends, so the +// process-wide handle cache doesn't accumulate open DBs (and TempDir +// cleanup works on platforms that refuse to delete open files). +func closeOnCleanup(t *testing.T, s *Store) *Store { + t.Helper() + t.Cleanup(func() { _ = s.Close() }) + return s +} + func TestAddObservation_PerLanguageBucket(t *testing.T) { path := testLedgerPath(t) s, err := Open(path) + if err == nil { + closeOnCleanup(t, s) + } if err != nil { t.Fatal(err) } @@ -34,10 +56,13 @@ func TestAddObservation_PerLanguageBucket(t *testing.T) { s.AddObservation(Observation{Repo: "/repo-c", Tool: "smart_context", Returned: 10, Saved: 20}) reopened, err := Open(path) + if err == nil { + closeOnCleanup(t, reopened) + } if err != nil { t.Fatal(err) } - snap := reopened.Snapshot() + snap := mustSnapshot(t, reopened) if got, want := snap.Totals.CallsCounted, int64(4); got != want { t.Errorf("CallsCounted = %d, want %d", got, want) @@ -64,6 +89,9 @@ func TestAddObservation_DurableImmediately(t *testing.T) { path := testLedgerPath(t) s, err := Open(path) + if err == nil { + closeOnCleanup(t, s) + } if err != nil { t.Fatal(err) } @@ -72,7 +100,7 @@ func TestAddObservation_DurableImmediately(t *testing.T) { if _, err := os.Stat(path); err != nil { t.Fatalf("ledger DB must exist immediately after the first observation: %v", err) } - snap := s.Snapshot() + snap := mustSnapshot(t, s) if snap.Totals.CallsCounted != 1 || snap.Totals.TokensSaved != 90 { t.Errorf("snapshot = %+v, want calls=1 saved=90 with no flush", snap.Totals) } @@ -92,6 +120,9 @@ func TestConcurrentWriters_SameLedger(t *testing.T) { stores := make([]*Store, 4) for i := range stores { s, err := Open(path) + if err == nil { + closeOnCleanup(t, s) + } if err != nil { t.Fatal(err) } @@ -110,7 +141,7 @@ func TestConcurrentWriters_SameLedger(t *testing.T) { } wg.Wait() - snap := stores[0].Snapshot() + snap := mustSnapshot(t, stores[0]) wantCalls := int64(len(stores) * perStore) if got := snap.Totals.CallsCounted; got != wantCalls { t.Errorf("CallsCounted = %d, want %d (observation lost across writers)", got, wantCalls) @@ -133,10 +164,13 @@ func TestConcurrentWriters_SameLedger(t *testing.T) { // had never recorded anything. func TestOpen_FreshLedger_EmptySnapshot(t *testing.T) { s, err := Open(testLedgerPath(t)) + if err == nil { + closeOnCleanup(t, s) + } if err != nil { t.Fatalf("Open: %v", err) } - snap := s.Snapshot() + snap := mustSnapshot(t, s) if snap.Totals.CallsCounted != 0 { t.Errorf("new ledger has CallsCounted=%d, want 0", snap.Totals.CallsCounted) } @@ -150,6 +184,9 @@ func TestOpen_FreshLedger_EmptySnapshot(t *testing.T) { func TestObservation_StampsFirstAndLastSeen(t *testing.T) { s, err := Open(testLedgerPath(t)) + if err == nil { + closeOnCleanup(t, s) + } if err != nil { t.Fatal(err) } @@ -157,7 +194,7 @@ func TestObservation_StampsFirstAndLastSeen(t *testing.T) { s.AddObservation(Observation{Tool: "test", Returned: 1, Saved: 1}) after := time.Now().UTC().Add(time.Second) - snap := s.Snapshot() + snap := mustSnapshot(t, s) if snap.FirstSeen.Before(before) || snap.FirstSeen.After(after) { t.Errorf("FirstSeen = %v, want within [%v, %v]", snap.FirstSeen, before, after) } @@ -187,7 +224,7 @@ func TestAddObservation_ConcurrentSafe(t *testing.T) { } wg.Wait() - snap := s.Snapshot() + snap := mustSnapshot(t, s) if got, want := snap.Totals.CallsCounted, int64(workers*per); got != want { t.Errorf("CallsCounted = %d, want %d", got, want) } @@ -205,7 +242,7 @@ func TestOpen_EmptyPath_InMemoryOnly(t *testing.T) { if err := s.Flush(); err != nil { t.Errorf("Flush on in-memory store should no-op, got: %v", err) } - snap := s.Snapshot() + snap := mustSnapshot(t, s) if snap.Totals.CallsCounted != 1 { t.Errorf("in-memory store should track, got CallsCounted=%d", snap.Totals.CallsCounted) } @@ -222,12 +259,13 @@ func TestReset_ClearsLedger(t *testing.T) { path := testLedgerPath(t) s, _ := Open(path) + closeOnCleanup(t, s) s.AddObservation(Observation{Repo: "/r", Tool: "test", Returned: 50, Saved: 500}) if err := s.Reset(); err != nil { t.Fatalf("Reset: %v", err) } - snap := s.Snapshot() + snap := mustSnapshot(t, s) if snap.Totals.CallsCounted != 0 { t.Errorf("totals should be cleared after reset, got CallsCounted=%d", snap.Totals.CallsCounted) } @@ -247,9 +285,10 @@ func TestReset_ClearsLedger(t *testing.T) { // expose — the surface contract of cumulative_savings. func TestSnapshot_JSONShape(t *testing.T) { s, _ := Open(testLedgerPath(t)) + closeOnCleanup(t, s) s.AddObservation(Observation{Repo: "/repo-a", Language: "go", Tool: "test", Returned: 10, Saved: 100}) - data, err := json.Marshal(s.Snapshot()) + data, err := json.Marshal(mustSnapshot(t, s)) if err != nil { t.Fatal(err) } @@ -266,6 +305,7 @@ func TestSnapshot_JSONShape(t *testing.T) { func TestEventsSince_Filters(t *testing.T) { s, _ := Open(testLedgerPath(t)) + closeOnCleanup(t, s) s.AddObservation(Observation{Tool: "a", Returned: 1, Saved: 1}) time.Sleep(5 * time.Millisecond) cutoff := time.Now().UTC() @@ -313,6 +353,9 @@ func TestImportLegacy_FullFlatFiles(t *testing.T) { } s, err := Open(testLedgerPath(t)) + if err == nil { + closeOnCleanup(t, s) + } if err != nil { t.Fatal(err) } @@ -320,7 +363,7 @@ func TestImportLegacy_FullFlatFiles(t *testing.T) { t.Fatalf("ImportLegacy: %v", err) } - snap := s.Snapshot() + snap := mustSnapshot(t, s) if snap.Totals.CallsCounted != 10 || snap.Totals.TokensSaved != 1000 { t.Errorf("imported totals = %+v, want calls=10 saved=1000", snap.Totals) } @@ -351,7 +394,7 @@ func TestImportLegacy_FullFlatFiles(t *testing.T) { if err := s.ImportLegacy(jsonPath); err != nil { t.Fatalf("second ImportLegacy: %v", err) } - if got := s.Snapshot().Totals.CallsCounted; got != 10 { + if got := mustSnapshot(t, s).Totals.CallsCounted; got != 10 { t.Errorf("totals after second import = %d, want 10 (no double count)", got) } } @@ -376,10 +419,11 @@ func TestImportLegacy_EventsOnlyRebuildsTotals(t *testing.T) { } s, _ := Open(testLedgerPath(t)) + closeOnCleanup(t, s) if err := s.ImportLegacy(jsonPath); err != nil { t.Fatalf("ImportLegacy: %v", err) } - snap := s.Snapshot() + snap := mustSnapshot(t, s) if snap.Totals.CallsCounted != 3 || snap.Totals.TokensSaved != 90 { t.Errorf("rebuilt totals = %+v, want calls=3 saved=90", snap.Totals) } @@ -399,6 +443,7 @@ func TestImportLegacy_NothingToImportMarksDone(t *testing.T) { jsonPath := filepath.Join(legacyDir, "savings.json") s, _ := Open(testLedgerPath(t)) + closeOnCleanup(t, s) if err := s.ImportLegacy(jsonPath); err != nil { t.Fatalf("ImportLegacy on missing files: %v", err) } @@ -412,7 +457,7 @@ func TestImportLegacy_NothingToImportMarksDone(t *testing.T) { if err := s.ImportLegacy(jsonPath); err != nil { t.Fatal(err) } - if got := s.Snapshot().Totals.CallsCounted; got != 0 { + if got := mustSnapshot(t, s).Totals.CallsCounted; got != 0 { t.Errorf("late-appearing legacy file must not import, got calls=%d", got) } } @@ -449,3 +494,98 @@ func TestDefaultDBPath_HonorsXDGDataHome(t *testing.T) { t.Fatalf("DefaultDBPath() with XDG_DATA_HOME = %s, want %s", got, want) } } + +// A legacy file with JSON null bucket values must import cleanly — a +// nil *Totals dereference here would crash-loop every server start. +func TestImportLegacy_NullBucketValues(t *testing.T) { + legacyDir := t.TempDir() + jsonPath := filepath.Join(legacyDir, "savings.json") + body := `{"version":1,"totals":{"tokens_saved":10,"tokens_returned":1,"calls_counted":1},"per_repo":{"x":null},"per_language":{"y":null}}` + if err := os.WriteFile(jsonPath, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + + s, _ := Open(testLedgerPath(t)) + closeOnCleanup(t, s) + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatalf("ImportLegacy with null buckets: %v", err) + } + snap := mustSnapshot(t, s) + if snap.Totals.CallsCounted != 1 { + t.Errorf("totals = %+v, want calls=1", snap.Totals) + } + if len(snap.PerRepo) != 0 || len(snap.PerLanguage) != 0 { + t.Errorf("null buckets must be dropped, got repo=%v lang=%v", snap.PerRepo, snap.PerLanguage) + } + if _, err := os.Stat(jsonPath + ".bak"); err != nil { + t.Errorf("legacy file should be renamed after import: %v", err) + } +} + +// The flat-file cumulative was flush-batched while the event log +// appended eagerly; the import floors totals at what the events +// reconstruct so "Last 7 days" can never exceed "All time". +func TestImportLegacy_FlushLaggedTotalsFlooredByEvents(t *testing.T) { + legacyDir := t.TempDir() + jsonPath := filepath.Join(legacyDir, "savings.json") + jsonlPath := filepath.Join(legacyDir, "savings.jsonl") + + lagged := File{ + Version: schemaVersion, + Totals: Totals{TokensSaved: 10, TokensReturned: 1, CallsCounted: 1}, + } + data, _ := json.Marshal(lagged) + if err := os.WriteFile(jsonPath, data, 0o644); err != nil { + t.Fatal(err) + } + ts := time.Date(2026, 6, 1, 10, 0, 0, 0, time.UTC) + var lines []byte + for i := 0; i < 3; i++ { + line, _ := json.Marshal(Event{TS: ts.Add(time.Duration(i) * time.Minute), Tool: "t", Returned: 1, Saved: 10}) + lines = append(append(lines, line...), '\n') + } + if err := os.WriteFile(jsonlPath, lines, 0o644); err != nil { + t.Fatal(err) + } + + s, _ := Open(testLedgerPath(t)) + closeOnCleanup(t, s) + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatal(err) + } + snap := mustSnapshot(t, s) + if snap.Totals.CallsCounted != 3 || snap.Totals.TokensSaved != 30 { + t.Errorf("totals = %+v, want floored at the events' calls=3 saved=30", snap.Totals) + } +} + +// A hard event-log read error aborts the import without marking or +// renaming, so the next open retries instead of permanently losing the +// unread tail. +func TestImportLegacy_UnreadableEventsAborts(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("permission bits don't bind as root") + } + legacyDir := t.TempDir() + jsonPath := filepath.Join(legacyDir, "savings.json") + jsonlPath := filepath.Join(legacyDir, "savings.jsonl") + if err := os.WriteFile(jsonlPath, []byte("{}\n"), 0o000); err != nil { + t.Fatal(err) + } + + s, _ := Open(testLedgerPath(t)) + closeOnCleanup(t, s) + if err := s.ImportLegacy(jsonPath); err == nil { + t.Fatal("unreadable event log must abort the import") + } + if _, err := os.Stat(jsonlPath); err != nil { + t.Errorf("aborted import must not rename the event log: %v", err) + } + // A later open (permissions fixed) imports successfully. + if err := os.Chmod(jsonlPath, 0o644); err != nil { + t.Fatal(err) + } + if err := s.ImportLegacy(jsonPath); err != nil { + t.Fatalf("retry after fixing permissions: %v", err) + } +} From f38a6c768acc751388cbc8e9cb52fed1e5cbd051 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Fri, 12 Jun 2026 01:24:46 +0200 Subject: [PATCH 12/12] savings: one machine-global ledger for every entry point; non-destructive reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three wiring holes from the review. The embedded server's --cache-dir still relocated its ledger away from the dashboard's default read path — the exact writer/reader split this branch exists to fix; the flag now moves only the graph cache, and ledger isolation comes from XDG_DATA_HOME / XDG_CACHE_HOME, which both ledger paths honour. The savings/gain CLIs run the one-shot legacy import only against the default locations: pointing a dashboard at a directory with --cache-dir must never rename files there as a side effect of looking. And the serverstack constructor test pinned its SavingsPath + SavingsLegacyJSON to temp paths — with both empty it opened the REAL machine-global sidecar and imported (renaming!) the developer's live flat-file ledger on every 'go test ./internal/serverstack'. The config doc now matches the machine-global behavior and carries the warning. Also states the percentage semantics in the dashboard help: bars cover ALL recorded source fetches, including uncompressed read_file calls that saved nothing. --- cmd/gortex/gain.go | 8 ++++--- cmd/gortex/mcp.go | 22 ++++++++---------- cmd/gortex/savings.go | 26 ++++++++++++++-------- internal/serverstack/shared_server.go | 18 +++++++++------ internal/serverstack/shared_server_test.go | 12 +++++++--- 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/cmd/gortex/gain.go b/cmd/gortex/gain.go index 0f516fc1..36c680ad 100644 --- a/cmd/gortex/gain.go +++ b/cmd/gortex/gain.go @@ -328,16 +328,18 @@ func (h *gainHistory) toJSON() map[string]any { // Calls=0 so the caller can decide whether to render. func loadHistory(cacheDir string, since time.Duration) (*gainHistory, error) { path := savings.DefaultDBPath() - legacyJSON := savings.DefaultPath() if cacheDir != "" { path = persistence.DefaultSidecarPath(cacheDir) - legacyJSON = filepath.Join(cacheDir, "savings.json") } store, err := savings.Open(path) if err != nil { return nil, err } - _ = store.ImportLegacy(legacyJSON) + // Same rule as `gortex savings`: the legacy import only runs against + // the default location — a --cache-dir read must not rename files. + if cacheDir == "" { + _ = store.ImportLegacy(savings.DefaultPath()) + } snap, serr := store.Snapshot() if serr != nil { fmt.Fprintf(os.Stderr, "[gortex gain] savings totals read failed: %v\n", serr) diff --git a/cmd/gortex/mcp.go b/cmd/gortex/mcp.go index e5206120..7315ac6a 100644 --- a/cmd/gortex/mcp.go +++ b/cmd/gortex/mcp.go @@ -139,16 +139,14 @@ func runMCP(cmd *cobra.Command, args []string) error { if sideStoreCacheDir == "" { sideStoreCacheDir = platform.CacheDir() } - // The savings ledger defaults to the machine-global sidecar the - // `gortex savings` CLI reads. A custom --cache-dir relocates both - // the ledger and the flat-file era's savings.json the one-shot - // legacy import looks for — the isolation tests rely on. - savingsDBPath := "" - savingsLegacyJSON := "" - if mcpCacheDir != "" { - savingsDBPath = persistence.DefaultSidecarPath(mcpCacheDir) - savingsLegacyJSON = filepath.Join(mcpCacheDir, "savings.json") - } + // The savings ledger is machine-global — the same sidecar database + // every entry point writes and the `gortex savings` CLI reads. + // --cache-dir deliberately does NOT relocate it: users set that flag + // to move the graph cache, and quietly splitting the ledger away + // from the dashboard's default read path recreates the + // empty-dashboard failure mode. Isolation (tests, sandboxes) comes + // from XDG_DATA_HOME / XDG_CACHE_HOME, which both ledger paths + // honour. ss, err := serverstack.NewSharedServer(serverstack.SharedServerConfig{ Lifecycle: serverstack.LifecycleOneshot, @@ -171,9 +169,7 @@ func runMCP(cmd *cobra.Command, args []string) error { FeedbackRepo: mcpIndex, NotebookPath: mcpIndex, }, - SavingsPath: savingsDBPath, - SavingsLegacyJSON: savingsLegacyJSON, - SavingsRepo: mcpIndex, + SavingsRepo: mcpIndex, }) if err != nil { return fmt.Errorf("build server stack: %w", err) diff --git a/cmd/gortex/savings.go b/cmd/gortex/savings.go index 3854e478..9edfadd6 100644 --- a/cmd/gortex/savings.go +++ b/cmd/gortex/savings.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "os" - "path/filepath" "sort" "strings" "time" @@ -43,10 +42,16 @@ database (~/.gortex/sidecar.sqlite); flat-file ledgers from older releases (savings.json / savings.jsonl under the cache dir) are imported once and renamed *.bak. -Override the ledger location with --cache-dir (uses that directory's -sidecar.sqlite and imports its legacy files), override pricing by -exporting GORTEX_MODEL_PRICING_JSON, and pass --verbose for a per-tool -breakdown inside each bucket.`, +Percentages are computed over ALL recorded source fetches — including +uncompressed read_file calls that returned the whole file and saved +nothing — so the bars reflect how the agent actually reads, not just +the best cases; per-tool rates live in --verbose. + +Override the ledger location with --cache-dir (reads that directory's +sidecar.sqlite; the one-shot legacy import runs only against the +default location), override pricing by exporting +GORTEX_MODEL_PRICING_JSON, and pass --verbose for a per-tool breakdown +inside each bucket.`, RunE: runSavings, } @@ -63,18 +68,21 @@ func init() { func runSavings(_ *cobra.Command, _ []string) error { dbPath := savings.DefaultDBPath() - legacyJSON := savings.DefaultPath() if savingsCacheDir != "" { dbPath = persistence.DefaultSidecarPath(savingsCacheDir) - legacyJSON = filepath.Join(savingsCacheDir, "savings.json") } store, err := savings.Open(dbPath) if err != nil { return fmt.Errorf("open savings ledger: %w", err) } - if ierr := store.ImportLegacy(legacyJSON); ierr != nil { - fmt.Fprintf(os.Stderr, "[gortex savings] legacy import failed: %v\n", ierr) + // Legacy flat-file import runs only against the default locations: + // pointing the dashboard at a directory with --cache-dir must never + // rename files there as a side effect of looking. + if savingsCacheDir == "" { + if ierr := store.ImportLegacy(savings.DefaultPath()); ierr != nil { + fmt.Fprintf(os.Stderr, "[gortex savings] legacy import failed: %v\n", ierr) + } } if savingsReset { diff --git a/internal/serverstack/shared_server.go b/internal/serverstack/shared_server.go index c7d1d266..ac586378 100644 --- a/internal/serverstack/shared_server.go +++ b/internal/serverstack/shared_server.go @@ -87,13 +87,17 @@ type SharedServerConfig struct { // builds the call graph; anything else (default) type-checks. SemanticMode string // SavingsPath overrides the token-savings ledger database; empty - // derives /sidecar.sqlite (the same sidecar the - // notes/memories managers share), falling back to the machine - // default under the data dir. SavingsRepo scopes the accumulated - // totals (empty = workspace-global). SavingsLegacyJSON names the - // flat-file era's cumulative savings.json to import once — its - // sibling .jsonl event log rides along; empty uses the historical - // default location under the cache dir. + // defaults to the machine-global sidecar under the data dir + // (savings.DefaultDBPath() — the same database the `gortex savings` + // CLI reads), independent of SideStores: deriving it from a per-mode + // side-store dir would split the ledger between writer and reader. + // Tests MUST set it (with SavingsLegacyJSON) to temp paths or the + // constructor mutates the developer's real ledger. SavingsRepo + // scopes the accumulated totals (empty = workspace-global). + // SavingsLegacyJSON names the flat-file era's cumulative + // savings.json to import once — its sibling .jsonl event log rides + // along; empty uses the historical default location under the + // cache dir. SavingsPath string SavingsLegacyJSON string SavingsRepo string diff --git a/internal/serverstack/shared_server_test.go b/internal/serverstack/shared_server_test.go index 7c1d3126..e6dfeb2b 100644 --- a/internal/serverstack/shared_server_test.go +++ b/internal/serverstack/shared_server_test.go @@ -22,12 +22,18 @@ func TestNewSharedServer_OneshotMemory(t *testing.T) { } ss, err := NewSharedServer(SharedServerConfig{ - Lifecycle: LifecycleOneshot, - Index: repo, - Config: config.Default(), + Lifecycle: LifecycleOneshot, + Index: repo, + Config: config.Default(), Logger: zap.NewNop(), Version: "test", SideStores: SideStores{NotesDir: t.TempDir(), NotesRepo: "test"}, + // Pin the savings ledger + legacy-import probe to temp paths: + // with both empty the constructor opens the REAL machine-global + // sidecar and imports (renaming!) the developer's real flat-file + // ledger — a unit test must never mutate ~/.gortex. + SavingsPath: filepath.Join(t.TempDir(), "sidecar.sqlite"), + SavingsLegacyJSON: filepath.Join(t.TempDir(), "savings.json"), }) if err != nil { t.Fatalf("NewSharedServer: %v", err)