[front] - fix(conversations): render conversations with archived agents #8900

JulesBelveze · 2024-11-26T09:51:14Z

Description

This PR modifies fetchWorkspaceAgentConfigurationsWithoutActions to return either active versions if they exist, or otherwise the latest versions.

This was recently introduced by #8892. The goal of #8892 was to fix #8888.

Context: we recently found out that:

return AgentConfiguration.findAll({
          where: {
            workspaceId: owner.id,
            ...(agentPrefix ? { name: { [Op.iLike]: `${agentPrefix}%` } } : {}),
            sId: agentsGetView.agentIds.filter((id) => !isGlobalAgentId(id)),
          },
          order: [["version", "DESC"]],
          ...(agentsGetView.allVersions ? {} : { limit: 1 }),
        });
      }

was causing a bunch of errors on the /participants endpoint as it was limiting to return only one agent configuration. Because of this the conversation participants were broken.

The initial fix was to retrieve all "active" agent configurations, which fixed the conversation participants issue. However, this led to not be able to retrieve conversation with an "archived" agent.

This PR ensures that we either return the active agent configuration or its latest archived version.

Risk

Low

Deploy Plan

Deploy front

…ersions are fetched - Modify fetchWorkspaceAgentConfigurationsWithoutActions to return either active versions if they exist, or otherwise the latest versions - Implement reduction logic to group agents by sId and select the correct version to return based on status and existence in the accumulator

spolu · 2024-11-26T12:03:58Z

@JulesBelveze

So that I understand correctly this was the limit: 1 change right?

So the diffs we had so far are:

https://github.com/dust-tt/dust/pull/8888/files
https://github.com/dust-tt/dust/pull/8892/files

And now this one?

Let's step back and really think through the change to make sure we have a full understanding here. To do so please update your PR description with references to previous 2 PRs what they changes what they broke and why this is guaranteed to be the right end-state 🙏

flvndvd · 2024-11-26T12:49:00Z

front/lib/api/assistant/configuration.ts

@@ -366,15 +366,35 @@ async function fetchWorkspaceAgentConfigurationsWithoutActions(
      });
    default:
      if (typeof agentsGetView === "object" && "agentIds" in agentsGetView) {
-        return AgentConfiguration.findAll({
+        const agents = await AgentConfiguration.findAll({


It seems a bit tough to not leverage the SQL query, no? Agent configurations can have hundreds or versions 🤔

Can go full sql, we're gonna hvave to go for raw sql though don't think we can use sequelize for such complicated query

LEt's see the SQL query once you have it. Maybe we can map it to Sequelize.

spolu · 2024-11-26T13:14:16Z

Because of this the conversation participants were broken.

@JulesBelveze do we understand when it broke and how it broke? This query has been here for a long time no? Yet participants have been working in the past? What exactly broke them?

JulesBelveze · 2024-11-26T13:24:14Z

@spolu I'm not sure to follow how this change was working: 99771e8

spolu · 2024-11-26T13:48:40Z

You mean the limit: 1 was at odd with the move to sId array I presume. Seems plausibe that it was problematic cc @philipperolet ?

Good that we're getting down to a very clear picture. Feel like we now have reached full context, which is great \o/

spolu · 2024-11-26T13:49:46Z

@JulesBelveze if you can add @philipperolet as reviewer this would be great to make sure we have full context on getting the good final state 🙏

Thanks for digging!

…rations - Implement a more efficient SQL query to directly fetch the necessary agent configurations - Remove the previous post-processing logic in favor of database-level aggregation and sorting

philipperolet · 2024-11-26T14:23:50Z

After looking at it I concur with @JulesBelveze the query didn't work. Apologies for missing that.

It's strange that nobody noticed since it's been 2 months, but at the same time since it only impacts the fact that you don't see all the avatars on the top right of your screen in a conversation it's possible

Thanks a lot for solving this @JulesBelveze

philipperolet

LGTM and thanks on fixing the issue from my PR moving to agentIds array 🙏
however, I think the 'status:active' filter is what's breaking the preview; AFAICT, the preview is grabbing for a 'draft' ? continuing investigation there

philipperolet · 2024-11-26T14:37:11Z

LGTM and thanks on fixing the issue from my PR moving to agentIds array 🙏

however, I think the 'status:active' filter is what's breaking the preview; AFAICT, the preview is grabbing for a 'draft' ? continuing investigation there

For reference, I'm talking about this wrt the preview broken: https://github.com/dust-tt/tasks/issues/1703

philipperolet · 2024-11-26T14:46:46Z

Other topic: conversations with archived agents generate unhandled errors

=> https://dust4ai.slack.com/archives/C05F84CFP0E/p1732630658549379

philipperolet · 2024-11-26T14:53:08Z

Discussed IRL:

@JulesBelveze proposed grouping by agentId and taking the latest version instead of status filtering
this should solve 2 problems mentioned above, and avoid the raw SQL query
🙏

…uery - Include option to fetch all versions of agent configurations - Modify query to retrieve only latest versions of agent configurations using Sequelize ORM instead of raw query

philipperolet

👍 (make sure to test)

JulesBelveze · 2024-11-26T15:25:40Z

Tested the following:

conversation with multiple agents
conversation with delete agent
assistant preview

…ts (#8900) * [front/lib/api/assistant] - fix: ensure only active or latest agent versions are fetched - Modify fetchWorkspaceAgentConfigurationsWithoutActions to return either active versions if they exist, or otherwise the latest versions - Implement reduction logic to group agents by sId and select the correct version to return based on status and existence in the accumulator * [front/lib/api/assistant] - refactor: optimize fetching agent configurations - Implement a more efficient SQL query to directly fetch the necessary agent configurations - Remove the previous post-processing logic in favor of database-level aggregation and sorting * [front/lib/api/assistant] - refactor: optimize agent configurations query - Include option to fetch all versions of agent configurations - Modify query to retrieve only latest versions of agent configurations using Sequelize ORM instead of raw query

JulesBelveze linked an issue Nov 26, 2024 that may be closed by this pull request

Render conversation with deleted agent #8898

Closed

JulesBelveze requested a review from flvndvd November 26, 2024 09:52

flvndvd reviewed Nov 26, 2024

View reviewed changes

JulesBelveze requested a review from philipperolet November 26, 2024 13:52

[front/lib/api/assistant] - refactor: optimize fetching agent configu…

10d94a8

…rations - Implement a more efficient SQL query to directly fetch the necessary agent configurations - Remove the previous post-processing logic in favor of database-level aggregation and sorting

philipperolet reviewed Nov 26, 2024

View reviewed changes

[front/lib/api/assistant] - refactor: optimize agent configurations q…

9e7a465

…uery - Include option to fetch all versions of agent configurations - Modify query to retrieve only latest versions of agent configurations using Sequelize ORM instead of raw query

JulesBelveze requested review from flvndvd and philipperolet November 26, 2024 15:21

philipperolet approved these changes Nov 26, 2024

View reviewed changes

JulesBelveze merged commit e762f86 into main Nov 26, 2024
3 checks passed

JulesBelveze deleted the fix/conversation-deleted-agent branch November 26, 2024 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[front] - fix(conversations): render conversations with archived agents #8900

[front] - fix(conversations): render conversations with archived agents #8900

JulesBelveze commented Nov 26, 2024 •

edited

Loading

spolu commented Nov 26, 2024

flvndvd Nov 26, 2024

JulesBelveze Nov 26, 2024

flvndvd Nov 26, 2024

spolu commented Nov 26, 2024

JulesBelveze commented Nov 26, 2024

spolu commented Nov 26, 2024

spolu commented Nov 26, 2024

philipperolet commented Nov 26, 2024

philipperolet left a comment

philipperolet commented Nov 26, 2024

philipperolet commented Nov 26, 2024

philipperolet commented Nov 26, 2024

philipperolet left a comment

JulesBelveze commented Nov 26, 2024

[front] - fix(conversations): render conversations with archived agents #8900

[front] - fix(conversations): render conversations with archived agents #8900

Conversation

JulesBelveze commented Nov 26, 2024 • edited Loading

Description

Risk

Deploy Plan

spolu commented Nov 26, 2024

flvndvd Nov 26, 2024

Choose a reason for hiding this comment

JulesBelveze Nov 26, 2024

Choose a reason for hiding this comment

flvndvd Nov 26, 2024

Choose a reason for hiding this comment

spolu commented Nov 26, 2024

JulesBelveze commented Nov 26, 2024

spolu commented Nov 26, 2024

spolu commented Nov 26, 2024

philipperolet commented Nov 26, 2024

philipperolet left a comment

Choose a reason for hiding this comment

philipperolet commented Nov 26, 2024

philipperolet commented Nov 26, 2024

philipperolet commented Nov 26, 2024

philipperolet left a comment

Choose a reason for hiding this comment

JulesBelveze commented Nov 26, 2024

JulesBelveze commented Nov 26, 2024 •

edited

Loading