Rewrite workflows:review with context-managed map-reduce architecture#157
Rewrite workflows:review with context-managed map-reduce architecture#157Drewx-Design wants to merge 3 commits intoEveryInc:mainfrom
Conversation
Replace the v1 review command with a context-managed v2 that prevents context overflow by having sub-agents write findings to .review/ on disk and return only single-sentence summaries to the parent. Key changes: - Add PR Intent Analysis phase (shared context for all specialists) - Agents write JSON to .review/, return ~100 tokens each to parent - Add validation step to catch silent agent failures and invalid JSON - Add Judge phase for dedup, hallucination removal, and ranking - Add Deep Analysis phase for P1/P2 enrichment - Parent reads only ENRICHED_FINDINGS.json (~8k tokens vs ~30-50k in v1) - Smart agent selection based on PR content instead of running all agents - Cross-platform safe: uses project-relative .review/ instead of /tmp/ 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…exploration - Add git-history-analyzer and code-philosopher to conditional agent selection - Expand Deep Analysis Phase with detailed stakeholder impact analysis (Developer, Operations, End User, Security, Business perspectives) - Add comprehensive scenario exploration checklist (invalid inputs, concurrency, scale, network, resource exhaustion, security vectors, etc.) - Note that deep analysis inherits the Ultra-Thinking approach from v1 but scoped to judged findings in an isolated context window 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
|
I ran into this issue today. I asked for a PR review, and it hit the limit. I am on the Max 5 plan. |
|
|
||
| Ensure that the code is ready for analysis (either in worktree or on current branch). ONLY then proceed to the next step. | ||
|
|
||
| - [ ] If ALREADY on the target branch → proceed with analysis on current branch |
There was a problem hiding this comment.
I wonder why we keep - [ ] instead of a numbered list. What is the benefit of using - [ ] in this case?
- Remove 4 ghost agents (rails-turbo-expert, dependency-detective, devops-harmony-analyst, code-philosopher) that don't exist in plugin - Add 5 missing real agents to selection matrix (kieran-typescript-reviewer, kieran-python-reviewer, julik-frontend-races-reviewer, schema-drift-detector, learnings-researcher) - Replace Python3 JSON validation with Node.js for cross-platform compat - Add excluded paths check to judge prompt (not just agent prompts) - Add 0-findings short-circuit for clean PRs - Fix .gitignore append duplication with grep -q guard - Update frontmatter description to match v2 purpose - Bump version to 2.31.0
|
This one is really good and works. It consumes significantly fewer tokens on the main agent. |
|
Closing — the upstream This PR's key ideas (phased file-based map-reduce, judge dedup, validation phase, intent analysis) may still be worth proposing as targeted improvements to the current upstream command rather than a wholesale replacement. But the 5-version drift makes this PR unmergeable as-is — it would be a complete file conflict. Architectural patterns from this work influenced the |
|
Great! I really think the Deepin plan is very interesting. I just have to test it out a little bit since it will add some complexity to the plugin. But I like the idea a lot. |
Summary
.review/on disk and return only ~100 token summaries to the parent, keeping parent context under ~12k tokens (vs ~30-50k in v1).review/instead of/tmp/(fixes Windows path issues)Key Architecture Changes
.review/, return 1 sentence each (~100 tokens vs 2-4k)ENRICHED_FINDINGS.json(~8k tokens max), spawns todo-creation agentsTest plan
/workflows:reviewon a PR with mixed file types (Ruby + JS) to verify agent selection.review/directory is created with expected agent output files.review/path works