feat(knowledge): delivery_policy column + typed-memory backfill (ADR 0108 D4)

[mabry1985]

What

The last ADR 0108 D4 typed-memory column — delivery_policy — plus the one-shot backfill that #3205 never shipped. Context v2 Phase 2b; the gate for D6 (delivery budget), D7 (write lifecycle) and D9 (digest gating).

• delivery_policy TEXT (nullable) on chunks: always (every turn — what domain="hot" has always meant) · retrieved (on a RAG match; NULL = retrieved, and a retrieved filter matches NULL rows) · on_demand (tool call only). Index idx_chunks_delivery_policy. One _delivery_policy_clause() helper (next to _namespace_clause) builds the predicate at all four SQL sites — FTS, LIKE fallback, list_chunks, hybrid vector.

• Plumbed everywhere the other typed columns are: Chunk + as_dict(), add_chunk / add_document, search (hybrid filters BOTH rankings), list_chunks, layered search (both tiers) and promote().

• domain="hot" forces delivery_policy="always" in add_chunk — not default-only. Today's reader keys on the domain, so a hot row IS always-on whatever the column says; the column must be truthful or D6's flip would strand a hot+on_demand row. A caller passing something else is overridden (logged at debug). memory_kind is not inferred at write time — feat: typed memory schema — additive columns for memory classification #3205's omitted-stays-NULL contract holds.

• One-shot backfill of legacy rows (both memory_kind and delivery_policy — feat: typed memory schema — additive columns for memory classification #3205 left every existing row NULL), run on the first open of an existing store:

  domain	condition	→ memory_kind	→ delivery_policy
  hot	—	standing	always
  preferences	—	profile	NULL (retrieved)
  general	source_type="conversation"	note	NULL
  general	other source_type	reference	NULL
  finding	—	fact	NULL
  fact	—	fact	NULL
  conversation	—	note	NULL
  anything else	—	legacy	NULL

  Only NULL cells are written (an already-typed row keeps what it has); the mapping lives in one place (infer_memory_kind / infer_delivery_policy) shared by the backfill and the write path. The pass is stamped in _kb_meta (typed_memory_backfill) rather than keyed on the column's existence, so a pass that dies after the ALTER TABLE retries on the next open and a finished one never re-runs. A fresh DB gets the column from _SCHEMA and is stamped with nothing to classify.

• Agent tools: memory_ingest(delivery_policy=), memory_recall(delivery_policy=), memory_list(delivery_policy=) + a policy= tag in listings. The value is validated and normalized at the tool boundary (_norm_delivery_policy against DELIVERY_POLICIES: " ALWAYS " → always; "hot" → Error: delivery_policy must be one of …); the store and SDK stay permissive, matching feat: typed memory schema — additive columns for memory classification #3205's memory_kind stance.

• The always-on confirm gate (knowledge.hot_write_confirm) now covers every agent path to an always-on row: memory_ingest with domain="hot" or delivery_policy="always", and knowledge_ingest with domain="hot" (it had no check — a local file was an ungated path). One shared _ALWAYS_ON_REFUSAL text names both tools; settings description, config comment, guide, explanation and reference docs updated to match.

• Console + SDK: _knowledge_row exposes memory_kind / review_state / delivery_policy (the typed columns weren't reaching the console at all); sdk.knowledge_add(memory_kind=, delivery_policy=), forwarded only when set so pre-D4 plugin backends keep working. docs/reference/plugin-sdk-api.md regenerated (scripts/gen_plugin_api.py).

• Chunk.from_row() replaces every strict Chunk(**dict(row)) — unknown columns are dropped, not fatal (see Compatibility).

Boundary — what this PR deliberately does NOT do

D6 (#3187) owns switching hot-memory reads to delivery_policy. get_hot_memory_entries still selects on domain="hot", _publish_hot_write still fires on domain == "hot", and the console hot-edit route still pins "hot" (which now stamps always). D4 records the policy; D6 makes the delivery layer read it.

D6 note. After this PR no hot+non-always row can exist — the write path forces it and the backfill stamps every legacy hot row — so D6 can flip readers to delivery_policy='always' with no migration step and no NULL special-casing (_delivery_policy_clause already treats NULL as retrieved).

Compatibility

The commons DB is host-level (~/.protoagent/commons/knowledge.db, shared by the default and dev instances). The first upgraded instance ALTERs it; an older build then reads a widened table, and its strict Chunk(**dict(r)) fails in LayeredKnowledgeStore.list_chunks (console Store view, memory_list, the fact consolidator). That is exactly the exposure #3205 already created when it added four columns. This PR makes this build immune to future columns (D7's) via Chunk.from_row(); older builds remain as exposed as they were.

Rollback

Nullable additive column; the backfill only fills NULLs. Reverting the code restores v1 reads.

Gates (local, worktree off origin/main @ 49aeb6bd)

• uv run ruff check . → All checks passed!
• uv run lint-imports → Contracts: 3 kept, 0 broken.
• Focused (test_knowledge_typed_memory, test_knowledge_layered, test_knowledge_trust, test_hot_memory, test_memory_routes, test_sdk_knowledge, test_plugin_api_reference) → 120 passed
• Full python -m pytest tests/ -q (after the review round, head 1c13de5e) → 6846 passed, 16 skipped, 156 warnings in 333.19s — EXIT=0

Refs #3072 (closed by #3205), #3184 (tracker).

🤖 Generated with Claude Code

https://claude.ai/code/session_01WEMxBi71vjtmmmziFCMcby

[coderabbitai]

Warning

Review limit reached

Next included review available in 59 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 16bd836f-f873-4d18-a3da-1fd6c7119855

📥 Commits

Reviewing files that changed from the base of the PR and between f2c6d57 and 1c13de5.

📒 Files selected for processing (16)

• changelog.d/3242.added.md
• docs/explanation/memory-and-knowledge.md
• docs/guides/knowledge.md
• docs/reference/plugin-sdk-api.md
• docs/reference/starter-tools.md
• graph/config.py
• graph/sdk.py
• graph/settings_schema.py
• knowledge/hybrid_store.py
• knowledge/layered.py
• knowledge/store.py
• operator_api/knowledge_routes.py
• tests/test_knowledge_trust.py
• tests/test_knowledge_typed_memory.py
• tests/test_sdk_knowledge.py
• tools/lg_tools.py

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[protoreview]

QA panel review — PASS

code-review-structural · head 1c13de5ed63e · formal

Low-risk PR: a documentation addition explaining the memory/knowledge trust gate. No code defects surfaced. The one finding is a doc-accuracy nit — the explanation doc's phrasing implies knowledge_ingest also checks delivery_policy, but its signature has no such parameter; the sibling guide doc states the distinction correctly. Fix first: a one-line reword in docs/explanation/memory-and-knowledge.md to match the guide doc's precision. The panel did not disagree on any point. Verification confirmed the finding but flagged a gap: the exact gate code in tools/lg_tools.py was unreadable via API (404 on both head and default branch), so the claim rests on the doc signatures, the guide doc's wording, and the test suite rather than the source directly.

Findings

	Severity	Location	Finding	Verified
⚪	nit	docs/explanation/memory-and-knowledge.md:127	The explanation doc says the confirm gate "refuses that policy [delivery_policy=always] on any domain — through memory_ingest and knowledge_ingest alike", but …	confirmed

findings JSON (machine-readable)

[
  {
    "file": "docs/explanation/memory-and-knowledge.md",
    "line": 127,
    "severity": "nit",
    "category": "conventions",
    "claim": "The explanation doc says the confirm gate \"refuses that policy [delivery_policy=always] on any domain \u2014 through memory_ingest and knowledge_ingest alike\", but knowledge_ingest has no delivery_policy parameter, so it cannot refuse that policy \u2014 its gate keys on domain=\"hot\" only; the sibling guide doc states this correctly and the explanation doc is the loose one.",
    "evidence": "docs/explanation/memory-and-knowledge.md: \"and the gate refuses that policy on *any*\\n   domain \u2014 through `memory_ingest` and `knowledge_ingest` alike\" \u2014 while tools/lg_tools.py knowledge_ingest gate is `if dom.lower() == \"hot\" and getattr(graph_config, \"knowledge_hot_write_confirm\", False):` (no delivery_policy check) and its signature is knowledge_ingest(source, domain, title?).",
    "verdict": "confirmed",
    "note": "The explanation doc's 'refuses that policy on any domain \u2014 through memory_ingest and knowledge_ingest alike' overstates: knowledge_ingest has no delivery_policy param (signature confirmed in starter-tools.md) and the guide doc correctly scopes its gate to domain=\"hot\" only. The exact lg_tools.py gate code is unverified (file unreadable via API), but the substance is grounded in the doc signatures, the guide doc's precise wording, and the test suite (which only exercises domain=\"hot\" for knowledge_ingest)."
  }
]

---

1 finding(s) downgraded to uncertain: the code they quote as evidence does not appear in the file at the reviewed head, nor in this PR's patch for it. A finding that cannot be grounded does not gate a merge (issue #25) — it still stands for a human to judge.

• docs/explanation/memory-and-knowledge.md (nit) — quoted evidence not found at this head: if dom.lower() == "hot" and getattr(graph_config, "knowledge_hot_write_confirm", False):

---

1 panel step(s) hit their time budget and were skipped this round: find_crossfile. The verdict stands on the remaining angles; a finding only that step would have caught could be missed — the next push re-runs the full panel.