Scope workflow execution to the active trigger graph - #60
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
⏳ Repowise has not indexed this repository yet No analysis on this PR because there is no index to compare against. Indexing usually runs automatically after install; if this persists, start it from the dashboard. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesTrigger-scoped workflow execution
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (7)
frontend/scripts/check-workflow-regressions.mjs (1)
1126-1139: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd a behavioral regression test for validation scope.
readSource()andassert.match()only prove that source strings exist. They do not executevalidateDraft(), verify unique parked-node counting, testnodeIdandnode_id, render the summary, or confirm that a graph change clears stale state. Add a behavior-level test with a mocked validation response and a graph update.As per coding guidelines, “Do not claim completion without fresh evidence; use targeted tests, typecheck, lint, build, integration, or browser smoke checks according to change risk.” This source-only assertion is not fresh behavioral evidence.
🤖 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-workflow-regressions.mjs` around lines 1126 - 1139, Replace the source-string assertions in the “trigger scope validation” test with an executable behavioral regression test that renders the workflow editor, mocks the validation response, and invokes validateDraft(). Verify active and parked counts with unique parked nodes, supporting both nodeId and node_id, and confirm the rendered summary uses the stable testid. Apply a graph update and assert that stale validation scope is cleared.Source: Coding guidelines
backend/workflow/trigger_scope.py (3)
119-125: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
edge_target_indexmap.
_downstream_active_idsfillsedge_target_indexbut never reads it. Traversal uses onlyadjacency. The map allocates one list per edge target on every call and suggests a reverse-reachability feature that does not exist.♻️ Proposed cleanup
adjacency: dict[str, list[str]] = {node.id: [] for node in nodes} - edge_target_index: dict[str, list[str]] = {} for edge in edges: if edge.source not in adjacency: adjacency[edge.source] = [] adjacency.setdefault(edge.target, []) adjacency[edge.source].append(edge.target) - edge_target_index.setdefault(edge.target, []).append(edge.source)🤖 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/trigger_scope.py` around lines 119 - 125, Remove the unused edge_target_index map and its population from the edge-building logic in _downstream_active_ids, while preserving adjacency initialization and target appending behavior.
55-60: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider rejecting an unrecognized
trigger_kindinstead of defaulting tomanual.
_normalize_requested_kindmaps any unknown string, andNone, to"manual". A caller that passes a typo such as"scedule"then silently selects a manual trigger, or receives aworkflow_trigger_kind_mismatcherror that names the wrong kind. Theselect_trigger_scopesignature acceptsstr | None, so the function cannot rely on an upstream literal type.Keep
Noneas"manual"and return a selection error for a non-empty unsupported value.🤖 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/trigger_scope.py` around lines 55 - 60, Update _normalize_requested_kind to keep mapping None to "manual" while rejecting non-empty unsupported trigger_kind values instead of defaulting them to "manual"; propagate a selection error through select_trigger_scope for invalid values such as typos, preserving the existing handling of recognized kinds and the "ai" alias.
313-322: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider returning an empty-scope project when selection fails.
On a selection error this returns the full, unscoped
projectwhile reportingactive_node_ids=set(). A caller that readsresult.projectwithout checkingselection_errorfirst then compiles and runs every canvas node.spec.mdline 23 states the system "SHALL NOT fall back to compiling or running every canvas node".
backend/workflow/opencli_hda_tracer.pychecksselection_errorbefore readingscope_result.project, so no current path is affected. The field pairing still invites that mistake. Document the contract in the docstring, or return a scope that cannot execute.🤖 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/trigger_scope.py` around lines 313 - 322, Update the selection-failure branch in the trigger-scope function that returns TriggerScopeResult so result.project is an empty, non-executable scope rather than the full project; alternatively, document in that function’s docstring that callers must check selection_error before accessing project. Ensure the returned scope cannot compile or run unselected canvas nodes, while preserving the existing selection_error and empty active-node behavior.tests/integration/test_trigger_scoped_workflow_execution.py (1)
153-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake bootstrap slugs unique per test instead of deleting matching projects.
_bootstrap_workflowdeletes every existing project whose slug equals the graph id. Four tests bootstrap the same fixture idwf-trigger-scope-v2(lines 273, 477, 532, 558). Each run therefore deletes the project the previous test created. The tests share mutable server state, so they cannot run in parallel and a failure in one can change the outcome of another.Append a unique suffix to the slug, and drop the delete loop.
♻️ Proposed change
- slug = graph.get("id", "trigger-scope-test") - existing = (await client.get(f"/api/v1/workspaces/{workspace_id}/projects")).json()["data"] - for p in existing: - if p.get("slug") == slug: - await client.delete(f"/api/v1/workspaces/{workspace_id}/projects/{p['id']}") + slug = f"{graph.get('id', 'trigger-scope-test')}-{uuid.uuid4().hex[:8]}"Add
import uuidat module level.🤖 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_trigger_scoped_workflow_execution.py` around lines 153 - 158, Update _bootstrap_workflow to generate a unique project slug by appending a uuid-based suffix to the graph id, add the required module-level uuid import, and remove the existing project lookup and deletion loop. Keep the unique slug used consistently for subsequent bootstrap requests.backend/workflow/opencli_hda_tracer.py (2)
313-321: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve trigger candidates once per Run instead of twice.
has_supported_triggersscans every node with_trigger_candidates, andselect_trigger_scoperepeats that identical scan on the next line. Each candidate check callsresolve_node_originand thenresolve_runtime_metadata, andresolve_runtime_metadataevaluates a long predicate chain (backend/workflow/runtime_registry.py, lines 108-185). Every Run start therefore resolves runtime metadata for the whole graph twice.
TriggerScopeResultdoes not report whether any supported trigger exists, which is why the caller needs the separate probe. Add that flag to the result —select_active_unionalready returnshas_supported_trigger— then call the selector once and branch on the flag.♻️ Sketch
- has_triggers = has_supported_triggers(body.project) - if has_triggers: - scope_result = select_trigger_scope( - body.project, - trigger_kind=body.trigger.kind, - trigger_node_id=body.trigger.triggerNodeId, - ) - if scope_result.selection_error is not None: + scope_result = select_trigger_scope( + body.project, + trigger_kind=body.trigger.kind, + trigger_node_id=body.trigger.triggerNodeId, + ) + if scope_result.has_supported_trigger: + if scope_result.selection_error is not None:Add
has_supported_trigger: booltoTriggerScopeResultinbackend/workflow/trigger_scope.pyand set it from the candidate list.🤖 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 313 - 321, Update TriggerScopeResult and select_trigger_scope in trigger_scope.py to include and populate a has_supported_trigger boolean from the resolved candidate list, reusing the value returned by select_active_union where applicable. In the Run-start flow around has_supported_triggers and select_trigger_scope, remove the separate has_supported_triggers scan, call select_trigger_scope once, and branch on scope_result.has_supported_trigger while preserving existing scope handling.
322-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused selection-error assignments and move the trigger_scope import upward.
scope_result.selection_error is not Nonereturns beforescope_projectorruntime_nodesare read, so remove the branch assignments. Movefrom backend.workflow.trigger_scope import ...to module scope unless the compiler flow creates a circular import.🤖 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 322 - 325, In the selection-error branch of the workflow handling flow, remove the unused scope_project and runtime_nodes assignments while preserving the existing error behavior. Move the trigger_scope import to module scope, unless doing so introduces a circular import; retain it locally only in that case.
🤖 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 `@backend/api/v1/studio_lifecycle.py`:
- Around line 270-278: Rename the private _external_workflow_ids helper in
trigger_scope.py to external_workflow_ids and expose it for import, then update
the studio lifecycle scoped_project call to use that helper instead of its
inline externalWorkflow predicate. Preserve the helper’s exact filtering
behavior so both paths share the same governed exception.
- Around line 279-288: Move the _parked_diagnostics(project,
active_union.parked_node_ids) computation in the scoped validation branch so it
runs whenever the scoped graph is processed, before the compilation and validity
checks. Extend warnings independently of both _isolated_source_errors(scoped)
and scoped_result.valid, while preserving the existing valid-graph handling for
stored_graph and valid.
- Around line 84-112: Update compile_workflow_project and the parked-node
handling around parked_set so parked-node membership is evaluated before
structural edge errors. Collect matching structural errors separately, then
append node-anchored configuration diagnostics in their original order, ensuring
parked source nodes and nodes with required upstream dependencies report all
relevant failures.
In `@backend/workflow/opencli_hda_tracer.py`:
- Around line 1617-1618: The run projection currently ignores failures returned
by build_opencli_hda_trace, allowing failed traces to appear valid and
completed. Update the projection using compile_result and trace so validity
requires both results to be valid and errors include trace errors when a trace
exists; apply the same combined logic to the session-reloaded projection.
Preserve existing behavior when trace is None.
In `@backend/workflow/trigger_scope.py`:
- Around line 163-210: Update _scoped_edges to preserve selected edge instances
with edge.model_copy() instead of reconstructing WorkflowProjectEdge fields
explicitly. Update scoped_project to use project.model_copy(update=...) so only
nodes and edges are replaced, retaining all extra fields on the project and
edges in the resolved artifact.
- Around line 250-265: Update select_trigger_scope’s trigger_node_id matching
logic to distinguish an existing node that is not a supported trigger from an
absent node: return unsupported_workflow_trigger for the former, while
preserving workflow_trigger_not_found for IDs with no matching node. Keep the
existing supported-trigger selection and error metadata behavior unchanged.
In `@frontend/components/flow/workflow-editor-session.tsx`:
- Line 230: Update the workflowProject-change handling near setValidationScope
and the validateDraft/publishDraft flow to invalidate pending validation
requests using a generation token or abort guard. Capture the current workflow
identity, revision, and graph fingerprint when validation starts, and only apply
validationScope, validationRunId, or releaseState if those values still match
when the request completes.
In `@tests/integration/test_trigger_scoped_workflow_execution.py`:
- Around line 292-301: Rewrite the P1 section header above
test_no_trigger_graph_preserves_full_compilation_path to state that graphs
without supported triggers preserve the legacy full-graph validation path and
may remain valid; remove the contradictory “must be invalid (no fallback)”
wording.
- Around line 95-139: Correct the parked-node grouping comments in the fixture:
classify document under “Parked — unknown bindings” because
primitive.document.extract is asserted as unknown, and classify review under
“Parked — valid configuration.” Move the nodes or relabel the comments while
preserving the existing node definitions.
- Around line 181-197: Update
test_trigger_selector_parity_with_compiled_detector to remove the unused client
fixture and asyncio marker, then compile _trigger_scope_acceptance_fixture and
invoke the compiled-runtime selector via _runtime_trigger_kind or
_select_runtime_nodes_for_trigger from opencli_hda_tracer.py. Compare the
compiled trigger IDs and kinds against the _trigger_candidates result, while
retaining the existing expected manual trigger assertion.
- Around line 509-523: Simplify the active event assertion to only check
active_expected. In the checkpoint/node-state validation, remove the unreachable
parked-node batch loop and retain the direct absence assertion; also remove the
tautological eventCount >= 0 assertion in
test_parked_nodes_have_zero_dispatch_events_batches_items. Update that test to
explicitly assert zero batch and item totals for each of the four active node
IDs, matching its promised behavior.
---
Nitpick comments:
In `@backend/workflow/opencli_hda_tracer.py`:
- Around line 313-321: Update TriggerScopeResult and select_trigger_scope in
trigger_scope.py to include and populate a has_supported_trigger boolean from
the resolved candidate list, reusing the value returned by select_active_union
where applicable. In the Run-start flow around has_supported_triggers and
select_trigger_scope, remove the separate has_supported_triggers scan, call
select_trigger_scope once, and branch on scope_result.has_supported_trigger
while preserving existing scope handling.
- Around line 322-325: In the selection-error branch of the workflow handling
flow, remove the unused scope_project and runtime_nodes assignments while
preserving the existing error behavior. Move the trigger_scope import to module
scope, unless doing so introduces a circular import; retain it locally only in
that case.
In `@backend/workflow/trigger_scope.py`:
- Around line 119-125: Remove the unused edge_target_index map and its
population from the edge-building logic in _downstream_active_ids, while
preserving adjacency initialization and target appending behavior.
- Around line 55-60: Update _normalize_requested_kind to keep mapping None to
"manual" while rejecting non-empty unsupported trigger_kind values instead of
defaulting them to "manual"; propagate a selection error through
select_trigger_scope for invalid values such as typos, preserving the existing
handling of recognized kinds and the "ai" alias.
- Around line 313-322: Update the selection-failure branch in the trigger-scope
function that returns TriggerScopeResult so result.project is an empty,
non-executable scope rather than the full project; alternatively, document in
that function’s docstring that callers must check selection_error before
accessing project. Ensure the returned scope cannot compile or run unselected
canvas nodes, while preserving the existing selection_error and empty
active-node behavior.
In `@frontend/scripts/check-workflow-regressions.mjs`:
- Around line 1126-1139: Replace the source-string assertions in the “trigger
scope validation” test with an executable behavioral regression test that
renders the workflow editor, mocks the validation response, and invokes
validateDraft(). Verify active and parked counts with unique parked nodes,
supporting both nodeId and node_id, and confirm the rendered summary uses the
stable testid. Apply a graph update and assert that stale validation scope is
cleared.
In `@tests/integration/test_trigger_scoped_workflow_execution.py`:
- Around line 153-158: Update _bootstrap_workflow to generate a unique project
slug by appending a uuid-based suffix to the graph id, add the required
module-level uuid import, and remove the existing project lookup and deletion
loop. Keep the unique slug used consistently for subsequent bootstrap requests.
🪄 Autofix
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: 641e1b97-0ad2-472f-81c8-4d262c1cc624
📒 Files selected for processing (12)
backend/api/v1/studio_lifecycle.pybackend/workflow/opencli_hda_tracer.pybackend/workflow/trigger_scope.pyfrontend/components/flow/workflow-editor-session.tsxfrontend/scripts/check-workflow-regressions.mjsopenspec/changes/trigger-scoped-workflow-execution/.openspec.yamlopenspec/changes/trigger-scoped-workflow-execution/README.mdopenspec/changes/trigger-scoped-workflow-execution/design.mdopenspec/changes/trigger-scoped-workflow-execution/proposal.mdopenspec/changes/trigger-scoped-workflow-execution/specs/trigger-scoped-workflow-execution/spec.mdopenspec/changes/trigger-scoped-workflow-execution/tasks.mdtests/integration/test_trigger_scoped_workflow_execution.py
| # Compile parked nodes in isolation to surface configuration diagnostics | ||
| # (unknown bindings, missing params, etc.) as warnings without edges that | ||
| # would produce irrelevant port-mismatch noise. | ||
| if not parked_set: | ||
| return diagnostics | ||
| parked_nodes = [n for n in project.nodes if n.id in parked_set] | ||
| parked_project = workflow_schemas.WorkflowProject( | ||
| id=project.id, | ||
| name=project.name, | ||
| profile=project.profile, | ||
| version=project.version, | ||
| nodes=parked_nodes, | ||
| edges=[], | ||
| settings=project.settings, | ||
| adapters=list(project.adapters), | ||
| agentPermissions=project.agentPermissions, | ||
| ) | ||
| parked_result = compile_workflow_project(parked_project) | ||
| for error in parked_result.errors: | ||
| if error.node_id and error.node_id in parked_set: | ||
| diagnostics.append( | ||
| workflow_schemas.WorkflowCompileError( | ||
| code=error.code, | ||
| message=error.message, | ||
| node_id=error.node_id, | ||
| path=error.path, | ||
| ) | ||
| ) | ||
| return diagnostics |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect _validate_project to see which structural errors precede per-node binding diagnostics.
ast-grep outline backend/workflow/compiler.py --items all --match '_validate_project|compile_workflow_project'
ast-grep run --lang python --pattern 'def _validate_project($$$):
$$$' backend/workflow/compiler.py
rg -n -C3 'unknown_node_library_binding' backend/workflowRepository: 2233admin/opencli-Razormind
Length of output: 1532
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== compiler outline =="
ast-grep outline backend/workflow/compiler.py --items all || true
echo "== relevant compiler.py sections =="
sed -n '150,330p' backend/workflow/compiler.py
echo "== compiler.py diagnostics sections =="
sed -n '420,520p' backend/workflow/compiler.py
echo "== studio lifecycle relevant section =="
sed -n '1,140p' backend/api/v1/studio_lifecycle.py
echo "== schemas/workflow around WorkflowProject constraints =="
sed -n '220,260p' backend/schemas/workflow.pyRepository: 2233admin/opencli-Razormind
Length of output: 19991
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== _validate_project continuation =="
sed -n '300,420p' backend/workflow/compiler.py
echo "== source/isolated validation usages =="
rg -n -C 4 'isolated_source|source node|unconnected|require.*source|connected.*source|source.*required' backend tests
echo "== parked diagnostic tests =="
rg -n -C 6 'isolated_source_node|parked_node|_parked_diagnostics|_isolated_source_errors' backend testsRepository: 2233admin/opencli-Razormind
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
compiler_path = Path("backend/workflow/compiler.py")
studio_path = Path("backend/api/v1/studio_lifecycle.py")
tree = ast.parse(compiler_path.read_text())
for fn in tree.body:
if isinstance(fn, ast.FunctionDef) and fn.name == "_validate_project":
calls = []
for stmt in fn.body:
if isinstance(stmt, ast.Return) and isinstance(stmt.value, ast.Call):
calls = [ast.unparse(stmt.value)]
elif isinstance(stmt, ast.Return) and isinstance(stmt.value, ast.Tuple):
calls = [ast.unparse(call) for call in stmt.value.elts[:3]]
elif isinstance(stmt, ast.Else):
for nested in stmt.body:
if isinstance(nested, ast.With):
for assign in nested.body:
if isinstance(assign, ast.Assign):
print("assign", ast.unparse(assign))
print("returns:")
for c in calls:
print("-", c)
if isinstance(fn, ast.FunctionDef) and fn.name == "compile_workflow_project":
for stmt in fn.body:
if isinstance(stmt, ast.Assign):
for target in stmt.targets:
if isinstance(target, ast.Name) and target.id in {"project", "errors"}:
print("initial", target.id, "=", ast.unparse(stmt.value))
studio_tree = ast.parse(studio_path.read_text())
for fn in studio_tree.body:
if isinstance(fn, ast.FunctionDef) and fn.name == "_parked_diagnostics":
print("function", fn.name, "body_count", len(fn.body))
for stmt_idx, stmt in enumerate(fn.body):
print(f"{fn.name}.{stmt_idx}: " + ast.unparse(stmt).replace("\n", " ")[:220])
if isinstance(fn, ast.FunctionDef) and fn.name == "_isolated_source_errors":
print("function", fn.name, "body_count", len(fn.body))
for stmt_idx, stmt in enumerate(fn.body):
print(f"{fn.name}.{stmt_idx}: " + ast.unparse(stmt).replace("\n", " ")[:220])
PYRepository: 2233admin/opencli-Razormind
Length of output: 389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
compiler_path = Path("backend/workflow/compiler.py")
studio_path = Path("backend/api/v1/studio_lifecycle.py")
tree = ast.parse(compiler_path.read_text())
for fn in tree.body:
if isinstance(fn, ast.FunctionDef) and fn.name == "_validate_project":
print("function", fn.name, "lineno", fn.lineno)
for stmt_idx, stmt in enumerate(fn.body):
line = ast.unparse(stmt).replace("\n", " ")[:220]
print(f"{fn.name}.{stmt_idx}: {line}")
if isinstance(fn, ast.FunctionDef) and fn.name == "compile_workflow_project":
print("function", fn.name, "lineno", fn.lineno)
for stmt_idx, stmt in enumerate(fn.body[:8]):
line = ast.unparse(stmt).replace("\n", " ")[:220]
print(f"{fn.name}.{stmt_idx}: {line}")
studio_tree = ast.parse(studio_path.read_text())
for fn in studio_tree.body:
if isinstance(fn, ast.FunctionDef) and fn.name in {"_parked_diagnostics", "_isolated_source_errors"}:
print("function", fn.name, "lineno", fn.lineno)
for stmt_idx, stmt in enumerate(fn.body[:80]):
line = ast.unparse(stmt).replace("\n", " ")[:220]
print(f"{fn.name}.{stmt_idx}: {line}")
PYRepository: 2233admin/opencli-Razormind
Length of output: 4866
Preserve node configuration diagnostics before structural edge errors.
compile_workflow_project returns as soon as _validate_project reports any error. When edges=[], structural diagnostics such as orphan-merge/port-mismatch messages can block later node-anchored diagnostics like unknown_node_library_binding. Check for parked_node membership first, collect matching structural errors separately, then append matching node-anchored configuration diagnostics in their original order so parked source nodes and parked nodes with required upstream dependencies surface all relevant failures.
🤖 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/api/v1/studio_lifecycle.py` around lines 84 - 112, Update
compile_workflow_project and the parked-node handling around parked_set so
parked-node membership is evaluated before structural edge errors. Collect
matching structural errors separately, then append node-anchored configuration
diagnostics in their original order, ensuring parked source nodes and nodes with
required upstream dependencies report all relevant failures.
| scoped = scoped_project( | ||
| project=project, | ||
| active_ids=active_union.active_node_ids, | ||
| external_ids={ | ||
| node.id | ||
| for node in project.nodes | ||
| if isinstance(node.params.get("externalWorkflow"), dict) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Reuse the selector's external-workflow predicate instead of duplicating it.
backend/workflow/trigger_scope.py lines 140-146 already define _external_workflow_ids with the same isinstance(node.params.get("externalWorkflow"), dict) test. This endpoint reimplements it inline. The governed external-workflow exception then has two definitions, and design.md Decision 4 describes it as exact behavior that must not broaden. Any future change to one copy silently changes which nodes reach the published graph.
Rename the helper to external_workflow_ids, export it, and call it here.
♻️ Proposed refactor
-from backend.workflow.trigger_scope import scoped_project, select_active_union
+from backend.workflow.trigger_scope import (
+ external_workflow_ids,
+ scoped_project,
+ select_active_union,
+) scoped = scoped_project(
project=project,
active_ids=active_union.active_node_ids,
- external_ids={
- node.id
- for node in project.nodes
- if isinstance(node.params.get("externalWorkflow"), dict)
- },
+ external_ids=external_workflow_ids(project.nodes),
)🤖 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/api/v1/studio_lifecycle.py` around lines 270 - 278, Rename the
private _external_workflow_ids helper in trigger_scope.py to
external_workflow_ids and expose it for import, then update the studio lifecycle
scoped_project call to use that helper instead of its inline externalWorkflow
predicate. Preserve the helper’s exact filtering behavior so both paths share
the same governed exception.
| errors.extend(_isolated_source_errors(scoped)) | ||
| if not errors: | ||
| scoped_result = compile_workflow_project(scoped) | ||
| errors = list(scoped_result.errors) | ||
| if scoped_result.valid and scoped_result.plan is not None: | ||
| valid = True | ||
| stored_graph = scoped.model_dump(mode="json") | ||
| warnings.extend( | ||
| _parked_diagnostics(project, active_union.parked_node_ids) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Emit parked-node warnings even when the active graph is invalid.
warnings.extend(_parked_diagnostics(...)) sits inside the scoped_result.valid branch. Two paths therefore return zero parked_node warnings while parked nodes exist:
_isolated_source_errors(scoped)adds an error, so theif not errorsguard at line 280 skips compilation.scoped_result.validisFalsebecause the active chain has a defect.
spec.md lines 54-57 require that when validation finds nodes outside every supported trigger-reachable component, "the response exposes deterministic parked-node warnings from which clients can derive the parked count and node identifiers". That requirement does not depend on active-chain validity. The frontend derives parkedCount from these warnings and activeCount from the authored count minus parkedCount, so an author who has one active error plus six parked nodes sees an active count of ten instead of four.
Compute the parked diagnostics once in the scoped branch, before the validity check.
🐛 Proposed fix
errors.extend(_isolated_source_errors(scoped))
+ warnings.extend(
+ _parked_diagnostics(project, active_union.parked_node_ids)
+ )
if not errors:
scoped_result = compile_workflow_project(scoped)
errors = list(scoped_result.errors)
if scoped_result.valid and scoped_result.plan is not None:
valid = True
stored_graph = scoped.model_dump(mode="json")
- warnings.extend(
- _parked_diagnostics(project, active_union.parked_node_ids)
- )📝 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.
| errors.extend(_isolated_source_errors(scoped)) | |
| if not errors: | |
| scoped_result = compile_workflow_project(scoped) | |
| errors = list(scoped_result.errors) | |
| if scoped_result.valid and scoped_result.plan is not None: | |
| valid = True | |
| stored_graph = scoped.model_dump(mode="json") | |
| warnings.extend( | |
| _parked_diagnostics(project, active_union.parked_node_ids) | |
| ) | |
| errors.extend(_isolated_source_errors(scoped)) | |
| warnings.extend( | |
| _parked_diagnostics(project, active_union.parked_node_ids) | |
| ) | |
| if not errors: | |
| scoped_result = compile_workflow_project(scoped) | |
| errors = list(scoped_result.errors) | |
| if scoped_result.valid and scoped_result.plan is not None: | |
| valid = True | |
| stored_graph = scoped.model_dump(mode="json") |
🤖 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/api/v1/studio_lifecycle.py` around lines 279 - 288, Move the
_parked_diagnostics(project, active_union.parked_node_ids) computation in the
scoped validation branch so it runs whenever the scoped graph is processed,
before the compilation and validity checks. Extend warnings independently of
both _isolated_source_errors(scoped) and scoped_result.valid, while preserving
the existing valid-graph handling for stored_graph and valid.
| valid=compile_result.valid, | ||
| errors=list(compile_result.errors) if trace is None else compile_result.errors, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find assertions tied to trace-level failure codes on run projections.
rg -n -C4 'missing_opencli_hda_package|missing_opencli_hda_sources' --glob '!**/node_modules/**'
rg -n -C3 'trace\.valid|trace\.errors' backend/workflow/opencli_hda_tracer.pyRepository: 2233admin/opencli-Razormind
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -u
printf 'Files matching tracer/compiler:\n'
fd -a 'opencli_hda_tracer.py|compiler.py' . || true
printf '\nRelevant lines in opencli_hda_tracer.py:\n'
if [ -f backend/workflow/opencli_hda_tracer.py ]; then
wc -l backend/workflow/opencli_hda_tracer.py
sed -n '340,420p' backend/workflow/opencli_hda_tracer.py | nl -ba -v340
sed -n '1490,1535p' backend/workflow/opencli_hda_tracer.py | nl -ba -v1490
sed -n '1595,1650p' backend/workflow/opencli_hda_tracer.py | nl -ba -v1595
fi
printf '\nSearch failure codes in repository:\n'
rg -n 'missing_opencli_hda_package|missing_opencli_hda_sources|compile_result\.valid|compile_result\.errors|trace\.valid|trace\.errors' . --glob '!**/node_modules/**' || true
printf '\nRelevant lines in compiler.py:\n'
if [ -f backend/workflow/compiler.py ]; then
wc -l backend/workflow/compiler.py
sed -n '200,255p' backend/workflow/compiler.py | nl -ba -v200
fiRepository: 2233admin/opencli-Razormind
Length of output: 2919
🏁 Script executed:
#!/bin/bash
set -u
printf 'Files matching tracer/compiler:\n'
fd -a 'opencli_hda_tracer.py|compiler.py' . || true
printf '\nRelevant lines in opencli_hda_tracer.py:\n'
if [ -f backend/workflow/opencli_hda_tracer.py ]; then
wc -l backend/workflow/opencli_hda_tracer.py
sed -n '340,420p' backend/workflow/opencli_hda_tracer.py | nl -ba -v340
sed -n '1490,1535p' backend/workflow/opencli_hda_tracer.py | nl -ba -v1490
sed -n '1595,1650p' backend/workflow/opencli_hda_tracer.py | nl -ba -v1595
fi
printf '\nSearch failure codes in repository:\n'
rg -n 'missing_opencli_hda_package|missing_opencli_hda_sources|compile_result\.valid|compile_result\.errors|trace\.valid|trace\.errors' . --glob '!**/node_modules/**' || true
printf '\nRelevant lines in compiler.py:\n'
if [ -f backend/workflow/compiler.py ]; t
wc -l backend/workflow/compiler.py
sed -n '200,255p' backend/workflow/compiler.py | nl -ba -v200
fiRepository: 2233admin/opencli-Razormind
Length of output: 2919
🏁 Script executed:
#!/bin/bash
set -u
printf 'backend/workflow/opencli_hda_tracer.py relevant sections:\n'
sed -n '340,410p' backend/workflow/opencli_hda_tracer.py
sed -n '190,280p' backend/workflow/opencli_hda_tracer.py
sed -n '1485,1532p' backend/workflow/opencli_hda_tracer.py
sed -n '1605,1652p' backend/workflow/opencli_hda_tracer.py
printf '\nbackend/workflow/compiler.py compile success path:\n'
sed -n '200,255p' backend/workflow/compiler.py
printf '\nintegration test context around missing_opencli_hda_sources:\n'
sed -n '1150,1225p' tests/integration/test_workflow_opencli_hda_trace_api.pyRepository: 2233admin/opencli-Razormind
Length of output: 14159
🏁 Script executed:
#!/bin/bash
set -u
printf 'Trace result construction / failure return context:\n'
python3 - <<'PY'
from pathlib import Path
p = Path("backend/workflow/opencli_hda_tracer.py")
text = p.read_text()
for name in (
"def build_opencli_hda_trace(",
"selected_package_id =",
"if not dispatches:",
"return WorkflowOpenCLIHDATraceResponse",
):
try:
idx = text.index(name)
except ValueError:
print(f"MISSING {name}", idx if "idx" in locals() else "")
continue
line = text[:idx].count("\n") + 1
lines = text[idx:]
print(f"\n-- {name} at line {line} --")
end = lines.find("\n")
while end != -1 and (end == 0 or not lines[end-1:end+5].startswith("\n\n")):
end = lines.find("\n", end + 1)
print(lines[:250])
PY
printf '\nCall paths containing build_opencli_hda_trace:\n'
rg -n -C3 'build_opencli_hda_trace\(' . --glob '!**/node_modules/**' || trueRepository: 2233admin/opencli-Razormind
Length of output: 3026
🏁 Script executed:
#!/bin/bash
set -u
printf 'Workflow execution path for trace parameter handling:\n'
sed -n '460,510p' backend/workflow/opencli_hda_tracer.py
printf '\n'
rg -n -C2 'opencliHdaTrace|opencli_hda_trace|traceId|trace_id|WorkflowOpenCLIHDATraceResponse' backend/api backend/tests backend/workflow tests/integration --glob '!**/node_modules/**' || trueRepository: 2233admin/opencli-Razormind
Length of output: 50384
Fold the OpenCLI HDA trace result into the run projection.
should_trace_opencli calls build_opencli_hda_trace(...) whenever OpenCLI/HDA tracing should run, and the returned trace can be valid=False with missing_opencli_hda_package or missing_opencli_hda_sources. The projection currently uses only compile_result.valid and compile_result.errors, so a trace failure can make the run report status == "completed", valid == True, errors == [] even though no dispatch envelope was built. Update the projection and the session-reloaded projection to make trace validity/errors part of the projected validity/errors.
🤖 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 1617 - 1618, The run
projection currently ignores failures returned by build_opencli_hda_trace,
allowing failed traces to appear valid and completed. Update the projection
using compile_result and trace so validity requires both results to be valid and
errors include trace errors when a trace exists; apply the same combined logic
to the session-reloaded projection. Preserve existing behavior when trace is
None.
| def _scoped_edges( | ||
| *, | ||
| edges: list[WorkflowProjectEdge], | ||
| active_ids: set[str], | ||
| ) -> list[WorkflowProjectEdge]: | ||
| scoped: list[WorkflowProjectEdge] = [] | ||
| for edge in edges: | ||
| if edge.source in active_ids and edge.target in active_ids: | ||
| scoped.append( | ||
| WorkflowProjectEdge( | ||
| id=edge.id, | ||
| source=edge.source, | ||
| target=edge.target, | ||
| sourcePort=edge.sourcePort, | ||
| targetPort=edge.targetPort, | ||
| label=edge.label, | ||
| condition=edge.condition, | ||
| semantic=edge.semantic, | ||
| weight=edge.weight, | ||
| contractId=edge.contractId, | ||
| proposalState=edge.proposalState, | ||
| ui=edge.ui, | ||
| ) | ||
| ) | ||
| return scoped | ||
|
|
||
|
|
||
| def scoped_project( | ||
| *, | ||
| project: WorkflowProject, | ||
| active_ids: set[str], | ||
| external_ids: set[str], | ||
| ) -> WorkflowProject: | ||
| include = active_ids | external_ids | ||
| scoped_nodes = [node for node in project.nodes if node.id in include] | ||
| scoped_node_ids = {node.id for node in scoped_nodes} | ||
| scoped_edges = _scoped_edges(edges=project.edges, active_ids=scoped_node_ids) | ||
| return WorkflowProject( | ||
| id=project.id, | ||
| name=project.name, | ||
| profile=project.profile, | ||
| version=project.version, | ||
| nodes=scoped_nodes, | ||
| edges=scoped_edges, | ||
| settings=project.settings, | ||
| adapters=list(project.adapters), | ||
| agentPermissions=project.agentPermissions, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm extra="allow" on the project/edge schemas and find hand-rebuilt constructions.
ast-grep outline backend/schemas/workflow.py --match 'WorkflowProject|WorkflowProjectEdge' --items all
rg -n -C4 'class WorkflowProject\b|class WorkflowProjectEdge\b|class WorkflowProjectNode\b' backend/schemas/workflow.py
rg -n -C2 'WorkflowProject\(' backend --glob '!**/tests/**'Repository: 2233admin/opencli-Razormind
Length of output: 3202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== schema classes with extra config and fields =="
sed -n '158,190p' backend/schemas/workflow.py
sed -n '194,255p' backend/schemas/workflow.py
echo "== trigger_scope scoped methods =="
sed -n '130,230p' backend/workflow/trigger_scope.py
echo "== lifecycle persist path =="
sed -n '1,130p' backend/api/v1/studio_lifecycle.py
rg -n "resolved_graph|scoped_project|parked_project|publish|publication" backend/api/v1/studio_lifecycle.py backend --glob '!**/tests/**' > /tmp/lifecycle_mentions.txt || true
sed -n '250,305p' /tmp/lifecycle_mentions.txt
echo "== call sites of scoped_project =="
rg -n -C3 "scoped_project\(" backend --glob '!**/tests/**'Repository: 2233admin/opencli-Razormind
Length of output: 22612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== persist call around scoped_project =="
sed -n '255,310p' backend/api/v1/studio_lifecycle.py
echo "== immutable version references under workspace =="
rg -n -C3 'resolved_graph|StudioWorkflowDraft|StudioWorkflowVersion|WorkflowVersion|immutable|version' backend/api/v1/studio_lifecycle.py backend --glob '!**/tests/**' | python3 - <<'PY'
import sys
for line in sys.stdin:
if 'resolved_graph' in line or 'StudioWorkflowVersion' in line or 'StudioWorkflowVersion' in next(sys.stdin, ''):
print(line, end='')
PY
echo "== pydantic model_copy behavior for extra fields =="
python3 - <<'PY'
from dataclasses import dataclass, field, fields
try:
from pydantic import BaseModel, ConfigDict
except Exception as exc:
print(f"pydantic unavailable: {exc}")
raise SystemExit
class WorkflowProjectEdge(BaseModel):
model_config = ConfigDict(extra="allow")
id: str
source: str
target: str
class WorkflowProject(BaseModel):
model_config = ConfigDict(extra="allow")
id: str
nodes: list[str]
edges: list[WorkflowProjectEdge]
edge = WorkflowProjectEdge.model_validate({"id":"e1","source":"a","target":"b","extraEdge":"kept1"})
project = WorkflowProject.model_validate({"id":"p1","nodes":["a","b"],"edges":[edge.model_dump()],"extraProject":"kept2"})
def manual_edge(edges, active_ids):
return [
WorkflowProjectEdge(
id=e.id, source=e.source, target=e.target,
sourcePort=e.sourcePort, targetPort=e.targetPort, label=e.label, condition=e.condition,
semantic=e.semantic, weight=e.weight, contractId=e.contractId, proposalState=e.proposalState, ui=e.ui,
)
for e in edges if e.source in active_ids and e.target in active_ids
]
def manual_project(nodes, edges):
return WorkflowProject(
id=project.id,
profile=project.profile,
version=project.version,
nodes=nodes,
edges=edges,
settings=project.settings,
adapters=list(project.adapters),
agentPermissions=project.agentPermissions,
)
scoped_manual = manual_project([n for n in project.nodes if n in {"a","b"}], manual_edge(project.edges, {"a","b"}))
scoped_copy = project.model_copy(update={"nodes":[n for n in project.nodes if n in {"a","b"}], "edges":manual_edge(project.edges, {"a","b"})})
print("extraProject preserved by manual WorkflowProject constructor:", "extraProject" in scoped_manual)
print("extraEdge preserved by manual WorkflowProjectEdge constructor:", any("extraEdge" in e for e in scoped_manual.edges))
print("extraProject preserved by project.model_copy:", "extraProject" in scoped_copy)
print("extraEdge preserved by model_copy edge:", any("extraEdge" in e for e in scoped_copy.edges))
PYRepository: 2233admin/opencli-Razormind
Length of output: 2617
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate resolved_graph and validation run model =="
rg -n -C4 'resolved_graph' backend/models backend --glob '!**/tests/**'
echo "== locate studio lifecycle create/submit routes =="
rg -n -C8 'submit|validation_run|validation|workflow_id|version|resolution|resolved_graph' backend/api/v1/studio_lifecycle.py --glob '!**/tests/**'
echo "== locate validation run route and published version route =="
rg -n -C8 'ValidationRun|validation' backend/api/v1/studio_lifecycle.py --glob '!**/tests/**'Repository: 2233admin/opencli-Razormind
Length of output: 24901
Preserve extra workflow data when scoping the project.
WorkflowProject and WorkflowProjectEdge allow extra fields, but scoped_project and _scoped_edges rebuild both by naming fields explicitly. Trigger-scoped validation stores scoped.model_dump(mode="json") as resolved_graph, and publish uses that JSON for the immutable version, so extra keys are dropped from the validation/publishable artifact even if they are in the draft.
Use model_copy(update=...) for the project and preserve each selected edge with edge.model_copy() so only nodes and edges change.
🤖 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/trigger_scope.py` around lines 163 - 210, Update
_scoped_edges to preserve selected edge instances with edge.model_copy() instead
of reconstructing WorkflowProjectEdge fields explicitly. Update scoped_project
to use project.model_copy(update=...) so only nodes and edges are replaced,
retaining all extra fields on the project and edges in the resolved artifact.
| setValidationRunId(null) | ||
| setReleaseBlocker(null) | ||
| setPublishedVersion(null) | ||
| setValidationScope(null) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Invalidate stale validation results after graph changes.
When workflowProject changes, Line 230 clears validationScope, but it does not invalidate an in-flight validateDraft() call. If the user edits while validation is pending, the older continuation can still restore counts and validationRunId for the previous graph. publishDraft() can then re-enable publishing for a validation result that does not describe the current canvas.
Add a validation generation token or abort guard. Before applying validationScope, validationRunId, or releaseState, verify that the workflow identity, revision, and graph fingerprint still match the request.
Also applies to: 246-263
🤖 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/workflow-editor-session.tsx` at line 230, Update the
workflowProject-change handling near setValidationScope and the
validateDraft/publishDraft flow to invalidate pending validation requests using
a generation token or abort guard. Capture the current workflow identity,
revision, and graph fingerprint when validation starts, and only apply
validationScope, validationRunId, or releaseState if those values still match
when the request completes.
| # Parked — unknown bindings | ||
| { | ||
| "id": "llm-a", | ||
| "kind": "agent", | ||
| "capability": "normalize", | ||
| "params": {"prompt": "Summarise in zh-CN"}, | ||
| "ui": {"catalogId": "primitive.ai.llm"}, | ||
| }, | ||
| { | ||
| "id": "llm-b", | ||
| "kind": "agent", | ||
| "capability": "normalize", | ||
| "params": {"prompt": "Extract key entities"}, | ||
| "ui": {"catalogId": "primitive.ai.llm"}, | ||
| }, | ||
| { | ||
| "id": "plugin", | ||
| "kind": "agent", | ||
| "capability": "normalize", | ||
| "params": {"trigger": "onNewRecord"}, | ||
| "ui": {"catalogId": "primitive.plugin.trigger"}, | ||
| }, | ||
| # Parked — valid configuration | ||
| { | ||
| "id": "review", | ||
| "kind": "control", | ||
| "capability": "accept", | ||
| "params": {}, | ||
| "ui": {"catalogId": "intelligence.control.record-acceptance"}, | ||
| }, | ||
| { | ||
| "id": "document", | ||
| "kind": "agent", | ||
| "capability": "normalize", | ||
| "params": {"format": "pdf"}, | ||
| "ui": {"catalogId": "primitive.document.extract"}, | ||
| }, | ||
| { | ||
| "id": "notify", | ||
| "kind": "notify", | ||
| "capability": "send", | ||
| "adapter": "webhook-notifier", | ||
| "params": {"target": "webhook"}, | ||
| "ui": {"catalogId": "intelligence.output.webhook"}, | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the parked-node grouping comments.
The docstring at lines 20-21 classifies document as parked-invalid and review as parked-valid. The inline comments place them the other way: review appears under "Parked — unknown bindings" and document appears under "Parked — valid configuration". document uses primitive.document.extract, which the assertion at line 264 expects among the four unknown bindings.
Move review below the second comment, or relabel both comments, so the fixture reads consistently.
🤖 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_trigger_scoped_workflow_execution.py` around lines 95
- 139, Correct the parked-node grouping comments in the fixture: classify
document under “Parked — unknown bindings” because primitive.document.extract is
asserted as unknown, and classify review under “Parked — valid configuration.”
Move the nodes or relabel the comments while preserving the existing node
definitions.
| @pytest.mark.asyncio | ||
| async def test_trigger_selector_parity_with_compiled_detector(client): | ||
| """Reuse canonical origin/binding: the source-level selector must match | ||
| the compiled-runtime trigger recognition exactly.""" | ||
|
|
||
| from backend.schemas.workflow import WorkflowProject | ||
| from backend.workflow.trigger_scope import _trigger_candidates | ||
|
|
||
| project = WorkflowProject.model_validate(_trigger_scope_acceptance_fixture()) | ||
| pairs = _trigger_candidates( | ||
| project.nodes, | ||
| adapters={a.id: a for a in project.adapters}, | ||
| ) | ||
| trigger_ids = [n.id for n, _ in pairs] | ||
| kinds_by_id = {n.id: k for n, k in pairs} | ||
| assert trigger_ids == ["trigger"], f"expected [trigger], got {trigger_ids}" | ||
| assert kinds_by_id["trigger"] == "manual" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
This test does not compare against the compiled detector.
The body calls only _trigger_candidates and asserts a hardcoded expectation. It never invokes the compiled-runtime recognition path — _runtime_trigger_kind or _select_runtime_nodes_for_trigger in backend/workflow/opencli_hda_tracer.py. A divergence between the two implementations passes this test.
design.md line 97 names parity tests as the mitigation for the risk that source-level trigger recognition diverges from compiled-runtime recognition. Two independent trigger recognizers now exist, and this file is the only place that claims to hold them together.
Compile the fixture, run the compiled selector, and assert that the trigger ids and kinds it recognizes equal the source-level result. Also drop the unused client fixture and the asyncio marker, because the body performs no await.
🤖 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_trigger_scoped_workflow_execution.py` around lines 181
- 197, Update test_trigger_selector_parity_with_compiled_detector to remove the
unused client fixture and asyncio marker, then compile
_trigger_scope_acceptance_fixture and invoke the compiled-runtime selector via
_runtime_trigger_kind or _select_runtime_nodes_for_trigger from
opencli_hda_tracer.py. Compare the compiled trigger IDs and kinds against the
_trigger_candidates result, while retaining the existing expected manual trigger
assertion.
| # --------------------------------------------------------------------------- | ||
| # P1: no supported trigger → must be invalid (no fallback) | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_no_trigger_graph_preserves_full_compilation_path(client): | ||
| """A draft with no supported trigger preserves the existing full-graph | ||
| validation path. Valid nodes without a trigger still pass (the legacy | ||
| / media-canvas fallback). Trigger scope and parked-node classification | ||
| only activate when at least one supported trigger is present.""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the contradictory section header.
Line 293 states "no supported trigger → must be invalid (no fallback)". The test asserts v["valid"] is True at line 315, and the docstring states that the legacy full-graph path is preserved. spec.md lines 64-67 require the legacy behavior. Delete or rewrite the header so it matches the asserted behavior.
🤖 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_trigger_scoped_workflow_execution.py` around lines 292
- 301, Rewrite the P1 section header above
test_no_trigger_graph_preserves_full_compilation_path to state that graphs
without supported triggers preserve the legacy full-graph validation path and
may remain valid; remove the contradictory “must be invalid (no fallback)”
wording.
| assert active_expected.issubset(event_ids) or event_ids == active_expected, f"Missing active: {active_expected - event_ids}" | ||
|
|
||
| # checkpoint node states | ||
| checkpoint = trace.json()["data"]["trace"]["checkpoint"]["nodeStates"] | ||
| cp_ids = {s["nodeId"] for s in checkpoint} | ||
| assert not (cp_ids & parked_expected), f"Parked ids in checkpoint: {cp_ids & parked_expected}" | ||
|
|
||
| # no batch/item/record presence for parked nodes | ||
| for state in proj["nodeStates"]: | ||
| if state["nodeId"] in parked_expected: | ||
| assert not state.get("batches"), f"Parked {state['nodeId']} has batches" | ||
| for s in proj["nodeStates"]: | ||
| assert s["nodeId"] not in parked_expected, ( | ||
| f"Parked node {s['nodeId']} appeared in nodeStates" | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the assertions that cannot fail, and assert batches and items directly.
Three assertions in this test carry no information:
- Line 509:
active_expected.issubset(event_ids) or event_ids == active_expected. Equality implies the subset relation, so theorclause is redundant. The check reduces to the subset test. - Lines 517-519: the loop body only runs for a state whose
nodeIdis inparked_expected. Lines 520-523 assert that no such state exists. The batch check is unreachable. - Line 593 in
test_parked_nodes_have_zero_dispatch_events_batches_items:state["eventCount"] >= 0holds for any counter.
The test name promises zero batches and zero items for parked nodes. Assert the batch and item totals for the four active ids explicitly, and drop the unreachable and tautological checks.
🤖 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_trigger_scoped_workflow_execution.py` around lines 509
- 523, Simplify the active event assertion to only check active_expected. In the
checkpoint/node-state validation, remove the unreachable parked-node batch loop
and retain the direct absence assertion; also remove the tautological eventCount
>= 0 assertion in test_parked_nodes_have_zero_dispatch_events_batches_items.
Update that test to explicitly assert zero batch and item totals for each of the
four active node IDs, matching its promised behavior.
2 participants
Summary
Root cause
Studio validation and Run compiled the entire editable canvas before choosing an execution entrance. Disconnected design-time nodes therefore blocked a valid trigger-connected chain and could leak parked-node configuration errors into runtime projections.
User impact
A workflow such as the ten-node A-share collection canvas can validate, publish, and run its four-node schedule-triggered chain while six disconnected nodes remain visible for later wiring. Reconnecting a parked invalid node correctly promotes its error back into the blocking active scope.
Acceptance performed by lead
72 passed: trigger-scope, workflow compile, and Studio lifecycle suites.114 passed, 4 skipped: expanded workflow runtime/trace/compatibility suites selected from Code Intel impact.56/56 passed: frontend workflow regression checks.6/6 passed: FMHY tests after merging latestmain.openspec validate trigger-scoped-workflow-execution --strictpassed.git diff --checkpassed.活动节点 4 · 未接入节点 6and allowed publication.27fb9bbd-1565-42bb-aedb-1fd2a8242a8c: valid, zero compile errors, four top-level active nodes, zero parked states/events, 66 dispatch events, 34 batches, 1,046 items, and 523 records.Structural quality disclosure
Sentrux rule checks pass with zero unresolved imports and zero cycles. The full-repository no-degradation scan remains red against the existing baseline:
1512 -> 145257.24 -> 57.307012 -> 70310 -> 043 -> 43The scan covers the whole dirty worktree, including four unrelated user-owned modifications. The baseline was not reset or repinned to hide the warning.
Delivery notes
mainwas merged locally before validation and push.