fix(telemetry): usage readout grouped by a rebuilt CASE — group by the label instead (#2663 root cause)

CI's new integration tests reproduced the outage on a clean database and
named the half: the writes land fine, and usage_for_notes fails on EVERY
call. The GROUP BY rebuilt the ambient case() expression, and asyncpg's
expanding IN-parameters give each instance its own bind names — so Postgres
sees a SELECT expression the GROUP BY doesn't cover and rejects the query
with a GroupingError, which the old code swallowed into zeros. One labelled
expression, grouped by its label. The deployed table has been accumulating
events all along; history appears as soon as this deploys.

Also: the two hook-execution tests now run with the real PATH and skip
when the hook's tools are absent (the restricted-PATH convention next door
is for silence contracts, where empty-for-the-wrong-reason still passes) —
and the unit lane installs jq so 'skip' never quietly becomes 'nowhere'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

.forgejo/workflows/ci.yml

@@ -225,6 +225,16 @@ jobs:
           UV_PROJECT_ENVIRONMENT: /opt/venv
         run: uv sync --locked --extra dev
 
+      # The hook-EXECUTION tests (test_write_path_trigger's nudge pair) run the
+      # real bash hook, which exits silently without jq — and those tests skip
+      # rather than fail when it's absent, so without this step they would
+      # quietly never be verified anywhere (ci-python ships without jq; same
+      # install the Plugin hooks job does).
+      - name: Install jq for hook execution tests
+        run: |
+          apt-get update -qq
+          apt-get install -y -qq --no-install-recommends jq
+
       - name: Run tests
         # Integration tests (real Postgres) run in the `integration` job below.
         run: /opt/venv/bin/python -m pytest tests/ -q -m "not integration"

src/scribe/services/note_usage.py

@@ -186,6 +186,18 @@ async def usage_for_notes(note_ids: list[int]) -> dict[int, dict]:
     if not ids:
         return out
 
+    # Classified in SQL so the group count stays small: per note we get at most
+    # (surfaced-ranked, surfaced-ambient, pulled) rather than one row per
+    # distinct source. ONE labelled expression, grouped by its label — a second
+    # case() instance in GROUP BY renders with its own expanding-IN bind names
+    # under asyncpg, so the database sees two DIFFERENT expressions and rejects
+    # the query with a GroupingError. That rejection was swallowed, which is
+    # how every counter read zero in production while the writes were landing
+    # fine (#2663).
+    ambient = case(
+        (NoteUsageEvent.source.in_(AMBIENT_SOURCES), True),
+        else_=False,
+    ).label("ambient")
     try:
         async with async_session() as session:
             rows = (
@@ -195,22 +207,13 @@ async def usage_for_notes(note_ids: list[int]) -> dict[int, dict]:
                         NoteUsageEvent.event,
                         func.count().label("n"),
                         func.max(NoteUsageEvent.created_at).label("last_at"),
-                        # Classified in SQL so the group count stays small: per
-                        # note we get at most (surfaced-ranked, surfaced-ambient,
-                        # pulled) rather than one row per distinct source.
-                        case(
-                            (NoteUsageEvent.source.in_(AMBIENT_SOURCES), True),
-                            else_=False,
-                        ).label("ambient"),
+                        ambient,
                     )
                     .where(NoteUsageEvent.note_id.in_(ids))
                     .group_by(
                         NoteUsageEvent.note_id,
                         NoteUsageEvent.event,
-                        case(
-                            (NoteUsageEvent.source.in_(AMBIENT_SOURCES), True),
-                            else_=False,
-                        ),
+                        ambient,
                     )
                 )
             ).all()

tests/test_write_path_trigger.py

@@ -791,18 +791,39 @@ def test_plugin_version_bumped_with_the_hook():
     assert version >= (0, 1, 18)
 
 
+def _hook_runtime_env():
+    """Env for tests that need the hook's tools to actually RUN.
+
+    The silence-contract tests above deliberately restrict PATH — the hook must
+    exit quietly when its tools are missing, and asserting on empty output
+    doesn't care why it was empty. These tests assert on CONTENT, so the tools
+    must resolve wherever the image installed them; skip (don't fail) on an
+    image that lacks them, because that image cannot exercise this behaviour
+    at all.
+    """
+    import os
+    import shutil
+
+    for tool in ("git", "jq", "curl", "bash"):
+        if shutil.which(tool) is None:
+            pytest.skip(f"hook runtime tool {tool!r} not installed")
+    return {"PATH": os.environ["PATH"],
+            "SCRIBE_URL": "http://127.0.0.1:9", "SCRIBE_TOKEN": "t"}
+
+
 def test_hook_nudges_recording_when_copies_exist_but_nothing_is_recorded(tmp_path):
     """#2664: the local arm proves duplication; when Scribe has no record of it,
     the same context block must ask for create_snippet — the one moment the
     recording nudge is earned rather than noise. An unreachable server counts
     as "nothing recorded": the local finding needed no server, and the nudge
     fails open with it (here: a refused connection stands in for the instance)."""
+    env = _hook_runtime_env()
     repo = tmp_path / "repo"
     repo.mkdir()
-    subprocess.run(["git", "init", "-q"], cwd=repo, check=True)
+    subprocess.run(["git", "init", "-q"], cwd=repo, check=True, env=env)
     (repo / "a.py").write_text("def debounce(fn):\n    return fn\n")
     # git grep searches the index, so the existing copy must be staged.
-    subprocess.run(["git", "add", "."], cwd=repo, check=True)
+    subprocess.run(["git", "add", "."], cwd=repo, check=True, env=env)
     out = subprocess.run(
         ["bash", str(HOOK)],
         input=json.dumps({
@@ -810,11 +831,13 @@ def test_hook_nudges_recording_when_copies_exist_but_nothing_is_recorded(tmp_pat
             "tool_input": {"file_path": str(repo / "b.py"),
                            "content": "def debounce(fn):\n    return fn\n"},
         }),
-        capture_output=True, text=True,
-        env={"PATH": "/usr/bin:/bin", "SCRIBE_URL": "http://127.0.0.1:9",
-             "SCRIBE_TOKEN": "t"},
+        capture_output=True, text=True, env=env,
     )
     assert out.returncode == 0
+    assert out.stdout.strip(), (
+        "hook produced no output — the local arm should have found the "
+        "staged duplicate and nudged"
+    )
     ctx = json.loads(out.stdout)["hookSpecificOutput"]["additionalContext"]
     assert "already defined" in ctx          # the duplication finding
     assert "create_snippet" in ctx           # the recording ask riding it
@@ -823,9 +846,10 @@ def test_hook_nudges_recording_when_copies_exist_but_nothing_is_recorded(tmp_pat
 def test_hook_stays_quiet_about_recording_when_nothing_is_duplicated(tmp_path):
     """A brand-new helper with no other copies earns no nudge — a reflex that
     fires on every Write is one sessions learn to skip."""
+    env = _hook_runtime_env()
     repo = tmp_path / "repo"
     repo.mkdir()
-    subprocess.run(["git", "init", "-q"], cwd=repo, check=True)
+    subprocess.run(["git", "init", "-q"], cwd=repo, check=True, env=env)
     out = subprocess.run(
         ["bash", str(HOOK)],
         input=json.dumps({
@@ -833,9 +857,7 @@ def test_hook_stays_quiet_about_recording_when_nothing_is_duplicated(tmp_path):
             "tool_input": {"file_path": str(repo / "b.py"),
                            "content": "def debounce(fn):\n    return fn\n"},
         }),
-        capture_output=True, text=True,
-        env={"PATH": "/usr/bin:/bin", "SCRIBE_URL": "http://127.0.0.1:9",
-             "SCRIBE_TOKEN": "t"},
+        capture_output=True, text=True, env=env,
     )
     assert out.returncode == 0
     assert "create_snippet" not in out.stdout