fix(rescore): handle uncategorised results without dividing by zero - #61
Conversation
The rescore script's print_scorecard buckets results by LoCoMo's integer category (1-4) and computed totals as `tot["count"] += len(rs)` inside the per-category loop. LongMemEval-KU results have no category field (always None), so the loop ran zero iterations, `tot["count"]` stayed at 0, and the next line — `all_f1 = tot["f1"] / n` — raised `ZeroDivisionError` after the script had already done all the actual rescore work. Result: every LMEKU rescore failed AT THE END with the heavy lifting wasted, including LMEKU Phase A Cell 1 (`lmeku_baseline-vector_gemma4_e2b`) which finished today with all 78 external-judge values populated but no scorecard. Now: when no LoCoMo categories matched but results exist, fall back to a single Overall bucket using all results. The per-category breakdown stays unchanged for LoCoMo runs. An empty results list prints "no results" instead of crashing. Also extracted the per-bucket computation into `_row_stats()` so the LoCoMo and fallback paths share the same code. 4 new regression tests cover: LoCoMo with categories (positive), LMEKU shape with no categories (regression), empty results (edge case), partial rescore coverage. All 127 tests pass. Recovered Cell 1 number from the existing rescored_v2 JSON: LongMemEval-KU baseline-vector / gemma4:e2b → 0.5385 ext-judge (78/78 rescored).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors per-bucket metric computation into a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
Review rate limit: 8/10 reviews remaining, refill in 8 minutes and 24 seconds. 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)
benchmarks/locomo_rescore_streaming.py (1)
175-216:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOverall can undercount when results are mixed categorized + uncategorised.
If at least one category in
[1,2,3,4]exists, uncategorised rows are excluded fromtotand therefore omitted fromOverall. This can silently bias aggregate metrics/counts on partially malformed inputs.Proposed fix (include uncategorised rows in overall aggregation when mixed)
def print_scorecard(path: Path, data: dict, judge_model: str) -> None: @@ - for c in [1, 2, 3, 4]: + known_cats = {1, 2, 3, 4} + for c in [1, 2, 3, 4]: rs = [r for r in results if r.get("category") == c] if not rs: continue @@ - n = tot["count"] + # Include uncategorised leftovers in Overall if the dataset is mixed. + leftovers = [r for r in results if r.get("category") not in known_cats] + if tot["count"] > 0 and leftovers: + s = _row_stats(leftovers) + m = len(leftovers) + tot["count"] += m + tot["f1"] += s["f1"] * m + tot["orig"] += s["orig"] * m + tot["tol"] += s["tol"] * m + tot["rj_sum"] += sum(s["rv"]) + tot["rj_n"] += len(s["rv"]) + + n = tot["count"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/locomo_rescore_streaming.py` around lines 175 - 216, The Overall aggregation currently only accumulates rows in categories 1–4 into tot, so uncategorised results (category missing or not in 1..4) get omitted when at least one category exists; to fix, after the for c in [1,2,3,4] loop compute an "uncat" list = [r for r in results if r.get("category") not in (1,2,3,4)] and if uncat: compute s = _row_stats(uncat) and add its contributions to tot exactly like the per-category updates (increment tot["count"], add s["f1"]*len(uncat), s["orig"]*len(uncat), s["tol"]*len(uncat), tot["rj_sum"] += sum(s["rv"]), tot["rj_n"] += len(s["rv"])); this ensures n = tot["count"] and the Overall calculations (all_f1, all_orig, all_tol, all_rj, all_delta, cov_all) include uncategorised rows when present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@benchmarks/locomo_rescore_streaming.py`:
- Around line 175-216: The Overall aggregation currently only accumulates rows
in categories 1–4 into tot, so uncategorised results (category missing or not in
1..4) get omitted when at least one category exists; to fix, after the for c in
[1,2,3,4] loop compute an "uncat" list = [r for r in results if
r.get("category") not in (1,2,3,4)] and if uncat: compute s = _row_stats(uncat)
and add its contributions to tot exactly like the per-category updates
(increment tot["count"], add s["f1"]*len(uncat), s["orig"]*len(uncat),
s["tol"]*len(uncat), tot["rj_sum"] += sum(s["rv"]), tot["rj_n"] +=
len(s["rv"])); this ensures n = tot["count"] and the Overall calculations
(all_f1, all_orig, all_tol, all_rj, all_delta, cov_all) include uncategorised
rows when present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 41767194-f6a4-4488-9362-73d199d190b1
📒 Files selected for processing (2)
benchmarks/locomo_rescore_streaming.pytests/test_locomo_rescore_streaming.py
CodeRabbit caught an edge case in the previous fix: when a results
JSON is mixed (some rows have category in {1,2,3,4}, some don't), the
LoCoMo path silently dropped the uncategorised rows from the Overall
aggregate. The pure-LMEKU path (all rows uncategorised) was already
covered; this is the in-between case.
Now: after the per-category loop, any leftover rows whose category is
not in {1,2,3,4} are folded into the running totals before the Overall
line is printed. Empty results still print the (no results) placeholder;
all-uncategorised still falls back to a single Overall bucket.
Adds a regression test that asserts a 2-categorised + 2-uncategorised
input produces an Overall count of 4, not 2.
|
Pushed `d11b5f5` addressing the mixed-categories finding. Real edge case — when the dataset has some rows with `category in {1,2,3,4}` and some without, the previous fix silently dropped the uncategorised rows from Overall. Now folds them in before computing the totals, with a regression test (2 categorised + 2 uncategorised → Overall count = 4, not 2). Pure-LMEKU and pure-LoCoMo paths are unchanged. Note: a flaky failure in `test_agents.py::test_reflect_gate_disabled_returns_empty` showed up once when running the full suite (passes on isolated re-run, passes on full re-run). That's pre-existing on master — `tests/test_api.py` from #60 holds onto SQLite handles in its fixture cleanup that occasionally leak into `test_agents.py`. Not introduced here. Worth a separate small PR. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by grok-code-fast-1:optimized:free · 120,134 tokens |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@benchmarks/locomo_rescore_streaming.py`:
- Around line 160-166: The printed banner currently hardcodes "LoCoMo Rescore
(streaming)" which is misleading for LongMemEval-KU inputs; change that literal
to a benchmark-neutral heading such as "Rescore (streaming)" or "Rescore Results
(streaming)" where the banner string is emitted (replace the "LoCoMo Rescore
(streaming)" literal used when printing the scorecard in this function/docstring
area) so the output no longer claims a specific benchmark when falling back to
the overall bucket.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7503eecb-d814-4db3-89c0-172ea032a8af
📒 Files selected for processing (2)
benchmarks/locomo_rescore_streaming.pytests/test_locomo_rescore_streaming.py
1 participant
Summary
`benchmarks/locomo_rescore_streaming.py` crashed at the very end of every LongMemEval-KU rescore with `ZeroDivisionError: division by zero`. The script bucketed results by LoCoMo's integer category (1-4) and accumulated `tot["count"] += len(rs)` inside the per-category loop. LMEKU results have no category field (always `None`), so the loop ran zero iterations, `tot["count"]` stayed at 0, and `all_f1 = tot["f1"] / n` raised — after all the heavy external-judge work was already done.
This bit LMEKU Phase A Cell 1 (`lmeku_baseline-vector_gemma4_e2b`) earlier today: bench finished, all 78 `judge_rejudged` values landed in the JSON, but no scorecard printed.
What changes
What does NOT change
Recovered number
Cell 1 already had all 78 rejudge values in its `rescored_v2.json`, so its number is recoverable today:
The other 5 cells in Phase A weren't run — the chain was killed because the `vector+kg` and `full+supersede` configs were taking ~45-100 min/QA on ingest (the KG-extraction LLM-call volume), which would push Phase A to ~80h+ instead of the original ~4.5h estimate. A redesigned Phase A is for a separate session.
Test plan
Summary by CodeRabbit