Repository navigation
perf(security): avoid Unicode folding for ASCII characters - #907
Conversation
Skip NFKC normalization and confusable translation for ASCII characters in the security skeleton helpers. Preserve the Unicode fallback and the one-letter ASCII sentinel, without adding caches or changing runtime checks. Add regression coverage for ASCII equivalence, Unicode edge cases, non-character inputs, and unchanged instruction matches and runtime checks. Refs NVIDIA#866 Signed-off-by: longankang <1790716272@qq.com>
longfie
left a comment
There was a problem hiding this comment.
Self-review of d4fb9f9:
- The published diff contains only the two ASCII fast paths and their regression tests.
- The non-ASCII path, one-letter sentinel, source spans, and runtime-check cadence remain unchanged. There is no new cache or retained input.
- The ASCII shortcut is guarded by a regression asserting the pinned confusable map contains no ASCII keys.
- New regression: 1 failure on the base revision, all 27 tests pass on this commit. Focused suites: 606 passed. Full local unit suite: 15427 passed; integration suite: 241 passed.
- Exhaustive helper comparison across all Unicode code points found no output differences. Full scanner fixture reports agree apart from timestamps/random IDs.
- Performance numbers are helper-level measurements only; this does not claim to resolve every hotspot in #866.
No blocking findings in this self-review. This is not independent maintainer approval; upstream CI and maintainer review are still required.
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Value and readiness
The five production lines remove Unicode normalization work for ASCII while retaining the original non-ASCII pipeline and runtime checkpoints. I found no required code change at d4fb9f9e26eb103e15b05ef7f4756af308542de2.
Material findings
No required change found. ASCII NFKC/confusable/casefold output equals lowercasing; the one-letter alphabetic sentinel is preserved for empty, multicharacter, control and punctuation inputs. The new tests guard the confusable-map invariant, all ASCII code points, representative Unicode cases, detection output and callback counts. The author self-review raises no outstanding finding.
PIC tradeoffs
The measured speedup is limited to these helpers; the PR correctly avoids claiming an end-to-end scanner improvement.
Verification and gaps
Read both complete changed files, description, author self-review, and empty comments/threads. The authored regression includes a base failure and reports exhaustive Unicode comparison; these are author-reported results. All six hosted checks succeeded. I performed static review without running contributor code.
Summary
Refs #866.
Add ASCII fast paths to
_security_skeleton_pieceand_fold_security_character. NFKC and the pinned confusable map leave ASCII unchanged, so ordinary characters need only lowercasing (and the existing one-letter alphabetic check).This is a deliberately narrow change: five production lines, no new cache, no change to non-ASCII normalization, source offsets, or runtime checks. It addresses two of the per-character hotspots in #866, not every bottleneck in that issue. It does not duplicate the token-gap seeking, confusable containment, or view-cache work in #583.
Regression coverage
Validation
macOS, Python 3.14.8, base
d7338f58200a9ae2a24810cee08911ab68b107a7:make format,make lint,make format-check: pass.make test-unit: 15427 passed, 14 skipped, 250 deselected, 4 xfailed.make test-integration: 241 passed, 5 skipped, 15449 deselected.--no-llm --format jsonscans of three local fixtures agree on every report field except timestamps and random finding IDs: plain text (score 0, 0 findings), ASCII-obfuscated instruction (42, 2), Unicode-obfuscated instruction (50, 3).The suites report existing unregistered
timeoutmarker warnings and a duplicate ZIP member fixture warning. Live provider tests were not run; provider credentials were removed from the test environment.Performance scope
For all 128 ASCII characters repeated 100 times through both helpers, normalization calls fall from 25,600 to 0, with identical output. Restoring the original source restores 25,600 calls and the original file hash.
An eight-round alternating microbenchmark over 270,336 ASCII characters measured median combined helper time of 69.1 ms before / 28.5 ms after (~2.42x). This is a helper microbenchmark, not an end-to-end scanner speedup claim.
Checklist