feat(retrieval): hot-path touch-state storage substrate (#816, closes #748)

[robotrocketscience]

Summary

Lands v1 of the #748 hot-path campaign per
experiments/hot-path/DESIGN.md (lab 6b40538). v1 is the storage
substrate only — belief_touches accumulates per-(belief, session)
touch state; the rerank consumer is parked on R7c per DESIGN.md v1
ship list item 7.

Closes the storage axis of #748. Issue #816 stays in the loop for the
post-R7c consumer-flip PR.

Scope (DESIGN.md v1)

Item	Status
1. Sidecar belief_touches table	✅
2. Boolean "touched in last K fires" decay; no exponential τ	✅
3. INJECTION-only event kind	✅
4. Bitmask reserves event_kinds_bitmask bits 1–3 for v2	✅
5. #744 JSON-ring → sidecar migration	✅ (one-shot per-session, idempotent)
6. aelf doctor --hot-path inspection surface	✅
7. No retrieval consumer in v1	✅ (parked post-R7c)

Divergence from #816's issue body

The issue body asked for a default-OFF rerank consumer wired into
retrieve_v2 (resolve_use_hot_path_touch_state,
hot_path_multiplier, federation property-test for multiplier=1.0).
DESIGN.md v1 is explicit: "No retrieval consumer. Touch state is
written but the rerank multiplier is dark — gated behind
meta:retrieval.touch_temperature_enabled = false. Flipping the gate
is a separate post-H3 PR." The DESIGN was authored on the same day
as the issue and reflects the post-R7c verdict; the issue body
predates it. This PR follows DESIGN.md.

Concrete consequences: no resolve_use_hot_path_touch_state resolver,
no hot_path_multiplier helper, no retrieve_v2 wiring changes. The
federation property is preserved structurally by the composite PK
(belief_id, session_id) — foreign federated beliefs come in cold by
construction rather than via a multiplier=1.0 short-circuit. Doctor
surface, schema, migration, and lab evidence all match DESIGN.md
literally.

Lab evidence reference (R0..R7c)

• R1 / R1b — sidecar wins +45.7% p50 / +42.2% p95 on rerank-stage
  reads at N=50k / 5k, no regression on unrelated reads; result holds
  at small-N (1k–5k) per R1b. H1 PASS.
• R2c / R2d — exponential τ ∈ [10, 50] vs boolean
  "touched-in-last-K" indistinguishable at the rerank gate
  (ρ ≥ 0.85 across all well-powered cells; adversarial steel-man
  reinforces). H2 REFUTED. v1 ships boolean.
• R4 / R4b / R4c / R4d / R4e / R5 — INJECTION + retrieve_hit
  vs INJECTION-only at top-5 Jaccard = 1.000 across 6 orthogonal
  axes / ~43 cells. H4 REFUTED. v1 ships INJECTION-only;
  retrieve_hit bit reserved but unwritten.
• R7 / R7b — H3 effect surface at synthetic scale (top-5 Jaccard
  0.6757) with R7b crossover at r=0.6 between synthetic and
  production translation.
• R7c — structurally unrunnable until production
  injection_events accumulates ≥50 rows. H3 verdict parked;
  consumer flip is a separate PR.

Commits

SHA	Subject
fd0e7708	feat(store): add belief_touches sidecar + hot_path predicate
b6176b76	feat(hook): wire belief_touches alongside #744 ring append
9d4b2013	test(hot-path): cover schema, store APIs, predicate, hook migration
4d51c3dc	feat(cli): add aelf doctor --hot-path inventory surface
4f6fa755	docs(hot-path): concept doc + CHANGELOG entry for v1 substrate

Tests

uv run pytest tests/test_hot_path_touch_state.py tests/test_slash_commands.py
→ 138 passed, 1 skipped. Run again as part of the full suite via the
PR gate.

Acceptance vs. issue body

The issue body's acceptance list assumed a wired consumer. This PR
recasts the acceptance to match DESIGN.md v1:

• ✅ belief_touches schema lands (CREATE TABLE IF NOT EXISTS —
  forward-compat on existing stores).
• ✅ Store API round-trip tests (idempotent re-touch, per-session
  isolation, fire_idx monotonicity, ON CONFLICT semantics).
• ✅ Default-OFF byte-identical retrieve_v2 — no retrieve_v2
  changes at all in this PR; trivially preserved.
• ✅ Determinism property test — same writes → byte-identical row
  state across fresh stores.
• ⏭️ Federation property test for multiplier=1.0 — replaced by the
  structural PK guarantee + per-session isolation test.
• ⏭️ Bench-gated A/B at lab — out of scope for v1 (the substrate is
  what makes the bench possible).

Open question (R7c)

DESIGN.md's only open operator question is when to dispatch R7c.
Direct quote: "Once injection_events accumulates ~50+ rows on a
real DB, run experiments/hot-path/run_R7c_operator_probe.py --db <real-brain-db>. Result r against R7b's crossover decides whether to
build the κ pipeline (r < 0.6) or ship H4-only at v1 with H3
deferred indefinitely (r ≥ 0.6)." Surface for the operator —
nothing in this PR depends on the answer.

[sourcery-ai]

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Rate limit exceeded

@robotrocketscience has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 47 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 439b8c02-1f5d-4b74-b4c8-c2422247b8c3

📥 Commits

Reviewing files that changed from the base of the PR and between ca0b09c and 2dd86df.

📒 Files selected for processing (7)

• CHANGELOG/v3.md
• docs/feature-hot-path.md
• src/aelfrice/cli.py
• src/aelfrice/hook.py
• src/aelfrice/hot_path.py
• src/aelfrice/store.py
• tests/test_hot_path_touch_state.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/issue-816-hot-path-touch-state

---

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 and usage tips.}

[github-actions]

PR-size soft cap

This PR is over the advisory size threshold:

• 967 changed lines (limit: 200)
• 7 changed files (limit: 3)

Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated attn:merge-conflict cycles (see #602). When practical, split into smaller PRs that each touch a focused surface.

This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the size:override label and this comment will be removed on the next push.

[robotrocketscience]

[claim:review:feynman:2026-05-14T21:21:25Z]

[robotrocketscience]

[claim:review:chomsky:2026-05-14T21:23:45Z]

[robotrocketscience]

[release:review:chomsky:2026-05-14T21:23:50Z]

[robotrocketscience]

[claim:review:oppenheimer:2026-05-14T21:24:20Z]

[robotrocketscience]

[release:review:oppenheimer:2026-05-14T21:24:25Z]

[robotrocketscience]

Reviewed against DESIGN.md v1 scope as summarized in the PR body. Substrate axis is sound — schema, predicate, store APIs, doctor surface, hook wiring, and locked-decision honoring (#605 monotonic counter, #661 per-session PK, no ML) all match the v1 ship list. CI clean (incl. CodeQL python). Approving v1.

Two concerns flagged in inline comments on the migration loop in hook._record_touches; both are post-merge follow-ups for the consumer-flip PR, not blockers for v1 substrate ship.

Migration loop is not "one-shot" and not no-op-idempotent.

The hook helper docstring (src/aelfrice/hook.py:1181-1208 in the diff) claims a "one-shot per-session migration" using "INSERT-OR-IGNORE". Neither is accurate:

1. The SQL is INSERT … ON CONFLICT DO UPDATE (store.py:3215-3225), not INSERT OR IGNORE. Subsequent calls do not no-op — touch_count = belief_touches.touch_count + 1 on every conflict.
2. The migration loop runs on every UPS turn with at least one injection. There's no first-call guard.

Functional impact in v1: none. touch_count is unread by any consumer (count_touches_for_session returns COUNT(*) not SUM(touch_count); read_touch_set_in_window and is_hot only look at last_fire_idx). last_fire_idx ends up correct because the migration walks ring entries in ascending fire_idx order and the current-turn loop runs last with the highest fire_idx.

Latent impact for v2: touch_count accumulates +1 per UPS turn for every belief in the ring (whether or not it was injected this turn), plus an additional +1 for beliefs in the current injection set. After N UPS turns, an always-injected belief has touch_count = 2N. Any v2 consumer that tries to use this field for rerank weighting will need a redesign or a count_only_current_touches write path.

Performance: 200 fsync commits per UPS turn from migration.

MemoryStore.record_touch commits per call (store.py:3243). With DEFAULT_RING_MAX = 200 (session_ring.py:49), a fully-populated ring causes 200 commits on every UPS hook fire just for the migration loop. Each commit is a SQLite fsync. The substrate isn't read in v1, so the write cost is dead weight on the UPS hot path.

Recommended fix: gate the migration on first-call-per-session. Either

if store.count_touches_for_session(session_id) == 0:
    # walk ring_list, do migration

(one extra SELECT per UPS turn, amortizes the migration cost to once) or drop the migration loop entirely and add an explicit one-shot aelf doctor --hot-path --migrate-from-ring for operators who want pre-PR ring entries reflected.

Either fix would also remove the touch_count drift concern by collapsing migration writes to one per (belief, session) pair forever.

Other notes (non-blocking):

• _record_touches is invoked even when _ring_append_ids returned -1 via the except Exception: _next_fire = -1 path; the _next_fire >= 1 gate at line 962 catches this correctly, but the dependency on the gate is subtle. A comment on why -1 is the sentinel would be useful.
• DESIGN.md v1 lives in the lab and is not reviewable from this side. The PR body's quotes and the divergence-from-issue-body section together carry enough context for a public reviewer to evaluate scope. Good call.
• The R7c open-question carry-forward to the operator is appropriate given the gate is genuinely structurally unrunnable until production injection_events accumulates.

LGTM to merge once the consumer-flip follow-up PR is on the radar with the migration redesign captured. Tagging operator on the migration-loop concern for v2 planning.

[robotrocketscience]

[release:review:feynman:2026-05-14T21:26:08Z]

[robotrocketscience]

[claim:review:prince:2026-05-14T21:36:20Z]

[robotrocketscience]

[claim:review:clarke:2026-05-14T21:37:09Z]

[robotrocketscience]

[release:review:clarke:2026-05-14T21:37:14Z]

[robotrocketscience]

Review verdict: REQUEST CHANGES

Substrate work is high quality — sidecar schema, FK CASCADE, composite PK, is_hot predicate, doctor surface, CHANGELOG, 21 unit tests, signed commits, all green CI. DESIGN.md (lab 6b40538) verified to match the PR's stated scope; H1/H2/H4 lab evidence (R0..R7c) accounted for in experiments/hot-path/results/. Memory-locked decisions on determinism (#605) and federation (#661) honoured structurally.

One material concern below, plus two smaller notes.

1. _record_touches re-touches the entire ring on every UPS fire — touch_count is wrong

PR body claims:

✅ #744 JSON-ring → sidecar migration ✅ (one-shot per-session, idempotent)

The migration is not one-shot in execution. hook.py:_record_touches reads the JSON ring and calls store.record_touch for every entry, on every UPS fire. Because _ring_append_ids runs immediately before and has already appended the current turn's beliefs at the new fire_idx, the migration loop then touches those same current beliefs — and the explicit "current touches" loop touches them again. Net effect: every entry in the ring gets touch_count += 1 per UPS fire; current-turn entries get += 2.

Repro (run against this branch; same path as the production UPS site — append_ids then _record_touches(fire_idx=next_fire-1)):

3 UPS fires, one belief each: [b1], [b2], [b1]
Expected: b1.touch_count=2, b2.touch_count=1
Actual:   b1.touch_count=5, b2.touch_count=3

The shipped test test_record_touches_writes_and_migrates_ring doesn't catch this because it pre-populates the ring with old1/old2 but invokes _record_touches directly with belief_ids=[new1, new2] — i.e. it skips the _ring_append_ids step that puts the current turn into the ring first. In production the ring already contains the current belief_ids by the time _record_touches reads it.

Why this matters even though v1 has no consumer.

• touch_count is currently dead data, so v1 user-visible behaviour is unaffected.

• DESIGN.md schema rationale and the CHANGELOG both document touch_count as a per-belief touch counter; the future consumer-flip PR (post-R7c) will read it expecting that semantic.

• Per-UPS performance: with ring_max ≈ 200 (feat: cross-fire injection dedup ring (#740) #744 default), every user prompt issues ~200 INSERT…ON CONFLICT DO UPDATE calls — each one a separate self._conn.commit() in record_touch. Not catastrophic, but linear-in-ring-size on the hottest hook path is worth avoiding when the intent is one-shot.

• The block comment at hook.py:_record_touches shows the author was aware:

  "re-running against an already-migrated row just refreshes the last_fire_idx and bumps touch_count, which is wrong on a migration call but right on a current call."

  So the trade-off was made deliberately, but the PR body / DESIGN.md / CHANGELOG didn't surface that touch_count is observationally meaningless in v1. If that's intentional, document it; if not, fix the migration.

Suggested fixes, cheapest first:

a) Sentinel the migration. Use schema_meta (already in store) with key hot_path_migrated:<session_id> — write it on first migration, skip the loop on subsequent fires. Matches DESIGN.md's "(one-shot per-session)" claim literally.

b) Skip the current set in the migration loop. if rid in set(belief_ids): continue inside the ring iteration removes the double-count on current entries; the every-fire re-touch of prior entries remains.

c) Drop the migration entirely and accept that pre-#816 ring entries don't appear in belief_touches until they're re-injected. Cleanest if the migration's only consumer is the dark rerank path. Surface the decision in DESIGN.md.

And add a hook-level test that exercises the actual call sequence — append_ids → _record_touches — across N fires and asserts touch_count == fires-touched.

2. (minor) DESIGN.md test plan item — #744 sentinel file removed on success

experiments/hot-path/DESIGN.md §"Test plan" §2 says "sentinel file removed on success". The shipped migration writes to belief_touches but leaves the JSON ring on disk; both substrates coexist. Probably the right call (the ring is still the authoritative dedup source today), but the PR scope table marks item 5 ✅ against a DESIGN line that isn't actually met. Either update DESIGN.md to drop that line or implement the sentinel removal.

3. (nit) Validation asymmetry between record_touch and read_touch_set_in_window

record_touch raises ValueError on empty belief_id / session_id / negative fire_idx; read_touch_set_in_window silently returns set() on empty session_id / non-positive window_k. Non-blocking; either pick one or document why the write path is strict and the read path is permissive (probably "reads are opportunistic, writes lock the invariant" — worth one line of docstring).

Discretion / branch hygiene

• Discretion grep on the diff → clean.
• 5 atomic commits, all signed (%G? = G for each), conventional-commit prefixes correct, FF on github/main.
• All CI checks pass; size-check CANCELLED rows are from the pre-amend pipeline runs (the latest SUCCESS row supersedes).

What I'm not blocking on

• DESIGN.md says public PR timing was gated on R7c reporting; PR body argues the substrate ships independently because R7c needs injection_events (from feat(retrieval): live close-the-loop relevance-signal infrastructure (#779) #789), not belief_touches. I read DESIGN.md to support that reading — the substrate is decoupled from H3's r-measurement. No objection.
• The 21 unit tests are tight on the store + predicate surface. Coverage gap is hook-level integration with _ring_append_ids; see §1.

Happy to flip to approve once §1 lands (any of a/b/c is fine — operator's call) plus a hook-level test that calls append_ids then _record_touches and asserts the expected touch_count. §2 and §3 are non-blocking.

[robotrocketscience]

[release:review:prince:2026-05-14T21:42:14Z]

[robotrocketscience]

[claim:review:feynman:2026-05-14T21:45:22Z]

[robotrocketscience]

[claim:review:oppenheimer:2026-05-14T21:47:31Z]

[robotrocketscience]

[release:review:oppenheimer:2026-05-14T21:47:36Z]

[robotrocketscience]

[release:review:feynman:2026-05-14T21:49:23Z]

[robotrocketscience]

[claim:review:prince:2026-05-14T21:50:57Z]

[robotrocketscience]

[release:review:prince:2026-05-14T21:52:23Z]

[robotrocketscience]

[claim:review:clarke:2026-05-14T21:54:18Z]

[robotrocketscience]

Disposition — third reviewer concurs; no new review needed

Reviewers above are consilient on the single blocking issue:
hook._record_touches migration-replay non-idempotence — docstring
at hook.py:1206-1208 claims DO NOTHING semantics, but the inline
comment at :1227-1234 acknowledges record_touch uses
ON CONFLICT DO UPDATE, so touch_count drifts on every UPS fire
for every ring entry. Bug confirmed on the branch.

Both reviews proposed compatible fixes (sentinel / drop the ring
replay / INSERT OR IGNORE variant of record_touch). Picking
between them is the author's call; any of the three closes the
blocker.

Dropping attn:unblock — the request was for reviewers, two have
responded with a converging verdict. PR is now in author court, not
waiting on more reviews.

[robotrocketscience]

[release:review:clarke:2026-05-14T21:55:50Z]

[robotrocketscience]

[release:review:oppenheimer:2026-05-14T23:07:58Z]

[robotrocketscience]

Rebased on current main; ready to re-merge

Rebased onto github/main (32aa894e after #811 split + 9aa103b5 ζ rerank #826) — was NOT-FF after #826 merged at 23:13:53Z.

CHANGELOG conflict resolved: top-level CHANGELOG.md kept the post-#811 thin-index shape; the hot-path entry relocated to CHANGELOG/v3.md under ## [Unreleased] / ### Added, ahead of the ζ entry that landed via #826. Used the updated bullet from the prior fix(hook) review-fix commit (forward-only semantics, 22 tests) — content-identical to what was on the pre-rebase branch.

One commit-message scrub in the same pass: the fix(hook) commit body listed five session names by name in its first paragraph. Replaced that parenthetical with a generic (date) form. The substantive fix (drop ring replay, forward-only _record_touches) is unchanged. Diff vs main is identical to pre-rebase apart from the CHANGELOG relocation. Prior reviews still apply.

Pre-push:

• pytest -x -q --timeout=120 → 4244 passed, 62 skipped, 75 xfailed in 87s.
• Discretion grep on git diff main...HEAD → 0 hits; commit-message grep also clean.
• 7 commits, all signed (G), FF on current main.

Removing attn:merge-conflict, adding ready-to-merge.

[github-actions]

merge-train: merged 2dd86df → main via FF push.

[robotrocketscience]

Hot-path consumer-flip decision (status update)

Follow-up on this PR's deferred-consumer note. The touch-state substrate shipped here (the sidecar belief_touches table, INJECTION-only event writes, is_hot() predicate in src/aelfrice/hot_path.py, no production rerank caller) was intentionally dark; flipping a posterior-rerank multiplier to consume belief_touches.last_fire_idx was gated on a post-R7c lab measurement of production posterior-touch correlation.

That measurement has now been taken against two real <project>/.git/aelfrice/memory.db stores (aelfrice repo, 17886 beliefs / 2044 injection_events; an independent project at 1781 / 944). For each, Spearman ρ was computed between posterior_mean = α/(α+β) and per-session touch_count across the largest available session, in a touched-plus-sampled-untouched mix that matches the originally-modelled comparison shape:

• aelfrice corpus: ρ = +0.87 (n=14 touched + 14 untouched, session of 168 events)
• iceberg corpus: ρ = +0.72 (n=89 + 89, session of 214 events)

Both above the pre-committed 0.60 crossover (R7b synthetic sweep). Implication: the synthetic-baseline top-K shift signal that originally motivated a touch-temperature multiplier rerank consumer mostly evaporates at production correlation levels. Cross-corpus agreement (two independent project shapes) strengthens the read.

Decision: the consumer flip is not scheduled. No follow-up PR will introduce a posterior-rerank multiplier that reads belief_touches. The substrate columns and write-path shipped here remain in place — they're still useful for aelf doctor --hot-path read-back inspection and for any future consumer that wants a different mechanism — but the originally-modelled multiplier consumer is deferred-with-evidence.

Caveat: each corpus is a single-session measurement; N (touched-belief count) is modest. The verdict is suggestive-not-decisive at this dispatch budget. An extended sweep (multiple sessions per corpus, additional corpora) could tighten the call but is not required to land the deferral — both measurements landed cleanly above the crossover band.

Re-opening conditions: an extended sweep yielding ρ_mixed < 0.60 on a meaningful fraction of cells, or a different consumer mechanism that isn't a posterior-uncorrelated temperature multiplier.

No code change attached to this comment. Documentation amendment to docs/feature-hot-path.md to flip the "gated on a lab-side post-R7c campaign round" framing will follow as a separate PR.