Skip to content

report card: fix unterminated regex literal that breaks every chart#5

Open
yurekami wants to merge 1 commit into
SALT-NLP:mainfrom
yurekami:fix/report-card-regex
Open

report card: fix unterminated regex literal that breaks every chart#5
yurekami wants to merge 1 commit into
SALT-NLP:mainfrom
yurekami:fix/report-card-regex

Conversation

@yurekami

Copy link
Copy Markdown

Summary

The Plotly→DataTables click handler in the HTML report card escapes user IDs with a regex that is syntactically invalid: the character class is unterminated, so JavaScript fails at parse time with SyntaxError: Invalid regular expression: missing /. Because both occurrences sit inside the main <script> block (lines 1002–1201), the error aborts the entire block, so the Plotly correlation/runtime/robustness charts never render and click-to-jump never wires up on any report card AutoMetrics emits.

This is purely a typo: one extra backslash before the closing ] of the class.

What's wrong

id.replace(/[.*+?^${}()|[\]\\]/g, '\$&')

Inside [...] the parser reads:

  • \] → literal ]
  • \ → literal \
  • \] → escaped ] ← consumes what should be the closing bracket of the class

…so the class never terminates and the regex literal is unparseable. Verified directly in V8:

$ node -e 'try { var r = /[.*+?^${}()|[\]\\]/g; } catch(e) { console.log(e.message); }'
Invalid regular expression: missing /

Fix

Drop one backslash so the class closes after \] + \ + ]. This is the standard MDN regex-metachar escape class.

- id.replace(/[.*+?^${}()|[\]\\]/g, '\$&')
+ id.replace(/[.*+?^${}()|[\]\]/g,  '\$&')

Code changes

  • autometrics/util/report_card.py:
    • Line 1092 (jQuery DataTables path): drop the extra \ before the closing ] of the class.
    • Line 1115 (vanilla DataTables 2 path): same fix.

Two single-character changes; no behaviour intent changed — this is what the surrounding code obviously meant to do.

Verification

# Before: SyntaxError -> entire <script> block aborts
$ node check.js
ERR Invalid regular expression: missing /

# After: parses, escapes the metacharacter set as intended
$ node check.js
OK source=[.*+?^${}()|[\]\]

Why this slipped through

The bug only manifests at JS parse time in the rendered HTML; pytest doesn't execute the embedded script, and the existing test_report_card_pvalue.py only asserts presence of substrings in the rendered HTML, not that the JS parses. (Happy to add a tiny js2py / node smoke test as a follow-up if the maintainers want it, but kept this PR to the one-line fix.)

The id-escape regex used by the Plotly->DataTables click handler is
unterminated as written: inside the character class, the parser reads
`\]\\]` as literal `]`, literal `\`, escaped `]`, consuming the
closing `]` of the class. JS raises `SyntaxError: Invalid regular
expression: missing /` at parse time.

Both occurrences sit in the main <script> block (1002-1201), so the
SyntaxError aborts the whole block -- Plotly chart drawing and
click-to-jump never run on any rendered report card.

Drop one backslash so the class closes after `\]\` + `]`.
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.

1 participant