Repository navigation
Reuse provider HTTP clients and manage model context lifetimes - #78
Conversation
There was a problem hiding this comment.
Requesting a change before merge: one lifecycle interaction.
A model-context exit closes the Provider even while the caller is still inside async with republic.get_provider(...). The Provider and its owned HTTP client are closed at the first inner model exit, the Provider cannot be reopened, and everything after that point in the caller's own scope fails with RuntimeError("Provider is closed") — including the natural loop that enters one model context per model. The inline comment carries the executed reproduction and the repair directions.
The rest of the new contract holds as documented: one lazily created client shared by requests, streams, list_models() and every model of that Provider; a supplied client stays caller-owned; close() is idempotent; stream exit closes only its response; the event-loop binding is enforced.
There was a problem hiding this comment.
No blocking findings.
The lifecycle interaction raised in the earlier review is fixed at this revision, and the documented client-reuse contract holds. Verified by replaying one probe on both revisions: on 03e5ac8 the model-context exit closed the Provider and the next request raised RuntimeError: Provider is closed; on 99c701d the Provider and its client stayed open through model-context exits and closed when the last context entry exited. Details are in the existing thread.
There was a problem hiding this comment.
No blocking findings.
The candidate is 99c701d, the revision at which the repaired nesting behavior in the existing thread was verified, so that finding stays settled and no inline comments were added. Native quality, tests, type-check, build and docs checks passed on the merge checkout 5006050 that contains this candidate; I re-ran the lifecycle suite on that same checkout (42 passed) rather than repeating the earlier scripted probes.
Repeated requests through a Provider currently create a new HTTP client each time, preventing connection-pool reuse. This change lazily creates one client per Provider and reuses it across models, requests, retries, streams, and model listing.
Providers gain asynchronous
close(),is_closed, and async context management. Chat, embedding, and decision model contexts register with the Provider, which closes automatically only after all Provider and model context entries have exited. Nested contexts for the same Provider or model and sequential model contexts inside a Provider scope keep the client available. Explicitprovider.close()still closes immediately. Supplied HTTP clients remain caller-owned, and stream exit closes only the response.Owned clients must be used and closed on the event loop that first used them. Client cleanup is documented in the Provider API; ordinary documentation examples retain their existing direct model usage.
Validation: 436 tests passed, including regressions for Provider/model nesting in both directions, sequential models, nested-context exceptions/cancellation, and explicit closure.
uv run prek run --all-files,uv run ty check, strict MkDocs build, anduv buildpassed.