feat: Aho-Corasick fast path, evasion normalization, and PY/JS detection parity#178
feat: Aho-Corasick fast path, evasion normalization, and PY/JS detection parity#178wlike wants to merge 11 commits into
Conversation
Improve profanity detection recall to 100% F1 on the shootout torture-set while keeping zero false positives, via Aho-Corasick matching, CJK word-script boundaries, evasion preprocessing, and Python context-aware filtering aligned with JS. Includes filter instance pools, expanded tests, and benchmark reports. Co-authored-by: Cursor <cursoragent@cursor.com>
…ports Map normalized match spans back to original text for replaceWith and context analysis, supplement AC with fuzzy-only legacy matching when word boundaries are disabled, and align JS/Python filter pool, dedupe, and config export behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
…d result parity Ensure check_profanity returns original-text spans via variant mapping and tier fallback, while keeping normalized-tier priority to avoid nested false positives. Unify contains/reason handling across AC, legacy, and context-aware paths in Python and JavaScript. Co-authored-by: Cursor <cursoragent@cursor.com>
…paths Collect FUZZY matches when word boundaries are disabled, and add original/aggressive tier fallback for context-aware legacy collection so check_profanity stays aligned with is_profane. Co-authored-by: Cursor <cursoragent@cursor.com>
Use config-aware result cache keys so runtime ignore/replace changes cannot return stale hits. Export full filter settings from get_config/getConfig, align Python leetspeak tables with JavaScript, and return legacy matches on the JS path. Co-authored-by: Cursor <cursoragent@cursor.com>
…nner edges Match Python combining-class semantics in JS variant mapping so emoji variation selectors no longer bleed into profane spans, and tighten CJK boundaries, accent folding, filter pool caching, and secret-pattern safety for consistent cross-language results. Co-authored-by: Cursor <cursoragent@cursor.com>
Skip digit substitutions in measurement-like tokens so 5m no longer normalizes to sm, and align contains_profanity with empty profane_words. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove machine-specific compare_csv_packages.py from the repo and gitignore it so the script can stay in the local workspace only. Co-authored-by: Cursor <cursoragent@cursor.com>
Automate lite shootout/test aggregation across release worktree and feat-performance-opt, and check in the comparison report with refreshed benchmark results. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds Aho-Corasick matching, evasion and span-mapping helpers, context-aware filtering, pooled filter reuse, scanner input hardening, parity tests, and benchmark/reporting updates across the JS and Python packages. ChangesProfanity detection overhaul
Benchmark scripts and generated reports
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/js/src/nlp/contextAnalyzer.ts (1)
137-140: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize phrase comparisons before applying positive overrides.
contextTextis built from lowercased punctuation-stripped tokens, whilematchWordcan preserve original casing andPOSITIVE_PHRASESincludes punctuation such asbomb.com. Normalize the operands consistently so phrase whitelisting works for capitalized/dotted cases.Suggested fix
private checkPhraseContext(contextText: string, matchWord: string): ContextAnalysisResult | null { + const normalizedMatchWord = matchWord.toLowerCase().replace(/[^\p{L}\p{N}_]/gu, ''); // Check positive phrases for (const [phrase, score] of POSITIVE_PHRASES.entries()) { - if (phrase.includes(matchWord) && contextText.includes(phrase)) { + const normalizedPhrase = phrase.toLowerCase().replace(/[^\p{L}\p{N}_\s]/gu, ''); + if (normalizedPhrase.includes(normalizedMatchWord) && contextText.includes(normalizedPhrase)) { return {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/nlp/contextAnalyzer.ts` around lines 137 - 140, The positive phrase override check in checkPhraseContext is comparing mixed-normalization inputs, so capitalized or dotted terms can miss whitelisting. Normalize matchWord and the POSITIVE_PHRASES phrase values the same way contextText is normalized before evaluating includes checks, and keep the comparison logic inside ContextAnalysisResult handling consistent so phrases like bomb.com match regardless of casing or punctuation.
🟠 Major comments (30)
packages/py/glin_profanity/utils/variant_mapping.py-191-218 (1)
191-218: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve source offsets for decoded HTML entities.
Entity-decoded variants break alignment at the original
&...;sequence, then_fallback_span()searches for the decoded word literally in the original text and returns an empty span. Downstreamfilter.pyonly keeps mapped matches with a non-empty word, so entity-evasion detections can lose their profane span.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/utils/variant_mapping.py` around lines 191 - 218, The alignment logic in variant_mapping.py is falling back to a literal span lookup when a decoded HTML entity no longer matches the original `&...;` sequence, which can produce an empty span. Update the span-mapping path around the alignment loop and `_fallback_span()` so decoded entity variants preserve the original source offsets instead of searching for the decoded word literally, ensuring `filter.py` still receives a non-empty mapped match.packages/js/src/filters/dictionaryAhoCorasick.ts-84-96 (1)
84-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize patterns for case-insensitive AC search.
haystackis lowercased whencaseSensitiveis false, but the automaton is built fromwordsas provided. A mixed-case custom word will not match its lowercase input. Keep separate sensitive/insensitive automatons, or build the searched automaton with lowercased patterns for insensitive mode.Also applies to: 119-135
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/filters/dictionaryAhoCorasick.ts` around lines 84 - 96, The case-insensitive path in DictionaryAhoCorasick is mismatched: hasAnyMatch lowercases the haystack, but the AhoCorasick automaton is still built from the original words, so mixed-case patterns can fail to match. Update the DictionaryAhoCorasick constructor and hasAnyMatch to either maintain separate automata for case-sensitive and case-insensitive searches or normalize the stored patterns to lowercase when building the insensitive matcher, while preserving the original words for grapheme counting and result reporting.packages/py/glin_profanity/filters/dictionary_aho_corasick.py-99-112 (1)
99-112: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize patterns for case-insensitive AC search.
With
case_sensitive=False, the haystack is lowercased but the automaton still contains the original word casing. Mixed-case custom dictionary entries therefore miss lowercase text. Mirror the JS fix with separate case-sensitive and lowercase automatons, or normalize inserted patterns for insensitive matching.Also applies to: 134-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/filters/dictionary_aho_corasick.py` around lines 99 - 112, The case-insensitive path in DictionaryAhoCorasick is inconsistent because has_any_match lowercases the haystack but __init__ still inserts original-cased words into the Automaton, so mixed-case patterns can miss matches. Update DictionaryAhoCorasick to normalize inserted patterns for insensitive matching, or build separate case-sensitive and lowercase automatons, and make has_any_match choose the correct one based on DictionarySearchOptions.case_sensitive. Also apply the same normalization approach to the related logic referenced by the second affected range so the stored grapheme lengths and lookup behavior stay aligned.packages/js/src/utils/variantMapping.ts-188-217 (1)
188-217: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIterate by code point when aligning normalized variants.
original[originalIndex]andvariantIndex++walk UTF-16 code units, so astral homoglyphs such as mathematical letters cannot align back to their original span. SinceFilter.tsdrops mapped matches with emptymatchedWord, this can turn detected normalized matches into missing reported profanity spans.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/utils/variantMapping.ts` around lines 188 - 217, The alignment loop in variantMapping’s span-mapping logic is advancing with UTF-16 code units, which breaks mapping for astral characters and can cause empty matched spans later in Filter.ts. Update the matching walk in the variant/original alignment code to iterate by Unicode code point instead of direct string indexing, and make sure the index advancement and char extraction in this loop still preserve the correct original span for charsAlign/normalizeCharForAlign.packages/js/src/utils/variantMapping.ts-220-247 (1)
220-247: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve source offsets for decoded HTML entities.
When normalization decodes an entity like
uintou, the scanner reaches&, breaks alignment, andfallbackSpan()searches for the decoded word literally in the original text. That returns an empty span, so downstream span collection discards the match. Carry an offset map from the evasion normalizer, or explicitly consume entity runs during mapping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/utils/variantMapping.ts` around lines 220 - 247, The span alignment in variantMapping.ts is dropping matches when normalization decodes HTML entities, because the scanner in the mapping logic breaks on the original entity text and then fallbackSpan() cannot recover the original offsets. Update the alignment flow in the mapping routine (including fallbackSpan and the span-scanning loop) to preserve source offsets for decoded entities, either by carrying an offset map from the evasion normalizer or by explicitly consuming entity runs as a single token during alignment so the original span is retained.packages/js/src/filters/dictionaryAhoCorasick.ts-43-49 (1)
43-49: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winFix the ESLint unused variable error.
for (const _ of ...)is still flagged by the configured rule. UseArray.from(...).lengthor otherwise consume the iterator without binding an unused variable.Suggested fix
function countGraphemes(text: string): number { - let count = 0; - for (const _ of graphemeSegmenter.segment(text)) { - count++; - } - return count; + return Array.from(graphemeSegmenter.segment(text)).length; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/filters/dictionaryAhoCorasick.ts` around lines 43 - 49, The unused variable warning comes from the countGraphemes helper using a for-of loop with an ignored binding. Update countGraphemes in dictionaryAhoCorasick.ts to consume graphemeSegmenter.segment(text) without declaring an unused loop variable, such as by using Array.from(...).length or an equivalent iterator-consuming approach, while keeping the function behavior the same.Source: Linters/SAST tools
packages/js/src/utils/variantMapping.ts-285-297 (1)
285-297: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDeduplicate partial overlaps, not only contained spans.
The implementation only removes spans fully contained by another span, but the function contract says overlapping ranges should keep the longest span. Partial overlaps like
[0, 5)and[3, 8)both survive today.Suggested fix
+const overlaps = (aStart: number, aEnd: number, bStart: number, bEnd: number) => + aStart < bEnd && bStart < aEnd; + if ( kept.some( ([, keptStart, keptEnd]) => - start >= keptStart && end <= keptEnd && keptEnd - keptStart > length, + overlaps(start, end, keptStart, keptEnd) && + keptEnd - keptStart >= length, ) ) { continue; } kept = kept.filter( ([, keptStart, keptEnd]) => - !(keptStart >= start && keptEnd <= end && length > keptEnd - keptStart), + !(overlaps(start, end, keptStart, keptEnd) && length > keptEnd - keptStart), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/utils/variantMapping.ts` around lines 285 - 297, The overlap deduplication in variantMapping.ts only handles fully contained spans, so partial overlaps can both survive even when the contract says to keep the longest range. Update the span-selection logic around the existing kept.some and kept.filter checks so any overlapping range is compared by length and only the longest overlapping span remains, including cases like [0, 5) vs [3, 8). Use the existing kept array processing in the same function to broaden the overlap test from containment to all intersections.packages/py/glin_profanity/utils/variant_mapping.py-245-260 (1)
245-260: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle partial overlaps in span deduplication.
This only removes fully contained spans. The documented “ranges overlap” behavior should compare any intersection and keep the longest span.
Suggested fix
+ def overlaps(a_start: int, a_end: int, b_start: int, b_end: int) -> bool: + return a_start < b_end and b_start < a_end + if any( - start >= kept_start - and end <= kept_end - and (kept_end - kept_start) > length + overlaps(start, end, kept_start, kept_end) + and (kept_end - kept_start) >= length for _, kept_start, kept_end in kept ): continue @@ if not ( - item[1] >= start - and item[2] <= end - and length > (item[2] - item[1]) + overlaps(start, end, item[1], item[2]) + and length > (item[2] - item[1]) )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/utils/variant_mapping.py` around lines 245 - 260, The span deduplication logic in variant_mapping.py only handles fully contained ranges, but the documented overlap behavior should treat any intersection as an overlap and keep the longest span. Update the dedupe flow around the kept-span filtering so the checks in the span-processing logic compare general interval intersection rather than only start/end containment, and ensure the existing kept list is pruned using overlap length/longest-span selection in the relevant span-mapping routine.packages/js/src/filters/Filter.ts-813-817 (1)
813-817: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftResolve fuzzy match positions before context filtering.
The context-aware fuzzy paths pass
matchIndex = 0, so profanity later in the string is scored using the beginning of the text. That can suppress or flag based on unrelated context. Return an approximate/resolved fuzzy span and pass its original start index intopassesContextFilter()/recordContextAwareMatch().Also applies to: 865-867, 1204-1208, 1239-1244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/filters/Filter.ts` around lines 813 - 817, Resolve the fuzzy-match position before running context filtering in Filter.ts: the current fuzzy paths in checkVariant and the other listed call sites pass matchIndex as 0, so passesContextFilter() and recordContextAwareMatch() evaluate context against the wrong part of the text. Update the fuzzy matching flow to compute or recover the approximate original start index for the matched variant, then pass that resolved index into passesContextFilter() and recordContextAwareMatch() so context scoring uses the actual match location.packages/py/glin_profanity/filters/filter.py-998-1004 (1)
998-1004: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftResolve fuzzy match positions before context filtering.
The context-aware fuzzy paths pass
match_index=0, so profanity later in the text is scored against the first token’s context. That can produce false suppressions/flags. Resolve an approximate fuzzy span and pass its original start index into_passes_context_filter()/_record_context_aware_match().Also applies to: 1032-1042, 1222-1223, 1275-1278
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/filters/filter.py` around lines 998 - 1004, The context-aware fuzzy matching paths in filter.py are using a fixed match_index of 0, which causes later profanity matches to be evaluated against the wrong token context. Update the fuzzy handling in the relevant matching flow around _passes_context_filter() and _record_context_aware_match() so they first resolve an approximate fuzzy span and then pass the original start index of that span instead of 0. Apply the same fix consistently in the other fuzzy call sites noted for the same logic so all context-aware fuzzy matches use the correct position.packages/js/src/filters/Filter.ts-5-5 (1)
5-5: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove ESLint-blocking unused symbols.
normalizeLeetspeak,_isOriginalVariant,_matchedWord, and_isOriginalVariantare reported as unused. Since these are private helpers, remove the unused parameters and update the call sites.Also applies to: 294-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/filters/Filter.ts` at line 5, Remove the ESLint-blocking unused symbols in Filter.ts by deleting the unused import normalizeLeetspeak and pruning the unused private helper parameters _isOriginalVariant and _matchedWord from the affected methods, then update any call sites within Filter and normalizeLeetspeakVariants usage to match the new signatures. Keep only the symbols that are actually referenced so the private helpers and their callers remain consistent and lint-clean.Source: Linters/SAST tools
packages/js/src/filters/Filter.ts-1441-1450 (1)
1441-1450: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor
caseSensitivewhen replacing matches.Detection uses
caseSensitive, but replacement always usesgi, so a case-sensitive match can redact differently-cased text that was not considered profane.Suggested fix
private getReplacementRegex(word: string): RegExp { const escaped = word.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const flags = this.caseSensitive ? 'g' : 'gi'; if (!this.wordBoundaries) { - return new RegExp(escaped, 'gi'); + return new RegExp(escaped, flags); } const script = classifyWordScript(word); if (script === 'cjk') { - return new RegExp(escaped, 'gi'); + return new RegExp(escaped, flags); } - return new RegExp(`\\b${escaped}\\b`, 'gi'); + return new RegExp(`\\b${escaped}\\b`, flags); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/filters/Filter.ts` around lines 1441 - 1450, The replacement regex in Filter.getReplacementRegex always uses the case-insensitive flag, which ignores the existing caseSensitive behavior. Update getReplacementRegex to build the RegExp flags based on this.caseSensitive so replacements match the same casing rules as detection, while still preserving the existing wordBoundaries and cjk handling in Filter.getReplacementRegex and classifyWordScript.packages/py/glin_profanity/filters/filter.py-588-594 (1)
588-594: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor
case_sensitivewhen replacing matches.Detection respects
case_sensitive, but replacement always compiles withre.IGNORECASE, so a case-sensitive hit can redact differently-cased text that was not detected as profane.Suggested fix
def _get_replacement_regex(self, word: str) -> re.Pattern[str]: escaped = re.escape(word) + flags = 0 if self.case_sensitive else re.IGNORECASE if not self.word_boundaries: - return re.compile(escaped, re.IGNORECASE) + return re.compile(escaped, flags) if classify_word_script(word) == "cjk": - return re.compile(escaped, re.IGNORECASE) - return re.compile(rf"\b{escaped}\b", re.IGNORECASE) + return re.compile(escaped, flags) + return re.compile(rf"\b{escaped}\b", flags)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/filters/filter.py` around lines 588 - 594, The replacement regex in `_get_replacement_regex` always uses `re.IGNORECASE`, which ignores the configured `case_sensitive` behavior. Update this helper so it compiles with case-insensitive matching only when `case_sensitive` is disabled, and otherwise preserves exact-case matching; keep the existing `word_boundaries` and `classify_word_script` logic intact while making the behavior consistent with detection.packages/py/glin_profanity/nlp/context_analyzer.py-278-283 (1)
278-283: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize phrase comparisons before applying positive overrides.
context_textis lowercased and punctuation-stripped, butmatch_wordmay preserve original casing, and phrases likebomb.comkeep punctuation. This makes capitalized matches or dotted whitelist phrases miss the override.Suggested fix
def _check_phrase_context( self, context_text: str, match_word: str ) -> ContextAnalysisResult | None: + normalized_match_word = re.sub( + r"[^\w]", "", match_word.lower(), flags=re.UNICODE + ) for phrase, score in POSITIVE_PHRASES.items(): - if match_word in phrase and phrase in context_text: + normalized_phrase = re.sub( + r"[^\w\s]", "", phrase.lower(), flags=re.UNICODE + ) + if normalized_match_word in normalized_phrase and normalized_phrase in context_text: return ContextAnalysisResult(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/nlp/context_analyzer.py` around lines 278 - 283, Normalize both sides of the phrase override check in _check_phrase_context before comparing: ensure match_word is lowercased and punctuation-stripped the same way as context_text, and compare against a normalized version of each POSITIVE_PHRASES key so dotted whitelist phrases like bomb.com still match. Keep the existing ContextAnalysisResult override path, but make the phrase membership test use the normalized values rather than the original phrase and match_word strings.packages/py/glin_profanity/data/dictionary.py-72-83 (1)
72-83: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate
wordsis a list before iterating it.
{"words": "bad"}currently caches["b", "a", "d"], and{"words": null}raises an uncaughtTypeError. Reject non-listwordsvalues before filtering entries.Proposed fix
if isinstance(data, dict) and "words" in data: raw_words = data["words"] elif isinstance(data, list): raw_words = data else: self._raise_format_error(filename) return + if not isinstance(raw_words, list): + self._raise_format_error(filename) + return # Guard against malformed entries (None/numbers/objects) so the🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/data/dictionary.py` around lines 72 - 83, The dictionary loader in `Dictionary` is treating any dict with a `words` key as valid, which lets non-list values like strings or null pass into the comprehension and causes bad caching or a `TypeError`. Update the parsing logic in the dictionary-loading path to explicitly verify `data["words"]` is a list before assigning it to `raw_words`, and fall back to `_raise_format_error(filename)` for any non-list `words` value. Keep the existing filtering of malformed entries after the type check so `self._dictionaries[language]` only stores valid strings.packages/js/src/utils/evasion.ts-10-29 (1)
10-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDecode entities before the final tag-strip pass.
<span>f</span>uckcurrently becomes<span>f</span>uckafter Line 24-Line 28, so encoded HTML wrappers still split the token and bypass the evasion-normalization path this PR is adding by default.Proposed fix
export function stripHtmlAndDecodeEntities(text: string): string { - let result = text.replace(/<[^>]*>/g, ''); + let result = text; result = result.replace(/&#(\d+);/g, (_, dec: string) => { const code = parseInt(dec, 10); return Number.isFinite(code) ? String.fromCodePoint(code) : _; }); @@ - return result + result = result .replace(/</gi, '<') .replace(/>/gi, '>') .replace(/&/gi, '&') .replace(/"/gi, '"') - .replace(/'/gi, "'"); + .replace(/'/gi, "'"); + + return result.replace(/<[^>]*>/g, ''); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/utils/evasion.ts` around lines 10 - 29, The entity decoding order in stripHtmlAndDecodeEntities is wrong, so encoded HTML wrappers can survive as literal tags and split tokens after normalization. Move the HTML-entity decoding in stripHtmlAndDecodeEntities before the final tag-stripping pass, then run the existing tag removal again on the decoded text so sequences like <span>...</span> are fully removed. Keep the numeric/entity handling and the named entity replacements in the same helper, but ensure the last step leaves no encoded wrapper tags behind.packages/js/src/utils/evasion.ts-13-20 (1)
13-20: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard invalid entity code points before
String.fromCodePoint.
parseIntcan return a finite value that is still outside the Unicode range, so inputs like�or�will throwRangeErrorhere and take down normalization on malformed user text.Proposed fix
+const decodeCodePoint = (code: number, fallback: string): string => + Number.isInteger(code) && code >= 0 && code <= 0x10ffff + ? String.fromCodePoint(code) + : fallback; + export function stripHtmlAndDecodeEntities(text: string): string { let result = text.replace(/<[^>]*>/g, ''); result = result.replace(/&#(\d+);/g, (_, dec: string) => { const code = parseInt(dec, 10); - return Number.isFinite(code) ? String.fromCodePoint(code) : _; + return decodeCodePoint(code, _); }); result = result.replace(/&`#x`([0-9a-fA-F]+);/g, (_, hex: string) => { const code = parseInt(hex, 16); - return Number.isFinite(code) ? String.fromCodePoint(code) : _; + return decodeCodePoint(code, _); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/utils/evasion.ts` around lines 13 - 20, The numeric entity decoding in evasion.ts can still throw in the replace handlers used by the normalization flow. Update the logic in the decimal and hex branches of the entity decoder to validate the parsed code point is within the Unicode range before calling String.fromCodePoint, and fall back to returning the original match for out-of-range values. Keep the fix localized to the existing replace callbacks so malformed inputs like `&`#1114112`;` and `&`#x110000`;` no longer break normalization.tests/cross_language_parity_test.py-25-37 (1)
25-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlways rebuild the JS bundle before running parity checks.
This fixture skips
npm run buildwheneverdist/index.cjsalready exists, so the suite can silently compare Python against a stale JS artifact from a previous checkout/build. That undermines the parity gate precisely when JS behavior changed.Suggested fix
`@pytest.fixture`(scope="session", autouse=True) def ensure_js_dist_built() -> None: - if JS_ENTRY.exists(): - return subprocess.run( ["npm", "run", "build"], cwd=JS_PACKAGE, check=True, capture_output=True, text=True, )If build time is a concern, gate on source-vs-dist mtimes instead of mere existence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/cross_language_parity_test.py` around lines 25 - 37, The ensure_js_dist_built fixture is skipping the JS build when JS_ENTRY already exists, which can leave cross_language_parity_test comparing against a stale bundle. Update ensure_js_dist_built to always run npm run build before the parity checks, or replace the existence check with a source-vs-dist mtime freshness check so the dist artifact is rebuilt whenever JS sources change.packages/py/tests/test_filter_pool.py-35-42 (1)
35-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis does not lock down FIFO vs hit-refreshed eviction.
get_pooled_filter()moves reused entries to the end before evicting, so both FIFO and LRU-like behavior pass this cold-overflow case. Re-access an old config before inserting the overflow item and assert which entry is supposed to survive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/tests/test_filter_pool.py` around lines 35 - 42, The current test only covers cold overflow, so it does not distinguish FIFO from hit-refreshed eviction in get_pooled_filter(). Update test_evicts_oldest_entry_when_pool_is_full in test_filter_pool.py to first re-access one of the earlier configs before inserting the extra config, then assert which specific pooled instance should survive eviction. Use get_pooled_filter() and the existing configs/instances setup to lock down the intended eviction policy unambiguously.packages/js/tests/leetspeak-unicode.test.ts-24-54 (1)
24-54: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd an explicit opt-out regression for evasion normalization.
These cases only prove the default-on path. The PR objective makes
enableEvasionNormalizationthe backward-compatibility escape hatch, so a bug wheref.u.c.k,a<br>ss, or masked forms still match after opt-out would pass this suite.Also applies to: 308-323
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/tests/leetspeak-unicode.test.ts` around lines 24 - 54, The current tests in stripHtmlAndDecodeEntities, collapseSeparatedCharacters, and normalizeEvasion only cover the default normalization behavior, so add a regression that explicitly disables enableEvasionNormalization and verifies these obfuscated forms are left unchanged or do not match. Update the leetspeak-unicode test suite to exercise the opt-out path through the same helpers and normalizeEvasion so backward-compatibility is covered.packages/js/tests/useProfanityChecker.test.tsx-11-16 (1)
11-16: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn distinct pooled filters from the mock.
Right now every
getPooledFilter()call shares the samemockFilterIsProfane, so this test cannot catch a stalefilterRef.currentafter config changes. The hook’s new behavior is specifically the ref refresh inuseEffect, and this mock shape makes that path unobservable.Also applies to: 123-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/tests/useProfanityChecker.test.tsx` around lines 11 - 16, The mock for getPooledFilter in useProfanityChecker.test.tsx is returning the same shared filter object each time, which hides whether filterRef.current is refreshed after config changes. Update the filterPool mock so each getPooledFilter() call can return a distinct pooled filter instance (with its own isProfane function or spy) and adjust the related tests around useProfanityChecker to assert the ref refresh behavior in useEffect rather than relying on one shared mock.packages/py/tests/test_aho_corasick_parity.py-17-42 (1)
17-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd the evasion-normalization opt-out to this parity matrix.
The new configs only exercise the default
enable_evasion_normalization=Truepath. Since that flag is the PR’s backward-compatibility contract, AC and legacy can drift when it is disabled without any failure here.Also applies to: 44-58, 86-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/tests/test_aho_corasick_parity.py` around lines 17 - 42, The parity matrix in FILTER_CONFIGS only covers the default enable_evasion_normalization=True behavior, so add at least one matching config variant with enable_evasion_normalization=False and assert parity for it in the Aho-Corasick tests. Update the existing test cases that use the shared filter sets and the legacy/AC comparison helpers so both code paths are exercised with the opt-out enabled, using the same symbols FILTER_CONFIGS, test fixtures, and parity assertions already in test_aho_corasick_parity.py.packages/py/tests/scanners/test_composite.py-96-101 (1)
96-101: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCover the
secretsbranch here too.
scan_all()gates redaction for bothSecretsScannerandPiiScanner, but this only asserts the PII result stays untouched. An accidental secrets redaction withvault=Nonewould still ship untested.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/tests/scanners/test_composite.py` around lines 96 - 101, Add coverage for the secrets path in the existing `test_vault_none_does_not_redact` case around `scan_all()`: right now it only checks the `pii` result, so an unintended `SecretsScanner` redaction with `vault=None` would go unnoticed. Update the test to also locate the `secrets` scanner result from `results` and assert its `sanitized` value remains unchanged when no vault is provided, alongside the existing `PiiScanner` assertion.benchmarks/run-branch-comparison.py-65-69 (1)
65-69: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon't treat an existing
node_modules/as “deps are current.”Line 67 skips installation whenever
packages/js/node_modulesexists. This PR adds a new JS runtime dependency, so rerunning the script from a pre-update checkout can benchmark against stale modules or fail on a missing import. Reinstall from the lockfile for the feature tree as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/run-branch-comparison.py` around lines 65 - 69, The build_js helper is incorrectly using the presence of packages/js/node_modules as a proxy for up-to-date dependencies. Update build_js so it reinstalls from the lockfile for the feature tree even when node_modules already exists, and keep the npm install step tied to the js_dir setup used by build_js before running npm run build.benchmarks/run-branch-comparison.py-72-91 (1)
72-91: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftProvision a Python environment per worktree before running these subprocesses.
These paths import code directly from each tree via
GLIN_REPO_ROOT, but they never install that tree's Python dependencies. This PR adds a new runtime dependency on the Python side, so the feature branch can fail—or worse, be compared under whatever site-packages happen to be installed on the host.Also applies to: 151-155, 168-176, 182-186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/run-branch-comparison.py` around lines 72 - 91, run_py_benchmark and the other benchmark subprocess helpers are using GLIN_REPO_ROOT from each worktree without first creating/installing that tree’s Python environment, so the feature branch can run against host site-packages instead of its own deps. Update the benchmark setup flow to provision a per-worktree Python environment before invoking the Python subprocesses, and make sure the same worktree-specific environment is used consistently in run_py_benchmark and the related benchmark calls mentioned in the comment.benchmarks/run-branch-comparison.py-119-122 (1)
119-122: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMatch the canonical shootout metric fallbacks.
Lines 119-122 use
0when the precision/recall denominators are empty, butbenchmarks/shootout/harness.mjs:104-124uses1for those degenerate cases. A branch that returns no hits will therefore get different precision/F1 here than in the committed shootout results.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/run-branch-comparison.py` around lines 119 - 122, Update the metric fallback logic in the branch comparison calculation so it matches the canonical shootout behavior. In the metrics block that computes precision, recall, f1, and fpr, change the degenerate-case fallback for precision and recall from zero to the same value used by the shootout harness, and let f1 follow from those values. Use the existing metric computation section in run_branch_comparison to keep branch-comparison results aligned with benchmarks/shootout/harness.mjs.benchmarks/compare-optimization-results.mjs-54-73 (1)
54-73: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the stale selection block and unused AC locals.
Lines 54-73 and 113-114 leave
keyRows,acBefore, andacAfterunused after switching toKEY_NAMES/summaryRows, so ESLint fails this file as written.Also applies to: 75-95, 109-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/compare-optimization-results.mjs` around lines 54 - 73, Remove the stale row-selection logic and unused autocomplete locals in compare-optimization-results.mjs: the key filtering block that creates keyRows is no longer needed now that KEY_NAMES and summaryRows are used, and the acBefore/acAfter variables should be deleted or repurposed in the main comparison flow. Update the relevant aggregation/reporting path so only the current symbols (KEY_NAMES, summaryRows, and the existing compare/render logic) remain referenced, preventing unused-variable lint errors.Source: Linters/SAST tools
benchmarks/optimization-comparison.mjs-118-138 (1)
118-138: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake torture-batch benchmark names unique per config.
measureTortureBatch()always emitstorture_set_60x_isProfane, but this file records that row for bothshootoutandcontext_aware.benchmarks/generate-comparison-report.mjskeys full results byname, so one row overwrites the other and Section 6 reports whichever config was pushed last. Include the config key in the benchmark name.Also applies to: 175-181
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/optimization-comparison.mjs` around lines 118 - 138, The torture-batch benchmark row name is hardcoded, so results from different configs overwrite each other in the comparison report. Update measureTortureBatch() to include the config key in the returned name so each row is unique, and make the callers in benchmarks/optimization-comparison.mjs pass that config identifier through when recording both shootout and context_aware results.benchmarks/optimization-comparison-lite.py-155-162 (1)
155-162: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThis legacy-mode detection is ineffective.
Filtertakes a config dict, so passing"disable_aho_corasick"will not fail withTypeErroron branches that simply ignore unknown keys. In that case this still benchmarks the normal path but writes it out asshootout_legacy_clean. Please verify the flag was honored instead of using exception-based feature detection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/optimization-comparison-lite.py` around lines 155 - 162, The legacy-mode detection in the benchmark setup is relying on a TypeError from Filter construction, but Filter({**shootout_config, "disable_aho_corasick": True}) may silently ignore unknown keys and still use the normal path. Update the logic around legacy_filter in optimization-comparison-lite.py to explicitly verify that the disable_aho_corasick setting is actually honored before labeling the result as shootout_legacy_clean, and only set has_ac_fast_path based on that verified behavior rather than exception handling.benchmarks/optimization-comparison-lite.mjs-159-172 (1)
159-172: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFix the legacy-path probe and row metadata.
This block can silently publish the wrong result:
catch {}converts anyFilterfailure into a fakeshootout_legacy_cleanrow, and the success branch labels the legacy run ashas_ac_fast_path: true. That makes the JSON ambiguous at best and incorrect at worst. Please make the probe explicit and record whether legacy mode was actually used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/optimization-comparison-lite.mjs` around lines 159 - 172, The legacy-path probe in the benchmark setup is ambiguous because the success branch and fallback row both use the same benchmark name while the metadata flag is inverted. Update the logic around the `Filter` construction and `measureIsProfane` calls so the row clearly records whether legacy mode was actually used, and make the `has_ac_fast_path` value match the mode that ran. Avoid a silent catch-all fallback that can mask unrelated `Filter` failures; use an explicit probe with distinct handling in the `shootout_legacy_clean` result assembly.
🟡 Minor comments (9)
packages/py/glin_profanity/utils/unicode.py-269-271 (1)
269-271: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
homoglyph_to_asciishould stay 1:1 or the span aligner needs to handle expansions.variant_mapping.pywalks one original char against one variant char, so entries like"œ" → "oe"and"Œ" → "OE"never line up and fall back to an empty span. Either keep this helper single-character only or extend the mapper to consume 1→N replacements.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/utils/unicode.py` around lines 269 - 271, `homoglyph_to_ascii` currently allows multi-character replacements like “œ”→“oe”, but `variant_mapping.py` and the span aligner assume a strict 1:1 character mapping. Update `homoglyph_to_ascii` in `unicode.py` to return only single-character ASCII equivalents, or adjust the downstream mapping logic in `variant_mapping.py`/the span aligner to explicitly support 1→N expansions so variant spans stay aligned.packages/js/src/utils/evasion.ts-37-39 (1)
37-39: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the escaped hyphen in this character class.
[\s._\-]trips ESLint'sno-useless-escapehere;-is already literal at the end of the class.Proposed fix
- /\b([a-zA-Z0-9@$!#*])(?:[\s._\-]+([a-zA-Z0-9@$!#*])){2,}\b/g; + /\b([a-zA-Z0-9@$!#*])(?:[\s._-]+([a-zA-Z0-9@$!#*])){2,}\b/g; - return text.replace(pattern, (match) => match.replace(/[\s._\-]+/g, '')); + return text.replace(pattern, (match) => match.replace(/[\s._-]+/g, ''));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/utils/evasion.ts` around lines 37 - 39, The regex in evasion’s text normalization uses an unnecessary escaped hyphen inside the character class, which triggers no-useless-escape. Update the replace pattern in the evasion utility so the character class treats the hyphen as literal without escaping it, while keeping the same matching behavior for whitespace, dots, underscores, and hyphens.Source: Linters/SAST tools
packages/py/glin_profanity/scanners/patterns/injection_patterns.py-398-400 (1)
398-400: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the word boundary on both role delimiters.
Line 400 makes the pattern case-insensitive, but the alternation still applies
\bonly toHUMAN. Inputs likefooASSISTANT:will now produce a spurious PI-034 hit even though the role marker is embedded inside another token.Suggested fix
- pattern=re.compile(r"\bHUMAN\s*:\s*|ASSISTANT\s*:\s*", _I), + pattern=re.compile(r"\b(?:HUMAN|ASSISTANT)\s*:\s*", _I),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/glin_profanity/scanners/patterns/injection_patterns.py` around lines 398 - 400, The PI-034 regex in InjectionPattern should keep a word boundary on both role delimiters, not just HUMAN. Update the pattern in injection_patterns.py so ASSISTANT is also guarded by a leading boundary in the same compiled expression, preventing embedded matches like fooASSISTANT: from triggering. Locate the change in the InjectionPattern definition for id "PI-034" and adjust the re.compile alternation accordingly.packages/js/src/scanners/patterns/injection-patterns.ts-294-296 (1)
294-296: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude
+in the PI-036 boundary class.Line 296 can still start or end a match in the middle of a larger Base64 token because
+is part of the payload alphabet but not part of the lookaround class. That means inputs like...AAAA+BBBB...— or any blob longer than 512 chars with a+near the cut point — can report a truncatedPI-036span instead of respecting the real token boundary.Suggested fix
- pattern: /(?<![a-zA-Z0-9/._-])([A-Za-z0-9+/]{40,512}={0,2})(?![a-zA-Z0-9/._-])/, + pattern: /(?<![A-Za-z0-9+/._=-])([A-Za-z0-9+/]{40,512}={0,2})(?![A-Za-z0-9+/._=-])/,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/scanners/patterns/injection-patterns.ts` around lines 294 - 296, The PI-036 Base64 pattern in injection-patterns.ts can still match inside a larger token because the lookaround boundary class omits `+`. Update the boundary logic in the Base64 regex used for PI-036 so `+` is treated as part of the surrounding token on both sides, keeping matches aligned with real Base64 blob boundaries even for long inputs and cut points. Refer to the Base64 pattern definition in the scanner patterns module when making the change.packages/js/tests/aho-corasick-parity.test.ts-79-85 (1)
79-85: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake the routing assertions check matcher state, not just output.
expect(fastFilter).toBeDefined()is vacuous, and the "legacy path" case only proves behavioral parity. Both tests still pass if the wrong engine runs underneath. AssertdictionaryMatcherdirectly so AC/legacy routing regressions are actually caught.Suggested fix
test('uses Aho-Corasick when eligible', () => { const expectsAc = (config.wordBoundaries ?? true) || Boolean(config.enableContextAware); if (expectsAc && !config.disableAhoCorasick) { - expect(fastFilter).toBeDefined(); + expect(fastFilter['dictionaryMatcher']).toBeTruthy(); } }); @@ test('legacy path is used when word boundaries are disabled', () => { @@ const fast = new Filter(config); const legacy = new Filter({ ...config, disableAhoCorasick: true }); const text = 'scunthorpe'; + expect(fast['dictionaryMatcher']).toBeNull(); expect(fast.isProfane(text)).toBe(legacy.isProfane(text)); });Also applies to: 112-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/tests/aho-corasick-parity.test.ts` around lines 79 - 85, The routing tests for Aho-Corasick parity are only checking that fastFilter exists or that outputs match, which can miss engine-selection regressions. Update the assertions in aho-corasick-parity.test.ts to inspect the matcher state directly by asserting dictionaryMatcher behavior in the eligible and legacy-path cases, using the existing test setup around fastFilter, config, and dictionaryMatcher so the tests verify which engine was actually chosen.packages/js/src/scanners/patterns/secret-patterns.ts-62-62 (1)
62-62: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDrop the redundant escapes so this file stays lint-clean.
ESLint is already flagging these regex literals with
no-useless-escape, so the updated pattern set will still fail lint as written.Suggested cleanup
- pattern: /(?:aws[_\-. ]?session[_\-. ]?token)['":\s=]+([A-Za-z0-9\/+=]{100,2048})/i, + pattern: /(?:aws[_\-. ]?session[_\-. ]?token)['":\s=]+([A-Za-z0-9/+=]{100,2048})/i, - pattern: /(?:firebase|fcm)[_\-. ]?(?:api[_\-. ]?)?key['":\s=]+([A-Za-z0-9_\-]{38,512})/i, + pattern: /(?:firebase|fcm)[_.- ]?(?:api[_.- ]?)?key['":\s=]+([A-Za-z0-9_-]{38,512})/i, - pattern: /sig=[A-Za-z0-9%+\/=]{20,512}(&|$)/, + pattern: /sig=[A-Za-z0-9%+/=]{20,512}(&|$)/,Apply the same cleanup to the other flagged character classes in this file.
Also applies to: 107-107, 140-140, 186-186, 225-225, 376-376, 495-495, 502-502, 520-520, 527-527, 534-534, 587-587, 594-594, 658-658, 718-718, 725-725, 732-732, 753-753, 848-848
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/js/src/scanners/patterns/secret-patterns.ts` at line 62, The secret-pattern regex literals in secret-patterns.ts contain unnecessary backslash escapes that trigger ESLint no-useless-escape. Update the affected pattern definitions in the secret-patterns collection, including the aws session token pattern and the other flagged regex entries in this file, by removing only the redundant escapes while keeping the matching behavior unchanged. Use the pattern constants/entries in the secret-pattern list as the reference points when cleaning up each character class.Source: Linters/SAST tools
packages/py/tests/scanners/test_prompt_injection.py-129-139 (1)
129-139: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExercise the right boundary as well.
textends immediately after the blob, so the(?!...)half of PI-036 never runs in this test. Wrap the payload with a trailing delimiter/word-like char too; otherwise a right-boundary regression will still pass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/py/tests/scanners/test_prompt_injection.py` around lines 129 - 139, The PI-036 span test only covers the left boundary, so it can miss regressions in the trailing lookahead. Update test_pi036_match_span_excludes_boundary_chars in test_prompt_injection.py to use a payload that has both a leading delimiter and a trailing delimiter/word-like character after the blob, then keep asserting the matched slice equals the blob and does not include either boundary character from PromptInjectionScanner.scan().README.md-89-95 (1)
89-95: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign this throughput claim with the shootout docs.
Line 89 says
~1,900 ops/sec, butbenchmarks/shootout/README.mdnow says~2,500for the same Node 22 torture-set snapshot. Please source both docs from the same committed results artifact so users do not get conflicting benchmark numbers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 89 - 95, The benchmark throughput in README.md is out of sync with the shootout docs, so align both from the same committed results artifact. Update the README’s glin-profanity performance claim to match the value used by benchmarks/shootout/README.md, or better, reference a single source of truth for the torture-set snapshot so the numbers stay consistent across both docs.benchmarks/generate-comparison-report.mjs-116-122 (1)
116-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCheck metric presence explicitly.
if (rs?.f1 && fs?.f1)treats a valid0F1 as “missing” and drops into the error row. Use!= nullchecks here so a failed run still renders as metrics instead of as an error payload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/generate-comparison-report.mjs` around lines 116 - 122, The metric gate in generateComparisonReport is treating valid zero values as missing, which sends failed-but-structured runs into the error row. Update the F1 presence check in the comparison table generation to use explicit null/undefined checks instead of truthiness, so the `rs` and `fs` metric fields render correctly even when `f1` is 0. Keep the existing metric rendering block and only adjust the condition around the `rs`/`fs` values.
Reject invalid numeric entities before chr/fromCodePoint and preserve them through html.unescape so malformed input cannot crash evasion normalization. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/py/glin_profanity/utils/evasion.py`:
- Around line 50-63: The HTML/entity normalization in
strip_html_and_decode_entities leaves entity-encoded tags intact because
html.unescape runs after the tag-stripping regex. Reorder the flow so named
entities are decoded before the final tag-removal pass, then preserve the
numeric-entity handling and remaining cleanup in strip_html_and_decode_entities
so inputs like f<b>u</b>ck are normalized to plain text.
- Line 13: The numeric-entity escape logic in evasion.py only matches lowercase
x, so uppercase hex entities can slip through and get decoded by the final
unescape step. Update _REMAINING_NUMERIC_ENTITY_PATTERN to match both x and X
(or use case-insensitive matching), and keep the existing escaping flow in the
helper that preserves entities before html.unescape so invalid values like
&`#XD800`; remain literal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2f3e263-1470-4b60-8f49-de183f36476c
📒 Files selected for processing (4)
packages/js/src/utils/evasion.tspackages/js/tests/leetspeak-unicode.test.tspackages/py/glin_profanity/utils/evasion.pypackages/py/tests/test_evasion.py
✅ Files skipped from review due to trivial changes (1)
- packages/js/src/utils/evasion.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/py/tests/test_evasion.py
- packages/js/tests/leetspeak-unicode.test.ts
…entities Decode </> before removing HTML tags to close entity-encoded tag evasion, and match &#X...; hex entities with safe bounds checking in PY/JS. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
This PR introduces an Aho-Corasick dictionary fast path, evasion normalization (HTML entities / separators / masking), and extensive PY/JS parity hardening on top of
release. Steady-state profanity checks are dramatically faster while torture-set accuracy improves from F1 75% → 98.8% with 0% FPR on the Python shootout benchmark.Public API remains backward compatible: existing
Filter,checkProfanity, andis_profane/isProfanesignatures are unchanged. Two optional config flags were added (enableEvasionNormalization/disableAhoCorasickand snake_case equivalents).Key changes
Performance
disable_aho_corasick: truefalls back to legacy regex)get_pooled_filter/create_filter_config/clear_filter_pool; JS: internal pool used bycheckProfanityenable_evasion_normalization)Detection quality
profane_wordscollection fixed on legacy fuzzy / context-aware paths\d+[A-Za-z]measurement tokens (fixes5m→smfalse positive)Testing & tooling
tests/cross_language_parity_test.pyas cross-language gatebenchmarks/run-branch-comparison.py,benchmarks/optimization-comparison-report.mdBenchmark highlights (vs
release@ a446a8f)shootout_cleanshootout_cleanTrade-off: Filter init with shootout config is slower (~28µs → ~6.3ms on Python) due to AC automaton build — mitigated by instance pooling and
cacheResults.Full report:
benchmarks/optimization-comparison-report.mdMigration notes
get_config()/getConfig()now exports a more complete config (languages, custom words, new flags)Test plan
pytest packages/py— 513 passednpm test -- --ciinpackages/jspython tests/cross_language_parity_test.pybenchmarks/results/branch-comparison.json)Reproduce benchmarks