chore(server): forbidden vi.mock and vi.hoist
This commit is contained in:
@@ -8,6 +8,7 @@ import type { BillingService } from '../../../services/domain/billing/billing-se
|
||||
import type { FluxMeter } from '../../../services/domain/billing/flux-meter'
|
||||
import type { FluxService } from '../../../services/domain/flux'
|
||||
import type { LlmRouteContext, LlmRouterService } from '../../../services/domain/llm-router'
|
||||
import type { ChatGenerationTrace, TtsGenerationTrace } from '../../../services/domain/llm-tracing'
|
||||
import type { RequestLogService } from '../../../services/domain/request-log'
|
||||
import type { HonoEnv } from '../../../types/hono'
|
||||
|
||||
@@ -36,6 +37,11 @@ import {
|
||||
|
||||
const tracer = trace.getTracer('v1-completions')
|
||||
|
||||
interface LlmTracingDeps {
|
||||
startChatGeneration: (input: Parameters<typeof startChatGeneration>[0]) => ChatGenerationTrace
|
||||
startTtsGeneration: (input: Parameters<typeof startTtsGeneration>[0]) => TtsGenerationTrace
|
||||
}
|
||||
|
||||
const SAFE_RESPONSE_HEADERS = new Set([
|
||||
'content-type',
|
||||
'content-length',
|
||||
@@ -94,6 +100,7 @@ export function createV1Routes(
|
||||
revenue?: RevenueMetrics | null,
|
||||
rateLimitMetrics?: RateLimitMetrics | null,
|
||||
posthog?: PostHog | null,
|
||||
llmTracing: LlmTracingDeps = { startChatGeneration, startTtsGeneration },
|
||||
) {
|
||||
const logger = useLogger('v1-completions').useGlobalConfig()
|
||||
// TODO: Extract this compat route into smaller facades/modules.
|
||||
@@ -199,7 +206,7 @@ export function createV1Routes(
|
||||
catch (err) {
|
||||
span.setStatus({ code: SpanStatusCode.ERROR, message: 'Router exhausted or unknown model' })
|
||||
span.end()
|
||||
startChatGeneration({
|
||||
llmTracing.startChatGeneration({
|
||||
input: body.messages,
|
||||
model: routeCtx.upstreamModel ?? requestModel,
|
||||
requestId,
|
||||
@@ -220,7 +227,7 @@ export function createV1Routes(
|
||||
// session cost. Use the router-resolved upstream model, not the client
|
||||
// alias (`auto` / `chat-auto`), so Langfuse model-cost grouping matches the
|
||||
// provider model that actually generated the tokens.
|
||||
const generationTrace = startChatGeneration({
|
||||
const generationTrace = llmTracing.startChatGeneration({
|
||||
input: body.messages,
|
||||
model: langfuseModel,
|
||||
requestId,
|
||||
@@ -598,7 +605,7 @@ export function createV1Routes(
|
||||
speed: typeof body.speed === 'number' ? body.speed : undefined,
|
||||
responseFormat: typeof body.response_format === 'string' ? body.response_format : undefined,
|
||||
}
|
||||
const generationTrace = startTtsGeneration({
|
||||
const generationTrace = llmTracing.startTtsGeneration({
|
||||
input: ttsInput,
|
||||
model: requestModel,
|
||||
requestId,
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { ConfigKVService } from '../../../services/adapters/config-kv'
|
||||
import type { BillingService } from '../../../services/domain/billing/billing-service'
|
||||
import type { FluxService } from '../../../services/domain/flux'
|
||||
import type { LlmRouterService } from '../../../services/domain/llm-router'
|
||||
import type { ChatGenerationTrace, TtsGenerationTrace } from '../../../services/domain/llm-tracing'
|
||||
import type { RequestLogService } from '../../../services/domain/request-log'
|
||||
import type { HonoEnv } from '../../../types/hono'
|
||||
|
||||
@@ -11,23 +12,6 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createV1Routes } from '.'
|
||||
import { ApiError } from '../../../utils/error'
|
||||
|
||||
const tracingSpies = vi.hoisted(() => ({
|
||||
startChatGeneration: vi.fn(() => ({
|
||||
appendStreamChunk: vi.fn(),
|
||||
succeed: vi.fn(),
|
||||
fail: vi.fn(),
|
||||
})),
|
||||
startTtsGeneration: vi.fn(() => ({
|
||||
succeed: vi.fn(),
|
||||
fail: vi.fn(),
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('../../../services/domain/llm-tracing', () => ({
|
||||
startChatGeneration: tracingSpies.startChatGeneration,
|
||||
startTtsGeneration: tracingSpies.startTtsGeneration,
|
||||
}))
|
||||
|
||||
function createMockFluxService(flux = 100): FluxService {
|
||||
return {
|
||||
getFlux: vi.fn(async () => ({ userId: 'user-1', flux })),
|
||||
@@ -101,6 +85,20 @@ function createMockTtsMeter(unitsPerFlux = 1000) {
|
||||
} as any
|
||||
}
|
||||
|
||||
function createMockLlmTracing() {
|
||||
return {
|
||||
startChatGeneration: vi.fn((): ChatGenerationTrace => ({
|
||||
appendStreamChunk: vi.fn(),
|
||||
succeed: vi.fn(),
|
||||
fail: vi.fn(),
|
||||
})),
|
||||
startTtsGeneration: vi.fn((): TtsGenerationTrace => ({
|
||||
succeed: vi.fn(),
|
||||
fail: vi.fn(),
|
||||
})),
|
||||
}
|
||||
}
|
||||
|
||||
function createMockLlmRouter(impl?: Partial<LlmRouterService>): LlmRouterService {
|
||||
return {
|
||||
// Default: forward to globalThis.fetch so existing chat tests that mock
|
||||
@@ -137,6 +135,7 @@ function createTestApp(
|
||||
requestLogService?: RequestLogService,
|
||||
ttsMeter?: ReturnType<typeof createMockTtsMeter>,
|
||||
llmRouter?: LlmRouterService,
|
||||
llmTracing = createMockLlmTracing(),
|
||||
) {
|
||||
const { openaiRoutes, audioRoutes } = createV1Routes(
|
||||
fluxService,
|
||||
@@ -146,6 +145,10 @@ function createTestApp(
|
||||
ttsMeter ?? createMockTtsMeter(),
|
||||
llmRouter ?? createMockLlmRouter(),
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
llmTracing,
|
||||
)
|
||||
const app = new Hono<HonoEnv>()
|
||||
|
||||
@@ -184,8 +187,7 @@ describe('v1CompletionsRoutes', () => {
|
||||
const originalFetch = globalThis.fetch
|
||||
|
||||
beforeEach(() => {
|
||||
tracingSpies.startChatGeneration.mockClear()
|
||||
tracingSpies.startTtsGeneration.mockClear()
|
||||
globalThis.fetch = originalFetch
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
@@ -419,7 +421,8 @@ describe('v1CompletionsRoutes', () => {
|
||||
})
|
||||
}) as any,
|
||||
})
|
||||
const app = createTestApp(createMockFluxService(), createMockConfigKV(), undefined, undefined, undefined, llmRouter)
|
||||
const llmTracing = createMockLlmTracing()
|
||||
const app = createTestApp(createMockFluxService(), createMockConfigKV(), undefined, undefined, undefined, llmRouter, llmTracing)
|
||||
|
||||
await app.fetch(
|
||||
new Request('http://localhost/api/v1/openai/chat/completions', {
|
||||
@@ -430,7 +433,7 @@ describe('v1CompletionsRoutes', () => {
|
||||
{ user: testUser } as any,
|
||||
)
|
||||
|
||||
expect(tracingSpies.startChatGeneration).toHaveBeenCalledWith(
|
||||
expect(llmTracing.startChatGeneration).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
model: 'openai/gpt-4o-mini',
|
||||
requestId: expect.any(String),
|
||||
@@ -742,6 +745,11 @@ describe('v1CompletionsRoutes', () => {
|
||||
// billing-failed request without a fluxConsumed value), but the
|
||||
// failure is now observable instead of hidden by a leaked span.
|
||||
it('tTS billing failure closes the span and surfaces error to onError (regression)', async () => {
|
||||
globalThis.fetch = vi.fn(async () => new Response(new Uint8Array([1]), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'audio/mpeg' },
|
||||
}))
|
||||
|
||||
const requestLogService = createMockRequestLogService()
|
||||
const ttsMeter = createMockTtsMeter()
|
||||
// Override accumulate to simulate a Redis INCRBY failure mid-billing.
|
||||
|
||||
@@ -64,6 +64,21 @@ export default defineConfig({
|
||||
'yaml/plain-scalar': 'off',
|
||||
'markdown/require-alt-text': 'off',
|
||||
},
|
||||
}, {
|
||||
files: ['apps/server/**/*.ts'],
|
||||
rules: {
|
||||
'no-restricted-syntax': [
|
||||
'error',
|
||||
{
|
||||
selector: 'CallExpression[callee.type=\'MemberExpression\'][callee.object.name=\'vi\'][callee.property.name=/^(mock|doMock)$/][arguments.0.type=\'Literal\'][arguments.0.value=/^(\\.|@proj-airi\\/|~)/]',
|
||||
message: 'Do not mock internal project modules with vi.mock or vi.doMock. Inject the collaborator through the route, service, or factory boundary and pass a fake or spy in tests.',
|
||||
},
|
||||
{
|
||||
selector: 'CallExpression[callee.type=\'MemberExpression\'][callee.object.name=\'vi\'][callee.property.name=\'hoisted\']',
|
||||
message: 'Do not use vi.hoisted. If a test needs a collaborator spy, expose an explicit dependency injection point instead of hoisting module mocks.',
|
||||
},
|
||||
],
|
||||
},
|
||||
}, {
|
||||
ignores: [
|
||||
'**/*.md',
|
||||
|
||||
Reference in New Issue
Block a user