[BENCH-623] TTS TTFA breakdown: persist the roundtrip / leading-silence split and visualize it on Latency Variation - #454
Conversation
TTFA = network roundtrip + leading silence, and the runner already computed both parts per sample before discarding the split. Persist the components as two new metric rows (TTFARoundtrip, TTFALeadingSilence), written only when both are known so they always sum back to the TTFA row — a transport-gated or arrival-only TTFA writes neither. Because metric_type is a plain string dimension everywhere downstream, the matviews, series rollups and aggregates API carry the new metrics with no schema or migration changes; every run's real split lands in the pipeline from the next deploy onward. Runner-only on purpose: the dashboard reads none of this yet. The visualization lands separately once the split has accumulated, so it can be built against real served data. Full runner suite 1249 passed; ruff format and check clean.
The Latency Variation card gains a Distribution / Breakdown toggle on TTS once the split metrics are served. Breakdown reuses the shared QualityMetricBars chassis (the accuracy chart's): every model as one stacked bar in its own palette color — the same color the Filters sidebar and every other chart use — with texture carrying the split: solid is the network roundtrip, hatched the leading silence. Ranked fastest first with the same frozen-axis horizontal scroll, so it reads identically at three models or thirty. The headline flips to the field's leading-silence share; tooltips give each part's ms and share. All values are window-aggregate averages served by the API: the runner writes the component pair over the same samples, so avg roundtrip + avg silence equals the split runs' average TTFA exactly — labeled "Average TTFA" throughout so the breakdown never reads against the distribution view's medians. Nothing is derived or estimated, and the per-run timeline stays untouched from main. The chassis grows optional stackSegments (fills resolvable per model), svgDefs for the hatch patterns, and tickFormatter; stacked rows must not carry a fill key (recharts prefers row fill over segment fill), and stacked mode keeps mobile tap-to-inspect tooltips since those bars have no click-to-compare action. Verified against a local API seeded with all 29 active registry TTS models: bars, labels and tooltips reconcile, the scroll matches the accuracy chart, and mobile keeps 44px+ targets. Web 108 passed, typecheck and lint clean.
The component metrics are consumed as window aggregates only (the Latency Variation breakdown reads model_stats); nothing reads them per bucket. TTS runs ~48x a day, so letting them flow into results_by_bucket would double the 30d aggregates response — 66k series rows / 16.9 MB on prod today — for rows no chart uses. Exclude them in refresh_bucket via SERIES_EXCLUDED_METRICS, keeping every aggregates payload byte-identical to today; the raw result rows still feed the stats matviews. Remove a metric from the set if a per-run surface ever ships for it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughThe backend adds TTFA roundtrip and leading-silence metrics, emits component rows when measurements are available, and excludes them from series rollups. The web layer prepares breakdown data and exposes it through dashboard state. The dashboard renders stacked TTFA bars, patterned silence segments, labels, legends, and tooltips. Tests cover metric registration, TTS measurement handling, orchestration, rollup filtering, chart data, and tooltip output. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/runner/orchestrator.py`:
- Around line 781-801: Update the component-row condition in the TTFA handling
flow around `ttfa_status` and `leading_silence_ms` to require `ttfa_status is
ResultStatus.SUCCESS` before appending `TTFA_ROUNDTRIP` and
`TTFA_LEADING_SILENCE` results. Add a regression test covering populated
`ttfa_ms` and `leading_silence_ms` with a provider-reported error, asserting
that no component rows are emitted.
In `@web/components/charts/tooltips/BarTooltip.tsx`:
- Around line 119-129: Update the percentage calculation in the tooltip
rendering within the parts map of BarTooltip so a zero row.ttfa produces 0% (or
omits the share) instead of dividing by zero; preserve the existing rounded
percentage behavior for nonzero totals and add a tooltip test covering a
zero-total TTFA.
🪄 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: c963af7a-9795-4494-837a-18c4d2a02134
📒 Files selected for processing (18)
runner/src/coval_bench/db/writer.pyrunner/src/coval_bench/providers/base.pyrunner/src/coval_bench/providers/tts/_common.pyrunner/src/coval_bench/registries/__init__.pyrunner/src/coval_bench/registries/metrics.pyrunner/src/coval_bench/runner/orchestrator.pyrunner/tests/unit/test_db_writer.pyrunner/tests/unit/test_metric_registry.pyrunner/tests/unit/test_orchestrator.pyrunner/tests/unit/test_tts_common.pyweb/components/charts/QualityMetricBars.tsxweb/components/charts/tooltips/BarTooltip.test.tsxweb/components/charts/tooltips/BarTooltip.tsxweb/components/dashboard/BoxPlotSection.tsxweb/hooks/useChartData.tsweb/hooks/useDashboardState.tsxweb/lib/config/metrics.tsweb/types/benchmark.types.ts
…d zero-total shares A provider error can arrive after audio has streamed, leaving ttfa_ms and the silence offset populated on a FAILED row — the component gate now requires the TTFA row's SUCCESS status, so failed samples never leak into the breakdown aggregates (regression assertion added to the provider-error test). The breakdown tooltip also skips the percent share when the total is zero instead of rendering NaN%.
2 participants
What
TTFA = network roundtrip + leading silence. The runner already measured both parts per sample and threw the split away; only the combined total was persisted. This PR persists the split and visualizes it.
Runner — two new metric rows per successful TTS sample,
TTFARoundtripandTTFALeadingSilence, written only when both are measured so they always sum back to the TTFA row exactly. A transport-gated (HTTP/1.1, cold connection) or arrival-only TTFA writes neither.Web — the Latency Variation card gains a Distribution / Breakdown toggle on TTS. Breakdown renders every model as one stacked bar on the shared
QualityMetricBarschassis (same frozen-axis horizontal scroll as the accuracy chart, so ~30 models read fine): the bar wears the model's palette color, texture carries the split — solid = roundtrip, hatched = leading silence — ranked fastest first with totals labeled. The headline flips to the field's leading-silence share; tooltips give each part's ms and percent.Data integrity
model_stats); the runner writes the component pair over the same samples, so avg roundtrip + avg silence equals the split runs' average TTFA. Nothing is derived, scaled, or estimated.No schema, no API, no backfill
metric_typeis a plain string dimension, so the new rows flow through the existing tables, matviews, and aggregates API untouched — no migration exists in this PR anddb migrateis a no-op for it. No backfill: the feature is forward-only; the 24h window fills the day the runner deploys, 7d/30d as they roll over (until then, 7d/30d breakdown bars average only post-deploy runs — real values, newer coverage).One payload guard: prod's 30d TTS aggregates response is already 16.9 MB / 66k series rows, and TTS buckets land ~48×/day. Since the breakdown reads only window aggregates, the component metrics are excluded from the per-bucket series rollup (
SERIES_EXCLUDED_METRICS, tested) — every aggregates response stays byte-identical to today.Rollout
Merge → deploy runner + web in either order (web-first hides the toggle; runner-first accumulates rows invisibly) → the next scheduled run makes the breakdown appear on the 24h view.
Testing
Greptile Summary
This PR persists the network-roundtrip and leading-silence components of successful, comparable TTS TTFA measurements and adds a data-gated stacked breakdown view to the Latency Variation card.
Confidence Score: 5/5
The PR appears safe to merge; no concrete blocking or independently actionable non-blocking issue was identified.
The component rows are emitted and persisted as an atomic pair, flow through the unrestricted window-statistics path, remain intentionally absent from series buckets, and are rendered only when both aggregates are available.
Reviews (1): Last reviewed commit: "[BENCH-623] Keep the TTFA split out of t..." | Re-trigger Greptile
Context used (3)