diff --git a/src/scribe/mcp/tools/projects.py b/src/scribe/mcp/tools/projects.py index 5e4ee07..c8bfddc 100644 --- a/src/scribe/mcp/tools/projects.py +++ b/src/scribe/mcp/tools/projects.py @@ -24,6 +24,7 @@ from scribe.services import projects as projects_svc from scribe.services import rulebooks as rulebooks_svc from scribe.services import systems as systems_svc from scribe.services import trash as trash_svc +from scribe.services.note_usage import record_surfaced async def list_projects() -> dict: @@ -94,6 +95,18 @@ async def enter_project(project_id: int) -> dict: # agent when it writes — which it never was, and tagging stopped within # three days of the feature landing (#2546's audit). systems = await systems_svc.list_systems(uid, project_id) + + # Probably the largest surfacing by volume, and it emitted nothing — so + # the pulls it caused floated unattributed and the surfaced:pulled ratio + # ran against a denominator missing its biggest contributor (#2477). An + # AMBIENT source: these are top-N-by-recency, not a ranked choice, and the + # readout counts them apart so dead-weight detection isn't poisoned by + # "recently updated in a project you opened". + record_surfaced( + user_id=uid, + note_ids=[int(t.id) for t in open_tasks] + [int(n.id) for n in recent_notes], + source="enter_project", + ) # A project need not have one, and most installs won't — null is ordinary # here, not a missing prerequisite. design_system = None diff --git a/src/scribe/services/note_usage.py b/src/scribe/services/note_usage.py index a268020..041b42a 100644 --- a/src/scribe/services/note_usage.py +++ b/src/scribe/services/note_usage.py @@ -26,7 +26,7 @@ from __future__ import annotations import asyncio import logging -from sqlalchemy import func, select +from sqlalchemy import case, func, select from scribe.models import async_session from scribe.models.note_usage import PULLED, SURFACED, NoteUsageEvent @@ -96,14 +96,30 @@ def record_pulled(*, user_id: int | None, note_id: int, source: str) -> None: _schedule(rows) +# Surfacings that are NOT a ranked choice. enter_project returns whatever the +# top-N-by-recency happen to be; the skill sync installs every Process the +# operator can reach. Counting those alongside auto-inject would make a note's +# surfaced_count dominated by "it was recently updated in a project you +# opened", and dead-weight detection would read that as popularity (#2477). +# They still matter — a pull that follows one must not float unattributed — so +# they land in their own bucket rather than not landing at all. +AMBIENT_SOURCES = ("enter_project", "process_skill_sync") + + def empty_usage() -> dict: """The zero readout — what a note with no recorded events looks like. Callers render this shape unconditionally, so a note predating the table reads as "never surfaced, never pulled" rather than as a missing key. + + `surfaced_count` is RANKED surfacings only — a scored surface chose this + record. `ambient_count` is the rest (see AMBIENT_SOURCES). The split is the + readout half of #2477: the "high surfaced, zero pulls → dead weight" + reading is only valid over surfacings that were choices. """ return { "surfaced_count": 0, + "ambient_count": 0, "pull_count": 0, "last_surfaced_at": None, "last_pulled_at": None, @@ -132,9 +148,23 @@ 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"), ) .where(NoteUsageEvent.note_id.in_(ids)) - .group_by(NoteUsageEvent.note_id, NoteUsageEvent.event) + .group_by( + NoteUsageEvent.note_id, + NoteUsageEvent.event, + case( + (NoteUsageEvent.source.in_(AMBIENT_SOURCES), True), + else_=False, + ), + ) ) ).all() except Exception: @@ -142,14 +172,21 @@ async def usage_for_notes(note_ids: list[int]) -> dict[int, dict]: logger.debug("note usage readout failed", exc_info=True) return out - for note_id, event, n, last_at in rows: + for note_id, event, n, last_at, ambient in rows: slot = out.get(int(note_id)) if slot is None: continue - if event == SURFACED: + if event == SURFACED and ambient: + slot["ambient_count"] = int(n) + elif event == SURFACED: slot["surfaced_count"] = int(n) slot["last_surfaced_at"] = last_at.isoformat() if last_at else None elif event == PULLED: - slot["pull_count"] = int(n) - slot["last_pulled_at"] = last_at.isoformat() if last_at else None + # Pulls are pulls regardless of what surfaced the record — the + # question a pull answers ("did anyone ever open this?") doesn't + # depend on how it was found. + slot["pull_count"] = slot["pull_count"] + int(n) + latest = last_at.isoformat() if last_at else None + if latest and (slot["last_pulled_at"] or "") < latest: + slot["last_pulled_at"] = latest return out diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 4e5ce51..64b08e4 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -243,6 +243,19 @@ async def build_process_manifest(user_id: int) -> dict: entry["shared"] = True entry["owner"] = it.get("owner") procs.append(entry) + + # The most consequential passive surface Scribe has (see SCOPE above), and + # it emitted nothing — a Process installed as a skill, matched on every + # relevant turn and never once opened, was indistinguishable from one never + # installed (#2477). The honest event is "installed on the operator's + # machine", which is a surfacing in effect: the skill description is in + # front of the model each session. AMBIENT source — installation is not a + # ranked choice — so it lands in ambient_count, not surfaced_count. + record_surfaced( + user_id=user_id, + note_ids=[int(p["id"]) for p in procs], + source="process_skill_sync", + ) return {"processes": procs, "total": len(procs)} diff --git a/tests/test_note_usage.py b/tests/test_note_usage.py index d223e6d..11c6e62 100644 --- a/tests/test_note_usage.py +++ b/tests/test_note_usage.py @@ -113,7 +113,13 @@ async def test_usage_for_notes_splits_counts_by_event(): from datetime import datetime, timezone ts = datetime(2026, 7, 28, tzinfo=timezone.utc) - rows = [(3, "surfaced", 9, ts), (3, "pulled", 2, ts)] + # Rows are (note_id, event, count, last_at, ambient) since #2477 split the + # readout. Ranked and ambient surfacings arrive as separate groups. + rows = [ + (3, "surfaced", 9, ts, False), + (3, "surfaced", 40, ts, True), + (3, "pulled", 2, ts, False), + ] session = MagicMock() session.execute = AsyncMock( return_value=MagicMock(all=MagicMock(return_value=rows)) @@ -123,7 +129,11 @@ async def test_usage_for_notes_splits_counts_by_event(): ctx.__aexit__ = AsyncMock(return_value=False) with patch.object(note_usage, "async_session", return_value=ctx): out = await usage_for_notes([3]) + # The dead-weight reading ("surfaced often, never pulled") is only valid + # over surfacings that were CHOICES. 40 enter_project appearances must not + # make a record look popular — they sit in ambient_count (#2477). assert out[3]["surfaced_count"] == 9 + assert out[3]["ambient_count"] == 40 assert out[3]["pull_count"] == 2 assert out[3]["last_pulled_at"] == ts.isoformat()