feat(telemetry): ambient surfacings count, apart — enter_project and the skill sync emit
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / Python tests (push) Successful in 47s
CI & Build / Build & push image (push) Successful in 25s
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / Python tests (push) Successful in 47s
CI & Build / Build & push image (push) Successful in 25s
#2477, option (a) as decided, with the readout changed in the same commit. ## The two silent surfaces enter_project returns open tasks + recent notes on every project entry — probably the largest surfacing by volume — and emitted nothing, so the pulls it caused floated unattributed and the surfaced:pulled ratio ran against a denominator missing its biggest contributor. Now source "enter_project". build_process_manifest installs every reachable Process as an auto-surfacing skill on the operator's machine — its own docstring calls it the most consequential passive surface Scribe has — and emitted nothing, so a Process matched on every relevant turn and never opened was indistinguishable from one never installed. Now source "process_skill_sync": the honest event is "installed", which is a surfacing in effect since the description sits in front of the model each session. ## The readout, same commit — the condition option (a) carried Both surfaces are AMBIENT: top-N-by-recency and install-everything are not ranked choices. Pooling them into surfaced_count would make a note's number dominated by "recently updated in a project you opened", and dead-weight detection would read that as popularity — the wrong number read confidently, which is the corrupts-data tier the survey ranked above everything else. So usage_for_notes splits: surfaced_count stays RANKED-ONLY (every existing consumer's reading — "surfaced often, never pulled → dead weight" — keeps meaning what it meant), and ambient_count is new. Classified in SQL via a CASE on AMBIENT_SOURCES so the group count stays three rows per note, not one per distinct source. Pulls stay pooled: "did anyone ever open this?" does not depend on how it was found. #1038 and #2085 read agent pulls and ranked surfacings; both are unaffected by ambient volume, which is the point. Refs #2477
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)}
|
||||
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user