feat(bench-canonical): calibrate v2.0.0 from 3-run pass (#437) - #489
Merged
Conversation
Mirror of the per_question strip rule — structmemeval emits a per_case list whose context strings bloat the canonical JSON to 6.4 MB on a clean 11/11 run. The detail isn't band-checked (tolerance.check_report walks summary leaves only), so it's pure diff weight. Test extended to assert both per_question and per_case are stripped from the merged output.
Replaces the partial first-pass canonical (6 of 11 invocations on git_commit bce8311, captured 2026-05-07) with a fully calibrated canonical from three back-to-back `aelf bench all --canonical` runs on git_commit 9e3f8de2 (after #473 fix shipped). Headline numbers (run-1, source of truth): mab/Conflict_Resolution f1=0.0065 sub_em=0.7025 N=800 mab/Test_Time_Learning f1=0.0001 sub_em=0.0929 N=700 mab/Long_Range_Understand. f1=0.1811 sub_em=0.0234 N=171 mab/Accurate_Retrieval f1=0.0106 sub_em=0.1530 N=2000 locomo overall_f1=0.0212 N=1986 longmemeval avg_latency_ms=5.55 N=500 structmemeval/location accuracy=0.9048 N=42 cases structmemeval/accounting accuracy=0.0000 N=15 cases structmemeval/recommend. accuracy=0.1540 N=84 cases structmemeval/tree accuracy=0.0000 N=22 cases amabench total_episodes=208 / qa=2496 Tolerance bands: spec called for observed range × 1.5 → metric override when required pct exceeds the default band. Across 3 runs, MAB and StructMemEval metrics are bit-deterministic (range = 0.0); LongMemEval avg_latency_ms varies 5.55–6.04 ms (well inside the default ±25% latency band). Result: metric_overrides = {} — defaults cover observed variance. Self-check: tolerance.check_report(canonical, canonical) → 36/36 pass. Cross-check: run-2 and run-3 against the canonical → 36/36 pass each. The earlier partial canonical's _calibration_notes referenced "missing /tmp/ data dirs" as the LoCoMo blocker and "_status: error" for StructMemEval × 4 (the #473 retrieve_v2 temporal_sort bug). Both are resolved at github/main HEAD. Closes the v2.0 reproducibility ship-gate. Per #437 acceptance: on a fresh clone, `uv sync --extra benchmarks && aelf bench all --canonical --out PATH` reproduces every published number within the documented bands.
Reviewer's GuideUpdates the benchmarks pipeline by stripping an additional detail field from merged outputs and recalibrating the v2.0.0 canonical benchmark results from a full 11/11 invocation pass, including tests that ratchet on the new canonical status and detail-stripping behavior. Sequence diagram for updated benchmark run and detail strippingsequenceDiagram
actor Dev
participant BenchDispatcher
participant BenchmarkRunner
participant StructMemEval
participant ResultsMerger
participant DetailStripper
participant CanonicalStore
participant ToleranceChecker
Dev->>BenchDispatcher: trigger_benchmarks()
BenchDispatcher->>BenchmarkRunner: run_all_adapters()
BenchmarkRunner->>StructMemEval: run_structmemeval()
StructMemEval-->>BenchmarkRunner: raw_results(per_question, per_case, metrics)
BenchmarkRunner-->>ResultsMerger: submit_results(raw_results)
ResultsMerger->>DetailStripper: strip_details(raw_results, _DETAIL_FIELDS_TO_STRIP)
DetailStripper-->>ResultsMerger: cleaned_results(no per_question, no per_case)
ResultsMerger-->>CanonicalStore: write_v2_0_0_canonical(cleaned_results)
Dev->>ToleranceChecker: check_against_canonical(run_results)
ToleranceChecker->>CanonicalStore: load_v2_0_0_canonical()
CanonicalStore-->>ToleranceChecker: canonical_results
ToleranceChecker-->>Dev: report(36_of_36_checks_pass)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis PR completes the v2.0.0 canonical benchmark calibration run from 2026-05-08, updating the baseline results file to reflect a full 11/11 adapter pass (upgrading from 6/11 partial). It extends output processing to strip per_case detail fields, updates dispatcher and badge tests to verify stripping and all-ok rendering, and fixes structmemeval from prior error state to successful completion. Changesv2.0.0 Canonical Calibration Completion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
robotrocketscience
added
the
attn:review
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/test_benchmarks_badge.py`:
- Around line 93-109: In the test_canonical_v200_full_pass docstring replace the
Unicode multiplication sign "×" (U+00D7) with a plain ASCII "x" to satisfy Ruff
RUF002; locate the triple-quoted string inside the test_canonical_v200_full_pass
function and edit the phrase "StructMemEval × 4" to "StructMemEval x 4" (no
other changes required).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7574a99e-4eff-4c10-ae90-eda7d040b9e8
📒 Files selected for processing (4)
benchmarks/results/v2.0.0.jsonbenchmarks/run.pytests/test_bench_dispatcher.pytests/test_benchmarks_badge.py
Comment on lines
+93
to
109
| def test_canonical_v200_full_pass(tmp_path): | ||
| """Sanity: today's checked-in canonical reports 11/11 ok. | ||
|
|
||
| Was 6/11 during the partial first-pass calibration on 2026-05-07 | ||
| (LoCoMo data missing + StructMemEval × 4 hitting the #473 | ||
| temporal_sort kwarg bug). Calibrated to 11/11 on 2026-05-08 once | ||
| #473 shipped and /tmp/LoCoMo + /tmp/StructMemEval data dirs were | ||
| populated. Ratchets to detect regression — flip back to a partial | ||
| cut would surface here. | ||
| """ | ||
| canonical = Path(__file__).parent.parent / "benchmarks" / "results" / "v2.0.0.json" | ||
| if not canonical.exists(): | ||
| pytest.skip("canonical baseline not present") | ||
| text = badge.compute_badge_text(canonical, today="2026-05-08") | ||
| assert "6/11 ok" in text | ||
| assert "11/11 ok" in text | ||
| assert text.startswith("reproducibility: ✅") | ||
|
|
There was a problem hiding this comment.
Replace ambiguous × (MULTIPLICATION SIGN) with x in the docstring.
Ruff RUF002 flags line 97: StructMemEval × 4 uses U+00D7 (×) which is visually ambiguous with the Latin letter x. A simple substitution silences the lint warning.
🔧 Proposed fix
- (LoCoMo data missing + StructMemEval × 4 hitting the `#473`
+ (LoCoMo data missing + StructMemEval x 4 hitting the `#473`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_canonical_v200_full_pass(tmp_path): | |
| """Sanity: today's checked-in canonical reports 11/11 ok. | |
| Was 6/11 during the partial first-pass calibration on 2026-05-07 | |
| (LoCoMo data missing + StructMemEval × 4 hitting the #473 | |
| temporal_sort kwarg bug). Calibrated to 11/11 on 2026-05-08 once | |
| #473 shipped and /tmp/LoCoMo + /tmp/StructMemEval data dirs were | |
| populated. Ratchets to detect regression — flip back to a partial | |
| cut would surface here. | |
| """ | |
| canonical = Path(__file__).parent.parent / "benchmarks" / "results" / "v2.0.0.json" | |
| if not canonical.exists(): | |
| pytest.skip("canonical baseline not present") | |
| text = badge.compute_badge_text(canonical, today="2026-05-08") | |
| assert "6/11 ok" in text | |
| assert "11/11 ok" in text | |
| assert text.startswith("reproducibility: ✅") | |
| def test_canonical_v200_full_pass(tmp_path): | |
| """Sanity: today's checked-in canonical reports 11/11 ok. | |
| Was 6/11 during the partial first-pass calibration on 2026-05-07 | |
| (LoCoMo data missing + StructMemEval x 4 hitting the `#473` | |
| temporal_sort kwarg bug). Calibrated to 11/11 on 2026-05-08 once | |
| `#473` shipped and /tmp/LoCoMo + /tmp/StructMemEval data dirs were | |
| populated. Ratchets to detect regression — flip back to a partial | |
| cut would surface here. | |
| """ | |
| canonical = Path(__file__).parent.parent / "benchmarks" / "results" / "v2.0.0.json" | |
| if not canonical.exists(): | |
| pytest.skip("canonical baseline not present") | |
| text = badge.compute_badge_text(canonical, today="2026-05-08") | |
| assert "11/11 ok" in text | |
| assert text.startswith("reproducibility: ✅") |
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 97-97: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF002)
🤖 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_benchmarks_badge.py` around lines 93 - 109, In the
test_canonical_v200_full_pass docstring replace the Unicode multiplication sign
"×" (U+00D7) with a plain ASCII "x" to satisfy Ruff RUF002; locate the
triple-quoted string inside the test_canonical_v200_full_pass function and edit
the phrase "StructMemEval × 4" to "StructMemEval x 4" (no other changes
required).
Owner
Author
|
[claim:review:Gylf:2026-05-08T15:38:02Z] |
Owner
Author
|
[release:review:Gylf:2026-05-08T15:39:08Z] |
This was referenced May 8, 2026
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #437.
What
Calibrates the v2.0.0 canonical reproducibility artifact from a fresh
3-run pass against the full 11/11 invocation set (now that #473 fixed
StructMemEval's
temporal_sortkwarg and #477/#479 closed thegate-commit follow-ups).
Two atomic commits:
fix(bench/run)— addsper_caseto_DETAIL_FIELDS_TO_STRIP.StructMemEval emits a
per_caselist whose context strings bloatthe canonical JSON to 6.4 MB on a clean 11/11 run. The detail isn't
band-checked, so it's pure diff weight. Test extended to assert both
per_questionandper_caseare stripped.feat(bench-canonical)— replaces the partial first-pass canonical(6/11 invocations on
bce8311) with the calibrated 11/11 canonicalon
9e3f8de.metric_overrides = {}because the default bandsalready cover observed variance.
Calibration evidence
Three back-to-back canonical runs on
9e3f8de(post-#473), each 11/11ok. Total wall time ~62 min (run-1: 24 min sequential, runs 2+3:
~20 min wall in parallel with isolated
TMPDIRs).Self-check:
tolerance.check_report(canonical, canonical)→ 36/36 pass.Cross-check: run-2 and run-3 against this canonical → 36/36 pass each.
Out of scope (followups)
_no-sub_keysentinel.
tolerance._walk_leavesskips keys starting with_toavoid
_status/_elapsed_sec, but that also skips the entiresingle-invocation adapter branch. Their accuracy metrics aren't
band-checked. The 36-leaf count above only covers the per-sub_key
adapters (mab × 4, structmemeval × 4 — minus a few zero-only metrics
the floor classifies as ±0.02 absolute). Filing a separate issue.
Test plan
uv run pytest tests/test_bench_dispatcher.py -q→ 11 passed_calibration_notesSummary by Sourcery
Calibrate the v2.0.0 benchmark canonical results to a full 11/11 successful invocation run and tighten output stripping for non-essential detail fields.
New Features:
Bug Fixes:
per_casedetail lists from merged benchmark outputs to prevent canonical result files from bloating with non-band-checked data.Tests:
per_questionandper_casefields are removed from merged outputs.Summary by CodeRabbit
Bug Fixes
Updates