-
Notifications
You must be signed in to change notification settings - Fork 722
test(responses): pin tool round-trip conformance starting slice #1618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
1a67bd4
eba7974
a6eb6e5
f4cee1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,139 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { bridgeToResponsesSSE, buildResponseJSON } from "../../src/bridge"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { AdapterEvent } from "../../src/types"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Shared harness for Responses tool round-trip conformance | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * (devlog/_plan/260813_routed_tool_discovery_profiles/030-034). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Every existing tool test re-implements `replay`/`collectSse` locally, which is why the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * streaming and non-streaming paths had never been compared: each test only looked at one. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The streamed side is read from BOTH surfaces on purpose. `response.completed` is what a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * client that reconnects or ignores deltas sees; `response.output_item.done` is what a client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * consuming normal incremental frames sees. The bridge builds them separately, so reading only | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * the snapshot hides a whole divergence class — an item can be correct in the final snapshot | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * and wrong in the incremental frame. devlog 034 requires the incremental assertions. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function* replay(events: readonly AdapterEvent[]): AsyncGenerator<AdapterEvent> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const event of events) yield event; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface SseFrame { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| event?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data: Record<string, unknown>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function collectSse(stream: ReadableStream<Uint8Array>): Promise<SseFrame[]> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const reader = stream.getReader(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const decoder = new TextDecoder(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let text = ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (;;) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { done, value } = await reader.read(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (done) break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| text += decoder.decode(value, { stream: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return text.split("\n\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(frame => frame.trim()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(frame => frame.length > 0 && frame !== "data: [DONE]") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(frame => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const lines = frame.split("\n"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const event = lines.find(line => line.startsWith("event: "))?.slice(7); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const dataLine = lines.find(line => line.startsWith("data: ")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { event, data: JSON.parse(dataLine?.slice(6) ?? "{}") as Record<string, unknown> }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** The tool-bearing fields every transport must agree on, in output order. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface NormalizedToolItem { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| call_id?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** `arguments` for function/tool_search, `input` for custom. Objects are preserved. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| payload?: unknown; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Namespace identity, when the restored item carries one. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function normalizeItem(item: Record<string, unknown>): NormalizedToolItem { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const payload = item.arguments !== undefined ? item.arguments : item.input; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: String(item.type ?? ""), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(typeof item.name === "string" ? { name: item.name } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(typeof item.call_id === "string" ? { call_id: item.call_id } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(payload !== undefined ? { payload } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(typeof item.status === "string" ? { status: item.status } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(typeof item.namespace === "string" ? { namespace: item.namespace } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isToolItem = (item: Record<string, unknown>): boolean => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String(item.type ?? "").includes("call"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+71
to
+72
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Line 72 classifies an item as a tool item when its Pin the accepted set explicitly to keep the harness meaning stable. ♻️ Proposed narrowing-const isToolItem = (item: Record<string, unknown>): boolean =>
- String(item.type ?? "").includes("call");
+const TOOL_CALL_ITEM_TYPES = new Set([
+ "function_call",
+ "custom_tool_call",
+ "tool_search_call",
+]);
+
+const isToolItem = (item: Record<string, unknown>): boolean =>
+ TOOL_CALL_ITEM_TYPES.has(String(item.type ?? ""));📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type BridgeMaps = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| toolNsMap?: Map<string, { namespace: string; name: string }>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| freeformToolNames?: Set<string>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| toolSearchToolNames?: Set<string>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface StreamedView { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Tool items from the terminal `response.completed` snapshot. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| snapshot: NormalizedToolItem[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Tool items from the incremental `response.output_item.done` frames. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| incremental: NormalizedToolItem[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Every frame's event name, in order. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eventNames: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Ordered payloads of every argument/input delta frame. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deltas: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Every non-call output item type from the snapshot, e.g. "message". */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| snapshotItemTypes: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function streamedView( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| events: readonly AdapterEvent[], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| modelId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...maps: BridgeMaps | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<StreamedView> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const frames = await collectSse(bridgeToResponsesSSE(replay(events), modelId, ...maps)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const completed = frames.find(frame => frame.event === "response.completed"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = completed?.data.response as Record<string, unknown> | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const output = Array.isArray(response?.output) ? response.output as Record<string, unknown>[] : []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const doneItems = frames | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(frame => frame.event === "response.output_item.done") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(frame => frame.data.item) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((item): item is Record<string, unknown> => !!item && typeof item === "object"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| snapshot: output.filter(isToolItem).map(normalizeItem), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| incremental: doneItems.filter(isToolItem).map(normalizeItem), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eventNames: frames.map(frame => frame.event ?? ""), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deltas: frames | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(frame => frame.event?.endsWith(".delta") && typeof frame.data.delta === "string") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(frame => String(frame.data.delta)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| snapshotItemTypes: output.map(item => String(item.type ?? "")), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Tool items from the non-streaming transport. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function jsonToolItems( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| events: readonly AdapterEvent[], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| modelId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options?: Parameters<typeof buildResponseJSON>[2], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): NormalizedToolItem[] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const body = buildResponseJSON([...events], modelId, options); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const output = Array.isArray(body.output) ? body.output as Record<string, unknown>[] : []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return output.filter(isToolItem).map(normalizeItem); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Every output item type from the non-streaming transport, including non-call items. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function jsonItemTypes( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| events: readonly AdapterEvent[], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| modelId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options?: Parameters<typeof buildResponseJSON>[2], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): string[] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const body = buildResponseJSON([...events], modelId, options); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const output = Array.isArray(body.output) ? body.output as Record<string, unknown>[] : []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return output.map(item => String(item.type ?? "")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+130
to
+139
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value Share one JSON body build between Lines 120-128 and Lines 131-139 repeat the same three steps: call ♻️ Proposed extraction+function jsonOutput(
+ events: readonly AdapterEvent[],
+ modelId: string,
+ options?: Parameters<typeof buildResponseJSON>[2],
+): Record<string, unknown>[] {
+ const body = buildResponseJSON([...events], modelId, options);
+ return Array.isArray(body.output) ? body.output as Record<string, unknown>[] : [];
+}
+
/** Tool items from the non-streaming transport. */
export function jsonToolItems(
events: readonly AdapterEvent[],
modelId: string,
options?: Parameters<typeof buildResponseJSON>[2],
): NormalizedToolItem[] {
- const body = buildResponseJSON([...events], modelId, options);
- const output = Array.isArray(body.output) ? body.output as Record<string, unknown>[] : [];
- return output.filter(isToolItem).map(normalizeItem);
+ return jsonOutput(events, modelId, options).filter(isToolItem).map(normalizeItem);
}
/** Every output item type from the non-streaming transport, including non-call items. */
export function jsonItemTypes(
events: readonly AdapterEvent[],
modelId: string,
options?: Parameters<typeof buildResponseJSON>[2],
): string[] {
- const body = buildResponseJSON([...events], modelId, options);
- const output = Array.isArray(body.output) ? body.output as Record<string, unknown>[] : [];
- return output.map(item => String(item.type ?? ""));
+ return jsonOutput(events, modelId, options).map(item => String(item.type ?? ""));
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Flush the
TextDecoderand support multi-linedata:frames incollectSse.Two gaps exist in the frame reader at Lines 27-45.
{ stream: true }, and the loop never calls a finaldecoder.decode(). If the last chunk ends inside a multi-byte UTF-8 sequence, the decoder retains those bytes and the harness drops the characters silently. The custom-tool case intests/responses-tool-conformance.test.ts(Lines 249-257) asserts non-ASCII fragments byte-exactly, so a silent truncation here would surface as a confusing parity failure instead of a decode bug.data:line of a frame. SSE allows severaldata:lines per event, which a consumer must join with\n. If the bridge ever emits a multi-line payload,JSON.parsereceives a truncated fragment and throws inside the harness.Both fixes are local to this function.
♻️ Proposed fix for decoder flush and multi-line data
for (;;) { const { done, value } = await reader.read(); if (done) break; text += decoder.decode(value, { stream: true }); } + text += decoder.decode(); return text.split("\n\n") .map(frame => frame.trim()) .filter(frame => frame.length > 0 && frame !== "data: [DONE]") .map(frame => { const lines = frame.split("\n"); const event = lines.find(line => line.startsWith("event: "))?.slice(7); - const dataLine = lines.find(line => line.startsWith("data: ")); - return { event, data: JSON.parse(dataLine?.slice(6) ?? "{}") as Record<string, unknown> }; + const dataLines = lines.filter(line => line.startsWith("data: ")).map(line => line.slice(6)); + const data = dataLines.length > 0 ? dataLines.join("\n") : "{}"; + return { event, data: JSON.parse(data) as Record<string, unknown> }; });📝 Committable suggestion
🤖 Prompt for AI Agents