Modernization: filelock + atomic writes, Python 3.10+/uv tooling, hardened C++ core, native token/terminal matching (2.2x faster parsing)#62
Merged
Conversation
- Write Greynir.grammar.bin via a temp file + os.replace(), so a killed process can never leave a truncated binary grammar behind - Replace the hand-rolled fcntl/msvcrt lock in glock.py with the cross-platform filelock package; the lock file now lives next to the binary grammar, scoping it per installation instead of /tmp - Time out lock acquisition after 180s with a clear GrammarError instead of hanging indefinitely - Skip the file lock entirely on the warm path (binary grammar up to date); thread safety of the class-level grammar caches is now provided by an in-process threading.Lock - Retry os.replace() on transient PermissionError (Windows, when another process has the binary grammar open for reading) - Keep GlobalLock as a thin backwards-compatible shim over filelock - Add CLAUDE.md with build/test commands and architecture notes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Require Python >= 3.10 (3.9 is EOL and held back the toolchain: tokenizer >= 3.6.3, current mypy and setuptools all need 3.10+); bump version to 3.7.0, tokenizer floor to >= 3.6.3 - Adopt uv properly: commit uv.lock, move dev dependencies to PEP 735 [dependency-groups] (adding ruff and mypy; mypy excluded on PyPy where its native dependencies don't build), use astral-sh/setup-uv and 'uv sync --locked' in CI - Switch license metadata to PEP 639 SPDX form (license = "MIT", license-files), drop the deprecated license classifier, require setuptools >= 77.0.3 in the build system - Move mypy configuration into pyproject.toml [tool.mypy] (target 3.10); drop the unused [tool.isort] section - Test on Windows and macOS (one job each) in addition to the Linux matrix, since wheels are shipped for all three platforms - Pin cibuildwheel (4.1.0) in the wheels workflow; build cp310 abi3 wheels; remove vestigial git-lfs configuration (no files use LFS) - Update README and CLAUDE.md accordingly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- bindb.py: stop reusing 'm' for both List[BIN_Tuple] and the Optional[BIN_Tuple] result of StaticPhrases.lookup() - reynir.py: annotate _Sentence._tree (and the local in parse()) as Optional[Node] instead of letting mypy infer type None - Add types-cffi to the dev dependency group so eparser_build.py type-checks - Run mypy in CI on all non-PyPy jobs now that it is clean Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single '*' in a GitHub Actions branch glob does not match '/', so pushes to branches like feat/modernization never triggered the tests workflow; use '**' instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
astral-sh/setup-uv has no floating 'v8' major tag, so @v8 fails to resolve in Actions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
C++ core: - Catch all C++ exceptions (out-of-memory in particular) in the extern "C" entry points newGrammar(), newParser() and earleyParse(), returning NULL instead of letting an exception propagate through the C ABI boundary into CFFI (undefined behavior) - Validate the binary grammar in readBinary(): sanity-check symbol counts, require a root index within the nonterminal range, check the previously unchecked production read, and bounds-check every production item; a corrupt or truncated .bin now fails cleanly with newGrammar() == NULL instead of crashing later during parsing. Also fix Nonterminal leaks on the readBinary() error paths. - Compare Labels member-wise instead of memcmp() of the whole struct, which silently relied on the absence of padding - Make the diagnostic allocation counters std::atomic (relaxed), so concurrent parses in multiple threads cause no data races - Add -std=c++11 to the macOS build flags (needed for <atomic>; Linux already had it, MSVC supports it natively) Python side: - Cap the token/terminal matching cache at 25,000 entries (~125 MB) to bound memory growth in long-running processes - Convert the interior-node coalescing in Node.from_c_node() from recursion to an explicit stack, removing the recursion depth limit on long coalescible node chains Add test_binary_grammar.py with regression tests for corrupt, truncated and minimal binary grammar files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Previously, unrecognized keyword options (e.g. a typo, or max_sent_tokens which belongs to the parse methods) were silently ignored. The constructor now validates options against a class-level frozenset of the known Greynir/bintokenizer/tokenizer options; derived classes accepting additional options can extend the set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Based on profiling showing ~40% of fresh-text parse time spent in C-to-Python matching callbacks. Key findings grounding the design: terminal variants already reduce to a 39-bit space (VariantHandler vbits/fbits), token meanings map into the same space via get_fbits(), and 4,893 of 6,012 grammar terminals are literals whose matching is pure interned-string identity plus bit tests. Proposes a TerminalSpec table and per-token MeaningRec arrays with a T_PYTHON escape hatch, GreynirCorrect-compatible gating, and a phased rollout with a parity harness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move the majority of token/terminal match decisions from Python callbacks into the C++ Earley core, per doc/cpp-matching-design.md. - binparser.py: build_matching_table() classifies each terminal into a native matching kind (strong/lemma literals, default-category, noun, adjective, adverb, masked abfn/pfn, töl) or T_PYTHON for semantics that stay in Python (verbs, prepositions, proper names, ending constraints, etc.); encode_token_matching_data() packs each token's BÍN meanings into MeaningRec arrays sharing the existing VBIT/fbits bit space. Literal lemmas/forms are interned so literal matching reduces to integer identity. - eparser.cpp/.h: Parser::evalMatch() decides matches natively from the TerminalSpec table and per-column TokenRec data (fetched once per column via the new MeaningsFunc callback and cached per token key); Column::matches() falls back to the Python callback for T_PYTHON terminals and non-word tokens. A parity mode double-checks every native decision against the Python matcher. - fastparser.py: install the table at parser construction, gated so that subclasses which override token wrapping (e.g. GreynirCorrect) automatically keep pure Python matching; also disable via the _USE_CPP_MATCHING class attribute or GREYNIR_DISABLE_CPP_MATCHING=1. Parity mode via GREYNIR_MATCHING_PARITY=1. Phase 0 measurement: 69% of match queries are natively answerable. Measured effect: ~89% of matching callbacks eliminated; typical fresh-text parsing 2.2x faster on CPython 3.13 (1.40s -> 0.65s for the benchmark set), test suite ~20% faster, PyPy ~20% faster cold; warm-cache and long-sentence workloads unchanged. Query-level parity: zero discrepancies over the test corpus. Incidental pre-existing finding, documented in the design doc: reduction of exact score ties is unstable across repeated parses, so the new equivalence test compares forests, not reduced trees. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Instrument the TerminalSpec table allocations in setMatchingTable() with an AllocCounter, following the codebase convention, so that printAllocationReport() shows the table balance. Also count native match evaluations separately, so the report shows the native/Python split of matching calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Grow the parity/equivalence corpus from 10 to 39 sentences, covering all producible token kinds (verified by a new coverage test asserting 24 required kinds) and all families of token/terminal matching: literal terminals, category terminals, verb frames with impersonal/ oblique subjects, middle voice, past participle and expletives, pronouns, degrees and subject-case adjectives, abbreviations, unknown/foreign words, street and person names, and the non-word token kinds (amounts, percentages, dates, times, timestamps, measurements, e-mail, URLs, domains, hashtags, usernames, telephone numbers, molecules, companies and entities). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes GreynirEngine’s tooling/runtime (Python 3.10+, uv-based dev/CI) and significantly optimizes/hardens the parser core by introducing atomic grammar writes, cross-process file locking via filelock, binary grammar validation, improved C++ boundary safety, and a new native (C++) token/terminal matching fast path.
Changes:
- Adopt
uv(committeduv.lock), update CI touv sync --locked, and raise minimum CPython to 3.10 (plus related packaging metadata updates). - Make binary grammar generation safer (temp file +
os.replace()), serialize regeneration across processes withfilelock, and add regression tests for corrupt/truncated binary grammars. - Implement native (C++) token/terminal matching with parity mode + gating, plus additional C++ hardening (exception safety, atomic diagnostics, safer struct comparisons).
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds locked dependency resolution for uv-based workflows. |
| test/test_reynir.py | Adds coverage for constructor unknown-option TypeError behavior. |
| test/test_native_matching.py | New tests for native matching equivalence, parity mode, and gating behavior. |
| test/test_binary_grammar.py | New regression tests ensuring corrupt/truncated binary grammar files fail cleanly. |
| src/reynir/reynir.py | Adds constructor option validation and improves typing for parse state fields. |
| src/reynir/grammar.py | Writes binary grammar via temp file + atomic replace with Windows retry logic. |
| src/reynir/glock.py | Replaces bespoke fcntl/msvcrt locking with filelock while keeping compatibility API. |
| src/reynir/fastparser.py | Adds filelock-based grammar regeneration lock, native matching integration, and bounded matching caches. |
| src/reynir/eparser.h | Adds native matching structures/APIs and makes some diagnostics thread-safe. |
| src/reynir/eparser.cpp | Implements native matching evaluation, improves binary grammar validation, and hardens C ABI boundary against C++ exceptions. |
| src/reynir/eparser_build.py | Updates the CFFI build interface for new callbacks/APIs and macOS compile flags. |
| src/reynir/binparser.py | Builds native matching tables and encodes per-token matching records for C++ fast path. |
| src/reynir/bindb.py | Minor refactor/rename for clarity in static phrase lookup. |
| README.md | Updates Python version requirements and documents uv-based test running + new lock-file behavior. |
| pyproject.toml | Raises minimum Python to 3.10, adds filelock, adopts dependency groups, and updates license metadata. |
| MANIFEST.in | Excludes lock/tmp artifacts for binary grammar from source distributions. |
| doc/cpp-matching-design.md | Documents the design and measured results of the native matching implementation. |
| CLAUDE.md | Adds repo guidance for Claude Code (commands + architecture notes). |
| .gitignore | Stops ignoring uv.lock; ignores binary grammar lock/tmp artifacts. |
| .github/workflows/wheels.yml | Updates wheel build targets to cp310 abi3 and pins cibuildwheel. |
| .github/workflows/python-package.yml | Switches CI to uv, adds mypy, broadens branch globs, and adds Windows/macOS coverage. |
| .gitattributes | Removes Git LFS configuration for large resource binaries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+887
to
+888
| self._matching_cache.clear() | ||
| self._meanings_cache.clear() |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in fb2d286. The caches are now replaced with fresh dict instances rather than cleared in place, so in-flight parse jobs (which hold a reference to the dict they were created with) keep the underlying CFFI buffers alive until they complete. Also verified with a threaded stress test: 8 threads × 12 parses on a shared parser with the cap forced to 2 entries, so replacement happens continuously mid-parse.
Comment on lines
+32
to
+34
| # Type check (config in pyproject.toml [tool.mypy]; carries a handful of | ||
| # known pre-existing errors, so it is not a CI gate) | ||
| uv run mypy src/reynir |
Member
Author
There was a problem hiding this comment.
Fixed in fb2d286 — the guidance was stale (written before the type errors were fixed later in this same branch). It now states that mypy runs as a CI gate on non-PyPy jobs and must stay clean.
| MeaningsFunc m_pMeaningsFunc; | ||
| MatchMasks m_masks; | ||
| BOOL m_bParity; // Parity checking mode | ||
| UINT m_nParityMismatches; |
Member
Author
There was a problem hiding this comment.
Fixed in fb2d286 — m_nParityMismatches is now std::atomic<UINT>, consistent with the other counters made atomic in this PR.
- fastparser.py: replace the shared matching/meanings cache dicts instead of clearing them in place when the size cap is hit; in-flight parse jobs in other threads hold references to the current dicts, which keeps the CFFI buffers that the C++ core points into alive until those jobs complete (avoiding a potential use-after-free) - eparser.h/.cpp: make the parity mismatch counter a relaxed atomic, consistent with the other counters, since it is incremented from within concurrent parses - CLAUDE.md: update stale guidance - mypy is now clean and runs as a CI gate on non-PyPy jobs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Summary
This branch modernizes the project infrastructure and significantly speeds up the parser core, in four areas. All 11 commits are individually CI-verified on Linux (CPython 3.10-3.14, PyPy 3.11), Windows and macOS.
1. Interprocess locking and atomic grammar writes
Greynir.grammar.binis now written via a temp file + atomicos.replace(), so a killed process can never leave a truncated binary grammar behind.fcntl/msvcrtlock inglock.pyis replaced by the cross-platform filelock package. The lock file lives next to the binary grammar (scoped per installation instead of a world-writable file in/tmp), acquisition times out with a clearGrammarErrorinstead of hanging, and the warm path (binary up to date) is entirely lock-free — thread safety of the class-level grammar caches is provided by an in-process lock.GlobalLockremains as a thin backwards-compatible shim.2. Project configuration
cp310.uv.lock, PEP 735[dependency-groups](pytest, ruff, mypy, types-cffi), CI viasetup-uv+uv sync --locked.pyproject.toml(target 3.10) and the codebase made fully mypy-clean, with mypy added as a CI gate.*does not match/in branch names, so slash-named branches never ran CI on push).3. Parser core hardening
extern "C"boundary and surface as cleanNULL/ParseErrorinstead of undefined behavior through CFFI.readBinary()fully validates the binary grammar (symbol counts, root index, production reads and item ranges); corrupt/truncated files now fail cleanly at load, with regression tests (test_binary_grammar.py).Label::operator==(no more padding-sensitivememcmp); relaxed-atomic diagnostic counters (no data races under concurrent parses); interior-node coalescing in forest conversion made iterative; the token matching cache is bounded (~125 MB cap).Greynir()now raisesTypeErroron unrecognized constructor options instead of silently ignoring them.4. Native (C++) token/terminal matching — 2.2x faster parsing
Per
doc/cpp-matching-design.md: profiling showed ~40% of fresh-text parse time spent in C-to-Python matching callbacks. A Phase 0 measurement classified 69% of match queries as natively answerable. The implementation moves those decisions into the C++ core:TerminalSpectable (literal terminals reduce to interned-id identity; category terminals to feature bit tests in the existing VBIT space) and packs per-token meaning records, fetched by C++ once per Earley column and cached per token key._USE_CPP_MATCHINGorGREYNIR_DISABLE_CPP_MATCHING=1.GREYNIR_MATCHING_PARITY=1) double-checks every native decision against the Python matcher; a CI test asserts zero discrepancies over a 39-sentence corpus covering all 24 producible token kinds and every matching family.Measured: ~89% of matching callbacks eliminated; typical fresh-text parsing 1.40s -> 0.65s (2.2x) on CPython 3.13; test suite ~20% faster; PyPy ~20% faster; warm-cache and long-sentence workloads unchanged. Allocation balances verified via the extended
printAllocationReport().Incidental findings (not addressed here)
Test plan
🤖 Generated with Claude Code