fix(faq-bot): restore the 0.2.7 regex screen - #111
Merged
Conversation
The adjacency rework in 0.2.8 refused rules that are provably fast,
including the two commonest shapes an operator writes: a capture
between two unbounded quantifiers. `^\s*(.*)\s*$` and `.*(\d+).*` each
measure 0.0 ms against a full 1000-character input and were both
accepted before 0.2.8. A refused rule is only logged, so an install
that relied on one stopped answering with no other symptom.
The same rework also let `a*(b)*a*a*$` through, at 1.3 s against 301
characters, because a group that can match empty was treated as
breaking the adjacency run when it actually joins the quantifiers on
either side of it.
Both follow from modelling a group as one run element without knowing
its width or whether it can match empty. Fixing that needs a screen
that carries both, which this one does not, so the change is withdrawn
rather than patched: a false reject silently disables a rule the
operator wrote correctly, which is the wider harm of the two.
rules.ts and rules.test.ts are byte-identical to 0.2.7 again. The
screen still refuses `(a+)+`, `.*.*.*`, `(a?){40}` and the ambiguous
repeated alternations 0.2.7 added. The shapes 0.2.8 aimed at, a run of
adjacent unbounded quantifiers spelled with groups or with overlapping
character classes, are accepted again.
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.
The adjacency rework shipped in faq-bot 0.2.8 refused rules that are provably fast, including the two commonest shapes an operator writes: a capture between two unbounded quantifiers.
^\s*(.*)\s*$(trim and capture)^\s*(.+)\s*$\s*(.*)\s*.*(\d+).*(extract a number)A refused rule is dropped with a log line and nothing else, so an install that relied on one of these simply stopped answering.
The same rework also loosened the screen in the other direction:
a*(b)*a*a*$was refused before 0.2.8 and is accepted by it, at 1.3 s against 301 characters. A group that can match empty was treated as breaking the adjacency run, when in fact it joins the quantifiers on either side of it.Why withdraw rather than patch
Both defects follow from the same thing: the rework modelled a group as a single run element without carrying its width or whether it can match empty. Getting that right needs a screen that tracks both, which this one does not. Patching around the two known symptoms would leave the model wrong.
Between the two error directions, the false reject is the wider harm. It disables a rule the operator wrote correctly, on patterns as ordinary as
.*(\d+).*. A false accept requires the operator to have written a catastrophic pattern themselves.What this restores
faq-bot/rules.tsandfaq-bot/rules.test.tsare byte-identical to 0.2.7. The screen still refuses(a+)+,.*.*.*,(a?){40}and the ambiguous repeated alternations added in 0.2.7, including((a|a))+$.The shapes 0.2.8 aimed at are accepted again: a run of adjacent unbounded quantifiers spelled with groups (
(a|b)*(a|b)*(a|b)*$,(a*)(a*)(a*)$) or with overlapping character classes ([ab]*[bc]*[cd]*$). Those are real, and they remain worth closing, but with a screen that models group width and nullability.Verification
npm run typecheck,npm test(658 pass, the 0.2.7 count),npm run catalog:check,npm run loader:check. Every pattern in the table above re-checked against this branch and against the commit that introduced the regression.faq-bot 0.2.9.