Skip to content

fix(verify): reject stale performance reports on reruns - #2930

Merged
richiemcilroy merged 5 commits into
CapSoftware:mainfrom
AksharP5:fix/verify-stale-perf-reports
Oct 23, 2026
Merged

richiemcilroy merged 5 commits into
CapSoftware:mainfrom
AksharP5:fix/verify-stale-perf-reports

Conversation

@AksharP5

@AksharP5 AksharP5 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

A local performance comparison rerun can pass using case reports left by an earlier run, even when the new binaries report no cases. It also overwrites the earlier logs.

Each invocation now requires a fresh report directory before opening its log or launching a child. The --out help explains this requirement. Existing empty parent output directories still work. The regression tests run in the existing performance workflow's scope job.

Closes #2929

Same CLI fixture scenario Before After
First comparison in a fresh output path Exit 0 Exit 0
Reuse output path, emit no new cases Exit 0 using stale rows Exit 1 before child launch
Earlier test.log Overwritten Preserved

Original verification on baseline 848d18fe7 with the actual Python CLI and subprocess fixtures implementing the native report contract. Three focused tests pass; the stale-report regression fails on baseline. The 31 adjacent report-diff tests, Python syntax/indentation checks, workflow YAML parsing and whitespace checks pass. The exact new CI test command also passes locally.

No native media binaries were built or benchmarked. This corrects result isolation and makes no performance claim.

Branch refresh on 23 October 2026: merged main a148d610e0af9725e4eba47e653ad4972d426bb3 in 379691dfd9312d8d46f5c9653a412fecfd47b9db, including #2785's crash-verification fix. The PR diff is unchanged.

The three report-isolation tests and all 31 adjacent report-diff tests pass after the merge. Python syntax/indentation, workflow YAML/test wiring, and whitespace checks also pass. No native media build or benchmark was run locally.

On this head, both previously failing static-screen crash cases now pass. The macOS engine job reports a separate crash/recorder-kill/oop/random0 failure: six handed frames are missing before the last complete fragment ends. Its automatic retry passes, but CI deliberately keeps intermittent failures red. The real-capture job also reports screen/microphone sync of -11 ms against a -10 ms lower bound. The VideoToolbox job also reports an unexpected pass for export/instant-gated-system-continuous-mic-late/output/mp4/native: measured microphone/system sync is +1.73 ms, while the case is still marked as an expected sync failure. This single run does not establish that the expectation can safely be removed. The final Engine verify and Verify recording failures summarize these platform results. These native paths are unchanged from the merged main revision. A targeted rerun request was denied because the GitHub integration lacks Actions write permission; these checks remain unresolved.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; the test rename and workflow command agree.

Summary

The PR rejects existing report directories before opening logs or starting a child, preventing stale reports from passing a new comparison.

  • Performance runs now refuse to reuse a report directory.

Reviews (2) · Last reviewed commit: "test(verify): use kebab-case for the per..." · Reviewed by Greptile

@vercel

vercel Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

@AksharP5 is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

Comment thread scripts/verify/test-perf-ab.py
@AksharP5

Copy link
Copy Markdown
Contributor Author

@greptileai review

@richiemcilroy
richiemcilroy merged commit 5f216e4 into CapSoftware:main Oct 23, 2026
38 of 40 checks passed
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.

Performance comparisons can pass using stale reports on reruns

2 participants