[VEPEAGE-000] Procedural memory: skill schema, session-grounded experience distillation, and tool-message support#136
Open
nanxingw wants to merge 7 commits into
Open
[VEPEAGE-000] Procedural memory: skill schema, session-grounded experience distillation, and tool-message support#136nanxingw wants to merge 7 commits into
nanxingw wants to merge 7 commits into
Conversation
…ion evolution
Blocking/regression fixes:
- migrate_procedural_to_skill.sql: ALTER steps to TEXT before textification
(assigning text to a json column fails at plan time with 42804 and aborted
the whole migration on every existing database); normalize legacy free-form
entry_type values into {workflow, guide, script}
- procedural entry_type validation: lenient normalize on read (to_pydantic
runs validators via from_attributes; legacy rows like 'process' must not
crash reads) while the write path stays strict in tool_validators
- startup schema-drift guard: fail fast with the exact migration scripts to
run when an existing DB is missing ORM columns (create_all never ALTERs);
README gains the upgrade-migration runbook
- Redis procedural index: detect a stale pre-skill index (no
description_embedding attribute), drop index-only and rebuild so skill
search doesn't silently fall back to PostgreSQL forever
Data lifecycle (PII/erasure):
- conversation_message + skill_experience now covered by the user/client
memory purge paths (delete_by_user_id / delete_by_client_id)
- distilled conversation turns are pruned after
MIRIX_CONVERSATION_RETENTION_DAYS (default 30, 0 = keep forever) at the end
of each procedural distillation pass
- skill experiences carry client attribution (created_by_id audit column)
Interface:
- SDK search()/search_all_users(): search_method now defaults to None and is
omitted from the request so the server per-type default (procedural ->
hybrid) applies; docstrings document hybrid; local retrieve_memory mirrors
the same per-type resolution
- restore DELETE /memory/procedural/{memory_id} for parity with the other
memory types (skill writes remain evolution-only, no public write routes)
- drop the unreachable memory-agent tool_calls[:1] truncation and the unused
AgentTriggerStateManager.get_state
Structure/hygiene:
- shared self-evicting KeyedLocks registry replaces three hand-rolled
dict[key, asyncio.Lock] registries (fixes the unbounded raw-memory lock
growth)
- get_server singleton moved to mirix.server.server; services no longer
import the REST module
- shared owner_org + _ingest_session_turns helpers replace duplicated blocks;
roadmap jargon (Goal-2/3, C3/C4) replaced with domain terms
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e endpoints
Codex review findings on the previous fix batch:
- DELETE /users/{user_id}/memories and DELETE /clients/{client_id}/memories
performed an irreversible cross-table hard delete (now including verbatim
conversation transcripts and skill experiences) with NO authentication and
no tenant check — any caller who knew a foreign user_id/client_id could
erase that tenant's data. Both endpoints now resolve the caller via
JWT/Client API Key and 404 on cross-organization targets (same response as
a missing id, so they can't be used as an existence oracle).
- Align the migration SQL entry_type mapping with normalize_entry_type():
any legacy value containing "how" maps to 'guide' in both places, so the
read-time normalizer and the migrated column can never disagree.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ry erasure
Codex re-review of the previous fix:
- DELETE /clients/{client_id}/memories compared the target to None, but
client_manager.get_client_by_id RAISES NoResultFound on a miss (it never
returns None), so a missing client_id fell through to a 500 while a
cross-org client_id returned 404 — a distinguishable response that leaks
client-id existence. Catch the miss and return the same 404 as the
cross-org case, matching the user endpoint.
- test_deletion_apis: pass the test client's API key on the two now-
authenticated /memories DELETE calls.
Not changed (out of scope): the keyless X-Client-ID auth mode is the
platform's documented trusted-network model used by the official SDK
(remote_client sends X-Client-ID when constructed without an api_key) and
by every memory endpoint; tightening it is a platform-wide auth-model
decision, not part of this feature.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…configurable transcript budgets
The distiller prompt has always treated tool errors/retries as strong
learning signals (signal_type: tool_error), but the ingestion seam dropped
every non-user/assistant message — the distiller never saw the work process
it was designed to learn from. Close that design/implementation gap:
- conversation_message role space: 'user' | 'assistant' | 'tool' (Literal
change only; the ORM column is a plain String, no DB migration needed)
- _extract_conversation_turns keeps tool results (role 'tool'/'function',
name-prefixed) and serializes assistant tool_calls (OpenAI shape) into the
turn content; a pure tool-call assistant turn with empty content still
yields a turn; [{"type":"text","text":...}] parts flatten to clean text
- role-collapse branches label tool/function messages [TOOL] instead of
mislabeling them [ASSISTANT] for the meta-agent path
- transcript budgets are env-configurable and sized for tool-heavy sessions:
MIRIX_DISTILLER_MAX_TRANSCRIPT_CHARS (default 80000, was hardcoded 16000)
and MIRIX_DISTILLER_MAX_MESSAGE_CHARS (default 8000, was 2000)
- remove the orphaned round-based session_distiller.txt prompt (zero
references; the production distiller uses auto_dream_agent/procedural.txt)
- README + SDK docstring document how to pass tool activity
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nd malformed input Two failure modes found while self-reviewing the tool-activity change: - extraction ran OUTSIDE the additive try/except in _ingest_session_turns, so a malformed message shape could abort the primary memory add; it now runs inside the guard (degrades to a missed session, never a dropped add) - one oversized turn (typically a huge tool result) failed the whole batch's Pydantic validation in record_turns, silently dropping every turn of the request; extraction now truncates at the store's per-row content cap with a visible marker Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fix collisions Two defects in migrate_procedural_to_skill.sql, both reproduced end-to-end against real Postgres 15/16 before the fix: 1. Step 2 slugification aborted the WHOLE migration on any row whose summary slugs to '' (whitespace-only, punctuation-only): NULLIF assigned NULL into the NOT NULL name column before the fallback UPDATE could run — NOT NULL is checked per row at assignment time, so the two-statement "set NULL, then patch" shape can never work. Slug and fallback are now one COALESCE in a single statement. 2. Step 2b's single-pass -N suffixing could mint a name that collides with a PRE-EXISTING slug (two 'task' rows produce 'task-2' while a 'task 2' summary already slugged to 'task-2'), failing the final UNIQUE constraint. The dedup now loops until no row needs a rename; terminates because renamed names strictly lengthen each pass. Verified against postgres:16-alpine with adversarial legacy data (whitespace/ punctuation/NULL summaries, 3-way slug collisions crossing a pre-existing suffixed name, legacy entry_type variants, >60-char summaries, cross-org isolation) and on an empty table: 13/13 assertions pass. Also close the two test gaps flagged in the ship-readiness review: - _ingest_session_turns isolation guard rails (store failure, extraction failure, no-session_id skip, session stamping + turn recording) — pins the "additive write must never abort the primary memory add" contract. - Distiller _render_transcript budgets: per-turn cap marker, whole-transcript head/tail elision, and the untouched-within-budget case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
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
2 participants
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
Rework procedural memory into a skill-based schema with session-grounded experience distillation, and make tool activity a first-class learning signal end-to-end.
procedural_memorymoves from flatsummary/stepstoname/description/instructions/triggers/examples/versionwith per-user unique skill names (DB-levelUNIQUE (organization_id, user_id, name))./memory/addand/memory/add_syncpersist real per-role turns (user/assistant/tool) into a dedicated Conversation Message Store keyed bysession_id— the single source of truth for skill distillation. The write is additive: a store failure degrades to a missed session, never a failed memory-add.worth_learning/worth_avoidingskill experiences (importance × credibility prioritization), which evolve skills through the auto-dream procedural flow (POST /memory/auto_dream,mode: "procedural").role: "tool"/"function"turns are stored (with[tool_name]prefixes), assistanttool_calls(OpenAI shape) are serialized into turn content, and the meta-agent role-collapse marks them[TOOL]— the distiller prompt'stool_errorsignal now actually receives tool turns instead of silently never matching.MIRIX_CONVERSATION_RETENTION_DAYS, erasure paths wired).scripts/migrate_procedural_to_skill.sqlverified end-to-end on real Postgres against adversarial legacy data; startup fail-fast (_find_missing_columns) names the migration script when a legacy DB is detected; stale pre-skill Redis index is dropped and rebuilt.Upgrade notes (existing deployments)
scripts/migrate_procedural_to_skill.sqlonce before deploying (the server fail-fasts on an un-migrated schema and names the script). New installs need nothing —create_allbuilds the final schema.migrate_add_conversation_message.sql(+_phase2),migrate_add_message_session_id.sql(+_phase2),migrate_add_skill_experience.sql,migrate_add_agent_trigger_state.sql. The_phase2files useCREATE INDEX CONCURRENTLYand must run outside a transaction, after their phase-1 counterparts.MIRIX_DISTILLER_MAX_TRANSCRIPT_CHARS(default 80000),MIRIX_DISTILLER_MAX_MESSAGE_CHARS(default 8000),MIRIX_CONVERSATION_RETENTION_DAYS(default 30; 0 = keep forever).Verification
pytest tests -q: 682 passed, 56 skipped (146 deselected = integration tests requiring live Postgres; recommend running them in CI:pytest -m integration).postgres:16-alpinewith adversarial legacy data — whitespace/punctuation/NULL summaries, three-way slug collisions crossing a pre-existing suffixed name, legacyentry_typevariants, >60-char summaries, cross-org isolation, and an empty table — 13/13 assertions pass.npm run build✓;git diff --check✓; ruff clean on changed files.Known follow-ups (deliberately out of scope)
X-Client-IDkeyless auth: the platform's existing keyless mode trusts this header on all endpoints; tightening it is a platform-level auth-model change and belongs in its own PR.distill_sessionsfans out one LLM call per session without a concurrency cap (bounded bylast_n_sessions, default 5).🤖 Generated with Claude Code