feat(performance): compare baseline reports - #87
ben-ranford wants to merge 11 commits into
Conversation
ben-ranford
added
area:rendering
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate review findings remain in report validation and compatibility handling.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an opt-in same-host performance report comparator with API, CLI, tests, documentation, and refreshed inventories.
Changes:
- Adds report validation, compatibility checks, and tolerance-based deltas.
- Adds
stave-performance-comparewith documented exit codes. - Adds deterministic tests and generated inventory updates.
File summaries
| File | Description |
|---|---|
scripts/rigor/generated/public-api.txt |
Updated public API inventory |
scripts/rigor/generated/dependency-inventory.json |
Updated dependency inventory |
README.md |
Comparator documentation link |
performance/comparison.go |
Comparison and report-validation logic |
performance/comparison_test.go |
Comparator and decoder tests |
docs/performance-baseline.md |
Usage and comparison policy documentation |
cmd/stave-performance-compare/main.go |
Comparison CLI |
cmd/stave-performance-compare/main_test.go |
CLI behavior tests |
Review details
Suppressed comments (4)
performance/comparison.go:110
measurementByNamelinearly scans the candidate list for every baseline metric, andcompatibleEnvironmentrepeats the same lookup. BecauseDecodeReportaccepts any number of metrics up to 16 MiB, this makes comparison O(n²) and allows a bounded but hostile artifact to consume excessive CPU; index candidate measurements by name once for both checks.
candidateMetric := measurementByName(candidate.Measurements, metric.Name)
performance/comparison.go:135
stave-performancewrites the sentinel"unknown"whenos.Hostname()fails, but this validation accepts it as a usable host. Reports from unrelated machines can therefore both pass the host equality check and receive same-host tolerance even though same-host provenance is unavailable. Reject unknown-host reports (or carry a verifiable host identity) before comparing them.
if report.Host == "" || report.GoVersion == "" || report.GOOS == "" || report.GOARCH == "" || report.CPUs < 1 || report.Nodes < 1 || report.NodeShape == "" || report.Renderer == "" || report.Viewport.Width < 1 || report.Viewport.Height < 1 {
performance/comparison.go:144
IdleCPU.Windowand the optionalIdleCPU.Attemptsvalues are part of the report measurement but are never validated. A direct caller can supply a negative window or NaN/infinite/negative attempts andComparewill accept the report, despite this API promising to reject invalid direct-report measurements. Validate the window and every supplied attempt before comparison.
if report.IdleCPU.Name != "idle_cpu.percent_one_core" || math.IsNaN(report.IdleCPU.Value) || math.IsInf(report.IdleCPU.Value, 0) || math.IsNaN(report.IdleCPU.Limit) || math.IsInf(report.IdleCPU.Limit, 0) || report.IdleCPU.Value < 0 || report.IdleCPU.Limit <= 0 || report.IdleCPU.Value >= report.IdleCPU.Limit || !report.IdleCPU.AllWithinBudget {
performance/comparison.go:215
- When
baseline == 0andcandidate > 0,PercentDeltaremains0even thoughDeltais positive andWithinToleranceis false. Consumers of the machine-readable envelope will interpret a regression from zero as no percentage change; represent a zero-baseline percentage as explicitly undefined rather than emitting zero.
percent := 0.0
withinTolerance := candidate == 0
if baseline > 0 {
percent = delta / baseline
withinTolerance = percent <= tolerance
- Files reviewed: 6/8 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34c13eb12f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3d4b4ebda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90d020a99d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fe13726db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f90cbe3d5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e198316b1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e198316 to
7dbfd6f
Compare
7dbfd6f to
19e6026
Compare
|
Sonar API auditReviewed commit: Live public SonarCloud API queries for PR #87 returned:
Audited via CLI at 2026-09-16T04:24:43.592620+00:00. Project visibility was verified as public. No findings were changed or suppressed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19e6026327
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| percent := 0.0 | ||
| withinTolerance := candidate == 0 | ||
| if baseline > 0 { | ||
| percent = delta / baseline |
There was a problem hiding this comment.
When a metric rises from 100 to 105, this calculation serializes percentDelta as 0.05, even though the actual percentage delta is 5%. Machine consumers displaying or otherwise interpreting the versioned field according to its name will understate every nonzero change by a factor of 100; either multiply this value by 100 for the wire field or name it as a relative fraction instead.
Useful? React with 👍 / 👎.
Closes #57
Summary
Adds the unreleased
stave-performance-comparecommand and opt-instave.performance.comparison/v1envelope. It compares saved reports without changing the existing report schema or absolute budget behavior.Comparison requires matching host, Go version, OS/architecture, CPU count, fixtures, capabilities, budget schema, and reproducibility parameters. Source revisions are preserved as comparison metadata and may differ. The executable location and report artifact destination are excluded from invocation matching because they are not measurement settings. All remaining arguments are compared.
Each p95 metric and allocation uses a 10% same-host noise tolerance; idle CPU uses 0.10 percentage points. Deltas are machine-readable. The API rejects negative, NaN, and infinite tolerance values and invalid direct-report measurements. Neither the command nor API refreshes a failed baseline.
Validation
go test ./performance ./cmd/stave-performance-compare -count=1make generated-refreshmake cipassed after rebasing onto current main.go test -race -count=1 ./performance ./cmd/stave-performance-comparepassed on the rebased commit.Release Notes
Unreleased v1.1.0 development tooling adds same-environment performance-report comparison. Existing
stave-performanceJSON output and its absolute budgets are unchanged.Scope firewall
This PR is limited to the #57 comparator library, CLI, deterministic synthetic tests, documentation, and generated inventories. It does not alter benchmark collection, release budget thresholds, dependencies, Go version, or baseline artifacts.