Skip to content

feat(clickhouse): add llm_message table - #183

Merged
victorludvig merged 5 commits into
mainfrom
vl/feat/llm_message_table
Sep 10, 2026
Merged

feat(clickhouse): add llm_message table#183
victorludvig merged 5 commits into
mainfrom
vl/feat/llm_message_table

Conversation

@victorludvig

@victorludvig victorludvig commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What is changing

  • platform/clickhouse/init/schema.sql — new table hexgate_audit.llm_message: the shared eight-column envelope, then model, turn_key, message_seq, resynced, truncated, and input_messages / output_messages / system_instructions as String CODEC(ZSTD(3)) holding the official gen_ai.* JSON shapes. ReplacingMergeTree(received_at), PARTITION BY toYYYYMM(received_at), ORDER BY (project_id, session_id, turn_key, message_seq, event_id), 180-day TTL.
  • platform/clickhouse/migrations/0003_add_llm_message.sql — the same CREATE TABLE, byte-identical once comments are stripped, for hand application to volumes that already exist. Header follows 0001: apply before the enricher that writes to the table is deployed, and what breaks if it is skipped.
  • docs/internals/audit-pipeline.md §5.1 — the table, its sort key, and why it is a separate table.

Nothing writes to or reads from the table yet; that is PR 4 and PR 6 of the stack.

Design: LLM message logging design · implementation spec.

Why is this change necessary

Message content is large, opt-in (nothing emits hexgate.messages yet) and read by session, so it does not belong as columns on llm_invocation, which is aggregated by user and model. The sort key is session-first with (turn_key, message_seq) next because the one read pattern is "reconstruct this session's transcript in order": a list's rows land adjacent and ordered, and event_id last keeps ReplacingMergeTree dedup to SDK retries of the same event. init/ runs once on an empty volume, so the committed migration is how staging and production get the table.

Tests

  • Both files applied in a throwaway clickhouse local (image clickhouse/clickhouse-server:24.10): init/schema.sql followed by the migration, confirming the statements parse and the migration is idempotent against a volume that already ran init. system.columns shows the 16 columns in the intended order; system.tables shows the sort key project_id, session_id, turn_key, message_seq, event_id and partition key toYYYYMM(received_at).
  • Comment-stripped diff of the two CREATE TABLE statements is empty.
  • No Python tests: the insert path and verify_schema that exercise the table arrive with PR 4.

🤖 Generated with Claude Code

@victorludvig victorludvig changed the title feat(clickhouse): add llm_message table feat(clickhouse): add llm_message table (2/15) Sep 8, 2026
@victorludvig victorludvig changed the title feat(clickhouse): add llm_message table (2/15) feat(clickhouse): add llm_message table Sep 8, 2026
@victorludvig
victorludvig force-pushed the vl/feat/llm_message_table branch from 4a282c5 to 16940b1 Compare September 9, 2026 11:41
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Base automatically changed from vl/feat/messages_semconv to main September 9, 2026 11:48
@victorludvig
victorludvig force-pushed the vl/feat/llm_message_table branch 4 times, most recently from 1f9b6db to d26ef02 Compare September 9, 2026 12:24
@victorludvig victorludvig self-assigned this Sep 9, 2026
@victorludvig
victorludvig marked this pull request as ready for review September 9, 2026 12:26
@victorludvig
victorludvig force-pushed the vl/feat/llm_message_table branch from d26ef02 to 8821af2 Compare September 9, 2026 14:09
@guillaume-hexamind

Copy link
Copy Markdown
Contributor

llm_message sort key omits occurred_at — score 78/100

platform/clickhouse/init/schema.sql:160 and platform/clickhouse/migrations/0003_add_llm_message.sql:49

Issue. The sort key is (project_id, session_id, turn_key, message_seq, event_id) — no time column. All three sibling tables end theirs …, occurred_at, event_id (schema.sql:60, :87, :116). But the read this table is designed for needs occurred_at: the doc bullet added in this PR (docs/internals/audit-pipeline.md:525) says "A session with a sub-agent or handoff has several turn_keys; readers order across them by occurred_at." That ordering has no index support, and it can never get any.

Why it can't be deferred. MODIFY ORDER BY cannot add occurred_at to this key at all — not mid-key, not appended. Verified on clickhouse/clickhouse-server:24.10 against this exact DDL:

-- mid-key
ALTER TABLE m MODIFY ORDER BY (project_id, session_id, turn_key, message_seq, occurred_at, event_id);
Code: 36. Primary key must be a prefix of the sorting key, but the column
in the position 4 is occurred_at, not event_id. (BAD_ARGUMENTS)

-- appended
ALTER TABLE m MODIFY ORDER BY (project_id, session_id, turn_key, message_seq, event_id, occurred_at);
Code: 36. Existing column occurred_at is used in the expression that was added
to the sorting key. You can add expressions that use only the newly added
columns. (BAD_ARGUMENTS)

The only later route is dropping and recreating the table. Unlike 0001/0002's additive ADD COLUMN, this is a one-way door.

Why now is free. Nothing emits hexgate.messages yet — hexgate/tracing/semconv.py:108-111 defines TURN_KEY/MESSAGE_SEQ/RESYNCED but there are no producers outside tests/tracing/test_semconv.py, no LlmMessageEvent, and no insert path in platform/api. The table is empty on every deployment, so today the fix costs two edited lines and nothing else.

Suggested fix — in both files:

ORDER BY (project_id, session_id, turn_key, message_seq, occurred_at, event_id)

This keeps event_id last (dedup unchanged), keeps a list's rows adjacent and ordered, matches the sibling convention, and gives the documented cross-turn_key read an indexed tiebreak. Worth updating the sort-key comment at schema.sql:157-159 and the doc bullet at :525 to match.

Partial mitigation if this ships as-is: ALTER TABLE … ADD INDEX occurred_at TYPE minmax + MATERIALIZE INDEX does apply later (verified), which recovers time pruning but not sort-key ordering.

@guillaume-hexamind guillaume-hexamind left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One blocking issue, approve of the logic otherwise

@victorludvig

Copy link
Copy Markdown
Contributor Author

Agreed the column belongs in the sort key — but not in the position suggested.

After message_seq, occurred_at only separates retries of one event. Third is the position that pays: it matches the order the PR 6 endpoint returns rows in (occurred_at, message_seq), so the read is in-order rather than sorted — and when session_id is empty, where the prefix stops being selective, it's the only thing left pruning. It has to be first after session_id because anything grouping the rows ahead of it wins; a sub-agent's message_seq restarts at 0, so time is all that orders the lists against each other.

ORDER BY (project_id, session_id, occurred_at, message_seq, event_id)

turn_key leaves the key — with occurred_at ahead of it, it would only break ties event_id already resolves. It stays a column for grouping and gap detection.

Also added run_id, mirroring llm_invocation, so a transcript is attributable to its run when session_id is empty. A column, not a key entry: before occurred_at it would split the session read into per-run blocks, and uuid4 wouldn't order runs chronologically anyway.

Applied both files in a throwaway clickhouse local (image clickhouse/clickhouse-server:24.10): system.tables reports the sorting key above, and system.columns lists all 17 columns in the intended order.

@victorludvig
victorludvig disabled the stack merge September 10, 2026 13:35
Storage for the hexgate.messages scope: one row per model call with the
shared envelope, turn_key / message_seq / resynced / truncated, and the
three gen_ai.* content columns as ZSTD-compressed JSON. Sorted by
(project_id, session_id, turn_key, message_seq, event_id) because the
read is "reconstruct this session's transcript in order"; same engine,
partitioning and 180-day TTL as the sibling tables.

Ships with the hand-applied migrations/0002_add_llm_message.sql (init/
never re-runs on a populated volume) and the §5.1 entry in the audit
pipeline spec. Nothing writes to the table yet.
The migration header described the emitter PR's behaviour as present fact:
it named an insert_llm_messages_batch that does not exist and claimed
verify_all refuses to boot on a missing table, when verify_all is passed
(verify_audit_schema, verify_llm_schema) only and never looks at
llm_message. Skipping the migration today is inert, so say that and put
the loud failure modes after the emitter PR is deployed.

Also drop HEXGATE_LOG_MESSAGES from the schema comment and the doc — no
such env var exists; capture is simply off until an emitter ships.

Comments only; the two CREATE TABLE statements stay identical.
message_seq only counts within one turn_key and restarts at 0 for a
sub-agent's or handoff's list, so wall-clock time is the only thing that
orders a session's rows across its several lists. Move occurred_at to
the first position after session_id, which is also the order the read
endpoint returns rows in; MODIFY ORDER BY cannot add an existing column
later, so this has to be right before the table ships.

turn_key leaves the sort key: with occurred_at ahead of it a list's rows
are no longer adjacent anyway, so there it would only break ties event_id
already resolves. It stays a plain column for grouping and gap detection.

Add run_id mirroring llm_invocation, so a transcript is attributable to
its run even when session_id is empty. A column, not a key entry: ahead
of occurred_at it would split the session read into per-run blocks, and
uuid4 does not order runs chronologically.
@victorludvig
victorludvig force-pushed the vl/feat/llm_message_table branch from 5d091b4 to 043657c Compare September 10, 2026 13:35
@victorludvig
victorludvig merged commit 4dd04f7 into main Sep 10, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants