feat: Replace Weaviate knowledge base with unified memory system - #556
feat: Replace Weaviate knowledge base with unified memory system#556OlivierTrudeau wants to merge 72 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR replaces knowledge-base, incident-feedback, and Weaviate-backed paths with memory-backed API routes, tools, prompts, and UI surfaces. It also updates database, Celery, and deployment wiring to use memory artifacts and remove the old feedback and Weaviate registrations. ChangesMemory cutover
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
* Remove t2v-transformers container The embedding service was only used by the correlation engine's SimilarityStrategy, which already had a Jaccard fallback. This removes the container entirely and simplifies the strategy to use Jaccard token similarity directly, reducing resource usage and deployment complexity. Co-authored-by: Cursor <cursoragent@cursor.com> * Add optional EMBEDDING_MODEL support for alert correlation Replaces the removed t2v-transformers container with an optional, provider-agnostic embedding client. Users can set EMBEDDING_MODEL (e.g., openai/text-embedding-3-small) to enable semantic similarity; otherwise the system falls back to Jaccard token similarity. Supported providers: openai, google, bedrock — reuses existing API keys already configured for the LLM. Co-authored-by: Cursor <cursoragent@cursor.com> * mini --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…#578) * refactor: eliminate postmortem tables in favor of artifacts Postmortems are now stored as artifacts with category='postmortem' and an incident_id FK. The old postmortems/postmortem_versions tables still exist (data preserved) but are no longer read or written by any code path. - Add incident_id + generation_session_id columns to artifacts table - Rewrite postmortem_tool.py to read/write artifacts directly - Rewrite all 10 postmortem_routes.py endpoints against artifacts + artifact_versions (same API contract, frontend unchanged) - Rewrite postmortem_action.py reservation logic to insert into artifacts - Update Notion export to read from artifacts, drop legacy UPDATE - Update introspection_tools list_postmortems to query artifacts - Remove postmortem section from memory migration_task (now at startup) - Drop FK constraint on postmortem_exports so it accepts artifact UUIDs - Startup migration backfills existing postmortem data into artifacts Co-authored-by: Cursor <cursoragent@cursor.com> * fix * fix for managed sql * adress ben comments --------- Co-authored-by: Cursor <cursoragent@cursor.com>
… prompt (#583) * feat: add memory injector — async prefetch injects relevant memories into prompt Single-pass LLM selector picks up to 5 memories from the index based on title + description, reads their full content, and injects into the system prompt. Runs as a non-blocking prefetch in parallel with prompt building. - Fires early in agentic_tool_flow before prompt segments are built - Consumed in composer.py alongside the static memory index - Session-level dedup and 60KB cumulative budget via Redis - Staleness headers on old memories - 4KB per-file cap with truncation note Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: extract memory data-access helpers into queries.py Move get_memory_entries and fetch_memory_content out of index_builder.py into a dedicated queries module. index_builder now only handles index formatting, and injector imports directly from queries. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: use centralized RCA_MODEL and reorder class before function Use ModelConfig.RCA_MODEL instead of a one-off env var for the memory selector. Move MemoryPrefetch class above start_memory_prefetch so the forward reference is no longer needed. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: inline prefetch pipeline into MemoryPrefetch._execute Remove the _run_prefetch wrapper function and put the pipeline logic directly in the class method that runs it. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: simplify memory injector — remove budget tracking, wrapper, and min-length guard - Remove session byte budget (redundant with context trim middleware) - Remove start_memory_prefetch wrapper (caller uses MemoryPrefetch directly) - Remove MIN_QUERY_LENGTH gate (short queries like "db down" are valid) - Remove redundant MAX_INDEX_ENTRIES constant (default lives in queries.py) Co-authored-by: Cursor <cursoragent@cursor.com> * fix memory injecter * j --------- Co-authored-by: Cursor <cursoragent@cursor.com>
![arvo-ai-staging[bot]](https://avatars.githubusercontent.com/in/4088260?s=60&v=4){kind=small}
There was a problem hiding this comment.
Aurora Risk Review — Latest changes
Verdict: RISKY
Reviewed the latest changes. The new MemoryPrefetch system introduces a synchronous blocking call (Future.result(timeout=6.0)) inside the async agentic_tool_flow coroutine, which will stall the asyncio event loop for up to 6 seconds on every chat turn for every user who has memory entries. Additionally, the module-level ThreadPoolExecutor(max_workers=4) is a process-wide singleton shared across all concurrent users; under modest concurrency the pool saturates and the blocking wait compounds, degrading all chat responses simultaneously.
Findings
| # | Severity | File | Finding |
|---|---|---|---|
| 1 | HIGH | server/chat/backend/agent/agent.py:380 |
Blocking Future.result() called inside async coroutine — stalls event loop up to 6s per chat turn |
| 2 | MEDIUM | server/services/memory/injector.py:39 |
Process-wide ThreadPoolExecutor(max_workers=4) saturates under concurrent chat load, compounding the event-loop block |
Aurora reviews PRs for incident prevention. This is advisory only and does not block merge.
* feat: add memory collector and dream consolidation Collector: Celery task that fires after each turn, reads the conversation, and saves durable learnings to org memory via LLM extraction. Consolidation (Dream): Nightly Celery Beat task that reviews all org memories, merges duplicates, removes stale entries, and keeps the memory bank clean and well-organized. - Collector hooked into workflow.py (interactive) and task.py (RCA) - Consolidation scheduled via celery beat (default: every 24h) - Redis-based distributed lock prevents parallel consolidations per org - Both tasks registered in celery_config.py includes + beat_schedule Co-authored-by: Cursor <cursoragent@cursor.com> * update collector * udpates * latest * change consolidation to make it an action * change prompt a bit * mo * jj * update * comments --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Aurora Risk Review — Latest changes
Verdict: RISKY
Reviewed the latest changes. The backfill value change from 'artifact' to 'context' introduces a category mismatch with the hardcoded INSERT in store.py, which still writes category='artifact' on every upsert. After this migration runs, pre-existing artifacts are stamped 'context' in the DB but the upsert conflict key targets 'artifact', so every subsequent write to an existing artifact silently creates a duplicate row instead of updating it. The DEFAULT column change is correct and safe; the backfill value change is the risk.
Findings
| # | Severity | File | Finding |
|---|---|---|---|
| 1 | HIGH | server/utils/db/db_utils.py:3018 |
Backfill value 'context' conflicts with hardcoded category='artifact' in store.py upsert |
Aurora reviews PRs for incident prevention. This is advisory only and does not block merge.
| """) | ||
| # Back-fill any rows missing a category | ||
| cursor.execute(""" | ||
| UPDATE artifacts SET category = 'context' WHERE category IS NULL OR category = ''; |
There was a problem hiding this comment.
[HIGH] Backfill value 'context' conflicts with hardcoded category='artifact' in store.py upsert
This migration stamps all NULL/empty category rows as 'context', but server/services/artifacts/store.py:upsert_artifact_by_title always inserts with category='artifact' and relies on ON CONFLICT (org_id, category, title) for idempotency. After deploy, any artifact that existed before this migration will have category='context' in the DB, so the conflict clause (keyed on 'artifact') will never fire — every agent or UI write to a pre-existing artifact will INSERT a new duplicate row under category='artifact' instead of updating the original. This silently corrupts artifact state for all existing orgs on upgrade, breaking the write_artifact / write_memory tools and causing the memory system to accumulate phantom duplicates.
There was a problem hiding this comment.
Aurora Risk Review — Latest changes
Verdict: RISKY
Reviewed the latest changes. The fix correctly identifies that FORCE ROW LEVEL SECURITY blocks the original bulk UPDATE, but the seed query that discovers which orgs need backfilling (SELECT DISTINCT org_id FROM artifacts WHERE ...) runs before any myapp.current_org_id is set, so it will itself be filtered to zero rows by the same RLS policy it is trying to work around. The loop never executes, backfill_orgs is empty, and the subsequent ALTER COLUMN SET NOT NULL will still raise a constraint violation on any production instance that has NULL-category artifact rows — crashing initialize_tables() on startup.
Findings
| # | Severity | File | Finding |
|---|---|---|---|
| 1 | HIGH | server/utils/db/db_utils.py:3018 |
Seed SELECT for backfill loop is itself blocked by RLS — loop never runs, NOT NULL migration still fails |
Aurora reviews PRs for incident prevention. This is advisory only and does not block merge.
| """) | ||
| # Backfill per-org to satisfy RLS (FORCE is already active) | ||
| cursor.execute("SELECT DISTINCT org_id FROM artifacts WHERE category IS NULL OR category = ''") | ||
| backfill_orgs = [row[0] for row in cursor.fetchall()] |
There was a problem hiding this comment.
[HIGH] Seed SELECT for backfill loop is itself blocked by RLS — loop never runs, NOT NULL migration still fails
The new code does SELECT DISTINCT org_id FROM artifacts WHERE category IS NULL OR category = '' with no myapp.current_org_id set in the session. Because FORCE ROW LEVEL SECURITY is active on artifacts and the RLS policy gates visibility on current_setting('myapp.current_org_id'), this query returns zero rows regardless of actual table contents. backfill_orgs is always empty, the per-org UPDATE loop never fires, and the immediately-following ALTER TABLE artifacts ALTER COLUMN category SET NOT NULL will raise ERROR: column "category" of relation "artifacts" contains null values on any production database that has legacy NULL-category rows — causing server startup to abort. The fix requires either (a) executing the seed SELECT via a BYPASSRLS role/connection, (b) using a raw SET LOCAL myapp.current_org_id = '' bypass sentinel that the RLS policy explicitly allows for admin operations, or (c) restructuring the migration to run before RLS FORCE is applied to the table.
|
3 participants
Description
Replaces the Weaviate-based Knowledge Base with a unified Memory system stored in PostgreSQL. Org knowledge (context, runbooks, infrastructure, learned patterns, postmortems) lives in the
artifactstable with categories, versioning, and a table-of-contents index injected into the agent system prompt. The agent reads memory on demand via tools instead of vector search. Existing KB data is migrated automatically on upgrade via a one-time Celery task. Weaviate is removed from the Docker Compose dev stack.What changed
artifactstable with newcategoryanddescriptioncolumns; unique index on(org_id, category, title)context,runbook,infrastructure,learned,postmortem(legacy rows backfilled asartifact)/api/memory— list, create, get, delete entries; file upload for.md,.txt,.pdfmemory:readfor viewers,memory:writefor editorslist_memories,read_memory,write_memory,append_to_memory,edit_memory,grep_memoriesbuild_memory_index) injected into system prompt so the agent knows what exists and callsread_memory()on demandmemory.mdskill — shared behavioral contract for foreground chat and background RCAoffset/limitparams onread_memorypypdfbefore storageservices/artifacts/store.pypostmortemstable intoartifactswithcategory='postmortem'migrate_kb_to_memory) — auto-triggered on startup if old KB data exists and nothing migrated yetknowledge_base_memory→context("Org Context")infrastructure_context→infrastructure("Infrastructure Context")knowledge_base_documents(S3) →runbookentries (PDF/text extracted)postmortems→postmortementries (titled from incident alert_title)list_memoriesandread_memory;search_runbooksnow queries memory runbookswrite_memoryinstead of old KB toolsweaviate_clientdependency, and health check from Docker Composeknowledge_base_search,rag_indexer,infra_context_tool,save_discovery_findingknowledge_base.cleanup_stale_documentsCelery periodic taskchat_with_aurora,ask_incident,regenerate_rca)What to test
artifactstable hascategory/descriptioncolumns and unique index[Migration] Complete; restart does not duplicate entries.md,.txt, and.pdffiles — content stored correctly, PDF text extracted with page markersgrep_memories,append_to_memory, andedit_memorywork as expectededit_memoryrejects ambiguous matches (multiple occurrences of old_text)read_memorypaginates large entries correctly via offset/limitinfrastructure/Infrastructure Contextentrylist_memories,read_memory, andsearch_runbooksreturn expected results/healthpasses without Weaviate(category, title)upserts instead of duplicating