feat: agent memory & institutional knowledge base - #47
Open
joshuajerin wants to merge 2 commits into
Open
Conversation
Migration, service, router, API client, and Zustand store for: - Agent memories with vector embeddings, decay, and similarity search - Workspace knowledge base with category/key/value entries and verification
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 38 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds agent memory and workspace knowledge-base features by introducing backend API endpoints, async service layer functions, and Supabase database tables with vector search capability. Frontend TypeScript interfaces, API client functions, and a Zustand store enable client-side state management and API interactions. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as Client
participant Router as API Router
participant Service as Service Layer
participant OpenAI as OpenAI API
participant Supabase as Supabase DB
Client->>Router: POST /agents/{id}/memories (store_memory_endpoint)
activate Router
Router->>Service: store_memory(agent_id, memory_type, content, ...)
activate Service
Service->>OpenAI: Generate embedding for content
activate OpenAI
OpenAI-->>Service: embedding vector (1536D)
deactivate OpenAI
Service->>Supabase: INSERT into agent_memories (content, embedding, ...)
activate Supabase
Supabase-->>Service: {id, created_at, ...}
deactivate Supabase
Service-->>Router: MemoryResponse
deactivate Service
Router-->>Client: 200 OK {memory}
deactivate Router
Client->>Router: POST /agents/{id}/memories/search (search_memories_endpoint)
activate Router
Router->>Service: search_memories(agent_id, query, ...)
activate Service
Service->>OpenAI: Generate embedding for query
activate OpenAI
OpenAI-->>Service: embedding vector (1536D)
deactivate OpenAI
Service->>Supabase: CALL search_agent_memories(embedding, ...)
activate Supabase
rect rgba(100, 150, 200, 0.5)
Note over Supabase: Vector similarity search<br/>(ivfflat cosine index)
end
Supabase-->>Service: [{id, content, similarity, ...}, ...]
deactivate Supabase
Service-->>Router: MemorySearchResponse
deactivate Service
Router-->>Client: 200 OK {results, count}
deactivate Router
Client->>Router: POST /knowledge/{id}/verify (verify_knowledge_endpoint)
activate Router
Router->>Service: verify_knowledge(knowledge_id, verified_by)
activate Service
Service->>Supabase: UPDATE agent_knowledge_base (verified_by, verified_at)
activate Supabase
Supabase-->>Service: {id, verified_by, verified_at, ...}
deactivate Supabase
Service-->>Router: KnowledgeResponse
deactivate Service
Router-->>Client: 200 OK {knowledge}
deactivate Router
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
core-api/supabase/migrations/20260409000003_agent_memory.sql (2)
120-140: Consider adding an index to optimize thedecay_agent_memoriesfunction.The
decay_agent_memoriesfunction performs a full table scan filtering onlast_accessed_at < NOW() - INTERVAL '7 days'andexpired_at IS NULL. For large datasets, this could be slow.Consider adding a partial index to optimize decay queries:
📊 Proposed index for decay optimization
-- Add after existing indexes (around line 252) CREATE INDEX "idx_agent_memories_decay_candidates" ON "public"."agent_memories" USING "btree" ("last_accessed_at") WHERE "expired_at" IS NULL AND "relevance_score" > 0.01;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core-api/supabase/migrations/20260409000003_agent_memory.sql` around lines 120 - 140, Add a partial btree index to speed the decay_agent_memories() updates by indexing agent_memories.last_accessed_at for only non-expired, still-relevant rows; create an index named idx_agent_memories_decay_candidates on agent_memories(last_accessed_at) with a WHERE clause restricting to expired_at IS NULL AND relevance_score > 0.01 and add it alongside the other agent_memories indexes so the UPDATEs in decay_agent_memories() can use the index instead of a full table scan.
246-246: IVFFlat indexes perform poorly on empty tables; consider HNSW or add post-population rebuild documentation.IVFFlat requires bulk-loaded data for effective clustering. When created on empty tables before data exists, the index won't provide accurate similarity search until rebuilt after data is populated. For these migration indexes:
Switch to HNSW if data is populated incrementally (recommended for dynamic workloads):
CREATE INDEX "idx_agent_memories_embedding" ON "public"."agent_memories" USING "hnsw" ("embedding" vector_cosine_ops);Or document that
REINDEXmust be executed after initial data load, and periodically as the dataset grows.Also applies to line 259 (agent_knowledge_base_embedding).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core-api/supabase/migrations/20260409000003_agent_memory.sql` at line 246, The IVFFlat index on "idx_agent_memories_embedding" (table agent_memories) is suboptimal when created on empty or incrementally populated tables; either change the index to HNSW by replacing the USING "ivfflat" clause with USING "hnsw" for idx_agent_memories_embedding (and similarly for idx_agent_knowledge_base_embedding on agent_knowledge_base) or add a migration-note that documents running REINDEX/REBUILD after initial bulk load and periodically as the dataset grows; update the migration SQL or accompanying migration README to reflect the chosen approach so similarity searches are accurate post-population.core-api/api/routers/agent_memory.py (1)
208-210: All exceptions return HTTP 400, masking server-side errors.The catch-all
except Exceptionblocks uniformly returnstatus_code=400. This masks infrastructure failures (DB down, timeout, etc.) as client errors, making debugging harder and violating REST semantics where 5xx indicates server issues.Consider distinguishing error types:
🔧 Suggested pattern for better error classification
+from fastapi import status + `@router.post`("/agents/{agent_id}/memories", response_model=MemoryResponse) async def store_memory_endpoint(...): try: memory = await store_memory(...) return memory + except ValueError as e: + # Client-side validation errors + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) except Exception as e: logger.error(f"Error storing memory: {e}") - raise HTTPException(status_code=400, detail=str(e)) + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Internal server error")Also applies to: 224-226, 246-248, 266-268, 280-282, 308-310, 324-326, 345-347, 360-362, 374-376
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core-api/api/routers/agent_memory.py` around lines 208 - 210, The current catch-all except Exception blocks (e.g., the one logging "Error storing memory") always return HTTP 400; change them to classify errors: re-raise existing HTTPException unchanged, map validation/argument errors (ValueError, TypeError) to HTTP 400 (or 422), map not-found semantics to 404 if you have a NotFound/KeyError, and for any other unexpected exceptions log full traceback via logger.exception(...) and raise HTTPException(status_code=500, detail="Internal server error"). Update all similar handlers (the shown block and the other except blocks referenced) so client errors return 4xx and server/infrastructure failures return 5xx, and ensure logs include traceback for diagnostics.core-web/src/stores/agentMemoryStore.ts (1)
117-122: Mutation actions lack error handling unlike fetch/search actions.The
addMemory,editMemory,removeMemory,addKnowledge,verifyKnowledge, andremoveKnowledgeactions don't wrap API calls in try/catch blocks, unlike the fetch and search actions. If the API call fails after optimistically updating state (or vice versa), the UI state could become inconsistent.Currently, errors propagate to the caller which may be intentional. However, for consistency and to prevent potential state desync, consider adding try/catch with rollback logic or documenting that callers must handle errors.
💡 Example: Adding error handling with rollback for removeMemory
removeMemory: async (memoryId) => { + const { memories } = get(); + // Optimistic update + set({ memories: memories.filter((m) => m.id !== memoryId) }); + try { await deleteAgentMemory(memoryId); - const { memories } = get(); - set({ memories: memories.filter((m) => m.id !== memoryId) }); + } catch (err) { + // Rollback on failure + set({ memories }); + throw err; + } },Also applies to: 136-148, 176-181, 195-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core-web/src/stores/agentMemoryStore.ts` around lines 117 - 122, The mutation actions (addMemory, editMemory, removeMemory, addKnowledge, verifyKnowledge, removeKnowledge) currently call their respective API helpers (storeAgentMemory, updateAgentMemory, deleteAgentMemory, storeAgentKnowledge, verifyAgentKnowledge, deleteAgentKnowledge) without try/catch or rollback; wrap each API call in a try/catch, perform the optimistic state update only after successful API response or revert state on error (e.g., restore previous memories/knowledge array or trigger a fresh fetch), log the error via the same logger used elsewhere, and either re-throw or return a standardized error so callers can handle it—apply this pattern in the functions addMemory, editMemory, removeMemory, addKnowledge, verifyKnowledge, and removeKnowledge.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core-api/api/routers/agent_memory.py`:
- Line 17: The import access_memory is unused in the agent memory router; remove
access_memory from the import list in the agent_memory module (or alternatively
implement and register an endpoint that calls access_memory such as a GET/POST
handler) so there are no dead imports; update the import statement that
currently lists access_memory and run linters/tests to confirm the unused-import
warning is resolved.
In `@core-api/api/services/agents/memory.py`:
- Around line 175-201: The access_memory function is imported in agent_memory.py
but never used; add a simple POST endpoint to call it: create a route POST
/memories/{memory_id}/access in the agent_memory.py router that extracts
memory_id from the path and the user's JWT from the request/auth context, calls
access_memory(memory_id, user_jwt), returns the updated memory object (or
appropriate 404/500 on error), and ensures proper auth/permission checks mirror
other agents endpoints; alternatively, if you prefer integrating into search
flow, call access_memory(memory_id, user_jwt) for each returned memory in
search_memories before returning results—update imports accordingly and handle
errors gracefully.
- Around line 316-324: In verify_knowledge, the update is assigning the literal
string "now()" to verified_at so the DB will store that string; change the
update to set verified_at to a real Python datetime (e.g.,
datetime.datetime.utcnow() or timezone-aware now) before calling
supabase.table("agent_knowledge_base").update(...). Use the existing variables
verified_by and knowledge_id and pass the datetime object in the update payload
for the "verified_at" key so the Supabase client stores a proper timestamp
rather than the string "now()".
- Around line 189-197: The update is passing the literal string "now()" so
last_accessed_at will store that text; change the update to set last_accessed_at
to a Python datetime (e.g., utcnow() or timezone-aware datetime) converted to an
ISO/UTC string or a Python datetime object acceptable by Supabase/Postgres,
e.g., import datetime and use datetime.datetime.utcnow().isoformat() (or
timezone-aware equivalent) in the dictionary passed to
supabase.table("agent_memories").update(...) so last_accessed_at receives an
actual timestamp rather than the string "now()".
In `@core-api/supabase/migrations/20260409000003_agent_memory.sql`:
- Around line 54-56: The functions search_agent_memories and
search_knowledge_base are defined with SECURITY DEFINER and the migration grants
ALL to anon, which allows unauthenticated RPCs to bypass RLS; remove the GRANT
ALL to anon for these functions and either (a) change the functions from
SECURITY DEFINER to SECURITY INVOKER or (b) add explicit authorization checks
inside the functions (e.g., validate current_setting('jwt.claims.workspace_id')
or session_user against the workspace/agent owner) before returning results so
unauthenticated callers cannot access other workspaces; update the SQL that
declares LANGUAGE "plpgsql" SECURITY DEFINER and the corresponding GRANT
statements for these functions accordingly.
---
Nitpick comments:
In `@core-api/api/routers/agent_memory.py`:
- Around line 208-210: The current catch-all except Exception blocks (e.g., the
one logging "Error storing memory") always return HTTP 400; change them to
classify errors: re-raise existing HTTPException unchanged, map
validation/argument errors (ValueError, TypeError) to HTTP 400 (or 422), map
not-found semantics to 404 if you have a NotFound/KeyError, and for any other
unexpected exceptions log full traceback via logger.exception(...) and raise
HTTPException(status_code=500, detail="Internal server error"). Update all
similar handlers (the shown block and the other except blocks referenced) so
client errors return 4xx and server/infrastructure failures return 5xx, and
ensure logs include traceback for diagnostics.
In `@core-api/supabase/migrations/20260409000003_agent_memory.sql`:
- Around line 120-140: Add a partial btree index to speed the
decay_agent_memories() updates by indexing agent_memories.last_accessed_at for
only non-expired, still-relevant rows; create an index named
idx_agent_memories_decay_candidates on agent_memories(last_accessed_at) with a
WHERE clause restricting to expired_at IS NULL AND relevance_score > 0.01 and
add it alongside the other agent_memories indexes so the UPDATEs in
decay_agent_memories() can use the index instead of a full table scan.
- Line 246: The IVFFlat index on "idx_agent_memories_embedding" (table
agent_memories) is suboptimal when created on empty or incrementally populated
tables; either change the index to HNSW by replacing the USING "ivfflat" clause
with USING "hnsw" for idx_agent_memories_embedding (and similarly for
idx_agent_knowledge_base_embedding on agent_knowledge_base) or add a
migration-note that documents running REINDEX/REBUILD after initial bulk load
and periodically as the dataset grows; update the migration SQL or accompanying
migration README to reflect the chosen approach so similarity searches are
accurate post-population.
In `@core-web/src/stores/agentMemoryStore.ts`:
- Around line 117-122: The mutation actions (addMemory, editMemory,
removeMemory, addKnowledge, verifyKnowledge, removeKnowledge) currently call
their respective API helpers (storeAgentMemory, updateAgentMemory,
deleteAgentMemory, storeAgentKnowledge, verifyAgentKnowledge,
deleteAgentKnowledge) without try/catch or rollback; wrap each API call in a
try/catch, perform the optimistic state update only after successful API
response or revert state on error (e.g., restore previous memories/knowledge
array or trigger a fresh fetch), log the error via the same logger used
elsewhere, and either re-throw or return a standardized error so callers can
handle it—apply this pattern in the functions addMemory, editMemory,
removeMemory, addKnowledge, verifyKnowledge, and removeKnowledge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc21ae3f-1ac3-4941-bdff-4b19832a0950
📒 Files selected for processing (6)
core-api/api/routers/agent_memory.pycore-api/api/services/agents/memory.pycore-api/index.pycore-api/supabase/migrations/20260409000003_agent_memory.sqlcore-web/src/api/client.tscore-web/src/stores/agentMemoryStore.ts
- Fix now() string bug: use datetime.now(timezone.utc).isoformat()
- Remove anon grants from SECURITY DEFINER search functions
- Add access_memory endpoint (POST /memories/{id}/access)
- Switch ivfflat to hnsw indexes for empty-table compatibility
- Add decay_candidates partial index for cron performance
- Improve error classification: ValueError→400, Exception→500
- Add try/catch + rollback to frontend store mutations
- Remove unused imports (ruff clean)
|
Contributor
Author
|
@hackertron since yall are going the agentic route, thought ill push something that u can use in that system |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
search_agent_memories()RPCsearch_knowledge_base()RPC, confidence scoring, and human verification flowFiles
core-api/supabase/migrations/20260409000003_agent_memory.sqlcore-api/api/services/agents/memory.pycore-api/api/routers/agent_memory.pycore-api/index.pycore-web/src/api/client.tscore-web/src/stores/agentMemoryStore.tsBranch 3 of the Agent Infra Plan
Test plan
Summary by CodeRabbit
New Features