Skip to content

Commit 4f929ea

Browse files
nizzlenitzclaude
andcommitted
test(chat,admin): cover clientIp/hashIp + extract & test metrics aggregation
Applying the "capture what /verify checked by hand" principle to two gaps the suite missed: 1. clientIp/hashIp were extracted into chat-pure.ts to be testable, but the unit test skipped them — even though every quota/metering /verify probe this session leaned on clientIp's X-Forwarded-For handling (spoofing surface). clientIp now reads CHAT_TRUST_PROXY at call time (was a module-load const) so both branches are testable and it reflects current env. Added tests: untrusted ignores the header (anti-spoof), trusted takes the rightmost hop / trims / skips empty hops, and hashIp is 16-hex, deterministic, collision-resistant. 2. The admin metrics aggregators (chatAnalytics/searchAnalytics) had zero coverage — /verify only ever eyeballed their output on the dashboard. Extract the pure compute (windowing, grounded/cache-hit/zero-rate math, faithfulness averaging, top-N, zero-filled volume) into lib/metrics-pure.ts; metrics.ts now just reads the rows and calls it. Added unit tests for the rate math, window exclusion, empty-input divide-by-zero, flagged newest-first, and the volume series. Verified the extraction is behavior-preserving: /admin/chat and /admin/search still render real numbers via the new pure path (271 chats, 13.3% cache hit, 92.8% faithfulness). Unit suite 23 → 32. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eijma5uKcPZi9tqV2aJBeh
1 parent 3ca7425 commit 4f929ea

5 files changed

Lines changed: 325 additions & 174 deletions

File tree

app/lib/chat-pure.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@ export const MAX_QUESTION = 1000; // reject longer questions
1515
// window on restart.
1616
const IP_SALT = process.env.CHAT_IP_SALT || randomBytes(16).toString('hex');
1717

18-
// Only trust X-Forwarded-For when explicitly behind a proxy that appends the
19-
// real client IP (CHAT_TRUST_PROXY=true). Off by default so the socket peer —
20-
// which a client cannot spoof — is used for quota bucketing.
21-
const TRUST_PROXY = process.env.CHAT_TRUST_PROXY === 'true';
22-
2318
// Minimal shape of the request fields clientIp reads — kept local so this module
2419
// needs no Harper types (importing them would boot the runtime).
2520
interface ClientRequest {
@@ -30,9 +25,13 @@ interface ClientRequest {
3025
// ── Client identity ──────────────────────────────────────────────────────────
3126

3227
export function clientIp(request: ClientRequest): string {
33-
// Behind a trusted proxy, take the RIGHTMOST X-Forwarded-For hop — the one the
34-
// proxy appended — since the leftmost entries are client-supplied and spoofable.
35-
if (TRUST_PROXY) {
28+
// Only trust X-Forwarded-For when explicitly behind a proxy that appends the
29+
// real client IP (CHAT_TRUST_PROXY=true). Off by default so the socket peer —
30+
// which a client cannot spoof — is used for quota bucketing. Read at call time
31+
// (not a module const) so it reflects the current env and is unit-testable.
32+
if (process.env.CHAT_TRUST_PROXY === 'true') {
33+
// Behind a trusted proxy, take the RIGHTMOST X-Forwarded-For hop — the one the
34+
// proxy appended — since the leftmost entries are client-supplied and spoofable.
3635
const fwd = request.headers.get('x-forwarded-for');
3736
if (fwd) {
3837
const hops = fwd

app/lib/metrics-pure.ts

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Pure aggregation for the admin analytics — no Harper runtime. Split out of
2+
// metrics.ts (which imports `tables` and so boots the database on import) so the
3+
// windowing + rate math is unit-testable. metrics.ts reads the rows and hands
4+
// them here. Rows are plain records straight off the tables, hence `any`.
5+
6+
import type { SearchAnalytics, ChatAnalytics, ChatRecent, ChatFlagged } from './admin.ts';
7+
8+
const DAY_MS = 86400000;
9+
10+
// Aggregate SearchQueryLog rows into top queries, the zero-result content-gap
11+
// list, per-day volume, and a section breakdown, over the last `windowDays`.
12+
export function aggregateSearch(rows: any[], now: number, windowDays: number): SearchAnalytics {
13+
const windowMs = windowDays * DAY_MS;
14+
const byQuery = new Map<string, { count: number; zeroCount: number }>();
15+
const bySection = new Map<string, number>();
16+
const byDay = new Map<string, number>();
17+
let queries = 0;
18+
let zeroResult = 0;
19+
20+
for (const row of rows) {
21+
const created = new Date(row.createdAt ?? 0).getTime();
22+
if (now - created > windowMs) continue; // outside the reporting window
23+
queries++;
24+
const q = String(row.query ?? '')
25+
.trim()
26+
.toLowerCase();
27+
const isZero = (row.resultCount ?? 0) === 0;
28+
if (isZero) zeroResult++;
29+
if (q) {
30+
const e = byQuery.get(q) ?? { count: 0, zeroCount: 0 };
31+
e.count++;
32+
if (isZero) e.zeroCount++;
33+
byQuery.set(q, e);
34+
}
35+
const sec = row.section || 'all';
36+
bySection.set(sec, (bySection.get(sec) ?? 0) + 1);
37+
const day = new Date(created).toISOString().slice(0, 10);
38+
byDay.set(day, (byDay.get(day) ?? 0) + 1);
39+
}
40+
41+
const topTerms = [...byQuery.entries()]
42+
.map(([query, e]) => ({ query, count: e.count, zero: e.count > 0 && e.zeroCount === e.count }))
43+
.sort((a, b) => b.count - a.count)
44+
.slice(0, 15);
45+
46+
// Content gaps: zero-result queries, excluding sub-3-char keystroke fragments
47+
// (the search UI logs each debounced partial, so "s"/"sc" are noise, not gaps).
48+
const zeroResultList = [...byQuery.entries()]
49+
.filter(([query, e]) => e.zeroCount > 0 && query.length >= 3)
50+
.map(([query, e]) => ({ query, count: e.zeroCount }))
51+
.sort((a, b) => b.count - a.count)
52+
.slice(0, 15);
53+
54+
// Fill every day in the window (including zero days) for a continuous chart.
55+
const volume: Array<{ day: string; count: number }> = [];
56+
for (let i = windowDays - 1; i >= 0; i--) {
57+
const day = new Date(now - i * DAY_MS).toISOString().slice(0, 10);
58+
volume.push({ day, count: byDay.get(day) ?? 0 });
59+
}
60+
61+
const bySectionArr = [...bySection.entries()]
62+
.map(([section, count]) => ({ section, count }))
63+
.sort((a, b) => b.count - a.count);
64+
65+
return {
66+
totals: {
67+
// zeroRate is a 0..1 fraction; the renderer formats it as a percentage.
68+
queries,
69+
zeroResult,
70+
zeroRate: queries ? zeroResult / queries : 0,
71+
windowDays,
72+
},
73+
topTerms,
74+
zeroResult: zeroResultList,
75+
volume,
76+
bySection: bySectionArr,
77+
};
78+
}
79+
80+
// Aggregate ChatLog rows into volume, grounded/cache-hit rates, latency, top
81+
// questions, model + feedback breakdowns, faithfulness, and recent conversations.
82+
export function aggregateChat(rows: any[], now: number, windowDays: number): ChatAnalytics {
83+
const windowMs = windowDays * DAY_MS;
84+
const byQuestion = new Map<string, number>();
85+
const byDay = new Map<string, number>();
86+
const byModel = new Map<string, number>();
87+
const recent: ChatRecent[] = [];
88+
const flagged: ChatFlagged[] = [];
89+
const feedback = { up: 0, down: 0, none: 0 };
90+
let chats = 0;
91+
let grounded = 0;
92+
let cacheHits = 0;
93+
let latencySum = 0;
94+
let latencyN = 0;
95+
let faithSum = 0;
96+
let faithN = 0;
97+
let flaggedCount = 0;
98+
99+
for (const row of rows) {
100+
const created = new Date(row.createdAt ?? 0).getTime();
101+
if (now - created > windowMs) continue;
102+
chats++;
103+
if (row.grounded) grounded++;
104+
if (row.cached) cacheHits++;
105+
if (typeof row.faithfulness === 'number') {
106+
faithSum += row.faithfulness;
107+
faithN++;
108+
if (row.flagged) {
109+
flaggedCount++;
110+
flagged.push({
111+
question: row.question ?? '',
112+
faithfulness: row.faithfulness,
113+
note: String(row.flaggedNote ?? ''),
114+
createdAt: row.createdAt ?? 0,
115+
});
116+
}
117+
}
118+
if (typeof row.latencyMs === 'number') {
119+
latencySum += row.latencyMs;
120+
latencyN++;
121+
}
122+
const q = String(row.question ?? '').trim();
123+
if (q) byQuestion.set(q.toLowerCase(), (byQuestion.get(q.toLowerCase()) ?? 0) + 1);
124+
const day = new Date(created).toISOString().slice(0, 10);
125+
byDay.set(day, (byDay.get(day) ?? 0) + 1);
126+
const model = row.model || 'unknown';
127+
byModel.set(model, (byModel.get(model) ?? 0) + 1);
128+
const fb = row.feedback ?? 0;
129+
if (fb > 0) feedback.up++;
130+
else if (fb < 0) feedback.down++;
131+
else feedback.none++;
132+
recent.push({
133+
question: row.question ?? '',
134+
answerPreview: String(row.answer ?? '').slice(0, 160),
135+
sources: Array.isArray(row.sources) ? row.sources.length : 0,
136+
model,
137+
latencyMs: row.latencyMs ?? 0,
138+
grounded: Boolean(row.grounded),
139+
createdAt: row.createdAt ?? 0,
140+
});
141+
}
142+
143+
recent.sort((a, b) => new Date(b.createdAt ?? 0).getTime() - new Date(a.createdAt ?? 0).getTime());
144+
flagged.sort((a, b) => new Date(b.createdAt ?? 0).getTime() - new Date(a.createdAt ?? 0).getTime());
145+
const topQuestions = [...byQuestion.entries()]
146+
.map(([question, count]) => ({ question, count }))
147+
.sort((a, b) => b.count - a.count)
148+
.slice(0, 15);
149+
const volume: Array<{ day: string; count: number }> = [];
150+
for (let i = windowDays - 1; i >= 0; i--) {
151+
const day = new Date(now - i * DAY_MS).toISOString().slice(0, 10);
152+
volume.push({ day, count: byDay.get(day) ?? 0 });
153+
}
154+
const byModelArr = [...byModel.entries()]
155+
.map(([model, count]) => ({ model, count }))
156+
.sort((a, b) => b.count - a.count);
157+
158+
return {
159+
totals: {
160+
chats,
161+
grounded,
162+
groundedRate: chats ? grounded / chats : 0,
163+
cacheHits,
164+
cacheHitRate: chats ? cacheHits / chats : 0,
165+
avgLatencyMs: latencyN ? latencySum / latencyN : 0,
166+
windowDays,
167+
avgFaithfulness: faithN ? faithSum / faithN : 0,
168+
scored: faithN,
169+
flaggedCount,
170+
},
171+
topQuestions,
172+
volume,
173+
byModel: byModelArr,
174+
feedback,
175+
flagged: flagged.slice(0, 12),
176+
recent: recent.slice(0, 12),
177+
};
178+
}

0 commit comments

Comments
 (0)