Skip to content

Commit 029ac9f

Browse files
waleedlatif1claude
andauthored
fix(logs): split summary/detail contracts to make trace tab gate type-safe (#4431)
* fix(logs): split summary/detail contracts to make trace tab gate type-safe The Trace tab was silently missing from the Log Details sidepanel because list and detail rows shared one WorkflowLog type with executionData: z.unknown(). The UI couldn't distinguish a summary row (no spans) from a detail row (with spans), so the tab gate read undefined and hid itself. Splits into WorkflowLogSummary (list) and WorkflowLogDetail (typed executionData with optional traceSpans). Detail and by-execution routes both write through to the same logKeys.detail(id) cache, eliminating the two-key fragmentation that caused the merge memo workaround. List route moves to cursor pagination on (sortValue, id) with proper NULLS LAST handling and SQL-side sort across workflow + job execution tables. Detail route now requires and asserts workspaceId. Deep-link path uses useLogByExecutionId instead of auto-paginating the entire workspace. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(logs): audit follow-ups — render side-effect, stats invalidation, enhanced spread order - Move onActiveTabChange call from render into useEffect to avoid side-effects during render (StrictMode safety). - Re-add logKeys.stats() invalidation to cancel/retry mutations so the dashboard reflects status flips immediately. - Reorder enhanced: true after ...execData spread in detail and by-execution routes so the literal discriminator is never overwritten by stale execData. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(logs): mirror SQL NULLS LAST in JS merge for cursor consistency The in-memory merge of workflow + job pages negated the comparator for DESC, which placed null sort values at the start. SQL orders both ASC and DESC with NULLS LAST, so DESC pages emitted a cursor {v: <last non-null>, id: ...} while null rows still satisfied the cursor predicate (OR sort_expr IS NULL) on the next page — producing duplicate null rows across pages on cost/duration sorts. Handle nulls explicitly in the JS comparator so they always sort last regardless of direction, matching the SQL ordering. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(logs): final-audit follow-ups — stable tab callback, byExecution invalidation, optimistic detail patch, trace loading state - Wrap LogDetails -> LogDetailsContent onActiveTabChange in useCallback so the child useEffect doesn't refire on every parent render. - Add logKeys.byExecutionAll() to cancel + retry invalidation so the table-embedded sidebar picks up status changes immediately. - Optimistic write-through to logKeys.detail in useCancelExecution so the open sidebar reflects 'cancelling' instantly; rolls back on error. - Distinguish trace loading from trace-empty: when log.executionData is not yet fetched, render "Loading trace…" instead of the empty state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(logs): migrate stores/components to contract types Replace the legacy `WorkflowLog` / `LogsResponse` / `WorkflowData` / `CostMetadata` / `ToolCallMetadata` shapes in `stores/logs/filters/types.ts` with direct use of the contract types `WorkflowLogSummary`, `WorkflowLogDetail`, and a new `WorkflowLogRow` alias for surfaces that render either form. Removes the `summaryToWorkflowLog` / `detailToWorkflowLog` bridge in the React Query layer along with their double-cast annotations. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(logs): address PR review feedback - Whitelist sort columns against logSortBy enum to prevent client crash when non-sortable headers (workflow, trigger) reach the contract parser. - Extract fetchLogDetail helper shared by /api/logs/[id] and /api/logs/by-execution/[executionId] — collapses ~360 duplicated lines to a single source of truth keyed on lookup column. * fix(logs): exclude job logs when level filter is workflow-only When level=running or level=pending (workflow-only states involving endedAt/pausedExecutions semantics), jobLevelConditions stayed empty so no level constraint reached jobConditions — every job log in the workspace leaked into the result. Skip the job side entirely when the level filter has no job-applicable values (error/info). * chore(logs): drop dead utils — mapToExecutionLog and friends Remove ExecutionLog/RawLogResponse/ExecutionCost/LogWithExecutionData/ TraceSpan/BlockExecution interfaces and the mapToExecutionLog, mapToExecutionLogAlt, extractOutput functions — all unreferenced after the contract split. -212 lines. * chore(logs): drop unused LOG_COLUMN_ORDER and LogColumnKey * fix(logs): hydrate filters from URL synchronously on mount The previous useEffect-based initializeFromURL caused useLogsList and useDashboardStats to fire once with default store filters, then refetch after the effect updated filters from the URL. Move the initial hydrate into a useState lazy initializer so the first render already reads URL-derived filters; the popstate handler keeps the existing effect for back/forward navigation. * chore(logs): trim verbose comments added during PR Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(logs): guard navigation arrows when selected log is off-page Deep-linked logs resolved via useLogByExecutionId may not be in the current page list, leaving selectedLogIndex at -1. The hasNext prop was evaluating -1 < logs.length - 1 (true for any non-empty list), which enabled the next arrow and jumped to the first item on click. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(logs): sync active-tab callback before paint to keep keyboard guards aligned Run the resolvedTab → onActiveTabChange propagation in useLayoutEffect so the parent's activeTabRef updates synchronously before the next paint. This closes the brief window where window keydown handlers in the logs page would still see activeTabRef.current === 'trace' and short-circuit arrow-key navigation immediately after switching to a log without a Trace tab. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 6d044a9 commit 029ac9f

16 files changed

Lines changed: 1103 additions & 1343 deletions

File tree

Lines changed: 21 additions & 168 deletions
Original file line numberDiff line numberDiff line change
@@ -1,183 +1,36 @@
1-
import { db } from '@sim/db'
2-
import {
3-
jobExecutionLogs,
4-
permissions,
5-
workflow,
6-
workflowDeploymentVersion,
7-
workflowExecutionLogs,
8-
} from '@sim/db/schema'
91
import { createLogger } from '@sim/logger'
10-
import { and, eq } from 'drizzle-orm'
112
import { type NextRequest, NextResponse } from 'next/server'
12-
import { logIdParamsSchema } from '@/lib/api/contracts/logs'
3+
import { getLogDetailContract } from '@/lib/api/contracts/logs'
4+
import { parseRequest } from '@/lib/api/server'
135
import { getSession } from '@/lib/auth'
14-
import { generateRequestId } from '@/lib/core/utils/request'
156
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
7+
import { fetchLogDetail } from '@/lib/logs/fetch-log-detail'
168

179
const logger = createLogger('LogDetailsByIdAPI')
1810

19-
export const revalidate = 0
20-
2111
export const GET = withRouteHandler(
22-
async (_request: NextRequest, { params }: { params: Promise<{ id: string }> }) => {
23-
const requestId = generateRequestId()
24-
25-
try {
26-
const session = await getSession()
27-
if (!session?.user?.id) {
28-
logger.warn(`[${requestId}] Unauthorized log details access attempt`)
29-
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
30-
}
31-
32-
const userId = session.user.id
33-
const { id } = logIdParamsSchema.parse(await params)
34-
35-
const rows = await db
36-
.select({
37-
id: workflowExecutionLogs.id,
38-
workflowId: workflowExecutionLogs.workflowId,
39-
executionId: workflowExecutionLogs.executionId,
40-
stateSnapshotId: workflowExecutionLogs.stateSnapshotId,
41-
deploymentVersionId: workflowExecutionLogs.deploymentVersionId,
42-
level: workflowExecutionLogs.level,
43-
status: workflowExecutionLogs.status,
44-
trigger: workflowExecutionLogs.trigger,
45-
startedAt: workflowExecutionLogs.startedAt,
46-
endedAt: workflowExecutionLogs.endedAt,
47-
totalDurationMs: workflowExecutionLogs.totalDurationMs,
48-
executionData: workflowExecutionLogs.executionData,
49-
cost: workflowExecutionLogs.cost,
50-
files: workflowExecutionLogs.files,
51-
createdAt: workflowExecutionLogs.createdAt,
52-
workflowName: workflow.name,
53-
workflowDescription: workflow.description,
54-
workflowColor: workflow.color,
55-
workflowFolderId: workflow.folderId,
56-
workflowUserId: workflow.userId,
57-
workflowWorkspaceId: workflow.workspaceId,
58-
workflowCreatedAt: workflow.createdAt,
59-
workflowUpdatedAt: workflow.updatedAt,
60-
deploymentVersion: workflowDeploymentVersion.version,
61-
deploymentVersionName: workflowDeploymentVersion.name,
62-
})
63-
.from(workflowExecutionLogs)
64-
.leftJoin(workflow, eq(workflowExecutionLogs.workflowId, workflow.id))
65-
.leftJoin(
66-
workflowDeploymentVersion,
67-
eq(workflowDeploymentVersion.id, workflowExecutionLogs.deploymentVersionId)
68-
)
69-
.innerJoin(
70-
permissions,
71-
and(
72-
eq(permissions.entityType, 'workspace'),
73-
eq(permissions.entityId, workflowExecutionLogs.workspaceId),
74-
eq(permissions.userId, userId)
75-
)
76-
)
77-
.where(eq(workflowExecutionLogs.id, id))
78-
.limit(1)
79-
80-
const log = rows[0]
81-
82-
// Fallback: check job_execution_logs
83-
if (!log) {
84-
const jobRows = await db
85-
.select({
86-
id: jobExecutionLogs.id,
87-
executionId: jobExecutionLogs.executionId,
88-
level: jobExecutionLogs.level,
89-
status: jobExecutionLogs.status,
90-
trigger: jobExecutionLogs.trigger,
91-
startedAt: jobExecutionLogs.startedAt,
92-
endedAt: jobExecutionLogs.endedAt,
93-
totalDurationMs: jobExecutionLogs.totalDurationMs,
94-
executionData: jobExecutionLogs.executionData,
95-
cost: jobExecutionLogs.cost,
96-
createdAt: jobExecutionLogs.createdAt,
97-
})
98-
.from(jobExecutionLogs)
99-
.innerJoin(
100-
permissions,
101-
and(
102-
eq(permissions.entityType, 'workspace'),
103-
eq(permissions.entityId, jobExecutionLogs.workspaceId),
104-
eq(permissions.userId, userId)
105-
)
106-
)
107-
.where(eq(jobExecutionLogs.id, id))
108-
.limit(1)
109-
110-
const jobLog = jobRows[0]
111-
if (!jobLog) {
112-
return NextResponse.json({ error: 'Not found' }, { status: 404 })
113-
}
12+
async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {
13+
const session = await getSession()
14+
if (!session?.user?.id) {
15+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
16+
}
11417

115-
const execData = jobLog.executionData as Record<string, any> | null
116-
const response = {
117-
id: jobLog.id,
118-
workflowId: null,
119-
executionId: jobLog.executionId,
120-
deploymentVersionId: null,
121-
deploymentVersion: null,
122-
deploymentVersionName: null,
123-
level: jobLog.level,
124-
status: jobLog.status,
125-
duration: jobLog.totalDurationMs ? `${jobLog.totalDurationMs}ms` : null,
126-
trigger: jobLog.trigger,
127-
createdAt: jobLog.startedAt.toISOString(),
128-
workflow: null,
129-
jobTitle: (execData?.trigger?.source as string) || null,
130-
executionData: {
131-
totalDuration: jobLog.totalDurationMs,
132-
...execData,
133-
enhanced: true,
134-
},
135-
cost: jobLog.cost as any,
136-
}
18+
const parsed = await parseRequest(getLogDetailContract, request, context)
19+
if (!parsed.success) return parsed.response
13720

138-
return NextResponse.json({ data: response })
139-
}
21+
const { id } = parsed.data.params
22+
const { workspaceId } = parsed.data.query
14023

141-
const workflowSummary = log.workflowId
142-
? {
143-
id: log.workflowId,
144-
name: log.workflowName,
145-
description: log.workflowDescription,
146-
color: log.workflowColor,
147-
folderId: log.workflowFolderId,
148-
userId: log.workflowUserId,
149-
workspaceId: log.workflowWorkspaceId,
150-
createdAt: log.workflowCreatedAt,
151-
updatedAt: log.workflowUpdatedAt,
152-
}
153-
: null
24+
const data = await fetchLogDetail({
25+
userId: session.user.id,
26+
workspaceId,
27+
lookupColumn: 'id',
28+
lookupValue: id,
29+
})
15430

155-
const response = {
156-
id: log.id,
157-
workflowId: log.workflowId,
158-
executionId: log.executionId,
159-
deploymentVersionId: log.deploymentVersionId,
160-
deploymentVersion: log.deploymentVersion ?? null,
161-
deploymentVersionName: log.deploymentVersionName ?? null,
162-
level: log.level,
163-
status: log.status,
164-
duration: log.totalDurationMs ? `${log.totalDurationMs}ms` : null,
165-
trigger: log.trigger,
166-
createdAt: log.startedAt.toISOString(),
167-
files: log.files || undefined,
168-
workflow: workflowSummary,
169-
executionData: {
170-
totalDuration: log.totalDurationMs,
171-
...(log.executionData as any),
172-
enhanced: true,
173-
},
174-
cost: log.cost as any,
175-
}
31+
if (!data) return NextResponse.json({ error: 'Not found' }, { status: 404 })
17632

177-
return NextResponse.json({ data: response })
178-
} catch (error: any) {
179-
logger.error(`[${requestId}] log details fetch error`, error)
180-
return NextResponse.json({ error: error.message }, { status: 500 })
181-
}
33+
logger.debug('Fetched log detail', { id, workspaceId })
34+
return NextResponse.json({ data })
18235
}
18336
)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { createLogger } from '@sim/logger'
2+
import { type NextRequest, NextResponse } from 'next/server'
3+
import { getLogByExecutionIdContract } from '@/lib/api/contracts/logs'
4+
import { parseRequest } from '@/lib/api/server'
5+
import { getSession } from '@/lib/auth'
6+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
7+
import { fetchLogDetail } from '@/lib/logs/fetch-log-detail'
8+
9+
const logger = createLogger('LogDetailsByExecutionAPI')
10+
11+
export const GET = withRouteHandler(
12+
async (request: NextRequest, context: { params: Promise<{ executionId: string }> }) => {
13+
const session = await getSession()
14+
if (!session?.user?.id) {
15+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
16+
}
17+
18+
const parsed = await parseRequest(getLogByExecutionIdContract, request, context)
19+
if (!parsed.success) return parsed.response
20+
21+
const { executionId } = parsed.data.params
22+
const { workspaceId } = parsed.data.query
23+
24+
const data = await fetchLogDetail({
25+
userId: session.user.id,
26+
workspaceId,
27+
lookupColumn: 'executionId',
28+
lookupValue: executionId,
29+
})
30+
31+
if (!data) return NextResponse.json({ error: 'Not found' }, { status: 404 })
32+
33+
logger.debug('Fetched log by execution id', { executionId, workspaceId })
34+
return NextResponse.json({ data })
35+
}
36+
)

0 commit comments

Comments
 (0)