[refacto] - refacto: agent popularity query #6145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
July 9, 2024 15:19
…timeframe and simplify data - Change the agent ranking calculation period from 30 days to a shorter 7 days timeframe for more recent data relevance - Remove the tracking and storage of user counts related to agent mentions, simplifying the data model and storage requirements - Clean up unused imports and functions, such as `agentMentionsUserCount`, to streamline the codebase - Update related functions to adapt to the removal of user count tracking and the new ranking timeframe - Ensure backwards compatibility by incrementing message count only if the counts have already been computed
- The userCount field is removed from the usage data structure for agent configurations [types] - refactor: update AgentUsageType interface - Removed userCount property to reflect changes in usage data handling - Adjusted associated comments to remove references to userCount in the timePeriodSec field
…in SharingDropdown - Remove conditions related to initial state and user count when setting new scope [front/components/assistant] - refactor: streamline assistant usage message in Usage.tsx - Remove user count detail from the usage message display logic
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: front/lib/api/assistant/agent_usage.ts
Did you find this useful? React with a 👍 or 👎 |
- Removed unnecessary use of Promise.all for a single async operation
JulesBelveze
force-pushed
the
fix/agent-popularity-query
branch
from
254fd2e to
5715782
Compare
philipperolet
approved these changes
Jul 9, 2024
| // Ranking of agents is done over a 30 days period. | ||
| const rankingTimeframeSec = 60 * 60 * 24 * 30; // 30 days | ||
| // Ranking of agents is done over a 7 days period. | ||
| const rankingTimeframeSec = 60 * 60 * 24 * 7; // 30 days |
There was a problem hiding this comment.
nit: remove 30 days comment
There was a problem hiding this comment.
Oops overlooked that one sorry 👍🏼
- Fixed the inaccurate comment to reflect the actual 7 days period used for agent ranking computations
albandum
pushed a commit
that referenced
this pull request
Aug 28, 2024
* [front/lib/api/assistant] - refactor: update agent usage calculation timeframe and simplify data - Change the agent ranking calculation period from 30 days to a shorter 7 days timeframe for more recent data relevance - Remove the tracking and storage of user counts related to agent mentions, simplifying the data model and storage requirements - Clean up unused imports and functions, such as `agentMentionsUserCount`, to streamline the codebase - Update related functions to adapt to the removal of user count tracking and the new ranking timeframe - Ensure backwards compatibility by incrementing message count only if the counts have already been computed * [front] - refactor: remove userCount from agent usage data - The userCount field is removed from the usage data structure for agent configurations [types] - refactor: update AgentUsageType interface - Removed userCount property to reflect changes in usage data handling - Adjusted associated comments to remove references to userCount in the timePeriodSec field * [front/components/assistant] - refactor: simplify scope change logic in SharingDropdown - Remove conditions related to initial state and user count when setting new scope [front/components/assistant] - refactor: streamline assistant usage message in Usage.tsx - Remove user count detail from the usage message display logic * [front] - refactor: simplify retrieval of agent message count - Removed unnecessary use of Promise.all for a single async operation * [assistant] - fix: correct comment for agent ranking timeframe - Fixed the inaccurate comment to reflect the actual 7 days period used for agent ranking computations --------- Co-authored-by: Jules <[email protected]>
|
@JulesBelveze you might have a bit more context, do you know why we poke redis for the usage on the ui ? |
|
@albandum The main reason we were storing usage on Redis is because those queries are really expensive and we do not want them to be ran for each admin landing on the workspace page. |
|
@JulesBelveze looks like we removed that from the workspace page, now it only shows when you open an assistant modal if I'm not mistaken so might not be necessary anymore |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR refactors the agent popularity query to improve efficiency and reduce database load. The main changes include:
Risk
Overlooking files in which
userCountwas used.There will be a discrepancy between in the message count until the Redis caches expire. Indeed, we are switching from 30 days to 7 days and what's currently in Redis cache is the count for 30 days.
Deploy Plan
front-edgefront