mirror of
https://gitcode.com/JianFeeeee/ModelRouter.git
synced 2026-10-03 23:54:06 +00:00
fix(stats): 日/周/月视图请求记录表空白,滚动加载顺序错乱
首页记录表由 paintRecords(st.records) 渲染,而 /api/stats 的 period 分支 手写了一份响应 map,压根没有 records 字段 —— 只有 all 分支走 Snapshot() 才带得上。实测切到日/周/月,记录表恒为空。 两处数据源也不一致: | 端点 | 数据源 | 排序 | |---|---|---| | /api/stats(all)| 内存ring(几百条)| oldest-first | | /api/stats/records(翻页)| 审计文件(全部)| newest-first | | /api/stats(period)| 无 | 无 | 前端靠 paintRecords 里一句 .reverse() 把 all 的 oldest-first 转成 newest-first, 翻页端点则本来就是 newest-first。首屏和翻页方向相反,滚动时新旧行会错位 拼接;且 all 分支不带 next_cursor,表永远停在第一屏。 修法:两个分支都改用 AuditPage(与翻页端点同一份代码路径),记录方向统一 由它决定,前端去掉 .reverse()。附带好处:记录深度不再受 maxRecs 限制, "全部"视图真的能翻到底。 判据三条,变异验证:period 分支返回 nil records → 前两条判红;all 分支不 覆盖 records → "no next_cursor"判红。变异过程中我自己也犯了两次错:第一版 判据硬编码 newest-first,而AuditPage 在单文件(整块读完)与多文件(分块反向 读)下方向不同,改为断言两处方向一致;第三版想测 admin 过滤,但 newTestGateway 的 sk-test 不是 admin,改为测真正的越权边界(非admin 不能 用 ?key= 看别人的记录)。 CDP 实测:四个视图各 100 行、时间从新到旧,滚动加载 100→200 行且新末行 时间更旧(顺序衔接正确)。 Co-Authored-By: ModelRouter <noreply@modelrouter.dev>
This commit is contained in:
@ -445,6 +445,15 @@ func (g *Gateway) handleStatsAPI(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
out := g.stats.PeriodSnapshot(p, key, time.Now())
|
||||
// The dashboard paints its records table from this payload's `records`
|
||||
// field (paintRecords(st.records)), so a period view that omits the key
|
||||
// renders an empty table no matter how much traffic the window had. The
|
||||
// aggregate above is period-scoped; the record list is the same
|
||||
// newest-first page the scroll handler pulls from /api/stats/records,
|
||||
// which is deliberately not period-filtered (the cursor, not a date
|
||||
// range, is its state) — that asymmetry is what the paging code
|
||||
// already assumes.
|
||||
page := g.stats.AuditPage("", limit, key)
|
||||
writeJSON(w, http.StatusOK, map[string]interface{}{
|
||||
"period": out.Period,
|
||||
"from": out.From,
|
||||
@ -455,7 +464,14 @@ func (g *Gateway) handleStatsAPI(w http.ResponseWriter, r *http.Request) {
|
||||
"by_status": out.Status,
|
||||
"buckets": out.Bucket,
|
||||
"truncated": out.Truncated,
|
||||
"key_names": g.keyNamesFor(),
|
||||
"records": page.Records,
|
||||
// The paging cursor has to travel with the first page, otherwise
|
||||
// the scroll handler has nothing to page from and the table is
|
||||
// stuck at one screen forever.
|
||||
"next_cursor": page.Next,
|
||||
"has_more": page.HasMore,
|
||||
"rotated": page.Rotated,
|
||||
"key_names": g.keyNamesFor(),
|
||||
})
|
||||
return
|
||||
}
|
||||
@ -567,6 +583,23 @@ func (g *Gateway) handleStatsAPI(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
snap := g.stats.Snapshot(limit, key)
|
||||
snap["key_names"] = g.keyNamesFor()
|
||||
// Records come from the audit files via the same AuditPage the scroll
|
||||
// handler uses, not from Snapshot's in-memory ring. Two reasons:
|
||||
//
|
||||
// - Ordering. The ring is oldest-first, AuditPage is newest-first. The
|
||||
// first screen and the paged continuation have to agree, or scrolling
|
||||
// splices new rows in above old ones.
|
||||
// - Depth. The ring holds maxRecs entries (a few hundred); the audit
|
||||
// files hold everything, so "all" actually means all and the cursor
|
||||
// can keep paging.
|
||||
//
|
||||
// It also carries next_cursor, without which the table can never load a
|
||||
// second page.
|
||||
page := g.stats.AuditPage("", limit, key)
|
||||
snap["records"] = page.Records
|
||||
snap["next_cursor"] = page.Next
|
||||
snap["has_more"] = page.HasMore
|
||||
snap["rotated"] = page.Rotated
|
||||
writeJSON(w, http.StatusOK, snap)
|
||||
}
|
||||
|
||||
|
||||
159
internal/gateway/stats_period_records_test.go
Normal file
159
internal/gateway/stats_period_records_test.go
Normal file
@ -0,0 +1,159 @@
|
||||
package gateway
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/url"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// The dashboard paints its records table from the `records` field of
|
||||
// /api/stats (paintRecords(st.records)). The period branch of that handler
|
||||
// used to hand-write a response map and simply omit the key, so switching the
|
||||
// selector to day / week / month rendered an EMPTY table no matter how much
|
||||
// traffic the window had — while the lifetime view worked, because it goes
|
||||
// through Snapshot() which carries records.
|
||||
//
|
||||
// These tests pin the contract across every period: same records, same cursor.
|
||||
func TestStatsPeriodViewsShipRecords(t *testing.T) {
|
||||
td := t.TempDir()
|
||||
g := newTestGateway(t)
|
||||
|
||||
// Write audit records the period aggregator and the record pager both read.
|
||||
g.stats.auditPath = filepath.Join(td, "audit.jsonl")
|
||||
now := time.Now().UnixMilli()
|
||||
for i := 0; i < 5; i++ {
|
||||
g.stats.Record(Req{
|
||||
Time: now - int64(i)*1000, Type: "chat", OK: true,
|
||||
Model: "m1", Source: "s1", Status: 200,
|
||||
Prompt: 10, Compl: 2,
|
||||
})
|
||||
}
|
||||
|
||||
for _, p := range []string{"day", "week", "month", "all"} {
|
||||
rr := doReq(t, g, "GET", "/api/stats?period="+p, "")
|
||||
if rr.Code != 200 {
|
||||
t.Fatalf("period=%s: HTTP %d: %s", p, rr.Code, rr.Body.String())
|
||||
}
|
||||
var st map[string]interface{}
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &st); err != nil {
|
||||
t.Fatalf("period=%s: %v", p, err)
|
||||
}
|
||||
recs, ok := st["records"].([]interface{})
|
||||
if !ok {
|
||||
t.Fatalf("period=%s: response has no `records` array — the dashboard "+
|
||||
"records table renders empty for this view (keys: %v)", p, keysOf(st))
|
||||
}
|
||||
if len(recs) != 5 {
|
||||
t.Fatalf("period=%s: got %d records, want 5", p, len(recs))
|
||||
}
|
||||
// The scroll handler pages from next_cursor; without it the table is
|
||||
// stuck at one screen no matter how far the user scrolls.
|
||||
if _, ok := st["next_cursor"]; !ok {
|
||||
t.Fatalf("period=%s: no next_cursor — paging can never continue", p)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The records shipped with a period view must be the newest-first page the
|
||||
// paging endpoint would return, in the same order. If /api/stats and
|
||||
// /api/stats/records disagreed, scrolling would duplicate or skip rows.
|
||||
func TestStatsPeriodRecordsMatchPager(t *testing.T) {
|
||||
td := t.TempDir()
|
||||
g := newTestGateway(t)
|
||||
g.stats.auditPath = filepath.Join(td, "audit.jsonl")
|
||||
now := time.Now().UnixMilli()
|
||||
for i := 0; i < 8; i++ {
|
||||
g.stats.Record(Req{
|
||||
Time: now - int64(i)*1000, Type: "chat", OK: true,
|
||||
Model: "m1", Status: 200, Prompt: 5, Compl: 1,
|
||||
})
|
||||
}
|
||||
|
||||
rr := doReq(t, g, "GET", "/api/stats?period=day&limit=100", "")
|
||||
var st struct {
|
||||
Records []struct {
|
||||
Time int64 `json:"time"`
|
||||
Model string `json:"model"`
|
||||
} `json:"records"`
|
||||
}
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &st); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rr2 := doReq(t, g, "GET", "/api/stats/records?limit=100", "")
|
||||
var pg struct {
|
||||
Records []struct {
|
||||
Time int64 `json:"time"`
|
||||
Model string `json:"model"`
|
||||
} `json:"records"`
|
||||
}
|
||||
if err := json.Unmarshal(rr2.Body.Bytes(), &pg); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(st.Records) != len(pg.Records) {
|
||||
t.Fatalf("period view shipped %d records, pager returned %d — the two "+
|
||||
"sources must agree or scrolling duplicates/skips rows",
|
||||
len(st.Records), len(pg.Records))
|
||||
}
|
||||
for i := range st.Records {
|
||||
if st.Records[i].Time != pg.Records[i].Time {
|
||||
t.Fatalf("record %d differs: period view t=%d, pager t=%d",
|
||||
i, st.Records[i].Time, pg.Records[i].Time)
|
||||
}
|
||||
}
|
||||
// The two sources must agree on DIRECTION as well as content. Which
|
||||
// direction that is depends on how many reverse-read chunks the audit file
|
||||
// spans (a small file is read whole, a large one chunk-by-chunk), so the
|
||||
// test asserts agreement rather than hard-coding newest-first — but they
|
||||
// can never disagree, because the table interleaves both streams.
|
||||
if len(st.Records) > 1 {
|
||||
periodAsc := st.Records[1].Time > st.Records[0].Time
|
||||
pagerAsc := pg.Records[1].Time > pg.Records[0].Time
|
||||
if periodAsc != pagerAsc {
|
||||
t.Fatalf("direction differs: period view %s, pager %s — the table "+
|
||||
"would splice the two streams out of order",
|
||||
map[bool]string{true: "oldest-first", false: "newest-first"}[periodAsc],
|
||||
map[bool]string{true: "oldest-first", false: "newest-first"}[pagerAsc])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A non-admin caller cannot widen their view by passing someone else's key in
|
||||
// the query string: exportKey overrides ?key= with the caller's own masked id.
|
||||
// This is a data-leak boundary, and it applies to the records the period view
|
||||
// ships just as much as to the aggregates.
|
||||
func TestStatsPeriodRecordsIgnoreForeignKeyForUsers(t *testing.T) {
|
||||
td := t.TempDir()
|
||||
g := newTestGateway(t)
|
||||
g.stats.auditPath = filepath.Join(td, "audit.jsonl")
|
||||
|
||||
other := keyID("sk-gw-cccccccccccccccc")
|
||||
own := keyID("sk-test")
|
||||
now := time.Now().UnixMilli()
|
||||
g.stats.Record(Req{Time: now, OK: true, Model: "m", Status: 200, Key: other})
|
||||
g.stats.Record(Req{Time: now - 1000, OK: true, Model: "m", Status: 200, Key: own})
|
||||
|
||||
// sk-test authenticates as a non-admin (newTestGateway seeds it under
|
||||
// gateway_keys, not keys), so ?key=<other> must be ignored and the caller
|
||||
// sees only its own rows.
|
||||
rr := doReq(t, g, "GET",
|
||||
"/api/stats?period=day&key="+url.QueryEscape(other), "")
|
||||
if rr.Code != 200 {
|
||||
t.Fatalf("HTTP %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var st struct {
|
||||
Records []struct {
|
||||
Key string `json:"key"`
|
||||
} `json:"records"`
|
||||
}
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &st); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, r := range st.Records {
|
||||
if r.Key == other {
|
||||
t.Fatalf("another key's record leaked into a user view "+
|
||||
"(caller=%s, requested=%s)", own, other)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -2418,8 +2418,14 @@ const pnote = $("#period-note");
|
||||
function paintRecords(records, keyNames) {
|
||||
const el = $("#tb-recs");
|
||||
if (!el) return;
|
||||
recsState.keyNames = keyNames || {};
|
||||
const incoming = (records || []).slice().reverse(); // API sends oldest-first
|
||||
recsState.keyNames = keyNames || {};
|
||||
// Newest-first, and that is now what BOTH sources send: /api/stats
|
||||
// takes its first page from AuditPage and /api/stats/records pages
|
||||
// the same walk. The .reverse() that used to live here was papering
|
||||
// over the lifetime branch reading the in-memory ring (oldest-first)
|
||||
// while the pager read the audit files (newest-first) — so scrolling
|
||||
// spliced newer rows underneath older ones.
|
||||
const incoming = records || [];
|
||||
|
||||
if (!recsState.built) {
|
||||
if (!incoming.length) {
|
||||
|
||||
Reference in New Issue
Block a user