Skip to content

Commit

Permalink
Fix: include file action use the same list of files as list files act…
Browse files Browse the repository at this point in the history 
…ion (#9106)

* Fix: include file action use the same list of files as list files action

* Review fdbk
  • Loading branch information
@Fraggle
Fraggle authored Dec 3, 2024
1 parent 0248150 commit 474bbf9
Show file tree
Hide file tree
Showing 5 changed files with 217 additions and 202 deletions.
112 changes: 43 additions & 69 deletions front/lib/api/assistant/actions/conversation/include_file.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type {
AgentActionSpecification,
ContentFragmentType,
ConversationIncludeFileActionType,
ConversationIncludeFileConfigurationType,
ConversationIncludeFileErrorEvent,
Expand All @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
129 changes: 2 additions & 127 deletions front/lib/api/assistant/jit_actions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import type {
ModelMessageTypeMultiActions,
Result,
RetrievalConfigurationType,
SupportedContentFragmentType,
TablesQueryConfigurationType,
} from "@dust-tt/types";
import {
Expand All @@ -22,7 +21,6 @@ import {
isAgentMessageType,
isContentFragmentMessageTypeModel,
isContentFragmentType,
isSupportedImageContentType,
isUserMessageType,
Ok,
removeNulls,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
{
Expand Down
Loading

0 comments on commit 474bbf9

Please sign in to comment.