chore(perf): trim verbose comments to terse why-notes

Author: waleedlatif1

apps/sim/lib/api-key/byok.ts

@@ -37,10 +37,8 @@ function nextRotationIndex(poolKey: string, poolSize: number): number {
  * creation order. A key that fails to decrypt is skipped in favor of the next
  * one in the pool.
  *
- * The key list is read fresh from the database on every call rather than
- * cached: BYOK lookups are not a hot database query, and reading fresh keeps
- * key revocation/rotation effective immediately across every ECS task with no
- * cross-instance cache-coherence concern.
+ * The key list is read fresh every call (not cached): BYOK is not a hot query,
+ * and reading fresh keeps revocation immediate across ECS tasks.
  */
 export async function getBYOKKey(
   workspaceId: string | undefined | null,

apps/sim/lib/billing/calculations/usage-monitor.ts

@@ -467,10 +467,9 @@ export async function checkOrgMemberUsageLimit(
       return { isExceeded: false, currentUsage: 0, limit: null }
     }
 
-    // Resolve the member cap first and short-circuit when none is set — the
-    // common case. Computing usage is only worthwhile once a cap exists, so the
-    // two queries stay sequential rather than racing (parallelizing would add a
-    // usage query on every uncapped member's execution).
+    // Resolve the cap first and short-circuit when unset (the common case); only
+    // then is computing usage worthwhile. Kept sequential, not raced, to avoid a
+    // usage query on every uncapped member's execution.
     const limit = await getOrgMemberUsageLimit(organizationId, userId)
     if (limit === null) {
       return { isExceeded: false, currentUsage: 0, limit: null }

apps/sim/lib/billing/core/plan.test.ts

@@ -127,7 +127,6 @@ describe('getHighestPrioritySubscription', () => {
 
     const result = await getHighestPrioritySubscription('user-1')
 
-    // Enterprise (org) always wins over Pro (personal).
     expect(result?.id).toBe('sub-org-enterprise')
   })
 
@@ -147,7 +146,7 @@ describe('getHighestPrioritySubscription', () => {
 
   it('returns the personal sub and skips org follow-ups when there are no memberships', async () => {
     queue('subscription', [personalPro('user-1')])
-    queue('member', []) // no org memberships
+    queue('member', [])
 
     const result = await getHighestPrioritySubscription('user-1')
 
@@ -159,18 +158,18 @@ describe('getHighestPrioritySubscription', () => {
   })
 
   it('returns null when neither personal nor org subscriptions exist', async () => {
-    queue('subscription', []) // no personal subs
-    queue('member', []) // no memberships
+    queue('subscription', [])
+    queue('member', [])
 
     const result = await getHighestPrioritySubscription('user-1')
 
     expect(result).toBeNull()
   })
 
   it('excludes orphaned org memberships whose organization row no longer exists', async () => {
-    queue('subscription', []) // no personal subs
+    queue('subscription', [])
     queue('member', [{ organizationId: 'ghost-org' }]) // membership points at a deleted org
-    queue('organization', []) // org-existence returns nothing -> orphaned
+    queue('organization', [])
 
     const result = await getHighestPrioritySubscription('user-1')
 
@@ -184,7 +183,7 @@ describe('getHighestPrioritySubscription', () => {
   it('falls back to the personal sub when the only org is orphaned', async () => {
     queue('subscription', [personalPro('user-1')])
     queue('member', [{ organizationId: 'ghost-org' }])
-    queue('organization', []) // orphaned org
+    queue('organization', [])
 
     const result = await getHighestPrioritySubscription('user-1')
 

apps/sim/lib/execution/preprocessing.ts

@@ -323,11 +323,9 @@ export async function preprocessExecution(
   }
 
   // ========== STEPS 3.5–6: Preflight Gates ==========
-  // The read-only gates run concurrently to cut latency: ban + subscription
-  // together, then usage (which needs the subscription). The rate-limit gate is
-  // stateful — it debits a token — so it runs sequentially only after ban and
-  // usage pass. Failures apply in fixed precedence (ban 403 → usage 402 → rate
-  // 429), and the sole write (the STEP 7 reservation) stays last.
+  // Read-only gates run concurrently (ban + subscription, then usage). The
+  // rate-limit gate debits a token, so it runs sequentially only after ban and
+  // usage pass. Failures apply in fixed precedence: ban 403 → usage 402 → rate 429.
 
   /**
    * A failing gate's deferred outcome: the response to return, plus an optional

apps/sim/lib/workflows/persistence/utils.ts

@@ -100,39 +100,21 @@ export async function blockExistsInDeployment(
   }
 }
 
-/**
- * Each entry is keyed by an immutable `deploymentVersionId` and holds a
- * fully-migrated {@link DeployedWorkflowData} snapshot (tens of KB to ~1MB);
- * the bound keeps worst-case memory within a sane envelope.
- */
 const DEPLOYED_STATE_CACHE_MAX_ENTRIES = 500
 const DEPLOYED_STATE_CACHE_TTL_MS = 5 * 60 * 1000
 
 /**
- * Process-local cache of fully-loaded, post-migration deployed workflow state,
- * keyed by the immutable `deploymentVersionId`.
- *
- * The id is unique per deploy — a redeploy mints a new id and a rollback
- * reactivates an existing id — so the active-version lookup naturally selects a
- * different (or already-cached) key whenever the active deployment changes,
- * making the cache self-invalidating across redeploy/rollback.
- *
- * The TTL is absolute (not reset on read) on purpose: it bounds the one piece
- * of the cached state that is not strictly immutable — `applyBlockMigrations`
- * resolves legacy credential references via a live lookup — so a credential
- * change propagates across ECS tasks even for a continuously-running workflow.
+ * Caches post-migration deployed state by the immutable `deploymentVersionId`, so
+ * a redeploy/rollback (which changes the active id) self-invalidates. The TTL is
+ * absolute on purpose — it bounds the one non-immutable part, the live credential
+ * remap in `applyBlockMigrations` — so credential changes still propagate.
  */
 const deployedStateCache = new LRUCache<string, DeployedWorkflowData>({
   max: DEPLOYED_STATE_CACHE_MAX_ENTRIES,
   ttl: DEPLOYED_STATE_CACHE_TTL_MS,
 })
 
-/**
- * Drop cached deployed state. Pass a `deploymentVersionId` to evict a single
- * entry, or omit it to clear the entire cache. Explicit invalidation is not
- * required for correctness — redeploy/rollback change the active id and key the
- * cache anew — but the helper is exported for completeness and testing.
- */
+/** Evicts one deployed-state entry, or clears the cache when no id is given. */
 export function invalidateDeployedStateCache(deploymentVersionId?: string): void {
   if (deploymentVersionId) {
     deployedStateCache.delete(deploymentVersionId)

apps/sim/providers/bedrock/index.ts

@@ -139,10 +139,8 @@ export const bedrockProvider: ProviderConfig = {
       }
     }
 
-    // Memoized: each BedrockRuntimeClient owns its own connection pool (AWS SDK
-    // best practice is to reuse the client), so reusing it keeps connections warm
-    // across requests. Keyed by region + credential identity (a rotated key pair
-    // changes the access key id and so yields a fresh client).
+    // AWS SDK clients own a per-client connection pool and are meant to be reused.
+    // Keyed by region + credential identity (a rotated key pair yields a new key).
     const client = getCachedProviderClient(
       `bedrock::${region}::${request.bedrockAccessKeyId ?? 'default-chain'}`,
       () => new BedrockRuntimeClient(clientConfig)

apps/sim/providers/client-cache.ts

@@ -1,35 +1,21 @@
 import { LRUCache } from 'lru-cache'
 
-/**
- * Shared, bounded, idle-expiring cache of provider SDK clients. Reusing a client
- * across requests lets the underlying HTTP agent keep connections alive, avoiding
- * a fresh TLS handshake (and client construction) on every request.
- *
- * Provider SDK clients (Anthropic, OpenAI, Groq, …) hold no per-request mutable
- * state — abort signals and timeouts are passed at the call site, not on the
- * client — so a single instance is safe to share across concurrent requests.
- *
- * Keys must be namespaced per provider and must encode every input that varies
- * the constructed client. The API key is always part of the key, making it the
- * tenant security boundary: clients are never shared across different keys.
- */
-
 const CLIENT_CACHE_MAX_ENTRIES = 1_000
 const CLIENT_CACHE_TTL_MS = 30 * 60 * 1_000
 
+// updateAgeOnGet makes the TTL idle-based, so a continuously-used client keeps
+// its warm keep-alive connections while idle keys age out.
 const clientCache = new LRUCache<string, object>({
   max: CLIENT_CACHE_MAX_ENTRIES,
   ttl: CLIENT_CACHE_TTL_MS,
-  // Idle expiry: the TTL resets on every hit so a continuously-used client
-  // (and its warm keep-alive connections) survives, while idle keys age out.
   updateAgeOnGet: true,
 })
 
 /**
- * Returns a cached provider client for the given key, constructing and storing
- * one via `factory` on a miss. The key must encode every input that varies the
- * constructed client (provider namespace + API key at minimum); identical keys
- * safely share a single client instance.
+ * Memoizes provider SDK clients so connections stay warm across requests rather
+ * than re-handshaking per call. The key must be namespaced per provider and
+ * encode every input that varies the client; the API key is always part of it,
+ * making it the tenant boundary (clients are never shared across keys).
  */
 export function getCachedProviderClient<T extends object>(key: string, factory: () => T): T {
   const existing = clientCache.get(key)

apps/sim/providers/vllm/index.ts

@@ -135,9 +135,8 @@ export const vllmProvider: ProviderConfig = {
     }
 
     const apiKey = request.apiKey || env.VLLM_API_KEY || 'empty'
-    // Memoized: a pinned endpoint gets its own undici Agent per call, so reusing
-    // the client (keyed by the resolved IP) keeps its connections warm. DNS
-    // re-validation still runs every request; a new IP yields a new key/client.
+    // A pinned endpoint gets its own undici Agent, so reuse keeps connections
+    // warm. DNS re-validation still runs every request; a new IP rekeys.
     const vllm = getCachedProviderClient(
       `vllm::${apiKey}::${baseUrl}::${pinnedIP ?? 'no-pin'}`,
       () =>