Add task_id to spans and fix Investigate Traces button#197
Open
declan-scale wants to merge 5 commits intomainfrom
Open
Add task_id to spans and fix Investigate Traces button#197declan-scale wants to merge 5 commits intomainfrom
declan-scale wants to merge 5 commits intomainfrom
Conversation
Comment on lines
+25
to
+34
| # Backfill task_id from trace_id where trace_id is a valid task ID. | ||
| # Uses a JOIN instead of IN (subquery) to avoid a full scan of the tasks table per row, | ||
| # and batches updates to avoid long-held locks on large tables. | ||
| op.execute(""" | ||
| UPDATE spans | ||
| SET task_id = spans.trace_id | ||
| FROM tasks | ||
| WHERE spans.trace_id = tasks.id | ||
| AND spans.task_id IS NULL | ||
| """) |
There was a problem hiding this comment.
Comment claims batching that the SQL does not implement
The comment says the backfill "batches updates to avoid long-held locks on large tables," but the UPDATE … FROM tasks WHERE … is a single unbounded statement. It acquires row-level locks on every matching span for the entire transaction duration — exactly the behaviour the comment says it avoids. On a large spans table this can stall concurrent writers for minutes and trigger statement timeouts.
Either remove the misleading claim, or implement actual batching, e.g.:
op.execute("""
DO $$
DECLARE
updated_count INT;
BEGIN
LOOP
UPDATE spans
SET task_id = spans.trace_id
FROM (
SELECT spans.id
FROM spans
JOIN tasks ON spans.trace_id = tasks.id
WHERE spans.task_id IS NULL
LIMIT 1000
FOR UPDATE SKIP LOCKED
) batch
WHERE spans.id = batch.id;
GET DIAGNOSTICS updated_count = ROW_COUNT;
EXIT WHEN updated_count = 0;
END LOOP;
END
$$;
""")Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/database/migrations/alembic/versions/2026_04_14_1126_add_task_id_to_spans_57c5ed4f59ae.py
Line: 25-34
Comment:
**Comment claims batching that the SQL does not implement**
The comment says the backfill "batches updates to avoid long-held locks on large tables," but the `UPDATE … FROM tasks WHERE …` is a single unbounded statement. It acquires row-level locks on every matching span for the entire transaction duration — exactly the behaviour the comment says it avoids. On a large `spans` table this can stall concurrent writers for minutes and trigger statement timeouts.
Either remove the misleading claim, or implement actual batching, e.g.:
```python
op.execute("""
DO $$
DECLARE
updated_count INT;
BEGIN
LOOP
UPDATE spans
SET task_id = spans.trace_id
FROM (
SELECT spans.id
FROM spans
JOIN tasks ON spans.trace_id = tasks.id
WHERE spans.task_id IS NULL
LIMIT 1000
FOR UPDATE SKIP LOCKED
) batch
WHERE spans.id = batch.id;
GET DIAGNOSTICS updated_count = ROW_COUNT;
EXIT WHEN updated_count = 0;
END LOOP;
END
$$;
""")
```
How can I resolve this? If you propose a fix, please make it concise.6a84b54 to
1e16dad
Compare
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
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.
Summary
task_idcolumn to spans: New nullabletask_idforeign key on thespanstable with a migration that backfills existing spans wheretrace_idmatches a valid task ID. This lets the UI and API query spans by task directly instead of relying on the convention thattrace_idequals the task ID.trace_idfrom spans (fetched via the newtask_idquery) rather than assumingtask_id == trace_id. This makes the SGP Monitor link work correctly for agents whose trace IDs differ from their task IDs.contentandsummary— it showscontentwhen available, falling back tosummary.Backend changes
task_idcolumn tospanstable with FK totasks, backfills fromtrace_id, and adds indexSpanORM,SpanEntity, create/update schemas, and use case all supporttask_id/spanslist endpoint acceptstask_idquery parametertask_idon create, update, and null defaultFrontend changes
useSpanshook queries bytask_idfirst, falls back totrace_idfor backward compatInvestigateTracesButtonacceptstraceIdinstead oftaskIdTaskHeaderfetches spans and extracts the realtrace_idfor the buttonFollow-up required
Test plan
task_id/spans?task_id=<id>returns the correct spansmake testfor backend integration tests🤖 Generated with Claude Code
Greptile Summary
This PR adds a
task_idforeign key to thespanstable with a backfill migration, threads the new field through the full backend stack (ORM → entity → schema → use case → route), and updates the frontend to query spans bytask_idfirst (falling back totrace_id) so the Investigate Traces button resolves the realtrace_idinstead of assuming it equalstask_id. Two frontend fixes land alongside: the reasoning content no longer concatenatescontentandsummary, and the "Thinking…" indicator now correctly fires after completed tool calls.Confidence Score: 4/5
Safe to merge after addressing the loading-state race on the Investigate Traces button (flagged in prior review thread) and adding a workaround comment for the SDK type cast.
The FK constraint issue in tests has been fixed. The only remaining P1 (button renders with the wrong trace_id URL during the initial spans fetch — flagged in prior thread) is still unresolved; all other findings are P2.
agentex-ui/components/task-header/task-header.tsx (loading-state race), agentex-ui/hooks/use-spans.ts (SDK type cast comment)
Important Files Changed
Sequence Diagram
sequenceDiagram participant TH as TaskHeader participant Hook as useSpans participant API as /spans endpoint participant DB as Database TH->>Hook: useSpans(taskId) Hook->>API: GET /spans?task_id={taskId} API->>DB: SELECT * FROM spans WHERE task_id = ? DB-->>API: spans[] API-->>Hook: spans[] alt spans found by task_id (new path) Hook-->>TH: spans with task_id TH->>TH: traceId = spans[0].trace_id else no spans found — backward compat fallback Hook->>API: GET /spans?trace_id={taskId} API->>DB: SELECT * FROM spans WHERE trace_id = ? DB-->>API: legacy spans[] API-->>Hook: legacy spans[] Hook-->>TH: legacy spans TH->>TH: traceId = spans[0].trace_id ?? taskId end TH->>TH: InvestigateTracesButton traceId={traceId}Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "Add more tests" | Re-trigger Greptile