feat(studio): complete Houdini workflow editing and domestic OODA - #48
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes add collect-per-source handling for OpenCLI workflows, expand workflow editor primitives and inspector interactions, and broaden domestic OODA source configuration with updated regression tests. ChangesOpenCLI execution status handling
Workflow authoring experience
Domestic OODA workflow sources
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
|
✅ Health: 8.0 📋 At a glance 🚨 Change risk: 9.6/10 (high)
📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-27 18:24 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
frontend/scripts/check-opencli-business-workflows.mjs (1)
16-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the actual slot and gap records, not independent file-wide strings.
These regexes can pass when a required
site,command, andsourceGroupoccur in different objects; they also do not enforce 14 slots or prohibit a runnablegelonghuislot. Scope assertions toASHARE_OPENCLI_SOURCESand assert exact{ id, sourceGroup, site, command }tuples, slot count, and that no source slot uses a site marked unavailable.As per coding guidelines, “Do not claim completion without fresh evidence”; these checks need to prove the configured records rather than incidental text.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/scripts/check-opencli-business-workflows.mjs` around lines 16 - 37, Strengthen the tests around ASHARE_OPENCLI_SOURCES instead of matching independent file-wide strings: extract the configured source records and assert exactly 14 slots with the expected { id, sourceGroup, site, command } tuples. Also validate the explicit gap records and ensure no runnable source slot uses a site marked unavailable, including gelonghui, while preserving the existing deterministic, raw-item, and empty-display expectations.Source: Coding guidelines
tests/integration/test_workflow_opencli_hda_trace_api.py (1)
1008-1013: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider asserting the successful source's terminal status too.
The test verifies
multi-source-opencli::source-xiaohongshuisfailedbut doesn't explicitly assertmulti-source-opencli::source-bilibiliiscompleted, only inferring it from the overallpartial_successoutcome.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_workflow_opencli_hda_trace_api.py` around lines 1008 - 1013, Extend the node-state assertions in the workflow test to explicitly verify that states["multi-source-opencli::source-bilibili"]["status"] is "completed", alongside the existing failed Xiaohongshu assertion.backend/workflow/opencli_hda_tracer.py (1)
2184-2210: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid re-deriving invariant state per node in
_collect_per_source_package_ids.This helper is invoked once per
node_statefrom_run_status's dict comprehension (line 2124-2130). Each call does a linearnext(... for node in runtime_nodes ...)lookup and rebuildstolerant_package_idsfrom scratch via a full scan ofruntime_nodes, even though both are invariant across all states for a givenruntime_nodeslist. This is O(N·M) redundant work; for typical workflow sizes this is negligible, but it's an easy, low-risk cleanup.♻️ Proposed refactor: precompute lookups once in `_run_status`
def _run_status( node_states: list[WorkflowRunNodeState], valid: bool, runtime_nodes: list[CompiledWorkflowNode] | None = None, ) -> WorkflowRunStatus: if not valid: return "failed" statuses = {state.status for state in node_states} + nodes_by_id = {node.id: node for node in (runtime_nodes or [])} + tolerant_package_ids = { + node.id for node in (runtime_nodes or []) if _collects_per_source_failures(node) + } collect_per_source_package_ids_by_state = ( { - state.nodeId: _collect_per_source_package_ids(state, runtime_nodes) + state.nodeId: _collect_per_source_package_ids( + state, nodes_by_id, tolerant_package_ids + ) for state in node_states } if runtime_nodes else {} )def _collect_per_source_package_ids( state: WorkflowRunNodeState, - runtime_nodes: list[CompiledWorkflowNode], + nodes_by_id: dict[str, CompiledWorkflowNode], + tolerant_package_ids: set[str], ) -> set[str]: source_groups = getattr(state, "sourceGroups", []) node_path = getattr(state, "nodePath", []) - runtime_node = next( - (node for node in runtime_nodes if node.id == state.nodeId), - None, - ) + runtime_node = nodes_by_id.get(state.nodeId) if ( not source_groups or runtime_node is None or not ( _read_string(runtime_node.params.get("sourceGroup")) or _read_string(runtime_node.params.get("source_group")) ) ): return set() - tolerant_package_ids = { - node.id for node in runtime_nodes if _collects_per_source_failures(node) - } return {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/workflow/opencli_hda_tracer.py` around lines 2184 - 2210, Refactor `_run_status` and `_collect_per_source_package_ids` so invariant data is computed once per `runtime_nodes` list: prebuild a node-id lookup and the set of IDs from `_collects_per_source_failures`, then pass or reuse these values for each state instead of performing `next(...)` and rescanning `runtime_nodes` inside every helper call. Preserve the helper’s existing filtering and returned package IDs.frontend/components/flow/nodes/workflow-node.tsx (1)
403-448: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPort handle
aria-labelhardcodes Chinese text regardless of the language toggle.
"输出"/"输入"are hardcoded in thearia-label, while the rest of this component (and the app) respects thelanguagesetting (zh-CN/en-US) for user-facing/assistive text. English-language users will still get a Chinese-only accessibility label for every port handle.🌐 Suggested fix
- "aria-label": `${isBusinessLevel ? businessLabel : nodeViewContract.identity.label} · ${handleType === "source" ? "输出" : "输入"} · ${port.id ?? "default"} · ${port.type ?? "unknown"}`, + "aria-label": `${isBusinessLevel ? businessLabel : nodeViewContract.identity.label} · ${handleType === "source" ? (language === "zh-CN" ? "输出" : "Output") : (language === "zh-CN" ? "输入" : "Input")} · ${port.id ?? "default"} · ${port.type ?? "unknown"}`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/flow/nodes/workflow-node.tsx` around lines 403 - 448, Update the port handle aria-label construction in the returned attributes to use the active language setting for the source/output and target/input direction text instead of hardcoded Chinese strings. Preserve the existing label structure and provide English wording when the language is en-US while retaining Chinese wording for zh-CN.frontend/components/flow/inspector.tsx (1)
1016-1033: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win"Insert variable" replaces the whole field instead of inserting.
variableSelector'sonValueChangecallsupdateParameterField(field, value), which overwrites the entire field value with just the selected{{portId}}reference. For textarea/template fields where users compose static text plus one or more variable references, selecting a variable wipes out anything already typed (including a previously-inserted variable). The label ("Reference upstream output" / "引用上游输出") implies composing, not replacing.♻️ Lower-effort improvement: append instead of replace
- <Select onValueChange={(value) => value && updateParameterField(field, value)}> + <Select onValueChange={(value) => value && updateParameterField(field, `${typeof raw === "string" ? raw : ""}${value}`)}>True cursor-position insertion would be a better long-term fix but requires tracking caret position in the textarea/input ref.
Also applies to: 1123-1133, 1261-1271
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/flow/inspector.tsx` around lines 1016 - 1033, The onValueChange callback in variableSelector overwrites the entire field value instead of appending the selected variable reference. Update the callback to append the selected value to the existing field content rather than replacing it entirely. This applies to all three occurrences of variableSelector in the file (at the ranges mentioned). The fix should preserve any previously typed text or inserted variables by appending the new variable reference to what is already in the field, not overwriting it with updateParameterField(field, value).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/components/flow/inspector.tsx`:
- Around line 1323-1356: Update the upstream variable option construction in the
node inspector so deduplication uses both the candidate source node identity and
output port id, rather than only workflowInputReferenceForPort(port.id).
Preserve distinct picker entries for different upstream nodes while keeping each
option’s binding value correctly scoped to its selected source.
---
Nitpick comments:
In `@backend/workflow/opencli_hda_tracer.py`:
- Around line 2184-2210: Refactor `_run_status` and
`_collect_per_source_package_ids` so invariant data is computed once per
`runtime_nodes` list: prebuild a node-id lookup and the set of IDs from
`_collects_per_source_failures`, then pass or reuse these values for each state
instead of performing `next(...)` and rescanning `runtime_nodes` inside every
helper call. Preserve the helper’s existing filtering and returned package IDs.
In `@frontend/components/flow/inspector.tsx`:
- Around line 1016-1033: The onValueChange callback in variableSelector
overwrites the entire field value instead of appending the selected variable
reference. Update the callback to append the selected value to the existing
field content rather than replacing it entirely. This applies to all three
occurrences of variableSelector in the file (at the ranges mentioned). The fix
should preserve any previously typed text or inserted variables by appending the
new variable reference to what is already in the field, not overwriting it with
updateParameterField(field, value).
In `@frontend/components/flow/nodes/workflow-node.tsx`:
- Around line 403-448: Update the port handle aria-label construction in the
returned attributes to use the active language setting for the source/output and
target/input direction text instead of hardcoded Chinese strings. Preserve the
existing label structure and provide English wording when the language is en-US
while retaining Chinese wording for zh-CN.
In `@frontend/scripts/check-opencli-business-workflows.mjs`:
- Around line 16-37: Strengthen the tests around ASHARE_OPENCLI_SOURCES instead
of matching independent file-wide strings: extract the configured source records
and assert exactly 14 slots with the expected { id, sourceGroup, site, command }
tuples. Also validate the explicit gap records and ensure no runnable source
slot uses a site marked unavailable, including gelonghui, while preserving the
existing deterministic, raw-item, and empty-display expectations.
In `@tests/integration/test_workflow_opencli_hda_trace_api.py`:
- Around line 1008-1013: Extend the node-state assertions in the workflow test
to explicitly verify that
states["multi-source-opencli::source-bilibili"]["status"] is "completed",
alongside the existing failed Xiaohongshu assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 81ccf603-f556-4658-bc77-82ebca7c366f
📒 Files selected for processing (17)
backend/workflow/hda_templates.pybackend/workflow/opencli_hda_tracer.pyfrontend/components/flow/command-palette.tsxfrontend/components/flow/inspector-shell.tsxfrontend/components/flow/inspector.tsxfrontend/components/flow/nodes/workflow-node.tsxfrontend/lib/workflow/node-primitives.tsfrontend/lib/workflow/opencli-business-workflows.tsfrontend/lib/workflow/studio-templates.tsfrontend/lib/workflow/workflow-outline.tsfrontend/package.jsonfrontend/scripts/check-dify-p0-regressions.mjsfrontend/scripts/check-inspector-workflow-regressions.mjsfrontend/scripts/check-opencli-business-workflows.mjsfrontend/scripts/check-workflow-regressions.mjstests/integration/test_workflow_compile_api.pytests/integration/test_workflow_opencli_hda_trace_api.py
Comment on lines
+1323
to
+1356
| const upstreamNodeIds = workflowDirectUpstreamNodeIds(node.id, edges) | ||
| const upstreamVariableOptions = Array.from(new Map(nodes.flatMap((candidate) => { | ||
| if (!upstreamNodeIds.has(candidate.id)) return [] | ||
| const candidateProjectNode = hydrateProjectNodeIdentity( | ||
| findWorkflowProjectNodeByCanvasId(workflowProject, candidate.id), | ||
| candidate.data, | ||
| ) | ||
| const candidateContract = buildCanonicalNodeViewContract( | ||
| candidateProjectNode, | ||
| candidate.data, | ||
| candidate.id, | ||
| ) | ||
| const localized = localizeNodeText( | ||
| getNodeDisplayId(candidate.data), | ||
| { label: candidate.data.label, description: candidate.data.description }, | ||
| language, | ||
| ) | ||
| return candidateContract.ports | ||
| .filter((port) => port.direction === "output") | ||
| .flatMap((port) => { | ||
| const value = workflowInputReferenceForPort(port.id) | ||
| return value ? [{ | ||
| value, | ||
| label: `${localized.label} · ${port.id} (${port.type})`, | ||
| }] : [] | ||
| }) | ||
| }).map((option) => [option.value, option])).values()) | ||
| const parameterGroups = parameterInterfaceView?.groups ?? [] | ||
| const activeParameterGroupId = parameterGroups.some((group) => group.id === parameterGroupTab) | ||
| ? parameterGroupTab | ||
| : parameterGroups[0]?.id | ||
| const activeParameterFields = parameterInterfaceView?.fields.filter((field) => field.groupId === activeParameterGroupId) ?? [] | ||
| const regularParameterFields = activeParameterFields.filter((field) => field.type !== "json") | ||
| const advancedParameterFields = activeParameterFields.filter((field) => field.type === "json") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how {{portId}} references are resolved server-side to see if collisions across
# multiple upstream nodes sharing a port id are actually safe.
rg -nP '\{\{.*?\}\}|workflowInputReferenceForPort|input_reference' backend -g '*.py' -C3Repository: 2233admin/opencli-admin
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
fd 'inspector\.tsx$|nodes\.(ts|tsx)$|.*flow.*\.ts$|.*work.*flow.*\.ts$|.*workflow.*\.ts$' frontend backend | sed -n '1,120p'
echo
echo "== inspect frontend component around upstreamVariableOptions =="
nl -ba frontend/components/flow/inspector.tsx | sed -n '1280,1375p'
echo
echo "== search definitions/usages of helper functions =="
rg -n "function workflowInputReferenceForPort|const workflowInputReferenceForPort|export .*workflowInputReferenceForPort|workflowInputReferenceForPort\\(" frontend -C3
rg -n "function workflowDirectUpstreamNodeIds|const workflowDirectUpstreamNodeIds|workflowDirectUpstreamNodeIds\\(" frontend -C3Repository: 2233admin/opencli-admin
Length of output: 1057
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect frontend component around upstreamVariableOptions =="
awk '{printf "%5d\t%s\n", NR, $0}' frontend/components/flow/inspector.tsx | sed -n '1280,1375p'
echo
echo "== search definitions/usages of helper functions =="
rg -n "function workflowInputReferenceForPort|const workflowInputReferenceForPort|export .*workflowInputReferenceForPort|workflowInputReferenceForPort\\(" frontend frontend/components/flow frontend/lib/workflow -C3 || true
rg -n "function workflowDirectUpstreamNodeIds|const workflowDirectUpstreamNodeIds|workflowDirectUpstreamNodeIds\\(" frontend frontend/components/flow frontend/lib/workflow -C3 || true
echo
echo "== focused backend search for workflow input references / port-id-only resolution =="
rg -n "workflowInputReferenceForPort|input_reference|input reference|referenceForPort|port_id|ports\\.output|output_ports|\\{\\{[^{}]*\\}\\}" backend frontend -g '*.py' -g '*.ts' -g '*.tsx' -g '*.json' | head -n 200Repository: 2233admin/opencli-admin
Length of output: 36408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect runtime template resolver =="
awk '{printf "%5d\t%s\n", NR, $0}' backend/workflow/native_node_runtime.py | sed -n '1,140p'
echo
echo "== inspect compiler output/input binding handling =="
awk '{printf "%5d\t%s\n", NR, $0}' backend/workflow/compiler.py | sed -n '740,830p'
echo
echo "== inspect frontend outline helpers =="
awk '{printf "%5d\t%s\n", NR, $0}' frontend/lib/workflow/workflow-outline.ts | sed -n '120,160p'
echo
echo "== read-only behavioral probe for Map key dedupe =="
node - <<'JS'
function workflowInputReferenceForPort(portId) {
const WORKFLOW_INPUT_REFERENCE_PATH = /^[A-Za-z_][\w.-]*$/;
const path = portId.trim();
return WORKFLOW_INPUT_REFERENCE_PATH.test(path) ? `{{${path}}}` : undefined;
}
const upstreamNodeIds = new Set(['nodeA', 'nodeB']);
const nodes = [
{ id: 'nodeA', data: { label: 'A' } },
{ id: 'nodeB', data: { label: 'B' } },
];
const output = Array.from(new Map(nodes.flatMap((candidate) => {
if (!upstreamNodeIds.has(candidate.id)) return [];
const ports = [{ id: 'result', type: 'any' }];
return ports.map((port) => {
const value = workflowInputReferenceForPort(port.id);
return value ? [{ value, label: `${candidate.data.label} · ${port.id} (${port.type})`, nodeId: candidate.id, portId: port.id }] : [];
});
}).map((option) => [option.value, option])).values());
console.log(JSON.stringify(output, null, 2));
console.log('count', output.length);
console.log('labels', output.map((o) => o.label).join(', '));
JSRepository: 2233admin/opencli-admin
Length of output: 10680
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect rest of native runtime template resolver =="
awk '{printf "%5d\t%s\n", NR, $0}' backend/workflow/native_node_runtime.py | sed -n '140,380p'
echo
echo "== inspect compiler input binding construction =="
rg -n "_build.*input|input_binding|binding|\\{\\{[^{}]*\\}\\}|port_id|input_value|source_node|edge\\.source|edge\\.target" backend/workflow/compiler.py backend/workflow/turbopush_executor.py backend/workflow/swarm_simulation.py -C3 | head -n 260
echo
echo "== inspect selected runtime executor path =="
rg -n "templates|template_convert|_TEMPLATE_REFERENCE|context|variables|source|edge|input" backend/workflow/native_node_runtime.py backend/workflow/turbopush_executor.py backend/workflow/swarm_simulation.py -C3 | head -n 360Repository: 2233admin/opencli-admin
Length of output: 50379
Dedupe and display upstream variable options by source node plus port id.
workflowInputReferenceForPort(port.id) creates {{portId}}, and the frontend Map(option.value, option) dedupes solely by that value. If two direct upstream nodes both expose an output with the same id, one option disappears from the picker even though the labels still show different nodes. Use a node/scoped key for dedupe, or avoid making {{result}} the value for both options so users can choose which upstream source to bind.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/flow/inspector.tsx` around lines 1323 - 1356, Update the
upstream variable option construction in the node inspector so deduplication
uses both the candidate source node identity and output port id, rather than
only workflowInputReferenceForPort(port.id). Preserve distinct picker entries
for different upstream nodes while keeping each option’s binding value correctly
scoped to its selected source.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Completes Houdini-style port interactions and the right inspector, adds the curated Dify/common node set and full OpenCLI capability catalog, and expands the domestic OODA template to 14 real sources across five groups. Preserves collect-per-source semantics end-to-end: mixed source failures are partial_success, while all-source failures remain failed with the package blocked. Verified with frontend regression suites (51/8/11/6), TypeScript, ESLint, 109 backend tests, Ruff, real OpenCLI source probes, and an independent merge review.