Refactor Rust guard response item extraction and PR merged detection by Copilot · Pull Request #8422 · github/gh-aw-mcpg · GitHub
Skip to content

Refactor Rust guard response item extraction and PR merged detection#8422

Merged
lpcox merged 4 commits into
mainfrom
copilot/rust-guard-extract-items-slice
Jul 1, 2026
Merged

Refactor Rust guard response item extraction and PR merged detection#8422
lpcox merged 4 commits into
mainfrom
copilot/rust-guard-extract-items-slice

Conversation

Copilot AI commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

The Rust guard had two duplicated response-shape extraction blocks in response_items.rs and duplicated PR merged-state detection in both helpers.rs and backend.rs. This change consolidates both paths and preserves the existing response-shape handling while fixing the merged_at: nullmerged: true fallback case.

  • Consolidate response item extraction

    • Add extract_items_slice(response, list_field) in labels/response_items.rs
    • Reuse it for both PR and issue labeling paths
    • Preserve the existing extraction order:
      1. root array
      2. items
      3. type-specific envelope (pull_requests / issues)
      4. GraphQL nodes
      5. GraphQL single object
      6. bare REST singleton object
  • Centralize PR merged-state detection

    • Add shared is_pr_merged(item) in labels/helpers.rs
    • Replace inline merged checks in:
      • pr_integrity(...)
      • get_pull_request_facts_with_callback(...)
    • Remove raw "merged_at" / "merged" handling from backend.rs
  • Behavioral cleanup

    • Correct merged detection when merged_at is present but null, so the logic now falls back to merged: true as intended
  • Targeted coverage

    • Add focused tests for:
      • type-specific item envelopes
      • singleton REST object promotion
      • is_pr_merged timestamp and boolean fallback behavior
      • backend PR fact extraction via merged boolean fallback
fn extract_items_slice<'a>(response: &'a Value, list_field: &str) -> &'a [Value] {
    if let Some(arr) = response.as_array() {
        arr.as_slice()
    } else if let Some(arr) = response.get("items").and_then(|v| v.as_array()) {
        arr.as_slice()
    } else if let Some(arr) = response.get(list_field).and_then(|v| v.as_array()) {
        arr.as_slice()
    } else if let Some(nodes) = extract_graphql_nodes(response) {
        nodes.as_slice()
    } else if let Some(obj) = extract_graphql_single_object(response) {
        std::slice::from_ref(obj)
    } else {
        &[]
    }
}

GitHub Advanced Security started work on behalf of lpcox July 1, 2026 14:58 View session
GitHub Advanced Security finished work on behalf of lpcox July 1, 2026 14:59
Copilot AI changed the title [WIP] Extract extract_items_slice helper to eliminate duplicated item-extraction block Refactor Rust guard response item extraction and PR merged detection Jul 1, 2026
GitHub Advanced Security started work on behalf of lpcox July 1, 2026 15:01 View session
Copilot finished work on behalf of lpcox July 1, 2026 15:01
Copilot AI requested a review from lpcox July 1, 2026 15:01
GitHub Advanced Security finished work on behalf of lpcox July 1, 2026 15:01
@lpcox lpcox marked this pull request as ready for review July 1, 2026 15:26
Copilot AI review requested due to automatic review settings July 1, 2026 15:26
@lpcox

lpcox commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

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

This PR refactors the Rust GitHub guard’s response-shape handling and PR merged-state detection to remove duplicated logic while preserving existing extraction behavior, and fixes the merged_at: null case so it correctly falls back to merged: true.

Changes:

  • Consolidates response item extraction into a shared helper (extract_items_slice) and reuses it across PR and issue labeling paths.
  • Centralizes PR merged detection via is_pr_merged and reuses it in both integrity calculation and backend fact extraction.
  • Adds targeted Rust tests to cover type-specific envelopes, REST singleton promotion, and merged boolean fallback behavior.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/labels/response_items.rs Introduces extract_items_slice to unify REST/search/GraphQL item extraction and adds focused extraction tests.
guards/github-guard/rust-guard/src/labels/helpers.rs Adds is_pr_merged helper and updates PR integrity logic + tests to use it.
guards/github-guard/rust-guard/src/labels/backend.rs Removes inline merged detection in PR fact extraction, reusing is_pr_merged, and adds a regression test for boolean fallback.

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: 1
  • Review effort level: Low

Comment on lines 1790 to +1791
// Check if PR is merged (either merged_at field exists or merged boolean is true)
let mut is_merged = item
.get(field_names::MERGED_AT)
.map(|v| !v.is_null())
.or_else(|| item.get(field_names::MERGED).and_then(|v| v.as_bool()))
.unwrap_or(false);
let mut is_merged = is_pr_merged(item);

Copilot AI commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

GitHub Advanced Security started work on behalf of lpcox July 1, 2026 15:38 View session
Copilot finished work on behalf of lpcox July 1, 2026 15:39
GitHub Advanced Security finished work on behalf of lpcox July 1, 2026 15:39
@lpcox lpcox merged commit 9d7d6cb into main Jul 1, 2026
26 checks passed
@lpcox lpcox deleted the copilot/rust-guard-extract-items-slice branch July 1, 2026 15:50
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.

[rust-guard] Rust Guard: Extract extract_items_slice helper to eliminate duplicated item-extraction block

3 participants