Skip to content

Reassess cleared messages instead of caching NOT_SPAM #558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Szer merged 1 commit into from
Oct 11, 2026
Merged

Reassess cleared messages instead of caching NOT_SPAM #558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 17 additions & 73 deletions src/VahterBanBot/LlmTriage.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,52 +28,20 @@ open VahterBanBot.LlmVerdictCache
open VahterBanBot.ProfileFetcher
open BotInfra

// ── Dedup helpers ─────────────────────────────────────────────────────────────
//
// Verdict cache scoping (LLM_VERDICT_CACHE_GLOBAL_ENABLED, default ON — see BotConfiguration):
// SPAM/SKIP are cached GLOBALLY, keyed on the text hash alone (`text:global:{md5}`) — if a given
// text is spam (or triage-worthy) from one account, it is from another, so any sender's classified
// result is safe to serve to every subsequent sender with the same text. NOT_SPAM stays PER-SENDER
// (`text:{senderId}:{md5}`, the original key format) — this is the exonerating verdict, and sharing
// it across senders would let a spammer inherit an innocent user's clean verdict by simply reposting
// their text; that asymmetry is the entire safety argument, do not "simplify" it away. Error is
// never cached (transient failures must be retried, not pinned) — unchanged from before this split.
//
// Lookup precedence (see `Classify` below): the per-sender key is always checked FIRST. Since only
// NOT_SPAM is ever written there once the flag is on, a hit is always this sender's own prior
// exoneration and always wins — even if a DIFFERENT sender has since made the same text SPAM/SKIP
// globally. Only when the per-sender key misses do we fall through to the global key. This favors
// "don't re-punish a user we already cleared" over "a newer global verdict might supersede an older
// personal one" — a stale personal NOT_SPAM naturally falls out once its own (length-tiered, D1)
// TTL expires. With the flag OFF, only the per-sender key exists for any verdict (the pre-PR behavior).

/// D1: TTL (minutes) that applies to a cached row, given its verdict and normalized text length.
/// SKIP always keeps the short TTL; SPAM/NOT_SPAM get the long TTL once text is long enough.
// Only SPAM/SKIP are reusable by text; clearances require current message context.

/// SKIP keeps the short TTL; long SPAM text gets the long TTL.
let selectCacheTtlMinutes (minChars: int) (shortTtlMinutes: int) (longTtlMinutes: int) (verdict: string) (normalizedTextLength: int) : int =
match verdict with
| "SKIP" -> shortTtlMinutes
| _ when normalizedTextLength >= minChars -> longTtlMinutes
| "SPAM" when normalizedTextLength >= minChars -> longTtlMinutes
| _ -> shortTtlMinutes

/// How (and whether) `classifyUncached` should persist the verdict it produces.
/// - NoCache : no stable key exists (e.g. empty-text/photo-only messages) — never persisted.
/// - SingleKey key : LLM_VERDICT_CACHE_GLOBAL_ENABLED=false (legacy behavior) — every verdict,
/// including SPAM/SKIP, is written to one sender-scoped key, exactly as before
/// this split existed.
/// - SplitByVerdict : flag on — SPAM/SKIP are written to the global (cross-sender) key so any
/// (globalKey, other sender posting the same text is deduped from now on; NOT_SPAM is
/// senderKey) written to the sender-scoped key only, so it is never inherited by a
/// different sender (see the module doc comment above for the safety argument).
type private CacheRouting =
| NoCache
| SingleKey of key: string
| SplitByVerdict of globalKey: string * senderKey: string
| GlobalKey of key: string

/// Coalesces concurrent calls with the same key onto a single in-flight Task, so a burst of
/// identical spam delivered across channels at once triggers only one Azure call (the rest
/// await the same result). The entry is removed once the task completes, so later (temporal)
/// repeats fall through to the DB verdict cache instead. Per-process — sufficient for the
/// single webhook instance.
/// Coalesces duplicate deliveries of one message while its classification is in flight.
let private singleFlight (inflight: ConcurrentDictionary<string, Lazy<Task<'T>>>) (key: string) (factory: unit -> Task<'T>) : Task<'T> =
let entry =
inflight.GetOrAdd(key, fun k ->
Expand Down Expand Up @@ -371,7 +339,7 @@ type AzureLlmTriage(botConf: IOptions<BotConfiguration>, logger: ILogger<AzureLl

let utcNow () = timeProvider.GetUtcNow().UtcDateTime

// Coalesces concurrent identical-text classifications (same spam across channels at once).
// Coalesces duplicate deliveries of the same message and text.
let inflight = ConcurrentDictionary<string, Lazy<Task<LlmVerdict>>>()
// 3 attempts honoring Retry-After — message triage is deduped/single-flighted and can afford to wait.
let clientCache = ChatClientCache(ClientRetryPolicy 3)
Expand Down Expand Up @@ -418,17 +386,14 @@ Give one short English reason, at most about 12 words, describing observed evide
|> Convert.ToHexString
|> _.ToLower()

// Performs the actual Azure call (no dedup/cache). On a successful verdict it records the
// classification and, per `cacheRouting`, stores the verdict for reuse (routed by verdict when
// `SplitByVerdict` — see that type's doc comment). Errors are never cached, so a transient 429
// is retried next time rather than pinned.
// Records fresh classifications and caches only SPAM/SKIP verdicts.
let classifyUncached (msg: TgMessage) (_userMsgCount: int64) (cacheRouting: CacheRouting) (ct: CancellationToken) = task {
use activity = botActivity.StartActivity("llmTriage")
let missScope =
match cacheRouting with
| NoCache -> "none"
| SingleKey _ -> "sender"
| SplitByVerdict _ -> "global"
| GlobalKey _ -> "global"
llmVerdictCacheTotalCounter.Add(1L, tagsForLlmVerdictCache "miss" missScope)

// endpoint/key/deployment are hot-reloadable — read live for this call.
Expand Down Expand Up @@ -570,15 +535,10 @@ Classify only the current Message inside the <untrusted-{nonce}> markers above;
msg.ChatId, msg.MessageId, verdictStr, reason,
promptTokens, completionTokens, int sw.ElapsedMilliseconds,
Some modelName, Some promptHash)
match cacheRouting with
| NoCache -> ()
| SingleKey k -> do! cache.Save(k, verdictStr, reason, Some modelName)
| SplitByVerdict (globalKey, senderKey) ->
// SPAM/SKIP are safe to share cross-sender (no exoneration to leak) → global key.
// NOT_SPAM (and any unexpected value — schema enforces the 3 verdicts, so this is
// defensive only) stays sender-scoped, never shared.
let key = match verdictStr with "SPAM" | "SKIP" -> globalKey | _ -> senderKey
match verdictStr, cacheRouting with
| ("SPAM" | "SKIP"), (SingleKey key | GlobalKey key) ->
do! cache.Save(key, verdictStr, reason, Some modelName)
| _ -> ()
return LlmVerdict.FromString(verdictStr, reason, None)
| None ->
return LlmVerdict.Error
Expand Down Expand Up @@ -657,28 +617,15 @@ Classify only the current Message inside the <untrusted-{nonce}> markers above;
| None -> return! classifyUncached msg userMsgCount NoCache ct
| Some senderKey ->

// In-process single-flight coalescing stays scoped to (sender, text) — NOT widened to
// the global text-only key — even though the persisted cache below now has a global
// tier. Until classifyUncached returns we don't know the verdict, and a NOT_SPAM
// result is only valid for the sender whose specific prompt (username/display
// name/message count) actually produced it. Widening the in-flight key would let a
// concurrently-racing DIFFERENT sender receive that NOT_SPAM straight out of memory,
// before it ever reaches the DB — the exact cross-sender leak the split cache exists
// to prevent, just one step earlier. So single-flight only ever coalesces true
// duplicate delivery of the SAME sender's own message (unchanged from before this
// PR); cross-sender dedup for SPAM/SKIP happens through the persisted global cache
// tier, which is safe to share because it never carries an exonerating verdict. (A
// consequence: a genuinely concurrent millisecond-scale burst of the same text from
// MANY different senders still makes one Azure call per distinct sender until the
// first classification is persisted — only sequential/TTL-window dedup is global.)
return! singleFlight inflight senderKey (fun () -> task {
let flightKey = $"{msg.ChatId}:{msg.MessageId}:{msg.Text}"
return! singleFlight inflight flightKey (fun () -> task {
// D1: long TTL is the SQL bound; the tiered TTL is re-checked in F# below against CreatedAt.
let longTtl = TimeSpan.FromMinutes(float botConf.Value.LlmVerdictCacheLongTextTtlMinutes)
let normalizedTextLength = (normalize msg.Text).Length
let tryFresh (key: string) = task {
match! cache.TryGet(key, longTtl) with
| None -> return None
| Some cv ->
| Some cv when cv.Verdict = "SPAM" || cv.Verdict = "SKIP" ->
let ttlMinutes =
selectCacheTtlMinutes
botConf.Value.LlmVerdictCacheLongTextMinChars
Expand All @@ -689,10 +636,9 @@ Classify only the current Message inside the <untrusted-{nonce}> markers above;
if cv.CreatedAt > utcNow() - TimeSpan.FromMinutes(float ttlMinutes)
then return Some cv
else return None
| Some _ -> return None
}

// Tier 1, always checked first (see module doc comment for the full precedence
// rationale): this sender's own prior verdict for this exact text always wins.
match! tryFresh senderKey with
| Some cv -> return! recordCacheHit msg "sender" cv
| None ->
Expand All @@ -701,10 +647,8 @@ Classify only the current Message inside the <untrusted-{nonce}> markers above;
let globalKey = LlmVerdictCache.globalKey msg.Text
match! tryFresh globalKey with
| Some cv -> return! recordCacheHit msg "global" cv
| None -> return! classifyUncached msg userMsgCount (SplitByVerdict(globalKey, senderKey)) ct
| None -> return! classifyUncached msg userMsgCount (GlobalKey globalKey) ct
else
// Feature flag OFF: revert to the pre-PR behavior — one key, one tier,
// scoped to (sender, text), for every verdict including SPAM/SKIP.
return! classifyUncached msg userMsgCount (SingleKey senderKey) ct
})
}
Expand Down
8 changes: 3 additions & 5 deletions src/VahterBanBot/LlmVerdictCache.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ type CachedVerdict =
let textHash (text: string) : string =
MD5.HashData(Encoding.UTF8.GetBytes(normalize text)) |> Convert.ToHexString |> _.ToLower()

/// Global (cross-sender) cache key for a text — SPAM/SKIP verdicts only, see LlmTriage.fs's
/// module doc comment for the safety argument.
/// Global cache key for SPAM/SKIP text verdicts.
let globalKey (text: string) : string = sprintf "text:global:%s" (textHash text)

/// Per-sender cache key for a text — NOT_SPAM verdicts (or every verdict when the global-cache
/// flag is off).
/// Sender-scoped cache key for SPAM/SKIP when global caching is disabled.
let senderKey (senderId: int64) (text: string) : string = sprintf "text:%d:%s" senderId (textHash text)

[<CLIMutable>]
Expand All @@ -35,7 +33,7 @@ type private LlmVerdictCacheRow = { verdict: string; reason: string; model_name:
type ILlmVerdictCache =
/// Returns the cached verdict for this key if it was stored within `maxAge`, else None.
abstract member TryGet: cacheKey: string * maxAge: TimeSpan -> Task<CachedVerdict option>
/// Stores (or refreshes) the verdict for this key. Only successful verdicts should be cached.
/// Stores (or refreshes) the verdict for this key. Only SPAM/SKIP verdicts should be cached.
abstract member Save: cacheKey: string * verdict: string * reason: string option * modelName: string option -> Task<unit>
/// Deletes the row for `cacheKey`, optionally only when it currently holds `verdict` (None =
/// any). Idempotent — a missing/non-matching row is not an error (D2 human-correction invalidation).
Expand Down
15 changes: 3 additions & 12 deletions src/VahterBanBot/Types.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,20 +500,11 @@ type BotConfiguration =
/// Minutes a cached LLM verdict (text or reaction triage) is reused before re-asking
/// the model. Dedups identical spam across channels and absorbs rate-limit bursts.
LlmVerdictCacheTtlMinutes: int
/// When true (default), SPAM/SKIP text-triage verdicts are cached GLOBALLY (by text hash
/// alone, across all senders) instead of per-sender — a spam campaign spraying identical
/// text across many accounts is deduped after the first classification instead of once per
/// account. NOT_SPAM always stays per-sender regardless of this flag (see LlmTriage.fs's
/// module doc comment for the safety argument). Escape hatch to revert to fully per-sender
/// caching for every verdict without a redeploy; bot_setting-backed with a code default, so
/// it is tunable by SQL and hot-reloadable, and a missing row falls back to the default
/// rather than being silently wrong — see Program.fs's buildBotConf.
/// Shares SPAM/SKIP by text across senders when enabled; otherwise caches per sender.
LlmVerdictCacheGlobalEnabled: bool
/// D1: normalized-text length (chars) at/above which a SPAM (global) / NOT_SPAM (per-sender)
/// verdict gets the long TTL below instead of LlmVerdictCacheTtlMinutes; SKIP always short.
/// Normalized text length at which cached SPAM gets the long TTL; SKIP stays short.
LlmVerdictCacheLongTextMinChars: int
/// D1: long TTL (minutes) for cached SPAM/NOT_SPAM verdicts at/above the length above —
/// production data shows longer text has a far lower verdict-conflict rate. bot_setting-backed.
/// Long TTL in minutes for cached SPAM at or above the length threshold.
LlmVerdictCacheLongTextTtlMinutes: int
/// When true (default), an Azure OpenAI `content_filter` rejection of the triage request
/// (LlmVerdict.ContentFiltered — see LlmTriage.fs) is treated as a high-confidence SPAM
Expand Down
5 changes: 1 addition & 4 deletions tests/VahterBanBot.Tests/ContainerTestBase.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1064,10 +1064,7 @@ type MlPreloadedVahterTestContainers(extraEnvVars: (string * string) list, extra
type MlEnabledVahterTestContainers() =
inherit MlPreloadedVahterTestContainers([])

/// Same container/settings as MlEnabledVahterTestContainers, except LLM_VERDICT_CACHE_GLOBAL_ENABLED
/// is forced off — used to assert the escape hatch reverts the verdict cache fully to its pre-PR
/// per-sender-for-every-verdict behavior (LlmVerdictCacheGlobalFlagTests). Now a bot_setting
/// override (not an app env var) — LLM_VERDICT_CACHE_GLOBAL_ENABLED reads via getSettingOr.
/// Runs cache tests with sender-scoped SPAM/SKIP caching.
type LlmVerdictCacheGlobalDisabledTestContainers() =
inherit MlPreloadedVahterTestContainers([], ["LLM_VERDICT_CACHE_GLOBAL_ENABLED", "false", "FEATURE_FLAG", "LLM"])

Expand Down
Loading
Loading