feat(store): add recall tracking and promoted observations endpoint#96
Conversation
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
This is thoughtful work, but it is not a quick merge. You are changing persistence semantics, schema, endpoint surface, and MCP behavior in one shot. Before I merge something at that depth, I need the linked issue approved and I need the design justified explicitly around migration safety, read-side side effects, and product scope. Tighten the design case first, then we can review the implementation.
|
Thanks for the review @Alan-TheGentleman. I've posted a detailed design justification on the linked issue (#95) covering the three areas you raised:
Full details: #95 (comment) Once the issue is reviewed, I'll rebase on current main (no conflicts expected — the 21 upstream commits don't touch store/mcp/server) and force-push. |
6c9a7ce to
a9d35df
Compare
Add read-side instrumentation to observations: recall_count and last_recalled_at columns track how often each observation is retrieved via Search and GetObservation. Frequently recalled observations can be queried through a new /promoted HTTP endpoint and mem_promoted MCP tool. - Add recall_count and last_recalled_at to observations schema - Increment recall_count on Search() and GetObservation() (fire-and-forget) - Split GetObservation into public (with recall) and private (without) to prevent Timeline navigation from inflating counts - Add PromotedObservations() store method with configurable threshold - Add GET /promoted HTTP endpoint with project/scope/min_recalls/limit params - Add mem_promoted MCP tool to agent profile (deferred loading) - Add 4 tests covering recall increment and promotion filtering Closes Gentleman-Programming#95
a9d35df to
637dffe
Compare
|
Closing: upstream removed recall tracking in b6b1f6f (perf(mcp): defer 4 rare tools). Feature didn't generate practical value after testing. Syncing fork with upstream direction. |
Linked Issue
Closes #95
PR Type
Summary
recall_count,last_recalled_at) to observations, tracking how often each memory is retrieved viaSearchandGetObservationGET /promotedHTTP endpoint andmem_promotedMCP tool that surface frequently-recalled observations above a configurable thresholdChanges
internal/store/store.gorecall_count/last_recalled_atto schema, struct, all 13 scan sites, migration. NewincrementRecall()(fire-and-forget),PromotedObservations(), and privategetObservation()(no side effects)internal/store/store_test.gointernal/server/server.goGET /promotedendpoint withproject,scope,min_recalls,limitquery paramsinternal/mcp/mcp.gomem_promotedtool in agent profile (deferred loading), updated tool counts in package docinternal/mcp/mcp_test.goDesign Decisions
Fire-and-forget recall tracking:
incrementRecall()uses_, _ = s.db.Exec(...)— recall tracking must never break reads. Errors are silently ignored.Public vs private GetObservation: Split into
GetObservation()(increments recall, for external callers) andgetObservation()(no side effects, for internal use likeTimeline()). This prevents navigational reads from inflating counts.No sync for recall data:
recall_countis analytics metadata, not content. NoenqueueSyncMutationTxcalls — avoids noise in cross-device sync.Schema migration: Uses existing
addColumnIfNotExistspattern.recall_countdefaults to0for existing rows,last_recalled_atdefaults toNULL. Safe for databases with existing data.Test Plan
TestRecallCountIncrementsOnSearch— verifies Search bumps recall_countTestRecallCountIncrementsOnGetObservation— verifies GetObservation bumps recall_countTestPromotedObservationsReturnsHighRecallOnly— verifies threshold filteringTestPromotedObservationsExcludesDeleted— verifies soft-deleted excluded/promotedreturns correct resultsAutomated Checks
Contributor Checklist
Notes for Reviewers
TestNewErrorBranches/fails_when_migration_encounters_conflicting_object— a pre-existing Windows file-locking issue unrelated to this PRrecall_countis monotonically increasing by design. Staleness filtering (e.g., "not recalled in last 30 days") is a natural follow-up but not in scope for v1mem_promotedtool uses deferred loading to avoid bloating the initial tool context for agents that don't need it