test(platform-scripts): cover hexgate.messages in the OTLP smoke check - #192
Draft
victorludvig wants to merge 14 commits into
Draft
victorludvig wants to merge 14 commits into
victorludvig wants to merge 14 commits into
Conversation
This was referenced Sep 8, 2026
victorludvig
changed the title
Placeholder commit opening PR 6/15 of the LLM message logging stack; the change lands on this branch.
Add GET /v1/projects/{project_id}/audit/llm-messages?session_id=…, the
session-scoped transcript behind the Audit drawer: list_llm_messages()
ordered (occurred_at, message_seq, event_id) — the storage sort key after
project/session — plus LlmMessageRow / LlmMessagePage.
The session is the whole scope: no window or date-range parameter, unlike
the other project-scoped reads. scope_filters always emits a time predicate,
so the read passes the new RETENTION_HOURS floor. A dashboard window could
only cut the head off a conversation, and the cut is unreadable — the
surviving rows begin at a nonzero message_seq, which schema.sql tells a
reader means the pipeline lost a row. Long transcripts page instead.
_decode_json_column moves from features/audit/service.py to
core/clickhouse.py so both read paths share one decoder.
… alone session_id is caller-supplied and most SDK users never set it: HexgateContext leaves it None, emit_llm_messages folds that to "", and the column defaults to "". Requiring it left every transcript from an unnamed session stored and unreadable for its whole 180-day TTL — the common path, not an edge case. list_llm_messages now takes session_id, run_id, or both, and raises NoMessageScope (422) when neither names a transcript. A zero run_id is not a scope: it is the column's "outside any run" value, shared by every unattributed row in the project. Both present narrows to the intersection. list_decisions returns run_id alongside session_id so the detail drawer has the fallback scope to pass; zero reads back as null, as on the message rows.
Half of it restated schema.sql's own comment on the sort key and the non-FINAL retry window already documented on insert_llm_messages_batch. Keeps what the code and schema cannot say: the scope rule, why there are two scopes, and why there is no window.
…ckHouse The mocked read tests assert on the SQL text and never execute it, so a wrong column name or a broken ORDER BY passes all 37 of them. This inserts through the real batch path and reads back through list_llm_messages: both scopes, the session-less row that only run_id reaches, JSON decoding, paging and the past-the-end count fallback.
Placeholder commit opening PR 10/15 of the LLM message logging stack; the change lands on this branch.
on_llm_start stashes (system_prompt, input_items) per turn key (id(context) + agent name, so a handoff's own list is tracked separately); on_llm_end converts the Responses-API items into the OTel GenAI role/parts shape, asks MessageCursor what is new, and emits usage and messages from one call site. Only the delta reaches the wire — the input list is the whole conversation so far — and tool results ride with it, since a policy_decision row records a tool call but never its return value. HEXGATE_LOG_MESSAGES=0 skips the stash and the conversion, not only the emit, so an opted-out process pays nothing.
Review fixes on the OpenAI message hooks: - turn_key is Hexgate's run id, not id(context). The run context is freed at run end and CPython hands the next run the same address, so a process serving many runs filed unrelated conversations under one turn_key, each restarting message_seq at 0 — silently, since the rows still insert. - _output_messages routed reasoning items through the content branch, whose value is None on them, emitting an empty message. Their text lives under summary; the last turn's reasoning was recorded nowhere. - The cursor is advanced only once the conversion has succeeded, so a failed conversion no longer spends a seq on an event that never goes out. - TOOL_CALL_JSON_KEYS gains "response": frameworks stringify a tool's return value, so a tool that serialises its own result landed a JSON string whose secrets redaction never opened — the claim already in tracing/messages.py that tool results get the arguments rule. - log_messages_enabled is public (one name, not an alias) so a hook can skip stashing and converting, not just emitting.
The existing case proves the rows reach ClickHouse; this one proves they
come back out of GET /v1/projects/{id}/audit/llm-messages — a wrong column
name or a broken ORDER BY in that query is invisible to a direct
ClickHouse read.
Scoped by run_id rather than session_id: the run is the stronger key, and
it is the scope that exists for the common SDK caller who never sets a
session. project_id comes off the api key's own envelope, so the tests
need no second source of truth and no import from the platform package.
The read endpoints are cookie-authed dashboard reads, so the fty_live_ key
the SDK exports with does not open them. require_dashboard_login skips on
the HEXGATE_SMOKE_* pair the OTLP smoke script already reads, keeping the
default integration run — infra plus one API key — working unchanged.
adapters/openai/messages.py takes the Responses-API → GenAI shape translation; usage.py keeps the RunHooks pair, the turn key and the model resolution both events share. 341 lines become 202 + 161. The hooks stay together on purpose — on_llm_end is the one callback carrying both the token counts and the completion, and both events take the same _resolve_model(agent), so splitting those would duplicate it and let the two disagree about the model for one call. The converters share none of that: they are pure functions over plain dicts that decide nothing about when or whether to emit. Done now because PRs 11-13 each need the same layer against their own framework's message types, and whatever this PR does is the pattern they copy. Tests split the same way: test_messages.py for the conversion cases, test_usage.py for the hooks.
_Prompt (NamedTuple) replaces tuple[str | None, list[Any]] for the on_llm_start stash, and list[TResponseInputItem] replaces list[Any] on both the stash and the hook signature — the SDK's own type, so Any is now gone from usage.py entirely. Comment sweep in the same pass: six blocks that justified an *absence* (why no on_agent_end reset, why no json.loads on arguments, why convert before advancing) were costing a reader more than they saved, since there is no code to anchor them to. Kept where someone would plausibly try the alternative and break something; cut to a line otherwise. The handoff keying trade-off moves to issue #215, which is where a decision still in flight belongs. usage.py goes 47% comment lines to 38%, in line with the rest of the package rather than above it.
Placeholder commit opening PR 7/15 of the LLM message logging stack; the change lands on this branch.
victorludvig
force-pushed
the
vl/test/otlp_smoke_messages
branch
from
September 14, 2026 12:54
19ca2d2 to
b90490c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Adds six hexgate.messages events to the smoke run and reads them back through the session-scoped llm-messages endpoint, asserting turn_key, message_seq and the truncated flag. Five of them carry an input message past the SDK's 256 KiB cap. That is what makes the run a check on the deployment rather than on the code: a single capped span is ~256 KiB and rides the 1 MB record the exporter and the broker default to, so five is the smallest number that overflows it and fails on a stage whose topics were never altered and whose collector and enricher were never restarted.
Review follow-ups on the oversized message events. The record has to clear two different defaults, not one: the exporter's 1,000,000 bytes and the broker's 1 MiB. The count was derived from the smaller, and the comment claimed they were the same number. Deriving it from 1 MiB, where the cap divides evenly, makes the +1 the span that actually crosses the line rather than one of two unexplained spares; the count stays five. Assert the landed content size, not just the truncated flag: that flag is the SDK's cut OR'd with the enricher's, so a row clipped to nothing by a hop in between still carries it and passed every check. An all-MISSING result now has a second cause worth naming — ~1.3 MB goes out as one uncompressed POST inside the exporter's 5 s deadline, so a slow uplink drops the batch locally and looks exactly like an undeployed stage. DEPLOY.md says which traceback tells them apart.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is Changing
PR 7/15 of the LLM message logging stack (§IV.4 of the implementation spec).
platform/scripts/otlp_smoke.pynow sends sixhexgate.messageseventsalongside the five it already sent, and reads them back through the PR 6
endpoint (
GET /v1/projects/{id}/audit/llm-messages?session_id=…). Per row itasserts the row landed, that
turn_keyandmessage_seqround-tripped, thattruncatedis set on exactly the events that were over the cap, and that theoversized rows came back at full size rather than clipped.
Five of the six carry an input message past the SDK's 256 KiB cap. The spec
called for one ~40 KiB event, but that number was written against the 32 KiB
input cap that was later rejected in favour of 256 KiB — at the accepted cap a
40 KiB message is not truncated at all, and a single capped span (~256 KiB)
rides both record limits it would have to cross. The record has to clear two
different defaults: the kafka exporter's 1,000,000 bytes and the broker's
1 MiB. Four spans land exactly on the 1 MiB; five clear both (~1.25 MiB),
derived in the script from
MAX_INPUT_MESSAGES_BYTESrather than hardcoded.Docs updated to match:
platform/DEPLOY.md§4 anddocs/internals/audit-pipeline.md§4.1, which until now carried a "checked by hand" note for this gap.
Why is this change necessary?
Raising the pipeline's record-size limits (PR 5) is an operational change:
merged is not enough, the topics have to be altered and the collector and
enricher restarted. Nothing verified that had happened — the old smoke run's
five events fit a sub-1-MiB export on any stage, deployed or not, so it passed
either way.
With five oversized spans it does not. On a stage still at the defaults the
record is rejected whole and every event reports
MISSING; on a raised stagethe same 1.25 MiB sits inside the 8 MiB record. The run is the post-deploy
check on staging and the release gate on prod, so this is where that has to be
caught.
It is also the only check anywhere that a payload past the SDK cap is
degraded rather than dropped — the whole design of capping head+tail instead
of rejecting.
Tests
make check-all: SDK 2672 passed, platform-api 796 passed, ruff clean.dashboard-lintaborts on a broken local Node install (libsimdutfmissing)— unrelated, and this PR touches no dashboard files.
_message_fields):the ordinary row lands
truncated=False, each oversized rowtruncated=Trueat exactly 262144 bytes,
turn_keyandmessage_seq(0–5, contiguous) intact.the 1,000,000-byte exporter default and the 1 MiB broker default, and well
inside the deployed 8 MiB.
from the larger of the two limits; the landed content size is asserted (the
truncatedflag alone passes on a gutted row); andDEPLOY.mdnames thesecond cause of an all-
MISSINGrun — a ~1.3 MB uncompressed POST againstthe exporter's 5 s deadline drops the batch locally below ~2 Mbit/s and looks
identical to an undeployed stage.
and prod as the release gate.
🤖 Generated with Claude Code