Skip to content

Turn-level observability — correlation_id binds a question's agent refinements - #54

Merged
ashwin-agami merged 4 commits into
mainfrom
ACE-015-turn-level-correlation-id
Jun 28, 2026
Merged

ashwin-agami merged 4 commits into
mainfrom
ACE-015-turn-level-correlation-id

Conversation

@ashwin-agami

Copy link
Copy Markdown
Contributor

Spec: ACE-015

Summary

Adds the turn level to the activity log — the deferred middle of ACE-008's thread_id (conversation) ▸ correlation_id (turn) ▸ step. One user question fans out into N agent sub-queries; this binds them, so the admin Sessions drawer reads "User asked X → 2 queries". Grounded in real data (a meridian session showed the gap and that Claude drifts user_question onto later refinements).

Changes

  • correlation_id — migration 009 adds the nullable column; ToolCallRecord + the DB sink + _TOOL_CALL_COLS carry it; record_tool_call captures args.get("correlation_id").
  • Self-reported, per turnexecute_sql's inputSchema gains a correlation_id param, and SERVER_INSTRUCTIONS now tells Claude to keep user_question verbatim across a turn (refinement goes only in raw_query) and mint one correlation_id per user question. (The server can't mint it — the MCP protocol carries no turn boundary.)
  • Turn groupingmodel_store._group_turns folds a session's calls into turns by correlation_id; the turn's question is the earliest call's user_question (drift-proof); a call with no correlation_id is its own singleton turn. _session_drawer renders one "User asked … · N queries" block per turn with each refinement's agent_query + SQL.
  • Preview seeds a 2-query turn; README documents the turn level.

Safety / degradation

Best-effort + graceful: no correlation_id → reads like the old flat list, never errors. Read-only + admin-gated; every self-reported field escaped; correlation_id is never rendered into HTML (used only as a grouping key); no new egress.

Test plan

tests/test_admin_activity.py: correlation_id round-trips; the inputSchema exposes it; SERVER_INSTRUCTIONS carries the verbatim + per-turn guidance; turns group by correlation_id; the drift case (2nd call's question differs → earliest wins); degradation (no correlation_id → singleton turns); the drawer renders "User asked" + both agent_query refinements; the existing XSS-escape test still covers the sessions tab.

Checklist

  • uv run dev.py check green — 953 tests, ruff, gitleaks
  • 100% patch coverage (uv run dev.py cover)
  • /code-review (2 finder angles) — 0 findings; Agami rubric 0 must-fix
  • Read-only + admin-gated; self-report fields escaped; migration portable; no new egress
  • Decisions recorded in the spec; manual smoke pending (validate Claude supplies correlation_id via the live Claude connection)

…tions)

Migration 009 adds tool_calls.correlation_id; ToolCallRecord + the DB sink +
_TOOL_CALL_COLS carry it; record_tool_call captures args.get('correlation_id').
execute_sql's inputSchema gains a correlation_id param, and SERVER_INSTRUCTIONS
now tells Claude to keep user_question VERBATIM across a turn (refinement only in
raw_query) and mint one correlation_id per user question. Slice A of ACE-015.
model_store._group_turns folds a session's calls into turns (one user question
-> N agent queries) keyed on correlation_id; a call with no correlation_id is its
own singleton turn (degrades to the old flat list). The turn's question is the
EARLIEST call's user_question (drift-proof). _session_drawer renders one
'User asked X · N queries' block per turn with each refinement's agent_query +
SQL. Slice B of ACE-015.
render_previews seeds one turn (correlation c1) with two agent queries so the
Sessions preview shows the 'User asked X · 2 queries' grouping; README documents
the turn level. Slice C of ACE-015.
Copilot AI review requested due to automatic review settings June 28, 2026 06:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds “turn-level” observability to the admin activity log by introducing a correlation_id that groups multiple agent sub-queries spawned from a single user question within a conversation/thread.

Changes:

  • Add nullable correlation_id to tool_calls (migration 009) and plumb it through contracts, sinks, and the tool-call recorder.
  • Extend execute_sql tool schema + server instructions so the model self-reports one correlation_id per user question/turn and keeps user_question verbatim across refinements.
  • Group session queries into turns in model_store, and render the Sessions drawer as “User asked … · N queries” blocks; add preview seeds and tests/docs for grouping + drift/degradation behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_admin_activity.py Adds coverage for correlation_id round-trip, schema/instruction presence, turn grouping (incl. drift + degradation), and Sessions drawer rendering.
render_previews.py Updates preview seed data to include multi-query turns via correlation_id.
packages/agami-core/src/tools.py Records correlation_id from tool args; updates execute_sql input schema + SERVER_INSTRUCTIONS guidance.
packages/agami-core/src/model_store.py Adds correlation_id to tool-call read/write paths and groups session queries into turns (_group_turns).
packages/agami-core/src/contracts.py Extends ToolCallRecord with correlation_id.
packages/agami-core/src/admin.py Renders Sessions drawer grouped by turns instead of a flat per-call list.
packages/agami-core/README.md Documents turn-level grouping semantics and drift behavior in the Sessions tab.
migrations/core/009_tool_calls_correlation.sql Adds correlation_id column to tool_calls.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +318 to +322
'<div style="border-top:2px solid var(--line);padding:14px 0 2px;margin-top:8px">'
f'<div style="margin-bottom:4px"><span class="muted">User asked</span> '
f'<strong>{ui.esc(question)}</strong> '
f'<span class="muted" style="font-size:13px">· {n} {"query" if n == 1 else "queries"}</span>'
f"</div>{qrows}</div>"
Comment on lines 365 to 375
@@ -373,6 +375,31 @@ def list_tool_calls(store: Store, *, limit: int = 200) -> list[dict[str, Any]]:
)
@ashwin-agami

Copy link
Copy Markdown
Contributor Author

Thanks @copilot:

  1. Self-reported label — fixed in 42d99d7: the turn question now renders User asked <q> · self-reported, consistent with the rest of the activity log.
  2. Read path 500s if migration 009 isn't applied — fair, but this is the established convention: every migration (incl. ACE-008's 008 that created tool_calls) requires the deploy to apply it before the new code reads the new column; making one column's read path defensive would diverge from the other 13. The robust fix you suggest — auto-applying migrations on startup/open-store — is a good general improvement that belongs in the deploy work (ACE-009, the agami deploy package), not this slice. Logged it there.

@ashwin-agami
ashwin-agami merged commit 2f7e875 into main Jun 28, 2026
6 checks passed
@ashwin-agami
ashwin-agami deleted the ACE-015-turn-level-correlation-id branch June 28, 2026 07:29
Copilot stopped work on behalf of ashwin-agami due to an error June 28, 2026 07:29
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants