Skip to content

feat(tracing): stamp sgp evals run and row ids on SGP spans - #545

Open
mohammadatallah-scale wants to merge 13 commits into
nextfrom
mohammad/ove-1253-sgp-evals-span-metadata
Open

mohammadatallah-scale wants to merge 13 commits into
nextfrom
mohammad/ove-1253-sgp-evals-span-metadata

Conversation

@mohammadatallah-scale

@mohammadatallah-scale mohammadatallah-scale commented Oct 7, 2026 •

Copy link
Copy Markdown

OVE-1253. Pairs with an sgp-evaluations PR that tags each generation-unit task.

The problem. Spans of an evals generation-unit task carry no run or row id, so a spans search cannot find them.

The fix.

  • Stamp: the SGP copy of each span of an eval task gets flat run, row and attempt keys. A task that loses the eval marker stops being stamped.
  • Sync and async agents: the ACP server reads the task it receives.
  • Temporal agents: the ids travel through the workflow memo to the worker's activities, child workflows included. A span keeps the ids its task had when it started, so a reused task id cannot strip a queued span. Not covered: spans whose trace_id and task_id are both not the task id.

Test plan

  • Real Temporal dev server run: eval workflow stamps the three keys, a plain one stamps nothing
  • Real Temporal dev server run with a child workflow: its activity is stamped with the parent task's ids (unstamped before this change), and a history recorded with the old interceptor replays with no nondeterminism

Changes since review

  • A span's run and row ids are fixed when it starts, so a task id reused before export cannot change them.
  • A span that started with no eval task can no longer pick up a later run's ids, including one that started before SGP tracing was set up.
  • A failed upload keeps the ids, so a retry sends the same span.
  • Activities of a child workflow carry the ids of the task that started it.
  • A Temporal span that ends on a different worker than it started on keeps its ids.
  • The tutorial tests pin pydantic below 2.14 for the web search MCP server, which pydantic 2.14 broke on every branch.

RetriggerView in GreptileConfidence Score: 5/5

The numbered finding is addressed, and no separate new blocking finding was accepted.

Summary

Adds run, row, and attempt IDs to the SGP copy of eval-task spans.

  • SGP spans show the eval run, row, and attempt that started them.
  • Temporal activities carry their eval task's ids into SGP spans.
  • Tutorial web-search servers use a compatible Pydantic version.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Task metadata] --> B[ACP task registration]
  A --> C[Temporal workflow memo]
  C --> D[Child workflow memo]
  C --> E[Activity headers]
  D --> E
  E --> F[Worker task registration]
  B --> G[Span-start capture]
  F --> G
  G --> H[SGP metadata copy]
  F --> H
  H --> I[Upload]
  I --> J[Release saved span IDs]
Loading

Reviews (8) · Last reviewed commit: "ci(tutorials): pin pydantic below 2.14 f..." · Reviewed by Greptile

mohammadatallah-scale and others added 2 commits October 7, 2026 08:04
Spans of a task whose task_metadata carries sgp_evals now get flat
sgp_evals_generation_run_id, sgp_evals_row_id and sgp_evals_attempt_idx keys on
the SGP copy. Sync and async agents read the task in the ACP server. Temporal
agents carry it in the workflow memo and activity headers to the worker.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…val attrs

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Comment thread src/agentex/lib/core/tracing/sgp_evals.py Outdated
Comment thread src/agentex/lib/core/tracing/sgp_evals_interceptor.py
Comment thread src/agentex/lib/core/tracing/sgp_evals_interceptor.py
@mohammadatallah-scale

Copy link
Copy Markdown
Author

P3 sgp_evals_interceptor.py - activities of a child workflow get no header (memo is not inherited and their workflow id is not the task id), so their spans stay unstamped even when trace_id is the task id.
P3 worker.py:278 - no test pins SGPEvalsInterceptor in the worker's interceptor list, so dropping it keeps the suite green.

Refuted: continue-as-new keeps the memo (real dev-server run stamps the post-CAN run), ContextInterceptor headers coexist with this one in either order, and task_metadata does reach the agent inside params.task, so the event/send pop never drops a live eval task.

mohammadatallah-scale and others added 2 commits October 7, 2026 08:19
Local activities go through start_local_activity, which the outbound
interceptor did not override, so their spans carried no run or row id.
Also pin the worker's interceptor wiring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ry none

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Comment thread src/agentex/lib/core/tracing/sgp_evals_interceptor.py
…id cannot strip a queued span

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Comment thread src/agentex/lib/core/tracing/sgp_evals.py Outdated
Comment thread src/agentex/lib/core/tracing/processors/sgp_tracing_processor.py Outdated
mohammadatallah-scale and others added 2 commits October 7, 2026 08:56
…load succeeds

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Comment thread src/agentex/lib/core/tracing/sgp_evals.py Outdated
…o a span flood cannot evict them

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@mohammadatallah-scale
mohammadatallah-scale requested review from andrewvo-scale and xsfa and removed request for xsfa October 7, 2026 13:08

@andrewvo-scale andrewvo-scale 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.

LGTM. The SDK is the right layer, since it is the only place that sees both the task metadata and the span. The keys it reads (generation_run_id, row_id, attempt_idx under the sgp_evals marker) match what sgp-evaluations writes in _unit_task_metadata. With no eval ids, _sgp_metadata returns exactly what base returns, so existing users see no change. All seven threads are resolved with fixes at head. On the first P1 (unregister on later events), keeping the unregister is right, because the server sends the full task, task_metadata included, on every call. Two non-blocking notes inline.

Comment thread src/agentex/lib/core/tracing/sgp_evals.py
Comment thread src/agentex/lib/core/tracing/sgp_evals_interceptor.py
…ns without an SGP processor

Child workflows inherit no memo and their workflow id is not the task id, so
the parent hands both down in the child's memo and activities carry the task id
in a header. Span pinning now starts only once an SGP processor exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mohammadatallah-scale

Copy link
Copy Markdown
Author

P3 sgp_evals_interceptor.py - activities of a child workflow get no header
P3 worker.py:278 - no test pins SGPEvalsInterceptor in the worker's interceptor list

Both closed. Child workflows now inherit the ids and the task id (a789175), and test_worker_runs_with_the_interceptor_ahead_of_agent_interceptors pins the worker's interceptor list.

Comment thread src/agentex/lib/core/tracing/sgp_evals.py
@andrewvo-scale

Copy link
Copy Markdown

@mohammadatallah-scale I re-checked a789175/8a170c1. The child-workflow memo + task-id header fix and skipping pins with no SGP processor both look right, and neither runs without eval ids. Two things before merge:

  • CI is red on 040_other_sdks and 020_state_machine (and 010_agent_chat was cancelled), which all passed on 848212a. The assertions are live-model ones ("Did not use tool requests", "Did not find an agent message"), the image pulls retried, and 040_other_sdks isn't Temporal, so this looks like flake rather than this change. Worth a rerun before merging.
  • Greptile's new P1 on sgp_evals.py:96 is real but narrow: it needs the SGP processor to be created after a span starts and that task to gain eval ids later. Fine to leave as a named gap; my approval stands.

…it started

A span that started before an SGP processor existed has no pin, so the export
fell back to the live registry and could pick up a run registered later under
the same task id. The registry now records when each task's ids took effect,
and the fallback skips ids registered after the span started.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mohammadatallah-scale

Copy link
Copy Markdown
Author

CI is red on 040_other_sdks and 020_state_machine (and 010_agent_chat was cancelled) ... Worth a rerun before merging.
Greptile's new P1 on sgp_evals.py:96 is real but narrow ... Fine to leave as a named gap

@andrewvo-scale The P1 is fixed rather than left open (b45d3b3): an unpinned span now only gets ids its task had at its start. On CI, the same three tutorials fail or cancel the same way on two unrelated branches (jerome/worker-startup-compatibility, dm/retry-pydantic-ai-pypi-fetches), so it is the live-model flake. The push re-runs them.

Comment thread src/agentex/lib/core/tracing/sgp_evals.py
mohammadatallah-scale and others added 2 commits October 14, 2026 09:09
…orker

start-span and end-span are separate activities, so a span can end on a
worker that first registered its task after the span started, and the
start-time gate dropped its ids. Ids read from a workflow memo are fixed for
the workflow's life, so they now cover every span of the workflow. Only ids
the ACP server registers keep the start-time gate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…erver

openai-websearch-mcp pins mcp 1.13.1, which imports a pydantic internal that
pydantic 2.14 removed, so the server died at import and the 010, 020 and 040
tutorials timed out waiting for tool calls on every branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mohammadatallah-scale

Copy link
Copy Markdown
Author

040_other_sdks isn't Temporal, so this looks like flake rather than this change. Worth a rerun before merging.

@andrewvo-scale Correction to my earlier reply: it is not a flake, and not this change either. openai-websearch-mcp pins mcp==1.13.1, which imports pydantic._internal._typing_extra.eval_type_backport, and pydantic 2.14.0 (Oct 8) removed it. The server dies at import, so 010, 020 and 040 (the only tutorials that launch it and need its tool) time out on every branch. 0a79a58 pins pydantic<2.14 for that server in all four tutorials that launch it. Locally the server fails the MCP handshake on 2.14.0 and 2.14.1 and lists openai_web_search with the pin.

This branch has not been deployed

No deployments
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