From f7d8dc2e55b41742c27529b1a15d479854d57d52 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 1 Oct 2026 13:25:27 -0400 Subject: [PATCH] =?UTF-8?q?feat(lessons):=20judged=20when=20written=20?= =?UTF-8?q?=E2=80=94=20a=20new=20lesson=20is=20offered=20its=20rules,=20an?= =?UTF-8?q?d=20"no=20rule=20fits"=20is=20an=20answer=20(milestone=20440=20?= =?UTF-8?q?step=202,=20#4631)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Which rule is this lesson an instance of?" now has three recorded answers: a rule named (a confirmed link, #4630), no rule fits (new), or unjudged. - Model + migration 0112: lesson_no_rule (lesson_id PK, CASCADE from the note; why; judged_at). A table rather than a key in notes.data, because that mirror is re-composed from the body on every edit and would erase it. - Service (lesson_rules): set_no_rule rejects any confirmed link with the reason; a confirmation (set_lesson_rules or judge_link) deletes the answer; require_one_answer refuses both answers in one call before any write; judgments_for_lessons + attach_lesson_rules add rule_judgment (and no_rule) to every lesson payload; list_unjudged lists the open ones; rule_candidates searches rules with the lesson's claim + trigger at the explicit-search bar, None when the search could not run. - MCP: create_lesson/update_lesson take no_rule; an unanswered create returns rule_candidates, rule_judgment and a rule_hint; list_lessons(unjudged=true). - REST: the same on POST/PATCH /api/lessons and GET ?unjudged=1; create returns rule_candidates. - Backup v19: a lesson_no_rule section, export (full and per-user) and import. - Guidance: create_lesson docstring, writing-records.md in using-scribe (owner, pinned in test_guidance_ownership), create_rule docstring on linking the lessons a new rule governs. Plugin version minted. - Tests: door units, integration for the three states, the rejection reason, scoping, cascade; backup registries. Co-Authored-By: Claude Opus 5.5 --- alembic/versions/0112_lesson_no_rule.py | 33 +++ plugin/.claude-plugin/plugin.json | 2 +- plugin/skills/using-scribe/writing-records.md | 24 ++ src/scribe/mcp/tools/lessons.py | 89 +++++++- src/scribe/mcp/tools/rulebooks.py | 8 + src/scribe/models/__init__.py | 2 +- src/scribe/models/lesson_rule_link.py | 37 ++- src/scribe/routes/lessons.py | 53 +++-- src/scribe/services/backup.py | 48 +++- src/scribe/services/lesson_rules.py | 210 +++++++++++++++++- tests/conftest.py | 7 +- tests/test_guidance_ownership.py | 9 + tests/test_integration_lesson_rule_links.py | 81 +++++++ tests/test_lesson_rule_link_doors.py | 120 ++++++++++ tests/test_services_backup.py | 10 +- 15 files changed, 696 insertions(+), 37 deletions(-) create mode 100644 alembic/versions/0112_lesson_no_rule.py diff --git a/alembic/versions/0112_lesson_no_rule.py b/alembic/versions/0112_lesson_no_rule.py new file mode 100644 index 0000000..da806af --- /dev/null +++ b/alembic/versions/0112_lesson_no_rule.py @@ -0,0 +1,33 @@ +"""lesson_no_rule — "no rule fits" is a recorded answer (milestone 440 step 2, +#4631) + +Revision ID: 0112 +Revises: 0111 +Create Date: 2026-10-01 + +One row per lesson that was judged to fall under no rule, with the reason. A +lesson with neither a confirmed link nor this row is UNJUDGED, which is what +`list_lessons(unjudged=true)` lists. CASCADE from the lesson. No backfill: no +lesson has been judged yet, and writing "no rule fits" for one nobody looked +at would assert a judgment nobody made. +""" +import sqlalchemy as sa +from alembic import op + +revision = "0112" +down_revision = "0111" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "lesson_no_rule", + sa.Column("lesson_id", sa.Integer(), sa.ForeignKey("notes.id", ondelete="CASCADE"), primary_key=True), + sa.Column("why", sa.Text(), nullable=False), + sa.Column("judged_at", sa.DateTime(timezone=True), nullable=False, server_default=sa.text("now()")), + ) + + +def downgrade() -> None: + op.drop_table("lesson_no_rule") diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 45eb2b3..a6ac874 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).", - "version": "2026.10.01.1246", + "version": "2026.10.01.1724", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/skills/using-scribe/writing-records.md b/plugin/skills/using-scribe/writing-records.md index 9876571..0c86018 100644 --- a/plugin/skills/using-scribe/writing-records.md +++ b/plugin/skills/using-scribe/writing-records.md @@ -8,6 +8,7 @@ arrives that names the situation you are actually in; and before filling ## Contents - Where a new rule goes — its home, its trigger, what already covers the moment - A lesson grows each time it proves itself +- A lesson names the rule it is an instance of - A note that asserts a fact can carry its own check ## Where a new rule goes @@ -76,6 +77,29 @@ waiting in a situation nobody is in. One claim that has met the same failure four times is worth more than four claims that each met it once, so when a near-duplicate create hands back an existing id, that is the record to grow. +## A lesson names the rule it is an instance of + +A lesson records one situation; a rule records the binding choice for a class +of them. Linking the two is what lets a rule be reached through the situations +that keep proving it — moments that resemble why it was written more closely +than its own wording does. A lesson never becomes a rule. It points at one. + +The writer is best placed to say which rule, so answer while writing, in +`create_lesson` itself or in the `update_lesson` right after it: + +- **`rule_ids=[…]`** — the rule or preference whose situation this is an + instance of. +- **`no_rule="why"`** — no rule governs it, in a line. That is a full answer, + not a gap: lessons that stand alone and keep landing in one situation are + what a missing rule looks like, and the reason is what lets the answer be + re-judged once a rule exists. + +A lesson created with neither comes back with `rule_candidates`, the rules it +most resembles, each with its trigger. Read them against the situation the +lesson describes, not its topic — a rule about the same subsystem that governs +a different moment is not its rule — and give one of the two answers. +`list_lessons(unjudged=true)` gathers the ones still open. + ## A note that asserts a fact can carry its own check **A few notes assert a FACT, and those can carry their own check.** diff --git a/src/scribe/mcp/tools/lessons.py b/src/scribe/mcp/tools/lessons.py index 9e11293..3f116eb 100644 --- a/src/scribe/mcp/tools/lessons.py +++ b/src/scribe/mcp/tools/lessons.py @@ -29,7 +29,7 @@ _to_dict = lessons_svc.lesson_to_dict async def list_lessons( q: str = "", tag: str = "", limit: int = 50, offset: int = 0, - project_id: int = 0, + project_id: int = 0, unjudged: bool = False, ) -> dict: """List lessons — the kind enumerated, rather than only what a query resembles. @@ -51,16 +51,32 @@ async def list_lessons( project_id: Narrow to where a lesson was WRITTEN. 0 = every project. A lesson is retrievable from anywhere regardless (step 3); this filters the listing, not the reach. + unjudged: List only the lessons nobody has answered "which rule is + this an instance of?" for — no rule named, and no "no rule fits" + recorded. Work down this list with `update_lesson(rule_ids=…)` or + `update_lesson(no_rule="why")`. A listing rather than a search, so + it takes `tag` and `project_id` but not `q`. Returns {"lessons": [{id, title, when_to_apply, tags, preview}], "total"}. """ uid = current_user_id() - items, total = await knowledge_svc.query_knowledge( - user_id=uid, note_type=lessons_svc.LESSON_NOTE_TYPE, - tags=[tag] if tag else [], sort="modified", q=q or None, - limit=max(1, min(limit, 100)), offset=max(0, offset), - project_id=project_id or None, - ) + if unjudged: + if q: + raise ValueError( + "unjudged lists every unjudged lesson rather than searching " + "them — call it with tag/project_id and without q." + ) + items, total = await lesson_rules_svc.list_unjudged( + uid, tag=tag, project_id=project_id or None, + limit=max(1, min(limit, 100)), offset=max(0, offset), + ) + else: + items, total = await knowledge_svc.query_knowledge( + user_id=uid, note_type=lessons_svc.LESSON_NOTE_TYPE, + tags=[tag] if tag else [], sort="modified", q=q or None, + limit=max(1, min(limit, 100)), offset=max(0, offset), + project_id=project_id or None, + ) labelled = await access_svc.label_shared_items(uid, items) # One aggregate for the page, like the snippet listing — surfaced-vs-opened # per lesson (#4196). An agent listing lessons can see which of its own @@ -91,11 +107,29 @@ async def create_lesson( project_id: int = 0, system_ids: list[int] | None = None, rule_ids: list[int] | None = None, + no_rule: str = "", force: bool = False, ) -> dict: """Record something you LEARNED, so a later session meets it at the moment it applies — on this project or any other. + A LESSON POINTS AT THE RULE IT IS AN INSTANCE OF. While you write it, you + know the situation better than anyone will again, so that is when to say + which binding choice it falls under. Answer one of two ways, in this call + or right after it with `update_lesson`: + + - `rule_ids=[…]` — the rule(s) or preference(s) it is an instance of. The + rule then becomes reachable through the situation the lesson describes, + which is closer to why it was written than its own wording often is. + - `no_rule="why"` — no rule governs this situation, in a line. That is a + real answer: a lesson that stands alone is what a rule nobody has + written yet is made from, and the reason lets it be re-judged later. + + A lesson created with neither comes back with `rule_candidates` — the + rules it most resembles, each with its trigger — and `rule_judgment: + "unjudged"`. Read them against the lesson and answer; `list_lessons( + unjudged=true)` gathers any left open. + A LESSON OR A RULE? The difference is FORCE, not importance. A rule is something that must be followed; a lesson is something worth knowing. If ignoring it would be a mistake, it is a rule (create_rule) and needs the @@ -151,11 +185,14 @@ async def create_lesson( the binding choice its situation falls under. A lesson never becomes a rule; it points at the one that governs it, and the rule is then reachable through the situation the lesson describes - (milestone 440). Naming a rule here confirms the link. Leave it - empty when no rule governs this situation. + (milestone 440). Naming a rule here confirms the link. + no_rule: The reason no rule governs this situation, in a line — the + other answer to "which rule?". Give one or the other, not both. force: Create even if a near-duplicate exists. - Returns the created lesson. On a near-duplicate, returns the existing id + Returns the created lesson with its `rules` and `rule_judgment`, plus + `rule_candidates` when it is unjudged (absent when the rule search could + not run). On a near-duplicate, returns the existing id instead of creating — two lessons about one failure class want to be one lesson, so update that one rather than adding a second. """ @@ -172,6 +209,7 @@ async def create_lesson( # Validated before anything is written, so a lesson naming a rule the # caller cannot read fails whole rather than saving half-linked. linked = await lesson_rules_svc.require_rules(uid, rule_ids) + lesson_rules_svc.require_one_answer(linked, no_rule) title, body = lessons_svc.lesson_document( what, when_to_apply, insight, sources, ) @@ -192,12 +230,34 @@ async def create_lesson( await systems_svc.set_record_systems(uid, note.id, system_ids) if linked: await lesson_rules_svc.set_lesson_rules(uid, note.id, linked) + elif no_rule.strip(): + await lesson_rules_svc.set_no_rule(uid, note.id, no_rule) data = _to_dict(note) await systems_tools.attach_systems(uid, uid, data, note.id, project_id or None) await lesson_rules_svc.attach_lesson_rules(uid, [data]) + if not linked and not no_rule.strip(): + await _offer_candidates(uid, data, what, when_to_apply, project_id) return data +async def _offer_candidates(uid: int, data: dict, what: str, trigger: str, project_id: int) -> None: + """Put the rules an unjudged lesson resembles in front of its writer. + + `rule_judgment` is set here too, because the attach above is fail-open and + may have left it off; a lesson just created with no answer IS unjudged. + """ + data["rule_judgment"] = lesson_rules_svc.UNJUDGED + candidates = await lesson_rules_svc.rule_candidates(uid, what, trigger, project_id or None) + if candidates is not None: + data["rule_candidates"] = candidates + data["rule_hint"] = ( + "Which rule is this lesson an instance of? If one of rule_candidates " + "governs its situation, update_lesson(lesson_id, rule_ids=[…]) links " + "it; if none does, update_lesson(lesson_id, no_rule=\"why\") records " + "that it stands alone." + ) + + async def get_lesson(lesson_id: int, project_id: int = 0) -> dict: """Fetch one lesson by id, with its trigger and sources read back out. @@ -245,6 +305,7 @@ async def update_lesson( tags: list[str] | None = None, system_ids: list[int] | None = None, rule_ids: list[int] | None = None, + no_rule: str = "", ) -> dict: """Update a lesson. Empty fields are left unchanged. @@ -278,12 +339,18 @@ async def update_lesson( leaves unchanged; pass the FULL list. A rule that was linked and is left out is recorded as REJECTED — "not an instance of this one" — so the pair is not proposed again; `[]` rejects them all. + Naming a rule replaces any "no rule fits" answer. + no_rule: Record that no rule governs this lesson's situation, with the + reason in a line. Any rule still linked is rejected with that + reason. Empty leaves the answer unchanged; give this or a + non-empty `rule_ids`, not both. """ uid = current_user_id() linked = ( await lesson_rules_svc.require_rules(uid, rule_ids) if rule_ids is not None else None ) + lesson_rules_svc.require_one_answer(linked, no_rule) note = await lessons_svc.update_lesson( uid, lesson_id, what=what or None, @@ -302,6 +369,8 @@ async def update_lesson( note = await lessons_svc.get_lesson(uid, lesson_id) or note if linked is not None: await lesson_rules_svc.set_lesson_rules(uid, lesson_id, linked) + if no_rule.strip(): + await lesson_rules_svc.set_no_rule(uid, lesson_id, no_rule) out = _to_dict(note) await lesson_rules_svc.attach_lesson_rules(uid, [out]) return out diff --git a/src/scribe/mcp/tools/rulebooks.py b/src/scribe/mcp/tools/rulebooks.py index 7d44473..d6b4562 100644 --- a/src/scribe/mcp/tools/rulebooks.py +++ b/src/scribe/mcp/tools/rulebooks.py @@ -393,6 +393,14 @@ async def create_rule( observations belong, and the reason the kind question is worth asking out loud rather than settled silently before the proposal. + A RULE WRITTEN FOR A SITUATION LESSONS ALREADY RECORD is linked from + them once it exists: `update_lesson(lesson_id, rule_ids=[…, ])` + for each lesson it governs. Those lessons are the concrete situations + that proved it, and linked, they are how the rule is reached in moments + closer to why it was written than its own wording. A lesson that + answered "no rule fits" before this rule existed is the first to + re-judge; `get_lesson` shows the answer and its reason. + Where the interface offers structured choices, ask it that way — a question with named options is answered in a click, while the same question inside a paragraph is answered by scrolling past. Where it does diff --git a/src/scribe/models/__init__.py b/src/scribe/models/__init__.py index 4af89b9..178fb84 100644 --- a/src/scribe/models/__init__.py +++ b/src/scribe/models/__init__.py @@ -78,7 +78,7 @@ from scribe.models.rulebook import ( # noqa: E402, F401 Rulebook, RulebookTopic, Rule, RuleRelation, rule_systems, ) # After notes and rules: it foreign-keys both (milestone 440). -from scribe.models.lesson_rule_link import LessonRuleLink # noqa: E402, F401 +from scribe.models.lesson_rule_link import LessonNoRule, LessonRuleLink # noqa: E402, F401 from scribe.models.repo_binding import RepoBinding # noqa: E402, F401 from scribe.models.forge_connection import ForgeConnection # noqa: E402, F401 from scribe.models.code_shape import CodeShape, CodeShapeConsumer, CodeShapeEvent, CodeShapeUse # noqa: E402, F401 diff --git a/src/scribe/models/lesson_rule_link.py b/src/scribe/models/lesson_rule_link.py index f9177d7..ea08374 100644 --- a/src/scribe/models/lesson_rule_link.py +++ b/src/scribe/models/lesson_rule_link.py @@ -1,6 +1,6 @@ from datetime import datetime -from sqlalchemy import BigInteger, DateTime, ForeignKey, Integer, Text, UniqueConstraint +from sqlalchemy import BigInteger, DateTime, ForeignKey, Integer, Text, UniqueConstraint, func from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import Mapped, mapped_column @@ -75,3 +75,38 @@ class LessonRuleLink(Base, CreatedAtMixin): "judged_at": iso(self.judged_at), "created_at": iso(self.created_at), } + + +class LessonNoRule(Base): + """A judgment that no rule governs a lesson's situation (#4631). + + The third answer to "which rule is this an instance of?", beside naming + one and leaving it open. It is what lets an UNJUDGED lesson be told apart + from one that was looked at and found to stand alone — and a lesson with + no rule is the raw material #4634 reads for convergence, so the + difference is the whole signal. + + A row of its own rather than a key in the lesson's `data`: that column is + a mirror re-composed from the body on every edit (services/lessons.py), + so anything kept there that the body does not render is erased by the + next rewording — and the lesson would silently read as unjudged again. + + One row per lesson. Naming a rule later deletes it: "no rule fits" and + "this rule fits" cannot both be the current answer. + """ + + __tablename__ = "lesson_no_rule" + + lesson_id: Mapped[int] = mapped_column( + Integer, ForeignKey("notes.id", ondelete="CASCADE"), primary_key=True, + ) + # Why none fits — required, because "no rule" with no reason cannot be + # re-judged when a rule is later written for the situation. + why: Mapped[str] = mapped_column(Text) + judged_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now(), + ) + + def to_dict(self) -> dict: + return {"why": self.why, "judged_at": iso(self.judged_at)} + diff --git a/src/scribe/routes/lessons.py b/src/scribe/routes/lessons.py index 0335b07..a68ced5 100644 --- a/src/scribe/routes/lessons.py +++ b/src/scribe/routes/lessons.py @@ -63,16 +63,25 @@ async def list_lessons_route(): project_id = None limit, offset = parse_pagination(default_limit=24, max_limit=100) - items, total = await knowledge_svc.query_knowledge( - user_id=uid, - note_type=lessons_svc.LESSON_NOTE_TYPE, - tags=[tag] if tag else [], - sort="modified", - q=q, - limit=limit, - offset=offset, - project_id=project_id, - ) + # The lessons nobody has answered "which rule?" for (#4631). A listing, + # so it does not combine with a search — the MCP door says the same. + if request.args.get("unjudged") in ("1", "true"): + if q: + return jsonify({"error": "unjudged is a listing; drop q"}), 400 + items, total = await lesson_rules_svc.list_unjudged( + uid, tag=tag, project_id=project_id, limit=limit, offset=offset, + ) + else: + items, total = await knowledge_svc.query_knowledge( + user_id=uid, + note_type=lessons_svc.LESSON_NOTE_TYPE, + tags=[tag] if tag else [], + sort="modified", + q=q, + limit=limit, + offset=offset, + project_id=project_id, + ) items = await label_shared_items(uid, items) # One aggregate for the whole page — a per-row lookup would be N+1 by # construction. Every row gets the key, zero-filled, so the UI renders @@ -139,8 +148,10 @@ async def create_lesson_route(): learned_from = data.get("learned_from") or [] # The rules this lesson is an instance of (milestone 440). Validated before # anything is written, as the MCP door does, so a bad id saves nothing. + no_rule = (data.get("no_rule") or "").strip() try: linked = await lesson_rules_svc.require_rules(uid, data.get("rule_ids") or []) + lesson_rules_svc.require_one_answer(linked, no_rule) except ValueError as exc: return jsonify({"error": str(exc)}), 400 @@ -174,11 +185,21 @@ async def create_lesson_route(): await systems_svc.set_record_systems(uid, note.id, data["system_ids"]) if linked: await lesson_rules_svc.set_lesson_rules(uid, note.id, linked) + elif no_rule: + await lesson_rules_svc.set_no_rule(uid, note.id, no_rule) out = lessons_svc.lesson_to_dict(note) out["systems"] = [ s.to_dict() for s in await systems_svc.list_record_systems(uid, note.id) ] await lesson_rules_svc.attach_lesson_rules(uid, [out]) + # The same offer the MCP door makes, so the form can show the rules to + # judge against while the writer still has the situation in mind. + if not linked and not no_rule: + candidates = await lesson_rules_svc.rule_candidates( + uid, what, when_to_apply, project_id, + ) + if candidates is not None: + out["rule_candidates"] = candidates return jsonify(out), 201 @@ -245,11 +266,13 @@ async def update_lesson_route(lesson_id: int): }), 400 linked = None - if data.get("rule_ids") is not None: - try: + no_rule = (data.get("no_rule") or "").strip() + try: + if data.get("rule_ids") is not None: linked = await lesson_rules_svc.require_rules(uid, data["rule_ids"]) - except ValueError as exc: - return jsonify({"error": str(exc)}), 400 + lesson_rules_svc.require_one_answer(linked, no_rule) + except ValueError as exc: + return jsonify({"error": str(exc)}), 400 updated = await lessons_svc.update_lesson(owner_uid, lesson_id, **kwargs) if updated is None: @@ -258,6 +281,8 @@ async def update_lesson_route(lesson_id: int): # The CALLER, for the reason system_ids below uses it: the rules named # must be ones the person making the edit can read. await lesson_rules_svc.set_lesson_rules(uid, lesson_id, linked) + if no_rule: + await lesson_rules_svc.set_no_rule(uid, lesson_id, no_rule) if data.get("system_ids") is not None: # The CALLER, not owner_uid (#4249). `set_record_systems` runs its own # `can_write_note` and links only Systems the acting user can read; diff --git a/src/scribe/services/backup.py b/src/scribe/services/backup.py index 335327e..c472d94 100644 --- a/src/scribe/services/backup.py +++ b/src/scribe/services/backup.py @@ -16,7 +16,7 @@ from scribe.models.rule_usage import RuleUsageEvent from scribe.models.retrieval_tuning import RetrievalTuningEvent from scribe.models.canonical_system import CanonicalSystem from scribe.models.rulebook import RuleRelation, rule_systems as rule_systems_t -from scribe.models.lesson_rule_link import LessonRuleLink +from scribe.models.lesson_rule_link import LessonNoRule, LessonRuleLink from scribe.models.code_shape import CodeShape, CodeShapeEvent, CodeShapeUse from scribe.models.project import Project from scribe.models.repo_binding import RepoBinding @@ -85,8 +85,11 @@ logger = logging.getLogger(__name__) # is an instance of, and the judgments that confirmed or rejected each pair. A # confirmed link is a judgment nothing else records, and a rejected one is # what stops the pair being proposed again — losing either undoes work. +# v19 (2026-10) added lesson_no_rule (#4631): the "no rule fits" answer and its +# reason. Without it a restored lesson that was judged to stand alone reads as +# never judged, and lands back on the unjudged list. # Bump when the serialized schema changes. -BACKUP_VERSION = 18 +BACKUP_VERSION = 19 # Every table this backup carries, by its REAL name. Paired with _NOT_INCLUDED # below, these two lists must together account for the entire schema — which is @@ -126,6 +129,8 @@ _BACKED_UP = [ "retrieval_tuning_events", # v18 (2026-10): lesson → rule links and their judgments (milestone 440). "lesson_rule_links", + # v19 (2026-10): "no rule fits" answers (#4631). + "lesson_no_rule", ] # Tables intentionally NOT in the backup, surfaced in the payload so the gap is @@ -221,6 +226,8 @@ _COLUMN_EXCLUSIONS: dict[str, set[str]] = { "rule_relations": {"id", "created_at"}, # The pair is the row; everything else is the judgment and its evidence. "lesson_rule_links": {"id"}, + # Keyed by the lesson itself; nothing to exclude, every column travels. + "lesson_no_rule": set(), "note_usage_events": {"id"}, # Same as the note twin: the surrogate key is re-issued on insert. "rule_usage_events": {"id"}, @@ -309,6 +316,7 @@ _IMPORT_COLUMN_EXCLUSIONS: dict[str, set[str]] = { "note_supersessions": {"id", "created_at"}, "rule_relations": {"id", "created_at"}, "lesson_rule_links": {"id"}, + "lesson_no_rule": set(), "note_usage_events": {"id"}, "rule_usage_events": {"id"}, "retrieval_tuning_events": {"id"}, @@ -717,6 +725,17 @@ def _lesson_rule_link_rows(rows) -> list[dict]: ] +def _lesson_no_rule_rows(rows) -> list[dict]: + """The "no rule fits" answers, by SOURCE lesson id (#4631).""" + return [ + { + "lesson_id": r.lesson_id, "why": r.why, + "judged_at": r.judged_at.isoformat() if r.judged_at else None, + } + for r in rows + ] + + def _rule_rows(rows) -> list[dict]: return [ { @@ -765,6 +784,7 @@ async def export_full_backup() -> dict: )).all() rule_relations = (await session.execute(select(RuleRelation))).scalars().all() lesson_rule_links = (await session.execute(select(LessonRuleLink))).scalars().all() + lesson_no_rule = (await session.execute(select(LessonNoRule))).scalars().all() record_systems = (await session.execute(select(RecordSystem))).scalars().all() supersessions = ( await session.execute(select(NoteSupersession)) @@ -824,6 +844,7 @@ async def export_full_backup() -> dict: "rule_systems": _rule_system_rows(rule_system_rows), "rule_relations": _rule_relation_rows(rule_relations), "lesson_rule_links": _lesson_rule_link_rows(lesson_rule_links), + "lesson_no_rule": _lesson_no_rule_rows(lesson_no_rule), "systems": _system_rows( systems, {c.id: c.slug for c in canonical_systems} ), @@ -992,6 +1013,9 @@ async def export_user_backup(user_id: int) -> dict: LessonRuleLink.rule_id.in_(_rule_ids), ) )).scalars().all() if (_rule_ids and note_ids) else [] + lesson_no_rule = (await session.execute( + select(LessonNoRule).where(LessonNoRule.lesson_id.in_(note_ids)) + )).scalars().all() if note_ids else [] return { "version": BACKUP_VERSION, @@ -1020,6 +1044,7 @@ async def export_user_backup(user_id: int) -> dict: "rule_systems": _rule_system_rows(rule_system_rows), "rule_relations": _rule_relation_rows(rule_relations), "lesson_rule_links": _lesson_rule_link_rows(lesson_rule_links), + "lesson_no_rule": _lesson_no_rule_rows(lesson_no_rule), "systems": _system_rows( systems, {c.id: c.slug for c in canonical_systems} ), @@ -1365,6 +1390,17 @@ def _build_rule_relation(row: dict, maps: _Maps) -> RuleRelation | None: ) +def _build_lesson_no_rule(row: dict, maps: _Maps) -> LessonNoRule | None: + """Skipped when its lesson did not restore — the answer is about that + lesson and no other.""" + lesson = maps.notes.get(row.get("lesson_id", 0)) + if lesson is None or not (row.get("why") or "").strip(): + return None + return LessonNoRule( + lesson_id=lesson, why=row["why"], judged_at=_dt(row.get("judged_at")), + ) + + def _build_lesson_rule_link(row: dict, maps: _Maps) -> LessonRuleLink | None: """Both ends must map: a link whose lesson or rule did not restore points at whatever took that number in the destination.""" @@ -1782,6 +1818,7 @@ async def _restore_v2(data: dict) -> dict: "code_shape_uses": 0, "canonical_systems": 0, "rule_systems": 0, "rule_relations": 0, "rule_versions": 0, "retrieval_tuning_events": 0, "lesson_rule_links": 0, + "lesson_no_rule": 0, } async with async_session() as session: @@ -1985,6 +2022,13 @@ async def _restore_v2(data: dict) -> dict: continue session.add(link) stats["lesson_rule_links"] += 1 + # "No rule fits" answers (#4631); archives before v19 carry none. + for nr in data.get("lesson_no_rule", []): + answer = _build_lesson_no_rule(nr, maps) + if answer is None: + continue + session.add(answer) + stats["lesson_no_rule"] += 1 # A rule's edit history (milestone 323). Must come after the rules # themselves — the rule map is only populated above — and both ids are diff --git a/src/scribe/services/lesson_rules.py b/src/scribe/services/lesson_rules.py index 63c27ad..f8e6424 100644 --- a/src/scribe/services/lesson_rules.py +++ b/src/scribe/services/lesson_rules.py @@ -10,6 +10,13 @@ judgment is made. Every write here is a JUDGMENT, so every write lands as confirmed or rejected; `suggested` rows come from the co-surfacing recorder (#4637), never from a caller naming a rule. +THE THIRD ANSWER (#4631). "Which rule is this an instance of?" has three +answers, and only two of them are links: a rule named (confirmed), or NO RULE +FITS — a `lesson_no_rule` row carrying the reason. A lesson with neither is +UNJUDGED, and `list_unjudged` lists those. The three are kept apart because a +lesson that stands alone is the raw material for a rule nobody has written yet +(#4634), and that is unreadable if "nobody looked" means the same thing. + ACL (rule 78). Linking changes what a lesson says about itself, so it needs WRITE on the lesson (`access.can_write_note`, share-aware). It names a rule, so it needs the rule to be one the caller may read — rules are owner-scoped, and @@ -22,10 +29,12 @@ from __future__ import annotations import logging from datetime import datetime, timezone -from sqlalchemy import select +from sqlalchemy import and_, delete, exists, func, not_, select from scribe.models import async_session -from scribe.models.lesson_rule_link import CONFIRMED, REJECTED, SUGGESTED, LessonRuleLink +from scribe.models.lesson_rule_link import ( + CONFIRMED, REJECTED, SUGGESTED, LessonNoRule, LessonRuleLink, +) from scribe.models.note import Note from scribe.models.rulebook import Rule @@ -39,6 +48,18 @@ VERDICTS = {"confirm": CONFIRMED, "reject": REJECTED} # row can tell an explicit "not this rule" from a link dropped by a rewrite. _REMOVED_NOTE = "removed from the lesson's rules by an update" +# Recorded on a confirmed link that a "no rule fits" answer overturned, ahead +# of that answer's reason. +_NO_RULE_NOTE = "no rule fits: " + +# What `rule_judgment` reads on a lesson payload — the three answers. +LINKED, NO_RULE, UNJUDGED = "linked", "no_rule", "unjudged" + +# How many rules a new lesson is offered to judge against. A handful, not the +# fifty `what_might_apply` returns: this is read in the create response, at +# the moment of writing, and a long tail there buries the one that fits. +CANDIDATE_LIMIT = 5 + def _ids(values) -> list[int]: """Positive ints, de-duplicated, in the order given.""" @@ -83,6 +104,16 @@ async def require_rules(user_id: int, rule_ids) -> list[int]: return wanted +def require_one_answer(linked, no_rule: str) -> None: + """Refuse "these rules fit" and "no rule fits" together, before any write + — they contradict, and either order of applying them loses one.""" + if linked and (no_rule or "").strip(): + raise ValueError( + "rule_ids and no_rule are the two answers to \"which rule is this " + "an instance of?\" — give the one that holds. Nothing was written." + ) + + async def _upsert(session, lesson_id: int, rule_id: int, state: str, note: str) -> None: now = datetime.now(timezone.utc) row = (await session.execute( @@ -104,6 +135,11 @@ async def _upsert(session, lesson_id: int, rule_id: int, state: str, note: str) row.judged_at = now +async def _clear_no_rule(session, lesson_id: int) -> None: + """A confirmed rule overturns "no rule fits" — both cannot be the answer.""" + await session.execute(delete(LessonNoRule).where(LessonNoRule.lesson_id == lesson_id)) + + async def set_lesson_rules( user_id: int, lesson_id: int, rule_ids, *, note: str = "", ) -> None: @@ -134,9 +170,48 @@ async def set_lesson_rules( row.judged_at = datetime.now(timezone.utc) for rid in wanted: await _upsert(session, lesson_id, rid, CONFIRMED, note) + if wanted: + await _clear_no_rule(session, lesson_id) await session.commit() +async def set_no_rule(user_id: int, lesson_id: int, why: str) -> dict: + """Record that no rule governs this lesson's situation, and why. + + A confirmed link the lesson still carries becomes `rejected`, with the + reason: saying none fits is saying the linked one does not either. A + second answer replaces the first, so the reason on file is the latest. + """ + why = (why or "").strip() + if not why: + raise ValueError( + "no_rule takes the reason no rule fits, in a line — it is what " + "lets the answer be re-judged when a rule is later written for " + "this situation." + ) + await _require_lesson_writable(user_id, lesson_id) + now = datetime.now(timezone.utc) + async with async_session() as session: + confirmed = (await session.execute( + select(LessonRuleLink).where( + LessonRuleLink.lesson_id == lesson_id, + LessonRuleLink.state == CONFIRMED, + ) + )).scalars().all() + for row in confirmed: + row.state = REJECTED + row.note = _NO_RULE_NOTE + why + row.judged_at = now + answer = await session.get(LessonNoRule, lesson_id) + if answer is None: + session.add(LessonNoRule(lesson_id=lesson_id, why=why, judged_at=now)) + else: + answer.why = why + answer.judged_at = now + await session.commit() + return {"why": why, "judged_at": now.isoformat()} + + async def judge_link( user_id: int, lesson_id: int, rule_id: int, verdict: str, note: str = "", ) -> dict: @@ -148,6 +223,8 @@ async def judge_link( [rid] = await require_rules(user_id, [rule_id]) async with async_session() as session: await _upsert(session, lesson_id, rid, state, note) + if state == CONFIRMED: + await _clear_no_rule(session, lesson_id) await session.commit() row = (await session.execute( select(LessonRuleLink).where( @@ -210,22 +287,147 @@ async def lessons_for_rule(user_id: int, rule_id: int) -> list[dict]: ] +async def judgments_for_lessons(lesson_ids) -> dict[int, dict]: + """{lesson_id: {"rule_judgment": linked|no_rule|unjudged, "no_rule"?}}. + + Read from the link and answer tables directly, NOT from the reader's view + of the rules: a lesson shared with someone who cannot see its owner's rule + is still a judged lesson, and calling it unjudged would invite them to + judge it again. + """ + ids = [int(i) for i in lesson_ids or []] + if not ids: + return {} + async with async_session() as session: + linked = set((await session.execute( + select(LessonRuleLink.lesson_id).where( + LessonRuleLink.lesson_id.in_(ids), + LessonRuleLink.state == CONFIRMED, + ) + )).scalars().all()) + answers = { + a.lesson_id: a for a in (await session.execute( + select(LessonNoRule).where(LessonNoRule.lesson_id.in_(ids)) + )).scalars().all() + } + out: dict[int, dict] = {} + for i in ids: + if i in linked: + out[i] = {"rule_judgment": LINKED} + elif i in answers: + out[i] = {"rule_judgment": NO_RULE, "no_rule": answers[i].to_dict()} + else: + out[i] = {"rule_judgment": UNJUDGED} + return out + + async def attach_lesson_rules(user_id: int, rows: list[dict], *, key: str = "id") -> None: - """Add `rules` to each lesson payload row, in place — one query per page. + """Add `rules` and `rule_judgment` (plus `no_rule` when that is the answer) + to each lesson payload row, in place — a fixed number of queries per page. Fail-open (snippet #4286): this decorates a lesson the caller already has, - so a failed lookup leaves the key off and logs, rather than refusing the + so a failed lookup leaves the keys off and logs, rather than refusing the lesson. An absent key reads as "not attached", never as "no rule". """ ids = [int(r[key]) for r in rows if isinstance(r.get(key), int)] try: found = await rules_for_lessons(user_id, ids) + judged = await judgments_for_lessons(ids) except Exception: logger.warning("lesson→rule links could not be read", exc_info=True) return for r in rows: if isinstance(r.get(key), int): r["rules"] = found.get(r[key], []) + r.update(judged.get(r[key], {})) + + +def unjudged_clause(): + """SQL: a lesson (a `notes` row) with no confirmed rule and no "no rule + fits" answer. A rejected link alone leaves it unjudged — "not that rule" + does not say whether another one fits.""" + from scribe.models.note import Note + + return and_( + not_(exists().where(and_( + LessonRuleLink.lesson_id == Note.id, + LessonRuleLink.state == CONFIRMED, + ))), + not_(exists().where(LessonNoRule.lesson_id == Note.id)), + ) + + +async def list_unjudged( + user_id: int, *, tag: str = "", project_id: int | None = None, + limit: int = 50, offset: int = 0, +) -> tuple[list[dict], int]: + """The lessons nobody has answered "which rule?" for, newest edit first. + + The browse scope (`browsable_notes_clause`), the list it sits beside uses: + a lesson shared directly with the caller is search-only and stays out of + an ambient list. Rows come back in the knowledge list's item shape. + """ + from scribe.models.note import Note + from scribe.services.access import browsable_notes_clause + from scribe.services.knowledge import _note_to_item + from scribe.services.lessons import LESSON_NOTE_TYPE + + base = ( + select(Note) + .where(browsable_notes_clause(user_id)) + .where(Note.note_type == LESSON_NOTE_TYPE) + .where(Note.deleted_at.is_(None)) + .where(unjudged_clause()) + ) + if tag: + base = base.where(Note.tags.contains([tag])) + if project_id is not None: + base = base.where(Note.project_id == project_id) + async with async_session() as session: + total = (await session.execute( + select(func.count()).select_from(base.subquery()) + )).scalar_one() + rows = (await session.execute( + base.order_by(Note.updated_at.desc()).limit(limit).offset(offset) + )).scalars().all() + return [_note_to_item(n) for n in rows], int(total) + + +async def rule_candidates( + user_id: int, what: str, when_to_apply: str, project_id: int | None = None, +) -> list[dict] | None: + """The rules a lesson most resembles, for the writer to judge against. + + Searched with the lesson's claim and its trigger — the same two halves a + rule's own document leads with (`{title} — {trigger}`), so like is + compared with like; the insight is the story and would dilute the vector. + Scoped as any project read is: global rules plus the lesson's project's + own. The bar is the explicit rule search's, since this is an explicit + question rather than an injection. + + None when the search could not run (no embedder, a failed query), so a + caller can say "candidates unavailable" rather than "nothing resembles". + """ + from scribe.services.embeddings import ( + DEFAULT_SIMILARITY_THRESHOLD, semantic_search_rules, + ) + + query = " — ".join(p for p in ((what or "").strip(), (when_to_apply or "").strip()) if p) + report: dict = {} + found = await semantic_search_rules( + user_id, query, limit=CANDIDATE_LIMIT, + threshold=DEFAULT_SIMILARITY_THRESHOLD, report=report, + project_id=project_id or None, + ) + if not report.get("searched"): + return None + return [ + { + "id": rule.id, "title": rule.title, "kind": rule.kind, + "when_to_apply": rule.when_to_apply or "", "score": round(score, 3), + } + for score, rule in found + ] async def attach_rule_lessons(user_id: int, data: dict, rule_id: int) -> None: diff --git a/tests/conftest.py b/tests/conftest.py index b64e527..489d199 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -152,6 +152,10 @@ def _no_lesson_rule_links(request): Both decorations are fail-open, so the cost would be a slow failed connect per test rather than a failure, which is worse: it would never be noticed. + The candidate search a new lesson runs (#4631) is stubbed to "nothing + resembles it" for the same reason: it embeds, and the model load would + otherwise ride along with every unit test that creates a lesson. + Skipped for integration tests, which exercise the real links against Postgres (tests/test_integration_lesson_rule_links.py). """ @@ -159,7 +163,8 @@ def _no_lesson_rule_links(request): yield return with patch("scribe.services.lesson_rules.attach_lesson_rules", AsyncMock()), \ - patch("scribe.services.lesson_rules.attach_rule_lessons", AsyncMock()): + patch("scribe.services.lesson_rules.attach_rule_lessons", AsyncMock()), \ + patch("scribe.services.lesson_rules.rule_candidates", AsyncMock(return_value=[])): yield diff --git a/tests/test_guidance_ownership.py b/tests/test_guidance_ownership.py index a03fc21..ccffb91 100644 --- a/tests/test_guidance_ownership.py +++ b/tests/test_guidance_ownership.py @@ -145,6 +145,15 @@ TOPICS: tuple[Topic, ...] = ( # one fires mid-work, the moment a lesson arrives in a situation it # actually names. `lessons inform` already points at the kind. shared_with=("docstrings",)), + Topic("a lesson names the rule it is an instance of", U, + ("rule_ids", "no_rule", "unjudged=true"), + "a rule about the same subsystem that governs a different moment " + "is not its rule", + # Mid-work, like the topic above: it fires when a lesson is written, + # so no index marker. create_lesson's docstring states the two + # answers at the moment they are given, and may — tool contracts + # are not copy-scanned. + shared_with=("docstrings",)), Topic("preferences shape how work is done, never what is recorded", U, ("never what gets recorded",), "a preference never makes a task into a note"), diff --git a/tests/test_integration_lesson_rule_links.py b/tests/test_integration_lesson_rule_links.py index 5156bcb..539a8c5 100644 --- a/tests/test_integration_lesson_rule_links.py +++ b/tests/test_integration_lesson_rule_links.py @@ -160,3 +160,84 @@ async def test_one_row_per_pair(world): ]) with pytest.raises(IntegrityError): await s.commit() + + +# ── #4631: "no rule fits" and the unjudged list ───────────────────────────── + + +async def _judgment(lesson_id: int) -> dict: + return (await links_svc.judgments_for_lessons([lesson_id]))[lesson_id] + + +async def _unjudged_ids(user_id: int) -> set[int]: + items, _total = await links_svc.list_unjudged(user_id, limit=100) + return {i["id"] for i in items} + + +async def test_a_new_lesson_is_unjudged_until_it_is_answered_either_way(world): + owner, lesson = world["owner"], world["lesson"] + assert (await _judgment(lesson))["rule_judgment"] == "unjudged" + assert lesson in await _unjudged_ids(owner) + + await links_svc.set_no_rule(owner, lesson, "specific to this CI host") + judged = await _judgment(lesson) + assert judged["rule_judgment"] == "no_rule" + assert judged["no_rule"]["why"] == "specific to this CI host" + assert lesson not in await _unjudged_ids(owner) + + # Naming a rule overturns the answer — both cannot stand. + await links_svc.set_lesson_rules(owner, lesson, [world["r1"]]) + judged = await _judgment(lesson) + assert judged == {"rule_judgment": "linked"} + assert lesson not in await _unjudged_ids(owner) + + +async def test_no_rule_rejects_a_confirmed_link_with_its_reason(world): + owner, lesson = world["owner"], world["lesson"] + await links_svc.set_lesson_rules(owner, lesson, [world["r1"]]) + await links_svc.set_no_rule(owner, lesson, "the log was not the issue") + + assert await _states(lesson) == {world["r1"]: "rejected"} + rules = (await links_svc.rules_for_lessons(owner, [lesson]))[lesson] + assert rules[0]["note"].endswith("the log was not the issue") + assert (await _judgment(lesson))["rule_judgment"] == "no_rule" + + +async def test_a_confirmation_by_judgment_also_clears_no_rule(world): + owner, lesson = world["owner"], world["lesson"] + await links_svc.set_no_rule(owner, lesson, "nothing yet") + await links_svc.judge_link(owner, lesson, world["r2"], "confirm") + assert (await _judgment(lesson))["rule_judgment"] == "linked" + + +async def test_a_rejection_alone_leaves_the_lesson_unjudged(world): + """"Not that rule" says nothing about whether another one fits.""" + owner, lesson = world["owner"], world["lesson"] + await links_svc.judge_link(owner, lesson, world["r1"], "reject", "different failure") + assert (await _judgment(lesson))["rule_judgment"] == "unjudged" + assert lesson in await _unjudged_ids(owner) + + +async def test_no_rule_needs_a_reason_and_a_writer(world): + with pytest.raises(ValueError): + await links_svc.set_no_rule(world["owner"], world["lesson"], " ") + with pytest.raises((ValueError, PermissionError)): + await links_svc.set_no_rule(world["stranger"], world["lesson"], "mine now") + assert (await _judgment(world["lesson"]))["rule_judgment"] == "unjudged" + + +async def test_the_unjudged_list_is_scoped_to_the_reader(world): + assert world["lesson"] not in await _unjudged_ids(world["stranger"]) + + +async def test_deleting_the_lesson_takes_its_answer_with_it(world): + from scribe.models.lesson_rule_link import LessonNoRule + + await links_svc.set_no_rule(world["owner"], world["lesson"], "alone") + async with async_session() as s: + await s.execute(delete(Note).where(Note.id == world["lesson"])) + await s.commit() + left = (await s.execute( + select(LessonNoRule).where(LessonNoRule.lesson_id == world["lesson"]) + )).scalars().all() + assert left == [] diff --git a/tests/test_lesson_rule_link_doors.py b/tests/test_lesson_rule_link_doors.py index 8715fad..96ec27a 100644 --- a/tests/test_lesson_rule_link_doors.py +++ b/tests/test_lesson_rule_link_doors.py @@ -143,3 +143,123 @@ def test_backup_keeps_an_unjudged_link_unjudged(): ) assert (built.lesson_id, built.rule_id, built.state) == (110, 120, "suggested") assert built.judged_at is None + + +# ── #4631: judged when written ─────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_an_unanswered_lesson_comes_back_with_rules_to_judge_against(): + """Neither answer given: the create offers the rules the lesson resembles, + searched with its claim and trigger and scoped to its project.""" + _user_id_ctx.set(7) + offered = [{"id": 5, "title": "Read the log", "kind": "rule", + "when_to_apply": "a run overran", "score": 0.61}] + search = AsyncMock(return_value=offered) + p1, p2, p3 = _create_patches(AsyncMock(return_value=_stub_note())) + with p1, p2, p3, patch.object(links_svc, "rule_candidates", search): + out = await lesson_tools.create_lesson(what="x", when_to_apply=TRIGGER, project_id=2) + assert search.await_args.args == (7, "x", TRIGGER, 2) + assert out["rule_candidates"] == offered + assert out["rule_judgment"] == links_svc.UNJUDGED + assert "no_rule" in out["rule_hint"] and "rule_ids" in out["rule_hint"] + + +@pytest.mark.asyncio +async def test_a_search_that_could_not_run_offers_no_list_rather_than_an_empty_one(): + """None means unavailable; an empty list would claim nothing resembles it.""" + _user_id_ctx.set(7) + p1, p2, p3 = _create_patches(AsyncMock(return_value=_stub_note())) + with p1, p2, p3, patch.object(links_svc, "rule_candidates", AsyncMock(return_value=None)): + out = await lesson_tools.create_lesson(what="x", when_to_apply=TRIGGER) + assert "rule_candidates" not in out + assert out["rule_judgment"] == links_svc.UNJUDGED + + +@pytest.mark.asyncio +async def test_no_rule_on_create_records_the_answer_and_offers_nothing(): + _user_id_ctx.set(7) + answered = AsyncMock() + search = AsyncMock(return_value=[]) + p1, p2, p3 = _create_patches(AsyncMock(return_value=_stub_note(id=41))) + with p1, p2, p3, patch.object(links_svc, "set_no_rule", answered), \ + patch.object(links_svc, "rule_candidates", search): + out = await lesson_tools.create_lesson( + what="x", when_to_apply=TRIGGER, no_rule="a one-off of this CI host", + ) + assert answered.await_args.args[1:] == (41, "a one-off of this CI host") + search.assert_not_awaited() + assert "rule_candidates" not in out + + +@pytest.mark.asyncio +async def test_rules_and_no_rule_together_write_nothing(): + _user_id_ctx.set(7) + created = AsyncMock(return_value=_stub_note()) + p1, p2, p3 = _create_patches(created) + with p1, p2, p3, patch.object(links_svc, "require_rules", AsyncMock(return_value=[5])): + with pytest.raises(ValueError, match="no_rule"): + await lesson_tools.create_lesson( + what="x", when_to_apply=TRIGGER, rule_ids=[5], no_rule="none fits", + ) + created.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_update_records_no_rule_and_refuses_it_beside_named_rules(): + _user_id_ctx.set(7) + answered = AsyncMock() + updated = AsyncMock(return_value=_stub_note()) + with patch.object(lessons_svc, "update_lesson", updated), \ + patch.object(links_svc, "set_no_rule", answered): + await lesson_tools.update_lesson(lesson_id=41, no_rule="stands alone") + assert answered.await_args.args[1:] == (41, "stands alone") + + updated.reset_mock() + with patch.object(lessons_svc, "update_lesson", updated), \ + patch.object(links_svc, "require_rules", AsyncMock(return_value=[5])): + with pytest.raises(ValueError): + await lesson_tools.update_lesson(lesson_id=41, rule_ids=[5], no_rule="none") + updated.assert_not_awaited() + + +def test_an_empty_rule_list_beside_no_rule_is_one_answer_not_two(): + """`rule_ids=[]` clears; it does not name a rule, so it cannot contradict.""" + links_svc.require_one_answer([], "none fits") + links_svc.require_one_answer(None, "none fits") + links_svc.require_one_answer([5], "") + with pytest.raises(ValueError): + links_svc.require_one_answer([5], "none fits") + + +@pytest.mark.asyncio +async def test_list_unjudged_is_a_listing_and_routes_to_its_own_query(): + _user_id_ctx.set(7) + listed = AsyncMock(return_value=([], 0)) + with patch.object(links_svc, "list_unjudged", listed), \ + patch("scribe.mcp.tools.lessons.access_svc.label_shared_items", AsyncMock(return_value=[])), \ + patch("scribe.mcp.tools.lessons.attach_usage", AsyncMock()): + out = await lesson_tools.list_lessons(unjudged=True, tag="ci", project_id=2) + assert out == {"lessons": [], "total": 0} + assert listed.await_args.kwargs["tag"] == "ci" + assert listed.await_args.kwargs["project_id"] == 2 + with pytest.raises(ValueError): + await lesson_tools.list_lessons(unjudged=True, q="log") + + +def test_backup_skips_a_no_rule_answer_whose_lesson_did_not_restore(): + maps = backup._Maps() + assert backup._build_lesson_no_rule({"lesson_id": 10, "why": "alone"}, maps) is None + maps.notes[10] = 110 + built = backup._build_lesson_no_rule({"lesson_id": 10, "why": "alone"}, maps) + assert (built.lesson_id, built.why) == (110, "alone") + # An answer with no reason is not an answer; it restores as unjudged. + assert backup._build_lesson_no_rule({"lesson_id": 10, "why": " "}, maps) is None + + +def test_the_no_rule_migration_follows_the_link_migration(): + path = Path(__file__).resolve().parents[1] / "alembic" / "versions" / "0112_lesson_no_rule.py" + spec = importlib.util.spec_from_file_location("m0112", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + assert (module.revision, module.down_revision) == ("0112", "0111") diff --git a/tests/test_services_backup.py b/tests/test_services_backup.py index 56a6978..a7ee02e 100644 --- a/tests/test_services_backup.py +++ b/tests/test_services_backup.py @@ -27,7 +27,7 @@ def test_backup_version_is_current(): (Named for the number it asserted until v10, which is exactly the drift a name-carrying-a-value invites; it now says what it checks.)""" - assert backup.BACKUP_VERSION == 18 + assert backup.BACKUP_VERSION == 19 def _exportable_note(**over): @@ -131,7 +131,7 @@ def test_a_repo_binding_carries_the_branch_its_ledger_follows(): def _column_guard_targets(): from scribe.models.canonical_system import CanonicalSystem from scribe.models.code_shape import CodeShape, CodeShapeEvent, CodeShapeUse - from scribe.models.lesson_rule_link import LessonRuleLink + from scribe.models.lesson_rule_link import LessonNoRule, LessonRuleLink from scribe.models.design_system import DesignSystem, DesignToken from scribe.models.milestone import Milestone from scribe.models.note import Note @@ -169,6 +169,7 @@ def _column_guard_targets(): "note_supersessions": (NoteSupersession, backup._note_supersession_rows), "rule_relations": (RuleRelation, backup._rule_relation_rows), "lesson_rule_links": (LessonRuleLink, backup._lesson_rule_link_rows), + "lesson_no_rule": (LessonNoRule, backup._lesson_no_rule_rows), "note_usage_events": (NoteUsageEvent, backup._usage_event_rows), "rule_usage_events": (RuleUsageEvent, backup._rule_usage_event_rows), "retrieval_tuning_events": ( @@ -286,6 +287,7 @@ def _import_guard_targets(): "note_supersessions": backup._build_note_supersession, "rule_relations": backup._build_rule_relation, "lesson_rule_links": backup._build_lesson_rule_link, + "lesson_no_rule": backup._build_lesson_no_rule, "note_usage_events": backup._build_usage_event, "rule_usage_events": backup._build_rule_usage_event, "retrieval_tuning_events": backup._build_retrieval_tuning_event, @@ -539,7 +541,9 @@ async def test_export_full_backup_contains_every_declared_section(): # v16: the reasons beside the settings they explain. "retrieval_tuning_events", # v18: which rule each lesson is an instance of. - "lesson_rule_links"): + "lesson_rule_links", + # v19: the lessons judged to fall under no rule. + "lesson_no_rule"): assert key in out, f"missing export section: {key}" assert out[key] == []