From 9b2444fa103c7fb61f300dd5b77e444499398475 Mon Sep 17 00:00:00 2001 From: hallelx2 Date: Tue, 13 Oct 2026 12:07:45 +0100 Subject: [PATCH 1/3] fix(retrieval): cache the whole result, so a repeat keeps its pincites The retrieval cache stored only the selected section IDs and rebuilt a bare Result on a hit, without the evidence pages, cited pages, pages read or confidences a page strategy found. The answer path then took the section route, so the second time anyone asked a question within the TTL it came back with no pincites. Measured on the 3M 2018 10-K: 4 pincites on the first call, 0 on the identical second call. The cache now keeps a deep copy of the whole Result and hands each hit its own copy, with zero usage and the strategy's trace token. Three identical calls now each return 4 pincites, retrieval 0-2 ms on hits. --- pkg/retrieval/cached.go | 80 +++++++++++++++++++------ pkg/retrieval/cached_evidence_test.go | 84 +++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 17 deletions(-) create mode 100644 pkg/retrieval/cached_evidence_test.go diff --git a/pkg/retrieval/cached.go b/pkg/retrieval/cached.go index 1e951b1..774685a 100644 --- a/pkg/retrieval/cached.go +++ b/pkg/retrieval/cached.go @@ -11,9 +11,13 @@ import ( // Cached wraps any Strategy and caches its Select results in an LRU. // // Cache key = sha256(document_id || query || strategy_name || model_name). -// The cached value is the []tree.SectionID slice; the wrapping is -// intentionally transparent — callers interact with the standard Strategy -// interface and don't know caching is present. +// The cached value is a copy of the whole Result, so a hit returns what +// the strategy found: for a page strategy that includes the evidence +// pages the answer and its pincites are built from. Caching only the +// section IDs dropped them, and a repeated question came back with no +// pincites (HAL-2846). The wrapping is intentionally transparent — +// callers interact with the standard Strategy interface and don't know +// caching is present. // // Cached also implements CostStrategy if the wrapped strategy does. On a // cache hit, CostUSD and LLMCalls are both zero (no LLM call was made). @@ -61,8 +65,8 @@ func (c *Cached) Name() string { return c.inner.Name() } func (c *Cached) Select(ctx context.Context, t *tree.Tree, query string, budget ContextBudget) ([]tree.SectionID, error) { key := cache.Key(string(t.DocumentID), query, c.inner.Name(), budget.ModelName) - if v, ok := c.cache.Get(key); ok { - return v.([]tree.SectionID), nil + if r, ok := c.get(key); ok { + return r.SelectedIDs, nil } ids, err := c.inner.Select(ctx, t, query, budget) @@ -70,8 +74,8 @@ func (c *Cached) Select(ctx context.Context, t *tree.Tree, query string, budget return nil, err } - c.cache.Set(key, ids, c.ttl) - return ids, nil + c.cache.Set(key, cloneResult(&Result{SelectedIDs: ids}), c.ttl) + return append([]tree.SectionID(nil), ids...), nil } // SelectWithCost checks the cache first. On a hit it returns zero usage @@ -86,15 +90,16 @@ func (c *Cached) Select(ctx context.Context, t *tree.Tree, query string, budget func (c *Cached) SelectWithCost(ctx context.Context, t *tree.Tree, query string, budget ContextBudget) (*Result, error) { key := cache.Key(string(t.DocumentID), query, c.inner.Name(), budget.ModelName) - if v, ok := c.cache.Get(key); ok { - ids := v.([]tree.SectionID) - return &Result{ - SelectedIDs: ids, - Reasoning: "cached", - ModelUsed: budget.ModelName, - Usage: Usage{}, // zero — no LLM call - TraceToken: ComputeTraceToken(t.DocumentID, traceDocVersionV1, budget.ModelName, ids), - }, nil + if r, ok := c.get(key); ok { + r.Reasoning = "cached" + r.Usage = Usage{} // zero — no LLM call + if r.ModelUsed == "" { + r.ModelUsed = budget.ModelName + } + if r.TraceToken == "" { + r.TraceToken = ComputeTraceToken(t.DocumentID, traceDocVersionV1, budget.ModelName, r.SelectedIDs) + } + return r, nil } var result *Result @@ -115,10 +120,51 @@ func (c *Cached) SelectWithCost(ctx context.Context, t *tree.Tree, query string, } } - c.cache.Set(key, result.SelectedIDs, c.ttl) + c.cache.Set(key, cloneResult(result), c.ttl) return result, nil } +// get returns a private copy of the cached Result for key, so a caller +// that edits its result (the answer path trims evidence, for one) never +// changes what the next hit sees. +func (c *Cached) get(key string) (*Result, bool) { + v, ok := c.cache.Get(key) + if !ok { + return nil, false + } + r, ok := v.(*Result) + if !ok || r == nil { + return nil, false + } + return cloneResult(r), true +} + +// cloneResult deep-copies the parts of a Result a caller could mutate: +// its slices and map. +func cloneResult(r *Result) *Result { + if r == nil { + return nil + } + out := *r + out.SelectedIDs = append([]tree.SectionID(nil), r.SelectedIDs...) + if r.Confidences != nil { + out.Confidences = make(map[tree.SectionID]float64, len(r.Confidences)) + for k, v := range r.Confidences { + out.Confidences[k] = v + } + } + if r.PagesRead != nil { + out.PagesRead = make([]PageReadEntry, len(r.PagesRead)) + for i, p := range r.PagesRead { + p.SectionIDs = append([]tree.SectionID(nil), p.SectionIDs...) + out.PagesRead[i] = p + } + } + out.EvidencePages = append([]EvidencePage(nil), r.EvidencePages...) + out.CitedPages = append([][2]int(nil), r.CitedPages...) + return &out +} + // SelectStream delegates to the inner strategy's SelectStream if it // implements StreamStrategy. Streaming results are NOT cached because // the stream is consumed as events arrive; the non-streaming Select path diff --git a/pkg/retrieval/cached_evidence_test.go b/pkg/retrieval/cached_evidence_test.go new file mode 100644 index 0000000..7eb42a6 --- /dev/null +++ b/pkg/retrieval/cached_evidence_test.go @@ -0,0 +1,84 @@ +package retrieval_test + +import ( + "context" + "testing" + "time" + + "github.com/hallelx2/vectorless-engine/pkg/retrieval" + "github.com/hallelx2/vectorless-engine/pkg/tree" +) + +// pageStrategy stands in for judgewalk: a page-based strategy whose +// result carries evidence pages, cited pages and confidences. +type pageStrategy struct{ calls int } + +func (p *pageStrategy) Name() string { return "pages" } + +func (p *pageStrategy) Select(ctx context.Context, t *tree.Tree, q string, b retrieval.ContextBudget) ([]tree.SectionID, error) { + r, err := p.SelectWithCost(ctx, t, q, b) + if err != nil { + return nil, err + } + return r.SelectedIDs, nil +} + +func (p *pageStrategy) SelectWithCost(_ context.Context, t *tree.Tree, _ string, _ retrieval.ContextBudget) (*retrieval.Result, error) { + p.calls++ + ids := []tree.SectionID{"sec_b"} + return &retrieval.Result{ + SelectedIDs: ids, + Confidences: map[tree.SectionID]float64{"sec_b": 0.9}, + Usage: retrieval.Usage{LLMCalls: 3, InputTokens: 900}, + PagesRead: []retrieval.PageReadEntry{{StartPage: 60, EndPage: 60}}, + EvidencePages: []retrieval.EvidencePage{{Page: 60, Title: "Cash flows", Text: "Purchases of PP&E (1,577) (1,373)", Confidence: 0.9}}, + CitedPages: [][2]int{{60, 60}}, + Confidence: 0.8, + TraceToken: "tok-from-strategy", + }, nil +} + +// A cache hit must return what the strategy found, not just the section +// IDs. It used to rebuild a bare Result from the IDs, so the second time +// anyone asked a question the evidence pages were gone and the answer +// came back with no pincites (HAL-2846). +func TestCachedHitKeepsEvidencePages(t *testing.T) { + inner := &pageStrategy{} + cached := retrieval.NewCached(inner, retrieval.CachedConfig{TTL: time.Minute}) + tr := buildTree() + budget := retrieval.ContextBudget{MaxTokens: 100000, ModelName: "m"} + + first, err := cached.SelectWithCost(context.Background(), tr, "capex?", budget) + if err != nil { + t.Fatal(err) + } + // A caller mutating its result must not reach into the cache. + first.EvidencePages[0].Text = "mutated" + first.Confidences["sec_b"] = 0 + + hit, err := cached.SelectWithCost(context.Background(), tr, "capex?", budget) + if err != nil { + t.Fatal(err) + } + if inner.calls != 1 { + t.Fatalf("second call should be a cache hit: strategy ran %d times", inner.calls) + } + if len(hit.EvidencePages) != 1 || hit.EvidencePages[0].Page != 60 || hit.EvidencePages[0].Text != "Purchases of PP&E (1,577) (1,373)" { + t.Fatalf("evidence pages lost or aliased on a hit: %+v", hit.EvidencePages) + } + if len(hit.CitedPages) != 1 || len(hit.PagesRead) != 1 || hit.Confidence != 0.8 || hit.Confidences["sec_b"] != 0.9 { + t.Fatalf("page result lost on a hit: cited=%v read=%v conf=%v confs=%v", hit.CitedPages, hit.PagesRead, hit.Confidence, hit.Confidences) + } + if hit.Usage.LLMCalls != 0 || hit.Usage.InputTokens != 0 { + t.Fatalf("a hit costs nothing: usage = %+v", hit.Usage) + } + if hit.TraceToken != "tok-from-strategy" || hit.Reasoning != "cached" { + t.Fatalf("hit trace token %q reasoning %q", hit.TraceToken, hit.Reasoning) + } + + // Select on the same key returns the same IDs. + ids, err := cached.Select(context.Background(), tr, "capex?", budget) + if err != nil || len(ids) != 1 || ids[0] != "sec_b" || inner.calls != 1 { + t.Fatalf("Select hit: ids=%v err=%v calls=%d", ids, err, inner.calls) + } +} From 2323916d47e27d61fdaa007fcb3ea8f048ec133d Mon Sep 17 00:00:00 2001 From: hallelx2 Date: Tue, 13 Oct 2026 12:17:04 +0100 Subject: [PATCH 2/3] fix(retrieval): no IDs-only cache entries, and a hit reports no hops Review on #86: when Select filled the cache first, the entry was IDs-only and a later SelectWithCost hit served it without evidence pages. Select now goes through SelectWithCost whenever the strategy reports a full result. A hit also zeroes HopsTaken: it made no navigation requests. Tests cover both, and edits to a hit's result not reaching the next hit. --- pkg/retrieval/cached.go | 12 ++++++++ pkg/retrieval/cached_evidence_test.go | 40 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/pkg/retrieval/cached.go b/pkg/retrieval/cached.go index 774685a..8f337fe 100644 --- a/pkg/retrieval/cached.go +++ b/pkg/retrieval/cached.go @@ -63,6 +63,17 @@ func (c *Cached) Name() string { return c.inner.Name() } // Select checks the cache first. On a miss it delegates to the inner // strategy and caches the result. func (c *Cached) Select(ctx context.Context, t *tree.Tree, query string, budget ContextBudget) ([]tree.SectionID, error) { + // A strategy that reports a full Result is always cached through + // SelectWithCost, so no entry is ever an IDs-only stand-in that a + // later SelectWithCost hit would serve without its evidence pages. + if _, ok := c.inner.(CostStrategy); ok { + r, err := c.SelectWithCost(ctx, t, query, budget) + if err != nil { + return nil, err + } + return r.SelectedIDs, nil + } + key := cache.Key(string(t.DocumentID), query, c.inner.Name(), budget.ModelName) if r, ok := c.get(key); ok { @@ -93,6 +104,7 @@ func (c *Cached) SelectWithCost(ctx context.Context, t *tree.Tree, query string, if r, ok := c.get(key); ok { r.Reasoning = "cached" r.Usage = Usage{} // zero — no LLM call + r.HopsTaken = 0 // and no navigation on this request if r.ModelUsed == "" { r.ModelUsed = budget.ModelName } diff --git a/pkg/retrieval/cached_evidence_test.go b/pkg/retrieval/cached_evidence_test.go index 7eb42a6..1d4b719 100644 --- a/pkg/retrieval/cached_evidence_test.go +++ b/pkg/retrieval/cached_evidence_test.go @@ -34,6 +34,7 @@ func (p *pageStrategy) SelectWithCost(_ context.Context, t *tree.Tree, _ string, EvidencePages: []retrieval.EvidencePage{{Page: 60, Title: "Cash flows", Text: "Purchases of PP&E (1,577) (1,373)", Confidence: 0.9}}, CitedPages: [][2]int{{60, 60}}, Confidence: 0.8, + HopsTaken: 4, TraceToken: "tok-from-strategy", }, nil } @@ -76,9 +77,48 @@ func TestCachedHitKeepsEvidencePages(t *testing.T) { t.Fatalf("hit trace token %q reasoning %q", hit.TraceToken, hit.Reasoning) } + if hit.HopsTaken != 0 { + t.Fatalf("a hit navigated nothing: HopsTaken = %d", hit.HopsTaken) + } + + // Editing a hit's result must not reach the next hit either. + hit.EvidencePages[0].Text = "edited" + hit.CitedPages[0] = [2]int{1, 1} + again, err := cached.SelectWithCost(context.Background(), tr, "capex?", budget) + if err != nil { + t.Fatal(err) + } + if again.EvidencePages[0].Text != "Purchases of PP&E (1,577) (1,373)" || again.CitedPages[0] != [2]int{60, 60} { + t.Fatalf("a hit's edits leaked into the cache: %+v %v", again.EvidencePages, again.CitedPages) + } + // Select on the same key returns the same IDs. ids, err := cached.Select(context.Background(), tr, "capex?", budget) if err != nil || len(ids) != 1 || ids[0] != "sec_b" || inner.calls != 1 { t.Fatalf("Select hit: ids=%v err=%v calls=%d", ids, err, inner.calls) } } + +// Select may fill the cache before SelectWithCost asks for the same key. +// The entry must still be the full result, or the later hit would answer +// without evidence pages. +func TestCachedSelectFirstStillCachesEvidence(t *testing.T) { + inner := &pageStrategy{} + cached := retrieval.NewCached(inner, retrieval.CachedConfig{TTL: time.Minute}) + tr := buildTree() + budget := retrieval.ContextBudget{MaxTokens: 100000, ModelName: "m"} + + if _, err := cached.Select(context.Background(), tr, "capex?", budget); err != nil { + t.Fatal(err) + } + hit, err := cached.SelectWithCost(context.Background(), tr, "capex?", budget) + if err != nil { + t.Fatal(err) + } + if inner.calls != 1 { + t.Fatalf("strategy ran %d times, want 1", inner.calls) + } + if len(hit.EvidencePages) != 1 || len(hit.CitedPages) != 1 { + t.Fatalf("Select-first entry lost evidence: %+v %v", hit.EvidencePages, hit.CitedPages) + } +} From 6888ada00d2bcedecae27b13dc60da249da46188 Mon Sep 17 00:00:00 2001 From: hallelx2 Date: Tue, 13 Oct 2026 12:17:49 +0100 Subject: [PATCH 3/3] test(retrieval): a cache hit reports no usage at all Compare the hit's whole Usage with zero, with every field set on the original run, rather than two fields (review on #86). --- pkg/retrieval/cached_evidence_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/retrieval/cached_evidence_test.go b/pkg/retrieval/cached_evidence_test.go index 1d4b719..38cac7e 100644 --- a/pkg/retrieval/cached_evidence_test.go +++ b/pkg/retrieval/cached_evidence_test.go @@ -29,7 +29,7 @@ func (p *pageStrategy) SelectWithCost(_ context.Context, t *tree.Tree, _ string, return &retrieval.Result{ SelectedIDs: ids, Confidences: map[tree.SectionID]float64{"sec_b": 0.9}, - Usage: retrieval.Usage{LLMCalls: 3, InputTokens: 900}, + Usage: retrieval.Usage{LLMCalls: 3, InputTokens: 900, OutputTokens: 40, TotalTokens: 940, CostUSD: 0.0001}, PagesRead: []retrieval.PageReadEntry{{StartPage: 60, EndPage: 60}}, EvidencePages: []retrieval.EvidencePage{{Page: 60, Title: "Cash flows", Text: "Purchases of PP&E (1,577) (1,373)", Confidence: 0.9}}, CitedPages: [][2]int{{60, 60}}, @@ -70,7 +70,7 @@ func TestCachedHitKeepsEvidencePages(t *testing.T) { if len(hit.CitedPages) != 1 || len(hit.PagesRead) != 1 || hit.Confidence != 0.8 || hit.Confidences["sec_b"] != 0.9 { t.Fatalf("page result lost on a hit: cited=%v read=%v conf=%v confs=%v", hit.CitedPages, hit.PagesRead, hit.Confidence, hit.Confidences) } - if hit.Usage.LLMCalls != 0 || hit.Usage.InputTokens != 0 { + if hit.Usage != (retrieval.Usage{}) { t.Fatalf("a hit costs nothing: usage = %+v", hit.Usage) } if hit.TraceToken != "tok-from-strategy" || hit.Reasoning != "cached" {