Skip to content

fix: workflow incorrectly marked as completed while nodes are still executing#13

Open
tomerqodo wants to merge 4 commits into
coderabbit_full_base_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr13from
coderabbit_full_head_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr13
Open

fix: workflow incorrectly marked as completed while nodes are still executing#13
tomerqodo wants to merge 4 commits into
coderabbit_full_base_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr13from
coderabbit_full_head_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr13

Conversation

@tomerqodo

@tomerqodo tomerqodo commented Jan 30, 2026

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#13

Summary by CodeRabbit

  • Bug Fixes

    • Fixed skip propagation behavior in workflow graph traversal to correctly start node execution and mark skipped edges based on edge states.
  • Tests

    • Added comprehensive unit test coverage for skip propagation logic, including edge state handling, execution sequencing, and recursive propagation in workflow graphs.

@coderabbitai

coderabbitai Bot commented Jan 30, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR modifies the skip propagation logic in the graph engine's skip_propagator module to start execution for downstream nodes when incoming edges are TAKEN, and marks edges as skipped after propagation. Comprehensive unit tests are added to validate skip propagation behavior across various graph traversal scenarios.

Changes

Cohort / File(s) Summary
Skip Propagation Logic
api/core/workflow/graph_engine/graph_traversal/skip_propagator.py
Modified propagate_skip_from_edge to start node execution before enqueuing when a TAKEN edge is detected; reordered edge marking in _propagate_skip_to_node to occur after recursive propagation.
Test Infrastructure
api/tests/unit_tests/core/workflow/graph_engine/graph_traversal/__init__.py
Added module-level docstring documenting the package's purpose for graph traversal component testing.
Skip Propagator Tests
api/tests/unit_tests/core/workflow/graph_engine/graph_traversal/test_skip_propagator.py
New comprehensive test suite validating skip propagation across edge states (unknown, taken, skipped), node enqueuing, execution starting, recursive propagation chains, and edge/node marking behavior.

Sequence Diagram

sequenceDiagram
    participant Node as Downstream Node
    participant SP as SkipPropagator
    participant GSM as GraphStateManager
    participant Graph as Graph

    rect rgba(200, 100, 100, 0.5)
    Note over SP,GSM: propagate_skip_from_edge (TAKEN edge detected)
    SP->>Graph: get incoming edges
    SP->>Graph: check edge states
    alt Any TAKEN edge found
        SP->>GSM: enqueue_node(downstream_node)
        SP->>GSM: start_execution(downstream_node)
        SP->>SP: log debug message
    else All edges SKIPPED
        SP->>SP: _propagate_skip_to_node
        SP->>GSM: mark_node_skipped(node)
        SP->>Graph: get outgoing edges
        SP->>GSM: mark_edge_skipped(edge)
        SP->>SP: propagate_skip_from_edge (recursive)
    else Unknown states present
        SP->>SP: stop processing
    end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops through graphs with skips so fine,
Edges marked when paths align,
Propagate the skip so true,
Tests now prove what code will do!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is minimal and references a benchmark PR but lacks required template sections like detailed summary, context, motivation, and issue linkage. Expand the description with sections following the template: provide a detailed summary of the changes, context/motivation, issue link using 'Fixes #', and confirm all checklist items are completed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main fix: addressing a workflow completion issue while nodes are still executing, which matches the core changes in skip_propagator.py.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch coderabbit_full_head_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr13

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tomerqodo

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jan 30, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@tomerqodo

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jan 30, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@tomerqodo

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jan 30, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ravidqodo

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Feb 2, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@OmriGM

OmriGM commented Feb 2, 2026

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Feb 2, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants