Skip to content

rust-guard: use Option<&'static str> for items_path and Cow<'_, str> for baseline_scope#5606

Merged
lpcox merged 3 commits into
mainfrom
copilot/rust-guard-change-items-path-type
May 13, 2026
Merged

rust-guard: use Option<&'static str> for items_path and Cow<'_, str> for baseline_scope#5606
lpcox merged 3 commits into
mainfrom
copilot/rust-guard-change-items-path-type

Conversation

Copilot AI commented May 13, 2026

Copy link
Copy Markdown
Contributor

✨ Enhancement

items_path and baseline_scope in the hot-path labeling code were declared as String/Option<String> despite always holding either &'static str constants or short-lived borrows, causing unnecessary heap allocations on every labeling call — especially costly in WASM.

What does this improve?

  • items_path (response_paths.rs, lib.rs): PathLabelResult.items_path and PathLabeledOutput.items_path changed from Option<String> to Option<&'static str>. All values originate from extract_items_array (returns &'static str) or static literals. Eliminates 3 allocations per collection labeling call (list_pull_requests, list_issues, list_project_items).
  • baseline_scope (tool_rules.rs): Changed from String to Cow<'_, str>. The 8 assignments from scope_names::USER/scope_names::GITHUB constants become Cow::Borrowed (zero-cost); the 3 assignments from runtime-owned strings (owner, s_repo_id) use Cow::Owned. Call site (ensure_integrity_baseline(&baseline_scope, ...)) is unchanged. Eliminates up to 8 allocations per apply_tool_labels invocation on the most common tool paths.

Implementation approach:

// Before
pub items_path: Option<String>,
// ...
items_path: if items_path.is_empty() { None } else { Some(items_path.to_string()) }

// After
pub items_path: Option<&'static str>,
// ...
items_path: if items_path.is_empty() { None } else { Some(items_path) }
// Before
let mut baseline_scope = repo_id.to_string();
baseline_scope = scope_names::GITHUB.to_string(); // ×8

// After
let mut baseline_scope: Cow<'_, str> = Cow::Borrowed(repo_id);
baseline_scope = Cow::Borrowed(scope_names::GITHUB); // zero-cost
baseline_scope = Cow::Owned(owner.clone());          // only where an owned value is genuinely needed

One additional fix in response_paths.rs: the search_repositories branch was using format!("/{}", items_key) to construct the path. Since items_key is always "items" or "repositories", this is replaced with a static match to stay consistent with the &'static str type.

- Change PathLabelResult.items_path from Option<String> to Option<&'static str>
  in response_paths.rs, eliminating 3 heap allocations per collection labeling call
- Update PathLabeledOutput.items_path in lib.rs to match
- Fix search_repositories branch (line 105) to use static match instead of format!
- Update test assertions in mod.rs to use &'static str literals
- Change baseline_scope in apply_tool_labels (tool_rules.rs) from String to
  Cow<'_, str>: static constant assignments use Cow::Borrowed (zero-cost),
  owned assignments (owner, s_repo_id) use Cow::Owned, eliminating up to 8
  heap allocations per apply_tool_labels invocation
Copilot AI changed the title [WIP] Change PathLabelResult.items_path to Option<&'static str> and use Cow for baseline_scope rust-guard: use Option<&'static str> for items_path and Cow<'_, str> for baseline_scope May 13, 2026
Copilot finished work on behalf of lpcox May 13, 2026 14:00
Copilot AI requested a review from lpcox May 13, 2026 14:00
@lpcox lpcox marked this pull request as ready for review May 13, 2026 14:23
Copilot AI review requested due to automatic review settings May 13, 2026 14:23

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 reduces per-call heap allocations in the Rust GitHub guard’s hot-path labeling by switching items_path to a static string reference and baseline_scope to a borrow-friendly Cow<str>, improving WASM performance.

Changes:

  • Change PathLabelResult.items_path / PathLabeledOutput.items_path from Option<String> to Option<&'static str> and adjust path construction accordingly.
  • Change baseline_scope in apply_tool_labels from String to Cow<'_, str> to avoid repeated .to_string() allocations for constant scopes.
  • Update unit tests to match the new items_path type and values.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/lib.rs Updates the serialized path-label output struct to carry items_path as Option<&'static str>.
guards/github-guard/rust-guard/src/labels/tool_rules.rs Switches baseline_scope to Cow<str> to reduce allocations when scoping integrity baselines.
guards/github-guard/rust-guard/src/labels/response_paths.rs Converts computed items_path values to &'static str and replaces a format! with a static match for search repos.
guards/github-guard/rust-guard/src/labels/mod.rs Updates tests to assert items_path as Option<&'static str> (no .to_string()).

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

guards/github-guard/rust-guard/src/labels/tool_rules.rs:600

  • baseline_scope = Cow::Owned(owner.clone()) still clones the owner string. Since owner outlives baseline_scope for the rest of apply_tool_labels, this can be Cow::Borrowed(owner.as_str()) to avoid the extra allocation on this hot path.
            // Projects are org-scoped; write responses carry the same labels as reads.
            // I = approved:<owner>
            if !owner.is_empty() {
                baseline_scope = Cow::Owned(owner.clone());
                integrity = writer_integrity(&baseline_scope, ctx);
  • Files reviewed: 4/4 changed files
  • Comments generated: 2

Comment on lines 406 to 409
let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo);
if !s_repo_id.is_empty() {
baseline_scope = s_repo_id.clone();
baseline_scope = Cow::Owned(s_repo_id.clone());
desc = format!("search_code:{}", s_repo_id);
// label_response_paths for list_project_items
if !owner.is_empty() {
baseline_scope = owner.clone();
baseline_scope = Cow::Owned(owner.clone());
@lpcox

lpcox commented May 13, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

- search_code branch: reorder assignments so s_repo_id is moved into
  Cow::Owned after its last use, avoiding an extra .clone()
- projects read/write branches: change Cow::Owned(owner.clone()) to
  Cow::Borrowed(owner.as_str()) since owner lives for the full duration
  of apply_tool_labels — zero allocation on these paths

Copilot AI commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Done in 0f1e9bd:

  • search_code branch: reordered the assignments so s_repo_id is used for desc, secrecy, and integrity first, then moved into Cow::Owned(s_repo_id) at the end — no clone needed.
  • list_projects/projects_write branches: changed Cow::Owned(owner.clone()) to Cow::Borrowed(owner.as_str()) since owner is a local String that outlives baseline_scope — zero allocation on both paths.

Copilot finished work on behalf of lpcox May 13, 2026 14:44
@lpcox lpcox merged commit 3df9663 into main May 13, 2026
16 checks passed
@lpcox lpcox deleted the copilot/rust-guard-change-items-path-type branch May 13, 2026 15:21
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: Change PathLabelResult.items_path to Option<&'static str> and use Cow for baseline_scope

3 participants