From 78109589ef32d4f6854ce2fab444e70206b8190b Mon Sep 17 00:00:00 2001 From: Ayrat Hudaygulov Date: Sun, 11 Oct 2026 23:56:06 +0100 Subject: [PATCH] Reassess cleared messages instead of caching NOT_SPAM --- src/VahterBanBot/LlmTriage.fs | 90 ++++-------------- src/VahterBanBot/LlmVerdictCache.fs | 8 +- src/VahterBanBot/Types.fs | 15 +-- tests/VahterBanBot.Tests/ContainerTestBase.fs | 5 +- .../LlmTriageResilienceTests.fs | 95 +++++++++++-------- tests/VahterBanBot.Tests/LlmTriageTests.fs | 4 - .../LlmVerdictCacheGlobalFlagTests.fs | 29 +++--- .../LlmVerdictCacheTieredTtlTests.fs | 4 +- 8 files changed, 97 insertions(+), 153 deletions(-) diff --git a/src/VahterBanBot/LlmTriage.fs b/src/VahterBanBot/LlmTriage.fs index 45fd5518..a53fa5ac 100644 --- a/src/VahterBanBot/LlmTriage.fs +++ b/src/VahterBanBot/LlmTriage.fs @@ -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>>) (key: string) (factory: unit -> Task<'T>) : Task<'T> = let entry = inflight.GetOrAdd(key, fun k -> @@ -371,7 +339,7 @@ type AzureLlmTriage(botConf: IOptions, logger: ILogger>>() // 3 attempts honoring Retry-After — message triage is deduped/single-flighted and can afford to wait. let clientCache = ChatClientCache(ClientRetryPolicy 3) @@ -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. @@ -570,15 +535,10 @@ Classify only the current Message inside the 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 @@ -657,28 +617,15 @@ Classify only the current Message inside the 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 @@ -689,10 +636,9 @@ Classify only the current Message inside the 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 -> @@ -701,10 +647,8 @@ Classify only the current Message inside the 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 }) } diff --git a/src/VahterBanBot/LlmVerdictCache.fs b/src/VahterBanBot/LlmVerdictCache.fs index 673d27b8..ddf89188 100644 --- a/src/VahterBanBot/LlmVerdictCache.fs +++ b/src/VahterBanBot/LlmVerdictCache.fs @@ -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) [] @@ -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 - /// 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 /// 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). diff --git a/src/VahterBanBot/Types.fs b/src/VahterBanBot/Types.fs index ff2cf3ff..596c385e 100644 --- a/src/VahterBanBot/Types.fs +++ b/src/VahterBanBot/Types.fs @@ -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 diff --git a/tests/VahterBanBot.Tests/ContainerTestBase.fs b/tests/VahterBanBot.Tests/ContainerTestBase.fs index cc5b1cf0..fe055571 100644 --- a/tests/VahterBanBot.Tests/ContainerTestBase.fs +++ b/tests/VahterBanBot.Tests/ContainerTestBase.fs @@ -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"]) diff --git a/tests/VahterBanBot.Tests/LlmTriageResilienceTests.fs b/tests/VahterBanBot.Tests/LlmTriageResilienceTests.fs index a701c859..d4d2e955 100644 --- a/tests/VahterBanBot.Tests/LlmTriageResilienceTests.fs +++ b/tests/VahterBanBot.Tests/LlmTriageResilienceTests.fs @@ -2,20 +2,12 @@ module VahterBanBot.Tests.LlmTriageResilienceTests open System open System.Threading.Tasks +open System.Text.Json +open VahterBanBot.LlmVerdictCache open VahterBanBot.Tests.ContainerTestBase open BotTestInfra open Xunit -/// Thorough LLM-triage tests for the verdict cache, concurrent single-flight dedup, and 429 -/// retry/backoff. They assert ONLY on observable outcomes — the exact number of Azure -/// chat-completion calls, auto-deletion, the recorded verdict, and whether a message leaked to -/// the vahter action channel — so the same tests are meaningful on the un-fixed master branch, -/// where they go red: -/// • dedup / single-flight tests → master makes N Azure calls, the fix makes 1. -/// • 429-retry tests → master makes 1 call then leaks to the action channel; -/// the fix retries (>=2 calls) and handles the message. -/// -/// Test names start with "LLM triage" to share the ML-enabled container's name-ordering band. type LlmTriageResilienceTests(fixture: MlEnabledVahterTestContainers, _ml: MlAwaitFixture) = // One scripted Azure rate-limit response. The bot does not parse a non-2xx body, so its @@ -137,8 +129,6 @@ type LlmTriageResilienceTests(fixture: MlEnabledVahterTestContainers, _ml: MlAwa let! calls = fixture.GetAzureLlmCalls() Assert.Equal(1, calls.Length) - // NOT_SPAM (the other same-sender cache case) posts no channel message to check, so this - // SKIP case is the stand-in coverage for a cache-served report's channel-text annotation. let! sends = fixture.GetFakeCalls "sendMessage" let toPotential = sends |> Array.filter (fun c -> c.Body.Contains $"\"chat_id\":{fixture.PotentialSpamChannel.Id}") Assert.True(toPotential |> Array.exists (fun c -> c.Body.Contains "cached/global"), "cache-served SKIP report must be marked in the channel text") @@ -146,9 +136,6 @@ type LlmTriageResilienceTests(fixture: MlEnabledVahterTestContainers, _ml: MlAwa [] let ``LLM triage does NOT dedup different senders' NOT_SPAM: a second sender with identical text still gets its own Azure call`` () = task { - // The safety property the split cache depends on: NOT_SPAM stays per-sender, so sharing it - // across senders can never let a spammer inherit an innocent user's clean verdict just by - // reposting their exact text. Neither user's firstName contains "kill"/"spam" → NOT_SPAM. do! resetFakes () let a = Tg.user() let b = Tg.user() @@ -170,23 +157,52 @@ type LlmTriageResilienceTests(fixture: MlEnabledVahterTestContainers, _ml: MlAwa } [] - let ``LLM triage caches NOT_SPAM per-sender even with the global flag ON: the same sender's repeat is Tier-1 sender-first`` () = task { + let ``LLM triage reassesses repeated cleared messages with updated cross-chat history`` () = task { do! resetFakes () - let a = Tg.user() - - let m1 = Tg.quickMsg(chat = fixture.ChatsToMonitor[0], text = "77", from = a) - let! _ = fixture.SendMessage m1 - let! v1 = fixture.TryGetLlmTriageVerdict m1.Message.Value - Assert.Equal(Some "NOT_SPAM", v1) - - let m2 = Tg.quickMsg(chat = fixture.ChatsToMonitor[1], text = "77", from = a) - let! _ = fixture.SendMessage m2 + let sender = Tg.user() + let text = String.replicate 17 "77 " |> _.TrimEnd() + let response verdict : AzureScriptedResponse = + { status = 200 + body = JsonSerializer.Serialize({| + choices = [| {| finish_reason = "stop"; index = 0 + message = {| role = "assistant"; content = JsonSerializer.Serialize({| verdict = verdict; reason = "scripted verdict" |}) |} |} |] + created = 1774736361; id = "chatcmpl-reassessment"; model = "fake"; ``object`` = "chat.completion" + usage = {| completion_tokens = 10; prompt_tokens = 200; total_tokens = 210 |} |}) + delayMs = 0; errorMode = "" } + do! fixture.SetAzureLlmScript [| response "NOT_SPAM"; response "NOT_SPAM"; response "SPAM" |] + let messages = + [| for i in 0..2 -> Tg.quickMsg(chat = fixture.ChatsToMonitor[i % 2], text = text, from = sender) |] + for i in 0..2 do + let! _ = fixture.SendMessage messages[i] + let! deleted = fixture.MessageIsAutoDeleted messages[i].Message.Value + Assert.Equal((i = 2), deleted) + let! calls = fixture.GetAzureLlmCalls() + Assert.Equal(3, calls.Length) + for i in 0..2 do + use body = JsonDocument.Parse(calls[i].Body) + let prompt = body.RootElement.GetProperty("messages").[1].GetProperty("content").GetString() + Assert.Contains($"Total messages seen from this user: {i} ", prompt) + if i > 0 then + Assert.Contains($"Identical message seen earlier: {i} times", prompt) + Assert.Contains($"Chat {messages[i-1].Message.Value.Chat.Id},", prompt) + } + [] + [] + [] + let ``LLM triage ignores persisted NOT_SPAM entries`` (globalEntry: bool) = task { + do! resetFakes () + let sender = Tg.user(firstName = "kill persisted-clearance") + let text = "77" + let cache = LlmVerdictCacheRepository(fixture.DbConnectionString) :> ILlmVerdictCache + let key = if globalEntry then globalKey text else senderKey sender.Id text + do! cache.Save(key, "NOT_SPAM", Some "earlier clearance", None) + let msg = Tg.quickMsg(chat = fixture.ChatsToMonitor[0], text = text, from = sender) + let! _ = fixture.SendMessage msg + let! deleted = fixture.MessageIsAutoDeleted msg.Message.Value + Assert.True(deleted) let! calls = fixture.GetAzureLlmCalls() Assert.Equal(1, calls.Length) - - let! cacheHit = fixture.TryGetLlmVerdictCacheHit m2.Message.Value - Assert.Equal(Some ("NOT_SPAM", Some "keyword match: none", "sender"), cacheHit) } [] @@ -216,25 +232,22 @@ type LlmTriageResilienceTests(fixture: MlEnabledVahterTestContainers, _ml: MlAwa // ── Concurrency (single-flight) ─────────────────────────────────────────── [] - let ``LLM triage collapses a concurrent multi-channel burst into ONE Azure call`` () = task { - // The literal scenario: a spammer posts the same text to several channels at once and the - // webhooks arrive concurrently. Single-flight must coalesce them onto one Azure call. - // On master each of the 5 fires its own call. + let ``LLM triage classifies concurrent cleared copies independently`` () = task { do! resetFakes () - let spammer = Tg.user(firstName = "kill concurrent-burst") + let sender = Tg.user() + let delayedResponse: AzureScriptedResponse = + { status = 200 + body = """{"choices":[{"finish_reason":"stop","index":0,"message":{"content":"{\"verdict\":\"NOT_SPAM\",\"reason\":\"no promotion\"}","role":"assistant"}}],"created":1774736361,"id":"chatcmpl-delayed","model":"fake","object":"chat.completion","usage":{"completion_tokens":10,"prompt_tokens":200,"total_tokens":210}}""" + delayMs = 1000; errorMode = "" } + do! fixture.SetAzureLlmScript(Array.create 5 delayedResponse) let msgs = - [ for i in 0 .. 4 -> Tg.quickMsg(chat = fixture.ChatsToMonitor[i % 2], text = "77", from = spammer) ] - - // Fire all webhooks concurrently. + [ for i in 0 .. 4 -> Tg.quickMsg(chat = fixture.ChatsToMonitor[i % 2], text = "77", from = sender) ] let! _ = Task.WhenAll(msgs |> List.map (fun m -> fixture.SendMessage m)) - let! calls = fixture.GetAzureLlmCalls() - Assert.Equal(1, calls.Length) - - // Every copy is still handled (deleted) from the shared verdict. + Assert.Equal(5, calls.Length) for m in msgs do let! deleted = fixture.MessageIsAutoDeleted m.Message.Value - Assert.True(deleted, "each concurrent copy should be auto-deleted") + Assert.False(deleted) } // ── Unhappy path: 429 retry ─────────────────────────────────────────────── diff --git a/tests/VahterBanBot.Tests/LlmTriageTests.fs b/tests/VahterBanBot.Tests/LlmTriageTests.fs index 514aec37..da30c97e 100644 --- a/tests/VahterBanBot.Tests/LlmTriageTests.fs +++ b/tests/VahterBanBot.Tests/LlmTriageTests.fs @@ -59,10 +59,6 @@ type LlmTriageTests(fixture: MlEnabledVahterTestContainers, _ml: MlAwaitFixture) [] let ``LLM triage NOT_SPAM verdict takes no action`` () = task { - // Global SPAM/SKIP cache — see the SPAM test above. This sender is always fresh (random - // Guid firstName), so the risk here isn't cross-sender NOT_SPAM leakage (impossible — NOT_SPAM - // stays per-sender) but a STALE SPAM/SKIP entry from an earlier test's "77" leaking in via the - // global tier and pre-empting this message's own (correct) NOT_SPAM classification. do! fixture.ClearLlmVerdictCache() // Default user firstName is a Guid — no keywords → NOT_SPAM let msgUpdate = Tg.quickMsg(chat = fixture.ChatsToMonitor[0], text = "77") diff --git a/tests/VahterBanBot.Tests/LlmVerdictCacheGlobalFlagTests.fs b/tests/VahterBanBot.Tests/LlmVerdictCacheGlobalFlagTests.fs index f9552e45..841c8a1f 100644 --- a/tests/VahterBanBot.Tests/LlmVerdictCacheGlobalFlagTests.fs +++ b/tests/VahterBanBot.Tests/LlmVerdictCacheGlobalFlagTests.fs @@ -1,19 +1,10 @@ module VahterBanBot.Tests.LlmVerdictCacheGlobalFlagTests open VahterBanBot.Tests.ContainerTestBase +open VahterBanBot.LlmVerdictCache open BotTestInfra open Xunit -/// Exercises the LLM_VERDICT_CACHE_GLOBAL_ENABLED=false escape hatch against a SEPARATE -/// container (LlmVerdictCacheGlobalDisabledTestContainers), since the flag is fixed for the -/// whole container's lifetime via a seeded bot_setting row rather than being flipped mid-run. -/// (The flag itself is a hot-reloadable bot_setting in production — see Program.fs's -/// buildBotConf — this test just avoids racing it against other tests sharing a container.) -/// -/// With the flag off, the verdict cache reverts fully to its pre-PR behavior: every verdict, -/// including SPAM/SKIP, is scoped per-sender only — two different senders with identical text -/// are NEVER deduped. This is exactly the property LlmTriageResilienceTests narrows to NOT_SPAM -/// only when the flag is on (default). type LlmVerdictCacheGlobalFlagTests(fixture: LlmVerdictCacheGlobalDisabledTestContainers, _ml: MlAwaitFixture) = let resetFakes () = task { @@ -45,8 +36,6 @@ type LlmVerdictCacheGlobalFlagTests(fixture: LlmVerdictCacheGlobalDisabledTestCo [] let ``LLM verdict cache global flag off: the same sender's repeated identical text is still cached`` () = task { - // The per-sender tier itself must keep working with the flag off — this is a sanity check - // that "flag off" means "revert to legacy scoping", not "cache disabled entirely". do! resetFakes () let a = Tg.user(firstName = "kill flagoff-repeat") @@ -71,4 +60,20 @@ type LlmVerdictCacheGlobalFlagTests(fixture: LlmVerdictCacheGlobalDisabledTestCo Assert.True(toDetected |> Array.exists (fun c -> c.Body.Contains "cached/sender"), "sender-tier cache hit must be marked in the channel text") } + [] + let ``LLM verdict cache global flag off: repeated NOT_SPAM is reclassified`` () = task { + do! resetFakes () + let sender = Tg.user() + let text = "77" + let cache = LlmVerdictCacheRepository(fixture.DbConnectionString) :> ILlmVerdictCache + do! cache.Save(senderKey sender.Id text, "NOT_SPAM", None, None) + for chat in fixture.ChatsToMonitor |> List.take 2 do + let msg = Tg.quickMsg(chat = chat, text = text, from = sender) + let! _ = fixture.SendMessage msg + let! deleted = fixture.MessageIsAutoDeleted msg.Message.Value + Assert.False(deleted) + let! calls = fixture.GetAzureLlmCalls() + Assert.Equal(2, calls.Length) + } + interface IClassFixture diff --git a/tests/VahterBanBot.Tests/LlmVerdictCacheTieredTtlTests.fs b/tests/VahterBanBot.Tests/LlmVerdictCacheTieredTtlTests.fs index bac81cd6..80cb371d 100644 --- a/tests/VahterBanBot.Tests/LlmVerdictCacheTieredTtlTests.fs +++ b/tests/VahterBanBot.Tests/LlmVerdictCacheTieredTtlTests.fs @@ -158,7 +158,7 @@ type LlmVerdictCacheTieredTtlTests(fixture: MlEnabledVahterTestContainers, _ml: } [] - let ``LLM verdict cache: /ban invalidates both the per-sender NOT_SPAM row and the global SKIP row`` () = task { + let ``LLM verdict cache: /ban invalidates the global SKIP row`` () = task { do! resetFakes () let x = Tg.user() let y = Tg.user(firstName = "spam ban-inval-y") @@ -192,7 +192,7 @@ type LlmVerdictCacheTieredTtlTests(fixture: MlEnabledVahterTestContainers, _ml: } [] - let ``LLM verdict cache: /vahter markspam invalidates both the per-sender NOT_SPAM row and the global SKIP row`` () = task { + let ``LLM verdict cache: /vahter markspam invalidates the global SKIP row`` () = task { do! resetFakes () let x = Tg.user() let y = Tg.user(firstName = "spam markspam-inval-y")