CL-5865/5866/5872/5873: Resident memory distillation foundation - #31
Conversation
TheGreatAxios
left a comment
TheGreatAxios
left a comment
There was a problem hiding this comment.
GaaSbot
CTO take: this is the right foundation slice for Resident Memory Distillation Levels 1–2. Ship the schema/temporal/share pieces; fix demote dense restore before calling transform "done."
Ship
- Claim-bearing + enum hard-cut: unblocks derived claims without putting inference in core. Correct priority.
- Temporal classes + generation-filtered timeline: cheap, high leverage for ranking and for not leaking staged rows into live list/search.
- Share grants via optional
WritableGrantStore: composable, host-owned, no mini-ACL. Good.
Fix before merge (or drop demote from the claim)
- Demote that does not restore the prior active embed model is a half-built rollback. Either finish it or do not advertise demote as rollback in the PR/ticket closeout.
- Add one focused test that staged ensure does not steal live active, and that promote/demote generation swap behaves. Unit tests on the registry alone do not cover the product path.
Follow-ups (do not block if demote is fixed or scoped)
- HTTP admin surface for transform — fine deferred; plane methods are enough for the distiller host.
MemoryAddResultshould growversionIdwhen share audit needs it.- Promote should refuse
status !== 'completed'unless product wants partial promote. - Squash commit subjects before merge (
feat:/ ticket noise).
API surface
Optional transform methods on Memory + 501 when no engine store is acceptable. Exporting transform helpers from the package root is fine for the distiller package as a consumer. Do not grow HTTP routes in this PR.
Verdict: request changes on demote dense restore (or explicitly demote demote). Rest is solid foundation.
TheGreatAxios
left a comment
TheGreatAxios
left a comment
There was a problem hiding this comment.
Bruckheimer
Plain read: does this foundation serve the company-brain / resident distiller hook?
Audience / hook / win
- Audience: Interchange hosts that will run a resident distiller over memory, not end users of this package.
- Hook: distiller can write claim-bearing versions with provenance, re-run derivation under a staged generation without poisoning live search, and share documents so peers actually see them.
- Win for this PR: those three capabilities exist on the library plane without standing up a service.
What lands
Claim schema, temporal ranking, staged transform APIs, and real peer grants — that is the right v1 foundation. Scope is not bloated with the distiller workflow itself (CL-5869) or the capture feed (CL-5868). Good cut.
What still blocks the hook
- Re-distill safely is incomplete if demote does not put live dense search back where it was. "Promote and hope" is not a recovery story operators will trust.
- Distiller still cannot ship without CL-5868 (exactly-once feed) and CL-5869 (workflow body). This PR is foundation only — say that in the merge note so nobody thinks the company brain is live.
- Share works only when the host implements
WritableGrantStore+appendAccessTags. Document that as a host checklist item or the "share" demo looks broken.
Scope call
Keep this PR as Levels 1–2. Do not pull the distiller into it. Fix demote (or drop demote from the sold story), merge, then feed + workflow.
TheGreatAxios
left a comment
TheGreatAxios
left a comment
There was a problem hiding this comment.
Critique
This branch lands claim-bearing schema, temporal ranking, staged transform/replay with ensure-vs-activate embed isolation, and share grant materialization. bun run typecheck clean; bun run test 358 pass.
High
-
Demote does not restore the prior dense model.
promoteGenerationactivates the staged embed model (src/services/transform.ts:561) butdemoteGenerationexplicitly leaves the active model alone (src/services/transform.ts:628-631). After demote, live versions sit in the pre-promote embed table while live dense search still resolves the promoted model — empty/degraded dense channel. Rollback acceptance for CL-5872 is not met. -
No automated test for promote/demote or the embed-flip regression end-to-end. Registry unit tests cover ensure vs activate (
embed-model-registry.test.ts), but nothing asserts: staged run under model B does not flip live active; promote swaps generation and activates; demote restores both generation and dense table. The ticket regression test for the flip bug is missing.
Medium
-
sourceVersionIdis the document id, not a version id (src/memory.ts:720). Audit payload on share grants claims a version and stores the document id. PlaneMemoryAddResultonly returns{ documentId }, so this is an API gap as much as a typo — provenance is wrong as shipped. -
promoteGenerationallows non-completed runs — only blocksrunningand already-promoted (transform.ts:511-519). Afailedpartial run can be promoted into live. -
Embed activation is outside the promote transaction (
transform.ts:559-561). IfactivateEmbedModelfails after the generation swap commits, live corpus and active dense model diverge with no automatic rollback.
Low
- Share path warns and continues when
appendAccessTagsis missing (memory.ts:707-714) while still materializing grants onmemory.doc:*— grants exist but the document may lack the matching tag, so peers still fail-closed. Fail-soft is intentional; the warn is easy to miss in production.
TheGreatAxios
left a comment
TheGreatAxios
left a comment
There was a problem hiding this comment.
Greybeard
Architecture review of the distillation foundation against package invariants and the staged-replay design.
Invariants
- Authenticate nothing: hold. Share path still uses host GrantStore / WritableGrantStore; no API keys or sessions introduced.
- One knowledge DB: hold. Migrations stay in knowledge.*; no control-plane FKs.
- Never embed in-process: hold. Still HTTP embed/rerank clients; registry only provisions tables.
- arktype at edges: hold for claim-bearing schemas and enum lockstep tests.
Design
- ensure vs activate is the right cut for staged replay. Live capture activates; deriveFromRawCapture passes promoteActive: false. Dense search for non-live generations uses resolveEmbedTableByModelKey — good.
- Share materialization correctly avoids reintroducing a document mini-ACL: tags + host grants, document-scoped memory.doc:*, write-narrow-then-widen helpers for later widen. MEMORY_SHARE_CONDITION_REGISTRY merge in resolveGrantConfig is necessary given @intx/authz skips conditioned grants without a registry.
- Docs: IMPLEMENTATION.md and AUTHZ-DOCUMENT-ACCESS.md largely match the code for embed isolation and share materialize. Demote's "does not re-activate prior embed" note in code is honest but undercuts the product rollback story documented for CL-5872.
Commit messages
Subjects use feat: prefixes and Linear ticket IDs in bodies. Project style prefers plain-English subjects without conventional-commit or ticket prefixes; rewrite before squash-merge if that bar is still enforced.
Blockers (architecture)
Demote without embed restore is not a complete generation cutover design. Either demote re-activates the archived live model (or records/restores model_key on the run), or docs must demote rollback of dense search to a separate operator step — and acceptance tests must say so.
TheGreatAxios
left a comment
TheGreatAxios
left a comment
There was a problem hiding this comment.
Primary review
Foundation for Resident Memory Distillation (claim-bearing schema, temporal model, staged transform/replay, share grant materialization). Diff is source/docs/migrations only (~1.9k LOC). Checks run in this review: git log / git diff --stat vs origin/main; bun run typecheck exit 0; bun run test 358 pass / 0 fail.
Blocking (would be request-changes; own-PR so comment)
-
demoteGeneration does not restore the prior active embed model (src/services/transform.ts:628-631 after promote activates at :561). Live versions return to the pre-promote generation, but dense search can remain on the promoted model table. That breaks the CL-5872 rollback story. Record the pre-promote model key on promote and re-activate it on demote, or stop claiming demote as full rollback until that exists.
-
Missing product-path tests for staged embed isolation + promote/demote. Registry unit tests prove ensure!=activate; nothing exercises search/generation cutover. Add at least one test that fails if staged ensure flips live active, and one for generation swap (and embed restore once fixed).
Non-blocking
- sourceVersionId: result.documentId in share materialize (src/memory.ts:720) — audit field is misnamed/mis-valued until MemoryAddResult exposes a version id.
- Promote allows failed runs (src/services/transform.ts:516-519).
- Activate-after-txn on promote can leave corpus and dense model inconsistent if activate throws.
- Commit subjects use feat: / ticket noise; squash to plain English before merge.
What looks solid
- ensure vs activate split + generation-scoped dense resolution for non-live search.
- Enum lockstep + claim-bearing provenance without inference in core.
- Share via WritableGrantStore + memory.doc:* without reintroducing a mini-ACL.
- Timeline/search generation filtering so staged rows do not leak into live views.
Verdict: request changes on demote dense restore + cutover tests (posted as comment because GitHub disallows request-changes on own PR). Rest is merge-ready foundation for Levels 1-2.
Align arktype edge/lineage enums with the Postgres CHECK set, split ranking sourceClass from stored lineage, and add version.provenance so derived claims can be written with derived_from edges without putting inference in core. Capture resolves native edge endpoints to principal entities and no longer writes ranking priors into the lineage column (which would fail the CHECK).
Add temporal_class (event/deadline/state/lesson) and valid_from/valid_until on knowledge.version; class-aware recency in hybrid search; fix timeline to filter by generation so staged replay never leaks into live views.
CL-5872: expose transform plane methods with staged promote/demote; split ensureEmbedModel vs activateEmbedModel so replay never steals live dense search; generation-scoped dense table resolution. CL-5873: materialize peer share grants via WritableGrantStore on memory.doc tags; merge MEMORY_SHARE_CONDITION_REGISTRY; write-narrow- then-widen helpers and AUTHZ docs.
Promote only completed runs, activate the staged embed model before swapping generation tags, and record the pre-promote model key so demote can restore dense search fail-closed. Add returns versionId so share grants cite the written version rather than the document id.
Scope generation search params and appendAccessTags by tenant so cross-tenant ids cannot resolve embed overrides or retag documents. Activate one dense model exclusively; demote with no prior model clears active status instead of leaving the promoted table live.
Hosts pass memory.databaseUrl or DATABASE_URL (same Postgres as the hub is fine); tables live under the memory schema. KNOWLEDGE_DATABASE_URL remains a deprecated alias. Fresh installs only for the schema rename.
Dense search with entityIds still queried knowledge_edge after the schema moved to memory.edge. Point the subquery at memory.edge, assert the table name in tests, drop legacy knowledge_embed_model mock matches, and align CHANGELOG/IMPLEMENTATION with versionId on add and host- privileged transform APIs. Round-2 multi-lens review + convergence notes under .corbits/.
Wire living relevancy from supports/contradicts edges into hybrid search ranking and evidence:strong gating. Add cursor-based capture feed (feed_seq, memory.feed, GET .../memory/feed) for the resident distiller. Also single-statement exclusive embed activation, grantsMaterialized on share add, light search attribution, and substrate docs.
…5869) Search hits surface additive provenance/temporal/corroboration/derived_from attribution. Retention classes + plane write paths (deprecate/tombstone/ hard-delete/sweep) land with migration 0007. Distiller remains host-side on feed+add; docs and CHANGELOG updated for Level 3 close-out.
Matches CL-5871 plan: host-schedulable sweep auto-deprecates; hard-delete stays an explicit verb.
Unit-test toHit/SearchHitSchema attribution shapes; document list without attribution; feed status filter comment reflects 4b retention outcomes; memory_search tool blurb mentions attribution guidance.
Ship @corbits/memory/distiller so Corbits apps can opt into continuous distillation with createResidentDistiller or runDistillTick. Claim-aware add + memory_feed tool + accessTags on feed entries complete the substrate. Inference stays host-injected.
IMPLEMENTATION.md lists distiller/ and tools/; DISTILLER.md adds a manual dev-hub integration checklist for CL-5869.
Product narrative: one host pipeline; pull feed + resident distiller are optional multi-writer/backfill process helpers, not primary ingest.
Static package.json + MEMORY_GRANT_REQUIREMENTS SSOT so host installers can learn memory:add / memory:search without executing the package.
Post-filter used the last allowed feedSeq for nextCursor, so a fully denied page returned null and stalled consumers. Keep the raw page cursor; wire agentId into the distiller default system prompt.
Comment references to memory.version.source_class in adapted-document.ts and a misleading example grant tag in grant-tags.test.ts still said knowledge; align with the memory rename.
…5872) resolveActiveEmbedTable picked the tenant's newly-activated embed model independent of which generation's version rows were tagged live, so a promote or demote briefly left the active dense table pointed at one generation while `version.generation` still pointed at the other — live dense search silently returned zero rows for the gap between the two writes. Fold the embed-model activation and the generation-tag swap into a single Postgres transaction so a concurrent reader only ever observes the fully pre- or fully post-promote state. A thrown error mid-transaction now rolls back the embed activation along with the tag swap, so the manual restore-on-failure branch in promoteGeneration is no longer needed. createRawSqlClient now accepts a `sql.begin()` transaction handle in addition to the top-level connection, since embed-model-registry calls need to run inside the caller's transaction rather than committing on their own.
…grant (CL-5873) grantsMaterialized was set from isWritableGrantStore(grantStore) alone, so a host with a writable GrantStore but a DocumentStore that doesn't implement the optional appendAccessTags reported grantsMaterialized: true even though the memory.doc:<id> tag was never stamped on the document — leaving the peer grant unreachable via canAccessDocument, which only checks tags actually present on the document. The flag now requires both the tag write and the grant write to have succeeded. Also wrap the tag-append and grant-materialize calls in try/catch: they run after store.add() has already durably committed the document, so a failure here must downgrade grantsMaterialized rather than reject add() for a document that in fact exists (which would invite a caller retry and a duplicate).
c4b701c to
5b6b04a
Compare
Review + rebase notesThis branched before #32 ( Adversarial review — fixed on the branch
Checked, no issue foundSQL injection / raw-identifier interpolation (all new identifiers are Noted, not fixed (flagged for follow-up, not blocking)
Full checks green: |
1 participant
Summary
Foundation work for Resident Memory Distillation (Levels 1–2 of the dispatch DAG):
sourceClasssplit from stored lineagetemporal_class+ validity window on versions, class-aware recency ranking, timeline filtered by generationensureEmbedModelvsactivateEmbedModelso staged replay never steals live dense searchWritableGrantStoreonmemory.doc:*; condition registry merge; write-narrow-then-widen helpersTest plan
bun run typecheckbun run test(358 pass)0003claim-bearing,0004temporal,0005transform promote) on a fresh DBshare.principals+ writable grant store → peer can search, non-peer cannotLinear: