Skip to content

Optimize Rust guard label sharing and tool classification lookups#6846

Merged
lpcox merged 3 commits into
mainfrom
copilot/rust-guard-hoist-default-secrecy
Jun 1, 2026
Merged

Optimize Rust guard label sharing and tool classification lookups#6846
lpcox merged 3 commits into
mainfrom
copilot/rust-guard-hoist-default-secrecy

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 1, 2026

The Rust guard still deep-cloned per-response secrecy labels in the PR and issue path labeling hot paths, and tool classification still relied on linear scans over static operation lists. This change aligns both areas with the existing fast-path patterns already used elsewhere in the guard.

  • Shared secrecy hoisting in response_paths.rs

    • Converts PR and issue default_secrecy values to SharedLabels before entering per-item loops
    • Reuses the shared labels for each item and for default_labels, avoiding repeated Vec<String> deep clones on collection responses
    • Keeps search behavior unchanged by continuing to derive per-item repo-specific secrecy where required
  • Binary-search classification in tools.rs

    • Sorts WRITE_OPERATIONS and READ_WRITE_OPERATIONS lexicographically
    • Replaces contains() with binary_search(...).is_ok() in is_write_operation and is_read_write_operation
    • Preserves existing classification behavior while reducing lookup cost on the guard’s invocation path
  • Regression coverage

    • Adds sorted-order tests so future additions cannot silently break binary_search correctness
    • Adds focused response-path assertions confirming list PR and list issue item secrecy still matches the default labels after the sharing change
let default_secrecy_shared: crate::SharedLabels = default_secrecy.into();

secrecy: if tool_name == "search_pull_requests" {
    repo_visibility_secrecy_for_repo_id(repo_for_labels, ctx).into()
} else {
    default_secrecy_shared.clone()
},

pub fn is_write_operation(tool_name: &str) -> bool {
    WRITE_OPERATIONS.binary_search(&tool_name).is_ok()
        || is_lock_operation(tool_name)
        || is_unlock_operation(tool_name)
}

Copilot AI changed the title [WIP] Optimize default secrecy hoisting in response_paths.rs Optimize Rust guard label sharing and tool classification lookups Jun 1, 2026
@lpcox lpcox marked this pull request as ready for review June 1, 2026 14:59
Copilot AI review requested due to automatic review settings June 1, 2026 14:59
Copilot AI requested a review from lpcox June 1, 2026 15:00
Copilot finished work on behalf of lpcox June 1, 2026 15:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 optimizes two Rust guard hot paths by (1) hoisting/sealing default secrecy labels into SharedLabels once per response instead of deep-cloning per item, and (2) switching tool classification lookups from linear scans to binary_search over sorted static lists.

Changes:

  • Hoist default_secrecy into SharedLabels in PR/issue response path labeling loops to avoid repeated Vec<String> clones.
  • Sort WRITE_OPERATIONS / READ_WRITE_OPERATIONS and use binary_search(...).is_ok() for faster tool classification.
  • Add regression tests to enforce sorted operation lists and verify PR/issue list item secrecy matches default_labels.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/tools.rs Sorts tool operation lists, switches to binary search, and adds tests to prevent accidental unsorted regressions.
guards/github-guard/rust-guard/src/labels/response_paths.rs Hoists default secrecy into SharedLabels before per-item loops and adds tests confirming list item secrecy matches default labels.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@lpcox lpcox merged commit d962d73 into main Jun 1, 2026
29 checks passed
@lpcox lpcox deleted the copilot/rust-guard-hoist-default-secrecy branch June 1, 2026 15:13
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: SharedLabels hoisting in response_paths.rs + binary search in tools.rs

3 participants