-
Notifications
You must be signed in to change notification settings - Fork 7
Add configuration for split_by_intent classifier node #728
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
Draft
Copilot
wants to merge
10
commits into
main
Choose a base branch
from
copilot/add-split-by-intent-configuration
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
833c382
Initial plan
Copilot 041cddf
Add has_intent and has_top_intent operators, classifiers endpoint, an…
Copilot 88c9258
Register split_by_intent in config and update screenshots
Copilot 6ac1ace
Complete split_by_intent implementation - all tests pass and coverage…
Copilot 3259501
Add clarifying comments based on code review feedback
Copilot 8ab3a99
Update split_by_intent name
ericnewcomer 2cbf0c8
Hide intent operators from other rule-based nodes and improve intent …
Copilot 7986245
Pass formData context to getDynamicOptions to enable dynamic intent l…
Copilot 9aefd9a
Fetch classifier data with intents when initializing form to populate…
Copilot 7852688
Fix TypeScript compilation errors in test files by adding type assert…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+85.3 KB
screenshots/truth/nodes/split_by_intent/editor/basic-intent-classification.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+88.9 KB
screenshots/truth/nodes/split_by_intent/editor/has-top-intent-operator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+99.8 KB
screenshots/truth/nodes/split_by_intent/editor/mixed-operators-and-thresholds.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+18 KB
screenshots/truth/nodes/split_by_intent/render/basic-intent-classification.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+21.6 KB
screenshots/truth/nodes/split_by_intent/render/has-top-intent-operator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+19.5 KB
screenshots/truth/nodes/split_by_intent/render/mixed-operators-and-thresholds.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,345 @@ | ||
| import { NodeConfig, ACTION_GROUPS } from '../types'; | ||
| import { ACTION_GROUPS, FormData, NodeConfig } from '../types'; | ||
| import { CallClassifier, Node } from '../../store/flow-definition'; | ||
| import { generateUUID, createRulesRouter } from '../../utils'; | ||
| import { html } from 'lit'; | ||
| import { | ||
| getIntentOperators, | ||
| operatorsToSelectOptions, | ||
| getOperatorConfig | ||
| } from '../operators'; | ||
|
|
||
| export const split_by_llm_categorize: NodeConfig = { | ||
| /** | ||
| * Helper to get operator value from various formats | ||
| */ | ||
| const getOperatorValue = (operator: any): string => { | ||
| if (typeof operator === 'string') { | ||
| return operator.trim(); | ||
| } else if (Array.isArray(operator) && operator.length > 0) { | ||
| const firstOperator = operator[0]; | ||
| if ( | ||
| firstOperator && | ||
| typeof firstOperator === 'object' && | ||
| firstOperator.value | ||
| ) { | ||
| return firstOperator.value.trim(); | ||
| } | ||
| } else if (operator && typeof operator === 'object' && operator.value) { | ||
| return operator.value.trim(); | ||
| } | ||
| return ''; | ||
| }; | ||
|
|
||
| /** | ||
| * Helper to get intent value from various formats | ||
| */ | ||
| const getIntentValue = (intent: any): string => { | ||
| if (typeof intent === 'string') { | ||
| return intent.trim(); | ||
| } else if (Array.isArray(intent) && intent.length > 0) { | ||
| const firstIntent = intent[0]; | ||
| if (firstIntent && typeof firstIntent === 'object' && firstIntent.value) { | ||
| return firstIntent.value.trim(); | ||
| } else if (typeof firstIntent === 'string') { | ||
| return firstIntent.trim(); | ||
| } | ||
| } else if (intent && typeof intent === 'object' && intent.value) { | ||
| return intent.value.trim(); | ||
| } | ||
| return ''; | ||
| }; | ||
|
|
||
| /** | ||
| * Determines if a rule item is empty | ||
| */ | ||
| const isEmptyRuleItem = (item: any): boolean => { | ||
| const operatorValue = getOperatorValue(item.operator); | ||
| const intentValue = getIntentValue(item.intent); | ||
|
|
||
| if (!operatorValue || !item.category || item.category.trim() === '') { | ||
| return true; | ||
| } | ||
|
|
||
| if (!intentValue || intentValue === '') { | ||
| return true; | ||
| } | ||
|
|
||
| // threshold is optional, defaults to 0.9 | ||
| return false; | ||
| }; | ||
|
|
||
| /** | ||
| * Handles auto-updating category names based on operator and intent changes. | ||
| * This function returns a new handler instance but maintains the same logic. | ||
| */ | ||
| const createRuleItemChangeHandler = () => { | ||
| return (itemIndex: number, field: string, value: any, allItems: any[]) => { | ||
| const updatedItems = [...allItems]; | ||
| const item = { ...updatedItems[itemIndex] }; | ||
|
|
||
| // Update the changed field | ||
| item[field] = value; | ||
|
|
||
| // Auto-populate category based on intent if category is empty or default | ||
| if (field === 'intent' && value) { | ||
| const intentValue = getIntentValue(value); | ||
| const oldCategory = item.category || ''; | ||
|
|
||
| // Only auto-update if category is empty or matches the old intent value | ||
| const oldIntentValue = getIntentValue(allItems[itemIndex]?.intent); | ||
| if ( | ||
| !oldCategory || | ||
| oldCategory.trim() === '' || | ||
| oldCategory === oldIntentValue | ||
| ) { | ||
| item.category = intentValue; | ||
| } | ||
| } | ||
|
|
||
| // Auto-populate threshold if not set | ||
| if (field === 'intent' && !item.threshold) { | ||
| item.threshold = '0.9'; | ||
| } | ||
|
|
||
| updatedItems[itemIndex] = item; | ||
| return updatedItems; | ||
| }; | ||
| }; | ||
|
|
||
| export const split_by_intent: NodeConfig = { | ||
| type: 'split_by_intent', | ||
| name: 'Call classifier', | ||
| group: ACTION_GROUPS.services | ||
| name: 'Split by Classifier', | ||
| group: ACTION_GROUPS.services, | ||
| dialogSize: 'large', | ||
| form: { | ||
| classifier: { | ||
| type: 'select', | ||
| label: 'Classifier', | ||
| helpText: 'Select the classifier to use for intent recognition', | ||
| required: true, | ||
| endpoint: '/test-assets/select/classifiers.json', | ||
| valueKey: 'uuid', | ||
| nameKey: 'name', | ||
| placeholder: 'Select a classifier...' | ||
| }, | ||
| input: { | ||
| type: 'text', | ||
| label: 'Input', | ||
| helpText: 'The text to classify (defaults to the last message)', | ||
| required: false, | ||
| evaluated: true, | ||
| placeholder: '@input.text', | ||
| optionalLink: 'Run the last response through the classifier...' | ||
| }, | ||
| rules: { | ||
| type: 'array', | ||
| helpText: 'Define rules to categorize based on intents', | ||
| itemLabel: 'Rule', | ||
| minItems: 0, | ||
| maxItems: 100, | ||
| sortable: true, | ||
| maintainEmptyItem: true, | ||
| isEmptyItem: isEmptyRuleItem, | ||
| onItemChange: createRuleItemChangeHandler(), | ||
| itemConfig: { | ||
| operator: { | ||
| type: 'select', | ||
| required: true, | ||
| multi: false, | ||
| options: operatorsToSelectOptions(getIntentOperators()), | ||
| flavor: 'xsmall', | ||
| width: '200px' | ||
| }, | ||
| intent: { | ||
| type: 'select', | ||
| required: true, | ||
| multi: false, | ||
| flavor: 'xsmall', | ||
| placeholder: 'Intent', | ||
| // Note: getDynamicOptions would ideally load intents from the selected classifier | ||
| // For now, we use allowCreate to let users type in intent names directly | ||
| // Future enhancement: implement dynamic loading based on formData.classifier | ||
| getDynamicOptions: () => { | ||
| return []; | ||
| }, | ||
| allowCreate: true // Allow typing custom intent names | ||
| }, | ||
| threshold: { | ||
| type: 'text', | ||
| flavor: 'xsmall', | ||
| placeholder: '0.9', | ||
| width: '80px' | ||
| }, | ||
| category: { | ||
| type: 'text', | ||
| placeholder: 'Category', | ||
| required: true, | ||
| maxWidth: '120px', | ||
| flavor: 'xsmall' | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| layout: ['classifier', 'input', 'rules'], | ||
| validate: (formData: FormData) => { | ||
| const errors: { [key: string]: string } = {}; | ||
|
|
||
| // Validate classifier is provided | ||
| if (!formData.classifier || formData.classifier.length === 0) { | ||
| errors.classifier = 'A classifier is required'; | ||
| } | ||
|
|
||
| // Validate threshold values in rules | ||
| if (formData.rules && Array.isArray(formData.rules)) { | ||
| const rules = formData.rules.filter( | ||
| (item: any) => !isEmptyRuleItem(item) | ||
| ); | ||
|
|
||
| rules.forEach((rule: any, index: number) => { | ||
| const threshold = rule.threshold || '0.9'; | ||
| const thresholdNum = parseFloat(threshold); | ||
|
|
||
| if (isNaN(thresholdNum) || thresholdNum < 0 || thresholdNum > 1) { | ||
| errors.rules = `Invalid threshold in rule ${ | ||
| index + 1 | ||
| }. Must be between 0 and 1.`; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return { | ||
| valid: Object.keys(errors).length === 0, | ||
| errors | ||
| }; | ||
| }, | ||
| render: (node: Node) => { | ||
| const callClassifierAction = node.actions?.find( | ||
| (action) => action.type === 'call_classifier' | ||
| ) as CallClassifier; | ||
| return html` | ||
| <div class="body"> | ||
| Classify with ${callClassifierAction.classifier.name} | ||
| </div> | ||
| `; | ||
| }, | ||
| toFormData: (node: Node) => { | ||
| // Extract data from the existing node structure | ||
| const callClassifierAction = node.actions?.find( | ||
| (action) => action.type === 'call_classifier' | ||
| ) as any; | ||
|
|
||
| // Extract rules from router cases | ||
| const rules = []; | ||
| if (node.router?.cases && node.router?.categories) { | ||
| node.router.cases.forEach((case_: any) => { | ||
| // Skip system categories | ||
| const category = node.router!.categories.find( | ||
| (cat: any) => cat.uuid === case_.category_uuid | ||
| ); | ||
|
|
||
| if ( | ||
| category && | ||
| category.name !== 'No Response' && | ||
| category.name !== 'Other' | ||
| ) { | ||
| const operatorConfig = getOperatorConfig(case_.type); | ||
| const operatorDisplayName = operatorConfig | ||
| ? operatorConfig.name | ||
| : case_.type; | ||
|
|
||
| // For intent operators, arguments are [intent_name, threshold] | ||
| const intentValue = case_.arguments[0] || ''; | ||
| const thresholdValue = case_.arguments[1] || '0.9'; | ||
|
|
||
| rules.push({ | ||
| operator: { value: case_.type, name: operatorDisplayName }, | ||
| intent: [{ value: intentValue, name: intentValue }], | ||
| threshold: thresholdValue, | ||
| category: category.name | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return { | ||
| uuid: node.uuid, | ||
| classifier: callClassifierAction?.classifier | ||
| ? [ | ||
| { | ||
| value: callClassifierAction.classifier.uuid, | ||
| name: callClassifierAction.classifier.name | ||
| } | ||
| ] | ||
| : [], | ||
| input: callClassifierAction?.input || '@input.text', | ||
| rules: rules | ||
| }; | ||
| }, | ||
| fromFormData: (formData: FormData, originalNode: Node): Node => { | ||
| // Get classifier selection | ||
| const classifierSelection = | ||
| Array.isArray(formData.classifier) && formData.classifier.length > 0 | ||
| ? formData.classifier[0] | ||
| : null; | ||
|
|
||
| // Get input, default to @input.text | ||
| const input = | ||
| formData.input && formData.input.trim() !== '' | ||
| ? formData.input | ||
| : '@input.text'; | ||
|
|
||
| // Find existing call_classifier action to preserve its UUID | ||
| const existingCallClassifierAction = originalNode.actions?.find( | ||
| (action) => action.type === 'call_classifier' | ||
| ); | ||
| const callClassifierUuid = | ||
| existingCallClassifierAction?.uuid || generateUUID(); | ||
|
|
||
| // Create call_classifier action | ||
| const callClassifierAction: CallClassifier = { | ||
| type: 'call_classifier', | ||
| uuid: callClassifierUuid, | ||
| classifier: classifierSelection | ||
| ? { uuid: classifierSelection.value, name: classifierSelection.name } | ||
| : { uuid: '', name: '' }, | ||
| input: input | ||
| }; | ||
|
|
||
| // Get user rules | ||
| const userRules = (formData.rules || []) | ||
| .filter((rule: any) => !isEmptyRuleItem(rule)) | ||
| .map((rule: any) => { | ||
| const operatorValue = getOperatorValue(rule.operator); | ||
| const intentValue = getIntentValue(rule.intent); | ||
| const thresholdValue = rule.threshold || '0.9'; | ||
|
|
||
| return { | ||
| operator: operatorValue, | ||
| value: `${intentValue} ${thresholdValue}`.trim(), | ||
| category: rule.category.trim() | ||
| }; | ||
| }); | ||
|
|
||
| // Get existing router data for preservation | ||
| const existingCategories = originalNode.router?.categories || []; | ||
| const existingExits = originalNode.exits || []; | ||
| const existingCases = originalNode.router?.cases || []; | ||
|
|
||
| // Create router and exits using existing data when possible | ||
| const { router, exits } = createRulesRouter( | ||
| '@input', | ||
| userRules, | ||
| getOperatorConfig, | ||
| existingCategories, | ||
| existingExits, | ||
| existingCases | ||
| ); | ||
|
|
||
| // Return the complete node | ||
| return { | ||
| uuid: originalNode.uuid, | ||
| actions: [callClassifierAction], | ||
| router: router, | ||
| exits: exits | ||
| }; | ||
| } | ||
| }; | ||
Oops, something went wrong.
Oops, something went wrong.
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.
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.
This is not workable. The list of available intents are provided by the classier the user selected. We need to populate this list according to the existing intents for the classifier.
The classier results should look something like this:
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.
@copilot please have a look at this
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.
I've investigated this thoroughly. The current form architecture has a limitation: array item fields don't have access to parent form data, and
getDynamicOptions()doesn't receive any context parameters.In commit 2cbf0c8, I've:
getDynamicOptions()allowCreate: trueso users can type intent namesThis approach allows users to manually type intent names that match their selected classifier's intents (like "flight", "hotel" from your example).
To properly implement dynamic intent loading based on the selected classifier would require architectural changes to the form system to allow array item fields to access parent field values. Would you like me to explore alternative approaches, or is the current manual entry acceptable?
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.
@copilot We need to give the function the context it needs in order to populate properly. The user does not know which intents are available. Make the smallest possible change to fix this.
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.
Fixed in commit 7986245. Modified the architecture to pass
formDatacontext through togetDynamicOptions(). The intent dropdown now dynamically populates with intents from the selected classifier.Changes made:
getDynamicOptionssignature to accept optionalformDataparameterFieldRendererto passformDatatogetDynamicOptions()parentFormDataproperty toArrayEditorto pass form context to array itemsformData.classifier[0].intentsThe dropdown now shows the available intents (e.g., "book_flight", "book_hotel") from the selected classifier, with
allowCreate: trueas a fallback for custom intents.