test(session_id): ≥80% population guard via two-session ingest run (#554) - #561
Conversation
) Adds an integration-level assertion that at least 80% of beliefs in a synthetic two-session run carry a non-NULL session_id. The synthetic run drives 12 ingest calls across all three entry points (ingest_turn, cli._cmd_lock, mcp_server.tool_lock) into a single shared store, interleaving explicit session_id kwargs and AELF_SESSION_ID env fallbacks for session-a and session-b, with two NULL-session calls left as the intentional remainder so the population rate is meaningful rather than trivially 100%.
Reviewer's GuideAdds an integration-level test that performs a synthetic two-session ingest run across CLI, ingest_turn, and MCP tool surfaces against a shared MemoryStore, then asserts that at least 80% of resulting beliefs have a non-NULL session_id, providing a regression guard for session_id propagation coverage across entry points. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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 selected for processing (1)
✨ 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 |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The test reaches into
MemoryStore._connand assumes abeliefs(session_id)schema; if possible, prefer going through a public API or helper to avoid coupling the integration guard to internal implementation details. - The population-rate logic implicitly relies on the current number of ingest calls (10/12); consider computing the expected non-null/total values from the scenario definition and asserting those first so future edits to the scenario don’t accidentally weaken the guard while still clearing the 80% threshold.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test reaches into `MemoryStore._conn` and assumes a `beliefs(session_id)` schema; if possible, prefer going through a public API or helper to avoid coupling the integration guard to internal implementation details.
- The population-rate logic implicitly relies on the current number of ingest calls (10/12); consider computing the expected non-null/total values from the scenario definition and asserting those first so future edits to the scenario don’t accidentally weaken the guard while still clearing the 80% threshold.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[claim:review:Maxwell:2026-05-10T05:17:53Z] |
|
[release:review:Maxwell:2026-05-10T05:18:56Z] |
Closes #554. Adds a single integration-level test on top of the per-surface unit tests already in
tests/test_session_id_propagation.py.tests/test_session_id_population_rate.pydrives 12 ingest calls into one sharedMemoryStore:ingest_turn(2 explicit, 2 env-fallback) + 2_cmd_lock(1 explicit, 1 env-fallback).mcp_server.tool_lock(2 explicit, 2 env-fallback).ingest_turnwith neither explicitsession_idnor$AELF_SESSION_ID(the NULL remainder).Asserts
count(beliefs WHERE session_id IS NOT NULL) / count(beliefs) >= 0.80. The synthetic run yields 10/12 ≈ 83.3%, which is above threshold but with the NULL remainder kept non-zero so the threshold is meaningful.No production code changes. The integration guard catches regressions where a future surface lands without going through
resolve_session_idand silently inflates the NULL rate.Summary by Sourcery
Tests: