Skip to content

Fix dashboard feedback handoffs - #345

Open
trask wants to merge 31 commits into
mainfrom
trask-dashboard-feedback-lifecycle
Open

Fix dashboard feedback handoffs#345
trask wants to merge 31 commits into
mainfrom
trask-dashboard-feedback-lifecycle

Conversation

@trask

@trask trask commented Sep 2, 2026

Copy link
Copy Markdown
Member

The PR dashboard can keep stale feedback assigned to an author after the work has moved on. This PR makes those handoffs durable without changing the classifier ownership policy from #344.

  • A completed top-level author response retires only the feedback it addresses. Deferred and unrelated requests stay active, while an old CHANGES_REQUESTED state alone does not keep the pull request with its author.
  • /dashboard route:reviewers establishes a permanent content-time cutoff. Top-level comments and review summaries last changed at or before the command do not return. Inline review threads remain open until resolved or outdated.
  • The reviewer handoff remains active across author pushes. Existing review threads do not override it, but newer actionable human feedback ends it, including a reply or edit on an older thread.
  • Unresolved, non-outdated Copilot threads remain author work until someone resolves them or GitHub marks them outdated.

Lifecycle comparisons use content-edit timestamps where GitHub provides them. Bot comments, informational feedback, feedback at or before a handoff, missing timestamps, and classifier failures retain conservative behavior.

trask and others added 2 commits September 2, 2026 12:14
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b4fb04e6-510c-4053-8380-1207e1dad14a
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Sep 2, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on maintainers · refreshed 2026-09-05 11:45 UTC

Merge when ready.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b4fb04e6-510c-4053-8380-1207e1dad14a

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Pull request overview

Fixes dashboard routing when feedback changes across author/reviewer handoffs.

Changes:

  • Tracks effective content-edit timestamps for comments and reviews.
  • Clears stale handoffs and scopes author responses to eligible feedback.
  • Associates Copilot findings with source reviews for accurate supersession.
File summaries
File Description
.github/scripts/pull-request-dashboard/test_pull_request_source.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/test_github_cli.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/test_discussion_lifecycle.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/test_dashboard.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/test_copilot_review.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/pull_request_source.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/pull_request_evaluation.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/pull_request_activity.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/github_cli.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/discussion_lifecycle.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/dashboard_test_support.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/dashboard_override.py Updated as part of this pull request.
.github/scripts/pull-request-dashboard/copilot_review.py Updated as part of this pull request.
Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@trask
trask force-pushed the trask-dashboard-feedback-lifecycle branch from 2699082 to 579756f Compare September 2, 2026 19:58
@trask
trask requested a balanced review from Copilot September 2, 2026 19:59
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b4fb04e6-510c-4053-8380-1207e1dad14a
@trask
trask force-pushed the trask-dashboard-feedback-lifecycle branch from 579756f to beb0d00 Compare September 2, 2026 20:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review details

Suppressed comments (2)

.github/scripts/pull-request-dashboard/copilot_review.py:204

  • The operator and design documentation now describes behavior this branch no longer implements. dashboard.py:162-165 says every unresolved, non-outdated Copilot thread makes a review necessary, and RATIONALE.md:398-410 says such threads clear only on resolution or a push; this code can instead suppress an older open thread after a clean current-head review. Please update both descriptions so users and maintainers understand the supersession rule.
        bool(open_copilot_findings(
            review_threads,
            known_review_ids=known_review_ids,
            superseded_review_ids=superseded_review_ids,
            superseded_before=superseded_before,

.github/scripts/pull-request-dashboard/discussion_lifecycle.py:367

  • RATIONALE.md:479-484 still says only feedback posted after the handoff command is considered, but this now also treats an edit to older feedback as post-command activity. Please document that effective content-edit timestamps can end a handoff so the stated lifecycle matches this filter.
            and after_cutoff(
                comment.get("activity_timestamp")
                or comment.get("timestamp")
                or ""
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

One or more issues must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 3
  • Review effort level: Balanced

Comment thread .github/scripts/pull-request-dashboard/dashboard_test_support.py
Comment thread .github/scripts/pull-request-dashboard/dashboard_test_support.py Outdated
Comment thread .github/scripts/pull-request-dashboard/RATIONALE.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Copilot fallback can incorrectly hide an active finding edited after a clean review.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread .github/scripts/pull-request-dashboard/copilot_review.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The unresolved critical and moderate Copilot supersession issues can incorrectly route pull requests.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread .github/scripts/pull-request-dashboard/pull_request_evaluation.py Outdated
Comment thread .github/scripts/pull-request-dashboard/copilot_review.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical review-ordering regression and a moderate handoff-classification gap remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread .github/scripts/pull-request-dashboard/pull_request_activity.py Outdated
Comment thread .github/scripts/pull-request-dashboard/copilot_review.py Outdated
@trask trask changed the title Fix dashboard feedback lifecycle Fix dashboard feedback handoffs Sep 2, 2026
@trask
trask requested a balanced review from Copilot September 2, 2026 21:07
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b4fb04e6-510c-4053-8380-1207e1dad14a
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 378be2b9-85df-4ac8-a125-a99b653412ac

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

.github/scripts/pull-request-dashboard/discussion_lifecycle.py:1

  • thread_url and strict_author_action are derived from raw_comments[0], but the code explicitly sorts comments into ordered. If the upstream API ever returns comments out of chronological order, raw_comments[0] may not be the thread root, causing (1) incorrect thread URL selection and (2) incorrect Copilot-thread ownership (strict_author_action). Use the chronological root (ordered[0] when present) for both the URL and “thread started by Copilot” detection.
from __future__ import annotations

Comment thread .github/scripts/pull-request-dashboard/dashboard_override.py Outdated
Comment thread .github/scripts/pull-request-dashboard/discussion_lifecycle.py
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 378be2b9-85df-4ac8-a125-a99b653412ac

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The status-comment revision must be bumped so durable cutoff markers reach existing open pull requests.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 31/31 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread .github/scripts/pull-request-dashboard/pr_status_comment.py
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 378be2b9-85df-4ac8-a125-a99b653412ac

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The changes require final human review because they are too complex or risky for automated approval.

Review details
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Base automatically changed from trask-dashboard-classifier-fixes to main September 4, 2026 17:05
Keep what both sides meant to do in every conflicted file.

.github/scripts/pull-request-dashboard/CONTEXT.md: CONTEXT.md: The head side documents the version 17 lifecycle state and its safe migration inputs. The base side documents its version 16 routing state. The result keeps version 17 because it accepts version 16 and therefore preserves the base state while adding the head lifecycle fields.

reviewer_feedback_cases.json: The head side removes per-case measurement dates in favor of corpus-level provenance. The base side adds the still-broken-fix case. The result keeps the new case and omits its per-case date so it follows the final corpus policy.

state.py: The head side adds persistent handoff state as version 17 and accepts version 16. The base side adds workflow-approval routing state as version 16 and advances status rendering to revision 19. The result keeps version 17 with version 16 compatibility, retains the base fields merged elsewhere, and uses status revision 19.

test_eval_baseline.py: The head side removes the per-case date contract. The base side covers the new eval behavior under the earlier per-case date schema. The result keeps the base behavior tests merged elsewhere and removes only the obsolete per-case date assertion.

test_eval_fixture.py: The head side validates corpus-level measurement dates without requiring a date on each case. The base side validates the added case under the earlier per-case date schema. The result keeps the added case and validates it under the corpus-level policy.

test_routing_decision.py: The head side tests persistent and legacy reviewer handoff bindings. The base side tests maintainer-owned workflow approval routing. The result keeps all four tests and uses the persistent-handoff test name for its expanded body.

test_state.py: The head side expects version 17 and migration from version 16. The base side expects its version 16 routing state and status revision 19. The result tests version 17 compatibility with version 16 and status revision 19.
.github/scripts/pull-request-dashboard/eval/reviewer_feedback_cases.json: CONTEXT.md: The head side documents the version 17 lifecycle state and its safe migration inputs. The base side documents its version 16 routing state. The result keeps version 17 because it accepts version 16 and therefore preserves the base state while adding the head lifecycle fields.

reviewer_feedback_cases.json: The head side removes per-case measurement dates in favor of corpus-level provenance. The base side adds the still-broken-fix case. The result keeps the new case and omits its per-case date so it follows the final corpus policy.

state.py: The head side adds persistent handoff state as version 17 and accepts version 16. The base side adds workflow-approval routing state as version 16 and advances status rendering to revision 19. The result keeps version 17 with version 16 compatibility, retains the base fields merged elsewhere, and uses status revision 19.

test_eval_baseline.py: The head side removes the per-case date contract. The base side covers the new eval behavior under the earlier per-case date schema. The result keeps the base behavior tests merged elsewhere and removes only the obsolete per-case date assertion.

test_eval_fixture.py: The head side validates corpus-level measurement dates without requiring a date on each case. The base side validates the added case under the earlier per-case date schema. The result keeps the added case and validates it under the corpus-level policy.

test_routing_decision.py: The head side tests persistent and legacy reviewer handoff bindings. The base side tests maintainer-owned workflow approval routing. The result keeps all four tests and uses the persistent-handoff test name for its expanded body.

test_state.py: The head side expects version 17 and migration from version 16. The base side expects its version 16 routing state and status revision 19. The result tests version 17 compatibility with version 16 and status revision 19.
.github/scripts/pull-request-dashboard/state.py: CONTEXT.md: The head side documents the version 17 lifecycle state and its safe migration inputs. The base side documents its version 16 routing state. The result keeps version 17 because it accepts version 16 and therefore preserves the base state while adding the head lifecycle fields.

reviewer_feedback_cases.json: The head side removes per-case measurement dates in favor of corpus-level provenance. The base side adds the still-broken-fix case. The result keeps the new case and omits its per-case date so it follows the final corpus policy.

state.py: The head side adds persistent handoff state as version 17 and accepts version 16. The base side adds workflow-approval routing state as version 16 and advances status rendering to revision 19. The result keeps version 17 with version 16 compatibility, retains the base fields merged elsewhere, and uses status revision 19.

test_eval_baseline.py: The head side removes the per-case date contract. The base side covers the new eval behavior under the earlier per-case date schema. The result keeps the base behavior tests merged elsewhere and removes only the obsolete per-case date assertion.

test_eval_fixture.py: The head side validates corpus-level measurement dates without requiring a date on each case. The base side validates the added case under the earlier per-case date schema. The result keeps the added case and validates it under the corpus-level policy.

test_routing_decision.py: The head side tests persistent and legacy reviewer handoff bindings. The base side tests maintainer-owned workflow approval routing. The result keeps all four tests and uses the persistent-handoff test name for its expanded body.

test_state.py: The head side expects version 17 and migration from version 16. The base side expects its version 16 routing state and status revision 19. The result tests version 17 compatibility with version 16 and status revision 19.
.github/scripts/pull-request-dashboard/test_eval_baseline.py: CONTEXT.md: The head side documents the version 17 lifecycle state and its safe migration inputs. The base side documents its version 16 routing state. The result keeps version 17 because it accepts version 16 and therefore preserves the base state while adding the head lifecycle fields.

reviewer_feedback_cases.json: The head side removes per-case measurement dates in favor of corpus-level provenance. The base side adds the still-broken-fix case. The result keeps the new case and omits its per-case date so it follows the final corpus policy.

state.py: The head side adds persistent handoff state as version 17 and accepts version 16. The base side adds workflow-approval routing state as version 16 and advances status rendering to revision 19. The result keeps version 17 with version 16 compatibility, retains the base fields merged elsewhere, and uses status revision 19.

test_eval_baseline.py: The head side removes the per-case date contract. The base side covers the new eval behavior under the earlier per-case date schema. The result keeps the base behavior tests merged elsewhere and removes only the obsolete per-case date assertion.

test_eval_fixture.py: The head side validates corpus-level measurement dates without requiring a date on each case. The base side validates the added case under the earlier per-case date schema. The result keeps the added case and validates it under the corpus-level policy.

test_routing_decision.py: The head side tests persistent and legacy reviewer handoff bindings. The base side tests maintainer-owned workflow approval routing. The result keeps all four tests and uses the persistent-handoff test name for its expanded body.

test_state.py: The head side expects version 17 and migration from version 16. The base side expects its version 16 routing state and status revision 19. The result tests version 17 compatibility with version 16 and status revision 19.
.github/scripts/pull-request-dashboard/test_eval_fixture.py: CONTEXT.md: The head side documents the version 17 lifecycle state and its safe migration inputs. The base side documents its version 16 routing state. The result keeps version 17 because it accepts version 16 and therefore preserves the base state while adding the head lifecycle fields.

reviewer_feedback_cases.json: The head side removes per-case measurement dates in favor of corpus-level provenance. The base side adds the still-broken-fix case. The result keeps the new case and omits its per-case date so it follows the final corpus policy.

state.py: The head side adds persistent handoff state as version 17 and accepts version 16. The base side adds workflow-approval routing state as version 16 and advances status rendering to revision 19. The result keeps version 17 with version 16 compatibility, retains the base fields merged elsewhere, and uses status revision 19.

test_eval_baseline.py: The head side removes the per-case date contract. The base side covers the new eval behavior under the earlier per-case date schema. The result keeps the base behavior tests merged elsewhere and removes only the obsolete per-case date assertion.

test_eval_fixture.py: The head side validates corpus-level measurement dates without requiring a date on each case. The base side validates the added case under the earlier per-case date schema. The result keeps the added case and validates it under the corpus-level policy.

test_routing_decision.py: The head side tests persistent and legacy reviewer handoff bindings. The base side tests maintainer-owned workflow approval routing. The result keeps all four tests and uses the persistent-handoff test name for its expanded body.

test_state.py: The head side expects version 17 and migration from version 16. The base side expects its version 16 routing state and status revision 19. The result tests version 17 compatibility with version 16 and status revision 19.
.github/scripts/pull-request-dashboard/test_routing_decision.py: CONTEXT.md: The head side documents the version 17 lifecycle state and its safe migration inputs. The base side documents its version 16 routing state. The result keeps version 17 because it accepts version 16 and therefore preserves the base state while adding the head lifecycle fields.

reviewer_feedback_cases.json: The head side removes per-case measurement dates in favor of corpus-level provenance. The base side adds the still-broken-fix case. The result keeps the new case and omits its per-case date so it follows the final corpus policy.

state.py: The head side adds persistent handoff state as version 17 and accepts version 16. The base side adds workflow-approval routing state as version 16 and advances status rendering to revision 19. The result keeps version 17 with version 16 compatibility, retains the base fields merged elsewhere, and uses status revision 19.

test_eval_baseline.py: The head side removes the per-case date contract. The base side covers the new eval behavior under the earlier per-case date schema. The result keeps the base behavior tests merged elsewhere and removes only the obsolete per-case date assertion.

test_eval_fixture.py: The head side validates corpus-level measurement dates without requiring a date on each case. The base side validates the added case under the earlier per-case date schema. The result keeps the added case and validates it under the corpus-level policy.

test_routing_decision.py: The head side tests persistent and legacy reviewer handoff bindings. The base side tests maintainer-owned workflow approval routing. The result keeps all four tests and uses the persistent-handoff test name for its expanded body.

test_state.py: The head side expects version 17 and migration from version 16. The base side expects its version 16 routing state and status revision 19. The result tests version 17 compatibility with version 16 and status revision 19.
.github/scripts/pull-request-dashboard/test_state.py: CONTEXT.md: The head side documents the version 17 lifecycle state and its safe migration inputs. The base side documents its version 16 routing state. The result keeps version 17 because it accepts version 16 and therefore preserves the base state while adding the head lifecycle fields.

reviewer_feedback_cases.json: The head side removes per-case measurement dates in favor of corpus-level provenance. The base side adds the still-broken-fix case. The result keeps the new case and omits its per-case date so it follows the final corpus policy.

state.py: The head side adds persistent handoff state as version 17 and accepts version 16. The base side adds workflow-approval routing state as version 16 and advances status rendering to revision 19. The result keeps version 17 with version 16 compatibility, retains the base fields merged elsewhere, and uses status revision 19.

test_eval_baseline.py: The head side removes the per-case date contract. The base side covers the new eval behavior under the earlier per-case date schema. The result keeps the base behavior tests merged elsewhere and removes only the obsolete per-case date assertion.

test_eval_fixture.py: The head side validates corpus-level measurement dates without requiring a date on each case. The base side validates the added case under the earlier per-case date schema. The result keeps the added case and validates it under the corpus-level policy.

test_routing_decision.py: The head side tests persistent and legacy reviewer handoff bindings. The base side tests maintainer-owned workflow approval routing. The result keeps all four tests and uses the persistent-handoff test name for its expanded body.

test_state.py: The head side expects version 17 and migration from version 16. The base side expects its version 16 routing state and status revision 19. The result tests version 17 compatibility with version 16 and status revision 19.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

One or more issues must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 35/35 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread .github/scripts/pull-request-dashboard/eval/regenerate_baseline.py Outdated
Copilot comment:

`measurement_date` was already captured at function entry and assigned to every rebuilt case. Recomputing it here can cross UTC midnight, leaving the top-level baseline dates one day later than the cases generated in the same rebuild—the invariant that the removed test previously enforced. Reuse the existing value instead.

Copilot comment:

This scored case still contains measured `run_actions` and a `recorded_label`, but it is now the only case without the `measurement_date` required by the fixture's field definition. That makes the checked-in baseline untraceable and the relaxed fixture test no longer detects the malformed case; restore the date recorded by this measurement update.

Analysis: The rebuild now captures UTC once and reuses that date for every case and both corpus-level fields. The appended scored case records its September 2 measurement date. The fixture test again requires every case date and checks the corpus bounds.

Upsides: A rebuild cannot split its provenance across UTC midnight, and malformed checked-in cases fail the fixture test.

Downsides: The fixture repeats a date on every case, which adds mechanical JSON data.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Durable routing, lifecycle, and state-migration changes require final human review.

Review details
  • Files reviewed: 32/32 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

trask and others added 2 commits September 4, 2026 15:53
…ewer route

Review finding:

This new contract rule turns a reachable state into a failed evaluation. `dashboard_override_facts` sets `persistent_handoff` whenever a bound head exists and the previous pass already recorded a persistent binding, without consulting the currently observed head. `reviewer_handoff_active` in routing_decision.py now requires `bool(facts.head_sha)`, so when a pass observes an empty head SHA the handoff is inactive and the route can resolve to AUTHOR while the command is still pending and unacknowledged. `append_command_ack_reply` then builds a routed reply with `persistent_handoff=True` and `route=DashboardRoute.AUTHOR`, and this check raises. I confirmed it end to end: with previous facts carrying `dashboard_override_persistent=True` and `dashboard_override_head_sha='abc'`, a pending command 5, `head_sha=''`, and one review thread classified as an author action, `evaluate_pull_request` returns `EvaluationFailure` with route UNKNOWN and error `ValueError('persistent reviewer handoff replies require a reviewer route')`. The same input with a matching head returns EvaluationSuccess/APPROVER, so the empty head is the only trigger. The PR itself treats an empty head as a real state: it added `test_override_without_head_uses_non_persistent_acknowledgement`, and it added the `bool(facts.head_sha)` guard to `reviewer_handoff_active`. Concrete fix: make the acknowledgement reply consistent with the route it reports, for example in `append_command_ack_reply` pass `persistent_handoff=facts.dashboard_override_persistent and route is not DashboardRoute.AUTHOR`, so a pre-review-route reply is acknowledged without the persistence marker instead of raising.

Analysis: `DashboardCommandReply` rejects a persistent handoff on an author route, but `append_command_ack_reply` builds the reply from two values that no longer agree. `dashboard_override_persistent` comes from the stored binding, while `route` comes from routing, which `reviewer_handoff_active` now suppresses whenever the observed head is empty. A pending command plus an empty head therefore raises out of `evaluate_pull_request`, and `dashboard_state_update` refuses to publish a failed result. Acknowledging without the persistence marker would resolve the contract but lose the durable record: the marker in the delivered comment is how persistence survives a cache loss, and the reply would also tell the author the wrong thing. Holding the acknowledgement instead keeps the reply truthful and lets the next pass with a real head deliver it as a persistent reviewer handoff.

Upsides: The evaluation no longer fails on a state the pull request already models. The delivered acknowledgement always records the handoff it announces, so the durable marker and the reply text stay consistent. A new test covers both the deferred pass and the delivery once the head returns.

Downsides: A command stays unacknowledged for as long as the observed head is empty, so an author sees no reply in that window.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review finding:

This PR changes what a rendered status comment says but pins `STATUS_COMMENT_REVISION` at its existing value of 19. The constant's own comment in state.py states the rule: 'Rendered status-comment behavior. Increment when existing comments need to adopt a change; hourly runs durably roll it out to all open PRs.' `author_override_guidance()` in dashboard_override.py changed from 'The dashboard binds the request to the head it sees when it reads the command, and a later push restores normal routing.' to text saying the handoff remains active across pushes and that top-level feedback through the command is retired. That guidance is rendered into every waiting-on-author status comment, so the old sentence is now factually wrong for open PRs. `render_status_comment` also now emits a `pull-request-dashboard-top-level-feedback-cutoff` marker that existing comments lack. Without a bump, `prepare_rollout_state` never queues open PRs, so a PR whose status comment is not otherwise re-rendered keeps telling its author that a push restores normal routing. Two precedents in this repository changed the same guidance and bumped the revision in the same commit: 5819c68 ('Clear open items on a reviewer-routing override', 14 to 15) and 225dbdb ('Keep conflicted PR routing actionable without gate failures', 15 to 16). Concrete fix: set `STATUS_COMMENT_REVISION = 20` in state.py and update this assertion to 20.

Analysis: `render_status_comment` now emits a `pull-request-dashboard-top-level-feedback-cutoff` marker, and `upsert_status_comment` preserves it across later renders. That marker is the durable record of the permanent feedback cutoff, so a status comment written before this change carries no cutoff at all. The branch allocated a revision for exactly this rollout in 304849a, which moved the constant from 16 to 17, but the merge with a6d35ab kept main's newer 19 and dropped the increment. The published rollout state already reads target 19 and completed 19 with an empty queue, so `prepare_rollout_state` queues nothing. A result that is otherwise unchanged does not enqueue a comment update, so an open pull request with no further activity would never adopt the marker. Moving to 20 restores the increment the branch intended. The finding's claim about `author_override_guidance()` does not hold: that helper is no longer rendered into a status comment, and only the marker rollout is at stake.

Upsides: Every open pull request adopts the new lifecycle markers on the next hourly run, so the permanent feedback cutoff survives a state cache loss on pull requests that predate this change.

Downsides: The rollout re-renders the status comment of every open pull request once, which costs one delivery pass per pull request.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes persistent routing, migration, and feedback lifecycle behavior across many components.

Review details
  • Files reviewed: 32/32 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@trask
trask marked this pull request as ready for review September 5, 2026 01:06
@trask
trask requested a review from a team as a code owner September 5, 2026 01:06
@trask
trask requested a review from adrielp September 5, 2026 01:06
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.

3 participants