CL-5865/5866/5872/5873: Resident memory distillation foundation

.corbits/review-round2-bruckheimer.md

@@ -0,0 +1,41 @@
+# Bruckheimer — PR #31 round 2 (HEAD `6189b56`)
+
+## One-liner
+
+Foundation so a company-brain distiller can write **inferred claims** with
+provenance, rank them in time, re-distill offline without trashing live search,
+and share docs with real grants — not just tags.
+
+## Hook progress
+
+| Piece | Status for the hook |
+|-------|---------------------|
+| Claim-bearing / derived_from / provenance | Shipped |
+| Temporal classes + validity | Shipped |
+| Staged transform / promote / demote | Shipped (code); weak tests |
+| Share grants materialization | Shipped (fail-soft without writable store) |
+| Capture feed / distiller workflow | **Not this PR** (CL-5868/5869) |
+| Schema/config host-friendly | DATABASE_URL + memory schema — good |
+
+## Product blockers
+
+1. **Dense entity filter broken after rename** — if any host/search UI filters by
+   entity, dense path dies. Fix before merge.
+2. **Promote rollback untested** — if demote is the safety valve for bad
+   distillation, untested demote is a product risk, not just eng debt.
+3. **Share without WritableGrantStore** only warns — hosts will think share
+   “worked.” Consider fail-loud option or return receipt with
+   `grantsMaterialized: false` (follow-up OK).
+
+## Breaking-change cost/benefit
+
+- `knowledge` → `memory` schema: right name for the product; cost is fresh
+  install only. Acceptable if no production tenants on old schema.
+- `DATABASE_URL` preferred: lowers host friction (one DB). Risk: host with two
+  URLs silently picks the wrong one — document clearly (done in mount-config).
+
+## Ship advice
+
+Fix the dense SQL bug, clarify CHANGELOG on versionId, merge as foundation.
+Do not hold the PR for the distiller itself. Next product milestone is feed +
+workflow, not more schema polish.

.corbits/review-round2-convergence.md

@@ -0,0 +1,88 @@
+# Convergence review — PR #31 round 2 (HEAD `6189b56`)
+
+**Method:** 8 deep lenses in-session (fleet spawn blocked: Codex profile
+`fleur` unauthorized). Lenses: critique, greybeard, gaasbot, neckbeard,
+bruckheimer, security, schema, OSS. Artifacts:
+`.corbits/review-round2-*.md`.
+
+## Converged verdict
+
+**CHANGES REQUESTED — one hard blocker, then merge-eligible.**
+
+All eight lenses agree the distillation foundation is the right shape and that
+prior promote/demote/versionId work improved the bar. All eight that touched
+search/schema flag the same ship-blocker.
+
+---
+
+## Must-fix before merge (unanimous / multi-lens)
+
+| ID | Finding | Lenses | Action |
+|----|---------|--------|--------|
+| **M1** | Dense entity filter SQL still uses `knowledge_edge`; table is `"memory"."edge"` (`src/services/search.ts:537`) | critique, greybeard, gaasbot, neckbeard, security, schema, OSS, bruckheimer | Fix SQL + add regression test (dense path with entityIds) |
+| **M2** | CHANGELOG Unreleased “Previously” claims add returns only `{ documentId }`; code returns `versionId` | critique, gaasbot, greybeard, OSS | Correct CHANGELOG |
+
+## Should-fix (same PR if cheap; else ticket)
+
+| ID | Finding | Lenses | Action |
+|----|---------|--------|--------|
+| **S1** | No promote/demote service E2E regression | critique, greybeard, gaasbot, bruckheimer | Add transform test: ensure→promote→demote restores model_key + generation |
+| **S2** | search.test mocks still accept `FROM knowledge_embed_model` | neckbeard, OSS | Match only `"memory"."embed_model"` |
+| **S3** | Document that transform/promote/demote are host-privileged (no principal on API, no HTTP) | security, gaasbot, greybeard | Short IMPLEMENTATION / AUTHZ note |
+
+## Follow-up (do not block merge)
+
+| ID | Finding | Lenses |
+|----|---------|--------|
+| F1 | `setActiveEmbedModelExclusive` two-step race | critique, security |
+| F2 | Promote/demote multi-step windows; consider tenant advisory lock | critique, greybeard, security |
+| F3 | Share fail-soft without WritableGrantStore / appendAccessTags — receipt field | critique, bruckheimer, security |
+| F4 | TS `knowledge*` identifiers + migration filenames + comment rot | neckbeard, schema |
+| F5 | Ops note for `ALTER SCHEMA knowledge RENAME TO memory` if any old install | greybeard, schema |
+| F6 | README advanced surface (transform exports) | OSS, bruckheimer |
+
+## Explicit non-goals this PR
+
+- Distiller workflow / capture feed (CL-5868/5869)
+- HTTP routes for transform
+- Bulk rename of knowledge* TypeScript symbols
+- Re-introducing separate knowledge DB
+
+## Steelman of “merge as-is”
+
+Tests are green; entityIds on dense may be rare; rename is fresh-install-only.
+**Rejected:** a single untested raw-SQL island after a schema rename is exactly
+the class of bug that survives CI and fails first real use. Fix is trivial.
+
+## Steelman of “hold for promote E2E”
+
+Demote is the safety valve for bad distillation. **Partial accept:** S1 is
+high value but not a correctness hole in the current code path (demote restore
+exists). Prefer same-PR if <1h; else ticket linked from PR.
+
+## Converged fix order
+
+1. **M1** fix + test  
+2. **M2** CHANGELOG  
+3. **S2** tighten mocks (with M1 test)  
+4. **S1** if time  
+5. **S3** one paragraph  
+6. Re-run typecheck + test → push  
+
+## Bar after fixes
+
+| Bar | Status after M1+M2 |
+|-----|--------------------|
+| Security | Pass (with S3 doc preferred) |
+| Product | Pass foundation |
+| Greybeard / architecture | Pass |
+| OSS quality | Pass pre-1.0 |
+| Critique | Pass with S1 follow-up |
+
+---
+
+## Note on fleet
+
+All 8 `task` spawns failed: `Codex profile "fleur" is not authorized`. Reviews
+were executed in-session with the same multi-lens briefs. Re-auth `/model`
+(profile fleur) to restore sub-agent fleet for future rounds.

.corbits/review-round2-critique.md

@@ -0,0 +1,74 @@
+# Critique — PR #31 round 2 (HEAD `6189b56`)
+
+In-session (fleet blocked: Codex profile `fleur`). Diff `origin/main..HEAD`.
+Typecheck clean; 367 tests green at last gate.
+
+## Verdict
+
+**CHANGES REQUESTED.** Prior promote/demote and versionId fixes landed, but the
+`knowledge` → `memory` rename left a **live dense-path SQL bug**, and
+promote/demote still lack end-to-end regression tests.
+
+## Critical
+
+1. **Dense entity filter still queries `knowledge_edge`**
+   (`src/services/search.ts:537`). After schema rename, the table is
+   `"memory"."edge"`. Any dense search with `entityIds` will fail at runtime
+   (`relation "knowledge_edge" does not exist`). Lexical path is fine (Drizzle
+   `knowledgeEdge` → `memory.edge`). No test exercises dense+entityIds.
+
+## High
+
+2. **No automated promote → demote → dense restore E2E.** Registry unit tests
+   cover ensure/activate/byKey, but nothing asserts: staged run under model B
+   does not flip live active; promote swaps generation + activates staged;
+   demote restores generation **and** prior model_key. CL-5872 rollback
+   acceptance still untested at the service layer.
+
+3. **Promote/demote are multi-step outside a single transaction.** Activate
+   dense, then version swap (or reverse on demote). Concurrent promote of two
+   generations, or crash mid-window, can leave dense target and generation tags
+   briefly inconsistent. Documented preference is intentional; still a
+   production footgun without locks / single-flight.
+
+## Medium
+
+4. **`setActiveEmbedModelExclusive` is two non-atomic UPDATEs**
+   (`embed-model-registry.ts:309-325`). Concurrent activate of A and B can
+   leave two `active` rows until next exclusive call; `ORDER BY updated_at`
+   picks one, but window exists.
+
+5. **Transform plane methods take only `tenantId` / `configId` — no principal.**
+   In-process API; no HTTP routes. Correct for library shape, but any host that
+   re-exports without its own grant check hands promote/demote to any caller
+   who can reach the plane. Docs should state “host must authorize.”
+
+6. **Share path still fail-soft** when `appendAccessTags` or WritableGrantStore
+   missing (`memory.ts:707-727`): warns, continues, peers fail-closed. Easy to
+   miss in production.
+
+7. **CHANGELOG drift:** Unreleased “Previously” still says `add` returns
+   `{ documentId }` only; wire now returns `{ documentId, versionId }`.
+
+## Low / Nits
+
+8. Comments and enums still say `knowledge.version` / `knowledge.embed_model`
+   (`enums.ts`, `embed-model-registry.ts:34`, `generation.ts`).
+9. Migration file still named `0002_knowledge_baseline.sql` while creating
+   `memory.*`.
+10. TS exports still `knowledgeDocument` / `knowledgeVersion` under memory schema.
+
+## Test gaps
+
+- Dense search + `entityIds` (would catch Critical #1).
+- `promoteGeneration` / `demoteGeneration` service tests with fake SQL + version rows.
+- Concurrent exclusive activate (optional stress).
+- `loadMemoryConfig` DATABASE_URL vs KNOWLEDGE preference already covered.
+
+## Assumptions challenged
+
+- “Rename was complete because migrations and drizzle use memory” — raw SQL
+  island in dense path was not.
+- “367 green ⇒ rename safe” — unit tests mock embed_model with dual match
+  (`knowledge_embed_model` OR `memory.embed_model`) and never hit entity filter
+  raw SQL.

.corbits/review-round2-gaasbot.md

@@ -0,0 +1,44 @@
+# Gaasbot (CTO) — PR #31 round 2 (HEAD `6189b56`)
+
+## CTO verdict
+
+**Right foundation, one ship-blocker.** This is the correct shape for
+resident distillation: claim-bearing + temporal + staged transform + grants.
+Do not expand scope into the distiller workflow (CL-5869) on this PR.
+
+## Must-fix-before-merge
+
+1. Fix dense-path `knowledge_edge` → `"memory"."edge"` (`search.ts:537`).
+2. Add a regression test that would fail on that bug (dense fetch with
+   entityIds, assert SQL contains `"memory"."edge"` or run against real SQL
+   mock that only knows memory.edge).
+3. Fix CHANGELOG: add returns `versionId`; remove contradictory “Previously”
+   line or mark superseded.
+
+## Can-ship-with-followups
+
+- Promote/demote service-level tests.
+- Tenant-scoped advisory lock on promote/demote.
+- Atomic exclusive activate (single SQL CTE or transaction).
+- Explicit “host authorizes transform APIs” note in IMPLEMENTATION.md.
+- Rename TS `knowledge*` symbols in a dedicated PR (not this one).
+
+## Defer
+
+- HTTP routes for transform/promote (in-process is fine for v1 distiller).
+- Capture feed (CL-5868), relevancy (CL-5867), retention (CL-5871).
+- Upgrade migration from `knowledge` schema for old DBs unless a customer exists.
+
+## Architecture notes
+
+- Exporting transform + embed registry from package root is aggressive but OK
+  for the distiller as first-party consumer. Keep them off HTTP until grants
+  exist.
+- Preferring `DATABASE_URL` is correct for “same Postgres, own schema.” Warn
+  hosts that still set both URLs with different values — preferred wins.
+- Do not re-introduce a separate knowledge DB requirement.
+
+## Priority
+
+Blocker fix is a one-liner + test. Merge after that; iterate on promote
+hardening in the same branch if cheap, else follow-up ticket.

.corbits/review-round2-greybeard.md

@@ -0,0 +1,59 @@
+# Greybeard — PR #31 round 2 (HEAD `6189b56`)
+
+## Verdict
+
+**HOLD for one correctness fix; then ship foundation.** Architecture of
+ensure-vs-activate, claim-bearing, temporal classes, and share materialization
+is sound. Schema rename + DATABASE_URL is the right long-term shape.
+
+## Ship / hold
+
+**Hold** until dense `entityIds` SQL is fixed (`knowledge_edge` → `"memory"."edge"`).
+After that: **ship with follow-ups** (promote E2E tests, exclusive activate
+transaction, host authz docs for transform).
+
+## Critical / High
+
+1. **Raw SQL residue after schema rename** — `search.ts:537` `knowledge_edge`.
+   Irreversible-looking renames that leave one path broken are worse than no
+   rename: green CI + red prod.
+
+2. **Fresh-install-only schema rename** is honest in CHANGELOG but operationally
+   harsh. Acceptable for pre-1.0 / no prod tenants; document a one-shot
+   `ALTER SCHEMA knowledge RENAME TO memory` + table renames for anyone who
+   already migrated under `knowledge`.
+
+## Medium — design debt (acceptable for now)
+
+3. **JS identifiers lag schema** (`knowledgeDocument` table → `document`). Fine
+   if intentional transitional; pick a rename PR later — do not half-rename.
+4. **Embedding tables keyed only by model_key**, multi-tenant rows inside.
+   Tenant filter on every dense query is load-bearing; keep that invariant in
+   review checklist forever.
+5. **Promote activate-then-swap** preference is documented and reasonable.
+   Prefer advisory lock per tenant around promote/demote before multi-tenant
+   production load.
+6. **Docs generally lockstep** with DATABASE_URL / memory schema after last
+   commit; IMPLEMENTATION table and AGENTS.md match. CHANGELOG “Previously”
+   still contradicts versionId on add.
+
+## Doc drift
+
+| Claim | Reality |
+|-------|---------|
+| CHANGELOG: add returns `{ documentId }` | Returns `{ documentId, versionId }` |
+| Comments: knowledge.embed_model | Table is memory.embed_model |
+| open.type "memory" | Correct in search.ts |
+
+## Design decisions that aged well
+
+- `ensureEmbedModel` vs `activateEmbedModel` split (replay must not steal live).
+- `activateEmbedModelByKey` for demote without re-probe.
+- `archived_live_model_key` on transform_run.
+- Share grants + pass-through condition registry.
+- Enum lockstep test (enums.lockstep.test.ts).
+
+## Recommendation
+
+Fix Critical SQL → add dense+entityIds test → optional promote E2E → merge.
+Do not block on knowledge* TypeScript renames.

.corbits/review-round2-neckbeard.md

@@ -0,0 +1,32 @@
+# Neckbeard — PR #31 round 2 (HEAD `6189b56`)
+
+## Real bugs hiding as nits
+
+1. **`knowledge_edge` in raw SQL** (`search.ts:537`) — not a naming nit. **Bug.**
+2. **search.test.ts still accepts `FROM knowledge_embed_model`** as a success
+   path for mocks (`search.test.ts:237, 412`). Teaches the wrong table name;
+   should only match `"memory"."embed_model"`.
+
+## Naming debt inventory (cosmetic unless noted)
+
+| Residue | Severity |
+|---------|----------|
+| `knowledgeDocument`, `knowledgeVersion`, `knowledgeChunk`, `knowledgeEdge`, `knowledgeEntity`, `knowledgeEmbedModel` exports | Cosmetic / API-internal |
+| `KNOWLEDGE_SCHEMA` deprecated alias | OK transitional |
+| `migrations/0002_knowledge_baseline.sql` filename | Cosmetic; content correct |
+| Comments `knowledge.version`, `knowledge.embed_model` | Doc rot |
+| Id prefixes `kver`, `kdoc` | Cosmetic |
+| grant-tags test still uses `knowledge.project:ke` as a free-form tag | Fine (host tags) |
+| CHANGELOG “Postgres schema name remains knowledge” removed; good | — |
+
+## Nits
+
+- Dual match in tests for old embed_model table should die with the rename.
+- `### Previously` in CHANGELOG is nonstandard Keep-a-Changelog structure.
+- Package still says “knowledge plane” in a few comments (`config.ts` FTS).
+- `openTarget` comment still says “generic knowledge doc” (`search.ts:156`).
+
+## What not to rewrite
+
+Do not rename all `knowledge*` TS symbols in this PR. Ship the SQL fix and
+stop. A bulk rename PR with codemod is fine later.

.corbits/review-round2-oss.md

@@ -0,0 +1,53 @@
+# OSS / public API quality — PR #31 round 2 (HEAD `6189b56`)
+
+## Public surface inventory (package root)
+
+- `createMemory`, `loadMemoryConfig`, `runMemoryMigrations`, `MemoryError`
+- Routes: `registerMemoryRoutes`
+- Ports: DocumentStore types, fakes, WritableGrantStore
+- Share: buildShareGrants, materializeShareGrants, MEMORY_SHARE_*
+- Transform: createTransformConfig, runTransform, promote/demoteGeneration, …
+- Embed registry: ensure/activate/resolve helpers
+- Degrade metrics, FTS helpers
+
+**Note:** Transform + embed registry on the root export is a large surface for
+an “add/search/list” product blurb. Acceptable for distiller-as-consumer;
+README should mention advanced APIs.
+
+## Breaking changes completeness
+
+| Change | CHANGELOG | Code |
+|--------|-----------|------|
+| Schema knowledge → memory | Yes | Yes |
+| DATABASE_URL preferred | Yes | Yes |
+| open.type memory | Yes | Yes |
+| add returns versionId | **Stale “Previously” says no** | Yes |
+| Claim-bearing / temporal / transform | Partial (IMPLEMENTATION) | Yes |
+
+## Quality bar
+
+| Area | Pass? | Notes |
+|------|-------|-------|
+| arktype at edges | Pass | transform params, raw_capture replay |
+| Enum lockstep tests | Pass | enums.lockstep.test.ts |
+| Tenant SQL discipline | Pass* | *except broken edge table name |
+| Module focus | Pass | services split reasonably |
+| Docs match exports | Partial | CHANGELOG versionId; comments knowledge.* |
+| Test coverage public contracts | Partial | no promote E2E; dense+entityIds missing |
+| Semver honesty | Partial | fix Unreleased Previously section |
+
+## Must-fix for OSS merge
+
+1. Dense entity SQL table name.
+2. CHANGELOG honesty on `MemoryAddResult` / wire body.
+3. Regression test for (1).
+
+## Nice-to-have before wider publish
+
+- README section: transform/promote privileged, host-gated.
+- Drop dual mock match for `knowledge_embed_model` in tests.
+- Do not bulk-rename knowledge* TS identifiers in this PR.
+
+## Verdict
+
+**Fail OSS bar until Critical SQL + CHANGELOG; then pass for pre-1.0 foundation.**