[codex] Add parse benchmark watchdog - #6
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a standalone parse-benchmark tool for comparing ANTLR-generated parsers against tree-sitter across Kotlin and C# fixtures, integrated with GitHub Actions to detect performance regressions in pull requests. ChangesParse Benchmark Tool
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive benchmarking suite in tools/parse-bench to evaluate the parse throughput of ANTLR and tree-sitter parsers across Rust, Python, and Go runtimes. The implementation includes Kotlin and C# fixtures, automated parser generation, and a regression comparison script. Feedback from the review focused on adding safety checks to prevent accidental directory deletion and ensuring compatibility with older Python versions by adjusting the UTC timezone reference.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/parse-bench.yml (1)
1-10: ⚡ Quick winDeclare explicit workflow permissions (least privilege).
Add an explicit
permissionsblock (for example,contents: read) to avoid inheriting broader defaults.Suggested patch
name: Parse Benchmark on: pull_request: workflow_dispatch: +permissions: + contents: read + jobs: parse-bench:🤖 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 @.github/workflows/parse-bench.yml around lines 1 - 10, The workflow currently lacks an explicit permissions block which may grant broader-than-needed access; add a top-level permissions key (for example permissions: contents: read) to the workflow YAML (near the existing name: Parse Benchmark / on: pull_request entries) to declare least-privilege access for this workflow and ensure the parse-bench job runs with only the necessary permissions.tools/parse-bench/fixtures/manifest.json (1)
6-56: ⚡ Quick winUse commit-pinned source permalinks for fixture provenance.
The
sourceURLs point to mutable branch heads (main/master). Please switch these to commit-specific blob URLs so fixture attribution and provenance remain stable over time.🤖 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 `@tools/parse-bench/fixtures/manifest.json` around lines 6 - 56, Replace mutable branch-head URLs in the "source" fields with commit-pinned blob URLs in tools/parse-bench/fixtures/manifest.json: for each entry (e.g., paths kotlin/kotlinx-coroutines-flow.kt, kotlin/ktor-android-client.kt, kotlin/compose-window.kt, csharp/dotnet-runtime-boolean.cs, csharp/roslyn-syntax-kind.cs, csharp/newtonsoft-jsonconvert.cs, csharp/dotnet-samples-teleprompter.cs) find the current canonical commit SHA on the upstream repo and update the "source" value from a branch URL (…/blob/main or …/blob/master/...) to the commit-specific form …/blob/<commit-sha>/<file-path>; ensure every "source" key in the manifest is replaced so provenance is stable.
🤖 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 @.github/workflows/parse-bench.yml:
- Line 14: Replace the mutable action tags with immutable commit SHAs: update
each `uses:` entry that currently uses moving tags (`actions/checkout@v4`,
`actions/checkout@v5`, `actions/setup-java@v4`, `actions/setup-go@v4`,
`actions/setup-python@v4`, `actions/upload-artifact@v4`) to the corresponding
full commit SHA for that action release; locate the `uses:` lines in the
workflow and substitute the tag (e.g., `@vX`) with the action's pinned commit
SHA so the workflow references a fixed immutable revision.
- Around line 13-17: The checkout steps (e.g., the step named "Checkout head"
that uses actions/checkout@v4 and the other actions/checkout@v4 step in the same
workflow) are missing persist-credentials: false; update both checkout steps to
add persist-credentials: false under their with: block so the GITHUB_TOKEN is
not persisted into the repository git config during the job.
In `@tools/parse-bench/compare.py`:
- Around line 68-76: The code currently prints a success message even when no
rows were comparable (variable compared == 0); update the block after computing
compared in compare.py (the code that prints the "parse benchmark compare
passed" message) to detect compared == 0 and fail closed: write a clear error
message indicating "no comparable benchmark rows" and exit with a non-zero
status (e.g., call sys.exit(1) or raise SystemExit(1)) instead of reporting
success; keep the existing success/threshold print for compared > 0.
- Around line 20-23: The current load_results function uses a dict comprehension
that overwrites duplicate keys; update load_results to iterate over
data["results"] and for each result compute the key with result_key(result),
check if that key already exists in the accumulator dict, and if so raise a
ValueError (or otherwise fail fast) listing the duplicate (language, fixture,
runtime) key and the offending entries; otherwise insert the entry. This ensures
duplicate (language, fixture, runtime) rows are rejected instead of silently
overwritten.
---
Nitpick comments:
In @.github/workflows/parse-bench.yml:
- Around line 1-10: The workflow currently lacks an explicit permissions block
which may grant broader-than-needed access; add a top-level permissions key (for
example permissions: contents: read) to the workflow YAML (near the existing
name: Parse Benchmark / on: pull_request entries) to declare least-privilege
access for this workflow and ensure the parse-bench job runs with only the
necessary permissions.
In `@tools/parse-bench/fixtures/manifest.json`:
- Around line 6-56: Replace mutable branch-head URLs in the "source" fields with
commit-pinned blob URLs in tools/parse-bench/fixtures/manifest.json: for each
entry (e.g., paths kotlin/kotlinx-coroutines-flow.kt,
kotlin/ktor-android-client.kt, kotlin/compose-window.kt,
csharp/dotnet-runtime-boolean.cs, csharp/roslyn-syntax-kind.cs,
csharp/newtonsoft-jsonconvert.cs, csharp/dotnet-samples-teleprompter.cs) find
the current canonical commit SHA on the upstream repo and update the "source"
value from a branch URL (…/blob/main or …/blob/master/...) to the
commit-specific form …/blob/<commit-sha>/<file-path>; ensure every "source" key
in the manifest is replaced so provenance is stable.
🪄 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
Run ID: 59770980-6c1f-46c0-9d47-10e9ad833ba5
📒 Files selected for processing (15)
.github/workflows/parse-bench.yml.gitignoretools/parse-bench/README.mdtools/parse-bench/compare.pytools/parse-bench/fixtures/csharp/dotnet-runtime-boolean.cstools/parse-bench/fixtures/csharp/dotnet-samples-teleprompter.cstools/parse-bench/fixtures/csharp/newtonsoft-jsonconvert.cstools/parse-bench/fixtures/csharp/roslyn-syntax-kind.cstools/parse-bench/fixtures/kotlin/compose-window.kttools/parse-bench/fixtures/kotlin/jetbrains-kotlin-collections.kttools/parse-bench/fixtures/kotlin/kotlinx-coroutines-flow.kttools/parse-bench/fixtures/kotlin/ktor-android-client.kttools/parse-bench/fixtures/manifest.jsontools/parse-bench/requirements.txttools/parse-bench/run.py
![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: 75636aa5d8
ℹ️ 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".
| - name: Install Python benchmark dependencies | ||
| run: python -m pip install -r head/tools/parse-bench/requirements.txt |
There was a problem hiding this comment.
The workflow installs Python packages only from head/tools/parse-bench/requirements.txt and then executes base/tools/parse-bench/run.py; this makes the base run depend on head’s dependency set rather than the base commit’s own requirements. On PRs that add/remove benchmark dependencies, the base step can fail with import errors or run under different package versions, producing spurious failures or invalid baseline comparisons. Install dependencies separately per checkout (or use isolated envs) so each script runs with its own pinned requirements.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds a parse-performance benchmark harness for the Rust runtime against Python and Go ANTLR4 runtimes plus tree-sitter parser baselines.
What changed
tools/parse-bench/run.py, which regenerates Kotlin and C# ANTLR parsers from pinnedgrammars-v4inputs, builds Rust and Go timing runners, and runs Python ANTLR plus tree-sitter baselines.fixtures/manifest.json.tools/parse-bench/compare.pyto fail CI whenrust-antlrregresses past a configurable threshold.Why
This gives the runtime a repeatable performance signal that can be run locally and wired into PRs as a regression watchdog, while still showing how the Rust runtime lines up against established reference parsers.
Validation
python3 -m py_compile tools/parse-bench/run.py tools/parse-bench/compare.py/tmp/antlr-bench-probe/venv/bin/python tools/parse-bench/run.py --quick --python /tmp/antlr-bench-probe/venv/bin/python --json target/parse-bench/quick-final.jsontools/parse-bench/compare.py --baseline target/parse-bench/quick-final.json --current target/parse-bench/quick-final.jsonSummary by CodeRabbit
New Features
Documentation
Chores