feat(sdk): capture OpenAI Agents messages in HexgateUsageHooks by victorludvig · Pull Request #191 · HexamindOrganisation/hexgate

victorludvig · 2026-09-08T15:34:22Z

Design: LLM message logging design · implementation spec. Plan target: merge by Mon 14 Sep.

Stacked on #190 (vl/feat/llm_messages_read), not main — the read endpoint is what the second integration case verifies through.

What is changing

adapters/openai/usage.py: on_llm_start stashes (system_prompt, input_items) per turn_key; on_llm_end emits usage and messages from one call site, then clears the stash. The runner already forwards on_llm_start. The Responses-API → gen_ai.* shape translation lives in the new adapters/openai/messages.py, so each of PRs 11–13 gets the same layer for its own framework.

turn_key is f"{run_id}:{agent.name}" — Hexgate's run id, not id(context): that address is reused once the run context is freed, so two unrelated runs in one process would share a key and both restart message_seq at 0.

TOOL_CALL_JSON_KEYS gains "response": frameworks stringify a tool's return value, so a tool serialising its own result landed a JSON string whose secrets redaction never opened.

Why is this change necessary

The response hook carries no prompt, so the pair is needed. 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.

Tests

tests/adapters/openai/test_usage.py (hooks) and test_messages.py (conversion); tests/audit/test_message_caps.py for the new redaction key. Two integration cases: one asserting the rows concatenate back to the conversation with contiguous message_seq, one reading the same transcript back through #190's GET /v1/projects/{id}/audit/llm-messages (cookie-authed, so it skips without HEXGATE_SMOKE_EMAIL/_PASSWORD).

Run locally against the full stack — Postgres, ClickHouse, Redpanda, platform-api, Collector, span-enricher:

Suite	Result
`ruff check` + `format --check`	clean (hexgate, tests)
SDK unit	2672 passed, 15 skipped
platform-api unit	796 passed, 10 skipped
platform-api integration	8 passed
SDK integration (full OTLP path)	7 passed

Known and tracked, not fixed here: #215 (should a handoff continue one transcript or start a new one — the handoff integration case lands with that decision) and #216 (Agent.as_tool nested runs get no hooks, so no usage or message rows).

🤖 Generated with Claude Code

codecov · 2026-09-14T09:59:17Z

Codecov Report

❌ Patch coverage is 88.03419% with 14 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
hexgate/adapters/openai/messages.py	75.43%	7 Missing and 7 partials ⚠️

📢 Thoughts on this report? Let us know!

guillaume-hexamind · 2026-09-15T12:26:15Z

Review — 2 issues

Verified against the branch tip, hexgate/tracing/messages.py / semconv.py, and openai-agents==0.15.1. Score is severity out of 100.

1. `resynced` is `true` on every row under a server-managed conversation — 55/100

usage.py:115 documents input_items as "the whole conversation, not a delta". That only holds when no OpenAIServerConversationTracker is in play.

HexgateRunner.run/run_sync/run_streamed forward **kwargs to Runner.run, so conversation_id= / previous_response_id= / auto_previous_response_id= reach agents/run.py:542 and build a tracker. From then on run_loop.py:1348 builds the prompt with tracker.prepare_input(...), which returns only the un-sent items — oai_conversation.py:421-441 sends the initial input once and mark_input_as_sent clears remaining_initial_input.

MessageCursor stores count=1, fingerprint="1:digest(user,user)" after turn 1; turn 2 hands it a short list starting with a tool output, the prefix fingerprint can never match, and advance returns resynced=True — on every turn, for the life of the run.

The content stays correct (the short list is the delta), so nothing is lost or duplicated. The flag is what's wrong: semconv.py:46 defines RESYNCED as "restates the whole list", so a reader honouring that drops every earlier row and renders a two-message conversation. Latent today only because nothing consumes the column yet.

Same cause, minor: run_loop.py:1387 permits an empty filtered.input when a tracker is present, and MessageCursor._diff([]) stores _TurnState(0, _UNMATCHABLE_FINGERPRINT, 0). Harmless here, since a resync is already the steady state in this mode.

Suggested fix — the hook can't see the tracker, but the runner sees the kwargs that create it. Pass the mode down and skip the diff when the framework is already sending deltas:

# usage.py
_SERVER_CONVERSATION_KWARGS = frozenset(
    {"conversation_id", "previous_response_id", "auto_previous_response_id"}
)

class HexgateUsageHooks(RunHooks):
    def __init__(self, *, api_key: str, framework_sends_deltas: bool = False) -> None:
        ...
        self._framework_sends_deltas = framework_sends_deltas

In _emit_messages, when self._framework_sends_deltas, emit new_input as-is with resynced=False and take only the seq from the cursor (a MessageCursor.advance_delta(key, count)-style entry point, or a counter here), instead of diffing a list that was never the full history.

_merge_hooks already has run_config and kwargs in scope at all four call sites (run, run_sync, run_streamed/arun_streamed via _launch_streamed), so it becomes:

def _merge_hooks(self, hooks: RunHooks | None, *, kwargs: dict[str, Any]) -> RunHooks:
    installed: list[RunHooksBase] = [
        HexgateUsageHooks(
            api_key=self.api_key,
            framework_sends_deltas=any(
                kwargs.get(name) for name in _SERVER_CONVERSATION_KWARGS
            ),
        ),
        _HexgateReachHooks(self),
    ]

2. `_resolve_model` ignores `RunConfig.model` — 50/100

agents/run_internal/turn_preparation.py:123-132 get_model gives run_config.model precedence over agent.model, for both the Model and the str form:

def get_model(agent: Agent[Any], run_config: RunConfig) -> Model:
    if isinstance(run_config.model, Model):
        return run_config.model
    elif isinstance(run_config.model, str):
        return run_config.model_provider.get_model(run_config.model)
    elif isinstance(agent.model, Model):
        return agent.model
    return run_config.model_provider.get_model(agent.model)

usage.py:60 _resolve_model(agent) reads only agent.model, and runner.py:274/298 accepts and forwards run_config. So HexgateRunner.run(agent, ..., run_config=RunConfig(model="gpt-4o-mini")) against an Agent(model="gpt-4o") records gpt-4o — now in llm_message as well as llm_usage, so this PR doubles the blast radius of a pre-existing bug.

RunContextWrapper (run_context.py:43-62) carries context, usage, turn_input, _approvals, tool_input and no run_config, so the hook can't recover this on its own.

Suggested fix — inject run_config alongside the flag above and mirror get_model's precedence:

def _resolve_model(agent: Agent, run_config: RunConfig | None) -> str:
    configured = run_config.model if run_config is not None else None
    model = configured if configured is not None else agent.model
    if isinstance(model, str):
        return model
    if model is None:
        return "default"
    return getattr(model, "model", None) or type(model).__name__

with HexgateUsageHooks.__init__ taking run_config: RunConfig | None = None and on_llm_end calling _resolve_model(agent, self._run_config). The "default" placeholder for an unset model stays as documented.

guillaume-hexamind

2 comments above; the logic look good otherwise

victorludvig · 2026-09-15T13:34:59Z

Review — 2 issues

Verified against the branch tip, hexgate/tracing/messages.py / semconv.py, and openai-agents==0.15.1. Score is severity out of 100.

1. resynced is true on every row under a server-managed conversation — 55/100

usage.py:115 documents input_items as "the whole conversation, not a delta". That only holds when no OpenAIServerConversationTracker is in play.

HexgateRunner.run/run_sync/run_streamed forward **kwargs to Runner.run, so conversation_id= / previous_response_id= / auto_previous_response_id= reach agents/run.py:542 and build a tracker. From then on run_loop.py:1348 builds the prompt with tracker.prepare_input(...), which returns only the un-sent items — oai_conversation.py:421-441 sends the initial input once and mark_input_as_sent clears remaining_initial_input.

MessageCursor stores count=1, fingerprint="1:digest(user,user)" after turn 1; turn 2 hands it a short list starting with a tool output, the prefix fingerprint can never match, and advance returns resynced=True — on every turn, for the life of the run.

The content stays correct (the short list is the delta), so nothing is lost or duplicated. The flag is what's wrong: semconv.py:46 defines RESYNCED as "restates the whole list", so a reader honouring that drops every earlier row and renders a two-message conversation. Latent today only because nothing consumes the column yet.

Same cause, minor: run_loop.py:1387 permits an empty filtered.input when a tracker is present, and MessageCursor._diff([]) stores _TurnState(0, _UNMATCHABLE_FINGERPRINT, 0). Harmless here, since a resync is already the steady state in this mode.

Suggested fix — the hook can't see the tracker, but the runner sees the kwargs that create it. Pass the mode down and skip the diff when the framework is already sending deltas:
# usage.py
_SERVER_CONVERSATION_KWARGS = frozenset(
    {"conversation_id", "previous_response_id", "auto_previous_response_id"}
)

class HexgateUsageHooks(RunHooks):
    def __init__(self, *, api_key: str, framework_sends_deltas: bool = False) -> None:
        ...
        self._framework_sends_deltas = framework_sends_deltas
In _emit_messages, when self._framework_sends_deltas, emit new_input as-is with resynced=False and take only the seq from the cursor (a MessageCursor.advance_delta(key, count)-style entry point, or a counter here), instead of diffing a list that was never the full history.

_merge_hooks already has run_config and kwargs in scope at all four call sites (run, run_sync, run_streamed/arun_streamed via _launch_streamed), so it becomes:
def _merge_hooks(self, hooks: RunHooks | None, *, kwargs: dict[str, Any]) -> RunHooks:
    installed: list[RunHooksBase] = [
        HexgateUsageHooks(
            api_key=self.api_key,
            framework_sends_deltas=any(
                kwargs.get(name) for name in _SERVER_CONVERSATION_KWARGS
            ),
        ),
        _HexgateReachHooks(self),
    ]
2. _resolve_model ignores RunConfig.model — 50/100

agents/run_internal/turn_preparation.py:123-132 get_model gives run_config.model precedence over agent.model, for both the Model and the str form:
def get_model(agent: Agent[Any], run_config: RunConfig) -> Model:
    if isinstance(run_config.model, Model):
        return run_config.model
    elif isinstance(run_config.model, str):
        return run_config.model_provider.get_model(run_config.model)
    elif isinstance(agent.model, Model):
        return agent.model
    return run_config.model_provider.get_model(agent.model)
usage.py:60 _resolve_model(agent) reads only agent.model, and runner.py:274/298 accepts and forwards run_config. So HexgateRunner.run(agent, ..., run_config=RunConfig(model="gpt-4o-mini")) against an Agent(model="gpt-4o") records gpt-4o — now in llm_message as well as llm_usage, so this PR doubles the blast radius of a pre-existing bug.

RunContextWrapper (run_context.py:43-62) carries context, usage, turn_input, _approvals, tool_input and no run_config, so the hook can't recover this on its own.

Suggested fix — inject run_config alongside the flag above and mirror get_model's precedence:
def _resolve_model(agent: Agent, run_config: RunConfig | None) -> str:
    configured = run_config.model if run_config is not None else None
    model = configured if configured is not None else agent.model
    if isinstance(model, str):
        return model
    if model is None:
        return "default"
    return getattr(model, "model", None) or type(model).__name__
with HexgateUsageHooks.__init__ taking run_config: RunConfig | None = None and on_llm_end calling _resolve_model(agent, self._run_config). The "default" placeholder for an unset model stays as documented.

Both fixed in 02afcb61 and b3bbfaaa, implemented as suggested — kept resynced=False in delta mode rather than adding a third state, since a reader concatenating rows gets the right transcript either way.

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.

The rebase onto #190 brought in 50f5725/3d985373: session_id is the second column of the storage sort key, so the endpoint now asks callers to send it even blank — pinning it lets the scan stop at limit + offset rows instead of reading every session in the project and sorting the whole match. llm_messages_via_api omitted it, and the endpoint test omitted it while holding a real session, so the one read we exercise took the slow path the base had just documented against. The helper now always sends session_id, defaulting to blank, and the test passes both scopes — which narrows to their intersection and is the stronger assertion anyway. Also corrected two claims the re-review falsified: the converters do not sit "beside its hooks" for pydantic-ai, which has no per-call hook, and the conftest ordering comment called (occurred_at, message_seq) the sort key's tail when it drops the event_id tiebreak.

MessageCursor assumes each hook call carries the whole conversation and subtracts the prefix it emitted last time. That holds until a caller passes conversation_id, previous_response_id or auto_previous_response_id: the SDK then builds an OpenAIServerConversationTracker and sends the model — and on_llm_start — only the items the server has not seen. Diffing a delta against a prefix that was never re-sent matches nothing, so every event from turn two on went out resynced, for the life of the run. The content was right (the short list is the delta); the flag was not. semconv defines RESYNCED as "restates the whole list", so a reader honouring it drops every earlier row and renders a two-message conversation — a transcript present in storage and invisible in the tool an auditor uses. Latent only because nothing consumes the column yet. The hook cannot see the tracker, but the runner sees the kwargs that create it, so _merge_hooks passes framework_sends_deltas down and the hook skips the diff, emitting the items as they arrive with a seq counted per turn_key. resynced stays false: the flag means "earlier rows are superseded", which is as untrue here as for an ordinary extension. Also corrects the docstrings that asserted input_items is always the whole conversation — that claim is what made this invisible on a read-through — and guards the kwarg names against an upstream rename, which would silently restore the bug.

turn_preparation.get_model gives run_config.model precedence over agent.model in both its str and Model forms, so a run started with RunConfig(model="gpt-4o-mini") against Agent(model="gpt-4o") is served by gpt-4o-mini. _resolve_model read agent.model alone and recorded gpt-4o — a model that never answered — and this PR carries that onto llm_message as well as llm_usage, so the wrong name now lands on two streams instead of one. RunContextWrapper holds context, usage, turn_input and the approval/tool state, not the run config, so the hook cannot recover it from its callback arguments. The runner already receives run_config and now passes it to HexgateUsageHooks alongside framework_sends_deltas, and _resolve_model mirrors get_model's precedence. The "default" placeholder for a model unset on both sides is unchanged.

This was referenced Sep 8, 2026

feat(sdk): add hexgate.messages wire contract and content caps #182

Merged

feat(clickhouse): add llm_message table #183

Merged

victorludvig changed the title ~~feat(sdk): capture OpenAI Agents messages in HexgateUsageHooks (10/15)~~ feat(sdk): capture OpenAI Agents messages in HexgateUsageHooks Sep 8, 2026

victorludvig deleted the branch main September 8, 2026 15:40

victorludvig closed this Sep 8, 2026

victorludvig deleted the vl/feat/openai_messages branch September 8, 2026 15:40

victorludvig restored the vl/feat/openai_messages branch September 14, 2026 07:40

victorludvig reopened this Sep 14, 2026

victorludvig force-pushed the vl/feat/openai_messages branch from 899c4ef to d4edd46 Compare September 14, 2026 07:43

victorludvig changed the base branch from vl/feat/llm_messages_read to main September 14, 2026 07:43

victorludvig force-pushed the vl/feat/openai_messages branch 2 times, most recently from f211da2 to dc3d7e7 Compare September 14, 2026 10:08

victorludvig changed the base branch from main to vl/feat/llm_messages_read September 14, 2026 10:08

victorludvig added this pull request to stack #214 September 14, 2026 10:16

victorludvig marked this pull request as ready for review September 14, 2026 12:41

victorludvig force-pushed the vl/feat/openai_messages branch from 89bd448 to 0444fda Compare September 14, 2026 15:17

victorludvig self-assigned this Sep 15, 2026

victorludvig requested a review from guillaume-hexamind September 15, 2026 07:29

victorludvig force-pushed the vl/feat/openai_messages branch from 0444fda to 1472f6b Compare September 15, 2026 07:34

Base automatically changed from vl/feat/llm_messages_read to main September 15, 2026 07:38

victorludvig force-pushed the vl/feat/openai_messages branch from 1472f6b to 9b3d7ca Compare September 15, 2026 07:38

guillaume-hexamind reviewed Sep 15, 2026

View reviewed changes

victorludvig requested a review from guillaume-hexamind September 15, 2026 13:36

victorludvig added 2 commits September 15, 2026 15:39

victorludvig added 6 commits September 15, 2026 15:39

victorludvig force-pushed the vl/feat/openai_messages branch from b3bbfaa to ac0bcac Compare September 15, 2026 13:39

Navigation Menu

Uh oh!

feat(sdk): capture OpenAI Agents messages in HexgateUsageHooks - #191

feat(sdk): capture OpenAI Agents messages in HexgateUsageHooks#191
victorludvig wants to merge 8 commits into
mainfrom
vl/feat/openai_messages

victorludvig commented Sep 8, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 14, 2026 •

edited

Loading

Uh oh!

guillaume-hexamind commented Sep 15, 2026

Uh oh!

guillaume-hexamind left a comment

Uh oh!

victorludvig commented Sep 15, 2026 •

edited

Loading

Review — 2 issues

1. `resynced` is `true` on every row under a server-managed conversation — 55/100

2. `_resolve_model` ignores `RunConfig.model` — 50/100

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

victorludvig commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changing

Why is this change necessary

Tests

Uh oh!

codecov Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

guillaume-hexamind commented Sep 15, 2026

Review — 2 issues

1. resynced is true on every row under a server-managed conversation — 55/100

2. _resolve_model ignores RunConfig.model — 50/100

Uh oh!

guillaume-hexamind left a comment

Choose a reason for hiding this comment

Uh oh!

victorludvig commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review — 2 issues

1. resynced is true on every row under a server-managed conversation — 55/100

2. _resolve_model ignores RunConfig.model — 50/100

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

victorludvig commented Sep 8, 2026 •

edited

Loading

codecov Bot commented Sep 14, 2026 •

edited

Loading

1. `resynced` is `true` on every row under a server-managed conversation — 55/100

2. `_resolve_model` ignores `RunConfig.model` — 50/100

victorludvig commented Sep 15, 2026 •

edited

Loading

1. `resynced` is `true` on every row under a server-managed conversation — 55/100

2. `_resolve_model` ignores `RunConfig.model` — 50/100