From df3fa7cc66aae526af6d7cab521c10321658d244 Mon Sep 17 00:00:00 2001 From: radimsem Date: Sat, 6 Jun 2026 19:12:28 +0200 Subject: [PATCH] feat(mcp): bound MemoryDelta results with a limit Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/versioning.md | 2 +- integration_test.go | 2 +- pkg/mcp/tools/delta.go | 41 +++++++- pkg/mcp/tools/tools_test.go | 52 ++++++++++ pkg/query/engine.go | 37 ++++++- pkg/query/engine_test.go | 102 +++++++++++++++++++- pkg/store/queries.go | 2 +- pkg/store/snapshot.go | 4 +- pkg/store/store_test.go | 2 +- skills/remind/references/snapshots-diffs.md | 13 ++- 10 files changed, 243 insertions(+), 14 deletions(-) diff --git a/docs/versioning.md b/docs/versioning.md index 7229094..c5d3ba4 100644 --- a/docs/versioning.md +++ b/docs/versioning.md @@ -32,7 +32,7 @@ One subtlety worth knowing: a `mod` whose `old_hash == new_hash` is a **structur Three tools, picked by which end of the range is fixed: -- **`MemoryDelta`** — "what changed since X?" The upper bound is always HEAD. This is the resync call: pass the last snapshot `id` you saw and get back just the changed nodes, not the whole tree. +- **`MemoryDelta`** — "what changed since X?" The upper bound is always HEAD. This is the resync call: pass the last snapshot `id` you saw and get back just the changed nodes, not the whole tree. Results are capped (`limit`, default 500, ceiling 5000) to keep a busy vault from blowing the context window; when more changes exist the response ends with a `note:` telling you to resume from a later snapshot `id` or raise the cap. - **`MemoryDiff`** — "what changed between X and Y?" Both ends fixed, git-diff-style hunks. State at X versus state at Y, not the per-snapshot event log between them — intermediate jitter collapses to the net change. This is the forensic call (rollback target vs. result, yesterday's compile vs. today's). - **`MemoryHistory`** — the diff trail for one specific node. Use it before overwriting something, or to cite prior wording. diff --git a/integration_test.go b/integration_test.go index 34a2550..6e0422b 100644 --- a/integration_test.go +++ b/integration_test.go @@ -564,7 +564,7 @@ func TestRecompileWorkflow(t *testing.T) { // Delta query should show changes between v1 and v2. eng := query.NewEngine(st) - diffs, err := eng.Delta(ctx, snaps[1].ID) + diffs, _, _, err := eng.Delta(ctx, snaps[1].ID, 10) if err != nil { t.Fatalf("Delta: %v", err) } diff --git a/pkg/mcp/tools/delta.go b/pkg/mcp/tools/delta.go index 237c806..708bafd 100644 --- a/pkg/mcp/tools/delta.go +++ b/pkg/mcp/tools/delta.go @@ -7,16 +7,25 @@ import ( "time" gomcp "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/radimsem/remindb/pkg/store" +) + +const ( + defaultDeltaLimit = 500 + maxDeltaLimit = 5000 ) type DeltaInput struct { SinceSnapshot int64 `json:"since_snapshot,omitempty" jsonschema:"Snapshot ID to diff from (0 for all changes)"` + Limit int `json:"limit,omitempty" jsonschema:"Max diff rows to return (1-5000, default 500); past the cap the result is truncated with a since_snapshot continuation note"` } func (d *Deps) HandleDelta(ctx context.Context, _ *gomcp.CallToolRequest, input DeltaInput) (_ *gomcp.CallToolResult, _ any, err error) { - defer d.logCall(ctx, "MemoryDelta", &err, time.Now(), "since_snapshot", input.SinceSnapshot) + limit := resolveDeltaLimit(input.Limit) + defer d.logCall(ctx, "MemoryDelta", &err, time.Now(), "since_snapshot", input.SinceSnapshot, "limit", limit) - diffs, err := d.Engine.Delta(ctx, input.SinceSnapshot) + diffs, truncated, overflow, err := d.Engine.Delta(ctx, input.SinceSnapshot, limit) if err != nil { return nil, nil, fmt.Errorf("failed to get delta: %w", err) } @@ -29,5 +38,33 @@ func (d *Deps) HandleDelta(ctx context.Context, _ *gomcp.CallToolRequest, input for _, dr := range diffs { fmt.Fprintf(&b, "[%s] %s (snapshot %d)\n", dr.Op, dr.NodeID, dr.SnapshotID) } + if truncated { + fmt.Fprintf(&b, "\n%s\n", deltaTruncationNote(diffs, input.SinceSnapshot, limit, overflow)) + } return textResult(b.String()), nil, nil } + +func resolveDeltaLimit(limit int) int64 { + if limit <= 0 { + return defaultDeltaLimit + } + if limit > maxDeltaLimit { + return maxDeltaLimit + } + return int64(limit) +} + +// Continuation note for a truncated delta. A snapshot that overflows the limit can't be resumed +// past by advancing since_snapshot; steer to a higher limit while one helps, and once the page is +// already maxed to MemoryDiff scoped to that snapshot — which is exactly the immediate-next +// snapshot after since, so it remains reachable in full. +func deltaTruncationNote(diffs []*store.DiffRecord, since, limit int64, overflow bool) string { + last := diffs[len(diffs)-1].SnapshotID + if !overflow { + return fmt.Sprintf("note: truncated at limit %d; more changes exist. Re-call MemoryDelta(since_snapshot=%d) to continue.", limit, last) + } + if limit < maxDeltaLimit { + return fmt.Sprintf("note: snapshot %d alone has more than %d diffs; raise limit (max %d) to see the rest.", last, limit, maxDeltaLimit) + } + return fmt.Sprintf("note: snapshot %d exceeds the max page of %d diffs and can't be paged by MemoryDelta; read it in full with MemoryDiff(from_snapshot_id=%d, to_snapshot_id=%d).", last, maxDeltaLimit, since, last) +} diff --git a/pkg/mcp/tools/tools_test.go b/pkg/mcp/tools/tools_test.go index b152ad4..687f14e 100644 --- a/pkg/mcp/tools/tools_test.go +++ b/pkg/mcp/tools/tools_test.go @@ -623,6 +623,58 @@ func TestHandleDelta(t *testing.T) { } } +func TestHandleDelta_LimitTruncationNote(t *testing.T) { + d, _ := setup(t) + ctx := context.Background() + + mustWriteSnapshot(t, d, ctx, "", "one") + mustWriteSnapshot(t, d, ctx, "", "two") + mustWriteSnapshot(t, d, ctx, "", "three") + + // limit=1 over three single-diff snapshots: a clean boundary, not an overflow. + result, _, err := d.HandleDelta(ctx, &gomcp.CallToolRequest{}, DeltaInput{SinceSnapshot: 0, Limit: 1}) + if err != nil { + t.Fatalf("HandleDelta: %v", err) + } + + text := textContent(t, result) + if !strings.Contains(text, "note: truncated at limit 1") { + t.Errorf("missing continuation note, got:\n%s", text) + } + if !strings.Contains(text, "since_snapshot=") { + t.Errorf("note lacks since_snapshot continuation, got:\n%s", text) + } + if strings.Contains(text, "raise limit") { + t.Errorf("clean boundary should not advise raising limit, got:\n%s", text) + } +} + +func TestDeltaTruncationNote(t *testing.T) { + multi := []*store.DiffRecord{{SnapshotID: 10}, {SnapshotID: 20}} + single := []*store.DiffRecord{{SnapshotID: 30}, {SnapshotID: 30}} + + // Boundary truncation: resume from the last complete snapshot. + note := deltaTruncationNote(multi, 0, 500, false) + if !strings.Contains(note, "since_snapshot=20") || strings.Contains(note, "raise limit") { + t.Errorf("boundary note = %q", note) + } + + // Overflow below the ceiling: raising the limit can help. + note = deltaTruncationNote(single, 5, 500, true) + if !strings.Contains(note, "raise limit (max 5000)") { + t.Errorf("overflow-below-ceiling note = %q", note) + } + + // Overflow at the ceiling: raising is impossible, steer to MemoryDiff scoped to the snapshot. + note = deltaTruncationNote(single, 5, maxDeltaLimit, true) + if !strings.Contains(note, "MemoryDiff(from_snapshot_id=5, to_snapshot_id=30)") { + t.Errorf("overflow-at-ceiling note = %q", note) + } + if strings.Contains(note, "raise limit") { + t.Errorf("at-ceiling note should not advise raising limit: %q", note) + } +} + func TestHandleDiff_FullRangeAndSubset(t *testing.T) { d, st := setup(t) ctx := context.Background() diff --git a/pkg/query/engine.go b/pkg/query/engine.go index 7d5251a..5b25c5e 100644 --- a/pkg/query/engine.go +++ b/pkg/query/engine.go @@ -21,7 +21,7 @@ type QueryStore interface { SearchRanked(ctx context.Context, query string, limit int) ([]*store.RankedNode, error) - GetDiffsSince(ctx context.Context, sinceSnapshotID int64) ([]*store.DiffRecord, error) + GetDiffsSince(ctx context.Context, sinceSnapshotID, limit int64) ([]*store.DiffRecord, error) GetDiffsBetween(ctx context.Context, fromSnapshotID, toSnapshotID int64) ([]*store.DiffRecord, error) } @@ -135,8 +135,39 @@ func (e *Engine) Search(ctx context.Context, query string, budget int) (*Result, return &filled, nil } -func (e *Engine) Delta(ctx context.Context, sinceSnapshot int64) ([]*store.DiffRecord, error) { - return e.store.GetDiffsSince(ctx, sinceSnapshot) +// Return diffs after sinceSnapshot (upper bound HEAD), bounded to limit rows. truncated reports +// that more diffs exist; the trailing partial snapshot is trimmed so a caller resumes losslessly +// by id. overflow (implies truncated) means one snapshot alone exceeds limit, so there's nothing +// complete to trim back to — a caller must raise limit rather than advance. A non-positive +// limit yields no rows. +func (e *Engine) Delta(ctx context.Context, sinceSnapshot, limit int64) (diffs []*store.DiffRecord, truncated, overflow bool, err error) { + if limit < 1 { + return nil, false, false, nil + } + + diffs, err = e.store.GetDiffsSince(ctx, sinceSnapshot, limit+1) + if err != nil { + return nil, false, false, err + } + if int64(len(diffs)) <= limit { + return diffs, false, false, nil + } + + peek := diffs[limit].SnapshotID + diffs = diffs[:limit] + last := diffs[len(diffs)-1].SnapshotID + if peek != last { + return diffs, true, false, nil // limit aligned on a snapshot boundary; last is complete + } + if diffs[0].SnapshotID == last { + return diffs, true, true, nil // one snapshot exceeds limit; nothing complete to trim back to + } + + cut := len(diffs) + for cut > 0 && diffs[cut-1].SnapshotID == last { + cut-- + } + return diffs[:cut], true, false, nil } func (e *Engine) Diff(ctx context.Context, fromSnapshot, toSnapshot int64) ([]*store.DiffRecord, error) { diff --git a/pkg/query/engine_test.go b/pkg/query/engine_test.go index e539202..105fb4b 100644 --- a/pkg/query/engine_test.go +++ b/pkg/query/engine_test.go @@ -428,10 +428,13 @@ func TestDelta(t *testing.T) { eng := NewEngine(st) - diffs, err := eng.Delta(ctx, 1) + diffs, truncated, _, err := eng.Delta(ctx, 1, 10) if err != nil { t.Fatalf("Delta: %v", err) } + if truncated { + t.Error("truncated = true, want false") + } if len(diffs) != 1 { t.Fatalf("len = %d, want 1", len(diffs)) } @@ -440,6 +443,103 @@ func TestDelta(t *testing.T) { } } +func writeDiffSnapshot(t *testing.T, st *store.Store, ctx context.Context, hash, msg string, nodeIDs ...string) int64 { + t.Helper() + + var id int64 + err := st.Tx(ctx, func(tx *sql.Tx) error { + var err error + id, err = st.CreateSnapshotTx(ctx, tx, store.WithCursorHash(hash), store.WithMessage(msg)) + if err != nil { + return err + } + for _, nid := range nodeIDs { + diff := &store.DiffRecord{SnapshotID: id, NodeID: nid, Op: "add", NewHash: "h", NewContent: "x"} + if err := st.InsertDiffTx(ctx, tx, diff); err != nil { + return err + } + } + return st.AdvanceCursorTx(ctx, tx, id) + }) + if err != nil { + t.Fatalf("writeDiffSnapshot %s: %v", msg, err) + } + return id +} + +func TestDelta_Limit(t *testing.T) { + st := testutil.OpenTestDB(t) + ctx := context.Background() + + a := writeDiffSnapshot(t, st, ctx, "hashaaaa", "A", "a0000001", "a0000002") + writeDiffSnapshot(t, st, ctx, "hashbbbb", "B", "b0000001", "b0000002", "b0000003") + writeDiffSnapshot(t, st, ctx, "hashcccc", "C", "c0000001", "c0000002") + + eng := NewEngine(st) + + // Fits under the limit: all 7 diffs, no truncation. + diffs, truncated, overflow, err := eng.Delta(ctx, 0, 10) + if err != nil { + t.Fatalf("Delta fit: %v", err) + } + if truncated || overflow || len(diffs) != 7 { + t.Fatalf("fit: len=%d truncated=%v overflow=%v, want 7,false,false", len(diffs), truncated, overflow) + } + + // Cap lands inside snapshot B: trim B, end on complete snapshot A. + diffs, truncated, overflow, err = eng.Delta(ctx, 0, 3) + if err != nil { + t.Fatalf("Delta trim: %v", err) + } + if !truncated || overflow { + t.Errorf("trim: truncated=%v overflow=%v, want true,false", truncated, overflow) + } + if len(diffs) != 2 { + t.Fatalf("trim: len=%d, want 2 (snapshot A only)", len(diffs)) + } + if last := diffs[len(diffs)-1].SnapshotID; last != a { + t.Errorf("trim: continuation = %d, want %d (last complete snapshot)", last, a) + } + + // Limit aligns on a snapshot boundary (snapshot A's 2 diffs): complete, not overflow. + diffs, truncated, overflow, err = eng.Delta(ctx, 0, 2) + if err != nil { + t.Fatalf("Delta boundary: %v", err) + } + if !truncated || overflow { + t.Errorf("boundary: truncated=%v overflow=%v, want true,false", truncated, overflow) + } + if len(diffs) != 2 || diffs[len(diffs)-1].SnapshotID != a { + t.Errorf("boundary: len=%d last=%d, want 2 ending on snapshot %d", len(diffs), diffs[len(diffs)-1].SnapshotID, a) + } + + // A single snapshot exceeds the limit: can't trim, flag overflow. + diffs, truncated, overflow, err = eng.Delta(ctx, 0, 1) + if err != nil { + t.Fatalf("Delta overflow: %v", err) + } + if !truncated || !overflow { + t.Errorf("overflow: truncated=%v overflow=%v, want true,true", truncated, overflow) + } + if len(diffs) != 1 { + t.Fatalf("overflow: len=%d, want 1", len(diffs)) + } + if diffs[0].SnapshotID != a { + t.Errorf("overflow: snapshot = %d, want %d", diffs[0].SnapshotID, a) + } + + // A non-positive limit yields no rows instead of panicking on the trim slice. + for _, lim := range []int64{0, -1} { + diffs, truncated, overflow, err = eng.Delta(ctx, 0, lim) + if err != nil { + t.Fatalf("Delta limit=%d: %v", lim, err) + } + if len(diffs) != 0 || truncated || overflow { + t.Errorf("limit=%d: len=%d truncated=%v overflow=%v, want 0,false,false", lim, len(diffs), truncated, overflow) + } + } +} + func TestFormat(t *testing.T) { result := &Result{ Nodes: []ScoredNode{ diff --git a/pkg/store/queries.go b/pkg/store/queries.go index 37ce17e..2af16dd 100644 --- a/pkg/store/queries.go +++ b/pkg/store/queries.go @@ -157,7 +157,7 @@ const ( qSelectDiffsBySnapshot = `SELECT ` + diffColumns + ` FROM diffs WHERE snapshot_id = ? ORDER BY id` - qSelectDiffsSince = `SELECT ` + diffColumns + ` FROM diffs WHERE snapshot_id > ? ORDER BY snapshot_id, id` + qSelectDiffsSince = `SELECT ` + diffColumns + ` FROM diffs WHERE snapshot_id > ? ORDER BY snapshot_id, id LIMIT ?` qSelectDiffsBetween = `SELECT ` + diffColumns + ` FROM diffs WHERE snapshot_id > ? AND snapshot_id <= ? ORDER BY snapshot_id, id` diff --git a/pkg/store/snapshot.go b/pkg/store/snapshot.go index b27dcca..b3ed5e2 100644 --- a/pkg/store/snapshot.go +++ b/pkg/store/snapshot.go @@ -224,8 +224,8 @@ func (s *Store) GetDiffsBySnapshot(ctx context.Context, snapshotID int64) ([]*Di return collectDiffRows(rows) } -func (s *Store) GetDiffsSince(ctx context.Context, sinceSnapshotID int64) ([]*DiffRecord, error) { - rows, err := s.db.QueryContext(ctx, qSelectDiffsSince, sinceSnapshotID) +func (s *Store) GetDiffsSince(ctx context.Context, sinceSnapshotID, limit int64) ([]*DiffRecord, error) { + rows, err := s.db.QueryContext(ctx, qSelectDiffsSince, sinceSnapshotID, limit) if err != nil { return nil, err } diff --git a/pkg/store/store_test.go b/pkg/store/store_test.go index 55d92b4..f1f89c2 100644 --- a/pkg/store/store_test.go +++ b/pkg/store/store_test.go @@ -1018,7 +1018,7 @@ func TestGetDiffsSince(t *testing.T) { must(t, err) // Diffs since snapshot 1 should only include snapshot 2's diffs. - diffs, err := st.GetDiffsSince(ctx, 1) + diffs, err := st.GetDiffsSince(ctx, 1, 10) if err != nil { t.Fatalf("GetDiffsSince: %v", err) } diff --git a/skills/remind/references/snapshots-diffs.md b/skills/remind/references/snapshots-diffs.md index 09888eb..637f314 100644 --- a/skills/remind/references/snapshots-diffs.md +++ b/skills/remind/references/snapshots-diffs.md @@ -21,12 +21,21 @@ Picked by which end of the range is fixed. **`MemoryDelta`** — "what changed since X?", upper bound always HEAD. Use on resume / after external writes; pass the last snapshot **id** seen: ``` -remindb__MemoryDelta(since_snapshot=42) # snapshot ID (int64), not cursor_hash -remindb__MemoryDelta(since_snapshot=0) # all changes ever — expensive, rarely wanted +remindb__MemoryDelta(since_snapshot=42) # snapshot ID (int64), not cursor_hash +remindb__MemoryDelta(since_snapshot=0) # all changes ever, bounded by limit +remindb__MemoryDelta(since_snapshot=42, limit=1000) # raise the cap (default 500, max 5000) ``` Returns `[op] node_id (snapshot N)` lines; fetch nodes you need. Keep the last snapshot id from a prior tree/search/write result. +**`limit`** caps rows (default 500, ceiling 5000 — over-asks clamp, not error). When more changes exist the result ends with a `note:` line — **follow it, don't assume you're synced**: + +- `note: truncated at limit N; ... Re-call MemoryDelta(since_snapshot=M)` → the cap landed on a snapshot boundary; resume from `M` (the last *complete* snapshot, no rows skipped). +- `note: snapshot M alone has more than N diffs; raise limit (max 5000)` → one snapshot is bigger than the current cap, so advancing would skip its tail; bump `limit` instead. +- `note: snapshot M exceeds the max page of 5000 diffs ... read it in full with MemoryDiff(...)` → a single snapshot is bigger than the hard ceiling; `MemoryDelta` can't page within one snapshot, so the note hands you the exact `MemoryDiff` call (that snapshot is the immediate next one, so the diff is scoped to it alone). + +Truncation is signalled by the **note's presence**, not the row count (a trimmed page can return fewer than `limit` rows). + **`MemoryDiff`** — "what changed between X and Y?", both ends fixed. Like `git diff X Y`: compares state-at-X vs state-at-Y, not the event log between. Lower bound exclusive, upper inclusive: ```