Hoist NLU classifier call out of body.links loop in link_mailto_placeholder#4802
Open
thunderkatz wants to merge 3 commits into
Open
Hoist NLU classifier call out of body.links loop in link_mailto_placeholder#4802thunderkatz wants to merge 3 commits into
thunderkatz wants to merge 3 commits into
Conversation
…holder
The ml.nlu_classifier(body.current_thread.text) call doesn't depend on the
looped link, so nesting it inside any(body.links, ...) means the call site
is reached once per qualifying link instead of once overall, and the
memoized-call lookup does a full-string compare against current_thread.text
on every hit. Restructuring via any(P) or (any(Q) and C) == any(P or (Q and C))
(valid since C doesn't depend on the loop variable) moves the NLU check to a
single top-level any(), evaluated once and only when no @{domain} match
exists anywhere in the message.
github-actions
Bot
added
the
review-needed
Contributor
Shared Samples Sync - Action RequiredThis PR was not automatically synced to shared-samples because the author is not a member of the To enable syncing, an organization member can comment Once triggered, the rules will be synced on the next scheduled run (every 10 minutes). |
zoomequipd
previously approved these changes
Jul 6, 2026
zoomequipd
left a comment
zoomequipd
left a comment
Member
There was a problem hiding this comment.
Approving without typical hunts due to performance concerns with running the hunts.
@thunderkatz has validated this change should have no efficacy impact.
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
2 participants
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.
Description
The classifier check on
body.current_thread.textdoesn't depend on link content, so nesting it insideany(body.links, ...)means the call site is reached once per qualifying link instead of once overall, and even though it's cached, it's not free. Hoisting the loop-independent check out of the loop saves that. Note that this requires splitting into twoanys, since it was previouslyany(P or (Q and C), so now it'sany(P) or (any(Q) and C)(the scheme check is duplicated in both loops but is significantly cheaper than the cached nlu call).Associated samples
n/a - this is a no-op optimization
Associated hunts
n/a
Screenshot (insights)
n/a