[Repo Assist] perf(rust-guard): hoist invariant integrity/secrecy calls outside per-item loops#6005
Merged
lpcox merged 3 commits intoMay 19, 2026
Conversation
…-item loops Eliminates redundant Vec allocations in three label_response_items branches: - list_gists: hoist reader_integrity() outside loop (1 alloc instead of N) - list_notifications: hoist private_user_label() + none_integrity() (2 allocs instead of 2N) - list_releases: hoist merged_integrity() outside loop (1 alloc instead of N) For a default 30-item batch this removes ~90 redundant heap allocations, matching the pattern already used correctly in the get_file_contents branch. Also extracts DEFAULT_BRANCH_NAMES constant in helpers.rs, consistent with WRITE_OPERATIONS, BLOCKED_TOOLS and similar named arrays in the codebase. Closes #5997 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets performance in the Rust guard labeling pipeline by reducing repeated work inside per-item response labeling loops, and slightly improves maintainability in branch-ref handling.
Changes:
- Hoists invariant integrity/secrecy label construction out of several per-item loops in
label_response_items. - Clones the pre-built label vectors per item instead of recomputing them each iteration.
- Extracts a
DEFAULT_BRANCH_NAMESconstant used byis_default_branch_ref.
Show a summary per file
| File | Description |
|---|---|
guards/github-guard/rust-guard/src/labels/response_items.rs |
Hoists invariant label construction out of loops for gists, notifications, and releases. |
guards/github-guard/rust-guard/src/labels/helpers.rs |
Introduces DEFAULT_BRANCH_NAMES and uses it in is_default_branch_ref for easier updates. |
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
388
to
392
| labels: ResourceLabels { | ||
| description: format!("gist:{}", id), | ||
| secrecy, | ||
| integrity: reader_integrity(scope_names::USER, ctx), | ||
| integrity: gist_integrity.clone(), | ||
| }, |
Comment on lines
+402
to
412
| let notif_secrecy = private_user_label(); | ||
| let notif_integrity = none_integrity("", ctx); | ||
| for item in items.iter() { | ||
| let id = get_str_or(item, "id", "unknown"); | ||
| labeled_items.push(LabeledItem { | ||
| data: item.clone(), | ||
| labels: ResourceLabels { | ||
| description: format!("notification:{}", id), | ||
| secrecy: private_user_label(), | ||
| integrity: none_integrity("", ctx), | ||
| secrecy: notif_secrecy.clone(), | ||
| integrity: notif_integrity.clone(), | ||
| }, |
Comment on lines
428
to
439
| @@ -431,7 +435,7 @@ pub fn label_response_items( | |||
| labels: ResourceLabels { | |||
| description: format!("release:{}@{}", repo_full_name, tag), | |||
| secrecy: secrecy.clone(), | |||
| integrity: merged_integrity(&repo_full_name, ctx), | |||
| integrity: release_integrity.clone(), | |||
| }, | |||
Collaborator
|
@copilot address review feedback |
Contributor
Addressed in commits
|
lpcox
deleted the
repo-assist/fix-issue-5997-hoist-invariant-loop-allocs-280bab3dda94e923
branch
This was referenced May 19, 2026
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Repo Assist — automated AI assistant.
Summary
Eliminates redundant
Vecallocations in threelabel_response_itemsbranches by hoisting constant integrity/secrecy calls outside their loops.Problem
Three item-labeling loops were recomputing identical
Vecvalues on every iteration:list_gists/get_gistreader_integrity(scope_names::USER, ctx)× Nlist_notifications/get_notification_detailsprivate_user_label()× N andnone_integrity("", ctx)× Nlist_releases/get_latest_release/get_release_by_tagmerged_integrity(&repo_full_name, ctx)× NFor a 30-item batch (the default limit) this was ~90 redundant heap allocations per call. In WASM, every allocation is expensive since the linear memory allocator scans free lists on each call.
Fix
Hoist each invariant call before its loop; clone the pre-built
Vecper item. Also extracts aDEFAULT_BRANCH_NAMESconstant inhelpers.rs, consistent withWRITE_OPERATIONS,BLOCKED_TOOLS, and similar named arrays.The
get_file_contentsbranch already uses this pattern correctly — this change makes the rest of the code consistent.Closes #5997
Test Status
cargo check✅cargo test✅ — 411 tests passed, 0 failed