From 7861607fb842cbea9c99bdde44c3ba960a6c2c3c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 1 Jun 2026 02:26:20 -0400 Subject: [PATCH] feat(rules): project rule + topic suppressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../0060_project_rule_suppressions.py | 73 +++++++ frontend/src/api/rulebooks.ts | 34 ++++ .../src/components/rules/ProjectRulesTab.vue | 165 +++++++++++++-- src/fabledassistant/mcp/tools/projects.py | 4 + src/fabledassistant/mcp/tools/rulebooks.py | 58 ++++++ src/fabledassistant/mcp/tools/tasks.py | 2 + src/fabledassistant/models/rulebook.py | 19 ++ src/fabledassistant/routes/rulebooks.py | 48 +++++ src/fabledassistant/services/planning.py | 2 + src/fabledassistant/services/rulebooks.py | 191 +++++++++++++++++- src/fabledassistant/services/trash.py | 17 ++ tests/test_mcp_tool_rulebooks.py | 50 ++++- tests/test_routes_rulebooks.py | 25 +++ tests/test_services_rulebooks.py | 77 +++++-- tests/test_services_trash.py | 14 +- 15 files changed, 729 insertions(+), 50 deletions(-) create mode 100644 alembic/versions/0060_project_rule_suppressions.py diff --git a/alembic/versions/0060_project_rule_suppressions.py b/alembic/versions/0060_project_rule_suppressions.py new file mode 100644 index 0000000..99edc3d --- /dev/null +++ b/alembic/versions/0060_project_rule_suppressions.py @@ -0,0 +1,73 @@ +"""project rule + topic suppressions + +Revision ID: 0060 +Revises: 0059 +Create Date: 2026-06-01 + +Lets a project mute specific rules or whole topics from rulebooks it +subscribes to, without unsubscribing the rulebook. Two pure many-to-many +association tables; FKs CASCADE so removing a project / rule / topic +cleans the suppression rows automatically. +""" +from alembic import op +import sqlalchemy as sa + + +revision = "0060" +down_revision = "0059" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "project_rule_suppressions", + sa.Column( + "project_id", + sa.BigInteger(), + sa.ForeignKey("projects.id", ondelete="CASCADE"), + primary_key=True, + nullable=False, + ), + sa.Column( + "rule_id", + sa.BigInteger(), + sa.ForeignKey("rules.id", ondelete="CASCADE"), + primary_key=True, + nullable=False, + ), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + ) + op.create_table( + "project_topic_suppressions", + sa.Column( + "project_id", + sa.BigInteger(), + sa.ForeignKey("projects.id", ondelete="CASCADE"), + primary_key=True, + nullable=False, + ), + sa.Column( + "topic_id", + sa.BigInteger(), + sa.ForeignKey("rulebook_topics.id", ondelete="CASCADE"), + primary_key=True, + nullable=False, + ), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + ) + + +def downgrade() -> None: + op.drop_table("project_topic_suppressions") + op.drop_table("project_rule_suppressions") diff --git a/frontend/src/api/rulebooks.ts b/frontend/src/api/rulebooks.ts index d048290..858d854 100644 --- a/frontend/src/api/rulebooks.ts +++ b/frontend/src/api/rulebooks.ts @@ -45,7 +45,9 @@ export interface ApplicableRules { id: number; title: string; statement: string; + topic_id: number; topic_title: string; + rulebook_id: number; rulebook_title: string; }[]; project_rules: { @@ -53,6 +55,20 @@ export interface ApplicableRules { title: string; statement: string; }[]; + suppressed_rules: { + id: number; + title: string; + topic_id: number; + topic_title: string; + rulebook_id: number; + rulebook_title: string; + }[]; + suppressed_topics: { + id: number; + title: string; + rulebook_id: number; + rulebook_title: string; + }[]; truncated: boolean; subscribed_rulebooks: { id: number; title: string }[]; } @@ -147,3 +163,21 @@ export async function createProjectRule( ): Promise { return apiPost(`/api/projects/${projectId}/rules`, data); } + +// ── Suppressions ─────────────────────────────────────────────────── + +export async function suppressRuleForProject(projectId: number, ruleId: number): Promise { + await apiPost(`/api/projects/${projectId}/suppressions/rules/${ruleId}`, {}); +} + +export async function unsuppressRuleForProject(projectId: number, ruleId: number): Promise { + return apiDelete(`/api/projects/${projectId}/suppressions/rules/${ruleId}`); +} + +export async function suppressTopicForProject(projectId: number, topicId: number): Promise { + await apiPost(`/api/projects/${projectId}/suppressions/topics/${topicId}`, {}); +} + +export async function unsuppressTopicForProject(projectId: number, topicId: number): Promise { + return apiDelete(`/api/projects/${projectId}/suppressions/topics/${topicId}`); +} diff --git a/frontend/src/components/rules/ProjectRulesTab.vue b/frontend/src/components/rules/ProjectRulesTab.vue index 65e4f24..424b390 100644 --- a/frontend/src/components/rules/ProjectRulesTab.vue +++ b/frontend/src/components/rules/ProjectRulesTab.vue @@ -4,6 +4,8 @@ import { useRouter } from "vue-router"; import { getProjectApplicableRules, subscribeProject, unsubscribeProject, listRulebooks, getRule, createProjectRule, deleteRule, + suppressRuleForProject, unsuppressRuleForProject, + suppressTopicForProject, unsuppressTopicForProject, } from "@/api/rulebooks"; import type { ApplicableRules, Rulebook } from "@/api/rulebooks"; @@ -62,18 +64,32 @@ function openInRulesView(rulebookId: number, ruleId?: number) { router.push({ path: "/rules", query }); } -function groupByRulebookAndTopic(rules: ApplicableRules["rules"]) { - const grouped: Record> = {}; - for (const r of rules) { - if (!grouped[r.rulebook_title]) grouped[r.rulebook_title] = {}; - if (!grouped[r.rulebook_title][r.topic_title]) grouped[r.rulebook_title][r.topic_title] = []; - grouped[r.rulebook_title][r.topic_title].push(r); - } - return grouped; +interface TopicGroup { + topic_id: number; + topic_title: string; + rules: ApplicableRules["rules"]; } - -function rulebookIdForTitle(title: string): number | undefined { - return applicable.value?.subscribed_rulebooks.find((rb) => rb.title === title)?.id; +interface RulebookGroup { + rulebook_id: number; + rulebook_title: string; + topics: TopicGroup[]; +} +function groupByRulebookAndTopic(rules: ApplicableRules["rules"]): RulebookGroup[] { + const byRulebook = new Map(); + for (const r of rules) { + let rb = byRulebook.get(r.rulebook_id); + if (!rb) { + rb = { rulebook_id: r.rulebook_id, rulebook_title: r.rulebook_title, topics: [] }; + byRulebook.set(r.rulebook_id, rb); + } + let topic = rb.topics.find((t) => t.topic_id === r.topic_id); + if (!topic) { + topic = { topic_id: r.topic_id, topic_title: r.topic_title, rules: [] }; + rb.topics.push(topic); + } + topic.rules.push(r); + } + return Array.from(byRulebook.values()); } async function submitProjectRule() { @@ -96,6 +112,28 @@ async function removeProjectRule(ruleId: number) { await load(); } +const showSuppressed = ref(false); + +async function suppressRule(ruleId: number) { + await suppressRuleForProject(props.projectId, ruleId); + await load(); +} + +async function unsuppressRule(ruleId: number) { + await unsuppressRuleForProject(props.projectId, ruleId); + await load(); +} + +async function suppressTopic(topicId: number) { + await suppressTopicForProject(props.projectId, topicId); + await load(); +} + +async function unsuppressTopic(topicId: number) { + await unsuppressTopicForProject(props.projectId, topicId); + await load(); +} + onMounted(async () => { await load(); await loadAllRulebooks(); @@ -204,18 +242,32 @@ watch(() => props.projectId, load); Rulebooks.

-

{{ rbTitle }}

-
-
{{ topicTitle }}
+

{{ rb.rulebook_title }}

+
+
+ {{ topic.topic_title }} + +
    -
  • -
    - {{ r.title }} - {{ r.statement }} +
  • +
    +
    + {{ r.title }} + {{ r.statement }} +
    +
    @@ -226,7 +278,7 @@ watch(() => props.projectId, load);
    @@ -239,6 +291,32 @@ watch(() => props.projectId, load); Truncated at 50 rules — there are more applicable rules.

    + +
    + +
    +
      +
    • + topic + {{ t.rulebook_title }} → {{ t.title }} + +
      • +
    +
      +
    • + rule + {{ r.rulebook_title }} → {{ r.topic_title }} → {{ r.title }} + +
      • +
    +
    +
    @@ -312,4 +390,49 @@ ul { list-style: none; padding: 0; margin: 0; } background: none; border: none; cursor: pointer; color: var(--color-destructive, #b85a4a); padding: 0.5rem 0 0 0; } +/* Per-rule / per-topic suppress affordance — quiet by default, reveal on hover */ +.topic-group h5 { + display: flex; justify-content: space-between; align-items: center; gap: 0.5rem; +} +.rule-head { + display: flex; justify-content: space-between; align-items: flex-start; gap: 0.5rem; +} +.rule-head-text { flex: 1; cursor: pointer; } +.skip-btn { + background: none; border: none; cursor: pointer; + color: var(--color-muted, #888); font-size: 0.75rem; + padding: 0.1rem 0.4rem; opacity: 0; transition: opacity 0.15s; + white-space: nowrap; +} +.topic-group h5:hover .skip-btn, +.rule:hover .skip-btn, +.skip-btn:focus { opacity: 1; } +.skip-btn:hover { color: var(--color-destructive, #b85a4a); } +/* Suppressed section */ +.suppressed { margin-top: 1.5rem; } +.suppressed-toggle { + display: flex; align-items: center; gap: 0.4rem; + background: none; border: none; cursor: pointer; + font-size: 0.85rem; opacity: 0.7; padding: 0.25rem 0; color: inherit; +} +.suppressed-toggle:hover { opacity: 1; } +.suppressed-toggle .caret { font-size: 0.7em; } +.suppressed-body { margin-top: 0.5rem; } +.suppressed-list { padding-left: 0; } +.suppressed-list li { + display: flex; align-items: center; gap: 0.5rem; + padding: 0.25rem 0; opacity: 0.75; +} +.suppressed-kind { + font-size: 0.7em; text-transform: uppercase; letter-spacing: 0.05em; + padding: 0.1rem 0.4rem; border-radius: 3px; + background: var(--color-bg, #111113); + border: 1px solid var(--color-border, #2a2a2e); +} +.suppressed-path { flex: 1; } +.reenable-btn { + background: none; border: none; cursor: pointer; + color: var(--color-primary, #6366f1); font-size: 0.85em; +} +.reenable-btn:hover { text-decoration: underline; } diff --git a/src/fabledassistant/mcp/tools/projects.py b/src/fabledassistant/mcp/tools/projects.py index d31c47c..422a11a 100644 --- a/src/fabledassistant/mcp/tools/projects.py +++ b/src/fabledassistant/mcp/tools/projects.py @@ -80,6 +80,8 @@ async def enter_project(project_id: int) -> dict: "milestone_summary": milestone_summary, "applicable_rules": applicable["rules"], "project_rules": applicable.get("project_rules", []), + "suppressed_rules": applicable.get("suppressed_rules", []), + "suppressed_topics": applicable.get("suppressed_topics", []), "subscribed_rulebooks": applicable["subscribed_rulebooks"], "applicable_rules_truncated": applicable["truncated"], "open_tasks": [ @@ -122,6 +124,8 @@ async def get_project(project_id: int) -> dict: data["applicable_rules_truncated"] = applicable["truncated"] data["subscribed_rulebooks"] = applicable["subscribed_rulebooks"] data["project_rules"] = applicable.get("project_rules", []) + data["suppressed_rules"] = applicable.get("suppressed_rules", []) + data["suppressed_topics"] = applicable.get("suppressed_topics", []) return data diff --git a/src/fabledassistant/mcp/tools/rulebooks.py b/src/fabledassistant/mcp/tools/rulebooks.py index 809fc46..fabb55e 100644 --- a/src/fabledassistant/mcp/tools/rulebooks.py +++ b/src/fabledassistant/mcp/tools/rulebooks.py @@ -364,6 +364,62 @@ async def unsubscribe_project_from_rulebook( return {"project_id": project_id, "rulebook_id": rulebook_id, "subscribed": False} +# ── Suppressions — project-level mute of rulebook rules / topics ──────── + +async def suppress_rule_for_project( + project_id: int, rule_id: int, +) -> dict: + """Mute a single rulebook rule for one project. + + The rule stays in its rulebook for other projects; only this project + skips it. Idempotent. Use unsuppress_rule_for_project to re-enable. + Project-scoped rules (create_project_rule) are NOT suppressible — delete + them with delete_rule instead. + """ + uid = current_user_id() + await rulebooks_svc.suppress_rule_for_project( + project_id=project_id, rule_id=rule_id, user_id=uid, + ) + return {"project_id": project_id, "rule_id": rule_id, "suppressed": True} + + +async def unsuppress_rule_for_project( + project_id: int, rule_id: int, +) -> dict: + """Re-enable a previously-suppressed rule for one project. Idempotent.""" + uid = current_user_id() + await rulebooks_svc.unsuppress_rule_for_project( + project_id=project_id, rule_id=rule_id, user_id=uid, + ) + return {"project_id": project_id, "rule_id": rule_id, "suppressed": False} + + +async def suppress_topic_for_project( + project_id: int, topic_id: int, +) -> dict: + """Mute every rule under a topic for one project. + + Equivalent to suppressing each rule in the topic individually, but + auto-includes new rules added to the topic later. Idempotent. + """ + uid = current_user_id() + await rulebooks_svc.suppress_topic_for_project( + project_id=project_id, topic_id=topic_id, user_id=uid, + ) + return {"project_id": project_id, "topic_id": topic_id, "suppressed": True} + + +async def unsuppress_topic_for_project( + project_id: int, topic_id: int, +) -> dict: + """Re-enable a previously-suppressed topic for one project. Idempotent.""" + uid = current_user_id() + await rulebooks_svc.unsuppress_topic_for_project( + project_id=project_id, topic_id=topic_id, user_id=uid, + ) + return {"project_id": project_id, "topic_id": topic_id, "suppressed": False} + + def register(mcp) -> None: for fn in ( list_rulebooks, get_rulebook, create_rulebook, update_rulebook, delete_rulebook, @@ -371,5 +427,7 @@ def register(mcp) -> None: list_rules, list_always_on_rules, get_rule, create_rule, create_project_rule, update_rule, delete_rule, subscribe_project_to_rulebook, unsubscribe_project_from_rulebook, + suppress_rule_for_project, unsuppress_rule_for_project, + suppress_topic_for_project, unsuppress_topic_for_project, ): mcp.tool(name=fn.__name__)(fn) diff --git a/src/fabledassistant/mcp/tools/tasks.py b/src/fabledassistant/mcp/tools/tasks.py index ebe603f..056c769 100644 --- a/src/fabledassistant/mcp/tools/tasks.py +++ b/src/fabledassistant/mcp/tools/tasks.py @@ -83,6 +83,8 @@ async def get_task(task_id: int) -> dict: data["subscribed_rulebooks"] = applicable["subscribed_rulebooks"] data["applicable_rules_truncated"] = applicable["truncated"] data["project_rules"] = applicable.get("project_rules", []) + data["suppressed_rules"] = applicable.get("suppressed_rules", []) + data["suppressed_topics"] = applicable.get("suppressed_topics", []) return data diff --git a/src/fabledassistant/models/rulebook.py b/src/fabledassistant/models/rulebook.py index e382bb5..e65f9b9 100644 --- a/src/fabledassistant/models/rulebook.py +++ b/src/fabledassistant/models/rulebook.py @@ -131,3 +131,22 @@ project_rulebook_subscriptions = Table( Column("rulebook_id", BigInteger, ForeignKey("rulebooks.id", ondelete="CASCADE"), primary_key=True), Column("created_at", DateTime(timezone=True), default=lambda: datetime.now(timezone.utc)), ) + +# Suppressions — let a project mute individual rules or whole topics from +# rulebooks it subscribes to, without unsubscribing the rulebook itself. +# FKs CASCADE so the row vanishes when its parent is removed. +project_rule_suppressions = Table( + "project_rule_suppressions", + Base.metadata, + Column("project_id", BigInteger, ForeignKey("projects.id", ondelete="CASCADE"), primary_key=True), + Column("rule_id", BigInteger, ForeignKey("rules.id", ondelete="CASCADE"), primary_key=True), + Column("created_at", DateTime(timezone=True), default=lambda: datetime.now(timezone.utc)), +) + +project_topic_suppressions = Table( + "project_topic_suppressions", + Base.metadata, + Column("project_id", BigInteger, ForeignKey("projects.id", ondelete="CASCADE"), primary_key=True), + Column("topic_id", BigInteger, ForeignKey("rulebook_topics.id", ondelete="CASCADE"), primary_key=True), + Column("created_at", DateTime(timezone=True), default=lambda: datetime.now(timezone.utc)), +) diff --git a/src/fabledassistant/routes/rulebooks.py b/src/fabledassistant/routes/rulebooks.py index 8404072..4f63d76 100644 --- a/src/fabledassistant/routes/rulebooks.py +++ b/src/fabledassistant/routes/rulebooks.py @@ -241,6 +241,54 @@ async def get_project_rules(project_id: int): return jsonify(result) +@rulebooks_bp.post("/projects//suppressions/rules/") +@login_required +async def suppress_project_rule(project_id: int, rule_id: int): + try: + await rulebooks_svc.suppress_rule_for_project( + project_id=project_id, rule_id=rule_id, user_id=_uid(), + ) + except ValueError as exc: + return jsonify({"error": str(exc)}), 404 + return "", 204 + + +@rulebooks_bp.delete("/projects//suppressions/rules/") +@login_required +async def unsuppress_project_rule(project_id: int, rule_id: int): + try: + await rulebooks_svc.unsuppress_rule_for_project( + project_id=project_id, rule_id=rule_id, user_id=_uid(), + ) + except ValueError as exc: + return jsonify({"error": str(exc)}), 404 + return "", 204 + + +@rulebooks_bp.post("/projects//suppressions/topics/") +@login_required +async def suppress_project_topic(project_id: int, topic_id: int): + try: + await rulebooks_svc.suppress_topic_for_project( + project_id=project_id, topic_id=topic_id, user_id=_uid(), + ) + except ValueError as exc: + return jsonify({"error": str(exc)}), 404 + return "", 204 + + +@rulebooks_bp.delete("/projects//suppressions/topics/") +@login_required +async def unsuppress_project_topic(project_id: int, topic_id: int): + try: + await rulebooks_svc.unsuppress_topic_for_project( + project_id=project_id, topic_id=topic_id, user_id=_uid(), + ) + except ValueError as exc: + return jsonify({"error": str(exc)}), 404 + return "", 204 + + @rulebooks_bp.post("/projects//rules") @login_required async def create_project_rule(project_id: int): diff --git a/src/fabledassistant/services/planning.py b/src/fabledassistant/services/planning.py index 0385a42..3fa2f08 100644 --- a/src/fabledassistant/services/planning.py +++ b/src/fabledassistant/services/planning.py @@ -59,6 +59,8 @@ async def start_planning(user_id: int, project_id: int, title: str) -> dict: "subscribed_rulebooks": applicable["subscribed_rulebooks"], "applicable_rules_truncated": applicable["truncated"], "project_rules": applicable.get("project_rules", []), + "suppressed_rules": applicable.get("suppressed_rules", []), + "suppressed_topics": applicable.get("suppressed_topics", []), "project_goal": getattr(project, "goal", "") or "", "open_task_count": open_count, } diff --git a/src/fabledassistant/services/rulebooks.py b/src/fabledassistant/services/rulebooks.py index 8ea04e2..658f6f5 100644 --- a/src/fabledassistant/services/rulebooks.py +++ b/src/fabledassistant/services/rulebooks.py @@ -256,6 +256,30 @@ async def _assert_project_owned(session, project_id: int, user_id: int) -> None: raise ValueError(f"project {project_id} not found") +async def _assert_rulebook_rule_owned(session, rule_id: int, user_id: int) -> None: + """Raise ValueError if rule isn't a rulebook rule the user owns. + + Project-scoped rules (Rule.project_id set, topic_id NULL) are NOT + suppressible — they belong to the project; delete them instead. This + helper deliberately excludes them. + """ + from fabledassistant.models.rulebook import Rule + result = await session.execute( + select(Rule) + .join(RulebookTopic, Rule.topic_id == RulebookTopic.id) + .join(Rulebook, RulebookTopic.rulebook_id == Rulebook.id) + .where( + Rule.id == rule_id, + Rulebook.owner_user_id == user_id, + Rule.deleted_at.is_(None), + RulebookTopic.deleted_at.is_(None), + Rulebook.deleted_at.is_(None), + ) + ) + if result.scalar_one_or_none() is None: + raise ValueError(f"rule {rule_id} not found or not a rulebook rule") + + async def create_rule( topic_id: int, user_id: int, title: str, statement: str, why: str = "", how_to_apply: str = "", order_index: int = 0, @@ -503,24 +527,115 @@ async def unsubscribe_project( await session.commit() +# ── Suppressions — project-level mute of rulebook rules / topics ──────── + +async def suppress_rule_for_project( + project_id: int, rule_id: int, user_id: int, +) -> None: + """Mute one rulebook rule for one project. Idempotent.""" + from fabledassistant.models.rulebook import project_rule_suppressions + + async with async_session() as session: + await _assert_project_owned(session, project_id, user_id) + await _assert_rulebook_rule_owned(session, rule_id, user_id) + try: + await session.execute( + insert(project_rule_suppressions).values( + project_id=project_id, rule_id=rule_id, + ) + ) + await session.commit() + except Exception: + await session.rollback() # PK collision = already suppressed; fine. + + +async def unsuppress_rule_for_project( + project_id: int, rule_id: int, user_id: int, +) -> None: + """Unmute one rulebook rule for one project. Idempotent.""" + from fabledassistant.models.rulebook import project_rule_suppressions + + async with async_session() as session: + await _assert_project_owned(session, project_id, user_id) + await session.execute( + sql_delete(project_rule_suppressions).where( + project_rule_suppressions.c.project_id == project_id, + project_rule_suppressions.c.rule_id == rule_id, + ) + ) + await session.commit() + + +async def suppress_topic_for_project( + project_id: int, topic_id: int, user_id: int, +) -> None: + """Mute every rule under one topic for one project. Idempotent.""" + from fabledassistant.models.rulebook import project_topic_suppressions + + async with async_session() as session: + await _assert_project_owned(session, project_id, user_id) + await _assert_topic_owned(session, topic_id, user_id) + try: + await session.execute( + insert(project_topic_suppressions).values( + project_id=project_id, topic_id=topic_id, + ) + ) + await session.commit() + except Exception: + await session.rollback() + + +async def unsuppress_topic_for_project( + project_id: int, topic_id: int, user_id: int, +) -> None: + """Unmute a topic for one project. Idempotent.""" + from fabledassistant.models.rulebook import project_topic_suppressions + + async with async_session() as session: + await _assert_project_owned(session, project_id, user_id) + await session.execute( + sql_delete(project_topic_suppressions).where( + project_topic_suppressions.c.project_id == project_id, + project_topic_suppressions.c.topic_id == topic_id, + ) + ) + await session.commit() + + async def get_applicable_rules( project_id: int, user_id: int, limit: int = 50, ) -> dict: """Return rules applicable to a project — both via rulebook subscriptions - and project-scoped rules (Rule.project_id matches). + and project-scoped rules (Rule.project_id matches), with suppressed rules + and suppressed topics filtered out. Shape: { - "rules": [{id, title, statement, topic_title, rulebook_title}, ...], + "rules": [{id, title, statement, + topic_id, topic_title, + rulebook_id, rulebook_title}, ...], "project_rules": [{id, title, statement}, ...], + "suppressed_rules": [{id, title, + topic_id, topic_title, + rulebook_id, rulebook_title}, ...], + "suppressed_topics": [{id, title, + rulebook_id, rulebook_title}, ...], "truncated": bool, "subscribed_rulebooks": [{id, title}, ...] } - `rules` is the subscription-derived set (legacy shape preserved). - `project_rules` is the project-scoped set; empty list when none exist. + `rules` is the subscription-derived set with project-level suppressions + applied. `project_rules` is the project-scoped set (never suppressed — + delete instead). `suppressed_rules` / `suppressed_topics` carry the + titles + rulebook context callers need to display what was filtered + without round-tripping for names. """ - from fabledassistant.models.rulebook import project_rulebook_subscriptions + from fabledassistant.models.rulebook import ( + project_rulebook_subscriptions, + project_rule_suppressions, + project_topic_suppressions, + ) async with async_session() as session: # Subscribed rulebooks for the project (ownership-scoped). @@ -542,11 +657,64 @@ async def get_applicable_rules( {"id": rb_id, "title": rb_title} for rb_id, rb_title in sub_rows ] - # Applicable rules (limit + 1 so we can detect truncation). + # Suppressed rules — joined to topic + rulebook so callers can render + # context without a follow-up lookup. Ownership-scoped via rulebook. + suppressed_rules_q = ( + select( + Rule.id, Rule.title, + RulebookTopic.id.label("topic_id"), + RulebookTopic.title.label("topic_title"), + Rulebook.id.label("rulebook_id"), + Rulebook.title.label("rulebook_title"), + ) + .join(project_rule_suppressions, project_rule_suppressions.c.rule_id == Rule.id) + .join(RulebookTopic, Rule.topic_id == RulebookTopic.id) + .join(Rulebook, RulebookTopic.rulebook_id == Rulebook.id) + .where( + project_rule_suppressions.c.project_id == project_id, + Rulebook.owner_user_id == user_id, + ) + .order_by(Rulebook.title, RulebookTopic.title, Rule.title) + ) + suppressed_rule_rows = (await session.execute(suppressed_rules_q)).all() + suppressed_rules = [ + {"id": rid, "title": rt, "topic_id": ti, "topic_title": tt, + "rulebook_id": rbi, "rulebook_title": rbt} + for rid, rt, ti, tt, rbi, rbt in suppressed_rule_rows + ] + suppressed_rule_ids = [r["id"] for r in suppressed_rules] + + # Suppressed topics — joined to rulebook for context. + suppressed_topics_q = ( + select( + RulebookTopic.id, RulebookTopic.title, + Rulebook.id.label("rulebook_id"), + Rulebook.title.label("rulebook_title"), + ) + .join(project_topic_suppressions, project_topic_suppressions.c.topic_id == RulebookTopic.id) + .join(Rulebook, RulebookTopic.rulebook_id == Rulebook.id) + .where( + project_topic_suppressions.c.project_id == project_id, + Rulebook.owner_user_id == user_id, + ) + .order_by(Rulebook.title, RulebookTopic.title) + ) + suppressed_topic_rows = (await session.execute(suppressed_topics_q)).all() + suppressed_topics = [ + {"id": tid, "title": tt, "rulebook_id": rbi, "rulebook_title": rbt} + for tid, tt, rbi, rbt in suppressed_topic_rows + ] + suppressed_topic_ids = [t["id"] for t in suppressed_topics] + + # Applicable rules (limit + 1 so we can detect truncation). Filter + # in SQL so truncation reflects the post-suppression count, not the + # raw subscription count. rules_q = ( select( Rule.id, Rule.title, Rule.statement, + RulebookTopic.id.label("topic_id"), RulebookTopic.title.label("topic_title"), + Rulebook.id.label("rulebook_id"), Rulebook.title.label("rulebook_title"), ) .join(RulebookTopic, Rule.topic_id == RulebookTopic.id) @@ -567,14 +735,19 @@ async def get_applicable_rules( ) .limit(limit + 1) ) + if suppressed_rule_ids: + rules_q = rules_q.where(Rule.id.notin_(suppressed_rule_ids)) + if suppressed_topic_ids: + rules_q = rules_q.where(Rule.topic_id.notin_(suppressed_topic_ids)) rule_rows = (await session.execute(rules_q)).all() truncated = len(rule_rows) > limit rules = [ { "id": rid, "title": rtitle, "statement": stmt, - "topic_title": tt, "rulebook_title": rbt, + "topic_id": ti, "topic_title": tt, + "rulebook_id": rbi, "rulebook_title": rbt, } - for rid, rtitle, stmt, tt, rbt in rule_rows[:limit] + for rid, rtitle, stmt, ti, tt, rbi, rbt in rule_rows[:limit] ] # Project-scoped rules — verifies ownership via Project.user_id. @@ -599,6 +772,8 @@ async def get_applicable_rules( return { "rules": rules, "project_rules": project_rules, + "suppressed_rules": suppressed_rules, + "suppressed_topics": suppressed_topics, "truncated": truncated, "subscribed_rulebooks": subscribed_rulebooks, } diff --git a/src/fabledassistant/services/trash.py b/src/fabledassistant/services/trash.py index 30a9072..92ab824 100644 --- a/src/fabledassistant/services/trash.py +++ b/src/fabledassistant/services/trash.py @@ -55,6 +55,23 @@ async def _cascade(session, user_id: int, etype: str, eid: int, batch: str, now) await _set(session, Milestone, [Milestone.user_id == user_id, Milestone.project_id == eid], batch, now) # Project-scoped rules cascade with the project they're attached to. await _set(session, Rule, [Rule.project_id == eid], batch, now) + # Suppressions are pure associations (no deleted_at) — hard-delete + # them here so restoring the project doesn't bring stale mutes back. + # FK CASCADE would handle a full DELETE on the project row, but the + # soft-delete path keeps the project row alive; this guarantees the + # rows are gone whether or not the project ever gets purged. + from sqlalchemy import delete as _sql_delete + from fabledassistant.models.rulebook import ( + project_rule_suppressions, project_topic_suppressions, + ) + await session.execute( + _sql_delete(project_rule_suppressions) + .where(project_rule_suppressions.c.project_id == eid) + ) + await session.execute( + _sql_delete(project_topic_suppressions) + .where(project_topic_suppressions.c.project_id == eid) + ) await _set(session, Project, [Project.user_id == user_id, Project.id == eid], batch, now) elif etype == "milestone": await _set(session, Note, [Note.user_id == user_id, Note.milestone_id == eid], batch, now) diff --git a/tests/test_mcp_tool_rulebooks.py b/tests/test_mcp_tool_rulebooks.py index f2b3495..7c480e2 100644 --- a/tests/test_mcp_tool_rulebooks.py +++ b/tests/test_mcp_tool_rulebooks.py @@ -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} diff --git a/tests/test_routes_rulebooks.py b/tests/test_routes_rulebooks.py index 891f95d..f647b00 100644 --- a/tests/test_routes_rulebooks.py +++ b/tests/test_routes_rulebooks.py @@ -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..""" + 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 diff --git a/tests/test_services_rulebooks.py b/tests/test_services_rulebooks.py index babed27..45e5605 100644 --- a/tests/test_services_rulebooks.py +++ b/tests/test_services_rulebooks.py @@ -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" diff --git a/tests/test_services_trash.py b/tests/test_services_trash.py index 09af133..0586ec9 100644 --- a/tests/test_services_trash.py +++ b/tests/test_services_trash.py @@ -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