docs(6209): rule usage, write path and telemetry tests state the behaviour they pin
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 57s
CI & Build / integration (push) Successful in 1m18s
CI & Build / Python tests (push) Successful in 2m2s
CI & Build / Build & push image (push) Successful in 17s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 57s
CI & Build / integration (push) Successful in 1m18s
CI & Build / Python tests (push) Successful in 2m2s
CI & Build / Build & push image (push) Successful in 17s
Docstrings, comments and assertion messages in the three most-cited test files now say which property each test holds and why, without the issue, milestone or rule numbers, dates and one-install measurements that recorded how each property was discovered. Fixture ids and calibration scores stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -57,7 +57,7 @@ def test_build_payload_rounds_scores_to_5dp():
|
||||
assert p["result_ids"][0]["score"] == 0.12346
|
||||
|
||||
|
||||
# ─── suppression: "not measured" is not "none" (#3497) ───────────────────────
|
||||
# ─── suppression: "not measured" is not "none" ──────────────────────────────
|
||||
|
||||
|
||||
def test_a_caller_that_cannot_measure_suppression_stores_null():
|
||||
@@ -65,7 +65,7 @@ def test_a_caller_that_cannot_measure_suppression_stores_null():
|
||||
|
||||
A surface that passes its exclusions into the search never sees what was
|
||||
dropped. Storing 0 would assert a clean run nobody observed — reading an
|
||||
artifact as a measurement, which is exactly #3311's mistake.
|
||||
artifact as a measurement.
|
||||
"""
|
||||
p = _build_payload(
|
||||
user_id=1, source="auto_inject", query="q", threshold=0.6,
|
||||
@@ -167,12 +167,11 @@ async def test_insert_retrieval_log_roundtrip(_dispose_engine):
|
||||
|
||||
|
||||
# ─── the read half: retrieval_summary (integration) ──────────────────────────
|
||||
# Integration, not mocked, and deliberately so. #2663 is the bug where a
|
||||
# GROUP BY the database rejected was swallowed by a broad except, so every
|
||||
# counter read zero in production while the writes landed fine and the mocked
|
||||
# tests passed. `retrieval_summary` runs a grouped aggregate with
|
||||
# percentile_cont ... WITHIN GROUP and a two-label CASE — precisely the shape
|
||||
# that failed then. Only a real Postgres can say it parses.
|
||||
# Integration, not mocked, and deliberately so. A GROUP BY the database
|
||||
# rejects, swallowed by a broad except, reads zero on every counter while the
|
||||
# writes land fine and mocked tests pass. `retrieval_summary` runs a grouped
|
||||
# aggregate with percentile_cont ... WITHIN GROUP and a two-label CASE —
|
||||
# precisely that shape. Only a real Postgres can say it parses.
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@@ -226,7 +225,7 @@ async def test_retrieval_summary_reads_what_the_writer_wrote(_dispose_engine):
|
||||
assert ai["zero_result_calls"] == 1
|
||||
assert "cleared_threshold" not in ai, (
|
||||
"the tautology is back: it was true exactly when result_count > 0, "
|
||||
"so it reported nothing zero_result_calls did not (#3670)"
|
||||
"so it reported nothing zero_result_calls did not"
|
||||
)
|
||||
# p50 over the three scored calls; the empty one contributes no score.
|
||||
assert ai["top_score"]["p50"] == pytest.approx(0.72, abs=1e-4)
|
||||
@@ -251,8 +250,8 @@ async def test_retrieval_summary_reads_what_the_writer_wrote(_dispose_engine):
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.asyncio
|
||||
async def test_retrieval_summary_is_empty_not_broken_for_a_fresh_install(_dispose_engine):
|
||||
"""Rule #115: an install with no telemetry gets a coherent zero readout,
|
||||
and `read_failed` stays False — the distinction #2663 says must exist."""
|
||||
"""An install with no telemetry gets a coherent zero readout, and
|
||||
`read_failed` stays False — "no rows" is not "the read broke"."""
|
||||
from scribe.services.retrieval_telemetry import retrieval_summary
|
||||
|
||||
out = await retrieval_summary(990003, days=30)
|
||||
@@ -293,11 +292,10 @@ async def test_retrieval_summary_sees_only_its_own_users_telemetry(_dispose_engi
|
||||
await s.commit()
|
||||
|
||||
|
||||
# ─── per-source pull-through (#3311) ─────────────────────────────────────────
|
||||
# ─── per-source pull-through ─────────────────────────────────────────────────
|
||||
# Integration for the same reason the block above is: this is a self-join with
|
||||
# two DISTINCT subqueries and a LIKE escape, which is a new SQL shape in a
|
||||
# module whose one production outage (#2663) was a new SQL shape the database
|
||||
# rejected inside a broad except. A mock would pass on a query Postgres refuses.
|
||||
# two DISTINCT subqueries and a LIKE escape, in a module whose read sits
|
||||
# inside a broad except: a new SQL shape the database rejects reads as zeros. A mock would pass on a query Postgres refuses.
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@@ -426,11 +424,9 @@ async def test_the_agent_pull_filter_does_not_treat_its_underscore_as_a_wildcard
|
||||
await s.commit()
|
||||
|
||||
|
||||
# ─── rule usage (milestone 333 step 3) ───────────────────────────────────────
|
||||
# ─── rule usage ──────────────────────────────────────────────────────────────
|
||||
# Integration, for the same reason the block above is: these are real GROUP BYs
|
||||
# and count(distinct) against a table that did not exist a commit ago, in a
|
||||
# module whose one production outage (#2663) was a SQL shape the database
|
||||
# rejected inside a broad except. A mock would agree with whatever the code
|
||||
# and count(distinct), in a module whose read sits inside a broad except. A mock would agree with whatever the code
|
||||
# does, including nothing.
|
||||
|
||||
|
||||
@@ -464,7 +460,7 @@ async def test_rule_usage_is_a_coherent_zero_on_a_fresh_install(_dispose_engine)
|
||||
"""Every rule in an existing install predates this table, so "no events" is
|
||||
the normal state for a while. It must read as zero, not as a missing key
|
||||
and not as a failure — the same "no rows" / "read broke" distinction the
|
||||
rest of this readout keeps (#2663).
|
||||
rest of this readout keeps.
|
||||
|
||||
`pull_through` is None rather than 0.0, matching the note block: a ratio of
|
||||
zero asserts "rules were shown and none opened", which with an empty
|
||||
@@ -581,7 +577,7 @@ async def test_rule_usage_sees_only_its_own_users_events(_dispose_engine):
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_preload_lands_in_ambient_and_never_in_the_ratio(_dispose_engine):
|
||||
"""The split that makes the always-on set judgeable (#3473).
|
||||
"""The split that makes the always-on set judgeable.
|
||||
|
||||
Pull-through asks "was that hint any use", and only a surface that CHOSE
|
||||
what it showed can be judged by it. If the preload counted toward the
|
||||
@@ -647,10 +643,10 @@ async def test_ambient_alone_reports_no_ratio(_dispose_engine):
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_outcome_counts_come_back_split(_dispose_engine):
|
||||
"""A pull says the rule was read; an outcome says it changed something
|
||||
(#4213). Integration rather than a mock because `distinct_rules_acted` is
|
||||
a count(distinct) with an IN over a column that carries no CHECK — the
|
||||
kind of SQL shape #2663 was, where a mock agrees with whatever the code
|
||||
does including nothing.
|
||||
Integration rather than a mock because `distinct_rules_acted` is a
|
||||
count(distinct) with an IN over a column that carries no CHECK — a SQL
|
||||
shape where a mock agrees with whatever the code does, including
|
||||
nothing.
|
||||
|
||||
`applied` and `departed` stay APART. Summed they would say "an outcome was
|
||||
recorded", which is true of both and useful about neither: a departure is
|
||||
@@ -681,7 +677,7 @@ async def test_the_outcome_counts_come_back_split(_dispose_engine):
|
||||
|
||||
# An outcome is NOT a pull. If `applied` leaked into the pull counters
|
||||
# the silently-unchanged rule would vanish into a compliant-looking
|
||||
# total, which is the exact confusion #4212 was opened to end.
|
||||
# total — the confusion keeping them apart exists to prevent.
|
||||
assert ru["pulled"] == 3
|
||||
finally:
|
||||
await cleanup()
|
||||
@@ -709,19 +705,18 @@ async def test_an_outcome_is_never_split_by_ambient(_dispose_engine):
|
||||
await cleanup()
|
||||
|
||||
|
||||
# ── Window coverage (#3712) ────────────────────────────────────────────
|
||||
# ── Window coverage ────────────────────────────────────────────────────
|
||||
#
|
||||
# A counter added last week, read over a 30-day window, reports a real count
|
||||
# against an imagined denominator. The result is a plausible FRACTION rather
|
||||
# than an obvious zero, which is what makes it dangerous — #379 spent five
|
||||
# planned steps on a defect that turned out to be a window opening before the
|
||||
# recording it was measuring existed.
|
||||
# than an obvious zero, which is what makes it dangerous: a window opening
|
||||
# before the recording it measures existed looks like a defect.
|
||||
|
||||
|
||||
def test_coverage_says_nothing_rather_than_false_when_nothing_was_recorded():
|
||||
"""Null, never False. "No measurement" is not "partial measurement".
|
||||
|
||||
The same distinction `suppression`'s null carries (#3497): absent must not
|
||||
The same distinction `suppression`'s null carries: absent must not
|
||||
read as a verdict. A False here would assert the window is under-covered,
|
||||
which is a claim nobody is in a position to make.
|
||||
"""
|
||||
@@ -814,11 +809,11 @@ async def test_coverage_is_per_source_because_the_table_is_older_than_its_arms(
|
||||
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
|
||||
"""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
|
||||
A surface losing its recorder is one of the failures coverage 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.
|
||||
"""
|
||||
@@ -853,7 +848,7 @@ async def test_a_surface_that_went_silent_is_not_the_same_as_one_that_never_ran(
|
||||
# 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.
|
||||
# zeroed score would assert a measurement nobody took.
|
||||
assert quiet["top_score"] == {
|
||||
"p10": None, "p50": None, "p90": None, "min": None, "max": None,
|
||||
}
|
||||
@@ -918,16 +913,14 @@ async def test_a_section_is_complete_only_from_its_latest_contributor(
|
||||
await s.commit()
|
||||
|
||||
|
||||
# ─── the bar can only be judged from what it rejected (#3670) ────────────────
|
||||
# ─── the bar can only be judged from what it rejected ────────────────────────
|
||||
#
|
||||
# `cleared_threshold` was the number the docstring told a reader to look at
|
||||
# first. It was `calls - zero_result_calls` under another name: the search
|
||||
# applies the bar before returning, so every returned result cleared it by
|
||||
# construction and a call with nothing has no score to compare.
|
||||
# `zero_result_calls + cleared_threshold == calls` held on all nineteen
|
||||
# source/window readings ever taken — no near-misses, no exceptions.
|
||||
# A count of calls that cleared the threshold is `calls - zero_result_calls`
|
||||
# under another name: the search applies the bar before returning, so every
|
||||
# returned result cleared it by construction and a call with nothing has no
|
||||
# score to compare.
|
||||
#
|
||||
# What replaced it cannot go the same way, and the reason is structural rather
|
||||
# `near_misses` cannot go the same way, and the reason is structural rather
|
||||
# than careful naming: `near_misses` is measured on the calls the bar TURNED
|
||||
# AWAY, using a score the bar never saw. No arrangement of `calls`,
|
||||
# `zero_result_calls` and `result_count` derives it.
|
||||
@@ -957,7 +950,7 @@ def test_a_call_that_returned_nothing_still_records_what_it_nearly_showed():
|
||||
def test_a_caller_that_did_not_measure_the_near_miss_stores_null():
|
||||
"""Null, never 0.0. A zero here reads as "the corpus held nothing remotely
|
||||
relevant" — a claim about the corpus invented out of a caller's silence,
|
||||
which is #3311's substitution in a new field."""
|
||||
which substitutes an artifact for a measurement."""
|
||||
p = _build_payload(
|
||||
user_id=1, source="auto_inject", query="q", threshold=0.6,
|
||||
limit=3, project_id=None, is_task=None, results=[], duration_ms=None,
|
||||
@@ -988,13 +981,12 @@ def test_the_readout_reports_unmeasured_near_misses_as_none():
|
||||
|
||||
|
||||
def test_the_readout_carries_no_field_derivable_from_its_neighbours():
|
||||
"""The guard that would have caught #3670 on the day it shipped.
|
||||
"""No readout field is a sum of the two beside it.
|
||||
|
||||
`cleared_threshold` survived because it had its own name and its own
|
||||
docstring paragraph, and nobody added the two numbers beside it. This
|
||||
asserts the identity that held on every reading ever taken — and if a
|
||||
future field reintroduces it under a new name, the sum below is where it
|
||||
shows up.
|
||||
A derived field survives review when it has its own name and its own
|
||||
docstring paragraph, and nobody adds the two numbers beside it. If a
|
||||
field reintroduces `calls - zero_result_calls` under a new name, the sum
|
||||
below is where it shows up.
|
||||
"""
|
||||
from scribe.services.retrieval_telemetry import _bucket
|
||||
|
||||
@@ -1008,7 +1000,7 @@ def test_the_readout_carries_no_field_derivable_from_its_neighbours():
|
||||
}
|
||||
assert not derivable, (
|
||||
f"{sorted(derivable)} equals calls - zero_result_calls on this row. "
|
||||
f"That is how `cleared_threshold` read for its whole life (#3670): a "
|
||||
f"That is a "
|
||||
f"figure presented as an independent measurement that a reader can "
|
||||
f"compute from the two numbers next to it. Either it is a tautology, "
|
||||
f"or this fixture happens to make it look like one — check which "
|
||||
@@ -1023,10 +1015,9 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
|
||||
|
||||
`near_misses` is a `percentile_cont(...) WITHIN GROUP` over a CASE
|
||||
expression, inside the same grouped aggregate that already carries four
|
||||
other CASEs. That is within one step of the shape that produced #2663 — a
|
||||
query the database rejected, swallowed by this module's broad `except`, so
|
||||
every counter read zero in production while the writes landed fine and the
|
||||
mocked tests passed. Only a real Postgres can say this parses, and if it
|
||||
other CASEs. A query the database rejects is swallowed by this module's
|
||||
broad `except`, so every counter reads zero while the writes land fine and
|
||||
mocked tests pass. Only a real Postgres can say this parses, and if it
|
||||
does not, the symptom is silence rather than an error.
|
||||
|
||||
The numbers are chosen so a bar at 0.72 is visibly the wrong bar: three
|
||||
@@ -1055,12 +1046,12 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
|
||||
limit=1, project_id=None, is_task=None,
|
||||
results=[(0.88, _note(7))], duration_ms=4.0, best_available=0.88,
|
||||
))
|
||||
# An unmeasured decline, standing in for every row written before #3670.
|
||||
# An unmeasured decline, standing in for a row written before the column existed.
|
||||
await _insert_retrieval_log(_build_payload(
|
||||
user_id=UID, source="pre_tool_rule", query="ls", threshold=0.72,
|
||||
limit=1, project_id=None, is_task=None, results=[], duration_ms=4.0,
|
||||
))
|
||||
# THE CASE WHOSE ABSENCE LET THIS GUARD PASS OVER BROKEN CODE (#3739).
|
||||
# THE CASE WITHOUT WHICH THIS GUARD PASSES OVER BROKEN CODE.
|
||||
# A zero-result call whose zero was a REPEAT, not a rejection: the ranker
|
||||
# cleared the bar at 0.9 and the session had already been shown that rule,
|
||||
# so the arm dropped it in Python after the search. Without the suppression
|
||||
@@ -1076,7 +1067,7 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
|
||||
out = await retrieval_summary(UID, days=30)
|
||||
assert out["read_failed"] is False, (
|
||||
"the aggregate did not execute — a rejected query here reads as "
|
||||
"zeros everywhere, which is #2663 exactly"
|
||||
"zeros everywhere"
|
||||
)
|
||||
src = out["sources"]["pre_tool_rule"]
|
||||
assert src["calls"] == 6
|
||||
@@ -1089,7 +1080,7 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
|
||||
"Three qualify. Excluded: the unmeasured row (predates the column, "
|
||||
"not a scoreless decline), the call that showed something (its "
|
||||
"best-available is just its top score), and the REPEAT — a zero "
|
||||
"the reader caused, not the bar (#3739)"
|
||||
"the reader caused, not the bar"
|
||||
)
|
||||
assert nm["max"] == pytest.approx(0.7189, abs=1e-4), (
|
||||
"the closest thing the bar turned away — 0.7189 against a 0.72 "
|
||||
@@ -1099,7 +1090,7 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
|
||||
"a rejection that outscores the bar is not a rejection. This is "
|
||||
"structural once the suppression arm is in the predicate: an "
|
||||
"above-bar candidate that was not excluded would have been "
|
||||
"RETURNED, so its call cannot be in this population (#3739)"
|
||||
"RETURNED, so its call cannot be in this population"
|
||||
)
|
||||
assert 0.70 <= nm["p50"] <= 0.7189
|
||||
finally:
|
||||
@@ -1108,17 +1099,16 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
|
||||
await s.commit()
|
||||
|
||||
|
||||
# ─── a search that never ran is not a decline (#3765) ────────────────────────
|
||||
# ─── a search that never ran is not a decline ────────────────────────────────
|
||||
#
|
||||
# `best_available_score` was added by #3670 so a bar could be judged from what
|
||||
# it rejected, and it arrived null on four unrelated causes: the corpus offered
|
||||
# nothing, the query was empty, the embedder was down, or the DATABASE QUERY
|
||||
# FAILED. Only the first is a measurement. The fourth is the #2663 shape — a
|
||||
# swallowed failure rendering as a clean zero — inside the field added to fix
|
||||
# an instance of the #2663 shape.
|
||||
# `best_available_score` lets a bar be judged from what it rejected, and it
|
||||
# could be null on four unrelated causes: the corpus offered nothing, the
|
||||
# query was empty, the embedder was down, or the DATABASE QUERY FAILED. Only
|
||||
# the first is a measurement. The fourth is a swallowed failure rendering as a
|
||||
# clean zero.
|
||||
#
|
||||
# The fix is not a new column. A call that never searched writes no row, so
|
||||
# every remaining null means one thing. That is the convention the pre-tool arm
|
||||
# The answer is not a new column. A call that never searched writes no row, so
|
||||
# every null means one thing. That is the convention the pre-tool arm
|
||||
# already follows for a blank command, extended from the case a caller can see
|
||||
# in advance to the ones only the search knows about.
|
||||
|
||||
@@ -1143,7 +1133,7 @@ def test_a_search_that_never_ran_writes_no_row():
|
||||
assert not build.called, (
|
||||
"a search that never ran was recorded as a retrieval. It would "
|
||||
"read as a ranker decline — evidence about a threshold, from a "
|
||||
"call where no threshold was ever applied (#3765)"
|
||||
"call where no threshold was ever applied"
|
||||
)
|
||||
|
||||
|
||||
@@ -1151,7 +1141,7 @@ def test_a_search_that_ran_and_found_nothing_still_writes_its_row():
|
||||
"""The half that stops the fix from being 'log less'.
|
||||
|
||||
A call that searched and came back empty is the ONLY evidence a threshold
|
||||
is too high (#3497). Dropping it too would trade one silent distortion for
|
||||
is too high. Dropping it too would trade one silent distortion for
|
||||
another, and this assertion is what makes the pair discriminate: a blanket
|
||||
`return` passes the test above and fails this one.
|
||||
"""
|
||||
@@ -1188,19 +1178,17 @@ def test_a_caller_that_never_asked_is_assumed_to_have_searched():
|
||||
assert build.called
|
||||
|
||||
|
||||
# ─── the bar's refusals have names now (#3807) ───────────────────────────────
|
||||
# ─── the bar's refusals have names ───────────────────────────────────────────
|
||||
#
|
||||
# #3670 recorded how CLOSE the bar came to firing. That is the half a decision
|
||||
# does not need: live, pre_tool_rule sits at a ~0.72 bar with a near-miss p90 of
|
||||
# 0.7071, so dropping to 0.707 would take it from 22 hits a day to roughly 139.
|
||||
# The percentile says the mass is there and says nothing about whether it is
|
||||
# worth showing, and pull-through cannot referee it because the injected rule
|
||||
# The near-miss score records how CLOSE the bar came to firing. That is the
|
||||
# half a decision does not need: the percentile says the mass is there and
|
||||
# says nothing about whether it is worth showing, and pull-through cannot referee it because the injected rule
|
||||
# line already carries title and trigger — a session can comply without ever
|
||||
# calling get_rule.
|
||||
#
|
||||
# Reading the rejected records is the method that answers it, and until now
|
||||
# `result_ids` held only what was RETURNED, so on a zero-result call the
|
||||
# near-missed record had no name.
|
||||
# Reading the rejected records is the method that answers it, and
|
||||
# `result_ids` holds only what was RETURNED, so on a zero-result call the
|
||||
# near-missed record needs a name of its own.
|
||||
|
||||
|
||||
def test_the_rejected_record_is_named_beside_the_score_it_scored():
|
||||
@@ -1271,7 +1259,7 @@ async def test_the_listing_names_the_highest_declines_per_source(_dispose_engine
|
||||
out = await retrieval_summary(UID, days=30, near_miss_samples=2)
|
||||
assert out["read_failed"] is False, (
|
||||
"the listing query did not execute — a rejected query here reads "
|
||||
"as an absent listing, which is #2663 again"
|
||||
"as an absent listing"
|
||||
)
|
||||
|
||||
top = out["sources"]["pre_tool_rule"]["near_miss_records"]
|
||||
|
||||
Reference in New Issue
Block a user