Skip to content

fix(database): replace SQL JSON subscript with Python-side aggregation in pre-OTEL leaderboard - #2730

Open
Aftabbs wants to merge 5 commits into
truera:mainfrom
Aftabbs:fix/leaderboard-aggregates-pre-otel-text-json
Open

fix(database): replace SQL JSON subscript with Python-side aggregation in pre-OTEL leaderboard#2730
Aftabbs wants to merge 5 commits into
truera:mainfrom
Aftabbs:fix/leaderboard-aggregates-pre-otel-text-json

Conversation

@Aftabbs

@Aftabbs Aftabbs commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Problem

get_leaderboard_aggregates() crashes with NotImplementedError whenever TRULENS_OTEL_TRACING is not enabled (the default for most users).

The root cause is in _get_leaderboard_aggregates_pre_otel: the SQL query uses SQLAlchemy's [] subscript operator to extract values from cost_json and references a non-existent Record.latency column:

sa.func.avg(self.orm.Record.cost_json["n_tokens"].as_float())  # raises NotImplementedError
sa.func.avg(self.orm.Record.latency)                            # AttributeError – column doesn't exist

cost_json and perf_json are declared as TYPE_JSON = Text (orm.py:35), so they are plain TEXT columns. SQLAlchemy raises NotImplementedError for [] subscript access on Text columns at statement construction time, before any query ever hits the database.

Fixes #2729.

Fix

Replace the broken SQL-level aggregation with Python-side aggregation using the existing module-level helpers _extract_tokens_and_cost and _extract_latency (already used by the get_records_and_feedback code path):

  1. record_stmt now selects individual rows with raw cost_json and perf_json text columns (no subscript, no missing latency column).
  2. After fetching, parse cost/latency with _extract_tokens_and_cost / _extract_latency.
  3. Use pandas.groupby(...).agg(...) for the per-app aggregation that was previously done in SQL.

The output schema (Records, Total Tokens, Average Latency (s), Total Cost (USD)) is unchanged.

Tests

Added tests/unit/test_leaderboard_pre_otel.py with three test cases:

  • empty database – returns empty DataFrame without raising.
  • records present – aggregated Records, Total Tokens, Total Cost (USD), and Average Latency (s) are computed correctly.
  • app_name filter – only the matching app's rows are returned.

All tests run against an in-memory SQLite database, matching the existing test pattern used in test_dashboard_utils.py.

Checklist

  • Bug reproduced and root cause confirmed in orm.py (TYPE_JSON = Text)
  • Fix avoids dialect-specific JSON extraction (works with SQLite, PostgreSQL, Snowflake)
  • Output schema preserved
  • Unit tests added

…n in pre-OTEL leaderboard

cost_json and perf_json are stored as TEXT (TYPE_JSON = Text in orm.py), so
SQLAlchemy's column["key"].as_float() raises NotImplementedError at statement
construction time.  The pre-OTEL leaderboard path hit this on every call when
TRULENS_OTEL_TRACING is not set.

Instead of database-level JSON extraction (dialect-specific and broken on Text
columns), fetch individual record rows with the raw text fields and aggregate
in Python using the existing _extract_tokens_and_cost and _extract_latency
helpers.  The output schema (Records, Total Tokens, Average Latency (s),
Total Cost (USD)) is unchanged.

Fixes truera#2729
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Aug 26, 2026
joshreini1

This comment was marked as outdated.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 26, 2026

@joshreini1 joshreini1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OTEL has been enabled by default for more than a year and its aggregation path already works. This PR should keep its scope on the legacy non-OTEL path, but implement that aggregation in SQL rather than moving it into frontend Python. SQL aggregation is materially more scalable and avoids transferring raw records to the frontend for processing.

@dosubot dosubot Bot removed the lgtm This PR has been approved by a maintainer label Aug 27, 2026
Ruff F401: core_schema and base_schema imported but not used in
test_leaderboard_with_records_aggregates_correctly. Move datetime import
to module level. Fix multi-line dict literal to one key-per-line style
for ruff-format compliance.
Replace Python-side pandas groupby with database-level aggregation
in _get_leaderboard_aggregates_pre_otel.

cost_json/perf_json are stored as TYPE_JSON = Text. Use
_json_path_expr (json_extract on SQLite/MySQL, json_extract_path_text
on PostgreSQL) to extract scalar values at the database level.

Aggregation now uses:
- SUM(CAST(json_extract(cost_json, '$.n_tokens') AS FLOAT)) for tokens
- SUM(CAST(json_extract(cost_json, '$.cost') AS FLOAT)) for cost
- COUNT(DISTINCT record_id) for record count
- AVG((julianday(end_time) - julianday(start_time)) * 86400) for
  latency (SQLite), or AVG(EXTRACT(EPOCH FROM ...)) for PostgreSQL

This avoids transferring O(n_records) rows to the frontend for
Python-side processing, which is the scalable behaviour joshreini1
requested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] get_leaderboard_aggregates always crashes when TRULENS_OTEL_TRACING is disabled

2 participants