Skip to content

[Repo Assist] perf(rust-guard): hoist invariant label calls and dedup PR number extraction#6201

Open
github-actions[bot] wants to merge 2 commits into
mainfrom
repo-assist/fix-issue-6193-hoist-invariant-response-paths-c01c1091ab084948
Open

[Repo Assist] perf(rust-guard): hoist invariant label calls and dedup PR number extraction#6201
github-actions[bot] wants to merge 2 commits into
mainfrom
repo-assist/fix-issue-6193-hoist-invariant-response-paths-c01c1091ab084948

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Closes #6193

Root Cause

Two response_paths.rs collection branches recomputed constant labels on every loop iteration (and again in default_labels), and the get_pull_request / pull_request_read arm in tool_rules.rs scanned JSON args twice for the same PR number.

Changes

response_paths.rs

  • list_notifications: hoist private_user_label() before the loop. It always returns the same Vec<String>, was allocated N+1 times per response. Now computed once and Arc::cloned (free) per item.
  • list_gists: hoist reader_integrity(scope_names::USER, ctx) before the loop. The function may traverse scope-policy lookups before allocating label strings; was called N+1 times, now N Arc::clones.

tool_rules.rs

  • get_pull_request / pull_request_read / list_pull_requests arm: extract pull_number once at the top. Eliminates a redundant JSON field scan on every single-PR read call. Also creates a single source of truth for how the PR number is extracted.

Why This Matters

SharedLabels wraps Arc<Vec<String>>, so .clone() is a single atomic ref-count increment — essentially free. The hoisted calls eliminate N redundant Vec allocations per list_notifications and list_gists response. For a typical page of 30–50 items this removes ~30–50 heap allocations per request.

This mirrors the same improvement already applied to response_items.rs on 2026-05-19.

Test Status

  • ✅ Rust: cargo test413 tests passed
  • ⚠️ Go: make build / make test-unit blocked by Go toolchain network download (go1.25.0 proxy.golang.org Forbidden) — infrastructure constraint, not caused by this change. The Rust-only files changed have no Go counterpart.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Repo Assist · ● 2.7M ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@851905c06e905bf362a9f6cc54f912e3df747d55

…raction

response_paths.rs:
- list_notifications: hoist private_user_label() before loop; was
  allocated N+1 times per response, now N Arc::clones (free)
- list_gists: hoist reader_integrity(scope_names::USER, ctx) before
  loop; was called N+1 times, now N Arc::clones

tool_rules.rs:
- pull_request arm: extract pull_number once at the top; eliminates a
  redundant JSON field scan on every get_pull_request/pull_request_read
  call

Closes #6193

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review May 21, 2026 14:02
Copilot AI review requested due to automatic review settings May 21, 2026 14:02
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 targets small performance improvements in the Rust guard labeling layer by reducing per-item allocations in response labeling and deduplicating pull request number extraction in tool labeling.

Changes:

  • Hoists loop-invariant label computations in label_response_paths for list_notifications and list_gists to avoid repeated allocations.
  • Deduplicates PR number extraction in apply_tool_labels for PR-related tool names to avoid scanning tool args twice.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/labels/response_paths.rs Hoists invariant SharedLabels computations out of per-item loops for notifications and gists.
guards/github-guard/rust-guard/src/labels/tool_rules.rs Extracts pull_number once per PR tool arm and reuses it for description/back-end lookup.

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: 3

Comment on lines 216 to 220
//
// Extract once; reused for desc and backend lookup to avoid a second JSON field scan.
let pull_number = extract_number_as_string(tool_args, field_names::PULL_NUMBER)
.or_else(|| extract_number_as_string(tool_args, "pullNumber"));
if !owner.is_empty() && !repo.is_empty() {
Comment on lines 518 to 522
labels: crate::ResourceLabels {
description: format!("notification:{}", id),
secrecy: private_user_label().into(),
secrecy: notif_secrecy.clone(),
integrity: vec![].into(),
},
Comment on lines 563 to 567
labels: crate::ResourceLabels {
description: format!("gist:{}", id),
secrecy: secrecy.into(),
integrity: reader_integrity(scope_names::USER, ctx).into(),
integrity: gist_integrity.clone(),
},
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 21, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor

Copilot AI commented May 21, 2026

@copilot address review feedback

All three review points addressed in the latest commit:

  • tool_rules.rs: pull_number extraction moved inside the get_pull_request/pull_request_read branch — list_pull_requests skips the JSON scan entirely now.
  • response_paths.rs (notifications): empty_integrity: SharedLabels hoisted before the loop; both the per-item entry and default_labels clone the same Arc instead of each allocating a fresh Arc<Vec<String>>.
  • response_paths.rs (gists): public_gist_secrecy and private_gist_secrecy hoisted before the loop; private_user_label() is called once and the per-item assignment is a free Arc::clone for both public and private gists.

413 tests pass.

Copilot finished work on behalf of lpcox May 21, 2026 14:39
Copilot AI requested a review from lpcox May 21, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rust-guard] Rust Guard: Hoist invariant label calls outside per-item loops in response_paths.rs

3 participants