fix(metrics): exclude metric-suppressed accounts from Postgres reaction sums (#4959)

* fix(metrics): exclude metric-suppressed accounts from Postgres reaction sums The reaction counts shown on posts, articles and bounty entries are summed in Postgres from ImageReaction/ArticleReaction/BountyEntryReaction with no exclusion predicate, so they count accounts the reaction-abuse detector already suppressed from every other reaction surface. Unlike the ClickHouse totals these never decay: the jobs recompute the same unfiltered sum from the same rows, so the numbers stay wrong until the queries filter. Post has two live reaction queries, not one — post.metrics.ts delegates to post.metrics-old.ts whenever the simplified-post-metrics flag reads false, which includes Flipt being unreachable. Both filter now. The jobs read the list through a new getMetricExcludedUserIdsOrThrow rather than the existing lenient reader. The lenient one degrades to [] so the reaction milestone keeps firing during an outage; a metric job doing that would write an unfiltered total that nothing later recomputes, because a job only revisits an entity that receives another reaction. Rejecting instead leaves the cursor and the queue untouched in createMetricProcessor, so the window is recomputed next run. Answer and Question reaction metrics have the same shape and are left alone — out of the scope this was asked for, and named as exemptions in the guard rather than skipped silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(metrics): zero the entities whose reactions are ALL excluded, and filter the milestones too Five review lanes over 9cb53ed. Everything here is a finding they raised, each verified against the code or the replica before acting on it. Filtering the aggregate was not enough. An entity whose remaining countable reactions are zero produces NO ROW from a GROUP BY over the reaction table, and a missing row means "no change" to every writer downstream — so the pre-exclusion total survived even a full recompute. Measured on the replica: 18 of 594 affected articles and 338 of 1,154 affected bounty entries are in that state. Post and article seed zeros into ctx.updates before the aggregate overwrites them; bountyEntry has no JS intermediate, so its CTE now drives from the affected ids with a LEFT JOIN. That LEFT JOIN needed timeframeSum to be NULL-safe. Its leading `WHEN NOT (cond) THEN 0` is NULL for an unmatched row and falls through to the AllTime arm, counting a reaction that is not there. `(cond) IS NOT TRUE` is identical for every inner-joined caller. Verified on the replica: with the old form a fully-excluded entry returns 1 heart / 1 like, with the new form 0. The seeding was only safe once a pre-existing bug was fixed. Both post jobs bound their chunk with `BETWEEN ids[0] AND ids[ids.length - 1]` over an unordered Set, so roughly half of all chunks matched nothing. Seeding zeros into a chunk that matches nothing would have written zeros over real counts. The chunk is sorted now. The article and bounty-entry milestone notifications counted unfiltered. Before this work both halves were unfiltered and therefore agreed; filtering only the displayed half would have manufactured, for those two entities, the exact display-vs-notification divergence this defect is a sibling of. They use the LENIENT reader on purpose — a notification should degrade to the old count, not to silence. The guard now covers the notifications too, and three mutations that were demonstrated to pass against it: a `.catch(() => [])` on the strict read, a filter spliced inside an SQL line comment, and a wrong column argument. The last is fixed by construction — the column is hardcoded rather than passed, since a raw-SQL parameter beside an integer guard reads as though the guard covered it. Also: the strict reader now reports the outage to Axiom instead of surfacing only as a generic job error, and a comment claiming coercion parity with metric-reaction-repair.service.ts was false and now says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(metrics): assert the emitted SQL, not that a token appears in the file Round 2 of review demonstrated six mutations that pass the source guard, each measured rather than argued: a second unfiltered count of the same table in the same template literal; a consistent alias swap so `r."userId"` names the image owner instead of the reactor; `.then().catch()` and a plain try/catch around the strict read; a key moved into the exemption list, which had no length pin; a `/* */` comment around the splice; and the bounty-entry filter moved out of the LEFT JOIN's ON into a WHERE, which collapses it to an inner join and restores the no-row defect the rewrite exists to fix. A source guard checks that a token appears in a file. It cannot see what the composed statement does, which is why six separate textual assertions each missed one of these in their own way. `post-reaction-metrics-sql.test.ts` calls the real getReactionTasks with a fake pg that captures every statement, and asserts on the SQL that was actually sent — one test that catches the alias swap, the swallow, the commented splice, the second count, the missing sort and the missing zero-fill. Its fixture crosses the 30,000-image chunk boundary and returns a LOWER run of post ids second. That is not decoration: the first version of the sort control PASSED, because `getAffected` sorts its own return, so a single-chunk fixture cannot produce the out-of-order set the bug needs. It was an assertion that could not fail for the case it was named after. The guard keeps the cases it can see, hardened: block comments as well as line comments, a requirement that the filter be built from a direct `await` of the reader rather than any expression with somewhere to swallow a rejection, a requirement that the alias `r` is bound to the reaction table and to nothing else, a shape pin on the bounty-entry ON clause, and a length pin on the exemption list. Two fixes to the round-1 fix. The Axiom report was effectively unreachable: one latch shared by both readers, and the lenient one runs on every reaction toggle, so it wins every race and the only line for an incident would say a notification degraded while the metric jobs stalled silently. Keyed per outcome now. And `post.metrics.ts` chunked image ids from a ClickHouse query with no ORDER BY under the same inverted-BETWEEN bug fixed one block below; post.metrics-old.ts has that ORDER BY, the live path did not. Both `!clickhouse` branches had no test at all, in any file, because every other test supplies a client. Backfill note, recorded here because a squash merge takes commit messages and not the PR body: this does NOT close ClickUp 868m6vftv. The filter only corrects an entity the next time it is affected, so the already-wrong rows stay wrong until a backfill recomputes them. That ships separately against main, not stacked on this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(notifications): keep the ClickHouse client out of the client bundle The milestone filter pulled the exclusion-list reader — and through it the ClickHouse client — into the notification processor files. Those files are in the _app client graph, because prepareMessage renders there. no-server-infra-in-app-graph caught it: it ran and failed in 44ms before the full suite was killed by a daemon restart, so the one real result that run produced was this. A lazy import inside prepareQuery does not fix it. The guard says why and is right: a dynamic import() still compiles the chunk into the client bundle. So the processors no longer read the list at all. The server-only runner, send-notifications.ts, reads it once per job run with the lenient reader and passes it through NotificationProcessorRunInput, which prepareQuery already receives. The pure SQL builder moves to ~/shared/utils/excluded-reactor-filter.ts with no imports, and metrics/metric-helpers re-exports it. One read per run instead of one per processor. The field is optional and the two reaction milestones default a missing list to [], because degrading to the pre-exclusion count is already the posture these notifications want, and ten existing processor tests construct the input without it. That makes the runner the single point deciding whether milestones are filtered at all, so the guard now pins it: it must read with the lenient reader and pass the list to every prepareQuery, and a processor may import neither reader. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(notifications): pin the runner-to-milestone hand-off by behaviour, not spelling Round 3 of review measured eight mutations that ship the reaction milestones unfiltered with the source guard green, because the guard pinned how the hand-off is SPELLED rather than what is passed: a shadowing `const excludedUserIds = []` inside the batch loop, `excludedUserIds.length = 0` after the read, a spread that overrides the shorthand, a processor that ignores its input, a filter built but never spliced, and the filter moved into the `affected` CTE — valid SQL that narrows which entities are revisited while the COUNT stays unfiltered. send-notifications.excluded.test.ts runs the real job with the real processors, captures the SQL they send, and asserts the filter lands in the CTE that COUNTS. All six of the lane's mutations fail it by name; the processor-level ones fail only their own milestone. It also asserts no processor logged an error, because the runner swallows a per-processor throw and a milestone whose SQL no longer builds would otherwise read as "no query". NotificationProcessorRunInput.excludedUserIds is now required but nullable rather than optional. The milestones default a missing list to [], so a second runner that simply omitted the key would ship every milestone unfiltered without a sound. Required, tsc rejects it. Verified that the protection is real rather than vacuous: typecheck does not read __tests__, which is why ten fixtures building this input still pass, so the control was on the production caller — dropping the key from send-notifications.ts fails with TS2345 at line 49. Plus direct tests of the shared SQL builder's empty-list and non-integer branches, which only the non-empty path had reached. With the integer guard removed, the three non-integer cases fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(notifications): bound the counting-CTE slice instead of failing open countingCte sliced from `affected_value AS (` to the next `), ` and, finding none, fell back to the end of the query. Round 4 of review measured that as a green mutant: put the next CTE's name on its own line and move the filter into `reaction_milestone`, a CTE that counts nothing, and the slice ran on far enough to include it. The helper now bounds the slice by the next CTE header and requires it to find one. That mutant is red now. The same round confirmed the other six mutants go red on the assertion named for their defect rather than incidentally, and found an alias-revert mutant (bounty table back to `br` beside a filter that names `r`) that this test does not catch because it never executes the SQL. It does not need to: no-unfiltered-reaction-metric-sum's alias assertion fails on it, verified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(notifications): give the pgDb mock its full export set pgDbMock.parity requires every inline `~/server/db/pgDb` factory to list the complete export set, because kyselyDb.ts destructures all of them at module-eval time and Vitest throws on any omitted name — during module LOAD, so a suite that reaches it dies at collection and reports zero tests rather than failing. The job test listed only pgDbRead. It passed because kyselyDb is not in its graph, which is the case the guard exists to stop depending on. Caught by the full suite, the only thing that runs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-20 22:08:18 +08:00 · 2026-09-18 20:23:31 -06:00
parent 165da40294
commit 8113e62777
20 changed files with 1058 additions and 68 deletions
@@ -178,7 +178,7 @@ Worked examples of both fixes: the two retry tests in

 ### Convention guards

-40 live in `src/server/services/__tests__/no-*.test.ts`:
+41 live in `src/server/services/__tests__/no-*.test.ts`:
 `no-agent-ground-truth-write`, `no-coerce-boolean-in-api`, `no-direct-shared-module-mock`,
 `no-divergent-active-sales-cap` (SERVER side only: the `model.getActiveSales` parser enforces the id cap, and the chunk size a card surface splits to does not exceed it — it CANNOT see the call site, which is pinned behaviourally by `src/components/Cards/__tests__/useModelSaleBadges.test.ts`, a file in the full unit suite but NOT in `test:lint-rules`, so a `test:lint-rules` run alone does not cover that half; the procedure was rejecting every call from a scrolled feed as an input-validation 400, so no 5xx was recorded and the sale badge simply vanished from the grid), `no-divergent-author-fee-base` (every `recordSpendAttribution` call site must pass the App Blocks author fee the orchestrator's `submitted.cost.base`, never the snapshot and never the gross `buzzAmount`), `no-divergent-can-generate-derivation`, `no-divergent-generation-submit-payload` (the two generation footers must submit the same payload keys — the form-graph lane silently dropped `sourceProvenance`, so its remixes lost the only VERIFIED half of their provenance while the unverified `remixOfId` went through), `no-divergent-model-recency-derivation` (the New/Updated card rule and its day-old cutoff each have one definition — three cards restated them, and when the paid badge took ModelCard's single status slot only that copy knew, so a paid model published minutes ago showed "Paid" on the feed and "New" in the resource picker), `no-divergent-paid-gate-derivation` (the feed and the search index must derive the paid badge from one helper, never two copies of the query), `no-divergent-safetensor-rule` (the coverage view and `checkLoadable` state the checkpoint SafeTensor rule twice and nothing executes the SQL, so the two literals and the checkpoint scoping are pinned textually), `no-doubled-free-slot-noun`, `no-hand-typed-redis-key-constants` (the Redis key-constant
 ratchet — hand-typed `REDIS_KEYS` in an allowlisted mock had drifted 15 times), `no-io-in-transaction`,
@@ -193,7 +193,9 @@ owner checked — see `assertWorkflowOwner`),
 `withBlockScope`, which is the only place the REST surface takes the approved-status
 decision — an open-coded `verifyBlockToken` in a route is the same shape the bridge had),
 `no-unguarded-block-bridge-token` (every tRPC bridge proc must resolve its claims through
-`authorizeBlockBridgeToken`, never a bare `verifyBlockToken`), `no-unguarded-user-text`, `no-unhydrated-home-block-reactions` (a home block hands its images to `ImagesProvider` with `reactions: []`, because its payload is one shared anonymous cache entry — an un-highlighted reaction is one the viewer clicks OFF), `no-unloadable-image-fixture`,
+`authorizeBlockBridgeToken`, never a bare `verifyBlockToken`), `no-unfiltered-reaction-metric-sum` (a metric job that SUMs a reaction table must exclude the metric-suppressed
+accounts — the Postgres sums never decay, so an unfiltered total is permanent rather than
+stale), `no-unguarded-user-text`, `no-unhydrated-home-block-reactions` (a home block hands its images to `ImagesProvider` with `reactions: []`, because its payload is one shared anonymous cache entry — an un-highlighted reaction is one the viewer clicks OFF), `no-unloadable-image-fixture`,
 `no-unmoderated-blob-retraction` (the ledger of flows allowed to ask the image-cache service to
 destroy an image's SHARED stored object — a cross-account, irreversible act; moderation only),
 `no-unmuteable-comment-processor`, `no-unscoped-email-verification-exemption`,
@@ -208,7 +210,7 @@ fail only in a full-suite run. Five were missing when this was last audited, on
 wired in then. If the diff adds a guard, check it was wired into the script, and don't treat a green
 `test:lint-rules` as "all guards passed".

-`test:lint-rules` names 45 files today.
+`test:lint-rules` names 46 files today.

 Both numbers and the list are checked by `no-lint-rules-script-drift`, which reads the two phrasings
 above literally — edit the numbers, not the shapes.
@@ -211,7 +211,7 @@ Use a top-level `import type * as PromClient` — an inline `typeof import('...'
 **Before widening a mock, check whether the import edge is needed at all.** A failing suite may be telling you the code pulled in a dependency it doesn't want, not that the mock is too narrow, and widening it would hide that. (Bit us twice in one day, Aug 2026, on two branches; one of those three suites was fixed by extracting the helpers into their own module instead.)

 #### Convention guards run as tests
-Several repo conventions are enforced by tests, not by eslint. 40 live in `src/server/services/__tests__/no-*.test.ts` — `no-agent-ground-truth-write`, `no-coerce-boolean-in-api`,
+Several repo conventions are enforced by tests, not by eslint. 41 live in `src/server/services/__tests__/no-*.test.ts` — `no-agent-ground-truth-write`, `no-coerce-boolean-in-api`,
 `no-direct-shared-module-mock` (the shared-mock ratchet, see `docs/testing/shared-module-mocks.md`),
 `no-divergent-active-sales-cap` (SERVER side only: the `model.getActiveSales` parser enforces the id cap, and the chunk size a card surface splits to does not exceed it — it CANNOT see the call site, which is pinned behaviourally by `src/components/Cards/__tests__/useModelSaleBadges.test.ts`, a file in the full unit suite but NOT in `test:lint-rules`, so a `test:lint-rules` run alone does not cover that half; the procedure was rejecting every call from a scrolled feed as an input-validation 400, so no 5xx was recorded and the sale badge simply vanished from the grid), `no-divergent-author-fee-base` (every `recordSpendAttribution` call site must pass the App Blocks author fee the orchestrator's `submitted.cost.base`, never the snapshot and never the gross `buzzAmount` — the three are indistinguishable positive Buzz integers, so a percentage of the wrong one takes a cut of another creator's licensing fee),
 `no-divergent-can-generate-derivation` (coverage alone is not canGenerate — the ecosystem must also support the model TYPE, and the pair is composed only in `isGenerationEligible`),
@@ -233,7 +233,9 @@ owner checked — see `assertWorkflowOwner`),
 decision — an open-coded `verifyBlockToken` in a route is the same shape the bridge had),
 `no-unguarded-block-bridge-token` (every tRPC bridge proc must resolve its claims through
 `authorizeBlockBridgeToken` — a bare `verifyBlockToken` honours a revoked install and a
-suspended app for a whole token lifetime), `no-unguarded-user-text`, `no-unhydrated-home-block-reactions` (a home block hands its images to `ImagesProvider` with `reactions: []`, because its payload is one shared anonymous cache entry — an un-highlighted reaction is one the viewer clicks OFF), `no-unloadable-image-fixture`,
+suspended app for a whole token lifetime), `no-unfiltered-reaction-metric-sum` (a metric job that SUMs a reaction table must exclude the metric-suppressed
+accounts — the Postgres sums never decay, so an unfiltered total is permanent rather than
+stale), `no-unguarded-user-text`, `no-unhydrated-home-block-reactions` (a home block hands its images to `ImagesProvider` with `reactions: []`, because its payload is one shared anonymous cache entry — an un-highlighted reaction is one the viewer clicks OFF), `no-unloadable-image-fixture`,
 `no-unmoderated-blob-retraction` (the ledger of flows allowed to ask the image-cache service to
 destroy an image's SHARED stored object — content-addressed, so it takes every byte-identical image
 of every owner with it; moderation only),
@@ -254,7 +256,7 @@ was last audited, on 2026-08-24, and were wired in then. **Add a new guard to th
 you write it**, and don't read a green `test:lint-rules` as "all guards passed" without checking the directory
 against the script.

-`test:lint-rules` names 45 files today.
+`test:lint-rules` names 46 files today.

 The count above, the count in the list, and the list itself are what went stale three times, so
 `no-lint-rules-script-drift` fails when they disagree with the directory or the script. It reads two exact
@@ -107,7 +107,7 @@
    "test:packages:run": "vitest run --project '@civitai/*'",
    "test:apps": "vitest --project 'app:*'",
    "test:apps:run": "vitest run --project 'app:*'",
-    "test:lint-rules": "vitest run --project 'unit*' src/server/notifications/__tests__/notification-settings-polarity.test.ts src/server/schema/__tests__/track.addView.schema.test.ts src/server/services/__tests__/hub-filter-parity.test.ts src/server/services/__tests__/poi-checks-strip-benign-phrases.test.ts src/server/services/__tests__/video-leaderboard-badge-staging.test.ts src/server/services/__tests__/no-agent-ground-truth-write.test.ts src/server/services/__tests__/no-coerce-boolean-in-api.test.ts src/server/services/__tests__/no-direct-shared-module-mock.test.ts src/server/services/__tests__/no-divergent-active-sales-cap.test.ts src/server/services/__tests__/no-divergent-author-fee-base.test.ts src/server/services/__tests__/no-divergent-can-generate-derivation.test.ts src/server/services/__tests__/no-divergent-model-recency-derivation.test.ts src/server/services/__tests__/no-divergent-generation-submit-payload.test.ts src/server/services/__tests__/no-divergent-paid-gate-derivation.test.ts src/server/services/__tests__/no-divergent-safetensor-rule.test.ts src/server/services/__tests__/no-doubled-free-slot-noun.test.ts src/server/services/__tests__/no-hand-typed-redis-key-constants.test.ts src/server/services/__tests__/no-io-in-transaction.test.ts src/server/services/__tests__/no-job-kind-on-remix-mint.test.ts src/server/services/__tests__/no-lint-rules-script-drift.test.ts src/server/services/__tests__/no-menu-target-tooltip-nesting.test.ts src/server/services/__tests__/no-module-scope-cache.test.ts src/server/services/__tests__/no-pk-addressed-engagement-write.test.ts src/server/services/__tests__/no-server-infra-in-app-graph.test.ts src/server/services/__tests__/no-sharp-outside-native-project.test.ts src/server/services/__tests__/no-ssr-divergent-media-query.test.ts src/server/services/__tests__/no-stale-moderator-route-probe.test.ts src/server/services/__tests__/no-static-html2canvas-import.test.ts src/server/services/__tests__/no-unbounded-paging-fake.test.ts src/server/services/__tests__/no-unbumped-draft-status-write.test.ts src/server/services/__tests__/no-unguarded-billable-submit.test.ts src/server/services/__tests__/no-unguarded-block-bridge-token.test.ts src/server/services/__tests__/no-unguarded-block-rest-token.test.ts src/server/services/__tests__/no-unguarded-user-text.test.ts src/server/services/__tests__/no-unhydrated-home-block-reactions.test.ts src/server/services/__tests__/no-unloadable-image-fixture.test.ts src/server/services/__tests__/no-unmoderated-blob-retraction.test.ts src/server/services/__tests__/no-unmuteable-comment-processor.test.ts src/server/services/__tests__/no-unpriced-default-model.test.ts src/server/services/__tests__/no-unroled-image-resource-match.test.ts src/server/services/__tests__/no-unscoped-email-verification-exemption.test.ts src/server/services/__tests__/no-untruthy-query-gate.test.ts src/server/services/__tests__/no-unverified-provenance-write.test.ts src/server/services/__tests__/no-unwrapped-knob-rotation.test.ts src/server/services/__tests__/no-wholesale-module-mock.test.ts",
+    "test:lint-rules": "vitest run --project 'unit*' src/server/notifications/__tests__/notification-settings-polarity.test.ts src/server/schema/__tests__/track.addView.schema.test.ts src/server/services/__tests__/hub-filter-parity.test.ts src/server/services/__tests__/poi-checks-strip-benign-phrases.test.ts src/server/services/__tests__/video-leaderboard-badge-staging.test.ts src/server/services/__tests__/no-agent-ground-truth-write.test.ts src/server/services/__tests__/no-coerce-boolean-in-api.test.ts src/server/services/__tests__/no-direct-shared-module-mock.test.ts src/server/services/__tests__/no-divergent-active-sales-cap.test.ts src/server/services/__tests__/no-divergent-author-fee-base.test.ts src/server/services/__tests__/no-divergent-can-generate-derivation.test.ts src/server/services/__tests__/no-divergent-model-recency-derivation.test.ts src/server/services/__tests__/no-divergent-generation-submit-payload.test.ts src/server/services/__tests__/no-divergent-paid-gate-derivation.test.ts src/server/services/__tests__/no-divergent-safetensor-rule.test.ts src/server/services/__tests__/no-doubled-free-slot-noun.test.ts src/server/services/__tests__/no-hand-typed-redis-key-constants.test.ts src/server/services/__tests__/no-io-in-transaction.test.ts src/server/services/__tests__/no-job-kind-on-remix-mint.test.ts src/server/services/__tests__/no-lint-rules-script-drift.test.ts src/server/services/__tests__/no-menu-target-tooltip-nesting.test.ts src/server/services/__tests__/no-module-scope-cache.test.ts src/server/services/__tests__/no-pk-addressed-engagement-write.test.ts src/server/services/__tests__/no-server-infra-in-app-graph.test.ts src/server/services/__tests__/no-sharp-outside-native-project.test.ts src/server/services/__tests__/no-ssr-divergent-media-query.test.ts src/server/services/__tests__/no-stale-moderator-route-probe.test.ts src/server/services/__tests__/no-static-html2canvas-import.test.ts src/server/services/__tests__/no-unbounded-paging-fake.test.ts src/server/services/__tests__/no-unbumped-draft-status-write.test.ts src/server/services/__tests__/no-unfiltered-reaction-metric-sum.test.ts src/server/services/__tests__/no-unguarded-billable-submit.test.ts src/server/services/__tests__/no-unguarded-block-bridge-token.test.ts src/server/services/__tests__/no-unguarded-block-rest-token.test.ts src/server/services/__tests__/no-unguarded-user-text.test.ts src/server/services/__tests__/no-unhydrated-home-block-reactions.test.ts src/server/services/__tests__/no-unloadable-image-fixture.test.ts src/server/services/__tests__/no-unmoderated-blob-retraction.test.ts src/server/services/__tests__/no-unmuteable-comment-processor.test.ts src/server/services/__tests__/no-unpriced-default-model.test.ts src/server/services/__tests__/no-unroled-image-resource-match.test.ts src/server/services/__tests__/no-unscoped-email-verification-exemption.test.ts src/server/services/__tests__/no-untruthy-query-gate.test.ts src/server/services/__tests__/no-unverified-provenance-write.test.ts src/server/services/__tests__/no-unwrapped-knob-rotation.test.ts src/server/services/__tests__/no-wholesale-module-mock.test.ts",
    "test:component": "node scripts/test-component-run.mjs",
    "test:component:watch": "vitest --project component",
    "test:geometry": "vitest run --project geometry",
@@ -0,0 +1,105 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type * as ExcludedUsers from '~/server/services/metric-excluded-users.service';
+import { loggingMock } from '~/__tests__/mocks/logging.mock';
+
+/**
+ * The one test that runs the notification runner end to end with the REAL processors and
+ * reads the SQL they emit.
+ *
+ * The reaction milestones cannot read the exclusion list themselves — the processor files
+ * are in the client graph — so the runner reads it and passes it in. A source guard pins
+ * the spelling of that hand-off and was shown not to pin its value: eight mutations
+ * shipped the milestones unfiltered with the guard green, among them a shadowing
+ * `const excludedUserIds = []` inside the loop, `excludedUserIds.length = 0` after the
+ * read, a processor that ignores its input, and the filter moved into the `affected` CTE,
+ * where it only narrows which entities are revisited while the COUNT stays unfiltered.
+ * Every one of those changes what reaches Postgres, so this asserts on that.
+ */
+
+const EXCLUDED = [7, 9];
+const FILTER = `r."userId" NOT IN (${EXCLUDED.join(',')})`;
+
+const h = vi.hoisted(() => ({
+  captured: [] as string[],
+}));
+
+vi.mock('~/server/jobs/job', () => ({
+  createJob: (name: string, cron: string, fn: (e: unknown) => Promise<unknown>) => ({
+    name,
+    cron,
+    run: () => fn({ checkIfCanceled: () => undefined, on: () => undefined }),
+  }),
+  getJobDate: vi.fn().mockResolvedValue([new Date(0), vi.fn()]),
+}));
+
+vi.mock('~/server/services/metric-excluded-users.service', async (importOriginal) => ({
+  ...(await importOriginal<typeof ExcludedUsers>()),
+  getMetricExcludedUserIds: vi.fn().mockResolvedValue(EXCLUDED),
+}));
+
+vi.mock('~/server/db/pgDb', () => ({
+  pgDbRead: {
+    cancellableQuery: vi.fn(async (sql: string) => {
+      h.captured.push(sql);
+      return { result: async () => [], cancel: async () => undefined };
+    }),
+  },
+  pgDbReadLong: {},
+  pgDbWrite: {},
+}));
+
+const { sendNotificationsJob } = await import('~/server/jobs/send-notifications');
+
+/**
+ * The counting CTE of a milestone query. The filter has to land HERE: the `affected` CTE
+ * above it only chooses which entities to revisit, so a filter there is valid SQL that
+ * still counts every excluded reaction.
+ */
+function countingCte(sql: string) {
+  const start = sql.indexOf('affected_value AS (');
+  expect(start, 'the milestone query no longer has an affected_value CTE').toBeGreaterThan(-1);
+  // Bounded by the NEXT CTE header, and required to find one. Falling back to the end of
+  // the query failed open: put the separator's CTE name on its own line and move the
+  // filter into a CTE that counts nothing, and the slice ran on to include it — measured
+  // green before this.
+  const next = /\)\s*,\s*\w+\s+AS\s*\(/g;
+  next.lastIndex = start + 'affected_value AS ('.length;
+  const end = next.exec(sql)?.index ?? -1;
+  expect(end, 'could not find where the affected_value CTE ends').toBeGreaterThan(start);
+  return sql.slice(start, end);
+}
+
+function milestoneSql(key: string) {
+  const sql = h.captured.filter((s) => s.includes(`'${key}'`));
+  expect(sql, `${key} issued no query — did its prepareQuery throw?`).toHaveLength(1);
+  return sql[0];
+}
+
+beforeEach(() => {
+  h.captured.length = 0;
+  loggingMock.logToAxiom.mockClear();
+});
+
+describe('send-notifications passes the exclusion list to the reaction milestones', () => {
+  it.each(['article-reaction-milestone', 'bounty-reaction-milestone'])(
+    '%s filters the reactors it COUNTS',
+    async (key) => {
+      await sendNotificationsJob.run();
+
+      const sql = milestoneSql(key);
+      expect(countingCte(sql), 'the filter is not in the CTE that counts').toContain(FILTER);
+      expect(sql.split(FILTER).length - 1, 'the filter appears more than once').toBe(1);
+    }
+  );
+
+  it('runs without any processor throwing', async () => {
+    // The runner swallows a per-processor throw and logs it, so without this a milestone
+    // whose SQL no longer builds would read as "issued no query" rather than as a failure.
+    await sendNotificationsJob.run();
+
+    const errors = loggingMock.logToAxiom.mock.calls
+      .map(([arg]: [{ type?: string; details?: unknown; message?: string }]) => arg)
+      .filter((arg) => arg.type === 'error');
+    expect(errors).toEqual([]);
+  });
+});
@@ -6,6 +6,7 @@ import { notifications } from '~/server/notifications/client';
 import { pgDbRead } from '~/server/db/pgDb';
 import { logToAxiom } from '~/server/logging/client';
 import { notificationBatches } from '~/server/notifications/utils.notifications';
+import { getMetricExcludedUserIds } from '~/server/services/metric-excluded-users.service';
 import { limitConcurrency } from '~/server/utils/concurrency-helpers';
 import { createLogger } from '~/utils/logging';
 import { createJob, getJobDate } from './job';
@@ -28,6 +29,10 @@ const NOTIFICATION_QUERY_TIMEOUT_MS = 20_000;
 export const sendNotificationsJob = createJob('send-notifications', '*/1 * * * *', async (e) => {
  try {
    const [lastRun, setLastRun] = await getJobDate('last-sent-notifications');
+    // The LENIENT reader: a milestone that fires on the pre-exclusion count is how these
+    // behaved before; one that silently never fires is worse. Read once per run, not per
+    // processor, and passed in because the processor files cannot import it.
+    const excludedUserIds = await getMetricExcludedUserIds();

    // Run batches
    for (const batch of notificationBatches) {
@@ -45,6 +50,7 @@ export const sendNotificationsJob = createJob('send-notifications', '*/1 * * * *
            lastSent: lastSent.toISOString(),
            lastSentDate: lastSent,
            clickhouse,
+            excludedUserIds,
          });
          if (query) {
            const start = Date.now();
@@ -0,0 +1,192 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+/**
+ * The only test that reads the SQL the post reaction job actually sends.
+ *
+ * Everything else protecting this change is a source guard, and a source guard checks
+ * that a token appears in a file — not what the composed statement does. Four separate
+ * mutations were demonstrated to pass a token check while leaving the defect in place:
+ * swapping the table aliases so `r."userId"` names the image owner instead of the
+ * reactor, swallowing the strict read in a try/catch, commenting the splice out, and
+ * counting the same table a second time in the same template literal. All four change
+ * the emitted SQL, which is why this asserts on that.
+ */
+
+const h = vi.hoisted(() => ({
+  excludedIds: vi.fn(),
+}));
+
+vi.mock('~/server/services/metric-excluded-users.service', () => ({
+  getMetricExcludedUserIdsOrThrow: h.excludedIds,
+}));
+
+vi.mock('~/server/flipt/client', () => ({
+  isFlipt: vi.fn().mockResolvedValue(true),
+  FLIPT_FEATURE_FLAGS: {},
+  buildEntityMetricPerDaySource: (where: string) => `entityMetricEvents_day ${where}`,
+}));
+
+const { getReactionTasks } = await import('~/server/metrics/post.metrics');
+
+type Captured = { sql: string; params?: unknown[] };
+
+/**
+ * Two image chunks, because one cannot expose the ordering hazard.
+ *
+ * `getAffected` sorts its own return, so a single chunk always lands in `affected`
+ * ascending. In production the image ids are chunked at 30,000 and the chunks run
+ * CONCURRENTLY, so `affected` ends up as several sorted runs concatenated — globally
+ * unsorted. The job then bounds each post chunk with `BETWEEN ids[0] AND ids[last]`, and
+ * an inverted range matches nothing; with zeros seeded, a chunk that matches nothing
+ * writes zeros over real counts. So the fixture crosses the chunk boundary and returns a
+ * LOWER run second, which is the shape that actually breaks.
+ */
+const IMAGE_CHUNK = 30_000;
+const IMAGE_IDS = Array.from({ length: IMAGE_CHUNK + 1 }, (_, i) => i + 1);
+const POST_RUNS = [
+  [500, 600],
+  [100, 200],
+];
+const POST_IDS = POST_RUNS.flat();
+
+function makeCtx(reactionRows: Record<string, unknown>[]) {
+  const captured: Captured[] = [];
+  let imageChunkCalls = 0;
+  const updates: Record<number, Record<string, number>> = {};
+  const ctx = {
+    ch: { $query: vi.fn().mockResolvedValue(IMAGE_IDS.map((imageId) => ({ imageId }))) },
+    pg: {
+      cancellableQuery: vi.fn(async (sql: string, params?: unknown[]) => {
+        captured.push({ sql, params });
+        // The affected-post lookup selects `id`; the reaction aggregate selects counts.
+        // Each image chunk returns its own run of post ids, second run lower than first.
+        if (/FROM "Image" i/.test(sql)) {
+          const run = POST_RUNS[imageChunkCalls++] ?? [];
+          return { result: async () => run.map((id) => ({ id })), cancel: async () => undefined };
+        }
+        return { result: async () => reactionRows, cancel: async () => undefined };
+      }),
+    },
+    jobContext: { checkIfCanceled: () => undefined, on: () => undefined },
+    queue: [] as number[],
+    affected: new Set<number>(),
+    addAffected: (id: number | number[]) => {
+      if (Array.isArray(id)) id.forEach((x) => ctx.affected.add(x));
+      else ctx.affected.add(id);
+    },
+    updates,
+    idKey: 'postId',
+    lastUpdate: new Date('2026-09-01T00:00:00Z'),
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  } as any;
+  return { ctx, captured, updates };
+}
+
+const aggregateSql = (captured: Captured[]) =>
+  captured.map((c) => c.sql).find((sql) => /FROM "ImageReaction"/.test(sql));
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  h.excludedIds.mockResolvedValue([11, 22]);
+});
+
+describe('post reaction metrics SQL', () => {
+  it('filters the REACTOR, by the alias the reaction table is bound to', async () => {
+    const { ctx, captured } = makeCtx([]);
+    const tasks = await getReactionTasks(ctx);
+    await Promise.all(tasks.map((t) => t()));
+
+    const sql = aggregateSql(captured);
+    expect(sql, 'the reaction aggregate was never issued').toBeDefined();
+
+    // The alias the filter names must be the alias bound to the reaction table, not to
+    // `Image` — `Image."userId"` exists, so the wrong alias is valid SQL that filters by
+    // the post's owner and is invisible to any check that only looks for the predicate.
+    const alias = /FROM "ImageReaction"\s+(\w+)/.exec(sql!)?.[1];
+    expect(alias).toBeDefined();
+    expect(sql).toContain(`AND ${alias}."userId" NOT IN (11,22)`);
+  });
+
+  it('puts the filter in the executed statement, not in a comment', async () => {
+    const { ctx, captured } = makeCtx([]);
+    const tasks = await getReactionTasks(ctx);
+    await Promise.all(tasks.map((t) => t()));
+
+    const sql = aggregateSql(captured)!;
+    const filterLine = sql.split('\n').find((l) => l.includes('NOT IN (11,22)'))!;
+    expect(filterLine, 'the filter is inside an SQL line comment').not.toMatch(/--/);
+    expect(sql, 'the filter is inside a block comment').not.toMatch(
+      /\/\*[\s\S]*NOT IN \(11,22\)[\s\S]*\*\//
+    );
+  });
+
+  it('counts the reaction table exactly once, all of it filtered', async () => {
+    // A second, unfiltered count of the same table added to the same statement is the
+    // realistic next defect — "also show the raw total" lands inside the existing query.
+    const { ctx, captured } = makeCtx([]);
+    const tasks = await getReactionTasks(ctx);
+    await Promise.all(tasks.map((t) => t()));
+
+    const sql = aggregateSql(captured)!;
+    const references = sql.match(/"ImageReaction"/g) ?? [];
+    const filters = sql.match(/NOT IN \(11,22\)/g) ?? [];
+    expect(filters.length).toBe(references.length);
+  });
+
+  it('seeds zero for a post the aggregate returns no row for', async () => {
+    // The all-excluded case: a GROUP BY returns no row, and a missing row means
+    // "no change" to every writer downstream, so the pre-exclusion total would survive.
+    const { ctx, updates } = makeCtx([]);
+    const tasks = await getReactionTasks(ctx);
+    await Promise.all(tasks.map((t) => t()));
+
+    for (const postId of POST_IDS) {
+      expect(updates[postId], `post ${postId} missing from updates`).toBeDefined();
+      expect(updates[postId].heartCount).toBe(0);
+      expect(updates[postId].likeCount).toBe(0);
+    }
+  });
+
+  it('lets the aggregate overwrite the seeded zero', async () => {
+    // Negative control for the test above: the zeros must be a floor, not a ceiling.
+    const { ctx, updates } = makeCtx([
+      { postId: 500, timeframe: 'AllTime', heartCount: 7, likeCount: 3 },
+    ]);
+    const tasks = await getReactionTasks(ctx);
+    await Promise.all(tasks.map((t) => t()));
+
+    expect(updates[500].heartCount).toBe(7);
+    expect(updates[500].likeCount).toBe(3);
+    expect(updates[100].heartCount).toBe(0);
+  });
+
+  it('bounds each chunk with an ascending range', async () => {
+    // `BETWEEN ids[0] AND ids[last]` over an unsorted chunk matches nothing, and with the
+    // seeding above that writes zeros over real counts. The fixture ids are unsorted.
+    const { ctx, captured } = makeCtx([]);
+    const tasks = await getReactionTasks(ctx);
+    await Promise.all(tasks.map((t) => t()));
+
+    const ranges = captured
+      .map((c) => /BETWEEN (\d+) AND (\d+)/.exec(c.sql))
+      .filter((m): m is RegExpExecArray => !!m);
+
+    expect(ranges.length, 'no BETWEEN-bounded query was issued').toBeGreaterThan(0);
+    for (const [, lo, hi] of ranges) {
+      expect(Number(lo), `range ${lo}..${hi} is inverted and matches nothing`).toBeLessThanOrEqual(
+        Number(hi)
+      );
+    }
+  });
+
+  it('issues no query at all when the exclusion list cannot be read', async () => {
+    // The strict reader must reject before any task is built, so the run fails before the
+    // cursor advances rather than writing an unfiltered total.
+    h.excludedIds.mockRejectedValue(new Error('clickhouse unreachable'));
+    const { ctx, captured } = makeCtx([]);
+
+    await expect(getReactionTasks(ctx)).rejects.toThrow('clickhouse unreachable');
+    expect(captured).toEqual([]);
+    expect(ctx.ch.$query).not.toHaveBeenCalled();
+  });
+});
@@ -14,7 +14,14 @@ import { SearchIndexUpdateQueueAction } from '~/server/common/enums';
 import { createLogger } from '~/utils/logging';
 import type { Task } from '~/server/utils/concurrency-helpers';
 import { limitConcurrency } from '~/server/utils/concurrency-helpers';
-import { executeRefresh, getAffected, getEntityMetricTasks } from '~/server/metrics/metric-helpers';
+import {
+  executeRefresh,
+  getAffected,
+  getEntityMetricTasks,
+  reactionCountKeys,
+  snippets,
+} from '~/server/metrics/metric-helpers';
+import { getMetricExcludedUserIdsOrThrow } from '~/server/services/metric-excluded-users.service';
 import type { ArticleMetric } from '~/shared/utils/prisma/models';
 import { templateHandler } from '~/server/db/db-helpers';

@@ -105,6 +112,7 @@ export const articleMetrics = createMetricProcessor({

 async function getReactionTasks(ctx: MetricContext) {
  log('getReactionTasks', ctx.lastUpdate);
+  const excludedFilter = snippets.excludedReactorFilter(await getMetricExcludedUserIdsOrThrow());
  const affected = await getAffected(ctx)`
    -- get recent article reactions
    SELECT
@@ -116,6 +124,15 @@ async function getReactionTasks(ctx: MetricContext) {
  const tasks = chunk(affected, 1000).map((ids, i) => async () => {
    ctx.jobContext.checkIfCanceled();
    log('getReactionTasks', i + 1, 'of', tasks.length);
+    // An entity whose remaining reactions are all excluded yields NO ROW from the
+    // aggregate below, and a missing row means "no change" to every writer downstream —
+    // so the pre-exclusion total would survive even a full recompute. Seeding zeros
+    // first makes the absence of a row mean zero; the aggregate overwrites whatever it
+    // does return.
+    for (const id of ids) {
+      const row = (ctx.updates[id] ??= { [ctx.idKey]: id });
+      for (const key of reactionCountKeys) row[key] ??= 0;
+    }
    await getMetrics(ctx)`
      -- get article reaction metrics
      SELECT
@@ -129,6 +146,7 @@ async function getReactionTasks(ctx: MetricContext) {
      FROM "ArticleReaction" r
      WHERE r."articleId" IN (${ids})
        AND r."articleId" BETWEEN ${ids[0]} AND ${ids[ids.length - 1]}
+        ${excludedFilter}
      GROUP BY r."articleId"
    `;
    log('getReactionTasks', i + 1, 'of', tasks.length, 'done');
@@ -10,6 +10,7 @@ import {
  snippets,
 } from '~/server/metrics/metric-helpers';
 import { chunk } from 'lodash-es';
+import { getMetricExcludedUserIdsOrThrow } from '~/server/services/metric-excluded-users.service';

 const log = createLogger('metrics:bounty');

@@ -45,6 +46,7 @@ export const bountyEntryMetrics = createMetricProcessor({

 async function getReactionTasks(ctx: MetricProcessorRunContext) {
  log('getReactionTasks', ctx.lastUpdate);
+  const excludedFilter = snippets.excludedReactorFilter(await getMetricExcludedUserIdsOrThrow());
  const affected = await getAffected(ctx)`
    -- get recent bounty entry reactions
    SELECT
@@ -61,15 +63,23 @@ async function getReactionTasks(ctx: MetricProcessorRunContext) {
    const metrics = await getMetricJson(ctx)`
      -- Aggregate bounty entry reaction metrics into JSON
      WITH metric_data AS (
+        -- Driven from the affected ids with a LEFT JOIN, not from the reaction rows: an
+        -- entry whose remaining reactions are all excluded must come back as zero rather
+        -- than as no row, because a missing row means "no change" downstream and the
+        -- pre-exclusion total would survive the recompute. timeframeSum scores an
+        -- outer-joined NULL as 0, so the empty case sums to zero rather than counting a
+        -- reaction that is not there.
        SELECT
-          r."bountyEntryId",
+          be.id AS "bountyEntryId",
          tf.timeframe,
          ${snippets.reactionTimeframes()}
-        FROM "BountyEntryReaction" r
-        JOIN "BountyEntry" be ON be.id = r."bountyEntryId" -- ensure the bountyEntry exists
+        FROM unnest(${ids}::int[]) AS affected(id)
+        JOIN "BountyEntry" be ON be.id = affected.id -- ensure the bountyEntry exists
        CROSS JOIN (SELECT unnest(enum_range(NULL::"MetricTimeframe")) AS timeframe) tf
-        WHERE r."bountyEntryId" = ANY(${ids}::int[])
-        GROUP BY r."bountyEntryId", tf.timeframe
+        LEFT JOIN "BountyEntryReaction" r
+          ON r."bountyEntryId" = be.id
+          ${excludedFilter}
+        GROUP BY be.id, tf.timeframe
      )
      SELECT jsonb_agg(
        jsonb_build_object(
@@ -6,6 +6,7 @@ import { parameterizedTemplateHandler, templateHandler } from '~/server/db/db-he
 import type { JobContext } from '~/server/jobs/job';
 import { createLogger } from '~/utils/logging';
 import { buildEntityMetricPerDaySource } from '~/server/flipt/client';
+import { excludedReactorFilter } from '~/shared/utils/excluded-reactor-filter';

 const log = createLogger('metric-helpers');

@@ -61,7 +62,12 @@ function timeframeSum(
  additionalConditions = '',
  timeframeAlias = 'tf'
 ) {
-  const conditionCheck = additionalConditions ? `WHEN NOT (${additionalConditions}) THEN 0` : '';
+  // `IS NOT TRUE` rather than `NOT (...)` so an outer-joined row, where the condition is
+  // NULL rather than false, scores 0 instead of falling through to the AllTime arm and
+  // counting a reaction that is not there. Identical for every inner-joined caller.
+  const conditionCheck = additionalConditions
+    ? `WHEN (${additionalConditions}) IS NOT TRUE THEN 0`
+    : '';
  additionalConditions =
    additionalConditions && !additionalConditions.startsWith('AND')
      ? `AND ${additionalConditions}`
@@ -122,6 +128,15 @@ function reactionTimeframes(reactionElementAlias = 'r', timeframeAlias = 'tf') {
    .join(',\n');
 }

+/**
+ * The metric columns a reaction aggregate writes. Exported because a job has to seed them
+ * to zero for the entities the aggregate returns no row for, which the three post/article
+ * reaction tasks do inline before issuing their query.
+ */
+export const reactionCountKeys = Object.keys(ReviewReactions).map(
+  (reaction) => `${reaction.toLowerCase()}Count`
+);
+
 const reactionMetricNames = Object.keys(ReviewReactions)
  .map((reaction) => `"${reaction.toLowerCase()}Count"`)
  .join(', ');
@@ -131,6 +146,7 @@ const reactionMetricUpserts = Object.keys(ReviewReactions)
  .join(', ');

 export const snippets = {
+  excludedReactorFilter,
  reactionTimeframes,
  timeframeSum,
  timeframeCount,
@@ -1,7 +1,13 @@
 import { chunk } from 'lodash-es';
 import type { MetricProcessorRunContext } from '~/server/metrics/base.metrics';
 import { createMetricProcessor } from '~/server/metrics/base.metrics';
-import { executeRefresh, getAffected, snippets } from '~/server/metrics/metric-helpers';
+import {
+  executeRefresh,
+  getAffected,
+  reactionCountKeys,
+  snippets,
+} from '~/server/metrics/metric-helpers';
+import { getMetricExcludedUserIdsOrThrow } from '~/server/services/metric-excluded-users.service';
 import type { Task } from '~/server/utils/concurrency-helpers';
 import { limitConcurrency } from '~/server/utils/concurrency-helpers';
 import { createLogger } from '~/utils/logging';
@@ -144,6 +150,7 @@ export async function update(baseCtx: MetricProcessorRunContext) {

 async function getReactionTasks(ctx: MetricContext) {
  log('getReactionTasks', ctx.lastUpdate);
+  const excludedFilter = snippets.excludedReactorFilter(await getMetricExcludedUserIdsOrThrow());
  const affectedImages = await ctx.ch.$query<{ imageId: number }>`
      SELECT DISTINCT entityId as imageId
      FROM entityMetricEvents_month
@@ -174,9 +181,27 @@ async function getReactionTasks(ctx: MetricContext) {
  });
  await limitConcurrency(postFetchTasks, 3);

-  const tasks = chunk([...affected], 100).map((ids, i) => async () => {
+  // Sorted because the query below bounds the chunk with
+  // `BETWEEN ids[0] AND ids[ids.length - 1]`. `affected` is a Set in insertion order, so
+  // an unsorted chunk whose first id exceeds its last matches NOTHING — and with the
+  // zero-seeding below, a chunk that matches nothing would write zeros over real counts.
+  const tasks = chunk(
+    [...affected].sort((a, b) => a - b),
+    100
+  ).map((ids, i) => async () => {
    ctx.jobContext.checkIfCanceled();
    log('getReactionTasks', i + 1, 'of', tasks.length);
+    // A post whose remaining reactions are all excluded yields NO ROW from the aggregate
+    // below, and a missing row means "no change" to every writer downstream — so the
+    // pre-exclusion total would survive even a full recompute. Seeding zeros first makes
+    // the absence of a row mean zero; the aggregate overwrites whatever it does return.
+    // One slot per timeframe here, matching the shape getMetrics writes.
+    for (const id of ids) {
+      const row = (ctx.updates[id] ??= { postId: id } as Record<MetricKey, TimeframeData | number>);
+      for (const key of reactionCountKeys) {
+        (row as Record<string, TimeframeData | number>)[key] ??= [0, 0, 0, 0, 0];
+      }
+    }
    await getMetrics(ctx)`
      -- get post reaction metrics
      SELECT
@@ -188,6 +213,7 @@ async function getReactionTasks(ctx: MetricContext) {
      CROSS JOIN (SELECT unnest(enum_range('AllTime'::"MetricTimeframe", NULL)) AS "timeframe") tf
      WHERE i."postId" IN (${ids})
        AND i."postId" BETWEEN ${ids[0]} AND ${ids[ids.length - 1]}
+        ${excludedFilter}
      GROUP BY i."postId", tf.timeframe
    `;
    log('getReactionTasks', i + 1, 'of', tasks.length, 'done');
@@ -1,7 +1,14 @@
 import { chunk } from 'lodash-es';
 import type { MetricProcessorRunContext } from '~/server/metrics/base.metrics';
 import { createMetricProcessor } from '~/server/metrics/base.metrics';
-import { executeRefresh, getAffected, getEntityMetricTasks } from '~/server/metrics/metric-helpers';
+import {
+  executeRefresh,
+  getAffected,
+  getEntityMetricTasks,
+  reactionCountKeys,
+  snippets,
+} from '~/server/metrics/metric-helpers';
+import { getMetricExcludedUserIdsOrThrow } from '~/server/services/metric-excluded-users.service';
 import type { Task } from '~/server/utils/concurrency-helpers';
 import { limitConcurrency } from '~/server/utils/concurrency-helpers';
 import { createLogger } from '~/utils/logging';
@@ -87,8 +94,11 @@ export const postMetrics = createMetricProcessor({
  // },
 });

-async function getReactionTasks(ctx: MetricContext) {
+// Exported for the SQL-shape test: nothing in the suite executes these queries, so the
+// only way to assert the filter reaches the statement is to capture what is sent.
+export async function getReactionTasks(ctx: MetricContext) {
  log('getReactionTasks', ctx.lastUpdate);
+  const excludedFilter = snippets.excludedReactorFilter(await getMetricExcludedUserIdsOrThrow());
  const affectedImages = await ctx.ch.$query<{ imageId: number }>`
    -- get recent images with reactions
    SELECT DISTINCT entityId as imageId
@@ -99,8 +109,13 @@ async function getReactionTasks(ctx: MetricContext) {
  `;

  const affected = new Set<number>();
+  // Sorted for the same reason the post chunk below is: the query bounds each chunk with
+  // `BETWEEN ids[0] AND ids[ids.length - 1]`, and the ClickHouse query above has no
+  // ORDER BY, so an unsorted chunk whose first id exceeds its last matches nothing and
+  // those images never become affected posts. Sorted here rather than in ClickHouse so a
+  // future edit to that query cannot quietly re-break it.
  const postFetchTasks = chunk(
-    affectedImages.map((x) => x.imageId),
+    affectedImages.map((x) => x.imageId).sort((a, b) => a - b),
    30000
  ).map((ids, i) => async () => {
    ctx.jobContext.checkIfCanceled();
@@ -119,9 +134,25 @@ async function getReactionTasks(ctx: MetricContext) {
  });
  await limitConcurrency(postFetchTasks, 3);

-  const tasks = chunk([...affected], 100).map((ids, i) => async () => {
+  // Sorted because the query below bounds the chunk with
+  // `BETWEEN ids[0] AND ids[ids.length - 1]`. `affected` is a Set in insertion order, so
+  // an unsorted chunk whose first id exceeds its last matches NOTHING — and with the
+  // zero-seeding below, a chunk that matches nothing would write zeros over real counts.
+  const tasks = chunk(
+    [...affected].sort((a, b) => a - b),
+    100
+  ).map((ids, i) => async () => {
    ctx.jobContext.checkIfCanceled();
    log('getReactionTasks', i + 1, 'of', tasks.length);
+    // An entity whose remaining reactions are all excluded yields NO ROW from the
+    // aggregate below, and a missing row means "no change" to every writer downstream —
+    // so the pre-exclusion total would survive even a full recompute. Seeding zeros
+    // first makes the absence of a row mean zero; the aggregate overwrites whatever it
+    // does return.
+    for (const id of ids) {
+      const row = (ctx.updates[id] ??= { [ctx.idKey]: id });
+      for (const key of reactionCountKeys) row[key] ??= 0;
+    }
    await getMetrics(ctx)`
      -- get post reaction metrics
      SELECT
@@ -136,6 +167,7 @@ async function getReactionTasks(ctx: MetricContext) {
      JOIN "Image" i ON i.id = r."imageId"
      WHERE i."postId" IN (${ids})
        AND i."postId" BETWEEN ${ids[0]} AND ${ids[ids.length - 1]}
+        ${excludedFilter}
      GROUP BY i."postId"
    `;
    log('getReactionTasks', i + 1, 'of', tasks.length, 'done');
@@ -180,7 +212,7 @@ async function getCollectionTasks(ctx: MetricContext) {
 }

 type MetricKey = keyof PostMetric;
-type MetricContext = MetricProcessorRunContext & {
+export type MetricContext = MetricProcessorRunContext & {
  updates: Record<number, Record<string, number>>;
  idKey: string;
 };
@@ -98,6 +98,16 @@ export type NotificationProcessorRunInput = {
  lastSent: string;
  lastSentDate: Date;
  clickhouse: CustomClickHouseClient | undefined;
+  /**
+   * Read once per job run by the server-only runner and passed in, because the processor
+   * files are in the client graph and cannot import the reader.
+   *
+   * Required but nullable, not optional: the reaction milestones default a missing list to
+   * `[]`, so a caller that simply omitted the key would ship every milestone unfiltered
+   * without a sound. Required, `tsc` rejects any new caller that forgets it; nullable, a
+   * test can still say `undefined` on purpose.
+   */
+  excludedUserIds: number[] | undefined;
 };

 export function createNotificationProcessor(processor: Record<string, NotificationProcessor>) {
@@ -1,6 +1,7 @@
 import { milestoneNotificationFix } from '~/server/common/constants';
 import { NotificationCategory } from '~/server/common/enums';
 import { createNotificationProcessor } from '~/server/notifications/base.notifications';
+import { excludedReactorFilter } from '~/shared/utils/excluded-reactor-filter';

 const reactionMilestones = [5, 10, 20, 50, 100] as const;

@@ -147,7 +148,13 @@ export const bountyNotifications = createNotificationProcessor({
      }" has reached ${details.reactionCount.toLocaleString()} reactions`,
      url: `/bounties/${details.bountyId}/entries/${details.bountyEntryId}`,
    }),
-    prepareQuery: async ({ lastSent }) => `
+    prepareQuery: async ({ lastSent, excludedUserIds }) => {
+      // Same rule as the displayed bounty-entry count, and the lenient reader for the
+      // same reason as the article milestone: degrade to the pre-exclusion number rather
+      // than to silence. The reaction table is aliased `r` because that is the alias
+      // `excludedReactorFilter` emits.
+      const excludedFilter = excludedReactorFilter(excludedUserIds ?? []);
+      return `
      WITH milestones AS (
        SELECT * FROM (VALUES ${reactionMilestones.map((x) => `(${x})`).join(', ')}) m(value)
      ), affected AS (
@@ -157,11 +164,11 @@ export const bountyNotifications = createNotificationProcessor({
        WHERE "createdAt" > '${lastSent}'
      ), affected_value AS (
        SELECT
-          br."bountyEntryId",
+          r."bountyEntryId",
          COUNT(*) "reaction_count"
        FROM affected a
-        JOIN "BountyEntryReaction" br ON br."bountyEntryId" = a."bountyEntryId"
-        GROUP BY br."bountyEntryId"
+        JOIN "BountyEntryReaction" r ON r."bountyEntryId" = a."bountyEntryId" ${excludedFilter}
+        GROUP BY r."bountyEntryId"
      ), data AS (
        SELECT DISTINCT
          be."userId" "ownerId",
@@ -186,7 +193,8 @@ export const bountyNotifications = createNotificationProcessor({
        details
      FROM data
      WHERE NOT EXISTS (SELECT 1 FROM "UserNotificationSettings" WHERE "userId" = "ownerId" AND type = 'bounty-reaction-milestone')
-    `,
+    `;
+    },
  },
  // Moveable
  'bounty-entry': {
@@ -1,6 +1,7 @@
 import { milestoneNotificationFix } from '~/server/common/constants';
 import { NotificationCategory } from '~/server/common/enums';
 import { createNotificationProcessor } from '~/server/notifications/base.notifications';
+import { excludedReactorFilter } from '~/shared/utils/excluded-reactor-filter';
 import { getModelCommentThreadUrl } from '~/utils/comment-url-helpers';
 import { humanizeList } from '~/utils/humanizer';

@@ -107,7 +108,13 @@ export const reactionNotifications = createNotificationProcessor({

      return { message, url: `/articles/${details.articleId}` };
    },
-    prepareQuery: ({ lastSent }) => `
+    prepareQuery: async ({ lastSent, excludedUserIds }) => {
+      // The displayed article count filters metric-suppressed accounts; a milestone that
+      // did not would congratulate someone on a number their own page never shows. The
+      // LENIENT reader on purpose — a failed read here degrades to the pre-exclusion
+      // count, which is how this fired before, rather than to no notification at all.
+      const excludedFilter = excludedReactorFilter(excludedUserIds ?? []);
+      return `
      WITH milestones AS (
        SELECT * FROM (VALUES ${articleReactionMilestones.map((x) => `(${x})`).join(', ')}) m(value)
      ), affected AS (
@@ -120,7 +127,7 @@ export const reactionNotifications = createNotificationProcessor({
          a.affected_id,
          COUNT(r."articleId") reaction_count
        FROM "ArticleReaction" r
-        JOIN affected a ON a.affected_id = r."articleId"
+        JOIN affected a ON a.affected_id = r."articleId" ${excludedFilter}
        GROUP BY a.affected_id
        HAVING COUNT(*) >= ${articleReactionMilestones[0]}
      ), reaction_milestone AS (
@@ -143,6 +150,7 @@ export const reactionNotifications = createNotificationProcessor({
        details
      FROM reaction_milestone
      WHERE NOT EXISTS (SELECT 1 FROM "UserNotificationSettings" WHERE "userId" = "ownerId" AND type = 'article-reaction-milestone')
-    `,
+    `;
+    },
  },
 });
@@ -0,0 +1,34 @@
+import { describe, expect, it, vi } from 'vitest';
+
+/**
+ * Its own file because `vi.mock` is per-module: every other test of this service supplies
+ * a present ClickHouse client, so the two `!clickhouse` branches were reachable by no
+ * test at all. Mutating the strict one to `return []` is the whole defect — metric jobs
+ * writing permanently unfiltered totals wherever ClickHouse is unconfigured — and it was
+ * green against the source guard, which only checks the export exists.
+ */
+
+vi.mock('~/server/clickhouse/client', () => ({ clickhouse: undefined }));
+vi.mock('~/server/utils/cache-helpers', () => ({
+  fetchThroughCache: vi.fn(() => {
+    throw new Error('fetchThroughCache must not be reached without a clickhouse client');
+  }),
+}));
+import '~/__tests__/mocks/logging.mock';
+
+const { getMetricExcludedUserIds, getMetricExcludedUserIdsOrThrow } = await import(
+  '~/server/services/metric-excluded-users.service'
+);
+
+describe('with no clickhouse client', () => {
+  it('the strict reader rejects rather than reporting an empty exclusion list', async () => {
+    await expect(getMetricExcludedUserIdsOrThrow()).rejects.toThrow(
+      'clickhouse client unavailable'
+    );
+  });
+
+  it('the lenient reader still degrades to []', async () => {
+    // Asserted beside the strict one because the property is the difference between them.
+    await expect(getMetricExcludedUserIds()).resolves.toEqual([]);
+  });
+});
@@ -0,0 +1,126 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type * as CacheHelpers from '~/server/utils/cache-helpers';
+// The lenient reader logs the outage it swallows; the canonical mock keeps that off the
+// wire and gives a stable spy to assert the strict reader's own report on.
+import { loggingMock } from '~/__tests__/mocks/logging.mock';
+
+/**
+ * The two readers of the exclusion list must fail in opposite directions.
+ *
+ * The lenient one degrades to `[]`, so the reaction milestone keeps firing on an
+ * unfiltered count during an outage rather than going silent. The strict one rejects,
+ * because a metric job that wrote an unfiltered total would bake it in permanently:
+ * `PostMetric`/`ArticleMetric`/`BountyEntryMetric` are only recomputed for entities
+ * that receive another reaction, so a quiet entity never gets a second chance.
+ */
+
+const h = vi.hoisted(() => ({
+  fetchThroughCache: vi.fn(),
+  chQuery: vi.fn(),
+}));
+
+vi.mock('~/server/utils/cache-helpers', async (importOriginal) => ({
+  ...(await importOriginal<typeof CacheHelpers>()),
+  fetchThroughCache: h.fetchThroughCache,
+}));
+
+vi.mock('~/server/clickhouse/client', () => ({
+  clickhouse: { $query: h.chQuery },
+}));
+
+const { getMetricExcludedUserIds, getMetricExcludedUserIdsOrThrow } = await import(
+  '~/server/services/metric-excluded-users.service'
+);
+
+/** Runs the real fetch function, so the row mapping and filtering are exercised. */
+const cachePassthrough = () =>
+  h.fetchThroughCache.mockImplementation(async (_key: string, fn: () => Promise<unknown>) => fn());
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  h.fetchThroughCache.mockReset();
+  h.chQuery.mockReset();
+});
+
+describe('getMetricExcludedUserIdsOrThrow', () => {
+  it('returns the same ids as the lenient reader when the read succeeds', async () => {
+    cachePassthrough();
+    h.chQuery.mockResolvedValue([{ userId: 11 }, { userId: 22 }]);
+
+    await expect(getMetricExcludedUserIdsOrThrow()).resolves.toEqual([11, 22]);
+    await expect(getMetricExcludedUserIds()).resolves.toEqual([11, 22]);
+  });
+
+  it('rejects when the read fails, where the lenient reader returns []', async () => {
+    h.fetchThroughCache.mockRejectedValue(new Error('clickhouse unreachable'));
+
+    // Asserted as a pair, in one test, because the property is the DIFFERENCE. Split
+    // across two tests, deleting the strict reader and re-pointing its callers at the
+    // lenient one leaves a green suite.
+    await expect(getMetricExcludedUserIdsOrThrow()).rejects.toThrow('clickhouse unreachable');
+    await expect(getMetricExcludedUserIds()).resolves.toEqual([]);
+  });
+
+  it('rejects when the cache hands back something that is not an array', async () => {
+    // `fetchThroughCache` returns any present `data` unvalidated, so this is a real
+    // shape rather than a defensive one.
+    h.fetchThroughCache.mockResolvedValue(null);
+
+    await expect(getMetricExcludedUserIdsOrThrow()).rejects.toThrow('not an array');
+    await expect(getMetricExcludedUserIds()).resolves.toEqual([]);
+  });
+
+  it('still reports the stalled metric run after the lenient reader already reported', async () => {
+    // The report is deduped per outage so the reaction write path cannot amplify logs.
+    // Deduped on ONE flag it was unreachable: the lenient reader runs on every reaction
+    // toggle, so it fails first, claims the slot, and the only line for the whole incident
+    // says a notification degraded — while the metric jobs stall silently once a minute.
+    // The dedupe set is module scope and survives `clearAllMocks`, so an earlier failing
+    // test would otherwise leave it populated and these assertions would read a report
+    // that never happened. One successful read clears it.
+    cachePassthrough();
+    h.chQuery.mockResolvedValue([]);
+    await getMetricExcludedUserIds();
+    loggingMock.logToAxiom.mockClear();
+
+    h.fetchThroughCache.mockRejectedValue(new Error('clickhouse unreachable'));
+
+    await expect(getMetricExcludedUserIds()).resolves.toEqual([]);
+    await expect(getMetricExcludedUserIdsOrThrow()).rejects.toThrow();
+
+    const messages = loggingMock.logToAxiom.mock.calls.map(
+      ([arg]: [{ message: string }]) => arg.message
+    );
+    expect(messages).toContain('Exclusion list unavailable, falling back to an unfiltered count');
+    expect(messages).toContain('Exclusion list unavailable, skipping the metric run');
+  });
+
+  it('does not repeat the same outcome while the outage continues', async () => {
+    // The other half of the property: keyed per outcome, not per call.
+    // The dedupe set is module scope and survives `clearAllMocks`, so an earlier failing
+    // test would otherwise leave it populated and these assertions would read a report
+    // that never happened. One successful read clears it.
+    cachePassthrough();
+    h.chQuery.mockResolvedValue([]);
+    await getMetricExcludedUserIds();
+    loggingMock.logToAxiom.mockClear();
+
+    h.fetchThroughCache.mockRejectedValue(new Error('clickhouse unreachable'));
+
+    await expect(getMetricExcludedUserIds()).resolves.toEqual([]);
+    await expect(getMetricExcludedUserIds()).resolves.toEqual([]);
+
+    const lenient = loggingMock.logToAxiom.mock.calls.filter(
+      ([arg]: [{ message: string }]) =>
+        arg.message === 'Exclusion list unavailable, falling back to an unfiltered count'
+    );
+    expect(lenient).toHaveLength(1);
+  });
+
+  it('drops a null userId rather than suppressing user 0', async () => {
+    cachePassthrough();
+    h.chQuery.mockResolvedValue([{ userId: null }, { userId: 7 }]);
+
+    await expect(getMetricExcludedUserIdsOrThrow()).resolves.toEqual([7]);
+  });
+});
@@ -0,0 +1,307 @@
+import { readFileSync, readdirSync, statSync } from 'fs';
+import path from 'path';
+import { describe, expect, it } from 'vitest';
+
+/**
+ * Anything that turns reaction rows into a number must exclude the metric-suppressed
+ * accounts — the displayed metric AND the milestone notification for the same entity.
+ *
+ * Every ClickHouse path that produces a reaction total already filters them. The
+ * Postgres sums did not, so the number a viewer saw on a post, article or bounty entry
+ * counted accounts the platform had already decided not to count. Fixing only the
+ * displayed half would have been worse than fixing neither: the milestone would then
+ * congratulate a creator on a number their own page never shows, which is the defect
+ * this one is a sibling of.
+ *
+ * It is pinned textually because nothing in the suite executes this SQL: the queries are
+ * template literals handed to `pg`, so a deleted `${excludedFilter}` is invisible to the
+ * typechecker, to lint and to every suite. The thing under test is the text.
+ *
+ * 🔴 If you are about to delete this: the unfiltered sum is not a stale value that a
+ * re-run heals. The metric tables are recomputed from the same Postgres rows every time,
+ * so an unfiltered total is permanent until the query itself filters.
+ *
+ * What this guard does NOT do, stated so a green run is not over-read: it ratchets the
+ * call sites listed in EXPECTED_SITES. A reaction total written in a shape the scan does
+ * not recognise, or in a directory it does not read, is not covered — rewriting an
+ * existing site into such a shape goes red, but a brand new one does not.
+ */
+
+const repoRoot = path.resolve(__dirname, '../../../..');
+
+/**
+ * Two directories, two different correct ways to get the list, so the scan carries which
+ * is which.
+ *
+ * A metric job reads it itself, through the THROWING reader: it writes a total that
+ * nothing later recomputes, so degrading to an unfiltered count on a failed read would be
+ * permanent. A notification processor cannot read it at all — the processor files are in
+ * the client graph, where the reader's ClickHouse import is forbidden and a dynamic import
+ * does not help — so the server-only runner reads it with the LENIENT reader and passes it
+ * in. Degrading to the pre-exclusion count is how the milestones behaved before; the
+ * alternative is one that silently never fires.
+ */
+const SCOPES = [
+  {
+    dir: 'src/server/metrics',
+    reader: 'getMetricExcludedUserIdsOrThrow',
+    build: 'snippets.excludedReactorFilter(await getMetricExcludedUserIdsOrThrow())',
+  },
+  {
+    dir: 'src/server/notifications',
+    reader: null,
+    build: 'excludedReactorFilter(excludedUserIds ?? [])',
+  },
+] as const;
+
+const NOTIFICATION_RUNNER = 'src/server/jobs/send-notifications.ts';
+
+/**
+ * Reaction tables whose count is NOT expected to filter, with the reason. Both belong to
+ * the retired Q&A feature and neither surfaces on a browsable feed, so they were left
+ * out of the scope this guard was written for rather than overlooked.
+ *
+ * The list's length is asserted below: adding a table here has to be a visible change,
+ * because an exemption is how the same defect gets written again unseen.
+ */
+const EXEMPT_TABLES = ['AnswerReaction', 'QuestionReaction'] as const;
+
+/**
+ * The call sites this guard expects to find. A regex that matches nothing passes every
+ * prohibition in this file, so the set it discovered is asserted before it is judged.
+ */
+const EXPECTED_SITES = [
+  'src/server/metrics/article.metrics.ts:ArticleReaction',
+  'src/server/metrics/bountyEntry.metrics.ts:BountyEntryReaction',
+  'src/server/metrics/post.metrics-old.ts:ImageReaction',
+  'src/server/metrics/post.metrics.ts:ImageReaction',
+  'src/server/notifications/bounty.notifications.ts:BountyEntryReaction',
+  'src/server/notifications/reaction.notifications.ts:ArticleReaction',
+] as const;
+
+/**
+ * Sites left unfiltered on purpose, each with the reason it is not a divergence.
+ *
+ * The two Q&A ones are the retired feature. The comment milestone is different: the
+ * displayed comment reaction count is not filtered either, so the pair still AGREES.
+ * Filtering one half is what creates the defect this guard exists for, so the comment
+ * milestone moves when the comment display does, not before.
+ */
+const EXEMPT_SITES = [
+  'src/server/metrics/answer.metrics.ts:AnswerReaction',
+  'src/server/metrics/question.metrics.ts:QuestionReaction',
+  'src/server/notifications/reaction.notifications.ts:CommentReaction',
+] as const;
+
+type Site = {
+  rel: string;
+  table: string;
+  literal: string;
+  reader: string | null;
+  build: string;
+};
+
+/** Odd-indexed chunks of a backtick split are the template literals. */
+function templateLiterals(text: string) {
+  return text.split('`').filter((_, i) => i % 2 === 1);
+}
+
+function walk(dir: string, out: string[] = []) {
+  for (const entry of readdirSync(dir)) {
+    if (entry === '__tests__' || entry.startsWith('.')) continue;
+    const full = path.join(dir, entry);
+    if (statSync(full).isDirectory()) walk(full, out);
+    else if (entry.endsWith('.ts')) out.push(full);
+  }
+  return out;
+}
+
+const sites: Site[] = [];
+const fileText = new Map<string, string>();
+for (const scope of SCOPES) {
+  for (const full of walk(path.join(repoRoot, scope.dir))) {
+    const rel = path.relative(repoRoot, full).split(path.sep).join('/');
+    const text = readFileSync(full, 'utf8');
+    fileText.set(rel, text);
+    for (const literal of templateLiterals(text)) {
+      // A query that only locates affected ids needs no filter — counting an excluded
+      // user's reaction as "this entity changed" is harmless, because the recompute that
+      // follows is the thing that must exclude them. What must filter is a query that
+      // turns reaction rows into a number.
+      // `reactionTimeframes` is an interpolation, so a job using it has no literal
+      // `SUM(` in its own text — matching only SUM/COUNT silently dropped two sites.
+      const counts = /\b(SUM|COUNT)\s*\(/i.test(literal) || literal.includes('reactionTimeframes');
+      if (!counts) continue;
+      // FROM *or* JOIN: the bounty-entry job drives from the affected ids and reaches the
+      // reaction table through a LEFT JOIN, so a FROM-only match loses exactly the site
+      // whose shape changed most.
+      for (const [, table] of literal.matchAll(/(?:FROM|JOIN)\s+"(\w+Reaction)"/g)) {
+        sites.push({ rel, table, literal, reader: scope.reader, build: scope.build });
+      }
+    }
+  }
+}
+
+const key = (s: Site) => `${s.rel}:${s.table}`;
+const isExempt = (s: Site) =>
+  (EXEMPT_TABLES as readonly string[]).includes(s.table) ||
+  (EXEMPT_SITES as readonly string[]).includes(key(s));
+const covered = sites.filter((s) => !isExempt(s));
+
+describe('no unfiltered reaction count', () => {
+  it('found the reaction aggregates it is meant to judge', () => {
+    // Without this the file is vacuous: every assertion below is a prohibition, and a
+    // scan that discovers nothing satisfies all of them.
+    const found = [...new Set(sites.map(key))].sort();
+    const expected = [...EXPECTED_SITES, ...EXEMPT_SITES].sort();
+
+    expect(
+      found,
+      'The scan no longer finds the reaction counts this guard exists to cover. A job or ' +
+        'notification was renamed, moved, or rewritten — fix the scan, do not delete the guard.'
+    ).toEqual(expected);
+  });
+
+  it('the exemption lists are the size they are — widening either must be visible', () => {
+    // Both, not just the tables: EXEMPT_SITES carries the comment-milestone carve-out, so
+    // a line added there moves a real site out of `covered` with nothing else noticing.
+    expect(EXEMPT_TABLES).toHaveLength(2);
+    expect(EXEMPT_SITES).toHaveLength(3);
+  });
+
+  it('every reaction count splices the exclusion filter', () => {
+    const offenders = covered.filter((s) => !s.literal.includes('${excludedFilter}')).map(key);
+
+    expect(
+      offenders,
+      'These queries count a reaction table without splicing `${excludedFilter}`, so the ' +
+        'number a viewer sees includes metric-suppressed accounts. Build the snippet with ' +
+        '`snippets.excludedReactorFilter(...)`.'
+    ).toEqual([]);
+  });
+
+  it('no splice is commented out', () => {
+    // `literal.includes(...)` is satisfied by `-- ${excludedFilter}` and by
+    // `/* ${excludedFilter} */`, both of which Postgres applies as nothing. Both were
+    // measured green before this assertion covered them.
+    const offenders = covered
+      .filter(
+        (s) =>
+          s.literal
+            .split('\n')
+            .some(
+              (line) =>
+                line.includes('${excludedFilter}') && /--[^\n]*\$\{excludedFilter\}/.test(line)
+            ) || /\/\*[\s\S]*?\$\{excludedFilter\}[\s\S]*?\*\//.test(s.literal)
+      )
+      .map(key);
+
+    expect(
+      offenders,
+      'The exclusion filter is inside an SQL line comment in these queries, so it is spliced ' +
+        'into the statement and then ignored.'
+    ).toEqual([]);
+  });
+
+  it('every filtering file gets the list the way its scope requires', () => {
+    // The two readers differ by a suffix, so the lenient one is matched only where
+    // `OrThrow` does not follow it.
+    const lenient = /getMetricExcludedUserIds(?!OrThrow)/;
+    const offenders = [...new Set(covered.map((s) => s.rel))]
+      .map((rel) => ({ rel, site: covered.find((s) => s.rel === rel)! }))
+      .filter(({ rel, site }) => {
+        const text = fileText.get(rel)!;
+        if (site.reader === 'getMetricExcludedUserIdsOrThrow') {
+          return !text.includes('getMetricExcludedUserIdsOrThrow') || lenient.test(text);
+        }
+        // A notification processor must not import either reader: it is client-graph code.
+        return lenient.test(text) || text.includes('getMetricExcludedUserIdsOrThrow');
+      })
+      .map(({ rel }) => rel);
+
+    expect(
+      offenders,
+      'A metric job must read the list with `getMetricExcludedUserIdsOrThrow` — a lenient read ' +
+        'turns an outage into a permanently unfiltered total, because the jobs only revisit an ' +
+        'entity that receives another reaction. A notification processor must not read it at ' +
+        'all: it is in the client graph, and the runner passes the list in.'
+    ).toEqual([]);
+  });
+
+  it('the notification runner reads the list leniently and passes it to every processor', () => {
+    // The processors default a missing list to [], which is the lenient posture — so a
+    // runner that stopped passing it would degrade EVERY milestone to unfiltered, silently.
+    // This is the one place that decides whether they are filtered at all.
+    const runner = readFileSync(path.join(repoRoot, NOTIFICATION_RUNNER), 'utf8');
+    expect(runner).toMatch(/const excludedUserIds = await getMetricExcludedUserIds\(\);/);
+    expect(runner).not.toContain('getMetricExcludedUserIdsOrThrow');
+    expect(runner).toMatch(/prepareQuery\?\.\(\{[\s\S]*?excludedUserIds,[\s\S]*?\}\)/);
+  });
+
+  it('builds the filter from exactly the expression its scope requires', () => {
+    // Prohibiting `.catch` was not enough: `.then(x => x).catch(() => [])` and a plain
+    // `try { … } catch { /* degrade */ }` around the strict call both restore the whole
+    // defect and were measured green against a name-based check. So require the exact
+    // expression instead of enumerating the ways to avoid it — anything that routes the
+    // strict read through a variable has somewhere to swallow the rejection.
+    const offenders = [...new Set(covered.map((s) => s.rel))].filter((rel) => {
+      const site = covered.find((s) => s.rel === rel)!;
+      return !fileText.get(rel)!.includes(site.build);
+    });
+
+    expect(
+      offenders,
+      'A metric job must build the filter as ' +
+        '`snippets.excludedReactorFilter(await getMetricExcludedUserIdsOrThrow())`, with no ' +
+        'intermediate variable: a try/catch or .catch around the strict read is the lenient ' +
+        'reader written the long way. A notification processor must build it as ' +
+        '`excludedReactorFilter(excludedUserIds ?? [])` from the list the runner passes in.'
+    ).toEqual([]);
+  });
+
+  it('aliases the reaction table `r`, and nothing else', () => {
+    // The emitted filter hardcodes `r."userId"`. Swapping the aliases so the REACTION
+    // table is `ir` and `Image` is `r` is valid SQL that filters by the post's owner
+    // instead of the reactor — measured green against every other assertion here.
+    const offenders = covered
+      .filter((s) => {
+        const wrongReactionAlias = [
+          ...s.literal.matchAll(/(?:FROM|JOIN)\s+"(\w+Reaction)"\s+(\w+)/g),
+        ].some(([, , alias]) => alias !== 'r' && !/^(ON|WHERE|GROUP|LEFT|CROSS)$/i.test(alias));
+        const rBoundElsewhere = [...s.literal.matchAll(/(?:FROM|JOIN)\s+"(\w+)"\s+r\b/g)].some(
+          ([, table]) => !table.endsWith('Reaction')
+        );
+        return wrongReactionAlias || rBoundElsewhere;
+      })
+      .map(key);
+
+    expect(
+      offenders,
+      'In these queries the alias `r` is not the reaction table. The exclusion filter is ' +
+        'emitted as `AND r."userId" NOT IN (...)`, so it would filter on whatever `r` is ' +
+        'bound to — valid SQL, wrong person, no error.'
+    ).toEqual([]);
+  });
+
+  it('keeps the bounty-entry filter in the LEFT JOIN, not a WHERE', () => {
+    // Moving it to a WHERE collapses the LEFT JOIN back to an inner join: for an entry
+    // whose remaining reactions are all excluded, `NULL NOT IN (...)` is NULL, the row is
+    // dropped, no row comes back, and the pre-exclusion total survives the recompute —
+    // which is the exact defect the rewrite exists to prevent. Pinned as a shape because
+    // nothing executes this SQL.
+    const text = fileText.get('src/server/metrics/bountyEntry.metrics.ts')!;
+    expect(text).toMatch(
+      /LEFT JOIN "BountyEntryReaction" r\s*\n\s*ON r\."bountyEntryId" = be\.id\s*\n\s*\$\{excludedFilter\}/
+    );
+  });
+
+  it('the strict reader still exists under that name', () => {
+    // The prohibitions above are all satisfied by deleting the feature. This is the one
+    // assertion that fails if the reader is renamed away rather than misused.
+    const service = readFileSync(
+      path.join(repoRoot, 'src/server/services/metric-excluded-users.service.ts'),
+      'utf8'
+    );
+    expect(service).toContain('export async function getMetricExcludedUserIdsOrThrow');
+  });
+});
@@ -7,9 +7,9 @@ import { logToAxiom } from '~/server/logging/client';
 /**
 * Users suppressed from metrics by the reaction-abuse detector
 * (`/api/admin/reaction-abuse`). Every ClickHouse path that produces a metric total
- * filters them, and as of #4584 so does the event-engine's Redis cache — but a
- * Postgres `count()` over `ImageReaction` does not, which is why the reaction
- * milestone fires on numbers no displayed count agrees with.
+ * filters them, as of #4584 so does the event-engine's Redis cache, and the reaction
+ * sums in `src/server/metrics/*.metrics.ts` read this list through
+ * `getMetricExcludedUserIdsOrThrow`.
 *
 * These accounts are NOT banned, deleted or muted: the list suppresses metrics only.
 */
@@ -29,52 +29,91 @@ export async function getMetricExcludedUserIds(): Promise<number[]> {
  if (!clickhouse) return [];

  try {
-    const cached = await fetchThroughCache(
-      REDIS_KEYS.CACHES.METRIC_EXCLUDED_USERS,
-      async () => {
-        const rows = await clickhouse!.$query<{ userId: number }>`
-          SELECT userId FROM metricExcludedUsers FINAL WHERE active = 1
-        `;
-        // > 0 because `Number(null)` is 0, not NaN: a null column would otherwise
-        // enter the list as user 0 and silently suppress whatever writes that id.
-        // `isFinite` is belt-and-braces here, kept for parity with the identical
-        // guard in metric-reaction-repair.service.ts and the event-engine copy.
-        return rows.map((r) => Number(r.userId)).filter((id) => Number.isFinite(id) && id > 0);
-      },
-      // Passed explicitly, not because it differs from fetchThroughCache's default —
-      // it does not — but so a change to that default cannot silently widen this past
-      // the "within ~5 min" the admin endpoint promises.
-      { ttl: CACHE_TTL }
-    );
-
-    // The cache read is the one failure this function's own try does NOT cover:
-    // `fetchThroughCache` returns any present `data` unvalidated, and a `null` would
-    // reach the caller's `.length` outside this catch — a thrown TypeError, which the
-    // caller's `.catch(handleLogError)` turns into the silent skip this whole design
-    // exists to avoid. Validate the shape rather than trust the type.
-    if (!Array.isArray(cached)) throw new Error('cached exclusion list was not an array');
-    unavailable = false;
-    return cached;
+    return await fetchExcludedUserIds();
  } catch (error) {
-    reportUnavailable(error);
+    reportUnavailable(error, 'falling back to an unfiltered count');
    return [];
  }
 }

 /**
- * Logged once per outage — on the first failure, and again only after a success has
- * reset the flag. This runs on every created reaction, so once an outage outlives the
- * cache entry every reaction would otherwise emit its own Axiom ingest, turning the
- * busiest write path into a log amplifier exactly when infrastructure is degraded.
+ * Same list, but a read failure rejects instead of degrading to `[]`.
+ *
+ * The lenient reader above is right for the notification path, where an unfiltered
+ * count is a wrong number shown once. It is wrong for a metric job: the Postgres
+ * reaction sums never decay, so a total written unfiltered during an outage stays
+ * wrong until that entity happens to receive another reaction — which for a quiet
+ * post is never. `createMetricProcessor` calls `setLastUpdate()` and `queue.commit()`
+ * only after `update()` resolves, so rejecting leaves the cursor and the queue where
+ * they were and the window is recomputed on the next run.
+ *
+ * What stops is the whole metric SET, not just the reaction part: `update-metrics.ts`
+ * runs each set's processors sequentially with no per-processor catch, so a throw here
+ * also skips the sibling processor and the rank refresh for that set.
 */
-let unavailable = false;
-function reportUnavailable(error: unknown) {
-  if (unavailable) return;
-  unavailable = true;
+export async function getMetricExcludedUserIdsOrThrow(): Promise<number[]> {
+  if (!clickhouse) throw new Error('clickhouse client unavailable');
+  try {
+    return await fetchExcludedUserIds();
+  } catch (error) {
+    // Reported as well as thrown. The rejection surfaces only as a generic job-error for
+    // whichever metric set happened to call first, which does not say that the metric
+    // jobs are stalled or why.
+    reportUnavailable(error, 'skipping the metric run');
+    throw error;
+  }
+}
+
+async function fetchExcludedUserIds(): Promise<number[]> {
+  const cached = await fetchThroughCache(
+    REDIS_KEYS.CACHES.METRIC_EXCLUDED_USERS,
+    async () => {
+      const rows = await clickhouse!.$query<{ userId: number }>`
+        SELECT userId FROM metricExcludedUsers FINAL WHERE active = 1
+      `;
+      // > 0 because `Number(null)` is 0, not NaN: a null column would otherwise
+      // enter the list as user 0 and silently suppress whatever writes that id.
+      // `isFinite` is belt-and-braces here, matching the event-engine copy.
+      // metric-reaction-repair.service.ts reads the same rows WITHOUT this coercion,
+      // which is inert only because no row has a null userId today.
+      return rows.map((r) => Number(r.userId)).filter((id) => Number.isFinite(id) && id > 0);
+    },
+    // Passed explicitly, not because it differs from fetchThroughCache's default —
+    // it does not — but so a change to that default cannot silently widen this past
+    // the "within ~5 min" the admin endpoint promises.
+    { ttl: CACHE_TTL }
+  );
+
+  // `fetchThroughCache` returns any present `data` unvalidated, so a `null` would
+  // otherwise reach a caller's `.length` as a thrown TypeError from outside the
+  // lenient reader's catch — which its caller's `.catch(handleLogError)` turns into
+  // the silent skip that design exists to avoid. Validate the shape rather than
+  // trust the type.
+  if (!Array.isArray(cached)) throw new Error('cached exclusion list was not an array');
+  reportedOutcomes.clear();
+  return cached;
+}
+
+/**
+ * Logged once per outage PER OUTCOME, and again only after a success has cleared the set.
+ * The lenient reader runs on every created reaction, so once an outage outlives the cache
+ * entry every reaction would otherwise emit its own Axiom ingest, turning the busiest
+ * write path into a log amplifier exactly when infrastructure is degraded.
+ *
+ * Keyed by outcome rather than a single flag, because a single flag made the metric-job
+ * report unreachable: the lenient reader fails within milliseconds of an outage starting
+ * and would claim the one slot, so the only line for the whole incident said a
+ * notification had degraded, while the metric jobs stalled silently once a minute — which
+ * is the thing the reporting was added to make visible.
+ */
+const reportedOutcomes = new Set<string>();
+function reportUnavailable(error: unknown, outcome: string) {
+  if (reportedOutcomes.has(outcome)) return;
+  reportedOutcomes.add(outcome);
  logToAxiom({
    type: 'warning',
    name: 'metric-excluded-users-unavailable',
-    message: 'Falling back to an unfiltered count',
+    message: `Exclusion list unavailable, ${outcome}`,
    details: { error: (error as Error)?.message },
  }).catch(() => undefined);
 }
@@ -0,0 +1,22 @@
+import { describe, expect, it } from 'vitest';
+import { excludedReactorFilter } from '~/shared/utils/excluded-reactor-filter';
+
+// Only the non-empty path is reached through the job tests; these two branches are not.
+// The throw matters most: this builds SQL TEXT, so an id it cannot vouch for must stop the
+// query rather than be dropped — dropping it would keep counting that user's reactions.
+describe('excludedReactorFilter', () => {
+  it('emits nothing for an empty list, so the query is valid and unfiltered', () => {
+    expect(excludedReactorFilter([])).toBe('');
+  });
+
+  it('emits the predicate against the reaction alias', () => {
+    expect(excludedReactorFilter([7, 9])).toBe('AND r."userId" NOT IN (7,9)');
+  });
+
+  it.each([[[1.5]], [[Number.NaN]], [['1; DROP TABLE x' as unknown as number]]])(
+    'refuses %j rather than splicing it into SQL',
+    (ids) => {
+      expect(() => excludedReactorFilter(ids)).toThrow('non-integer excluded user id');
+    }
+  );
+});
@@ -0,0 +1,27 @@
+/**
+ * `AND r."userId" NOT IN (...)` for the metric-excluded users, or `''` when the list is
+ * empty. Emitted as literal SQL rather than a bound parameter because the reaction
+ * queries run through `templateHandler`, which interpolates; a string is the one value
+ * both template handlers pass through verbatim, so one snippet serves both.
+ *
+ * Lives in `~/shared/` with no imports because the notification processors that use it
+ * are in the client graph — `prepareMessage` renders there — and a dynamic import does
+ * not keep a module out of the client bundle.
+ *
+ * The column is hardcoded rather than a parameter. Every reaction aggregate aliases its
+ * reaction table `r`, and a parameter here would be raw SQL text that the integer guard
+ * beside it does not cover — while reading as though it did. A caller passing the wrong
+ * alias (`i."userId"` in the post job, which joins `Image i`) is valid SQL that filters
+ * by the post's OWNER instead of the reactor.
+ *
+ * Non-integer ids throw. They cannot arrive from `metric-excluded-users.service`, which
+ * already coerces — but this builds SQL text, and dropping an unexpected id would
+ * silently keep counting that user's reactions, which is the bug this exists to fix.
+ */
+export function excludedReactorFilter(excludedUserIds: number[]) {
+  if (!excludedUserIds.length) return '';
+  for (const id of excludedUserIds) {
+    if (!Number.isInteger(id)) throw new Error(`non-integer excluded user id: ${id}`);
+  }
+  return `AND r."userId" NOT IN (${excludedUserIds.join(',')})`;
+}