Skip to content

chore: a hand-written parser prototype, priced and not taken - #358

Closed
FBumann wants to merge 1 commit into
claude/package-performance-zmbv5afrom
claude/package-performance-zmbv5a-parser
Closed

chore: a hand-written parser prototype, priced and not taken#358
FBumann wants to merge 1 commit into
claude/package-performance-zmbv5afrom
claude/package-performance-zmbv5a-parser

Conversation

@FBumann

@FBumann FBumann commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Prompt: discuss why we are so slow still / We aren't doing much work… / Investigate

Note

The following content was generated by AI.

Opened to be closed. This is the record of an investigation, not a proposal — the conclusion is that replacing pyparsing is not worth it at this point, and the code is here so the next person to ask does not have to build it again. Stacked on #357, which is the work that is actually proposed.

What this changes

Nothing in src/. Two hand-written parsers under tools/prototypes/, not wired in, plus the two harnesses that produced every number below.

pixi run python -m tools.prototypes.benchmark     # what it would be worth
pixi run python -m tools.prototypes.differential  # whether it is the same parser

Why

The question was why a 64 KB file with 258 declarations takes ~100 ms to load when the package is barely doing anything. It isn't: 77% of a cold load is pyparsing, which charges ~100 µs to parse the single character p. Everything else — pydantic, macro expansion, resolution, dimensions, degree, over all 258 declarations — is 15 ms.

a cold to_spec(examples/pypsa.yaml) ms
pyparsing, today 85.2 1.0×
expression grammar replaced 33.6 2.5×
both grammars replaced 22.3 3.8×
nothing left to parse 19.2 4.4×

The parser step alone is 23×. The load is 3.8×, and no parser beats 4.4×, because a quarter of the load is not parsing. Those are different questions, and I quoted the first while meaning the second at one point in the discussion — the benchmark now prints both tables so it cannot happen again.

Why it is not worth it

The prize is bounded. 3.8× on the largest model in the tree, on top of the 4–5× that #357 already buys by memoising the parse and taking libyaml's scanner. Nothing in the repository is waiting on 60 ms.

The cost is ~300 lines in the files that define the language — and not the happy path, which agrees with pyparsing on all 207 expressions and 84 where strings the repository contains, and on 601,024 fuzzed inputs. The cost is the tail, and the tail is where a language quietly changes shape:

Three ways the prototype was wrong, none found by reading
  • Whitespace. The tokenizer first spelt it \s, which is 28 characters where this language allows four. x +\xa01 — a non-breaking space, one paste away in YAML — parsed. It failed open, widening the language by one regex character, and 300,000 fuzz inputs missed it because the alphabet was ASCII. A hand-written list of odd spaces found it. Fixing it, I immediately shipped the mirror bug ([ \t\n], dropping \r).
  • pp.Keyword checks the character before the word as well as after, so 0AND y is not the AND keyword. A tokenizer that splits 0 and AND loses that adjacency. Three of these still disagree in differential.py today, and they are left in deliberately rather than papered over.
  • A bare NOT is a parameter named NOT, because NOT is only the connective when an atom follows it. Matching that needed real backtracking. The fuzz caught this one in about a thousand inputs.

Each is small, each fails towards accepting more than the specification, and none was visible in the grammar.

If it is ever taken, pyparsing should move to the test feature rather than be deleted, and differential.py should run in CI. The oracle is what makes that tail tractable, and it costs a dependency that no longer ships to users.

Why ast.parse was rejected first, despite being faster

All 207 corpus expressions parse as Python expressions and produce identical trees, at 29× — marginally better than writing one by hand. Rejected anyway, because it buys 15% over a parser we own and costs the grammar:

  • It cannot spell .inf, which is in the published EBNF (docs/reference/language/expressions.md). The language would have had to change to suit the tool — which is the wrong way round.
  • The grammar stops being a declaration and becomes "Python's expression grammar, minus whatever a walk rejects": a subtractive definition with no correspondence to the published EBNF.
  • Closure stops being checkable. expansion.py, degree.py and boundedness.py close their walks with assert_never over this package's own unions, and pyrefly verifies exhaustiveness. CPython's node set is not ours and grows between releases.
  • I had also claimed it improves error messages. Only half true: it is exact for input that parses as Python but is not in this language (p ^ 2BinOp(BitXor)), and worse for genuinely malformed input, where you inherit CPython's SyntaxError text.

Two findings that deserve their own issues

Neither depends on the parser question, and I have not filed them:

  • to_spec raises RecursionError on a deeply nested expression — 300 nested parens is enough — where its docstring promises LanguageError. parse_expression catches only pp.ParseException. pyparsing itself dies at 1,000 nested parens where the prototype survives to 5,000, so this is an exposure that exists today.
  • The documented grammar and the implemented one disagree about names. expressions.md:20 says NAME ::= [a-zA-Z][a-zA-Z0-9_]*; expression_parser.py:400 is [a-zA-Z_][a-zA-Z0-9_]*, and _x + 1 parses today.
What was verified
  • pytest — 1009 passed, 5 skipped, unchanged; nothing here is imported by the package or collected by the suite.
  • ruff check, ruff format --check — clean across the tree. prettier on the README — clean. SPDX headers on all six new files.
  • tools/prototypes/ is not recursive-globbed by test_every_generator_is_asked, and contains no page_main(, so the generator table is unaffected.
  • Not run: pixi run ci as a whole, and within it docs-build and compile-tex — pixi cannot install in this environment (the egress proxy refuses pixi.sh), so the gates above ran in a plain venv. The README lives under tools/, outside docs_dir, so the strict docs build has no new page to resolve.
  • Timings are best-of-9 with the parse memo cleared before each run, on Python 3.13 in a container; absolute numbers drift between runs, the ratios hold.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HSgvWYbQLFB8GkLdiJSyuv


Generated by Claude Code

pyparsing is 77% of a cold load and charges ~100us to parse the single
character `p`. Two hand-written parsers measure what replacing it would be
worth: 23x on the parse step, 3.8x on a cold to_spec, against a 4.5x
ceiling that no parser can beat because a quarter of the load is not
parsing.

Not wired in, and not proposed. The happy path agrees with pyparsing on
all 207 expressions and 84 where strings in the tree; the tail is where a
hand-written parser quietly widens a language, and three divergences
survive in differential.py. tools/prototypes/README.md carries the
reasoning, the numbers and what an implementation would owe.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSgvWYbQLFB8GkLdiJSyuv

FBumann commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Note

The following content was generated by AI.

Closing: not worth it at this point.

The measurement stands — pyparsing is 77% of a cold load, and replacing both grammars is 3.8× end to end. But the prize is bounded at 4.4× no matter what, #357 already takes the same load from ~415 ms to ~115 ms, and nothing in the repository is waiting on the remaining 60 ms.

Against that, the cost lands in the two files that define the language, and the tail is not the happy path. The prototype agreed with pyparsing on all 207 expressions and 84 where strings in the tree and on 601,024 fuzzed inputs, and was still wrong three times — whitespace spelt \s (28 characters where the language allows four, so a pasted non-breaking space parsed), pp.Keyword's left-hand boundary check, and a bare NOT being a parameter of that name. Each failed towards accepting more than the specification, and none was visible in the grammar. That is the wrong risk to take for 60 ms in a repository whose product is the language's definition.

Reopen if bulk load time becomes a real constraint for a consumer. The branch is the starting point, differential.py is the oracle that makes the tail tractable, and the condition for doing it is written down in tools/prototypes/README.md: pyparsing moves to the test feature rather than being deleted, and the differential runs in CI.

Two findings here are independent of this decision and outlive it — to_spec raising RecursionError where it documents LanguageError, and the NAME drift between expressions.md and expression_parser.py. Both are described in the PR body and neither is filed yet.


Generated by Claude Code

FBumann commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Note

The following content was generated by AI.

Correction to this PR's body and to tools/prototypes/README.md.

Both say the RecursionError is the parser's, and that "a depth guard cannot be added inside pyparsing; a parser we own is what would make it fixable." That is wrong about the case that matters, and it overstated the argument for replacing pyparsing.

parse_expression handles a 1000-term chain without trouble. The recursion is in this package's own tree walksexpansion._expand/_descend, three Python frames per node — so a 400-term sum parses fine and then blows up on the way through validation. Only the nested-parentheses variant fails inside pyparsing, and catching RecursionError at the front door covers that.

The fix needed no new parser. It is #359: a depth cap checked iteratively where a string becomes a tree, both grammars, with a message naming the rewrite. Nothing about the decision to close this changes — if anything the case for the rewrite is weaker than stated here, since one of the two findings it claimed as motivation was not the parser's fault.

The other finding recorded here — the NAME drift between expressions.md and expression_parser.py — is also fixed in #359, along with a test comparing the published production against the constant, so it cannot drift again.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants