feat(grammar): add support for minItems, maxItems, and numeric bounds #60

Merged

rcsheets merged 3 commits from feat/grammar-schema-keywords into main

2026-08-04 22:21:43 +00:00

rcsheets commented

2026-08-04 22:19:43 +00:00

Owner

also fix a CI inefficiency and resiliency issue

rcsheets added 3 commits

2026-08-04 22:19:43 +00:00

feat(grammar): enforce minItems/maxItems and numeric bounds 6b923acb55

Both keywords were 400s at compile time. Array counts are a per-frame
tally gating the closer and the separator; numeric bounds are decided on
the digits as they arrive.

The bounds are the interesting half. Checking only the finished number
would let the model spell a prefix no suffix can rescue -- under
minimum 100 the digit `5` misses at every scale -- and then refuse every
way of ending it, so the sampler emits digits until it runs out of
budget. Each byte is therefore tested against the whole set a prefix can
still reach, which for a free exponent is the union over e of
[D*10^e, (D+1)*10^e) and for an integer only the e >= 0 part of it.

The number is carried as a fixed-size summary rather than its raw bytes:
re-reading them on every token trial costs the number's length per step
and its length squared overall, which a bound like exclusiveMinimum 0
reaches easily, since a run of zeros is a legal prefix that can never be
terminated. That shape took the masking walk from 0.2s to 42s before the
summary replaced it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

test(grammar): fuzz the new keywords; document the prefix reasoning 2bf67ab415

The random-schema generator now emits minItems/maxItems and numeric
bounds, so the matcher walk exercises them for the dead-end property
rather than shipping them unfuzzed. Sampling a bounded number has to
watch which side the room is on: the sign of a literal covers its
fraction too, so "-10.5" is half a step below -10, not above it.

README drops both from the still-to-do list; AGENTS.md gains the
prefix-feasibility reasoning and why the number is carried as a summary.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

fix(ci): resolve the changelog model before generating

ci / test_and_build (pull_request) Successful in 23s

Details

246940b5e0

The changelog step took the model name on faith from .changelog.env. When
the serving side renames a model that name 404s, and the action cannot
tell that from any other failure: it spends three attempts of up to
INFERENCE_TIMEOUT (600s) finding out, then warns and returns 0, on a job
that has already tagged the release. The entry is silently lost and the
release goes green.

Probe first instead. Ask the configured name for one token; if it does
not answer, ask /v1/models what is actually served and try the names that
differ from the one configured. Whatever answers is passed to the action
as inference-model. If nothing does, the step is skipped with a warning
rather than retried blindly for half an hour.

A 5xx or a dead connection is the no-backend-registered case, which
clears on its own, so those are retried; a 4xx is a considered answer
about that model name and is not. Both defaults match the action's own
script, so an unset INFERENCE_URL probes exactly what the action would
have called, and the config file is parsed rather than sourced, as the
action parses it.

Like the action, this never fails the job: a release with no entry is
recoverable, one with a wrong entry is not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

pr-reviewer-bot commented

2026-08-04 22:19:45 +00:00

Collaborator

Automated review by pr-reviewer v0.44.2 | Safety Check | Claude Sonnet 5 | tracking id r-726581-a0c78b
This is an AI-generated review and may contain mistakes.

Status: ✅ Completed

⚠️ The primary review (Mistral Small) failed. The review below is from Claude Sonnet 5, which reviewed the same diff.

✅ Verdict: LGTM — The PR adds well-tested numeric-bounds/array-count grammar support and a defensive CI pre-flight check, with no signs of secrets, accidental commits, or logic that would break existing behavior.

Went through the diff for logic errors and anything alarming.

CI change (build-push.yaml): Adds a pre-flight probe step to check whether the inference backend/model is reachable before invoking the changelog action, and skips the changelog step gracefully (if: steps.changelog-model.outputs.ok == 'true') rather than retrying blindly. This is purely additive/defensive and doesn't touch the release/build/push steps themselves, so it can't break the actual release; worst case is a missing changelog entry, which is explicitly the intended fallback.

Grammar/schema changes (Go): The core addition is internal/grammar/number.go, implementing prefix-feasibility checking for numeric bounds (minimum/maximum/exclusive forms) and item-count gating for minItems/maxItems. This is intricate math (mantissa/exponent tracking, half-open interval feasibility, settle/topScale binary-search-like probing), but:

It's heavily commented with the reasoning inline, matching the documented edge cases (e.g., 73e-333 underflow, exponent-frozen mantissa, dead-end avoidance).
Extensive new tests (bounds_test.go) cover exactly the tricky edges called out in the comments (unreachable prefixes, exclusive bounds at the terminator, exponent-frozen mantissa, negative bounds).
The fuzzer/generator (schema_gen_test.go) was updated to emit bounded arrays/numbers, and the walk-schema list in schema_invariant_test.go gained bounds/array-count cases, so the new logic is exercised by both table tests and the fuzz-based soundness/completeness checks the AGENTS.md documentation for this package explicitly relies on.
Schema compilation (schema.go) validates unsatisfiable bounds (minItems > maxItems, empty numeric ranges, conflicting exclusive/inclusive bounds) at compile time rather than silently producing an unsatisfiable/dead-end acceptor — consistent with the stated design philosophy ("a schema that looks enforced and is not is worse than one that is refused").
schema_state.go wires the new numState into the existing per-byte acceptor and cache signature (sig()), including the array frame's new count field. The signature changes are appended consistently, preserving the cache-isolation invariant described in AGENTS.md (schema identity + cursor state need to be part of the sig).

Nothing here looks like a secret, credential, large binary, or unrelated debug artifact. Test file schema_test.go correctly updates the previously-"unsupported" test cases for minItems/minimum to reflect the new support, and adds new compile-time rejection tests for the newly-invalid cases (negative counts, empty ranges, bounds on non-numeric types) — consistent with the code changes, no orphaned assertions.

Given the depth of the math involved I'd normally be cautious, but the PR includes thorough targeted tests plus fuzz-generator updates for exactly this feature, and the diff is internally consistent with its own documentation updates (AGENTS.md, README.md). I don't see anything that looks like it was pushed by accident or introduces an obvious regression.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.44.2 | Safety Check | Claude Sonnet 5 | tracking id `r-726581-a0c78b`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- > ⚠️ The primary review (**Mistral Small**) failed. The review below is from **Claude Sonnet 5**, which reviewed the same diff. **✅ Verdict: LGTM** — The PR adds well-tested numeric-bounds/array-count grammar support and a defensive CI pre-flight check, with no signs of secrets, accidental commits, or logic that would break existing behavior. Went through the diff for logic errors and anything alarming. **CI change (`build-push.yaml`):** Adds a pre-flight probe step to check whether the inference backend/model is reachable before invoking the changelog action, and skips the changelog step gracefully (`if: steps.changelog-model.outputs.ok == 'true'`) rather than retrying blindly. This is purely additive/defensive and doesn't touch the release/build/push steps themselves, so it can't break the actual release; worst case is a missing changelog entry, which is explicitly the intended fallback. **Grammar/schema changes (Go):** The core addition is `internal/grammar/number.go`, implementing prefix-feasibility checking for numeric bounds (`minimum`/`maximum`/exclusive forms) and item-count gating for `minItems`/`maxItems`. This is intricate math (mantissa/exponent tracking, half-open interval feasibility, `settle`/`topScale` binary-search-like probing), but: - It's heavily commented with the reasoning inline, matching the documented edge cases (e.g., `73e-333` underflow, exponent-frozen mantissa, dead-end avoidance). - Extensive new tests (`bounds_test.go`) cover exactly the tricky edges called out in the comments (unreachable prefixes, exclusive bounds at the terminator, exponent-frozen mantissa, negative bounds). - The fuzzer/generator (`schema_gen_test.go`) was updated to emit bounded arrays/numbers, and the walk-schema list in `schema_invariant_test.go` gained bounds/array-count cases, so the new logic is exercised by both table tests and the fuzz-based soundness/completeness checks the AGENTS.md documentation for this package explicitly relies on. - Schema compilation (`schema.go`) validates unsatisfiable bounds (`minItems > maxItems`, empty numeric ranges, conflicting exclusive/inclusive bounds) at compile time rather than silently producing an unsatisfiable/dead-end acceptor — consistent with the stated design philosophy ("a schema that looks enforced and is not is worse than one that is refused"). - `schema_state.go` wires the new `numState` into the existing per-byte acceptor and cache signature (`sig()`), including the array frame's new `count` field. The signature changes are appended consistently, preserving the cache-isolation invariant described in AGENTS.md (schema identity + cursor state need to be part of the sig). Nothing here looks like a secret, credential, large binary, or unrelated debug artifact. Test file `schema_test.go` correctly updates the previously-"unsupported" test cases for `minItems`/`minimum` to reflect the new support, and adds new compile-time rejection tests for the newly-invalid cases (negative counts, empty ranges, bounds on non-numeric types) — consistent with the code changes, no orphaned assertions. Given the depth of the math involved I'd normally be cautious, but the PR includes thorough targeted tests plus fuzz-generator updates for exactly this feature, and the diff is internally consistent with its own documentation updates (AGENTS.md, README.md). I don't see anything that looks like it was pushed by accident or introduces an obvious regression.

rcsheets merged commit 128e914f61 into main

2026-08-04 22:21:43 +00:00

rcsheets deleted branch feat/grammar-schema-keywords

2026-08-04 22:21:43 +00:00

rcsheets referenced this pull request from a commit

2026-08-04 22:21:43 +00:00

Merge pull request 'feat(grammar): add support for minItems, maxItems, and numeric bounds' (#60) from feat/grammar-schema-keywords into main