allenai / allenai/asta-autodiscovery
Replace the hand-maintained LLM price table with litellm's cost data
- Lingua principale
- Python
- Stelle
- 10
- Fork
- 2
- Merge medio
- 1g 21h
- PR unite (30g)
- 11
Descrizione
Split out of #68 (per review) so that PR stays scoped to provider/model routing. Related: #67.
## Problem
`api/metrics/costs.py` carries a hand-maintained price table and a provider-prefix stripper:
```python
# Pricing per 1M tokens (USD) — Vertex AI standard pricing
LLM_PRICING: dict[str, dict[str, float]] = {
"gemini-3.1-pro-preview": {"input": 2.00, "output": 12.00},
... # 26 entries
}
# Fallback pricing for unknown models (conservative: uses gemini-3.1-pro rates)
_FALLBACK_PRICING = {"input": 2.00, "output": 12.00}
def _lookup_pricing(model_name: str) -> dict[str, float]:
if model_name in LLM_PRICING:
return LLM_PRICING[model_name]
# Strip provider prefix (e.g., "google/gemini-3.1-pro-preview" -> "gemini-3.1-pro-preview")
stripped = model_name.split("/")[-1] if "/" in model_name else model_name
...
return _FALLBACK_PRICING
```
This is the last piece of model-name-specific logic in the repo after #68 — same category as the `is_gemini_model` / `google/` prefix handling that #67 removed everywhere else. It has three concrete failure modes:
1. **Unknown models are silently mispriced.** `_FALLBACK_PRICING` is Gemini 3.1 Pro's rate ($2/$12 per 1M). Any model not in the 26-entry table — including every model added after the table was last touched — is billed at Gemini Pro rates with no warning in the output.
2. **Prefix stripping discards the provider.** `github_copilot/gpt-4o` and `openai/gpt-4o` both strip to `gpt-4o` and get OpenAI list price. Copilot doesn't bill per token at all (it bills "premium requests"), so any Copilot run's reported cost is meaningless. #68 makes mixed-provider runs expressible, so this is now reachable rather than theoretical.
3. **It drifts by construction.** Prices are a snapshot with no update mechanism, and the header says "Vertex AI standard pricing" while the table also contains OpenAI models.
## Why this is worth doing now
After #68, litellm is the transport for every model call, and it **already computes the cost of each call** — `response._hidden_params["response_cost"]`, derived from its own maintained per-model `input_cost_per_token` / `output_cost_per_token`.
We currently throw that away. `LiteLLMAG2Client.get_usage()` surfaces it to AG2, but `UsageTracker` does not: `_extract_usage_from_response` reads only `model` and the token counts, and `_empty_bucket()` has no cost field. So the authoritative number is computed on every call and discarded, and then approximated later from a stale table.
## Proposal
**Record the cost at the source, not in the dashboard.**
1. Have `UsageTracker.record_response` read `_hidden_params["response_cost"]` and store it on the event; add `cost_usd` to `_empty_bucket()` so it aggregates into `by_model` / `by_agent` / `by_node` / `by_component` alongside the token counts.
2. `api/metrics/costs.py` then sums a field instead of pricing anything. `calculate_llm_cost` keeps its signature and return type, so `aggregator.py` (3 call sites: lines 191, 398, 952) is unaffected.
3. Delete `LLM_PRICING`, `_FALLBACK_PRICING` and `_lookup_pricing`.
Note that `_lookup_pricing` is imported directly by `aggregator.py:29` and used at line 191, so that call site needs looking at too — it prices a single model outside `calculate_llm_cost`.
**Explicitly not** adding litellm to `api/`. It is a separate lightweight Flask service with its own `requirements.txt`, pinned to Python 3.11, and pulling litellm in for a price lookup is a bad trade. Keeping the cost computation on the producing side avoids that entirely.
## Backwards compatibility
Runs completed before this change have `llm_usage_summary.json` files with no cost field. The dashboard needs to either:
- fall back to the current table for those runs (keeping it around as a legacy path, defeating much of the point), or
- report cost as unavailable for pre-change runs, or
- backfill the summaries.
Worth a decision before implementing. Reporting "unavailable" is probably right — the old numbers are approximations anyway, and silently mixing two costing methods in one dashboard is worse than a gap.
## Also worth checking
- Whether litellm has cost data for `github_copilot/*` at all. If it does not, the honest output is "cost unavailable for this provider" rather than a token-based estimate — see failure mode 2.
- `calculate_llm_cost` currently derives output tokens as `total - prompt` to capture reasoning tokens. litellm's own cost already accounts for reasoning tokens, so that arithmetic goes away with the table.
Guida per i contributori
Nessuna guida per i contributori indicizzata per questo repository
Valutazione
Questa issue non è ancora stata valutata.