Conversation actions scafolding + initial list_files action #8646

spolu · 2024-11-14T14:01:12Z

Description

Introduce a new conversaton_list_files_action which is never stored in DB but emulated as first tool use for the current agent message if there are files present in the conversation.

This is an attempt at a slightly more principled approach to injecting that emulated actions to be compared to a more iterative approach (from viz current state) proposed here: #8643. It does require a bit more code to fully represent the new AgentActionType but it allows the injection of the emulated action directly in the current agentMessage by prepending to any already run actions to properly always inject as first action.

When the flag jit_conversations_actions is enabled we rely on this new emulated action to make the list of files in the conversations available to the agent in the context of visualization.

Risk

N/A behind flag

Deploy Plan

deploy front

spolu · 2024-11-14T16:13:42Z

front/lib/api/assistant/agent.ts

@@ -362,6 +362,16 @@ async function* runMultiActionsAgent(

  const MIN_GENERATION_TOKENS = 2048;

+  const fakeJITListFiles = makeJITListFilesAction(


would have to be done behind flags + we can generalize to a "injectFakeActions" as this could be used for other things in the future

Fraggle · 2024-11-15T07:34:36Z

sdks/js/src/types.ts

+  contentType: z.string(),
+});
+
+const ConversationListFilesActionTypeSchema = BaseActionSchema.extend({


I'm surprised to see them in the public api types as it should not be needed to return them in the public json of the conversation, does it ?

In practice they are never returned but I don't think it's necessarily an issue to have them leak because it's not entirely a lie.

The next ConversationAgentActionType will be returned by the API and will live along this one so we're paving the way for them.

The next ones yes, that one no.

Feels like we are mixing apple and oranges here.

fontanierh · 2024-11-14T16:23:45Z

front/lib/api/assistant/actions/jit/list_files.ts

+  isTablesQueryActionType,
+} from "@dust-tt/types";
+
+interface JITListFilesActionBlob {


I think the "JIT" codename made sense internally so far because we have shared context but I don't think it's really a name that should exist in our codebase, at least not jsut "JIT" alone. Thinking of all the future engineers this will be hard to grasp IMHO

Fraggle · 2024-11-15T08:01:44Z

front/lib/api/assistant/jit_actions.ts

-      for (const action of actions) {
-        const stepIndex = action.step;
+      for (let stepIndex = 0; stepIndex < actions.length; stepIndex++) {
+        const action = actions[stepIndex];


so now the action.step do not necessary match the stepIndex ?

Yes this one still need a bit of work I was a bit too rapid to change the behavior here

spolu · 2024-11-15T12:27:54Z

Fully tested locally rendering works as expected

fontanierh

Feel free to address the nits or not.

Otherwise looks great 👍

fontanierh · 2024-11-15T13:07:54Z

front/lib/api/assistant/actions/conversation/list_files.ts

+    };
+  }
+
+  renderForMultiActionsModel(): FunctionMessageTypeModel {


unrelated to this PR, but we need to rename this

fontanierh · 2024-11-15T13:09:09Z

front/lib/api/assistant/actions/conversation/list_files.ts

+  agentMessage: AgentMessageType,
+  conversation: ConversationType
+): ConversationListFilesActionType | null {
+  const jitFiles: ConversationFileType[] = [];


nit: "jitFiles" could really just be "files". They aren't really "jit" IMHO

Good catch. Reminiscence from early versions. Will fix.

fontanierh · 2024-11-15T13:10:37Z

front/lib/api/assistant/agent.ts

+  }
+
+  // We ensure that all emulated actions are injected with step -1.
+  assert(actions.every((a) => a.step === -1));


nit: IMHO a regular exception with a clear error message is better (for eng runner) than using assert (which looks like something you'd use in a test)

I guess I can add a message to the assert?

Yeah but why use a function for that ? Looks like we're just wrapping a basic language construct with a useless function.

fontanierh · 2024-11-15T13:14:11Z

front/lib/api/assistant/jit_actions.ts

+      // calls).  Actions all have a step index which indicates how they should be grouped but some
+      // actions injected by `getEmulatedAgentMessageActions` have a step index of `-1`. We
+      // therefore group by index, then order and transform in a 2D array to present to the model.
+      const stepsByStepIndex = {} as Record<


nit: I think naming is getting confusing here. A "step" to me is a list of actions that were taken at this step.
So it should really be stepByIndex no ?

Yep will fix 👍

fontanierh · 2024-11-15T13:15:55Z

sdks/js/src/types.ts

+  step: z.number(),
+  type: z.literal("conversation_list_files_action"),
+});
+// type ConversationListFIlesActionPublicType = z.infer<


nit: to cleanup

fontanierh · 2024-11-15T13:42:14Z

front/lib/api/assistant/agent.ts

+
+  // We ensure that all emulated actions are injected with step -1.
+  assert(
+    actions.every((a) => a.step === -1),


can't this also be enforced in type system removing the need for assert ?

That would mix their type and the fact that they are emulated. One could imagine that a classic action is emulated.

@spolu

nit: the -1 index is going to be limiting if we need to order multiple emulated actions, I suggest < 0

Fraggle · 2024-11-15T14:18:21Z

front/lib/api/assistant/jit_actions.ts

+      // calls).  Actions all have a step index which indicates how they should be grouped but some
+      // actions injected by `getEmulatedAgentMessageActions` have a step index of `-1`. We
+      // therefore group by index, then order and transform in a 2D array to present to the model.
+      const stepByStepIndex = {} as Record<


nit: I'd rather use Map() insead of Record, unless we are serializing as this is not catched by the typechecking system:

const aRecord:Record<string, {"k": string}> = {}; aRecord["ruc"].k

Fraggle · 2024-11-15T14:21:27Z

front/lib/api/assistant/jit_actions.ts

@@ -81,46 +79,55 @@ export async function renderConversationForModelJIT({
  const now = Date.now();
  const messages: ModelMessageTypeMultiActions[] = [];

-  // Render loop.
-  // Render all messages and all actions.
+  // Render loop: dender all messages and all actions.


nit: typo (dender => render)

Fraggle · 2024-11-15T14:22:45Z

front/lib/api/assistant/jit_actions.ts


      for (const action of actions) {
        const stepIndex = action.step;
-        steps[stepIndex] = steps[stepIndex] || emptyStep();
-        steps[stepIndex].actions.push({
+        stepByStepIndex[stepIndex] = stepByStepIndex[stepIndex] || emptyStep();


nit: i know it's previous code but how can a step be empty ? (a comment would go a long way)

Fraggle · 2024-11-15T14:25:32Z

front/lib/api/assistant/jit_actions.ts

          call: action.renderForFunctionCall(),
          result: action.renderForMultiActionsModel(),
        });
      }

      for (const content of m.rawContents) {
-        steps[content.step] = steps[content.step] || emptyStep();
+        stepByStepIndex[content.step] =
+          stepByStepIndex[content.step] || emptyStep();


nit: shorter version stepByStepIndex[content.step] ??= emptyStep(); (same above)

Fraggle · 2024-11-15T14:26:34Z

front/lib/api/assistant/visualization.ts

-  const isJITEnabled = await isJITActionsEnabled(auth);
+  // If `jit_conversations_actions` is enabled we rely on the `conversations_list_files` emulated
+  // actions to make the list of files available to the agent.
+  if (await isJITActionsEnabled(auth)) {


nit: i have been told by @flvndvd not to do await in conditions.

spolu commented Nov 14, 2024

View reviewed changes

spolu force-pushed the spolu-jit_action_scafolding branch from 483c4e2 to cf97767 Compare November 15, 2024 06:17

spolu changed the title ~~JIT actions scafolding~~ Conversation actions scafolding + initial list_files action Nov 15, 2024

spolu marked this pull request as ready for review November 15, 2024 07:24

spolu requested a review from Fraggle November 15, 2024 07:29

Fraggle reviewed Nov 15, 2024

View reviewed changes

fontanierh reviewed Nov 15, 2024

View reviewed changes

Fraggle reviewed Nov 15, 2024

View reviewed changes

Fraggle self-requested a review November 15, 2024 08:28

Fraggle approved these changes Nov 15, 2024

View reviewed changes

spolu added 18 commits November 15, 2024 12:42

jit actions scafolding

c5270e4

list_files types

b26fc95

more

8c3adf8

simplify

ee51e66

simplify

64c6d62

clean

74d904e

simplify

0079271

one way of doing it

fabbf9b

fix imports

b5a4360

typing work

84b3b32

fixes

53892e2

rename

1ad4f02

emulated actions injection/scrub

fe06c9c

reintroduce step

17eea88

fix imports

bbbfb4d

clean-up visualiation

d771ae1

fix steps

a6b2342

rebase and comments

f7ae369

spolu force-pushed the spolu-jit_action_scafolding branch from c6196bf to f7ae369 Compare November 15, 2024 12:27

spolu requested review from fontanierh and Fraggle November 15, 2024 12:30

fontanierh approved these changes Nov 15, 2024

View reviewed changes

spolu added 4 commits November 15, 2024 14:37

rename jitFiles

6eea7d7

assert message

37ce947

stepByIndex

7ab6c5f

remove comments

deb9b77

fontanierh reviewed Nov 15, 2024

View reviewed changes

spolu merged commit 0e7714d into main Nov 15, 2024
2 checks passed

spolu deleted the spolu-jit_action_scafolding branch November 15, 2024 14:06

Fraggle reviewed Nov 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conversation actions scafolding + initial list_files action #8646

Conversation actions scafolding + initial list_files action #8646

spolu commented Nov 14, 2024 •

edited

Loading

spolu Nov 14, 2024

Fraggle Nov 15, 2024

spolu Nov 15, 2024

spolu Nov 15, 2024

Fraggle Nov 15, 2024

fontanierh Nov 14, 2024

Fraggle Nov 15, 2024

spolu Nov 15, 2024

spolu commented Nov 15, 2024

fontanierh left a comment

fontanierh Nov 15, 2024

fontanierh Nov 15, 2024

spolu Nov 15, 2024

fontanierh Nov 15, 2024

spolu Nov 15, 2024

fontanierh Nov 15, 2024

fontanierh Nov 15, 2024

spolu Nov 15, 2024

fontanierh Nov 15, 2024

fontanierh Nov 15, 2024

Fraggle Nov 15, 2024

fontanierh Nov 15, 2024

Fraggle Nov 15, 2024

Fraggle Nov 15, 2024

Fraggle Nov 15, 2024

Fraggle Nov 15, 2024

Fraggle Nov 15, 2024

Fraggle Nov 15, 2024

		@@ -362,6 +362,16 @@ async function* runMultiActionsAgent(

		const MIN_GENERATION_TOKENS = 2048;

		const fakeJITListFiles = makeJITListFilesAction(

Conversation actions scafolding + initial list_files action #8646

Conversation actions scafolding + initial list_files action #8646

Conversation

spolu commented Nov 14, 2024 • edited Loading

Description

Risk

Deploy Plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spolu commented Nov 15, 2024

fontanierh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spolu commented Nov 14, 2024 •

edited

Loading