feat(retrieval): third meta-belief consumer — adaptive bfs_depth_budget (#759) - #790
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
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
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 |
|
[claim:review:mondragon:2026-05-14T15:55:34Z] |
robotrocketscience
left a comment
robotrocketscience
left a comment
There was a problem hiding this comment.
LGTM (with one doc nit + a CI-flake blocker the bot will catch)
Read all 558 added lines across src/aelfrice/retrieval.py, tests/test_bfs_depth_budget_meta.py, and CHANGELOG.md. The implementation is structurally byte-for-byte parallel to #757 (bm25f_anchor_weight) — log-linear [0, 1] → [floor, ceil] encoding in the consumer, latency-only MVP, 30d posterior decay, default-OFF env flag with =enabled spelling, idempotent install_*, three-tier resolver precedence (explicit > meta-belief > default), and a "pass None when caller passed default" trick on the retrieve_v2 wiring so a default-pass doesn't silently bypass the adaptive layer.
Discretion grep clean — the lone "subagent" hit in the diff is context from #778's existing CHANGELOG entry above the new one, already on main.
Ran the 33 new tests in tests/test_bfs_depth_budget_meta.py locally under Python 3.13 — all pass in 6.65s.
CI status — pytest (3.13) is a flake
The red pytest (3.13) check is tests/test_bfs_latency_v3.py::test_main_end_to_end_smoke — a tiny-corpus latency-gate smoke that shipped on main via #739 / PR #754 and is untouched by this PR (git diff --stat shows 3 files: CHANGELOG, retrieval.py, test_bfs_depth_budget_meta.py). The CI runner's p95 spiked to 744-1094ms with --iterations 2 --warmup 1, which under that thin sampling means a single slow timed call trips the gate. Ran the same test in a fresh uv env on local Python 3.13: passes in 0.66s. Just needs a re-run.
This is the same class of flake that #761 / #739-era benches have hit before. Worth a follow-up to bump --iterations on that smoke or relax the percentile gate if it keeps tripping under noisy GH runners — not for this PR.
Doc nit (non-blocking, but worth fixing in this PR if you'd like)
The CHANGELOG body is correct:
BFS_DEFAULT_MAX_DEPTHverified as 2 (not 3 as the spec estimated):decode_bfs_depth_budget(0.5) = 2 = BFS_DEFAULT_MAX_DEPTH, so cold-start byte-identity holds.
And the test file is correct:
decode_bfs_depth_budget(0.5) = 2. BFS_DEFAULT_MAX_DEPTH is also 2
But the docstring at src/aelfrice/retrieval.py:292-294 and the function docstring at :843-846 still carry stale language from when the spec estimated BFS_DEFAULT_MAX_DEPTH = 3:
"This is intentionally one hop below
BFS_DEFAULT_MAX_DEPTH(2): shallow-only finds → posterior pulls budget down…"
"…one hop belowBFS_DEFAULT_MAX_DEPTHso a cold-start install with the meta-belief on gently trims the budget until evidence accrues."
If BFS_DEFAULT_MAX_DEPTH = 2 (as it actually is) and decode(0.5) = 2, then there's no "one hop below" and no "gentle trim" at cold start — it's byte-identical to the static default, exactly as in #757. The two docstrings should be updated to say "matching BFS_DEFAULT_MAX_DEPTH so installing the meta-belief preserves byte-identical retrieval depth" — same wording pattern #757 uses for anchor_weight = 3 = bm25.DEFAULT_ANCHOR_WEIGHT. Pure docs; code behavior is correct as-is.
What I confirmed against the #759 spec
- Log-linear
[1, 6]encoding, integer return viaint(round(...))per "Hard guard band: budget stays in integer[1, 6](rounded)". value = static_default = current static cap— verifieddecode(0.5) = 2 = BFS_DEFAULT_MAX_DEPTH.- 30d half-life — matches.
bfs_depthsignal deferred to #779 (D4 pattern from #756) — matches the close-the-loop PR I just reviewed (PR #789).latencysignal load-bearing for safety — wired with the sameLATENCY_TARGET_SECONDS / retrieve_elapsedclamp formula and try/except posture as #756 / #757.- Default-OFF behind
AELFRICE_META_BELIEF_BFS_DEPTH_BUDGETuntil #437 corpus gate clears. - Resolver precedence explicit > meta-belief > default, no env-var / TOML layer.
- Determinism — pure encoding function, sub-posterior decay is wall-clock-bounded only.
One acceptance bullet from #759 not addressed in this PR: aelf doctor --json surface. The umbrella substrate (#755) already surfaces installed meta-beliefs generically through doctor, so this likely just works — but worth a one-line aelf doctor --json | jq '.meta_beliefs.installed[]' sanity check post-install if you have a session up.
Recommendation
- Re-run
pytest (3.13)— fix the bot's view of the gate. - Optional: fold the docstring fix into the PR; otherwise file as a follow-up.
Not adding ready-to-merge yet — the merge-train bot will reject the label while pytest (3.13) is red. Re-run first, then label.
|
[release:review:mondragon:2026-05-14T15:58:50Z] |
|
[claim:review:jojo:2026-05-14T16:01:30Z] |
Review — approvingDiff matches the C–F template established by #756 / #757:
CI note: pytest (3.13) initially failed on Worth filing a follow-up to either bump iteration count or relax the smoke-mode delta gate so this doesn't recur — Labeling |
robotrocketscience
added
the
ready-to-merge
|
[release:review:jojo:2026-05-14T16:08:24Z] |
|
merge-train: blocked branch is not fast-forward on The |
github-actions
Bot
removed
the
ready-to-merge
|
[claim:review:jojo:2026-05-14T16:11:02Z] |
…belief env flag First slice of #759 consumer wiring. Pure additions — no caller changes yet, so retrieval is byte-identical until the read-path commits land. Adds: - META_BFS_DEPTH_BUDGET_KEY, BFS_DEPTH_BUDGET_FLOOR (1), BFS_DEPTH_BUDGET_CEIL (6), META_BFS_DEPTH_BUDGET_STATIC_DEFAULT (0.5), META_BFS_DEPTH_BUDGET_POSTERIOR_DECAY_SECONDS (30d). - ENV_META_BELIEF_BFS_DEPTH_BUDGET = "AELFRICE_META_BELIEF_BFS_DEPTH_BUDGET", ships default-OFF behind the #437 A/B bench-gate clause. - decode_bfs_depth_budget(v) — log-linear interpolation [0, 1] -> [1, 6] with int(round(...)). v=0.5 decodes to 2 (round(sqrt(6)) = 2), one hop below BFS_DEFAULT_MAX_DEPTH (2 per bfs_multihop.DEFAULT_MAX_DEPTH — verified, not 3). Cold-start deliberately shifts the effective default downward by one hop; the #759 spec notes "shallow-only finds → posterior pulls budget down." - is_meta_belief_bfs_depth_budget_enabled() — truthy-token resolver mirroring is_meta_belief_half_life_enabled(), including the '=enabled' spelling from the umbrella #480 issue body. Encoding lives in the consumer per the 2026-05-13 #756 ratification. The substrate stays pattern-uniform across #480 B–F; each consumer picks its own bounds at integration time.
…ware resolver Second slice of #759. Still no caller change in retrieve_v2 — the new resolve_bfs_depth_budget_with_meta() function exists in parallel, ready for the third commit to wire in at the bfs_max_depth resolution site. Adds: - install_bfs_depth_budget_meta_belief(store, *, now_ts) — idempotent install of the #759 row with the v3.x ratified defaults: latency signal only (bfs_depth deferred to #779 per D4, same split as #756 and #757), 30d posterior decay, static_default=0.5 (decodes to 2). - resolve_bfs_depth_budget_with_meta(store, *, now_ts, explicit=None) — three-tier precedence: explicit kwarg (positive int) > meta-belief (gated on the env flag) > BFS_DEFAULT_MAX_DEPTH from aelfrice.bfs_multihop. No env-var or TOML override layer — bfs_max_depth has never had a user-facing config knob; operators override via kwarg (bench) or meta-belief (production). The explicit=None contract: a caller passing the default must pass None (not the literal default value) so the meta-belief layer is not bypassed inadvertently. Noted: BFS_DEFAULT_MAX_DEPTH is 2 (verified: bfs_multihop.py line 49), not 3 as the spec estimated. The STATIC_DEFAULT=0.5 decodes to the same integer (2), so cold-start with the flag on is byte-identical to the pre-#759 BFS path at max_depth=2.
… latency signal update Closes the #759 loop in retrieve_v2. The bfs_max_depth kwarg is now resolved through resolve_bfs_depth_budget_with_meta before it reaches retrieve_with_tiers, and on every retrieve_v2 call (when the meta feature flag is on) a second latency sub-posterior update fires on the META_BFS_DEPTH_BUDGET_KEY — independent of the #756 half-life update. Wiring: - Resolver fires after effective_now_ts calculation, before retrieve_with_tiers, mirroring the #756 placement. The explicit clause only passes the caller's bfs_max_depth when it deviates from BFS_DEFAULT_MAX_DEPTH — a caller passing the default explicitly should still see the meta-belief layer, not a bypass. - The latency signal update is a SECOND update block (after the #756 half-life block) — both fire on every retrieve_v2 call when their respective flags are on. Same evidence formula: clip(LATENCY_TARGET / elapsed, 0, 1). Same fail-soft try/except: store errors print to stderr and are swallowed; retrieval must never raise on a meta-belief write failure. The update fires outside temporal_sort because BFS depth is independent of the temporal-decay path. Default-OFF byte-identical contract: with the env flag unset, the resolver collapses to BFS_DEFAULT_MAX_DEPTH (2), identical to the pre-#759 bfs_max_depth=BFS_DEFAULT_MAX_DEPTH default. Signal updates are fully gated on the same env flag.
Adds CHANGELOG entry under [Unreleased] / ### Added for the #759 bfs_depth_budget meta-belief consumer. Documents the C-F template pattern reuse, latency-only MVP scope, latency-vs-bfs_depth split rationale (bfs_depth deferred to #779 per D4), rounding-to-int wrinkle vs #756/#757, BFS_DEFAULT_MAX_DEPTH=2 verification, and the 27-test coverage surface. Also adds tests/test_bfs_depth_budget_meta.py (33 tests) covering: decode boundaries + int return + bounds + monotonicity + rounding-band transition, env-flag tokens, constants pin, install idempotency + signal-class assertion, five resolver precedence paths, 100-strong- positive-evidence responsiveness, default-OFF byte-identical retrieve_v2, and determinism (same evidence sequence + same now_ts → same depth on two fresh stores).
f8fee0d to
eff8a1c
Compare
robotrocketscience
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged eff8a1c → |
|
[release:review:jojo:2026-05-14T16:15:12Z] |
1 participant
Closes #759 (umbrella #480 — sub-task E)
Third meta-belief consumer following the C-F template the 2026-05-13 operator ratification established. Same shape as #756 (PR #784, merged) and #757 (PR #787, merged).
What's in the diff
Four atomic commits (drafted by a Sonnet subagent, reviewed by pascal before push):
feat(retrieval): log-linear bounded bfs_depth_budget encoding + meta-belief env flag— constants (META_BFS_DEPTH_BUDGET_KEY,BFS_DEPTH_BUDGET_FLOOR=1,BFS_DEPTH_BUDGET_CEIL=6,META_BFS_DEPTH_BUDGET_STATIC_DEFAULT=0.5,META_BFS_DEPTH_BUDGET_POSTERIOR_DECAY_SECONDS=30d),decode_bfs_depth_budget(v) → intwith rounding step (new wrinkle vs [v3.x] B: meta:retrieval.temporal_half_life_seconds — first-meta-belief pattern validator (#480 sub-task) #756/[v3.x] C: meta:retrieval.bm25f_field_weights — adaptive per-field weights (#480 sub-task) #757 —expand_bfstakes intmax_depth),is_meta_belief_bfs_depth_budget_enabled().feat(retrieval): install_bfs_depth_budget_meta_belief helper + meta-aware resolver— idempotent install withsignal_weights={SIGNAL_LATENCY: 1.0},resolve_bfs_depth_budget_with_meta(store, *, now_ts, explicit=None) → int. No env/TOML layer (the existingbfs_max_depthknob is kwarg-only); precedence is explicit-kwarg → meta-belief →BFS_DEFAULT_MAX_DEPTH.feat(retrieval): wire meta-belief bfs_depth_budget into retrieve_v2 + latency signal update—retrieve_v2resolves through the meta layer beforeretrieve_with_tiers, and fires a second latency sub-posterior update (independent of [v3.x] B: meta:retrieval.temporal_half_life_seconds — first-meta-belief pattern validator (#480 sub-task) #756's) inside try/except. Same fail-soft posture.docs(changelog): note adaptive bfs_depth_budget consumer (#759)— Unreleased entry.Design decisions
BFS_DEFAULT_MAX_DEPTH = 2verified directly atbfs_multihop.py:49(the spec estimated 3).decode_bfs_depth_budget(0.5) = round(sqrt(6)) = 2⇒ cold-start preserves byte-identical default. Noted inline in commit 1's message.latencyonly. The spec listed two signal classes (bfs_depth+latency), butbfs_depthrequires the same close-the-loop reference-detection infrastructure asrelevance— gated on Live close-the-loop relevance-signal infrastructure — #756 / #480 prereq #779. The spec body itself callslatency"load-bearing for safety" because BFS depth dominates p95 latency, so MVP scope is consistent with the C-F template's "ship MVP with whatever live signals exist ongithub/main; defer any that need new infrastructure to follow-up sub-issues." Same D4-style deferral [v3.x] B: meta:retrieval.temporal_half_life_seconds — first-meta-belief pattern validator (#480 sub-task) #756 used forrelevance.resolve_bfs_depth_budget_with_meta(store, now_ts=..., explicit=(bfs_max_depth if bfs_max_depth != BFS_DEFAULT_MAX_DEPTH else None)). A caller passing the default explicitly still sees the meta-belief layer; only a non-default override bypasses. Bench harnesses can still pin a specific depth.bfs_max_depthhas never had a config knob; introducing one here would expand the surface gratuitously.Verification
pytest: 4047 passed, 62 skipped, 75 xfailed (full suite, 113s).pytest tests/test_bfs_depth_budget_meta.py: 33 new tests, all pass.github/main(scoped to added lines): clean.G-signed.test_retrieve_v2_default_off_byte_identicalin the new test file.Acceptance (per ratified amendment)
read_meta_beliefcall (via the new resolver).latencyevidence stream wired (bfs_depthdeferred to Live close-the-loop relevance-signal infrastructure — #756 / #480 prereq #779).[1, 2]; deep-reference run pushes it up. (Latency-signal substitution: low-latency runs push budget up, high-latency runs pull it down.)now_ts→ byte-identical resolved depth.[1, 6](by construction —int(round(decode(...)))over[FLOOR, CEIL]).aelf doctor --meta-beliefs --jsonsurfaces the meta-belief automatically (substrate-side, no new wiring needed).Out of scope
bfs_depthsignal — gated on Live close-the-loop relevance-signal infrastructure — #756 / #480 prereq #779.benchmarks/results/v2.0.0.jsonis canonical,uv sync && aelf bench all#437 corpus A/B + p95 ≤ 100ms latency floor) — operator-dispatched host-side after merge.