diff --git a/cmd/prime.go b/cmd/prime.go index d117d7e..0d818db 100644 --- a/cmd/prime.go +++ b/cmd/prime.go @@ -22,7 +22,11 @@ type primeOutput struct { UnknownCount int `json:"unknown_count"` Triggers []primeTrigger `json:"triggers"` TopicCounts []cairn.TopicCount `json:"topic_counts"` - Warnings []string `json:"warnings"` + // TopicCountsTruncated reports topic keys the byte budget kept out of + // TopicCounts (crn-s3749). Non-zero means topic_counts is PARTIAL -- + // total_visible and the freshness counts still cover everything. + TopicCountsTruncated int `json:"topic_counts_truncated"` + Warnings []string `json:"warnings"` } type primeTrigger struct { @@ -54,7 +58,8 @@ func projectPrime(result cairn.PrimeResult) primeOutput { return primeOutput{ Instruction: primeInstruction, TotalVisible: result.TotalVisible, FreshCount: result.FreshCount, StaleCount: result.StaleCount, UnknownCount: result.UnknownCount, - Triggers: triggers, TopicCounts: nonNil(result.TopicCounts), Warnings: nonNil(warnings), + Triggers: triggers, TopicCounts: nonNil(result.TopicCounts), + TopicCountsTruncated: result.TopicCountsTruncated, Warnings: nonNil(warnings), } } diff --git a/internal/cairn/prime.go b/internal/cairn/prime.go index 65ea777..4bdf9a3 100644 --- a/internal/cairn/prime.go +++ b/internal/cairn/prime.go @@ -63,23 +63,33 @@ type TopicCount struct { // PrimeResult is Prime's structured output: a budget-bounded, // deterministically ordered slice of the caller's visible entries, plus an -// index view -- TotalVisible, TopicCounts, and the aggregate freshness -// counts -- that always covers the entire visible set regardless of what -// the byte budget let through into Items (crn-0vqk FR-2, FR-3; crn-3476 -// FR-1, FR-2). +// index view -- TotalVisible and the aggregate freshness counts -- that +// always covers the entire visible set regardless of what the byte budget +// let through into Items (crn-0vqk FR-2, FR-3; crn-3476 FR-1, FR-2). +// +// TopicCounts is the one part of that index view which is NOT unconditional: +// it is an enumeration, so its size grows with the number of distinct topic +// keys, and crn-s3749 measured it at 88.2% of a payload running 2.5x over +// budget. It is now bounded like Items, with TopicCountsTruncated reporting +// what the cap dropped. The scalars stay unconditional because they are O(1) +// bytes -- bounding those would turn an honest overrun into an undercount. type PrimeResult struct { - Store string `json:"store"` - Identity []string `json:"identity"` - Items []PrimeItem `json:"items"` - TotalVisible int `json:"total_visible"` - TopicCounts []TopicCount `json:"topic_counts"` - TruncatedCount int `json:"truncated_count"` - FreshCount int `json:"fresh_count"` - StaleCount int `json:"stale_count"` - UnknownCount int `json:"unknown_count"` - ChecksCapped bool `json:"checks_capped"` - Warnings []string `json:"warnings"` - Conflicts []TopicConflict `json:"conflicts,omitempty"` + Store string `json:"store"` + Identity []string `json:"identity"` + Items []PrimeItem `json:"items"` + TotalVisible int `json:"total_visible"` + TopicCounts []TopicCount `json:"topic_counts"` + // TopicCountsTruncated is how many topic keys the byte budget kept out of + // TopicCounts. Non-zero means the index enumeration is PARTIAL; the + // aggregate scalars above still describe the whole visible set. + TopicCountsTruncated int `json:"topic_counts_truncated"` + TruncatedCount int `json:"truncated_count"` + FreshCount int `json:"fresh_count"` + StaleCount int `json:"stale_count"` + UnknownCount int `json:"unknown_count"` + ChecksCapped bool `json:"checks_capped"` + Warnings []string `json:"warnings"` + Conflicts []TopicConflict `json:"conflicts,omitempty"` } // Prime computes an agent's always-in-context payload: a budget-bounded, @@ -105,36 +115,18 @@ func Prime(ctx context.Context, store string, identity []string, budgetBytes int } } - ordered := make([]*Entry, len(visible)) - copy(ordered, visible) - explorationBand := explorationBandIDs(ordered, explorationSlots) - sort.SliceStable(ordered, func(i, j int) bool { - a, b := ordered[i], ordered[j] - if aBand, bBand := explorationBandRank(a, explorationBand), explorationBandRank(b, explorationBand); aBand != bBand { - return aBand < bBand - } - if a.HitCount != b.HitCount { - return a.HitCount > b.HitCount - } - if a.CreatedAt != b.CreatedAt { - // CreatedAt is stamped with time.RFC3339 (see remember.go); - // lexicographic order matches chronological order for that - // format, so no time parsing is needed here. - return a.CreatedAt > b.CreatedAt - } - return a.ID < b.ID - }) + ordered := orderByPriority(visible) result := PrimeResult{ Store: store, Identity: identity, TotalVisible: len(ordered), - TopicCounts: topicCounts(visible), Warnings: scopeMismatchWarnings(all, visible, identity), Conflicts: resolution.Conflicts, } budget := &freshnessBudget{remaining: maxFreshnessChecksPerPrime} + itemBudget := itemBudgetFor(budgetBytes) usedBytes := 0 truncating := false for _, e := range ordered { @@ -173,7 +165,7 @@ func Prime(ctx context.Context, store string, identity []string, budgetBytes int // len==0 guard) so a budget too small for even one item doesn't // leave a caller with zero items despite a nonzero visible count. cost := itemByteCost(item) - if usedBytes+cost > budgetBytes && len(result.Items) > 0 { + if usedBytes+cost > itemBudget && len(result.Items) > 0 { truncating = true continue } @@ -183,9 +175,109 @@ func Prime(ctx context.Context, store string, identity []string, budgetBytes int result.ChecksCapped = budget.capped result.TruncatedCount = result.TotalVisible - len(result.Items) + applyIndexBudget(&result, visible, budgetBytes-usedBytes) + return result, nil } +// applyIndexBudget fills in the topic index from whatever the items left +// unspent, and records what the cap dropped. Items keep first claim because +// they are the ranked, high-salience payload; the index is for routing. +// +// Split out of Prime rather than inlined so Prime stays under the cyclomatic +// complexity limit, and so the "report what you dropped" half sits next to the +// truncation that makes it necessary. +func applyIndexBudget(result *PrimeResult, visible []*Entry, allowance int) { + // Clamp at zero: the always-itemize-one guard in Prime can push usedBytes + // past itemBudget on a tiny budget, and the index must not then be handed + // a negative allowance. + if allowance < 0 { + allowance = 0 + } + allTopics := topicCounts(visible) + result.TopicCounts, result.TopicCountsTruncated = boundTopicCounts(allTopics, allowance) + if result.TopicCountsTruncated == 0 { + return + } + result.Warnings = append(result.Warnings, fmt.Sprintf( + "topic index truncated by the byte budget: %d of %d topic keys not listed (aggregate counts still cover all %d entries)", + result.TopicCountsTruncated, len(allTopics), result.TotalVisible)) +} + +// orderByPriority returns visible in the order Items should be itemized: the +// exploration band first, then most-hit, then newest, then ID as a deterministic +// tiebreak. Split out of Prime to keep it inside the funlen limit; the ordering +// is load-bearing because byte-budget truncation walks this slice in order and +// stops at the first entry that does not fit. +func orderByPriority(visible []*Entry) []*Entry { + ordered := make([]*Entry, len(visible)) + copy(ordered, visible) + explorationBand := explorationBandIDs(ordered, explorationSlots) + sort.SliceStable(ordered, func(i, j int) bool { + a, b := ordered[i], ordered[j] + if aBand, bBand := explorationBandRank(a, explorationBand), explorationBandRank(b, explorationBand); aBand != bBand { + return aBand < bBand + } + if a.HitCount != b.HitCount { + return a.HitCount > b.HitCount + } + if a.CreatedAt != b.CreatedAt { + // CreatedAt is stamped with time.RFC3339 (see remember.go); + // lexicographic order matches chronological order for that + // format, so no time parsing is needed here. + return a.CreatedAt > b.CreatedAt + } + return a.ID < b.ID + }) + return ordered +} + +// itemBudgetFor carves the index reserve OUT of budgetBytes before itemizing, +// rather than adding it on top -- otherwise Items could spend the whole budget +// and the reserve would push the total over, which is crn-s3749 in a different +// place. Items keep first claim on everything above the reserve, and anything +// they leave unspent flows back to the index (see applyIndexBudget). +func itemBudgetFor(budgetBytes int) int { + return budgetBytes - budgetBytes/indexReserveDivisor +} + +// indexReserveDivisor sets the share of budgetBytes withheld from Items so the +// topic index cannot be starved to nothing by a store with many entries. +const indexReserveDivisor = 4 + +// boundTopicCounts caps the topic enumeration at allowance bytes, returning the +// kept rows and the number dropped. +// +// Ordering is count-descending: a topic holding more than one entry is the only +// row carrying information the per-entry Items list does not already have, so +// those must survive a cap that singletons do not. Ties break on topic_key so +// output stays deterministic. +func boundTopicCounts(all []TopicCount, allowance int) ([]TopicCount, int) { + ranked := append([]TopicCount(nil), all...) + sort.SliceStable(ranked, func(i, j int) bool { + if ranked[i].Count != ranked[j].Count { + return ranked[i].Count > ranked[j].Count + } + return ranked[i].TopicKey < ranked[j].TopicKey + }) + + used, kept := 0, 0 + for _, tc := range ranked { + b, _ := json.Marshal(tc) + if used+len(b) > allowance { + break + } + used += len(b) + kept++ + } + out := ranked[:kept] + // Deterministic, human-scannable output: rank decides WHICH rows survive, + // topic_key decides what order they print in -- matching the key-sorted + // contract callers had before this cap existed. + sort.SliceStable(out, func(i, j int) bool { return out[i].TopicKey < out[j].TopicKey }) + return out, len(all) - kept +} + // topicCounts reduces entries to a per-topic_key breakdown for Prime's index // view (crn-3476 FR-1/FR-2): a pure in-memory map reduction over the // already-loaded visible set, no new query and no I/O (NFR-2), so this is as diff --git a/internal/cairn/prime_test.go b/internal/cairn/prime_test.go index b0e3c96..db563a0 100644 --- a/internal/cairn/prime_test.go +++ b/internal/cairn/prime_test.go @@ -1,6 +1,8 @@ package cairn import ( + "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -388,16 +390,33 @@ func TestPrimeIndexBreakdownIndependentOfByteBudget(t *testing.T) { writeFile(t, dir, "global/a1.md", "+++\nid = \"a1\"\ntitle = \"a1\"\ntopic_key = \"topic-a\"\nscope = []\n+++\nx\n") writeFile(t, dir, "global/b1.md", "+++\nid = \"b1\"\ntitle = \"b1\"\ntopic_key = \"topic-b\"\nscope = []\n+++\nx\n") - tiny, err := Prime(t.Context(), dir, nil, 1) + // AMENDED by crn-s3749. This test used to assert the breakdown listed + // every topic even at budget=1. That guarantee was the bug: the + // enumeration was exempt from the budget, so it grew without limit (88.2% + // of a payload measured 2.5x over budget on the mayor's live store). + // + // What survives is the half that was always right, and is what FR-1/NFR-2 + // actually needed: the index view's cost is independent of ENTRY CONTENT + // size, and its aggregate scalars still describe the whole visible set. The + // per-topic enumeration is now bounded like Items, and says what it dropped. + generous, err := Prime(t.Context(), dir, nil, testBudget) require.NoError(t, err) - require.Less(t, len(tiny.Items), 2, "precondition: the tiny budget must actually truncate something") - counts := map[string]int{} - for _, tc := range tiny.TopicCounts { + for _, tc := range generous.TopicCounts { counts[tc.TopicKey] = tc.Count } - assert.Equal(t, 1, counts["topic-a"], "the topic breakdown must reflect the full visible set even when the byte budget truncates Items") - assert.Equal(t, 1, counts["topic-b"], "the topic breakdown must reflect the full visible set even when the byte budget truncates Items") + assert.Equal(t, 1, counts["topic-a"], "with budget to spare the breakdown must still list every topic") + assert.Equal(t, 1, counts["topic-b"], "with budget to spare the breakdown must still list every topic") + assert.Zero(t, generous.TopicCountsTruncated, "nothing should be reported dropped when everything fits") + + tiny, err := Prime(t.Context(), dir, nil, 1) + require.NoError(t, err) + require.Less(t, len(tiny.Items), 2, "precondition: the tiny budget must actually truncate something") + + assert.Equal(t, 2, tiny.TotalVisible, + "the index view's SCALARS must still cover the whole visible set under any budget") + assert.Equal(t, 2-len(tiny.TopicCounts), tiny.TopicCountsTruncated, + "whatever the budget keeps out of the enumeration must be reported, not silently missing") } // TestPrimeTruncatesOversizedTitleAndSummaryToCap covers crn-3476 FR-3's @@ -472,3 +491,121 @@ func TestRenderPrimeTextIncludesSummary(t *testing.T) { out := RenderPrimeText(result) assert.Contains(t, out, "a distinctive summary sentence") } + +// topicCountsByteCost mirrors the production accounting for the index view so +// these tests measure the same bytes the budget is supposed to govern. +func topicCountsByteCost(tcs []TopicCount) int { + total := 0 + for _, tc := range tcs { + b, _ := json.Marshal(tc) + total += len(b) + } + return total +} + +// seededTopics is how many single-topic entries seedTopics writes: enough that +// the index view dominates the payload the way it does on a real store. +const seededTopics = 200 + +// seedTopics writes seededTopics entries each carrying its own distinct topic_key, which +// is the shape that makes TopicCounts grow: in the real store 277 of 278 topic +// keys held exactly one entry, so the index is the key list rather than an +// aggregation. +func seedTopics(t *testing.T, dir string) { + t.Helper() + for i := range seededTopics { + writeFile(t, dir, fmt.Sprintf("global/e%03d.md", i), + fmt.Sprintf("+++\nid = \"e%03d\"\ntitle = \"entry %03d\"\ntopic_key = \"topic-%03d\"\nscope = []\n+++\nx\n", i, i, i)) + } +} + +// TestPrimeBoundsTotalPayloadNotJustItems is crn-s3749. budgetBytes was +// enforced only against Items, while TopicCounts -- exempted as an "index +// view" -- grew unbounded with the number of distinct topic keys. Measured on +// the mayor's live store: 20,807 bytes emitted against an 8,192-byte budget, +// 88.2% of it TopicCounts. The budget must bound the whole itemized payload. +func TestPrimeBoundsTotalPayloadNotJustItems(t *testing.T) { + dir := t.TempDir() + seedTopics(t, dir) + + const budget = 2048 + result, err := Prime(t.Context(), dir, nil, budget) + require.NoError(t, err) + + itemBytes := 0 + for _, it := range result.Items { + itemBytes += itemByteCost(it) + } + topicBytes := topicCountsByteCost(result.TopicCounts) + + require.Equal(t, seededTopics, result.TotalVisible, + "precondition: every seeded entry is visible, so the index view has something to bound") + assert.LessOrEqual(t, itemBytes+topicBytes, budget, + "the byte budget must bound the TOTAL itemized payload (items + topic counts), not items alone") +} + +// TestPrimeReportsOmittedTopicsRatherThanTruncatingSilently pins the half that +// makes the cap safe. A silent cap is worse than the overrun it replaces: a +// reader cannot tell a short index from a complete one, which is the same +// false-success shape the store already warns about elsewhere. +func TestPrimeReportsOmittedTopicsRatherThanTruncatingSilently(t *testing.T) { + dir := t.TempDir() + seedTopics(t, dir) + + result, err := Prime(t.Context(), dir, nil, 2048) + require.NoError(t, err) + + require.Less(t, len(result.TopicCounts), seededTopics, + "precondition: this budget must actually drop topics from the index") + assert.Equal(t, seededTopics-len(result.TopicCounts), result.TopicCountsTruncated, + "every topic dropped from the index must be counted in TopicCountsTruncated") + assert.Contains(t, strings.Join(result.Warnings, "\n"), "topic", + "a truncated index must say so in Warnings, not just in a field a caller might not read") +} + +// TestPrimeIndexKeepsRealAggregationsWhenTruncating covers the ordering the cap +// forces us to choose. Sorted by topic_key, truncation keeps whatever sorts +// early -- arbitrary. Topics holding more than one entry are the only rows that +// carry information a per-entry list does not, so they must survive. +func TestPrimeIndexKeepsRealAggregationsWhenTruncating(t *testing.T) { + dir := t.TempDir() + seedTopics(t, dir) + // "zzz-shared" sorts last by key and holds 3 entries. Under the old + // key-ascending order it would be dropped first; it must now survive. + for i := range 3 { + writeFile(t, dir, fmt.Sprintf("global/shared%d.md", i), + fmt.Sprintf("+++\nid = \"shared%d\"\ntitle = \"shared %d\"\ntopic_key = \"zzz-shared\"\nscope = []\n+++\nx\n", i, i)) + } + + result, err := Prime(t.Context(), dir, nil, 2048) + require.NoError(t, err) + require.Less(t, len(result.TopicCounts), seededTopics+1, "precondition: the index must be truncated") + + var shared *TopicCount + for i := range result.TopicCounts { + if result.TopicCounts[i].TopicKey == "zzz-shared" { + shared = &result.TopicCounts[i] + } + } + require.NotNil(t, shared, "a multi-entry topic must survive index truncation ahead of singletons") + assert.Equal(t, 3, shared.Count) +} + +// TestPrimeAggregateCountsStillCoverEverythingWhenIndexTruncates keeps the +// half of crn-3476 FR-1/FR-2 that is still right. The per-topic ENUMERATION is +// now bounded, but the scalars are O(1) bytes and must keep describing the +// whole visible set -- otherwise bounding the index would turn an honest +// overrun into an undercount. +func TestPrimeAggregateCountsStillCoverEverythingWhenIndexTruncates(t *testing.T) { + dir := t.TempDir() + seedTopics(t, dir) + + result, err := Prime(t.Context(), dir, nil, 2048) + require.NoError(t, err) + + require.Less(t, len(result.TopicCounts), seededTopics, "precondition: the index must be truncated") + assert.Equal(t, seededTopics, result.TotalVisible, + "TotalVisible must still cover the whole visible set even when the index enumeration is capped") + assert.Equal(t, seededTopics, result.FreshCount+result.StaleCount+result.UnknownCount, + "the aggregate freshness counts must still sum to the whole visible set") +}