CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 46s
CI & Build / TypeScript typecheck (push) Successful in 52s
CI & Build / Python tests (push) Successful in 1m44s
CI & Build / Build & push image (push) Successful in 28s
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
277 lines
11 KiB
Python
277 lines
11 KiB
Python
"""Rule and preference creates surface the records they overlap (#4134).
|
|
|
|
Rules and preferences share one table and one ranking, so a preference that
|
|
restates a rule splits one instruction in two — and when only the preference
|
|
places, binding guidance arrives labelled optional. The title gate could not
|
|
see it: a second record about the same moment is exactly the one written
|
|
under a different title.
|
|
|
|
These tests pin three things:
|
|
|
|
- WHAT is compared: the new record's rule_document (the shape the corpus is
|
|
embedded as), against BOTH kinds, in the scope the new record will rank in.
|
|
- THAT it advises rather than blocks. The measurement above
|
|
dedup._RULE_OVERLAP_FLOOR found distinct neighbours scoring as high as a
|
|
true restatement, so a block would refuse legitimate records.
|
|
- WHERE it runs: all three create doors, before the create, so the new record
|
|
cannot match itself.
|
|
|
|
`find_overlapping_rules` is bound here at import time, before conftest's
|
|
autouse `_no_rule_overlap` replaces the module attribute, so the service tests
|
|
exercise the real function and stub the search beneath it.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import ast
|
|
import inspect
|
|
import pathlib
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
|
|
from scribe.services import dedup
|
|
from scribe.services.dedup import (
|
|
RuleOverlap,
|
|
find_overlapping_rules,
|
|
overlap_response,
|
|
)
|
|
from tests.helpers import fake_rule
|
|
from tests.helpers import plain_rule_detail as _plain_detail
|
|
|
|
pytestmark = pytest.mark.usefixtures("_bind_user")
|
|
|
|
SEARCH = "scribe.services.dedup.embeddings_svc.semantic_search_rules"
|
|
TOOLS = "scribe.mcp.tools.rulebooks"
|
|
|
|
# Long enough to clear _MIN_BODY_FOR_SEMANTIC in the rule_document shape.
|
|
TRIGGER = (
|
|
"About to push a commit to dev and stop to ask whether pushing is allowed, "
|
|
"or ending a turn with a question about landing work that was already "
|
|
"committed on the branch the operator treats as home."
|
|
)
|
|
STATEMENT = "Push to dev after committing without asking first."
|
|
|
|
|
|
# ── what is compared ──────────────────────────────────────────────────────
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_query_is_the_embedded_document_shape():
|
|
search = AsyncMock(return_value=[])
|
|
with patch(SEARCH, search):
|
|
await find_overlapping_rules(7, "Push without asking", STATEMENT, TRIGGER)
|
|
query = search.call_args.args[1]
|
|
title, body = dedup.embeddings_svc.rule_document(
|
|
"Push without asking", STATEMENT, TRIGGER,
|
|
)
|
|
assert query == f"{title}\n\n{body}"
|
|
assert "When to apply:" in query
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_both_kinds_are_searched():
|
|
"""The harm is ACROSS kinds; a kind filter would hide exactly it."""
|
|
search = AsyncMock(return_value=[])
|
|
with patch(SEARCH, search):
|
|
await find_overlapping_rules(7, "t", STATEMENT, TRIGGER)
|
|
assert search.call_args.kwargs.get("kind") is None
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_global_record_is_compared_with_every_rule_the_caller_owns():
|
|
search = AsyncMock(return_value=[])
|
|
with patch(SEARCH, search):
|
|
await find_overlapping_rules(7, "t", STATEMENT, TRIGGER)
|
|
kw = search.call_args.kwargs
|
|
assert kw["everywhere"] is True
|
|
assert kw["project_id"] is None
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_project_rule_is_compared_with_what_it_will_rank_against():
|
|
search = AsyncMock(return_value=[])
|
|
with patch(SEARCH, search):
|
|
await find_overlapping_rules(7, "t", STATEMENT, TRIGGER, project_id=5)
|
|
kw = search.call_args.kwargs
|
|
assert kw["project_id"] == 5
|
|
assert kw["everywhere"] is False
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_floor_is_the_measured_one():
|
|
search = AsyncMock(return_value=[])
|
|
with patch(SEARCH, search):
|
|
await find_overlapping_rules(7, "t", STATEMENT, TRIGGER)
|
|
assert search.call_args.kwargs["threshold"] == dedup._RULE_OVERLAP_FLOOR
|
|
|
|
|
|
def test_the_floor_sits_below_the_measured_restatement():
|
|
"""A reworded duplicate of a real rule scored 0.850 (the measurement above
|
|
_RULE_OVERLAP_FLOOR). A floor at or above that misses the case the check
|
|
exists for; this fails if someone raises it there by analogy with the
|
|
blocking gates' 0.90+."""
|
|
assert dedup._RULE_OVERLAP_FLOOR < 0.85
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_short_document_is_not_searched():
|
|
search = AsyncMock(return_value=[])
|
|
with patch(SEARCH, search):
|
|
out = await find_overlapping_rules(7, "t", "s", "when")
|
|
assert out == []
|
|
search.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_failed_search_lets_the_create_through():
|
|
with patch(SEARCH, AsyncMock(side_effect=RuntimeError("no embedder"))):
|
|
out = await find_overlapping_rules(7, "t", STATEMENT, TRIGGER)
|
|
assert out == []
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_hits_carry_their_kind():
|
|
hits = [
|
|
(0.8504, fake_rule(id=1, title="`dev` is home", kind="rule")),
|
|
(0.81, fake_rule(id=9, title="Report pushes", kind="preference", project_id=3)),
|
|
]
|
|
with patch(SEARCH, AsyncMock(return_value=hits)):
|
|
out = await find_overlapping_rules(7, "t", STATEMENT, TRIGGER)
|
|
assert out == [
|
|
RuleOverlap(1, "`dev` is home", "rule", None, 0.85),
|
|
RuleOverlap(9, "Report pushes", "preference", 3, 0.81),
|
|
]
|
|
|
|
|
|
# ── what it says ──────────────────────────────────────────────────────────
|
|
|
|
|
|
def test_no_overlap_adds_nothing():
|
|
assert overlap_response([], "rule") == {}
|
|
|
|
|
|
def test_the_note_names_the_record_to_read_and_both_outcomes():
|
|
out = overlap_response(
|
|
[RuleOverlap(1, "`dev` is home", "rule", None, 0.85)], "preference",
|
|
)
|
|
assert out["overlaps"] == [{
|
|
"id": 1, "title": "`dev` is home", "kind": "rule",
|
|
"project_id": None, "similarity": 0.85,
|
|
}]
|
|
note = out["overlap_note"]
|
|
assert "Created" in note # it did not block
|
|
assert "update_rule" in note # the top match's own door
|
|
assert "delete this preference" in note # the new record's kind
|
|
assert "keep both" in note # distinct records are legitimate
|
|
|
|
|
|
# ── where it runs ─────────────────────────────────────────────────────────
|
|
|
|
|
|
def _rule_one_overlaps():
|
|
return [RuleOverlap(1, "`dev` is home", "rule", None, 0.85)]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_rule_checks_before_creating_and_reports():
|
|
order: list[str] = []
|
|
find = AsyncMock(side_effect=lambda *a, **k: order.append("find") or _rule_one_overlaps())
|
|
create = AsyncMock(side_effect=lambda **k: order.append("create") or fake_rule(id=50))
|
|
with patch(f"{TOOLS}.dedup_svc.find_duplicate_rule", AsyncMock(return_value=None)), \
|
|
patch(f"{TOOLS}.dedup_svc.find_overlapping_rules", find), \
|
|
patch(f"{TOOLS}.rulebooks_svc.create_rule", create), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_rule
|
|
out = await create_rule(
|
|
topic_id=10, title="Push without asking", statement=STATEMENT,
|
|
when_to_apply=TRIGGER,
|
|
)
|
|
assert order == ["find", "create"]
|
|
assert find.call_args.args == (7, "Push without asking", STATEMENT, TRIGGER)
|
|
assert out["id"] == 50
|
|
assert out["overlaps"][0]["id"] == 1
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_project_rule_checks_in_its_project():
|
|
find = AsyncMock(return_value=_rule_one_overlaps())
|
|
with patch(f"{TOOLS}.dedup_svc.find_duplicate_rule", AsyncMock(return_value=None)), \
|
|
patch(f"{TOOLS}.dedup_svc.find_overlapping_rules", find), \
|
|
patch(f"{TOOLS}.rulebooks_svc.create_project_rule",
|
|
AsyncMock(return_value=fake_rule(id=51, project_id=5, topic_id=None))), \
|
|
_plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_project_rule
|
|
out = await create_project_rule(
|
|
project_id=5, title="Push without asking", statement=STATEMENT,
|
|
when_to_apply=TRIGGER,
|
|
)
|
|
assert find.call_args.kwargs == {"project_id": 5}
|
|
assert "overlap_note" in out
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_preference_checks_and_names_itself_a_preference():
|
|
find = AsyncMock(return_value=_rule_one_overlaps())
|
|
with patch(f"{TOOLS}.dedup_svc.find_duplicate_rule", AsyncMock(return_value=None)), \
|
|
patch(f"{TOOLS}.dedup_svc.find_overlapping_rules", find), \
|
|
patch(f"{TOOLS}.rulebooks_svc.create_rule",
|
|
AsyncMock(return_value=fake_rule(id=52, kind="preference"))), \
|
|
_plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_preference
|
|
out = await create_preference(
|
|
topic_id=10, title="Push without asking", statement=STATEMENT,
|
|
when_to_apply=TRIGGER, arose_from_id=42,
|
|
)
|
|
find.assert_awaited_once()
|
|
assert "delete this preference" in out["overlap_note"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_create_with_no_overlap_reads_as_before():
|
|
with patch(f"{TOOLS}.dedup_svc.find_duplicate_rule", AsyncMock(return_value=None)), \
|
|
patch(f"{TOOLS}.dedup_svc.find_overlapping_rules", AsyncMock(return_value=[])), \
|
|
patch(f"{TOOLS}.rulebooks_svc.create_rule",
|
|
AsyncMock(return_value=fake_rule(id=53))), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_rule
|
|
out = await create_rule(
|
|
topic_id=10, title="t", statement=STATEMENT, when_to_apply=TRIGGER,
|
|
)
|
|
assert "overlaps" not in out and "overlap_note" not in out
|
|
|
|
|
|
CREATE_DOORS = ("create_rule", "create_project_rule", "create_preference")
|
|
|
|
|
|
def test_every_rule_create_door_asks():
|
|
"""Structural, so a fourth create door fails here rather than shipping
|
|
with the title gate alone. Keyed on the doors that call
|
|
find_duplicate_rule: any door gated by title must also be checked by
|
|
meaning, because the title gate is the one that cannot see this."""
|
|
root = pathlib.Path(__file__).resolve().parents[1] / "src" / "scribe"
|
|
tree = ast.parse((root / "mcp" / "tools" / "rulebooks.py").read_text())
|
|
title_gated, overlap_checked = set(), set()
|
|
for fn in tree.body:
|
|
if not isinstance(fn, ast.AsyncFunctionDef):
|
|
continue
|
|
for node in ast.walk(fn):
|
|
if isinstance(node, ast.Attribute):
|
|
if node.attr == "find_duplicate_rule":
|
|
title_gated.add(fn.name)
|
|
elif node.attr == "find_overlapping_rules":
|
|
overlap_checked.add(fn.name)
|
|
assert title_gated >= set(CREATE_DOORS), "registry drifted from the module"
|
|
assert title_gated <= overlap_checked, (
|
|
f"title-gated but never checked by meaning: "
|
|
f"{sorted(title_gated - overlap_checked)}"
|
|
)
|
|
|
|
|
|
def test_the_stale_premise_is_gone():
|
|
"""find_duplicate_rule claimed rules were not a semantic-retrieval surface
|
|
— false since rules were embedded, and the reason this gap survived."""
|
|
# Flattened: the docstring is hard-wrapped, and the claim straddled a
|
|
# line. Keyed on the CONCLUSION it drew, not the premise — the corrected
|
|
# docstring quotes the premise in order to retire it.
|
|
doc = " ".join((inspect.getdoc(dedup.find_duplicate_rule) or "").split())
|
|
assert "the right (and only) signal" not in doc
|
|
assert "find_overlapping_rules" in doc
|