From 474bbf9d90b4deee6ef2b10cf083736271edbdbd Mon Sep 17 00:00:00 2001 From: Sebastien Flory Date: Tue, 3 Dec 2024 22:26:25 +0100 Subject: [PATCH] Fix: include file action use the same list of files as list files action (#9106) * Fix: include file action use the same list of files as list files action * Review fdbk --- .../actions/conversation/include_file.ts | 112 +++++------- front/lib/api/assistant/jit_actions.ts | 129 +------------- front/lib/api/assistant/jit_utils.ts | 162 ++++++++++++++++++ .../resources/content_fragment_resource.ts | 10 +- .../actions/conversation/list_files.ts | 6 +- 5 files changed, 217 insertions(+), 202 deletions(-) create mode 100644 front/lib/api/assistant/jit_utils.ts diff --git a/front/lib/api/assistant/actions/conversation/include_file.ts b/front/lib/api/assistant/actions/conversation/include_file.ts index 1d189650a53f..880413e9f3b9 100644 --- a/front/lib/api/assistant/actions/conversation/include_file.ts +++ b/front/lib/api/assistant/actions/conversation/include_file.ts @@ -1,6 +1,5 @@ import type { AgentActionSpecification, - ContentFragmentType, ConversationIncludeFileActionType, ConversationIncludeFileConfigurationType, ConversationIncludeFileErrorEvent, @@ -12,16 +11,9 @@ import type { ModelConfigurationType, ModelId, Result, - SupportedContentFragmentType, } from "@dust-tt/types"; import { CoreAPI, Err, Ok } from "@dust-tt/types"; -import { - assertNever, - BaseAction, - isContentFragmentType, - isSupportedImageContentType, - isTextContent, -} from "@dust-tt/types"; +import { BaseAction, isTextContent } from "@dust-tt/types"; import { DEFAULT_CONVERSATION_INCLUDE_FILE_ACTION_DESCRIPTION, @@ -31,41 +23,18 @@ import { } from "@app/lib/api/assistant/actions/constants"; import type { BaseActionRunParams } from "@app/lib/api/assistant/actions/types"; import { BaseActionConfigurationServerRunner } from "@app/lib/api/assistant/actions/types"; +import { listFiles } from "@app/lib/api/assistant/jit_utils"; import config from "@app/lib/api/config"; import { getSupportedModelConfig } from "@app/lib/assistant"; import type { Authenticator } from "@app/lib/auth"; import { AgentConversationIncludeFileAction } from "@app/lib/models/assistant/actions/conversation/include_file"; -import { renderContentFragmentForModel } from "@app/lib/resources/content_fragment_resource"; +import { + CONTENT_OUTDATED_MSG, + renderFromFileId, +} from "@app/lib/resources/content_fragment_resource"; import { generateRandomModelSId } from "@app/lib/resources/string_ids"; import logger from "@app/logger/logger"; -export function isConversationIncludableFileContentType( - contentType: SupportedContentFragmentType -): boolean { - if (isSupportedImageContentType(contentType)) { - return false; - } - // For now we only allow including text files. - switch (contentType) { - case "application/msword": - case "application/vnd.openxmlformats-officedocument.wordprocessingml.document": - case "application/pdf": - case "text/markdown": - case "text/plain": - case "dust-application/slack": - case "text/vnd.dust.attachment.slack.thread": - case "text/comma-separated-values": - case "text/csv": - return true; - - case "text/tab-separated-values": - case "text/tsv": - return false; - default: - assertNever(contentType); - } -} - interface ConversationIncludeFileActionBlob { id: ModelId; agentMessageId: ModelId; @@ -121,41 +90,46 @@ export class ConversationIncludeFileAction extends BaseAction { // be able to undertstand the state of affair. We use content.flat() to consider all versions of // messages here (to support rendering a file that was part of an old version of a previous // message). - const m = (conversation.content.flat().find((m) => { - if ( - isContentFragmentType(m) && - isConversationIncludableFileContentType(m.contentType) && - m.fileId === fileId - ) { - return true; + const files = listFiles(conversation); + for (const f of files) { + if (f.fileId === fileId && f.isIncludable) { + if (f.contentFragmentVersion === "superseded") { + return new Ok({ + fileId, + title: f.title, + content: CONTENT_OUTDATED_MSG, + }); + } + + const r = await renderFromFileId(conversation.owner, { + contentType: f.contentType, + excludeImages: true, + fileId: f.fileId, + forceFullCSVInclude: true, // We are using JIT so if we are rendering a CSV file we want to include it in full + model, + title: f.title, + textBytes: null, + contentFragmentVersion: f.contentFragmentVersion, + }); + + if (r.isErr()) { + return new Err(`${r.error}`); + } + if (!isTextContent(r.value.content[0])) { + return new Err(`File \`${fileId}\` has no text content`); + } + + return new Ok({ + fileId, + title: f.title, + content: r.value.content[0].text, + }); } - return false; - }) || null) as ContentFragmentType | null; - - if (!m) { - return new Err( - `File \`${fileId}\` not includable or not found in conversation` - ); } - const rRes = await renderContentFragmentForModel(m, conversation, model, { - // We're not supposed to get images here and we would not know what to do with them. - excludeImages: true, - }); - - if (rRes.isErr()) { - return new Err(`${rRes.error}`); - } - if (!isTextContent(rRes.value.content[0])) { - return new Err(`File \`${fileId}\` has no text content`); - } - const text = rRes.value.content[0].text; - - return new Ok({ - fileId, - title: m.title, - content: text, - }); + return new Err( + `File \`${fileId}\` not includable or not found in conversation` + ); } renderForFunctionCall(): FunctionCallType { diff --git a/front/lib/api/assistant/jit_actions.ts b/front/lib/api/assistant/jit_actions.ts index 4c10fa49599d..0cce87335468 100644 --- a/front/lib/api/assistant/jit_actions.ts +++ b/front/lib/api/assistant/jit_actions.ts @@ -13,7 +13,6 @@ import type { ModelMessageTypeMultiActions, Result, RetrievalConfigurationType, - SupportedContentFragmentType, TablesQueryConfigurationType, } from "@dust-tt/types"; import { @@ -22,7 +21,6 @@ import { isAgentMessageType, isContentFragmentMessageTypeModel, isContentFragmentType, - isSupportedImageContentType, isUserMessageType, Ok, removeNulls, @@ -36,11 +34,9 @@ import { DEFAULT_CONVERSATION_SEARCH_ACTION_DATA_DESCRIPTION, DEFAULT_CONVERSATION_SEARCH_ACTION_NAME, } from "@app/lib/api/assistant/actions/constants"; -import { - isConversationIncludableFileContentType, - makeConversationIncludeFileConfiguration, -} from "@app/lib/api/assistant/actions/conversation/include_file"; +import { makeConversationIncludeFileConfiguration } from "@app/lib/api/assistant/actions/conversation/include_file"; import { makeConversationListFilesAction } from "@app/lib/api/assistant/actions/conversation/list_files"; +import { listFiles } from "@app/lib/api/assistant/jit_utils"; import { getTextContentFromMessage, getTextRepresentationFromMessages, @@ -68,127 +64,6 @@ export async function isJITActionsEnabled( return use; } -function isQueryableContentType( - contentType: SupportedContentFragmentType -): boolean { - if (isSupportedImageContentType(contentType)) { - return false; - } - // For now we only allow including text files. - switch (contentType) { - case "application/msword": - case "application/vnd.openxmlformats-officedocument.wordprocessingml.document": - case "application/pdf": - case "text/markdown": - case "text/plain": - case "dust-application/slack": - case "text/vnd.dust.attachment.slack.thread": - case "text/tab-separated-values": - case "text/tsv": - return false; - - case "text/comma-separated-values": - case "text/csv": - return true; - default: - assertNever(contentType); - } -} - -function isSearchableContentType( - contentType: SupportedContentFragmentType -): boolean { - if (isSupportedImageContentType(contentType)) { - return false; - } - // For now we only allow including text files. - switch (contentType) { - case "application/msword": - case "application/vnd.openxmlformats-officedocument.wordprocessingml.document": - case "application/pdf": - case "text/markdown": - case "text/plain": - case "dust-application/slack": - case "text/vnd.dust.attachment.slack.thread": - case "text/tab-separated-values": - case "text/tsv": - return true; - - case "text/comma-separated-values": - case "text/csv": - return false; - default: - assertNever(contentType); - } -} - -function isListableContentType( - contentType: SupportedContentFragmentType -): boolean { - // We allow listing all content-types that are not images. Note that - // `isSupportedPlainTextContentType` is not enough because it is limited to uploadable (as in from - // the conversation) content types which does not cover all non image content types that we - // support in the API such as `dust-application/slack`. - return !isSupportedImageContentType(contentType); -} - -export function listFiles( - conversation: ConversationType -): ConversationFileType[] { - const files: ConversationFileType[] = []; - for (const versions of conversation.content) { - const m = versions[versions.length - 1]; - - if ( - isContentFragmentType(m) && - isListableContentType(m.contentType) && - m.contentFragmentVersion === "latest" - ) { - if (m.fileId) { - const canDoJIT = m.snippet !== null; - const isIncludable = isConversationIncludableFileContentType( - m.contentType - ); - const isQueryable = canDoJIT && isQueryableContentType(m.contentType); - const isSearchable = canDoJIT && isSearchableContentType(m.contentType); - - files.push({ - fileId: m.fileId, - title: m.title, - contentType: m.contentType, - snippet: m.snippet, - isIncludable, - isQueryable, - isSearchable, - }); - } - } else if (isAgentMessageType(m)) { - const generatedFiles = m.actions.flatMap((a) => a.getGeneratedFiles()); - - for (const f of generatedFiles) { - const canDoJIT = f.snippet != null; - const isIncludable = isConversationIncludableFileContentType( - f.contentType - ); - const isQueryable = canDoJIT && isQueryableContentType(f.contentType); - const isSearchable = canDoJIT && isSearchableContentType(f.contentType); - - files.push({ - fileId: f.fileId, - contentType: f.contentType, - title: f.title, - snippet: f.snippet, - isIncludable, - isQueryable, - isSearchable, - }); - } - } - } - - return files; -} - async function getJITActions( auth: Authenticator, { diff --git a/front/lib/api/assistant/jit_utils.ts b/front/lib/api/assistant/jit_utils.ts new file mode 100644 index 000000000000..ca043efee2b2 --- /dev/null +++ b/front/lib/api/assistant/jit_utils.ts @@ -0,0 +1,162 @@ +import type { + ConversationFileType, + ConversationType, + SupportedContentFragmentType, +} from "@dust-tt/types"; +import { + assertNever, + isAgentMessageType, + isContentFragmentType, + isSupportedImageContentType, +} from "@dust-tt/types"; + +function isConversationIncludableFileContentType( + contentType: SupportedContentFragmentType +): boolean { + if (isSupportedImageContentType(contentType)) { + return false; + } + // For now we only allow including text files. + switch (contentType) { + case "application/msword": + case "application/vnd.openxmlformats-officedocument.wordprocessingml.document": + case "application/pdf": + case "text/markdown": + case "text/plain": + case "dust-application/slack": + case "text/vnd.dust.attachment.slack.thread": + case "text/comma-separated-values": + case "text/csv": + return true; + + case "text/tab-separated-values": + case "text/tsv": + return false; + default: + assertNever(contentType); + } +} + +function isQueryableContentType( + contentType: SupportedContentFragmentType +): boolean { + if (isSupportedImageContentType(contentType)) { + return false; + } + // For now we only allow including text files. + switch (contentType) { + case "application/msword": + case "application/vnd.openxmlformats-officedocument.wordprocessingml.document": + case "application/pdf": + case "text/markdown": + case "text/plain": + case "dust-application/slack": + case "text/vnd.dust.attachment.slack.thread": + case "text/tab-separated-values": + case "text/tsv": + return false; + + case "text/comma-separated-values": + case "text/csv": + return true; + default: + assertNever(contentType); + } +} + +function isSearchableContentType( + contentType: SupportedContentFragmentType +): boolean { + if (isSupportedImageContentType(contentType)) { + return false; + } + // For now we only allow including text files. + switch (contentType) { + case "application/msword": + case "application/vnd.openxmlformats-officedocument.wordprocessingml.document": + case "application/pdf": + case "text/markdown": + case "text/plain": + case "dust-application/slack": + case "text/vnd.dust.attachment.slack.thread": + case "text/tab-separated-values": + case "text/tsv": + return true; + + case "text/comma-separated-values": + case "text/csv": + return false; + default: + assertNever(contentType); + } +} + +function isListableContentType( + contentType: SupportedContentFragmentType +): boolean { + // We allow listing all content-types that are not images. Note that + // `isSupportedPlainTextContentType` is not enough because it is limited to uploadable (as in from + // the conversation) content types which does not cover all non image content types that we + // support in the API such as `dust-application/slack`. + return !isSupportedImageContentType(contentType); +} + +// Moved to a separate file to avoid circular dependency issue. +export function listFiles( + conversation: ConversationType +): ConversationFileType[] { + const files: ConversationFileType[] = []; + for (const versions of conversation.content) { + const m = versions[versions.length - 1]; + + if ( + isContentFragmentType(m) && + isListableContentType(m.contentType) && + m.contentFragmentVersion === "latest" + ) { + if (m.fileId) { + const canDoJIT = m.snippet !== null; + const isIncludable = isConversationIncludableFileContentType( + m.contentType + ); + const isQueryable = canDoJIT && isQueryableContentType(m.contentType); + const isSearchable = canDoJIT && isSearchableContentType(m.contentType); + + files.push({ + fileId: m.fileId, + title: m.title, + contentType: m.contentType, + snippet: m.snippet, + contentFragmentVersion: m.contentFragmentVersion, + isIncludable, + isQueryable, + isSearchable, + }); + } + } else if (isAgentMessageType(m)) { + const generatedFiles = m.actions.flatMap((a) => a.getGeneratedFiles()); + + for (const f of generatedFiles) { + const canDoJIT = f.snippet != null; + const isIncludable = isConversationIncludableFileContentType( + f.contentType + ); + const isQueryable = canDoJIT && isQueryableContentType(f.contentType); + const isSearchable = canDoJIT && isSearchableContentType(f.contentType); + + files.push({ + fileId: f.fileId, + contentType: f.contentType, + title: f.title, + snippet: f.snippet, + contentFragmentVersion: "latest", + isIncludable, + isQueryable, + isSearchable, + }); + } + } + } + + return files; +} diff --git a/front/lib/resources/content_fragment_resource.ts b/front/lib/resources/content_fragment_resource.ts index dee9a5380ae3..fb3ef5312276 100644 --- a/front/lib/resources/content_fragment_resource.ts +++ b/front/lib/resources/content_fragment_resource.ts @@ -28,6 +28,8 @@ import type { ReadonlyAttributesType } from "@app/lib/resources/storage/types"; import { generateRandomModelSId } from "@app/lib/resources/string_ids"; import logger from "@app/logger/logger"; +export const CONTENT_OUTDATED_MSG = + "Content is outdated. Please refer to the latest version of this content."; const MAX_BYTE_SIZE_CSV_RENDER_FULL_CONTENT = 500 * 1024; // 500 KB // Attributes are marked as read-only to reflect the stateless nature of our Resource. @@ -376,7 +378,7 @@ async function getSignedUrlForProcessedContent( return getPrivateUploadBucket().getSignedUrl(fileCloudStoragePath); } -async function renderFromFileId( +export async function renderFromFileId( workspace: WorkspaceType, { contentType, @@ -568,8 +570,7 @@ export async function renderContentFragmentForModel( contentType, title, version: contentFragmentVersion, - content: - "Content is outdated. Please refer to the latest version of this content.", + content: CONTENT_OUTDATED_MSG, }), }, ], @@ -581,8 +582,7 @@ export async function renderContentFragmentForModel( contentType, excludeImages, fileId, - // If there is a snippet, it means that JIT was used, therefor if we are rendering the content fragment: we want to include the full content. - forceFullCSVInclude: message.snippet != null, + forceFullCSVInclude: false, model, title, textBytes, diff --git a/types/src/front/assistant/actions/conversation/list_files.ts b/types/src/front/assistant/actions/conversation/list_files.ts index a00af27055c2..0979f429c5d4 100644 --- a/types/src/front/assistant/actions/conversation/list_files.ts +++ b/types/src/front/assistant/actions/conversation/list_files.ts @@ -1,11 +1,15 @@ import { BaseAction } from "../../../../front/assistant/actions/index"; -import { SupportedContentFragmentType } from "../../../../front/content_fragment"; +import { + ContentFragmentVersion, + SupportedContentFragmentType, +} from "../../../../front/content_fragment"; import { ModelId } from "../../../../shared/model_id"; export type ConversationFileType = { fileId: string; title: string; contentType: SupportedContentFragmentType; + contentFragmentVersion: ContentFragmentVersion; snippet: string | null; isIncludable: boolean; isSearchable: boolean;