Repository navigation
fix(parser): Hypatia directives and a safe update mode for the grammar test - #433
Conversation
The Hypatia code scan reported `code_safety/panic_macro` (error) and `code_safety/expect_in_hot_path` (warning) in grammar_conformance.rs, on #432. The file is a test-only module (`#[cfg(test)] mod grammar_conformance` in lib.rs), but it is a file of its own below src/, so the scanner reads it as library code. The file now has the 2 `hypatia: allow` directives, with the reason, in the form that other files of the repository use. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 18 minutes. View limit details
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. |
Review finding on #432. With `UPDATE_GRAMMAR=1`, one test wrote spec/ephapax-v2-grammar.ebnf again while another test read the same file on a second thread. The reader could get the old text or a part of the new text, so the update command could fail or check stale content. The tests now take the text from one function with a `OnceLock`. The first call reads the file and, in update mode, writes part 1 again; each other test waits for that call and gets the same text. Checked: a file with a damaged part 1 fails in normal mode; in update mode, 5 runs in sequence pass and give back the committed text. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u
hyperpolymath
changed the title
|
Codex Review Summary: ❌ FailedThis 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. |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
Summary
2 small follow-ups to #432, which merged with one red check and one open review finding. Both are in
src/ephapax-parser/src/grammar_conformance.rs.1. Hypatia alerts
The Hypatia code scan reported 2 new alerts in that file on #432:
code_safety/panic_macro(error, 4 places)code_safety/expect_in_hot_path(warning, 11 places)The file is a test-only module:
lib.rsdeclares it as#[cfg(test)] mod grammar_conformance. But it is a file of its own belowsrc/, so the scanner reads it as library code. In a test, a panic with a message is how the test fails, and there is no hot path.The file now has the 2
hypatia: allowdirectives, each with its reason. This is the form that other files of the repository use (for examplesrc/ephapax-vram-cache/benches/cache_bench.rs).2. Update mode (Codex finding on #432)
With
UPDATE_GRAMMAR=1, one test wrotespec/ephapax-v2-grammar.ebnfagain while another test read the same file on a second thread. The reader could get the old text or a part of the new text.The tests now take the text from one function with a
OnceLock. The first call reads the file and, in update mode, writes part 1 again. Each other test waits for that call and gets the same text.Verification
cargo fmt --all --checkandcargo clippy -p ephapax-parser --all-targets: clean.cargo test -p ephapax-parser grammar_conformance: 3 passed.🤖 Generated with Claude Code
https://claude.ai/code/session_0136P7ZmKrYLDC4WHi2yt11u