fix(engine): cache response codecs and bound tool execution - #326
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Effect Agent review
Tip
No actionable findings.
| Scope | Files | New findings |
|---|---|---|
| Full diff | 11 reviewed | ✅ None |
Summary
No concrete defects found in the supplied change.
3 model calls · 61,918 input (675 uncached · 11,214 cached · 50,029 cache write; 18.1% cache reads) / 1,873 output tokens · ≈ $0.2948 at GPT-5.6 Sol rates · $0.999999 spending ceiling · inspected at c49e015 · 4 automatic reviews remain
Contributor
Bundle size
Minified ESM for es2022, browser target, including Effect and other dependencies. Gzip is measured per chunk. Initial includes statically imported shared chunks; deferred is the remaining output. New exports have no prior baseline. Chunks, module analysis, and exact bytes for |
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
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.
Reuse each live Toolkit's response codec across streamed chunks, bound tool-batch fibers by the resolved concurrency, and return broker results from the exact JSON bytes admitted by the broker. Published tool progress now uses owned snapshots with an 8 MiB cumulative payload allowance per run, including detached replay.
The native Channel merge keeps a finite number of call fibers even at concurrency one. Using
Stream.mergeAlldirectly selects its sequential fast path at one, which broke existing early-close telemetry and failure-observer tests. The explicit tool semaphore and sequential barriers remain in place.sequenceDiagram participant Handler participant Broker participant Redactor participant Caller Handler->>Broker: encoded result opt redactor configured Broker->>Redactor: validated JSON Redactor-->>Broker: replacement result end Broker->>Broker: serialize once, admit bytes, decode owned snapshot Broker-->>Caller: ProgrammaticCallSuccess with admitted snapshotCallers can tighten the progress allowance:
Exceeding that cumulative progress allowance fails with
ModelProtocolError; progress is not truncated. Application progress must be plain JSON; accessors and custom serialization are rejected. Terminal tool results retain their separatetoolResultBoundspolicy. This PR bounds published payloads. The native Toolkit producer queue needs the separate upstream backpressure fix, which is not yet part of the pinned Effect release.A local Node 24.20.0 ARM64 benchmark measured complete
AgentRuntime.runcalls, each streaming 1,005 parts for a 16,000-character ASCII string answer without invoking tools. One warm-up preceded five measured samples per case in separate baseline and candidate processes:These are local full-run measurements. Hosted CPU and provider latency were not measured. A separate blocked-handler diagnostic at concurrency two measured 11 suspended fibers for both 4-call and 64-call batches after the fix, versus 11 and 71 before.
Validation passed with 311 engine tests, engine type checks, and
vp run ready. Regressions cover mutable/getter-backed broker results, detached progress ownership and byte limits, provider progress accounting, declaration order, and cleanup at concurrency one and two. The new progress cohort failed five cases before the fix and passed all six afterward.The first full gate hit an intermittent existing Cloudflare alarm assertion at
alarm.test.ts:274. An isolated Cloudflare rerun passed 419 tests with two skips, then the full gate passed at the unchanged commit. Independent source review found no further correctness issues.