Add Go M0 runtime provider contracts#50
Merged
Merged
Conversation
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
WalkthroughThis PR establishes the foundational streaming API for Zero Runtime: core types (messages, tool calls, usage), the Provider interface contract, stream collection helpers, and tests validating the complete flow. ChangesZero Runtime Streaming Infrastructure
🎯 3 (Moderate) | ⏱️ ~20 minutes Sequence DiagramsequenceDiagram
participant Client
participant Provider
participant EventsChannel
participant CollectStream
Client->>Provider: StreamCompletion(ctx, CompletionRequest)
Provider->>EventsChannel: send StreamEvent(s)
CollectStream->>EventsChannel: read StreamEvent(s)
CollectStream->>CollectStream: aggregate text, usage, tool-call fragments
CollectStream->>Client: return CollectedStream
Suggested Reviewers
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/zeroruntime/provider_test.go (1)
44-76: ⚡ Quick winAdd edge-case tests for non-happy stream termination.
Please add cases for: (1) channel close before
StreamEventDonewith an open tool call, (2) context cancellation with pending deltas, and (3)StreamEventErrorhandling once surfaced byCollectStream.🤖 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 `@internal/zeroruntime/provider_test.go` around lines 44 - 76, Add three new tests alongside TestCollectStreamAccumulatesTextToolCallsAndUsage that exercise CollectStream for non-happy paths: (1) "channel closed before StreamEventDone with an open tool call" — create events that send StreamEventToolCallStart and some ToolCallDelta fragments then close the events channel without sending StreamEventToolCallEnd/StreamEventDone and assert CollectStream returns (does not hang) and includes the accumulated tool call (ID ToolCallID "call_..." with concatenated Arguments) in collected.ToolCalls; (2) "context cancellation with pending deltas" — start a goroutine that emits a start and some ToolCallDelta fragments, cancel the context before sending ToolCallEnd, call CollectStream with that context and assert it returns promptly and either includes the partial tool call or records no incomplete state per current CollectStream semantics (assert whichever behavior is expected); (3) "StreamEventError is surfaced" — send a StreamEvent{Type: StreamEventError, /* set error details */} and ensure CollectStream surfaces/returns that error (or sets an error field) instead of hanging; use the same StreamEvent constants (StreamEventToolCallStart, StreamEventToolCallDelta, StreamEventToolCallEnd, StreamEventDone, StreamEventError) and CollectStream to locate the code under test.
🤖 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 `@internal/zeroruntime/helpers.go`:
- Around line 25-30: The loop exits on ctx.Done() and when events is closed
without flushing partially collected tool calls; ensure any pending calls in the
local collected slice are finalized on all exit paths (ctx.Done, !ok from
events, and other returns) similar to how StreamEventDone is handled. Update the
event-processing function (the loop that reads from events and uses collected
and StreamEventDone) to call the same flush/finalize logic for collected before
returning in those branches so no open tool calls are dropped.
- Around line 32-57: The switch currently ignores StreamEventError events; add
an explicit case for StreamEventError in the same switch that records the event
error into the collected result (e.g., set a collected.Error or
collected.ErrorMessage field — add that field to the Collected type if it
doesn't exist), ensure any open tool calls are appended by calling
appendOpenToolCalls(&collected, toolCallOrder, pendingToolCalls), and then
return collected (so the caller can distinguish a failed stream from partial
success); reference symbols: StreamEventError, collected, appendOpenToolCalls,
pendingToolCalls, toolCallOrder, and the event variable in the switch.
---
Nitpick comments:
In `@internal/zeroruntime/provider_test.go`:
- Around line 44-76: Add three new tests alongside
TestCollectStreamAccumulatesTextToolCallsAndUsage that exercise CollectStream
for non-happy paths: (1) "channel closed before StreamEventDone with an open
tool call" — create events that send StreamEventToolCallStart and some
ToolCallDelta fragments then close the events channel without sending
StreamEventToolCallEnd/StreamEventDone and assert CollectStream returns (does
not hang) and includes the accumulated tool call (ID ToolCallID "call_..." with
concatenated Arguments) in collected.ToolCalls; (2) "context cancellation with
pending deltas" — start a goroutine that emits a start and some ToolCallDelta
fragments, cancel the context before sending ToolCallEnd, call CollectStream
with that context and assert it returns promptly and either includes the partial
tool call or records no incomplete state per current CollectStream semantics
(assert whichever behavior is expected); (3) "StreamEventError is surfaced" —
send a StreamEvent{Type: StreamEventError, /* set error details */} and ensure
CollectStream surfaces/returns that error (or sets an error field) instead of
hanging; use the same StreamEvent constants (StreamEventToolCallStart,
StreamEventToolCallDelta, StreamEventToolCallEnd, StreamEventDone,
StreamEventError) and CollectStream to locate the code under test.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 92f354d2-0801-4d8c-883e-0cacc8a944a1
📒 Files selected for processing (3)
internal/zeroruntime/helpers.gointernal/zeroruntime/provider_test.gointernal/zeroruntime/types.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/zeroruntime/helpers.go (1)
52-54:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftTool-call ordering can violate start-order contract.
At Line 52-Line 54, completed calls are appended immediately, which yields end order when multiple calls finish out of order. That conflicts with the stated start-order contract.
Please defer final emission to a single ordered flush path (or maintain a unified ordered buffer for both open and closed calls).Proposed direction
case StreamEventToolCallEnd: - if toolCall, ok := pendingToolCalls[event.ToolCallID]; ok { - collected.ToolCalls = append(collected.ToolCalls, *toolCall) - delete(pendingToolCalls, event.ToolCallID) - } + // Mark as closed if needed, but do not append here. + // Emit once via ordered flush to preserve start order.Also applies to: 77-84
🤖 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 `@internal/zeroruntime/helpers.go` around lines 52 - 54, The code currently appends finished tool calls directly into collected.ToolCalls when found in pendingToolCalls (symbols: pendingToolCalls, collected.ToolCalls), which emits by end-order; instead, stop appending immediately — move the append into the unified flush path (the single ordered flush function) or add an intermediate closed-buffer (e.g., closedToolCalls slice/map) that stores completed calls keyed by their start-order index, remove from pendingToolCalls but do not emit; then have the existing flush/emit routine (the ordered flush you already use elsewhere) merge pending and closed buffers and append to collected.ToolCalls in start-order, ensuring start-order contract is preserved (apply same change for the other similar block that uses pendingToolCalls).
🧹 Nitpick comments (1)
internal/zeroruntime/provider_test.go (1)
78-140: ⚡ Quick winAdd a regression test for out-of-order tool-call completion.
Current tests validate flushing and error surfacing, but not start-order stability across interleaved calls. Add one case with
call_1thencall_2starts,call_2ends beforecall_1, and assert output order remains[call_1, call_2].🤖 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 `@internal/zeroruntime/provider_test.go` around lines 78 - 140, Add a regression test that verifies CollectStream preserves start order when tool calls complete out-of-order: create a new test (e.g. TestCollectStreamPreservesStartOrderWithInterleavedCompletion) that sends StreamEventToolCallStart for "call_1" then for "call_2", then emits deltas and closes/completes "call_2" before "call_1" (use StreamEventToolCallEnd events or close the channel as appropriate), call CollectStream(ctx, events) and assert collected.ToolCalls has length 2 and the order is first toolCall.ID == "call_1" then toolCall.ID == "call_2"; reference CollectStream, StreamEvent, StreamEventToolCallStart, StreamEventToolCallEnd, and the existing TestCollectStream* tests for structure and assertions.
🤖 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.
Outside diff comments:
In `@internal/zeroruntime/helpers.go`:
- Around line 52-54: The code currently appends finished tool calls directly
into collected.ToolCalls when found in pendingToolCalls (symbols:
pendingToolCalls, collected.ToolCalls), which emits by end-order; instead, stop
appending immediately — move the append into the unified flush path (the single
ordered flush function) or add an intermediate closed-buffer (e.g.,
closedToolCalls slice/map) that stores completed calls keyed by their
start-order index, remove from pendingToolCalls but do not emit; then have the
existing flush/emit routine (the ordered flush you already use elsewhere) merge
pending and closed buffers and append to collected.ToolCalls in start-order,
ensuring start-order contract is preserved (apply same change for the other
similar block that uses pendingToolCalls).
---
Nitpick comments:
In `@internal/zeroruntime/provider_test.go`:
- Around line 78-140: Add a regression test that verifies CollectStream
preserves start order when tool calls complete out-of-order: create a new test
(e.g. TestCollectStreamPreservesStartOrderWithInterleavedCompletion) that sends
StreamEventToolCallStart for "call_1" then for "call_2", then emits deltas and
closes/completes "call_2" before "call_1" (use StreamEventToolCallEnd events or
close the channel as appropriate), call CollectStream(ctx, events) and assert
collected.ToolCalls has length 2 and the order is first toolCall.ID == "call_1"
then toolCall.ID == "call_2"; reference CollectStream, StreamEvent,
StreamEventToolCallStart, StreamEventToolCallEnd, and the existing
TestCollectStream* tests for structure and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a5e31f0e-9968-46a5-89eb-acb59dce454e
📒 Files selected for processing (3)
internal/zeroruntime/helpers.gointernal/zeroruntime/provider_test.gointernal/zeroruntime/types.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/zeroruntime/types.go
Collaborator
|
Blockers Non-Blocking
Looks Good
|
Vasanthdev2004
approved these changes
Jun 4, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Approved current head 48954cc after reviewing the Go runtime contract skeleton, checking CI Go validation, and validating both PR branch and latest-main test-merge TS/Bun paths.
This was referenced Jun 4, 2026
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
2 participants
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.
Summary
Scope
This intentionally does not implement an OpenAI/Anthropic/Gemini provider, model registry, CLI behavior, or stream-json protocol. It is a narrow M0 contract skeleton for Gnanam to extend.
Testing
Local note
bun run teststill has 5 unrelated pre-existing local failures in headless-exec/MCP tests; Go contract tests pass and CI should verify the full matrix.Summary by CodeRabbit
New Features
Tests