t3030: Address high-severity quality-debt in batch-strategy-helper.sh#3033
t3030: Address high-severity quality-debt in batch-strategy-helper.sh#3033marcusquinn merged 1 commit intomainfrom
Conversation
Consolidate shell loops into single jq passes for performance and fix flawed circular dependency detection from PR #3000 review feedback. Changes: - Replace get_dispatchable_tasks O(n*m) shell loop with single jq pass - Remove now-unused is_task_unblocked function - Replace order_depth_first shell loop with single jq pass - Rewrite circular dependency detection with proper stack-based DFS using jq until(), fixing false positives on diamond graphs and false negatives from incomplete traversal - Remove blanket 2>/dev/null from jq calls (preserve stderr for debugging) Closes #3030
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
WalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Mar 7 02:52:28 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.agents/scripts/batch-strategy-helper.sh (1)
449-494: Correct stack-based DFS with proper diamond graph handling.The separation of
path(current ancestors) fromprocessed(fully explored nodes) correctly distinguishes back-edges (cycles) from cross-edges (diamonds). Theuntil()loop with frame-based state is an elegant jq pattern for iterative algorithms.One minor observation: the cycle message at line 480 (
"\($start_id) -> ... -> \($dep)") uses the DFS start node rather than extracting the actual cycle from$frame.path. For debugging, the precise cycle path would be more helpful, though this is functionally correct for detection.🔍 Optional: More precise cycle reporting
If desired, extract the actual cycle path from
$frame.path:- .cycles += ["\($start_id) -> ... -> \($dep)"] + # Extract cycle: from $dep's position to end, plus $dep + (($frame.path | index($dep)) // 0) as $cycle_start + | .cycles += [($frame.path[$cycle_start:] + [$dep]) | join(" -> ")]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/batch-strategy-helper.sh around lines 449 - 494, The cycle message currently uses the DFS start node ($start_id) instead of the actual ancestor chain; update the code that appends to .cycles (the line producing "\($start_id) -> ... -> \($dep)") to build the cycle string from the current frame's path ($frame.path) by extracting the suffix that starts at the first occurrence of $dep and joining those nodes plus $dep into a readable "A -> B -> C" cycle representation so .cycles contains the precise cycle rather than just the start node hint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/scripts/batch-strategy-helper.sh:
- Around line 449-494: The cycle message currently uses the DFS start node
($start_id) instead of the actual ancestor chain; update the code that appends
to .cycles (the line producing "\($start_id) -> ... -> \($dep)") to build the
cycle string from the current frame's path ($frame.path) by extracting the
suffix that starts at the first occurrence of $dep and joining those nodes plus
$dep into a readable "A -> B -> C" cycle representation so .cycles contains the
precise cycle rather than just the start node hint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea388f0d-c252-42b1-8862-1e7e383918af
📒 Files selected for processing (1)
.agents/scripts/batch-strategy-helper.sh
1 participant
Summary
get_dispatchable_tasksandorder_depth_firstshell loops into single jq passes (2 HIGH findings from PR t1408.4: Add batch execution strategies for task decomposition dispatch #3000 Gemini review)jq until(), fixing false positives on diamond graphs and false negatives from incomplete traversal (MEDIUM finding)2>/dev/nullfrom jq calls to preserve stderr for debugging (MEDIUM finding)is_task_unblockedfunctionNet result: -46 lines, fewer jq invocations (O(1) vs O(n*m)), correct cycle detection.
Closes #3030
Summary by CodeRabbit