test+fix: parity-line e2e coverage, and fix #4 built-ins unreachable via REPL - #25
Merged
Merged
Conversation
… parser whitelist — were unreachable via REPL (#4)
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.
Brings Playwright e2e coverage up to date for everything shipped since the last e2e (MODIFY STRUCTURE, 2026-06-24), and fixes a real bug the e2e surfaced.
The bug (fixed)
The #4 built-ins (
ROUND,MOD,MAX,MIN,TIME,YEAR,MONTH,DAY— @kas2804's PR #17, shipped in 0.8.0) were implemented inBuiltins.tsbut never registered in the parser'sBUILTIN_FUNCTIONSwhitelist. So? ROUND(3.14159, 2)failed withUnknown command: (— the functions were unreachable from the REPL. The vitest unit tests passed only because they callcallStatelessdirectly, bypassing the parser. This is exactly the unit-vs-e2e gap that motivated this work. Fixed by registering the eight names; addedtests/BuiltinsParse.test.ts(8 cases) that drive the functions through the parser as a regression guard.New e2e —
tests/parity-commands.spec.ts(4 tests)?/??print (Implement the?command — print an expression #2)SUM/AVERAGE(AddSUMandAVERAGEcommands #3)SORT ON … TO(SORT TO— physically sorted copy of a table #8)Process fix
CLAUDE.md Definition of Done now requires a Playwright e2e per user-facing command, not just a unit test — the root cause of this drift.
Verification
Full vitest 224 passing,
tscclean; the 4 new e2e tests pass live against a real browser (re-ran independently).Targets
release/v1.0.0per GitFlow.