diff --git a/alembic/versions/0121_retire_report_preference.py b/alembic/versions/0121_retire_report_preference.py new file mode 100644 index 00000000..1f34195c --- /dev/null +++ b/alembic/versions/0121_retire_report_preference.py @@ -0,0 +1,59 @@ +"""retire the report_preference arm's rows (milestone 500 step 4) + +Revision ID: 0121 +Revises: 0120 +Create Date: 2026-10-09 + +The fixed-question arm that searched for completion-report preferences when a +task closed is gone: the reply shapes and the preferences mounted beside them +now arrive on the moments (milestone 500 step 3). Its rows go with it (rule +22 — no legacy to preserve), because each one would otherwise mean something +different once the arm is no longer registered: + +- `retrieval_logs` / `retrieval_judgments` — a source missing from the + registry reads as `unregistered_source`, whose advice is "add it to the + registry": the opposite of what happened. +- `rule_usage_events` — a source missing from `RANKED_SOURCES` reads as + ambient, so its surfacings would silently move into the other column of + every rule's pull-through. +- `retrieval_tuning_events` — the tuning history of a surface that no longer + exists. +- `settings` — the arm's floor and budget keys, which nothing reads. + +COST IS BOUNDED BY AN INDEX, NOT BY THE TABLE. A migration runs at boot +against the live tables, which CI never has (lesson #5221). `retrieval_logs` +and `retrieval_judgments` are indexed on `source`. `rule_usage_events` is not, +so its delete is also bounded by `created_at` (indexed): the arm shipped with +milestone 409 step 4, decided 2026-09-14, so no row of it predates +`ARM_BORN`, and the scan covers weeks rather than the table's whole history. + +Not reversible: the arm is not coming back, so neither are its rows. +""" +from alembic import op + +revision = "0121" +down_revision = "0120" +branch_labels = None +depends_on = None + +SOURCE = "report_preference" +# A safe floor under the arm's first row (it shipped after 2026-09-14). +ARM_BORN = "2026-09-01" + + +def upgrade() -> None: + op.execute(f"DELETE FROM retrieval_logs WHERE source = '{SOURCE}'") + op.execute(f"DELETE FROM retrieval_judgments WHERE source = '{SOURCE}'") + op.execute( + f"DELETE FROM rule_usage_events WHERE created_at >= '{ARM_BORN}' " + f"AND source = '{SOURCE}'" + ) + op.execute(f"DELETE FROM retrieval_tuning_events WHERE surface = '{SOURCE}'") + op.execute( + "DELETE FROM settings WHERE key IN " + "('kb_reportpref_threshold', 'kb_reportpref_top_k')" + ) + + +def downgrade() -> None: + pass diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index e994238b..a10a6582 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -168,7 +168,6 @@ const kbToolRuleThreshold = ref("0.68"); // And the prompt boundary is a third query shape again — the operator's own // prose rather than anything a tool produced (#3852). const kbPromptRuleThreshold = ref("0.72"); -const kbReportPrefThreshold = ref("0.72"); // The reply arm (milestone 458): its bar is the bar at which a finished reply // is HELD for one read, so it sits with the checkpoint, not with the hints. const kbReplyRuleThreshold = ref("0.8"); @@ -181,7 +180,6 @@ const kbWritePathTopK = ref("3"); const kbRuleHintTopK = ref("5"); const kbToolRuleTopK = ref("5"); const kbPromptRuleTopK = ref("3"); -const kbReportPrefTopK = ref("3"); const kbReplyRuleTopK = ref("1"); // What has been changed about retrieval, newest first — the review surface for // changes the model made on the operator's behalf (#4102). @@ -387,7 +385,6 @@ async function saveKbInject() { // this is the only number that can STOP a call, so a fallback of 0 would // hold the first command of every session behind whatever ranked first. const cpT = asBar(kbCheckpointThreshold.value, 0.8); - const rpT = asBar(kbReportPrefThreshold.value, 0.72); // A stop bar like the checkpoint's: a fallback of 0 would hold every reply. const ryT = asBar(kbReplyRuleThreshold.value, 0.8); // The budgets, clamped the way the server clamps them: a whole number in @@ -400,13 +397,11 @@ async function saveKbInject() { const rhK = asK(kbRuleHintTopK.value, 5); const trK = asK(kbToolRuleTopK.value, 5); const prK = asK(kbPromptRuleTopK.value, 3); - const rpK = asK(kbReportPrefTopK.value, 3); const ryK = asK(kbReplyRuleTopK.value, 1); kbWritePathTopK.value = String(wpK); kbRuleHintTopK.value = String(rhK); kbToolRuleTopK.value = String(trK); kbPromptRuleTopK.value = String(prK); - kbReportPrefTopK.value = String(rpK); kbReplyRuleTopK.value = String(ryK); kbInjectThreshold.value = String(t); kbInjectTopK.value = String(k); @@ -424,7 +419,6 @@ async function saveKbInject() { kbCheckpointThreshold.value = String(cpT); kbToolRuleThreshold.value = String(trT); kbPromptRuleThreshold.value = String(prT); - kbReportPrefThreshold.value = String(rpT); kbReplyRuleThreshold.value = String(ryT); savingKbInject.value = true; kbInjectSaved.value = false; @@ -453,12 +447,6 @@ async function saveKbInject() { // queries are different shapes. Moving one must not move the others. kb_toolrule_threshold: String(trT), kb_promptrule_threshold: String(prT), - // A SIXTH, and the one that most needed its own key: this arm's query - // is a fixed string, so its score is a constant for a given corpus. - // While it borrowed the prompt bar, tuning prose silently retuned it — - // and a constant that lands under the bar is a dead arm, not a quiet - // one (#3860). - kb_reportpref_threshold: String(rpT), // The reply backstop's own bar: it HOLDS a reply, so like the // checkpoint it is never derived from a hint bar. kb_replyrule_threshold: String(ryT), @@ -470,7 +458,6 @@ async function saveKbInject() { kb_rulehint_top_k: String(rhK), kb_toolrule_top_k: String(trK), kb_promptrule_top_k: String(prK), - kb_reportpref_top_k: String(rpK), kb_replyrule_top_k: String(ryK), kb_duplicate_threshold_snippet: String(dupSnip), kb_duplicate_threshold_note: String(dupNote), @@ -941,9 +928,6 @@ onMounted(async () => { if (allSettings.kb_promptrule_threshold !== undefined) { kbPromptRuleThreshold.value = allSettings.kb_promptrule_threshold; } - if (allSettings.kb_reportpref_threshold !== undefined) { - kbReportPrefThreshold.value = allSettings.kb_reportpref_threshold; - } if (allSettings.kb_replyrule_threshold !== undefined) { kbReplyRuleThreshold.value = allSettings.kb_replyrule_threshold; } @@ -967,9 +951,6 @@ onMounted(async () => { if (allSettings.kb_promptrule_top_k !== undefined) { kbPromptRuleTopK.value = allSettings.kb_promptrule_top_k; } - if (allSettings.kb_reportpref_top_k !== undefined) { - kbReportPrefTopK.value = allSettings.kb_reportpref_top_k; - } if (allSettings.kb_replyrule_top_k !== undefined) { kbReplyRuleTopK.value = allSettings.kb_replyrule_top_k; } @@ -1974,42 +1955,6 @@ async function deleteUser(userId: number) { />

How many rules or preferences one message may be shown (1–10).

-
- - -

- The bar for a preference about how a completion report should be - written, looked up when a task closes. Unlike every other bar - here, the question this arm asks never changes — so its score is - fixed by your preferences alone, and it will either always find one - or never find one, and no run of calls will reveal a dead one on its - own. That is why this arm is worth looking up in the panel below when - a report preference never seems to arrive. -

-
-
- - -

How many preferences a finished task may be shown (1–10).

-
str: """kebab-case slug for a skill directory name (a-z0-9 + single hyphens).""" @@ -726,7 +707,8 @@ async def get_autoinject_config(user_id: int) -> dict: THE TWO NUMBERS COME FROM THE REGISTRY NOW (#4102). They used to be read and clamped here, and identically again in `get_writepath_config`, and again in - three rule arms, and once more in `reply_preferences`. That was + three rule arms, and once more in the completion-report arm (since + retired, milestone 500). That was tolerable while the values were shipped constants. It stops being tolerable once a tool is expected to MOVE them, because a tuning surface cannot be consistent across arms that each spell their configuration differently. diff --git a/src/scribe/services/reply_preferences.py b/src/scribe/services/reply_preferences.py deleted file mode 100644 index 97766d5d..00000000 --- a/src/scribe/services/reply_preferences.py +++ /dev/null @@ -1,141 +0,0 @@ -"""The operator's own preferences for a completion report, at the moment it is written. - -WHY THIS EXISTS (milestone 409 step 4) - -The reporting-back skill ships DEFAULT shapes. An operator will want some of -them different ("my completion reports also say how it was tested", "decisions -as a numbered list"), and those adjustments are `preference` records. The gap -is the query: prompt-time retrieval matches the OPERATOR'S MESSAGE, and a shape -preference is about the REPLY. "Fix the flaky test" never retrieves "completion -reports should say how it was tested", so the preference is on file and never -arrives. - -THE DECISION (operator, 2026-09-14, logged on the step): two deliveries, split -by reply kind. - - - A COMPLETION REPORT has a moment the server can see — a task closing — so - the server retrieves for it and hands the matches back beside - `report_back`. That is this module. - - EVERY OTHER REPLY KIND (a finding, a decision, a handoff…) has no tool call - in front of it, so the reporting-back skill asks: it tells the agent to - `search(content_type="rule")` for that kind before writing. - -Loading reply-shape preferences at session start was the rejected third option: -it is a small copy of the preloading milestone 394 retired, and just as -unmeasurable. - -HOW A PREFERENCE SAYS IT IS ABOUT COMPLETION REPORTS - -By its trigger, which is what it already has — no tag, no new column. A -preference's `when_to_apply` dominates its embedded document, so one written -for this moment ("writing the report after finishing a task") resembles -COMPLETION_QUERY below, and one about anything else does not. That keeps -delivery entirely in retrieval, as 394 decided, and leaves an operator nothing -new to learn: a preference reaches the completion report the same way every -other record reaches its moment. - -THE BAR IS ITS OWN, AND THE FIRST READING EARNED IT (#3860) - -This borrowed the prompt arm's key when it shipped, on the argument that a -fixed query against triggers is a different score distribution from an -operator's message against the same documents — true, and the reason the two -could not stay one dial. Five days of traffic settled it. - -What the readout said: 69 calls, 69 declines, every one naming the SAME record -at the SAME score (rule 77 at 0.7194 against a 0.72 bar). That constancy is -the signature of this arm — `COMPLETION_QUERY` never varies, so for a given -corpus its best score is a constant, and a constant sitting under the bar is a -dead arm rather than a quiet one. The record it kept declining was about -reading a REQUEST, not about the shape of a report, so the decline was right -and the arm is healthy: this install simply has no completion-report -preference on file. - -The bar stayed at 0.72, and the key moved out (REPORTPREF_THRESHOLD_KEY) so -that staying is a decision rather than a side effect of what the prose arm is -set to. A surface whose score cannot vary is the one surface where a borrowed -bar can be wrong forever without a single call looking unusual. -""" -from __future__ import annotations - -import logging - -from scribe.services import retrieval_pipeline as rp -from scribe.services.embeddings import semantic_search_rules -from scribe.services.retrieval_surfaces import SURFACES, budget_for, floor_for -from scribe.services.retrieval_telemetry import record_retrieval -from scribe.services.rule_usage import record_rule_surfaced - -logger = logging.getLogger(__name__) - -SOURCE = rp.REPORT_PREFERENCE.source - -# Written in the vocabulary of the MOMENT, because that is what a trigger is -# written in and what this query is scored against. Domain-neutral on purpose -# (rule #115): a writing project or a home-infrastructure project closes tasks -# too, and its operator's preferences must match as well as a developer's. -COMPLETION_QUERY = ( - "writing the completion report to the operator after finishing a task — " - "how that reply should be laid out and what it should include" -) - -# A handful, not a menu. More than a few shape preferences for ONE kind of -# reply would contradict each other before they helped; the limit is here to -# keep one noisy corpus from turning a status change into a wall of text. -# The STARTING budget, not the budget (#4102). Both numbers this arm runs on -# now come from the surface registry, so the model that reads this arm's -# telemetry can move either — which matters more here than anywhere else, -# because a fixed query makes this arm's score a constant and a floor a hair -# above it produces a dead arm no amount of traffic will ever reveal. -LIMIT = SURFACES["report_preference"].budget_default - - -async def _threshold(user_id: int) -> float: - return await floor_for(user_id, SOURCE) - - -async def _limit(user_id: int) -> int: - return await budget_for(user_id, SOURCE) - - -async def completion_preferences(user_id: int, *, project_id: int | None = None) -> list[dict]: - """The operator's preferences for a completion report, best match first. - - KIND-FILTERED, for the reason `_reserve_slot_for_preference` gives: a - binding rule that happened to resemble the query would otherwise ride out - under a key that says "how the operator likes this written", which is a - claim about force the record does not make. - - Every call is logged, the empty ones included: a surface that records only - the calls it liked reports a flawless clear-rate however badly its bar is - set. An install with no preferences at all logs nothing, because no search - ran (`searched` stays False) — that is not a decline. - - Fails open to an empty list: this decorates a write that has already - happened, and a lookup that errors must not turn it into a failure. - """ - try: - # The stages — the kind filter, the unconditional call row, the - # fresh-only surfacing rows — are the one pipeline's (milestone 456). - # RANKED: this surface chose what it showed, so the name is in - # rule_usage.RANKED_SOURCES and its hits count toward pull-through. - # There is no session ledger here: a completion report is written - # once, so nothing it could repeat has been shown before. - result = await rp.run_rule_arm( - rp.REPORT_PREFERENCE, - rp.RuleMoment( - user_id=user_id, query=COMPLETION_QUERY, project_id=project_id, - ), - floor=await _threshold(user_id), budget=await _limit(user_id), - io=rp.RuleIO( - search=semantic_search_rules, - record_retrieval=record_retrieval, - record_rule_surfaced=record_rule_surfaced, - ), - ) - return [ - {"id": rule.id, "title": rule.title, "statement": rule.statement, "kind": "preference"} - for _score, rule in result.shown - ] - except Exception: # noqa: BLE001 - a decoration never breaks its payload - logger.warning("completion preference lookup failed", exc_info=True) - return [] diff --git a/src/scribe/services/retrieval_migration.py b/src/scribe/services/retrieval_migration.py index 13941272..847137a6 100644 --- a/src/scribe/services/retrieval_migration.py +++ b/src/scribe/services/retrieval_migration.py @@ -117,7 +117,6 @@ _RESCORERS = { "pre_tool_rule": lambda u, q, p: _rescore_rules(u, q, p, None), "reply_rule": lambda u, q, p: _rescore_rules(u, q, p, None), "prompt_rule": lambda u, q, p: _rescore_rules(u, q, p, None), - "report_preference": lambda u, q, p: _rescore_rules(u, q, p, "preference"), } # The embedding table each surface's re-scorer reads. A migration from a corpus @@ -131,7 +130,6 @@ _CORPUS = { "pre_tool_rule": RuleEmbedding, "reply_rule": RuleEmbedding, "prompt_rule": RuleEmbedding, - "report_preference": RuleEmbedding, } diff --git a/src/scribe/services/retrieval_pipeline.py b/src/scribe/services/retrieval_pipeline.py index adbf03f4..e0fc76c9 100644 --- a/src/scribe/services/retrieval_pipeline.py +++ b/src/scribe/services/retrieval_pipeline.py @@ -421,10 +421,6 @@ class Declared: quiet_because: str = "" """Set when silence over an active window is correct, saying why (#2475).""" - fixed_query: bool = False - """The arm always searches the same string, so its decline rate is 0% or - 100% and `cannot_decline` says nothing about it.""" - @dataclass(frozen=True) class RankedSource: @@ -523,37 +519,6 @@ WRITE_PATH_RULE = RuleArm( ), declared=Declared("rules that may govern the file being written"), ) -# The completion report's preferences (milestone 409 step 4): a FIXED query, -# preferences only, read by update_task as records rather than as lines. Its -# query is `reply_preferences.COMPLETION_QUERY`. -REPORT_PREFERENCE = RuleArm( - "report_preference", band=False, compact_tail=False, checkpoint=False, - preference_slot=False, kind="preference", - tuning=Surface( - name="report_preference", - floor_key="kb_reportpref_threshold", - floor_default=0.72, - budget_key="kb_reportpref_top_k", - budget_default=3, - # THE ONE FIXED QUERY, and the reason this arm behaves unlike the rest. - # The others score something that varies per call; this one scores a - # constant string, so its top score for a given corpus is also a - # constant. A floor a hair above that constant is not a quiet arm, it - # is a dead one, and no amount of traffic will ever reveal it — which - # is precisely how this arm spent 69 calls declining the same record. - asks="a fixed question about how to lay out a completion report", - over="preferences", - fires="when a task finishes", - ), - # `fixed_query`: COMPLETION_QUERY is a module constant, so this arm's top - # score is the same number on every call — measured at 0.791 across 45 - # consecutive calls, with p10, p50, p90, min and max all identical. Five - # equal percentiles is the tell. - declared=Declared( - "the fixed question asked when a task finishes: how should this report read", - fixed_query=True, - ), -) # The backstop for every arm that ran earlier in the turn and missed: the # finished reply against every rule's trigger. Its floor IS its stop bar # (the reply_rule surface), so it is passed as both. @@ -581,7 +546,7 @@ REPLY_RULE = RuleArm( ), ) RULE_ARMS: tuple[RuleArm, ...] = ( - WRITE_PATH_RULE, PRE_TOOL_RULE, PROMPT_RULE, REPORT_PREFERENCE, REPLY_RULE, + WRITE_PATH_RULE, PRE_TOOL_RULE, PROMPT_RULE, REPLY_RULE, ) PREFERENCE_SLOT_SOURCE = "preference_slot" diff --git a/src/scribe/services/retrieval_registry.py b/src/scribe/services/retrieval_registry.py index 7b13598a..11d038c2 100644 --- a/src/scribe/services/retrieval_registry.py +++ b/src/scribe/services/retrieval_registry.py @@ -94,29 +94,6 @@ class Point: quiet_because: str = "" - fixed_query: bool = False - """Whether this arm always searches the SAME query string. - - THE #3497 GUARD, ONE STEP OVER. `logs_unconditionally` below exists - because a warning computed over a LOGGING property read as a ranking - problem. This field exists because a warning computed over a QUERY-SHAPE - property does the same thing. - - An arm with a fixed query scores against one constant. Its decline rate is - therefore 0% or 100% and nothing in between — which of the two depends - only on whether the bar sits below or above that single number. So "never - returned nothing" says nothing at all about whether a floor is applied, - and `cannot_decline` — whose whole remedy is "check that it applies its - floor" — is uninformative here and skips these arms. - - What IS informative for them is the mirror image, and `reply_preferences` - names it in its own docstring: every call returning nothing means the bar - sits above the constant, no traffic will ever move it, and the arm is - dead. That has happened — 69 consecutive declines at 0.0006 under the bar - (see `retrieval_surfaces`) — so it gets its own warning rather than - inheriting one written for arms whose score can vary. - """ - logs_unconditionally: bool = True """Whether this arm writes a row even when it returns NOTHING. @@ -144,8 +121,7 @@ POINTS: dict[str, Point] = dict([ # ambient or pulled. *(_p(spec.source, UNBIDDEN, spec.declared.what, expects_traffic=not spec.declared.quiet_because, - quiet_because=spec.declared.quiet_because, - fixed_query=spec.declared.fixed_query) + quiet_because=spec.declared.quiet_because) for spec in RANKED), # A LOOKUP, not a ranker (#4796): a record the operator named by number # in the message, fetched by id. No score and no bar, so it writes no diff --git a/src/scribe/services/retrieval_surfaces.py b/src/scribe/services/retrieval_surfaces.py index ee8286c7..ca997bc2 100644 --- a/src/scribe/services/retrieval_surfaces.py +++ b/src/scribe/services/retrieval_surfaces.py @@ -5,9 +5,9 @@ WHY THIS EXISTS Six push arms each carried their own loose copy of the same shape: a settings key, a default, and a limit that was usually a module constant nobody could change. The read-and-clamp was written out separately in `plugin_context` (twice -over, for auto-inject and the write path), in three rule arms, and again as -`reply_preferences._threshold`. That was survivable while the numbers were -shipped constants an operator occasionally edited. +over, for auto-inject and the write path), in three rule arms, and again in +the completion-report arm (since retired, milestone 500). That was survivable +while the numbers were shipped constants an operator occasionally edited. It stops being survivable once the numbers are meant to MOVE. The operator's decision for this step: diff --git a/src/scribe/services/retrieval_telemetry.py b/src/scribe/services/retrieval_telemetry.py index 1d73ead9..5762ae28 100644 --- a/src/scribe/services/retrieval_telemetry.py +++ b/src/scribe/services/retrieval_telemetry.py @@ -591,22 +591,12 @@ def _compute_warnings(sources: dict, usage: dict, rule_usage: dict, # arms once recorded only their hits, so their decline count was # structurally zero and this warning would have fired on a LOGGING # defect while pointing the reader at the threshold. - # - # FOUR NOW. A FIXED-QUERY arm is exempt for the same reason one step - # over (#4232): it scores against one constant, so its decline rate is - # 0% or 100% and never in between, and which one it is depends only on - # where the bar sits relative to that single number. "Never returned - # nothing" is then not evidence about the floor — it is arithmetic — - # and this warning's own remedy, "check that it applies its floor", - # cannot be answered from it. Those arms get `fixed_query_never_clears` - # below, which asks the question that IS answerable for them. if ( calls >= min_calls and (b.get("zero_result_calls") or 0) == 0 and point is not None and point.kind == UNBIDDEN and point.logs_unconditionally - and not point.fixed_query ): out.append(_warn( "cannot_decline", @@ -617,46 +607,6 @@ def _compute_warnings(sources: dict, usage: dict, rule_usage: dict, source=name, calls=calls, zero_result_calls=0, )) - # ── A fixed-query arm that never clears its bar ────────────────── - # - # The mirror image of `cannot_decline`, and the state that actually - # threatens these arms. `reply_preferences` names it in its own - # docstring: "a fixed query makes this arm's score a constant and a - # floor a hair above it produces a dead arm no amount of traffic will - # ever reveal." - # - # For an arm whose score can vary, a window of all-empty calls is - # ordinary — it means nothing matched, which is an answer. For one - # whose score is a constant it means the bar is above that constant, - # and no volume of further calls will ever produce a different result. - # The arm is not quiet; it is switched off, and nothing else in this - # readout would say so. - # - # It has happened: `report_preference` logged 69 consecutive declines - # at 0.0006 under the bar. Note what that incident also proves — the - # fix is NOT automatically to lower the floor. Reading the refused - # record showed the refusal was correct, so this warning sends the - # reader to `near_miss_samples` rather than to the dial. - if ( - calls >= min_calls - and point is not None - and point.fixed_query - and point.logs_unconditionally - and (b.get("zero_result_calls") or 0) == calls - ): - out.append(_warn( - "fixed_query_never_clears", - f"{calls} calls, every one of them empty — and this arm always " - f"searches the same query, so its score is a constant. That " - f"means the bar sits above it and no amount of further traffic " - f"will change the result: the arm is off, not quiet. Read the " - f"record it refused (`near_miss_samples`) before touching the " - f"floor — the last time this arm sat here, every percentile " - f"said lower it and the refused record showed the refusal was " - f"right.", - source=name, calls=calls, zero_result_calls=calls, - )) - # ── Band hugs its floor ────────────────────────────────────────── # # Read on p10, the WEAKEST tenth of what the arm returned. If even diff --git a/tests/test_mcp_tool_report_back_cue.py b/tests/test_mcp_tool_report_back_cue.py index f06338e9..feee6511 100644 --- a/tests/test_mcp_tool_report_back_cue.py +++ b/tests/test_mcp_tool_report_back_cue.py @@ -4,8 +4,10 @@ The in-band half of milestone 409 step 3: the skill and static context only exist in the Claude Code plugin, and a tool response reaches every MCP client at the moment a piece of work closes. Pinned on the response, not the wording. -Step 4 adds the operator's own completion-report preferences beside the cue, -retrieved at that same moment — and only then, and only when there are any. +The operator's own completion-report preferences used to ride here too, from +a fixed-question search (409 step 4). Milestone 500 retired that: they are +mounted on finishing work and arrive in `moment_rules` beside the delivered +reply shape, so this response carries no key of its own for them. """ from unittest.mock import AsyncMock, MagicMock, patch @@ -13,49 +15,37 @@ import pytest pytestmark = pytest.mark.usefixtures("_bind_user") -_PREF = {"id": 41, "title": "Say how it was checked", "statement": "…", "kind": "preference"} - -async def _update(prefs=None, owed=None, **kwargs): +async def _update(owed=None, **kwargs): from scribe.mcp.tools.tasks import update_task note = MagicMock(id=5, user_id=7, project_id=3, started_at="2026-10-06T09:00:00+00:00") note.to_dict.return_value = {"id": 5} - lookup = AsyncMock(return_value=list(prefs or [])) with patch("scribe.mcp.tools.tasks.notes_svc.update_note", AsyncMock(return_value=note)), \ patch("scribe.mcp.tools.tasks.systems_tools.attach_systems", AsyncMock()), \ patch("scribe.mcp.tools.tasks.placement_svc.attach_placement", AsyncMock()), \ patch("scribe.mcp.tools.tasks.family_adoption_svc.owed_since", - AsyncMock(return_value=list(owed or []))), \ - patch("scribe.mcp.tools.tasks.reply_prefs_svc.completion_preferences", lookup): - return await update_task(task_id=5, **kwargs), lookup + AsyncMock(return_value=list(owed or []))): + return await update_task(task_id=5, **kwargs) @pytest.mark.parametrize("status", ["done", "cancelled"]) async def test_closing_a_task_carries_the_cue(status): - out, _ = await _update(status=status) + out = await _update(status=status) assert "placement" in out["report_back"] and "needs them" in out["report_back"] @pytest.mark.parametrize("kwargs", [{"status": "in_progress"}, {"status": "todo"}, {"body": "more notes"}]) async def test_other_updates_do_not(kwargs): - out, lookup = await _update(prefs=[_PREF], **kwargs) - assert "report_back" not in out and "reply_preferences" not in out - lookup.assert_not_awaited() + out = await _update(**kwargs) + assert "report_back" not in out -async def test_closing_hands_back_the_operators_report_preferences(): - out, lookup = await _update(prefs=[_PREF], status="done") - assert out["reply_preferences"] == [_PREF] - # The cue says the key is there, so it never arrives unexplained. - assert "reply_preferences" in out["report_back"] - lookup.assert_awaited_once_with(7, project_id=3) - - -async def test_no_preferences_means_no_key_and_the_plain_cue(): +async def test_closing_carries_the_plain_cue_and_no_preference_key_of_its_own(): + """The retired key does not come back beside the moment delivery.""" from scribe.mcp.tools.tasks import REPORT_BACK_CUE - out, _ = await _update(prefs=[], status="done") + out = await _update(status="done") assert "reply_preferences" not in out assert out["report_back"] == REPORT_BACK_CUE @@ -69,11 +59,11 @@ async def test_closing_names_the_owed_adoptions_filed_while_the_task_was_open(): into a project during this task comes back for the report to name.""" from scribe.services.family_adoption import OWED_CUE - out, _ = await _update(owed=[_OWED], status="done") + out = await _update(owed=[_OWED], status="done") assert out["family_owed"] == [_OWED] assert out["report_back"].endswith(OWED_CUE) and "family_owed" in out["report_back"] async def test_no_owed_adoptions_means_no_key(): - out, _ = await _update(owed=[], status="done") + out = await _update(owed=[], status="done") assert "family_owed" not in out diff --git a/tests/test_retrieval_registry.py b/tests/test_retrieval_registry.py index d6be704a..a87c2796 100644 --- a/tests/test_retrieval_registry.py +++ b/tests/test_retrieval_registry.py @@ -103,10 +103,8 @@ def test_the_extractor_finds_something() -> None: def test_the_extractor_resolves_the_constant_sources() -> None: """The specific capability a grep would lose. See the module docstring. - `report_preference` was the second example until milestone 456 moved it - into the retrieval pipeline, where its source is a spec field; the - pipeline's reserved slot still records through a module constant, so it - carries the case instead. + The pipeline's reserved slot records through a module constant, so it + carries the case. """ found, _ = call_sites() for via_constant in ("wide_net", "preference_slot"): diff --git a/tests/test_retrieval_specs.py b/tests/test_retrieval_specs.py index c7754fda..75b3f9d9 100644 --- a/tests/test_retrieval_specs.py +++ b/tests/test_retrieval_specs.py @@ -34,7 +34,6 @@ def test_every_ranked_source_is_measured_as_its_spec_declares(): point = POINTS[spec.source] assert point.kind == UNBIDDEN assert point.what == d.what - assert point.fixed_query == d.fixed_query # A quiet source must say why, and only a quiet source may (#2475). assert point.expects_traffic == (not d.quiet_because) assert point.quiet_because == d.quiet_because diff --git a/tests/test_retrieval_surfaces.py b/tests/test_retrieval_surfaces.py index 237761fc..cc524e2c 100644 --- a/tests/test_retrieval_surfaces.py +++ b/tests/test_retrieval_surfaces.py @@ -24,9 +24,9 @@ WHAT THIS PINS different file. Renaming one without the other produces a surface that can be tuned and cannot be measured, or measured and not tuned, and both fail silently. - 2. **Keys are unique.** Two surfaces sharing a settings key is how - `report_preference` spent its first release moving whenever the prompt arm - was tuned (#3860) — one dial wearing two labels. + 2. **Keys are unique.** Two surfaces sharing a settings key is how the + completion-report arm (since retired) spent its first release moving + whenever the prompt arm was tuned (#3860) — one dial wearing two labels. 3. **An unknown surface is refused.** Settings keys are free-form strings in a generic table, so a typo'd name would write a key nothing reads: a change that appears to succeed, reports a new value, and alters nothing. @@ -44,7 +44,7 @@ import pytest from scribe.services import retrieval_surfaces as rs -SERVICES = ("plugin_context", "reply_preferences") +SERVICES = ("plugin_context",) def _service_source(name: str) -> str: @@ -63,10 +63,6 @@ def test_every_surface_name_is_a_real_telemetry_source(): to justify a change describes a different arm from the one the change hits. """ blob = "\n".join(_service_source(n) for n in SERVICES) - # `report_preference` passes its name through a module constant rather than - # a literal, so that one name is satisfied by the constant holding it. - from scribe.services.reply_preferences import SOURCE - # The rule arms record through the one pipeline (milestone 456), where # `source` is the spec's own field — so for them the spec IS the string # `record_retrieval` receives, and the join key is checked against it. @@ -76,7 +72,7 @@ def test_every_surface_name_is_a_real_telemetry_source(): missing = [ s.name for s in rs.SURFACES.values() if f'source="{s.name}"' not in blob - and s.name != SOURCE and s.name not in via_pipeline + and s.name not in via_pipeline ] assert not missing, ( f"these surfaces can be tuned but never measured: {missing}. The " diff --git a/tests/test_retrieval_warnings.py b/tests/test_retrieval_warnings.py index 2f2e36bf..ffb3694c 100644 --- a/tests/test_retrieval_warnings.py +++ b/tests/test_retrieval_warnings.py @@ -471,83 +471,6 @@ def test_a_quiet_arm_is_not_suspended_either_way() -> None: assert "floor_moved_mid_window" not in codes(ws) -# ── a fixed-query arm: decline rate is arithmetic, not evidence (#4232) ───── -# -# `report_preference` searches one constant string (COMPLETION_QUERY), so it -# scores against one number on every call. Its decline rate is therefore 0% or -# 100% and never in between, and which one depends only on where the bar sits -# relative to that constant. -# -# So the two warnings swap roles for these arms. "Never declined" stops being -# evidence about the floor — `cannot_decline`'s own remedy, "check that it -# applies its floor", is unanswerable from it. "Always declined" starts being -# evidence, because for a constant score it means the bar is above it and no -# further traffic will ever say otherwise. - - -def test_cannot_decline_is_silent_on_a_fixed_query_arm(): - """The live readout fired this on `report_preference` at 45 calls, 0 - empty, with p10 = p50 = p90 = min = max = 0.791 — five identical - percentiles, which is one record at one score rather than a ranking.""" - ws = warn({"report_preference": src(calls=45, zero_result_calls=0, p10=0.791)}) - assert "cannot_decline" not in codes(ws, "report_preference") - - -def test_cannot_decline_still_fires_where_the_rate_means_something(): - """The falsifier for the case above (rule 167). If this passes only - because the check was disabled rather than narrowed, this fails.""" - ws = warn({"auto_inject": src(calls=N, zero_result_calls=0)}) - assert "cannot_decline" in codes(ws, "auto_inject") - - -def test_a_fixed_query_arm_that_never_clears_its_bar_is_named(): - """69 consecutive declines at 0.0006 under the bar is a state this arm has - actually been in. Nothing else in the readout would have said so: it looks - exactly like an arm with nothing to report.""" - ws = warn({"report_preference": src(calls=45, zero_result_calls=45)}) - assert "fixed_query_never_clears" in codes(ws, "report_preference") - - detail = next(w["detail"] for w in ws if w["code"] == "fixed_query_never_clears") - assert "near_miss_samples" in detail, ( - "the last time this fired, every percentile said lower the floor and " - "the refused record showed the refusal was right — so the warning has " - "to send the reader to the record, not to the dial" - ) - - -def test_an_ordinary_arm_returning_nothing_all_window_is_not_dead(): - """For an arm whose score can vary, an empty window means nothing matched, - which is an answer rather than a fault.""" - ws = warn({"auto_inject": src(calls=N, zero_result_calls=N)}) - assert "fixed_query_never_clears" not in codes(ws, "auto_inject") - - -def test_a_fixed_query_arm_that_sometimes_clears_is_not_dead(): - """Only ALL-empty says the bar is above the constant. Anything in between - means the score is not actually constant, and the premise is wrong.""" - ws = warn({"report_preference": src(calls=45, zero_result_calls=44)}) - assert "fixed_query_never_clears" not in codes(ws, "report_preference") - - -def test_the_dead_arm_warning_still_needs_volume(): - ws = warn({"report_preference": src(calls=N - 1, zero_result_calls=N - 1)}) - assert "fixed_query_never_clears" not in codes(ws) - - -def test_the_registry_declares_which_arms_ask_a_fixed_question(): - """Asserted on structure (rule 167), and able to fail: if `fixed_query` - is dropped or defaults to True, one of these two halves breaks.""" - from scribe.services.retrieval_registry import POINTS - - assert POINTS["report_preference"].fixed_query is True, ( - "services/reply_preferences.py::COMPLETION_QUERY is a module constant" - ) - # An arm whose query is built from the prompt, the file or the command is - # not fixed, and marking one would silence a warning that works there. - for varying in ("auto_inject", "write_path", "pre_tool_rule", "prompt_rule"): - assert POINTS[varying].fixed_query is False, varying - - # ── floor_history_unknown: the window reaches back past the ledger ───────── # # THE SAME SUSPENSION, FOR THE CASE THE ONE ABOVE CANNOT SEE. diff --git a/tests/test_rule_usage_wiring.py b/tests/test_rule_usage_wiring.py index 9903da29..c6b4bd9b 100644 --- a/tests/test_rule_usage_wiring.py +++ b/tests/test_rule_usage_wiring.py @@ -1702,7 +1702,6 @@ def test_every_hook_rule_search_says_which_project_it_is_for(): # ranked-search stage, checked below — so a direct search appearing # here again is a copy of the arm coming back, and fails the count. "src/scribe/services/plugin_context.py": 0, - "src/scribe/services/reply_preferences.py": 0, } for path, expected in sources.items(): calls = [ diff --git a/tests/test_services_reply_preferences.py b/tests/test_services_reply_preferences.py deleted file mode 100644 index d742b2ce..00000000 --- a/tests/test_services_reply_preferences.py +++ /dev/null @@ -1,134 +0,0 @@ -"""The completion-report preference lookup (milestone 409 step 4). - -What it pins: the lookup asks for PREFERENCES only, logs every call under its -own source (the empty ones too), counts only what it showed as surfaced, and -fails open. The query stays domain-neutral, because every kind of project -closes tasks. -""" -import re -from unittest.mock import AsyncMock, MagicMock, patch - -M = "scribe.services.reply_preferences" -# Its two numbers are resolved through the registry now (#4102), so the -# settings reader to patch lives there rather than in the arm's own module. -RS = "scribe.services.retrieval_surfaces" - - -def _rule(rid, kind="preference"): - return MagicMock(id=rid, kind=kind, title=f"t{rid}", statement=f"s{rid}") - - -async def _run(hits, *, searched=True, raises=None): - from scribe.services.reply_preferences import completion_preferences - - async def search(user_id, query, **kw): - if raises: - raise raises - kw["report"].update({"searched": searched, "best_available_score": 0.7}) - return hits - - search_mock = AsyncMock(side_effect=search) - with patch(f"{M}.semantic_search_rules", search_mock), \ - patch(f"{RS}.get_setting", AsyncMock(return_value="0.72")), \ - patch(f"{M}.record_retrieval") as logged, \ - patch(f"{M}.record_rule_surfaced") as surfaced: - out = await completion_preferences(7, project_id=3) - return out, search_mock, logged, surfaced - - -async def test_asks_for_preferences_only_and_returns_them_best_first(): - out, search, logged, surfaced = await _run([(0.9, _rule(1)), (0.8, _rule(2))]) - assert search.await_args.kwargs["kind"] == "preference" - assert [p["id"] for p in out] == [1, 2] - assert all(p["kind"] == "preference" for p in out) - assert logged.call_args.kwargs["source"] == "report_preference" - assert surfaced.call_args.kwargs == {"user_id": 7, "rule_ids": [1, 2], "source": "report_preference"} - - -async def test_a_rule_that_slips_through_is_not_handed_back_as_a_preference(): - out, _, _, surfaced = await _run([(0.9, _rule(1, kind="rule")), (0.8, _rule(2))]) - assert [p["id"] for p in out] == [2] - assert surfaced.call_args.kwargs["rule_ids"] == [2] - - -async def test_an_empty_call_is_still_logged_and_surfaces_nothing(): - out, _, logged, surfaced = await _run([]) - assert out == [] - logged.assert_called_once() - assert logged.call_args.kwargs["results"] == [] - surfaced.assert_not_called() - - -async def test_a_search_that_never_ran_says_so_to_the_log(): - _, _, logged, _ = await _run([], searched=False) - assert logged.call_args.kwargs["searched"] is False - - -async def test_fails_open(): - out, _, _, surfaced = await _run([], raises=RuntimeError("embedder down")) - assert out == [] - surfaced.assert_not_called() - - -def test_it_is_a_ranked_source(): - from scribe.services.rule_usage import is_ambient - - assert not is_ambient("report_preference") - - -def test_its_bar_is_its_own_key_not_the_prompt_arm_s(monkeypatch): - """Regression on the coupling #3860 found (and on the fix being real). - - This arm shipped reading PROMPTRULE_THRESHOLD_KEY, so an operator tuning - the bar for their own prose moved this one with it and was never told. - That is worse here than anywhere else: every other arm scores a query that - varies per call, while COMPLETION_QUERY is fixed — so this arm's score for - a given corpus is a CONSTANT, and a constant that lands under the bar is a - dead arm rather than a quiet one. No amount of traffic reveals it. - - Asserted on the key the lookup actually asks for, which is the thing that - broke, rather than on the constant being defined somewhere. - """ - import asyncio - - from scribe.services import plugin_context - from scribe.services.reply_preferences import completion_preferences - - asked: list[str] = [] - - async def get_setting(user_id, key, default=""): - asked.append(key) - return default - - async def search(user_id, query, **kw): - kw["report"].update({"searched": True}) - return [] - - with patch(f"{RS}.get_setting", AsyncMock(side_effect=get_setting)), \ - patch(f"{M}.semantic_search_rules", AsyncMock(side_effect=search)), \ - patch(f"{M}.record_retrieval"): - asyncio.run(completion_preferences(7)) - - # Both numbers are its own since #4102 — a floor AND a budget — so the arm - # asks for two keys, and neither may be the prompt arm's. - from scribe.services.retrieval_surfaces import get_surface - - mine = get_surface("report_preference") - assert asked == [mine.floor_key, mine.budget_key] - theirs = get_surface("prompt_rule") - assert mine.floor_key != theirs.floor_key, ( - "the two keys are the same string again, so the settings form has one " - "dial driving two arms — which is the defect, whatever the value is" - ) - assert mine.floor_key == plugin_context.REPORTPREF_THRESHOLD_KEY, ( - "the registry and the module constant disagree about this arm's key, " - "so the Settings form would write one and the arm would read the other" - ) - - -def test_the_query_assumes_no_particular_domain(): - from scribe.services.reply_preferences import COMPLETION_QUERY - - dev_only = [w for w in (r"\bCI\b", r"\bcommit", r"\bpull request", r"\bcode\b", r"\btest") - if re.search(w, COMPLETION_QUERY, re.IGNORECASE)] - assert not dev_only, f"software-only vocabulary in a query every project runs: {dev_only}" diff --git a/tests/test_settings_defaults_agree.py b/tests/test_settings_defaults_agree.py index 975d79c8..e35691cb 100644 --- a/tests/test_settings_defaults_agree.py +++ b/tests/test_settings_defaults_agree.py @@ -53,7 +53,6 @@ _SURFACE_PAIRS = ( ("write_path_rule", "kbRuleHintThreshold"), ("pre_tool_rule", "kbToolRuleThreshold"), ("prompt_rule", "kbPromptRuleThreshold"), - ("report_preference", "kbReportPrefThreshold"), ("reply_rule", "kbReplyRuleThreshold"), )