models: resolve Fireworks-hosted models to friendly display names#787
Conversation
Fireworks fleet models arrive as `accounts/fireworks/models/<slug>` and were leaking the raw path/slug in reports and the dashboard (e.g. `fireworks/mode…`, `glm-5p2`). getShortModelName's path fallback now takes the last path segment and re-resolves it, so a known slug earns a friendly name (GLM-5.2, Qwen 3.7 Plus, Kimi K2.7 Code, DeepSeek v4 Pro/Flash) while an unmapped slug still falls through to the raw segment. Display-only: getModelCosts prices off the full id, so dollar amounts are unchanged. The By Model dashboard panel now renders through getShortModelName too, so already-cached raw keys normalize without a cache rebuild.
Once path-style ids normalize, a mixed-vintage cache can hold more than one raw key for the same model (the full accounts/fireworks/models/<slug> path from an older build and the bare slug/friendly name from a newer one). The dashboard By Model panel aggregated by raw id, so those rendered as duplicate rows once both mapped to the same display name. Extract the aggregation into a pure, tested aggregateModelTotals() keyed by the resolved display name so the rows merge into one. This also aligns the panel with modelEfficiency, which is already keyed by getShortModelName.
…w-path-model-ids # Conflicts: # src/dashboard.tsx
therickfactr
changed the title
|
Reviewed in depth, and this is exactly the PR the issue thread hoped for. Security pass over every line: display-only, no network, environment, or process surface, sources and tests only, nothing touching CI. Correctness: the path-fallback re-resolution cannot recurse (segments carry no slashes) and the empty-segment edge is guarded; the new aggregateModelTotals merges mixed-vintage cache keys exactly as discussed, and your test covers the merge case; friendly names round-trip idempotently through the resolver so merged keys cannot drift; the dashboard now has zero direct getShortModelName calls, so no double resolution. Typecheck clean, your three touched suites green (163 tests) on the head, and the one full-suite failure was ours, not yours (a stale key-list assertion from an unrelated titles change, fixed separately in #789). Verdict: ready to merge. Thanks for the quality here, from the report to the doc comments. |
|
@iamtoruk, you are more than welcome. I don't like (or do) sloppy work unless there is no other option. |
2 participants
Summary
accounts/fireworks/models/<slug>, arriving via the OpenCode provider) were leaking the raw path/slug:fireworks/mode…in the dashboard By Model panel andmodels/glm-5p2incodeburn models.getShortModelName's path fallback now takes the last segment and re-resolves it, so a known slug earns a friendly name (GLM-5.2, Qwen 3.7 Plus, Kimi K2.7 Code, DeepSeek v4 Pro/Flash) while an unmapped slug still falls through to the raw segment.getModelCostsprices off the full id, so no dollar amounts move.aggregateModelTotals), so a mixed-vintage cache holding both the fullaccounts/fireworks/models/<slug>path (older sessions) and the bare slug (newer ones) merges into one row instead of rendering duplicates. This also aligns the panel withmodelEfficiency, which is already keyed bygetShortModelName.Closes #779.
Testing
npm testpasses for the affected areasnpm run buildsucceedsVerified against real OpenCode/Fireworks sessions —
codeburn modelsnow showsGLM-5.2 / DeepSeek v4 Pro / Kimi K2.7 Code / Qwen 3.7 Plus / DeepSeek v4 Flash with
unchanged costs.
tests/models.test.tsextended (the last-segment fallbackassertion is kept for a still-unmapped id); new
tests/model-breakdown.test.tscovers the row-merge case.
tsc --noEmitclean.Note on the full suite: the only failures are two pre-existing flakes in
app/electron/cli.test.ts(spawnSpecFor) andtests/cli-status-menubar.test.ts(menubar payload filter) that also fail on a clean
main— unrelated to this change.