Skip to content

Commit 2718187

Browse files
DiadkoShmeklforst
andauthored
fix: preserve UTF-8 in logged ReadableStream byte chunks (#2701)
Co-authored-by: lforst <8118419+lforst@users.noreply.github.com>
1 parent 9ce9308 commit 2718187

4 files changed

Lines changed: 126 additions & 6 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"braintrust": patch
3+
---
4+
5+
fix: preserve UTF-8 in logged ReadableStream byte chunks

‎js/src/functions/stream.test.ts‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect, test } from "vitest";
1+
import { expect, test, vi } from "vitest";
22
import {
33
BraintrustStream,
44
BraintrustStreamChunk,
@@ -103,3 +103,64 @@ test("final value passthrough with abort", async () => {
103103
"This operation was aborted",
104104
);
105105
});
106+
107+
async function collectFinalValue(
108+
chunks: (string | Uint8Array | BraintrustStreamChunk)[],
109+
) {
110+
const final = vi.fn();
111+
const error = vi.fn();
112+
const emitted: BraintrustStreamChunk[] = [];
113+
const input = new ReadableStream({
114+
start(controller) {
115+
for (const chunk of chunks) controller.enqueue(chunk);
116+
controller.close();
117+
},
118+
});
119+
await input
120+
.pipeThrough(createFinalValuePassThroughStream(final, error))
121+
.pipeTo(
122+
new WritableStream({
123+
write(chunk) {
124+
emitted.push(chunk);
125+
},
126+
}),
127+
);
128+
return { final, error, emitted };
129+
}
130+
131+
test.each(["é", "€", "🧠", "Пам’ять 🧠 漢字"])(
132+
"final value and forwarded text preserve every UTF-8 split: %s",
133+
async (text) => {
134+
const bytes = new TextEncoder().encode(text);
135+
for (let split = 1; split < bytes.length; split++) {
136+
const { final, error, emitted } = await collectFinalValue([
137+
bytes.subarray(0, split),
138+
new Uint8Array(),
139+
bytes.subarray(split),
140+
]);
141+
expect(final).toHaveBeenCalledExactlyOnceWith(text);
142+
expect(error).not.toHaveBeenCalled();
143+
expect(emitted.map((chunk) => chunk.data).join("")).toBe(text);
144+
}
145+
},
146+
);
147+
148+
test.each([
149+
{ chunks: [Uint8Array.of(0xf0, 0x9f)], expected: "�" },
150+
{ chunks: [Uint8Array.of(0xf0), Uint8Array.of(0x9f)], expected: "�" },
151+
{
152+
chunks: [
153+
Uint8Array.of(0xc3),
154+
{ type: "text_delta", data: "middle" } as const,
155+
Uint8Array.of(0xa9),
156+
],
157+
expected: "�middle�",
158+
},
159+
])(
160+
"final value replaces incomplete byte runs: $expected",
161+
async ({ chunks, expected }) => {
162+
const { final, emitted } = await collectFinalValue(chunks);
163+
expect(final).toHaveBeenCalledExactlyOnceWith(expected);
164+
expect(emitted.map((chunk) => chunk.data).join("")).toBe(expected);
165+
},
166+
);

‎js/src/functions/stream.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,20 +317,34 @@ export function createFinalValuePassThroughStream<
317317
const textChunks: string[] = [];
318318
const jsonChunks: string[] = [];
319319
const reasoningChunks: string[] = [];
320+
const flushDecoder = (
321+
controller: TransformStreamDefaultController<BraintrustStreamChunk>,
322+
) => {
323+
const tail = decoder.decode();
324+
if (tail) {
325+
textChunks.push(tail);
326+
controller.enqueue({ type: "text_delta", data: tail });
327+
}
328+
};
320329
const transformStream = new TransformStream<T, BraintrustStreamChunk>({
321330
transform(chunk, controller) {
331+
// A non-byte chunk ends the current byte run, so flush it in order.
332+
if (!(chunk instanceof Uint8Array)) flushDecoder(controller);
322333
if (typeof chunk === "string") {
323334
textChunks.push(chunk);
324335
controller.enqueue({
325336
type: "text_delta",
326337
data: chunk,
327338
});
328339
} else if (chunk instanceof Uint8Array) {
329-
textChunks.push(decoder.decode(chunk));
330-
controller.enqueue({
331-
type: "text_delta",
332-
data: decoder.decode(chunk),
333-
});
340+
const text = decoder.decode(chunk, { stream: true });
341+
textChunks.push(text);
342+
if (text) {
343+
controller.enqueue({
344+
type: "text_delta",
345+
data: text,
346+
});
347+
}
334348
} else if (braintrustStreamChunkSchema.safeParse(chunk).success) {
335349
const chunkType = chunk.type;
336350
switch (chunkType) {
@@ -361,6 +375,7 @@ export function createFinalValuePassThroughStream<
361375
}
362376
},
363377
flush(controller) {
378+
flushDecoder(controller);
364379
if (jsonChunks.length > 0) {
365380
// If we received both text and json deltas in the same stream, we
366381
// only return the json delta

‎js/src/logger.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,45 @@ test("verify MemoryBackgroundLogger intercepts logs", async () => {
456456
_exportsForTestingOnly.clearTestBackgroundLogger(); // can go back to normal
457457
});
458458

459+
test.each([
460+
["single chunk", false],
461+
["split bytes", true],
462+
])(
463+
"logs a Unicode byte stream without changing its text: %s",
464+
async (_, split) => {
465+
_exportsForTestingOnly.simulateLoginForTests();
466+
const memoryLogger = _exportsForTestingOnly.useTestBackgroundLogger();
467+
const logger = initLogger({
468+
projectName: "test",
469+
projectId: "test-project-id",
470+
});
471+
472+
const text = "Пам’ять 🧠 漢字";
473+
const bytes = new TextEncoder().encode(text);
474+
const output = new ReadableStream<Uint8Array>({
475+
start(controller) {
476+
if (split) {
477+
for (const byte of bytes) controller.enqueue(Uint8Array.of(byte));
478+
} else {
479+
controller.enqueue(bytes);
480+
}
481+
controller.close();
482+
},
483+
});
484+
const span = logger.startSpan({ name: "byte-stream" });
485+
span.log({ output });
486+
span.end();
487+
await memoryLogger.flush();
488+
489+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
490+
const events = (await memoryLogger.drain()) as any[];
491+
expect(events).toHaveLength(1);
492+
expect(events[0].output).toBe(text);
493+
494+
_exportsForTestingOnly.clearTestBackgroundLogger();
495+
},
496+
);
497+
459498
test("init validation", () => {
460499
expect(() => init({})).toThrow(
461500
"Must specify at least one of project or projectId",

0 commit comments

Comments
 (0)