ARCHITECTURE 7.8 gains the pool resource semantics, the two-sentence
failure verdict and the two config keys; the ownership rule lands in a
new 4.5 because it is a cross-subsystem discipline, not a telemetry
convention. CHANGELOG leads with the three items downstream must read
first: the 3.12 floor, the connection count going from 10 per client to
on demand, and aclose no longer closing injected components.
The design traces the incident to four stacked defects rather than one bad
default: the pool is the only resource in the library that pre-allocates,
the kill switch keys off which step failed instead of what failed, the
degraded state can neither recover nor be observed, and the ownership rules
make the sanctioned sharing path unusable.
The plan sequences the tracker ahead of the pool and failure work so every
commit stays green, and records two facts the implementer needs up front:
the pool-construction path has zero test coverage today, and the commit
gate runs the full suite plus a complexity ceiling.
Branch review caught the docs claiming something the code does not do.
CHANGELOG, README and the design's behaviour matrix all said a
force-opened source under circuit_open=wait waits out the full stall
window. It does not: the probe let through after each cooldown is a
real attempt, so it burns a max_attempts slot like any other, and a
401 source usually runs out of retry budget first -- reason is
retry_exhausted, not stalled. Which budget wins depends on
max_attempts against the cooldowns and the stall window.
The behaviour is right; only the prose was wrong. Charging the probe
to the retry budget is exactly the split issue #8 settled: the
question is who spends max_attempts, and a probe does send a real
request. A test now pins it so the claim cannot drift again.
Also drops the planned "woke up" log line. Each wait round already
logs on entry with its duration, and a still-blocked wake-up logs the
next round immediately, so a second line would only double the volume.
The breaker conflates "this source is unhealthy" with "kill this call
now". Limiter rejections already choose between wait and fail_fast;
breaker rejections had no such choice, so a single-source scope loses
its whole retry budget the moment the gate opens.
Design adds {SCOPE}__CIRCUIT_OPEN (default fail_fast, so existing
deployments keep their control flow) and pins retry_after_s to "time
until a *certain* retry moment" across all six gate exits. The latter
also fixes a separate bug the issue missed: a half-open rejection fed
the probe lease (up to 2x timeout) into the source cooldown memo, whose
set_until only moves forward -- so a recovered source stayed blacklisted
in-process long after the gate closed. That one bites multi-source
deployments too, it is just hidden when other sources absorb the load.
Human-approved 2026-08-19; both documents revised after Codex review.
Postgres requires a partitioned table's unique constraints to cover the
partition key, so ranging on created_at forces the primary key to
(call_id, created_at) -- and ON CONFLICT (call_id) DO NOTHING then
matches no constraint at all. The retention design claimed INSERT stays
transparent under partitioning; that holds for the routing, not for the
conflict target, and telemetry would have failed outright on any
partitioned deployment. The write drops its conflict target, which is
byte-equivalent on a plain table and legal on both.
The cap design gains the three emitter construction sites it has to
touch and the relationship to the 200-char caps embed and OCR already
carry: they stay, and the new cap is the stricter of the two. Covering
all three call paths is deliberate -- their rows land in one table, and
issue #11 settled that argument already.
Both open issues ask the same question from opposite sides: how much
power the library holds over a downstream database. #13 wants the
structural writes back, #12 wants the data retention back. The two
designs share one boundary -- the library does SELECT and INSERT plus
an optional CREATE, and everything that alters structure or deletes
rows belongs to the downstream, with the library obliged to print the
exact SQL they need to run.
Two findings shape #13 beyond what the issue argues. The precedents it
cites (Hangfire's lock queue, Prefect's multi-instance race, Alembic's
audit trail) all live on a shared production Postgres, while the SQLite
side is a local file with no DBA and no migration tool, so the defaults
split by backend rather than uniformly. And turning ALTER off only
works together with trimming the INSERT to the columns that exist:
without it a stale table drops every row instead of two columns, which
breaks the telemetry rule harder than the automatic ALTER ever did.
For #12 only the body cap touches library code; retention and access
control land in the README, because the sdist carries src and the
README alone -- a template that lives in the wiki is one a downstream
pip install cannot reach.
The design said three times that retention and the _BACKFILL question
would be filed separately, and neither had been. That is the failure
mode the release checklist already records: a closing step nobody does
and nobody notices. Filed as #12 and #13, and the design now names them
so a later reader can follow the thread instead of trusting a promise.
OcrClient emits through the same helper and its rows land in the same
table as chat rows. Covering only chat and embed would leave one table
holding rows that have a tenant and rows that never will, and the
issue's own irreversibility argument applies to those rows too.
The design said two paths because the issue said two paths. Corrected
at the source rather than only in the plan, so a later reader does not
find OCR work with no design behind it.
Eight tasks against the approved design, ordered so the port and both
telemetry backends land before the three call paths that feed them.
Writing the plan turned up a third telemetry path the design missed:
OcrClient emits through the same helper and builds its ChatRequest on
the spot, just as embedding does. OCR rows share the table with chat
rows, so leaving them out would put a hole in a multi-tenant caller's
audit trail, and the same irreversibility argument applies. Listed as
Task 6 and flagged as beyond the approved scope -- it may be dropped,
but only by stating the limitation in the CHANGELOG, not silently.
The integration task pins the issue's own argument as a test: build a
22-column table, open it with the current recorder, and assert the old
rows read back as the empty string rather than NULL -- NULL under an
RLS policy is invisible to everyone, not merely unassigned.
The embedding client does not go through the chat onion -- embed() runs
its own chain down to _emit(), which builds a ChatRequest on the spot
and so far only fills session_id and parent_call_id. Changing chat()
alone would have left every embed row with empty dimensions, which is
exactly what the issue's second request asks for.
The bigger find: the draft claimed serialization could not fail because
the entry check already restricts values to scalars. It can. A float
passes a naive type check and json.dumps writes it as the literal NaN,
which is not valid JSON and which JSONB rejects; the failure then lands
in the emitter's degrade path and turns a caller's input error into
silently dropped telemetry. Now rejected at the entry with isfinite and
again at serialization with allow_nan=False.
Also states the validation runs at both public entries, not just chat(),
and adds the RLS template the design had promised but never wrote down.
Issue #11 asks for a tenant column so a multi-tenant caller can isolate
rows in the database. Widened to caller-defined dimensions in general,
but only the caller's own: model name and friends keep their existing
columns, and the library writes nothing into the new container.
Two independent findings force tenant_id to be a real column rather than
a key inside JSON. An RLS policy on meta->>'tenant_id' parses fine, but
the planner discards statistics for non-LEAKPROOF functions under RLS,
and ->> is not marked leakproof; the pgsql-general report that hit this
ended up moving the indexed column out of JSONB. Separately, the planner
has no usable statistics for JSONB at all -- @> falls back to a
hardcoded 0.1% selectivity.
A configurable promoted-column whitelist is rejected: when two
downstreams infer different types for the same key, the second
ADD COLUMN is silently skipped by IF NOT EXISTS and the wrong type is
written from then on, without an error.
The library stops at the column plus a documented policy template. It
must never enable RLS itself -- with no matching policy that is
default-deny, which would silently fail every write for the two
downstreams that are not multi-tenant.
The 500-char head-only rule came from a single sample. k8s client-go
caps the same thing at 2048; reprlib keeps head and tail because the
text is meant to be read. Gateway error bodies are JSON whose code and
request_id sit at the very end, so a head-only cut drops exactly what
you need to chase the provider. Version pinned at 1.2.0, which forces
the README install pin off ==1.1.*.
"Truncate at cap and append the ellipsis" admits both 501 and 500 total
length; the two would desync test assertions from the telemetry length
promise. Cap is now the total including the marker.
Issue #10: the 400 body dies in _status_to_error, and telemetry only
writes str(exc), so adding a field alone would not make the refusal
queryable after the fact. Design keeps the summary in both the message
and a new base-class body_text, across every non-2xx branch and both
transports.
PostgreSQL checks the schema CREATE privilege before the IF NOT EXISTS
existence test, so an account with only table-level INSERT was denied on
CREATE TABLE IF NOT EXISTS even though the table was right there and
writable. The denial set _failed and the whole recorder went no-op for
the process lifetime, silently: 150+ calls downstream lost their latency,
token and cost rows with nothing but one warning to show for it.
The probe is the direct fix. The larger fix is the criterion: structural
degradation now means "provably cannot write" (pool creation failed, or
the table is absent and cannot be created), not "something threw during
init" -- a probe or acquire failure just skips the row and retries on the
next call.
SQLite stays as it is on purpose. Measured: it short-circuits the
statement at parse time, so it passes even under another connection's
EXCLUSIVE lock or on a read-only file. A probe there would buy nothing;
the docstring now says so to keep symmetry-minded future edits away.
Registration pages carry the chosen approach, why the split is by "which
budget the time consumes", the five rejected alternatives with reasons,
and the 3.6 correction found during independent verification.
Independent verification found the first cut had swapped one bug for a
worse one. The budgets were split by "did we send a request", so a 429
attempt counted as productive — but 429 is exempt from the retry budget,
so its time burned neither budget. Against a queueing gateway that holds
the request for the full timeout before answering 429, a call could hang
for 301 attempts / 25.2 hours, measured, versus 301 seconds before the
change.
The split is now by which budget the time consumes: time that burns
max_attempts is excluded from stall, time that does not (429 attempts
included) belongs to stall. Measured again: back to one attempt / 301s.
Only the chat loop needs this — embedding and ocr count 429 against
max_attempts unconditionally, so the gap never existed there. The stall
verdict moved into _stalled(), which both call sites had duplicated, to
keep __call__ under the complexity gate.
Six tasks: StallClock plus the chat loop, then embedding, ocr, the config
comments, the full-suite regression with doc sync, and independent
verification. Codex review raised four points, all confirmed and folded in:
a stale line reference in the fidelity section, explicit cancellation
acceptance for T2/T3 (the new attempting() wrapper now wraps their existing
cancel paths), a telemetry-boundary test pinning the design's claim that
telemetry jitter must not feed the stall verdict, and concrete test
construction for the embedding/ocr regressions.
Two internal-consistency fixes from the independent design review:
the 429 saturation argument wrongly claimed exponential backoff growth
(429 skips the retry budget, so max(fails, 1) pins the delay to the base
tier), and "productive" was defined as waiting on the response while the
StallClock actually wraps all of _attempt. The boundary is now stated as
_attempt itself, including per-attempt accounting and telemetry, with the
rationale that telemetry jitter must not participate in the stall verdict.
Issue #8: a single request that burns its full timeout_s also exhausts
stall_window_s, so the retry budget silently never applies. Root cause is
that both budgets charge the same wall-clock time. The design makes the two
budgets orthogonal — real attempts bill the retry budget, everything else
bills the stall budget — which drops the timeout_s / stall_window_s coupling
instead of guarding it with an assembly-time check.
Letting SourceNotConfiguredError through the gate wrappers opened a hole
the recheck caught: _record_quietly only degrades GovernanceBackendError,
so an assembly defect raised from the accounting side would now escape and
destroy a response from a call that had already genuinely succeeded. That
inverts the exact invariant _record_quietly exists to hold.
Widening _record_quietly is the right fix rather than narrowing the
wrappers, because that layer degrades by what the path is (accounting, the
call is already done) rather than by which error type shows up. Narrowing
would have left 4 of 9 wrapper methods as exceptions to a rule nobody can
remember.
No backend raises it from an accounting method today, so this is a
guardrail for whoever adds source-name validation to a breaker backend.
The stub that first reported this green was wrong: its record_success
lacked count_attempt, so it raised TypeError and the wrapper relabeled it.
Fixed signature, then the test failed as it should have.
Also finishes the three-to-five leak path correction across the four
remaining spots, including the wiki summary card that indexes this design.
Independent verification caught that the split shipped in the previous
commit did not actually hold on the only path production uses. The gate
wrappers re-raise GovernanceBackendError but nothing else, so
SourceNotConfiguredError fell into the following `except Exception` and
came back out as a governance_backend_down failure with retry_after_s=5.0.
A misconfigured source name would still retry forever and never surface.
The existing tests missed it because both of them call the private _cfg()
directly, one layer below the wrapper the governance loops actually go
through. The regression test goes through QuotaGate.
telemetry.py has to widen its terminal catch in the same commit: once the
wrapper stops relabeling the error, it is no longer a GovernanceBackendError,
and it is raised before any attempt exists, so the path would have recorded
no telemetry at all.
Also corrects the leak path count from three to five. QuotaGate.stats and
BreakerGate.retry_after_s are not wrapped by _record_quietly either.
Five tasks, with the ARCHITECTURE section 6.1 revision first so the code
never contradicts the single source of truth, and the reparenting kept
atomic because scope is a required keyword argument and any split would
leave an unrunnable tree.
The Codex review caught that the planned test evidence pointed at the
wrong stubs: the ones at test_backpressure.py:176-186 cover accounting-side
degradation, not the three gate paths that actually leak to callers, and
try_acquire and try_enter have no stub at all.
All three open decisions were settled as proposed: a dedicated
SourceNotConfiguredError so a misconfigured source reaches the dead
letter queue instead of retrying forever, a 5 second retry_after_s so a
backlog does not stampede a backend that is already down, and public
export so callers can alarm on assembly defects specifically.
Pins the ARCHITECTURE section 6.1 revision to land before or with the
implementation, since the new scope reason contradicts the current single
source of truth. Documents why SourceNotConfiguredError may sit outside
the four-way classification: that rule governs transport-translated call
failures, and the GatewayUnavailableError family already lives outside it.
Also collapses the ten per-field response ternaries in emit_attempt into
an _AttemptUsage view. They all expressed the same decision and pushed the
method to cyclomatic complexity C, which blocked the commit gate.
Fail-closed governance backend failures are semantically scope-level
unavailability, yet GovernanceBackendError sits directly under
PolyGatewayError, so callers writing only `except GatewayUnavailableError`
drop them into the catch-all bucket and burn their failure budget on a
fault that a restart would clear.
The design reparents it under GatewayUnavailableError with a new
governance_backend_down reason, splits the two "unknown source" sites into
a separate assembly-defect error so a misconfiguration still reaches the
dead letter queue, and picks a non-zero retry_after_s to avoid a
zero-delay retry storm against a backend that is already down.
The design claimed merging would immediately break dissect. It would
not: dissect keeps running whatever version it already has, and this
release does not touch it. What is true is narrower -- once dissect
moves to 1.0.6, the M2.7 scope will refuse to assemble.
Worth recording because the distinction is not academic here.
dissect/requirements.txt declares polygateway>=1.0.1,<1.1, a range
rather than a pin, so 1.0.6 satisfies it and any routine reinstall picks
it up without anyone deciding to upgrade. So it is not "breaks on
merge", it is "breaks on the next dependency install".
The paragraph now carries both corrections it went through, since a
claim about downstream impact that was wrong twice is worth leaving
visible rather than quietly rewriting.
The thinking matrix had been running inside make ci all along, which is
not what the design claimed. It takes seven minutes, spends 137 real
calls, and its criteria are statistical, so a network hiccup fails the
build for reasons unrelated to the change under test -- one run died on
three consecutive network errors exhausting the source.
The project already has the mechanism for this: the slow marker, which
addopts excludes by default and the config comments describe as "CI runs
it on demand". Marking the matrix slow brings make ci back down from
seven minutes to ninety seconds while the matrix stays a merge
requirement via -m slow.
The design also claimed e2e does not run in CI. It does: make test runs
pytest over tests/, e2e included, and the existing smoke tests really
call the gateway whenever .env has credentials. Only slow-marked tests
are excluded. Both documents now say so.
Version sources are pyproject and __init__; a test enforces they agree,
and it caught the second one being missed.
The verifier caught that the disable-direction evidence only proved "no
regression", not "actually took effect": on M3 the disabled runs and the
no-opinion baseline are identically distributed, because that model does
not reason by default anyway. So the disable runs alone cannot rule out
the very failure mode issue #5 is about -- the parameter being silently
dropped upstream. The bogus-value experiment that does rule it out was
sitting in the findings document instead of the test suite; it is now
case L3b, and the L3 assertion that could never fail is gone.
Also from the review: the e2e helper caught bare Exception, which would
have disguised a library bug as an unavailable source, exactly the
silence the reporting discipline exists to prevent; the unregistered
model warning fired on every request instead of once per source; and the
transport caught ValueError broadly enough to mislabel unrelated errors,
now narrowed to a dedicated ThinkingUnsupportedError.
The design and plan still described the original judgement criteria,
which the measurements had already overturned. Both now match what the
tests actually do, and the design no longer claims the only new failure
surface is the openai one -- dissect configures MiniMax-M2.7 with
ENABLE_THINKING=false and will fail at assembly, which has to be
coordinated before this merges.
Findings: live-API measurements across MiniMax M3/M2.7/M2.5, qwen and
deepseek, plus a survey of how nine unified gateways model per-model
parameter divergence. Key facts: reasoning_effort is MiniMax's real
switch, M2.x reasoning is mandatory and cannot be disabled, and the
relay's local token-count fallback silently drops reasoning_tokens.
Design: keep the parameter shape at provider level, push capability
down to model level, split "unknown" / "unsupported" / "no opinion"
into three distinct values, and fail at assembly time when a model
cannot honour enable_thinking=False.
Pin the telemetry column semantics across all three emitter entry points,
add the cross-layer sampling snapshot, and reject extra_body on the
embedding and OCR paths instead of accepting it silently.
The reviewer confirmed the T1-T4 ordering holds -- it re-derived every
intermediate state and checked that no construction path can produce
est_tokens=0 with tpm>0 before T4 -- but found four gaps.
Two existing tests go red and the plan never said so: test_types.py:94
asserts the very constraint T4 deletes, and test_embedding.py:105 is a
transport-level case for the fallback T3 rewrites, easy to miss while
looking only at test_openai_compat.py.
The T4 acceptance line claimed all three settlement sides use the
derived value, but the cancel branch never assigns actual and leaves it
at the retry.py:329 initial zero -- an implementer would have "fixed"
a branch the design freezes. Corrected here and in the design section
5 sentence it came from.
USAGE_SOURCES would have landed with no consumer, so T1 now carries the
two value-domain assertions the design asks for, including the one that
pins the no-runtime-validation ruling.
Records the approved option, the four rejected alternatives with their
reasons, the intentionally dropped CHS migration item, and the two
defects the independent review caught. Links the entry to m1-core-design
as a refinement, since that milestone is where est_tokens froze with
both jobs attached.
The value-domain table still listed the OCR endpoint as a producer of
"unavailable" while section 3.3 had just decided to keep its "measured"
label -- an implementer following the normative table would have redone
the change that was explicitly dropped, and the guard test would fail.
Also corrects the derivation call-site count to five, qualifies the
retained conservative settlement to the non-dead transient branch only,
and pins the gap metric to "AND cache_hit = false" so cache hits, which
carry cost 0.0 by design, do not inflate it.
The reviewer found two real defects. First, changing the usage fallback
to (0, 0) breaks the success-side settlement too, not just the failure
side: retry.py:338 and embedding.py:271 take actual from the same
return value, so a call whose gateway never sends a usage frame would
have its whole pre-deduction refunded -- systematic TPM undercounting.
Added as change item 9. Second, dropping the OCR item: types.py:51 and
ocr.py:9 both state OCR's zero token count is a fact, not an unknown,
so "measured" was already accurate, and relabelling it would pollute
the very metric used to justify the chosen option.
Also pins the cost short-circuit after the cache_hit branch, confines
value-domain enforcement to producers so no bare ValueError escapes
chat(), completes the authoritative-document list, and narrows the
p90 rejection to the read-port argument.
Split the two jobs SourceConfig.est_tokens has been doing: TPM entry
pre-deduction, where conservative means safe, and the telemetry usage
fallback, where feeding a worst-case upper bound through the output
price inflates cost by ~26x.
Records the approved decisions: usage_source gains an "unavailable"
state whose cost is NULL, and an unset est_tokens derives from
tpm//60 so the in-flight ceiling stays scale-invariant. Also declares
the CHS "conservative accounting" migration item as intentionally
dropped, and the pre-existing global-TPM gap as knowingly unfixed.
Refs: gitea issue #2
Mutation testing showed the negative structured-retries and expected-dim checks
in the env parsing path could be deleted with every test still passing. Their
value is the env key name in the message, so they need tests that assert it.
Changelog now states the real scope of this release and warns that normalising
scope moves the Redis keys, the one change here that silently relocates runtime
state. Records the breaker threshold derivation as deliberately env-only so it
does not resurface as another round.
The verifier found four more env-only behaviours of the same class the branch
was already fixing. The worst is scope: it goes straight into the Redis keys
(pgw:limit:{scope}, pgw:gate:{scope}), so one process using from_env("LLM")
and another constructing scope="LLM" by hand split the rate limit and breaker
state across two namespaces, each tracking its own quota, with no error.
Blank redis_url and pricing_path now collapse to None as from_env has always
done, so they fall into the required-field checks instead of reaching the redis
client as an unparseable URL. EmbeddingSettings gains the __post_init__ it never
had, moving its batch_size and expected_dim checks off the from_env-only path.
Also adds the cache backend whitelist test that mutation testing showed missing.
Round two of the from_env-only validation problem. Fifteen checks still lived
in the env parsing functions: six enum domains, the redis_url requirement for
redis-backed limiter/breaker/cache, cache namespace and TTL, telemetry path and
DSN, non-negative structured retries and non-blank scope. from_settings and
direct construction bypassed all of them.
The five asserts in client.py that claimed config had already validated
redis_url and the telemetry targets now hold on every path, so they revert to
what CLAUDE.md permits: internal invariant declarations that also narrow the
Optional for type checkers. Their comments now name the method that guarantees
them, since the previous wording is exactly what went stale.
Postgres DSNs built by hand now get the SQLAlchemy +driver suffix stripped the
way from_env has always stripped it, with a warning so the rewrite is not
silent. The env path strips earlier, so it stays quiet.
iomgaa