Skip to content

Fix PR dashboard top-level feedback routing#107

Draft
trask wants to merge 15 commits into
open-telemetry:mainfrom
trask:fix-pr-dashboard-top-level-feedback
Draft

Fix PR dashboard top-level feedback routing#107
trask wants to merge 15 commits into
open-telemetry:mainfrom
trask:fix-pr-dashboard-top-level-feedback

Conversation

@trask

@trask trask commented Jul 18, 2026

Copy link
Copy Markdown
Member

Resolves #106

Improve review-feedback routing by giving top-level classification the PR author identity, so requests and questions directed to other participants are not assigned to the author. Prefer commit evidence for implementation-choice feedback so later author commits clear those items, distinguish completed author replies from promises of future work, exclude minimized PR comments, and use content edit timestamps so non-content comment updates do not reopen feedback.

This corrects reported statuses on open-telemetry/opentelemetry-proto#340, open-telemetry/weaver#1600, open-telemetry/opentelemetry-java#8076, and open-telemetry/semantic-conventions-genai#197 and #188, and addresses part of #106.

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

Routes top-level PR feedback using the author’s identity and improves lifecycle handling for implementation-choice feedback.

Changes:

  • Adds PR author identity to top-level classification inputs.
  • Clarifies participant routing and commit-evidence prompts.
  • Adds regression coverage for routing and commit resolution.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
classification.py Updates classification input and guidance.
dashboard.py Adds author identity to derived feedback items.
test_top_level_actions.py Covers participant routing and commit evidence.

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

@trask
trask force-pushed the fix-pr-dashboard-top-level-feedback branch from 557f8ba to 9b79008 Compare July 18, 2026 16:30

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines +360 to +361
if c.get("minimized"):
continue

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

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment on lines +192 to +196
nodes {
databaseId
createdAt
lastEditedAt
}

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

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/scripts/pull-request-dashboard/dashboard.py:378

  • This condition cannot exclude real minimized comments: raw["issue_comments"] is populated from the REST issue-comments endpoint, whose payload has no minimized field, and the new GraphQL enrichment currently supplies only timestamps. The test passes only because it manually injects this field. Fetch isMinimized in the GraphQL query and merge that metadata into each REST comment before normalization.
        if c.get("minimized"):
            continue

endCursor
}
nodes {
databaseId

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

.github/scripts/pull-request-dashboard/github_cli.py:193

  • databaseId is the legacy 32-bit field, but current issue-comment IDs (including the linked 5011501871 example) exceed that range. GraphQL can therefore return no usable value (or fail serialization), causing the parser to skip the comment and fetch_pr_raw to fall back to created_at, which defeats the new content-edit timestamp behavior. Query and parse fullDatabaseId, as the review path already does at github_cli.py:139 and github_cli.py:274-281.
                    databaseId

.github/scripts/pull-request-dashboard/dashboard.py:382

  • This check does not exclude minimized comments in production. issue_comments comes from the REST endpoint, whose records are only augmented with content_updated_at; the new GraphQL query does not fetch or merge isMinimized. The test's synthetic minimized field therefore never exists for fetched PR data. Fetch minimization state through GraphQL and merge it into each REST comment before normalization.
        if c.get("minimized"):
            continue

Comment thread .github/scripts/pull-request-dashboard/dashboard.py Outdated
Comment thread .github/scripts/pull-request-dashboard/dashboard.py Outdated
Comment thread .github/scripts/pull-request-dashboard/classification.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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

.github/scripts/pull-request-dashboard/dashboard.py:382

  • This check never sees minimization data in production. issue_comments comes from the REST issue-comments endpoint, which does not expose a minimized field, while the new GraphQL helper returns only timestamps. As a result, minimized comments still become feedback items despite the PR's stated behavior. Fetch isMinimized in the GraphQL query and merge it into each REST comment (or source the comments from GraphQL) before normalizing.
        if c.get("minimized"):
            continue

.github/scripts/pull-request-dashboard/classification.py:663

  • This adds one serial Copilot process per uncached author comment, with no per-PR bound or batching. A long-lived PR can have hundreds of comments, and each call permits a 180-second timeout, so the first refresh after this change can monopolize the workflow/model quota—the exact failure mode the top-level 200-item cap and batching avoid (RATIONALE.md:211-216). Please batch and/or cap author-reply classification with a safe unclassified fallback that can be retried on later refreshes.
    author_reply_classifications = classify_review_threads(
        number, author_reply_items, model, cache_in, cache_out
    )

.github/scripts/pull-request-dashboard/github_cli.py:195

  • databaseId is a nullable GraphQL Int, so modern issue-comment IDs above 2,147,483,647 cannot be represented and come back null; the comments cited in this PR already have IDs around 5,011,501,871. The loop then skips them, and fetch_pr_raw falls back to created_at, so edits to current comments are not detected. Query the GraphQL node id and join it to the REST comment's node_id instead of joining on the numeric REST ID.
                nodes {
                    databaseId
                    createdAt
                    lastEditedAt

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/scripts/pull-request-dashboard/dashboard.py:382

  • This guard cannot exclude minimized PR comments in production: raw["issue_comments"] comes from the REST issue-comments endpoint, whose response does not expose minimized state, while the new GraphQL query only returns edit timestamps. As a result, c.get("minimized") is always absent for fetched comments and minimized feedback is still classified. Fetch IssueComment.isMinimized in GraphQL and merge that metadata by databaseId before normalization.
        if c.get("minimized"):
            continue

Comment thread .github/scripts/pull-request-dashboard/dashboard.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.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

.github/scripts/pull-request-dashboard/dashboard.py:382

  • The production issue-comment payload never supplies this flag. fetch_pr_raw gets comment bodies from the REST issue-comments endpoint, which does not expose minimization, while the new GraphQL query only requests timestamps and the merge only adds content_updated_at. As a result, minimized reviewer comments still become feedback (and minimized author comments can still count as replies); the test passes only because it injects a synthetic minimized field. Fetch isMinimized through GraphQL and carry it into the REST comment records before normalization.
        if c.get("minimized"):
            continue

.github/scripts/pull-request-dashboard/dashboard.py:781

  • A self-deferral or unclear author comment is returned here and masks every later external-blocker comment. For example, “I'll test this” followed by “this is blocked on the upstream spec” leaves the item waiting on the author because the caller only propagates this first outcome when it is external. Skip non-external outcomes while searching so a later external handoff can route the earlier feedback externally.
        if (
            event.get("kind") == "issue-comment"
            and event.get("actor_role") == "author"
            and timestamp > root_timestamp
            and outcome is not None
        ):
            return outcome

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

.github/scripts/pull-request-dashboard/dashboard.py:382

  • fetch_pr_raw() builds these objects from the REST issue-comments response plus a GraphQL map containing only edit timestamps. REST issue comments do not expose minimization state, and PR_ISSUE_COMMENT_EDITS_QUERY does not request isMinimized, so production comments never receive this minimized key and minimized feedback is still classified. Fetch the GraphQL minimization flag and merge it into each REST comment before normalization.
        if c.get("minimized"):
            continue

.github/scripts/pull-request-dashboard/dashboard.py:781

  • This returns the first classified author comment after the feedback even when its action is author or unclear. If the author first promises more work and later reports an upstream blocker, the first outcome prevents the later external outcome from ever reaching the routing branch, so the PR incorrectly remains waiting on the author. Select the earliest external outcome instead.
        if (
            event.get("kind") == "issue-comment"
            and event.get("actor_role") == "author"
            and timestamp > root_timestamp
            and outcome is not None
        ):
            return outcome

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

.github/scripts/pull-request-dashboard/dashboard.py:382

  • The production REST comment objects are not populated with a minimized field, and the new GraphQL query only joins edit timestamps, so this condition never excludes minimized comments. Fetch isMinimized in PR_ISSUE_COMMENT_EDITS_QUERY (or consume the REST API's actual minimization field) and carry that value onto each REST comment before normalization; otherwise minimized feedback still appears on the dashboard despite this test.
        if c.get("minimized"):
            continue

.github/scripts/pull-request-dashboard/dashboard.py:780

  • This returns the first classified author comment even when it is an author self-deferral. If the author first promises future work and later reports an upstream blocker, the caller ignores the first outcome and never reaches the later external outcome, so the item incorrectly remains waiting on the author. Skip non-external outcomes here; completed none replies are already handled by collect_author_evidence.
            and outcome is not None
        ):

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

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

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

.github/scripts/pull-request-dashboard/github_cli.py:193

  • Use the 64-bit ID field here. The cited issue-comment IDs already exceed GraphQL's legacy 32-bit databaseId range (for example 5011501871), so this field can be null and line 220 then skips the timestamp. Those comments fall back to created_at, defeating the non-content-update fix. The existing review query uses fullDatabaseId for this reason (github_cli.py:139, with >32-bit coverage at test_github_cli.py:324,351). An alias keeps the parser unchanged.
                    databaseId

.github/scripts/pull-request-dashboard/dashboard.py:382

  • This guard is never populated by the production fetch path. fetch_pr_raw gets issue comments from the REST endpoint and only merges content_updated_at; the GraphQL query requests no minimization field. Consequently, live minimized comments still become feedback (and minimized author comments can still become reply evidence), while the test passes only because it injects a synthetic minimized key. Fetch isMinimized from GraphQL and propagate it onto the REST comments before normalization.
        if c.get("minimized"):
            continue

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

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

.github/scripts/pull-request-dashboard/github_cli.py:193

  • databaseId is nullable for modern issue comments whose numeric IDs exceed GraphQL's 32-bit Int limit; the affected comments named in this PR already have IDs above 5 billion. Those nodes are skipped below, so fetch_pr_raw falls back to created_at and real content edits never advance the feedback timestamp. Join these nodes to the REST comments using the GraphQL node id/REST node_id (or another full-width identifier) instead of databaseId.
                    databaseId

.github/scripts/pull-request-dashboard/dashboard.py:382

  • This check cannot exclude minimized comments in production: raw["issue_comments"] comes from the REST issue-comments endpoint, whose payload does not expose minimization state, and the new GraphQL enrichment only adds timestamps. As a result, the new test passes only because it injects a minimized key that fetch_pr_raw never produces. Fetch isMinimized via GraphQL and merge it into each REST comment before normalization.
        if c.get("minimized"):
            continue

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.

PR Dashboard: Investigate incorrect PR statuses

2 participants