bvandeusenandClaude Opus 5
cd9aa87aa4
CI & Build / Python lint (push) Failing after 6s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Failing after 26s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Skipped
The payoff step: a rule stops having to be resident to be honoured. - list_always_on_rules returns the ALWAYS-ON tier only. It is the session-start call, made before any project is in scope, so there is no area vocabulary to match a conditional rule against yet. - get_applicable_rules carries a conditional rule when the project works in an area the rule is tagged to — resolved through systems.canonical_id, so the project's own NAME for the area is irrelevant, which is the entire reason the catalog exists. The gate is applied IN SQL, so `limit` counts rules that will actually surface rather than rules about to be dropped. - Bindingness is a deterministic TAG match, never a similarity score (D7). The vector channel stays a suggestion, in search. co_surfaced_partners is the fix that rule 144 never had. It was split off rule 46 and folded back the same day because "either rule could surface without the other and miss exposing a project to what the entire shape is intended to be" — correct, and merging was the only remedy available. Now a partner ARRIVES with its other half even when nothing else selected it, tagged `via: co_surfaces` so the payload says why. Two limits, both deliberate: only rules the caller owns, because an edge is not a back door into someone else's rulebook; and a project's suppressions are passed as exclusions, because an explicit mute is a decision and an edge does not outrank it. COMPATIBILITY, asserted first in the integration test rather than reasoned about: a rule with no tier, no areas and no edges binds exactly as it did before any of this existed. `tier` defaults to always_on, so an install upgrades and every rule it already had keeps arriving. Getting that backwards would silently stop enforcing rules people rely on, which is worse than any amount of payload bloat. Four unit tests were coupled to the ORDER of a mocked session's execute() calls, so a new query broke them. Rather than pad the sequence and deepen that coupling, the three post-query lookups are stubbed by name — they have their own coverage, and the real wiring is proven against Postgres. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
395 lines
17 KiB
Python
395 lines
17 KiB
Python
"""Tests for services/rulebooks.py — mocks async_session, no real DB.
|
|
|
|
Mirrors the pattern in tests/test_events_service.py.
|
|
"""
|
|
from datetime import datetime, timezone
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
from tests.helpers import fake_rule, fake_rulebook, fake_topic, make_mock_session
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _no_exclusions():
|
|
"""get_applicable_rules asks for the project's always-on exclusions
|
|
(milestone 297) through its own session; these mocked-session tests
|
|
script the rule queries only, so the exclusions lookup is stubbed empty."""
|
|
with patch("scribe.services.rulebooks.excluded_always_on_rulebooks", AsyncMock(return_value=[])):
|
|
yield
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_rulebook_stores_to_db():
|
|
mock_session = make_mock_session()
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import create_rulebook
|
|
await create_rulebook(
|
|
user_id=7, title="FabledSword family", description="rules for the family",
|
|
)
|
|
assert mock_session.add.called
|
|
assert mock_session.commit.called
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_list_rulebooks_returns_owned_only():
|
|
rb = fake_rulebook(id=1)
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalars.return_value.all.return_value = [rb]
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import list_rulebooks
|
|
results = await list_rulebooks(user_id=7)
|
|
assert len(results) == 1
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_rulebook_returns_none_when_not_owner():
|
|
"""get_rulebook scopes by owner_user_id — wrong user gets None."""
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalar_one_or_none.return_value = None
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import get_rulebook
|
|
result = await get_rulebook(rulebook_id=1, user_id=99)
|
|
assert result is None
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_update_rulebook_only_sets_provided_fields():
|
|
rb = fake_rulebook(id=1, title="old")
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalar_one_or_none.return_value = rb
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import update_rulebook
|
|
await update_rulebook(rulebook_id=1, user_id=7, title="new")
|
|
assert rb.title == "new"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delete_rulebook_calls_delete():
|
|
rb = fake_rulebook(id=1)
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalar_one_or_none.return_value = rb
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
mock_session.delete = AsyncMock()
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import delete_rulebook
|
|
await delete_rulebook(rulebook_id=1, user_id=7)
|
|
assert mock_session.delete.called
|
|
|
|
|
|
# ── Topic CRUD ───────────────────────────────────────────────────────────
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_topic_requires_owned_rulebook():
|
|
"""create_topic raises ValueError if the rulebook isn't owned by user."""
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalar_one_or_none.return_value = None
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import create_topic
|
|
with pytest.raises(ValueError, match="not found"):
|
|
await create_topic(
|
|
rulebook_id=999, user_id=7, title="git-workflow",
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_list_topics_returns_topics_for_owned_rulebook():
|
|
rb = fake_rulebook(id=1)
|
|
topic = fake_topic(id=10, rulebook_id=1, title="git-workflow")
|
|
|
|
# Two execute calls: ownership check, then topic select.
|
|
mock_session = make_mock_session()
|
|
rb_result = MagicMock()
|
|
rb_result.scalar_one_or_none.return_value = rb
|
|
topic_result = MagicMock()
|
|
topic_result.scalars.return_value.all.return_value = [topic]
|
|
mock_session.execute = AsyncMock(side_effect=[rb_result, topic_result])
|
|
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import list_topics
|
|
results = await list_topics(rulebook_id=1, user_id=7)
|
|
assert len(results) == 1
|
|
assert results[0].title == "git-workflow"
|
|
|
|
|
|
# ── Rule CRUD ───────────────────────────────────────────────────────────
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_rule_requires_owned_topic():
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalar_one_or_none.return_value = None # topic not found
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import create_rule
|
|
with pytest.raises(ValueError, match="topic .* not found"):
|
|
await create_rule(
|
|
topic_id=999, user_id=7, title="x", statement="y",
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_list_rules_filters_by_topic_id():
|
|
"""list_rules(topic_id=X) returns rules in that topic, ownership-scoped."""
|
|
rule = fake_rule(id=1, topic_id=10)
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalars.return_value.all.return_value = [rule]
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import list_rules
|
|
results = await list_rules(user_id=7, topic_id=10)
|
|
assert len(results) == 1
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_rule_returns_none_when_not_owner():
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalar_one_or_none.return_value = None
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import get_rule
|
|
result = await get_rule(rule_id=1, user_id=99)
|
|
assert result is None
|
|
|
|
|
|
# ── Subscriptions + applicable_rules ────────────────────────────────────
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_subscribe_project_requires_owned_rulebook():
|
|
"""subscribe_project raises if user doesn't own the rulebook."""
|
|
mock_session = make_mock_session()
|
|
mock_result = MagicMock()
|
|
mock_result.scalar_one_or_none.return_value = None
|
|
mock_session.execute = AsyncMock(return_value=mock_result)
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import subscribe_project
|
|
with pytest.raises(ValueError, match="not found"):
|
|
await subscribe_project(
|
|
project_id=1, rulebook_id=999, user_id=7,
|
|
)
|
|
|
|
|
|
def _empty():
|
|
"""A MagicMock result whose .all() returns [] (or .scalars().all() returns [])."""
|
|
r = MagicMock()
|
|
r.all.return_value = []
|
|
r.scalars.return_value.all.return_value = []
|
|
return r
|
|
|
|
|
|
def _no_edges():
|
|
"""Silence the three post-query lookups get_applicable_rules now makes.
|
|
|
|
They are separate service functions with their own coverage (and the real
|
|
wiring is proven against Postgres in test_integration_rule_surfacing), so
|
|
stubbing them here keeps each of these tests about the one projection it
|
|
was written to check — rather than about the order a mocked session's
|
|
execute() calls happen to arrive in.
|
|
"""
|
|
return (
|
|
patch("scribe.services.rulebooks.co_surfaced_partners",
|
|
AsyncMock(return_value=[])),
|
|
patch("scribe.services.rulebooks.list_rule_relations",
|
|
AsyncMock(return_value={})),
|
|
patch("scribe.services.rulebooks.list_rule_systems",
|
|
AsyncMock(return_value={})),
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_applicable_rules_returns_shape():
|
|
"""get_applicable_rules returns the full projection — including the
|
|
new suppression fields and rulebook/topic IDs on each rule."""
|
|
mock_session = make_mock_session()
|
|
sub_result = MagicMock()
|
|
sub_result.all.return_value = [(1, "FabledSword family")]
|
|
rules_result = MagicMock()
|
|
rules_result.all.return_value = [
|
|
# The query selects the ENTITY plus three labels, so rule_brief stays
|
|
# the one place deciding what a surfaced rule carries (note 3026).
|
|
(fake_rule(id=i, title=f"Rule {i}", statement=f"Statement {i}", topic_id=2),
|
|
"git-workflow", 1, "FabledSword family")
|
|
for i in range(50)
|
|
]
|
|
# Execute order: sub_q, suppressed_rules_q, suppressed_topics_q,
|
|
# project-areas_q (milestone 307), rules_q, proj_rules_q
|
|
mock_session.execute = AsyncMock(side_effect=[
|
|
sub_result, _empty(), _empty(), _empty(), rules_result, _empty(),
|
|
])
|
|
|
|
_p1, _p2, _p3 = _no_edges()
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls, _p1, _p2, _p3:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import get_applicable_rules
|
|
result = await get_applicable_rules(project_id=3, user_id=7, limit=50)
|
|
|
|
assert "rules" in result
|
|
assert "project_rules" in result
|
|
assert "suppressed_rules" in result
|
|
assert "suppressed_topics" in result
|
|
assert "truncated" in result
|
|
assert "subscribed_rulebooks" in result
|
|
assert result["subscribed_rulebooks"] == [{"id": 1, "title": "FabledSword family"}]
|
|
assert len(result["rules"]) == 50
|
|
assert result["rules"][0]["topic_id"] == 2
|
|
assert result["rules"][0]["rulebook_id"] == 1
|
|
assert result["project_rules"] == []
|
|
assert result["suppressed_rules"] == []
|
|
assert result["suppressed_topics"] == []
|
|
assert result["truncated"] is False
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_applicable_rules_truncates_when_over_limit():
|
|
"""When limit+1 rows are returned, truncated=True and only `limit` returned."""
|
|
mock_session = make_mock_session()
|
|
sub_result = MagicMock()
|
|
sub_result.all.return_value = []
|
|
rules_result = MagicMock()
|
|
rules_result.all.return_value = [
|
|
(fake_rule(id=i, title=f"r{i}"), "topic", 1, "rb") for i in range(51)
|
|
]
|
|
mock_session.execute = AsyncMock(side_effect=[
|
|
sub_result, _empty(), _empty(), _empty(), rules_result, _empty(),
|
|
])
|
|
|
|
_p1, _p2, _p3 = _no_edges()
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls, _p1, _p2, _p3:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import get_applicable_rules
|
|
result = await get_applicable_rules(project_id=3, user_id=7, limit=50)
|
|
|
|
assert result["truncated"] is True
|
|
assert len(result["rules"]) == 50
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_applicable_rules_includes_project_scoped_rules():
|
|
"""Project-scoped rules surface in the project_rules field."""
|
|
mock_session = make_mock_session()
|
|
proj_rules_result = MagicMock()
|
|
proj_rules_result.all.return_value = [
|
|
(fake_rule(id=100, topic_id=None, project_id=3, title="Use alembic",
|
|
statement="Always run migrations via alembic, never raw SQL."),),
|
|
(fake_rule(id=101, topic_id=None, project_id=3, title="PR-bound",
|
|
statement="Land schema changes in their own PR."),),
|
|
]
|
|
mock_session.execute = AsyncMock(side_effect=[
|
|
_empty(), _empty(), _empty(), _empty(), _empty(), proj_rules_result,
|
|
])
|
|
|
|
_p1, _p2, _p3 = _no_edges()
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls, _p1, _p2, _p3:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import get_applicable_rules
|
|
result = await get_applicable_rules(project_id=3, user_id=7)
|
|
|
|
assert len(result["project_rules"]) == 2
|
|
assert result["project_rules"][0]["title"] == "Use alembic"
|
|
assert result["project_rules"][1]["id"] == 101
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_applicable_rules_surfaces_suppressed_with_context():
|
|
"""Suppressed rules and topics come back with full title + rulebook context
|
|
so the UI can render them without an extra round-trip."""
|
|
mock_session = make_mock_session()
|
|
suppressed_rules_result = MagicMock()
|
|
suppressed_rules_result.all.return_value = [
|
|
# (rule_id, title, topic_id, topic_title, rulebook_id, rulebook_title)
|
|
(17, "Old rule", 5, "old-topic", 1, "FabledSword family"),
|
|
]
|
|
suppressed_topics_result = MagicMock()
|
|
suppressed_topics_result.all.return_value = [
|
|
# (topic_id, topic_title, rulebook_id, rulebook_title)
|
|
(22, "design-system", 1, "FabledSword family"),
|
|
]
|
|
mock_session.execute = AsyncMock(side_effect=[
|
|
# sub_q, suppressed_rules_q, suppressed_topics_q, project-areas_q,
|
|
# rules_q, proj_rules_q
|
|
_empty(), suppressed_rules_result, suppressed_topics_result,
|
|
_empty(), _empty(), _empty(),
|
|
])
|
|
|
|
_p1, _p2, _p3 = _no_edges()
|
|
with patch("scribe.services.rulebooks.async_session") as mock_cls, _p1, _p2, _p3:
|
|
mock_cls.return_value = mock_session
|
|
from scribe.services.rulebooks import get_applicable_rules
|
|
result = await get_applicable_rules(project_id=3, user_id=7)
|
|
|
|
assert len(result["suppressed_rules"]) == 1
|
|
assert result["suppressed_rules"][0]["id"] == 17
|
|
assert result["suppressed_rules"][0]["rulebook_title"] == "FabledSword family"
|
|
assert len(result["suppressed_topics"]) == 1
|
|
assert result["suppressed_topics"][0]["title"] == "design-system"
|
|
|
|
|
|
# ── rule_brief + tier (milestone 307) ───────────────────────────────────
|
|
|
|
def test_rule_brief_carries_age_but_not_the_deep_fields():
|
|
"""The shape a SURFACED rule takes, and the reason it exists.
|
|
|
|
There were three hand-written copies of this dict and they had already
|
|
diverged — none carried the timestamps the model has always held, which is
|
|
why a rule written before the capability it duplicates was
|
|
indistinguishable at read time from one still doing work (note 3026).
|
|
"""
|
|
from scribe.services.rulebooks import rule_brief
|
|
|
|
out = rule_brief(fake_rule(
|
|
when_to_apply="before any git push",
|
|
updated_at=datetime(2026, 6, 1, 14, 30, tzinfo=timezone.utc),
|
|
))
|
|
assert out["when_to_apply"] == "before any git push"
|
|
assert out["tier"] == "always_on"
|
|
# A DATE, not a stamp: the question is "how old is this", and a full ISO
|
|
# string across the always-on set is ~2k characters of payload.
|
|
assert out["updated_at"] == "2026-06-01"
|
|
# The depth stays with get_rule — putting it in every listing is the bloat
|
|
# this milestone is about.
|
|
assert "why" not in out and "how_to_apply" not in out
|
|
|
|
|
|
def test_rule_brief_omits_keys_a_rule_has_no_value_for():
|
|
"""#2483: a null key reads as a capability the record has and isn't using,
|
|
which is a different claim from not having one."""
|
|
from scribe.services.rulebooks import rule_brief
|
|
|
|
out = rule_brief(fake_rule())
|
|
assert "when_to_apply" not in out
|
|
assert "arose_from_id" not in out
|
|
|
|
|
|
def test_an_unknown_tier_falls_back_to_binding():
|
|
"""The asymmetry that decides the direction: a rule that preloads when it
|
|
needn't costs context; a rule that quietly stops preloading costs the
|
|
behaviour it was written for. So a typo binds."""
|
|
from scribe.services.rulebooks import _valid_tier
|
|
|
|
assert _valid_tier("conditional") == "conditional"
|
|
assert _valid_tier("always_on") == "always_on"
|
|
assert _valid_tier("Conditional") == "always_on"
|
|
assert _valid_tier("") == "always_on"
|
|
assert _valid_tier("occasionally") == "always_on"
|