Fix #587: Strict parsing tests for html structural signatures - #734
Merged
Conversation
Adds strict positive/negative, ambiguity-sweep, and ReDoS regression coverage in tests/core_engine/test_language_standards_strict.py for all 39 non-None html structural signatures (54 tests). Found and fixed 8 real bugs in html's rules dict, all the same root shape: a `\b` sitting adjacent to a non-word character inside a shared alternation group. Also found and filed a separate, real pipeline bug as issue #733 before writing any tests: html is tagged `line_exclusive` but should be `block_exclusive` (its own inline comment already says so), and block_exclusive's own delimiter config is empty/broken -- net effect, <!-- --> comments are never actually stripped from code_stream for html today. Not fixed here (matches the epic's established pattern of keeping per-language issues scoped to the rules-dict regexes and filing pipeline gaps separately, e.g. #691/#694/#697). Tests below are written against html's real (unstripped) code_stream behavior -- dead_code/macros search for the literal "<!--" prefix directly, which is why they still work despite the pipeline bug. Real bugs found and fixed (8, all the shared-`\b`-next-to-non-word-char trap): 1. `branch`: `*ngIf` (Angular) was inside `\b(...)"[^"]*"` -- `*` is always preceded by whitespace in real markup, so the leading `\b` (boundary between two non-word chars) never fired. Never matched. 2. `comprehensions`: `*ngFor`, identical shape to #1. 3. `safety`: `pattern="..."`/`sandbox="..."`/`rel="noopener..."`/ `integrity="..."` all ended on `"` inside a shared `\b(...)\b` group -- the char after a closing attribute quote is also non-word, so the trailing `\b` never fired. 4 of 7 alternatives were dead. 4. `concurrency`: `loading="lazy"`/`fetchpriority="high"/"low"`, same shape as #3 (`decoding="async"` happened to self-heal via a coincidental substring match on the separate bare "async" alternative -- not a real rescue, still fixed for the right reason). 5. `immutability_locks`: `aria-disabled="true"`, same shape as #3 (also self-healed via "disabled" being a substring of "aria-disabled" -- matched regardless of true/false value, which was never the intent). 6. `dependency_injection`: `<script type="importmap"\b` -- trailing `\b` right after the closing `"`. Never matched at all, unconditionally. 7. `decorators`: `hidden`/`inert` are true boolean attributes almost always written bare (`<div hidden>`); the pattern required `[ \t]*=` unconditionally, so the dominant real-world bare form never matched. Found while writing this test's own positive case. 8. `reflection_metaprogramming`: `style="[^"]*;"` required a literal trailing semicolon before the closing quote -- CSS allows omitting the last declaration's semicolon and most real inline styles don't carry one. Dropped the requirement. All 8 independently verified: old-pattern-reproduces / new-pattern-fixes for each regression, including the two "self-heals but for the wrong reason" cases (concurrency's decoding=async, immutability_locks' aria-disabled=true). Noted but NOT fixed (deliberate scope, not a bug): every attribute-value pattern in this file assumes double-quoted values only (`"[^"]*"`). HTML permits single quotes too. Double-quotes are the dominant real-world/ tooling-enforced convention; fixing this would mean doubling nearly every one of the 39 patterns rather than a narrow, isolated change -- flagged for awareness, not attempted here. Verification: strict test file (1502 passed), full default suite (2446 passed, 1 deselected, 1 xfailed, no regressions), ruff/mypy/dead-key audits clean (3 pre-existing, non-deterministic mypy findings in untouched files confirmed unrelated), golden_crucible via tests/tools/crucible_check.py: real diff found (118 differences, confined to exactly 5 real .html files across 3 different corpus repos), fixtures regenerated and re-verified clean in both venvs. Part of #518, closes #587. Pipeline bug filed separately as #733. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
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
Part of #518, closes #587.
Adds strict positive/negative, ambiguity-sweep, and ReDoS regression coverage in
tests/core_engine/test_language_standards_strict.pyfor all 39 non-Nonehtmlstructuralsignatures (54 tests). Found and fixed 8 real bugs, all the same root shape: a
\bsittingadjacent to a non-word character inside a shared alternation group.
Also found and filed a separate, real pipeline bug as #733 before writing any tests:
htmlis tagged
line_exclusivebut should beblock_exclusive(its own inline comment already saysso), and
block_exclusive's own delimiter config is empty/broken -- net effect,<!-- -->comments are never actually stripped from
code_streamfor html today. Not fixed here, matchingthe epic's established pattern of keeping per-language issues scoped to the
rules-dict regexesand filing pipeline gaps separately (e.g. #691/#694/#697). Tests here are written against html's
real (unstripped)
code_streambehavior --dead_code/macrossearch for the literal<!--prefix directly, which is why they still work despite the pipeline bug.
Real bugs found and fixed
branch:*ngIf(Angular) inside\b(...)"[^"]*"--*is always preceded by whitespacein real markup, so the leading
\bnever fired. Never matched.comprehensions:*ngFor, identical shape to ci: implement cross-platform matrix testing #1.safety:pattern="..."/sandbox="..."/rel="noopener..."/integrity="..."all endedon
"inside a shared\b(...)\bgroup -- the char after a closing attribute quote is alsonon-word, so the trailing
\bnever fired. 4 of 7 alternatives were dead.concurrency:loading="lazy"/fetchpriority="high"/"low", same shape as Bump actions/setup-python from 5 to 6 #3(
decoding="async"happened to self-heal via a coincidental substring match on the separatebare "async" alternative -- fixed for the right reason anyway).
immutability_locks:aria-disabled="true", same shape as Bump actions/setup-python from 5 to 6 #3 (also self-healed via"disabled" being a substring of "aria-disabled" -- matched regardless of true/false value).
dependency_injection:<script type="importmap"\b-- trailing\bright after theclosing
". Never matched at all, unconditionally.decorators:hidden/inertare true boolean attributes almost always written bare(
<div hidden>); the pattern required[ \t]*=unconditionally, so the dominant real-worldbare form never matched. Found while writing this test's own positive case.
reflection_metaprogramming:style="[^"]*;"required a literal trailing semicolonbefore the closing quote -- CSS allows omitting the last declaration's semicolon. Dropped it.
All 8 independently verified: old-pattern-reproduces / new-pattern-fixes for each regression,
including the two "self-heals but for the wrong reason" cases.
Noted but NOT fixed (deliberate scope): every attribute-value pattern assumes double-quoted
values only. HTML permits single quotes too, but double-quotes are the dominant real-world/
tooling-enforced convention, and fixing it would mean touching nearly all 39 patterns rather than
a narrow change -- flagged for awareness, not attempted here.
Verification (all gates)
unrelated -- same finding count, different specific files each independent run)
golden_crucibleviatests/tools/crucible_check.py: real diff found (118 differences,confined to exactly 5 real
.htmlfiles across 3 different corpus repos -- confirmed viadirect
golden_diff.deep_compare(), not the truncated test message). Fixtures regenerated andre-verified clean in both venvs.
🤖 Generated with Claude Code