Until now the target came from whatever search_path resolved to. The
script printed what it found, but that print and the DELETE happen in
the same run with nobody in between, so it only ever helped the person
who ran a dry-run first. Swap the role that runs it and "$user" can
resolve somewhere else entirely.
--table takes the whole qualified name and resolves it directly. The
table half has to be llm_calls: a version that accepts any name turns
one typo into a general purpose row deleter, and any table with a
created_at and a tenant_id would go through the same batched DELETE
without complaint.
The tests that run it now run as a role that owns its own scratch table
and holds nothing on the shared one, so the row-count snapshot could
go. What replaced it is a case that lets the script fall through to the
shared table on purpose and asserts it exits 2 having deleted nothing.
That one has no red-first path, since making it red means running it as
the superuser, which is the thing being prevented; the finding's probe
covers it instead.
Five of the new usage tests passed before the flag existed, because
argparse rejects an unknown --table with exit 1 and the word --table in
stderr, which is exactly what they asserted. They now also assert the
error is not "unrecognized", which is the difference between testing
the validation and testing argparse.
Seven copies of "create a schema, hang it off search_path, drop it in
teardown" were spread across two files, each with its own cleanup. Any
one of them written wrong leaves the residue on a database shared with
real batch runs. This is one implementation, and it makes "the test
cannot reach the admin connection" a structural fact rather than a note
in a docstring.
Three role modes cover every fixture that exists today: none for plain
schema isolation, owner for the retention script's own runs, grantee
for the least-privilege deployment cases. Owner runs its DDL as itself
so it ends up owning the table; grantee is the opposite, since that
case only means anything when someone else built it.
The schema and the role deliberately get different prefixes. Give them
the same name and "$user" resolves to the sandbox, which hides the
shared table and quietly turns the worst-case test into a test of
nothing.
Writing it also turned up a bug in my first version: rolling back a
failed sandbox unwound the whole stack, so an earlier sandbox in the
same test lost its role mid-use. The test for it fails with a password
authentication error, which is what that looks like from the outside.
Each call now unwinds only what it created, and cleanup tries every
statement before raising, since one failure stranding the rest means
global roles left behind by hand.
The reasoning_tokens docstring was still teaching downstream to treat
None or 0 as no reasoning. The changelog and the schema page had both
been corrected; the docstring had not, and it is the copy that ships in
the wheel and shows up on hover. Someone writing a report from it would
have counted every real MiniMax reasoning call as not reasoning, which
is issue #16 all over again with the tests green.
The original wording stays, since reading pre-1.3.1 rows still needs
it. What follows it now says when it expired and what to read instead.
Two more places had drifted the same way: the changelog and the
architecture doc described the throttle and the cache fallback as they
were before this review, which is to say as the opposite of what the
code now does.
The claim that the two throttle sets would suppress each other does not
survive checking, as the mutation testing showed: their key spaces do
not overlap. Keeping them apart is still right, but for the honest
reason, which is that the two warnings have unrelated lifetimes.
The telemetry field count is taken from inspect.signature, not from
memory, because that is the one the release checklist keeps catching.
llm-calls.md said 22 and was two rounds stale; fixing the title alone
would have left the table contradicting it, so tenant_id and meta are
documented too.
The production template needed no new column — it derives them with
LIKE. What it gained is an assertion that it must keep deriving them
and must not inline a column name, which is the drift that could
actually happen.
The changelog leads with the three breaking items. A patch number
carries no warning by design, so the entry has to.
This issue surfaced only because someone ran a slow suite that is
excluded by default and had not been run for eighteen days. As a column
it becomes a query: which model stopped being observable, and when.
The emitter unwraps the enum to a plain str at the single _record exit.
asyncpg makes no promise about encoding a str subclass, and a telemetry
write that fails is downgraded to one warning — it would not crash, it
would just quietly cost the Postgres path a column. Normalising at the
emitter follows what tenant_id, meta and sampling already do.
The column is appended last in COLUMNS and in both DDLs. An existing
table can only take ALTER at the end, so putting it anywhere else
forks the physical column order between a freshly built database and a
backfilled one.
The min_size=10 default survived to 1.2.4 because every PG test injected a
pool and thus skipped the pool-building path entirely. Unit tests now assert
the create_pool arguments, but "we passed min_size=0" and "the server really
opened that many backends" are two different claims, and only a real instance
can settle the second one. Count via a run-unique application_name carried on
the DSN: the instance is shared with other projects, so counting by database
or role would fold their connections into ours and make the case flaky by
construction.
Degradation is exercised through an unreachable DSN rather than by exhausting
the shared instance's connections. A refused connection lands in the same
class as exhaustion, and the fake clock lets the 60s cooldown be observed
without sleeping. retry_after_s is the signal that separates a real retry
(which renews the window) from the cheap short circuit (which does not).
Evidence: with create_pool reverted to its pre-fix form both cases go red
(observed 10 backends after a single write, and refusal surfacing at pool
creation instead of at prepare time).
The pool exhaustion in issue #15 was fatal only because min_size=10 forced
a transient error to surface at pool creation, and that step was hardcoded
to permanent death. Step is the wrong axis: it conflates "the DSN cannot
be parsed" with "someone else holds all the connections right now".
Failures are now classified by two rules. Fatal means the cause lies
entirely inside this process and cannot change, which only the
construction-time DSN satisfies. Everything else splits on whether the
failure has anything to do with this row's data: row-level failures drop
one row and keep trying, environment-level failures cool down for 60s and
then get exactly one retry, so a restarted database or a DBA creating the
table heals on its own.
42703 (missing column) is the single named exception and stays row-level
even though every row fails alike: issue #13 promised that the manual mode
trims the INSERT and exposes drift per row, and that promise outranks the
rule. Any future exception owes the same argument.
The _failed boolean is gone; the tracker is the only degradation state,
because two copies of the same fact drift apart. Closing stays outside
that state: it is the caller's own decision, not an anomaly to recover
from, so the snapshot reports it through dropped_rows and the drop reason
instead of raising the degraded flag on every clean shutdown.
The pool was the only external resource in the library that pre-allocated:
asyncpg's default min_size=10 turned pool creation into an all-or-nothing
action, so on a shared instance running low on connection budget the first
thing to fall over was the one component that must not fail silently
(4 clients x 10 = 40 idle connections just to write telemetry).
min_size=0 means "do not pre-connect" - asyncpg only builds holders - so
pool creation becomes free and never touches the database; connection
failures then land on acquire, the path that already drops one row and lets
the pool recover. max_size and the write budget become the library's
explicit statement about its own footprint, configurable through two new
keys whose defaults live in config alone (the recorder parameters are
required keyword-only, same discipline as auto_migrate).
The whole write - prepare, acquire, execute - now runs inside one
asyncio.timeout: acquire used to have no timeout at all, so a full pool
would hang forever on the caller's path. Release is explicit rather than
`async with`, because asyncpg shields release and reuses the acquire
timeout, which would let a single telemetry write consume twice the budget.
retry_after_s never had a written definition, so each backend improvised
and they drifted apart. It now answers exactly one question: how long
until a retry is *certainly* worth attempting. OPEN has such a moment
(the cooldown deadline); HALF_OPEN does not, because the probe can come
back at any time -- so it reports 0.0, which already means "retry now"
elsewhere in the library.
Six exits are brought in line. The half-open rejection is the one issue
14 reported: it returned the probe lease remainder, a deadlock-guard
value derived from 2x the slowest timeout, so a 60s cooldown told
callers to wait 600s. Worse, retry.py fed that number into the source
cooldown memo, whose set_until only moves forward -- a source stayed
skipped in-process for the whole lease even after its probe succeeded
and the gate closed. That now writes an already-expired deadline, so
the memo goes back to recording only real OPEN cooldowns.
The other five were pre-existing memory/redis divergences hidden by a
contract-test blind spot (the suite pinned that a second caller gets
rejected, never what number it got): redis reported the probe TTL on
grant and the lease remainder on fenced-out writes, where memory has
always reported 0. Contract cases now pin all four half-open exits on
both backends, with 1:1 real-wait variants for redis since the
fake-clock ones skip there.
The library only ever SELECTs/INSERTs into llm_calls (D15), so expiring
rows has to live outside it — holding DELETE would contradict the
REVOKE UPDATE, DELETE the deployment template recommends.
tools/telemetry_retention.py is dry-run by default and prints the row
count, the created_at window and the tenant_id spread so an operator can
tell whether the rows about to go are the intended ones. The Postgres
branch refuses partitioned targets with exit code 3 (DETACH/DROP
PARTITION is O(1); DELETE is not) and otherwise deletes in per-batch
transactions. Missing asyncpg exits 2 rather than degrading quietly:
this is an ops tool, and a silent "0 rows" reads as "already clean".
Exit codes are the contract with the scheduler, so argparse errors were
moved off 2 (now 1) to keep "bad flags" distinguishable from "cannot
reach the database".
The Postgres cases run against the real instance in throwaway schemas —
never public.llm_calls — and the batch case asserts the shared table's
row count is unchanged, so a search_path that failed to apply lands as a
red test instead of a deletion.
PG requires a partitioned table's unique constraints to include the
partition key, so issue #12's RANGE partitioning on created_at forces
the primary key to (call_id, created_at). The old
`ON CONFLICT (call_id) DO NOTHING` then matches no constraint and PG
rejects every row with
there is no unique or exclusion constraint matching the
ON CONFLICT specification
which the recorder swallows as a per-row warning: telemetry would go
silently dark under a partitioned deployment. The target-free form is
valid on both table shapes and is literally equivalent on a plain table
(the primary key is its only unique constraint). SQLite's
`INSERT OR IGNORE` already carries no target and is untouched.
Integration coverage on the real PG instance, both inside self-created
temp schemas: a plain table still keeps one row per call_id, and a
table partitioned by created_at now accepts writes and reads them back.
The second case was red before this change with the error above.
DDL, column order and backfill statements lived twice, once in each
recorder. A public telemetry_schema_sql() would have made three copies,
and the drift shows up downstream as "I ran the printed SQL and the
library still reports a missing column".
Move both DDLs, both backfill lists and the 24 INSERT fields into
telemetry/schema.py verbatim; the recorders now import them and build
_INSERT through insert_sql(backend, COLUMNS) at import time. The
generated statements are byte-identical to the previous constants, so
runtime behaviour is unchanged (the postgres conflict target stays
bound to call_id for now).
insert_sql() validates its columns against COLUMNS: from the next task
on those names come from database probing, not from a constant, so the
subset check is the gate on the only injection surface. The new
telemetry_schema_sql() prints a paste-ready migration script; its
postgres backfill deliberately uses ADD COLUMN IF NOT EXISTS while the
library's own statements do not, because that form takes an ACCESS
EXCLUSIVE lock even when the column exists. Both variants are derived
from one declaration list so their column sets cannot drift.
The cache-key test only asserted a hit, so a key degraded to a constant
would still pass it. Adding a namespace control group that must miss
proves the key still distinguishes inputs; verified by degrading
build_cache_key to a constant and watching the case go red.
The allow_nan=False branch had no test at all. A ChatRequest built with
a nan meta value (bypassing the entry validation, i.e. a future entry
point that forgets to validate) must drop the row and not raise;
verified red by removing allow_nan=False.
Also restore the read-only file permissions in a finally block, so a
failing assertion does not get masked by a PermissionError from tmp_path
cleanup; rename the warnings fixture to captured_warnings so it stops
shadowing the stdlib module; and drop a downstream business term from a
fixture value (zero-business-assumption rule).
Both telemetry backends gain tenant_id and meta at the end of the
column list, and TelemetryEmitter fills them from the request. The two
halves ship together because the emitter is the only caller of
record_llm_call: adding the columns without filling them leaves every
row short of two keys, and the backends read those keys outside their
try block, so the KeyError degrades to a warning and the whole table
stops filling.
The columns are appended, never inserted. An old table can only gain
columns through ALTER, which puts them last; a new table built from the
DDL would put them wherever the DDL says. Anywhere but the end and the
two paths produce different physical column orders, while the INSERT
uses positional placeholders.
The two backends spell the default differently for different reasons.
SQLite refuses a NOT NULL column without a non-NULL constant default
outright, so the default is what makes the backfill legal at all. On
Postgres a non-volatile constant default is what keeps the ALTER from
rewriting the table, and NOT NULL DEFAULT '' is what keeps old rows out
of the black hole a NULL tenant_id falls into under an RLS policy.
Normalisation happens in the emitter, not the recorder, matching how
canonical_sampling_json already settles the sampling column: None
becomes the empty string, an empty mapping becomes the literal '{}'.
Keys are sorted so one set of dimensions serialises identically on
every row, and allow_nan=False is a second gate behind the entry
validation -- json.dumps would otherwise write a bare NaN, which JSONB
rejects, and the failed insert would be swallowed as a warning.
All three emit entry points read the request. Cache hits read it too,
rather than the replayed response: the dimensions answer who made this
call, not who made the one whose result is being replayed.
Issue #10 Task 5, the acceptance claim. Before the fix this asserted
against 'qwen_1 请求被拒: 400' and failed on the first substring - which
is exactly what the downstream batch was left with. Uses the real body
from the issue, and checks the trailing code too, since a head-only cut
would drop the one field you quote when chasing the provider.
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.
A fail-closed limiter or breaker backend means the scope cannot emit a
single request, which is exactly scope-level unavailability. But the error
sat directly under PolyGatewayError, so a caller writing only
`except GatewayUnavailableError` dropped it into the catch-all branch:
Redis blips once and a backlog of tasks burns its business failure budget
into the dead letter queue, over a fault a restart would clear.
Three gate paths leak to callers rather than being absorbed by
_record_quietly (try_acquire, try_enter, progress_age_s); each is now
pinned by a test, since none of them had one before.
The two unknown-source sites move to SourceNotConfiguredError instead of
following along. They report a misconfigured source name, not an outage,
and letting them into the retryable family would be the mirror of the bug
being fixed here: the task would retry forever and never surface.
Reasoning tokens are already counted inside completion_tokens, so the
cost total was never wrong -- what was missing is the attribution: how
much of a call was spent thinking rather than answering.
LLMResponse and TransportResult each gain a trailing reasoning_tokens
field, and the telemetry port grows from 21 to 22 columns with the new
column appended in both backends so fresh and migrated schemas keep the
same physical order.
None means this particular call did not report the field, not that the
source never reports it: a relay that falls back to a local tokenizer
replaces the whole usage object and drops completion_tokens_details.
Downstream checks must therefore read "in (None, 0)"; no provider was
observed reporting a literal zero.
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.
httpx reads the macOS system proxy config (not just env vars) when
trust_env is on, and the proxy answers 403 for the LAN service at
10.77.0.20. The raw-httpx case in this file already passed
trust_env=False; the OcrClient cases relied on the default and failed on
any machine with a system proxy enabled.
The fixture used to DROP the shared llm_calls table on every run, wiping
concurrent migration-batch telemetry (and its own count assertion was
polluted in return). Assertions now filter by a per-run call_id prefix
and teardown deletes only its own rows.
Dual-channel opening: consecutive failures (CHS-compatible, no streak
bump) plus windowed failure rate (two 30s buckets, min_calls guard).
429s bypass both channels as backpressure, and a probe hitting 429
releases instead of holding the lease. Open duration doubles per
rate/probe reopen up to max_cooldown_s, decaying after stable CLOSED.
record_success gains count_attempt so bad-result successes stay out of
the window.
Classify empty completions as transient per human ruling (fixes flaky
real-gateway smoke and prevents caching empty responses), rename the
factory injection parameter gate to breaker per the frozen design,
rewrite the probe-entry cleanup without except BaseException, declare
python-dotenv explicitly, add a mid-backoff cancellation test, and
record all implementation errata in the design and architecture docs.
iomgaa