Skip to content

Reuse provider HTTP clients and manage model context lifetimes - #78

Merged
frostming merged 3 commits into
mainfrom
feat/provider-client-lifecycle
Oct 10, 2026
Merged

frostming merged 3 commits into
mainfrom
feat/provider-client-lifecycle

Conversation

@frostming

@frostming frostming commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

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. Explicit provider.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, and uv build passed.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/republic/providers/base.py Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@frostming
frostming merged commit c37c253 into main Oct 10, 2026
17 of 24 checks passed
@frostming
frostming deleted the feat/provider-client-lifecycle branch October 10, 2026 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant