Skip to content

feat(TagImprovement): Improved tag dropdown and outputselect #904

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(TagImprovement): Improved tag dropdown and outputselect #904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions .../[workflowId]/components/panel/components/chat/components/output-select/output-select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getBlock } from '@/blocks'
import { useWorkflowDiffStore } from '@/stores/workflow-diff/store'
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'

interface OutputSelectProps {
workflowId: string | null
Expand All @@ -15,6 +16,78 @@ interface OutputSelectProps {
placeholder?: string
}

// Helper function to get all current values for a block
const getBlockCurrentValues = (blockId: string, workflowId: string): Record<string, any> => {
const workflowValues = useSubBlockStore.getState().workflowValues[workflowId]
return workflowValues?.[blockId] || {}
}

// Evaluate output condition similar to input condition evaluation
const evaluateOutputCondition = (
condition: {
field: string
value: any
not?: boolean
and?: { field: string; value: any; not?: boolean }
} | undefined,
currentValues: Record<string, any>
): boolean => {
if (!condition) return true

// Get the field value
const fieldValue = currentValues[condition.field]

// Check if the condition value is an array
const isValueMatch = Array.isArray(condition.value)
? fieldValue != null &&
(condition.not
? !condition.value.includes(fieldValue)
: condition.value.includes(fieldValue))
: condition.not
? fieldValue !== condition.value
: fieldValue === condition.value

// Check both conditions if 'and' is present
const isAndValueMatch =
!condition.and ||
(() => {
const andFieldValue = currentValues[condition.and!.field]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Non-null assertion operator used without null check. Consider adding explicit null check for condition.and before accessing nested properties.

return Array.isArray(condition.and!.value)
? andFieldValue != null &&
(condition.and!.not
? !condition.and!.value.includes(andFieldValue)
: condition.and!.value.includes(andFieldValue))
: condition.and!.not
? andFieldValue !== condition.and!.value
: andFieldValue === condition.and!.value
})()

return isValueMatch && isAndValueMatch
}

// Filter outputs based on conditions
const filterOutputsByConditions = (
outputs: Record<string, any>,
currentValues: Record<string, any>
): Record<string, any> => {
const filteredOutputs: Record<string, any> = {}

for (const [key, value] of Object.entries(outputs)) {
let shouldInclude = true

// Check if this output has a condition
if (typeof value === 'object' && value !== null && 'condition' in value) {
shouldInclude = evaluateOutputCondition(value.condition, currentValues)
}

if (shouldInclude) {
filteredOutputs[key] = value
}
}

return filteredOutputs
}

export function OutputSelect({
workflowId,
selectedOutputs = [],
Expand Down Expand Up @@ -105,6 +178,12 @@ export function OutputSelect({
outputsToProcess = blockConfig?.outputs || {}
}

// Apply conditional filtering to outputs
if (workflowId && Object.keys(outputsToProcess).length > 0) {
const currentValues = getBlockCurrentValues(block.id, workflowId)
outputsToProcess = filterOutputsByConditions(outputsToProcess, currentValues)
}

// Add response outputs
if (Object.keys(outputsToProcess).length > 0) {
const addOutput = (path: string, outputObj: any, prefix = '') => {
Expand Down
15 changes: 14 additions & 1 deletion apps/sim/blocks/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,20 @@ export type BlockOutput =
// Output field definition with optional description
export type OutputFieldDefinition =
| PrimitiveValueType
| { type: PrimitiveValueType; description?: string }
| {
type: PrimitiveValueType;
description?: string;
condition?: {
field: string
value: string | number | boolean | Array<string | number | boolean>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The union type for value is quite complex. Consider creating a separate type alias for ConditionValue to improve readability.

not?: boolean
and?: {
field: string
value: string | number | boolean | Array<string | number | boolean> | undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: The undefined type for value in the and condition seems inconsistent with the main condition which doesn't allow undefined. This could lead to unexpected behavior.

Suggested change
value: string | number | boolean | Array<string | number | boolean> | undefined
value: string | number | boolean | Array<string | number | boolean>

not?: boolean
}
}
Comment on lines +84 to +93
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider extracting the condition type into a separate interface since it's duplicated in SubBlockConfig (lines 134-154). This would improve maintainability and ensure consistency.

Context Used: Context - When defining properties for components, use a dedicated config file (.ts) for configuration and keep rendered components in their respective component files. (link)

}

// Parameter validation rules
export interface ParamConfig {
Expand Down
124 changes: 112 additions & 12 deletions apps/sim/components/ui/tag-dropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,93 @@ export const checkTagTrigger = (text: string, cursorPosition: number): { show: b
return { show: false }
}

// Helper function to get all current values for a block
const getBlockCurrentValues = (blockId: string): Record<string, any> => {
const activeWorkflowId = useWorkflowRegistry.getState().activeWorkflowId
if (!activeWorkflowId) return {}

const workflowValues = useSubBlockStore.getState().workflowValues[activeWorkflowId]
return workflowValues?.[blockId] || {}
}

// Evaluate output condition similar to input condition evaluation
const evaluateOutputCondition = (
condition: {
field: string
value: any
not?: boolean
and?: { field: string; value: any; not?: boolean }
} | undefined,
currentValues: Record<string, any>
): boolean => {
if (!condition) return true

// Get the field value
const fieldValue = currentValues[condition.field]

// Check if the condition value is an array
const isValueMatch = Array.isArray(condition.value)
? fieldValue != null &&
(condition.not
? !condition.value.includes(fieldValue)
: condition.value.includes(fieldValue))
: condition.not
? fieldValue !== condition.value
: fieldValue === condition.value

// Check both conditions if 'and' is present
const isAndValueMatch =
!condition.and ||
(() => {
const andFieldValue = currentValues[condition.and!.field]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Using non-null assertion operator (!) on condition.and!.field could throw if condition.and is undefined despite the guard check. Consider using optional chaining: condition.and?.field

Suggested change
const andFieldValue = currentValues[condition.and!.field]
const andFieldValue = currentValues[condition.and?.field]

return Array.isArray(condition.and!.value)
? andFieldValue != null &&
(condition.and!.not
? !condition.and!.value.includes(andFieldValue)
: condition.and!.value.includes(andFieldValue))
: condition.and!.not
? andFieldValue !== condition.and!.value
: andFieldValue === condition.and!.value
})()

return isValueMatch && isAndValueMatch
}

// Filter outputs based on conditions
const filterOutputsByConditions = (
outputs: Record<string, any>,
currentValues: Record<string, any>
): Record<string, any> => {
const filteredOutputs: Record<string, any> = {}

for (const [key, value] of Object.entries(outputs)) {
let shouldInclude = true

// Check if this output has a condition
if (typeof value === 'object' && value !== null && 'condition' in value) {
shouldInclude = evaluateOutputCondition(value.condition, currentValues)
}

if (shouldInclude) {
filteredOutputs[key] = value
}
}

return filteredOutputs
}

// Generate output paths from block configuration outputs
const generateOutputPaths = (outputs: Record<string, any>, prefix = ''): string[] => {
const generateOutputPaths = (
outputs: Record<string, any>,
prefix = '',
currentValues?: Record<string, any>
): string[] => {
const paths: string[] = []

// Filter outputs by conditions if currentValues are provided
const outputsToProcess = currentValues ? filterOutputsByConditions(outputs, currentValues) : outputs

for (const [key, value] of Object.entries(outputs)) {
for (const [key, value] of Object.entries(outputsToProcess)) {
const currentPath = prefix ? `${prefix}.${key}` : key

if (typeof value === 'string') {
Expand All @@ -63,7 +145,7 @@ const generateOutputPaths = (outputs: Record<string, any>, prefix = ''): string[
paths.push(currentPath)
} else {
// Legacy nested object - recurse
const subPaths = generateOutputPaths(value, currentPath)
const subPaths = generateOutputPaths(value, currentPath, currentValues)
paths.push(...subPaths)
}
} else {
Expand Down Expand Up @@ -143,8 +225,11 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
const blockName = sourceBlock.name || sourceBlock.type
const normalizedBlockName = blockName.replace(/\s+/g, '').toLowerCase()

// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(activeSourceBlockId)

// Generate output paths for the mock config
const outputPaths = generateOutputPaths(mockConfig.outputs)
const outputPaths = generateOutputPaths(mockConfig.outputs, '', currentValues)
const blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)

const blockTagGroups: BlockTagGroup[] = [
Expand Down Expand Up @@ -190,7 +275,9 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
)
} else {
// Fallback to default evaluator outputs if no metrics are defined
const outputPaths = generateOutputPaths(blockConfig.outputs)
// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(activeSourceBlockId)
const outputPaths = generateOutputPaths(blockConfig.outputs, '', currentValues)
blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
}
} else if (responseFormat) {
Expand All @@ -200,7 +287,9 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
blockTags = schemaFields.map((field) => `${normalizedBlockName}.${field.name}`)
} else {
// Fallback to default if schema extraction failed
const outputPaths = generateOutputPaths(blockConfig.outputs || {})
// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(activeSourceBlockId)
const outputPaths = generateOutputPaths(blockConfig.outputs || {}, '', currentValues)
blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
}
} else if (!blockConfig.outputs || Object.keys(blockConfig.outputs).length === 0) {
Expand Down Expand Up @@ -240,7 +329,9 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
}
} else {
// Use default block outputs
const outputPaths = generateOutputPaths(blockConfig.outputs || {})
// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(activeSourceBlockId)
const outputPaths = generateOutputPaths(blockConfig.outputs || {}, '', currentValues)
blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
}

Expand Down Expand Up @@ -424,8 +515,11 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
const blockName = accessibleBlock.name || accessibleBlock.type
const normalizedBlockName = blockName.replace(/\s+/g, '').toLowerCase()

// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(accessibleBlockId)

// Generate output paths for the mock config
const outputPaths = generateOutputPaths(mockConfig.outputs)
const outputPaths = generateOutputPaths(mockConfig.outputs, '', currentValues)
const blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)

blockTagGroups.push({
Expand Down Expand Up @@ -465,7 +559,9 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
)
} else {
// Fallback to default evaluator outputs if no metrics are defined
const outputPaths = generateOutputPaths(blockConfig.outputs)
// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(accessibleBlockId)
const outputPaths = generateOutputPaths(blockConfig.outputs, '', currentValues)
blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
}
} else if (responseFormat) {
Expand All @@ -475,7 +571,9 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
blockTags = schemaFields.map((field) => `${normalizedBlockName}.${field.name}`)
} else {
// Fallback to default if schema extraction failed
const outputPaths = generateOutputPaths(blockConfig.outputs || {})
// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(accessibleBlockId)
const outputPaths = generateOutputPaths(blockConfig.outputs || {}, '', currentValues)
blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
}
} else if (!blockConfig.outputs || Object.keys(blockConfig.outputs).length === 0) {
Expand Down Expand Up @@ -515,7 +613,9 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
}
} else {
// Use default block outputs
const outputPaths = generateOutputPaths(blockConfig.outputs || {})
// Get current values for conditional filtering
const currentValues = getBlockCurrentValues(accessibleBlockId)
const outputPaths = generateOutputPaths(blockConfig.outputs || {}, '', currentValues)
blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
}

Expand Down Expand Up @@ -892,4 +992,4 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
</div>
</div>
)
}
}