Skip to content

Commit 906a9eb

Browse files
SantiagoDePoloniaclaude
SantiagoDePolonia
and
claude
committed
fix: address review comments across pricing estimation feature
- Unquote Menlo/Consolas in CSS and JS chart font-family - Fix fetchUsagePage to call fetchUsageLog(true) - Add swagger annotations for admin API endpoints - Add nil guard in enricher test for InputPerMtok - Add TestFetch_OversizedBody for 10MB body limit - Replace unbounded recursion in merger with inline lookup - Add Capabilities override in merger - Refactor types.go with strings.Cut - Replace hardcoded category count with len(core.AllCategories()) - Add CategoryCount display name fallback - Use shared HTTP client in fetcher with 60s timeout - Fix init goroutine to use parent context - Escape SQL LIKE wildcards to prevent injection - Add slog.Warn for json.Unmarshal errors - Add appliedFields pointer tracking to prevent cost double-counting - Add Go doc comments on reader methods Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c598dcf commit 906a9eb

18 files changed

Lines changed: 1135 additions & 26 deletions

File tree

‎cmd/gomodel/docs/docs.go‎

Lines changed: 491 additions & 0 deletions
Large diffs are not rendered by default.

‎cmd/gomodel/docs/swagger.json‎

Lines changed: 492 additions & 1 deletion
Large diffs are not rendered by default.

‎internal/admin/dashboard/static/css/dashboard.css‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ body {
828828
}
829829

830830
.mono {
831-
font-family: 'SF Mono', 'Menlo', 'Consolas', monospace;
831+
font-family: 'SF Mono', Menlo, Consolas, monospace;
832832
font-size: 13px;
833833
}
834834

@@ -837,7 +837,7 @@ body {
837837
}
838838

839839
td.col-price {
840-
font-family: 'SF Mono', 'Menlo', 'Consolas', monospace;
840+
font-family: 'SF Mono', Menlo, Consolas, monospace;
841841
font-size: 13px;
842842
color: var(--text-muted);
843843
}

‎internal/admin/dashboard/static/js/dashboard.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ function dashboard() {
629629
},
630630

631631
async fetchUsagePage() {
632-
await Promise.all([this.fetchModelUsage(), this.fetchUsageLog(false)]);
632+
await Promise.all([this.fetchModelUsage(), this.fetchUsageLog(true)]);
633633
this.renderDonutChart();
634634
},
635635

@@ -803,7 +803,7 @@ function dashboard() {
803803
grid: { display: false },
804804
ticks: {
805805
color: colors.text,
806-
font: { size: 11, family: "'SF Mono', 'Menlo', 'Consolas', monospace" },
806+
font: { size: 11, family: "'SF Mono', Menlo, Consolas, monospace" },
807807
maxRotation: 45,
808808
minRotation: 0
809809
}
@@ -812,7 +812,7 @@ function dashboard() {
812812
grid: { color: colors.grid, drawBorder: false },
813813
ticks: {
814814
color: colors.text,
815-
font: { size: 11, family: "'SF Mono', 'Menlo', 'Consolas', monospace" },
815+
font: { size: 11, family: "'SF Mono', Menlo, Consolas, monospace" },
816816
callback: (v) => {
817817
if (this.usageMode === 'costs') return '$' + v.toFixed(2);
818818
return this.formatTokensShort(v);

‎internal/admin/handler.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,18 @@ func (h *Handler) DailyUsage(c echo.Context) error {
180180
}
181181

182182
// UsageByModel handles GET /admin/api/v1/usage/models
183+
//
184+
// @Summary Get usage breakdown by model
185+
// @Tags admin
186+
// @Produce json
187+
// @Security BearerAuth
188+
// @Param days query int false "Number of days (default 30)"
189+
// @Param start_date query string false "Start date (YYYY-MM-DD)"
190+
// @Param end_date query string false "End date (YYYY-MM-DD)"
191+
// @Success 200 {array} usage.ModelUsage
192+
// @Failure 400 {object} core.GatewayError
193+
// @Failure 401 {object} core.GatewayError
194+
// @Router /admin/api/v1/usage/models [get]
183195
func (h *Handler) UsageByModel(c echo.Context) error {
184196
if h.usageReader == nil {
185197
return c.JSON(http.StatusOK, []usage.ModelUsage{})
@@ -203,6 +215,23 @@ func (h *Handler) UsageByModel(c echo.Context) error {
203215
}
204216

205217
// UsageLog handles GET /admin/api/v1/usage/log
218+
//
219+
// @Summary Get paginated usage log entries
220+
// @Tags admin
221+
// @Produce json
222+
// @Security BearerAuth
223+
// @Param days query int false "Number of days (default 30)"
224+
// @Param start_date query string false "Start date (YYYY-MM-DD)"
225+
// @Param end_date query string false "End date (YYYY-MM-DD)"
226+
// @Param model query string false "Filter by model name"
227+
// @Param provider query string false "Filter by provider"
228+
// @Param search query string false "Search across model, provider, request_id, provider_id"
229+
// @Param limit query int false "Page size (default 50, max 200)"
230+
// @Param offset query int false "Offset for pagination"
231+
// @Success 200 {object} usage.UsageLogResult
232+
// @Failure 400 {object} core.GatewayError
233+
// @Failure 401 {object} core.GatewayError
234+
// @Router /admin/api/v1/usage/log [get]
206235
func (h *Handler) UsageLog(c echo.Context) error {
207236
if h.usageReader == nil {
208237
return c.JSON(http.StatusOK, usage.UsageLogResult{
@@ -275,6 +304,14 @@ func (h *Handler) ListModels(c echo.Context) error {
275304
}
276305

277306
// ListCategories handles GET /admin/api/v1/models/categories
307+
//
308+
// @Summary List model categories with counts
309+
// @Tags admin
310+
// @Produce json
311+
// @Security BearerAuth
312+
// @Success 200 {array} providers.CategoryCount
313+
// @Failure 401 {object} core.GatewayError
314+
// @Router /admin/api/v1/models/categories [get]
278315
func (h *Handler) ListCategories(c echo.Context) error {
279316
if h.registry == nil {
280317
return c.JSON(http.StatusOK, []providers.CategoryCount{})

‎internal/modeldata/enricher_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func TestEnrich_ReverseCustomModelIDLookup(t *testing.T) {
124124
if meta.DisplayName != "GPT-4o" {
125125
t.Errorf("DisplayName = %s, want GPT-4o", meta.DisplayName)
126126
}
127-
if meta.Pricing == nil || *meta.Pricing.InputPerMtok != 2.50 {
127+
if meta.Pricing == nil || meta.Pricing.InputPerMtok == nil || *meta.Pricing.InputPerMtok != 2.50 {
128128
t.Error("expected pricing from base model via reverse lookup")
129129
}
130130
}

‎internal/modeldata/fetcher.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,16 @@ import (
66
"fmt"
77
"io"
88
"net/http"
9+
"time"
910
)
1011

12+
// httpClient is a shared HTTP client for model list fetching.
13+
// The 60-second timeout acts as a safety net; callers should use context
14+
// deadlines for finer-grained control.
15+
var httpClient = &http.Client{
16+
Timeout: 60 * time.Second,
17+
}
18+
1119
// Fetch downloads and parses the model list from the given URL.
1220
// Returns the parsed ModelList, the raw JSON bytes (for caching), and any error.
1321
// Returns nil, nil, nil if the URL is empty (feature disabled).
@@ -17,7 +25,7 @@ func Fetch(ctx context.Context, url string) (*ModelList, []byte, error) {
1725
return nil, nil, nil
1826
}
1927

20-
client := &http.Client{}
28+
client := httpClient
2129

2230
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
2331
if err != nil {

‎internal/modeldata/fetcher_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"net/http"
66
"net/http/httptest"
7+
"strings"
78
"testing"
89
"time"
910
)
@@ -92,6 +93,25 @@ func TestFetch_Timeout(t *testing.T) {
9293
}
9394
}
9495

96+
func TestFetch_OversizedBody(t *testing.T) {
97+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
98+
// Write just over 10 MB
99+
w.Header().Set("Content-Type", "application/json")
100+
_, _ = w.Write([]byte(`{"data":"`))
101+
_, _ = w.Write([]byte(strings.Repeat("x", 10*1024*1024)))
102+
_, _ = w.Write([]byte(`"}`))
103+
}))
104+
defer server.Close()
105+
106+
_, _, err := Fetch(context.Background(), server.URL)
107+
if err == nil {
108+
t.Error("expected error for oversized body")
109+
}
110+
if err != nil && !strings.Contains(err.Error(), "too large") {
111+
t.Errorf("expected 'too large' error, got: %v", err)
112+
}
113+
}
114+
95115
func TestParse_ValidJSON(t *testing.T) {
96116
raw := []byte(`{
97117
"version": 1,

‎internal/modeldata/merger.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,35 @@ func Resolve(list *ModelList, providerType string, modelID string) *core.ModelMe
3535
}
3636
}
3737

38-
// No match at all — try reverse lookup via provider_model_id
38+
// No match at all — try reverse lookup via provider_model_id.
39+
// Inline the lookup instead of recursing to avoid unbounded recursion
40+
// if the reverse index contains cycles or stale entries.
3941
if model == nil && pm == nil {
4042
if list.providerModelByActualID != nil {
4143
reverseKey := providerType + "/" + modelID
4244
if compositeKey, ok := list.providerModelByActualID[reverseKey]; ok {
43-
return Resolve(list, providerType, compositeKey[len(providerType)+1:])
45+
canonicalModelID := compositeKey[len(providerType)+1:]
46+
// Look up the provider_model and base model directly
47+
if entry, ok := list.ProviderModels[compositeKey]; ok {
48+
pm = &entry
49+
if baseEntry, ok := list.Models[entry.ModelRef]; ok {
50+
model = &baseEntry
51+
}
52+
} else if baseEntry, ok := list.Models[canonicalModelID]; ok {
53+
model = &baseEntry
54+
}
4455
}
4556
}
46-
return nil
57+
if model == nil && pm == nil {
58+
return nil
59+
}
4760
}
4861

62+
return buildMetadata(model, pm)
63+
}
64+
65+
// buildMetadata merges base model fields with provider-model overrides into ModelMetadata.
66+
func buildMetadata(model *ModelEntry, pm *ProviderModelEntry) *core.ModelMetadata {
4967
meta := &core.ModelMetadata{}
5068

5169
// Apply base model fields
@@ -77,6 +95,9 @@ func Resolve(list *ModelList, providerType string, modelID string) *core.ModelMe
7795
if pm.Pricing != nil {
7896
meta.Pricing = pm.Pricing
7997
}
98+
if pm.Capabilities != nil {
99+
meta.Capabilities = pm.Capabilities
100+
}
80101
}
81102

82103
return meta

‎internal/modeldata/types.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,10 @@ func (l *ModelList) buildReverseIndex() {
3333
continue
3434
}
3535
// compositeKey is "providerType/modelID"
36-
slashIdx := strings.IndexByte(compositeKey, '/')
37-
if slashIdx < 0 {
36+
providerType, _, ok := strings.Cut(compositeKey, "/")
37+
if !ok {
3838
continue
3939
}
40-
providerType := compositeKey[:slashIdx]
4140
actualID := *pm.CustomModelID
4241
reverseKey := providerType + "/" + actualID
4342
// Only add if the actual ID differs from the key's model portion

0 commit comments

Comments
 (0)