[repository-quality] 🎯 Repository Quality Improvement Report - Performance (2026-09-23)

[github-actions[bot]]

🎯 Repository Quality Improvement Report - Performance

Analysis Date: 2026-09-23
Focus Area: Performance
Strategy Type: Standard
Custom Area: No

Executive Summary

gh aw compile (invoked via make recompile on every workflow-touching push/PR through cgo.yml) is a fully sequential, single-goroutine pipeline: it iterates config.MarkdownFiles in a plain for loop in compileSpecificFiles (pkg/cli/compile_pipeline.go) and calls compileWorkflowFile once per file against one shared *workflow.Compiler. Locally, compiling all 299 top-level workflow .md files (431 including shared imports) takes ~10.6–11.9s wall time regardless of GOMAXPROCS (16.3s at GOMAXPROCS=1 vs 10.7s at GOMAXPROCS=4, i.e. ~35% headroom left entirely unused), confirming the hot path never fans out across the machine's 4 available cores. The Compiler struct's own doc comment on ownerTypeCache (line 72 of compiler_types.go) explicitly states the struct is "not goroutine-safe (Compiler is used sequentially)", so parallelizing today would require either per-file compiler cloning or targeted mutex/sync-map guards around the handful of shared caches (actionCache, actionResolver, importCache, ownerTypeCache, actionPinWarnings, permissionWarningShown, allowedDomainsCache, featureUsage).

At the micro-benchmark level, BenchmarkCompileWorkflow in pkg/workflow/compiler_benchmark_test.go reuses a single *Compiler across b.Loop() iterations without resetting caches, so its first iteration is a ~5–8x outlier (83ms/497K allocs at -benchtime=1x vs 16.8ms/105K allocs at -benchtime=5x) purely from cold-cache effects rather than genuine per-call cost — this understates true steady-state variance and could mask real regressions in the benchstat-gated CI performance job (cgo.yml bench job, main-branch only, 10% regression threshold). Given the project already invests in make bench + benchstat CI regression gating and a growing (431-file) workflow corpus compiled on every relevant push, parallelizing the compile-all loop is a concrete, low-risk win: even a conservative 3x speedup (using ~3 of 4 available cores, leaving headroom for actionlint/zizmor/yamllint post-processing) would cut make recompile from ~11s to under 4s, and CI compile-validation steps proportionally, at zero cost to compile correctness once cache access is synchronized.

Full Analysis Report

Focus Area: Performance

Current State Assessment

The compile pipeline (gh aw compile, backing make recompile) is the single most frequently re-run expensive operation in this repository — it fires on essentially every PR that touches .github/workflows/*.md via cgo.yml. It was measured as fully single-threaded:

• compileSpecificFiles (pkg/cli/compile_pipeline.go:63) drives a plain for _, markdownFile := range config.MarkdownFiles loop with no goroutines, sync.WaitGroup, or errgroup.Group — unlike other CLI subsystems (pkg/cli/audit_analysis_fanout.go, pkg/cli/shellcheck.go, pkg/cli/logs_multi.go, pkg/cli/forecast_compute.go) that already use errgroup/worker-pool patterns for fan-out work.
• GOMAXPROCS=1 vs GOMAXPROCS=4 full-repo compile --no-emit runs measured 16.29s vs 10.68s — the only speedup came from background GC and I/O scheduling, not from the compile loop itself using multiple cores.
• The Compiler struct (pkg/workflow/compiler_types.go:18-90) is explicitly documented as sequential-only via the ownerTypeCache field comment, confirming this is a known, intentional (if unaddressed) design constraint rather than an oversight.
• BenchmarkCompileWorkflow reuses one *Compiler instance across the whole testing.B loop with no per-iteration reset, so allocation/timing numbers reported to benchstat mix a cold-start outlier with N-1 warm-cache runs, understating variance the regression gate is meant to catch.

Metrics Collected:

Metric	Value	Status
Full compile --no-emit wall time (299 top-level workflows)	~10.6–11.9s	⚠️
GOMAXPROCS=1 vs GOMAXPROCS=4 wall-time delta	16.29s → 10.68s (34% from OS scheduling only, 0% from app parallelism)	❌
Compile-loop goroutine/errgroup usage	None (compileSpecificFiles, pkg/cli/compile_pipeline.go)	❌
Total workflow .md files (incl. shared imports)	431 (299 top-level + 137 shared)	⚠️
--parallel/--workers/-j CLI flag on compile	Absent	❌
Existing fan-out patterns elsewhere in pkg/cli (errgroup/WaitGroup)	10+ files (audit_analysis_fanout.go, shellcheck.go, logs_multi.go, forecast_compute.go, drain3_train.go, audit_cache.go, logs_orchestrator_render.go)	✅ (proves the pattern is idiomatic here)
BenchmarkCompileWorkflow iteration variance (-benchtime=1x vs -benchtime=5x)	83.2ms/497K allocs vs 16.8ms/105K allocs per op	⚠️
CI benchmark regression gate (cgo.yml bench job)	benchstat, 10% threshold, main-branch only	✅

Findings

Strengths

• The repo already has mature parallel-execution idioms (errgroup.Group, bounded sync.WaitGroup + semaphore channels) used consistently in pkg/cli for I/O-bound fan-out (audit analyses, log processing, shellcheck), so introducing a bounded worker pool for compilation follows an established, reviewable pattern rather than a novel one.
• CI already has a dedicated bench job with benchstat-based automatic regression detection (>10% fails the build), meaning any performance work here has an existing safety net to prove it doesn't regress hot paths.
• The compiler's own field-level documentation (ownerTypeCache comment) is refreshingly honest about the sequential-only constraint, making the exact synchronization boundary self-documenting for whoever picks this up.

Areas for Improvement

• ⚠️ High: compileSpecificFiles's per-file loop has zero parallelism despite 4 idle cores being available in the benchmark environment; this directly inflates the wall-clock cost of every make recompile invocation and any CI job gating on it (cgo.yml).
• ⚠️ Medium: Seven-plus mutable maps/caches on *Compiler (ownerTypeCache, actionPinWarnings, permissionWarningShown, allowedDomainsCache, featureUsage, copilotRequestsTipShown, scheduleWarnings) are read/written without any synchronization primitive, meaning naive parallelization would introduce data races detectable by go test -race — this needs to be solved deliberately, not accidentally.
• ⚠️ Medium: BenchmarkCompileWorkflow (and likely its siblings in compiler_performance_benchmark_test.go) reuse one *Compiler for all iterations, producing benchmark numbers dominated by first-iteration cold-cache cost; this weakens the trustworthiness of the benchstat 10%-regression CI gate for exactly the metric it's supposed to protect.
• ⚪ Low: No --parallel/-j flag exists on gh aw compile, so even users who want to opt into concurrent compilation locally have no lever to pull.

Detailed Analysis

Root cause: pkg/cli/compile_pipeline.go's compileSpecificFiles function was written for correctness and clear sequential error-aggregation (via stats *CompilationStats, *validationResults, and multiple lockFilesFor* slices), and the *workflow.Compiler it drives accumulates cross-file state intentionally (shared action-pin cache, shared import cache) to avoid redundant network/parsing work across the batch. This shared-state design is exactly why it was never parallelized — but the shared caches are also exactly what should let per-file compilation be embarrassingly parallel once access is either (a) mutex-guarded, (b) migrated to sync.Map, or (c) pre-warmed once up front and then treated as read-only during a parallel phase.

Concrete remediation path: Introduce a bounded worker pool (e.g., errgroup.Group with SetLimit(runtime.GOMAXPROCS(0)), mirroring the exact pattern already used in pkg/cli/audit_analysis_fanout.go) around the body of the for _, markdownFile := range config.MarkdownFiles loop in compileSpecificFiles. Aggregation slices (lockFilesForActionlint, lockFilesForZizmor, etc.) and stats/validationResults writes need a mutex or per-worker-then-merge pattern. The seven identified *Compiler maps need equivalent guarding (or migration to sync.Map/mutex-wrapped accessors) before this is safe under -race.

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items.

Improvement Tasks

Task 1: Parallelize the full-workflow compile loop with a bounded worker pool

Priority: High
Estimated Effort: Large
Focus Area: Performance

Description: Replace the sequential for _, markdownFile := range config.MarkdownFiles loop in compileSpecificFiles (pkg/cli/compile_pipeline.go) with a bounded errgroup.Group (using SetLimit(runtime.GOMAXPROCS(0))), following the existing fan-out pattern in pkg/cli/audit_analysis_fanout.go. Each per-file compileWorkflowFile call currently mutates shared stats *CompilationStats, *validationResults []ValidationResult, and multiple lockFilesFor* slices directly; these must be collected per-worker (or guarded with a sync.Mutex) and merged deterministically after the group completes so output ordering (summary printing, JSON output) remains stable and reproducible across runs.

Acceptance Criteria:

• compileSpecificFiles compiles files using a bounded worker pool instead of a single goroutine
• Compilation output (summary counts, JSON output, validation results ordering) is byte-for-byte or semantically identical to the sequential version for a fixed input set (add/adjust a test asserting this)
• go test -race ./pkg/cli/... passes with the new parallel path exercised
• make recompile wall time on the full 299-workflow corpus is measured before/after and shows measurable improvement (document in PR description)

Code Region: pkg/cli/compile_pipeline.go (compileSpecificFiles, lines ~63-160+)

In pkg/cli/compile_pipeline.go, the compileSpecificFiles function compiles every workflow file sequentially in a single for loop calling compileWorkflowFile once per file against a shared *workflow.Compiler. Parallelize this loop using golang.org/x/sync/errgroup with a bounded concurrency limit (errgroup.Group.SetLimit(runtime.GOMAXPROCS(0))), following the existing pattern already used in pkg/cli/audit_analysis_fanout.go for structuring bounded fan-out work in this codebase.

Requirements:
- Preserve the existing sequential semantics: stats *CompilationStats, *validationResults []ValidationResult, and the lockFilesForActionlint/lockFilesForZizmor/lockFilesForDirTools/lockFilesForSyft/lockFilesForGrype/lockFilesForGrant/lockFilesForYamllint/lockFilesForShellcheck/compiledLockFiles slices must end up with the same aggregate contents as today, but per-worker writes need proper synchronization (a mutex around the shared aggregation state is acceptable; per-goroutine local slices merged after errgroup.Wait() is preferred if it keeps code simple).
- Respect existing ctx.Done() cancellation behavior — if the context is cancelled mid-flight, in-flight file compilations should still be allowed to finish but no new ones should start, and the function should still return the correct error/result set.
- The shared *workflow.Compiler instance already documents (see pkg/workflow/compiler_types.go, ownerTypeCache field comment) that it is "not goroutine-safe (Compiler is used sequentially)". Before enabling the parallel loop, either (a) confirm and fix all the mutable un-guarded Compiler fields identified as unsafe for concurrent access (ownerTypeCache, actionPinWarnings, permissionWarningShown, allowedDomainsCache, featureUsage, copilotRequestsTipShown, scheduleWarnings, warningCount) with mutexes or sync.Map, or (b) determine that a subset of these fields can be pre-populated/read-only during the parallel phase and document that invariant clearly in code comments. Update the ownerTypeCache field comment once it is made goroutine-safe.
- Do not change compileWorkflowFile's own signature or the resolveWorkflowFile / trackWorkflowFailure helper call sites unless strictly necessary for thread-safety.
- Add or update tests under pkg/cli to verify: (1) parallel and sequential compilation produce identical ValidationResult sets and CompilationStats for a fixed test fixture set of workflow files, and (2) go test -race ./pkg/cli/... passes cleanly with the new code path exercised (e.g. compiling 5+ files in one invocation).
- Run `make fmt` after changes. Run the smallest relevant test target covering pkg/cli compile-related tests before finishing; do not run the full test suite unless required.

---

Task 2: Add mutex/sync.Map guards to shared Compiler caches to make parallel compilation safe

Priority: High
Estimated Effort: Medium
Focus Area: Performance

Description: The *workflow.Compiler struct (pkg/workflow/compiler_types.go:18-90) has at least seven mutable maps/slices (ownerTypeCache, actionPinWarnings, permissionWarningShown, allowedDomainsCache, featureUsage, copilotRequestsTipShown, scheduleWarnings) accessed without synchronization, explicitly documented as safe only because "Compiler is used sequentially." This task is a prerequisite/companion to Task 1: add appropriate mutex-guarded accessor methods (or migrate to sync.Map where iteration order doesn't matter) for each of these fields so the compiler can be safely shared across goroutines during parallel compilation.

Acceptance Criteria:

• Each of the seven identified fields has synchronized read/write access (mutex-guarded methods or sync.Map)
• go test -race ./pkg/workflow/... passes
• Field-level doc comments are updated to remove/replace the now-inaccurate "not goroutine-safe (Compiler is used sequentially)" note
• No behavioral change to single-threaded callers (existing sequential compile paths still work identically)

Code Region: pkg/workflow/compiler_types.go (lines 18-90, Compiler struct)

In pkg/workflow/compiler_types.go, the Compiler struct has several mutable map/slice fields that are currently accessed without any synchronization primitive and are explicitly documented as unsafe for concurrent use: ownerTypeCache (line ~72, "not goroutine-safe (Compiler is used sequentially)"), actionPinWarnings, permissionWarningShown, allowedDomainsCache, featureUsage, copilotRequestsTipShown, and scheduleWarnings.

Add a sync.Mutex (or per-field sync.RWMutex if read-heavy access patterns benefit from it) to the Compiler struct, and wrap all existing direct field accesses to these maps/slices throughout pkg/workflow/*.go with small accessor methods, e.g. (c *Compiler) getOwnerType(login string) (string, bool) and (c *Compiler) setOwnerType(login, ownerType string), following existing Go idioms in this codebase. Search for all read/write sites of each field with grep before refactoring, since some of these fields are touched from multiple files across pkg/workflow.

Do not use sync.Map as a blanket replacement unless a field's access pattern is write-once-read-many across goroutines; prefer a shared mutex guarding several small maps together if that's simpler and doesn't create a measurable contention bottleneck (this compiler work is not on an ultra-hot per-nanosecond path, so simplicity should be favored over lock-free tricks).

Update the misleading field comment on ownerTypeCache (and any similarly worded comments on the other fields) once they are made safe for concurrent access.

Add or extend tests in pkg/workflow to verify concurrent access to these fields via go test -race, simulating what the parallel compile loop from a companion task will do (e.g., spin up N goroutines calling the same Compiler's relevant methods concurrently).

Run `make fmt` after changes and run the smallest relevant pkg/workflow test target before finishing.

---

Task 3: Fix cold-cache bias in BenchmarkCompileWorkflow that undermines the CI benchstat regression gate

Priority: Medium
Estimated Effort: Small
Focus Area: Performance

Description: BenchmarkCompileWorkflow in pkg/workflow/compiler_benchmark_test.go (lines 12-52) creates a single *Compiler via NewCompiler() outside the b.Loop() body and reuses it across all iterations without resetting per-run caches. This causes the first iteration to be a cold-cache outlier (measured 83.2ms/497,214 allocs at -benchtime=1x) versus subsequent warm-cache iterations (measured 16.8ms/105,311 allocs at -benchtime=5x), which distorts the ns/op and allocs/op numbers that cgo.yml's bench job feeds into benchstat for its 10%-regression CI gate. Either reset the compiler's mutable caches each iteration (to measure true steady-state per-call cost consistently) or explicitly document why cache-reuse across iterations is the intended behavior, and consider splitting into a BenchmarkCompileWorkflow_ColdCache (fresh NewCompiler() per iteration) and BenchmarkCompileWorkflow_WarmCache (current behavior) so both cost profiles are tracked separately and comparably by benchstat across commits.

Acceptance Criteria:

• BenchmarkCompileWorkflow's iteration-to-iteration variance is either eliminated (fresh compiler per iteration) or split into explicitly named cold/warm variants
• Benchmark still passes go test -bench=BenchmarkCompileWorkflow -benchmem -run=^$ ./pkg/workflow/
• make bench output remains parseable by the existing benchstat step in cgo.yml
• A short code comment explains the cache-reuse (or non-reuse) decision so future contributors don't reintroduce the ambiguity

Code Region: pkg/workflow/compiler_benchmark_test.go (lines 12-52, BenchmarkCompileWorkflow)

In pkg/workflow/compiler_benchmark_test.go, BenchmarkCompileWorkflow (lines 12-52) creates one *workflow.Compiler via NewCompiler() before the b.Loop() loop and reuses it for every iteration. Because the compiler accumulates caches across calls to CompileWorkflow (action-pin cache, import cache, and other per-instance state), the first loop iteration is measurably more expensive (allocation-heavy) than subsequent warm-cache iterations, which skews the ns/op and B/op numbers that feed into the benchstat-based 10%-regression CI gate in .github/workflows/cgo.yml's bench job.

Fix this by doing one of the following (pick whichever keeps the benchmark's intent clearest):
1. Move compiler := NewCompiler() inside the b.Loop() body so every iteration measures a true cold-cache single compile (most representative of a one-off `gh aw compile <file>` invocation), or
2. Keep the current warm-cache reuse behavior but rename the benchmark to BenchmarkCompileWorkflow_WarmCache and add a new BenchmarkCompileWorkflow_ColdCache benchmark that constructs a fresh compiler per iteration, so both profiles are tracked distinctly and comparably across commits by benchstat.

Whichever approach you choose, add a short comment above the benchmark explaining the design decision so it isn't silently reintroduced as ambiguous behavior later. Check the other similarly-structured benchmarks in this file (BenchmarkCompileWorkflow_WithMCP, BenchmarkCompileWorkflow_WithImports, BenchmarkCompileWorkflow_WithValidate, BenchmarkCompileWorkflow_Complex) for the same pattern and apply the same fix consistently if they share the issue.

Verify with: go test -bench=BenchmarkCompileWorkflow -benchmem -benchtime=5x -run=^$ ./pkg/workflow/
Do not modify unrelated benchmarks in this file. Run `make fmt` after changes.

---

Task 4: Add a --parallel/-j flag to gh aw compile for explicit concurrency control

Priority: Low
Estimated Effort: Small
Focus Area: Performance

Description: Once Tasks 1 and 2 land, expose an opt-in/opt-out lever for the new parallel compile behavior via a --parallel <N> (or -j <N>) flag on gh aw compile, defaulting to runtime.GOMAXPROCS(0) when unset and to 1 (fully sequential) when explicitly set to 1, matching common CLI tool conventions (e.g. make -j, go build -p). This gives users and CI a way to control resource usage in constrained environments (e.g. small CI runners) without hardcoding worker-pool size in the codebase.

Acceptance Criteria:

• gh aw compile --help documents the new flag with a clear description and default value
• gh aw compile --parallel 1 produces identical output to today's sequential behavior
• Flag value is threaded through CompileConfig (pkg/cli/compile_orchestrator.go) into the worker-pool limit added in Task 1
• Existing gh aw compile invocations without the flag continue to work unchanged (default behavior should not silently break existing scripts/CI expecting current output ordering/timing assumptions)

Code Region: pkg/cli/compile_compiler_setup.go (flag registration), pkg/cli/compile_orchestrator.go (CompileConfig, CompileWorkflows)

Add a --parallel (int, default 0 meaning "use runtime.GOMAXPROCS(0)") flag to the gh aw compile command, registered in pkg/cli/compile_compiler_setup.go alongside the other existing compile flags (look for where flags like --validate, --strict, --no-emit are registered as the pattern to follow). Thread the flag value through CompileConfig in pkg/cli/compile_orchestrator.go so it reaches the worker-pool concurrency limit introduced in compileSpecificFiles (pkg/cli/compile_pipeline.go) by a companion parallelization task.

Requirements:
- Flag help text must clearly state the default (all available CPUs) and that --parallel 1 restores fully sequential compilation.
- Passing --parallel 1 must produce byte-identical CompilationStats/ValidationResults ordering to the pre-parallelization sequential behavior (add or extend a test asserting this).
- Do not implement this task before the underlying worker-pool parallelization (compileSpecificFiles) and Compiler thread-safety work exist; this flag is purely a control surface over that mechanism.
- Update any relevant CLI help documentation under docs/ that lists gh aw compile flags (check docs/src for a generated or hand-written compile command reference page) if one exists.
- Run `make fmt` after changes and the smallest relevant pkg/cli test target before finishing.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-09-15	WASM Build-Variant Documentation & Stub-Coverage Debt	Custom	Y	81% undocumented exported declarations in 27 WASM stub files
2026-09-16	Skill Documentation Drift (Stale JS Location References)	Custom	Y	20 stale pkg/workflow/js/ references across 4 skills
2026-09-17	Release Documentation & Tooling Debt	Custom	Y	CHANGELOG.md frozen since v0.40.1 despite 459 releases
2026-09-18	Unjustified nolint Suppression Debt	Custom	Y	96/260 (37%) nolint directives lack justification comments
2026-09-21	Custom Go Linter CI Enablement Gap (2nd Follow-up)	Custom	Y	13/72 registered linters still unwired in CI
2026-09-22	Integration Test Binary-Path Duplication & Brittleness	Custom	Y	32 duplicated hardcoded binary-discovery blocks in test files
2026-09-23	Performance (this report)	Standard	N	Compile pipeline fully sequential despite 4 idle cores; benchmark cache-reuse bias identified

---

🎯 Recommendations

Immediate Actions (This Week)

1. Land Task 2 (Compiler thread-safety guards) — Priority: High, unblocks all further compile-performance work
2. Land Task 1 (parallelize compile loop) — Priority: High, immediate wall-clock win for every make recompile/CI compile run

Short-term Actions (This Month)

1. Land Task 3 (fix benchmark cache-reuse bias) — Priority: Medium, restores trust in the existing benchstat CI regression gate
2. Measure actual CI wall-time savings from Task 1 in cgo.yml and report the delta in a follow-up run

Long-term Actions (This Quarter)

1. Land Task 4 (--parallel flag) — Priority: Low, gives operators explicit control once the underlying mechanism is proven stable
2. Consider extending the same worker-pool pattern to other sequential per-file loops discovered during this work (e.g. pkg/cli/compile_pipeline.go lines 343 and 809) if they show similar single-threaded bottlenecks

---

📈 Success Metrics

• make recompile wall time (299 workflows): ~10.6–11.9s → target <5s (≥2x speedup via bounded parallelism)
• go test -race ./pkg/workflow/... ./pkg/cli/... pass rate on parallel compile path: N/A (not yet implemented) → 100% clean
• Benchmark iteration variance for BenchmarkCompileWorkflow: ~5x (cold vs warm) → documented/eliminated via explicit cold/warm split or per-iteration reset

---

Next Steps

1. Review and prioritise the tasks above
2. Assign tasks to Copilot coding agent via planner agent
3. Track progress on improvement items
4. Re-evaluate this focus area in 4-6 weeks after Tasks 1-3 land, to measure actual CI wall-time impact

---

Generated by Repository Quality Improvement Agent
Next analysis: 2026-09-24 — Focus area selected by diversity algorithm

Warning

Firewall blocked 4 domains

The following domains were blocked by the firewall during workflow execution:

• o205451.ingest.us.sentry.io
• proxy.golang.org
• storage.googleapis.com
• sum.golang.org

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

network:
  allowed:
    - defaults
    - "o205451.ingest.us.sentry.io"
    - "proxy.golang.org"
    - "storage.googleapis.com"
    - "sum.golang.org"

See Network Configuration for more information.

Generated by ⚡ Repository Quality Improvement Agent · copilot · auto · 98.4 AIC · ⌖ 7.71 AIC · ⊞ 9.8K · ◷

• expires on Sep 24, 2026, 5:09 AM UTC-08:00

[github-actions[bot]]

This discussion has been marked as outdated by Repository Quality Improvement Agent.

A newer discussion is available at Discussion #63184.