-
Notifications
You must be signed in to change notification settings - Fork 4k
chore: Enable templates for agents #40214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes simplify the template filtering logic in the Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/Templates/TemplateContent/index.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (2)
app/client/src/pages/Templates/TemplateContent/index.tsx (2)
120-121
: Good practice using constants for string values.Using constants for string values improves maintainability and reduces the risk of typos.
146-146
: Good practice replacing string literal with constant.Properly utilizing the newly defined constant
AGENT_TEMPLATES_TITLE
instead of the string literal.
// For agents, we only show the templates that have the use case "Agent". | ||
// The "Agent" use case acts as a filter for use to just show the templates | ||
// that are relevant to agents. | ||
.filter((template) => { | ||
if (isAiAgentFlowEnabled) { | ||
return template.useCases.includes(AGENT_TEMPLATES_USE_CASE); | ||
} | ||
|
||
return true; | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undefined variable issue: isAiAgentFlowEnabled
is not declared.
The variable isAiAgentFlowEnabled
is used but not imported or declared anywhere in this file. This will cause a runtime error.
+ import { isAiAgentFlowEnabled } from "selectors/featureFlagsSelectors";
// or declare it properly where needed
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// For agents, we only show the templates that have the use case "Agent". | |
// The "Agent" use case acts as a filter for use to just show the templates | |
// that are relevant to agents. | |
.filter((template) => { | |
if (isAiAgentFlowEnabled) { | |
return template.useCases.includes(AGENT_TEMPLATES_USE_CASE); | |
} | |
return true; | |
}) | |
import { isAiAgentFlowEnabled } from "selectors/featureFlagsSelectors"; | |
// Other imports and code... | |
// For agents, we only show the templates that have the use case "Agent". | |
// The "Agent" use case acts as a filter for use to just show the templates | |
// that are relevant to agents. | |
.filter((template) => { | |
if (isAiAgentFlowEnabled) { | |
return template.useCases.includes(AGENT_TEMPLATES_USE_CASE); | |
} | |
return true; | |
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/client/src/selectors/templatesSelectors.tsx (1)
18-18
: Import for feature flag selector added.This correctly implements the missing import that was flagged in a previous review.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
app/client/src/pages/Templates/TemplateContent/index.tsx
(2 hunks)app/client/src/selectors/templatesSelectors.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/client/src/pages/Templates/TemplateContent/index.tsx (1)
app/client/src/selectors/templatesSelectors.tsx (1)
getSearchedTemplateList
(175-187)
app/client/src/selectors/templatesSelectors.tsx (2)
app/client/src/ce/reducers/index.tsx (1)
AppState
(98-191)app/client/src/ce/selectors/aiAgentSelectors.ts (1)
getIsAiAgentFlowEnabled
(9-11)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (3)
app/client/src/pages/Templates/TemplateContent/index.tsx (2)
87-87
: Added additional check for empty building blocks.This change enhances the rendering logic to only display the building blocks section when there are actually building blocks available. This prevents rendering an empty section when no building blocks exist.
126-128
: Simplified template filtering logic.The filtering logic has been streamlined to depend solely on the
filterWithAllowPageImport
flag. This simplification is connected to the new filtering mechanism implemented in thegetTemplatesSelector
function in the templatesSelectors.tsx file.app/client/src/selectors/templatesSelectors.tsx (1)
28-29
: Constants added for agent-related template filtering.Good practice to define these as constants rather than hardcoding the strings in the filter logic.
export const getTemplatesSelector = createSelector( | ||
(state: AppState) => state.ui.templates.templates, | ||
getIsAiAgentFlowEnabled, | ||
(templates, isAiAgentFlowEnabled) => { | ||
// For agents, we only show the templates that have the use case "Agent". | ||
// The "Agent" use case acts as a filter for use to just show the templates | ||
// that are relevant to agents. | ||
return ( | ||
templates | ||
.filter((template) => { | ||
if (isAiAgentFlowEnabled) { | ||
return template.useCases.includes(AGENT_TEMPLATES_USE_CASE); | ||
} | ||
|
||
return true; | ||
}) | ||
// We are using AI Agent template for creating ai agent app, | ||
// so we are not showing it in the templates list. | ||
// TODO: Once we have a new entity for ai agent, we need to remove this filter. | ||
.filter((template) => template.title !== AGENT_TEMPLATES_TITLE) | ||
); | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
New selector function for template filtering based on AI agent flow.
This selector implements a centralized filtering mechanism that:
- Shows only Agent-related templates when AI agent flow is enabled
- Always excludes the "AI Agent" template regardless of feature flag status
The comments explain the filtering logic well, including the TODO note about future refactoring once a new entity for AI agents is implemented.
🏁 Script executed:
#!/bin/bash
# Verify that all template references in the codebase now use the new selector
# Check if there are any direct accesses to state.ui.templates.templates that should use getTemplatesSelector instead
rg -t tsx -t ts "state\.ui\.templates\.templates" --no-filename | grep -v "getTemplatesSelector"
Length of output: 124
Selector Verification Update – Revised Verification Command
The new selector function in app/client/src/selectors/templatesSelectors.tsx (lines 31–53) correctly implements:
- Filtering for Agent-specific templates when AI agent flow is enabled.
- Consistently excluding the "AI Agent" template.
Since the previous verification command failed due to file type issues with TSX files, please re-run the revised script above. If no output is produced, it confirms that all references to state.ui.templates.templates
are appropriately routed through getTemplatesSelector
.
🏁 Script executed:
#!/bin/bash
# Revised check: search TS and TSX files for direct references to "state.ui.templates.templates"
# that are not using "getTemplatesSelector".
rg -g "*.tsx" -g "*.ts" "state\.ui\.templates\.templates" --no-filename | grep -v "getTemplatesSelector"
Length of output: 541
Attention: Inconsistent Selector Usage Detected
The new selector in app/client/src/selectors/templatesSelectors.tsx (lines 31–53) correctly encapsulates the filtering logic for Agent-related templates when AI agent flow is enabled and consistently excludes the "AI Agent" template. However, our verification reveals several direct references to state.ui.templates.templates
in other parts of the codebase, including:
- Direct access for reading the array and its length (e.g.,
(state: AppState) => state.ui.templates.templates.length
) - References used for operations like
.find()
and.filter()
- Other usages where state is directly manipulated (e.g.,
state.ui.templates.templatesModal
)
Action Items:
- Review each of the direct references above and determine if they should be updated to use
getTemplatesSelector
for consistent filtering logic. - Refactor the areas where direct state access bypasses the centralized selector, ensuring unified behavior across the application.
Please address these discrepancies to maintain consistency and ensure that the new centralized filtering is fully leveraged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/Templates/TemplateFilters/TemplateFilter.test.tsx (1)
40-46
: Good addition of feature flag data to the mock store.Adding the feature flag data to the mock store helps test the component's behavior with different feature flag configurations. This is a necessary update to support the template filtering changes related to AI agents.
However, I note that
license_ai_agent_enabled
is set tofalse
in the default values. If you're testing functionality that depends on this flag being enabled, consider adding a test case that overrides this flag totrue
.// Example of a potential additional test case it("displays AI agent templates when license_ai_agent_enabled is true", () => { store = mockStore({ // ...existing mock store setup ui: { // ...existing ui setup users: { featureFlag: { data: DEFAULT_FEATURE_FLAG_VALUE, overriddenFlags: { license_ai_agent_enabled: true }, isFetching: false, }, }, }, }); // Test that agent templates are displayed });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
app/client/src/pages/Templates/TemplateContent/index.tsx
(1 hunks)app/client/src/pages/Templates/TemplateFilters/TemplateFilter.test.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/Templates/TemplateContent/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/client/src/pages/Templates/TemplateFilters/TemplateFilter.test.tsx (1)
app/client/src/ce/entities/FeatureFlag.ts (1)
DEFAULT_FEATURE_FLAG_VALUE
(67-109)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (1)
app/client/src/pages/Templates/TemplateFilters/TemplateFilter.test.tsx (1)
15-15
: Appropriate import added for feature flags.The import of
DEFAULT_FEATURE_FLAG_VALUE
is necessary to set up the mock Redux store with feature flag data for testing.
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
/ok-to-test tags="@tag.Templates"
Summary by CodeRabbit
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14462054954
Commit: 4df9dc9
Cypress dashboard.
Tags:
@tag.Templates
Spec:
Tue, 15 Apr 2025 05:56:58 UTC