feat: add inline variable references (@Component.output syntax) #11301
Conversation
Add core infrastructure for @NodeSlug.output inline variable references: - Reference dataclass with node_slug, output_name, and dot_path fields - Regex pattern with negative lookbehind to prevent email false positives - parse_references() function to extract all @references from text
Add reference resolution logic: - resolve_references() to replace @references with actual values - _extract_text_value() to handle Message, Data, str, int, etc. - traverse_dot_path() for nested access with security protection - Rejects dunder/private attributes for security
Add mixin to mark inputs that can contain @references: - ReferenceMixin with has_references: bool field - Applied to StrInput, PromptInput, DefaultPromptField
Add reference slug generation for vertices: - generate_reference_slug() converts display name to valid identifier - reference_slug property on Vertex - get_vertex_by_slug() method on Graph with lazy-built index - Documented index invalidation behavior
Add reference resolution during parameter processing: - _should_resolve_references() checks if field has has_references - Calls resolve_references() during _process_direct_type_field() - Logs warning on resolution failure instead of crashing
Add tests covering: - Parser tests including email false positive prevention - Resolver tests including Message/Data extraction - Utils tests including dunder attribute protection - ReferenceMixin tests for input fields - Integration tests for param handler + resolution flow
Add type definitions for reference system: - ParsedReference type for parsed @references - UpstreamOutput type for autocomplete options - has_references field in APIClassType - nodeId in textModalPropsType - nodeReferenceSlugs in flow store types
Add utilities for reference handling: - parseReferences() to extract @references from text - getUpstreamOutputs() to collect referenceable outputs from upstream nodes - getCaretCoordinates() to calculate pixel position for dropdown
Add nodeReferenceSlugs state to track slug mappings: - Record<nodeId, slug> for quick lookup - Updates slugs when nodes are added/removed/renamed - Handles duplicate names with numeric suffixes
Add autocomplete components for reference input: - ReferenceInput wrapper with render props pattern - ReferenceAutocomplete dropdown with filtering and keyboard navigation - Portal support for modals, caret-position dropdown for textareas - Preview display while navigating options
Wire up reference support to existing input components: - Wrap StrRenderComponent inputs with ReferenceInput when supported - Pass nodeId and onKeyDown to modal components - Update has_references field when value changes - Add onKeyDown support to TextAreaComponent
Add frontend tests: - 16 referenceParser tests including email false positive prevention - 17 getUpstreamOutputs tests for upstream node discovery
- Extract helper functions in strRenderComponent to reduce duplication - Replace IIFE with useMemo in ReferenceInput for better optimization - Simplify ternary expressions in referenceParser - Remove unused code and improve readability in textAreaComponent - Add explicit type annotations in ReferenceAutocomplete
- Add 16 edge case tests for referenceParser (punctuation, brackets, nested paths, array indices, multiline, long slugs) - Add preview functionality tests for ReferenceInput component - Verify actualValue vs displayValue separation works correctly
- Test autocomplete appears when typing @ with upstream connections - Test option selection via mouse click and keyboard Enter - Test keyboard navigation (Arrow Down, Escape to cancel) - Test filtering by typing after @ - Test no autocomplete without upstream nodes - Test space closes autocomplete
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request introduces a comprehensive reference system enabling users to embed inline variable references in text inputs. The frontend adds UI components with autocomplete support for @ mentions to reference upstream node outputs, while the backend implements reference parsing, resolution, and graph slug management to substitute references with actual values during execution. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ReferenceInput
participant ReferenceAutocomplete
participant UpstreamResolver
participant TextArea
User->>TextArea: Type "@" character
TextArea->>ReferenceInput: onKeyDown event
ReferenceInput->>UpstreamResolver: getUpstreamOutputs(nodeId)
UpstreamResolver-->>ReferenceInput: [UpstreamOutput...]
ReferenceInput->>ReferenceAutocomplete: isOpen=true, options
ReferenceAutocomplete-->>User: Show dropdown list
User->>ReferenceAutocomplete: Select option or navigate
ReferenceAutocomplete->>ReferenceInput: onSelect(option)
ReferenceInput->>TextArea: Insert "@nodeSlug.outputName" at cursor
ReferenceInput->>ReferenceAutocomplete: Close dropdown
ReferenceAutocomplete-->>User: Hide dropdown
sequenceDiagram
participant Backend
participant ParameterHandler
participant Parser
participant Resolver
participant Graph
Backend->>ParameterHandler: process_field_parameters(field_data)
ParameterHandler->>ParameterHandler: _should_resolve_references(field)
alt has_references=true
ParameterHandler->>Parser: parse_references(text)
Parser-->>ParameterHandler: [ParsedReference...]
ParameterHandler->>Resolver: resolve_references(text, graph)
Resolver->>Graph: get_vertex_by_slug(node_slug)
Graph-->>Resolver: Vertex
Resolver->>Graph: Vertex.outputs_map[output_name]
Graph-->>Resolver: output_value
Resolver->>Resolver: traverse_dot_path(value, path)
Resolver-->>Resolver: resolved_value (text)
Resolver-->>ParameterHandler: substituted_text
end
ParameterHandler-->>Backend: processed_value
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 3❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is ❌ Your project check has failed because the head coverage (41.07%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11301 +/- ##
==========================================
- Coverage 34.15% 32.06% -2.09%
==========================================
Files 1408 1418 +10
Lines 66773 67300 +527
Branches 9858 9982 +124
==========================================
- Hits 22803 21581 -1222
- Misses 42777 44500 +1723
- Partials 1193 1219 +26
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@src/frontend/src/components/core/parameterRenderComponent/components/promptComponent/index.tsx`:
- Line 27: The destructured but unused nodeId prop should be fixed: either
remove nodeId from the component's props and the destructuring at the top (clean
up the props type/interface) if it's not needed, or if nodeId is intended to be
used, add nodeId to PromptModal's props signature and type (update PromptModal's
parameter list and prop type) and then pass nodeId through when rendering
PromptModal from this component so PromptModal receives and can use it; update
any related prop type/interface declarations to keep types consistent.
In
`@src/frontend/src/components/core/parameterRenderComponent/components/textAreaComponent/index.tsx`:
- Around line 30-44: getBackgroundClass is dead code; either remove it or wire
it into the component render where styling classes are applied. If you choose to
use it, replace the hard-coded/background class usage on the same element that
calls getGradientClass with a call to getBackgroundClass(disabled, editNode)
(ensure the same disabled and editNode props/locals are passed) so background
styles are computed consistently; otherwise delete the getBackgroundClass
function and its tests/imports to eliminate the unused symbol.
In `@src/lfx/src/lfx/graph/vertex/base.py`:
- Around line 42-59: The function generate_reference_slug uses
display_name.title() which lowercases acronyms (e.g., "HTTP Request" ->
"HttpRequest") contrary to the docstring; either update the docstring to match
the current behavior or change the implementation to preserve existing casing.
To preserve acronyms, remove the .title() call and instead strip
non-alphanumeric characters and spaces (e.g., use re.sub on the original
display_name and concat words) so "HTTP Request" becomes "HTTPRequest"; update
the docstring to describe the chosen behavior and keep the fallback to "Node"
when the result is empty.
🧹 Nitpick comments (30)
src/frontend/src/modals/promptModal/index.tsx (1)
2-2: Unused import:useCallbackis imported but not used.The
useCallbackhook was added to the imports but does not appear to be used anywhere in this file. This may be a leftover from a refactor or prepared for future use.♻️ Suggested fix
-import { useCallback, useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react";src/lfx/src/lfx/graph/reference/schema.py (1)
5-19: LGTM! Clean dataclass implementation.The
Referencedataclass correctly models inline variable references with well-typed fields and a properly implementedfull_pathproperty.Consider making the dataclass immutable with
@dataclass(frozen=True)sinceReferencerepresents a value object that shouldn't change after creation. This prevents accidental mutations and enables use in sets/as dict keys.♻️ Optional: Make dataclass immutable
-@dataclass +@dataclass(frozen=True) class Reference:src/frontend/src/utils/__tests__/referenceParser.test.ts (2)
206-211: Clarify expected behavior for numeric-prefixed output names.The test comment says "this should still match" for
@Node.123output, which is technically correct since\wincludes digits. However, this might allow unintended matches like@Node.123(if followed by text).Consider adding a test to verify that pure numeric output names (e.g.,
@Node.123.path) behave as expected:it("should handle output name that is purely numeric", () => { const refs = parseReferences("@Node.123.path"); expect(refs).toHaveLength(1); expect(refs[0].outputName).toBe("123"); expect(refs[0].dotPath).toBe("path"); });
116-218: Consider adding test for hyphenated slugs (negative case).The regex uses
\wwhich doesn't match hyphens. A test confirming that hyphenated node slugs like@my-node.outputdon't fully match (only@mywould be attempted but fail due to missing.output) would document this limitation:it("should not match hyphenated node slugs", () => { const refs = parseReferences("@my-node.output"); // Hyphen breaks the match since \w doesn't include hyphens expect(refs).toHaveLength(0); });This documents the expected behavior for users who might try to use hyphens in their node names.
src/frontend/src/utils/getCaretCoordinates.ts (1)
79-156: Consider wrapping DOM operations in try-finally for cleanup safety.If an exception occurs between
appendChild(line 89) andremoveChild(line 153), the mirror div will leak into the DOM. While unlikely in practice, a try-finally block would guarantee cleanup.♻️ Suggested safer cleanup pattern
export function getCaretCoordinates( element: HTMLTextAreaElement | HTMLInputElement, position: number, ): CaretCoordinates { if (!isBrowser) { return { top: 0, left: 0, height: 0 }; } const div = document.createElement("div"); div.id = "input-textarea-caret-position-mirror-div"; document.body.appendChild(div); + try { const style = div.style; // ... rest of the implementation ... const coordinates: CaretCoordinates = { top: span.offsetTop + parseInt(computed.borderTopWidth), left: span.offsetLeft + parseInt(computed.borderLeftWidth), height: parseInt(computed.lineHeight) || parseInt(computed.fontSize), }; - document.body.removeChild(div); - return coordinates; + } finally { + document.body.removeChild(div); + } }src/frontend/src/components/core/parameterRenderComponent/components/textAreaComponent/index.tsx (1)
147-150: Mixing inlinestylewith class-based approach.The
backgroundis set via inlinestyle(line 149), butgetGradientClassreturns class names. This inconsistency means the gradient class sets one style while the inline style overrides/duplicates it. Consider using only one approach for maintainability.src/lfx/tests/unit/inputs/test_inputs_reference.py (1)
11-18: Tests for MessageTextInput and MultilineInput only verify attribute existence.Consider extending these tests to also verify assignment and serialization (like
test_str_input_has_references_fieldandtest_str_input_reference_in_dictdo for StrInput). Since these classes inherit from StrInput, this would confirm the inheritance chain works as expected.src/lfx/tests/unit/graph/reference/test_utils.py (1)
1-94: Consider adding a test for negative array indices.The implementation checks
0 <= index < len(result), which correctly rejects negative indices. Adding an explicit test case would document this behavior.def test_traverse_negative_index_returns_none(): """Negative indices should not be supported.""" data = {"items": ["a", "b", "c"]} result = traverse_dot_path(data, "items[-1]") assert result is Nonesrc/lfx/tests/unit/inputs/test_prompt_inputs_reference.py (1)
1-14: Consider consolidating with test_inputs_reference.py.Both test files test the same
has_referencesbehavior for different input types. Consolidating into a single test file (or using parameterized tests) would reduce duplication and make the test suite easier to maintain.# Example parameterized approach import pytest from lfx.inputs.inputs import MessageTextInput, MultilineInput, PromptInput, StrInput `@pytest.mark.parametrize`("input_class", [StrInput, MessageTextInput, MultilineInput, PromptInput]) def test_input_has_references_field(input_class): inp = input_class(name="test", value="Hello `@Node.output`") assert hasattr(inp, "has_references") inp.has_references = True assert inp.has_references is Truesrc/lfx/tests/unit/inputs/test_reference_mixin.py (1)
27-31: Ambiguous assertion weakens test reliability.The OR condition
"has_references" not in d or d.get("has_references") is Falsewill pass in both scenarios, making it unclear what behavior is actually expected. Based on theReferenceMixindefinition (wherehas_referenceshasdefault=False),exclude_defaults=Trueshould exclude it entirely.Proposed stricter assertion
def test_reference_mixin_not_in_dict_when_false(): inp = TestInput(name="test", has_references=False) d = inp.model_dump(exclude_defaults=True) - # has_references should be excluded when False (default) - assert "has_references" not in d or d.get("has_references") is False + # has_references should be excluded when False (default) + assert "has_references" not in dsrc/frontend/src/components/core/referenceInput/__tests__/ReferenceAutocomplete.test.tsx (1)
273-294: Consider adding a test for portal rendering mode.The positioning tests cover inline rendering modes well, but when
anchorRefis provided, the component usescreatePortalto render intodocument.bodywith fixed positioning. Consider adding a test case that provides ananchorRefto verify portal behavior.src/lfx/tests/unit/graph/reference/test_parser.py (1)
63-68: Consider adding more email-like pattern tests.The email test is valuable. Consider adding edge cases like
[email protected](email-like prefix with valid reference suffix) or@domain.com(looks like a reference but is incomplete) to strengthen confidence in the negative lookbehind behavior.src/lfx/src/lfx/graph/graph/base.py (1)
1390-1419: Good implementation with clear documentation on invalidation behavior.The docstring clearly explains the intentional design decision not to auto-invalidate the index. The duplicate handling logic correctly assigns suffixes (
_1,_2, etc.) to subsequent vertices with the same base slug.One minor observation: the index is built lazily but there's no public method to force a rebuild. The docstring mentions deleting
_slug_to_vertex, but consider adding arebuild_slug_index()method for cleaner API if this becomes needed.src/frontend/src/utils/getUpstreamOutputs.ts (1)
75-92: Minor: Consider iterative approach for very deep graphs.The recursive
collectUpstreamNodesfunction works correctly with thevisitedSet preventing cycles. However, for extremely deep graphs, this could hit call stack limits. This is unlikely in practice for typical flow graphs, but worth noting.src/lfx/tests/unit/graph/reference/test_integration.py (1)
156-181: Good negative scenario coverage.Testing that missing node references are handled gracefully (logged warning, no crash, value unchanged) is important for robustness. Consider adding a test that verifies the warning is actually logged.
Optional: Verify warning is logged
def test_param_handler_handles_missing_node(self, caplog): """Test that missing node reference logs warning but doesn't crash.""" # ... existing setup ... with caplog.at_level("WARNING"): params, _ = handler.process_field_parameters() assert params["text"] == "@NonExistent.output" assert "NonExistent" in caplog.text # Verify warning was loggedsrc/frontend/src/components/core/referenceInput/ReferenceAutocomplete.tsx (1)
51-55: Consider memoizing filtered options.
filteredOptionsis recalculated on every render, which also causes the effect on lines 63-69 to run unnecessarily sincefilteredOptionsis in its dependency array. This could cause performance issues with large option lists.Memoize filteredOptions
+import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { useCallback, useEffect, useRef, useState } from "react"; ... - const filteredOptions = options.filter((opt) => { - const searchText = `${opt.nodeName} ${opt.outputName}`.toLowerCase(); - return searchText.includes(filter.toLowerCase()); - }); + const filteredOptions = useMemo(() => { + return options.filter((opt) => { + const searchText = `${opt.nodeName} ${opt.outputName}`.toLowerCase(); + return searchText.includes(filter.toLowerCase()); + }); + }, [options, filter]);src/lfx/src/lfx/graph/vertex/param_handler.py (1)
339-358: Unused method_resolve_field_references.This helper method is defined but never called—the reference resolution logic is inlined directly in
_process_direct_type_field(lines 194-201). Consider either removing this method or refactoring to use it consistently.Option 1: Remove unused method
- def _resolve_field_references(self, value: str, field: dict) -> str: - """Resolve `@references` in a field value. - - Args: - value: The field value containing references - field: The field definition - - Returns: - The value with references resolved - - Raises: - ReferenceResolutionError: If resolution fails - """ - if not self._should_resolve_references(field): - return value - - if not isinstance(value, str): - return value - - return resolve_references(value, self.vertex.graph)Option 2: Use the helper method
# Resolve `@references` if the field has them - if self._should_resolve_references(field) and field_name in params: - param_value = params[field_name] - if isinstance(param_value, str): - try: - params[field_name] = resolve_references(param_value, self.vertex.graph) - except ReferenceResolutionError as e: - logger.warning("Failed to resolve references in field %s: %s", field_name, e) + if field_name in params: + param_value = params[field_name] + if isinstance(param_value, str): + try: + params[field_name] = self._resolve_field_references(param_value, field) + except ReferenceResolutionError as e: + logger.warning("Failed to resolve references in field %s: %s", field_name, e)src/frontend/src/components/core/referenceInput/__tests__/ReferenceInput.test.tsx (2)
73-77: Mock'sonHighlightChangecall during render may cause issues.Calling
onHighlightChangeviasetTimeoutunconditionally on every render can lead to flaky tests and multiple callback invocations. Consider guarding this or using a ref to track if it's already been called.Suggested improvement
// Call onHighlightChange with first option when open - if (onHighlightChange && filteredOptions.length > 0) { - // Use setTimeout to avoid calling during render - setTimeout(() => onHighlightChange(filteredOptions[0]), 0); - } + const calledRef = React.useRef(false); + React.useEffect(() => { + if (onHighlightChange && filteredOptions.length > 0 && !calledRef.current) { + calledRef.current = true; + onHighlightChange(filteredOptions[0]); + } + }, [onHighlightChange, filteredOptions]);
473-514: Test for "no upstream outputs" does not effectively verify the scenario.The
jest.resetModules()andjest.doMock()pattern doesn't work here becauseReferenceInputwas already imported at module load time with the original mock. The comment on lines 511-513 acknowledges this limitation.Consider restructuring to test this scenario properly:
Option: Move to a separate test file with different mock setup
Create a separate test file
ReferenceInput.noUpstream.test.tsxwhere the mock returns an empty array from the start:// ReferenceInput.noUpstream.test.tsx jest.mock("@/utils/getUpstreamOutputs", () => ({ getUpstreamOutputs: jest.fn(() => []), })); // ... then import and test ReferenceInputsrc/frontend/src/stores/flowStore.ts (1)
236-264: Slug generation logic is duplicated in three places.The slug generation logic appears in
resetFlow,setNodes, andgenerateSlugForNodewith slight variations. This creates maintenance burden and potential for inconsistencies. Consider extracting to a shared helper function.Suggested refactor: Extract slug generation helper
// Add near the top of the file or in a utils file function generateBaseSlug(displayName: string): string { return ( displayName .split(/\s+/) .map((word: string) => word.charAt(0).toUpperCase() + word.slice(1)) .join("") .replace(/[^a-zA-Z0-9]/g, "") || "Node" ); } function generateUniqueSlug( baseSlug: string, existingSlugs: string[], currentNodeSlug?: string, ): string { let slug = baseSlug; let counter = 1; while (existingSlugs.includes(slug) && slug !== currentNodeSlug) { slug = `${baseSlug}_${counter}`; counter++; } return slug; }Then use these helpers in all three locations.
Also applies to: 313-365, 1221-1249
src/frontend/tests/core/unit/referenceAutocomplete.spec.ts (4)
9-9: Use explicit PlaywrightPagetype instead ofany.The
pageparameter should be typed withPagefrom Playwright for better type safety and IDE support.Suggested fix
+import { Page } from "@playwright/test"; import { expect, test } from "../../fixtures"; import { adjustScreenView } from "../../utils/adjust-screen-view"; import { awaitBootstrapTest } from "../../utils/await-bootstrap-test"; -async function setupFlowWithConnection(page: any) { +async function setupFlowWithConnection(page: Page) {Apply the same change to
setupFlowWithoutUpstreamat line 78.
142-153: Soft assertions may mask regressions.The pattern of checking
if (autocompleteVisible)and logging when not visible means this test can pass even when the feature is broken. Consider usingexpect(autocompleteVisible).toBe(true)to make the test fail definitively when the autocomplete doesn't appear, or skip the test entirely until the feature is fully integrated.Suggested stricter assertion
- if (autocompleteVisible) { - // Verify options are shown - const options = autocomplete.locator("button"); - const optionCount = await options.count(); - expect(optionCount).toBeGreaterThan(0); - } else { - // If autocomplete doesn't show, it means references aren't supported for this field yet - // This is acceptable - the feature may not be fully integrated - console.log( - "Autocomplete not visible - references may not be enabled for this field", - ); - } + expect(autocompleteVisible).toBe(true); + + // Verify options are shown + const options = autocomplete.locator("button"); + const optionCount = await options.count(); + expect(optionCount).toBeGreaterThan(0);
340-346: Avoid fixedwaitForTimeout- prefer conditional waits.Using
waitForTimeout(500)is fragile and can cause flaky tests. Consider using a negative assertion with a timeout instead, which will fail fast if the element appears.Suggested fix
- // Give autocomplete time to potentially appear - await page.waitForTimeout(500); - - // Autocomplete should NOT be visible (no upstream nodes) - const autocomplete = page.getByTestId("reference-autocomplete-dropdown"); - const isVisible = await autocomplete.isVisible().catch(() => false); - expect(isVisible).toBe(false); + // Autocomplete should NOT be visible (no upstream nodes) + const autocomplete = page.getByTestId("reference-autocomplete-dropdown"); + // Use a short timeout to verify it doesn't appear, failing fast if it does + await expect(autocomplete).not.toBeVisible({ timeout: 1000 });
112-115: Add test documentation comments.Per coding guidelines, each test should have a docstring explaining its purpose, scenario, and expected outcomes. Consider adding documentation like:
/** * Test: ReferenceAutocomplete appears on @ trigger with upstream connection * Scenario: User types @ in a text input that supports references with an upstream node connected * Expected: Autocomplete dropdown appears with available upstream outputs as options */ test( "ReferenceAutocomplete - typing @ shows autocomplete when upstream connected", ...Apply similar documentation to all other test cases. Based on learnings from coding guidelines.
src/frontend/src/components/core/parameterRenderComponent/components/strRenderComponent/index.tsx (3)
30-31: Clarify the semantics ofhas_references !== undefined.The condition
templateData.has_references !== undefinedmeans the field "supports" references ifhas_referencesis explicitly set to any value (includingfalse). This is a non-obvious convention. Consider adding a comment explaining thathas_referencesbeing undefined means "not supported" while any defined value (true/false) means "supported".Suggested clarification
- // Check if this field supports references (has_references is defined, not undefined) - const supportsReferences = templateData.has_references !== undefined; + // Check if this field supports references. + // Convention: has_references === undefined means references are not supported for this field. + // has_references === false means supported but no refs present; true means refs are present. + const supportsReferences = templateData.has_references !== undefined;
86-93: Potential redundant state update.
updateHasReferences(hasRefs)andhandleOnNewValue({ value: newValue, has_references: hasRefs })both updatehas_referencesin the node class template. This may cause unnecessary renders. Consider whether one of these calls is sufficient.Potential simplification
If
handleOnNewValuealready handles updatinghas_referencesin the template, you can remove the explicitupdateHasReferencescall:const handleReferenceInputChange = useCallback( (newValue: string, hasRefs: boolean) => { - updateHasReferences(hasRefs); handleOnNewValue({ value: newValue, has_references: hasRefs }); }, - [updateHasReferences, handleOnNewValue], + [handleOnNewValue], );Please verify whether
handleOnNewValuealready handles updatinghas_referencesin the template to confirm if the explicitupdateHasReferencescall is redundant.
138-143: Unusedrefprop from ReferenceInput children.The destructured props include
actualValue: storedValueandrefis available but not used. ThestoredValueis unused, andrefshould likely be passed toTextAreaComponentfor proper ref forwarding.Consider using the ref
{({ value: inputValue, - actualValue: storedValue, + actualValue: _storedValue, onChange, onKeyDown, + ref, }) => ( <TextAreaComponent {...baseInputProps} value={inputValue} nodeId={nodeId} onKeyDown={onKeyDown} + ref={ref}Note: This requires
TextAreaComponentto support ref forwarding. Verify if this is needed for autocomplete positioning.src/frontend/src/components/core/referenceInput/ReferenceInput.tsx (3)
78-83: Consider memoizingupstreamOutputscomputation.
getUpstreamOutputsis called on every render. If this function performs graph traversal, consider wrapping it inuseMemowith appropriate dependencies to avoid unnecessary recomputation.Suggested memoization
- const upstreamOutputs = getUpstreamOutputs( - nodeId, - nodes, - edges, - nodeReferenceSlugs, - ); + const upstreamOutputs = useMemo( + () => getUpstreamOutputs(nodeId, nodes, edges, nodeReferenceSlugs), + [nodeId, nodes, edges, nodeReferenceSlugs], + );
154-159: Navigation keys may cause unintended behavior.When autocomplete is open, the code returns without calling
e.preventDefault()forArrowDown,ArrowUp,Enter, andEscape. This means these keys will also trigger their default behavior (e.g., Enter may submit a form, arrows may move the cursor). Consider preventing default for these keys to avoid side effects.Suggested fix
if (isAutocompleteOpen) { if (["ArrowDown", "ArrowUp", "Enter", "Escape"].includes(e.key)) { // Let autocomplete handle these + e.preventDefault(); return; } }Verify if
ReferenceAutocompletealready handlespreventDefaultinternally, in which case this change may not be needed.
230-232: Type assertion may be unnecessary.The
inputRefis already typed asReact.RefObject<HTMLInputElement | HTMLTextAreaElement>, so the cast might be redundant. However, this is harmless.Potential simplification
- ref: inputRef as React.RefObject< - HTMLInputElement | HTMLTextAreaElement - >, + ref: inputRef,
| editNode = false, | ||
| id = "", | ||
| readonly = false, | ||
| nodeId, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the promptComponent/index.tsx file to see the full context
echo "=== promptComponent/index.tsx ==="
cat -n src/frontend/src/components/core/parameterRenderComponent/components/promptComponent/index.tsx | head -120
echo -e "\n=== PromptModal signature ==="
# Find PromptModal component and check its props
fd promptModal -t f | head -5
cat -n src/frontend/src/modals/promptModal/index.tsx | head -50Repository: langflow-ai/langflow
Length of output: 7152
🏁 Script executed:
#!/bin/bash
# Search for nodeId usage in promptComponent and related files
echo "=== nodeId usage in promptComponent ==="
rg -n "nodeId" src/frontend/src/components/core/parameterRenderComponent/components/promptComponent/ --type tsx
echo -e "\n=== PromptModal nodeId usage ==="
rg -n "nodeId" src/frontend/src/modals/promptModal/ --type tsx
echo -e "\n=== Where PromptModal is used with nodeId ==="
rg -n "PromptModal" src/frontend/src/ --type tsx -A 5 | grep -E "(PromptModal|nodeId)" | head -30Repository: langflow-ai/langflow
Length of output: 285
🏁 Script executed:
#!/bin/bash
# Check the interface/type definitions for PromptModal
echo "=== Check for PromptModal props interface ==="
rg -n "interface.*Prompt.*Props\|type.*Prompt.*Props" src/frontend/src/modals/promptModal/ --type tsx
# Check if PromptModal component definition mentions nodeId
echo -e "\n=== PromptModal component definition ==="
rg -n "function.*PromptModal\|const.*PromptModal.*=\|export.*PromptModal" src/frontend/src/modals/promptModal/ -A 10 --type tsx | head -50Repository: langflow-ai/langflow
Length of output: 224
Unused nodeId prop must be addressed — PromptModal doesn't currently accept this prop.
The nodeId prop is destructured at line 27 but never used in the component. PromptModal's signature (lines 103-111) does not include nodeId as a parameter, so the suggested fix to pass it through won't work without first updating PromptModal to accept it.
Either:
- Remove the unused
nodeIdprop if it's not needed, or - Update PromptModal to accept and use
nodeIdfor reference support, then pass it through from this component.
Clarify the intent and complete the implementation accordingly.
🤖 Prompt for AI Agents
In
`@src/frontend/src/components/core/parameterRenderComponent/components/promptComponent/index.tsx`
at line 27, The destructured but unused nodeId prop should be fixed: either
remove nodeId from the component's props and the destructuring at the top (clean
up the props type/interface) if it's not needed, or if nodeId is intended to be
used, add nodeId to PromptModal's props signature and type (update PromptModal's
parameter list and prop type) and then pass nodeId through when rendering
PromptModal from this component so PromptModal receives and can use it; update
any related prop type/interface declarations to keep types consistent.
| {({ value: inputValue, onChange }) => ( | ||
| <InputGlobalComponent | ||
| {...baseInputProps} | ||
| value={inputValue} | ||
| handleOnNewValue={handleInputGlobalNewValue(onChange)} | ||
| password={templateData.password} | ||
| load_from_db={templateData.load_from_db} | ||
| placeholder={placeholder} | ||
| display_name={display_name} | ||
| id={`input-${name}`} | ||
| isToolMode={isToolMode} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the file and get context around lines 215-227
wc -l src/frontend/src/components/core/parameterRenderComponent/components/strRenderComponent/index.tsxRepository: langflow-ai/langflow
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Read the file around the mentioned lines with more context
sed -n '200,240p' src/frontend/src/components/core/parameterRenderComponent/components/strRenderComponent/index.tsx | cat -nRepository: langflow-ai/langflow
Length of output: 1584
🏁 Script executed:
#!/bin/bash
# Search for InputGlobalComponent to find its definition and props
fd InputGlobalComponentRepository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for ReferenceInput component
fd ReferenceInputRepository: langflow-ai/langflow
Length of output: 212
🏁 Script executed:
#!/bin/bash
# Read the ReferenceInput component to see its signature
cat -n src/frontend/src/components/core/referenceInput/ReferenceInput.tsxRepository: langflow-ai/langflow
Length of output: 9934
🏁 Script executed:
#!/bin/bash
# Search for InputGlobalComponent definition
rg "export.*InputGlobalComponent|function InputGlobalComponent|const InputGlobalComponent" -A 5Repository: langflow-ai/langflow
Length of output: 1357
🏁 Script executed:
#!/bin/bash
# Look for multiline version in the same file - search for "multiline" context
sed -n '150,210p' src/frontend/src/components/core/parameterRenderComponent/components/strRenderComponent/index.tsx | cat -nRepository: langflow-ai/langflow
Length of output: 2665
🏁 Script executed:
#!/bin/bash
# Get full InputGlobalComponent definition
cat -n src/frontend/src/components/core/parameterRenderComponent/components/inputGlobalComponent/index.tsxRepository: langflow-ai/langflow
Length of output: 5530
🏁 Script executed:
#!/bin/bash
# Check for the multiline ReferenceInput wrapper to compare - look for the actual full multiline section
sed -n '100,210p' src/frontend/src/components/core/parameterRenderComponent/components/strRenderComponent/index.tsx | cat -nRepository: langflow-ai/langflow
Length of output: 4595
🏁 Script executed:
#!/bin/bash
# Check InputComponent to see if it accepts onKeyDown
cat -n src/frontend/src/components/core/parameterRenderComponent/components/inputComponent/index.tsx | head -100Repository: langflow-ai/langflow
Length of output: 3871
🏁 Script executed:
#!/bin/bash
# Check the full InputComponent props type
rg "interface InputProps|type InputProps" -A 20 src/frontend/src/components/core/parameterRenderComponent/types.tsRepository: langflow-ai/langflow
Length of output: 555
🏁 Script executed:
#!/bin/bash
# Check if InputComponent accepts onKeyDown prop - search full file
rg "onKeyDown" src/frontend/src/components/core/parameterRenderComponent/components/inputComponent/index.tsxRepository: langflow-ai/langflow
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Check InputComponentType definition
rg "type InputComponentType|interface InputComponentType" -A 50 src/frontend/srcRepository: langflow-ai/langflow
Length of output: 3736
🏁 Script executed:
#!/bin/bash
# Check InputGlobalComponent's full parameter list and how it renders
cat -n src/frontend/src/components/core/parameterRenderComponent/components/inputGlobalComponent/index.tsx | grep -A 30 "export default function InputGlobalComponent"Repository: langflow-ai/langflow
Length of output: 1144
🏁 Script executed:
#!/bin/bash
# Check what InputComponent does with onKeyDown
sed -n '1,150p' src/frontend/src/components/core/parameterRenderComponent/components/inputComponent/index.tsx | cat -nRepository: langflow-ai/langflow
Length of output: 6108
🏁 Script executed:
#!/bin/bash
# Check TextAreaComponent to understand how it handles onKeyDown
fd TextAreaComponent src/frontend/src/components/core/parameterRenderComponent/components/Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the full InputGlobalComponent render to see what it returns
sed -n '120,142p' src/frontend/src/components/core/parameterRenderComponent/components/inputGlobalComponent/index.tsx | cat -nRepository: langflow-ai/langflow
Length of output: 900
🏁 Script executed:
#!/bin/bash
# Find TextAreaComponent to check how it handles onKeyDown
fd -t f "TextAreaComponent" src/frontend/src/components/Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the actual line 229 of ReferenceInput where onKeyDown is provided to children
sed -n '223,233p' src/frontend/src/components/core/referenceInput/ReferenceInput.tsx | cat -nRepository: langflow-ai/langflow
Length of output: 476
🏁 Script executed:
#!/bin/bash
# Look at the full comparison - check if InputComponent accepts onKeyDown or if we need to add it
cat -n src/frontend/src/types/components/index.ts | grep -A 50 "export type InputComponentType"Repository: langflow-ai/langflow
Length of output: 1900
🏁 Script executed:
#!/bin/bash
# Verify: does InputComponent use onKeyDown from props or hardcoded?
sed -n '100,110p' src/frontend/src/components/core/parameterRenderComponent/components/inputComponent/index.tsxRepository: langflow-ai/langflow
Length of output: 369
🏁 Script executed:
#!/bin/bash
# Check if there's a way InputGlobalComponent could pass through onKeyDown
rg "handleOnNewValue|handleInputChange" src/frontend/src/components/core/parameterRenderComponent/components/inputGlobalComponent/index.tsx -A 3Repository: langflow-ai/langflow
Length of output: 1008
🏁 Script executed:
#!/bin/bash
# Final verification: confirm TextAreaComponent accepts onKeyDown
rg "TextAreaComponent" src/frontend/src/components/core/parameterRenderComponent/components/ -A 1 -B 1Repository: langflow-ai/langflow
Length of output: 5021
🏁 Script executed:
#!/bin/bash
# Check what props TextAreaComponent accepts
fd -t f TextAreaComponent src/frontend/src/Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Get the full TextAreaComponent definition to confirm onKeyDown handling
cat -n src/frontend/src/components/core/parameterRenderComponent/components/textAreaComponent/index.tsx | head -80Repository: langflow-ai/langflow
Length of output: 3451
Missing onKeyDown handler prevents @ autocomplete in single-line reference input.
The single-line ReferenceInput wrapper doesn't destructure the onKeyDown prop, which means ReferenceInput's @ key detection (line 122 of ReferenceInput.tsx) never fires. Without it, the autocomplete won't trigger when users type @ in single-line inputs with references.
This differs from the multiline case, which correctly destructures and forwards onKeyDown to TextAreaComponent. However, InputGlobalComponent (used for single-line) does not accept onKeyDown as a prop, and InputComponent (which it renders) has a hardcoded handler instead. To fix this, either:
- Modify
InputGlobalComponentandInputComponentto accept and forward anonKeyDownprop, or - Replace
InputGlobalComponentwithTextAreaComponent(which already supportsonKeyDown) for the single-line reference input case.
| function getGradientClass( | ||
| disabled: boolean, | ||
| editNode: boolean, | ||
| password: boolean, | ||
| ): string { | ||
| if (disabled || password) return ""; | ||
| if (editNode) return "gradient-fade-input-edit-node"; | ||
| return "gradient-fade-input"; | ||
| } | ||
|
|
||
| function getBackgroundClass(disabled: boolean, editNode: boolean): string { | ||
| if (disabled) return ""; | ||
| if (editNode) return "background-fade-input-edit-node"; | ||
| return "background-fade-input"; | ||
| } |
There was a problem hiding this comment.
getBackgroundClass is defined but never used.
The getGradientClass function is used in rendering (line 142), but getBackgroundClass is never called anywhere in this file. This appears to be dead code.
🔧 Suggested fix
Either remove the unused function or use it appropriately. If it was intended to be used for the background styling on line 149, consider:
-function getBackgroundClass(disabled: boolean, editNode: boolean): string {
- if (disabled) return "";
- if (editNode) return "background-fade-input-edit-node";
- return "background-fade-input";
-}Or if it should be used:
style={{
pointerEvents: "none",
- background: disabled ? "bg-background" : GRADIENT_CLASS,
+ background: GRADIENT_CLASS,
}}
+ className={cn(
+ getGradientClass(disabled, editNode, password!),
+ getBackgroundClass(disabled, editNode),
+ ...
+ )}📝 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.
| function getGradientClass( | |
| disabled: boolean, | |
| editNode: boolean, | |
| password: boolean, | |
| ): string { | |
| if (disabled || password) return ""; | |
| if (editNode) return "gradient-fade-input-edit-node"; | |
| return "gradient-fade-input"; | |
| } | |
| function getBackgroundClass(disabled: boolean, editNode: boolean): string { | |
| if (disabled) return ""; | |
| if (editNode) return "background-fade-input-edit-node"; | |
| return "background-fade-input"; | |
| } | |
| function getGradientClass( | |
| disabled: boolean, | |
| editNode: boolean, | |
| password: boolean, | |
| ): string { | |
| if (disabled || password) return ""; | |
| if (editNode) return "gradient-fade-input-edit-node"; | |
| return "gradient-fade-input"; | |
| } |
🤖 Prompt for AI Agents
In
`@src/frontend/src/components/core/parameterRenderComponent/components/textAreaComponent/index.tsx`
around lines 30 - 44, getBackgroundClass is dead code; either remove it or wire
it into the component render where styling classes are applied. If you choose to
use it, replace the hard-coded/background class usage on the same element that
calls getGradientClass with a call to getBackgroundClass(disabled, editNode)
(ensure the same disabled and editNode props/locals are passed) so background
styles are computed consistently; otherwise delete the getBackgroundClass
function and its tests/imports to eliminate the unused symbol.
| # Handle Message objects - extract the text content | ||
| if isinstance(value, Message): | ||
| return value.text or "" |
There was a problem hiding this comment.
Potential issue: Message.text can be an iterator.
Per the Message schema (lines 35-391 in schema.py), text is typed as str | AsyncIterator | Iterator | None. If the text is an async/sync iterator, accessing value.text or "" will return the iterator object rather than the text content.
Suggested fix
# Handle Message objects - extract the text content
if isinstance(value, Message):
- return value.text or ""
+ text = value.text
+ if text is None:
+ return ""
+ if isinstance(text, str):
+ return text
+ # If text is an iterator, it may not have been consumed yet
+ # Return empty string or raise an error
+ return ""| def generate_reference_slug(display_name: str) -> str: | ||
| """Generate a reference slug from a display name. | ||
| Converts "HTTP Request" to "HTTPRequest", removing spaces and special chars. | ||
| Args: | ||
| display_name: The display name to convert | ||
| Returns: | ||
| A valid slug for use in @references | ||
| """ | ||
| if not display_name: | ||
| return "Node" | ||
|
|
||
| # Remove all non-alphanumeric characters and capitalize each word | ||
| slug = re.sub(r"[^a-zA-Z0-9]", "", display_name.title().replace(" ", "")) | ||
|
|
||
| return slug if slug else "Node" |
There was a problem hiding this comment.
title() may not preserve intended casing for acronyms.
The docstring says "HTTP Request" converts to "HTTPRequest", but "HTTP Request".title() produces "Http Request", resulting in "HttpRequest" (not "HTTPRequest"). If preserving acronym casing is important, consider an alternative approach.
If this behavior is intentional (normalizing all slugs to title case), the docstring should be updated to reflect the actual output.
📝 Suggested docstring fix
def generate_reference_slug(display_name: str) -> str:
"""Generate a reference slug from a display name.
- Converts "HTTP Request" to "HTTPRequest", removing spaces and special chars.
+ Converts "HTTP Request" to "HttpRequest", removing spaces and special chars.
Args:
display_name: The display name to convert📝 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.
| def generate_reference_slug(display_name: str) -> str: | |
| """Generate a reference slug from a display name. | |
| Converts "HTTP Request" to "HTTPRequest", removing spaces and special chars. | |
| Args: | |
| display_name: The display name to convert | |
| Returns: | |
| A valid slug for use in @references | |
| """ | |
| if not display_name: | |
| return "Node" | |
| # Remove all non-alphanumeric characters and capitalize each word | |
| slug = re.sub(r"[^a-zA-Z0-9]", "", display_name.title().replace(" ", "")) | |
| return slug if slug else "Node" | |
| def generate_reference_slug(display_name: str) -> str: | |
| """Generate a reference slug from a display name. | |
| Converts "HTTP Request" to "HttpRequest", removing spaces and special chars. | |
| Args: | |
| display_name: The display name to convert | |
| Returns: | |
| A valid slug for use in `@references` | |
| """ | |
| if not display_name: | |
| return "Node" | |
| # Remove all non-alphanumeric characters and capitalize each word | |
| slug = re.sub(r"[^a-zA-Z0-9]", "", display_name.title().replace(" ", "")) | |
| return slug if slug else "Node" |
🤖 Prompt for AI Agents
In `@src/lfx/src/lfx/graph/vertex/base.py` around lines 42 - 59, The function
generate_reference_slug uses display_name.title() which lowercases acronyms
(e.g., "HTTP Request" -> "HttpRequest") contrary to the docstring; either update
the docstring to match the current behavior or change the implementation to
preserve existing casing. To preserve acronyms, remove the .title() call and
instead strip non-alphanumeric characters and spaces (e.g., use re.sub on the
original display_name and concat words) so "HTTP Request" becomes "HTTPRequest";
update the docstring to describe the chosen behavior and keep the fallback to
"Node" when the result is empty.
Wrap InputGlobalComponent with onKeyDown handler from ReferenceInput to enable @ key detection for autocomplete triggering. Also add onKeyDown prop support to InputComponent and InputComponentType.
Message.text can be AsyncIterator or Iterator, not just string. Add type check to handle this case and return empty string for iterators that haven't been consumed yet.
title() produces "HttpRequest" not "HTTPRequest" - updated docstring to reflect actual behavior with acronyms.
github-actions
bot
added
enhancement
- Add tests for reference parsing (test_parser.py) - Add tests for reference resolution (test_resolver.py) - Add tests for reference schema (test_schema.py) - Add tests for vertex slug generation (test_reference_slug.py) - Add tests for param handler references (test_param_handler_references.py) - Add tests for get_vertex_by_slug (test_get_vertex_by_slug.py)
github-actions
bot
added
enhancement
ogabrielluiz
changed the title
github-actions
bot
added
enhancement
There was a problem hiding this comment.
Pull request overview
This PR adds support for inline variable references in text inputs using @Component.output syntax, enabling users to reference outputs from upstream nodes directly in text fields.
Changes:
- Backend: Reference parsing, resolution infrastructure with slug-based node identification
- Frontend: Autocomplete component with keyboard navigation and real-time filtering
- Tests: 73 Jest unit tests and 7 Playwright e2e tests
Reviewed changes
Copilot reviewed 46 out of 49 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/lfx/src/lfx/graph/reference/* |
Backend reference parsing, resolution, and schema |
src/lfx/src/lfx/inputs/input_mixin.py |
ReferenceMixin for inputs supporting references |
src/lfx/src/lfx/graph/vertex/param_handler.py |
Integration with parameter processing for reference resolution |
src/lfx/src/lfx/graph/vertex/base.py |
Slug generation and vertex reference_slug property |
src/lfx/src/lfx/graph/graph/base.py |
Slug indexing and lookup |
src/frontend/src/components/core/referenceInput/* |
ReferenceInput and ReferenceAutocomplete components |
src/frontend/src/utils/* |
Reference parsing, upstream output collection, caret positioning |
src/frontend/src/stores/flowStore.ts |
Node slug tracking and generation |
| Test files | Comprehensive unit and e2e tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_parse_adjacent_references(): | ||
| text = "@[email protected]" | ||
| refs = parse_references(text) | ||
| assert len(refs) == 2 |
There was a problem hiding this comment.
This test expects 2 references but the negative lookbehind (?<!\\w) in the regex pattern prevents matching @Node2 because it's preceded by '1' (a word character). Based on the frontend test test_parse_consecutive_references_no_space, only the first reference should match. The assertion should be assert len(refs) == 1.
| assert len(refs) == 2 | |
| assert len(refs) == 1 |
| def generate_reference_slug(display_name: str) -> str: | ||
| """Generate a reference slug from a display name. | ||
| Converts "HTTP Request" to "HttpRequest", removing spaces and special chars. | ||
| Note: Uses title case normalization, so acronyms become title-cased. |
There was a problem hiding this comment.
The docstring note about acronyms is misleading. title() converts 'HTTP' to 'Http', not 'HTTP' (all caps). The comment should clarify: 'Uses title() which converts each word's first letter to uppercase and rest to lowercase, so "HTTP" becomes "Http"'.
| def _resolve_field_references(self, value: str, field: dict) -> str: | ||
| """Resolve @references in a field value. | ||
| Args: | ||
| value: The field value containing references | ||
| field: The field definition | ||
| Returns: | ||
| The value with references resolved | ||
| Raises: | ||
| ReferenceResolutionError: If resolution fails | ||
| """ | ||
| if not self._should_resolve_references(field): | ||
| return value | ||
|
|
||
| if not isinstance(value, str): | ||
| return value | ||
|
|
||
| return resolve_references(value, self.vertex.graph) |
There was a problem hiding this comment.
The _resolve_field_references method is defined but never called. Lines 194-201 call resolve_references directly instead of using this helper method. Either remove this unused method or refactor the code to use it.
src/frontend/src/stores/flowStore.ts
Outdated
|
|
||
| // Generate reference slugs for all nodes | ||
| const newSlugs: Record<string, string> = {}; | ||
| const slugCounts: Record<string, number> = {}; | ||
| const existingSlugs: string[] = []; | ||
|
|
||
| for (const node of nodes) { | ||
| const nodeData = node.data as NodeDataType; | ||
| const displayName = nodeData?.node?.display_name || "Node"; | ||
| // Remove non-alphanumeric, capitalize words | ||
| const baseSlug = | ||
| displayName | ||
| .split(/\s+/) | ||
| .map((word: string) => word.charAt(0).toUpperCase() + word.slice(1)) | ||
| .join("") | ||
| .replace(/[^a-zA-Z0-9]/g, "") || "Node"; |
There was a problem hiding this comment.
The slug generation logic is duplicated in three locations: setInitialFlow (lines 237-265), setNodes (lines 314-366), and generateSlugForNode (lines 1221-1249). Extract this into a shared utility function to maintain consistency and reduce duplication.
| const extractStringValue = useCallback((newValue: unknown): string | null => { | ||
| if (typeof newValue === "string") return newValue; | ||
| if ( | ||
| typeof newValue === "object" && | ||
| newValue !== null && | ||
| "value" in newValue && | ||
| typeof (newValue as { value: unknown }).value === "string" | ||
| ) { | ||
| return (newValue as { value: string }).value; | ||
| } | ||
| return null; | ||
| }, []); |
There was a problem hiding this comment.
The extractStringValue callback has no dependencies but is defined inside the component. Move it outside the component as a standalone helper function to avoid unnecessary re-creation on every render.
| if isinstance(parsed_value, dict): | ||
| return parsed_value.get("model_name"), parsed_value.get("provider") | ||
| except (ValueError, json.JSONDecodeError, TypeError): | ||
| except (ValueError, TypeError): # json.JSONDecodeError is a subclass of ValueError |
There was a problem hiding this comment.
The comment is correct that json.JSONDecodeError is a subclass of ValueError (since Python 3.5), but this change seems unrelated to the inline variable references feature. If this is intentional cleanup, it should be in a separate PR or at least mentioned in the PR description.
| @@ -1,5 +1,5 @@ | |||
| import type React from "react"; | |||
| import { useEffect, useRef, useState } from "react"; | |||
| import { useCallback, useEffect, useRef, useState } from "react"; | |||
There was a problem hiding this comment.
Unused import useCallback.
| import { useCallback, useEffect, useRef, useState } from "react"; | |
| import { useEffect, useRef, useState } from "react"; |
| : disabled | ||
| ? "bg-background" | ||
| : GRADIENT_CLASS, | ||
| background: disabled ? "bg-background" : GRADIENT_CLASS, |
There was a problem hiding this comment.
This use of variable 'disabled' always evaluates to false.
| background: disabled ? "bg-background" : GRADIENT_CLASS, | |
| background: GRADIENT_CLASS, |
|
|
||
| with patch.object(handler, "_resolve_field_references") as mock_resolve: | ||
| mock_resolve.return_value = "Hello resolved_value" | ||
| result = handler._resolve_field_references("Hello @Node.output", field) |
There was a problem hiding this comment.
Variable result is not used.
| result = handler._resolve_field_references("Hello @Node.output", field) | |
| handler._resolve_field_references("Hello @Node.output", field) |
github-actions
bot
added
enhancement
- Fix test_parser.py assertion: adjacent references test now correctly expects 1 match (negative lookbehind prevents second match) - Remove unused _resolve_field_references method from param_handler.py - Extract generateBaseSlug utility to reduce code duplication in flowStore.ts - Remove unused useCallback import from promptModal - Simplify textAreaComponent background (disabled is always false in context) - Clean up test_param_handler_references.py tests - Fix pre-existing lint issues (noExplicitAny, noAssignInExpressions)
github-actions
bot
added
enhancement
Summary
Adds support for inline variable references in text inputs using
@Component.outputsyntax. This enables users to reference outputs from upstream nodes directly in text fields, similar to template variable syntax but inline.Backend (lfx):
Frontend:
Tests:
Minor cleanup:
unified_models.pyaboutjson.JSONDecodeErrorbeing a subclass ofValueErrorSummary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.