Skip to content

fix(workflow): dedupe progress rows, close leaked coroutines, cap runaway loops#191

Merged
elkaix merged 1 commit into
mainfrom
fix/workflow-progress-dup-and-runaway-cap
Jul 2, 2026
Merged

fix(workflow): dedupe progress rows, close leaked coroutines, cap runaway loops#191
elkaix merged 1 commit into
mainfrom
fix/workflow-progress-dup-and-runaway-cap

Conversation

@elkaix

@elkaix elkaix commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Three bugs found in a post-merge review of #186 (Workflow tool):

  • render_progress() re-listed agents that had been truncated by the per-phase
    max_agents cap as duplicate "unphased" rows at the bottom of the progress
    block — a phase with 8 agents and max_agents=6 showed the two oldest
    agents twice. Fixed by filtering the trailing "unphased" section on phase
    membership instead of a rendered-id set.
  • parallel() raised on invalid arguments (non-list, or a bare function mixed
    into the list) without closing the agent() coroutines already created for
    the other list items, leaking "coroutine was never awaited" warnings.
  • The Python port of the reference implementation dropped its 1000-agent
    lifetime backstop. A workflow script with an unbounded loop (e.g.
    while True: await agent(...)) and no token_budget set could spawn
    subagents indefinitely. Restored the cap as MAX_TOTAL_AGENTS = 1000,
    checked inside the concurrency semaphore alongside the existing budget
    check.

No behavior changes outside these three fixes; no new dependencies.

Test plan

  • Added regression tests: truncated-phase dedup, phaseless-agent
    rendering, lifetime-cap enforcement, and coroutine-leak-on-validation-
    failure (via warnings.catch_warnings)
  • make check-pythinker-code (ruff + format + pyright + ty) — clean
  • make test-pythinker-code — 6505 passed, 7 skipped, 1 xfailed (unit);
    65 passed, 4 skipped (e2e)
  • CHANGELOG.md updated under ## Unreleased

Summary by CodeRabbit

  • Bug Fixes
    • Fixed workflow progress rendering so truncated agents no longer reappear later in the output.
    • Improved progress display for agents without a phase, ensuring they still appear when appropriate.
    • Added a safeguard to stop runaway workflow loops after a high number of agent runs.
    • Prevented invalid workflow input from leaving behind unclosed async tasks, reducing warning noise and resource leaks.

…away loops

- render_progress() re-listed agents truncated by the per-phase max_agents
  cap as duplicate "unphased" rows; now filters by phase membership instead
  of a rendered-id set.
- parallel() left already-created agent() coroutines unclosed when its
  argument validation raised, leaking "never awaited" warnings.
- The Python port dropped the reference implementation's 1000-agent
  lifetime backstop, so a workflow script with an unbounded loop and no
  token_budget could spawn subagents forever; restored the cap.
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a61d754f-03bc-483f-81e4-cd2785740e1f

📥 Commits

Reviewing files that changed from the base of the PR and between 3c44c7e and f149543.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/pythinker_code/tools/workflow/display.py
  • src/pythinker_code/tools/workflow/engine.py
  • tests/tools/test_workflow_display.py
  • tests/tools/test_workflow_engine.py

📝 Walkthrough

Walkthrough

This PR fixes duplicate agent rows in workflow progress rendering by switching truncation logic from ID-based tracking to phase-order membership, closes leaked coroutines when parallel() rejects invalid arguments, and adds a 1000-agent lifetime cap to stop runaway workflow loops. Includes regression tests and a changelog entry.

Changes

Workflow progress and engine fixes

Layer / File(s) Summary
Phase-based truncation fix in render_progress
src/pythinker_code/tools/workflow/display.py, tests/tools/test_workflow_display.py
Replaces the rendered ID-set with phase_order membership checks so truncated phase agents no longer reappear as unphased rows; adds tests for truncated and phaseless agent rendering.
Agent lifetime cap in engine
src/pythinker_code/tools/workflow/engine.py, tests/tools/test_workflow_engine.py
Adds MAX_TOTAL_AGENTS = 1000 and enforces it in agent(), raising WorkflowRuntimeError once the cap is hit; adds a monkeypatched regression test.
Coroutine cleanup on parallel() validation failure
src/pythinker_code/tools/workflow/engine.py, tests/tools/test_workflow_engine.py
Calls _close_coroutines before raising in parallel() for invalid arguments or non-awaitable callables; adds a test confirming no "never awaited" warnings.
Changelog entry
CHANGELOG.md
Documents the three fixes under Unreleased.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Parallel
  participant CloseCoroutines
  Caller->>Parallel: parallel(items)
  Parallel->>Parallel: validate items
  alt invalid argument or item
    Parallel->>CloseCoroutines: _close_coroutines(items)
    Parallel-->>Caller: raise WorkflowRuntimeError
  else valid
    Parallel-->>Caller: proceed with execution
  end
Loading

Suggested labels: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commits and accurately summarizes the workflow fixes.
Description check ✅ Passed The description is detailed and covers summary, test plan, and checklist items, with only the related issue section omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/workflow-progress-dup-and-runaway-cap

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

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pythinker_code/tools/workflow/engine.py 80.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@elkaix elkaix merged commit 29174c3 into main Jul 2, 2026
35 checks passed
@elkaix elkaix deleted the fix/workflow-progress-dup-and-runaway-cap branch July 2, 2026 08:45
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.

1 participant