Skip to content

Commit a98f272

Browse files
committed
fix(review): simplify generated document attachments
1 parent 1270bcb commit a98f272

16 files changed

Lines changed: 118 additions & 958 deletions

File tree

apps/sim/app/api/files/serve/[...path]/route.ts

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ async function compileDocumentIfNeeded(
3737
raw: boolean,
3838
ownerKey: string | undefined,
3939
signal: AbortSignal | undefined
40-
): Promise<{ buffer: Buffer; contentType: string; unrendered?: boolean }> {
40+
): Promise<{ buffer: Buffer; contentType: string }> {
4141
if (raw) return { buffer, contentType: getContentType(filename) }
4242
return resolveServableDocBytes({
4343
rawBuffer: buffer,
@@ -67,10 +67,6 @@ const WORKSPACE_REVALIDATE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
6767
* bumps on every edit — so the browser may cache it indefinitely; re-opens and
6868
* focus refetches then resolve from cache with no round trip. Unversioned workspace
6969
* reads stay revalidated because the same storage key is edited in place.
70-
*
71-
* Callers pass `versioned && !unrendered`: a render that failed returns the stored
72-
* bytes as opaque data, and marking that immutable for a year would pin the failure
73-
* to the URL long after a later compile succeeds on the same version.
7470
*/
7571
function resolveServeCacheControl(
7672
versioned: boolean,
@@ -199,19 +195,22 @@ async function handleLocalFile(
199195
const segment = filename.split('/').pop() || filename
200196
const displayName = stripStorageKeyPrefix(segment)
201197
const workspaceId = getWorkspaceIdForCompile(filename)
202-
const {
203-
buffer: fileBuffer,
204-
contentType,
205-
unrendered,
206-
} = await compileDocumentIfNeeded(rawBuffer, displayName, workspaceId, raw, ownerKey, signal)
198+
const { buffer: fileBuffer, contentType } = await compileDocumentIfNeeded(
199+
rawBuffer,
200+
displayName,
201+
workspaceId,
202+
raw,
203+
ownerKey,
204+
signal
205+
)
207206

208207
logger.info('Local file served', { userId, filename, size: fileBuffer.length })
209208

210209
return createFileResponse({
211210
buffer: fileBuffer,
212211
contentType,
213212
filename: displayName,
214-
cacheControl: resolveServeCacheControl(versioned && !unrendered, contextParam),
213+
cacheControl: resolveServeCacheControl(versioned, contextParam),
215214
})
216215
} catch (error) {
217216
logger.error('Error reading local file:', error)
@@ -258,11 +257,14 @@ async function handleCloudProxy(
258257
const segment = cloudKey.split('/').pop() || 'download'
259258
const displayName = stripStorageKeyPrefix(segment)
260259
const workspaceId = getWorkspaceIdForCompile(cloudKey)
261-
const {
262-
buffer: fileBuffer,
263-
contentType,
264-
unrendered,
265-
} = await compileDocumentIfNeeded(rawBuffer, displayName, workspaceId, raw, ownerKey, signal)
260+
const { buffer: fileBuffer, contentType } = await compileDocumentIfNeeded(
261+
rawBuffer,
262+
displayName,
263+
workspaceId,
264+
raw,
265+
ownerKey,
266+
signal
267+
)
266268

267269
logger.info('Cloud file served', {
268270
userId,
@@ -275,7 +277,7 @@ async function handleCloudProxy(
275277
buffer: fileBuffer,
276278
contentType,
277279
filename: displayName,
278-
cacheControl: resolveServeCacheControl(versioned && !unrendered, context),
280+
cacheControl: resolveServeCacheControl(versioned, context),
279281
})
280282
} catch (error) {
281283
logger.error('Error downloading from cloud storage:', error)

apps/sim/executor/handlers/agent/agent-handler.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -783,9 +783,6 @@ export class AgentBlockHandler implements BlockHandler {
783783
userId: ctx.userId,
784784
logger,
785785
maxBytes: INLINE_ATTACHMENT_THRESHOLD_BYTES,
786-
// These files are about to become provider attachments, so a document that
787-
// is still compiling must fail loudly rather than reach the model empty.
788-
throwOnDocNotReady: true,
789786
})
790787

791788
const missingFile = hydratedFiles.find(

apps/sim/executor/handlers/mothership/mothership-handler.test.ts

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -588,56 +588,6 @@ describe('MothershipBlockHandler', () => {
588588
])
589589
})
590590

591-
it('attaches a generated document under its rendered MIME type, not its generation-source marker', async () => {
592-
const fileContent = Buffer.from('%PDF-1.4 ...', 'utf8').toString('base64')
593-
mockGenerateId.mockReturnValueOnce('chat-uuid')
594-
mockGenerateId.mockReturnValueOnce('message-uuid')
595-
mockGenerateId.mockReturnValueOnce('request-uuid')
596-
mockReadUserFileContent.mockResolvedValueOnce(fileContent)
597-
598-
fetchMock.mockResolvedValue(
599-
new Response(
600-
JSON.stringify({
601-
content: 'analyzed',
602-
model: 'mothership',
603-
conversationId: 'chat-uuid',
604-
tokens: {},
605-
toolCalls: [],
606-
}),
607-
{
608-
status: 200,
609-
headers: { 'Content-Type': 'application/json' },
610-
}
611-
)
612-
)
613-
614-
await handler.execute(context, block, {
615-
prompt: 'Analyze this file',
616-
files: [
617-
{
618-
name: 'report.pdf',
619-
key: 'workspace/workspace-1/report.pdf',
620-
size: 16,
621-
type: 'text/x-python-pdf',
622-
},
623-
],
624-
})
625-
626-
const [, options] = fetchMock.mock.calls[0] as [string, RequestInit]
627-
const body = JSON.parse(String(options.body))
628-
expect(body.fileAttachments).toEqual([
629-
{
630-
type: 'document',
631-
source: {
632-
type: 'base64',
633-
media_type: 'application/pdf',
634-
data: fileContent,
635-
},
636-
filename: 'report.pdf',
637-
},
638-
])
639-
})
640-
641591
it('propagates local aborts to the mothership request', async () => {
642592
const abortController = new AbortController()
643593
context.abortSignal = abortController.signal

apps/sim/executor/handlers/mothership/mothership-handler.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import type {
2323
StreamingExecution,
2424
} from '@/executor/types'
2525
import { buildAPIUrl, buildAuthHeaders, extractAPIErrorMessage } from '@/executor/utils/http'
26-
import { inferAttachmentMimeType } from '@/providers/attachments'
2726
import type { SerializedBlock } from '@/serializer/types'
2827

2928
const logger = createLogger('MothershipBlockHandler')
@@ -310,7 +309,7 @@ async function buildMothershipFileAttachments(
310309
maxSourceBytes: MAX_MOTHERSHIP_ATTACHMENT_BYTES,
311310
})
312311

313-
const content = createFileContentFromBase64(base64, inferAttachmentMimeType(userFile))
312+
const content = createFileContentFromBase64(base64, userFile.type)
314313
if (!content) {
315314
throw new Error(`File type is not supported for Mothership attachments: ${userFile.name}`)
316315
}

apps/sim/executor/variables/resolvers/reference-async.server.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,6 @@ async function hydrateExplicitBase64(
8181
allowLargeValueWorkflowScope: context.executionContext.allowLargeValueWorkflowScope,
8282
userId: context.executionContext.userId,
8383
maxBytes: context.executionContext.base64MaxBytes,
84-
// An explicit `<file.base64>` reference has no degraded mode — the resolver throws
85-
// when content is missing. Opt in so a still-compiling document reports that
86-
// instead of the generic size/availability message below.
87-
throwOnDocNotReady: true,
8884
})
8985
if (!hydrated.base64) {
9086
throw new Error(

0 commit comments

Comments
 (0)