fix(bench): separate retrieval quality from reader-dependent scores, and say what cannot be measured (#1160)

[robotrocketscience]

Closes the last open acceptance criterion on #1160: "Separate retrieval-quality metrics (recall@k, MRR over the retrieved set) from reader-dependent metrics; report exact_match and LoCoMo cat-5 as n/a rather than 0.0." The other four ACs landed in #1213, #1195/#1194, #1209/#1200 and #1197.

The problem

aelf bench all runs no reader — there is no generation pass in benchmarks/run.py. Every adapter joins the retrieved beliefs into one string and hands it to a scorer written for a model's answer. Two things follow.

Token-F1 measures the token budget. Precision between ~2000 tokens of context and a three-token gold is about 3/2000, so halving the budget roughly doubles the reported F1 while retrieving strictly less. There was no metric in the canonical cut that moved the other way.

Some metrics were never measurements at all. exact_match asks for whole-normalised-string equality against a short gold; a multi-hundred-token context never satisfies it, at any retrieval quality, for any corpus. LoCoMo category 5 scores a refusal, and nothing in this path can refuse — the < 10 words ⇒ refusal heuristic meant to rescue it is unreachable at any realistic budget. Both reported 0.0, which is the worst possible score, so the canonical file read as a total retrieval failure. Worse, a tolerance band around 0.0 turned any genuine fix into a band excursion.

What this does

benchmarks/retrieval_metrics.py — MRR and recall@k over the ordered retrieved list, before it is joined. Retrieval fills the budget in rank order, so a smaller budget truncates the tail: these are monotone non-decreasing in the budget, and moving them requires ranking a relevant belief higher. That property is pinned by a test rather than asserted in prose. Multi-answer gold is treated as alternative surfaces of one answer, matching qa_scoring.score_multi_answer, and gold that normalises to empty (an article, a bare .) is guarded — "" in anything would otherwise award a free hit.

benchmarks/metric_status.py — the n/a sentinel and the _not_applicable reason block. Underscore-prefixed, so the reason rides in the artifact without landing on the band-check walk.

Adapters — locomo, mab, longmemeval and amabench each report a retrieval_quality block beside the existing scores, and mark the uncomputable metrics n/a. LoCoMo additionally drops category 5 from overall_f1, which becomes the mean over the categories that were scored; total_qa is untouched (it is a corpus invariant the band-check watches for drift) and scored_qa is reported beside it.

benchmarks/tolerance.py — Verdict.NOT_APPLICABLE. Without it, n/a lands on the existing "observed leaf is not numeric" branch and reports FAIL, converting an adapter's honest this cannot be computed into a fabricated regression. It is separate from SKIP because the two demand different responses: SKIP means fix the runner, n/a means the metric needs a reader or needs deleting. It rolls up like SKIP and, critically, does not count as evidence anything was measured — so an all-n/a run is NO_DATA, not PASS.

Effect on the nightly

benchmarks/results/v2.0.0.json predates this and still records 0.0 for the affected leaves. It cannot be recut from here — that needs the real datasets — so the coupling is exercised against the shipped file directly in test_new_adapter_shape_never_fails_the_shipped_canonical:

• Five leaves resolve to NOT_APPLICABLE (four MAB splits + LoCoMo category 5). LongMemEval's and AMA-Bench's exact_match also go n/a, but their canonical cut predates the metric, so those leaves are not in the tree being walked.
• locomo.overall_f1 can only rise. It is higher-is-better, so leaving the band is a WARN on the improving side (fix(bench): enforce tolerance bands on the regression side only (#1160) #1209), not a FAIL. Swept across category-5 shares from 0 to ~70% of the corpus: worst case is WARN, and bench-canonical.yml exits 0 on WARN.

Recutting the canonical baseline is not in this PR. It is operator-run and, per the standing note that every v3.0.1* file is schema 1.0 so the nightly cannot simply be repointed, a recut is its own piece of work.

Deliberately out of scope

• Renaming f1 → context_token_overlap. The finding's prose suggests it; the AC does not. A rename orphans every canonical leaf path and forces the recut above. substring_exact_match and f1 stay numeric — they are reader-dependent, and the docstrings now say so, but unlike exact_match they do move with retrieval, so they remain measurements.
• mab_adapter's forked scorer (the separate low-severity finding on the umbrella, including its missing empty-gold guard). Untouched here.
• score_qa's category-5 branch is kept, not deleted: it mirrors LoCoMo's own evaluation.py and is correct for a reader's answer. Wire a reader in and it is live again unchanged.

Verification

• Full suite: 6569 passed, 69 skipped, 71 xfailed.
• 39 new tests across test_retrieval_metrics.py, test_bench_metric_separation.py and additions to test_bench_tolerance.py.
• Each behavioural test carries a distinguishing assert — reverting the guard changes the result rather than leaving the test vacuously green. The three that matter: test_na_leaf_is_not_applicable_not_fail (reads FAIL without the sentinel branch), test_overall_f1_excludes_the_unscorable_categories (asserts the old 0.4 divisor alongside the new 0.5), and test_amabench_per_group_breakdown_does_not_rebuild_the_zero (an n/a contributes nothing to the group sum, so dividing by the row count silently reconstructs the zero).
• scripts/check_changelog_dupes.py clean; CHANGELOG edit is insert-only.

Refs #1160

Summary by Sourcery

Separate reader-dependent correctness metrics from reader-independent retrieval quality across benchmark adapters, and mark structurally uncomputable metrics as not applicable rather than failed, while adjusting nightly tolerance handling accordingly.

New Features:

• Introduce shared rank-based retrieval metrics (MRR and recall@k) that operate over the ordered retrieved beliefs and expose them as a retrieval_quality block in blob-scoring benchmark adapters.

Bug Fixes:

• Stop treating adversarial LoCoMo category 5 and exact_match in blob-scoring adapters as real zero scores by reporting them as not applicable and excluding them from aggregates.
• Ensure tolerance handling treats not-applicable metrics as a distinct verdict that neither contributes to pass/fail rollups nor allows all-uncomputable runs to be considered successful.

Enhancements:

• Refine LoCoMo, LongMemEval, AMA-Bench, and MAB adapters to track per-question retrieval rankings alongside answer scores and to clarify reader-dependent vs reader-independent metrics in their outputs.
• Adjust LoCoMo overall F1 to average only over scorable categories and add explicit counts of scored vs total QA pairs.
• Extend tolerance direction metadata so retrieval quality metrics are classified as higher-is-better and band-checked one-sided on the regression side.
• Add tests that lock in separation of metric families, correct handling of not-applicable leaves, and monotonicity of retrieval metrics with respect to retrieval budget.

Documentation:

• Document the distinction between reader-dependent and reader-independent benchmark metrics, including how not-applicable metrics are represented and interpreted in canonical results.
• Update the changelog to describe the new retrieval_quality metrics, not-applicable sentinel semantics, and their impact on canonical baselines and nightly checks.

Tests:

• Add targeted tests for retrieval_metrics, metric_status, and tolerance summarization to ensure correct classification of not-applicable metrics and one-sided bands for retrieval quality.
• Add adapter-level tests validating emission of retrieval_quality, n/a handling for uncomputable metrics, and compatibility of the new shapes with the shipped canonical results.

[sourcery-ai]

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Review limit reached

@robotrocketscience, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 152c1c1c-1232-42fe-8978-fb1da6a2abcc

📥 Commits

Reviewing files that changed from the base of the PR and between 30ce1e5 and fdc47f2.

📒 Files selected for processing (12)

• CHANGELOG/v4.md
• benchmarks/amabench_adapter.py
• benchmarks/locomo_adapter.py
• benchmarks/longmemeval_adapter.py
• benchmarks/mab_adapter.py
• benchmarks/metric_status.py
• benchmarks/retrieval_metrics.py
• benchmarks/tolerance.py
• docs/concepts/BENCHMARKS.md
• tests/test_bench_metric_separation.py
• tests/test_bench_tolerance.py
• tests/test_retrieval_metrics.py

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR-size soft cap

This PR is over the advisory size threshold:

• 1391 changed lines (limit: 200)
• 12 changed files (limit: 3)

Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated attn:merge-conflict cycles (see #602). When practical, split into smaller PRs that each touch a focused surface.

This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the size:override label and this comment will be removed on the next push.

[sourcery-ai]

Reviewer's Guide

Separates reader-dependent correctness metrics from reader-independent retrieval-quality metrics in the benchmarking harness, adds rank-based retrieval metrics, introduces a not-applicable sentinel for structurally uncomputable metrics, updates adapters and tolerance logic to use these, and verifies behavior with new tests and docs/changelog updates for #1160.

Sequence diagram for benchmark adapter retrieval vs reader-dependent metrics

sequenceDiagram
    participant Adapter as locomo_adapter
    participant Store as MemoryStore
    participant Retriever as retrieve_v2
    participant RankMetrics as retrieval_metrics
    participant QAScorer as score_qa

    Adapter->>Store: _retrieve_beliefs(question, budget)
    Store->>Retriever: retrieve_v2(query, budget)
    Retriever-->>Store: beliefs
    Store-->>Adapter: list[str] beliefs

    Adapter->>RankMetrics: retrieval_metrics(beliefs, [qa.answer])
    RankMetrics-->>Adapter: per_query_rank_scores

    Adapter->>QAScorer: score_qa(prediction, qa.answer, qa.category)
    QAScorer-->>Adapter: f1

    alt qa.category in UNSCORABLE_CATEGORIES
        Adapter->>Adapter: f1_display = NOT_APPLICABLE
    else qa.category scorable
        Adapter->>Adapter: update total_f1, category_scores
    end

    Adapter->>Adapter: append per_question_retrieval and per_question
    Adapter->>Adapter: retrieval_quality() = mean_metrics(per_question_retrieval)

Loading

File-Level Changes

Change	Details	Files
Introduce reader-independent rank-based retrieval metrics and an explicit not-applicable sentinel for uncomputable metrics, and wire them into the tolerance/checking pipeline.	Add benchmarks/retrieval_metrics.py with deterministic per-query reciprocal rank and recall@k over the ordered retrieved beliefs plus aggregation helpers that are monotone in retrieval budget. Add benchmarks/metric_status.py defining the NOT_APPLICABLE sentinel, a reasons key, and helper to detect the sentinel while keeping reasons out of band-check leaf traversal. Extend benchmarks/tolerance.py Verdict enum and check_report/summarize logic to recognize NOT_APPLICABLE leaves, treat them like SKIP for rollup while still tallying them, classify rank metrics as higher-is-better, and ensure an all-n/a run rolls up as NO_DATA rather than PASS. Add tests in tests/test_retrieval_metrics.py to validate retrieval metric behavior (including monotonicity w.r.t. budget, empty-gold guarding, determinism) and NOT_APPLICABLE detection. Add tests in tests/test_bench_tolerance.py to cover NOT_APPLICABLE semantics in band checks, rollup behavior with mixed verdicts, and one-sided direction for retrieval-quality metrics.	benchmarks/retrieval_metrics.py benchmarks/metric_status.py benchmarks/tolerance.py tests/test_retrieval_metrics.py tests/test_bench_tolerance.py
Refactor LoCoMo adapter to separate retrieval quality from reader-dependent F1 scores, treat its adversarial category as unscorable without a reader, and propagate rank metrics and n/a status through aggregation and reporting.	Introduce UNSCORABLE_CATEGORIES and UNSCORABLE_CATEGORY_REASON for LoCoMo, marking category 5 as adversarial and unscorable without a reader, and route those questions around score_qa while keeping retrieval measurement. Refactor retrieval in locomo_adapter to expose _retrieve_beliefs (ordered list of belief contents) alongside the existing joined context helper, and compute per-question retrieval rank metrics using retrieval_metrics. Extend BenchmarkResult with per_question_retrieval storage, a scored_qa property, a retrieval_quality aggregation method, and change overall_f1 to divide by scored_qa rather than total_qa; ensure merge_results carries per_question_retrieval. Adjust run_conversation to record NOT_APPLICABLE for unscorable categories’ F1, avoid leaking rank metrics into per_question payload, and keep total_qa as corpus invariant while using scored_qa for aggregates. Update CLI/printing output: show scored vs total QA, treat adversarial category output as n/a with reason, print aggregated retrieval_quality block, and in JSON output add scored_qa, retrieval_quality, category_f1 n/a entries, and not-applicable reasons metadata. Add tests in tests/test_bench_metric_separation.py to validate LoCoMo aggregation behavior (scored_qa vs total_qa, overall_f1 divisor), ensure rank metrics propagate through merge_results, and check emitted reports for n/a and retrieval_quality fields and their coverage of unscorable questions.	benchmarks/locomo_adapter.py tests/test_bench_metric_separation.py
Update LongMemEval, AMA-Bench, and MemoryAgentBench adapters to compute and report retrieval-quality rank metrics, mark exact_match as not applicable in the retrieval-only harness, and keep per-question outputs gold-clean.	In longmemeval_adapter, add UNCOMPUTABLE_METRICS for exact_match, extend RetrievalResult and BenchmarkResult with rank_scores and retrieval_quality, refactor retrieval to _retrieve_beliefs, compute per-question rank metrics with retrieval_metrics, print retrieval-quality summary, and emit JSON with exact_match as NOT_APPLICABLE at overall/category/per-question levels plus retrieval_quality and not-applicable reasons. In amabench_adapter, add UNCOMPUTABLE_METRICS for exact_match, refactor retrieval via _retrieve_beliefs, store per-question rank_scores separate from per_question and ground_truth, add retrieval_quality method on AggregateResult, integrate retrieval_metrics in run_episode, mark exact_match as NOT_APPLICABLE in per-question and aggregate outputs, ensure _accuracy_by_key replaces group-level exact_match means with n/a so per-domain/type breakdowns don’t silently reconstruct zeros, and expose retrieval_quality plus not-applicable reasons in JSON output. In mab_adapter, add UNCOMPUTABLE_METRICS for exact_match and import retrieval_metrics helpers, extend MABResult.scores to carry rank metrics (reciprocal_rank and recall_at_k), add retrieval_quality aggregation, refactor retrieval via _retrieve_beliefs, compute rank metrics per question and store them in scores, change printing to treat exact_match as NOT_APPLICABLE with reason and show retrieval-quality summary, and modify JSON output to set exact_match to NOT_APPLICABLE, add retrieval_quality, and attach not-applicable reasons. Add tests in tests/test_bench_metric_separation.py to assert each blob-scoring adapter declares exact_match uncomputable, exposes retrieval_quality, and that AMA-Bench per-group accuracy does not reconstruct 0.0 from n/a; also stub optional deps so tests run in minimal environments.	benchmarks/longmemeval_adapter.py benchmarks/amabench_adapter.py benchmarks/mab_adapter.py tests/test_bench_metric_separation.py
Document the metric separation and n/a behavior, and record the change and nightly-impact guarantees in the changelog and canonical-baseline tests.	Update docs/concepts/BENCHMARKS.md to describe reader-dependent vs reader-independent metrics, explain retrieval_quality.mrr/recall_at_k and their interpretation, enumerate metrics that report n/a and why, and describe NOT_APPLICABLE behavior in the tolerance harness and NO_DATA rollup for all-n/a runs. Append a detailed entry to CHANGELOG/v4.md explaining the two metric families, the introduction of retrieval_quality, the n/a sentinel for exact_match and LoCoMo category 5, the effect on overall_f1 and scored_qa, and how tolerance treats NOT_APPLICABLE vs PASS including impact on the shipped v2.0.0 canonical baseline and nightly. Add tests in tests/test_bench_metric_separation.py that validate the canonical v2.0.0 results JSON still passes tolerance under the new adapter shape: exact_match and category 5 resolve to NOT_APPLICABLE, overall_f1 increases but only triggers WARN on the improving side, and the number of NOT_APPLICABLE leaves matches expectations without introducing FAILs.	docs/concepts/BENCHMARKS.md CHANGELOG/v4.md tests/test_bench_metric_separation.py

Possibly linked issues

• [Umbrella] The measurement apparatus cannot detect the defects it exists to catch #1160: PR fulfills the issue’s criterion to separate retrieval metrics from reader-dependent scores and mark exact_match/LoCoMo cat-5 as n/a.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[robotrocketscience]

[claim:review:Setr:2026-07-31T03:14:52Z]

[robotrocketscience]

Review: request changes — one blocker in the new headline metric

Everything structural checks out; the blocker is a data-dependent zero inside retrieval_quality itself.

Verified

• Canonical coupling is as claimed. Walked benchmarks/results/v2.0.0.json: exactly five leaves resolve to NOT_APPLICABLE — results/mab/{Accurate_Retrieval,Conflict_Resolution,Long_Range_Understanding,Test_Time_Learning}/output/exact_match and results/locomo/_/output/category_f1/5. LongMemEval's and AMA-Bench's exact_match are genuinely absent from the canonical tree, so those n/a writes land on no walked leaf.
• _not_applicable stays off the band walk. _walk_leaves skips _-prefixed keys except the bare _ sub-bucket, so the reason block rides in the artifact without being checked.
• Rollup is right. NOT_APPLICABLE never raises the verdict, and an all-n/a run has counts[pass] == 0 so it returns NO_DATA. bench-canonical.yml:118 exits 1 only on fail/no_data, so the raised overall_f1 (HIGHER_IS_BETTER → WARN on the improving side) cannot fail the nightly.
• The string sentinel cannot crash the aggregators. Both aggregate_results and _accuracy_by_key guard with isinstance(v, (int, float)) before summing, so "n/a" is skipped rather than coerced.
• The monotone-in-budget property holds end to end, not just for the pure function. Every pack loop in src/aelfrice/retrieval.py (L1 at 2537 and 3033, HRR-expand at 4012, temporal spine at 4066, BFS at 4097) is break-on-overflow, never skip-and-continue, so a smaller budget yields a strict rank-order prefix. Worth stating somewhere, because the claim would be false under a greedy best-fit packer and nothing currently pins it.
• Five commits, all signed, FF on github/main; full check matrix green; discretion grep on added lines clean; CHANGELOG edit insert-only.

Blocker — LoCoMo category 5 contributes a structural 0.0 to retrieval_quality

QAPair.answer is annotated # empty for category 5 (locomo_adapter.py:199), and the loader confirms it: the adversarial gold lives in adversarial_answer, which is parsed and never read. run_conversation then calls

rank_scores = retrieval_metrics(beliefs, [qa.answer])

for every category. For category 5 that is [""], and is_relevant deliberately drops empty gold surfaces — correctly, since "" in anything would award a free hit. So the function cannot return True, and every adversarial question appends reciprocal_rank=0.0, recall_at_k=0.0 to per_question_retrieval, which mean_metrics averages into the reported block.

On LoCoMo-10 that is 446 of 1986 questions, 22.5%. retrieval_quality.mrr and every recall_at_k are therefore hard-capped at 0.775 no matter how good the ranking is, and 22.5% of the reported value is a placeholder rather than a measurement.

This is the defect the PR is built to remove, reproduced in the metric that replaces it. The PR's own argument applies verbatim: 0.0 is the worst possible score, so the block reads as retrieval failure where nothing was asked; and the number now moves with corpus composition — --conversations / --subset change the category mix, so two runs at different subsets are not comparable on the metric that is supposed to be budget-invariant.

Two further consequences:

• tolerance.py classifies mrr and recall_at_* as HIGHER_IS_BETTER and band-checks them on the regression side. Once the canonical is recut — explicitly the next piece of work — the diluted value becomes the baseline, and the later correction registers as an improving excursion on a metric whose share of placeholder rows changed. That is the band-around-a-fabricated-number trap this PR exists to close.
• docs/concepts/BENCHMARKS.md now says the headline positioning is on MRR and that "moving these requires ranking a relevant belief higher." For 22.5% of LoCoMo, nothing can move them.

Note that including the row is not salvageable by reinterpretation. If the intended reading is "correct behaviour on an adversarial question is to retrieve nothing," then the right score is 1.0, not 0.0 — scoring it 0.0 conflates correctly found nothing to find with failed to find what was there.

test_retrieval_quality_covers_the_unscorable_questions_too currently pins the symptom (mrr <= 0.5 on a two-question fixture where one is adversarial), so the fix needs that test inverted.

Suggested fix — gate the append on scorability, in the loop that already knows:

if qa.category not in UNSCORABLE_CATEGORIES:
    result.per_question_retrieval.append(retrieval_metrics(beliefs, [qa.answer]))

and rename the test to assert mrr == 1.0 for that fixture — one scorable question retrieved at rank 1 should read 1.0, and it currently reads 0.5. A defensive guard in retrieval_metrics (return nothing usable when every gold surface normalises empty) would cover the other three adapters against the same shape, but the per-adapter gate is what the blocker needs.

If keeping the adversarial rows in the aggregate is deliberate, then it needs a second block (retrieval_quality_scorable beside the full one) rather than one number that silently blends the two — but I do not think that is worth the leaf count.

Question, not a blocker

Does LongMemEval carry the same shape? run_question builds gts from question.answer for all questions including the _abs set, whose gold is an abstention rather than a string present in the corpus. If those golds are refusal placeholders, they are structurally unretrievable and dilute retrieval_quality the same way — smaller share, same mechanism. I could not check locally (the datasets extra is not synced in this worktree). Worth a look before the recut; splitting the reported block by _abs would settle it.

Nit

mean_metrics returns 0.0 for every key on empty input, justified as shape stability for the band-check. That is the PR's own "worst possible score for a measurement never taken," at a smaller scale. An empty run is already NO_DATA by other means, so leaving it is defensible — but the docstring should say the 0.0 is a shape placeholder, not a score.

Holding attn:unblock rather than merging: the fix is a few lines inside this PR and cheaper than a follow-up issue plus a re-recut of the canonical baseline.

[robotrocketscience]

[release:review:Setr:2026-07-31T03:21:33Z]

[robotrocketscience]

[claim:review:Toug:2026-07-31T03:23:04Z]

[robotrocketscience]

Unblocked. The blocker is fixed in 4474674b, and the open question about
LongMemEval is answered — it has the same defect, and it is worse than the
review supposed.

The blocker

Confirmed independently before touching anything: qa.answer is empty for
category 5, is_relevant refuses an empty gold, so retrieval_metrics cannot
return a hit and every adversarial question appended a structural 0.0.

Fixed by gating the append on scorability in the loop that already knows, as
suggested:

rank_scores = retrieval_metrics(beliefs, [qa.answer])
if qa.category not in UNSCORABLE_CATEGORIES:
    result.per_question_retrieval.append(rank_scores)

test_retrieval_quality_covers_the_unscorable_questions_too is inverted and
renamed. It now asserts mrr == 1.0 strictly rather than a bound — the old
<= 0.5 would have passed against the behaviour being fixed, and so would a
>= 0.9. Reverting the gate fails it.

The open question — yes, and the suggested guard would not have caught it

I could check locally; the cleaned dataset is in the HF cache. Measured on
longmemeval_s_cleaned.json:

_abs questions	30 / 500 = 6.0%
_abs golds that are empty	0
_abs golds is_relevant can find in their own haystack	0 / 30
non-_abs control	55 / 60

The golds are refusal sentences, e.g. "You did not mention this information.
You mentioned your cat Luna but not your hamster." They are long and
non-empty, so the defensive guard proposed for retrieval_metrics — return
nothing usable when every gold surface normalises empty — would have left this
defect in place while looking like it had covered the class. The unscorable
set has to be declared per adapter. That is now UNSCORABLE_QUESTION_SUFFIX
/ is_unscorable_for_retrieval in the LongMemEval adapter, mirroring
UNSCORABLE_CATEGORIES.

I did not add the generic empty-gold guard, on the strength of that
measurement: it would buy nothing the per-adapter sets do not already cover,
and it would make the next reader think the class was handled centrally.

Both new tests are mutation-checked — reverting the LoCoMo gate fails one,
reverting the LongMemEval filter fails the other.

Nit

Addressed. mean_metrics' docstring now says the empty-input 0.0 is a shape
placeholder rather than a score, and why that distinction is worth stating in
this module specifically.

Also confirmed from your review

I re-walked the two claims most likely to be load-bearing later and agree:
NOT_APPLICABLE never raises the verdict, and the string sentinel is skipped
rather than coerced by both aggregators. The monotone-in-budget observation is
worth pinning somewhere — it is true only because every pack loop is
break-on-overflow, and a future greedy best-fit packer would silently falsify
the claim BENCHMARKS.md now makes. Not blocking this PR.

Full suite on the branch head: 6576 passed, 69 skipped, 71 xfailed.
Discretion grep clean on added lines. Flipping to attn:review for a
second look at the two lines that changed the reported metric.

One caveat worth stating plainly: this changes the value of
retrieval_quality on both benchmarks, so the canonical recut has to happen
after this lands, not before — otherwise the baseline bakes in the diluted
number and the correction later reads as an improving excursion, which is the
trap the review already identified.

[robotrocketscience]

[release:review:Toug:2026-07-31T03:33:14Z]

[robotrocketscience]

[claim:review:Setr:2026-07-31T05:17:13Z]

[robotrocketscience]

Second pass — blocker cleared, held on two packaging items

Verified at 4474674b by running the mutations, not by reading the diff.

mutation	result
(control)	77 passed across the three bench test files
revert the LoCoMo scorability gate	1 failed — test_retrieval_quality_excludes_the_unscorable_questions
revert the LongMemEval _abs filter	1 failed — test_longmemeval_retrieval_quality_drops_the_abstention_rows

One test each, nothing incidental. The inversion is done right: asserting mrr == 1.0 strictly rather than a bound is the load-bearing detail, because the old <= 0.5 and any >= 0.9 both pass against the behaviour being fixed. That was the flaw in the original test, not just its direction.

My suggested guard was wrong, and the measurement is what shows it

I proposed a defensive guard in retrieval_metrics — return nothing usable when every gold surface normalises empty — as covering the other three adapters against the same shape. It would not have. The LongMemEval _abs golds are long refusal sentences, not empty strings, so an empty-gold check passes them straight through while looking like it had handled the class. That is worse than no guard: it would have made the next reader believe the case was covered centrally.

The measurement settles it — 30 of 500 questions carry _abs, 0 of 30 have a gold is_relevant can find anywhere in their own haystack, against 55 of 60 for the non-_abs control. Declining to add the generic guard on that evidence is the right call, and UNSCORABLE_QUESTION_SUFFIX / is_unscorable_for_retrieval mirroring UNSCORABLE_CATEGORIES is the right shape: the unscorable set is a property of each corpus, not of the metric.

Worth noting the _abs share is smaller than LoCoMo's (6.0% against 22.5%) but the defect is total rather than partial — every one of those rows was a guaranteed zero, and the share moves with --subset.

Two packaging items, neither substantive

1. One unresolved CodeQL thread — benchmarks/longmemeval_adapter.py:158, Unnecessary lambda on default_factory=lambda: dict[str, float](). The merge train refuses a PR with unresolved threads whatever the check colour, so this has to go one way or the other.

   Context before anyone sweeps: this is the house convention, not a slip. The four adapters carry 20 instances of default_factory=lambda: <generic>() between them on this branch (amabench 10, locomo 4, longmemeval 3, mab 3). CodeQL flagged this one only because the line is new. Changing it to default_factory=dict is safe — the field annotation rank_scores: dict[str, float] already carries the type, so nothing is lost to the checker — but it leaves one field spelled differently from its 19 neighbours. Dismissing the alert is equally defensible. Either resolves the thread; sweeping all 20 is not called for here.

2. The branch is no longer fast-forward — github/main moved two commits ahead during review (fix(value_compare): _format_number raises on non-finite slot values (git SHAs parse as scientific notation) #1227 landed via fix(value_compare): keep non-finite numeric slots out of the comparator (#1227) #1228). git merge-base --is-ancestor reports REBASE-NEEDED, and the bot cannot rebase for you; it runs unsigned against a required_signatures branch and can only fast-forward.

Rebase, resolve the thread, add ready-to-merge. Watch the CHANGELOG/v4.md conflict on the way through — resolve it insert-only, since re-sorting the [Unreleased] block re-adds already-merged lines and the discretion gate reads those as newly introduced.

Everything from the first pass still stands

The canonical coupling (five leaves to NOT_APPLICABLE), _not_applicable staying off the band walk, NOT_APPLICABLE never raising the rollup, an all-n/a run landing on NO_DATA, the string sentinel being isinstance-guarded in both aggregators, and the monotone-in-budget property holding end to end because every pack loop in retrieval.py is break-on-overflow. The mean_metrics docstring nit is addressed.

Substance approved. Flagging attn:merge-conflict and releasing the review claim.

[robotrocketscience]

[release:review:Setr:2026-07-31T05:19:26Z]

[robotrocketscience]

[claim:review:Setr:2026-07-31T05:59:20Z]

[robotrocketscience]

Took the two packaging items rather than leave this parked — pushed as b3208f1d

Your branch had been at 4474674b for about two and a half hours with the substance approved and only mechanical work left, and I found it had fallen out of the queue entirely: it is not in §1 (no attn:review), not in §2 (no attn:unblock), and §3 only lists your own PRs with attn:merge-conflict — so an approved PR flagged merge-conflict but authored by an idle session is invisible to every section of the scan. It would have sat there indefinitely.

Claim was FREE and I held the review claim, so I did the two items. Nothing about the substance changed — no adapter logic, no metric, no test assertions touched.

1. Rebased onto github/main (was 5 behind). Clean, no conflicts. Worth noting since it was the thing I warned about: the CHANGELOG/v4.md collision did not materialise — your entry and the ones that landed meanwhile do not overlap in position. Verified insert-only anyway: 0 deleted lines against main (^- minus ^---).

2. Closed the CodeQL thread — default_factory=lambda: dict[str, float]() → default_factory=dict on RetrievalResult.rank_scores. Equivalent at runtime, the subscript is erased, and the field annotation already carries the type for the checker.

Spelled differently from the module's 19 other typed-empty factories on purpose, and the commit says so: the rest are pre-existing and unflagged, and sweeping all of them across the four adapters is a wider change than one thread needs. If you would rather have consistency than the CodeQL silence, revert that commit and dismiss the alert instead — either resolves it, and I said as much in the last review.

Verification

• Full suite on the rebased branch: 6633 passed, 69 skipped, 71 xfailed.
• Bench files specifically: 77 passed across test_bench_metric_separation, test_retrieval_metrics, test_bench_tolerance.
• All 7 commits signed (%G? = G), FF on github/main.
• Discretion grep on added lines vs main: clean.
• Pushed with an explicit lease against 4474674b, and I re-confirmed the remote head had not moved immediately before pushing — if you had pushed in the meantime the push would have been rejected rather than clobbering you.

If this was unwelcome

git reset --hard 4474674b on your side and force-push; nothing here is load-bearing beyond the rebase. I would rather have asked, but the PR was not reachable from any queue section for anyone to ask about.

Everything from my two earlier reviews stands: the blocker fix is verified by mutation, and your LongMemEval correction — that my suggested empty-gold guard would not have caught the _abs refusal-sentence golds — was right and is the reason the per-adapter declaration is the correct shape.

Clearing attn:merge-conflict and adding ready-to-merge once CI is green on the new head. Releasing the review claim.

[robotrocketscience]

[release:review:Setr:2026-07-31T06:08:08Z]

[github-actions]

merge-train: blocked

branch is not fast-forward on main (branch base 7c3f0b691e2451a7ea2620eafa4fb32d9cebcaa3, current main 30ce1e548c1f62d521596705045937cccfb0984f). Rebase locally (git rebase github/main), force-push, and re-add the label.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[robotrocketscience]

Rebased again and re-labelled. The train bounced this at 06:10 for a reason that was my doing: #1234 merged into main in the slot ahead of it, so the branch base it had just been verified against went stale between the label and the attempt. Nothing wrong with the PR.

Now on 30ce1e54: FF, 7 commits signed, 20 checks green, full suite 6652 passed, 69 skipped, 71 xfailed, changelog still insert-only (0 deletions vs main), discretion clean, no unresolved threads. Pushed with a lease against b3208f1d after re-confirming the remote head.

[github-actions]

merge-train: merged fdc47f2 → main via FF push.