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.
`PGW_TELEMETRY_TEXT_CAP` now reaches the emitter on every assembly path.
Unset means no truncation, which stays the default: a truncated row is
no longer audit evidence and cannot be replayed, and downstreams rely on
that today. The flip side — contracts and bids sitting in `llm_calls`
indefinitely, multi-tenant — is spelled out in `.env.example` so readers
can weigh both.
All three `from_settings` paths are wired (chat, embedding, OCR): they
write the same table, so capping only chat would leave half of it
uncontrolled. `TelemetryEmitter.__init__` now rejects `text_cap <= 0`;
it is the single point where the three clients converge, so the direct
construction path — a public assembly route the settings guard never
sees — is covered too. `0` would otherwise reduce every body to a bare
elision marker.
Chat rows stored full message and response text with no upper bound, so
downstream contracts and tenders lived in llm_calls indefinitely. Add
_cap_text/_cap_messages in the single telemetry exit (_record), applied
after digest_messages and before json.dumps, plus to response/thinking.
Capping is per text, not over the serialized JSON: cutting the whole
string would emit invalid JSON into an unvalidated TEXT column. The cap
builds new dicts and never mutates in place — digest_messages passes
non-list content straight through as the same object, so an in-place cut
would silently poison the caller's messages and the cache key.
text_cap is required on TelemetryEmitter (internal class, three known
construction sites) and defaults to None on the three public clients, so
the default behaviour stays byte-for-byte identical. Settings wiring
lands separately.
issue #13: the library no longer alters a downstream Postgres table on
its own. PGW_TELEMETRY_SCHEMA_MODE is tri-state and defaults by backend
-- SQLite keeps auto-migrating a local file, Postgres switches to manual,
where a stale table gets a named warning with runnable SQL and the INSERT
is trimmed to the columns that exist rather than dropping every row.
Schema constants now live in telemetry/schema.py so the SQL the library
prints cannot drift from the DDL it runs, and telemetry_schema_sql is
exported for downstreams writing their own migrations. The PG write drops
its conflict target, which partitioned tables require and which issue #12
depends on.
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.
Not related to the schema work. These three fail ruff format --check on
main as well -- the pinned ruff is newer than whatever last formatted
them -- and a red make check makes the per-task quality gate useless for
everything that follows.
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 plan review caught three mistakes that would have gone red in the
tests rather than in the implementation. Column counts: COLUMNS is the
insert field list and excludes the database-filled created_at, so a
stale table has 23 physical columns and a current one 25, not 22 and 24.
Warning capture: the library logs through loguru, which never reaches
caplog, so that assertion would have passed forever without seeing a
single line. And the stale-table-under-least-privilege fixture is
least_privilege_pre_tenant_dsn -- the other one builds a complete table
and never reaches the missing-column path at all.
Three more: make lint rewrites files, so verification uses make check;
the recorder signature change now ships with its only call site instead
of leaving a TypeError between two commits; and the backfill statements
the library runs are not the ones it prints -- the library probes first
to dodge the exclusive lock, while a script handed to a DBA has to carry
IF NOT EXISTS or it cannot be run twice.
On the cap side, all three clients build their emitter inside __init__,
so a required parameter there would strand anyone constructing a client
directly. The emitter stays required, the clients take a defaulted one.
Twelve tasks across the two plans, each with the files it touches, the
evidence it has to produce, and the command that proves it. #13 goes
first: both branches edit config.py and client.py, and #12's
partitioning template leans on the schema SQL helper and the untargeted
conflict clause that #13 introduces.
Writing the cap plan surfaced a trap worth its own guard. digest_messages
appends the very same dict when a message's content is not a list, so
the telemetry copy, the caller's messages and the cache key all share
one object -- capping in place would poison the caller's request and the
cache key at once, silently. Two red-line tests now pin that down, and
the plan asks for an in-place version to be written and run first, to
prove the tests actually catch it.
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.
Dating the changelog and bumping both version strings is the cheap half.
The README is the half that gets frozen into the sdist, so it is fixed
first: the install pin now names 1.2.1 (1.2.0 has no tenant dimension),
and the per-source FIELD table finally lists MISSING_DONE and EXTRA_BODY
- the latter was already referenced elsewhere in the same file. The same
table was missing QUOTA_FULL, the embedding-only keys, the memory cache
backend and three optional PGW_* keys; all are reconciled against
_SOURCE_FIELDS and _load_pgw rather than from memory.
Issue #11: a multi-tenant caller could not isolate its rows in the
telemetry table, because llm_calls carried no tenant dimension at all --
only session_id and parent_call_id, both free-form strings the library
never validates. The table stores full message bodies, so several
tenants' contracts sat in one table with no way to filter by owner.
tenant_id is a real column rather than a key inside JSON, for two
independent reasons found during research. 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.
Separately, the planner has no usable JSONB statistics at all. Both
degrade unpredictably at real data volumes, and neither reports an
error.
Anything else the caller wants to attach goes into meta, a JSON column
with no index -- the same split LiteLLM, Loki, and six LLM observability
platforms arrived at independently.
The library stops at the column plus a documented policy template. It
never enables RLS itself: with no matching policy that is default-deny,
which would have silently failed every telemetry write for the two
downstreams that are not multi-tenant.
Old rows read back as the empty string rather than NULL. Under an RLS
policy NULL is invisible to everyone, which is not what "unassigned"
should mean.
Covers all three telemetry paths -- chat, embed, and OCR. The last was
not in the issue, but OCR rows land in the same table and the same
irreversibility argument applies to them.
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.
The CHANGELOG pointed at research-wiki for the RLS template and its
three traps, but setuptools has no MANIFEST.in here: the sdist carries
src/polygateway and the README only. A downstream pip install could not
reach any of it. The template and the traps now live in the README
section on multi-tenancy, and the CHANGELOG points there.
ARCHITECTURE.md is the single source of truth for architecture, and this
change had added nothing to it. Section 5.2 gains an entry in the same
shape as the issue #4 overlay one, and 7.8's field list gains tenant_id
and meta -- plus reasoning_tokens, which issue #6 had already left out,
so the port's field-count chain reads 18 to 20 to 21 to 22 to 24 with no
gaps.
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).
The key-count cap exists to catch a whole request body dumped into meta.
That is exactly the shape where the per-key regex runs tens of thousands
of times before the real reason surfaces, so the cheap check goes first.
Also correct two stale docstrings: postgres.py still claimed 22 columns
(it is 24), and _canonical_meta_json promised to raise on non-finite
floats. It is evaluated inside _record's degradation try, so the real
outcome is a warning plus a dropped row -- never an error the caller
sees. What the gate actually buys us is the SQLite side, whose meta is a
TEXT column that would happily store a literal NaN.
Issue #11 Task 8, repo files only (the Gitea wiki pages are handled
separately at merge time). CHANGELOG gains an unreleased section covering
the two new keyword-only parameters on all four public methods, the two
new telemetry columns (JSONB on PG, TEXT on SQLite), why backfilled rows
read as an empty string rather than NULL, the validation limits, and the
boundary that the library ships columns only - no index, no RLS.
Telemetry field count re-measured via inspect.signature: 22 -> 24, README
updated accordingly. The cache-key row also dropped the sampling
component and called the namespace a tenant, which now reads as the new
tenant_id; both corrected.
OcrClient is the third telemetry path that skips the chat onion: _emit
builds its own ChatRequest purely to reuse the shared TelemetryEmitter,
so wiring chat() and embed() alone left every OCR row without a tenant
while those rows land in the same llm_calls table. Take the dimensions
at both public entries, validate them there (anything failing further
down is degraded to a warning), and thread them through _call ->
_attempt -> _emit so success, rejection, cancellation and retryable
failure rows all carry the same pair.
EmbeddingClient does not go through the chat onion: it builds its own
ChatRequest inside _emit purely to reuse the shared TelemetryEmitter, so
wiring chat() alone left every embed row without a tenant. Validate the
dimensions at the embed() entry (before batching, since anything failing
further down is degraded to a warning) and thread them through
_embed_batch -> _attempt -> _emit so every batch row carries the same
pair.
Validation runs before the request enters the onion: every failure inside it
is downgraded to a warning by the telemetry layer, so validating in there
would not validate anything.
The dimensions stay out of the cache key — cache_namespace already carries
tenant isolation, and folding meta in would cold-start every existing entry.
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.
Adds validate_caller_dimensions and the two ChatRequest fields that
carry them. Every limit rejects rather than trims: Langfuse drops
metadata values past 200 characters, which leaves the caller believing
something was recorded when nothing was.
Whitespace on tenant_id is refused outright instead of stripped. " t1"
and "t1" compare unequal inside an RLS policy, so silently rewriting the
caller's value would hand them a tenant whose rows they cannot find.
Non-finite floats are refused for a concrete reason: json.dumps writes
them as the bare literals NaN and Infinity, which are not valid JSON and
which JSONB rejects. Letting one through turns a caller's input mistake
into a failed insert, and the telemetry layer degrades failed inserts to
a warning -- so the mistake would surface as missing rows, nowhere else.
The new fields go after sampling so no positional construction of
ChatRequest shifts. Validation is split across three helpers to keep
each one under the complexity gate.
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.
The tenant_id rule was wrong in a way that would have shipped: the plan
said reject when strip() is empty, but the design says reject leading and
trailing whitespace outright. " t1" survives the weaker rule and then
compares unequal to "t1" inside an RLS policy, so a caller who pads the
value silently loses rows.
Adds the test that guards a promise nothing else was guarding -- same
messages and namespace with different meta must still hit the cache.
Without it, folding meta into the key passes every other assertion and
costs a full cache cold start plus a permanently lower hit rate, which
degrades quietly instead of failing.
Also pins _record's new parameter positions, splits the backfill-failure
setup per backend (ownership check on PG, read-only file on SQLite, and
says what SQLite cannot assert), puts the red-green gate on the
integration task, and names the two wiki pages.
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.
Measured 201 on POST /api/v1/packages/iomgaa/pypi/polygateway/-/link/
PolyGateway during the 1.2.0 release. The note saying it 404s and must
be done through the web UI would have sent the next release down a
manual path that is no longer needed.
Verifier mutation test: flipping _translate_429 to parse the summary
left all 824 tests green. The padding was one long string value, so the
cut landed inside it - and head-and-tail retention kept the trailing
error object, leaving the summary parseable. Many keys put the cut
between structural tokens, where the summary stops being valid JSON.
Mutation now fails as it should. Also splits OCR 429 out on its own.
Issue #10 Task 6. The install pin moves from ==1.1.* to >=1.2,<2 - left
alone, everyone following the README would have stayed silently on
1.1.2 without this fix and without a warning. Telemetry field count
re-measured via inspect.signature: still 22.
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.
Issue #10 Task 4: the OCR side said only 'HTTP 404'. The issue reported
the chat path, but the batch that lost its 400 was reading tables - the
same blind spot, one transport over. Reuses the shared summarizer.
Issue #10 Task 3: five branches each built their own message, so adding
the summary would have meant five copies. Table-driven classification
composes it in one place instead, and the 429 split still parses the
untruncated body - reading the summary would demote an oversized
insufficient_quota to a plain rate limit and stop force_open.
Issue #10 Task 2: head-and-tail rather than a head-only cut, because the
code and request_id that let you chase the provider sit at the very end
of a JSON error body. Cap 2048 follows k8s client-go for the same job.
Issue #10 Task 1: a rejected call's reason had nowhere to live. The
field goes on the base class because these errors all come from one HTTP
response - which class it is and what the peer said are orthogonal.
Codex review: the code blocks reference httpx and PolyGatewayError, but
neither module imports them today. A zero-context implementer copying
them verbatim would stall on F821.
iomgaa