Skip to content

Acknowledge routing overrides with the label instead of a comment - #204

Closed
trask wants to merge 1 commit into
open-telemetry:mainfrom
trask:dashboard-override-ack-without-comment
Closed

Acknowledge routing overrides with the label instead of a comment#204
trask wants to merge 1 commit into
open-telemetry:mainfrom
trask:dashboard-override-ack-without-comment

Conversation

@trask

@trask trask commented Jul 30, 2026

Copy link
Copy Markdown
Member

The dashboard posted @user routed this pull request to reviewers. after applying dashboard:route-overridden, but the label event already shows that on the timeline, so the comment is redundant noise.

That comment was load-bearing: it carried the acknowledgement marker that retires a /dashboard route:reviewers command, and without it the command would be replayed each time the label is automatically released. The marker now rides the dashboard status comment, which is already maintained on every pull request. The status comment also records the override label it reported, so a label applied by hand stays distinguishable from the label a command just produced and a command on an already-overridden pull request still gets the "already routed" reply.

Existing acknowledgement comments still satisfy the marker scan, so overrides that are in flight today are not replayed.

@trask
trask requested a review from Copilot July 30, 2026 19:42

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.

Pull request overview

Note

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.

Removes the redundant “routed this pull request…” acknowledgement comment by moving the override acknowledgement marker into the dashboard status comment, while preserving behavior that prevents /dashboard route:reviewers from being replayed.

Changes:

  • Add override acknowledgement + override-label markers to the rendered PR status comment.
  • Adjust override facts/no-op logic to distinguish “own label in flight” vs. already-overridden states.
  • Update/expand unit tests and bump the status comment revision to roll out the new marker behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
.github/scripts/pull-request-dashboard/test_pr_status_comment.py Adds tests ensuring the status comment carries/omits the new override markers correctly.
.github/scripts/pull-request-dashboard/test_dashboard_override.py Updates tests to reflect removal of the routed acknowledgement comment and new ack/label-marker behavior.
.github/scripts/pull-request-dashboard/state.py Bumps STATUS_COMMENT_REVISION to roll out updated status comment rendering.
.github/scripts/pull-request-dashboard/pr_status_comment.py Injects override ack + label markers into the status comment body.
.github/scripts/pull-request-dashboard/dashboard_override.py Introduces OVERRIDE_LABEL_MARKER, derives new ack fields, removes comment-posting acknowledgement, and refines noop logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +128 to +133
# A command counts as acknowledged once its label is observed, and the
# status comment carries the marker from then on so label removal cannot
# replay the command.
"dashboard_override_ack_id": (
max(acknowledged_id, command_id) if label_applied else acknowledged_id
),
Comment on lines 255 to 276
message = (
"only the pull request author or a member of an approving team can "
"use `/dashboard route:reviewers`."
)
elif kind == "routed":
if reply.get("route") == "copilot":
message = (
"accepted the reviewer-routing override; the reviewer handoff "
"is waiting on Copilot."
)
else:
message = "routed this pull request to reviewers."
elif kind == "already_routed":
where = ROUTE_ALREADY_ROUTED_PHRASE.get(
reply.get("route") or "", "not currently waiting on you"
)
message = (
f"this pull request is {where}, so `/dashboard route:reviewers` had "
"no effect. The command only applies while the pull request is "
"waiting on you."
)
else:
subcommand = reply.get("subcommand") or ""
attempted = DASHBOARD_COMMAND_PREFIX + (f" {subcommand}" if subcommand else "")
message = (
f"`{attempted}` is not a recognized dashboard command. The only "
"supported command is `/dashboard route:reviewers`, which the pull "
"request author can use to move a pull request from waiting on the "
"author to waiting on reviewers."
)
Comment on lines +124 to +127
"dashboard_override_acknowledged_id": acknowledged_id,
"dashboard_override_label_observed": _observed_override_label(
raw.get("issue_comments")
),
Comment on lines +131 to +133
"dashboard_override_ack_id": (
max(acknowledged_id, command_id) if label_applied else acknowledged_id
),
@trask

trask commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Closing, going in a different direction with #205

@opentelemetry-pr-dashboard

Copy link
Copy Markdown

Pull request dashboard status

Closed · refreshed 2026-07-30 21:26 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

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.

2 participants