Dashboard › sentry › Session 0z1DUb9vFIXf
0z1DUb9vFIXftDqiqYou are reviewing PR #611 on BYK/loreai: "fix(gateway): remove cache fields from ZERO_USAGE and consolidate SSE extraction"
This is a follow-up PR that fixes issues found during self-review of previously merged PRs #600 and #605.
Read and review these files critically:
packages/gateway/src/translate/types.ts — find:
ZERO_USAGE constant (should only have inputTokens and outputTokens, NOT cacheReadInputTokens or cacheCreationInputTokens)extractJSONFromSSE() function (moved here from pipeline.ts)GatewayUsage type definition (for context on optional vs required fields)packages/gateway/src/pipeline.ts — verify:
extractJSONFromSSE() function was removedextractJSONFromSSE is now imported from ./translate/typesaccumulateNonStreamResponse() still works correctly with the importpackages/gateway/src/llm-adapter.ts — verify:
extractJSONFromSSE import{} — which behavior does the consolidated version use?)packages/gateway/src/stream/openai.ts — verify ZERO_USAGE import and usage replacing inline { inputTokens: 0, outputTokens: 0 }
packages/gateway/src/stream/openai-responses.ts — same check
packages/gateway/src/stream/anthropic.ts — same check, verify both inline fallbacks replaced
packages/gateway/test/translate-types.test.ts — review the new test:
For each file, check for:
Also review the PR description at https://github.com/BYK/loreai/pull/611 for accuracy, completeness, and whether a reviewer ...
I'll start by gathering all the necessary context: the PR description and all the files mentioned.
fix(gateway): remove cache fields from ZERO\_USAGE and consolidate SSE extraction by BYK · Pull Request #611 · BYK/loreai · GitHub
[Skip to content](#start-of-content)
## Navigation Menu
Toggle navigation
[](/)
[Sign in](/login?return_to=https%3A%2F%2Fgithub.com%2FBYK%2Floreai%2Fpull%2F611)
Appearance settings
- Platform
- AI CODE CREATION
- [
GitHub CopilotWrite better code with AI
](https://github.com/features/copilot)
- [
GitHub Copilot appDirect agents from issue to merge
](https://github.com/features/ai/github-app)
- [
MCP RegistryNewIntegrate external tools
](https://github.com/mcp)
- DEVELOPER WORKFLOWS
- [
ActionsAutomate any ...
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/translate/types.ts</path> <type>file</type> <content> 1: /** 2: * Internal representation types for the Lore gateway. 3: * 4: * The gateway accepts both Anthropic (`/v1/messages`) and OpenAI 5: * (`/v1/chat/completions`) protocol requests, normalizes them into these 6: * types for Lore pipeline processing, then translates back to the original 7: * protocol for the upstream response. 8: * 9: * Design: types are intentionally minimal — only fields that Lore's context 10: * management (gradient, LTM, distillation) actually reads/writes. Protocol- 11: * specific fields the gateway doesn't process live in `metadata`. ...
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/pipeline.ts</path> <type>file</type> <content> 1: /** 2: * Core request processing pipeline for the Lore gateway. 3: * 4: * Orchestrates the full flow for every request: 5: * session identification → LTM injection → gradient transform → 6: * upstream forwarding → response accumulation → calibration → 7: * temporal storage → background work scheduling. 8: * 9: * Three request classes are handled: 10: * 1. Compaction requests → intercepted, never forwarded upstream. 11: * 2. Meta requests (title gen, summaries, etc.) → forwarded transparently, no Lore processing. 12: * 3. Normal conversation turns → full pipeline. ...
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/llm-adapter.ts</path> <type>file</type> <content> 1: /** 2: * Gateway LLM adapter: implements LLMClient via direct API calls. 3: * Used by Lore's background workers (distillation, curation, query expansion) 4: * running inside the gateway process. 5: * 6: * Supports both Anthropic Messages API and OpenAI Chat Completions API. 7: * The wire protocol is determined by explicit protocol from the session's 8: * UpstreamSnapshot (threaded via opts.protocol), with fallback to the 9: * provider route registry (PROVIDER_ROUTES) and a safe default of 10: * "anthropic" for unknown/aggregator providers: 11: * - Anthropic protocol → POST /v1/messages 12: * - OpenAI protocol → POST /v1/chat/completions 13: * 14: * Protocol is decoupled from provider identity — proxy/aggregator 15: * providers (e.g. ...
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/stream/openai.ts</path> <type>file</type> <content> 1: /** 2: * Anthropic SSE → OpenAI Chat Completions SSE streaming translator. 3: * 4: * Reads Anthropic-format SSE events from an upstream Response and emits 5: * OpenAI Chat Completions streaming chunks incrementally, so the client 6: * receives tokens as they arrive rather than waiting for the full response. 7: * 8: * Anthropic lifecycle: 9: * message_start → content_block_start → content_block_delta (repeated) 10: * → content_block_stop → message_delta → message_stop 11: * 12: * OpenAI Chat Completions streaming lifecycle: 13: * chunk with delta.role → chunk with delta.content (repeated) 14: * → chunk with finish_reason → data: [DONE] 15: * 16: * Uses `parseSSEStream` from the Anthropic stream module to parse upstream 17: * events, and `createStreamAccumulator` to build the internal ...
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/stream/openai-responses.ts</path>
<type>file</type>
<content>
1: /**
2: * OpenAI Responses API SSE stream accumulator.
3: *
4: * Parses upstream Responses API streaming events and accumulates the full
5: * response into a `GatewayResponse`. The Responses API uses a different
6: * SSE event lifecycle than Anthropic:
7: *
8: * response.created → response.in_progress →
9: * response.output_item.added → response.output_text.delta (repeated) →
10: * response.output_item.done → response.function_call_arguments.delta →
11: * response.function_call_arguments.done →
12: * response.completed
13: *
14: * Reuses `parseSSEStream` from the Anthropic stream module since the
15: * underlying SSE wire format is the same.
16: */
17: import {
18: ZERO_USAGE,
19: type GatewayContentBlock,
20: type GatewayResponse,
21: type GatewayUsage,
22: } from ...
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/stream/anthropic.ts</path>
<type>file</type>
<content>
1: /**
2: * Anthropic SSE stream handling.
3: *
4: * Parses upstream Anthropic streaming responses (named SSE events), accumulates
5: * the full response into a `GatewayResponse`, and provides helpers for
6: * generating synthetic SSE event sequences (e.g. for compaction interception).
7: *
8: * Anthropic uses named SSE events with a lifecycle:
9: * message_start -> content_block_start/delta/stop (repeated) -> message_delta -> message_stop
10: *
11: * All functions are pure (no side effects) except `parseSSEStream` which is
12: * an async generator consuming a byte stream.
13: */
14: import {
15: ZERO_USAGE,
16: type GatewayContentBlock,
17: type GatewayResponse,
18: type GatewayUsage,
19: } from "../translate/types";
20: import { scaleUsageForClient } from "../compaction";
21:
22: // ...
Now let me read the test file:
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/test/translate-types.test.ts
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/test/translate-types.test.ts</path>
<type>file</type>
<content>
1: import { describe, it, expect } from "vitest";
2: import { ZERO_USAGE } from "../src/translate/types";
3:
4: describe("ZERO_USAGE", () => {
5: it("must not include optional cache fields", () => {
6: // INVARIANT: ZERO_USAGE must only contain required fields (inputTokens,
7: // outputTokens). The optional cache fields (cacheReadInputTokens,
8: // cacheCreationInputTokens) must NOT be present — their mere presence
9: // (even as 0) causes downstream `!= null` guards to emit cache fields
10: // in the wire response, leaking "cache_read_input_tokens: 0" to clients
11: // when no caching actually occurred.
12: expect(Object.keys(ZERO_USAGE)).toEqual(["inputTokens", "outputTokens"]);
13: expect(ZERO_USAGE).not.toHaveProperty("cacheReadInputTokens");
14: ...
Now let me check where accumulateNonStreamResponse is defined in pipeline.ts and verify it uses the imported extractJSONFromSSE:
Found 13 matches
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/translate/types.ts:
Line 250: export async function extractJSONFromSSE(
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/llm-adapter.ts:
Line 37: import { extractJSONFromSSE } from "./translate/types";
Line 530: ? await extractJSONFromSSE(response)
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/pipeline.ts:
Line 75: extractJSONFromSSE,
Line 2207: async function accumulateNonStreamResponse(
Line 2218: json = await extractJSONFromSSE(upstreamResponse);
Line 3666: const resp = await accumulateNonStreamResponse(
Line 3674: const resp = await accumulateNonStreamResponse(
Line 4654: // continuation is consumed (accumulateNonStreamResponse). ...
Let me read the accumulateNonStreamResponse function in pipeline.ts to verify it uses the imported function correctly:
<path>/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/pipeline.ts</path>
<type>file</type>
<content>
2200: * Accumulate a non-streaming upstream response into a GatewayResponse.
2201: *
2202: * Dispatches to the correct parser based on the upstream wire protocol:
2203: * - "anthropic": Anthropic Messages API format
2204: * - "openai": OpenAI Chat Completions API format
2205: * - "openai-responses": OpenAI Responses API format
2206: */
2207: async function accumulateNonStreamResponse(
2208: upstreamResponse: Response,
2209: protocol: "anthropic" | "openai" | "openai-responses" = "anthropic",
2210: ): Promise<GatewayResponse> {
2211: // Some providers (e.g. DeepSeek) return SSE-formatted responses even when
2212: // stream: false was sent. Detect this via content-type and extract the JSON
2213: // payload from the SSE data lines instead of calling response.json() which
2214: // would throw a ...
Now let me check the old inline SSE extraction that was in llm-adapter.ts to understand the error handling difference. Let me also check if there are any other references to ZERO_USAGE to make sure I haven't missed any inline fallbacks:
No files found
Good — no remaining inline { inputTokens: 0, outputTokens: 0 } patterns. Let me also check usage of ZERO_USAGE across the codebase for completeness, and verify the != null guards the PR description mentions:
Found 29 matches
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/test/translate-types.test.ts:
Line 2: import { ZERO_USAGE } from "../src/translate/types";
Line 4: describe("ZERO_USAGE", () => {
Line 6: // INVARIANT: ZERO_USAGE must only contain required fields (inputTokens,
Line 12: expect(Object.keys(ZERO_USAGE)).toEqual(["inputTokens", "outputTokens"]);
Line 13: expect(ZERO_USAGE).not.toHaveProperty("cacheReadInputTokens");
Line 14: expect(ZERO_USAGE).not.toHaveProperty("cacheCreationInputTokens");
Line 18: expect(ZERO_USAGE.inputTokens).toBe(0);
Line 19: expect(ZERO_USAGE.outputTokens).toBe(0);
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/translate/types.ts:
Line 231: export const ZERO_USAGE: GatewayUsage = ...
Found 9 matches
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/stream/anthropic.ts:
Line 476: ...(u.cacheReadInputTokens != null
Line 479: ...(u.cacheCreationInputTokens != null
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/stream/openai-responses.ts:
Line 662: if (ru.cacheReadInputTokens != null) {
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/stream/openai.ts:
Line 289: if (ru.cacheReadInputTokens != null) {
/home/byk/.local/share/opencode/worktree/51d1015fd1f616acf9a5ffaad9331ec6b304586b/clever-circuit/packages/gateway/src/translate/openai.ts:
Line 353: ...(usage.cacheReadInputTokens != ...
Now let me also verify ZERO_USAGE is not mutated anywhere (since it's a const object but its properties could still be mutated), and check if there's an Object.freeze:
No files found
Good — ZERO_USAGE is never directly property-accessed (it's always spread or used via ??), so no mutation risk there. Let me also check how the extractJSONFromSSE function's test coverage looks — are there tests for it?
No files found
Now I have all the context I need for a thorough review. Let me compile the findings.
fix(gateway): remove cache fields from ZERO_USAGE and consolidate SSE extractionNone found. The PR is correct in its core claims and the changes are safe.
M1. ZERO_USAGE is not frozen — mutation risk from spread-then-assign patterns
ZERO_USAGE is declared as export const ZERO_USAGE: GatewayUsage = { inputTokens: 0, outputTokens: 0 } but is not Object.freeze()-d. While no current code mutates it directly (all call sites use resp.usage ?? ZERO_USAGE and then read properties), the object is shared mutable state. A future change like:
const usage = resp.usage ?? ZERO_USAGE;
usage.cacheReadInputTokens = someValue; // silently corrupts ZERO_USAGE
...would silently corrupt the sentinel for all subsequent requests. Since the entire PR is about preventing accidental cache-field presence on ZERO_USAGE, adding Object.freeze() would make the invariant enforced at runtime, not just by a test.
Recommendation: export const ZERO_USAGE: GatewayUsage = Object.freeze({ inputTokens: 0, outputTokens: 0 });
This is safe — all existing != null property reads work on frozen objects, and the type already has cache fields as optional.
M2. extractJSONFromSSE error handling difference between old call sites is now uniform — but the throwing behavior is the correct default
The PR description correctly notes the consolidation: pipeline.ts threw on missing payload, llm-adapter.ts fell back to {}. The consolidated version throws. This is the right choice — a silent {} fallback in llm-adapter.ts would cause a downstream null return with no diagnostics (the parseAnthropicResponse/parseOpenAIResponse would return { text: null, usage: null } → return null). The throw is caught by the outer try/catch in the retry loop (line 735: `catch (e) { log.error("worker ...