fix(core): preserve natural-language direction evidence - #49
Merged
Conversation
CodeinScrubs
force-pushed
the
agent/fix-natural-language-evidence
branch
2 times, most recently
from
August 13, 2026 10:30
e02d770 to
5345a30
Compare
CodeinScrubs
force-pushed
the
agent/fix-natural-language-evidence
branch
from
August 13, 2026 11:10
5345a30 to
917ee66
Compare
CodeinScrubs
marked this pull request as ready for review
August 13, 2026 12:02
CodeinScrubs
added a commit
that referenced
this pull request
Aug 13, 2026
## What changed - replaces the raw-text math delimiter regex with a single forward scanner for `$...$`, `$$...$$`, `$$$$`, and `\(...\)` spans - adds adversarial one-line and multi-line unmatched-`\(` performance regressions - rewrites UTF-16/code-point offset mapping with bounded typed-array `fill` calls - restricts Unicode downloader writes to the two repository-owned, version-pinned paths after exact SHA-256 verification - updates the security/performance documentation and adds a core patch changeset ## Root cause and security impact CodeQL identified polynomial regex work on library input. A deterministic timing harness reproduced it: repeated unmatched `\(` input grew from about 1.9 ms at 2,000 UTF-16 units to about 845 ms at 64,000 units, close to quadratic behavior. BidiLens processes untrusted chat/model output, so this can become an application-level denial-of-service path. The typed-array property and Unicode download alerts are defense-in-depth hardening: offsets are already derived from bounded string iteration, and Unicode URLs/paths/checksums are repository constants. The new structure expresses those invariants directly instead of relying on analyzer inference. ## Correctness evidence - differential harness compared the new scanner with the previous regex on 2,396,745 exhaustive inputs over `$`, `\\`, parentheses, text, spaces, LF, and CR; outputs were identical - valid math, empty display math, newline boundaries, and later `$...$` after an unmatched `\(` have direct tests - `pnpm run check` passed on the final compact scanner: 16 files / 401 tests, 932-case corpus, docs, package builds, and bundled Action - the rebased combined branch passed the focused core suite and emits 124,509 bytes under main's unchanged 124 KiB budget - exact Unicode 17.0.0 reproducibility, typecheck, lint, docs, Action bundle, and `git diff --check` passed ## Performance evidence After the fix, representative local timings were: - 128,000 UTF-16 units of one-line unmatched `\(`: 12.6 ms - 96,002 units across 32,000 unmatched lines plus a late closer: 19.1 ms - 80,000 mixed unmatched `\(` and valid `$x$` spans: 17.7 ms These are regression measurements, not universal latency promises. ## Review boundary This is intentionally separate from PR #49's natural-language evidence change. The branch is rebased on merged PR #49, including its changelog evidence, and exact hosted CI is rerunning on the final combined commit; the prior JavaScript CodeQL analysis reported zero results for the same security diff. No alert will be dismissed merely to make the dashboard green.
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.
What changed - keeps ordinary English compounds such as
well-knownandstate-of-the-artas natural-language LTR evidence instead of discarding every hyphenated token as code - distinguishes uppercase prose (PLEASE READ THIS WARNING) from short acronyms using deterministic block context; short all-capital phrases such asHTTP APIremain acronym-shaped - preserves known technical tokens and hyphenated products such asHTTP,API,react-markdown, version strings, and caller-provided identifiers - keeps caller-supplied .NET technical identifiers case-insensitive regardless of the caller's set comparer - mirrors the direction policy across TypeScript batch and streaming APIs, Kotlin/Android, Swift/Apple, Rust, and C#/.NET - adds two canonical fixtures, bringing the shared corpus to 932 cases, plus direct platform regression tests - documents the unavoidable acronym/product-name heuristic boundary ## Root cause The technical-token filter treated any hyphen and every all-capital word as an identifier signal. Because those exclusions removed only Latin LTR evidence, ordinary English prose could be silently biased toward RTL when mixed with Persian, Arabic, or another RTL script. The first uppercase fix also needed a tighter boundary so an acronym-only phrase was not mistaken for emphasized prose. ## Developer impact Pure LTR input remains a no-op. Existing known technical terms and short acronym-only phrases remain excluded from language-majority decisions. Mixed blocks now retain genuine prose evidence, so base direction and opposite-direction isolation are more accurate without changing alignment policy or mutating source text. ## Tradeoff Shape alone cannot perfectly distinguish every acronym, emphasized word, and product name. The rule is deterministic and conservative: uppercase prose requires at least one capitalized word longer than the acronym bound, and ambiguous domain-specific names remain configurable throughtechnicalIdentifiersor explicit direction. ## Validation on current head416022d-pnpm run check— 16 files / 400 tests; 92.00% statements, 85.92% branches, 94.77% functions, 94.84% lines; 932-case corpus, docs, all packages, and Action bundle passed - JavaScript focused regression — 113 tests passed; character-by-character stream probes matched batch for acronym-only, mixed-case acronym, uppercase-prose, and hyphenated-prose cases - Android:core:testpassed with JDK 21 - Windows native verification passed 1,166 assertions across all 932 fixtures (run locally on .NET 10 with major runtime roll-forward because exact CI-pinned .NET SDK 8.0.423 is not installed here) -git diff --checkpassed - exact hosted CI passed all 18 CI jobs and all five CodeQL language jobs on this commit; the PR is ready for review ## Security and language review The new Persian fixtures are generated policy evidence and are explicitly not native-speaker-certified. Source text remains in logical order, pure-LTR behavior remains on the identity path, and no new bidi control interpretation is introduced.