fix(ci): the invisible-character gate never matched anything - #56
fix(ci): the invisible-character gate never matched anything#56hyperpolymath wants to merge 1 commit into
Conversation
MEASURED 2026-08-27: this gate's pattern caught 0 OF 6 invisible-character test
cases. It has never detected an NBSP, zero-width space, BOM, soft hyphen, bidi
override or word joiner.
ROOT CAUSE: the pattern used UTF-8 BYTE sequences (\xc2\xa0) while grep -P
matches CHARACTERS. Bytes c2 a0 are ONE character U+00A0; \xc2\xa0 asks for TWO
characters, U+00C2 then U+00A0, which is never present.
grep -P '\xc2\xa0' -> miss
grep -P '\x{a0}' -> MATCH
Only \x00 worked, being single-byte in both readings.
FIXED: codepoint escapes; C0 control characters \x01-\x08,\x0B,\x0C,\x0E-\x1F
added (TAB/LF/CR excluded); and grep -a, without which grep skips any NUL-bearing
file as binary.
The C0 range matters: a stray BACKSPACE byte made a workflow unparseable in
developer-ecosystem, so it never ran, and this linter called it clean.
Canonical fix: hyperpolymath/empty-linter#70. 1 file(s) here.
VERIFIED: YAML re-parsed, and the corrected pattern was confirmed to catch a real
NBSP before the change was kept.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe workflow’s invisible-character scan now matches Unicode code points, detects additional control and formatting characters, and scans binary files as text. ChangesInvisible-character gate
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This PR makes a localized correction to the invisible-character check in the CI workflow, with no actionable merge-blocking risk remaining beyond normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description clearly explains the root cause, lists the implemented changes, and records verification results. It omits the template checklist and screenshots section, but it remains substantially complete and relevant. Full details: Linked Issues checkExplanation The workflow changes implement the linked issue's code-point escapes, C0 control detection, and grep -a requirements. The provided summary does not confirm preservation of the separate leading-BOM check or alignment with the compiled linter. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR successfully addresses the issue where the invisible-character CI gate failed to detect targets due to improper regex patterns and file handling. The switch to PCRE Unicode escapes (\x{...}) and the inclusion of the C0 control range significantly improve detection reliability.
Codacy analysis indicates the changes are up to standards. However, the review identifies opportunities to improve the grep command's performance by batching file execution and increasing visibility by removing error suppression. There are also missing test scenarios required to verify the detection of specific characters like NBSP and NUL bytes.
Test suggestions
- Verify detection of Non-Breaking Space (U+00A0) using codepoint escape.
- Verify detection of C0 control characters (e.g., backspace \x08) while ignoring TAB/LF/CR.
- Verify that files containing NUL bytes are scanned (not skipped) due to the -a flag.
- Verify detection of Zero-Width Space (U+200B) and Byte Order Mark (U+FEFF).
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify detection of Non-Breaking Space (U+00A0) using codepoint escape.
2. Verify detection of C0 control characters (e.g., backspace \x08) while ignoring TAB/LF/CR.
3. Verify that files containing NUL bytes are scanned (not skipped) due to the -a flag.
4. Verify detection of Zero-Width Space (U+200B) and Byte Order Mark (U+FEFF).
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| -o -name '*.idr' -o -name '*.zig' -o -name '*.v' -o -name '*.jl' \ | ||
| -o -name '*.gleam' -o -name '*.hs' -o -name '*.ml' -o -name '*.sh' \) \ | ||
| -exec grep -Prl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null | ||
| -exec grep -aPrl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: Simplify the grep command for better performance and visibility. Using + instead of \; allows grep to process multiple files per invocation. Removing 2>/dev/null ensures that any issues with the PCRE engine are visible in logs.
Note: The addition of the -a flag is correct and necessary; it forces grep to treat files containing null bytes as text, ensuring the gate doesn't skip detections in files it would otherwise classify as binary.
| -exec grep -aPrl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null | |
| -exec grep -alP "$PATTERNS" {} + > /tmp/empty-lint-results.txt |
| # non-breaking spaces, null bytes, and other invisible Unicode in source files. | ||
| set +e | ||
| PATTERNS='\xc2\xa0|\xe2\x80\x8b|\xe2\x80\x8c|\xe2\x80\x8d|\xef\xbb\xbf|\xc2\xad|\xe2\x80\x8e|\xe2\x80\x8f|\xe2\x80\xaa|\xe2\x80\xab|\xe2\x80\xac|\xe2\x80\xad|\xe2\x80\xae|\x00' | ||
| PATTERNS='\x00|[\x01-\x08\x0B\x0C\x0E-\x1F]|\x{a0}|\x{ad}|\x{200b}|\x{200c}|\x{200d}|\x{200e}|\x{200f}|\x{202a}|\x{202b}|\x{202c}|\x{202d}|\x{202e}|\x{2060}|\x{feff}' |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: The switch to PCRE Unicode escapes (\x{...}) and the explicit control character range provides much better coverage than the previous UTF-8 byte sequences. To be fully comprehensive, include the \x7F (Delete) character in the excluded range.
| PATTERNS='\x00|[\x01-\x08\x0B\x0C\x0E-\x1F]|\x{a0}|\x{ad}|\x{200b}|\x{200c}|\x{200d}|\x{200e}|\x{200f}|\x{202a}|\x{202b}|\x{202c}|\x{202d}|\x{202e}|\x{2060}|\x{feff}' | |
| PATTERNS='\x00|[\x01-\x08\x0B\x0C\x0E-\x1F\x7F]|\x{a0}|\x{ad}|\x{200b}|\x{200c}|\x{200d}|\x{200e}|\x{200f}|\x{202a}|\x{202b}|\x{202c}|\x{202d}|\x{202e}|\x{2060}|\x{feff}' |
1 participant
Measured 2026-08-27: this gate caught 0 of 6 invisible-character test cases. It has never detected an NBSP, zero-width space, BOM, soft hyphen, bidi override or word joiner.
Root cause
The pattern used UTF-8 byte sequences (
\xc2\xa0) whilegrep -Pmatches characters. Bytesc2 a0are one character U+00A0;\xc2\xa0asks for two, U+00C2 then U+00A0 — never present.Only
\x00worked, being single-byte in both readings. The gate ran, passed, and could not see what it exists to see.Fixed
\x01-\x08,\x0B,\x0C,\x0E-\x1Fadded (TAB/LF/CR excluded)grep -a— without it grep skips any NUL-bearing file as binaryThe C0 range matters: a stray backspace byte made a workflow unparseable in
developer-ecosystem, so it never ran — and this linter called it clean.Canonical fix: hyperpolymath/empty-linter#70. 1 file(s) here.
Verified: YAML re-parsed, and the corrected pattern was confirmed to catch a real NBSP before the change was kept.