Repository navigation
feat(runtime): resolve capacity per eval - #38
Merged
Merged
Conversation
A runtime's capacity was fixed at construction, so every attempt under it held a slot in the shared pool, including evals that consume none of the pooled resource: a consumer's no-credentials eval provisions nothing from the pool yet still waited for and occupied a pool slot, throttling the attempts that needed one. capacity now also accepts a resolver over the eval's metadata, answering undefined for an eval that needs no slot. The scheduler asks it per attempt. A resolver is a capability function, so it leaves the experiment identity like containerImage does; the static form stays identity.
 [Medium impact] The PR appears safe to merge; the added scheduler test addresses the previous finding. SummaryAdds per-eval capacity selection through
No new actionable issues were found. Tests were not run during this review. Reviews (2) · Last reviewed commit: "test(runtime): prove an exempt eval star..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile's finding on #38: the resolver was tested in isolation, so the scheduler could regress to holding a slot for an exempt eval without a test noticing. Drive the resolver through schedule() and assert the exempt attempt runs beside the slot holder while the second pooled attempt waits.
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.
Summary
Runtime.capacityaccepts a resolver(metadata) => { key, limit } | undefinedbeside the static{ key, limit }. The scheduler asks it per attempt with the eval's metadata, so an eval that consumes no pooled resource answersundefinedand never waits for or holds a slot.workos-poolslot for every attempt in pool mode, including the no-credentials eval that provisions nothing (evals#20 review thread 11). The eval could not opt out because capacity was fixed when the runtime was constructed.containerImageandegressHostsresolvers are; the static form stays part of the experiment's identity.RuntimeCapacityis exported for consumers.tests/capacity.test.ts, plus a manifest check that a static capacity enters the runtime hash and a resolver does not. Docs:docs/extensions.md.bun run typecheck,bun run lint, fullbun test(one failure is the known Docker-absence case intests/mcp-proxy.test.ts, which passes with a daemon). No consumer name in harness code.