Merge pull request 'The web lesson editor records which rule a lesson is an instance of (milestone 440, #4658)' (#192) from dev into main

frontend/src/api/lessons.ts

@@ -86,6 +86,10 @@ export interface Lesson {
   rule_judgment?: RuleJudgment;
   /** The "no rule fits" answer, present when that is the judgment. */
   no_rule?: { why: string; judged_at: string | null };
+  /** Sent by a create or update that recorded "no rule fits" when that answer
+   *  completes a group: lessons with no rule that keep landing in one
+   *  situation, which may want a rule written for it (#4634). */
+  convergence?: { lessons: { id: number; title: string }[] };
   /** Set when another user owns this record. */
   shared?: boolean;
   owner?: string | null;
@@ -125,6 +129,13 @@ export interface LessonPayload {
   tags?: string[];
   project_id?: number | null;
   system_ids?: number[];
+  /** The rules this lesson is an instance of. On update, a list is the full
+   *  new set: a confirmed rule left out is judged NOT an instance and becomes
+   *  rejected. Omit it to leave the links alone. */
+  rule_ids?: number[];
+  /** "No rule fits", with the reason. Exclusive with a non-empty `rule_ids`:
+   *  the server refuses both in one request. */
+  no_rule?: string;
   /** Deliberate override of the near-duplicate gate, once the writer has seen
    *  the warning. Two lessons under one trigger compete for one reserved slot,
    *  so a duplicate displaces rather than merely clutters. */
@@ -163,6 +174,33 @@ export function updateLesson(
   return apiPatch<Lesson>(`/api/lessons/${id}`, payload);
 }
 
+/** A rule the lesson being written resembles (`rule_candidates`). */
+export interface RuleCandidate {
+  id: number;
+  title: string;
+  kind: RuleKind;
+  when_to_apply: string;
+  score: number;
+}
+
+/** The rules a lesson resembles, asked BEFORE it is saved so the writer can
+ *  name one in the same save. `null` means the search could not run, which is
+ *  different from an empty list ("nothing resembles it"). */
+export function ruleCandidates(params: {
+  what: string;
+  when_to_apply: string;
+  project_id?: number | null;
+}): Promise<{ candidates: RuleCandidate[] | null }> {
+  const qs = new URLSearchParams({
+    what: params.what,
+    when_to_apply: params.when_to_apply,
+  });
+  if (params.project_id) qs.set("project_id", String(params.project_id));
+  return apiGet<{ candidates: RuleCandidate[] | null }>(
+    `/api/lessons/rule-candidates?${qs}`,
+  );
+}
+
 /** Confirm or reject one lesson→rule link. Confirming also clears a "no rule
  *  fits" answer: the two cannot both be the current answer. */
 export function judgeLessonLink(

frontend/src/views/LessonEditorView.vue

@@ -16,6 +16,13 @@
  * lesson with no trigger saves, reads correctly in every listing, and never
  * surfaces — and there is nothing to notice afterwards, because it looks
  * exactly like a lesson that works. The form is where that gets caught.
+ *
+ * WHICH RULE IS THIS AN INSTANCE OF (milestone 440) is asked here, while the
+ * writer still has the situation in mind, the same way the create tool asks
+ * it. There are three answers: the rule(s), "no rule fits" with a reason, or
+ * leave it open. None is forced, because a lesson left open is still a
+ * lesson. But the form offers the rules it resembles before the save, so
+ * naming one costs a click.
  */
 import { computed, onMounted, ref, watch } from "vue";
 import { useRoute, useRouter } from "vue-router";
@@ -24,9 +31,13 @@ import { apiErrorMessage } from "@/api/client";
 import {
   createLesson,
   getLesson,
+  ruleCandidates,
   updateLesson,
   type Lesson,
+  type LessonPayload,
+  type RuleCandidate,
 } from "@/api/lessons";
+import type { RuleKind } from "@/api/rulebooks";
 import ProjectSelector from "@/components/ProjectSelector.vue";
 import TagInput from "@/components/TagInput.vue";
 import { useNotesStore } from "@/stores/notes";
@@ -67,8 +78,106 @@ const previewTitle = computed(() => {
   return subject || trigger;
 });
 
+// ── which rule is this an instance of ──────────────────────────────────────
+
+type Answer = "rules" | "no_rule" | "open";
+const answer = ref<Answer>("open");
+const selectedRuleIds = ref<number[]>([]);
+const noRuleWhy = ref("");
+/** The answer the lesson held when loaded. An edit sends an answer only when
+ *  it changed: re-sending the same rules would re-stamp their judgments, and
+ *  a shared editor who cannot see the owner's rule would reject it by
+ *  re-sending a list without it. */
+const original = ref<{ answer: Answer; ruleIds: number[]; why: string }>({
+  answer: "open", ruleIds: [], why: "",
+});
+/** The rules already linked, kept in the list so they can be unticked even
+ *  when the search no longer ranks them. */
+const linkedRules = ref<{ id: number; title: string; kind: RuleKind }[]>([]);
+/** null until a search has answered. */
+const candidates = ref<RuleCandidate[] | null>(null);
+const searchUnavailable = ref(false);
+const searching = ref(false);
+
+const ruleOptions = computed(() => {
+  const seen = new Set<number>();
+  const out: { id: number; title: string; kind: RuleKind; when_to_apply?: string }[] = [];
+  for (const r of [...linkedRules.value, ...(candidates.value ?? [])]) {
+    if (seen.has(r.id)) continue;
+    seen.add(r.id);
+    out.push(r);
+  }
+  return out;
+});
+
+/** The server has no way to take back "no rule fits" except by naming a
+ *  rule, so once that is the answer, "leave it open" is not offered. */
+const canLeaveOpen = computed(() => original.value.answer !== "no_rule");
+
+function sameIds(a: number[], b: number[]): boolean {
+  if (a.length !== b.length) return false;
+  const want = new Set(b);
+  return a.every((id) => want.has(id));
+}
+
+const answerChanged = computed(() => {
+  const o = original.value;
+  if (answer.value !== o.answer) return true;
+  if (answer.value === "rules") return !sameIds(selectedRuleIds.value, o.ruleIds);
+  if (answer.value === "no_rule") return noRuleWhy.value.trim() !== o.why;
+  return false;
+});
+
+/** Why the answer as it stands cannot be saved, or null when it can. */
+const answerProblem = computed(() => {
+  if (!answerChanged.value) return null;
+  if (answer.value === "rules" && !selectedRuleIds.value.length) {
+    return "Tick the rule this is an instance of, or choose another answer.";
+  }
+  if (answer.value === "no_rule" && !noRuleWhy.value.trim()) {
+    return "Say why no rule fits. A bare “none” cannot be judged again when a rule is later written for this situation.";
+  }
+  return null;
+});
+
+/** The answer's half of the payload. Empty when nothing changed. */
+function answerPayload(): Pick<LessonPayload, "rule_ids" | "no_rule"> {
+  if (!answerChanged.value) return {};
+  if (answer.value === "rules") return { rule_ids: selectedRuleIds.value };
+  if (answer.value === "no_rule") return { no_rule: noRuleWhy.value.trim() };
+  // Left open after being linked: an empty set rejects the linked rules.
+  return original.value.answer === "rules" ? { rule_ids: [] } : {};
+}
+
+async function findRules() {
+  if (!what.value.trim() && !whenToApply.value.trim()) return;
+  searching.value = true;
+  try {
+    const res = await ruleCandidates({
+      what: what.value.trim(),
+      when_to_apply: whenToApply.value.trim(),
+      project_id: projectId.value,
+    });
+    candidates.value = res.candidates ?? [];
+    searchUnavailable.value = res.candidates === null;
+  } catch (e) {
+    toast.show(apiErrorMessage(e, "Failed to look for rules"), "error");
+  } finally {
+    searching.value = false;
+  }
+}
+
+// Search the first time the writer says it is an instance of a rule, including
+// when an edit opens on a linked lesson.
+watch(answer, (a) => {
+  if (a === "rules" && candidates.value === null && !searching.value) findRules();
+});
+
 const canSave = computed(
-  () => what.value.trim().length > 0 && whenToApply.value.trim().length > 0,
+  () =>
+    what.value.trim().length > 0 &&
+    whenToApply.value.trim().length > 0 &&
+    answerProblem.value === null,
 );
 
 async function load() {
@@ -85,6 +194,18 @@ async function load() {
     tags.value = lesson.tags ?? [];
     projectId.value = lesson.project_id;
     learnedFrom.value = lesson.learned_from ?? [];
+    const confirmed = (lesson.rules ?? []).filter((r) => r.state === "confirmed");
+    linkedRules.value = confirmed;
+    const ids = confirmed.map((r) => r.id);
+    const why = lesson.no_rule?.why ?? "";
+    const held: Answer =
+      lesson.rule_judgment === "linked" ? "rules"
+        : lesson.rule_judgment === "no_rule" ? "no_rule"
+          : "open";
+    original.value = { answer: held, ruleIds: ids, why };
+    selectedRuleIds.value = [...ids];
+    noRuleWhy.value = why;
+    answer.value = held;
   } catch (e) {
     error.value = apiErrorMessage(e, "Failed to load this lesson");
   } finally {
@@ -105,12 +226,22 @@ async function save(force = false) {
       tags: tags.value,
       learned_from: learnedFrom.value,
       project_id: projectId.value,
+      ...answerPayload(),
       ...(force ? { force: true } : {}),
     };
     const saved = isEdit.value && lessonId.value !== null
       ? await updateLesson(lessonId.value, payload)
       : await createLesson(payload);
     toast.show(isEdit.value ? "Lesson updated" : "Lesson recorded");
+    // "No rule fits" just completed a group of lessons in one situation (#4634).
+    // Said once, here, because the page this goes to does not recompute it.
+    if (saved.convergence) {
+      toast.show(
+        `${saved.convergence.lessons.length} lessons now say no rule fits this ` +
+          "situation. A situation met this often may want a rule of its own.",
+        "warning",
+      );
+    }
     router.push(`/lessons/${saved.id}`);
   } catch (e) {
     // A 409 is the duplicate gate, not a failure: it hands back the record
@@ -238,6 +369,70 @@ onMounted(() => {
         <ProjectSelector v-model="projectId" />
       </label>
 
+      <fieldset class="le-field le-answer">
+        <legend class="le-label">Which rule is this an instance of?</legend>
+        <span class="le-hint">
+          A confirmed rule surfaces through this lesson whenever its situation
+          comes up. Saying no rule fits is an answer too: lessons that keep
+          landing in one situation with no rule are how a missing rule gets
+          noticed.
+        </span>
+
+        <label class="le-choice">
+          <input v-model="answer" type="radio" value="rules" />
+          An instance of a rule
+        </label>
+        <div v-if="answer === 'rules'" class="le-rules">
+          <p v-if="searching" class="le-muted">Looking for rules it resembles…</p>
+          <p v-else-if="searchUnavailable" class="le-muted">
+            Rule search is unavailable right now.
+          </p>
+          <p
+            v-else-if="candidates !== null && !ruleOptions.length"
+            class="le-muted"
+          >
+            No rule resembles this lesson closely. If none fits, say so below.
+          </p>
+          <label v-for="r in ruleOptions" :key="r.id" class="le-choice le-rule">
+            <input v-model="selectedRuleIds" type="checkbox" :value="r.id" />
+            <span>
+              {{ r.title }}
+              <span v-if="r.kind === 'preference'" class="rule-chip rule-chip-preference">
+                preference
+              </span>
+              <span v-if="r.when_to_apply" class="le-hint le-rule-when">
+                {{ r.when_to_apply }}
+              </span>
+            </span>
+          </label>
+          <button
+            type="button"
+            class="le-ghost le-small"
+            :disabled="searching || (!what.trim() && !whenToApply.trim())"
+            @click="findRules"
+          >
+            {{ candidates === null ? "Look for rules" : "Search again" }}
+          </button>
+        </div>
+
+        <label class="le-choice">
+          <input v-model="answer" type="radio" value="no_rule" />
+          No rule fits
+        </label>
+        <textarea
+          v-if="answer === 'no_rule'"
+          v-model="noRuleWhy"
+          class="le-input le-textarea le-why-input"
+          rows="2"
+          placeholder="Why none fits: a one-off of one host, a call with no single right answer…"
+        />
+
+        <label v-if="canLeaveOpen" class="le-choice">
+          <input v-model="answer" type="radio" value="open" />
+          Leave it open for now
+        </label>
+      </fieldset>
+
       <!-- The composed document, shown before saving. The writer is agreeing
            to a title they can read, not to one assembled out of sight. -->
       <div v-if="previewTitle" class="le-preview">
@@ -254,7 +449,8 @@ onMounted(() => {
         </button>
         <!-- Says WHY it is disabled. A greyed button with no reason is the
              thing that gets clicked repeatedly and then worked around. -->
-        <span v-if="!canSave" class="le-muted le-why">
+        <span v-if="answerProblem" class="le-muted le-why">{{ answerProblem }}</span>
+        <span v-else-if="!canSave" class="le-muted le-why">
           A lesson needs both a trigger and a claim — without the trigger it
           would save and never reach anyone.
         </span>
@@ -382,4 +578,35 @@ onMounted(() => {
 .le-dupe p { margin: 0 0 0.6rem; line-height: 1.5; }
 .le-dupe-actions { display: flex; flex-wrap: wrap; gap: 0.6rem; align-items: center; }
 .le-link { color: var(--fs-accent); }
+
+.le-answer {
+  margin: 0;
+  padding: var(--fs-space-4);
+  border: 1px solid var(--fs-border-color);
+  border-radius: var(--fs-radius-lg);
+}
+.le-answer legend { padding: 0 var(--fs-space-1); }
+.le-choice {
+  display: flex;
+  align-items: baseline;
+  gap: var(--fs-space-2);
+  font-size: 0.9rem;
+  cursor: pointer;
+}
+.le-choice input { accent-color: var(--fs-accent); }
+.le-rules {
+  display: flex;
+  flex-direction: column;
+  align-items: flex-start;
+  gap: var(--fs-space-2);
+  padding-left: var(--fs-space-5);
+}
+.le-rule-when { display: block; }
+.le-why-input { margin-left: var(--fs-space-5); width: calc(100% - var(--fs-space-5)); }
+.le-small { padding: 0.2rem 0.7rem; font-size: var(--fs-size-label); }
+.le-ghost:disabled { opacity: var(--fs-disabled-opacity); cursor: default; }
 </style>
+
+<!-- `.rule-chip-preference` marks a preference among the rules offered, as it
+     does everywhere else a rule's kind is shown. -->
+<style src="@/assets/rules-shared.css" />

src/scribe/routes/lessons.py

@@ -119,6 +119,34 @@ async def lessons_taught_by_route(record_id: int):
     })
 
 
+@lessons_bp.route("/rule-candidates", methods=["GET"])
+@login_required
+async def lesson_rule_candidates_route():
+    """The rules a lesson being written resembles, BEFORE it is saved
+    (milestone 440, #4658).
+
+    The create door offers the same list in its reply when neither answer was
+    given. A form needs it earlier, so the writer can name the rule in the
+    same save while the situation is still in mind. Query: `what`,
+    `when_to_apply`, optional `project_id`. Above the `/<int:lesson_id>`
+    routes for the reason `taught-by` is.
+
+    `candidates` is null when the search could not run. That means
+    "unavailable", which is different from "nothing resembles it" (an empty
+    list).
+    """
+    uid = get_current_user_id()
+    what = (request.args.get("what") or "").strip()
+    when_to_apply = (request.args.get("when_to_apply") or "").strip()
+    if not what and not when_to_apply:
+        return jsonify({"error": "what or when_to_apply is required"}), 400
+    project_id = request.args.get("project_id", type=int) or None
+    candidates = await lesson_rules_svc.rule_candidates(
+        uid, what, when_to_apply, project_id,
+    )
+    return jsonify({"candidates": candidates})
+
+
 @lessons_bp.route("", methods=["POST"])
 @login_required
 async def create_lesson_route():

tests/test_lesson_rule_link_ui.py

@@ -16,6 +16,7 @@ side fails here rather than rendering a page that quietly reads wrong:
 """
 from __future__ import annotations
 
+import inspect
 import re
 from pathlib import Path
 
@@ -85,7 +86,11 @@ def test_both_pages_link_through_to_the_other_record():
 def test_the_kind_and_state_markers_reuse_rule_chip():
     """No new chip style where `.rule-chip` serves: both components load the
     shared sheet and neither re-declares the class in its own block."""
-    for rel in ("views/LessonDetailView.vue", "components/rules/RuleEditorSlideOver.vue"):
+    for rel in (
+        "views/LessonDetailView.vue",
+        "components/rules/RuleEditorSlideOver.vue",
+        "views/LessonEditorView.vue",
+    ):
         src = _read(rel)
         assert 'class="rule-chip' in _template(src), rel
         assert '<style src="@/assets/rules-shared.css" />' in src, rel
@@ -115,3 +120,56 @@ def test_no_page_spells_its_own_write_check():
         and re.search(r'[!=]==\s*"(?:viewer|editor|edit)"', p.read_text())
     ]
     assert offenders == []
+
+
+# ── #4658: the web editor records the answer ─────────────────────────────────
+
+
+def _payload_fn(view: str) -> str:
+    start = view.index("function answerPayload(")
+    return view[start:view.index("\n}\n", start)]
+
+
+def test_the_editor_sends_the_answer_under_the_names_both_routes_read():
+    """Rule 33's contract check: a field the form sends under another name is
+    dropped without an error, and the lesson saves unjudged."""
+    from scribe.routes import lessons as routes
+
+    body = _payload_fn(_read("views/LessonEditorView.vue"))
+    assert "rule_ids:" in body and "no_rule:" in body
+    for route in (routes.create_lesson_route, routes.update_lesson_route):
+        src = inspect.getsource(route)
+        assert 'data.get("rule_ids")' in src or 'data["rule_ids"]' in src, route.__name__
+        assert 'data.get("no_rule")' in src, route.__name__
+
+
+def test_the_editor_offers_the_three_answers():
+    tpl = _template(_read("views/LessonEditorView.vue"))
+    offered = set(re.findall(r'v-model="answer" type="radio" value="([a-z_]+)"', tpl))
+    assert offered == {"rules", "no_rule", "open"}
+
+
+def test_leaving_it_open_after_linking_clears_the_links_rather_than_nothing():
+    """`rule_ids` omitted leaves the links alone; an empty list is what turns a
+    linked lesson back into an open one. The editor must send the list."""
+    body = _payload_fn(_read("views/LessonEditorView.vue"))
+    assert "{ rule_ids: [] }" in body
+
+
+def test_the_candidates_route_is_where_the_client_asks_and_above_the_id_route():
+    from scribe.routes import lessons as routes
+
+    src = inspect.getsource(routes)
+    assert src.index('"/rule-candidates"') < src.index('"/<int:lesson_id>"')
+    assert "/api/lessons/rule-candidates" in _read("api/lessons.ts")
+
+
+def test_the_candidates_route_keeps_unavailable_apart_from_none():
+    """The service returns None when the search could not run. The route has
+    to pass that through as null, not coerce it to an empty list, because
+    "nothing resembles it" is a claim the editor shows to the writer."""
+    from scribe.routes import lessons as routes
+
+    src = inspect.getsource(routes.lesson_rule_candidates_route)
+    assert 'jsonify({"candidates": candidates})' in src
+    assert "or []" not in src