From 02520255dbf7277fc3d3d71fa0e52d1eea5e285e Mon Sep 17 00:00:00 2001 From: cita-777 Date: Sat, 18 Apr 2026 13:19:29 +0800 Subject: [PATCH] Clear remaining low-risk review debt on the release branch The PR still had a few targeted review threads around contributor pagination, route-layer passthroughs, dead header fallbacks, and synthetic response ids. These are small enough to settle directly on the release branch without broadening the merge scope, and they tighten the repo back toward the service-owned boundaries this refactor is aiming for. Constraint: Keep the changes narrow and regression-covered so the release PR stays green while review threads are closed out Rejected: Leave the minor threads for follow-up after merge | they were isolated enough to fix now and would keep noise on the PR Confidence: high Scope-risk: narrow Directive: When a route shim stops serving any route-local behavior, point callers at the owning service instead of keeping a passthrough around Tested: Focused vitest for contributor updater, openai responses response bridge, upstream request builder, stats proxy logs, architecture boundaries; typecheck; repo drift check; git diff --check Not-tested: Full npm test; unresolved review threads that still require larger product decisions --- .../dev/update-readme-contributors.test.ts | 8 ++ scripts/dev/update-readme-contributors.ts | 23 ++++- src/server/routes/api/stats.ts | 2 +- src/server/routes/proxy/completions.ts | 2 +- src/server/routes/proxy/embeddings.ts | 2 +- src/server/routes/proxy/images.ts | 2 +- src/server/routes/proxy/logPathMeta.ts | 5 -- src/server/routes/proxy/search.ts | 2 +- src/server/services/upstreamRequestBuilder.ts | 1 - .../openai/responses/responseBridge.test.ts | 83 ++++++++++++++++++- .../openai/responses/responseBridge.ts | 15 +++- 11 files changed, 127 insertions(+), 18 deletions(-) delete mode 100644 src/server/routes/proxy/logPathMeta.ts diff --git a/scripts/dev/update-readme-contributors.test.ts b/scripts/dev/update-readme-contributors.test.ts index 6699097..9564f01 100644 --- a/scripts/dev/update-readme-contributors.test.ts +++ b/scripts/dev/update-readme-contributors.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import { + parsePaginatedJson, normalizeContributors, replaceContributorsSection, renderContributorsBlock, @@ -108,4 +109,11 @@ describe('README contributors updater', () => { expect(html).toContain('\n { + expect(parsePaginatedJson('[{"login":"a"}][{"login":"b"}]')).toEqual([ + { login: 'a' }, + { login: 'b' }, + ]); + }); }); diff --git a/scripts/dev/update-readme-contributors.ts b/scripts/dev/update-readme-contributors.ts index fd3c16a..d5b8430 100644 --- a/scripts/dev/update-readme-contributors.ts +++ b/scripts/dev/update-readme-contributors.ts @@ -127,9 +127,28 @@ export function fetchContributors(repo = REPO): ReadmeContributor[] { return normalizeContributors(parsePaginatedJson(raw) as GitHubContributor[]); } -function parsePaginatedJson(raw: string): unknown[] { +export function parsePaginatedJson(raw: string): unknown[] { + const trimmedRaw = raw.trim(); + if (!trimmedRaw) return []; + + try { + const parsed = JSON.parse(trimmedRaw); + return Array.isArray(parsed) ? parsed : [parsed]; + } catch { + // Fall through to paginated-array parsing. + } + + try { + const mergedArrays = JSON.parse(`[${trimmedRaw.replace(/\]\s*\[/g, '],[')}]`); + if (Array.isArray(mergedArrays)) { + return mergedArrays.flatMap((item) => (Array.isArray(item) ? item : [item])); + } + } catch { + // Fall through to line-by-line parsing. + } + const items: unknown[] = []; - for (const line of raw.split('\n')) { + for (const line of trimmedRaw.split('\n')) { const trimmed = line.trim(); if (!trimmed) { continue; diff --git a/src/server/routes/api/stats.ts b/src/server/routes/api/stats.ts index e2cad77..c6cdf8e 100644 --- a/src/server/routes/api/stats.ts +++ b/src/server/routes/api/stats.ts @@ -31,7 +31,7 @@ import { getProxyDebugTraceDetail, listProxyDebugTraces, } from "../../services/proxyDebugTraceStore.js"; -import { parseProxyLogMessageMeta } from "../proxy/logPathMeta.js"; +import { parseProxyLogMessageMeta } from "../../services/proxyLogMessage.js"; import { requiresManagedAccountTokens } from "../../services/accountExtraConfig.js"; import { ACCOUNT_TOKEN_VALUE_STATUS_READY } from "../../services/accountTokenService.js"; import { diff --git a/src/server/routes/proxy/completions.ts b/src/server/routes/proxy/completions.ts index 379dfed..49b5a29 100644 --- a/src/server/routes/proxy/completions.ts +++ b/src/server/routes/proxy/completions.ts @@ -10,7 +10,7 @@ import { mergeProxyUsage, parseProxyUsage, pullSseDataEvents } from '../../servi import { ensureModelAllowedForDownstreamKey, getDownstreamRoutingPolicy, recordDownstreamCostUsage } from './downstreamPolicy.js'; import { withSiteRecordProxyRequestInit } from '../../services/siteProxy.js'; import { getProxyUrlFromExtraConfig } from '../../services/accountExtraConfig.js'; -import { composeProxyLogMessage } from './logPathMeta.js'; +import { composeProxyLogMessage } from '../../services/proxyLogMessage.js'; import { formatUtcSqlDateTime } from '../../services/localTimeService.js'; import { detectProxyFailure } from '../../services/proxyFailureJudge.js'; import { resolveProxyLogBilling } from './proxyBilling.js'; diff --git a/src/server/routes/proxy/embeddings.ts b/src/server/routes/proxy/embeddings.ts index 1b076df..3328797 100644 --- a/src/server/routes/proxy/embeddings.ts +++ b/src/server/routes/proxy/embeddings.ts @@ -10,7 +10,7 @@ import { parseProxyUsage } from '../../services/proxyUsageParser.js'; import { ensureModelAllowedForDownstreamKey, getDownstreamRoutingPolicy, recordDownstreamCostUsage } from './downstreamPolicy.js'; import { withSiteRecordProxyRequestInit } from '../../services/siteProxy.js'; import { getProxyUrlFromExtraConfig } from '../../services/accountExtraConfig.js'; -import { composeProxyLogMessage } from './logPathMeta.js'; +import { composeProxyLogMessage } from '../../services/proxyLogMessage.js'; import { formatUtcSqlDateTime } from '../../services/localTimeService.js'; import { resolveProxyLogBilling } from './proxyBilling.js'; import { getProxyAuthContext } from '../../middleware/auth.js'; diff --git a/src/server/routes/proxy/images.ts b/src/server/routes/proxy/images.ts index 7b84c5c..6f15486 100644 --- a/src/server/routes/proxy/images.ts +++ b/src/server/routes/proxy/images.ts @@ -9,7 +9,7 @@ import { shouldRetryProxyRequest } from '../../services/proxyRetryPolicy.js'; import { ensureModelAllowedForDownstreamKey, getDownstreamRoutingPolicy, recordDownstreamCostUsage } from './downstreamPolicy.js'; import { withSiteRecordProxyRequestInit } from '../../services/siteProxy.js'; import { getProxyUrlFromExtraConfig } from '../../services/accountExtraConfig.js'; -import { composeProxyLogMessage } from './logPathMeta.js'; +import { composeProxyLogMessage } from '../../services/proxyLogMessage.js'; import { formatUtcSqlDateTime } from '../../services/localTimeService.js'; import { cloneFormDataWithOverrides, ensureMultipartBufferParser, parseMultipartFormData } from './multipart.js'; import { getProxyAuthContext } from '../../middleware/auth.js'; diff --git a/src/server/routes/proxy/logPathMeta.ts b/src/server/routes/proxy/logPathMeta.ts deleted file mode 100644 index d2396eb..0000000 --- a/src/server/routes/proxy/logPathMeta.ts +++ /dev/null @@ -1,5 +0,0 @@ -export { - composeProxyLogMessage, - parseProxyLogMessageMeta, - type ParsedProxyLogMessageMeta, -} from '../../services/proxyLogMessage.js'; diff --git a/src/server/routes/proxy/search.ts b/src/server/routes/proxy/search.ts index fc4c157..19fd4f7 100644 --- a/src/server/routes/proxy/search.ts +++ b/src/server/routes/proxy/search.ts @@ -8,7 +8,7 @@ import { shouldRetryProxyRequest } from '../../services/proxyRetryPolicy.js'; import { ensureModelAllowedForDownstreamKey, getDownstreamRoutingPolicy, recordDownstreamCostUsage } from './downstreamPolicy.js'; import { withSiteRecordProxyRequestInit } from '../../services/siteProxy.js'; import { getProxyUrlFromExtraConfig } from '../../services/accountExtraConfig.js'; -import { composeProxyLogMessage } from './logPathMeta.js'; +import { composeProxyLogMessage } from '../../services/proxyLogMessage.js'; import { formatUtcSqlDateTime } from '../../services/localTimeService.js'; import { getProxyAuthContext } from '../../middleware/auth.js'; import { buildUpstreamUrl } from './upstreamUrl.js'; diff --git a/src/server/services/upstreamRequestBuilder.ts b/src/server/services/upstreamRequestBuilder.ts index 7150675..020fee4 100644 --- a/src/server/services/upstreamRequestBuilder.ts +++ b/src/server/services/upstreamRequestBuilder.ts @@ -580,7 +580,6 @@ export function buildUpstreamEndpointRequest(input: { : {}; const anthropicVersion = ( claudeHeaders['anthropic-version'] - || passthroughHeaders['anthropic-version'] || '2023-06-01' ); const nativeClaudeBody = ( diff --git a/src/server/transformers/openai/responses/responseBridge.test.ts b/src/server/transformers/openai/responses/responseBridge.test.ts index a67d838..49aea73 100644 --- a/src/server/transformers/openai/responses/responseBridge.test.ts +++ b/src/server/transformers/openai/responses/responseBridge.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { buildNormalizedFinalToOpenAiResponsesPayload, @@ -211,6 +211,87 @@ describe('openai responses response bridge', () => { }); }); + it('generates unique synthetic ids within the same millisecond fallback window', () => { + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1700000000); + try { + const first = buildNormalizedFinalToOpenAiResponsesPayload({ + upstreamPayload: { + id: '', + model: 'gpt-5', + output: [ + { + id: '', + type: 'function_call', + name: 'lookup_weather', + arguments: '{"city":"Paris"}', + }, + { + id: '', + type: 'message', + role: 'assistant', + status: 'completed', + content: [{ type: 'output_text', text: 'first' }], + }, + ], + }, + normalized: { + id: '', + model: 'gpt-5', + created: 1700000000, + content: 'first', + reasoningContent: '', + finishReason: 'stop', + toolCalls: [{ + id: '', + name: 'lookup_weather', + arguments: '{"city":"Paris"}', + }], + }, + usage: { + promptTokens: 1, + completionTokens: 1, + totalTokens: 2, + }, + }); + const second = buildNormalizedFinalToOpenAiResponsesPayload({ + upstreamPayload: { + id: '', + model: 'gpt-5', + output: [ + { + id: '', + type: 'message', + role: 'assistant', + status: 'completed', + content: [{ type: 'output_text', text: 'second' }], + }, + ], + }, + normalized: { + id: '', + model: 'gpt-5', + created: 1700000000, + content: 'second', + reasoningContent: '', + finishReason: 'stop', + toolCalls: [], + }, + usage: { + promptTokens: 1, + completionTokens: 1, + totalTokens: 2, + }, + }); + + expect(first.id).not.toBe(second.id); + const firstOutput = first.output as Array>; + const secondOutput = second.output as Array>; + expect(firstOutput[1]?.id).not.toBe(secondOutput[0]?.id); + } finally { + nowSpy.mockRestore(); + } + }); + it('keeps the outbound facade pointed at the response bridge object', () => { expect(openAiResponsesOutbound).toBe(openAiResponsesResponseBridge); }); diff --git a/src/server/transformers/openai/responses/responseBridge.ts b/src/server/transformers/openai/responses/responseBridge.ts index 858b842..9d0f988 100644 --- a/src/server/transformers/openai/responses/responseBridge.ts +++ b/src/server/transformers/openai/responses/responseBridge.ts @@ -13,6 +13,13 @@ function asTrimmedString(value: unknown): string { return typeof value === 'string' ? value.trim() : ''; } +let syntheticIdCounter = 0; + +function createSyntheticId(prefix: 'resp' | 'msg' | 'call'): string { + syntheticIdCounter += 1; + return `${prefix}_${Date.now()}_${syntheticIdCounter}`; +} + function cloneJson(value: T): T { if (Array.isArray(value)) { return value.map((item) => cloneJson(item)) as T; @@ -26,18 +33,18 @@ function cloneJson(value: T): T { } function ensureResponseId(rawId: string): string { - const trimmed = rawId.trim() || `resp_${Date.now()}`; + const trimmed = rawId.trim() || createSyntheticId('resp'); return trimmed.startsWith('resp_') ? trimmed : `resp_${trimmed}`; } function ensureMessageId(rawId: string): string { - const trimmed = rawId.trim() || `msg_${Date.now()}`; + const trimmed = rawId.trim() || createSyntheticId('msg'); return trimmed.startsWith('msg_') ? trimmed : `msg_${trimmed}`; } function ensureFunctionCallId(rawId: string): string { const trimmed = rawId.trim(); - if (!trimmed) return `call_${Date.now()}`; + if (!trimmed) return createSyntheticId('call'); return trimmed.startsWith('call_') ? trimmed : `call_${trimmed}`; } @@ -331,7 +338,7 @@ export function buildNormalizedFinalToOpenAiResponsesPayload(input: { const normalizedId = typeof normalized.id === 'string' && normalized.id.trim() ? normalized.id.trim() - : `resp_${Date.now()}`; + : createSyntheticId('resp'); const responseId = ensureResponseId(normalizedId); const messageId = ensureMessageId(normalizedId); const syntheticOutput = extractSyntheticOutputItemsFromUpstream(upstreamPayload);