test(temporal_spine): G2 top-rank invariance evidence for the #1064 flip gate - #1074
Conversation
![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
📝 WalkthroughWalkthroughThis PR adds an optional G2 top-rank invariance evaluation mode to the temporal spine ablation benchmark via a new ChangesG2 Rank Invariance Benchmark
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Main
participant MemoryStore
participant RankInvarianceAccumulator
Main->>MemoryStore: retrieve (lane-off, use_temporal_spine=False)
MemoryStore-->>Main: baseline_ids, core_len_base
Main->>MemoryStore: retrieve (lane-on, use_temporal_spine=True)
MemoryStore-->>Main: spine_ids, core_len_spine
Main->>MemoryStore: last_lane_telemetry()
MemoryStore-->>Main: core boundary telemetry
Main->>RankInvarianceAccumulator: add(baseline_ids, spine_ids, core_len_base, core_len_spine, n_spine_added)
RankInvarianceAccumulator-->>Main: updated invariance metrics
Main->>Main: print/report passed()
Possibly related issues
🚥 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 |
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 |
robotrocketscience
added
the
attn:review
|
[claim:review:Gylf:2026-07-04T07:59:54Z] |
Review: APPROVETest + docs evidence for the #1064 G2 flip gate. Verified end-to-end: Refactor (lazy imports). Accumulator semantics are correct. API surface verified against the branch. Tests. All 8 pass locally (pure-append invariant, core drop/reorder = regression, BFS-tail eviction = not a regression, core-length mismatch fails, empty-acc fails, LCP tracking). Correctly mirror the implementation. Scope. Docs mark the bench-pool half DONE and correctly leave shadow-eval + G3 + the flip OPEN; PR body is "Part of #1064", not "Closes" — issue stays open. Correct. CI green (CodeRabbit/Sourcery rate-limited, no substantive automated findings). Discretion grep clean. FF-able, both commits signed. Over the advisory 200-line soft cap but cohesive — non-blocking. Adding |
robotrocketscience
added
the
ready-to-merge
|
[release:review:Gylf:2026-07-04T08:04:12Z] |
|
merge-train: blocked branch is not fast-forward on The |
github-actions
Bot
removed
the
ready-to-merge
…1064) Adds a `--rank-invariance` mode to `temporal_spine_ablation.py` that pairs lane-off vs lane-on retrieval per question on the same store and verifies the flip-gate's "no top-rank regression" criterion: the spine lane appends after the [locked, l25, l1, hrr] core and before BFS, so the core prefix must stay identical between arms. The pass reads `last_lane_telemetry()` to locate the core boundary exactly, then asserts no core belief is displaced or reordered; BFS-tail eviction (the lane spending budget above the lowest lane) is reported separately as by-design, not a regression. `RankInvarianceAccumulator` carries the aggregates and a `passed()` gate. `test_temporal_spine_ablation.py` pins the accumulator math (head invariance, core displacement, core reorder, BFS-tail eviction, core-length mismatch, empty guard) on hand-built id lists — no LoCoMo dataset needed, runs in the pytest matrix. The LoCoMo dataset stack (`locomo_adapter` -> `nltk`) is now imported lazily inside the functions that run the bench, so this module's pure logic imports — and unit-tests — without the benchmark dependency set.
The bench-pool half of the temporal-spine flip gate's G2 criterion is done: LoCoMo10 at budget 1500 / l1-limit 50 gives +19.45pp coverage (survives the production trim, exceeding the wide-budget dev figure) with 1,986/1,986 questions core-prefix invariant and 0 top-rank displacements. Records the result and the reproduce command, and splits G2 into the completed bench half and the still-open shadow-eval half (aggregate-only run on a real backfilled store).
e3fbc7a to
33b7ddd
Compare
robotrocketscience
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged 33b7ddd → |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
benchmarks/temporal_spine_ablation.py (1)
218-303: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a bounds guard for the telemetry-derived core boundary.
core = core_len_baseis used directly to slicebaseline_ids/spine_ids. This relies on the invariant thatcore_len_base <= len(baseline_ids)(and similarly for spine) always holding, per theLaneTelemetrycontract where packed counts should equal what lands inresult.beliefs. If that invariant is ever violated by an upstream change, slicing silently truncates instead of erroring, and the check could silently reporthead_invariant=Truefor a boundary that doesn't actually reflect the intended core — precisely the kind of false-positive this accumulator exists to prevent.🛡️ Proposed defensive check
self.n_questions += 1 if core_len_base != core_len_spine: self.core_mismatch += 1 core = core_len_base + assert core <= len(baseline_ids) and core <= len(spine_ids), ( + "core boundary exceeds retrieved belief count — telemetry and " + "result list are out of sync" + ) if baseline_ids[:core] == spine_ids[:core]: self.head_invariant += 1Since this depends on the packed-count contract of
LaneTelemetry/retrieve_v2insrc/aelfrice/retrieval.py, worth confirming that invariant can never be violated in practice before deciding whether to add the guard.🤖 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 `@benchmarks/temporal_spine_ablation.py` around lines 218 - 303, The `RankInvarianceAccumulator.add` method uses `core = core_len_base` directly to slice `baseline_ids` and `spine_ids`, which can hide upstream contract violations by silently truncating. Add a defensive bounds check before computing `baseline_ids[:core]` and `spine_ids[:core]` so the telemetry-derived core boundary is validated against both lists. If the boundary is invalid, fail fast or otherwise mark the sample as bad rather than letting `head_invariant` be computed on an out-of-range core; keep the fix localized to `RankInvarianceAccumulator.add` and its `core_len_base`/`core_len_spine` inputs.
🤖 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.
Nitpick comments:
In `@benchmarks/temporal_spine_ablation.py`:
- Around line 218-303: The `RankInvarianceAccumulator.add` method uses `core =
core_len_base` directly to slice `baseline_ids` and `spine_ids`, which can hide
upstream contract violations by silently truncating. Add a defensive bounds
check before computing `baseline_ids[:core]` and `spine_ids[:core]` so the
telemetry-derived core boundary is validated against both lists. If the boundary
is invalid, fail fast or otherwise mark the sample as bad rather than letting
`head_invariant` be computed on an out-of-range core; keep the fix localized to
`RankInvarianceAccumulator.add` and its `core_len_base`/`core_len_spine` inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bb9caf78-c67f-47da-9bdd-43ace03f2ec6
📒 Files selected for processing (3)
benchmarks/temporal_spine_ablation.pydocs/design/feature-temporal-spine.mdtests/test_temporal_spine_ablation.py
1 participant
What
Part of #1064 (temporal-spine flip gate). Produces the missing G2
top-rank invariance evidence and records the result. The lane + writer
already landed default-off in #1069; this is flip-gate evidence tooling, not
a product change — both flags stay default-off and the issue stays open.
The G2 question
G2 requires, at the production hook budget (1500 tokens), both a coverage
delta ≥ +3pp and no top-rank regression. The coverage half was already
measured; the "no top-rank regression" half had no measurement. This adds it.
The spine lane appends its hits after the
[locked, l25, l1, hrr]coreand before BFS (
retrieval.py§ temporal-spine lane), so the claim undertest is: turning the lane on must never displace or reorder a core belief — it
may only insert its own hits below the core (and, under budget, evict
BFS-tail items, which is by-design since BFS is the lowest-priority lane).
Change
benchmarks/temporal_spine_ablation.pygains--rank-invariance: a pairedlane-off/lane-on retrieval per question on the same store. It reads
last_lane_telemetry()to locate the core boundary(
locked + l25 + l1 + hrr_expand) exactly, then asserts the core prefixis identical between arms and that no dropped baseline belief sits inside the
core.
RankInvarianceAccumulatorcarries the aggregates + apassed()gate.tests/test_temporal_spine_ablation.py(8 tests): pins the accumulator math— head invariance, core displacement, core reorder, BFS-tail eviction,
core-length mismatch, empty-guard, LCP tracking — on hand-built id lists, no
dataset needed.
docs/design/feature-temporal-spine.md: records the result and splits G2into the completed bench half and the still-open shadow-eval half.
Result — LoCoMo10,
--budget 1500 --l1-limit 50 --rank-invarianceCoverage (production trim):
The gain survives the production trim and in fact exceeds the wide-budget
dev figure; the seeded shuffled control recovers almost nothing, so the value
is the chronology, not the added density.
Top-rank invariance (1,986 questions):
the core)
What this does not close
backfilled store (open question 1: chain-length distribution under
production
session_idsemantics).false-negatives under concurrent load).
clear.
Reproduce
Summary by CodeRabbit
New Features
Documentation
Tests