Skip to content

Flag a metric that rests on too few samples - #456

Open
seribaymadina wants to merge 1 commit into
mainfrom
wer-sample-floor
Open

Flag a metric that rests on too few samples#456
seribaymadina wants to merge 1 commit into
mainfrom
wer-sample-floor

Conversation

@seribaymadina

@seribaymadina seribaymadina commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

A provider that failed almost everything it ran still published the few measurements that survived. fishaudio/s1 scored 1 of ~480 attempts and read as 0.0% WER; Hume and Minimax did the same on 1–2 samples.

Stats under the modality's floor now carry insufficient_samples so the frontend can show "n/a", and a flagged entry sinks below every ranked one — the view orders by value alone, so one fast measurement would otherwise lead the board.

Values stay in the payload and no rows change. Series points are exempt: one bucket holds a single run's samples, so flagging them would blank every timeline.

Against prod this flags exactly the 8 affected rows and nothing else. The frontend half lands in benchmarks-web once this ships and codegen picks up the field.

Greptile Summary

The PR adds a modality-specific minimum sample floor and exposes an insufficient_samples response field while retaining raw metric values. It also moves flagged leaderboard entries behind sufficiently sampled entries, but the by-dataset aggregate path does not apply the new flag.

  • Adds shared minimum-sample thresholds for STT, TTS, and S2S
  • Flags thin pooled aggregate and leaderboard statistics
  • Preserves timeline series points without flags
  • Adds boundary and ordering tests for pooled aggregates and leaderboards

Confidence Score: 4/5

The by-dataset aggregates path must apply the same insufficient-sample rule before this is safe to merge.

Thin per-dataset ModelStatEntry values retain the schema default of false because that endpoint bypasses the newly introduced flagging helper, producing inconsistent API behavior for the same statistic type.

Files Needing Attention: runner/src/coval_bench/api/routers/aggregates.py

Reviews (1): Last reviewed commit: "Flag a metric that rests on too few samp..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

A provider that failed almost everything it ran still published the handful
of measurements that survived, so one lucky sample was presented as its real
WER or latency — a provider scoring 1 of 480 attempts read as 0.0% WER.

Stats under the modality's floor now carry insufficient_samples so the
frontend can show "n/a", and a flagged entry sinks below every ranked one on
the leaderboard. Values are left in place and no rows change: the floor only
governs how a stat is presented.

Series points are exempt — one bucket holds a single run's samples, so
flagging them would blank every timeline.
@seribaymadina
seribaymadina requested a review from a team as a code owner August 5, 2026 23:47
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The API defines sample-count floors for STT, TTS, and S2S benchmarks. Aggregate model statistics and leaderboard entries now expose insufficient_samples. Thin aggregate statistics retain their values, while series points remain unchanged. Thin leaderboard entries remain in responses but sort below sufficiently sampled entries. Tests cover below-threshold, exact-threshold, and above-threshold cases.

Suggested reviewers: coval-cale

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: flagging metrics that use too few samples.
Description check ✅ Passed The description directly explains the sample thresholds, insufficient_samples flag, leaderboard ordering, preserved values, and series behavior.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@runner/src/coval_bench/api/schemas.py`:
- Around line 75-79: Regenerate the web API schema so the generated
LeaderboardEntry and ModelStatEntry types in schema.ts include the backend
insufficient_samples boolean field with its default semantics. Use the project’s
existing schema-generation workflow rather than editing generated output
manually.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f9827d1-0881-4e92-b185-44e9b7d2c6ab

📥 Commits

Reviewing files that changed from the base of the PR and between 0799052 and e1089f7.

📒 Files selected for processing (6)
  • runner/src/coval_bench/api/common.py
  • runner/src/coval_bench/api/routers/aggregates.py
  • runner/src/coval_bench/api/routers/leaderboard.py
  • runner/src/coval_bench/api/schemas.py
  • runner/tests/api/test_aggregates.py
  • runner/tests/api/test_leaderboard.py

Comment on lines +75 to +79
# True when ``n`` is under the modality's floor: the values are real but rest
# on too few samples to rank on, so clients render "n/a". Sent alongside the
# numbers rather than in place of them, so a caller that wants the raw
# measurement still has it.
insufficient_samples: bool = False

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the known API type consumer.
ast-grep outline web/types/benchmark.types.ts --items all

# Find response types and rendering paths that must consume the new field.
rg -n -C 4 \
  -g '*.ts' -g '*.tsx' \
  '\binsufficient_samples\b|\bModelStatEntry\b|\bLeaderboardEntry\b|\bmodel_stats\b|\bentries\b' \
  web

Repository: coval-ai/benchmarks

Length of output: 33412


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== schemas.py relevant sections =="
sed -n '45,180p' runner/src/coval_bench/api/schemas.py

echo
echo "== generated schema generated from API =="
sed -n '65,105p' web/lib/api/generated/schema.ts

echo
echo "== all repository occurrences of insufficient_samples =="
rg -n -C 3 '\binsufficient_samples\b' .

echo
echo "== Leaderboard row renderers in arena leaderboard =="
sed -n '115,155p' web/app/overview/arena/leaderboard/arena-leaderboard.tsx
sed -n '1,180p' web/components/overview/OverviewLeaderboards.tsx

echo
echo "== ModelStatEntry consumers =="
sed -n '25,45p' web/hooks/useWerDatasetMatrix.ts
sed -n '27,42p' web/hooks/useDatasetScopedWer.ts
sed -n '175,215p' web/hooks/useDashboardState.tsx
sed -n '1,90p' web/lib/utils/werBreakdown.ts

Repository: coval-ai/benchmarks

Length of output: 23875


Propagate insufficient_samples to the web client schema.

insufficient_samples is added to LeaderboardEntry and ModelStatEntry in the backend schemas, but web/lib/api/generated/schema.ts still lacks these fields. Regenerate the generated API schema so web clients can consume the flag and render "n/a" where required.

Also applies to lines 151-155.

🤖 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 `@runner/src/coval_bench/api/schemas.py` around lines 75 - 79, Regenerate the
web API schema so the generated LeaderboardEntry and ModelStatEntry types in
schema.ts include the backend insufficient_samples boolean field with its
default semantics. Use the project’s existing schema-generation workflow rather
than editing generated output manually.

Comment on lines 112 to +124
)


def _flag_thin(stat: ModelStatEntry, benchmark: str) -> ModelStatEntry:
"""Mark a stat that rests on too few samples to present as a real number.

The values are left intact — a collapsed provider's one measurement is a real
measurement, and callers that want it still get it. Only the flag changes, so
the frontend can show "n/a" instead of ranking a lucky sample.
"""
if has_enough_samples(benchmark, stat.sample_count):
return stat
return stat.model_copy(update={"insufficient_samples": True})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 By-dataset stats bypass sample flag

When a per-dataset model statistic has fewer than five scored samples, /results/aggregates/by-dataset constructs ModelStatEntry directly without _flag_thin, leaving insufficient_samples false and causing clients to present an unreliable thin-cohort value as valid.

Knowledge Base Used: API service

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant