Repository navigation
ci: check the sentence corpus JSON in the spec job - #185
Merged
Merged
Conversation
spec/sentences_corpus.py pulls every sentence, for line and from line out of sentences-corpus.md into sentences-corpus.json, which OPA loads as data.spec.sentence_corpus, so a test can parse each one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Adds the parser for spec/sentences.abnf to ergo.rego: _sentence reads a check, _for_line a for line and _from_line a from line, each into a JSON shape that mirrors the sentence, or into a list of problems when the line is written wrong. _s_check_text and its two siblings print a parsed line back in the spelling the report will use. The grammar is flat, so the parser is layered regular expressions: one splits the assertions, one reads an assertion's parts, one reads the conditions of a where, and the phrase is picked by its leading words. The patterns are built once per call in _s_patterns and passed down, because Wasm rebuilds a constant rule at every reference, and the phrase union is factored by argument kind to keep them small. Every corpus line parses and prints back unchanged, in 11 seconds for all 306 under Wasm. Nothing calls it from ergo.report yet. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…take within Single quotes are only safe for a string with a backslash when it has no single quote, no two backslashes in a row, no backslash at the end and no control character, or parsing the printed sentence gives a different value. The written-wrong table loses the is empy row, since that sentence parses as a comparison with a field. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An index with a leading zero and a string with a bad escape stopped the evaluation instead of giving a problem. A value or key starting with true, false, null or empty was rejected, because the split took the first match; each piece now has to end at the separator or the end. The parser accepted a where on a plain path, a lone number on the left, a phrase other than a comparison after count of, sum of or plus, and a for name given twice; each is a problem now. A quoted grammar word is a key, and the dot in front of a key is spelling, not part of the parse, so the same check always reads the same. A position written in brackets at the end of a path reads and prints as item n of, and a string prints in single quotes only when that reads back to the same value. The messages for an unclosed quote, a missing phrase, a missing number after within and an unknown now say what the spec promises. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
So that count of item 1 of x, which is how the report prints count of x[0], is a sentence. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ree messages count of x[0] printed as count of item 1 of x, which did not parse, because a count or sum took no position. The unknown-name message skips quoted strings and names each name once, the invalid-string message gives a column so that a tab or newline in the string does not break the row, and the missing-phrase message quotes is and exists as the spec does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…time lacks Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ike it Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The parser and sentences.md already read it as a right-side path, where a quoted word is a value and not a key, so $input named by each of "T" doesn't parse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A number has to be 0 or have a magnitude between 2.2250738585072014e-308 and 1.7976931348623157e308, as in a policy. The parser now checks the size from the text first. Comparing a number whose exponent is beyond one million crashed OPA with "panic: illegal value", as in x is between -6 and -80e-08923553, and a number above 1.8e308 made x is between 1 and 1e400 fail with "doesn't parse". Problems now keep the order of their columns, so column 6 comes before column 20. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The squashed commit is core:, so the workflow change goes in a ci: pull request after this one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ttern x is between 1e400 and 1 said "min above max" too, and x matches 1e400 printed the parser's own marker as an invalid pattern. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sentences.md says the number after within is 0 or more, as REFERENCE.md says for today's within, but x is 1 within -1 parsed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jbpros
added a commit
that referenced
this pull request
Oct 13, 2026
First implementation PR after #180: the parser for `spec/sentences.abnf`, in `ergo.rego`, not yet called by `ergo.report`. **What it does.** `_sentence(text)` reads a check, `_for_line(text)` a `for` line and `_from_line(text)` a `from` line, each into a JSON shape that mirrors the sentence (quantifier, path, `where` conditions, phrase and its arguments, or `terms` for `count of`, `sum of` and `plus`), or into `{"problems": [...]}` when the line is written wrong. `_s_check_text` and its two siblings print a parsed line back in one spelling: `is` before a value and `equals` before a field, `is one of` before values and `is in` before a path, `an` before `object`, `item 3 of` for a position that ends a path, single quotes for a string with a backslash when that reads back the same, one space between words. Two spellings of one check give the same parse: the dot in front of a key, brackets or words for a position, and runs of spaces are spelling only. **How.** The grammar is flat, so the parser is layered regular expressions: one splits the assertions on `and`, one reads an assertion's parts, one reads the conditions of a `where`, one picks the phrase. Each split piece has to end at its separator or the end of the line, so the regex engine backtracks past a short match like `false` in `false-positive`. Rego can't recurse and this needs none. **Wasm.** The first version took over two minutes for the corpus under Wasm, because Wasm rebuilds a constant rule at every reference and the patterns were a tree of constants. They're now built once per call in `_s_patterns` and passed down, and the phrase union is factored by argument kind. The corpus test takes 20 s under Wasm and 0.3 s with `opa test`. **Tests.** Every one of the 306 corpus lines parses and prints back byte for byte, from `spec/sentences-corpus.json`, which `spec/sentences_corpus.py` writes from the markdown. The CI check that it's current is in #185, after this PR, so the squashed commit stays `core:`. Unit tests cover each phrase, each path form, the printed spellings, and the messages for sentences written wrong. **Review.** A fresh Opus review found twelve things, four of them real bugs: a leading-zero index and a bad string escape stopped the evaluation, keys starting with `true`, `false`, `null` or `empty` were rejected, and the parser accepted a `where` on a plain path, a lone number on the left, a non-comparison after `count of`, and a `for` name given twice. All twelve are fixed in the second `core:` commit, with the spec changed in two places it was wrong: when a string prints in single quotes, and the `is empy` row of the written-wrong table, since that sentence parses as a comparison with a field. **Second review.** A second review compared the parser with the ABNF, converted to a Lark grammar, on 6,000 changed corpus lines and 11,616 random sentences built from the grammar, and printed back every line the parser accepted. It found three things, fixed in the last commits: - `x is between -6 and -80e-08923553` crashed OPA with `panic: illegal value`. OPA (1.19.0, 1.20.2 and 1.21.2) panics when it compares a number whose exponent is beyond ±1,000,000, reported as open-policy-agent/opa#9365. The parser now works out a number's size from its text before `to_number`, and applies the rule a policy number already follows: `0` or a magnitude between `2.2250738585072014e-308` and `1.7976931348623157e308`. Otherwise the sentence gets `number out of range: <number>`. `sentences.md` says so. - That also fixes `x is between 1 and 1e400`, which failed with "doesn't parse" because OPA can't compare numbers above 1.8e308. - The ABNF gave `named by each of` a left-side path, where `"T"` is a key. The parser and `sentences.md` read a right-side path, so the ABNF now says `right-path`. Problems now keep their column order, so column 6 comes before column 20. A third review found that `x is 1 within -1` parsed, though the spec says the number after `within` is 0 or more. It now gets `"within" needs a number of 0 or more`. **Not in this PR.** Wiring into `ergo.report` and the semantics of each phrase. `spec/sentences.md` now says ergo reads sentences but runs none. Checks run here: `opa check`, `opa fmt`, `opa test` with coverage (every line reached), `--strict-builtin-errors`, Regal, no comments, OPA 1.20.2 `fmt`, Wasm tests in Docker, and the Wasm JS runtime script (1009 pass, 10.5 MB). On OPA 1.19, two of the 11,616 random sentences didn't print back the same inside one big evaluation, because 1.19 thinks `0.5e-7` equals `0.5e-70`. That's the known 1.19 number bug, and OPA 1.20.2 prints all of them back correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
jbpros
marked this pull request as ready for review
October 13, 2026 05:55
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.
Runs
python3 spec/sentences_corpus.py --checkin thespecjob and lists it inAGENTS.md. It fails whenspec/sentences-corpus.jsonno longer holds every line ofspec/sentences-corpus.md.This goes after #181, which adds the script and the JSON. It's separate so that #181's squashed commit stays
core:. Its base iscore/sentence-parser, so once #181 merges it needs rebasing ontomain.🤖 Generated with Claude Code