feat: add Zero Anthropic provider#15
Conversation
anandh8x
left a comment
anandh8x
left a comment
There was a problem hiding this comment.
Code Review: PR #15 — feat: add Zero Anthropic provider
Author: @gnanam1990 → feat/zero-anthropic-provider → main
Scope: 5 files, +723/-3. New src/providers/anthropic.ts (444 lines) with SSE streaming, message/tool mapping, and HTTP error classification. Wires Anthropic into createZeroProvider. Updates resolver/CLI/tests.
PRD alignment: exactly Gnanam's M1 feat/m1-anthropic-provider slice (PRD line 122: "Anthropic provider with streaming text, tool calls, system prompt, usage"). Owner is correct. Cross-owner contract change to createZeroProvider is small and additive. No scope creep.
Verdict: Request changes. Two real correctness issues that will hurt callers today, plus a few follow-ups. Nothing structural — the provider is well-organized and the tests are good. After the two issues below are addressed this is a clean M1 slice.
Important
1. max_tokens: 4096 hardcode will silently truncate Claude responses
src/providers/anthropic.ts:5 defines const DEFAULT_MAX_TOKENS = 4096; and src/zero-provider-runtime/resolver.ts:98-102 doesn't pass anything else:
return new AnthropicProvider({
apiKey: runtime.apiKey,
baseURL: runtime.baseURL,
model: runtime.apiModel,
});The model registry has:
claude-sonnet-4.5:maxOutputTokens: 64_000claude-opus-4.1/opus-4:maxOutputTokens: 32_000claude-haiku-4.5/haiku-3.5:maxOutputTokens: 8_192
A 4k cap will silently truncate Sonnet 4.5 / Opus 4.x mid-response. The visible symptom is "Claude stopped halfway through a refactor" with no error. This is the most common real-world Claude use case for an M1 coding agent.
Ask: In the resolver's anthropic branch, pass maxTokens: runtime.model?.context.maxOutputTokens ?? DEFAULT_MAX_TOKENS (with a sane floor like 8192 to match Haiku). For unknown models the 4k default is acceptable, but every registry-known Claude model should use its own ceiling.
2. parseToolArguments fallback will 400 against the Anthropic API
src/providers/anthropic.ts:406-417:
function parseToolArguments(argumentsJson: string): Record<string, unknown> {
if (!argumentsJson) return {};
try {
const parsed = JSON.parse(argumentsJson);
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
return parsed as Record<string, unknown>;
}
return { input: parsed };
} catch {
return { input: argumentsJson };
}
}The two non-object paths ({ input: parsed } and { input: argumentsJson }) send a string in the input field. Anthropic's tool_use.input must be a JSON object — a string will get a 400 from the API. The fallback only delays the failure with a less clear error.
Ask: Pick one of:
- (a) Throw with a clear message ("tool arguments for X were not a JSON object")
- (b) Return
{}and let the tool fail downstream with a Zod "missing field" error (still informative)
Option (a) surfaces the bug at the wire boundary; option (b) is consistent with "let the tool complain." I lean (a) since the model emitted a string where an object was expected — that's a contract violation worth surfacing.
Suggestions
3. buffer.replace(/\r\n/g, '\n') runs on every chunk, on the entire accumulator
src/providers/anthropic.ts:281:
buffer = buffer.replace(/\r\n/g, '\n');This is O(n) on the accumulator per chunk. For a long stream the buffer grows to several KB and the work compounds. Move the \r\n -> \n normalization into parseAnthropicSSEPayload (per-event, on the small rawEvent slice). Anthropic's protocol is LF anyway, so the cross-newline normalization isn't needed in the happy path; it's defensive against non-conforming servers.
4. CLI's "not implemented" hint duplicates the resolver's error
src/cli/index.ts:24-29 still does:
if (runtime?.provider === 'google') {
console.error(`[zero] ${runtime.provider} adapter is not yet implemented. ...`);
}The resolver at src/zero-provider-runtime/resolver.ts:105-108 already throws a similar message. A user with a google model now sees two back-to-back errors. Now that Anthropic is implemented, this hardcoded google-only check is also fragile — a third "pending" provider would slip past.
Ask: Have createZeroProvider distinguish "not implemented" from "configuration error" with a typed error (e.g., a ZeroPendingProviderError), and have the CLI hint on that type rather than a hardcoded provider name. Or just add runtime.pending: boolean to the resolved runtime and let the CLI check that.
5. No AbortController / signal support
AnthropicProvider.createStream and fetchImpl don't accept a signal. A user who hits Ctrl+C in the headless CLI can't cancel an in-flight Anthropic stream — the request keeps consuming tokens until the server finishes. Add signal?: AbortSignal to AnthropicProviderOptions, thread it to this.fetchImpl, and have the CLI pass controller.signal from the process's SIGINT handler. Not M1-blocking but a foundation piece for a long-running agent.
6. Test coverage gaps
- No test for multiple tool calls in the same response (the
toolBlocksmap andindexreuse path). - No test for the Anthropic
errorSSE event (src/providers/anthropic.ts:248-250). - No test for 429 / 529 rate limit classification (
src/providers/anthropic.ts:137-139). - No test for the
Zero Anthropic provider requires at least one non-system messageguard atsrc/providers/anthropic.ts:97. - No test for
toolmessage withouttoolCallId(thethrowatsrc/providers/anthropic.ts:336).
Each is small; together they cover the realistic error paths.
7. body.max_tokens not validated client-side
src/providers/anthropic.ts:102 sets max_tokens: this.maxTokens with no guard. If a caller passes maxTokens: 0, the request goes out and gets a 400. A Math.max(1, this.maxTokens) or explicit if (this.maxTokens < 1) throw is one line.
8. No retry on transient 5xx
Anthropic's load balancers occasionally return 502/503. A single retry with backoff would smooth these over. Optional for M1 — execa patterns elsewhere in the codebase don't retry either, so this is consistent.
Cross-owner notes
- The
createZeroProviderchange is additive and thesrc/cli/index.ts:24one-line update is correct (Anthropic moves out of "pending"). No contract note needed for Vasanth beyond the existingZeroResolvedProviderRuntimeshape, which is unchanged. - The
AnthropicProvideruses onlyfetch(global),ReadableStream,TextDecoder— all available in Bun and Node 18+. Windows-compatible.
Validation noted from PR body
npx --yes bun run typecheck, bun test ./tests --timeout 15000 (100 pass / 0 fail), bun run build, bun run smoke:build, ./zero --help, ./zero providers current, git diff --check origin/main..HEAD — all green per the author.
Recommendation
Request changes. The two important items are correctness issues that will hit real Claude users today (truncation on long responses, tool-arg 400s). After fixing #1 and #2 — both small — this is ready to merge.
| throw new Error('Zero anthropic provider requires an API key'); | ||
| } | ||
|
|
||
| return new AnthropicProvider({ |
There was a problem hiding this comment.
Important — maxTokens is not threaded from the registry.
return new AnthropicProvider({
apiKey: runtime.apiKey,
baseURL: runtime.baseURL,
model: runtime.apiModel,
});The registry has claude-sonnet-4.5: maxOutputTokens: 64_000, claude-opus-4.1: maxOutputTokens: 32_000, claude-haiku-4.5: maxOutputTokens: 8_192. The provider's default is 4096, so every Claude registry model is silently truncated to 4k output unless the caller passes maxTokens explicitly — which the resolver never does.
Visible symptom: a claude-sonnet-4.5 user asks for a refactor of a 500-line file and gets a 2-3k-token response that stops mid-function with no error.
Ask: Pass maxTokens: runtime.model?.context.maxOutputTokens here (with a sane floor of 8192). For unknown models the 4k default is acceptable; for every registry-known Anthropic model the runtime should use its own ceiling.
|
|
||
| const DEFAULT_ANTHROPIC_BASE_URL = 'https://api.anthropic.com'; | ||
| const DEFAULT_ANTHROPIC_VERSION = '2023-06-01'; | ||
| const DEFAULT_MAX_TOKENS = 4096; |
There was a problem hiding this comment.
Important — 4096 default is well below every Anthropic model's maxOutputTokens in the registry.
This default is only correct for unknown models. Every Claude registry model has a much higher ceiling (Haiku 4.5: 8_192, Sonnet 4.5: 64_000, Opus 4.1: 32_000). The resolver's anthropic branch doesn't pass anything else (see src/zero-provider-runtime/resolver.ts:98-102), so this 4096 is what every Claude call uses.
Ask: The default here is fine for runtime.model === undefined (unknown model). For registry models the resolver should pass maxTokens: runtime.model.context.maxOutputTokens. This constant can stay as a fallback floor.
| })); | ||
| } | ||
|
|
||
| function parseToolArguments(argumentsJson: string): Record<string, unknown> { |
There was a problem hiding this comment.
Important — the two non-object fallback paths will 400 against Anthropic's API.
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
return parsed as Record<string, unknown>;
}
return { input: parsed };
...
catch {
return { input: argumentsJson };
}{ input: parsed } (string inside an object) and { input: argumentsJson } (raw string) both send a string in Anthropic's tool_use.input, which the API requires to be a JSON object. The request will be rejected with a 400, and the user sees "Provider returned error" rather than a clear "model emitted non-object tool arguments."
Ask: Pick one of:
- (a) Throw:
throw new Error(tool arguments for ${name} were not a JSON object: ${argumentsJson}). Surfaces the contract violation at the wire boundary. - (b) Return
{}and let the tool's Zod schema fail downstream with "missing field" errors. Consistent with the rest of the slice's error model.
I lean (a) since "model emitted a string where an object was expected" is a contract violation worth surfacing — not a recoverable case for the agent.
| while (true) { | ||
| const { value, done } = await reader.read(); | ||
| buffer += decoder.decode(value, { stream: !done }); | ||
| buffer = buffer.replace(/\r\n/g, '\n'); |
There was a problem hiding this comment.
Suggestion — buffer.replace(/\r\n/g, '\n') runs on every chunk, on the entire accumulator.
For a long stream the buffer grows to several KB and the work compounds (O(n) per chunk, O(n²) over the stream). Anthropic's protocol is LF anyway, so this normalization isn't needed in the happy path. Move it into parseAnthropicSSEPayload (per-event, on the small rawEvent slice) so it only runs on the bytes that actually need parsing.
| } catch (err: any) { | ||
| console.error(`[zero] ${err?.message ?? String(err)}`); | ||
| if (runtime?.provider === 'anthropic' || runtime?.provider === 'google') { | ||
| if (runtime?.provider === 'google') { |
There was a problem hiding this comment.
Suggestion — the not-implemented hint is now a hardcoded google check that will miss future pending providers.
if (runtime?.provider === 'google') {
console.error(`[zero] ${runtime.provider} adapter is not yet implemented. ...`);
}When Anthropic moved to "implemented," the resolver's generic message (Zero ${runtime.provider} provider adapter is not implemented yet. The provider resolver can identify the model, but the streaming adapter lands in a later M1 slice.) still fires for google. The user sees both messages back-to-back. If a fourth provider (e.g., mistral) lands as pending, this hardcoded check silently misses it.
Ask: Have createZeroProvider throw a typed error (e.g., ZeroPendingProviderError) and let the CLI check err instanceof ZeroPendingProviderError rather than hardcoding a provider name. Or add runtime.pending: boolean to the resolved runtime and have the CLI check that. Either way, this hint should not be a string-equality on a provider name.
95a9e7d to
b5938a2
Compare
|
@anandh8x @Vasanthdev2004 updated PR #15 for re-review. Fixed the two requested-change blockers:
Also addressed low-risk review suggestions while in the same module:
Validation after rebasing on latest
|
BlockersNone found. Non-Blocking
Looks Good
Verdict: Approve — the Anthropic adapter is now dispatch-safe enough for this M1 provider slice. |
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Approved — prior Anthropic correctness blockers are fixed, runtime wiring is safe, and local validation passed.
anandh8x
left a comment
anandh8x
left a comment
There was a problem hiding this comment.
Code Review: PR #15 v2 — feat: add Zero Anthropic provider (re-review, head b5938a20)
Author: @gnanam1990 → feat/zero-anthropic-provider → main
Scope: 6 files, +909/-10. Same 5 files as v1 plus new src/zero-provider-runtime/index.ts barrel.
v1 items disposition
| v1 Issue | Status |
|---|---|
max_tokens: 4096 hardcode silently truncates Claude models |
FIXED — resolveAnthropicMaxTokens reads registry context.maxOutputTokens, floors at 8192, returns 4096 default only for unknown models |
parseToolArguments fallback sends string in tool_use.input → API 400 |
FIXED — throws on non-object tool args (chose option (a) from initial review) |
buffer.replace(/\r\n/g, '\n') per-chunk on the accumulator |
FIXED — findSSEBoundary checks both CRLF and LF natively; \r\n normalization moved to per-event parseAnthropicSSEPayload slice |
CLI hardcoded google not-implemented hint duplicates resolver error |
FIXED — ZeroPendingProviderError typed class thrown by resolver; CLI checks err instanceof ZeroPendingProviderError with generic hint. Also fixed latent v1 bug where CLI printed "anthropic adapter not implemented" for an actually-implemented provider. |
| No AbortController/signal support | NOT FIXED (non-blocking for M1 per initial review — type accepts AbortSignal via Parameters<typeof fetch> but never threaded) |
Missing test coverage (multi-tool, error SSE, 429, empty-messages guard, missing toolCallId) |
ALL 5 FIXED — new tests added. Also added: normalizeMaxTokens validation, non-JSON-object tool arguments rejection |
max_tokens not validated client-side |
FIXED — normalizeMaxTokens validates isFinite, isInteger, >= 1. Tested. |
| No retry on 5xx | NOT FIXED (optional, consistent with codebase) |
Key improvements in v2
ZeroPendingProviderError— typed error carryingproviderstring. Cleaninstanceofcheck in CLI instead of fragile string matching.resolveAnthropicMaxTokens—Math.max(runtime.model.context.maxOutputTokens, MIN_REGISTRY_ANTHROPIC_MAX_TOKENS)whereMIN_REGISTRY_ANTHROPIC_MAX_TOKENS = 8192(matches Haiku 4.5's ceiling). Correctly handles both large (Sonnet 64k, Opus 32k) and small (Haiku 8k) models.normalizeMaxTokens— validates before construction. Catches configuration errors early.findSSEBoundary/parseAnthropicSSEPayload— clean separation of SSE frame parsing from event processing.getDetailedErrorMessage/getResponseErrorMessage— consistent, descriptive error messages from fetch failures, API errors, and stream errors. Recursion guard prevents "Provider returned error: Provider returned error: ..." nesting.appendUserBlocks— merges consecutive user content blocks (Anthropic requires strictly alternatinguser/assistantroles).- Test coverage — 11 tests (was 4 in v1). Helpers
createFetch,streamResponse,event,collectEventskeep tests clean.
Remaining follow-ups (non-blocking, deferred per M1 scope)
AbortController/AbortSignalfor user-cancellable streams —FetchLiketype now acceptssignalviaParameters<typeof fetch>, butcreateStreamnever passes one. A Ctrl+C during a long Claude response cannot cancel the in-flight request. Worth adding for M2 agent-loop cancellation.ZERO_ANTHROPIC_VERSIONenv var —DEFAULT_ANTHROPIC_VERSION = '2023-06-01'hardcoded. If Anthropic ever ships a new version (unlikely but possible), users must modify source. Config-driven would be ideal.betaheader not tested —anthropic-betaoption exists but no test verifies it's sent. Low risk since the header is additive.- No happy-path test verifying
toolBlockscleanup — aftermessage_stop, remaining tool blocks are yielded astool-call-end. Themessage_stopfor...ofloop handles this, but no test explicitly asserts orphaned tool blocks get cleaned up on stream end.
Cross-owner / merge notes
src/zero-provider-runtime/index.tsis new (barrel re-export).src/cli/index.tsnow importsZeroPendingProviderErrorfrom the barrel. No conflict with any open PR.- PR #16 (
feat/m2-update-check, anandh8x) also touchessrc/index.ts(addsupdatecommand). No overlap — #15 touchessrc/cli/index.ts(error handling inrunHeadless), #16 touchessrc/index.ts(main program). Merge-safe. src/zero-provider-runtime/resolver.tschanges are additive (new anthropic branch, new helpers, new error class). No API contract changes toZeroResolvedProviderRuntimeorcreateZeroProvider's public signature.
What's not covered (non-blocking for M1)
betaheader transmission (forward-looking option; Anthropic's prompt caching beta)- Retry on 529 (overloaded) — no other Zero provider retries either
stream: falsemode (M1 is streaming-only)- Multi-image messages (Anthropic's
content: [{ type: 'image', source: ... }]) — not needed for M1 agent - Per-request
max_tokensoverride from the caller (always uses the provider-level setting)
Validation noted from PR body
bun test --timeout 15000 (pass), bun run typecheck (pass), bun run build (pass) — all green per author.
Recommendation: Approve
Both real correctness issues from v1 are properly fixed. The ZeroPendingProviderError refactor is a nice design upgrade. Test coverage went from 4 to 11 cases, covering every error path discussed in v1. Clean M1 slice — ready to merge.
|
|
||
| const DEFAULT_ANTHROPIC_BASE_URL = 'https://api.anthropic.com'; | ||
| const DEFAULT_ANTHROPIC_VERSION = '2023-06-01'; | ||
| const DEFAULT_MAX_TOKENS = 4096; |
There was a problem hiding this comment.
Follow-up (not blocking): DEFAULT_ANTHROPIC_VERSION = '2023-06-01' is hardcoded. If Anthropic ships a new Messages API version, users must modify source. Consider making this configurable via env var / options. Same pattern applies to DEFAULT_ANTHROPIC_BASE_URL.
| @@ -168,6 +191,14 @@ function ensureOpenAICompatibleGatewayBaseURL(baseURL: string | undefined): asse | |||
| } | |||
There was a problem hiding this comment.
Clean fix. resolveAnthropicMaxTokens reads runtime.model.context.maxOutputTokens, floors at MIN_REGISTRY_ANTHROPIC_MAX_TOKENS = 8192 (matching Haiku 4.5's ceiling), and returns undefined for unknown models (falls through to constructor's 4096 default). Math.max ensures every registry model gets its own ceiling: 64k for Sonnet 4.5, 32k for Opus 4.x/4.1, 8k for Haiku. This is the exact fix requested in v1.
| } | ||
| } | ||
|
|
||
| function normalizeMaxTokens(maxTokens: number): number { |
There was a problem hiding this comment.
Good addition: normalizeMaxTokens validates isFinite, isInteger, >= 1. Catches maxTokens: 0 or NaN before they reach the API. Tested at tests/anthropic-provider.test.ts:352-358.
| })); | ||
| } | ||
|
|
||
| function parseToolArguments(argumentsJson: string, toolName: string): Record<string, unknown> { |
There was a problem hiding this comment.
Clean fix — chose option (a) from v1 review: throw at the wire boundary. Two distinct error messages for "parsed but not an object" vs "unparseable JSON." The catch's includes('JSON object') guard preserves the first error through the re-throw chain. Tested at tests/anthropic-provider.test.ts:332-350.
| console.error( | ||
| `[zero] ${runtime.provider} adapter is not yet implemented. ` + | ||
| 'Set provider: "openai-compatible" with a custom gateway or use an OpenAI model.' | ||
| '[zero] Use an implemented provider, or set provider: "openai-compatible" with a custom gateway.' |
There was a problem hiding this comment.
Clean refactor. err instanceof ZeroPendingProviderError replaces the fragile runtime?.provider === 'anthropic' || runtime?.provider === 'google' string check. Also fixed a subtle bug from v1 where the CLI printed "anthropic adapter is not yet implemented" for an actually-implemented provider.
| yield* this.readStream(response); | ||
| } | ||
|
|
||
| private async createStream(body: Record<string, unknown>): Promise<Response> { |
There was a problem hiding this comment.
Follow-up (not blocking): createStream never passes an AbortSignal. For M2 agent-loop cancellation (Ctrl+C during a long Claude response), thread a signal option from streamCompletion → createStream → fetchImpl. The FetchLike type already accepts it via Parameters<typeof fetch>.
| } | ||
| } | ||
|
|
||
| async function* readAnthropicSSE( |
There was a problem hiding this comment.
Nice refactor from v1. findSSEBoundary handles both \n\n and \r\n\r\n natively at the buffer level. The \r\n → \n normalization is now inside parseAnthropicSSEPayload (line 310) — per-event slice, not per-chunk accumulator. Eliminates the O(n²) concern from v1's buffer.replace().
3 participants
Summary
Adds the Zero Anthropic provider module for the M1 provider-runtime slice.
src/providers/anthropic.ts, a Zero-native TypeScript adapter for Anthropic Messages streaming.systemuser/assistantmessage turnstool_useblockstool_resultblocksinput_schemaStreamEvents:text_delta->texttool_useblock start ->tool-call-startinput_json_delta->tool-call-deltacontent_block_stop->tool-call-endmessage_start/message_delta/message_stop-> usage + done handlingcreateZeroProviderso Anthropic registry models now instantiate the Anthropic adapter with API-key validation.Drac Reference Map
Used the Drac reverse-engineered source as behavioral reference, then rewrote the module cleanly in TypeScript for Zero:
by-function/llm-api/claude-api-50.jsmessage_start,content_block_start,content_block_delta,content_block_stop,message_delta, andmessage_stop.by-function/llm-api/message-handler.jsapp/llm/anthropic/OYA.jsValidation
npx --yes bun run typechecknpx --yes bun test ./tests --timeout 15000npx --yes bun run buildnpx --yes bun run smoke:build./zero --help./zero providers currentgit diff --check origin/main..HEADReviewers
@Vasanthdev2004 @anandh8x please review this provider module.