Release 0.2.0: receipts, transport injection, wider retries - #13
Merged
Merged
Conversation
Seven changes a caller can see, each closing a gap they could not work around. A receipt. Every response carries the versioned model that answered and the token usage of the request, and both were read onto a span and then discarded. Cost attribution and pinning a policy to the model version it was tuned against had no path short of running an OpenTelemetry pipeline. `ask_with_receipt` returns them beside the answer, with the same generated overload family `ask` has, and `ask` is now that call followed by `.answer`. `Receipt` lives in its own module because the generated surfaces name it in a return type and `guide` imports those. Context managers, over a counted pool. `with_policy` hands back a guide over the same httpx client, so `close()` on either closed it for both. That was survivable as a documented sharp edge and would have been a trap the moment `with` existed, so the pool now counts its holders and closes when the last one releases. Transport injection. Nothing could replace the httpx client: no proxy, no client certificate, and no way at all to test code that calls `ask` without standing up a server. `transport(..)` and `async_transport(..)` take one. They refuse a `timeout` beside them in either order, because httpx hands a transport that budget as a request extension it may ignore, and a silent no-op is worse than a `ConfigError`. `GET /v1/models` is retried. The API's own docs say an SDK handles a 429 for you, and a 429 there used to fail a process's startup. Both endpoints now run through one loop, so the promise has no hole in it. Connection failures are retried, inside the same budget. A refused connection or a connect timeout means the request never reached a server, so nothing was judged and a resend repeats nothing. A read timeout and a mid-response disconnect are still not retried: the request arrived, and asking again would buy the same judgment twice. Their retry event carries `error.type` in place of a status, and exactly one of the two is present, so a dashboard can tell a throttled API from an unreachable one. `OverloadedError` keeps its `retry-after`. The 529 path parsed the header and threw it away while the 429 path kept it, on the one status where it is the only thing that says when to come back. Breaking, and nobody outside this repository depends on 0.1.x yet. Nine names join the top-level list, the question types among them, so annotating a stored question no longer means importing from a module the README calls private. `guideme.api` and the four lower modules gain an `__all__`, and the surface test proves by AST that the wire mirror re-exports nothing it imported.
The README gains a configuration table, a "Testing your code" section that is a complete pytest function against an injected transport, and sections for the receipt and for concurrency. Two things it stated were true and written nowhere: that a guide is meant to be built once and shared across threads or tasks, and that `timeout` is spent per phase by httpx rather than as a deadline for the attempt, which is why a call can outlast the number a caller wrote. Both are now where a reader looks, and the timeout's scope is in `docs/contract.md` as a stated divergence from the Rust SDK rather than something to be discovered. `docs/contract.md` also drops the rubric asymmetry between the two SDKs' runtime constructors, which guideme-rust closed at 0.2.0, corrects a stale `Rubric::into_wire()` to `Rubric::render`, and states the receipt and the retry policy as interface-shape items. `docs/observability.md` records the retry event's two shapes and why the cause is which field is present rather than a value. The test ceiling goes from 47 to 51. Four functions, each reaching something no existing one could: two guides over one pool, a receipt read back by a caller rather than off a span, an ask answered with no server at all, and a transport made to fail on demand, which no local server can be. Everything else this release added rode a parameter of a test that was already there.
Both lock files record the version and both are resolved with --locked in CI, so the bump and the two refreshed locks go together or the example's build fails. `spec/` and `spec/SOURCE` are deliberately untouched: nothing in the Rust `spec/` moves at 0.2.0, and the provenance commit is re-vendored once the guideme-rust pull request has merged. Until then the `spec-drift` job reads this copy against a `main` that does not yet carry the contract wording this release refers to.
The shared retry policy was revised after the Rust review: only a failed connection is resent, and a timeout of any phase is not. `httpx.ConnectTimeout` passes the idempotency test that decides every other case here — the request never reached a server, so a resend repeats nothing — and is excluded anyway, because the contract is shared and Rust cannot draw that line. `reqwest` sets one deadline over the whole attempt, so a connect-phase timeout is `is_timeout()` there and never `is_connect()`. Resending a failure the other SDK cannot even see would be the two disagreeing about one incident, which is the thing `spec/` and `docs/contract.md` exist to stop. The second reason stands without the first: a retried timeout multiplies the wall time `timeout(..)` is set to bound, and httpx already spends that budget per phase, so the worst case is long enough before anything is resent. `resend_after` now narrows to `httpx.ConnectError` alone. That is enough by itself: in httpx, `ConnectTimeout` descends from `TimeoutException`, not from `ConnectError`, so every timeout is excluded by the one isinstance check and no separate exclusion list can fall out of step with it. The parametrised table that pins this keeps its four cases and gains a fifth, `PoolTimeout`, so the "no timeout, whatever phase it names" rule is asserted for a phase that has nothing to do with the network. The function count is unchanged.
Adding the connect-timeout cases took tests/test_wire.py from 999 lines to 1011. It holds every wire and contract assertion the SDK has, parametrised, so it is a table at its natural size rather than a module that grew tangled, and it will cross this line again on the next release whatever is trimmed now. Trimming to fit would leave the next person in the same place with no record of why, so the suppression names the state instead: the file wants splitting along the seam between the wire and schema checks and the failure, retry and transport ones, and that move is a follow-up rather than churn inside a diff under review.
Review wave 1. One bug, one thing I had wrongly called impossible, and seven
smaller corrections.
The bug: `share()` returned `self`, so a guide and one derived from it with
`with_policy` were the same client carrying one hold between them. Closing the
first guide twice released twice, and the pool shut under the second guide — its
next ask raised httpx's `RuntimeError: client has been closed`. A count cannot fix
this on its own, because it cannot tell which holder a release came from. So
`share()` now hands back a distinct client over the same pool, each with its own
release-once flag, and a guide releases exactly once however many times it is
closed. Sharing from an already-closed client is refused rather than copied: it
would hand back a client holding nothing over a pool that may already be shut, and
that only surfaces on the first ask.
The regression rides `runner.paired`, parametrised on how many times the derived
guide is closed, and it also asserts the pool ended on zero holds. Without that
count the test would pass against a version that leaks: all three asks answer
whether the last hold was released or never released at all, and only the count
tells those apart. Checked both ways — it fails against the original, and against
a flag-only fix that leaks.
The casts: I reported the recursive-alias approach as the only option and concluded
a cast was unavoidable. A `TypeGuard` is the option I missed. Narrowing by
assignment intersects with the declared type, so `object` narrowed to a bare
`tuple` stays `tuple[Unknown, ...]` whatever the annotation says; a `TypeGuard`
replaces it outright, so the element type is `object` and the predicate body is a
real isinstance a checker verifies. `rg -n 'cast\(' src/guideme` is now empty.
`Usage` on the top-level surface is a frozen dataclass of two `int`s, copied out of
the wire model in `_receipt` the way `_described` copies `ModelEntry` into
`ModelInfo`. Exporting the pydantic one would have saved copying two integers and
put 28 attributes that are not guideme's on a published type. The paragraphs
arguing the other way are gone; the precedent holds, and `AGENTS.md` now states it
as a rule rather than an exception. `guideme.api.Usage` stays the wire mirror.
The rest: `transport()` and `async_transport()` refuse each other at the setter,
since a builder holding both can build neither and two build-time errors would each
blame the other setter. "Throttled" became "resent" where retries now cover a
failed connection too. `docs/design.md` covers all four `Question`/`api.Question`
pairs rather than one, and the README says which is which. The prose paragraph that
was splitting `AGENTS.md`'s layout table moved below it.
The wire suite reached a thousand lines and was shipping a `too-many-lines` suppression into a release. A suppression is a note that something is known and unfixed; the split it was standing in for is small and the seam was already clear, so there is no reason for the note to outlive the review that found it. `tests/test_wire.py` keeps what guideme puts on the wire and reads back: the schemas, the docs examples, the request a shape builds, the errors a malformed response raises, the unsure ladder, and what a configuration mistake refuses. `tests/test_retries.py` takes the request that does not simply succeed: the status-to-error table, the retry policy on both endpoints, what is and is not resent before a response arrives, an injected transport, and the pool two guides share. That is one code path each rather than a line count cut in half. Everything in the new module goes through `Client._fetch` and the pure `step` beneath it, and every one of its cases needs a server that misbehaves on purpose; nothing in the other one does. 718 lines and 335. Two helpers are needed on both sides, so they moved to `conftest.py` rather than being imported across: `answering_offline`, which the transport tests answer with and the transport-and-timeout refusals build a `MockTransport` from, and `MODELS_BODY`, which the model-list test and the throttled-models case both serve. No behaviour change. 51 test functions and 223 collected, both unchanged, and the same 220 run.
guideme-rust squash-merged its 0.2.0 at 1565ce95015196e8f49bbc18dcb4b4b0cf429aed.
This copy of `spec/` was taken at d908ecf, the rubric-examples commit before it.
Nothing under `spec/` moves. 0.2.0's contract changes are the receipt, the retry
policy and the rubric asymmetry closing, and all three are statements in
`docs/contract.md` rather than schemas or vectors: the wire shapes are unchanged
and the 42 policy vectors regenerate byte-identically. `mise run spec-check`
confirms it against the merged commit:
vendored from 1565ce9...; guideme-rust main is 1565ce9...
spec/ matches guideme-rust main
So this is provenance and only provenance. The drift job diffs content with
`-x SOURCE` and was green before this commit too, which is exactly why the file
has to be written by hand: nothing fails when it goes stale, and a reader asking
which upstream commit this copy corresponds to would otherwise be told the one
before the release it ships with.
Review wave 2. One leak, one lock, one gap in the retry tests, and five corrections to prose that had drifted. The leak: `with_policy` built the derived guide in one expression, and Python evaluates arguments left to right, so `share()` took the hold before `_merged` settled the patch. A patch that could not settle then raised with the hold already taken and no guide in existence to release it — measured at two holders after one refused call, and a pool that never closes. The patch settles first now. This kind of leak is invisible until a process runs out of sockets, so the regression asserts the holder count rather than any symptom, as a third case on the pool test. The lock: the count and every holder's spent-flag now sit under one `threading.Lock` on the pool. `Guide`'s own docstring says to share a guide across threads, so two threads closing two guides over one pool is a documented thing to do, and a flag read, a flag flip and a decrement are three steps that must not interleave — one order closes a live transport, another leaks it. `take` and `drop` do not lock because their caller is already inside the critical section. Nothing slow happens under it: `httpx`'s close is called after it is released and no `await` is ever reached while it is held, so the asyncio client uses the same plain lock. `ask` is untouched and takes no lock at all. A `retry-after` past the 30 s cap had no test on either endpoint, which is the one branch in `step` where the call fails immediately *and* carries a duration. Four cases on the failures table cover it: 429 and 529, evaluate and models. Covering `models` meant the table had to say which call each case makes, so `Failure` gained `drive` and `asks`; the six existing cases keep their behaviour, and the absence of an ask span on a `models()` failure is now asserted rather than assumed. The prose: the retry event's docstring still described a connect timeout as resent, which stopped being true when the timeout rule changed. `docs/observability.md` gains the caveat Rust carries, that an injected transport decides which exception is raised and therefore which side of the line a failure falls on. `docs/design.md` adds `Usage` to the same-name pairs — the one this release created, and the only pair with both halves in a published `__all__`. The README's "Lower layers" framing claimed a tier that two of its bullets contradicted; the tier is now named as the three modules it actually is, and the other two bullets are introduced as what they are, which is where some top-level names are declared. `AGENTS.md` claimed every module declares an `__all__`, which was never true: six do, and the list earns its place by being checked.
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
1 participant
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.
guideme 0.2.0 for Python, against the validated proposal. Seven behaviour changes and
one breaking one, plus the documentation each of them needed.
What a caller can do now
Read what a request cost.
ask_with_receiptreturnsReceipt[T]—answer,model,usage— with the same generated overload familyaskhas. Every response already carriedthe versioned model that answered and the token counts; both were written to a span and
discarded, so cost attribution and pinning a policy to the model version it was tuned
against had no path short of running an OpenTelemetry pipeline.
askis now this callfollowed by
.answer.ReceiptandUsageare exported fromguideme.Use a guide as a context manager.
withandasync withon both guides, each closingon the way out. The pool underneath is counted and each holder releases exactly once:
with_policy(..)hands back a guide over a distinct client sharing the pool, so closingeither guide — however many times — leaves the other able to ask. Before this, closing a
derived guide closed its parent's pool, a documented sharp edge and a trap the moment
withexisted. Sharing from an already-closed guide is a
ConfigError, and a patch that cannotsettle is refused before any hold is taken. The count and every holder's spent-flag are
taken under one lock, because the docstring tells you to share a guide across threads.
Inject the transport.
GuideBuilder.transport(..)and.async_transport(..). A proxy,a client certificate, or an
httpx.MockTransportthat answers a test with no server, noport and no key; the README's new Testing your code section is that test written out.
A transport and
timeout(..)refuse each other in either order, because httpx hands atransport the budget as a request extension it may ignore and a silent no-op is worse than
a
ConfigError. So does building the wrong kind of guide from one.Survive a 429 at startup.
GET /v1/modelsis retried on 429 and 529 through the sameloop and the same spans an ask uses. The API's own docs say an SDK handles a 429 for you,
and saying that of one endpoint only was a promise with a hole in it.
Survive a dropped connection.
httpx.ConnectErroris retried inside the samemax_retriesbudget and backoff: the request never reached a server, so nothing was judgedand nothing is repeated. A mid-response disconnect and an undecodable body are not retried
— the request arrived, and a resend would buy the same judgment twice.
No timeout is retried, of any phase,
httpx.ConnectTimeoutincluded, revised after theRust review. It passes the idempotency test every other case here is decided by, and is
excluded anyway because the contract is shared and Rust cannot draw that line:
reqwestsets one deadline over the whole attempt, so a connect-phase timeout is
is_timeout()thereand never
is_connect(). Resending a failure the other SDK cannot see would be the twodisagreeing about one incident. Independently, a retried timeout multiplies the wall time
timeout(..)exists to bound.resend_afternarrows tohttpx.ConnectErroralone, which issufficient on its own:
ConnectTimeoutdescends fromTimeoutException, notConnectError,so one isinstance check excludes every timeout and no exclusion list can fall out of step.
Breaking
OverloadedError(retry_after: timedelta | None)with aretry_afterattribute, mirroringRateLimitedError. The 529 path parsed the header and threw it away while the 429 pathkept it, on the one status where it is the only thing that says when to come back.
Constructing the error by hand is the only code this moves; catching it is unchanged, and
nobody outside this repository depends on 0.1.x yet.
Telemetry
guideme.retrycarrieserror.type = "transport"and nohttp.response.status_codewhen the attempt it is resending never got a response. Exactly one of the two is on every
such event, neither is ever a placeholder, and the cause is which field is present rather
than a value inside one field — a dashboard grouping retries by cause has to tell a
throttled API from an unreachable one.
docs/observability.mdhas the table.Surface
guideme.__all__reaches 44 names:Question,NoulQuestion,ChoiceQuestion,ScoreQuestion,DetailedNoul,DetailedChoice,DetailedScore,ReceiptandUsage.Annotating a stored question no longer means importing from a module the README called
private.
guideme.apigains an__all__of its own, soimport *from it stops handingback
BaseModel,FieldandMapping, andtests/test_surface.pyproves by AST that itre-exports nothing it imported.
guideme.question,guideme.policy,guideme.enumsandguideme.errorseach gained one too.Usageon this surface is a frozen dataclass of twoints declared inguideme.receipt,copied out of the wire model in
_receiptthe way_describedcopiesModelEntryintoModelInfo.guideme.api.Usagestays the wire mirror. No pydantic type reaches thetop-level surface, which
AGENTS.mdnow states as a rule: a published type must not carrya dependency's methods or change shape when that dependency has a major release.
Documentation
The README gains a configuration table, Testing your code, and sections for the receipt
and for concurrency. Two things that were true and written nowhere are now where a reader
looks: build one guide and share it across threads or tasks, and
timeoutis spent perphase by httpx rather than as a deadline for the attempt, so a call can outlast the number
you wrote. That divergence from Rust's
reqwestdeadline is stated indocs/contract.md,which also drops the rubric asymmetry guideme-rust closed at 0.2.0, corrects a stale
Rubric::into_wire()toRubric::render, and states the receipt and retry policy asinterface-shape items.
Tests
51 functions, 233 collected (230 after the live tests are deselected); 47 and 193 before.
The ceiling moves to 51 in
AGENTS.mdwith the reason. Four new functions, each reachingsomething no existing one could: two guides over one pool, a receipt read back by a caller
rather than off a span, an ask answered with no server at all, and a transport made to fail
on demand, which no local server can be. Everything else rode a parameter of a test that was
already there — the 529's
retry-after, the models retry, and the five transport-and-timeoutrefusals, and the retry-exclusion table, which gained
ConnectTimeoutas a not-retried caseand
PoolTimeoutas a fifth so the rule is asserted for a phase with nothing to do with thenetwork.
The server-backed checks are now two modules.
tests/test_wire.py(718 lines) is whatguideme puts on the wire and reads back;
tests/test_retries.py(335) is the request thatdoes not simply succeed — the status-to-error table, the retry policy on both endpoints,
what is and is not resent before a response, an injected transport, and the pool two guides
share. That is one code path each, not a line count halved: everything in the new module
runs through
Client._fetchand the purestep, and every case needs a server thatmisbehaves on purpose.
answering_offlineandMODELS_BODYare needed on both sides andmoved to
conftest.py. Thetoo-many-linessuppression is gone. No behaviour change.Item J: done
src/guideme/ask.pywalks a shape through fourTypeGuardpredicates instead of fourcast()calls.rg -n 'cast\(' src/guidemereturns nothing. I had reported this asimpossible; that was wrong, and the reason is worth writing down. Narrowing by assignment
intersects with the declared type, so
objectnarrowed byisinstance(x, tuple)staystuple[Unknown, ...]no matter what the left-hand side is annotated as, and strict'sreportUnknown*family fires. ATypeGuardreplaces the narrowed type outright, so theelement type is
object— exactly whatencodeaccepts — and the predicate body is a realisinstancea checker verifies, which acastnever was.TypeIswould be the tighterspelling and needs a newer floor;
TypeGuardhas meant this since 3.10.Note on CI
spec/SOURCEis re-vendored to1565ce95015196e8f49bbc18dcb4b4b0cf429aed, the mergedguideme-rust 0.2.0. Nothing under
spec/moves: 0.2.0's contract changes are statements indocs/contract.md, the wire schemas are unchanged and the 42 policy vectors regeneratebyte-identically, which
mise run spec-checkconfirms against that commit. No tag wascreated.