CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 18s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / Python tests (push) Failing after 1m34s
CI & Build / Build & push image (push) Skipped
CI & Build / integration (push) Successful in 2m7s
Everything an agent can do with moments, a person can now see and change in the app. - Rule editor: a moment picker beside the trigger. Catalog moments are ticked; a named procedure's `skill.<name>` is typed and checked as the server checks it. `moments` is always sent, so unticking the last moment unmounts the rule. - Settings, Moments section (General tab): for each moment, what it means, the actions that reach it on this install (shipped ones can be switched off, the install's own removed), how many rules are mounted on it, and deliveries and agent opens over the window. Below that: named procedures with mounts, switched-off defaults with Restore, and a form to add an action. - retrieval_telemetry.moment_usage: per moment, `delivered`, `rules`, `opened` (agent pulls after the first delivery there; an upper bound, as by_source is) and `last_delivered_at`. No ratio, because a mount is a person's statement, not a ranker's guess. Guarded on its own, and also reported in retrieval_summary as `moment_usage`. - rulebooks.mount_counts; mounted_moments now derives from it. - GET /api/retrieval/moments carries `mounted` and `usage` (?days=). DELETE /moments/mappings also reads the mapping from query parameters, since the browser's DELETE sends no body. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
213 lines
8.9 KiB
Python
213 lines
8.9 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",
|
|
"/api/retrieval/moments",
|
|
"/api/retrieval/moments/mappings",
|
|
}
|
|
|
|
|
|
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()
|
|
|
|
|
|
# ── the moments view (milestone 458 step 6) ─────────────────────────────
|
|
|
|
async def _moments_call(handler_name, path, method="GET", **kw):
|
|
from types import SimpleNamespace
|
|
|
|
from quart import Quart, g
|
|
|
|
from scribe.routes import retrieval as routes
|
|
|
|
app = Quart(__name__)
|
|
async with app.test_request_context(path, method=method, **kw):
|
|
g.user = SimpleNamespace(id=7)
|
|
resp = await getattr(routes, handler_name).__wrapped__()
|
|
if isinstance(resp, tuple):
|
|
return await resp[0].get_json(), resp[1]
|
|
return await resp.get_json(), 200
|
|
|
|
|
|
async def test_the_view_gets_the_catalog_with_counts_beside_it():
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
from scribe.routes import retrieval as routes
|
|
|
|
usage = AsyncMock(return_value={"by_moment": {"work.finish": {"delivered": 2}}})
|
|
with patch.object(routes.moment_actions_svc, "actions_by_moment",
|
|
AsyncMock(return_value={"actions": {}, "removed_defaults": []})), \
|
|
patch.object(routes.rulebooks_svc, "mount_counts", AsyncMock(return_value={"work.finish": 1})), \
|
|
patch.object(routes, "moment_usage", usage):
|
|
body, status = await _moments_call("moments_route", "/api/retrieval/moments",
|
|
query_string={"days": "7"})
|
|
assert status == 200
|
|
assert body["mounted"] == {"work.finish": 1}
|
|
assert body["usage"]["days"] == 7
|
|
assert body["usage"]["by_moment"]["work.finish"]["delivered"] == 2
|
|
assert {m["name"] for m in body["moments"]} >= {"work.finish", "reply.report"}
|
|
|
|
|
|
async def test_a_failed_mount_count_is_flagged_not_shown_as_nothing():
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
from scribe.routes import retrieval as routes
|
|
|
|
with patch.object(routes.moment_actions_svc, "actions_by_moment",
|
|
AsyncMock(return_value={"actions": {}, "removed_defaults": []})), \
|
|
patch.object(routes.rulebooks_svc, "mount_counts", AsyncMock(side_effect=RuntimeError("down"))), \
|
|
patch.object(routes, "moment_usage", AsyncMock(return_value={"by_moment": {}})):
|
|
body, status = await _moments_call("moments_route", "/api/retrieval/moments")
|
|
assert status == 200
|
|
assert body["mounted"] == {} and body["mounted_failed"] is True
|
|
assert body["usage"]["days"] == 30
|
|
|
|
|
|
async def test_a_mapping_removal_can_come_as_query_parameters():
|
|
"""The browser's DELETE carries no body (api/client.apiDelete), so the
|
|
mapping it removes arrives in the query string."""
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
from scribe.routes import retrieval as routes
|
|
|
|
unmap = AsyncMock(return_value={"change": "removed this install's mapping"})
|
|
with patch.object(routes.moment_actions_svc, "unmap_action", unmap):
|
|
body, status = await _moments_call(
|
|
"unmap_action_route", "/api/retrieval/moments/mappings", method="DELETE",
|
|
query_string={"tool": "Bash", "match": "make ship", "moment": "work.deliver"},
|
|
)
|
|
assert status == 200
|
|
assert unmap.await_args.args == (7, "Bash", "make ship", "work.deliver")
|
|
assert unmap.await_args.kwargs["actor"] == "human"
|
|
|
|
|
|
async def test_a_mapping_with_neither_body_nor_parameters_is_refused():
|
|
body, status = await _moments_call(
|
|
"unmap_action_route", "/api/retrieval/moments/mappings", method="DELETE",
|
|
)
|
|
assert status == 400
|