fix(telemetry): a surface that stopped recording is not one that never ran (#3720)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / integration (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 24s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / integration (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 24s
`out["sources"]` was built only from the windowed aggregate, so a source with rows in `retrieval_logs` but none inside the window got no bucket at all. Absent is exactly how a source that never existed renders, so a surface that WAS recording and went silent became unreadable — #2663 one level up, the failure that looks like the correct answer. Two queries at different scopes, and only one shaped the output. `_complete_from` reads all-time and knows every source the table has ever held; the windowed loop dropped whatever it did not return. Every such source now gets a zero bucket. Zero is a real measurement here rather than a manufactured one: the all-time query proves the source was recording, and it made no calls across a window it fully covers. No `covers_window` special case is needed either — a source whose first row fell after `since` would have that row IN the window and already hold a bucket, so anything reaching this branch began before it. The counts are 0 and everything else is null. A sampled distribution is not the same claim as a call count, and rendering p50 as 0.0 for a source nobody sampled would assert a measurement — #3311's mistake, in the readout built to prevent it. Found while fixing #3712's fixture, which failed with KeyError for this exact reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ
This commit is contained in:
@@ -740,6 +740,62 @@ async def test_coverage_is_per_source_because_the_table_is_older_than_its_arms(
|
||||
await s.commit()
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_surface_that_went_silent_is_not_the_same_as_one_that_never_ran(
|
||||
_dispose_engine,
|
||||
):
|
||||
"""#3720 — absent is how "never existed" renders, so it cannot also be how
|
||||
"stopped recording" renders.
|
||||
|
||||
A surface losing its recorder is one of the failures this milestone exists
|
||||
to make visible, and dropping it from the readout is the most complete way
|
||||
to hide it. Zero here is a real measurement: the table proves the source
|
||||
was recording, and it made no calls across a window it fully covers.
|
||||
"""
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from sqlalchemy import delete
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.retrieval_log import RetrievalLog
|
||||
from scribe.services.retrieval_telemetry import retrieval_summary
|
||||
|
||||
UID = 990078
|
||||
now = datetime.now(timezone.utc)
|
||||
async with async_session() as s:
|
||||
# Recorded once, well before the window, and never since.
|
||||
s.add(RetrievalLog(
|
||||
user_id=UID, source="auto_inject", result_count=3, top_score=0.81,
|
||||
created_at=now - timedelta(days=60),
|
||||
))
|
||||
await s.commit()
|
||||
|
||||
try:
|
||||
out = await retrieval_summary(UID, days=7)
|
||||
|
||||
assert "auto_inject" in out["sources"], (
|
||||
"a source with rows in the table but none in the window was "
|
||||
"dropped from the readout — a surface that stopped recording now "
|
||||
"reads exactly like one that never existed"
|
||||
)
|
||||
quiet = out["sources"]["auto_inject"]
|
||||
assert quiet["calls"] == 0
|
||||
# The window IS covered; what was observed across it is nothing.
|
||||
assert quiet["covers_window"] is True
|
||||
# ...but nothing was sampled, so no distribution may be claimed. A
|
||||
# zeroed score would assert a measurement, which is #3311's mistake.
|
||||
assert quiet["top_score"] == {
|
||||
"p10": None, "p50": None, "p90": None, "min": None, "max": None,
|
||||
}
|
||||
assert quiet["suppression"] is None
|
||||
assert quiet["avg_result_count"] is None
|
||||
finally:
|
||||
async with async_session() as s:
|
||||
await s.execute(delete(RetrievalLog).where(RetrievalLog.user_id == UID))
|
||||
await s.commit()
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_section_is_complete_only_from_its_latest_contributor(
|
||||
|
||||
Reference in New Issue
Block a user