feat: add function calling config form #39252
Conversation
WalkthroughThis pull request adds a set of new React components and related utilities to manage function calling configurations in an AI assistant form. The changes include components for adding, editing, and removing function calls using Redux Form, new selectors and type definitions for entity options, and updates to the form control registry to integrate the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant FC as FunctionCallingConfigControl
participant FCF as FunctionCallingConfigForm
participant Redux as Redux Form
participant FCTF as FunctionCallingConfigToolField
U->>FC: Render component
FC->>FCF: Pass props via FieldArray
FCF->>Redux: Initialize dynamic function entries
U->>FCF: Click "Add Function"
FCF->>Redux: Add new function entry (using uuid)
Redux-->>FCF: Update form state with new entry
U->>FCF: Click "Remove Function"
FCF->>Redux: Remove function entry by index
Redux-->>FCF: Update form state after removal
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
github-actions
bot
added
Anvil Pod
...omponents/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigItem.tsx
Outdated
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13308820067. |
|
Deploy-Preview-URL: https://ce-39252.dp.appsmith.com |
znamenskii-ilia
force-pushed
the
feat/39152-function-calling-config
branch
from
6da4429 to
ed5f4f6
Compare
...omponents/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx
Outdated
Show resolved
Hide resolved
znamenskii-ilia
force-pushed
the
feat/39152-function-calling-config
branch
2 times, most recently
from
1bf2193 to
6a84c42
Compare
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13330058464. |
znamenskii-ilia
force-pushed
the
feat/39152-function-calling-config
branch
from
6a84c42 to
715fbdf
Compare
|
Deploy-Preview-URL: https://ce-39252.dp.appsmith.com |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
app/client/src/components/formControls/FunctionCallingConfigControl/types.ts (1)
8-11: Consider adding documentation for entity types.The union type is well-defined, but adding JSDoc comments explaining each entity type's purpose would improve maintainability.
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingEmpty.tsx (1)
19-19: Add descriptive alt text for accessibility.Empty alt text reduces accessibility. Consider adding descriptive alt text like "Empty function calls illustration".
- <img alt="" src={getAssetUrl(`${ASSETS_CDN_URL}/empty-state.svg`)} /> + <img alt="Empty function calls illustration" src={getAssetUrl(`${ASSETS_CDN_URL}/empty-state.svg`)} />app/client/src/components/formControls/FunctionCallingConfigControl/FunctionCallingConfigControl.tsx (1)
19-22: Consider creating a dedicated type for form props.The props object passed to FunctionCallingConfigForm could benefit from a dedicated interface.
interface FunctionCallingConfigFormProps { formName: string; configProperty: string; }app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (1)
33-40: Consider adding validation for maximum number of functions.The
handleAddFunctionButtonClickallows unlimited function additions. Consider adding a maximum limit to prevent performance issues.const handleAddFunctionButtonClick = useCallback(() => { + const MAX_FUNCTIONS = 10; + if (fields.length >= MAX_FUNCTIONS) { + return; + } fields.push({ id: uuid(), description: "",app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts (1)
63-75: Consider memoizing the flatMap operation.The
selectJsFunctionEntityOptionsperforms a potentially expensive flatMap operation on every call.+import { createSelector } from 'reselect'; -const selectJsFunctionEntityOptions = ( +const selectJsFunctionEntityOptions = createSelector( + getJSCollections, state: AppState, ): FunctionCallingEntityTypeOption[] => { - return getJSCollections(state).flatMap((jsCollection) => { + return (collections) => collections.flatMap((jsCollection) => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/client/packages/design-system/ads/src/Select/rc-styles.css(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/FunctionCallingConfigControl.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingEmpty.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/index.ts(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/types.ts(1 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx(2 hunks)app/client/src/utils/formControl/formControlTypes.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/components/formControls/FunctionCallingConfigControl/index.ts
🧰 Additional context used
🪛 GitHub Actions: Quality checks
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx
[error] 8-8: "../../DropDownControl" has no exported member named 'DropDownGroupedOptionsInterface'. Did you mean 'DropDownGroupedOptions'?
🔇 Additional comments (9)
app/client/packages/design-system/ads/src/Select/rc-styles.css (1)
6-6: LGTM! Good use of design system variable.The addition of background color using the design system variable maintains consistency with the design system.
app/client/src/components/formControls/FunctionCallingConfigControl/types.ts (2)
1-6: LGTM! Well-structured interface definition.The interface properties are clearly defined with appropriate optional fields.
13-19: LGTM! Clear and comprehensive interface definition.The interface captures all necessary fields for function calling configuration.
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingEmpty.tsx (1)
7-14: LGTM! Clean styled component implementation.Good use of CSS variables and flex layout for centering content.
app/client/src/utils/formControl/formControlTypes.ts (1)
26-26: LGTM! Consistent with existing pattern.The new form control type follows the established naming convention.
app/client/src/components/formControls/FunctionCallingConfigControl/FunctionCallingConfigControl.tsx (1)
8-11: LGTM! Well-documented component.Good use of JSDoc to explain the component's purpose and functionality.
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (1)
70-72: Add empty state handling for fields array.The conditional rendering at line 70 looks good, but consider adding a more descriptive empty state message.
app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts (1)
77-111: LGTM! Well-structured system functions mapping.The system functions mapping is well-organized and uses proper type safety with const assertions.
app/client/src/utils/formControl/FormControlRegistry.tsx (1)
235-242: LGTM! Control registration follows established pattern.The registration of FunctionCallingConfigControl follows the same pattern as other controls and is properly implemented.
| import { useDispatch, useSelector } from "react-redux"; | ||
| import { change, formValueSelector } from "redux-form"; | ||
| import styled from "styled-components"; | ||
| import type { DropDownGroupedOptionsInterface } from "../../DropDownControl"; |
There was a problem hiding this comment.
Fix the imported interface name.
The pipeline failure indicates that 'DropDownGroupedOptionsInterface' should be 'DropDownGroupedOptions'.
-import type { DropDownGroupedOptionsInterface } from "../../DropDownControl";
+import type { DropDownGroupedOptions } from "../../DropDownControl";📝 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.
| import type { DropDownGroupedOptionsInterface } from "../../DropDownControl"; | |
| import type { DropDownGroupedOptions } from "../../DropDownControl"; |
🧰 Tools
🪛 GitHub Actions: Quality checks
[error] 8-8: "../../DropDownControl" has no exported member named 'DropDownGroupedOptionsInterface'. Did you mean 'DropDownGroupedOptions'?
| const findEntityType = useCallback( | ||
| (entityId: string): string => { | ||
| switch (true) { | ||
| case findEntityOption(entityId, entityOptions.Query): | ||
| return "Query"; | ||
| case findEntityOption(entityId, entityOptions.JSFunction): | ||
| return "JSFunction"; | ||
| case findEntityOption(entityId, entityOptions.SystemFunction): | ||
| return "SystemFunction"; | ||
| } | ||
|
|
||
| return ""; | ||
| }, | ||
| [fieldValue.entityId], | ||
| ); |
There was a problem hiding this comment.
Dependency array in useCallback is incorrect.
The findEntityType callback depends on findEntityOption and entityOptions, but the dependency array only includes fieldValue.entityId.
- [fieldValue.entityId],
+ [findEntityOption, entityOptions],📝 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.
| const findEntityType = useCallback( | |
| (entityId: string): string => { | |
| switch (true) { | |
| case findEntityOption(entityId, entityOptions.Query): | |
| return "Query"; | |
| case findEntityOption(entityId, entityOptions.JSFunction): | |
| return "JSFunction"; | |
| case findEntityOption(entityId, entityOptions.SystemFunction): | |
| return "SystemFunction"; | |
| } | |
| return ""; | |
| }, | |
| [fieldValue.entityId], | |
| ); | |
| const findEntityType = useCallback( | |
| (entityId: string): string => { | |
| switch (true) { | |
| case findEntityOption(entityId, entityOptions.Query): | |
| return "Query"; | |
| case findEntityOption(entityId, entityOptions.JSFunction): | |
| return "JSFunction"; | |
| case findEntityOption(entityId, entityOptions.SystemFunction): | |
| return "SystemFunction"; | |
| } | |
| return ""; | |
| }, | |
| [findEntityOption, entityOptions], | |
| ); |
znamenskii-ilia
force-pushed
the
feat/39152-function-calling-config
branch
from
715fbdf to
989b568
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (2)
32-40: Consider extracting default values to constants.Extract the default function configuration to a constant to improve maintainability and reusability.
+const DEFAULT_FUNCTION_CONFIG = { + description: "", + entityId: "", + requiresApproval: false, + entityType: "Query", +} as const; const handleAddFunctionButtonClick = useCallback(() => { fields.push({ id: uuid(), - description: "", - entityId: "", - requiresApproval: false, - entityType: "Query", + ...DEFAULT_FUNCTION_CONFIG, }); }, [fields]);
49-89: Consider wrapping with an error boundary.The form handles dynamic data and user interactions. Adding an error boundary would improve reliability.
app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts (1)
77-111: Improve type safety for labelMap.The current type assertion doesn't guarantee that all system functions have corresponding labels.
- const labelMap: Record<keyof typeof systemFunctions, string> = { + const labelMap = { navigateTo: createMessage(NAVIGATE_TO), showAlert: createMessage(SHOW_ALERT), // ... other mappings - }; + } satisfies Record<keyof typeof systemFunctions, string>;app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx (1)
111-148: Consider fixing FormControl type definitions.Instead of using @ts-expect-error, consider updating the FormControl component's type definitions to properly support the isSearchable property.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/client/packages/design-system/ads/src/Select/rc-styles.css(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/FunctionCallingConfigControl.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingEmpty.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/index.ts(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/types.ts(1 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx(2 hunks)app/client/src/utils/formControl/formControlTypes.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- app/client/src/components/formControls/FunctionCallingConfigControl/index.ts
- app/client/packages/design-system/ads/src/Select/rc-styles.css
- app/client/src/utils/formControl/formControlTypes.ts
- app/client/src/utils/formControl/FormControlRegistry.tsx
- app/client/src/components/formControls/FunctionCallingConfigControl/FunctionCallingConfigControl.tsx
- app/client/src/components/formControls/FunctionCallingConfigControl/types.ts
- app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingEmpty.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: chromatic-deployment
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
🔇 Additional comments (3)
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (1)
1-13: LGTM! Props interface and imports are well structured.app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts (1)
113-128: LGTM! Excellent use of createSelector for performance optimization.app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx (1)
67-81: Fix useCallback dependencies.The dependency array is incorrect.
- [fieldValue.entityId, entityOptions], + [findEntityOption, entityOptions],
| useEffect( | ||
| // entityType is dependent on entityId | ||
| // Every time entityId changes, we need to find the new entityType | ||
| function handleEntityTypeChange() { | ||
| const entityType = findEntityType(fieldValue.entityId); | ||
|
|
||
| dispatch( | ||
| change(props.formName, `${props.fieldPath}.entityType`, entityType), | ||
| ); | ||
| }, | ||
| [fieldValue.entityId], | ||
| ); |
There was a problem hiding this comment.
Add missing dependencies to useEffect.
The effect's dependency array is missing several dependencies.
- [fieldValue.entityId],
+ [fieldValue.entityId, dispatch, props.formName, props.fieldPath, findEntityType],📝 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.
| useEffect( | |
| // entityType is dependent on entityId | |
| // Every time entityId changes, we need to find the new entityType | |
| function handleEntityTypeChange() { | |
| const entityType = findEntityType(fieldValue.entityId); | |
| dispatch( | |
| change(props.formName, `${props.fieldPath}.entityType`, entityType), | |
| ); | |
| }, | |
| [fieldValue.entityId], | |
| ); | |
| useEffect( | |
| // entityType is dependent on entityId | |
| // Every time entityId changes, we need to find the new entityType | |
| function handleEntityTypeChange() { | |
| const entityType = findEntityType(fieldValue.entityId); | |
| dispatch( | |
| change(props.formName, `${props.fieldPath}.entityType`, entityType), | |
| ); | |
| }, | |
| [fieldValue.entityId, dispatch, props.formName, props.fieldPath, findEntityType], | |
| ); |
...omponents/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx
Outdated
Show resolved
Hide resolved
...ents/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx
Show resolved
Hide resolved
...ents/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx
Outdated
Show resolved
Hide resolved
znamenskii-ilia
force-pushed
the
feat/39152-function-calling-config
branch
from
989b568 to
935effe
Compare
znamenskii-ilia
force-pushed
the
feat/39152-function-calling-config
branch
from
935effe to
32f0aa4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (1)
32-40: Consider extracting default values to a constant.Extract the default function configuration to improve maintainability and reusability.
+const DEFAULT_FUNCTION_CONFIG = { + description: "", + entityId: "", + requiresApproval: false, + entityType: "Query", +} as const; const handleAddFunctionButtonClick = useCallback(() => { fields.push({ id: uuid(), - description: "", - entityId: "", - requiresApproval: false, - entityType: "Query", + ...DEFAULT_FUNCTION_CONFIG, }); }, [fields]);app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts (1)
38-61: Optimize selectQueryEntityOptions to reduce iterations.The current implementation maps over the actions twice. This can be optimized into a single map operation.
export const selectQueryEntityOptions = ( state: AppState, ): FunctionCallingEntityTypeOption[] => { const plugins = getPlugins(state); - return getActions(state) - .map(({ config }) => ({ - id: config.id, - name: config.name, - pluginId: config.pluginId, - })) - .map((action) => { + return getActions(state).map(({ config }) => { const iconSrc = plugins.find( - (plugin) => plugin.id === action.pluginId, + (plugin) => plugin.id === config.pluginId, )?.iconLocation; return { - value: action.id, - label: action.name, + value: config.id, + label: config.name, optionGroupType: "Query", iconSrc, }; - }); + }); };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/client/src/components/formControls/FunctionCallingConfigControl/FunctionCallingConfigControl.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingEmpty.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/index.ts(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/types.ts(1 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx(2 hunks)app/client/src/utils/formControl/formControlTypes.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- app/client/src/components/formControls/FunctionCallingConfigControl/index.ts
- app/client/src/utils/formControl/formControlTypes.ts
- app/client/src/components/formControls/FunctionCallingConfigControl/FunctionCallingConfigControl.tsx
- app/client/src/utils/formControl/FormControlRegistry.tsx
- app/client/src/components/formControls/FunctionCallingConfigControl/types.ts
- app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingEmpty.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (6)
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (2)
1-26: LGTM! Clean imports and well-structured type definitions.
49-89: LGTM! Clean and well-structured render logic.Good use of conditional rendering and styled components for layout.
app/client/src/components/formControls/FunctionCallingConfigControl/components/selectors.ts (1)
77-128: LGTM! Well-structured selectors with proper memoization.Good use of createSelector and comprehensive message mapping for system functions.
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigToolField.tsx (3)
83-94: Add missing dependencies to useEffect.The effect's dependency array is missing several dependencies.
- [fieldValue.entityId], + [fieldValue.entityId, dispatch, props.formName, props.fieldPath, findEntityType],
67-81: Dependency array in useCallback is incorrect.The findEntityType callback depends on findEntityOption and entityOptions, but the dependency array includes fieldValue.entityId which is not used.
- [fieldValue.entityId, entityOptions], + [findEntityOption, entityOptions],
111-170: LGTM! Well-structured form controls with proper type checking.Good use of the satisfies operator for type checking the optionGroupConfig.
Description
fc.mov
Fixes #39152
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Anvil"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13369293716
Commit: 32f0aa4
Cypress dashboard.
Tags:
@tag.AnvilSpec:
Mon, 17 Feb 2025 11:59:23 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit