ci(bench): make embedded README benchmark table optional in the freshness gate#6739
Merged
proggeramlug merged 1 commit intoJul 21, 2026
Conversation
…reshness gate PerryTS#6736 rewrote README.md as a concise marketing landing page with a different, hand-curated performance section and intentionally dropped the auto-generated `<!-- public-node-bun:start/end -->` block. The lint job's "Public benchmark evidence freshness" step still ran `public_baseline.py check`, which *requires* that block, so it failed on every PR ("README generated markers are missing") — a repo-wide lint breakage unrelated to any PR's code. Relax the gate without touching the README: a new `benchmarks/ ci_public_baseline_check.py` reuses `public_baseline`'s functions to enforce artifact freshness/integrity (age, schema, source + harness fingerprints) and `suite/results/RESULTS.md` drift, but validates the README table only when its generated markers are present. The lint step now calls it instead of `public_baseline.py check`. The policy lives in a separate module on purpose: `public_baseline.py` is a fingerprinted harness file (`HARNESS_PATHS`), so relaxing the check *inside* it would change `harness_fingerprint` and trip `validate_public`'s "harness changed; regenerate" guard — unsatisfiable without re-running the full Node/Bun/Perry suite. Importing its functions from a new file leaves the fingerprint intact. README.md is unchanged.
📝 WalkthroughWalkthroughAdded a CI wrapper that validates public benchmark artifact freshness and consistency, then updated the test workflow to execute it while treating the embedded README table as optional. ChangesPublic benchmark freshness gate
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/ci_public_baseline_check.py`:
- Around line 47-49: Update the exception handling around the public baseline
validation to include ValueError alongside the existing ArtifactError, KeyError,
and OSError cases, so malformed generated_at timestamps produce the existing
“public baseline error” message and return status 2.
- Around line 42-46: Update the README validation condition around
pb.README_START, pb.README_END, and pb._replace_block to distinguish valid
marker pairs from malformed partial pairs: allow files containing neither
marker, but raise pb.ArtifactError when exactly one marker is present, while
preserving drift validation for complete pairs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce0a2bab-b0b9-45e5-ad23-b0137afd694c
📒 Files selected for processing (2)
.github/workflows/test.ymlbenchmarks/ci_public_baseline_check.py
Comment on lines
+42
to
+46
| if pb.README_START in readme and pb.README_END in readme: | ||
| if pb._replace_block(readme, pb.readme_block(artifact)) != readme: | ||
| raise pb.ArtifactError( | ||
| "README Node/Bun table has drifted from the public artifact" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject incomplete README marker pairs.
When only one generated marker is present, this condition skips README validation entirely. Treat an unmatched start/end marker as malformed, while continuing to allow README files with neither marker.
Proposed fix
readme = pb.README.read_text(encoding="utf-8")
- if pb.README_START in readme and pb.README_END in readme:
+ has_start = pb.README_START in readme
+ has_end = pb.README_END in readme
+ if has_start != has_end:
+ raise pb.ArtifactError("README generated markers are incomplete")
+ if has_start:
if pb._replace_block(readme, pb.readme_block(artifact)) != readme:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if pb.README_START in readme and pb.README_END in readme: | |
| if pb._replace_block(readme, pb.readme_block(artifact)) != readme: | |
| raise pb.ArtifactError( | |
| "README Node/Bun table has drifted from the public artifact" | |
| ) | |
| readme = pb.README.read_text(encoding="utf-8") | |
| has_start = pb.README_START in readme | |
| has_end = pb.README_END in readme | |
| if has_start != has_end: | |
| raise pb.ArtifactError("README generated markers are incomplete") | |
| if has_start: | |
| if pb._replace_block(readme, pb.readme_block(artifact)) != readme: | |
| raise pb.ArtifactError( | |
| "README Node/Bun table has drifted from the public artifact" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/ci_public_baseline_check.py` around lines 42 - 46, Update the
README validation condition around pb.README_START, pb.README_END, and
pb._replace_block to distinguish valid marker pairs from malformed partial
pairs: allow files containing neither marker, but raise pb.ArtifactError when
exactly one marker is present, while preserving drift validation for complete
pairs.
Comment on lines
+47
to
+49
| except (pb.ArtifactError, KeyError, OSError) as exc: | ||
| print(f"public baseline error: {exc}", file=sys.stderr) | ||
| return 2 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Map malformed timestamps to the normal CI failure status.
public_baseline.validate_public parses generated_at with datetime.fromisoformat; malformed values raise ValueError, which is not caught here. The wrapper will therefore emit a traceback and exit with status 1 instead of the documented public baseline error status 2.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/ci_public_baseline_check.py` around lines 47 - 49, Update the
exception handling around the public baseline validation to include ValueError
alongside the existing ArtifactError, KeyError, and OSError cases, so malformed
generated_at timestamps produce the existing “public baseline error” message and
return status 2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
lintjob's Public benchmark evidence freshness step fails on every PR:#6736 ("rewrite README as a concise, marketing-focused landing page") replaced
the auto-generated
<!-- public-node-bun:start/end -->benchmark block with adifferent, hand-curated performance section — but
benchmarks/public_baseline.py checkstill requires that block. Result:
linthas been red on main and every open PRsince #6736 merged, unrelated to any PR's code.
Fix
Relax the gate so the embedded README table is optional, without touching the
README:
benchmarks/ci_public_baseline_check.pyreusespublic_baseline's functionsto enforce artifact freshness/integrity (age ≤ 45d, schema, source + harness
fingerprints) and
suite/results/RESULTS.mddrift — but validates READMEdrift only when the generated markers are present.
lintstep now runs the wrapper instead ofpublic_baseline.py check.Why a separate module (not a patch to
public_baseline.py)public_baseline.pyis a fingerprinted harness file (HARNESS_PATHS). Editing itchanges
harness_fingerprint, which makesvalidate_publicfail with"harness changed; regenerate it" — unsatisfiable without re-running the full
Node/Bun/Perry benchmark suite. Importing its functions from a new, non-fingerprinted
file keeps the fingerprint (and the freshness guarantee) intact.
Validation
Local run of the exact lint step:
README.mdis unchanged. When a future change re-adds the markers, README drift isenforced again.
Summary by CodeRabbit