Tighten PR sous-chef nudge heuristics: 4-per-run cap, deterministic prioritization, and safe review-thread resolution#44805
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Hey The PR is focused, well-described, includes contract test coverage in
|
There was a problem hiding this comment.
Pull request overview
Adds bounded, prioritized PR nudging and automated review-thread resolution.
Changes:
- Caps nudges at four and defines deterministic prioritization.
- Adds safe-output thread resolution and reporting.
- Extends workflow contract assertions and recompiles the workflow.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/pr-sous-chef.md |
Updates workflow behavior and safe outputs. |
.github/workflows/pr-sous-chef.lock.yml |
Regenerates the compiled workflow. |
pkg/cli/pr_sous_chef_workflow_contract_test.go |
Extends contract checks. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Medium
| 5. Prioritize which PRs to nudge, in this order: | ||
| - `mergeStateStatus == "CONFLICTING"` first (explicit merge-conflict unblock request). | ||
| - PRs with unresolved review threads where at least one thread already has a follow-up response from the PR author or `@copilot` but remains unresolved. | ||
| - Remaining PRs by most-recent `updatedAt`. | ||
| If two PRs are still tied, prioritize the lower PR number first for deterministic behavior and stable reruns. |
| ```bash | ||
| safeoutputs resolve_pull_request_review_thread --thread_id PRRT_kwDOABCD1234 | ||
| ``` | ||
| - If resolving one thread fails, append `{pr_number: <N>, skip_reason: "resolve_review_thread_failed", thread_id: "<thread-id>"}` to the run-summary `skipped` array and continue with remaining thread IDs; do not fail the run solely because one resolution attempt failed. |
| 4. **Dismiss stale `github-actions[bot]` blocking reviews when all PR review threads are resolved** | ||
| - **Slash-command guard**: If triggered via the `/souschef` slash command (`pull_request_comment` event), skip this dismissal step entirely — slash-command runs are acknowledgment nudges and must not perform automated review cleanup. | ||
| - For `schedule` and `workflow_dispatch` runs, use the `dismiss_reviews` list returned by the `pr-processor` sub-agent. The sub-agent populates this list only when ALL review threads on the PR are resolved; leave reviews untouched if any thread remains unresolved. | ||
| - For `schedule` and `workflow_dispatch` runs, use the `dismiss_reviews` list returned by the `pr-processor` sub-agent. The sub-agent populates this list only when ALL review threads on the PR are resolved (including threads resolved in step 3); leave reviews untouched if any thread remains unresolved. |
| assert.Contains(t, text, "Process all eligible PRs per run.", "Workflow should require processing all eligible PRs") | ||
| assert.Contains(t, text, "Process at most 4 nudges per run.", "Workflow should cap nudges per run") | ||
| assert.Contains(t, text, "add-comment:\n max: 4", "Workflow should hard-cap add_comment safe-output calls to 4 per run") | ||
| assert.Contains(t, text, "Prioritize which PRs to nudge, in this order:", "Workflow should define deterministic PR prioritization for nudges") |
|
🎉 This pull request is included in a new release. Release: |
PR sous-chef could over-nudge and re-nudge low-signal targets, and it did not explicitly resolve review threads that already had responses. This update constrains nudge volume, prioritizes the highest-value PRs first, and adds safe-output-driven thread resolution before stale-review dismissal.
Nudge budget and ordering
mergeStateStatus == CONFLICTING@copilotfollow-up repliesupdatedAtdesc, then PR number asc for stable tiesRepeat-nudge suppression
marker + @copilot), with the existing conflict-state exception.Safe-output review-thread resolution
resolve-pull-request-review-threadto workflow safe-outputs.@copilot.skippedentries.Run-report accounting
resolved_review_threadsto run summary counters.github-actions[bot]blocking reviews contingent on all threads being resolved (including newly resolved ones).Contract coverage updates
pr_sous_chef_workflow_contract_test.goassertions for: