Skip to content

Holistic implementation planner - #61

Open
khezen wants to merge 2 commits into
mainfrom
holistic-planner
Open

khezen wants to merge 2 commits into
mainfrom
holistic-planner

Conversation

@khezen

@khezen khezen commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

What

The implementation planner (review.plan) now makes one plan for the whole review instead of one isolated plan per scope.

Before, each scope was planned in its own run, so a plan could not say that a fix in one scope needs a change in another scope first (e.g. a shared library before its consumers). Now one run sees every scope and every issue, with file tools rooted at the repo root, and orders steps by dependency across scopes, then severity.

The output is still detailed per scope:

  • Each step changes exactly one scope. Work that spans scopes is split into one step per scope, linked by depends_on.
  • Steps are numbered across the whole plan and grouped per scope (scopes ordered by their first step), so numbers inside a scope can skip.
  • New Depends on: line in the rendered plan. A dependency in another scope shows that scope's name.
  • JSON steps gain number, scope and depends_on. implementation_plan keeps its [{subroot, steps}] shape.

Budget

review.plan.max_iters / max_llm_calls stay per scope (defaults 5 / 16). The single run gets them multiplied by the number of scopes with issues, at most ×8. Example: 3 scopes with issues gives 15 iterations / 48 LLM calls. review.max_concurrent_scopes no longer applies to the planner.

Behaviour change

A planner failure now drops the whole plan; the review is still posted. Before, only the scope that failed was left out.

Tests

  • tests/test_implementation_plan.py passes (33 tests): it covers the scope check, numbering and grouping, dependency cleanup, the budget, the single run (one call, repo-rooted tools, inputs not mutated, cleanup on failure) and rendering.
  • ruff passes on the changed code. mypy shows no new errors.
  • Other suite failures are in files this PR doesn't touch (test_cerebral*, test_config*, test_doc_extractor, test_review_memories, test_review_statistics).

Docs (configuration.md, usage.md, architecture.md), action.yml input descriptions and the CHANGELOG ([Unreleased], no version bump) are updated.

Also: filesystem tool errors reach the agent

With mcp 2.2, a tool that raises anything other than ToolError is treated as a crash. It's logged at ERROR with a full traceback, and the agent only gets Error executing tool list_directory. An agent probing deploy/ or infra/ in a scope without them filled the log with tracebacks and couldn't tell a missing folder from a broken tool.

The filesystem MCP server now raises these failures as ToolError, keeping their message: missing path, file vs directory, permission denied, path escaping the root. The agent gets Directory not found: deploy and nothing is logged as a crash. Real crashes are unchanged. Covered by tests/test_filesystem_server.py (6 tests).

One planner run sees every scope and issue and orders steps by dependency across scopes. Steps stay grouped per scope with global numbers and a Depends on line. Budget scales with scopes that have issues (at most x8).
mcp 2.2 treats any non-ToolError exception as a crash: full traceback at ERROR and the reason hidden from the agent. Missing paths, file vs directory, permission and root-escape errors now reach the agent with their message.
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