feat(rules): project rule + topic suppressions
Lets a project mute individual rules or whole topics from rulebooks it subscribes to, without unsubscribing the rulebook. Two new association tables (migration 0060), 4 MCP tools (suppress/unsuppress × rule/topic), 4 REST endpoints, and an inline "× skip" affordance plus collapsed "Suppressed (N)" section in the project's Rules tab. get_applicable_rules now emits suppressed_rules and suppressed_topics (detail objects with rulebook/topic context, not just IDs) so the UI can render the suppressed list without a follow-up lookup. The main rules projection grew topic_id and rulebook_id columns for the per-row suppress affordance. Project deletion cascades the suppression rows via hard DELETE — they are pure associations with no soft-delete column, and restoring a deleted project should start fresh, not inherit stale mutes. Project-scoped rules (Rule.project_id) are deliberately not suppressible — delete them with delete_rule instead. Implements plan-task #187. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -179,13 +179,17 @@ def test_register_attaches_all_sixteen_tools():
|
||||
return decorator
|
||||
|
||||
register(FakeMCP())
|
||||
assert len(registered) == 18
|
||||
assert len(registered) == 22
|
||||
# spot-check a few names
|
||||
assert "list_rulebooks" in registered
|
||||
assert "create_rule" in registered
|
||||
assert "subscribe_project_to_rulebook" in registered
|
||||
assert "list_always_on_rules" in registered
|
||||
assert "create_project_rule" in registered
|
||||
assert "suppress_rule_for_project" in registered
|
||||
assert "unsuppress_rule_for_project" in registered
|
||||
assert "suppress_topic_for_project" in registered
|
||||
assert "unsuppress_topic_for_project" in registered
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -284,3 +288,47 @@ async def test_create_project_rule_uses_explicit_title_when_given():
|
||||
)
|
||||
kwargs = mock.call_args.kwargs
|
||||
assert kwargs["title"] == "no auto-docstrings"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_suppress_rule_for_project_passes_through():
|
||||
mock = AsyncMock(return_value=None)
|
||||
with patch("fabledassistant.mcp.tools.rulebooks.rulebooks_svc.suppress_rule_for_project", mock):
|
||||
from fabledassistant.mcp.tools.rulebooks import suppress_rule_for_project
|
||||
out = await suppress_rule_for_project(project_id=3, rule_id=17)
|
||||
kwargs = mock.call_args.kwargs
|
||||
assert kwargs == {"project_id": 3, "rule_id": 17, "user_id": 7}
|
||||
assert out == {"project_id": 3, "rule_id": 17, "suppressed": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unsuppress_rule_for_project_passes_through():
|
||||
mock = AsyncMock(return_value=None)
|
||||
with patch("fabledassistant.mcp.tools.rulebooks.rulebooks_svc.unsuppress_rule_for_project", mock):
|
||||
from fabledassistant.mcp.tools.rulebooks import unsuppress_rule_for_project
|
||||
out = await unsuppress_rule_for_project(project_id=3, rule_id=17)
|
||||
kwargs = mock.call_args.kwargs
|
||||
assert kwargs == {"project_id": 3, "rule_id": 17, "user_id": 7}
|
||||
assert out == {"project_id": 3, "rule_id": 17, "suppressed": False}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_suppress_topic_for_project_passes_through():
|
||||
mock = AsyncMock(return_value=None)
|
||||
with patch("fabledassistant.mcp.tools.rulebooks.rulebooks_svc.suppress_topic_for_project", mock):
|
||||
from fabledassistant.mcp.tools.rulebooks import suppress_topic_for_project
|
||||
out = await suppress_topic_for_project(project_id=3, topic_id=22)
|
||||
kwargs = mock.call_args.kwargs
|
||||
assert kwargs == {"project_id": 3, "topic_id": 22, "user_id": 7}
|
||||
assert out == {"project_id": 3, "topic_id": 22, "suppressed": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unsuppress_topic_for_project_passes_through():
|
||||
mock = AsyncMock(return_value=None)
|
||||
with patch("fabledassistant.mcp.tools.rulebooks.rulebooks_svc.unsuppress_topic_for_project", mock):
|
||||
from fabledassistant.mcp.tools.rulebooks import unsuppress_topic_for_project
|
||||
out = await unsuppress_topic_for_project(project_id=3, topic_id=22)
|
||||
kwargs = mock.call_args.kwargs
|
||||
assert kwargs == {"project_id": 3, "topic_id": 22, "user_id": 7}
|
||||
assert out == {"project_id": 3, "topic_id": 22, "suppressed": False}
|
||||
|
||||
@@ -48,6 +48,8 @@ def test_service_signatures_require_user_id():
|
||||
"list_rules", "list_always_on_rules",
|
||||
"get_rule", "update_rule", "delete_rule",
|
||||
"subscribe_project", "unsubscribe_project", "get_applicable_rules",
|
||||
"suppress_rule_for_project", "unsuppress_rule_for_project",
|
||||
"suppress_topic_for_project", "unsuppress_topic_for_project",
|
||||
):
|
||||
sig = inspect.signature(getattr(svc, fn_name))
|
||||
assert "user_id" in sig.parameters, f"{fn_name} missing user_id param"
|
||||
@@ -67,6 +69,29 @@ def test_create_project_rule_route_exists():
|
||||
assert callable(getattr(rb_routes, "create_project_rule"))
|
||||
|
||||
|
||||
def test_suppression_route_handlers_exist():
|
||||
"""The 4 suppression endpoint handlers are registered as Python callables."""
|
||||
from fabledassistant.routes import rulebooks as rb_routes
|
||||
for name in (
|
||||
"suppress_project_rule", "unsuppress_project_rule",
|
||||
"suppress_project_topic", "unsuppress_project_topic",
|
||||
):
|
||||
assert callable(getattr(rb_routes, name)), f"missing route handler: {name}"
|
||||
|
||||
|
||||
def test_suppression_association_tables_declared():
|
||||
"""Migration 0060 created two new association tables; the models module
|
||||
must declare matching Table() objects so the rest of the service layer
|
||||
can reference them via .c.<column>."""
|
||||
from fabledassistant.models import rulebook as rb_models
|
||||
for tbl_name in ("project_rule_suppressions", "project_topic_suppressions"):
|
||||
tbl = getattr(rb_models, tbl_name, None)
|
||||
assert tbl is not None, f"models.rulebook missing {tbl_name}"
|
||||
cols = {c.name for c in tbl.columns}
|
||||
assert "project_id" in cols
|
||||
assert "rule_id" in cols or "topic_id" in cols
|
||||
|
||||
|
||||
def test_rulebook_model_carries_always_on():
|
||||
"""Migration 0058 added rulebooks.always_on — verify the model declares it."""
|
||||
from fabledassistant.models.rulebook import Rulebook
|
||||
|
||||
@@ -241,20 +241,31 @@ async def test_subscribe_project_requires_owned_rulebook():
|
||||
)
|
||||
|
||||
|
||||
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
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_applicable_rules_returns_shape():
|
||||
"""get_applicable_rules returns {rules, project_rules, truncated, subscribed_rulebooks}."""
|
||||
"""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 = [
|
||||
(i, f"Rule {i}", f"Statement {i}", "git-workflow", "FabledSword family")
|
||||
# (rule_id, title, statement, topic_id, topic_title, rulebook_id, rulebook_title)
|
||||
(i, f"Rule {i}", f"Statement {i}", 2, "git-workflow", 1, "FabledSword family")
|
||||
for i in range(50)
|
||||
]
|
||||
proj_rules_result = MagicMock()
|
||||
proj_rules_result.all.return_value = []
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result, proj_rules_result])
|
||||
# Execute order: sub_q, suppressed_rules_q, suppressed_topics_q, rules_q, proj_rules_q
|
||||
mock_session.execute = AsyncMock(side_effect=[
|
||||
sub_result, _empty(), _empty(), rules_result, _empty(),
|
||||
])
|
||||
|
||||
with patch("fabledassistant.services.rulebooks.async_session") as mock_cls:
|
||||
mock_cls.return_value = mock_session
|
||||
@@ -263,12 +274,18 @@ async def test_get_applicable_rules_returns_shape():
|
||||
|
||||
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["truncated"] is False # exactly 50, not over
|
||||
assert result["suppressed_rules"] == []
|
||||
assert result["suppressed_topics"] == []
|
||||
assert result["truncated"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -278,13 +295,12 @@ async def test_get_applicable_rules_truncates_when_over_limit():
|
||||
sub_result = MagicMock()
|
||||
sub_result.all.return_value = []
|
||||
rules_result = MagicMock()
|
||||
# 51 rows means truncation detected
|
||||
rules_result.all.return_value = [
|
||||
(i, f"r{i}", "stmt", "topic", "rb") for i in range(51)
|
||||
(i, f"r{i}", "stmt", 2, "topic", 1, "rb") for i in range(51)
|
||||
]
|
||||
proj_rules_result = MagicMock()
|
||||
proj_rules_result.all.return_value = []
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result, proj_rules_result])
|
||||
mock_session.execute = AsyncMock(side_effect=[
|
||||
sub_result, _empty(), _empty(), rules_result, _empty(),
|
||||
])
|
||||
|
||||
with patch("fabledassistant.services.rulebooks.async_session") as mock_cls:
|
||||
mock_cls.return_value = mock_session
|
||||
@@ -299,16 +315,14 @@ async def test_get_applicable_rules_truncates_when_over_limit():
|
||||
async def test_get_applicable_rules_includes_project_scoped_rules():
|
||||
"""Project-scoped rules surface in the project_rules field."""
|
||||
mock_session = _make_mock_session()
|
||||
sub_result = MagicMock()
|
||||
sub_result.all.return_value = []
|
||||
rules_result = MagicMock()
|
||||
rules_result.all.return_value = []
|
||||
proj_rules_result = MagicMock()
|
||||
proj_rules_result.all.return_value = [
|
||||
(100, "Use alembic", "Always run migrations via alembic, never raw SQL."),
|
||||
(101, "PR-bound", "Land schema changes in their own PR."),
|
||||
]
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result, proj_rules_result])
|
||||
mock_session.execute = AsyncMock(side_effect=[
|
||||
_empty(), _empty(), _empty(), _empty(), proj_rules_result,
|
||||
])
|
||||
|
||||
with patch("fabledassistant.services.rulebooks.async_session") as mock_cls:
|
||||
mock_cls.return_value = mock_session
|
||||
@@ -318,3 +332,34 @@ async def test_get_applicable_rules_includes_project_scoped_rules():
|
||||
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=[
|
||||
_empty(), suppressed_rules_result, suppressed_topics_result, _empty(), _empty(),
|
||||
])
|
||||
|
||||
with patch("fabledassistant.services.rulebooks.async_session") as mock_cls:
|
||||
mock_cls.return_value = mock_session
|
||||
from fabledassistant.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"
|
||||
|
||||
@@ -48,18 +48,24 @@ async def test_delete_returns_none_when_not_found():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_project_cascades_to_notes_milestones_and_project_rules():
|
||||
async def test_delete_project_cascades_to_notes_milestones_project_rules_and_suppressions():
|
||||
session = _make_mock_session()
|
||||
# exists-check + 4 cascade updates (notes, milestones, project-scoped rules, project)
|
||||
# exists-check + 6 cascade ops:
|
||||
# notes (soft) → milestones (soft) → project-scoped rules (soft) →
|
||||
# project_rule_suppressions (hard DELETE) → project_topic_suppressions (hard DELETE) →
|
||||
# project (soft)
|
||||
session.execute = AsyncMock(side_effect=[
|
||||
_exists_result(True), MagicMock(), MagicMock(), MagicMock(), MagicMock(),
|
||||
_exists_result(True),
|
||||
MagicMock(), MagicMock(), MagicMock(),
|
||||
MagicMock(), MagicMock(),
|
||||
MagicMock(),
|
||||
])
|
||||
with patch("fabledassistant.services.trash.async_session") as cls:
|
||||
cls.return_value = session
|
||||
from fabledassistant.services.trash import delete
|
||||
batch = await delete(user_id=1, entity_type="project", entity_id=3)
|
||||
assert isinstance(batch, str)
|
||||
assert session.execute.await_count == 5
|
||||
assert session.execute.await_count == 7
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user