Files
FabledScribe/tests/test_routes_retrieval_tuning.py
T
bvandeusenandClaude Opus 5 6240652dce
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / integration (push) Successful in 49s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m30s
CI & Build / Build & push image (push) Canceled after 31s
feat(retrieval): the operator can see what was tuned, and every write is recorded (#4102)
The other half of the bargain in milestone 416 step 4. The model moves
these dials; this is what makes that reviewable rather than merely
automatic.

THE HOLE THIS CLOSES

Every retrieval floor is an ordinary settings key, and `/api/settings`
accepts any key at all. A floor written through it landed correctly and
recorded nothing — a tuning history with holes in it, which is worse
than no history because it reads as complete.

So the generic endpoint now routes registry-owned keys through
`set_dial` instead of writing them as plain rows. ROUTED, not refused:
refusing would only work for callers that had been updated, while this
way the form, a script, and an old client all leave the trail, and
there is no version of "forgot to use the other endpoint". Clearing a
control is written as an explicit set back to the shipped default,
because the operator reverting something is the single most important
move this history can record.

`set_dial` now also refuses to record a no-op. The Settings form
re-sends every field on every save, so without that one press of Save
would write six rows saying the operator set six dials to the numbers
they were already on — and a history nobody can skim is one nobody
reads.

WHAT THE OPERATOR GETS

`/api/retrieval/surfaces`, `/surfaces/<name>` and `/tuning-history`,
with `actor` fixed server-side rather than taken from the payload: a
payload-supplied actor would let a model claim to be the operator, and
"did I do this, or did the session?" is the first question this list
is asked.

In Settings: the five missing BUDGETS (until now only auto-inject had
one, so the only control over a noisy surface was to raise its bar —
which discards that surface's best candidates along with its worst),
and a "What has been tuned" panel showing each change, who made it, and
the reason given. The operator's own changes are marked.

The MCP tool demands a reason; these endpoints do not. That asymmetry
is deliberate and stated in routes/retrieval.py: the requirement exists
to make the MODEL read the records before moving a number on someone
else's behalf, and the operator is that someone — a mandatory
justification box on every control would be friction charged to the one
participant who owes no explanation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
2026-09-17 11:36:22 -04:00

134 lines
5.4 KiB
Python

"""The operator's half of the tuning surface (#4102).
WHY THIS EXISTS
The model moves these dials — that is the operator's decision for milestone
416 — so the browser's job is no longer "set the number". It is to show what
was set, by whom, and on what argument, and to let the operator disagree.
The load-bearing guard here is the last one. `/api/settings` is a generic
key-value endpoint that accepts any key at all, and every retrieval floor IS an
ordinary settings key. A floor written straight through it would land correctly
and record nothing — a tuning history with holes in it, which is worse than no
history because it reads as complete. So the generic endpoint routes those keys
through `set_dial`, and that routing is asserted rather than remembered.
"""
import inspect
import pytest
def test_retrieval_blueprint_registered():
from scribe.routes.retrieval import retrieval_bp
assert retrieval_bp.name == "retrieval"
assert retrieval_bp.url_prefix == "/api/retrieval"
def test_retrieval_blueprint_registered_in_app():
from scribe.app import create_app
app = create_app()
assert "retrieval" in app.blueprints
def test_every_endpoint_is_reachable_on_the_app():
"""Handlers existing is not the same as them being routed."""
from scribe.app import create_app
app = create_app()
rules = {
str(r.rule) for r in app.url_map.iter_rules()
if r.endpoint.startswith("retrieval.")
}
assert rules == {
"/api/retrieval/surfaces",
"/api/retrieval/surfaces/<surface>",
"/api/retrieval/tuning-history",
}
def test_the_browser_and_the_agent_call_the_same_service():
"""Rule 33 parity. Two callers, one service — or the two surfaces drift and
the guard that a reason is required exists on only one of them."""
from scribe.mcp.tools import retrieval_tuning as tool
from scribe.routes import retrieval as routes
from scribe.services import retrieval_tuning as svc
assert routes.set_dial is svc.set_dial
assert tool.tuning_svc is svc
def test_the_route_does_not_take_the_actor_from_the_caller():
"""`actor` is the one field a reviewer leans on to answer "did I do this,
or did the session?". A payload-supplied actor would let a model claim to
be the operator, which turns the column into decoration."""
from scribe.routes import retrieval as routes
src = inspect.getsource(routes.tune_surface_route)
assert 'actor="human"' in src
assert 'data.get("actor"' not in src and 'data["actor"]' not in src
# ── the hole the generic settings endpoint would otherwise leave ────────────
def test_every_registry_key_is_recognised_as_a_dial():
"""Derived from the registry, both directions. A seventh surface added
without a row here is a floor that can be written silently again."""
from scribe.services.retrieval_surfaces import SURFACES, dial_for_key
for name, surface in SURFACES.items():
assert dial_for_key(surface.floor_key) == (name, "floor")
assert dial_for_key(surface.budget_key) == (name, "budget")
def test_an_ordinary_setting_is_not_mistaken_for_a_dial():
"""The interception must be narrow. A false positive here would send an
unrelated setting through a service that parses it as a float and rejects
the save."""
from scribe.services.retrieval_surfaces import dial_for_key
for key in ("smtp_password", "kb_autoinject_enabled", "theme",
"kb_planmatch_threshold", ""):
assert dial_for_key(key) is None, key
def test_the_settings_endpoint_routes_a_dial_through_the_recorder():
"""THE GUARD. Asserted on the source because the alternative is a full
request-context round trip for a branch whose whole content is "which
function gets called" — and because what must not regress is precisely
that this module reaches for `set_dial` at all."""
from scribe.routes import settings as routes
src = inspect.getsource(routes.update_settings_route)
assert "dial_for_key" in src, (
"the generic settings endpoint no longer recognises retrieval dials — "
"a floor written through it now moves with no event recorded, and the "
"tuning history will say nothing happened"
)
assert "set_dial" in src
assert 'actor="human"' in src
@pytest.mark.asyncio
async def test_setting_a_dial_to_the_value_it_already_has_records_nothing():
"""The Settings form re-sends every field on every save. Without this, one
press of Save writes six rows saying the operator set six dials to the
numbers they were already on — and a history nobody can skim is one nobody
reads."""
from unittest.mock import AsyncMock, MagicMock, patch
from scribe.services import retrieval_tuning as rt
from tests.helpers import make_mock_session
session = make_mock_session()
with patch.object(rt, "async_session", MagicMock(return_value=session)), \
patch.object(rt, "set_setting", AsyncMock()) as setter, \
patch.object(rt, "floor_for", AsyncMock(return_value=0.72)), \
patch.object(rt, "budget_for", AsyncMock(return_value=3)):
out = await rt.set_dial(1, "prompt_rule", "floor", 0.72,
reason="re-saved the settings form untouched")
assert out["unchanged"] is True
session.add.assert_not_called()
# And the setting is left alone too: rewriting the same value would bump
# whatever timestamp the row carries for no reason.
setter.assert_not_called()