Skip to content

Minor: Reuse the runtime email scan in the spellcheck URL guard - #1360

Merged
krickert merged 3 commits into
mainfrom
minor-dedup-url-email-classifier
Oct 3, 2026
Merged

krickert merged 3 commits into
mainfrom
minor-dedup-url-email-classifier

Conversation

@rzo1

@rzo1 rzo1 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Makes the runtime MailAddressScan @internal public and replaces the URL_LIKE regex in SpellCheckingCharSequenceNormalizer with a cursor scan that reuses it for the email branch. A differential test checks the new scan against the old pattern.

@krickert

krickert commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Made a few cosmetic edits, details below.

Changes:

  • isNumberLike uses the DOT constant and a new COMMA instead of character constants inline.
  • MailAddressScan @throws is worded "Thrown if" like the rest.
  • The test built a new normalizer per call, 50,000 times in the generated-token loop. One instance in setUp now. Also uses the imported Pattern and a single string for the emoji token.

detailed generated AI validated by me:

The guard matches the old URL_LIKE regex on all inputs tried: the PR's differential test over 50,000 generated tokens, plus 52 hand-picked tokens with plus tags, digits and hyphens in the domain, non-ASCII local parts and domains, single and doubled @, trailing punctuation, user@host URLs and addresses inside longer tokens, 0 differences. MailAddressScan.isAddress without the dot rule is wider (it accepts user@localhost and user@example.); isDottedMailAddress restores parity and both tokens are pinned on both levels. No regex left in opennlp-spellcheck main, so this clears the module from the #1348 list. runtime and spellcheck pass locally.

@mawiesne mawiesne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the PR. Changes lgtm. Just one minor editorial edit in the comment.

}

/**
* Tests whether a whole text is one email address.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Tests" should read "Checks" here.

() -> new SpellCheckingCharSequenceNormalizer((SymSpell) null));
}
private boolean numberLike(String core) {
return new SpellCheckingCharSequenceNormalizer(symSpell).isNumberLike(core);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

rzo1 and others added 3 commits October 3, 2026 13:48
MailAddressScan in opennlp-runtime is now public @internal with isAddress.
SpellCheckingCharSequenceNormalizer replaces its URL_LIKE regex with a
cursor scan that uses MailAddressScan.isAddress for the email branch.
… test

isNumberLike compares against the DOT and a new COMMA constant instead of
literals, the MailAddressScan @throws reads "Thrown if", and the normalizer
test builds one SpellCheckingCharSequenceNormalizer in setUp instead of one
per call in the generated-token loops. The test uses the imported Pattern and
one literal for the emoji token.

Signed-off-by: Kristian Rickert <kristian@apache.org>
@rzo1

rzo1 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed your comments. Thx @mawiesne

@rzo1
rzo1 force-pushed the minor-dedup-url-email-classifier branch from 8a6e636 to 554f60b Compare October 3, 2026 11:51
@krickert
krickert merged commit e3d644c into main Oct 3, 2026
10 checks passed
@rzo1
rzo1 deleted the minor-dedup-url-email-classifier branch October 3, 2026 19:58
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.

3 participants