feat(#603): state-appropriate action buttons on dependency graph nodes - #623
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe dependency graph now provides a ChangesBacklog triage action
Estimated code review effort: 2 (Simple) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@sova/supervisor/dependency_graph.py`:
- Line 56: Update the workflow or architecture documentation associated with the
supervisor state-action model to describe TaskState.BACKLOG items and its
exposed Triage action, keeping the description concise and consistent with the
existing workflow documentation.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a7accaf-87ab-49f8-a8de-fc01cbaafc82
⛔ Files ignored due to path filters (1)
.claude/agent-memory/cookbook.mdis excluded by!.claude/**and included by none
📒 Files selected for processing (2)
sova/supervisor/dependency_graph.pytests/test_dependency_graph.py
22ae3e9 to
58842e4
Compare
Address Review: Round 1
|
Findings addressed or acknowledged. See Address Review comment.
|
@coderabbitai review |
|
xsovad06
left a comment
There was a problem hiding this comment.
PR Summary
This PR by xsovad06 (3 commits, 4 files, +24/-2) adds state-appropriate action buttons to the supervisor dependency graph node drawer. Previously, _STATE_ACTIONS had an empty list for BACKLOG and was missing entries for NEEDS_SPEC and ON_QA entirely. Now BACKLOG exposes "Triage", NEEDS_SPEC exposes "Run Researcher", and ON_QA gets an explicit empty-actions entry. Tests and architecture docs are updated accordingly.
Findings
[LOW] Test Coverage: NEEDS_SPEC and ON_QA not tested in action assertion
Location: tests/test_dependency_graph.py:346
Problem: The comprehensive test test_to_dict_node_has_available_actions was updated to cover BACKLOG but does not include the two other newly added _STATE_ACTIONS entries (NEEDS_SPEC, ON_QA). Commit 2 (fix(supervisor): add missing TaskState entries) has no dedicated test coverage.
Suggestion: Add NEEDS_SPEC and ON_QA to the multi-state test, or add a dedicated test similar to test_backlog_node_has_triage_action:
_task(6, state=TaskState.NEEDS_SPEC),
_task(7, state=TaskState.ON_QA),
# ...
assert any(a["role"] == "researcher" for a in by_id[6])
assert by_id[7] == [][LOW] Scope: Unrelated cookbook.md change
Location: .claude/agent-memory/cookbook.md:133
Problem: The model-pinning cookbook entry (from issue #619 post-mortem) is bundled into this PR. It is unrelated to #603.
Suggestion: Acceptable since it is agent-memory (gitignored in practice), but for cleaner history, unrelated learnings should go in their own commit on main or a separate PR.
Confirmed Bot Findings
Agree with CodeRabbit on sova/supervisor/dependency_graph.py:56: document the Backlog state action in architecture docs. See their inline comment. Addressed in commit 079ebb9.
Verdict
Approve. The code change is correct, well-scoped, and all TaskState enum values are now covered in _STATE_ACTIONS. The mapping logic is sound (BACKLOG->Triage, NEEDS_SPEC->Researcher, ON_QA->empty). Tests cover the primary new behavior (backlog triage action, epic exclusion). The two LOW findings are minor and do not block merge.
(Posted as comment: GitHub does not allow self-reviews with formal approval/rejection state.)
What's Done Well
- Complete enum coverage: all 9
TaskStatevalues now have explicit entries in_STATE_ACTIONS, eliminating the implicit.get()fallback for missing states. - Good test additions: the dedicated
test_backlog_node_has_triage_actiontest verifies the exact shape of the action dict (id, label, role), not just presence. The epic exclusion test fills a gap in ready-task filtering coverage. - Doc update in architecture.md describes the
_STATE_ACTIONSmapping concisely, addressing the CodeRabbit finding in the same commit that introduced the code change.
58842e4 to
58f0e97
Compare
Address Review: Round 2
|
|
@coderabbitai review |
|
58f0e97 to
85ae3a0
Compare
|



Summary
Automated changes for: feat(supervisor): state-appropriate action buttons on dependency graph nodes
Closes #603
Context
Summary
The dependency graph node drawer currently shows only "Run Researcher" regardless of issue state. Action buttons should adapt to the issue's current state so the user can take the right action directly from the graph without navigating to the priority queue.
Problem
Clicking a Backlog node shows "Run Researcher" — but Backlog issues need Triage first. Clicking a Researched node correctly shows "Run Researcher," but should show "Run Developer." There is no way to triage an iss...
Commits
Files changed