feat(retrieval): temporal-spine lane onto production retrieve() + flip default ON (#1107 phase 2, #1064) - #1114
Conversation
…im (#1107 phase 2) The #1107 Phase-1 shim hard-forced all six staged lanes off. Graduate the temporal-spine lane (#1064, the pilot) to resolver control: pass use_temporal_spine=None so the production retrieve() path honours is_temporal_spine_enabled (env -> TOML -> default) exactly like the eval surface. Behaviour-preserving on its own — the resolver default is still False, so the lane stays off until the default flip. The other five lanes remain hard-off.
…#1107 phase 2) Operator-directed lane release (2026-07-06). is_temporal_spine_enabled now defaults True, so the production retrieve() shim (resolver-driven since the prior commit) runs the temporal-spine lane on every host. Completes the #1064 flip end-to-end: writer default-ON shipped in #1111, lane default-ON here. Every pre-registered gate cleared (G1 +14.6pp LoCoMo coverage, G2 trim survival + top-rank invariance, G3 latency delta_p95 in-band, G5 determinism). Opt out with AELFRICE_TEMPORAL_SPINE=0 or [retrieval] use_temporal_spine = false. Tests: test_lane_default_off_flag -> test_lane_default_on_flag + a new test_lane_explicit_opt_out (env/TOML force off). The retrieve_v2 equivalence guard's baseline (LANES_OFF -> SHIM_LANES) now mirrors the shim exactly — spine resolver-driven, five lanes off — and the inverted test_retrieve_keeps_staged_lanes_off becomes test_shim_runs_temporal_spine_lane_others_off, proving on a TEMPORAL_NEXT corpus that the shim runs the spine lane and the other five stay off. Full suite 5849 passed.
…1064, #1107 phase 2) Reflect the lane flip end-to-end: CONFIG.md (the [retrieval] use_temporal_spine lane is default-on/live on the production retrieve() path, opt-out documented), feature-temporal-spine.md (status writer+lane both default-ON; #1107 Phase-2 cutover exposes the lane on the live hook path), and a CHANGELOG entry for the #1107 production convergence onto retrieve_v2 with temporal spine as the first graduated lane.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
robotrocketscience
added
the
attn:review
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe temporal-spine retrieval lane is flipped to default ON after the ChangesTemporal-spine default-ON cutover
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/aelfrice/retrieval.py (1)
3315-3325: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStale "default-OFF" docstrings now contradict the resolver's new default.
Both the
retrieve_with_tierscomment ("#1064temporal-spine lane (additive, default-OFF)", Line 3315) and theretrieve_v2docstring section foruse_temporal_spine("Default-OFF — the default-ON flip is gated on the pre-registered#1064criteria", Lines 3524-3525) were not updated alongside theis_temporal_spine_enableddefault flip toTrue. These now describe the opposite of actual behavior and will mislead future readers/maintainers.📝 Proposed fix
- # `#1064` temporal-spine lane (additive, default-OFF). Traverses + # `#1064` temporal-spine lane (additive, default-ON since `#1107` + # Phase 2). Traverses # TEMPORAL_NEXT chains from the top-5 packed L1 seeds, both- appends the chronological neighbours to the candidate set. The - lane is a no-op (byte-identical output) when the store has zero - TEMPORAL_NEXT edges. Default-OFF — the default-ON flip is gated - on the pre-registered `#1064` criteria. Opt in via - `AELFRICE_TEMPORAL_SPINE=1`, the kwarg, or - `[retrieval] use_temporal_spine = true`. + appends the chronological neighbours to the candidate set. The + lane is a no-op (byte-identical output) when the store has zero + TEMPORAL_NEXT edges. Default-ON since the `#1107` Phase 2 cutover + (the pre-registered `#1064` criteria all cleared). Opt out via + `AELFRICE_TEMPORAL_SPINE=0`, the kwarg, or + `[retrieval] use_temporal_spine = false`.Also applies to: 3518-3527
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aelfrice/retrieval.py` around lines 3315 - 3325, Update the stale temporal-spine documentation to match the new resolver default. In `retrieve_with_tiers` and the `retrieve_v2` docstring for `use_temporal_spine`, change the “default-OFF” wording to reflect that `is_temporal_spine_enabled` now defaults to True, and adjust any adjacent notes about the `#1064` gating so they no longer describe the opposite behavior.
🧹 Nitpick comments (1)
tests/test_retrieve_v2_equivalence.py (1)
269-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated clustered-corpus setup.
The "other five staged lanes stay off" section (Lines 270-291) re-implements the same clustered-corpus construction already used in
test_equivalence_clustered_corpus(Lines 186-198). Consider extracting a shared_build_clustered_corpus(store)helper to avoid the two copies drifting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_retrieve_v2_equivalence.py` around lines 269 - 291, The clustered-corpus setup is duplicated in the staged-lanes guard test and the existing clustered-corpus equivalence test, so factor that repeated MemoryStore/Edge construction into a shared helper (for example, a `_build_clustered_corpus` function) and reuse it from both cases. Update the assertions in `test_retrieve_v2_equivalence` to call the helper so the corpus shape stays identical across tests and the two copies cannot drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/aelfrice/retrieval.py`:
- Around line 3315-3325: Update the stale temporal-spine documentation to match
the new resolver default. In `retrieve_with_tiers` and the `retrieve_v2`
docstring for `use_temporal_spine`, change the “default-OFF” wording to reflect
that `is_temporal_spine_enabled` now defaults to True, and adjust any adjacent
notes about the `#1064` gating so they no longer describe the opposite behavior.
---
Nitpick comments:
In `@tests/test_retrieve_v2_equivalence.py`:
- Around line 269-291: The clustered-corpus setup is duplicated in the
staged-lanes guard test and the existing clustered-corpus equivalence test, so
factor that repeated MemoryStore/Edge construction into a shared helper (for
example, a `_build_clustered_corpus` function) and reuse it from both cases.
Update the assertions in `test_retrieve_v2_equivalence` to call the helper so
the corpus shape stays identical across tests and the two copies cannot drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d94011cf-6a95-484d-a445-87a82c57c359
📒 Files selected for processing (6)
CHANGELOG/v3.mddocs/design/feature-temporal-spine.mddocs/user/CONFIG.mdsrc/aelfrice/retrieval.pytests/test_retrieve_v2_equivalence.pytests/test_temporal_spine.py
|
[claim:review:Setr:2026-07-07T00:14:35Z] |
Review — temporal-spine lane onto production
|
|
[release:review:Setr:2026-07-07T00:18:56Z] |
…#1064, #1107 phase 2) The Phase-2 flip updated the resolver docstring, doc files, and CONFIG.md but left two in-code docstrings asserting the old default-OFF posture: retrieve_with_tiers' lane comment and retrieve_v2's use_temporal_spine entry. Both now describe default-ON with the opt-out env/kwarg/TOML paths, matching is_temporal_spine_enabled's flipped default.
|
Addressed the CodeRabbit maintainability finding in
The doc files (CHANGELOG, CONFIG.md, design doc) were already correct; these were the two the flip missed. Pushed FF onto the branch ( Declined the nitpick to extract a shared The branch is merge-ready. The lane-default-ON flip itself remains the operator's release call per the ratified #1064 gate — not adding |
|
[claim:review:Setr:2026-07-07T00:23:32Z] |
robotrocketscience
added
the
ready-to-merge
|
merge-train: merged 3a6814a → |
github-actions
Bot
removed
the
ready-to-merge
|
[release:review:Setr:2026-07-07T00:25:13Z] |
1 participant
What
#1107 Phase 2 — graduate the temporal-spine lane onto the production
retrieve()path and flip its default ON. Operator-directed lane release (2026-07-06). Completes the #1064 flip end-to-end: the writer shipped default-ON in #1111; this makes the retrieval lane default-ON and live on the production hook path.Commits (atomic)
feat(retrieval): mechanism — the epic(retrieval): converge production hook onto retrieve_v2 — staged lanes are not on the live path #1107 Phase-1 shim hard-forced all six staged lanes off. Passuse_temporal_spine=Noneso productionretrieve()honoursis_temporal_spine_enabled(env → TOML → default) exactly like the eval surface. Behaviour-preserving on its own (resolver default still False). The other five lanes stay hard-off.feat(retrieval): flip —is_temporal_spine_enableddefaultFalse → True. The spine lane is now live on every host'sretrieve(). Tests:test_lane_default_off_flag → test_lane_default_on_flag+ newtest_lane_explicit_opt_out; the equivalence baselineLANES_OFF → SHIM_LANES(spine resolver-driven, five off) now mirrors the shim exactly, and the invertedtest_retrieve_keeps_staged_lanes_offbecomestest_shim_runs_temporal_spine_lane_others_off(proves, on aTEMPORAL_NEXTcorpus, the shim runs spine and the other five stay off).docs: CONFIG.md + feature-temporal-spine.md (status: writer+lane both default-ON) + CHANGELOG entry.Verification
benchmarks/temporal_spine_latency.py, 10k beliefs / 200 sessions):Δp50 = −0.01ms,Δp95 = +26.27msvs theΔp95 ≤ 50msgate; lane fired 30/30 queries.tests/test_retrieve_v2_equivalence.py).Scope / relationship to #1092
Refs #1064, #1107 (Phase 2). Does not close the #1107 epic — the remaining five staged lanes graduate individually behind their own latency gates. This delivers what the #1092 HOLD draft's lane-half would; #1092 can be closed/reduced once this lands.
Opt-out
AELFRICE_TEMPORAL_SPINE=0or[retrieval] use_temporal_spine = false.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests