From 9d8c97763f0dcb3b058ec6545f845c68fae8d3ac Mon Sep 17 00:00:00 2001 From: Ayush8923 <80516839+Ayush8923@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:33:08 +0530 Subject: [PATCH 1/2] fix: analytics n+1 plus query issue --- backend/app/api/docs/analytics/monthly.md | 2 +- backend/app/crud/model_config.py | 55 ++++++++++++++--- backend/app/services/analytics/aggregation.py | 47 +++++++++------ .../app/tests/api/routes/test_analytics.py | 59 +++++++++++++++++++ docs/wiki/modules/platform.md | 2 +- 5 files changed, 137 insertions(+), 28 deletions(-) diff --git a/backend/app/api/docs/analytics/monthly.md b/backend/app/api/docs/analytics/monthly.md index cabb4da23..64ced67f5 100644 --- a/backend/app/api/docs/analytics/monthly.md +++ b/backend/app/api/docs/analytics/monthly.md @@ -163,5 +163,5 @@ GET /api/analytics/monthly?metric=eval_cost&modality=STT&from_month=2026-01-01 only to the `requests` metric (via the chain count), never to `cost` — its dollars come from the underlying `llm_call` rows. - **Cost computed on summed tokens per (provider, model) group**, which is - equivalent to per-row pricing because `estimate_model_cost` is linear in + equivalent to per-row pricing because the pricing function is linear in token counts. diff --git a/backend/app/crud/model_config.py b/backend/app/crud/model_config.py index 1d6ae8e83..ef7e2ca20 100644 --- a/backend/app/crud/model_config.py +++ b/backend/app/crud/model_config.py @@ -373,23 +373,39 @@ def is_summary_model(session: Session, provider: Provider, model_name: str) -> b return "summary" in model.config -def estimate_model_cost( +def list_active_model_pricing( session: Session, - provider: Provider, + pairs: set[tuple[str, str]], +) -> dict[tuple[str, str], dict[str, Any] | None]: + """Fetch pricing for many (provider, model_name) pairs in one query.""" + if not pairs: + return {} + providers = {provider for provider, _ in pairs} + model_names = {model_name for _, model_name in pairs} + stmt = select( + ModelConfig.provider, ModelConfig.model_name, ModelConfig.pricing + ).where( + ModelConfig.provider.in_(providers), # type: ignore[attr-defined] + ModelConfig.model_name.in_(model_names), # type: ignore[attr-defined] + ModelConfig.is_active, + ) + return { + (row.provider, row.model_name): row.pricing for row in session.exec(stmt).all() + } + + +def compute_cost_from_pricing( + pricing: dict[str, Any] | None, + provider: str, model_name: str, input_tokens: int, output_tokens: int, usage_type: Literal["response", "batch"] = "response", ) -> dict[str, Any] | None: - model = get_model_config(session=session, provider=provider, model_name=model_name) - if model is None or model.pricing is None: - return None - - if not isinstance(model.pricing, dict): + if not isinstance(pricing, dict): return None - pricing_source: dict[str, Any] = model.pricing - usage_pricing = pricing_source.get(usage_type) + usage_pricing = pricing.get(usage_type) if not isinstance(usage_pricing, dict): return None @@ -415,3 +431,24 @@ def estimate_model_cost( "total_cost": input_cost + output_cost, "currency": "USD", } + + +def estimate_model_cost( + session: Session, + provider: Provider, + model_name: str, + input_tokens: int, + output_tokens: int, + usage_type: Literal["response", "batch"] = "response", +) -> dict[str, Any] | None: + model = get_model_config(session=session, provider=provider, model_name=model_name) + if model is None: + return None + return compute_cost_from_pricing( + pricing=model.pricing, + provider=provider, + model_name=model_name, + input_tokens=input_tokens, + output_tokens=output_tokens, + usage_type=usage_type, + ) diff --git a/backend/app/services/analytics/aggregation.py b/backend/app/services/analytics/aggregation.py index 89b9e2357..39ee7d8b5 100644 --- a/backend/app/services/analytics/aggregation.py +++ b/backend/app/services/analytics/aggregation.py @@ -21,7 +21,8 @@ from app.crud.model_config import ( KNOWN_PROVIDERS, NATIVE_PROVIDER_SUFFIX, - estimate_model_cost, + compute_cost_from_pricing, + list_active_model_pricing, ) from app.models.analytics import Modality from app.models.config.version import ConfigVersion @@ -103,11 +104,12 @@ def _aggregate_llm_calls( ) -> None: """GROUP BY llm_call → merge counts, tokens, and per-group cost into buckets. - Cost is computed per (provider, model) group via `estimate_model_cost` - using the summed tokens. The pricing function is linear in tokens, so - summing first and pricing once is equivalent to per-row pricing. Skipped - for providers outside `KNOWN_PROVIDERS` (the `model_config.provider` - enum) since the lookup would raise InvalidTextRepresentation. + Cost is computed per (provider, model) group from the summed tokens. The + pricing function is linear in tokens, so summing first and pricing once + is equivalent to per-row pricing. Pricing for every group is fetched in + one query up front — a per-group lookup was an N+1. Providers outside + `KNOWN_PROVIDERS` (the `model_config.provider` enum) are excluded from + the lookup since Postgres would raise InvalidTextRepresentation. """ month_col = ( sa.func.date_trunc("month", LlmCall.inserted_at).cast(sa.Date).label("month") @@ -149,7 +151,17 @@ def _aggregate_llm_calls( if provider_filter is not None: stmt = stmt.where(LlmCall.provider == provider_filter) - for row in session.exec(stmt).all(): + rows = session.exec(stmt).all() + pricing_by_model = list_active_model_pricing( + session, + { + (row.provider, row.model) + for row in rows + if row.provider in KNOWN_PROVIDERS and row.model is not None + }, + ) + + for row in rows: modality_enum = Modality(row.modality) if modality_filter is not None and modality_enum is not modality_filter: continue @@ -160,16 +172,17 @@ def _aggregate_llm_calls( output_tokens = int(row.output_tokens or 0) bucket["input_tokens"] += input_tokens bucket["output_tokens"] += output_tokens - if (input_tokens or output_tokens) and row.provider in KNOWN_PROVIDERS: - estimate = estimate_model_cost( - session=session, - provider=row.provider, # type: ignore[arg-type] - model_name=row.model, - input_tokens=input_tokens, - output_tokens=output_tokens, - ) - if estimate is not None: - bucket["cost_usd"] += Decimal(str(estimate.get("total_cost", 0))) + if not (input_tokens or output_tokens): + continue + estimate = compute_cost_from_pricing( + pricing=pricing_by_model.get((row.provider, row.model)), + provider=row.provider, + model_name=row.model, + input_tokens=input_tokens, + output_tokens=output_tokens, + ) + if estimate is not None: + bucket["cost_usd"] += Decimal(str(estimate.get("total_cost", 0))) def _aggregate_chains( diff --git a/backend/app/tests/api/routes/test_analytics.py b/backend/app/tests/api/routes/test_analytics.py index dfd0c5b30..a7d7a48c4 100644 --- a/backend/app/tests/api/routes/test_analytics.py +++ b/backend/app/tests/api/routes/test_analytics.py @@ -2,6 +2,7 @@ from uuid import uuid4 import pytest +from sqlalchemy import event from fastapi.testclient import TestClient from sqlmodel import Session @@ -211,6 +212,64 @@ def test_metric_cost_uses_estimate_model_cost( total = sum(float(p["value"]) for p in data) assert total == pytest.approx(2.0) + def test_metric_cost_fetches_pricing_in_single_query( + self, + client: TestClient, + db: Session, + user_api_key: TestAuthContext, + user_api_key_header: dict[str, str], + llm_job: Job, + ): + model_names = [f"gpt-analytics-n1-{uuid4().hex[:8]}" for _ in range(3)] + for name in model_names: + db.add( + ModelConfig( + provider="openai", + model_name=name, + completion_type=["text"], + pricing={ + "response": {"input_token_cost": 1.0, "output_token_cost": 2.0} + }, + is_active=True, + ) + ) + db.commit() + for name in model_names: + _make_llm_call( + db, + job_id=llm_job.id, + project_id=user_api_key.project_id, + organization_id=user_api_key.organization_id, + provider="openai", + model=name, + input_tokens=1_000_000, + output_tokens=500_000, + ) + + model_config_queries: list[str] = [] + + def _capture(conn, cursor, statement, parameters, context, executemany): + if "model_config" in statement: + model_config_queries.append(statement) + + engine = db.get_bind() + event.listen(engine, "before_cursor_execute", _capture) + try: + data = _ok( + client.get( + MONTHLY_URL, + params={"metric": "cost", "provider": "openai"}, + headers=user_api_key_header, + ) + ) + finally: + event.remove(engine, "before_cursor_execute", _capture) + + total = sum(float(p["value"]) for p in data) + assert total == pytest.approx(6.0) + # One pricing lookup for llm_call + one DISTINCT ON subquery for evals. + assert len(model_config_queries) <= 2 + @pytest.mark.parametrize( "input_type,output_type,expected_modality", [ diff --git a/docs/wiki/modules/platform.md b/docs/wiki/modules/platform.md index 19d4923b4..55e9e954f 100644 --- a/docs/wiki/modules/platform.md +++ b/docs/wiki/modules/platform.md @@ -11,7 +11,7 @@ All paths relative to `backend/app/`. | Feature flags | `api/routes/features.py` | `feature_flag` (`models/feature_flag.py`) | `core/feature_flags/`, `crud/feature_flag.py` | | Languages | `api/routes/languages.py` | `global.languages` (`models/language.py`) | `crud/language.py` | | Credentials | `api/routes/credentials.py` | `credential` (`models/credentials.py`) | `crud/credentials.py`; provider keys per org/project; envelope encryption (KMS-wrapped data key + AES-GCM), prefix-versioned ciphertexts | -| Model config | `api/routes/model_config.py` | `model_config` (`models/model_config.py`) | `crud/model_config.py` | +| Model config | `api/routes/model_config.py` | `model_config` (`models/model_config.py`) | `crud/model_config.py`; cost: `estimate_model_cost` (single lookup) vs `list_active_model_pricing` + `compute_cost_from_pricing` (one batched lookup — use in loops/aggregations, e.g. `services/analytics/aggregation.py`) | | Bucket providers | — | reuses `credential` (`google-gcp`) | `services/buckets/` — global registry + resolver (`providers/`), GCS V4 signed + bulk-signed URLs with a 24h expiry cap (`providers/gcs.py`, cap in `providers/base.py`), attachment path selection + URL resolution (`attachments.py`) | | Cron | `api/routes/cron.py` | — | triggers batch polling (`crud/evaluations/cron.py`, `crud/assessment/cron.py` — the latter polls `method == RUN` assessments only; API-client BATCH runs are driven by their own Celery self-re-enqueue) | | Jobs | — | `job` (`models/job.py`), `batch_job` (`models/batch_job.py`) | `crud/jobs.py`, `crud/job/`, `services/job_monitoring.py` | From 29e28edd0264d8705649cd6a7330f9f99632ded4 Mon Sep 17 00:00:00 2001 From: Ayush8923 <80516839+Ayush8923@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:45:08 +0530 Subject: [PATCH 2/2] fix: few updates --- backend/app/crud/model_config.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/backend/app/crud/model_config.py b/backend/app/crud/model_config.py index ef7e2ca20..e25e1a531 100644 --- a/backend/app/crud/model_config.py +++ b/backend/app/crud/model_config.py @@ -1,4 +1,5 @@ import logging +from collections.abc import Mapping from datetime import datetime from typing import Any, Literal, get_args @@ -376,7 +377,7 @@ def is_summary_model(session: Session, provider: Provider, model_name: str) -> b def list_active_model_pricing( session: Session, pairs: set[tuple[str, str]], -) -> dict[tuple[str, str], dict[str, Any] | None]: +) -> dict[tuple[str, str], Mapping[str, Any] | None]: """Fetch pricing for many (provider, model_name) pairs in one query.""" if not pairs: return {} @@ -390,12 +391,13 @@ def list_active_model_pricing( ModelConfig.is_active, ) return { - (row.provider, row.model_name): row.pricing for row in session.exec(stmt).all() + (provider, model_name): pricing + for provider, model_name, pricing in session.exec(stmt).all() } def compute_cost_from_pricing( - pricing: dict[str, Any] | None, + pricing: Mapping[str, Any] | None, provider: str, model_name: str, input_tokens: int,