Dependency-aware dispatch ordering - #138
Conversation
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughFactory now supports ChangesDependency-aware dispatch
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant IssueProvider
participant FactoryLoop
participant BatchTracker
participant AgentSpawner
IssueProvider->>FactoryLoop: provide issue and blocker declarations
FactoryLoop->>BatchTracker: submit dependency admission
BatchTracker-->>FactoryLoop: park blocked issue
IssueProvider->>FactoryLoop: report blocker terminal
FactoryLoop->>BatchTracker: clear parked state
FactoryLoop->>AgentSpawner: dispatch dependent issue
Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a dependency-aware issue sequencing and admission mechanism to the orchestrator, allowing issues to be parked until their declared blockers are resolved. Feedback on the changes highlights critical issues, including potential runtime crashes due to unhandled null/undefined values in parseBlockedBy and #indexDependencyIssue, and a potential ReferenceError from using the undefined githubIssueIdentity function. Additionally, opportunities for code simplification and optimization were identified, such as refactoring the parsing logic in parseBlockedBy and avoiding duplicate asynchronous calls to retrieve the batch instance.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…uplicate batch() call - parseBlockedBy is now a public API export; accept null/undefined description text and return no dependencies instead of throwing. - Hoist the single #batch() lookup in the queued-lifecycle dependency check instead of calling it separately on both branches. Addresses gemini-code-assist review on PR #138.
…force-factory-fa7edb58 # Conflicts: # src/orchestrator/batch-tracker.test.ts # src/orchestrator/batch-tracker.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/orchestrator/factory.ts (1)
3955-4020: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffDependency blocker discovery can trigger a full
ISSUE_ROOTtree scan per blocked dispatch.When a declared blocker is not already indexed and the GitHub by-id probe misses,
#loadMissingDependencyIssuesfalls back to#githubIssuePaths()and a full#listRelayfileTree(ISSUE_ROOT)scan. These are memoized within a single call, but blockers are commonly non-ready (closed/in-progress/unlabeled) and therefore absent from the ready set, so arunOncewith several blocked issues can repeat the Linear/GitHub tree scans once per issue. Consider hoisting the closure-load caches (or the resolved-dependency index) across a single dispatch pass to keep this off the O(issues × tree) path on large boards.🤖 Prompt for 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. In `@src/orchestrator/factory.ts` around lines 3955 - 4020, Hoist the closure-load caches used by `#loadMissingDependencyIssues`—githubPathsByIdentity and linearTreeLoaded, or an equivalent resolved-dependency index—to the surrounding dispatch/runOnce scope so multiple blocked issues share each GitHub and Linear tree scan. Initialize them once per dispatch pass, reuse them across calls to `#loadMissingDependencyIssues`, and reset them before the next pass.
🤖 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 `@src/orchestrator/factory.ts`:
- Around line 2330-2348: Clear the dependency-park notice deduplication entry
whenever a dispatch successfully calls clearPark, starting at the shown dispatch
path and applying the same cleanup at the other clearPark sites near the related
flows. Reuse the existing `#dependencyParkNotices` state and ensure a later
identical blocker or cycle signature can be reported again after the issue
re-parks.
---
Nitpick comments:
In `@src/orchestrator/factory.ts`:
- Around line 3955-4020: Hoist the closure-load caches used by
`#loadMissingDependencyIssues`—githubPathsByIdentity and linearTreeLoaded, or an
equivalent resolved-dependency index—to the surrounding dispatch/runOnce scope
so multiple blocked issues share each GitHub and Linear tree scan. Initialize
them once per dispatch pass, reuse them across calls to
`#loadMissingDependencyIssues`, and reset them before the next pass.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c3f1fd9-9229-4469-8b80-acac84b6b325
📒 Files selected for processing (10)
README.mdsrc/index.tssrc/orchestrator/batch-tracker.test.tssrc/orchestrator/batch-tracker.tssrc/orchestrator/dependencies.tssrc/orchestrator/factory.test.tssrc/orchestrator/factory.tssrc/orchestrator/index.tssrc/ports/state.tssrc/types.ts
|
Addressed the CodeRabbit scalability note in fd642de: GitHub fallback-path discovery and the Linear tree-loaded flag are now shared across a dependency discovery pass and reset at the next runOnce, so multiple blocked issues reuse the same provider scans. Added the regression "shares missing-blocker tree scans across one discovery pass". Validation: npm run build and npm test (50 files, 1,021 tests) pass. |
…force-factory-fa7edb58
# Conflicts: # src/orchestrator/factory.ts
1 participant
Summary
Blocked by: #123, owner/repo#456line from issue body/description at discovery time (v1 text convention, per issue notes).issueKey()uses elsewhere, so a bare#123only matches within the routed repo.BatchTracker(start()/queue()): issues with unresolved blockers are parked, distinct from capacity queuing (ParkedIssue.capacityBlockedtracks both independently).hold.kind: 'dependency'/'dependency-cycle'reason in dispatch skip reporting so it's never confused with a safety-gate skip.findDependencyCycleand fail closed with an operator-visible error/comment instead of deadlocking.Fixture
Exercised against the exact #128 → #131 → #132 chain from the issue: a single discovery sweep dispatches only #128; #131 and #132 park with
Blocked by: AgentWorkforce/pear#128/#131respectively; closing #128 promotes #131 on the next cycle. Seefactory.test.ts: "dispatches a dependency chain in order and promotes the next issue after its blocker closes".Test plan
npm test— 974 passed (46 files), including new coverage: dependency extraction, cross-repo bare/qualified resolution without collision, cycle detection, and the Per-repo feature/test map: lazy, incremental bootstrap generation for customer repos #128/Consume the per-repo feature/test map in agent task templates (implementer/reviewer/babysitter test guidance) #131/CI drift-check for the per-repo feature/test map (catch stale locations and coverage rot) #132 fixture chain.npm run buildnpm pack --dry-runCloses #133
🤖 Generated with Claude Code