Address branch review findings 1-4 on feature/1.3.7-hedged-requests:
- src/polygateway/middleware/retry.py: recheck primary.done() after
hedge admission in _attempt_hedged; release the hedge permit via
settle_and_release(permit, 0) (release_probe for probe entries) and
adjudicate the primary directly instead of firing a billable HTTP
request that would be cancelled immediately
- src/polygateway/config.py: check_hedge_assembly raises a hedge-located
ValueError for empty sources instead of a bare min() error
- tests/unit/test_hedge.py: pin that an injected FakeClock jump of 10^6
seconds does not trigger hedging (design section 8); pin pick(exclude)
counting no gate_rejections and leaving reasons untouched; pin silent
hedge abandonment when the candidate circuit is open; deterministic
regression for the admission-window race (BlockingLimiter harness)
- tests/unit/test_config.py: assert the empty-sources guard message
locates the hedge key
Red-to-green evidence in tests/outputs/137/review-fixes/
CallStats gains hedges/generation_ms/hedge_won (all defaulted, appended
after total_latency_ms); _CallContext counts them via record_generation
(overwrite for chat/OCR, accumulate for embedding batches) and
register_hedge, and snapshot carries them out. All three _attempt
implementations time only the transport call itself on the same injected
clock as total_latency_ms; the chat sink is recorded by the orchestrator
so hedge winner attribution stays with T3. Hedge counters stay 0/False
until the T3 orchestration lands.
Red-green evidence: tests/outputs/137/t2/ (10 new tests AttributeError
red, then green; full unit+contracts 1621 passed).
Transport.complete gains the keyword-only first_token_event (no default,
per the port convention): streaming sets it on the first delta, the
non-streaming path accepts it but never sets it, None means the caller
does not observe the first token. All fake/wrapping transports and the
three direct call sites follow the signature; the e2e wrapper forwards.
Red-green evidence: tests/outputs/137/t1/ (batch A TypeError red, then
147 file tests + 1550 unit tests green).
The deadline governs waiting, not the moment a call returns: cleanup
still runs in finally, so the return time is the deadline plus the
cleanup cost (5-7x the deadline in the measured fixture). An expiry
therefore does not mean nothing was produced or nothing was billed.
Leaving the key unset keeps 1.3.5 semantics verbatim, which also keeps
its two long waits: a pure 429 sequence can still wait for a long time,
and a large finite Retry-After is still slept in full because the
library deliberately does not clamp the hint with backoff_max_s.
- CHANGELOG unreleased section states those three sentences, records the
cancellation settlement change (an attempt cancelled after the port
started but before the settlement is known keeps its reservation at
the source estimate: over-charge rather than refund something the
upstream may already have billed; known settlements and unclassified
escapes are untouched) and warns that except GatewayUnavailableError
does not catch CallDeadlineExceeded
- README in four places: the capability table, the exception handling
example, the "which exceptions reach the caller" table and the error
model section, which now spells out the remaining large-but-finite
Retry-After wait
- .env.example documents LLM__CALL_DEADLINE_S as commented out
- new findings file indexes the red/green evidence, the commands and
their exit codes, what was not run and who covers it, and repeats the
three residual risks
No version bump, no tag, no release: those belong to the release
checklist.
A gateway that answers 429 with Retry-After: inf (or 1e999, which float()
happily rounds to inf) used to reach backoff_delay as retry_after_s=inf.
max(delay, retry_after) then picked it, and since the library deliberately
does not clamp the hint with backoff_max_s, the attempt slept forever.
- _parse_retry_after now rejects non-finite values via math.isinf and
returns None, so the call falls back to plain exponential backoff
- it emits exactly one warning carrying the source name and the verdict
word retry_after_not_finite, never the raw header: under a 429 storm an
echoed header drowns the real signal and does not help localisation
- source_name becomes a required keyword-only argument; the function is
private, so no default is given and a missed call site fails loudly
instead of silently dropping the source identity from the warning
- nan keeps flowing through the existing seconds > 0 semantics; no new
branch, no reordering of the parse
Give one logical call an optional hard wall-clock boundary (issue #22).
Leaving it unset keeps 1.3.5 behaviour verbatim: the timeout context is
never entered when deadline_s is None.
- new deadline.py: ensure_call_deadline() range check (None or a finite
positive number; bool/0/nan/inf and out-of-range ints are rejected as
ValueError so OverflowError never leaks) plus with_call_deadline(),
which distinguishes an expiry from a TimeoutError raised by the body
or its cleanup via a local-variable identity comparison rather than
cm.expired() alone
- new CallDeadlineExceeded: deliberately outside the four categories and
not a GatewayUnavailableError, and carries no retry_after_s
- new {SCOPE}__CALL_DEADLINE_S key, guarded on the env, direct
construction and dataclasses.replace paths
- three clients take a call_deadline_s constructor argument and a
keyword-only per-call override on chat/embed/recognize_text/
parse_layout; None inherits the assembled value
- validation runs before the awaitable is created, so an illegal value
cannot strand an un-awaited coroutine
- one embed call shares a single deadline across all of its batches
- import-linter gains a polygateway.deadline layer
- cover where the deadline lands: backoff sleep, admission polling,
the structured re-ask ladder and embedding's batch loop, plus the
empty-texts early return that stays outside it
- cover what an expiry costs: exactly one terminal_failure row carrying
error_type=CallDeadlineExceeded, a cancelled attempt row sharing its
logical_call_id, cleanup that outlives the deadline (lower bound only)
and an already-billed success being discarded
- pin the injected clock as orthogonal: a 10^6 second jump never expires
a call, yet total_latency_ms still reads that clock
Merge-time gates, registry artifacts and anonymous page checks for 1.3.5
recorded as a separate finding; the validation finding gains a status
pointer so its historic 'not yet executed' section is not silently stale.
Released main/tag and uploaded artifacts are untouched.
Version bump in pyproject and __init__, CHANGELOG dated 2026-09-09,
README install lower bound raised to >=1.3.5, and release-prep evidence
(remote check, gates, real gateway smoke and one bounded live probe)
recorded in the 1.3.5 validation finding.
Field counts come from inspect, not memory: record_llm_call takes 36
parameters, COLUMNS has 36 entries, the physical table has 37.
- README: capability table says 36 fields and names the three row kinds;
new section covers reading call_stats, the five SQL migration items,
the attribution query and the storage-side upgrade
- README/.env.example/ARCHITECTURE: error_body follows the summarize_body
limit and the structured-exhaustion error carries its own bounded
explanation, so neither is inside PGW_TELEMETRY_TEXT_CAP coverage
- ARCHITECTURE 7.8: the ten columns with per-column semantics, the I3/I4
invariants, operation versus exc.operation, and the assembly gate
- CHANGELOG: unreleased section listing the four public changes and what
downstream must do, in particular counting failures by event_kind and
the assembly-time error for custom recorders
- schemas/llm-calls: the ten columns plus a three-row-kind section
- metrics/call-telemetry-coverage: 1.3.5 coverage contract, real live
baselines left unfilled rather than stating a fake percentage
Validation record records the T4 evidence: mechanical migration red then
green, the four PG acceptance cases, the seven-item mutation matrix with
all seven killed and the copy restored to an identical digest, plus the
PYTHONPATH pitfall that made the first mutation round silently test the
original source.
Version numbers and release steps are deliberately untouched.
Migrate the PG telemetry fixtures to the 36-field recorder and add the
storage compatibility acceptance the plan calls for.
Mechanical migration:
- _EXPECTED_COLUMNS 27 -> 37 physical columns
- _record_minimal gains the ten keys in the same shape as the unit suite
- _PRE_TENANT_COLUMNS now excludes 14 columns, derived from
_CALL_OBSERVABILITY_COLUMNS instead of a second hand-written list, and
the two manual-mode warnings assert a notice derived from COLUMNS order
so a column that silently drops out of the warning turns the test red
New TestCallObservabilityColumnsAcceptance, all on a 27-column 1.3.4
shaped table built by the existing pg_sandbox factory:
- auto appends the ten columns in the same order as a fresh database and
old rows keep NULL in every one of them (no backfill, no sentinel)
- manual sends no DDL, trims the INSERT, and still round-trips the other
26 columns value by value
- an old-version writer using insert_sql with the 1.3.4 column set and a
new-version writer share one table, and event_kind filtering counts
neither the old rows as failures nor as successes
_minimal_fields is split out of _record_minimal so the simulated old
process reuses the same values rather than copying them.
Verified against the real lab Postgres: 30 passed.
Grow the telemetry contract from 26 to 36 fields and give every logical
call a failure terminal row, so SQL can finally answer "how many calls
failed" and "why did the whole pool die".
Schema and port move together with the emitter writes in one commit:
splitting them would ship columns that nothing populates.
- schema: append 10 nullable columns (scope, operation, logical_call_id,
event_kind, http_status_code, error_type, cause_type, error_body,
attempts, total_latency_ms) to all five definition sites in one order
- ports: 10 keyword-only parameters without defaults; the protocol
signature is now the single source the assembly gate derives from
- emitter: take domain exception objects instead of pre-flattened text
and pin down the diagnostics in one helper; a relabelled 503 stays
503 and success rows leave all five columns NULL
- emitter: reject recorders whose record_llm_call cannot accept the
current field shape at assembly time, since _record would otherwise
swallow the TypeError and drop every row while calls keep succeeding
- clients: write at most one terminal row per logical call through a
single shared exit, deduplicated by the call context; TelemetryMW
stops writing terminals so the two sites cannot double count
- clients: cancellation stays best effort and propagates, non-domain
exceptions get no terminal row and keep their classification
- transports: give _status_to_error an explicit operation and fix the
historically mislabelled embedding HTTP failures
- structured: promote the bounded error formatter so the reask feedback
and the terminal explanation share one set of limits
Terminal rows carry no cost and no tokens, so cost aggregation is
unchanged; failure counts must now filter on event_kind.
The 2:25 slow run left exactly one red: kimi-for-coding answers 404
model_not_found because the channel removed it from the account group
between 09:44 (four green probes, correct model_reported) and 15:00. L8
was reading that as "the capability table drifted", which is a statement
about the model the channel no longer serves.
The 404/model_not_found rule already used by T10 now lives in one helper
and is applied on the L1-L9 side too, via the same unreachable fallback:
that one rejection skips and records an uncovered row, every other
RequestRejectedError still bubbles, since those are the real failures
this suite exists to catch.
L9's "unknown shape" sample was the openai profile, which 1.3.3 gave a real
shape (off/on_base/effort_key all set), so the guard had nothing to reject.
It now registers a shapeless provider of its own and tests the mechanism
rather than whichever profile happens to be blank that month.
L8 checks the reported model before judging the capability table: this channel
answers glm-5 / glm-5.1 / glm-5.2 with glm-5.3, which is a routing problem the
library already warns about, not drift. All three are guarded, including the
one that passed by luck.
T10 tells 404 model_not_found (the channel dropped the model) apart from 400
(the tier really is refused), reading the status code and the body's type field
rather than the whole message; only the latter still counts as a conclusion
about a tier. An all-skipped tier list now skips instead of going green.
TestMiniMaxM3 gained the unreachable fallback its own docstring promised: an
outage now skips and leaves an uncovered row, where before it failed ahead of
_record and left no trace of what happened.
iomgaa