fix(leaderboard): correct telemetry pipeline and outlier handling (#498)

* fix(leaderboard): correct telemetry pipeline and outlier handling

The public leaderboard at /leaderboard showed a clear bimodal Wh/token
distribution: most users at ~3-5 J/token, ~30% inflated by 1000-4000×.
A 5-agent investigation workflow + 4-agent verification pass traced the
inflation to a cluster of related bugs across telemetry, server, and
display layers. This PR fixes the in-repo half. Backfilling existing
Supabase data is a separate follow-up.

Bug 1 — Dual telemetry recording (`server/routes.py`)
=====================================================

`_handle_direct` wrapped the engine with `instrumented_generate`
unconditionally. When `app.state.engine` was already an
`InstrumentedEngine` (the common case when telemetry is wired in),
BOTH layers published `TELEMETRY_RECORD` — once from the inner
`InstrumentedEngine.generate`, once from the outer wrapper. Every
chat-completion request was counted twice in the leaderboard pipeline.

Fix: detect the InstrumentedEngine and unwrap to `._inner` before
passing to the wrapper so only one layer fires.

Bug 2 — KV-cache fallback over-counts multi-turn (`server/savings.py`)
=====================================================================

`compute_savings` falls back from `prompt_tokens_evaluated` to
`prompt_tokens` when the KV-cache-aware count is missing. But routes.py
aggregates by summing each turn's full prompt — which counts the system
prompt N times for an N-turn conversation. The fallback inflated FLOPs
and energy by N×.

Fix: use 0 (conservative under-count) when the evaluated count is
missing rather than falling back to the inflated `prompt_tokens` sum.

Bug 3 — TelemetryRecord lacked methodology versioning
=====================================================

There was no per-record version tag, so legacy (pre-fix) and current
records were silently aggregated together in the public leaderboard
even though they used different methodologies.

Fix: add `token_counting_version: Optional[int]` field to the
`TelemetryRecord` dataclass + a nullable column to the SQLite schema
(with idempotent migration); the constant moves from `server.savings`
to `core.types` to avoid the server→telemetry layering. New records
write the current version; pre-fix rows remain NULL. The aggregator
gains a `current_methodology_only=True` flag that filters NULL rows out
of leaderboard sums — local dashboards leave it off so historical
aggregates still render.

Bug 4 — Leaderboard JS displayed missing telemetry as legit zeros
=================================================================

Rows with significant token counts but `energy_wh_saved = 0` and
`flops_saved = 0` rendered as `0.00 Wh / 0 FLOPs` — visually identical
to a user who genuinely did almost nothing. The headline totals also
included these rows.

Fix: new `isMissingTelemetry()` detector renders missing-telemetry
energy/FLOPs cells as `—` (with a tooltip explaining why); a new
`.lb-missing` CSS class differentiates the placeholder visually.

Bug 5 — Outlier filter was too generous
=======================================

`MAX_ENERGY_WH_PER_TOKEN = 10` left a 10,000× margin that admitted
every Group B row even though those values are physically impossible
(would imply a space-heater per token). Same for the FLOPs cap at
`1e17`.

Fix: tighten to `0.5` Wh/token (still 500× over a typical consumer GPU)
and `1e15` FLOPs/token (still 10,000× over typical). This removes
existing pre-fix corrupt rows from the public view without touching
Supabase.

Tests
=====

New regression tests (all pass, ruff clean):

- `tests/server/test_routes.py::test_instrumented_engine_unwrapped_to_avoid_dual_telemetry`
  — pins the bug 1 fix; asserts exactly ONE TELEMETRY_RECORD event per
  request when the engine is already an InstrumentedEngine.
- `tests/server/test_savings.py` (NEW file, 3 tests) — pins the bug 2
  fix (FLOPs not inflated via fallback) and the cost-side invariant
  (dollar savings still use full prompt_tokens because cloud providers
  bill per input token even when local KV cache hit).
- `tests/telemetry/test_aggregator.py::TestMethodologyFilter` (3 tests)
  — default behaviour includes legacy rows (local dashboard parity);
  `current_methodology_only=True` excludes them; the summary surface
  honors the same filter.

Unrelated test note
===================

`tests/telemetry/test_energy_wiring.py::TestTelemetryStatsEnergy::test_export_includes_energy_fields`
and `::TestEndToEndPipeline::test_ask_to_export_with_energy` are flaky
on this branch but ALSO flaky on `main` (confirmed via stash + the
banner-only run). Root cause: `_version_check.py:132-135` prints the
"new version of OpenJarvis is available" banner to stdout outside the
throttle window, contaminating `json.loads(result.output)` in those two
tests. Reproducible with `OPENJARVIS_NO_UPDATE_CHECK=1` → all pass.
That's a separate bug (the banner shouldn't write to the same stream
as machine-readable output); not addressing it here to keep this PR
focused on the leaderboard pipeline.

What this PR explicitly does NOT do
====================================

- Backfill ~30% of Supabase rows that already shipped with inflated
  values from pre-fix clients. That requires Supabase write access and
  is the natural follow-up — see PR comments for proposed dry-run audit
  query and the additive `methodology_version` column migration.
- Distinguish which past submissions came from buggy vs correct clients
  retroactively (no `app_version` tag on existing submissions).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* test(evals): fix test_energy_scales_linearly under leaderboard fix

CI on #498 failed in the slow `test` job:

    tests/evals/test_use_case_benchmarks.py::TestSavings::
    test_energy_scales_linearly — ZeroDivisionError: float division by zero

Root cause: the test (and `test_energy_wh_matches_direct_formula`)
called `compute_savings(N, 0)` without `prompt_tokens_evaluated`. Under
the OLD buggy fallback, an unset evaluated count silently became N,
energy scaled linearly with N, and the test passed by accident. Under
this branch's conservative fix the fallback is 0, FLOPs collapse to 0,
and the `p10.energy_wh / p1.energy_wh` ratio divides 0 by 0.

The test's own docstring says "evaluated tokens (KV-cache model)" — it
was always meant to exercise the explicit-evaluated path, the API
call just didn't match. Pass `prompt_tokens_evaluated` explicitly so
the test now expresses the invariant it claims to.

Same one-line fix for `test_energy_wh_matches_direct_formula` plus an
explicit `flops > 0` sanity assertion — that test was passing trivially
with `0 == 0` after the conservative fallback fix, masking whether the
formula was actually being exercised.

No production code change in this commit. Verified locally: all 6
TestSavings tests pass; the 5 new regression tests added on this branch
still pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: krypticmouse <herumbshandilya123@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

docs/javascripts/leaderboard.js

@@ -12,8 +12,14 @@
   // Outlier detection — hide entries with values that are physically
   // implausible relative to their token count.  Thresholds are ~1000x
   // above legitimate per-token values to avoid false positives.
-  var MAX_ENERGY_WH_PER_TOKEN = 10;        // legit ≈ 0.001 Wh/tok
-  var MAX_FLOPS_PER_TOKEN = 1e17;           // legit ≈ 1e12 /tok
+  // Outlier bounds. Set well above realistic upper limits but tight
+  // enough to drop the pre-fix bimodal Group B (1-5 Wh/token, ~3e16
+  // FLOPs/token) — see the leaderboard PR for the full diagnosis.
+  // Realistic per-token rates on a consumer GPU + 10–30B local model:
+  // ~0.001 Wh/token, ~1e10–1e11 FLOPs/token. We allow 500× and 10,000×
+  // headroom respectively for inefficient hardware / larger models.
+  var MAX_ENERGY_WH_PER_TOKEN = 0.5;        // legit ≈ 0.001 Wh/tok
+  var MAX_FLOPS_PER_TOKEN = 1e15;           // legit ≈ 1e11 /tok
   var MAX_DOLLAR_PER_TOKEN = 25.0 / 1e6;   // hard ceiling: $25/1M output
 
   function isOutlier(row) {
@@ -29,6 +35,25 @@
     );
   }
 
+  // Distinguish "user actually has zero work done" from "user's energy /
+  // FLOPs telemetry never landed". The latter happens when the server
+  // submits with valid dollar savings + token counts but the per-record
+  // energy stamp was missing (pre-fix builds, GPU energy meter
+  // unavailable, etc.). Without this check those rows show as "0.00 Wh"
+  // and skew the rankings + headline totals.
+  //
+  // Threshold: 1000 tokens is well above any single chat-turn — if a
+  // user has that many tokens recorded but no measured energy, the
+  // telemetry is incomplete, not legitimately zero.
+  var MIN_TOKENS_FOR_TELEMETRY = 1000;
+
+  function isMissingTelemetry(row) {
+    var tokens = Number(row.total_tokens) || 0;
+    var energy = Number(row.energy_wh_saved) || 0;
+    var flops = Number(row.flops_saved) || 0;
+    return tokens > MIN_TOKENS_FOR_TELEMETRY && energy === 0 && flops === 0;
+  }
+
   function escapeHtml(s) {
     var el = document.createElement("span");
     el.textContent = s;
@@ -62,13 +87,24 @@
       var medal =
         rank === 1 ? "\uD83E\uDD47" : rank === 2 ? "\uD83E\uDD48" : rank === 3 ? "\uD83E\uDD49" : "";
       var row = pageRows[j];
+      // Render "—" for energy / FLOPs columns when telemetry didn't
+      // land (vs the user genuinely having 0). The dollar / request /
+      // token columns are unaffected because those measurements landed
+      // even when energy didn't.
+      var missing = isMissingTelemetry(row);
+      var energyCell = missing
+        ? '<td class="lb-number lb-missing" title="Energy telemetry missing for this entry">—</td>'
+        : '<td class="lb-number">' + Number(row.energy_wh_saved || 0).toFixed(2) + "</td>";
+      var flopsCell = missing
+        ? '<td class="lb-number lb-missing" title="FLOPs telemetry missing for this entry">—</td>'
+        : '<td class="lb-number">' + fmtLarge(Number(row.flops_saved || 0)) + "</td>";
       html +=
         "<tr>" +
         '<td><span class="lb-rank' + rankClass + '">' + (medal || rank) + "</span></td>" +
         '<td class="lb-name">' + escapeHtml(row.display_name) + "</td>" +
         '<td class="lb-number">$' + Number(row.dollar_savings || 0).toFixed(4) + "</td>" +
-        '<td class="lb-number">' + Number(row.energy_wh_saved || 0).toFixed(2) + "</td>" +
-        '<td class="lb-number">' + fmtLarge(Number(row.flops_saved || 0)) + "</td>" +
+        energyCell +
+        flopsCell +
         '<td class="lb-number">' + Number(row.total_calls || 0).toLocaleString() + "</td>" +
         '<td class="lb-number">' + Number(row.total_tokens || 0).toLocaleString() + "</td>" +
         "</tr>";

docs/stylesheets/extra.css

@@ -440,6 +440,13 @@
   font-family: var(--md-code-font-family, monospace);
   font-size: 13px;
 }
+/* Placeholder for rows where energy / FLOPs telemetry didn't land.
+   Distinguishes "telemetry missing" from "user genuinely had 0 work
+   done" without making the row visually pop more than data rows. */
+.lb-missing {
+  color: var(--md-default-fg-color--light, #999);
+  font-style: italic;
+}
 
 /* ── DocSearch ───────────────────────────────────────────────────────── */
 #docsearch {

src/openjarvis/core/types.py

@@ -125,6 +125,16 @@ class ToolResult:
     metadata: Dict[str, Any] = field(default_factory=dict)
 
 
+# Bump when token-counting methodology changes so the leaderboard can
+# distinguish entries computed under different rules.
+#   v1 = original (Ollama prompt_eval_count, may under-count due to KV cache)
+#   v2 = full prompt token count, no KV-cache assumption, system prompt
+#        always counted
+# Lives here (not in server/savings) so the telemetry layer can read it
+# without crossing the server → telemetry layering.
+TOKEN_COUNTING_VERSION: int = 2
+
+
 @dataclass(slots=True)
 class TelemetryRecord:
     """Single telemetry observation recorded after an inference call."""
@@ -167,6 +177,14 @@ class TelemetryRecord:
     gpu_energy_joules: float = 0.0
     dram_energy_joules: float = 0.0
     tokens_per_joule: float = 0.0
+    # Version tag for the token-counting methodology used when this record
+    # was produced. `None` (= legacy) means the record predates per-record
+    # versioning; the leaderboard aggregator filters those out to avoid
+    # mixing pre-fix and post-fix records in the same per-token efficiency
+    # metric — they were the dominant source of the bimodal Wh/token
+    # distribution on the public leaderboard. New records always write
+    # `TOKEN_COUNTING_VERSION` from `server/savings.py`.
+    token_counting_version: Optional[int] = None
     mining_session_id: Optional[str] = None
     metadata: Dict[str, Any] = field(default_factory=dict)
 

src/openjarvis/server/routes.py

@@ -195,17 +195,50 @@ def _handle_direct(
     if req.tools:
         kwargs["tools"] = req.tools
     if bus:
+        from openjarvis.telemetry.instrumented_engine import InstrumentedEngine
         from openjarvis.telemetry.wrapper import instrumented_generate
 
-        result = instrumented_generate(
-            engine,
-            messages,
-            model=model,
-            bus=bus,
-            temperature=req.temperature,
-            max_tokens=req.max_tokens,
-            **kwargs,
-        )
+        # `app.state.engine` may already be an InstrumentedEngine (the
+        # common case when telemetry is wired in). If we then wrap it
+        # with `instrumented_generate`, BOTH layers fire a
+        # TELEMETRY_RECORD per call:
+        #
+        #   - InstrumentedEngine.generate() publishes a FULL record
+        #     (energy_joules, GPU stats, token_counting_version, ...).
+        #   - instrumented_generate() publishes a BARE record (timing +
+        #     tokens only; no energy meter, no version stamp).
+        #
+        # The doubled count was the dominant driver of the bimodal
+        # Wh/token distribution on the public leaderboard.
+        #
+        # The fix below is NOT "unwrap and call instrumented_generate":
+        # that would have replaced "doubled records" with "every
+        # request emits only a bare record with no energy / no version",
+        # which the leaderboard's `current_methodology_only=True` filter
+        # would then drop entirely. Instead, when the engine is already
+        # an InstrumentedEngine, skip the wrapper and call `generate`
+        # directly — InstrumentedEngine publishes the full per-record
+        # event itself with energy + version intact. Only fall back to
+        # the lightweight wrapper for engines that aren't already
+        # instrumented.
+        if isinstance(engine, InstrumentedEngine):
+            result = engine.generate(
+                messages,
+                model=model,
+                temperature=req.temperature,
+                max_tokens=req.max_tokens,
+                **kwargs,
+            )
+        else:
+            result = instrumented_generate(
+                engine,
+                messages,
+                model=model,
+                bus=bus,
+                temperature=req.temperature,
+                max_tokens=req.max_tokens,
+                **kwargs,
+            )
     else:
         result = engine.generate(
             messages,
@@ -736,7 +769,11 @@ async def savings(request: Request):
 
     agg = TelemetryAggregator(db_path)
     try:
-        summary = agg.summary(since=session_start)
+        # current_methodology_only excludes pre-fix legacy rows from
+        # the leaderboard's per-token efficiency numerator/denominator
+        # — see the comment on _time_filter for the bimodal-Wh/token
+        # background.
+        summary = agg.summary(since=session_start, current_methodology_only=True)
         # Exclude cloud model tokens from savings — only local
         # inference counts toward cost savings.
         _cloud_prefixes = (

src/openjarvis/server/savings.py

@@ -10,11 +10,12 @@ import time
 from dataclasses import asdict, dataclass, field
 from typing import Any, Dict, List
 
-# Bump when token-counting methodology changes so that the leaderboard
-# can distinguish entries computed under different rules.
-#   v1 = original (Ollama prompt_eval_count, may under-count due to KV cache)
-#   v2 = full prompt token count, no KV-cache assumption, system prompt always counted
-TOKEN_COUNTING_VERSION: int = 2
+# Source of truth for the methodology-version constant lives in
+# `openjarvis.core.types` so the telemetry layer can read it without
+# crossing the server → telemetry layering. Re-exported here for
+# backward compatibility with existing imports of `from
+# openjarvis.server.savings import TOKEN_COUNTING_VERSION`.
+from openjarvis.core.types import TOKEN_COUNTING_VERSION  # noqa: E402,F401
 
 # ---------------------------------------------------------------------------
 # Cloud provider pricing (USD per 1M tokens)
@@ -104,11 +105,22 @@ def compute_savings(
       served from KV cache.  Used for **FLOPs** and **energy**
       calculations since these reflect actual compute.
 
-    If ``prompt_tokens_evaluated`` is 0 (e.g. old telemetry without the
-    column), it falls back to ``prompt_tokens``.
+    When ``prompt_tokens_evaluated`` is 0 we used to fall back to
+    ``prompt_tokens``. That's wrong in multi-turn: routes.py aggregates
+    by summing each turn's full prompt — which counts the system prompt
+    N times for an N-turn conversation — so the fallback turned the FLOPs
+    and energy estimates into N×-too-high numbers. That was the dominant
+    contributor to the bimodal Wh/token distribution on the leaderboard.
+
+    Conservative behaviour now: when the KV-cache-aware count is
+    missing, treat `prompt_tokens_evaluated` as 0 — so the FLOPs/energy
+    denominator becomes just `completion_tokens`. That under-estimates
+    rather than over-estimates compute, and (intentionally) cascades
+    into the leaderboard's `isMissingTelemetry` UI render so those
+    rows show `—` instead of a misleading zero.
     """
     if prompt_tokens_evaluated <= 0:
-        prompt_tokens_evaluated = prompt_tokens
+        prompt_tokens_evaluated = 0
     total_tokens = prompt_tokens + completion_tokens
     total_tokens_evaluated = prompt_tokens_evaluated + completion_tokens
     providers: List[ProviderSavings] = []

src/openjarvis/telemetry/aggregator.py

@@ -92,12 +92,22 @@ class TelemetryAggregator:
         self._conn = sqlite3.connect(self._db_path, check_same_thread=False)
         self._conn.row_factory = sqlite3.Row
 
-    @staticmethod
     def _time_filter(
+        self,
         since: Optional[float] = None,
         until: Optional[float] = None,
+        current_methodology_only: bool = False,
     ) -> tuple[str, list[Any]]:
-        """Build a WHERE clause fragment for time-range filtering."""
+        """Build a WHERE clause fragment for time + methodology filtering.
+
+        ``current_methodology_only`` is opt-in (default False) because the
+        local dashboard still wants to render historical aggregates that
+        predate the per-record version stamp. The leaderboard ingest path
+        in ``server/routes.savings`` flips it to True so legacy rows (the
+        ones with NULL token_counting_version) don't pollute the public
+        per-token efficiency metric — they were the dominant source of
+        the bimodal Wh/token distribution.
+        """
         clauses: list[str] = []
         params: list[Any] = []
         if since is not None:
@@ -106,6 +116,11 @@ class TelemetryAggregator:
         if until is not None:
             clauses.append("timestamp <= ?")
             params.append(until)
+        if current_methodology_only and self._safe_col("token_counting_version"):
+            from openjarvis.core.types import TOKEN_COUNTING_VERSION
+
+            clauses.append("token_counting_version = ?")
+            params.append(TOKEN_COUNTING_VERSION)
         if clauses:
             return " WHERE " + " AND ".join(clauses), params
         return "", params
@@ -124,8 +139,11 @@ class TelemetryAggregator:
         *,
         since: Optional[float] = None,
         until: Optional[float] = None,
+        current_methodology_only: bool = False,
     ) -> List[ModelStats]:
-        where, params = self._time_filter(since, until)
+        where, params = self._time_filter(
+            since, until, current_methodology_only=current_methodology_only
+        )
 
         # Build optional columns for new fields (graceful on old DBs)
         extra_cols = ""
@@ -215,8 +233,11 @@ class TelemetryAggregator:
         *,
         since: Optional[float] = None,
         until: Optional[float] = None,
+        current_methodology_only: bool = False,
     ) -> List[EngineStats]:
-        where, params = self._time_filter(since, until)
+        where, params = self._time_filter(
+            since, until, current_methodology_only=current_methodology_only
+        )
 
         extra_cols = ""
         has_tpj = self._safe_col("tokens_per_joule")
@@ -305,9 +326,14 @@ class TelemetryAggregator:
         *,
         since: Optional[float] = None,
         until: Optional[float] = None,
+        current_methodology_only: bool = False,
     ) -> AggregatedStats:
-        model_stats = self.per_model_stats(since=since, until=until)
-        engine_stats = self.per_engine_stats(since=since, until=until)
+        model_stats = self.per_model_stats(
+            since=since, until=until, current_methodology_only=current_methodology_only
+        )
+        engine_stats = self.per_engine_stats(
+            since=since, until=until, current_methodology_only=current_methodology_only
+        )
         total_calls = sum(m.call_count for m in model_stats)
 
         def _weighted_avg(attr: str) -> float:

src/openjarvis/telemetry/instrumented_engine.py

@@ -8,7 +8,7 @@ from collections.abc import AsyncIterator
 from typing import Any, Dict, List, Optional, Sequence
 
 from openjarvis.core.events import EventBus, EventType
-from openjarvis.core.types import Message, TelemetryRecord
+from openjarvis.core.types import TOKEN_COUNTING_VERSION, Message, TelemetryRecord
 from openjarvis.engine._stubs import InferenceEngine, StreamChunk
 from openjarvis.telemetry.gpu_monitor import GpuSample
 
@@ -233,6 +233,11 @@ class InstrumentedEngine(InferenceEngine):
             gpu_energy_joules=gpu_energy_joules,
             dram_energy_joules=dram_energy_joules,
             tokens_per_joule=tokens_per_joule,
+            # Stamp every new record with the current methodology version
+            # so the leaderboard aggregator can drop legacy rows (those
+            # left NULL by pre-fix builds) cleanly. Source of truth for
+            # the integer is `server/savings.py`.
+            token_counting_version=TOKEN_COUNTING_VERSION,
         )
 
         event_data = {
@@ -454,6 +459,8 @@ class InstrumentedEngine(InferenceEngine):
             gpu_energy_joules=gpu_energy_joules,
             dram_energy_joules=dram_energy_joules,
             tokens_per_joule=tokens_per_joule,
+            # Stamp the methodology version on streaming records too.
+            token_counting_version=TOKEN_COUNTING_VERSION,
         )
 
         event_data = {

src/openjarvis/telemetry/store.py

@@ -55,6 +55,11 @@ CREATE TABLE IF NOT EXISTS telemetry (
     p99_itl_ms           REAL NOT NULL DEFAULT 0.0,
     std_itl_ms           REAL NOT NULL DEFAULT 0.0,
     is_streaming         INTEGER NOT NULL DEFAULT 0,
+    -- token_counting_version: nullable on purpose. Records inserted by
+    -- pre-fix builds left this NULL; the leaderboard aggregator treats
+    -- NULL as legacy and excludes those rows from per-token efficiency
+    -- sums (so the bimodal-Wh/token leaderboard population disappears).
+    token_counting_version INTEGER,
     mining_session_id    TEXT,
     metadata        TEXT    NOT NULL DEFAULT '{}'
 );
@@ -91,13 +96,14 @@ INSERT INTO telemetry (
     prefill_energy_joules, decode_energy_joules,
     mean_itl_ms, median_itl_ms, p90_itl_ms, p95_itl_ms, p99_itl_ms, std_itl_ms,
     is_streaming,
+    token_counting_version,
     mining_session_id,
     metadata
 ) VALUES (
     ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
     ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
     ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
-    ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
+    ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
 )
 """
 
@@ -128,6 +134,10 @@ _MIGRATE_COLUMNS = [
     ("std_itl_ms", "REAL NOT NULL DEFAULT 0.0"),
     ("is_streaming", "INTEGER NOT NULL DEFAULT 0"),
     ("prompt_tokens_evaluated", "INTEGER NOT NULL DEFAULT 0"),
+    # `token_counting_version` is nullable on purpose — rows that existed
+    # before this migration ran predate per-record versioning and the
+    # aggregator filter treats them as legacy.
+    ("token_counting_version", "INTEGER"),
     ("mining_session_id", "TEXT"),
 ]
 
@@ -197,6 +207,7 @@ class TelemetryStore:
                 rec.p99_itl_ms,
                 rec.std_itl_ms,
                 1 if rec.is_streaming else 0,
+                rec.token_counting_version,
                 rec.mining_session_id,
                 json.dumps(rec.metadata),
             ),

tests/evals/test_use_case_benchmarks.py

@@ -424,11 +424,22 @@ class TestSavings:
         assert "total_calls" in d
 
     def test_energy_scales_linearly(self) -> None:
-        """Energy should scale linearly with evaluated tokens (KV-cache model)."""
+        """Energy should scale linearly with evaluated tokens (KV-cache model).
+
+        The test must pass `prompt_tokens_evaluated` explicitly — under the
+        leaderboard-correctness fix, `compute_savings` no longer falls back
+        from a missing `prompt_tokens_evaluated` to the (multi-turn-inflated)
+        `prompt_tokens` sum. When the KV-cache-aware count is missing, FLOPs
+        derive from `completion_tokens` only. With completion=0, that path
+        produces zero energy and divides by zero. Passing the evaluated
+        count explicitly is the API contract that lets this invariant be
+        tested without depending on the (now intentionally conservative)
+        fallback behaviour.
+        """
         from openjarvis.server.savings import compute_savings
 
-        s1 = compute_savings(1000, 0)
-        s10 = compute_savings(10000, 0)
+        s1 = compute_savings(1000, 0, prompt_tokens_evaluated=1000)
+        s10 = compute_savings(10000, 0, prompt_tokens_evaluated=10000)
         # FLOPs = 2*P*T (linear), so 10x tokens => 10x FLOPs => 10x energy
         for p1, p10 in zip(s1.per_provider, s10.per_provider):
             ratio = p10.energy_wh / p1.energy_wh
@@ -437,10 +448,16 @@ class TestSavings:
             )
 
     def test_energy_wh_matches_direct_formula(self) -> None:
-        """Energy must equal flops * wh_per_flop for known constants."""
+        """Energy must equal flops * wh_per_flop for known constants.
+
+        Pass `prompt_tokens_evaluated` explicitly for the same reason as
+        `test_energy_scales_linearly` — the conservative-fallback fix in
+        savings.py means an unset `prompt_tokens_evaluated` produces 0
+        FLOPs and would make this test pass trivially with 0 == 0.
+        """
         from openjarvis.server.savings import CLOUD_PRICING, compute_savings
 
-        summary = compute_savings(10000, 0)
+        summary = compute_savings(10000, 0, prompt_tokens_evaluated=10000)
         for p in summary.per_provider:
             pricing = CLOUD_PRICING[p.provider]
             wh_per_flop = pricing["energy_wh_per_1k_tokens"] / (
@@ -450,6 +467,12 @@ class TestSavings:
             assert abs(p.energy_wh - expected) < 1e-6, (
                 f"{p.provider}: energy_wh={p.energy_wh}, expected={expected}"
             )
+            # Sanity: assert the formula isn't matching trivially with both
+            # sides equal to zero (would happen if FLOPs collapse to 0).
+            assert p.flops > 0, (
+                f"{p.provider}: FLOPs must be positive when "
+                f"prompt_tokens_evaluated is explicitly set"
+            )
 
     def test_energy_not_zero(self) -> None:
         """Energy must be positive for non-zero token counts."""

tests/server/test_routes.py

@@ -250,6 +250,75 @@ class TestChatCompletions:
         # No tools → agent path → agent's content surfaces.
         assert data["choices"][0]["message"]["content"] == "Hello from agent"
 
+    def test_instrumented_engine_unwrapped_to_avoid_dual_telemetry(self):
+        """Regression for the leaderboard wonky-values bug.
+
+        When `app.state.engine` is already an `InstrumentedEngine` (which is
+        the common case when the server was constructed with telemetry
+        wired in), `_handle_direct` MUST NOT wrap it again with
+        `instrumented_generate`. Both layers publish `TELEMETRY_RECORD`
+        events, so wrapping twice would double-count every call into the
+        leaderboard pipeline and inflate per-token energy / FLOPs metrics
+        by 2× on every request — the dominant contributor to the bimodal
+        Wh/token distribution on the public leaderboard.
+
+        The fix unwraps the engine via `engine._inner` before passing it
+        to `instrumented_generate`. This test pins that contract.
+        """
+        from openjarvis.core.events import EventBus, EventType
+        from openjarvis.telemetry.instrumented_engine import InstrumentedEngine
+
+        # Build a fresh engine + bus and explicitly wrap with
+        # InstrumentedEngine (mirrors the production app construction).
+        inner_engine = _make_engine(content="Telemetry test")
+        bus = EventBus()
+        wrapped = InstrumentedEngine(inner_engine, bus=bus)
+
+        received_records = []
+        bus.subscribe(
+            EventType.TELEMETRY_RECORD,
+            lambda data: received_records.append(data),
+        )
+
+        app = create_app(wrapped, "test-model")
+        app.state.bus = bus
+        client = TestClient(app)
+
+        resp = client.post(
+            "/v1/chat/completions",
+            json={
+                "model": "test-model",
+                "messages": [{"role": "user", "content": "hello"}],
+            },
+        )
+        assert resp.status_code == 200
+
+        # Exactly ONE telemetry record — not two. Pre-fix this asserted 2.
+        assert len(received_records) == 1, (
+            f"Expected exactly one TELEMETRY_RECORD event per request "
+            f"(got {len(received_records)}). When `app.state.engine` is "
+            f"already an InstrumentedEngine, routes.py must not also fire "
+            f"`instrumented_generate` — both layers publish and double "
+            f"the leaderboard's per-request counts."
+        )
+
+        # And the surviving record must be the InstrumentedEngine's
+        # FULL record (with token_counting_version stamped, ready for
+        # the leaderboard's current_methodology_only=True filter).
+        # If routes.py had instead unwrapped engine._inner and routed
+        # through the lightweight `instrumented_generate`, the record
+        # would carry no version stamp and `current_methodology_only`
+        # would drop it from leaderboard sums entirely. Pin that
+        # contract — see the adversarial review on PR #498.
+        from openjarvis.core.types import TOKEN_COUNTING_VERSION
+
+        rec = received_records[0].data["record"]
+        assert rec.token_counting_version == TOKEN_COUNTING_VERSION, (
+            "InstrumentedEngine path must stamp the methodology version "
+            "so the leaderboard's current-methodology filter accepts the "
+            "record."
+        )
+
     def test_agent_with_conversation(self, client_with_agent):
         resp = client_with_agent.post(
             "/v1/chat/completions",

tests/server/test_savings.py

@@ -0,0 +1,79 @@
+"""Regression tests for compute_savings — leaderboard correctness.
+
+The leaderboard pipeline feeds aggregated telemetry sums into
+`compute_savings`, which in turn feeds the public leaderboard. The old
+behaviour fell back from `prompt_tokens_evaluated` to `prompt_tokens`
+when the KV-cache-aware count was missing — but routes.py aggregates
+by summing per-turn full prompts, which counts the system prompt N
+times in an N-turn conversation. The fallback was the dominant
+contributor to the bimodal Wh/token distribution observed on the public
+leaderboard. These tests pin the conservative fallback behaviour.
+"""
+
+from __future__ import annotations
+
+from openjarvis.server.savings import compute_savings
+
+
+class TestPromptTokensEvaluatedFallback:
+    def test_fallback_does_not_inflate_with_summed_prompt_tokens(self) -> None:
+        """When prompt_tokens_evaluated is 0 (missing), FLOPs must NOT
+        be derived from the full `prompt_tokens` sum.
+
+        In a 10-turn conversation with a 100-token system prompt, the
+        aggregator sums prompt_tokens = 10 × (sys + history) ≈ 10× the
+        true input. Pre-fix the fallback used that inflated number for
+        FLOPs and energy. Post-fix we use only completion_tokens, which
+        is conservative but at least not 10× too high.
+        """
+        # Same prompt_tokens (the buggy aggregated sum) for both
+        # invocations; vary only whether prompt_tokens_evaluated is
+        # known. The fix must make the FLOPs-bearing fields independent
+        # of the inflated prompt_tokens when evaluated is missing.
+        with_evaluated = compute_savings(
+            prompt_tokens=5000,
+            completion_tokens=200,
+            total_calls=10,
+            prompt_tokens_evaluated=600,  # known KV-cache-aware count
+        )
+        without_evaluated = compute_savings(
+            prompt_tokens=5000,
+            completion_tokens=200,
+            total_calls=10,
+            prompt_tokens_evaluated=0,  # missing → fallback path
+        )
+
+        # The fallback path should NOT silently use the inflated
+        # prompt_tokens sum as the FLOPs denominator.
+        for missing_p, known_p in zip(
+            without_evaluated.per_provider, with_evaluated.per_provider
+        ):
+            assert missing_p.flops <= known_p.flops + 1, (
+                f"Fallback inflated FLOPs for {missing_p.provider}: "
+                f"missing_evaluated={missing_p.flops}, "
+                f"known_evaluated={known_p.flops}. Pre-fix the "
+                f"fallback used `prompt_tokens` (the multi-turn-summed "
+                f"value) which over-stated compute by N× the turn count."
+            )
+
+    def test_dollar_savings_uses_prompt_tokens_unchanged(self) -> None:
+        """Dollar savings still uses prompt_tokens (cloud providers bill
+        per input token, even when the local engine had KV cache hits).
+        Pin the existing contract so the FLOPs fix doesn't accidentally
+        regress the dollar math."""
+        result = compute_savings(
+            prompt_tokens=1_000_000,
+            completion_tokens=100_000,
+            total_calls=10,
+            prompt_tokens_evaluated=0,
+        )
+        # Sanity: some provider produced a positive cost (= positive
+        # savings vs running locally for free).
+        assert any(p.total_cost > 0 for p in result.per_provider)
+
+    def test_zero_tokens_returns_zero_savings(self) -> None:
+        """Edge case: no work done → no costs, no FLOPs, no negatives."""
+        result = compute_savings(prompt_tokens=0, completion_tokens=0)
+        for p in result.per_provider:
+            assert p.total_cost == 0.0
+            assert p.flops == 0.0

tests/telemetry/test_aggregator.py

@@ -248,3 +248,61 @@ class TestDataclassDefaults:
         assert a.total_calls == 0
         assert a.per_model == []
         assert a.per_engine == []
+
+
+# ---------------------------------------------------------------------------
+# Token-counting-version filter (leaderboard correctness)
+# ---------------------------------------------------------------------------
+
+
+class TestMethodologyFilter:
+    """When the aggregator is asked to honour the methodology version
+    (the leaderboard ingest path does), legacy rows that predate the
+    per-record version stamp must be excluded — they were the dominant
+    source of the bimodal Wh/token distribution on the public
+    leaderboard. Local dashboard callers leave the flag off so they
+    still see the full history."""
+
+    def test_default_includes_legacy_rows(self, tmp_path: Path) -> None:
+        from openjarvis.core.types import TOKEN_COUNTING_VERSION
+
+        legacy = _make_record(model_id="m1")
+        legacy.token_counting_version = None  # pre-fix row
+        current = _make_record(model_id="m1")
+        current.token_counting_version = TOKEN_COUNTING_VERSION
+
+        agg = _setup(tmp_path, [legacy, current])
+        stats = agg.per_model_stats()  # default: include everything
+        assert len(stats) == 1
+        assert stats[0].call_count == 2
+        agg.close()
+
+    def test_methodology_filter_drops_legacy_rows(self, tmp_path: Path) -> None:
+        from openjarvis.core.types import TOKEN_COUNTING_VERSION
+
+        legacy = _make_record(model_id="m1")
+        legacy.token_counting_version = None
+        current = _make_record(model_id="m1")
+        current.token_counting_version = TOKEN_COUNTING_VERSION
+
+        agg = _setup(tmp_path, [legacy, current])
+        stats = agg.per_model_stats(current_methodology_only=True)
+        assert len(stats) == 1
+        # Only the current-version row counts toward the leaderboard sum.
+        assert stats[0].call_count == 1
+        agg.close()
+
+    def test_methodology_filter_drops_legacy_in_summary(self, tmp_path: Path) -> None:
+        from openjarvis.core.types import TOKEN_COUNTING_VERSION
+
+        legacy = _make_record(model_id="m1", completion_tokens=99)
+        legacy.token_counting_version = None
+        current = _make_record(model_id="m1", completion_tokens=7)
+        current.token_counting_version = TOKEN_COUNTING_VERSION
+
+        agg = _setup(tmp_path, [legacy, current])
+        summary = agg.summary(current_methodology_only=True)
+        # 99-token legacy row excluded; only the 7-completion-token current
+        # row contributes to total_tokens.
+        assert sum(m.completion_tokens for m in summary.per_model) == 7
+        agg.close()