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.
TransientError and SourceDeadError read like caller-facing contracts in
the taxonomy table, but the retry loop catches both and repackages them as
AllSourcesExhausted, so they never arrive. That is only discoverable by
reading middleware/retry.py, and a downstream project wrote a whole design
section on the false premise before checking.
The new table states the split outright, including that
GovernanceBackendError now sits on the caller-facing side and
SourceNotConfiguredError deliberately does not join the retryable family.
GovernanceBackendError arrived with the M2 distributed backends but never
made it into the section 6.1 table, so it had no place in the taxonomy
callers actually read. That omission is why the README missed it too.
Records the reparenting, the new governance_backend_down reason, and why
SourceNotConfiguredError deliberately stays outside the reparented family:
a misconfigured source name must burn its failure budget and surface,
not retry forever in silence.
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.
A sixteen-row matrix over 127 real calls: disable and enable on
MiniMax-M3 in both streaming and non-streaming mode, extra_body winning
over the profile slot, qwen and deepseek still disabling correctly, a
drift sentinel that re-derives every registered capability from live
behaviour, and the assembly guard refusing the models that cannot
comply.
Two judgement criteria had to be corrected by the data they were meant
to judge. Output length cannot separate the two regimes at all -- the
disabled runs reach 46 tokens when the model narrates its working in
the visible answer, and the enabled runs drop to 13 when medium effort
barely thinks. reasoning_tokens separates them cleanly in both
directions, which is precisely what issue #6 was collected for. A
second anchor compares prompt_tokens between the two regimes: the
vendor injects a reasoning instruction when thinking is on, so the
input side grows, and comparing the two runs relatively avoids
hardcoding any vendor number.
Provider names are mapped explicitly rather than guessed from the model
string; guessing had silently skipped the qwen row behind a "source
unavailable" reason that was not true.
Ten tasks in a fixed order: land reasoning_tokens first so it can serve
as the acceptance instrument for the thinking-switch fix, then reshape
the provider profile, add the model-level capability table, wire the
assembly guard, fold enable_thinking into the cache fingerprint, and
verify the whole thing against the live API.
Incorporates a read-only Codex review: resolve_thinking now takes the
model name so its errors can name it, and the warning assertion uses a
loguru sink because caplog cannot see loguru output.
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.
The test passed overlay and sampling as separate objects while production
aliases them, so an in-place mutation slipped through it. Also syncs the
telemetry schema page and adds the missing postgres round-trip assertion.
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.
Wiki site synced across six pages (commit 4c8dc09 on the wiki repo) and
issue #2 answered with the shipping conditions for the downstream
workaround removal.
Leaves the wiki-site sync, the issue #2 reply and the version bump
unticked -- those are external deliverables this repository cannot
self-certify, and the pre-merge review was right to flag their absence.
The migration doc is a standing constraint on library design, so an
outdated enum there states an outdated fact. B13's substance survives
the change -- GovDoc's zero was never the problem, the missing label
was -- but the row now names unavailable and the cache_hit-qualified
gap query alongside it.
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.
The ordering is the load-bearing part. All three changes interlock and
every wrong interleaving fails silently: flipping the usage fallback to
(0, 0) before the settlement points read the derived value refunds the
whole pre-deduction on success, and flipping the embedding transport
before _merge goes three-state mislabels unavailable batches as
measured. So the plan adds the capability first, moves all five call
sites onto it while it is still equivalent, only then lets the third
state take effect, and unbinds the constraint last.
Registers both wiki entries and links the plan to its design.
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.
The independent verifier found 15 more checks still living only in from_env:
six enum domains, seven conditional-required pairs and two scalar ranges.
More severe than round one because client.py has five asserts that claim
config already validated the redis_url and telemetry paths, which is false on
the from_settings path.
Also records a normalisation gap the verifier missed: _load_pg_dsn strips the
SQLAlchemy +asyncpg suffix, so a hand-built DSN reaches asyncpg unstripped.
Corrects the changelog claim that the old guard messages only named env keys:
they named fields too, it was the remediation advice that pointed at env keys.
Records the human approval of the design, the mutation-testing evidence from
the independent verifier, and the same-class gap it found: 14 checks (redis_url
presence, telemetry paths, enum validity) still live only in from_env, while
client.py asserts they were already validated. Deliberately out of scope here.
Implementation confirmed that no invalid GatewaySettings instance can exist, so
the factory test's exception fires while evaluating the argument rather than
inside from_settings. Recorded so the test is not misread as the factory
carrying its own validation.
iomgaa