Skip to content

fix(work-loop): compose the PR from every task's delivery, not just the last#80

Merged
OGtwelve merged 1 commit into
mainfrom
fix/aggregate-deliveries-79
Jun 30, 2026
Merged

fix(work-loop): compose the PR from every task's delivery, not just the last#80
OGtwelve merged 1 commit into
mainfrom
fix/aggregate-deliveries-79

Conversation

@OGtwelve

@OGtwelve OGtwelve commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

A multi-task group's PR was composed from only the last task's WorkerDelivery, so the body's ## Changes under-reported earlier tasks' edits. workTasks now merges every task's delivery and hands the aggregate to openPr.

Changes

  • src/loop/work-loop.ts — new mergeDeliveries() (exported for tests): unions changes per path (first-touch kind, latest summary), joins draft messages and progress entries; workTasks accumulates all deliveries and returns the merge.

Testing

  • src/loop/work-loop.test.ts: mergeDeliveries unit tests (single passthrough; union/first-touch-kind/latest-summary) + an integration test asserting openPr receives a delivery covering both tasks' changes.
  • bun test + node --test green (39); biome + tsc clean.

Closes #79

Summary by CodeRabbit

  • New Features

    • PRs now include combined results from all completed tasks, not just the last one.
    • Commit messages and progress updates are merged into a single, clearer delivery.
  • Bug Fixes

    • Fixed an issue where earlier task changes could be left out when opening a PR.
    • Improved handling when one task blocks after others have already made changes, so committed work is still captured.

…he last

workTasks now merges all task deliveries (changes unioned per path — first-touch kind, latest summary; messages and progress concatenated) and hands the aggregate to openPr, so a multi-task group's PR body reflects the whole group's changes instead of only the final task.

Closes #79
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds an exported mergeDeliveries function to work-loop.ts that unions changes by file path (preserving first-touch kind), concatenates draftCommitMessage, and flattens progressEntries. workTasks is updated to accumulate all per-task deliveries and pass the merged result to openPr. Tests cover the helper directly and validate the end-to-end PR delivery includes all tasks' changes.

Changes

mergeDeliveries and workTasks integration

Layer / File(s) Summary
mergeDeliveries helper
packages/aitm/src/loop/work-loop.ts
Adds FileChange import and implements exported mergeDeliveries that unions changes by path (first kind, latest summary), concatenates draftCommitMessage with blank-line separators, and flattens progressEntries.
workTasks delivery accumulation
packages/aitm/src/loop/work-loop.ts
Updates doc comment and implementation to accumulate per-task deliveries array, return a merged delivery when a task blocks after earlier commits, and return delivery: null when no task produced changes.
Tests and mock updates
packages/aitm/src/loop/work-loop.test.ts
Updates OrchestratorCalls and makeOrchestrator to capture delivery in openPr calls; adds unit tests for mergeDeliveries (single pass-through, multi-delivery merge) and an integration test asserting a multi-task group's openPr call receives a delivery covering all tasks' changed paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested labels

claudetm

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the primary change: composing PR delivery from every task instead of only the last one.
Linked Issues check ✅ Passed The PR implements the requested aggregate delivery merge and test coverage for multi-task PR composition.
Out of Scope Changes check ✅ Passed The added helper, delivery merge logic, and tests are all directly in scope for the linked issue.
Docstring Coverage ✅ Passed Docstring coverage is 50.00% which is sufficient. The required threshold is 30.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/aggregate-deliveries-79

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/aitm/src/loop/work-loop.ts (1)

413-448: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Preserve already-committed task deliveries on resume.

deliveries only contains results from this invocation. If a run crashes after runOneTask() commits and persists an earlier task as done, but before the group PR opens, the next run skips that task at Line 430 and Line 447 merges only later deliveries, so openPr can still miss committed changes from the skipped task. Persist or reconstruct per-task deliveries before skipping done tasks, or block opening the group PR until the full aggregate can be rebuilt.

🤖 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 `@packages/aitm/src/loop/work-loop.ts` around lines 413 - 448, The group
delivery aggregation in workTasks is only using deliveries from the current
invocation, so a resumed run can skip already-done tasks and still miss their
committed changes when mergeDeliveries is called. Update workTasks (and any
helper state around runOneTask, PrGroup, and task.done) to persist or
reconstruct each task’s delivery before skipping done tasks, so openPr can
rebuild the full aggregate for the group; otherwise, defer/block PR opening
until the complete delivery set is available.
🤖 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.

Outside diff comments:
In `@packages/aitm/src/loop/work-loop.ts`:
- Around line 413-448: The group delivery aggregation in workTasks is only using
deliveries from the current invocation, so a resumed run can skip already-done
tasks and still miss their committed changes when mergeDeliveries is called.
Update workTasks (and any helper state around runOneTask, PrGroup, and
task.done) to persist or reconstruct each task’s delivery before skipping done
tasks, so openPr can rebuild the full aggregate for the group; otherwise,
defer/block PR opening until the complete delivery set is available.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 42d6efcc-cbad-4438-a6c4-c34372b548f4

📥 Commits

Reviewing files that changed from the base of the PR and between 7140325 and df455f7.

📒 Files selected for processing (2)
  • packages/aitm/src/loop/work-loop.test.ts
  • packages/aitm/src/loop/work-loop.ts

@OGtwelve OGtwelve merged commit 6f1b9df into main Jun 30, 2026
3 checks passed
@OGtwelve OGtwelve deleted the fix/aggregate-deliveries-79 branch June 30, 2026 03:09
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.

Multi-task PR body under-reports changes: only the last task's delivery reaches composePr

1 participant