-
Notifications
You must be signed in to change notification settings - Fork 20
[rust-guard] Rust Guard: Deduplicate integrity label builders + lazy alloc in integrity_rank #2912
Description
Improvement 1: Extract build_integrity_labels to deduplicate four integrity-builder functions
Category: Duplication
File(s): guards/github-guard/rust-guard/src/labels/helpers.rs
Effort: Small (< 15 min)
Risk: Low
Problem
Four public functions — none_integrity (L207), reader_integrity (L833), writer_integrity (L851), and merged_integrity (L874) — share an additive, step-by-step structure where each higher-level function re-emits the full body of the lower-level ones verbatim, plus one additional label.
reader_integrity repeats the none_integrity body (2 format_integrity_label calls).
writer_integrity repeats reader_integrity's body (4 calls).
merged_integrity repeats writer_integrity's body (6 calls).
The NONE_PREFIX/READER_PREFIX/WRITER_PREFIX label-building blocks are copy-pasted a total of 9 times across these four functions.
Suggested Change
Introduce a private fn build_integrity_labels(normalized_scope: &str, level: u8) -> Vec<String> that drives all four:
Before
// labels/helpers.rs L207-L234 (none_integrity / blocked_integrity)
pub fn none_integrity(scope: &str, ctx: &PolicyContext) -> Vec<String> {
let normalized_scope = normalize_scope(scope, ctx);
vec![format_integrity_label(
label_constants::NONE_PREFIX,
&normalized_scope,
label_constants::NONE,
)]
}
// L833-L899 (reader / writer / merged — only reader shown for brevity)
pub fn reader_integrity(scope: &str, ctx: &PolicyContext) -> Vec<String> {
let normalized_scope = normalize_scope(scope, ctx);
vec![
format_integrity_label(
label_constants::NONE_PREFIX,
&normalized_scope,
label_constants::NONE,
),
format_integrity_label(
label_constants::READER_PREFIX,
&normalized_scope,
label_constants::READER_BASE,
),
]
}
// writer_integrity and merged_integrity follow the same pattern,
// each adding one more format_integrity_label call.After
/// Internal helper: build the cumulative integrity label set up to the given level.
/// level 0 = none, 1 = unapproved, 2 = approved, 3 = merged
fn build_integrity_labels(normalized_scope: &str, level: u8) -> Vec<String> {
let mut labels = Vec::with_capacity(level as usize + 1);
labels.push(format_integrity_label(
label_constants::NONE_PREFIX, normalized_scope, label_constants::NONE,
));
if level >= 1 {
labels.push(format_integrity_label(
label_constants::READER_PREFIX, normalized_scope, label_constants::READER_BASE,
));
}
if level >= 2 {
labels.push(format_integrity_label(
label_constants::WRITER_PREFIX, normalized_scope, label_constants::WRITER_BASE,
));
}
if level >= 3 {
labels.push(format_integrity_label(
label_constants::MERGED_PREFIX, normalized_scope, label_constants::MERGED_BASE,
));
}
labels
}
pub fn none_integrity(scope: &str, ctx: &PolicyContext) -> Vec<String> {
build_integrity_labels(&normalize_scope(scope, ctx), 0)
}
pub fn reader_integrity(scope: &str, ctx: &PolicyContext) -> Vec<String> {
build_integrity_labels(&normalize_scope(scope, ctx), 1)
}
pub fn writer_integrity(scope: &str, ctx: &PolicyContext) -> Vec<String> {
build_integrity_labels(&normalize_scope(scope, ctx), 2)
}
pub fn merged_integrity(scope: &str, ctx: &PolicyContext) -> Vec<String> {
build_integrity_labels(&normalize_scope(scope, ctx), 3)
}Why This Matters
- Eliminates ~30 lines of copy-pasted
format_integrity_labelcalls - Adding a new integrity tier in the future (e.g. "verified") requires changing exactly one place
- The
Vec::with_capacitypre-allocation avoids a realloc that the currentvec![...]macro may cause
Improvement 2: Lazy label allocation in integrity_rank
Category: Performance (WASM-specific)
File(s): guards/github-guard/rust-guard/src/labels/helpers.rs
Effort: Small (< 15 min)
Risk: Low
Problem
integrity_rank (L900–L935) allocates all four label strings upfront before it checks any of them:
fn integrity_rank(scope: &str, labels: &[String], ctx: &PolicyContext) -> u8 {
let normalized_scope = normalize_scope(scope, ctx);
let merged = format_integrity_label(...MERGED...); // always allocated
let writer = format_integrity_label(...WRITER...); // always allocated
let reader = format_integrity_label(...READER...); // always allocated
let none = format_integrity_label(...NONE...); // always allocated
if labels.iter().any(|l| l == &merged) { 4 }
else if labels.iter().any(|l| l == &writer) { 3 }
// ...
}When the first check (merged) matches, writer, reader, and none strings were already heap-allocated and must be dropped — wasted work. In a WASM environment with a constrained allocator, this matters on any hot call path.
Suggested Change
Build each label string only when needed (highest to lowest), returning as soon as a match is found:
Before
fn integrity_rank(scope: &str, labels: &[String], ctx: &PolicyContext) -> u8 {
let normalized_scope = normalize_scope(scope, ctx);
let merged = format_integrity_label(
label_constants::MERGED_PREFIX, &normalized_scope, label_constants::MERGED_BASE,
);
let writer = format_integrity_label(
label_constants::WRITER_PREFIX, &normalized_scope, label_constants::WRITER_BASE,
);
let reader = format_integrity_label(
label_constants::READER_PREFIX, &normalized_scope, label_constants::READER_BASE,
);
let none = format_integrity_label(
label_constants::NONE_PREFIX, &normalized_scope, label_constants::NONE,
);
if labels.iter().any(|l| l == &merged) { 4 }
else if labels.iter().any(|l| l == &writer) { 3 }
else if labels.iter().any(|l| l == &reader) { 2 }
else if labels.iter().any(|l| l == &none) { 1 }
else { 0 }
}After
fn integrity_rank(scope: &str, labels: &[String], ctx: &PolicyContext) -> u8 {
let normalized_scope = normalize_scope(scope, ctx);
// Build and check labels lazily from highest to lowest, short-circuiting
// as soon as a match is found to avoid unnecessary heap allocations.
let merged = format_integrity_label(
label_constants::MERGED_PREFIX, &normalized_scope, label_constants::MERGED_BASE,
);
if labels.iter().any(|l| l == &merged) { return 4; }
let writer = format_integrity_label(
label_constants::WRITER_PREFIX, &normalized_scope, label_constants::WRITER_BASE,
);
if labels.iter().any(|l| l == &writer) { return 3; }
let reader = format_integrity_label(
label_constants::READER_PREFIX, &normalized_scope, label_constants::READER_BASE,
);
if labels.iter().any(|l| l == &reader) { return 2; }
let none = format_integrity_label(
label_constants::NONE_PREFIX, &normalized_scope, label_constants::NONE,
);
if labels.iter().any(|l| l == &none) { return 1; }
0
}Why This Matters
- In the common case (
mergedorwriterrank), saves 2–3Stringheap allocations per call - Pure refactor: identical observable behavior, passes all existing tests unchanged
- WASM's linear memory allocator benefits most from fewer short-lived small string allocations
- Aligns with the existing top-down priority logic already in the function
Codebase Health Summary
- Total Rust files: 9 (permissions.rs removed per prior suggestion)
- Total lines: 10,956
- Areas analyzed:
helpers.rs,constants.rs,tools.rs,tool_rules.rs,response_paths.rs,backend.rs,mod.rs,lib.rs,response_items.rs - Areas with no further improvements: none yet
Generated by Rust Guard Improver • Run: §23790420870
Generated by Rust Guard Improver · ◷
- expires on Apr 7, 2026, 9:36 AM UTC