Lessons step 7 — a lesson is readable, writable and browsable by a human (#3734) #169
@@ -0,0 +1,146 @@
|
|||||||
|
import type { RecordUsage } from "@/types/usage";
|
||||||
|
|
||||||
|
import { apiGet, apiPost, apiPatch, apiDelete } from "@/api/client";
|
||||||
|
|
||||||
|
/** A lesson: a transferable insight, retrievable by the SITUATION it applies
|
||||||
|
* to rather than by its topic.
|
||||||
|
*
|
||||||
|
* The fields mirror what the backend composes and reads back
|
||||||
|
* (`services/lessons.py::lesson_to_dict`), not the stored row. `title` and
|
||||||
|
* `body` are DERIVED — the service builds them from `what`, `when_to_apply`
|
||||||
|
* and `insight` — so an editor sends the three parts and never the document.
|
||||||
|
* That is the whole design: the trigger ends up in the title and again at the
|
||||||
|
* head of the body, which is what makes a lesson rank on when it applies. */
|
||||||
|
export interface Lesson {
|
||||||
|
id: number;
|
||||||
|
/** The composed document title, `{what} — {when_to_apply}`. Read-only. */
|
||||||
|
title: string;
|
||||||
|
/** The composed body. Read-only — edit `insight` instead. */
|
||||||
|
body: string;
|
||||||
|
/** The claim itself, as you would say it. */
|
||||||
|
what: string;
|
||||||
|
/** WHEN this applies — the situation, in the words it presents itself in.
|
||||||
|
* The entire retrieval story: a lesson without one saves, reads correctly
|
||||||
|
* and never surfaces, so both doors refuse an empty one. */
|
||||||
|
when_to_apply: string;
|
||||||
|
/** The body with the composed lines stripped — what an edit form binds to,
|
||||||
|
* so saving doesn't accumulate a copy of the trigger line per save. */
|
||||||
|
insight: string;
|
||||||
|
/** Ids of the records that taught this — issues, tasks or notes. */
|
||||||
|
learned_from: number[];
|
||||||
|
/** The same sources RESOLVED, sent by the detail route only. A bare "#4181"
|
||||||
|
* on a page tells a reader nothing about whether it is worth opening, and
|
||||||
|
* the provenance is the point of a lesson — one that loses its incidents
|
||||||
|
* loses its evidence. A source that has been deleted drops out rather than
|
||||||
|
* rendering a link to nothing. */
|
||||||
|
learned_from_records?: {
|
||||||
|
id: number;
|
||||||
|
title: string;
|
||||||
|
note_type: string;
|
||||||
|
is_task: boolean;
|
||||||
|
task_kind: string | null;
|
||||||
|
status: string | null;
|
||||||
|
}[];
|
||||||
|
tags: string[];
|
||||||
|
note_type: string;
|
||||||
|
/** Where it was LEARNED. Kept as a fact, but not a limit on where it can be
|
||||||
|
* found: a lesson is retrievable from every project (milestone 385 step 3). */
|
||||||
|
project_id: number | null;
|
||||||
|
permission?: string;
|
||||||
|
created_at: string | null;
|
||||||
|
updated_at: string | null;
|
||||||
|
systems?: { id: number; name: string }[];
|
||||||
|
usage?: RecordUsage;
|
||||||
|
/** Set when another user owns this record. */
|
||||||
|
shared?: boolean;
|
||||||
|
owner?: string | null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** A row in the browse listing — the trigger travels with it, because a list
|
||||||
|
* of lessons without their triggers is a list of claims with the half that
|
||||||
|
* says when each one matters left off. */
|
||||||
|
export interface LessonListRow {
|
||||||
|
id: number;
|
||||||
|
title: string;
|
||||||
|
tags: string[];
|
||||||
|
when_to_apply?: string;
|
||||||
|
snippet?: string;
|
||||||
|
shared?: boolean;
|
||||||
|
owner?: string | null;
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface LessonListResponse {
|
||||||
|
lessons: LessonListRow[];
|
||||||
|
total: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** What the create/update forms send. `what` and `when_to_apply` are required
|
||||||
|
* on create; every field is optional on update, and the service re-composes
|
||||||
|
* the whole document from the merged set — so a partial save can never leave
|
||||||
|
* the title and body disagreeing about the trigger. */
|
||||||
|
export interface LessonPayload {
|
||||||
|
what?: string;
|
||||||
|
when_to_apply?: string;
|
||||||
|
insight?: string;
|
||||||
|
learned_from?: number[];
|
||||||
|
tags?: string[];
|
||||||
|
project_id?: number | null;
|
||||||
|
system_ids?: number[];
|
||||||
|
/** 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. */
|
||||||
|
force?: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function listLessons(params: {
|
||||||
|
q?: string;
|
||||||
|
tag?: string;
|
||||||
|
project_id?: number;
|
||||||
|
limit?: number;
|
||||||
|
offset?: number;
|
||||||
|
} = {}): Promise<LessonListResponse> {
|
||||||
|
const qs = new URLSearchParams();
|
||||||
|
if (params.q) qs.set("q", params.q);
|
||||||
|
if (params.tag) qs.set("tag", params.tag);
|
||||||
|
if (params.project_id) qs.set("project_id", String(params.project_id));
|
||||||
|
if (params.limit != null) qs.set("limit", String(params.limit));
|
||||||
|
if (params.offset != null) qs.set("offset", String(params.offset));
|
||||||
|
const suffix = qs.toString() ? `?${qs}` : "";
|
||||||
|
return apiGet<LessonListResponse>(`/api/lessons${suffix}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function getLesson(id: number): Promise<Lesson> {
|
||||||
|
return apiGet<Lesson>(`/api/lessons/${id}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function createLesson(payload: LessonPayload): Promise<Lesson> {
|
||||||
|
return apiPost<Lesson>("/api/lessons", payload);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function updateLesson(
|
||||||
|
id: number,
|
||||||
|
payload: LessonPayload,
|
||||||
|
): Promise<Lesson> {
|
||||||
|
return apiPatch<Lesson>(`/api/lessons/${id}`, payload);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Trash, not erase — recoverable. `apiDelete` discards the body, which is the
|
||||||
|
* established shape here (snippets delete the same way): the batch id is in
|
||||||
|
* the response, but no caller has needed it and inventing a second delete
|
||||||
|
* helper to carry it would be the duplication, not the feature. */
|
||||||
|
export function deleteLesson(id: number): Promise<void> {
|
||||||
|
return apiDelete(`/api/lessons/${id}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The lessons drawn FROM one record — the reverse of `learned_from`.
|
||||||
|
*
|
||||||
|
* The direction that gets forgotten, and arguably the more useful one: a
|
||||||
|
* reader opening an old issue wants to know what was learned from it, and
|
||||||
|
* without this the relation is only navigable from the lesson's side. */
|
||||||
|
export function lessonsTaughtBy(
|
||||||
|
recordId: number,
|
||||||
|
): Promise<{ lessons: Lesson[]; taught_by: number }> {
|
||||||
|
return apiGet<{ lessons: Lesson[]; taught_by: number }>(
|
||||||
|
`/api/lessons/taught-by/${recordId}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -0,0 +1,93 @@
|
|||||||
|
<script setup lang="ts">
|
||||||
|
/**
|
||||||
|
* "What was learned from this record" — the reverse of a lesson's
|
||||||
|
* `learned_from`.
|
||||||
|
*
|
||||||
|
* THE DIRECTION THAT GETS FORGOTTEN, and arguably the more useful one. The
|
||||||
|
* forward link is easy to remember because the lesson's author types it; this
|
||||||
|
* one has no author and so tends never to get built. A reader opening an old
|
||||||
|
* issue wants to know what came out of it, and without this the relation is
|
||||||
|
* navigable only from the lesson's side.
|
||||||
|
*
|
||||||
|
* A COMPONENT rather than markup inside the task editor, because the same
|
||||||
|
* question is worth answering on any record a lesson can cite — an issue, a
|
||||||
|
* spike, a dev-log. One panel, mounted wherever that is true, instead of the
|
||||||
|
* shape being written a second time the first time someone wants it on notes
|
||||||
|
* (#3207).
|
||||||
|
*
|
||||||
|
* SILENT WHEN EMPTY. Most records taught no lesson, and a panel that renders
|
||||||
|
* "None yet" on every page is a panel people learn to skip — which costs the
|
||||||
|
* pages where it does have something to say.
|
||||||
|
*/
|
||||||
|
import { onMounted, ref, watch } from "vue";
|
||||||
|
|
||||||
|
import { lessonsTaughtBy, type Lesson } from "@/api/lessons";
|
||||||
|
|
||||||
|
const props = defineProps<{ recordId: number }>();
|
||||||
|
|
||||||
|
const lessons = ref<Lesson[]>([]);
|
||||||
|
// No error surface on purpose: this is a secondary panel beside the record the
|
||||||
|
// reader actually came for, and a red box about a failed side-query would be
|
||||||
|
// louder than the thing it failed to fetch. It stays silent and stays absent.
|
||||||
|
const loaded = ref(false);
|
||||||
|
|
||||||
|
async function load() {
|
||||||
|
loaded.value = false;
|
||||||
|
lessons.value = [];
|
||||||
|
if (!props.recordId) return;
|
||||||
|
try {
|
||||||
|
const res = await lessonsTaughtBy(props.recordId);
|
||||||
|
lessons.value = res.lessons;
|
||||||
|
} catch {
|
||||||
|
lessons.value = [];
|
||||||
|
} finally {
|
||||||
|
loaded.value = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
watch(() => props.recordId, load);
|
||||||
|
onMounted(load);
|
||||||
|
</script>
|
||||||
|
|
||||||
|
<template>
|
||||||
|
<section v-if="loaded && lessons.length" class="ltp">
|
||||||
|
<h3 class="ltp-label">What was learned from this</h3>
|
||||||
|
<ul class="ltp-list">
|
||||||
|
<li v-for="l in lessons" :key="l.id" class="ltp-item">
|
||||||
|
<router-link :to="`/lessons/${l.id}`" class="ltp-link">
|
||||||
|
{{ l.what || l.title }}
|
||||||
|
</router-link>
|
||||||
|
<!-- The trigger travels with the row. A lesson listed without it is a
|
||||||
|
claim with the half that says when it matters left off. -->
|
||||||
|
<p v-if="l.when_to_apply" class="ltp-trigger">
|
||||||
|
{{ l.when_to_apply }}
|
||||||
|
</p>
|
||||||
|
</li>
|
||||||
|
</ul>
|
||||||
|
</section>
|
||||||
|
</template>
|
||||||
|
|
||||||
|
<style scoped>
|
||||||
|
.ltp {
|
||||||
|
margin-top: 1.5rem;
|
||||||
|
padding-top: 1rem;
|
||||||
|
border-top: 1px solid var(--fs-border-color);
|
||||||
|
}
|
||||||
|
.ltp-label {
|
||||||
|
margin: 0 0 0.6rem;
|
||||||
|
font-size: 0.72rem;
|
||||||
|
font-weight: 500;
|
||||||
|
text-transform: uppercase;
|
||||||
|
letter-spacing: 0.04em;
|
||||||
|
color: var(--fs-text-tertiary);
|
||||||
|
}
|
||||||
|
.ltp-list { list-style: none; margin: 0; padding: 0; }
|
||||||
|
.ltp-item { margin-bottom: 0.7rem; }
|
||||||
|
.ltp-link { color: var(--fs-accent); font-size: 0.9rem; }
|
||||||
|
.ltp-trigger {
|
||||||
|
margin: 0.15rem 0 0;
|
||||||
|
color: var(--fs-text-secondary);
|
||||||
|
font-size: 0.82rem;
|
||||||
|
line-height: 1.45;
|
||||||
|
}
|
||||||
|
</style>
|
||||||
@@ -69,6 +69,25 @@ const router = createRouter({
|
|||||||
name: "note-edit",
|
name: "note-edit",
|
||||||
component: () => import("@/views/NoteEditorView.vue"),
|
component: () => import("@/views/NoteEditorView.vue"),
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
// Lessons have no list view of their own: the Knowledge browse surface
|
||||||
|
// is where every kind is enumerated, and a second list would be a
|
||||||
|
// second vocabulary to keep in step with it (#3128's defect, in
|
||||||
|
// advance). `/knowledge?type=lesson` is the list.
|
||||||
|
path: "/lessons/new",
|
||||||
|
name: "lesson-new",
|
||||||
|
component: () => import("@/views/LessonEditorView.vue"),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
path: "/lessons/:id",
|
||||||
|
name: "lesson-view",
|
||||||
|
component: () => import("@/views/LessonDetailView.vue"),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
path: "/lessons/:id/edit",
|
||||||
|
name: "lesson-edit",
|
||||||
|
component: () => import("@/views/LessonEditorView.vue"),
|
||||||
|
},
|
||||||
{
|
{
|
||||||
path: "/snippets",
|
path: "/snippets",
|
||||||
name: "snippets",
|
name: "snippets",
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import {
|
|||||||
FileText,
|
FileText,
|
||||||
CheckSquare,
|
CheckSquare,
|
||||||
Workflow,
|
Workflow,
|
||||||
|
Lightbulb,
|
||||||
Search,
|
Search,
|
||||||
Share2,
|
Share2,
|
||||||
ShieldCheck,
|
ShieldCheck,
|
||||||
@@ -26,7 +27,7 @@ const router = useRouter();
|
|||||||
|
|
||||||
interface KnowledgeItem {
|
interface KnowledgeItem {
|
||||||
id: number;
|
id: number;
|
||||||
note_type: "note" | "task" | "process" | "snippet";
|
note_type: "note" | "task" | "process" | "snippet" | "lesson";
|
||||||
title: string;
|
title: string;
|
||||||
snippet: string;
|
snippet: string;
|
||||||
tags: string[];
|
tags: string[];
|
||||||
@@ -46,7 +47,8 @@ interface KnowledgeItem {
|
|||||||
|
|
||||||
// ─── The facet vocabulary ─────────────────────────────────────────────────────
|
// ─── The facet vocabulary ─────────────────────────────────────────────────────
|
||||||
// Mirrors services/knowledge._FACETS, which is where it is defined for real.
|
// Mirrors services/knowledge._FACETS, which is where it is defined for real.
|
||||||
// A facet spans BOTH typing axes — a record TYPE (note / process / snippet) or
|
// A facet spans BOTH typing axes — a record TYPE (note / process / snippet /
|
||||||
|
// lesson) or
|
||||||
// a task KIND (`task` for any, else issue / spike) — because that is what this
|
// a task KIND (`task` for any, else issue / spike) — because that is what this
|
||||||
// feed actually holds.
|
// feed actually holds.
|
||||||
//
|
//
|
||||||
@@ -54,7 +56,7 @@ interface KnowledgeItem {
|
|||||||
// it has no chip: retired in 0066, it kept a chip of its own for longer than
|
// it has no chip: retired in 0066, it kept a chip of its own for longer than
|
||||||
// `issue` — 17% of every task here — went without one (#3128). Those rows are
|
// `issue` — 17% of every task here — went without one (#3128). Those rows are
|
||||||
// still reachable under Tasks, wearing a Plan badge.
|
// still reachable under Tasks, wearing a Plan badge.
|
||||||
type Facet = "" | "note" | "task" | "issue" | "spike" | "snippet" | "process";
|
type Facet = "" | "note" | "task" | "issue" | "spike" | "snippet" | "process" | "lesson";
|
||||||
|
|
||||||
// The facets that select TASKS. Kinds are subsets of `task`, so any of them
|
// The facets that select TASKS. Kinds are subsets of `task`, so any of them
|
||||||
// means the duplicate report should be comparing tasks.
|
// means the duplicate report should be comparing tasks.
|
||||||
@@ -67,6 +69,7 @@ const FACET_CHIPS: [Exclude<Facet, "">, string][] = [
|
|||||||
["spike", "Spikes"],
|
["spike", "Spikes"],
|
||||||
["snippet", "Snippets"],
|
["snippet", "Snippets"],
|
||||||
["process", "Processes"],
|
["process", "Processes"],
|
||||||
|
["lesson", "Lessons"],
|
||||||
];
|
];
|
||||||
|
|
||||||
// ─── View mode ────────────────────────────────────────────────────────────────
|
// ─── View mode ────────────────────────────────────────────────────────────────
|
||||||
@@ -153,6 +156,11 @@ function createNew(type: string) {
|
|||||||
newNoteMenuOpen.value = false;
|
newNoteMenuOpen.value = false;
|
||||||
if (type === "task") {
|
if (type === "task") {
|
||||||
router.push("/tasks/new");
|
router.push("/tasks/new");
|
||||||
|
} else if (type === "lesson") {
|
||||||
|
// Its own editor, not /notes/new?type=lesson: the note editor offers one
|
||||||
|
// markdown box, and a lesson written that way saves without a trigger and
|
||||||
|
// never surfaces. The form has to ASK for the field by name.
|
||||||
|
router.push("/lessons/new");
|
||||||
} else {
|
} else {
|
||||||
router.push(type === "note" ? "/notes/new" : `/notes/new?type=${type}`);
|
router.push(type === "note" ? "/notes/new" : `/notes/new?type=${type}`);
|
||||||
}
|
}
|
||||||
@@ -328,6 +336,8 @@ function openItem(item: KnowledgeItem) {
|
|||||||
router.push(`/tasks/${item.id}`);
|
router.push(`/tasks/${item.id}`);
|
||||||
} else if (item.note_type === 'snippet') {
|
} else if (item.note_type === 'snippet') {
|
||||||
router.push(`/snippets/${item.id}`);
|
router.push(`/snippets/${item.id}`);
|
||||||
|
} else if (item.note_type === 'lesson') {
|
||||||
|
router.push(`/lessons/${item.id}`);
|
||||||
} else {
|
} else {
|
||||||
router.push(`/notes/${item.id}`);
|
router.push(`/notes/${item.id}`);
|
||||||
}
|
}
|
||||||
@@ -417,6 +427,10 @@ onUnmounted(() => {
|
|||||||
<Workflow :size="16" />
|
<Workflow :size="16" />
|
||||||
Process
|
Process
|
||||||
</button>
|
</button>
|
||||||
|
<button @click="createNew('lesson')">
|
||||||
|
<Lightbulb :size="16" />
|
||||||
|
Lesson
|
||||||
|
</button>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
@@ -574,6 +588,7 @@ onUnmounted(() => {
|
|||||||
<span v-else-if="item.note_type === 'task'">{{ item.task_kind === 'plan' ? 'Plan' : 'Task' }}</span>
|
<span v-else-if="item.note_type === 'task'">{{ item.task_kind === 'plan' ? 'Plan' : 'Task' }}</span>
|
||||||
<span v-else-if="item.note_type === 'process'">Process</span>
|
<span v-else-if="item.note_type === 'process'">Process</span>
|
||||||
<span v-else-if="item.note_type === 'snippet'">Snippet</span>
|
<span v-else-if="item.note_type === 'snippet'">Snippet</span>
|
||||||
|
<span v-else-if="item.note_type === 'lesson'">Lesson</span>
|
||||||
</span>
|
</span>
|
||||||
<!-- Kind sits BESIDE the type badge, not inside it: the type badge
|
<!-- Kind sits BESIDE the type badge, not inside it: the type badge
|
||||||
speaks the vocabulary of this view's type filter (note / task /
|
speaks the vocabulary of this view's type filter (note / task /
|
||||||
@@ -962,7 +977,11 @@ onUnmounted(() => {
|
|||||||
isn't an alarm reads better as plain. Standard body pair, so the contrast
|
isn't an alarm reads better as plain. Standard body pair, so the contrast
|
||||||
is the one the palette already guarantees. */
|
is the one the palette already guarantees. */
|
||||||
.badge--snippet,
|
.badge--snippet,
|
||||||
.badge--process { background: var(--fs-surface-raised); color: var(--fs-text-secondary); }
|
.badge--process,
|
||||||
|
/* A lesson joins the neutral pair for the same reason, and one of its own: it
|
||||||
|
binds nobody. A hue here would make the softest record in the corpus look
|
||||||
|
like the loudest, beside a rule that actually is binding. */
|
||||||
|
.badge--lesson { background: var(--fs-surface-raised); color: var(--fs-text-secondary); }
|
||||||
|
|
||||||
.k-card-body { flex: 1; padding-right: 40px; }
|
.k-card-body { flex: 1; padding-right: 40px; }
|
||||||
.k-card-title {
|
.k-card-title {
|
||||||
|
|||||||
@@ -0,0 +1,288 @@
|
|||||||
|
<script setup lang="ts">
|
||||||
|
/**
|
||||||
|
* Read a LESSON, and see both ends of what it came from.
|
||||||
|
*
|
||||||
|
* THE TRIGGER LEADS. A lesson is retrieved by the situation it applies to, so
|
||||||
|
* the page opens with that situation rather than with the claim — the same
|
||||||
|
* ordering the editor uses, and for the same reason: the trigger is the half
|
||||||
|
* a reader needs first to decide whether this is for them.
|
||||||
|
*
|
||||||
|
* GLOBAL BY DEFAULT HAS TO BE LEGIBLE. A lesson written on one project will
|
||||||
|
* surface on another, which reads as a bug unless the page says why. The
|
||||||
|
* origin line does exactly that, and says it as a property of the kind rather
|
||||||
|
* than as an apology.
|
||||||
|
*
|
||||||
|
* IT BINDS NOBODY, and the page says so once, plainly. That sentence is the
|
||||||
|
* difference between this kind and a rule, and it is the reason the kind
|
||||||
|
* exists (#3727) — sessions were proposing rules for things that should never
|
||||||
|
* have bound anyone.
|
||||||
|
*/
|
||||||
|
import { computed, onMounted, ref, watch } from "vue";
|
||||||
|
import { useRoute, useRouter } from "vue-router";
|
||||||
|
|
||||||
|
import { apiErrorMessage } from "@/api/client";
|
||||||
|
import { deleteLesson, getLesson, type Lesson } from "@/api/lessons";
|
||||||
|
import ConfirmDialog from "@/components/ConfirmDialog.vue";
|
||||||
|
import TagPill from "@/components/TagPill.vue";
|
||||||
|
import { useToastStore } from "@/stores/toast";
|
||||||
|
import { renderMarkdown } from "@/utils/markdown";
|
||||||
|
|
||||||
|
const route = useRoute();
|
||||||
|
const router = useRouter();
|
||||||
|
const toast = useToastStore();
|
||||||
|
|
||||||
|
const lesson = ref<Lesson | null>(null);
|
||||||
|
const loading = ref(true);
|
||||||
|
const error = ref<string | null>(null);
|
||||||
|
const confirming = ref(false);
|
||||||
|
|
||||||
|
const lessonId = computed(() => Number(route.params.id));
|
||||||
|
|
||||||
|
/** The insight rendered — the body minus the lines the service composed, so
|
||||||
|
* the trigger is not printed twice on a page that already leads with it. */
|
||||||
|
const insightHtml = computed(() =>
|
||||||
|
lesson.value?.insight ? renderMarkdown(lesson.value.insight) : "",
|
||||||
|
);
|
||||||
|
|
||||||
|
/** Where each source opens. A task and a note live at different routes, and a
|
||||||
|
* link that guesses wrong is worse than one that is plain. */
|
||||||
|
function sourceHref(rec: { id: number; is_task?: boolean; note_type?: string }) {
|
||||||
|
if (rec.is_task) return `/tasks/${rec.id}`;
|
||||||
|
if (rec.note_type === "snippet") return `/snippets/${rec.id}`;
|
||||||
|
if (rec.note_type === "lesson") return `/lessons/${rec.id}`;
|
||||||
|
return `/notes/${rec.id}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function load() {
|
||||||
|
loading.value = true;
|
||||||
|
error.value = null;
|
||||||
|
try {
|
||||||
|
lesson.value = await getLesson(lessonId.value);
|
||||||
|
} catch (e) {
|
||||||
|
error.value = apiErrorMessage(e, "Failed to load this lesson");
|
||||||
|
} finally {
|
||||||
|
loading.value = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async function remove() {
|
||||||
|
confirming.value = false;
|
||||||
|
try {
|
||||||
|
await deleteLesson(lessonId.value);
|
||||||
|
toast.show("Lesson moved to trash");
|
||||||
|
router.push("/knowledge?type=lesson");
|
||||||
|
} catch (e) {
|
||||||
|
toast.show(apiErrorMessage(e, "Failed to delete this lesson"), "error");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
watch(lessonId, load);
|
||||||
|
onMounted(load);
|
||||||
|
</script>
|
||||||
|
|
||||||
|
<template>
|
||||||
|
<div class="lesson-detail">
|
||||||
|
<p v-if="loading" class="ld-muted">Loading…</p>
|
||||||
|
<p v-else-if="error" class="ld-error" role="alert">{{ error }}</p>
|
||||||
|
|
||||||
|
<article v-else-if="lesson">
|
||||||
|
<header class="ld-head">
|
||||||
|
<span class="ld-kind">Lesson</span>
|
||||||
|
<!-- Said once, plainly. It is the whole difference from a rule. -->
|
||||||
|
<span class="ld-binds">binds nobody</span>
|
||||||
|
</header>
|
||||||
|
|
||||||
|
<!-- The trigger leads, in its own block: it is what a reader needs first
|
||||||
|
to decide whether this applies to them. -->
|
||||||
|
<section class="ld-trigger">
|
||||||
|
<h2 class="ld-trigger-label">When this applies</h2>
|
||||||
|
<p class="ld-trigger-text">{{ lesson.when_to_apply }}</p>
|
||||||
|
</section>
|
||||||
|
|
||||||
|
<h1 class="ld-what">{{ lesson.what }}</h1>
|
||||||
|
|
||||||
|
<div
|
||||||
|
v-if="insightHtml"
|
||||||
|
class="ld-insight markdown-body"
|
||||||
|
v-html="insightHtml"
|
||||||
|
/>
|
||||||
|
<p v-else class="ld-muted ld-empty">
|
||||||
|
No detail was recorded — the claim above is the whole lesson.
|
||||||
|
</p>
|
||||||
|
|
||||||
|
<!-- What taught it. The provenance is the point: a lesson that loses its
|
||||||
|
incidents loses its evidence. -->
|
||||||
|
<section
|
||||||
|
v-if="lesson.learned_from_records?.length"
|
||||||
|
class="ld-panel"
|
||||||
|
>
|
||||||
|
<h2 class="ld-panel-label">Learned from</h2>
|
||||||
|
<ul class="ld-sources">
|
||||||
|
<li v-for="rec in lesson.learned_from_records" :key="rec.id">
|
||||||
|
<router-link :to="sourceHref(rec)" class="ld-link">
|
||||||
|
{{ rec.title }}
|
||||||
|
</router-link>
|
||||||
|
<span v-if="rec.status" class="ld-source-status">{{ rec.status }}</span>
|
||||||
|
</li>
|
||||||
|
</ul>
|
||||||
|
</section>
|
||||||
|
|
||||||
|
<section v-if="lesson.tags?.length" class="ld-tags">
|
||||||
|
<TagPill v-for="t in lesson.tags" :key="t" :tag="t" />
|
||||||
|
</section>
|
||||||
|
|
||||||
|
<!-- Global-by-default, stated as a property rather than an apology. A
|
||||||
|
lesson meeting you on a project it was not written on is the kind
|
||||||
|
working, and the page has to say so or it reads as a bug. -->
|
||||||
|
<footer class="ld-origin">
|
||||||
|
<p v-if="lesson.project_id">
|
||||||
|
Learned on another project, and offered everywhere — a lesson is
|
||||||
|
retrieved by the situation it names, never by where it was written.
|
||||||
|
</p>
|
||||||
|
<p v-else>
|
||||||
|
Not tied to a project. Offered wherever the situation it names comes
|
||||||
|
up.
|
||||||
|
</p>
|
||||||
|
</footer>
|
||||||
|
|
||||||
|
<div class="ld-actions">
|
||||||
|
<router-link :to="`/lessons/${lesson.id}/edit`" class="ld-primary">
|
||||||
|
Edit
|
||||||
|
</router-link>
|
||||||
|
<button type="button" class="ld-ghost" @click="confirming = true">
|
||||||
|
Delete
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
</article>
|
||||||
|
|
||||||
|
<ConfirmDialog
|
||||||
|
v-if="confirming"
|
||||||
|
title="Delete this lesson?"
|
||||||
|
message="It moves to the trash and can be restored."
|
||||||
|
@confirm="remove"
|
||||||
|
@cancel="confirming = false"
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
</template>
|
||||||
|
|
||||||
|
<style scoped>
|
||||||
|
.lesson-detail {
|
||||||
|
max-width: 780px;
|
||||||
|
margin: 0 auto;
|
||||||
|
padding: var(--fs-layout-page-pad);
|
||||||
|
color: var(--fs-text-primary);
|
||||||
|
}
|
||||||
|
|
||||||
|
.ld-head {
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
gap: 0.5rem;
|
||||||
|
margin-bottom: 1rem;
|
||||||
|
}
|
||||||
|
/* Neutral, matching the browse badge. A hue here would make the softest
|
||||||
|
record in the corpus look like the loudest. */
|
||||||
|
.ld-kind {
|
||||||
|
padding: 0.15rem 0.5rem;
|
||||||
|
border-radius: 10px;
|
||||||
|
background: var(--fs-surface-raised);
|
||||||
|
color: var(--fs-text-secondary);
|
||||||
|
font-size: 0.7rem;
|
||||||
|
text-transform: uppercase;
|
||||||
|
letter-spacing: 0.04em;
|
||||||
|
font-weight: 500;
|
||||||
|
}
|
||||||
|
.ld-binds { color: var(--fs-text-tertiary); font-size: 0.8rem; }
|
||||||
|
|
||||||
|
.ld-trigger {
|
||||||
|
padding: 0.9rem 1rem;
|
||||||
|
border-left: 2px solid var(--fs-accent);
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: var(--fs-surface-raised);
|
||||||
|
margin-bottom: 1rem;
|
||||||
|
}
|
||||||
|
.ld-trigger-label {
|
||||||
|
margin: 0 0 0.3rem;
|
||||||
|
font-size: 0.72rem;
|
||||||
|
font-weight: 500;
|
||||||
|
text-transform: uppercase;
|
||||||
|
letter-spacing: 0.04em;
|
||||||
|
color: var(--fs-text-tertiary);
|
||||||
|
}
|
||||||
|
.ld-trigger-text { margin: 0; font-size: 0.98rem; line-height: 1.5; }
|
||||||
|
|
||||||
|
.ld-what {
|
||||||
|
margin: 0 0 1.25rem;
|
||||||
|
font-size: 1.25rem;
|
||||||
|
font-weight: 500;
|
||||||
|
line-height: 1.35;
|
||||||
|
}
|
||||||
|
|
||||||
|
.ld-insight { line-height: 1.6; font-size: 0.93rem; }
|
||||||
|
.ld-empty { font-style: italic; }
|
||||||
|
|
||||||
|
.ld-panel {
|
||||||
|
margin-top: 1.75rem;
|
||||||
|
padding-top: 1rem;
|
||||||
|
border-top: 1px solid var(--fs-border-color);
|
||||||
|
}
|
||||||
|
.ld-panel-label {
|
||||||
|
margin: 0 0 0.5rem;
|
||||||
|
font-size: 0.72rem;
|
||||||
|
font-weight: 500;
|
||||||
|
text-transform: uppercase;
|
||||||
|
letter-spacing: 0.04em;
|
||||||
|
color: var(--fs-text-tertiary);
|
||||||
|
}
|
||||||
|
.ld-sources { margin: 0; padding-left: 1.1rem; }
|
||||||
|
.ld-sources li { margin-bottom: 0.3rem; font-size: 0.9rem; }
|
||||||
|
.ld-link { color: var(--fs-accent); }
|
||||||
|
.ld-source-status {
|
||||||
|
margin-left: 0.4rem;
|
||||||
|
color: var(--fs-text-tertiary);
|
||||||
|
font-size: 0.78rem;
|
||||||
|
}
|
||||||
|
|
||||||
|
.ld-tags { display: flex; flex-wrap: wrap; gap: 0.35rem; margin-top: 1.25rem; }
|
||||||
|
|
||||||
|
.ld-origin {
|
||||||
|
margin-top: 1.5rem;
|
||||||
|
color: var(--fs-text-secondary);
|
||||||
|
font-size: 0.82rem;
|
||||||
|
line-height: 1.5;
|
||||||
|
}
|
||||||
|
.ld-origin p { margin: 0; }
|
||||||
|
|
||||||
|
.ld-actions {
|
||||||
|
display: flex;
|
||||||
|
gap: 0.6rem;
|
||||||
|
margin-top: 1.75rem;
|
||||||
|
padding-top: 1rem;
|
||||||
|
border-top: 1px solid var(--fs-border-color);
|
||||||
|
}
|
||||||
|
.ld-primary {
|
||||||
|
padding: 0.45rem 1rem;
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: var(--fs-accent);
|
||||||
|
color: var(--fs-accent-fg);
|
||||||
|
font-size: 0.88rem;
|
||||||
|
}
|
||||||
|
.ld-ghost {
|
||||||
|
padding: 0.45rem 1rem;
|
||||||
|
border: 1px solid var(--fs-border-color);
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: transparent;
|
||||||
|
color: var(--fs-text-primary);
|
||||||
|
font: inherit;
|
||||||
|
font-size: 0.88rem;
|
||||||
|
cursor: pointer;
|
||||||
|
}
|
||||||
|
|
||||||
|
.ld-muted { color: var(--fs-text-secondary); font-size: 0.88rem; }
|
||||||
|
.ld-error {
|
||||||
|
padding: 0.6rem 0.8rem;
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: color-mix(in srgb, var(--fs-error) 12%, var(--fs-surface-raised));
|
||||||
|
color: color-mix(in srgb, var(--fs-error) 55%, var(--fs-text-primary));
|
||||||
|
font-size: 0.88rem;
|
||||||
|
}
|
||||||
|
</style>
|
||||||
@@ -0,0 +1,385 @@
|
|||||||
|
<script setup lang="ts">
|
||||||
|
/**
|
||||||
|
* Write or edit a LESSON — a transferable insight, found by the situation it
|
||||||
|
* applies to rather than by its topic.
|
||||||
|
*
|
||||||
|
* THREE FIELDS, NOT ONE MARKDOWN BOX. The title and body are composed by the
|
||||||
|
* service from `what`, `when_to_apply` and `insight`; this form never sends a
|
||||||
|
* document. That is the design milestone 385 step 1 settled on, and the
|
||||||
|
* evidence for it is blunt: the snippet corpus carries a trigger on every
|
||||||
|
* record with no guard anywhere, because a service composes the title from a
|
||||||
|
* named parameter. What is at 100% is a named structured field — not a writer
|
||||||
|
* remembering a convention.
|
||||||
|
*
|
||||||
|
* THE TRIGGER IS THE FIELD THAT CANNOT BE MISSED, so it is given the most
|
||||||
|
* room, its own explanation, and a save button that refuses without it. A
|
||||||
|
* 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.
|
||||||
|
*/
|
||||||
|
import { computed, onMounted, ref, watch } from "vue";
|
||||||
|
import { useRoute, useRouter } from "vue-router";
|
||||||
|
|
||||||
|
import { apiErrorMessage } from "@/api/client";
|
||||||
|
import {
|
||||||
|
createLesson,
|
||||||
|
getLesson,
|
||||||
|
updateLesson,
|
||||||
|
type Lesson,
|
||||||
|
} from "@/api/lessons";
|
||||||
|
import ProjectSelector from "@/components/ProjectSelector.vue";
|
||||||
|
import TagInput from "@/components/TagInput.vue";
|
||||||
|
import { useNotesStore } from "@/stores/notes";
|
||||||
|
import { useToastStore } from "@/stores/toast";
|
||||||
|
|
||||||
|
const route = useRoute();
|
||||||
|
const router = useRouter();
|
||||||
|
const toast = useToastStore();
|
||||||
|
const notesStore = useNotesStore();
|
||||||
|
|
||||||
|
const lessonId = computed(() => {
|
||||||
|
const raw = route.params.id;
|
||||||
|
return raw ? Number(raw) : null;
|
||||||
|
});
|
||||||
|
const isEdit = computed(() => lessonId.value !== null);
|
||||||
|
|
||||||
|
const what = ref("");
|
||||||
|
const whenToApply = ref("");
|
||||||
|
const insight = ref("");
|
||||||
|
const tags = ref<string[]>([]);
|
||||||
|
const projectId = ref<number | null>(null);
|
||||||
|
const learnedFrom = ref<number[]>([]);
|
||||||
|
|
||||||
|
const loading = ref(false);
|
||||||
|
const saving = ref(false);
|
||||||
|
const error = ref<string | null>(null);
|
||||||
|
// The near-duplicate gate's answer, held so the writer can read it and then
|
||||||
|
// decide — rather than being silently overridden or silently blocked.
|
||||||
|
const duplicate = ref<{ id: number; title: string } | null>(null);
|
||||||
|
|
||||||
|
/** The composed title, shown live. The writer is agreeing to a document they
|
||||||
|
* can see, which is the same reason `create_rule` shows a rule's statement
|
||||||
|
* before asking for a yes. */
|
||||||
|
const previewTitle = computed(() => {
|
||||||
|
const subject = what.value.trim();
|
||||||
|
const trigger = whenToApply.value.trim();
|
||||||
|
if (subject && trigger) return `${subject} — ${trigger}`;
|
||||||
|
return subject || trigger;
|
||||||
|
});
|
||||||
|
|
||||||
|
const canSave = computed(
|
||||||
|
() => what.value.trim().length > 0 && whenToApply.value.trim().length > 0,
|
||||||
|
);
|
||||||
|
|
||||||
|
async function load() {
|
||||||
|
if (!isEdit.value || lessonId.value === null) return;
|
||||||
|
loading.value = true;
|
||||||
|
error.value = null;
|
||||||
|
try {
|
||||||
|
const lesson: Lesson = await getLesson(lessonId.value);
|
||||||
|
what.value = lesson.what;
|
||||||
|
whenToApply.value = lesson.when_to_apply;
|
||||||
|
// The insight WITHOUT the composed lines, so saving doesn't accumulate a
|
||||||
|
// copy of the trigger line on every edit.
|
||||||
|
insight.value = lesson.insight;
|
||||||
|
tags.value = lesson.tags ?? [];
|
||||||
|
projectId.value = lesson.project_id;
|
||||||
|
learnedFrom.value = lesson.learned_from ?? [];
|
||||||
|
} catch (e) {
|
||||||
|
error.value = apiErrorMessage(e, "Failed to load this lesson");
|
||||||
|
} finally {
|
||||||
|
loading.value = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async function save(force = false) {
|
||||||
|
if (!canSave.value || saving.value) return;
|
||||||
|
saving.value = true;
|
||||||
|
error.value = null;
|
||||||
|
duplicate.value = null;
|
||||||
|
try {
|
||||||
|
const payload = {
|
||||||
|
what: what.value.trim(),
|
||||||
|
when_to_apply: whenToApply.value.trim(),
|
||||||
|
insight: insight.value,
|
||||||
|
tags: tags.value,
|
||||||
|
learned_from: learnedFrom.value,
|
||||||
|
project_id: projectId.value,
|
||||||
|
...(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");
|
||||||
|
router.push(`/lessons/${saved.id}`);
|
||||||
|
} catch (e) {
|
||||||
|
// A 409 is the duplicate gate, not a failure: it hands back the record
|
||||||
|
// that already covers this moment so the writer can improve that one
|
||||||
|
// instead of standing a second beside it.
|
||||||
|
const body = (e as { status?: number; body?: Record<string, unknown> });
|
||||||
|
if (body?.status === 409 && body.body) {
|
||||||
|
const existing = body.body as { id?: number; title?: string };
|
||||||
|
if (existing.id) {
|
||||||
|
duplicate.value = { id: existing.id, title: existing.title ?? "" };
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
error.value = apiErrorMessage(e, "Failed to save this lesson");
|
||||||
|
} finally {
|
||||||
|
saving.value = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
watch(lessonId, load);
|
||||||
|
onMounted(() => {
|
||||||
|
load();
|
||||||
|
// Pre-fill from the link that brought you here, the same convention the
|
||||||
|
// snippet editor uses. Left null it is a lesson with no recorded origin,
|
||||||
|
// which is valid — `project_id` says where a lesson was LEARNED and was
|
||||||
|
// never the limit on where it can be found (step 3).
|
||||||
|
if (!isEdit.value && route.query.projectId) {
|
||||||
|
projectId.value = Number(route.query.projectId);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
</script>
|
||||||
|
|
||||||
|
<template>
|
||||||
|
<div class="lesson-editor">
|
||||||
|
<header class="le-head">
|
||||||
|
<h1>{{ isEdit ? "Edit lesson" : "Record a lesson" }}</h1>
|
||||||
|
<p class="le-sub">
|
||||||
|
Something worth knowing, kept so a later session meets it at the moment
|
||||||
|
it applies. A lesson binds nobody.
|
||||||
|
</p>
|
||||||
|
</header>
|
||||||
|
|
||||||
|
<p v-if="error" class="le-error" role="alert">{{ error }}</p>
|
||||||
|
|
||||||
|
<div v-if="duplicate" class="le-dupe" role="alert">
|
||||||
|
<p>
|
||||||
|
<strong>A lesson already covers this moment.</strong>
|
||||||
|
Improving that one keeps what was learned in a single place — two
|
||||||
|
lessons under one trigger compete for the same slot, so the second
|
||||||
|
displaces the first rather than adding to it.
|
||||||
|
</p>
|
||||||
|
<div class="le-dupe-actions">
|
||||||
|
<router-link :to="`/lessons/${duplicate.id}`" class="le-link">
|
||||||
|
Open “{{ duplicate.title }}”
|
||||||
|
</router-link>
|
||||||
|
<button type="button" class="le-ghost" @click="save(true)">
|
||||||
|
Record it anyway
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<p v-if="loading" class="le-muted">Loading…</p>
|
||||||
|
|
||||||
|
<form v-else class="le-form" @submit.prevent="save()">
|
||||||
|
<!-- The trigger comes FIRST, before the claim. It is what makes a lesson
|
||||||
|
findable, and putting it second invites it to be treated as an
|
||||||
|
afterthought to the thing the writer arrived wanting to say. -->
|
||||||
|
<label class="le-field le-field--primary">
|
||||||
|
<span class="le-label">When does this apply?</span>
|
||||||
|
<span class="le-hint">
|
||||||
|
The situation, in the words it will present itself in — what someone
|
||||||
|
would be seeing, saying, or about to do. “A test fails on code you
|
||||||
|
believe is correct” is a trigger. “Testing” is a topic, and a topic
|
||||||
|
matches everything and surfaces for nothing.
|
||||||
|
</span>
|
||||||
|
<textarea
|
||||||
|
v-model="whenToApply"
|
||||||
|
class="le-input le-textarea"
|
||||||
|
rows="3"
|
||||||
|
required
|
||||||
|
placeholder="a CI run has sat in_progress far longer than its suite takes"
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
|
||||||
|
<label class="le-field">
|
||||||
|
<span class="le-label">What did you learn?</span>
|
||||||
|
<span class="le-hint">The claim itself, in one line, as you would say it.</span>
|
||||||
|
<input
|
||||||
|
v-model="what"
|
||||||
|
class="le-input"
|
||||||
|
type="text"
|
||||||
|
required
|
||||||
|
placeholder="Read the job log before waiting longer"
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
|
||||||
|
<label class="le-field">
|
||||||
|
<span class="le-label">The detail <em>(optional)</em></span>
|
||||||
|
<span class="le-hint">
|
||||||
|
What you would want handed to you in the same situation next time —
|
||||||
|
the evidence, the reasoning, the thing that is not obvious.
|
||||||
|
</span>
|
||||||
|
<textarea
|
||||||
|
v-model="insight"
|
||||||
|
class="le-input le-textarea le-textarea--tall"
|
||||||
|
rows="10"
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
|
||||||
|
<label class="le-field">
|
||||||
|
<span class="le-label">Tags</span>
|
||||||
|
<TagInput
|
||||||
|
v-model="tags"
|
||||||
|
:fetchTags="(q: string) => notesStore.fetchAllTags(q)"
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
|
||||||
|
<label class="le-field">
|
||||||
|
<span class="le-label">Learned on <em>(optional)</em></span>
|
||||||
|
<span class="le-hint">
|
||||||
|
Where this was learned. Kept as a fact about its origin — a lesson is
|
||||||
|
retrievable from every project regardless, which is the point of the
|
||||||
|
kind.
|
||||||
|
</span>
|
||||||
|
<ProjectSelector v-model="projectId" />
|
||||||
|
</label>
|
||||||
|
|
||||||
|
<!-- 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">
|
||||||
|
<span class="le-preview-label">Stored as</span>
|
||||||
|
<p class="le-preview-title">{{ previewTitle }}</p>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div class="le-actions">
|
||||||
|
<button type="submit" class="le-primary" :disabled="!canSave || saving">
|
||||||
|
{{ saving ? "Saving…" : isEdit ? "Save lesson" : "Record lesson" }}
|
||||||
|
</button>
|
||||||
|
<button type="button" class="le-ghost" @click="router.back()">
|
||||||
|
Cancel
|
||||||
|
</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">
|
||||||
|
A lesson needs both a trigger and a claim — without the trigger it
|
||||||
|
would save and never reach anyone.
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
</form>
|
||||||
|
</div>
|
||||||
|
</template>
|
||||||
|
|
||||||
|
<style scoped>
|
||||||
|
.lesson-editor {
|
||||||
|
max-width: 820px;
|
||||||
|
margin: 0 auto;
|
||||||
|
padding: var(--fs-layout-page-pad);
|
||||||
|
color: var(--fs-text-primary);
|
||||||
|
}
|
||||||
|
|
||||||
|
.le-head h1 {
|
||||||
|
margin: 0 0 0.25rem;
|
||||||
|
font-size: 1.35rem;
|
||||||
|
font-weight: 500;
|
||||||
|
}
|
||||||
|
.le-sub {
|
||||||
|
margin: 0 0 1.5rem;
|
||||||
|
color: var(--fs-text-secondary);
|
||||||
|
font-size: 0.9rem;
|
||||||
|
}
|
||||||
|
|
||||||
|
.le-form { display: flex; flex-direction: column; gap: 1.25rem; }
|
||||||
|
|
||||||
|
.le-field { display: flex; flex-direction: column; gap: 0.35rem; }
|
||||||
|
|
||||||
|
/* The trigger gets visible weight, because it is the field whose absence is
|
||||||
|
invisible afterwards. */
|
||||||
|
.le-field--primary {
|
||||||
|
padding: 1rem;
|
||||||
|
border: 1px solid var(--fs-border-color);
|
||||||
|
border-radius: var(--fs-radius-lg);
|
||||||
|
background: var(--fs-surface-raised);
|
||||||
|
}
|
||||||
|
|
||||||
|
.le-label { font-weight: 500; font-size: 0.9rem; }
|
||||||
|
.le-label em { font-style: normal; color: var(--fs-text-tertiary); font-weight: 400; }
|
||||||
|
.le-hint {
|
||||||
|
color: var(--fs-text-secondary);
|
||||||
|
font-size: 0.82rem;
|
||||||
|
line-height: 1.45;
|
||||||
|
}
|
||||||
|
|
||||||
|
.le-input {
|
||||||
|
width: 100%;
|
||||||
|
padding: 0.55rem 0.7rem;
|
||||||
|
border: 1px solid var(--fs-border-color);
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: var(--fs-surface-page);
|
||||||
|
color: var(--fs-text-primary);
|
||||||
|
font: inherit;
|
||||||
|
font-size: 0.92rem;
|
||||||
|
}
|
||||||
|
.le-input:focus-visible {
|
||||||
|
outline: 2px solid var(--fs-accent);
|
||||||
|
outline-offset: 1px;
|
||||||
|
}
|
||||||
|
.le-textarea { resize: vertical; line-height: 1.5; }
|
||||||
|
.le-textarea--tall { font-family: var(--fs-font-mono); font-size: 0.85rem; }
|
||||||
|
|
||||||
|
.le-preview {
|
||||||
|
padding: 0.7rem 0.9rem;
|
||||||
|
border-left: 2px solid var(--fs-accent);
|
||||||
|
background: var(--fs-surface-raised);
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
}
|
||||||
|
.le-preview-label {
|
||||||
|
display: block;
|
||||||
|
font-size: 0.72rem;
|
||||||
|
text-transform: uppercase;
|
||||||
|
letter-spacing: 0.04em;
|
||||||
|
color: var(--fs-text-tertiary);
|
||||||
|
}
|
||||||
|
.le-preview-title { margin: 0.2rem 0 0; font-size: 0.92rem; }
|
||||||
|
|
||||||
|
.le-actions { display: flex; flex-wrap: wrap; align-items: center; gap: 0.6rem; }
|
||||||
|
|
||||||
|
.le-primary {
|
||||||
|
padding: 0.5rem 1.1rem;
|
||||||
|
border: none;
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: var(--fs-accent);
|
||||||
|
color: var(--fs-accent-fg);
|
||||||
|
font: inherit;
|
||||||
|
font-size: 0.9rem;
|
||||||
|
cursor: pointer;
|
||||||
|
}
|
||||||
|
.le-primary:disabled { opacity: 0.5; cursor: not-allowed; }
|
||||||
|
|
||||||
|
.le-ghost {
|
||||||
|
padding: 0.5rem 1rem;
|
||||||
|
border: 1px solid var(--fs-border-color);
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: transparent;
|
||||||
|
color: var(--fs-text-primary);
|
||||||
|
font: inherit;
|
||||||
|
font-size: 0.9rem;
|
||||||
|
cursor: pointer;
|
||||||
|
}
|
||||||
|
|
||||||
|
.le-muted { color: var(--fs-text-secondary); font-size: 0.85rem; }
|
||||||
|
.le-why { flex-basis: 100%; }
|
||||||
|
|
||||||
|
.le-error {
|
||||||
|
padding: 0.6rem 0.8rem;
|
||||||
|
border-radius: var(--fs-radius-sm);
|
||||||
|
background: color-mix(in srgb, var(--fs-error) 12%, var(--fs-surface-raised));
|
||||||
|
color: color-mix(in srgb, var(--fs-error) 55%, var(--fs-text-primary));
|
||||||
|
font-size: 0.88rem;
|
||||||
|
}
|
||||||
|
|
||||||
|
.le-dupe {
|
||||||
|
padding: 0.8rem 1rem;
|
||||||
|
margin-bottom: 1rem;
|
||||||
|
border: 1px solid var(--fs-border-color);
|
||||||
|
border-radius: var(--fs-radius-lg);
|
||||||
|
background: var(--fs-surface-raised);
|
||||||
|
font-size: 0.88rem;
|
||||||
|
}
|
||||||
|
.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); }
|
||||||
|
</style>
|
||||||
@@ -23,6 +23,7 @@ import WordCount from "@/components/WordCount.vue";
|
|||||||
import TagInput from "@/components/TagInput.vue";
|
import TagInput from "@/components/TagInput.vue";
|
||||||
import ProjectSelector from "@/components/ProjectSelector.vue";
|
import ProjectSelector from "@/components/ProjectSelector.vue";
|
||||||
import MilestoneSelector from "@/components/MilestoneSelector.vue";
|
import MilestoneSelector from "@/components/MilestoneSelector.vue";
|
||||||
|
import LessonsTaughtPanel from "@/components/LessonsTaughtPanel.vue";
|
||||||
import TaskLogSection from "@/components/TaskLogSection.vue";
|
import TaskLogSection from "@/components/TaskLogSection.vue";
|
||||||
import DiffView from "@/components/DiffView.vue";
|
import DiffView from "@/components/DiffView.vue";
|
||||||
import ConfirmDialog from "@/components/ConfirmDialog.vue";
|
import ConfirmDialog from "@/components/ConfirmDialog.vue";
|
||||||
@@ -546,6 +547,13 @@ useEditorGuards(dirty, save);
|
|||||||
<!-- Work log -->
|
<!-- Work log -->
|
||||||
<TaskLogSection v-if="taskId" :task-id="taskId" class="body-log" />
|
<TaskLogSection v-if="taskId" :task-id="taskId" class="body-log" />
|
||||||
|
|
||||||
|
<!-- What came OUT of this record. The forward link is typed by
|
||||||
|
the lesson's author; this direction has no author and so
|
||||||
|
tends never to get built — but a reader opening an old
|
||||||
|
issue wants to know what was learned from it. Silent when
|
||||||
|
there is nothing, because most records taught no lesson. -->
|
||||||
|
<LessonsTaughtPanel v-if="taskId" :record-id="taskId" />
|
||||||
|
|
||||||
<!-- Applicable rules (plan tasks only) -->
|
<!-- Applicable rules (plan tasks only) -->
|
||||||
<PlanRulesPanel
|
<PlanRulesPanel
|
||||||
v-if="store.currentTask?.task_kind === 'plan' && store.currentTask?.project_id"
|
v-if="store.currentTask?.task_kind === 'plan' && store.currentTask?.project_id"
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ from scribe.routes.trash import trash_bp
|
|||||||
from scribe.routes.dashboard import dashboard_bp
|
from scribe.routes.dashboard import dashboard_bp
|
||||||
from scribe.routes.systems import systems_bp
|
from scribe.routes.systems import systems_bp
|
||||||
from scribe.routes.canonical_systems import canonical_systems_bp
|
from scribe.routes.canonical_systems import canonical_systems_bp
|
||||||
|
from scribe.routes.lessons import lessons_bp
|
||||||
from scribe.routes.snippets import snippets_bp
|
from scribe.routes.snippets import snippets_bp
|
||||||
from scribe.routes.webhooks import webhooks_bp
|
from scribe.routes.webhooks import webhooks_bp
|
||||||
from scribe.mcp import mount_mcp
|
from scribe.mcp import mount_mcp
|
||||||
@@ -92,6 +93,7 @@ def create_app() -> Quart:
|
|||||||
app.register_blueprint(search_bp)
|
app.register_blueprint(search_bp)
|
||||||
app.register_blueprint(profile_bp)
|
app.register_blueprint(profile_bp)
|
||||||
app.register_blueprint(knowledge_bp)
|
app.register_blueprint(knowledge_bp)
|
||||||
|
app.register_blueprint(lessons_bp)
|
||||||
app.register_blueprint(rulebooks_bp)
|
app.register_blueprint(rulebooks_bp)
|
||||||
app.register_blueprint(plugin_bp)
|
app.register_blueprint(plugin_bp)
|
||||||
app.register_blueprint(design_systems_bp)
|
app.register_blueprint(design_systems_bp)
|
||||||
|
|||||||
@@ -20,21 +20,10 @@ from scribe.mcp.tools import systems as systems_tools
|
|||||||
from scribe.services.note_usage import record_pulled
|
from scribe.services.note_usage import record_pulled
|
||||||
|
|
||||||
|
|
||||||
def _to_dict(note) -> dict:
|
# The payload shape lives in the service (`lesson_to_dict`), shared with the
|
||||||
"""A lesson as the tools return it — the composed fields read back out,
|
# REST door — a shape spelled once per door answers the two of them
|
||||||
not the raw row, so a caller sees the same vocabulary it wrote with."""
|
# differently the first time a field is added.
|
||||||
return {
|
_to_dict = lessons_svc.lesson_to_dict
|
||||||
"id": note.id,
|
|
||||||
"title": note.title,
|
|
||||||
"body": note.body,
|
|
||||||
"when_to_apply": lessons_svc.lesson_trigger(note),
|
|
||||||
"learned_from": lessons_svc.lesson_sources(note),
|
|
||||||
"tags": list(note.tags or []),
|
|
||||||
"project_id": note.project_id,
|
|
||||||
"note_type": note.note_type,
|
|
||||||
"created_at": note.created_at.isoformat() if note.created_at else None,
|
|
||||||
"updated_at": note.updated_at.isoformat() if note.updated_at else None,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
async def list_lessons(
|
async def list_lessons(
|
||||||
|
|||||||
@@ -0,0 +1,256 @@
|
|||||||
|
"""REST routes for lessons — a transferable insight, retrievable by situation.
|
||||||
|
|
||||||
|
A lesson is a note with note_type='lesson' (see services/lessons.py). These
|
||||||
|
routes feed the web UI; the MCP tools (mcp/tools/lessons.py) are the
|
||||||
|
agent-facing surface. Both go through services/lessons.py, so the compose/parse
|
||||||
|
contract and the `data` mirror live in one place — the same division snippets
|
||||||
|
use, and for the same reason: two doors that each compose a lesson would
|
||||||
|
compose it two ways, and the document IS what ranks.
|
||||||
|
|
||||||
|
ACL (rule #78): reads and writes of a single lesson resolve through the
|
||||||
|
share-aware `get_lesson` / `can_write_note`, and writes are performed as the
|
||||||
|
OWNER so a shared editor isn't rejected by the owner-scoped service —
|
||||||
|
mirroring routes/snippets.py and routes/notes.py.
|
||||||
|
|
||||||
|
WHY THE TRIGGER IS A NAMED FIELD HERE TOO. The web editor could have posted a
|
||||||
|
body and let the service parse it. It doesn't, because the evidence behind
|
||||||
|
this kind (step 1) is that a trigger gets filled when a door ASKS for it by
|
||||||
|
name — the snippet corpus is at 100% on its trigger with no guard anywhere,
|
||||||
|
because a service composes the title from a parameter. A form that offered one
|
||||||
|
markdown box would be the option milestone 385 rejected, wearing a different
|
||||||
|
hat.
|
||||||
|
"""
|
||||||
|
import logging
|
||||||
|
|
||||||
|
from quart import Blueprint, jsonify, request
|
||||||
|
|
||||||
|
from scribe.auth import get_current_user_id, login_required
|
||||||
|
from scribe.routes.utils import not_found, parse_pagination
|
||||||
|
from scribe.services import dedup as dedup_svc
|
||||||
|
from scribe.services import knowledge as knowledge_svc
|
||||||
|
from scribe.services import lessons as lessons_svc
|
||||||
|
from scribe.services import systems as systems_svc
|
||||||
|
from scribe.services import trash as trash_svc
|
||||||
|
from scribe.services.access import (
|
||||||
|
can_write_note,
|
||||||
|
describe_provenance,
|
||||||
|
label_shared_items,
|
||||||
|
)
|
||||||
|
from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
lessons_bp = Blueprint("lessons", __name__, url_prefix="/api/lessons")
|
||||||
|
|
||||||
|
|
||||||
|
@lessons_bp.route("", methods=["GET"])
|
||||||
|
@login_required
|
||||||
|
async def list_lessons_route():
|
||||||
|
"""The kind enumerated, rather than only what a query resembles.
|
||||||
|
|
||||||
|
Semantic search is how a lesson REACHES a session; this is how a person
|
||||||
|
sees what exists at all. Each row carries its trigger, because a list of
|
||||||
|
lessons without them is a list of claims with the half that says when each
|
||||||
|
one matters left off.
|
||||||
|
"""
|
||||||
|
uid = get_current_user_id()
|
||||||
|
q = request.args.get("q") or None
|
||||||
|
tag = request.args.get("tag", "")
|
||||||
|
try:
|
||||||
|
project_id = int(request.args.get("project_id", 0) or 0) or None
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
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,
|
||||||
|
)
|
||||||
|
return jsonify({
|
||||||
|
"lessons": await label_shared_items(uid, items),
|
||||||
|
"total": total,
|
||||||
|
})
|
||||||
|
|
||||||
|
|
||||||
|
@lessons_bp.route("/taught-by/<int:record_id>", methods=["GET"])
|
||||||
|
@login_required
|
||||||
|
async def lessons_taught_by_route(record_id: int):
|
||||||
|
"""The lessons drawn FROM one record — the reverse of `learned_from`.
|
||||||
|
|
||||||
|
Registered ABOVE the `/<int:lesson_id>` routes on purpose: Quart matches
|
||||||
|
in registration order, and `taught-by` would otherwise never be reached
|
||||||
|
if the converter ever widened. The same ordering snippets' `/duplicates`
|
||||||
|
route documents.
|
||||||
|
|
||||||
|
This is the direction that gets forgotten. A reader opening an old issue
|
||||||
|
wants to know what was learned from it, and without this the relation is
|
||||||
|
only navigable from the lesson's side.
|
||||||
|
"""
|
||||||
|
uid = get_current_user_id()
|
||||||
|
notes = await lessons_svc.lessons_taught_by(uid, record_id)
|
||||||
|
return jsonify({
|
||||||
|
"lessons": [lessons_svc.lesson_to_dict(n) for n in notes],
|
||||||
|
"taught_by": record_id,
|
||||||
|
})
|
||||||
|
|
||||||
|
|
||||||
|
@lessons_bp.route("", methods=["POST"])
|
||||||
|
@login_required
|
||||||
|
async def create_lesson_route():
|
||||||
|
uid = get_current_user_id()
|
||||||
|
data = await request.get_json() or {}
|
||||||
|
what = (data.get("what") or "").strip()
|
||||||
|
when_to_apply = (data.get("when_to_apply") or "").strip()
|
||||||
|
if not what:
|
||||||
|
return jsonify({"error": "what is required"}), 400
|
||||||
|
# The trigger is not optional at this door even though the service will
|
||||||
|
# store a lesson without one. A lesson with no trigger saves, reads
|
||||||
|
# correctly in every listing, and never surfaces — there is nothing to
|
||||||
|
# notice afterwards, which is exactly why the form has to refuse it here
|
||||||
|
# rather than leave the writer a record that looks finished.
|
||||||
|
if not when_to_apply:
|
||||||
|
return jsonify({
|
||||||
|
"error": "when_to_apply is required",
|
||||||
|
"detail": (
|
||||||
|
"A lesson is found by the SITUATION it applies to. Without a "
|
||||||
|
"trigger it still saves and still reads correctly, and it "
|
||||||
|
"never reaches anyone — so it is refused here rather than "
|
||||||
|
"stored as a record that looks finished."
|
||||||
|
),
|
||||||
|
}), 400
|
||||||
|
|
||||||
|
project_id = data.get("project_id") or None
|
||||||
|
learned_from = data.get("learned_from") or []
|
||||||
|
|
||||||
|
# The same near-duplicate gate the MCP create path applies. Two lessons
|
||||||
|
# under one trigger compete in a single ranked list for one reserved slot,
|
||||||
|
# so the duplicate does not merely clutter — it displaces.
|
||||||
|
if not data.get("force"):
|
||||||
|
title, body = lessons_svc.lesson_document(
|
||||||
|
what, when_to_apply, data.get("insight", ""), learned_from,
|
||||||
|
)
|
||||||
|
dup = await dedup_svc.find_duplicate_note(
|
||||||
|
uid, title, body,
|
||||||
|
project_id=project_id,
|
||||||
|
is_task=False,
|
||||||
|
note_type=lessons_svc.LESSON_NOTE_TYPE,
|
||||||
|
)
|
||||||
|
if dup is not None:
|
||||||
|
return jsonify(dedup_svc.duplicate_response(dup, "lesson")), 409
|
||||||
|
|
||||||
|
note = await lessons_svc.create_lesson(
|
||||||
|
uid,
|
||||||
|
what=what,
|
||||||
|
when_to_apply=when_to_apply,
|
||||||
|
insight=data.get("insight", ""),
|
||||||
|
learned_from=learned_from,
|
||||||
|
tags=data.get("tags"),
|
||||||
|
project_id=project_id,
|
||||||
|
)
|
||||||
|
if data.get("system_ids") is not None:
|
||||||
|
await systems_svc.set_record_systems(uid, note.id, data["system_ids"])
|
||||||
|
out = lessons_svc.lesson_to_dict(note)
|
||||||
|
out["systems"] = [
|
||||||
|
s.to_dict() for s in await systems_svc.list_record_systems(uid, note.id)
|
||||||
|
]
|
||||||
|
return jsonify(out), 201
|
||||||
|
|
||||||
|
|
||||||
|
@lessons_bp.route("/<int:lesson_id>", methods=["GET"])
|
||||||
|
@login_required
|
||||||
|
async def get_lesson_route(lesson_id: int):
|
||||||
|
uid = get_current_user_id()
|
||||||
|
note = await lessons_svc.get_lesson(uid, lesson_id)
|
||||||
|
if note is None:
|
||||||
|
return not_found("Lesson")
|
||||||
|
out = lessons_svc.lesson_to_dict(note)
|
||||||
|
# As the OWNER: a shared reader isn't scoped to the owner's project, so
|
||||||
|
# their own id would come back empty (the write-as-owner pattern this
|
||||||
|
# module already uses, read side).
|
||||||
|
out["systems"] = [
|
||||||
|
s.to_dict()
|
||||||
|
for s in await systems_svc.list_record_systems(note.user_id, lesson_id)
|
||||||
|
]
|
||||||
|
# Resolved, not bare ids: "#4181" on a page tells a reader nothing about
|
||||||
|
# whether it is worth opening, and the provenance is the point of a lesson.
|
||||||
|
out["learned_from_records"] = await lessons_svc.source_records(
|
||||||
|
uid, out["learned_from"]
|
||||||
|
)
|
||||||
|
out.update(await describe_provenance(uid, note))
|
||||||
|
out["usage"] = (await usage_for_notes([lesson_id])).get(
|
||||||
|
lesson_id, empty_usage()
|
||||||
|
)
|
||||||
|
# Opening the detail view IS a pull — the operator chose to look. Tagged
|
||||||
|
# apart from the MCP sources so "an agent was handed it" and "a human read
|
||||||
|
# it" stay distinguishable; they mean different things for pruning (#2085).
|
||||||
|
record_pulled(user_id=uid, note_id=lesson_id, source="rest_lesson")
|
||||||
|
return jsonify(out)
|
||||||
|
|
||||||
|
|
||||||
|
@lessons_bp.route("/<int:lesson_id>", methods=["PATCH"])
|
||||||
|
@login_required
|
||||||
|
async def update_lesson_route(lesson_id: int):
|
||||||
|
uid = get_current_user_id()
|
||||||
|
note = await lessons_svc.get_lesson(uid, lesson_id)
|
||||||
|
if note is None:
|
||||||
|
return not_found("Lesson")
|
||||||
|
if not await can_write_note(uid, lesson_id):
|
||||||
|
return jsonify({"error": "Permission denied"}), 403
|
||||||
|
owner_uid = note.user_id
|
||||||
|
data = await request.get_json() or {}
|
||||||
|
|
||||||
|
# Partial update: only keys present in the payload change, and the service
|
||||||
|
# re-composes title, body and mirror from the merged set — so a form that
|
||||||
|
# sends one field cannot leave the halves of the document disagreeing.
|
||||||
|
kwargs = {
|
||||||
|
k: data[k]
|
||||||
|
for k in ("what", "when_to_apply", "insight", "learned_from", "tags")
|
||||||
|
if k in data
|
||||||
|
}
|
||||||
|
# An empty trigger would save and silently stop the lesson surfacing, so
|
||||||
|
# clearing it is refused for the same reason creating without one is.
|
||||||
|
if "when_to_apply" in kwargs and not (kwargs["when_to_apply"] or "").strip():
|
||||||
|
return jsonify({
|
||||||
|
"error": "when_to_apply cannot be cleared",
|
||||||
|
"detail": (
|
||||||
|
"A lesson with no trigger never surfaces, and nothing about "
|
||||||
|
"the stored record would show it. Rewrite the trigger rather "
|
||||||
|
"than emptying it."
|
||||||
|
),
|
||||||
|
}), 400
|
||||||
|
|
||||||
|
updated = await lessons_svc.update_lesson(owner_uid, lesson_id, **kwargs)
|
||||||
|
if updated is None:
|
||||||
|
return not_found("Lesson")
|
||||||
|
if data.get("system_ids") is not None:
|
||||||
|
await systems_svc.set_record_systems(
|
||||||
|
owner_uid, lesson_id, data["system_ids"]
|
||||||
|
)
|
||||||
|
out = lessons_svc.lesson_to_dict(updated)
|
||||||
|
out["systems"] = [
|
||||||
|
s.to_dict()
|
||||||
|
for s in await systems_svc.list_record_systems(owner_uid, lesson_id)
|
||||||
|
]
|
||||||
|
return jsonify(out)
|
||||||
|
|
||||||
|
|
||||||
|
@lessons_bp.route("/<int:lesson_id>", methods=["DELETE"])
|
||||||
|
@login_required
|
||||||
|
async def delete_lesson_route(lesson_id: int):
|
||||||
|
"""Trash, not erase — recoverable from the trash like every other kind."""
|
||||||
|
uid = get_current_user_id()
|
||||||
|
note = await lessons_svc.get_lesson(uid, lesson_id)
|
||||||
|
if note is None:
|
||||||
|
return not_found("Lesson")
|
||||||
|
if not await can_write_note(uid, lesson_id):
|
||||||
|
return jsonify({"error": "Permission denied"}), 403
|
||||||
|
batch_id = await trash_svc.delete(note.user_id, "note", lesson_id)
|
||||||
|
if batch_id is None:
|
||||||
|
return not_found("Lesson")
|
||||||
|
return jsonify({"deleted": lesson_id, "deleted_batch_id": batch_id})
|
||||||
@@ -203,6 +203,12 @@ def embedding_text(title: str | None, body: str | None) -> str:
|
|||||||
return f"{title}\n{body}".strip() if body else title
|
return f"{title}\n{body}".strip() if body else title
|
||||||
|
|
||||||
|
|
||||||
|
# The join between a situation-keyed record's subject and its trigger. A
|
||||||
|
# CONSTANT because `untrigger_title` below has to spell the same thing to undo
|
||||||
|
# it, and two literals that must match are one edit away from not matching.
|
||||||
|
TRIGGER_SEP = " — "
|
||||||
|
|
||||||
|
|
||||||
def trigger_title(subject: str | None, trigger: str | None) -> str:
|
def trigger_title(subject: str | None, trigger: str | None) -> str:
|
||||||
"""`{subject} — {trigger}` — the title half of a situation-keyed document.
|
"""`{subject} — {trigger}` — the title half of a situation-keyed document.
|
||||||
|
|
||||||
@@ -226,10 +232,37 @@ def trigger_title(subject: str | None, trigger: str | None) -> str:
|
|||||||
subject = (subject or "").strip()
|
subject = (subject or "").strip()
|
||||||
trigger = (trigger or "").strip()
|
trigger = (trigger or "").strip()
|
||||||
if subject and trigger:
|
if subject and trigger:
|
||||||
return f"{subject} — {trigger}"
|
return f"{subject}{TRIGGER_SEP}{trigger}"
|
||||||
return subject or trigger
|
return subject or trigger
|
||||||
|
|
||||||
|
|
||||||
|
def untrigger_title(title: str | None, trigger: str | None) -> str:
|
||||||
|
"""The subject back out of a `trigger_title` — the inverse of the join.
|
||||||
|
|
||||||
|
Kept HERE, beside the join, for the reason the join itself was
|
||||||
|
consolidated: a separator spelled in two files is a separator that will one
|
||||||
|
day be changed in one of them. #3207 records the shape — derive it before
|
||||||
|
the third copy — and an inverse written in a caller is that third copy
|
||||||
|
wearing a different name.
|
||||||
|
|
||||||
|
Needs the trigger passed in rather than guessing at the separator, because
|
||||||
|
a subject may legitimately contain an em dash. Given the trigger, the
|
||||||
|
suffix is exact and the split cannot be wrong.
|
||||||
|
|
||||||
|
Degrades to the whole title when the suffix is absent — a record written
|
||||||
|
before the join existed, or one with no trigger yet, still answers with
|
||||||
|
something a human recognises rather than with "".
|
||||||
|
"""
|
||||||
|
title = (title or "").strip()
|
||||||
|
trigger = (trigger or "").strip()
|
||||||
|
if not trigger:
|
||||||
|
return title
|
||||||
|
suffix = f"{TRIGGER_SEP}{trigger}"
|
||||||
|
if title.endswith(suffix):
|
||||||
|
return title[: -len(suffix)].strip()
|
||||||
|
return title
|
||||||
|
|
||||||
|
|
||||||
# --- chunking (#280): the document shape ------------------------------------
|
# --- chunking (#280): the document shape ------------------------------------
|
||||||
#
|
#
|
||||||
# bge-small reads at most 512 tokens and fastembed silently truncates the rest,
|
# bge-small reads at most 512 tokens and fastembed silently truncates the rest,
|
||||||
|
|||||||
@@ -88,6 +88,9 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import re
|
import re
|
||||||
|
|
||||||
|
from scribe.models import async_session
|
||||||
|
from scribe.models.note import Note
|
||||||
|
|
||||||
LESSON_NOTE_TYPE = "lesson"
|
LESSON_NOTE_TYPE = "lesson"
|
||||||
|
|
||||||
# The key in `notes.data`. Named for the field it mirrors on `rules`, because it
|
# The key in `notes.data`. Named for the field it mirrors on `rules`, because it
|
||||||
@@ -317,6 +320,46 @@ def compose_data(
|
|||||||
return data
|
return data
|
||||||
|
|
||||||
|
|
||||||
|
def recompose_data(note) -> dict:
|
||||||
|
"""Rebuild a lesson's `data` mirror from its own title and body.
|
||||||
|
|
||||||
|
For the GENERIC note door. `update_lesson` composes the mirror itself from
|
||||||
|
the merged field set and never needs this; a plain `update_note(body=...)`
|
||||||
|
has no idea the mirror exists and would leave it behind.
|
||||||
|
|
||||||
|
THE COST OF LEAVING IT BEHIND IS HIGHER HERE THAN FOR A SNIPPET. A stale
|
||||||
|
snippet mirror reports the wrong path. A stale lesson mirror reports the
|
||||||
|
wrong TRIGGER — and `lesson_trigger` prefers the mirror, so the lesson goes
|
||||||
|
on being retrieved for the situation it used to name while displaying the
|
||||||
|
one it now names. The trigger is the entire retrieval story (step 3), so
|
||||||
|
that is not a degraded record; it is a record that fires at the wrong
|
||||||
|
moment and looks right when it does.
|
||||||
|
|
||||||
|
The body is the authority and the mirror is derived — already this file's
|
||||||
|
rule. This is its enforcement on the path that bypasses `update_lesson`.
|
||||||
|
|
||||||
|
The subject comes back out of the title through `untrigger_title`, the
|
||||||
|
inverse of the join that composed it, rather than by splitting on a
|
||||||
|
separator spelled a second time here.
|
||||||
|
"""
|
||||||
|
from scribe.services.embeddings import untrigger_title
|
||||||
|
|
||||||
|
body = getattr(note, "body", None) or ""
|
||||||
|
trigger_match = _BODY_TRIGGER_RE.search(body)
|
||||||
|
trigger = trigger_match.group(1).strip() if trigger_match else ""
|
||||||
|
what = untrigger_title(getattr(note, "title", None), trigger)
|
||||||
|
# Sources through the normal read, which already falls back body →
|
||||||
|
# arose_from_id. A body edit that drops the provenance line should drop
|
||||||
|
# the mirror's copy too: the body is the authority, and carrying a value
|
||||||
|
# the reader just deleted is the failure this function exists to prevent.
|
||||||
|
sources_match = _BODY_SOURCES_RE.search(body)
|
||||||
|
sources = (
|
||||||
|
normalize_sources(_ID_RE.findall(sources_match.group(1)))
|
||||||
|
if sources_match else []
|
||||||
|
)
|
||||||
|
return compose_data(what, trigger, sources)
|
||||||
|
|
||||||
|
|
||||||
async def create_lesson(
|
async def create_lesson(
|
||||||
user_id: int,
|
user_id: int,
|
||||||
*,
|
*,
|
||||||
@@ -375,6 +418,122 @@ async def get_lesson(user_id: int, lesson_id: int):
|
|||||||
return note
|
return note
|
||||||
|
|
||||||
|
|
||||||
|
def lesson_to_dict(note) -> dict:
|
||||||
|
"""A lesson as either door returns it — the composed fields read back out,
|
||||||
|
not the raw row, so a caller sees the same vocabulary it wrote with.
|
||||||
|
|
||||||
|
In the SERVICE rather than in each door, on the `snippet_to_dict`
|
||||||
|
precedent: the REST route feeds the web UI and the MCP tools feed an
|
||||||
|
agent, and a shape spelled once per door is a shape that answers the two
|
||||||
|
of them differently the first time a field is added.
|
||||||
|
"""
|
||||||
|
return {
|
||||||
|
"id": note.id,
|
||||||
|
"title": note.title,
|
||||||
|
"body": note.body,
|
||||||
|
# The composed vocabulary, not the storage: a caller that wrote
|
||||||
|
# `when_to_apply` reads `when_to_apply` back.
|
||||||
|
"what": (note.data or {}).get("what", "") if isinstance(note.data, dict) else "",
|
||||||
|
"when_to_apply": lesson_trigger(note),
|
||||||
|
"learned_from": lesson_sources(note),
|
||||||
|
"insight": _strip_composed_lines(note.body),
|
||||||
|
"tags": list(note.tags or []),
|
||||||
|
"project_id": note.project_id,
|
||||||
|
"note_type": note.note_type,
|
||||||
|
"created_at": note.created_at.isoformat() if note.created_at else None,
|
||||||
|
"updated_at": note.updated_at.isoformat() if note.updated_at else None,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
async def source_records(user_id: int, ids: list[int]) -> list[dict]:
|
||||||
|
"""The records that taught a lesson, resolved to something linkable.
|
||||||
|
|
||||||
|
`learned_from` is a list of bare ids, which is right for storage and
|
||||||
|
useless on a page: "#4181" tells a reader nothing about whether it is
|
||||||
|
worth opening. This resolves each to its title and kind so the UI can
|
||||||
|
label the link, and so a source that has been deleted simply drops out
|
||||||
|
rather than rendering a link to nothing.
|
||||||
|
|
||||||
|
One query for the whole list, not one per id — a lesson with six sources
|
||||||
|
would otherwise be six round trips to draw one panel.
|
||||||
|
|
||||||
|
Share-aware (rule 78). Order follows `ids`, because that order is the
|
||||||
|
writer's: the first source is the one they reached for first.
|
||||||
|
"""
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
from scribe.services.access import readable_notes_clause
|
||||||
|
|
||||||
|
wanted = normalize_sources(ids)
|
||||||
|
if not wanted:
|
||||||
|
return []
|
||||||
|
async with async_session() as session:
|
||||||
|
result = await session.execute(
|
||||||
|
select(Note)
|
||||||
|
.where(Note.id.in_(wanted))
|
||||||
|
.where(Note.deleted_at.is_(None))
|
||||||
|
.where(readable_notes_clause(user_id))
|
||||||
|
)
|
||||||
|
found = {n.id: n for n in result.scalars().all()}
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
"id": n.id,
|
||||||
|
"title": n.title,
|
||||||
|
"note_type": n.note_type,
|
||||||
|
"is_task": n.status is not None,
|
||||||
|
"task_kind": n.task_kind,
|
||||||
|
"status": n.status,
|
||||||
|
}
|
||||||
|
for i in wanted
|
||||||
|
if (n := found.get(i)) is not None
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
async def lessons_taught_by(user_id: int, record_id: int, limit: int = 20):
|
||||||
|
"""The lessons drawn FROM one record — the reverse of `learned_from`.
|
||||||
|
|
||||||
|
THE DIRECTION THAT GETS FORGOTTEN, and arguably the more useful one: a
|
||||||
|
reader opening an old issue wants to know what was learned from it, and
|
||||||
|
without this the relation is only navigable from the lesson's side. A
|
||||||
|
record that taught something should say so on its own page.
|
||||||
|
|
||||||
|
Queried through `data[SOURCES_KEY]` rather than by scanning bodies: the
|
||||||
|
mirror is JSONB with a GIN index (0070), which is the whole reason step 4
|
||||||
|
put the list there. `path_exists` is the same dialect the snippet location
|
||||||
|
lookup uses, so both reverse lookups read the index the same way.
|
||||||
|
|
||||||
|
Share-aware (rule 78) via `readable_notes_clause`: this renders beside a
|
||||||
|
record the caller can already see, and a lesson someone shared with them
|
||||||
|
belongs in that list exactly as their own does.
|
||||||
|
"""
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
from scribe.services.access import readable_notes_clause
|
||||||
|
|
||||||
|
try:
|
||||||
|
wanted = int(record_id)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
return []
|
||||||
|
if wanted <= 0:
|
||||||
|
return []
|
||||||
|
|
||||||
|
# The id is an int we just validated, never caller text, so it cannot
|
||||||
|
# break out of the expression — the same guarantee `location_jsonpath`
|
||||||
|
# gets from JSON-quoting its values.
|
||||||
|
jsonpath = f"$.{SOURCES_KEY}[*] ? (@ == {wanted})"
|
||||||
|
async with async_session() as session:
|
||||||
|
result = await session.execute(
|
||||||
|
select(Note)
|
||||||
|
.where(Note.note_type == LESSON_NOTE_TYPE)
|
||||||
|
.where(Note.deleted_at.is_(None))
|
||||||
|
.where(Note.data.path_exists(jsonpath))
|
||||||
|
.where(readable_notes_clause(user_id))
|
||||||
|
.order_by(Note.updated_at.desc())
|
||||||
|
.limit(max(1, min(limit, 100)))
|
||||||
|
)
|
||||||
|
return list(result.scalars().all())
|
||||||
|
|
||||||
|
|
||||||
async def update_lesson(
|
async def update_lesson(
|
||||||
user_id: int,
|
user_id: int,
|
||||||
lesson_id: int,
|
lesson_id: int,
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
from collections.abc import Iterable
|
from collections.abc import Callable, Iterable
|
||||||
from datetime import date, datetime, timezone
|
from datetime import date, datetime, timezone
|
||||||
|
|
||||||
from sqlalchemy import func, or_, select, text
|
from sqlalchemy import func, or_, select, text
|
||||||
@@ -11,14 +11,44 @@ from scribe.models.base import iso
|
|||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
# The fields `snippets.parse_snippet_fields` reads. Writing any of them can
|
# The fields a derived `data` mirror is parsed out of. Writing any of them can
|
||||||
# change what a snippet's derived `data` mirror should say, so update_note
|
# change what the mirror should say, so update_note recomposes it when one
|
||||||
# recomposes the mirror when one moves. Kept here as a set of NAMES rather
|
# moves. Kept here as a set of NAMES rather than imported, because it
|
||||||
# than imported, because it describes update_note's own `fields` dict, not the
|
# describes update_note's own `fields` dict, not any parser's signature.
|
||||||
# parser's signature.
|
|
||||||
_PARSED_FROM_BODY = frozenset({"title", "body", "tags"})
|
_PARSED_FROM_BODY = frozenset({"title", "body", "tags"})
|
||||||
|
|
||||||
|
|
||||||
|
def _mirror_recomposers() -> dict[str, Callable[[Note], dict]]:
|
||||||
|
"""note_type -> the function that rebuilds that kind's derived `data`.
|
||||||
|
|
||||||
|
A TABLE rather than a chain of `if note_type == ...`, because the previous
|
||||||
|
shape tested one constant and the next kind with a derived mirror was
|
||||||
|
silently not covered — which is exactly what happened: the snippet guard
|
||||||
|
(#3128) was hard-coded, and lessons arrived in milestone 385 with the same
|
||||||
|
body-is-authority/`data`-is-mirror design and none of the protection.
|
||||||
|
|
||||||
|
The failure is invisible from here. Nothing raises, nothing logs; the row
|
||||||
|
simply keeps answering queries from a mirror that no longer matches its
|
||||||
|
body, and every surface that prefers the mirror — which is all of them, by
|
||||||
|
design, because parsing markdown to answer what an index can answer is how
|
||||||
|
a hot path rots — reports the old value confidently.
|
||||||
|
|
||||||
|
Imported inside the function, not at module scope: both services call back
|
||||||
|
into this module (`update_snippet`/`update_lesson` -> `update_note`), so a
|
||||||
|
top-level import is a cycle.
|
||||||
|
"""
|
||||||
|
from scribe.services.lessons import (
|
||||||
|
LESSON_NOTE_TYPE, recompose_data as _lesson_mirror,
|
||||||
|
)
|
||||||
|
from scribe.services.snippets import (
|
||||||
|
SNIPPET_NOTE_TYPE, recompose_data as _snippet_mirror,
|
||||||
|
)
|
||||||
|
return {
|
||||||
|
SNIPPET_NOTE_TYPE: _snippet_mirror,
|
||||||
|
LESSON_NOTE_TYPE: _lesson_mirror,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
# Text fields where EMPTY MEANS NULL (milestone 317). The sweep's whole signal
|
# Text fields where EMPTY MEANS NULL (milestone 317). The sweep's whole signal
|
||||||
# is `verify_with IS NULL` = "this is a decision, there is nothing to go and
|
# is `verify_with IS NULL` = "this is a decision, there is nothing to go and
|
||||||
# check". An empty string that is not NULL makes a norm look like a constraint
|
# check". An empty string that is not NULL makes a norm look like a constraint
|
||||||
@@ -604,23 +634,19 @@ async def update_note(
|
|||||||
# costs exactly what the sweep exists to catch.
|
# costs exactly what the sweep exists to catch.
|
||||||
if note.verify_with != check_before:
|
if note.verify_with != check_before:
|
||||||
note.verified_at = None
|
note.verified_at = None
|
||||||
# A snippet's `data` is DERIVED from its body — so a write that moves
|
# Some kinds derive `data` from their body — so a write that moves the
|
||||||
# the body through this generic door must move the mirror with it
|
# body through this generic door must move the mirror with it (#3128).
|
||||||
# (#3128). Without this, PATCH /api/notes/<snippet_id> {body} left the
|
# Without this, PATCH /api/notes/<id> {body} left the mirror behind,
|
||||||
# mirror behind, and snippet_fields PREFERS the mirror: the row went on
|
# and every read PREFERS the mirror: a snippet went on reporting its
|
||||||
# reporting its old repo/path/symbol to prior-art recall while showing
|
# old repo/path/symbol to prior-art recall while showing its new body,
|
||||||
# its new body. `update_snippet` composes the mirror itself and passes
|
# and a lesson would go on being retrieved for the situation it used to
|
||||||
# it explicitly, so an explicit `data` always wins — the caller that
|
# name. The kind's own updater composes the mirror itself and passes it
|
||||||
# knows the field set beats the one that can only re-read the body.
|
# explicitly, so an explicit `data` always wins — the caller that knows
|
||||||
|
# the field set beats the one that can only re-read the body.
|
||||||
if "data" not in fields and not _PARSED_FROM_BODY.isdisjoint(fields):
|
if "data" not in fields and not _PARSED_FROM_BODY.isdisjoint(fields):
|
||||||
# Imported here, not at module scope: services/snippets.py calls
|
recompose = _mirror_recomposers().get(note.note_type or "")
|
||||||
# back into this module (update_snippet -> update_note), so a
|
if recompose is not None:
|
||||||
# top-level import is a cycle.
|
note.data = recompose(note)
|
||||||
from scribe.services.snippets import (
|
|
||||||
SNIPPET_NOTE_TYPE, recompose_data,
|
|
||||||
)
|
|
||||||
if note.note_type == SNIPPET_NOTE_TYPE:
|
|
||||||
note.data = recompose_data(note)
|
|
||||||
# Auto-set lifecycle timestamps on status transitions
|
# Auto-set lifecycle timestamps on status transitions
|
||||||
if "status" in fields:
|
if "status" in fields:
|
||||||
_now = datetime.now(timezone.utc)
|
_now = datetime.now(timezone.utc)
|
||||||
|
|||||||
@@ -275,9 +275,21 @@ def parse_snippet_fields(
|
|||||||
|
|
||||||
``locations`` is a list of {repo,path,symbol}; ``repo``/``path``/``symbol``
|
``locations`` is a list of {repo,path,symbol}; ``repo``/``path``/``symbol``
|
||||||
mirror the FIRST location for back-compat with the single-location callers."""
|
mirror the FIRST location for back-compat with the single-location callers."""
|
||||||
|
from scribe.services.embeddings import TRIGGER_SEP
|
||||||
|
|
||||||
title = title or ""
|
title = title or ""
|
||||||
body = body or ""
|
body = body or ""
|
||||||
name, _, when_from_title = title.partition(" — ")
|
# The inverse of `embeddings.trigger_title`, at the SEPARATOR it composed
|
||||||
|
# with — imported rather than spelled again, because a separator written
|
||||||
|
# in two files is a separator that will one day be changed in one of them.
|
||||||
|
#
|
||||||
|
# `partition` rather than the `untrigger_title` a lesson uses: that one is
|
||||||
|
# handed the trigger and strips an exact suffix, which a lesson needs
|
||||||
|
# because its subject may legitimately contain a dash. A snippet's name is
|
||||||
|
# a symbol, so the first separator is the right split and no trigger has
|
||||||
|
# to be known in advance. Two inverses, suited to their callers; one
|
||||||
|
# constant, so they cannot disagree about where the seam is.
|
||||||
|
name, _, when_from_title = title.partition(TRIGGER_SEP)
|
||||||
fields = {
|
fields = {
|
||||||
"name": name.strip(),
|
"name": name.strip(),
|
||||||
"when_to_use": when_from_title.strip(),
|
"when_to_use": when_from_title.strip(),
|
||||||
|
|||||||
@@ -191,6 +191,34 @@ def fake_snippet(**attrs) -> MagicMock:
|
|||||||
}, attrs)
|
}, attrs)
|
||||||
|
|
||||||
|
|
||||||
|
def fake_lesson(**attrs) -> MagicMock:
|
||||||
|
"""A stand-in lesson: a note whose `note_type` is what makes it one.
|
||||||
|
|
||||||
|
The title carries the trigger because `compose_title` builds it that way —
|
||||||
|
`{what} — {when it applies}` — so a menu line rendering only the title is
|
||||||
|
already showing the reader when this lesson applies. Tests that used a bare
|
||||||
|
title here would be testing a record the product cannot create.
|
||||||
|
|
||||||
|
The check fields and `arose_from_id` are explicitly None for the reason
|
||||||
|
`fake_snippet`'s `data` is: `update_note` reads `verify_with` and
|
||||||
|
`expires_when` to decide whether to run the check-field guard, and an
|
||||||
|
auto-created MagicMock attribute is truthy — so a default lesson driven
|
||||||
|
through the update path would take a branch no real record takes.
|
||||||
|
"""
|
||||||
|
attrs.setdefault(
|
||||||
|
"title",
|
||||||
|
"Give absolutely-positioned siblings an explicit stacking order — "
|
||||||
|
"placing two absolutely-positioned elements in the same area",
|
||||||
|
)
|
||||||
|
attrs.setdefault("data", {"when_to_apply": "two absolute siblings overlap"})
|
||||||
|
attrs.setdefault("status", None)
|
||||||
|
attrs.setdefault("arose_from_id", None)
|
||||||
|
attrs.setdefault("verify_with", None)
|
||||||
|
attrs.setdefault("expires_when", None)
|
||||||
|
attrs.setdefault("verified_at", None)
|
||||||
|
return fake_note(note_type="lesson", **attrs)
|
||||||
|
|
||||||
|
|
||||||
def fake_project(**attrs) -> MagicMock:
|
def fake_project(**attrs) -> MagicMock:
|
||||||
"""design_system_id is explicit: a truthy auto-attribute would route every
|
"""design_system_id is explicit: a truthy auto-attribute would route every
|
||||||
project through the design-system branch and out to a real database."""
|
project through the design-system branch and out to a real database."""
|
||||||
|
|||||||
@@ -0,0 +1,298 @@
|
|||||||
|
"""A DERIVED `data` mirror survives the GENERIC note door.
|
||||||
|
|
||||||
|
Some kinds store a queryable mirror in `notes.data` that is derived from the
|
||||||
|
body: a snippet (name, when_to_use, locations…) and a lesson (what, the
|
||||||
|
trigger, what taught it). Their own updaters compose it from the field set they
|
||||||
|
just merged, so those were never the problem — the problem is every other way
|
||||||
|
the body can be written. `update_note` is a `hasattr` loop, and both doors
|
||||||
|
reach it: PATCH /api/notes/<id> and the MCP update_note tool. The Knowledge
|
||||||
|
feed hands you that path, because a card there routes to /notes/:id.
|
||||||
|
|
||||||
|
The failure is silent and the wrong way round, because every read PREFERS the
|
||||||
|
mirror — deliberately, since parsing markdown to answer what an index can
|
||||||
|
answer is how a hot path rots.
|
||||||
|
|
||||||
|
- A SNIPPET went on reporting its old repo/path/symbol to the location
|
||||||
|
reverse lookup and to prior-art recall while displaying its new body: a
|
||||||
|
record surfaced with full authority and wrong, which the drift-check
|
||||||
|
docstring calls worse than having no record at all (#3128).
|
||||||
|
|
||||||
|
- A LESSON is worse. Its mirror holds the TRIGGER, and the trigger is the
|
||||||
|
entire retrieval story — a stale one keeps the lesson firing for the
|
||||||
|
situation it used to name while it displays the one it now names.
|
||||||
|
|
||||||
|
#3128's fix was correct and did not generalise: it tested one constant, so
|
||||||
|
milestone 385's lesson arrived with the same design and none of the protection.
|
||||||
|
The registry tests below assert the PROPERTY — every kind with a derived mirror
|
||||||
|
is registered — so a third kind fails here rather than shipping quiet (#3734).
|
||||||
|
"""
|
||||||
|
import inspect
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from tests.helpers import drive_update_note as _update
|
||||||
|
from tests.helpers import fake_lesson, fake_note, fake_snippet
|
||||||
|
|
||||||
|
OLD_MIRROR = {
|
||||||
|
"name": "debounce",
|
||||||
|
"language": "javascript",
|
||||||
|
"locations": [{"repo": "Scribe", "path": "old/place.js", "symbol": "debounce"}],
|
||||||
|
"verification": {"status": "ok", "code_sha": "abc", "checked_at": "2026-01-01"},
|
||||||
|
"provenance": {"commit_sha": "deadbeef"},
|
||||||
|
}
|
||||||
|
|
||||||
|
MOVED_BODY = (
|
||||||
|
"**Locations:**\n"
|
||||||
|
"- `Scribe` · `new/place.ts` · `debounce`\n\n"
|
||||||
|
"```typescript\nexport const debounce = 1;\n```\n"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_body_write_moves_the_mirror_with_it():
|
||||||
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
||||||
|
await _update(note, body=MOVED_BODY)
|
||||||
|
assert note.data["locations"] == [
|
||||||
|
{"repo": "Scribe", "path": "new/place.ts", "symbol": "debounce"}
|
||||||
|
], "the mirror still describes where the snippet used to live"
|
||||||
|
assert note.data["language"] == "typescript"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_the_verdict_and_provenance_are_carried_not_dropped():
|
||||||
|
"""Neither is in the body to parse, so recomposing must carry them. An
|
||||||
|
ordinary edit must not erase the last drift check — and it needs no
|
||||||
|
invalidation branch either: `code_sha` is recomputed from the new code, so
|
||||||
|
a verdict stamped against the old code expires itself on read."""
|
||||||
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
||||||
|
await _update(note, body=MOVED_BODY)
|
||||||
|
assert note.data["verification"] == OLD_MIRROR["verification"]
|
||||||
|
assert note.data["provenance"] == OLD_MIRROR["provenance"]
|
||||||
|
assert note.data["code_sha"] != OLD_MIRROR["verification"]["code_sha"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_an_explicit_data_wins_over_recomposition():
|
||||||
|
"""`update_snippet` composes the mirror from the merged field set it holds
|
||||||
|
and passes it here. That caller knows things the body cannot be re-read for
|
||||||
|
— which locations were replaced, whether provenance survives the edit — so
|
||||||
|
an explicit mirror must not be recomputed out from under it."""
|
||||||
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
||||||
|
authoritative = {"name": "from the service", "locations": []}
|
||||||
|
await _update(note, body=MOVED_BODY, data=authoritative)
|
||||||
|
assert note.data == authoritative
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_plain_note_is_left_alone():
|
||||||
|
"""Only snippets carry a mirror; a note's `data` must not be invented."""
|
||||||
|
note = fake_note(note_type="note", data=None, project_id=None)
|
||||||
|
await _update(note, body="just some prose")
|
||||||
|
assert note.data is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_write_that_cannot_change_the_parse_does_not_touch_the_mirror():
|
||||||
|
"""Status, priority, project — none of them is an input to the body parser,
|
||||||
|
so recomposing on them would be work for nothing and would rebuild a mirror
|
||||||
|
from a body nobody claimed to have changed."""
|
||||||
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
||||||
|
await _update(note, project_id=4)
|
||||||
|
assert note.data == OLD_MIRROR
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_title_change_reaches_the_mirror_too():
|
||||||
|
"""A snippet's NAME lives in its title, not its body — `parse_snippet_fields`
|
||||||
|
reads both, so both are triggers."""
|
||||||
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
||||||
|
await _update(note, title="throttle — cap a callback's rate")
|
||||||
|
assert note.data["name"] == "throttle"
|
||||||
|
assert note.data["when_to_use"] == "cap a callback's rate"
|
||||||
|
|
||||||
|
|
||||||
|
# ── the registry, rather than a chain of ifs ─────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_kind_with_a_derived_mirror_is_registered():
|
||||||
|
"""The load-bearing one. Before #3734 this was a single `if` naming
|
||||||
|
snippets, and the kind added next was simply not covered."""
|
||||||
|
from scribe.services.lessons import LESSON_NOTE_TYPE
|
||||||
|
from scribe.services.notes import _mirror_recomposers
|
||||||
|
from scribe.services.snippets import SNIPPET_NOTE_TYPE
|
||||||
|
|
||||||
|
table = _mirror_recomposers()
|
||||||
|
assert SNIPPET_NOTE_TYPE in table, "the #3128 fix was lost"
|
||||||
|
assert LESSON_NOTE_TYPE in table, (
|
||||||
|
"a lesson's `data` holds its trigger and `lesson_trigger` prefers it, "
|
||||||
|
"so a body edit through the generic door would leave the lesson being "
|
||||||
|
"retrieved for a situation it no longer names (#3734)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_dispatch_is_a_lookup_not_a_named_kind():
|
||||||
|
"""Asserted on structure (rule 167). A lookup extends in one line in one
|
||||||
|
place; naming a kind inline is the shape that left lessons uncovered."""
|
||||||
|
from scribe.services import notes as notes_module
|
||||||
|
|
||||||
|
src = inspect.getsource(notes_module.update_note)
|
||||||
|
assert "_mirror_recomposers()" in src
|
||||||
|
assert "SNIPPET_NOTE_TYPE" not in src, (
|
||||||
|
"update_note names one kind again — that is the shape #3734 replaced"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_each_recomposer_takes_the_note_and_nothing_else():
|
||||||
|
"""A registry entry with the wrong signature fails inside a generic PATCH,
|
||||||
|
which is the one moment nobody is watching."""
|
||||||
|
from scribe.services.notes import _mirror_recomposers
|
||||||
|
|
||||||
|
for kind, fn in _mirror_recomposers().items():
|
||||||
|
assert callable(fn), f"{kind} maps to something not callable"
|
||||||
|
assert len(inspect.signature(fn).parameters) == 1, kind
|
||||||
|
|
||||||
|
|
||||||
|
# ── a lesson's mirror moves with its body ────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
NEW_TRIGGER = "a CI run has sat in_progress far longer than its suite takes"
|
||||||
|
|
||||||
|
|
||||||
|
def _lesson_body(trigger, insight="Read the job log.", sources=None):
|
||||||
|
from scribe.services.lessons import compose_body
|
||||||
|
return compose_body(insight, trigger, sources)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_body_write_moves_a_lessons_trigger_with_it():
|
||||||
|
from scribe.services.lessons import TRIGGER_KEY, compose_title
|
||||||
|
|
||||||
|
what = "Read the job log before waiting longer"
|
||||||
|
note = fake_lesson(
|
||||||
|
title=compose_title(what, NEW_TRIGGER),
|
||||||
|
data={TRIGGER_KEY: "a CI run is slow", "what": what},
|
||||||
|
project_id=None,
|
||||||
|
)
|
||||||
|
await _update(note, body=_lesson_body(NEW_TRIGGER))
|
||||||
|
assert note.data[TRIGGER_KEY] == NEW_TRIGGER, (
|
||||||
|
"the mirror kept the old trigger — the lesson would still be retrieved "
|
||||||
|
"for the situation it no longer names"
|
||||||
|
)
|
||||||
|
assert note.data["what"] == what
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_subject_containing_an_em_dash_still_splits():
|
||||||
|
"""Why `untrigger_title` is given the trigger instead of splitting on the
|
||||||
|
separator: a subject may legitimately contain one."""
|
||||||
|
from scribe.services.lessons import TRIGGER_KEY, compose_title
|
||||||
|
|
||||||
|
what = "A wait with no deadline — the shape, not the symptom"
|
||||||
|
trigger = "you are about to await something crossing a process boundary"
|
||||||
|
note = fake_lesson(
|
||||||
|
title=compose_title(what, trigger), data=None, project_id=None,
|
||||||
|
)
|
||||||
|
await _update(note, body=_lesson_body(trigger))
|
||||||
|
assert note.data["what"] == what
|
||||||
|
assert note.data[TRIGGER_KEY] == trigger
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dropping_the_provenance_line_drops_it_from_the_mirror():
|
||||||
|
"""The body is the authority. Carrying a value the reader just deleted is
|
||||||
|
the failure this recompose exists to prevent, not a courtesy — the
|
||||||
|
opposite call from a snippet's `verification`, which is carried because it
|
||||||
|
was never in the body to delete."""
|
||||||
|
from scribe.services.lessons import SOURCES_KEY, compose_title
|
||||||
|
|
||||||
|
note = fake_lesson(
|
||||||
|
title=compose_title("Something learned", "a situation"),
|
||||||
|
data={SOURCES_KEY: [999]},
|
||||||
|
project_id=None,
|
||||||
|
)
|
||||||
|
await _update(note, body=_lesson_body("a situation")) # no Learned from:
|
||||||
|
assert SOURCES_KEY not in note.data
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_an_explicit_data_wins_for_a_lesson_too():
|
||||||
|
"""`update_lesson` composes the mirror from the merged field set and passes
|
||||||
|
it here; that caller knows things a re-read of the body cannot recover."""
|
||||||
|
from scribe.services.lessons import TRIGGER_KEY
|
||||||
|
|
||||||
|
note = fake_lesson(data={TRIGGER_KEY: "old"}, project_id=None)
|
||||||
|
authoritative = {TRIGGER_KEY: "from the service", "what": "x"}
|
||||||
|
await _update(note, body=_lesson_body(NEW_TRIGGER), data=authoritative)
|
||||||
|
assert note.data == authoritative
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_lesson_title_change_reaches_the_mirror():
|
||||||
|
"""A lesson's subject lives in its title, so a title edit is a trigger for
|
||||||
|
recomposition exactly as it is for a snippet's name."""
|
||||||
|
from scribe.services.lessons import TRIGGER_KEY, compose_title
|
||||||
|
|
||||||
|
trigger = "two absolute siblings overlap"
|
||||||
|
note = fake_lesson(
|
||||||
|
body=_lesson_body(trigger),
|
||||||
|
data={TRIGGER_KEY: trigger, "what": "the old subject"},
|
||||||
|
project_id=None,
|
||||||
|
)
|
||||||
|
await _update(note, title=compose_title("the new subject", trigger))
|
||||||
|
assert note.data["what"] == "the new subject"
|
||||||
|
assert note.data[TRIGGER_KEY] == trigger
|
||||||
|
|
||||||
|
|
||||||
|
# ── the join and its inverse ─────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("subject", "trigger"),
|
||||||
|
[
|
||||||
|
("a subject", "a trigger"),
|
||||||
|
("a subject — with a dash", "a trigger"),
|
||||||
|
("a subject", ""),
|
||||||
|
("", "a trigger"),
|
||||||
|
("a subject", "a trigger — with a dash"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_untrigger_title_inverts_trigger_title(subject, trigger):
|
||||||
|
"""#3207's shape: the join had three copies before it was consolidated, so
|
||||||
|
its inverse lives beside it rather than in whichever caller wanted it."""
|
||||||
|
from scribe.services.embeddings import trigger_title, untrigger_title
|
||||||
|
|
||||||
|
title = trigger_title(subject, trigger)
|
||||||
|
assert untrigger_title(title, trigger) == (subject or trigger).strip()
|
||||||
|
|
||||||
|
|
||||||
|
def test_untrigger_title_degrades_to_the_whole_title():
|
||||||
|
"""A record written before the join existed still answers with something a
|
||||||
|
human recognises rather than with ""."""
|
||||||
|
from scribe.services.embeddings import untrigger_title
|
||||||
|
|
||||||
|
assert untrigger_title("a plain old title", "") == "a plain old title"
|
||||||
|
assert untrigger_title("a plain old title", "a trigger it lacks") == (
|
||||||
|
"a plain old title"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_trigger_separator_is_spelled_in_exactly_one_place():
|
||||||
|
"""Two literals that must match are one edit away from not matching."""
|
||||||
|
import pathlib
|
||||||
|
|
||||||
|
root = pathlib.Path(__file__).resolve().parents[1] / "src" / "scribe"
|
||||||
|
offenders = [
|
||||||
|
str(p.relative_to(root)) for p in root.rglob("*.py")
|
||||||
|
if '" \u2014 "' in p.read_text() and p.name != "embeddings.py"
|
||||||
|
]
|
||||||
|
assert not offenders, (
|
||||||
|
f"the trigger separator is spelled inline in {offenders} — use "
|
||||||
|
f"TRIGGER_SEP, trigger_title or untrigger_title (#3207)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_mirror_guards_can_fail():
|
||||||
|
"""Rule 167: shown turning red once."""
|
||||||
|
from scribe.services.lessons import TRIGGER_KEY, recompose_data
|
||||||
|
|
||||||
|
bare = fake_lesson(title="just a title", body="no composed lines", data=None)
|
||||||
|
assert TRIGGER_KEY not in recompose_data(bare)
|
||||||
@@ -0,0 +1,246 @@
|
|||||||
|
"""The REST door for lessons — the half the web UI can actually reach (#3734).
|
||||||
|
|
||||||
|
WHY THIS EXISTS AT ALL
|
||||||
|
|
||||||
|
Milestone 385 built the lesson kind through the MCP tools, which is the surface
|
||||||
|
an agent uses. The Vue app speaks REST, so until this blueprint existed a lesson
|
||||||
|
was a record a person could not create, read, edit or retire from the UI — rule
|
||||||
|
27's "no UI, no ship" failing at the door rather than in the view.
|
||||||
|
|
||||||
|
WHAT THESE PIN
|
||||||
|
|
||||||
|
Three things a second door tends to get wrong, and one that is specific to this
|
||||||
|
kind:
|
||||||
|
|
||||||
|
- PARITY. Both doors go through services/lessons.py, so the composed
|
||||||
|
document is identical whichever one wrote it. A REST door that composed its
|
||||||
|
own title would produce lessons that rank differently from the agent's, and
|
||||||
|
the document IS what ranks.
|
||||||
|
|
||||||
|
- ACL (rule 78). Share-aware resolve, write as the owner — the pattern
|
||||||
|
routes/snippets.py sets — so a shared editor isn't rejected by the
|
||||||
|
owner-scoped service.
|
||||||
|
|
||||||
|
- THE TRIGGER IS REFUSED WHEN EMPTY. This is the kind-specific one and the
|
||||||
|
reason the door is not a thin wrapper. The service will happily store a
|
||||||
|
lesson with no trigger: it saves, it reads correctly in every listing, and
|
||||||
|
it never surfaces. There is nothing to notice afterwards — it looks exactly
|
||||||
|
like a lesson that works. So the door refuses it at both create and update
|
||||||
|
rather than handing back a record that looks finished.
|
||||||
|
|
||||||
|
- THE REVERSE DIRECTION. `taught-by/<id>` answers "what was learned from this
|
||||||
|
record", which the task body calls the direction that gets forgotten and
|
||||||
|
arguably the more useful one.
|
||||||
|
"""
|
||||||
|
import inspect
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
|
# ── parity: one composer, two doors ──────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_both_doors_share_one_serializer():
|
||||||
|
"""A payload shape spelled once per door answers the two of them
|
||||||
|
differently the first time a field is added."""
|
||||||
|
from scribe.mcp.tools import lessons as mcp_lessons
|
||||||
|
from scribe.services import lessons as lessons_svc
|
||||||
|
|
||||||
|
assert mcp_lessons._to_dict is lessons_svc.lesson_to_dict
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_rest_door_composes_nothing_itself():
|
||||||
|
"""Asserted on structure (rule 167). The document is what ranks, so a door
|
||||||
|
that built its own title would produce lessons that rank differently from
|
||||||
|
the ones the agent writes."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes)
|
||||||
|
# It may CALL the service's composer (the dedup gate needs the document),
|
||||||
|
# but it must not assemble a title or a trigger line itself.
|
||||||
|
assert "trigger_title" not in src
|
||||||
|
assert "**When to apply:**" not in src
|
||||||
|
assert "lessons_svc.lesson_document" in src, (
|
||||||
|
"the dedup gate must hash the same document the service will store"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_door_is_registered():
|
||||||
|
"""A blueprint nobody registers is a file, not a door."""
|
||||||
|
from scribe import app as app_module
|
||||||
|
|
||||||
|
src = inspect.getsource(app_module)
|
||||||
|
assert "from scribe.routes.lessons import lessons_bp" in src
|
||||||
|
assert "app.register_blueprint(lessons_bp)" in src
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_blueprint_is_mounted_where_the_client_looks():
|
||||||
|
from scribe.routes.lessons import lessons_bp
|
||||||
|
|
||||||
|
assert lessons_bp.url_prefix == "/api/lessons"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reverse_lookup_is_registered_before_the_id_route():
|
||||||
|
"""Quart matches in registration order. The int converter protects
|
||||||
|
`taught-by` today, but the ordering is what keeps that true if the
|
||||||
|
converter is ever widened — the same care snippets' `/duplicates` takes."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes)
|
||||||
|
assert src.index('"/taught-by/<int:record_id>"') < src.index(
|
||||||
|
'"/<int:lesson_id>"'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── the trigger is not optional at this door ─────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_refuses_a_lesson_with_no_trigger():
|
||||||
|
"""The kind-specific guard. A triggerless lesson saves and never surfaces,
|
||||||
|
and nothing about the stored record shows it."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes.create_lesson_route)
|
||||||
|
assert "when_to_apply is required" in src
|
||||||
|
assert "never reaches anyone" in src, (
|
||||||
|
"the refusal must say WHY, or the next person reads it as a nag and "
|
||||||
|
"removes it"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_refuses_to_clear_the_trigger():
|
||||||
|
"""The other half. Creating without one is refused; emptying one later
|
||||||
|
would reach the same broken state by a different path."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes.update_lesson_route)
|
||||||
|
assert "cannot be cleared" in src
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_subject_is_required_too():
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes.create_lesson_route)
|
||||||
|
assert "what is required" in src
|
||||||
|
|
||||||
|
|
||||||
|
# ── ACL: rule 78's pattern, not a bare owner filter ──────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"handler", ["update_lesson_route", "delete_lesson_route"]
|
||||||
|
)
|
||||||
|
def test_writes_check_permission_and_act_as_the_owner(handler):
|
||||||
|
"""Resolve share-aware, then write as the owner — otherwise a shared
|
||||||
|
editor is rejected by the owner-scoped service."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(getattr(routes, handler))
|
||||||
|
assert "can_write_note" in src, f"{handler} does not check write permission"
|
||||||
|
assert "note.user_id" in src, (
|
||||||
|
f"{handler} writes as the caller rather than as the owner, which "
|
||||||
|
f"rejects a legitimately shared editor (rule 78)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_handler_builds_its_own_owner_filter():
|
||||||
|
"""Rule 78's actual failure mode: a route assembling its own
|
||||||
|
`Note.user_id == uid` clause instead of going through the service and the
|
||||||
|
access helpers. Passing `user_id=uid` INTO a service is the correct call
|
||||||
|
and is not what this looks for."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes)
|
||||||
|
assert "Note.user_id" not in src
|
||||||
|
assert "select(" not in src, (
|
||||||
|
"a route composing its own query has bypassed the access helpers"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_trashes_recoverably():
|
||||||
|
"""Every kind's delete is a trash, and the batch id is what restores it."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes.delete_lesson_route)
|
||||||
|
assert "trash_svc.delete" in src
|
||||||
|
assert "deleted_batch_id" in src
|
||||||
|
|
||||||
|
|
||||||
|
# ── the reverse direction ────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reverse_lookup_reads_the_indexed_mirror():
|
||||||
|
"""Not by scanning bodies: `data[taught_by]` is JSONB with a GIN index
|
||||||
|
(0070), which is the whole reason step 4 put the list there."""
|
||||||
|
from scribe.services import lessons as lessons_svc
|
||||||
|
|
||||||
|
src = inspect.getsource(lessons_svc.lessons_taught_by)
|
||||||
|
assert "path_exists" in src
|
||||||
|
assert "SOURCES_KEY" in src
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reverse_lookup_is_share_aware():
|
||||||
|
"""It renders beside a record the caller can already see, so a lesson
|
||||||
|
someone shared with them belongs in the list exactly as their own does."""
|
||||||
|
from scribe.services import lessons as lessons_svc
|
||||||
|
|
||||||
|
src = inspect.getsource(lessons_svc.lessons_taught_by)
|
||||||
|
assert "readable_notes_clause" in src
|
||||||
|
assert "deleted_at" in src
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reverse_lookup_refuses_a_nonsense_id_rather_than_interpolating():
|
||||||
|
"""The jsonpath is built by formatting, so the id has to be an int before
|
||||||
|
it gets near the expression."""
|
||||||
|
from scribe.services import lessons as lessons_svc
|
||||||
|
|
||||||
|
src = inspect.getsource(lessons_svc.lessons_taught_by)
|
||||||
|
assert "int(record_id)" in src
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_bad_id_returns_nothing_and_raises_nothing():
|
||||||
|
from scribe.services.lessons import lessons_taught_by
|
||||||
|
|
||||||
|
assert await lessons_taught_by(1, 0) == []
|
||||||
|
assert await lessons_taught_by(1, -3) == []
|
||||||
|
assert await lessons_taught_by(1, "not a number") == []
|
||||||
|
|
||||||
|
|
||||||
|
# ── the serializer speaks the vocabulary the caller wrote with ───────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_payload_reads_back_the_composed_fields():
|
||||||
|
"""A caller that wrote `when_to_apply` reads `when_to_apply` back, not a
|
||||||
|
body it has to parse."""
|
||||||
|
from tests.helpers import fake_lesson
|
||||||
|
from scribe.services.lessons import compose_body, compose_title, lesson_to_dict
|
||||||
|
|
||||||
|
what = "Read the job log before waiting longer"
|
||||||
|
trigger = "a CI run has sat in_progress longer than its suite takes"
|
||||||
|
note = fake_lesson(
|
||||||
|
id=7,
|
||||||
|
title=compose_title(what, trigger),
|
||||||
|
body=compose_body("The work is usually done.", trigger, [4181]),
|
||||||
|
data={"what": what, "when_to_apply": trigger, "taught_by": [4181]},
|
||||||
|
project_id=None,
|
||||||
|
created_at=None,
|
||||||
|
updated_at=None,
|
||||||
|
)
|
||||||
|
out = lesson_to_dict(note)
|
||||||
|
assert out["what"] == what
|
||||||
|
assert out["when_to_apply"] == trigger
|
||||||
|
assert out["learned_from"] == [4181]
|
||||||
|
# The insight comes back WITHOUT the lines compose_body added, so an edit
|
||||||
|
# form round-trips instead of accumulating a copy of them per save.
|
||||||
|
assert out["insight"] == "The work is usually done."
|
||||||
|
assert "**When to apply:**" not in out["insight"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_rest_guards_can_fail():
|
||||||
|
"""Rule 167: shown turning red once."""
|
||||||
|
from scribe.routes import lessons as routes
|
||||||
|
|
||||||
|
src = inspect.getsource(routes)
|
||||||
|
assert "a phrase that is definitely not in this module" not in src
|
||||||
|
assert "lessons_bp" in src
|
||||||
@@ -22,7 +22,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
|||||||
import pytest
|
import pytest
|
||||||
from scribe.services import plugin_context as pc
|
from scribe.services import plugin_context as pc
|
||||||
from scribe.services.lessons import LESSON_NOTE_TYPE
|
from scribe.services.lessons import LESSON_NOTE_TYPE
|
||||||
from tests.helpers import fake_note, writepath_cfg
|
from tests.helpers import fake_lesson, fake_note, writepath_cfg
|
||||||
|
|
||||||
pytestmark = pytest.mark.usefixtures("_no_supersession")
|
pytestmark = pytest.mark.usefixtures("_no_supersession")
|
||||||
|
|
||||||
@@ -33,23 +33,6 @@ _CFG = {"enabled": True, "threshold": 0.55, "top_k": 3}
|
|||||||
_BINDING_PHRASE = "before deciding it does not apply"
|
_BINDING_PHRASE = "before deciding it does not apply"
|
||||||
|
|
||||||
|
|
||||||
def fake_lesson(**attrs):
|
|
||||||
"""A stand-in lesson: a note whose `note_type` is what makes it one.
|
|
||||||
|
|
||||||
The title carries the trigger because `compose_title` builds it that way —
|
|
||||||
`{what} — {when it applies}` — so a menu line rendering only the title is
|
|
||||||
already showing the reader when this lesson applies. Tests that used a bare
|
|
||||||
title here would be testing a record the product cannot create.
|
|
||||||
"""
|
|
||||||
attrs.setdefault(
|
|
||||||
"title",
|
|
||||||
"Give absolutely-positioned siblings an explicit stacking order — "
|
|
||||||
"placing two absolutely-positioned elements in the same area",
|
|
||||||
)
|
|
||||||
attrs.setdefault("data", {"when_to_apply": "two absolute siblings overlap"})
|
|
||||||
return fake_note(note_type=LESSON_NOTE_TYPE, **attrs)
|
|
||||||
|
|
||||||
|
|
||||||
async def _menu(main_hits, *, lesson_hits=None, reuse_hits=None, cfg=None,
|
async def _menu(main_hits, *, lesson_hits=None, reuse_hits=None, cfg=None,
|
||||||
exclude_ids=None, rec=None, surf=None):
|
exclude_ids=None, rec=None, surf=None):
|
||||||
"""Run the prompt menu with each query stubbed by the kinds it asks for."""
|
"""Run the prompt menu with each query stubbed by the kinds it asks for."""
|
||||||
|
|||||||
@@ -1,95 +0,0 @@
|
|||||||
"""A snippet's `data` mirror survives the GENERIC note door.
|
|
||||||
|
|
||||||
`notes.data` is derived from the body. The snippet service always composed it
|
|
||||||
from the field set it had just merged, so `update_snippet` was never the
|
|
||||||
problem — the problem was every other way a snippet's body could be written.
|
|
||||||
`update_note` is a `hasattr` loop with no snippet awareness, and both doors
|
|
||||||
reach it: PATCH /api/notes/<id> and the MCP update_note tool. The Knowledge
|
|
||||||
feed handed you that path, because a snippet card there routed to /notes/:id.
|
|
||||||
|
|
||||||
The failure was silent and the wrong way round: `snippet_fields` PREFERS the
|
|
||||||
mirror, so the row went on reporting its old repo/path/symbol to the location
|
|
||||||
reverse lookup and to prior-art recall while displaying its new body — a record
|
|
||||||
surfaced with full authority and wrong, which the drift-check docstring calls
|
|
||||||
worse than having no record at all (#3128).
|
|
||||||
"""
|
|
||||||
import pytest
|
|
||||||
from tests.helpers import drive_update_note as _update
|
|
||||||
from tests.helpers import fake_note, fake_snippet
|
|
||||||
|
|
||||||
OLD_MIRROR = {
|
|
||||||
"name": "debounce",
|
|
||||||
"language": "javascript",
|
|
||||||
"locations": [{"repo": "Scribe", "path": "old/place.js", "symbol": "debounce"}],
|
|
||||||
"verification": {"status": "ok", "code_sha": "abc", "checked_at": "2026-01-01"},
|
|
||||||
"provenance": {"commit_sha": "deadbeef"},
|
|
||||||
}
|
|
||||||
|
|
||||||
MOVED_BODY = (
|
|
||||||
"**Locations:**\n"
|
|
||||||
"- `Scribe` · `new/place.ts` · `debounce`\n\n"
|
|
||||||
"```typescript\nexport const debounce = 1;\n```\n"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_a_body_write_moves_the_mirror_with_it():
|
|
||||||
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
||||||
await _update(note, body=MOVED_BODY)
|
|
||||||
assert note.data["locations"] == [
|
|
||||||
{"repo": "Scribe", "path": "new/place.ts", "symbol": "debounce"}
|
|
||||||
], "the mirror still describes where the snippet used to live"
|
|
||||||
assert note.data["language"] == "typescript"
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_the_verdict_and_provenance_are_carried_not_dropped():
|
|
||||||
"""Neither is in the body to parse, so recomposing must carry them. An
|
|
||||||
ordinary edit must not erase the last drift check — and it needs no
|
|
||||||
invalidation branch either: `code_sha` is recomputed from the new code, so
|
|
||||||
a verdict stamped against the old code expires itself on read."""
|
|
||||||
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
||||||
await _update(note, body=MOVED_BODY)
|
|
||||||
assert note.data["verification"] == OLD_MIRROR["verification"]
|
|
||||||
assert note.data["provenance"] == OLD_MIRROR["provenance"]
|
|
||||||
assert note.data["code_sha"] != OLD_MIRROR["verification"]["code_sha"]
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_an_explicit_data_wins_over_recomposition():
|
|
||||||
"""`update_snippet` composes the mirror from the merged field set it holds
|
|
||||||
and passes it here. That caller knows things the body cannot be re-read for
|
|
||||||
— which locations were replaced, whether provenance survives the edit — so
|
|
||||||
an explicit mirror must not be recomputed out from under it."""
|
|
||||||
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
||||||
authoritative = {"name": "from the service", "locations": []}
|
|
||||||
await _update(note, body=MOVED_BODY, data=authoritative)
|
|
||||||
assert note.data == authoritative
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_a_plain_note_is_left_alone():
|
|
||||||
"""Only snippets carry a mirror; a note's `data` must not be invented."""
|
|
||||||
note = fake_note(note_type="note", data=None, project_id=None)
|
|
||||||
await _update(note, body="just some prose")
|
|
||||||
assert note.data is None
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_a_write_that_cannot_change_the_parse_does_not_touch_the_mirror():
|
|
||||||
"""Status, priority, project — none of them is an input to the body parser,
|
|
||||||
so recomposing on them would be work for nothing and would rebuild a mirror
|
|
||||||
from a body nobody claimed to have changed."""
|
|
||||||
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
||||||
await _update(note, project_id=4)
|
|
||||||
assert note.data == OLD_MIRROR
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_a_title_change_reaches_the_mirror_too():
|
|
||||||
"""A snippet's NAME lives in its title, not its body — `parse_snippet_fields`
|
|
||||||
reads both, so both are triggers."""
|
|
||||||
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
||||||
await _update(note, title="throttle — cap a callback's rate")
|
|
||||||
assert note.data["name"] == "throttle"
|
|
||||||
assert note.data["when_to_use"] == "cap a callback's rate"
|
|
||||||
Reference in New Issue
Block a user