Skip to content

Commit aacc3e2

Browse files
committed
improvement(perf): drop a duplicate authorization, parallelize the credential reads, and trim the comments
1 parent d2755e9 commit aacc3e2

22 files changed

Lines changed: 108 additions & 213 deletions

File tree

apps/sim/app/api/copilot/checkpoints/revert/route.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
140140
workflowId: checkpoint.workflowId,
141141
userId,
142142
state: parsedState.data,
143+
/** Already resolved above; re-deriving it would repeat 2-3 sequential reads. */
144+
authorization,
143145
})
144146

145147
if (!saveResult.success) {

apps/sim/app/api/workflows/[id]/state/route.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
import { db } from '@sim/db'
22
import { workflow } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4-
import {
5-
authorizeWorkflowByWorkspacePermission,
6-
WorkflowLockedError,
7-
} from '@sim/platform-authz/workflow'
4+
import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow'
85
import { toError } from '@sim/utils/errors'
96
import { eq, sql } from 'drizzle-orm'
107
import { type NextRequest, NextResponse } from 'next/server'
@@ -132,10 +129,6 @@ export const PUT = withRouteHandler(
132129

133130
return NextResponse.json({ success: true, warnings: result.warnings }, { status: 200 })
134131
} catch (error: any) {
135-
if (error instanceof WorkflowLockedError) {
136-
return NextResponse.json({ error: error.message }, { status: error.status })
137-
}
138-
139132
const elapsed = Date.now() - startTime
140133
logger.error(
141134
`[${requestId}] Error saving workflow ${workflowId} state after ${elapsed}ms`,

apps/sim/app/workspace/[workspaceId]/prefetch.ts

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -93,38 +93,22 @@ async function seedWorkspaceList(
9393
}
9494

9595
/**
96-
* How many files the layout is willing to inline into the document.
96+
* How many files the layout is willing to inline into the document. Seeded on EVERY
97+
* workspace route, so at ~500 bytes of JSON per file this budgets the entry at ~150 KB.
9798
*
98-
* The file list is seeded on EVERY workspace route (see the call site), so its cost is
99-
* paid per navigation into the app, not per visit to Files. At roughly 500 bytes of
100-
* serialized JSON per file, this budgets the entry at ~150 KB; a workspace with
101-
* thousands of files would otherwise push more than a megabyte of HTML ahead of first
102-
* paint on the logs, settings, and editor routes that never read it.
103-
*
104-
* A workspace above the budget seeds NOTHING rather than a prefix: the sidebar search
105-
* filters this list client-side and the Files browser renders it as the workspace's
106-
* files, so a truncated seed would silently hide files. Those workspaces fetch the
107-
* complete list from the route instead — which for a list that large is also the
108-
* cheaper first paint.
99+
* A workspace above the budget seeds NOTHING rather than a prefix: the sidebar filters
100+
* this list client-side, so a truncated seed would silently hide files.
109101
*/
110102
export const WORKSPACE_FILE_SEED_MAX = 300
111103

112104
/**
113-
* Seeds the workspace's file list, which the sidebar's search modal reads on EVERY
114-
* workspace route — so this query is registered by sidebar chrome before any page
115-
* renders. That ordering is why it has to be seeded HERE and not only by the Files
116-
* pages: `HydrationBoundary` hydrates a query the cache has already seen from a
117-
* `useEffect`, which never runs during SSR, so a page-level boundary can only ever hand
118-
* this entry to the client. Seeding it with the layout's own boundary — the first one to
119-
* render — is what lets the server paint the Files browser and the open file's header
120-
* populated instead of shipping a spinner and resolving it a beat later on the client.
121-
*
122-
* Seeded rather than prefetched so it can decline to create an entry at all when the
123-
* workspace exceeds {@link WORKSPACE_FILE_SEED_MAX}: `prefetchQuery` always creates one,
124-
* and a partial one would be read as the whole list.
105+
* Seeds the workspace's file list, which sidebar chrome registers on EVERY workspace
106+
* route. It must be seeded HERE, not by the Files pages: `HydrationBoundary` defers a
107+
* query the cache has already seen to a `useEffect`, which SSR never runs.
125108
*
126-
* Parsed through the same response contract `GET /api/workspaces/[id]/files` validates
127-
* against, so a seeded entry is identical to what the client hook would cache.
109+
* Seeded rather than prefetched so it can decline to create an entry at all above
110+
* {@link WORKSPACE_FILE_SEED_MAX} — `prefetchQuery` always creates one, and a partial
111+
* entry would be read as the whole list. Parsed through the route's response contract.
128112
*/
129113
async function seedWorkspaceFiles(queryClient: QueryClient, workspaceId: string): Promise<void> {
130114
try {

apps/sim/executor/utils/provider-request.ts

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,32 +11,17 @@ interface ExecuteBlockProviderRequestInput {
1111
ctx: ExecutionContext
1212
providerId: string
1313
request: ProviderRequest
14-
/**
15-
* The fork the block's model input was projected through. Supplied to the
16-
* provider runtime in place of the provenance envelope the HTTP boundary used
17-
* to serialize and re-import.
18-
*/
14+
/** Supplied in place of the provenance envelope the HTTP boundary serialized and re-imported. */
1915
resolvedSecretTraceRegistry: ResolvedSecretTraceRegistry | undefined
2016
}
2117

2218
/**
23-
* Runs one non-streaming provider request for a block handler in-process.
24-
*
25-
* Replaces the executor's `POST /api/providers` round trip, which re-derived
26-
* everything it needed from claims the executor had itself just supplied. The
27-
* two admission checks the route owned are reproduced here so the outcome is
28-
* unchanged:
29-
*
30-
* - `checkInternalAuth` rejected a token carrying no user. The executor mints
31-
* that token from `ctx.userId`, so the check reduces to requiring one.
32-
* - `checkWorkspaceAccess` rejected an execution subject who is no longer a
33-
* member of the workspace being billed.
34-
*
35-
* The route's remaining work is either already done by the caller (the model
36-
* permission policy, via `validateModelProvider`; Vertex credential
37-
* authorization, via `resolveVertexCredential`) or lives inside
38-
* `executeProviderRequest` itself (BYOK key resolution, attachment provenance
39-
* filtering, cost policy).
19+
* Runs one non-streaming provider request for a block handler in-process, replacing the
20+
* executor's `POST /api/providers` round trip. The route's two admission checks are
21+
* reproduced so the outcome is unchanged: an internal token with no user is rejected (the
22+
* executor mints it from `ctx.userId`), and an execution subject who has left the billed
23+
* workspace is rejected. The route's remaining work is already done by the caller or lives
24+
* inside `executeProviderRequest`.
4025
*/
4126
export async function executeBlockProviderRequest({
4227
ctx,
@@ -56,16 +41,10 @@ export async function executeBlockProviderRequest({
5641
}
5742

5843
/**
59-
* `executionContext` is deliberately not supplied: it is only inherited by
60-
* model-emitted tool calls, and the route this replaces never carried one.
61-
* Router and evaluator requests declare no tools, so passing the executor's
62-
* context here would widen the trusted surface without changing any outcome.
63-
*
64-
* The whole runtime context is omitted when there is no registry, rather than
65-
* passed carrying `undefined`. `executeProviderTool` reads a present context with
66-
* an absent registry as "provenance was expected and is missing" and fails the
67-
* call closed with no error text — unreachable while these blocks declare no
68-
* tools, but a silent failure the day one does.
44+
* No `executionContext`: it is only inherited by model-emitted tool calls, and the route
45+
* this replaces never carried one. The whole context is omitted when there is no registry
46+
* rather than passed carrying `undefined` — `executeProviderTool` reads that as missing
47+
* provenance and fails the call closed with no error text.
6948
*/
7049
const response = await executeProviderRequest(
7150
providerId,

apps/sim/hooks/queries/schedules.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,7 @@ export function useRedeployWorkflowSchedule() {
249249
const { workflowId, blockId } = data
250250
await Promise.all([
251251
queryClient.invalidateQueries({ queryKey: scheduleKeys.schedule(workflowId, blockId) }),
252-
/**
253-
* A redeploy recreates the schedule, so the id-keyed reads go stale too. They are
254-
* a separate subtree from `schedule(workflowId, blockId)`, which does not cover them.
255-
*/
252+
/** A redeploy recreates the schedule; the id-keyed reads are a separate subtree. */
256253
queryClient.invalidateQueries({ queryKey: scheduleKeys.byIds() }),
257254
queryClient.invalidateQueries({ queryKey: deploymentKeys.info(workflowId) }),
258255
queryClient.invalidateQueries({ queryKey: deploymentKeys.versions(workflowId) }),

apps/sim/hooks/queries/tables.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -837,10 +837,6 @@ function withOptimisticAutoFireExec(groups: WorkflowGroup[], row: TableRow): Tab
837837
* shared parent, and handing this updater a `find` entry — a flat
838838
* {@link TableFindResult}, not pages — throws on `old.pages` inside `onMutate`, so
839839
* the whole cell edit would reject before reaching the server.
840-
*
841-
* A consequence worth knowing: an open search-results view is therefore left to its
842-
* own refetch rather than patched here, since it holds a different shape. Patching
843-
* it too would need its own updater keyed on {@link tableKeys.find}.
844840
*/
845841
function patchCachedRows(
846842
queryClient: ReturnType<typeof useQueryClient>,

apps/sim/hooks/queries/utils/invalidate-usage.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ import { workspaceUsageKeys } from '@/hooks/queries/utils/workspace-usage-keys'
99
const USAGE_SETTLE_DELAY_MS = 1000
1010

1111
/**
12-
* Invalidates the workspace credit/usage reads after anything that moves the balance —
13-
* a run that spends credits, a top-up, a plan change, or a usage-limit edit. Both
14-
* families are keyed per workspace but derive from the same billing account, so the
15-
* family prefixes (not a single workspace's key) are what has to be refetched.
12+
* Invalidates the workspace credit/usage reads after anything that moves the balance.
13+
* Both families are keyed per workspace but derive from one billing account, so the
14+
* family prefixes are what must refetch.
1615
*/
1716
export function invalidateWorkspaceUsage(queryClient: QueryClient) {
1817
return Promise.all([
@@ -23,9 +22,7 @@ export function invalidateWorkspaceUsage(queryClient: QueryClient) {
2322

2423
/**
2524
* Refreshes the billing reads a run touches, after {@link USAGE_SETTLE_DELAY_MS}.
26-
*
27-
* Shared by the surfaces that spend credits — workflow execution and wand generation —
28-
* so the delay and the key set stay in one place.
25+
* Shared by workflow execution and wand generation.
2926
*/
3027
export function scheduleUsageRefresh(queryClient: QueryClient) {
3128
setTimeout(() => {

apps/sim/hooks/queries/utils/subscription-keys.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
/**
2-
* React Query key factory for subscription and billing reads.
3-
*
4-
* Lives in this standalone module — like {@link file://./workspace-usage-keys.ts} — so
5-
* the shared billing invalidations can reference it without importing the hook module
6-
* that consumes those invalidations, which would close an import cycle between the two.
2+
* React Query key factory for subscription and billing reads. Standalone so the shared
3+
* billing invalidations can use it without closing an import cycle through the hook module.
74
*/
85
export const subscriptionKeys = {
96
all: ['subscription'] as const,

apps/sim/hooks/queries/utils/table-keys.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,9 @@ export const tableKeys = {
2626
[...tableKeys.all, 'export-jobs', workspaceId ?? ''] as const,
2727
rowsRoot: (tableId: string) => [...tableKeys.detail(tableId), 'rows'] as const,
2828
/**
29-
* Prefix covering only the paged row lists.
30-
*
31-
* `rowsRoot` is a shared parent — `find` hangs off it holding an entirely different
32-
* shape — so anything walking the cache to update or snapshot row pages must start
33-
* here instead. Reaching for `rowsRoot` and subtracting the siblings is a denylist
34-
* that rots the moment another subtree is added.
29+
* Prefix covering only the paged row lists. `rowsRoot` is a shared parent — `find`
30+
* hangs off it holding a different shape — so anything walking the cache for row
31+
* pages must start here.
3532
*/
3633
infiniteRowsRoot: (tableId: string) => [...tableKeys.rowsRoot(tableId), 'infinite'] as const,
3734
infiniteRows: (tableId: string, paramsKey: string) =>

apps/sim/hooks/queries/utils/workspace-usage-keys.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
/**
2-
* React Query key factory for the per-workspace credit and usage-gate reads.
3-
*
4-
* Standalone for the same reason as {@link file://./subscription-keys.ts}: the shared
5-
* billing invalidations need these keys without importing the hooks that call them.
2+
* React Query key factory for the per-workspace credit and usage-gate reads. Standalone
3+
* for the same import-cycle reason as {@link file://./subscription-keys.ts}.
64
*/
75
export const workspaceUsageKeys = {
86
all: ['workspace-usage'] as const,

0 commit comments

Comments
 (0)