From ce53e21ead827c0e98258e21465e0f462f43ccc3 Mon Sep 17 00:00:00 2001
From: Chris Coutinho <chris@coutinho.io>
Date: Sat, 13 Jun 2026 14:10:01 +0200
Subject: [PATCH] fix(ingest): zero queue-depth gauge on all-queues-drained
 (review round 4)

- metrics: update_ingest_queue_depth guarded on `not by_queue`, which conflated
  None (memory backend no-op) with {} (postgres, ALL queues drained). When every
  queue drains at once, get_ingest_job_counts_by_queue returns {} and the
  pre-zero loop was skipped, leaving a stale ghost backlog in the gauge. Guard on
  `by_queue is None` only; add an all-drained regression test.
- procrastinate: note that INGEST_TRANSIENT_MAX_ATTEMPTS is snapshotted at
  blueprint-build time (restart to pick up changes).

Deck #323.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 nextcloud_mcp_server/observability/metrics.py    | 16 ++++++++++------
 .../vector/queue/procrastinate.py                |  3 +++
 tests/unit/test_ingest_queue_depth_metric.py     | 14 ++++++++++++++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/nextcloud_mcp_server/observability/metrics.py b/nextcloud_mcp_server/observability/metrics.py
index 71dc4728..74dab2a5 100644
--- a/nextcloud_mcp_server/observability/metrics.py
+++ b/nextcloud_mcp_server/observability/metrics.py
@@ -656,16 +656,20 @@ def update_ingest_queue_depth(by_queue: dict[str, dict[str, int]] | None) -> Non
     """Set the per-tier-queue depth gauge from procrastinate job counts (#323).
 
     ``by_queue`` is ``{queue_name: {status: count}}`` (see
-    ``queue.procrastinate.get_ingest_job_counts_by_queue``). No-op on the memory
-    backend (``by_queue`` is None).
+    ``queue.procrastinate.get_ingest_job_counts_by_queue``). No-op only on the
+    memory backend (``by_queue is None``); an empty dict (postgres backend with
+    every queue drained) still runs the pre-zero so the gauge reads 0.
 
     Every managed queue is zeroed first: ``list_queues_async`` stops returning a
     queue once it has no jobs, so a queue that drained to empty drops out of
-    ``by_queue`` entirely. Without the pre-zero its gauge series would stick at
-    its last non-zero value (ghost backlog in Grafana/alerts) instead of reading
-    0. The live counts then overwrite the zeros for queues that still have work.
+    ``by_queue`` entirely (and when ALL drain, ``by_queue`` is ``{}``). Without
+    the pre-zero its gauge series would stick at its last non-zero value (ghost
+    backlog in Grafana/alerts) instead of reading 0. The live counts then
+    overwrite the zeros for queues that still have work.
     """
-    if not by_queue:
+    # ``is None`` not ``not by_queue``: an empty dict means "postgres, all queues
+    # drained" and MUST still zero the gauge -- only None (memory) is the no-op.
+    if by_queue is None:
         return
     # Lazy import to keep observability decoupled from the queue layer at module
     # load (and sidestep any import cycle); both names are public constants.
diff --git a/nextcloud_mcp_server/vector/queue/procrastinate.py b/nextcloud_mcp_server/vector/queue/procrastinate.py
index bc511f44..4b191beb 100644
--- a/nextcloud_mcp_server/vector/queue/procrastinate.py
+++ b/nextcloud_mcp_server/vector/queue/procrastinate.py
@@ -379,6 +379,9 @@ def _build_ingest_blueprint() -> Blueprint:
         # BaseRetryStrategy subclass is the documented extension point (and is
         # accepted at runtime by get_retry_strategy). The annotation is just too
         # narrow, hence the ignore.
+        # Settings are snapshotted here at blueprint-build time (build_app, first
+        # use), so a restart is needed to pick up INGEST_TRANSIENT_MAX_ATTEMPTS
+        # changes -- intentional: the strategy lives for the App's lifetime.
         retry=TieredEscalationStrategy(
             max_transient_attempts=get_settings().ingest_transient_max_attempts
         ),
diff --git a/tests/unit/test_ingest_queue_depth_metric.py b/tests/unit/test_ingest_queue_depth_metric.py
index 0fb1a169..1665d392 100644
--- a/tests/unit/test_ingest_queue_depth_metric.py
+++ b/tests/unit/test_ingest_queue_depth_metric.py
@@ -39,3 +39,17 @@ def test_none_is_noop(metric_sample):
     assert metric_sample(
         _METRIC, {"queue": "ingest-fast", "status": "doing"}
     ) == approx(2)
+
+
+def test_all_queues_drained_empty_dict_zeroes(metric_sample):
+    # postgres backend with every queue drained → get_ingest_job_counts_by_queue
+    # returns {} (list_queues_async drops empty queues). An empty dict is NOT the
+    # memory-backend no-op: it must still zero every managed queue's gauge.
+    update_ingest_queue_depth({"ingest-fast": {"todo": 9}})
+    assert metric_sample(_METRIC, {"queue": "ingest-fast", "status": "todo"}) == approx(
+        9
+    )
+    update_ingest_queue_depth({})
+    assert metric_sample(_METRIC, {"queue": "ingest-fast", "status": "todo"}) == approx(
+        0
+    )