Drafter hardening (milestone #232) — plugin hook fix, usage signal, drift check, duplicate finder, un-merge #82
@@ -0,0 +1,72 @@
|
||||
"""add note_usage_events — did anyone actually open what we surfaced?
|
||||
|
||||
Revision ID: 0071
|
||||
Revises: 0070
|
||||
Create Date: 2026-07-28
|
||||
|
||||
`retrieval_logs` records what the ranker returned and with what scores, which is
|
||||
the right substrate for tuning a similarity threshold. It cannot answer the
|
||||
different question the snippet corpus needs: was a surfaced snippet ever pulled
|
||||
in full? A snippet nobody opens still competes for the injection budget on every
|
||||
turn, so the surfaced:pulled ratio is what makes dead weight visible.
|
||||
|
||||
Two reasons this is its own table rather than columns on `notes` or rows in
|
||||
`retrieval_logs`:
|
||||
|
||||
- Counters on `notes` would answer "how many" but not "when, from where, and
|
||||
by which arm" — and the place arm vs semantic arm comparison is precisely
|
||||
what was missing (the write-path place arm surfaced snippets while leaving
|
||||
no trace anywhere).
|
||||
- Folding un-scored surfacing into `retrieval_logs` would corrupt the score
|
||||
distribution that table exists to capture. Location hits have no score.
|
||||
|
||||
Grain is one row per note per event, which is what the per-snippet readout needs
|
||||
and what `retrieval_logs.result_ids` (a JSONB array, one row per *call*) cannot
|
||||
be indexed at.
|
||||
|
||||
FK-free on note_id and user_id, matching retrieval_logs and app_logs: telemetry
|
||||
should outlive what it describes. Deleting a note must not erase the evidence
|
||||
that it was surfaced forty times and opened none.
|
||||
|
||||
Downgrade drops the table outright. The data is purely observational — nothing
|
||||
reads it for correctness, so losing it costs history and no behavior.
|
||||
"""
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
|
||||
revision = "0071"
|
||||
down_revision = "0070"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.create_table(
|
||||
"note_usage_events",
|
||||
sa.Column("id", sa.Integer(), primary_key=True),
|
||||
sa.Column(
|
||||
"created_at",
|
||||
sa.DateTime(timezone=True),
|
||||
nullable=False,
|
||||
server_default=sa.text("now()"),
|
||||
),
|
||||
sa.Column("user_id", sa.Integer(), nullable=True),
|
||||
sa.Column("note_id", sa.Integer(), nullable=False),
|
||||
sa.Column("event", sa.Text(), nullable=False),
|
||||
sa.Column("source", sa.Text(), nullable=False),
|
||||
)
|
||||
# Every readout is "these note ids, split by event", so the composite is the
|
||||
# one that actually gets used; the others serve pruning and per-user views.
|
||||
op.create_index(
|
||||
"ix_note_usage_note_event", "note_usage_events", ["note_id", "event"]
|
||||
)
|
||||
op.create_index("ix_note_usage_created_at", "note_usage_events", ["created_at"])
|
||||
op.create_index("ix_note_usage_user_id", "note_usage_events", ["user_id"])
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_index("ix_note_usage_user_id", table_name="note_usage_events")
|
||||
op.drop_index("ix_note_usage_created_at", table_name="note_usage_events")
|
||||
op.drop_index("ix_note_usage_note_event", table_name="note_usage_events")
|
||||
op.drop_table("note_usage_events")
|
||||
@@ -20,9 +20,13 @@ export interface SnippetFields {
|
||||
path: string;
|
||||
symbol: string;
|
||||
locations: SnippetLocation[];
|
||||
/** Ids of the snippets folded into this one by merge, oldest first. Read-only:
|
||||
* merge is the only thing that adds to it, and an edit carries it forward. */
|
||||
merged_from: number[];
|
||||
/** The snippets folded into this one by merge, oldest first. Read-only: merge
|
||||
* is the only thing that adds to it, and an edit carries it forward.
|
||||
*
|
||||
* Each entry records what THAT source contributed — never what the survivor
|
||||
* already had — which is what lets un-merge subtract exactly. An entry with
|
||||
* no `locations`/`tags` predates that attribution and cannot be un-merged. */
|
||||
merged_from: { id: number; locations?: SnippetLocation[]; tags?: string[] }[];
|
||||
code: string;
|
||||
}
|
||||
|
||||
@@ -46,6 +50,29 @@ export interface Snippet {
|
||||
owner?: string | null;
|
||||
}
|
||||
|
||||
/** How often a record was put in front of an agent versus actually opened.
|
||||
* A high `surfaced_count` with `pull_count: 0` is dead weight — it occupies a
|
||||
* slot in every future auto-inject menu while never being used. */
|
||||
export interface SnippetUsage {
|
||||
surfaced_count: number;
|
||||
pull_count: number;
|
||||
last_surfaced_at: string | null;
|
||||
last_pulled_at: string | null;
|
||||
}
|
||||
|
||||
/** Result of the last drift check — does the recorded location and code still
|
||||
* match source? The check runs agent-side (Scribe has no checkout); this is the
|
||||
* remembered verdict. `current` is false once the snippet has been edited since
|
||||
* the check, at which point the verdict describes code that's no longer there. */
|
||||
export interface SnippetVerification {
|
||||
status: "ok" | "missing" | "moved" | "changed" | "unverified";
|
||||
current: boolean;
|
||||
checked_at: string | null;
|
||||
detail?: string | null;
|
||||
path?: string | null;
|
||||
needs_attention?: boolean;
|
||||
}
|
||||
|
||||
/** Lightweight list item from the knowledge preview feed. Note: the `snippet`
|
||||
* field here is a truncated *body preview* (the knowledge feed's naming), not
|
||||
* the parsed fields above. */
|
||||
@@ -61,6 +88,11 @@ export interface SnippetListItem {
|
||||
* them, not one of your own. Absent means it's yours. */
|
||||
shared?: boolean;
|
||||
owner?: string | null;
|
||||
/** Always present from the backend, zero-filled for records with no events. */
|
||||
usage?: SnippetUsage;
|
||||
/** Present on the detail record; the list feed carries it when a check has
|
||||
* been recorded. */
|
||||
verification?: SnippetVerification;
|
||||
}
|
||||
|
||||
/** Create/update payload — discrete fields the backend serializes into the
|
||||
@@ -91,6 +123,8 @@ export async function listSnippets(
|
||||
repo?: string;
|
||||
path?: string;
|
||||
symbol?: string;
|
||||
/** Drift check: "attention" | "ok" | "unverified" | "drifted" | a status. */
|
||||
verification?: string;
|
||||
} = {},
|
||||
): Promise<{ snippets: SnippetListItem[]; total: number }> {
|
||||
const qs = new URLSearchParams();
|
||||
@@ -100,6 +134,7 @@ export async function listSnippets(
|
||||
if (params.repo) qs.set("repo", params.repo);
|
||||
if (params.path) qs.set("path", params.path);
|
||||
if (params.symbol) qs.set("symbol", params.symbol);
|
||||
if (params.verification) qs.set("verification", params.verification);
|
||||
const query = qs.toString();
|
||||
return apiGet(`/api/snippets${query ? `?${query}` : ""}`);
|
||||
}
|
||||
@@ -123,6 +158,45 @@ export async function deleteSnippet(id: number): Promise<void> {
|
||||
return apiDelete(`/api/snippets/${id}`);
|
||||
}
|
||||
|
||||
/** A set of snippets that resemble each other closely enough to be worth
|
||||
* merging. Grouping is transitive, so a set can hold members that don't
|
||||
* directly resemble each other — read it as a proposal, not a verdict. */
|
||||
export interface DuplicateGroup {
|
||||
note_ids: number[];
|
||||
snippets: { id: number; title: string }[];
|
||||
/** The strongest resemblance within the set — how confident the suggestion is. */
|
||||
top_score: number;
|
||||
}
|
||||
|
||||
/** Near-duplicates already in the record. The create gate prevents new ones and
|
||||
* merge cures the ones you point it at; this is what finds them. */
|
||||
export async function findDuplicateSnippets(
|
||||
threshold?: number,
|
||||
): Promise<{ groups: DuplicateGroup[]; threshold: number }> {
|
||||
const qs = threshold ? `?threshold=${threshold}` : "";
|
||||
return apiGet(`/api/snippets/duplicates${qs}`);
|
||||
}
|
||||
|
||||
/** Record a drift-check verdict. The check itself runs where the code is — an
|
||||
* agent with the working tree — since Scribe has no checkout. This stores what
|
||||
* was found, and is how the UI clears a stale marker after a manual fix. */
|
||||
export async function verifySnippet(
|
||||
id: number,
|
||||
verdict: { status: string; detail?: string; path?: string },
|
||||
): Promise<Snippet> {
|
||||
return apiPost(`/api/snippets/${id}/verify`, verdict);
|
||||
}
|
||||
|
||||
/** Pull one source back out of a merged survivor: restores it and strips exactly
|
||||
* what it contributed. Also repairs a half-undone merge — a source restored
|
||||
* from the trash by hand leaves the survivor still claiming its call sites. */
|
||||
export async function unmergeSnippet(
|
||||
survivorId: number,
|
||||
sourceId: number,
|
||||
): Promise<{ survivor: Snippet; restored: Snippet | null }> {
|
||||
return apiPost(`/api/snippets/${survivorId}/unmerge`, { source_id: sourceId });
|
||||
}
|
||||
|
||||
/** Unify `sourceIds` into the canonical snippet `targetId`. Returns the merged
|
||||
* survivor plus `merged_ids` — the sources actually folded in and trashed. */
|
||||
export async function mergeSnippets(
|
||||
|
||||
@@ -23,6 +23,10 @@ const kbInjectEnabled = ref(true);
|
||||
const kbInjectThreshold = ref("0.55");
|
||||
const kbInjectTopK = ref("3");
|
||||
const kbWritePathEnabled = ref(true);
|
||||
// Near-duplicate report floor. Deliberately looser than the 0.90 write-time
|
||||
// gate: that one BLOCKS a create and must be unforgiving of noise, this one only
|
||||
// suggests a merge the operator reviews (services/dedup.py).
|
||||
const kbDuplicateThreshold = ref("0.82");
|
||||
const savingKbInject = ref(false);
|
||||
const kbInjectSaved = ref(false);
|
||||
|
||||
@@ -68,8 +72,13 @@ async function saveRetention() {
|
||||
async function saveKbInject() {
|
||||
const t = Math.min(1, Math.max(0, Number(kbInjectThreshold.value) || 0));
|
||||
const k = Math.min(10, Math.max(1, Math.floor(Number(kbInjectTopK.value) || 1)));
|
||||
// `|| 0.82` not `|| 0`: an unparseable value here should fall back to the
|
||||
// default, not to 0 — a 0 floor would report every snippet as a duplicate of
|
||||
// every other one.
|
||||
const dupT = Math.min(1, Math.max(0, Number(kbDuplicateThreshold.value) || 0.82));
|
||||
kbInjectThreshold.value = String(t);
|
||||
kbInjectTopK.value = String(k);
|
||||
kbDuplicateThreshold.value = String(dupT);
|
||||
savingKbInject.value = true;
|
||||
kbInjectSaved.value = false;
|
||||
try {
|
||||
@@ -80,6 +89,7 @@ async function saveKbInject() {
|
||||
// Its own switch, but deliberately the same threshold/ceiling — see
|
||||
// WRITEPATH_ENABLED_KEY in services/plugin_context.py.
|
||||
kb_writepath_enabled: kbWritePathEnabled.value ? 'true' : 'false',
|
||||
kb_duplicate_threshold: String(dupT),
|
||||
});
|
||||
kbInjectSaved.value = true;
|
||||
setTimeout(() => (kbInjectSaved.value = false), 2000);
|
||||
@@ -464,6 +474,9 @@ onMounted(async () => {
|
||||
kbInjectTopK.value = allSettings.kb_autoinject_top_k;
|
||||
}
|
||||
kbWritePathEnabled.value = allSettings.kb_writepath_enabled !== "false";
|
||||
if (allSettings.kb_duplicate_threshold !== undefined) {
|
||||
kbDuplicateThreshold.value = allSettings.kb_duplicate_threshold;
|
||||
}
|
||||
if (allSettings.notify_task_reminders !== undefined) {
|
||||
notifyTaskReminders.value = allSettings.notify_task_reminders !== "false";
|
||||
}
|
||||
@@ -1211,6 +1224,25 @@ function formatUserDate(iso: string): string {
|
||||
edit. Off = prior art surfaces only on your own prompts.
|
||||
</p>
|
||||
</div>
|
||||
<div class="field">
|
||||
<label for="kb-duplicate-threshold">Near-duplicate report threshold</label>
|
||||
<input
|
||||
id="kb-duplicate-threshold"
|
||||
v-model="kbDuplicateThreshold"
|
||||
type="number"
|
||||
min="0"
|
||||
max="1"
|
||||
step="0.01"
|
||||
class="input"
|
||||
style="max-width: 8rem"
|
||||
/>
|
||||
<p class="field-hint">
|
||||
How alike two snippets must be before the Snippets page suggests merging
|
||||
them. Lower = more suggestions, more false pairs. Looser than the 0.90
|
||||
used to block a duplicate at creation, because this only proposes a merge
|
||||
you review — it never acts on its own.
|
||||
</p>
|
||||
</div>
|
||||
<div class="actions">
|
||||
<button class="btn-save" @click="saveKbInject" :disabled="savingKbInject">
|
||||
{{ savingKbInject ? 'Saving…' : 'Save' }}
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
<script setup lang="ts">
|
||||
import { ref, computed, onMounted } from "vue";
|
||||
import { useRoute, useRouter } from "vue-router";
|
||||
import { getSnippet, deleteSnippet, type Snippet } from "@/api/snippets";
|
||||
import {
|
||||
getSnippet,
|
||||
deleteSnippet,
|
||||
unmergeSnippet,
|
||||
type Snippet,
|
||||
} from "@/api/snippets";
|
||||
import { useToastStore } from "@/stores/toast";
|
||||
import ConfirmDialog from "@/components/ConfirmDialog.vue";
|
||||
|
||||
@@ -31,6 +36,32 @@ function locParts(loc: { repo: string; path: string; symbol: string }): string[]
|
||||
return [loc.repo, loc.path, loc.symbol].filter((p) => p && p.trim());
|
||||
}
|
||||
|
||||
// Un-merge (#2165)
|
||||
const unmerging = ref<number | null>(null);
|
||||
|
||||
/** Only entries carrying what the source contributed can be reversed exactly.
|
||||
* Without that, subtraction would be a guess that could strip call sites the
|
||||
* survivor owns in its own right — so the control isn't offered. */
|
||||
function canUnmerge(entry: { locations?: unknown[]; tags?: unknown[] }): boolean {
|
||||
return canWrite.value && (!!entry.locations?.length || !!entry.tags?.length);
|
||||
}
|
||||
|
||||
async function doUnmerge(sourceId: number) {
|
||||
unmerging.value = sourceId;
|
||||
try {
|
||||
await unmergeSnippet(id.value, sourceId);
|
||||
toast.show(`#${sourceId} pulled back out and restored`);
|
||||
await load();
|
||||
} catch (e: unknown) {
|
||||
// 409 carries the reason the record's state makes it impossible — show it
|
||||
// rather than a generic failure, since it's the actionable part.
|
||||
const detail = (e as { body?: { error?: string } }).body?.error;
|
||||
toast.show(detail || `Couldn't un-merge #${sourceId}`, "error");
|
||||
} finally {
|
||||
unmerging.value = null;
|
||||
}
|
||||
}
|
||||
|
||||
async function load() {
|
||||
loading.value = true;
|
||||
error.value = null;
|
||||
@@ -117,7 +148,27 @@ async function confirmDelete() {
|
||||
<template v-if="mergedFrom.length">
|
||||
<dt>Merged from</dt>
|
||||
<dd class="merged-from">
|
||||
<span v-for="mid in mergedFrom" :key="mid">#{{ mid }}</span>
|
||||
<span v-for="m in mergedFrom" :key="m.id" class="merged-entry">
|
||||
<span>#{{ m.id }}</span>
|
||||
<button
|
||||
v-if="canUnmerge(m)"
|
||||
class="unmerge-btn"
|
||||
:disabled="unmerging === m.id"
|
||||
:title="`Pull #${m.id} back out: restore it and remove the ${(m.locations ?? []).length} location(s) it contributed`"
|
||||
@click="doUnmerge(m.id)"
|
||||
>
|
||||
{{ unmerging === m.id ? "…" : "un-merge" }}
|
||||
</button>
|
||||
<!-- Says why rather than hiding the control: a disabled thing with
|
||||
no explanation reads as a bug. -->
|
||||
<span
|
||||
v-else-if="canWrite"
|
||||
class="unmerge-na"
|
||||
:title="`This merge predates per-source provenance, so what #${m.id} contributed isn't recorded. Restore it from the trash and adjust both records by hand — subtracting a guess could strip call sites this record genuinely owns.`"
|
||||
>
|
||||
(not reversible)
|
||||
</span>
|
||||
</span>
|
||||
<span class="merged-hint">
|
||||
folded in here — the originals are in the trash
|
||||
</span>
|
||||
@@ -297,6 +348,34 @@ async function confirmDelete() {
|
||||
color: var(--color-text-muted);
|
||||
font-size: 0.8rem;
|
||||
}
|
||||
.merged-entry {
|
||||
display: inline-flex;
|
||||
align-items: baseline;
|
||||
gap: 0.3rem;
|
||||
}
|
||||
.unmerge-btn {
|
||||
font-size: 0.72rem;
|
||||
padding: 0.05rem 0.35rem;
|
||||
border: 1px solid var(--color-border);
|
||||
border-radius: 4px;
|
||||
background: transparent;
|
||||
color: var(--color-text-muted);
|
||||
cursor: pointer;
|
||||
}
|
||||
.unmerge-btn:hover:not(:disabled) {
|
||||
color: var(--color-text);
|
||||
border-color: var(--color-text-muted);
|
||||
}
|
||||
.unmerge-btn:disabled {
|
||||
opacity: 0.6;
|
||||
cursor: default;
|
||||
}
|
||||
.unmerge-na {
|
||||
font-size: 0.72rem;
|
||||
color: var(--color-text-muted);
|
||||
/* Cursor cues that the explanation is in the tooltip. */
|
||||
cursor: help;
|
||||
}
|
||||
|
||||
.code-block {
|
||||
border: 1px solid var(--color-border);
|
||||
|
||||
@@ -1,7 +1,13 @@
|
||||
<script setup lang="ts">
|
||||
import { ref, computed, onMounted } from "vue";
|
||||
import { useRouter } from "vue-router";
|
||||
import { listSnippets, mergeSnippets, type SnippetListItem } from "@/api/snippets";
|
||||
import {
|
||||
findDuplicateSnippets,
|
||||
listSnippets,
|
||||
mergeSnippets,
|
||||
type DuplicateGroup,
|
||||
type SnippetListItem,
|
||||
} from "@/api/snippets";
|
||||
import { useToastStore } from "@/stores/toast";
|
||||
|
||||
const router = useRouter();
|
||||
@@ -34,6 +40,15 @@ function toggleLocationFilter() {
|
||||
if (!showLocationFilter.value && locationActive.value) clearLocation();
|
||||
}
|
||||
|
||||
// Drift check (#2086) — "attention" is everything whose recorded location or
|
||||
// code no longer checks out, plus everything whose verdict expired because the
|
||||
// snippet was edited since it was checked.
|
||||
const needsAttentionOnly = ref(false);
|
||||
function toggleAttention() {
|
||||
needsAttentionOnly.value = !needsAttentionOnly.value;
|
||||
loadSnippets();
|
||||
}
|
||||
|
||||
// Multi-select → merge
|
||||
const selectMode = ref(false);
|
||||
const selectedIds = ref<Set<number>>(new Set());
|
||||
@@ -41,13 +56,54 @@ const showMergeModal = ref(false);
|
||||
const canonicalId = ref<number | null>(null);
|
||||
const merging = ref(false);
|
||||
|
||||
const selectedList = computed(() =>
|
||||
snippets.value.filter((s) => selectedIds.value.has(s.id)),
|
||||
);
|
||||
// Near-duplicate report (#2088). Loaded on demand, not with the list: it's a
|
||||
// pairwise scan and most visits to this page aren't a tidy-up.
|
||||
const duplicateGroups = ref<DuplicateGroup[]>([]);
|
||||
const dupLoading = ref(false);
|
||||
const dupChecked = ref(false);
|
||||
// Set while merging a SUGGESTED group. The report reaches the whole corpus, so
|
||||
// its members need not all be on the current page — see selectedList.
|
||||
const reviewingGroup = ref<DuplicateGroup | null>(null);
|
||||
|
||||
/** The records the merge modal acts on.
|
||||
*
|
||||
* Normally that's the selection filtered against what's on screen. But a
|
||||
* suggested group is corpus-wide: filtering it by the current page would render
|
||||
* an incomplete set AND silently narrow what doMerge folds in, since it derives
|
||||
* its source ids from this list. When a group is under review it is the
|
||||
* authority. */
|
||||
const selectedList = computed<{ id: number; title: string }[]>(() => {
|
||||
if (reviewingGroup.value) return reviewingGroup.value.snippets;
|
||||
return snippets.value.filter((s) => selectedIds.value.has(s.id));
|
||||
});
|
||||
|
||||
async function loadDuplicates() {
|
||||
dupLoading.value = true;
|
||||
try {
|
||||
const data = await findDuplicateSnippets();
|
||||
duplicateGroups.value = data.groups;
|
||||
dupChecked.value = true;
|
||||
} catch {
|
||||
toast.show("Couldn't check for duplicates", "error");
|
||||
} finally {
|
||||
dupLoading.value = false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Hand a suggested group to the existing merge flow, pre-selected. The operator
|
||||
* still picks which record survives and confirms — the report proposes, it
|
||||
* never merges. */
|
||||
function reviewGroup(group: DuplicateGroup) {
|
||||
reviewingGroup.value = group;
|
||||
selectedIds.value = new Set(group.note_ids);
|
||||
canonicalId.value = group.note_ids[0] ?? null;
|
||||
showMergeModal.value = true;
|
||||
}
|
||||
|
||||
function exitSelectMode() {
|
||||
selectMode.value = false;
|
||||
selectedIds.value = new Set();
|
||||
reviewingGroup.value = null;
|
||||
}
|
||||
function toggleSelectMode() {
|
||||
if (selectMode.value) exitSelectMode();
|
||||
@@ -68,6 +124,15 @@ function openMerge() {
|
||||
canonicalId.value = selectedList.value[0]?.id ?? null;
|
||||
showMergeModal.value = true;
|
||||
}
|
||||
/** Dismiss the modal. Clears the reviewed group too — leaving it set would keep
|
||||
* selectedList pinned to a corpus-wide set the operator has walked away from. */
|
||||
function closeMerge() {
|
||||
showMergeModal.value = false;
|
||||
if (reviewingGroup.value) {
|
||||
reviewingGroup.value = null;
|
||||
selectedIds.value = new Set();
|
||||
}
|
||||
}
|
||||
async function doMerge() {
|
||||
const target = canonicalId.value;
|
||||
if (target == null) return;
|
||||
@@ -78,8 +143,13 @@ async function doMerge() {
|
||||
await mergeSnippets(target, sources);
|
||||
toast.show(`Merged ${sources.length} snippet${sources.length > 1 ? "s" : ""} in`);
|
||||
showMergeModal.value = false;
|
||||
const wasSuggested = reviewingGroup.value !== null;
|
||||
exitSelectMode();
|
||||
await loadSnippets();
|
||||
// The merged-away records are gone, so a stale report would keep offering
|
||||
// them. Re-run it rather than clearing, so the operator can work through
|
||||
// several groups without re-triggering the scan each time.
|
||||
if (wasSuggested && dupChecked.value) await loadDuplicates();
|
||||
} catch {
|
||||
toast.show("Failed to merge snippets", "error");
|
||||
} finally {
|
||||
@@ -96,6 +166,7 @@ async function loadSnippets() {
|
||||
repo: locRepo.value.trim() || undefined,
|
||||
path: locPath.value.trim() || undefined,
|
||||
symbol: locSymbol.value.trim() || undefined,
|
||||
verification: needsAttentionOnly.value ? "attention" : undefined,
|
||||
});
|
||||
snippets.value = data.snippets;
|
||||
} catch {
|
||||
@@ -126,6 +197,79 @@ function splitTitle(title: string): { name: string; when: string } {
|
||||
function languageOf(tags: string[]): string {
|
||||
return tags.find((t) => t && t !== "snippet") ?? "";
|
||||
}
|
||||
|
||||
/** A snippet that has been offered repeatedly and never opened. The threshold
|
||||
* is 3 rather than 1 because one or two surfacings is noise — the record may
|
||||
* simply not have come up in a relevant context yet. */
|
||||
function isDeadWeight(s: SnippetListItem): boolean {
|
||||
const u = s.usage;
|
||||
return !!u && u.pull_count === 0 && u.surfaced_count >= 3;
|
||||
}
|
||||
|
||||
/** Short badge text, or "" to render nothing. A record nobody has surfaced yet
|
||||
* gets no badge at all: "0 / 0" would read as a verdict when it's an absence
|
||||
* of evidence. */
|
||||
function usageBadge(s: SnippetListItem): string {
|
||||
const u = s.usage;
|
||||
if (!u || u.surfaced_count === 0) return "";
|
||||
return `${u.pull_count}/${u.surfaced_count} used`;
|
||||
}
|
||||
|
||||
/** Short label for the drift verdict, or "" when there's nothing to say.
|
||||
* An expired verdict is reported as "unchecked" whatever it used to say —
|
||||
* it was about code that is no longer in the record. */
|
||||
function driftBadge(s: SnippetListItem): string {
|
||||
const v = s.verification;
|
||||
if (!v) return "";
|
||||
if (!v.current) return "unchecked since edit";
|
||||
// Record<string, string>, not an object literal: `status` is a union that
|
||||
// includes "ok" and "unverified", and a literal would have to enumerate every
|
||||
// member just to say "nothing to show for these".
|
||||
const labels: Record<string, string> = {
|
||||
missing: "path gone",
|
||||
moved: "symbol moved",
|
||||
changed: "code drifted",
|
||||
};
|
||||
return labels[v.status] ?? "";
|
||||
}
|
||||
|
||||
function driftTitle(s: SnippetListItem): string {
|
||||
const v = s.verification;
|
||||
if (!v) return "";
|
||||
const when = v.checked_at
|
||||
? `Checked ${new Date(v.checked_at).toLocaleDateString()}`
|
||||
: "Checked";
|
||||
if (!v.current) {
|
||||
return (
|
||||
`${when}, but the snippet has been edited since — that verdict was about ` +
|
||||
`code this record no longer holds. Re-verify it.`
|
||||
);
|
||||
}
|
||||
const reasons: Record<string, string> = {
|
||||
missing: "the recorded path no longer exists",
|
||||
moved: "the file is there but the symbol isn't in it",
|
||||
changed: "the source no longer matches the recorded code",
|
||||
};
|
||||
const what = reasons[v.status] ?? "";
|
||||
return v.detail ? `${when}: ${what}. ${v.detail}` : `${when}: ${what}.`;
|
||||
}
|
||||
|
||||
function usageTitle(s: SnippetListItem): string {
|
||||
const u = s.usage;
|
||||
if (!u) return "";
|
||||
const last = u.last_pulled_at
|
||||
? `Last opened ${new Date(u.last_pulled_at).toLocaleDateString()}.`
|
||||
: "Never opened.";
|
||||
const verdict = isDeadWeight(s)
|
||||
? " Offered repeatedly without ever being opened — consider rewriting its" +
|
||||
" “when to reach for it” so it says when, or deleting it. It takes a slot" +
|
||||
" in every future auto-inject menu."
|
||||
: "";
|
||||
return (
|
||||
`Surfaced to an agent ${u.surfaced_count}×, opened in full ` +
|
||||
`${u.pull_count}×. ${last}${verdict}`
|
||||
);
|
||||
}
|
||||
</script>
|
||||
|
||||
<template>
|
||||
@@ -167,6 +311,47 @@ function languageOf(tags: string[]): string {
|
||||
a screen reader too. -->
|
||||
{{ locationActive ? "Location · filtering" : "Location" }}
|
||||
</button>
|
||||
<button
|
||||
class="btn-ghost"
|
||||
:class="{ 'filter-on': needsAttentionOnly }"
|
||||
:aria-pressed="needsAttentionOnly"
|
||||
title="Show only snippets whose recorded location or code no longer checks out — including ones edited since they were last verified"
|
||||
@click="toggleAttention"
|
||||
>
|
||||
{{ needsAttentionOnly ? "Needs attention · filtering" : "Needs attention" }}
|
||||
</button>
|
||||
<button
|
||||
class="btn-ghost"
|
||||
:disabled="dupLoading"
|
||||
title="Look for snippets already recorded that resemble each other closely enough to be worth merging"
|
||||
@click="loadDuplicates"
|
||||
>
|
||||
{{ dupLoading ? "Checking…" : "Find duplicates" }}
|
||||
</button>
|
||||
</div>
|
||||
|
||||
<!-- Near-duplicate report. Only ever a proposal — merging is a separate,
|
||||
confirmed act, and the operator chooses which record survives. -->
|
||||
<div v-if="dupChecked && !dupLoading" class="dup-panel">
|
||||
<p v-if="!duplicateGroups.length" class="dup-empty">
|
||||
No near-duplicates found. Nothing recorded resembles anything else closely
|
||||
enough to be worth merging.
|
||||
</p>
|
||||
<template v-else>
|
||||
<p class="dup-head">
|
||||
{{ duplicateGroups.length }} possible duplicate{{ duplicateGroups.length > 1 ? " sets" : " set" }}
|
||||
— review each before merging; a set is a suggestion, not a verdict.
|
||||
</p>
|
||||
<div v-for="(g, i) in duplicateGroups" :key="i" class="dup-group">
|
||||
<div class="dup-members">
|
||||
<span v-for="s in g.snippets" :key="s.id" class="dup-member">
|
||||
{{ splitTitle(s.title).name }}
|
||||
</span>
|
||||
</div>
|
||||
<span class="dup-score">{{ Math.round(g.top_score * 100) }}% alike</span>
|
||||
<button class="btn-ghost dup-action" @click="reviewGroup(g)">Review & merge</button>
|
||||
</div>
|
||||
</template>
|
||||
</div>
|
||||
|
||||
<!-- Reverse lookup: what's already kept in this repo / file / symbol. -->
|
||||
@@ -209,20 +394,27 @@ function languageOf(tags: string[]): string {
|
||||
<div v-else-if="snippets.length === 0" class="empty-state-rich">
|
||||
<div class="empty-icon">❭_</div>
|
||||
<p class="empty-title">
|
||||
{{ locationActive
|
||||
? "Nothing kept at that location yet"
|
||||
: search.trim()
|
||||
? "No snippets match your search"
|
||||
: "No snippets kept yet" }}
|
||||
{{ needsAttentionOnly
|
||||
? "Everything checks out"
|
||||
: locationActive
|
||||
? "Nothing kept at that location yet"
|
||||
: search.trim()
|
||||
? "No snippets match your search"
|
||||
: "No snippets kept yet" }}
|
||||
</p>
|
||||
<p class="empty-sub">
|
||||
{{ locationActive
|
||||
? "No recorded snippet lives there — so whatever you're about to write is new. Widen the path, or clear the filter."
|
||||
: search.trim()
|
||||
? "Try a different term, or clear the search."
|
||||
: "Record a reusable function or component and it will be offered back to you later." }}
|
||||
{{ needsAttentionOnly
|
||||
? "No snippet has drifted from its recorded location or code — as far as anything has been checked. Snippets nobody has verified yet don't appear here."
|
||||
: locationActive
|
||||
? "No recorded snippet lives there — so whatever you're about to write is new. Widen the path, or clear the filter."
|
||||
: search.trim()
|
||||
? "Try a different term, or clear the search."
|
||||
: "Record a reusable function or component and it will be offered back to you later." }}
|
||||
</p>
|
||||
<button v-if="locationActive" class="empty-action" @click="clearLocation">
|
||||
<button v-if="needsAttentionOnly" class="empty-action" @click="toggleAttention">
|
||||
Show all snippets
|
||||
</button>
|
||||
<button v-else-if="locationActive" class="empty-action" @click="clearLocation">
|
||||
Clear location filter
|
||||
</button>
|
||||
<button
|
||||
@@ -261,6 +453,17 @@ function languageOf(tags: string[]): string {
|
||||
</p>
|
||||
<div class="card-footer">
|
||||
<span class="meta-date">Updated {{ new Date(s.updated_at).toLocaleDateString() }}</span>
|
||||
<span v-if="driftBadge(s)" class="drift-tag" :title="driftTitle(s)">
|
||||
{{ driftBadge(s) }}
|
||||
</span>
|
||||
<span
|
||||
v-if="usageBadge(s)"
|
||||
class="usage-tag"
|
||||
:class="{ 'usage-dead': isDeadWeight(s) }"
|
||||
:title="usageTitle(s)"
|
||||
>
|
||||
{{ usageBadge(s) }}
|
||||
</span>
|
||||
<span v-if="s.shared" class="shared-tag" :title="`Shared by ${s.owner ?? 'another user'} — a suggestion, not your own record`">
|
||||
by {{ s.owner ?? "another user" }}
|
||||
</span>
|
||||
@@ -279,7 +482,7 @@ function languageOf(tags: string[]): string {
|
||||
|
||||
<!-- Merge modal -->
|
||||
<teleport to="body">
|
||||
<div v-if="showMergeModal" class="modal-overlay" @click.self="showMergeModal = false">
|
||||
<div v-if="showMergeModal" class="modal-overlay" @click.self="closeMerge">
|
||||
<div class="modal-card" role="dialog" aria-modal="true" aria-label="Merge snippets">
|
||||
<h3 class="modal-title">Merge snippets</h3>
|
||||
<p class="modal-desc">
|
||||
@@ -299,7 +502,7 @@ function languageOf(tags: string[]): string {
|
||||
</label>
|
||||
</div>
|
||||
<div class="modal-actions">
|
||||
<button class="modal-btn" @click="showMergeModal = false">Cancel</button>
|
||||
<button class="modal-btn" @click="closeMerge">Cancel</button>
|
||||
<button
|
||||
class="modal-btn modal-btn-primary"
|
||||
:disabled="merging || canonicalId == null"
|
||||
@@ -576,6 +779,91 @@ function languageOf(tags: string[]): string {
|
||||
color: var(--color-text-muted);
|
||||
}
|
||||
|
||||
/* Near-duplicate report */
|
||||
.dup-panel {
|
||||
margin-bottom: 1.25rem;
|
||||
padding: 0.85rem 1rem;
|
||||
border: 1px solid var(--color-border);
|
||||
border-radius: 8px;
|
||||
background: var(--color-surface-alt, var(--color-surface));
|
||||
}
|
||||
|
||||
.dup-empty,
|
||||
.dup-head {
|
||||
margin: 0 0 0.5rem;
|
||||
font-size: 0.85rem;
|
||||
color: var(--color-text-muted);
|
||||
}
|
||||
|
||||
.dup-empty {
|
||||
margin-bottom: 0;
|
||||
}
|
||||
|
||||
.dup-group {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 0.75rem;
|
||||
flex-wrap: wrap;
|
||||
padding: 0.5rem 0;
|
||||
border-top: 1px solid var(--color-border);
|
||||
}
|
||||
|
||||
.dup-members {
|
||||
display: flex;
|
||||
gap: 0.4rem;
|
||||
flex-wrap: wrap;
|
||||
flex: 1 1 20rem;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.dup-member {
|
||||
font-size: 0.8rem;
|
||||
padding: 0.1rem 0.45rem;
|
||||
border-radius: 4px;
|
||||
background: color-mix(in srgb, var(--color-text-muted) 12%, transparent);
|
||||
/* Long snippet names must not push the row into a horizontal scroll. */
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
.dup-score {
|
||||
font-size: 0.75rem;
|
||||
color: var(--color-text-muted);
|
||||
font-variant-numeric: tabular-nums;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.dup-action {
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
/* Drift is a stronger signal than dead weight: the record may be actively
|
||||
misleading, not merely unused. Danger tone, and it sits first in the footer. */
|
||||
.drift-tag {
|
||||
font-size: 0.7rem;
|
||||
padding: 0.1rem 0.4rem;
|
||||
border-radius: 4px;
|
||||
white-space: nowrap;
|
||||
background: color-mix(in srgb, var(--color-danger, #b91c1c) 15%, transparent);
|
||||
color: var(--color-danger, #b91c1c);
|
||||
}
|
||||
|
||||
.usage-tag {
|
||||
font-size: 0.7rem;
|
||||
padding: 0.1rem 0.4rem;
|
||||
border-radius: 4px;
|
||||
white-space: nowrap;
|
||||
font-variant-numeric: tabular-nums;
|
||||
background: color-mix(in srgb, var(--color-text-muted) 15%, transparent);
|
||||
color: var(--color-text-muted);
|
||||
}
|
||||
|
||||
/* Dead weight is a nudge, not an error — it warns in the warning colour rather
|
||||
than the danger one, because the record isn't broken, just unearned. */
|
||||
.usage-tag.usage-dead {
|
||||
background: color-mix(in srgb, var(--color-warning, #b45309) 18%, transparent);
|
||||
color: var(--color-warning, #b45309);
|
||||
}
|
||||
|
||||
/* Header + select-mode */
|
||||
.header-actions {
|
||||
display: flex;
|
||||
|
||||
@@ -11,9 +11,12 @@
|
||||
# static floor here. If the instance is unconfigured/unreachable, or anything
|
||||
# fails, the hook stays SILENT and exits 0 — it must never block a prompt.
|
||||
#
|
||||
# Config (same as scribe_session_context.sh), exported to the hook by Claude Code:
|
||||
# CLAUDE_PLUGIN_OPTION_api_endpoint base URL, no trailing slash
|
||||
# CLAUDE_PLUGIN_OPTION_api_token fmcp_ API key (sensitive)
|
||||
# Config (same as scribe_session_context.sh), exported to the hook by Claude Code
|
||||
# with the userConfig key UPPERCASED (see #2198 — reading the lowercase spelling
|
||||
# silently disables this hook, and silence is indistinguishable from "nothing
|
||||
# cleared the threshold"):
|
||||
# CLAUDE_PLUGIN_OPTION_API_ENDPOINT base URL, no trailing slash
|
||||
# CLAUDE_PLUGIN_OPTION_API_TOKEN fmcp_ API key (sensitive)
|
||||
# SCRIBE_URL / SCRIBE_TOKEN override for the settings.json dogfooding path.
|
||||
#
|
||||
# Session dedup: each surfaced note id is remembered in a per-session file so a
|
||||
@@ -32,8 +35,8 @@ event_cwd=$(printf '%s' "$event" | jq -r '.cwd // empty' 2>/dev/null) || event_c
|
||||
# Nothing to retrieve against.
|
||||
[ -n "$prompt" ] || exit 0
|
||||
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_api_endpoint:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_api_token:-}}
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_API_ENDPOINT:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_API_TOKEN:-}}
|
||||
# Guard against an unexpanded ${...} placeholder arriving as a literal.
|
||||
case "$url" in *'${'*) url="" ;; esac
|
||||
case "$token" in *'${'*) token="" ;; esac
|
||||
@@ -42,14 +45,18 @@ case "$token" in *'${'*) token="" ;; esac
|
||||
|
||||
# Cap the query length — a giant prompt makes a giant URL for no extra signal.
|
||||
q=$(printf '%s' "$prompt" | cut -c1-2000)
|
||||
q_enc=$(printf '%s' "$q" | jq -rR '@uri' 2>/dev/null) || exit 0
|
||||
# `-sRr`, not `-rR`: jq -R reads LINE BY LINE, so a multi-line prompt encoded as
|
||||
# several lines joined by raw newlines and the request died. Single-line prompts
|
||||
# worked, which is why this looked healthy — the long, substantial prompts most
|
||||
# worth retrieving against were exactly the ones silently dropped. -s slurps.
|
||||
q_enc=$(printf '%s' "$q" | jq -sRr '@uri' 2>/dev/null) || exit 0
|
||||
|
||||
# Resolve the working repo's remote so the server can scope to the bound project.
|
||||
repo_dir=${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}}
|
||||
repo=$(git -C "$repo_dir" remote get-url origin 2>/dev/null || true)
|
||||
repo_q=""
|
||||
if [ -n "$repo" ]; then
|
||||
enc=$(printf '%s' "$repo" | jq -rR '@uri' 2>/dev/null) || enc=""
|
||||
enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc=""
|
||||
[ -n "$enc" ] && repo_q="&repo=${enc}"
|
||||
fi
|
||||
|
||||
|
||||
@@ -13,9 +13,11 @@
|
||||
# Any failure — unconfigured, unreachable, malformed — exits 0 in silence. A
|
||||
# recall aid must not be able to stop the operator's work.
|
||||
#
|
||||
# Config (same as the other hooks), exported to the hook by Claude Code:
|
||||
# CLAUDE_PLUGIN_OPTION_api_endpoint base URL, no trailing slash
|
||||
# CLAUDE_PLUGIN_OPTION_api_token fmcp_ API key (sensitive)
|
||||
# Config (same as the other hooks), exported to the hook by Claude Code with the
|
||||
# userConfig key UPPERCASED (see #2198 — the lowercase spelling reads as empty
|
||||
# and this hook then exits 0 in silence, looking exactly like "no prior art"):
|
||||
# CLAUDE_PLUGIN_OPTION_API_ENDPOINT base URL, no trailing slash
|
||||
# CLAUDE_PLUGIN_OPTION_API_TOKEN fmcp_ API key (sensitive)
|
||||
# SCRIBE_URL / SCRIBE_TOKEN override for the settings.json dogfooding path.
|
||||
set -uo pipefail
|
||||
|
||||
@@ -46,8 +48,8 @@ case "$file_path" in
|
||||
exit 0 ;;
|
||||
esac
|
||||
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_api_endpoint:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_api_token:-}}
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_API_ENDPOINT:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_API_TOKEN:-}}
|
||||
# Guard against an unexpanded ${...} placeholder arriving as a literal.
|
||||
case "$url" in *'${'*) url="" ;; esac
|
||||
case "$token" in *'${'*) token="" ;; esac
|
||||
@@ -70,16 +72,24 @@ fi
|
||||
# token limit well before this, so a bigger slice buys no extra signal — and the
|
||||
# payload has to stay a GET (a read-scoped API key cannot POST, and every other
|
||||
# plugin hook works with a read key).
|
||||
q=$(printf '%s' "$code" | cut -c1-1200)
|
||||
# `head -c`, not `cut -c1-1200`: cut is line-oriented and caps each line
|
||||
# separately, so a 400-line edit sailed past the "1200 char" budget entirely and
|
||||
# built a URL from the whole payload. head -c caps the total, which is the point.
|
||||
q=$(printf '%s' "$code" | head -c 1200)
|
||||
|
||||
path_enc=$(printf '%s' "$rel_path" | jq -rR '@uri' 2>/dev/null) || exit 0
|
||||
code_enc=$(printf '%s' "$q" | jq -rR '@uri' 2>/dev/null) || code_enc=""
|
||||
# `-sRr`, not `-rR`: jq -R reads input LINE BY LINE, so a multi-line payload came
|
||||
# back as several separately-encoded lines joined by raw newlines — an invalid
|
||||
# URL that made curl fail, and this hook then exited 0 in silence. -s slurps the
|
||||
# whole input into one string first. Newlines are exactly what code contains, so
|
||||
# this hook could never have worked without it (issue #2198 / #2082).
|
||||
path_enc=$(printf '%s' "$rel_path" | jq -sRr '@uri' 2>/dev/null) || exit 0
|
||||
code_enc=$(printf '%s' "$q" | jq -sRr '@uri' 2>/dev/null) || code_enc=""
|
||||
|
||||
# Resolve the working repo's remote so the server can scope to the bound project.
|
||||
repo=$(git -C "$lookup_dir" remote get-url origin 2>/dev/null || true)
|
||||
repo_q=""
|
||||
if [ -n "$repo" ]; then
|
||||
enc=$(printf '%s' "$repo" | jq -rR '@uri' 2>/dev/null) || enc=""
|
||||
enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc=""
|
||||
[ -n "$enc" ] && repo_q="&repo=${enc}"
|
||||
fi
|
||||
|
||||
|
||||
@@ -4,16 +4,18 @@
|
||||
# Tier 1 (STATIC, always fires, no auth, no network): injects a bundled
|
||||
# behavioral mandate (scribe_static_context.md) so a fresh session knows to
|
||||
# reach for Scribe — record work, recall before acting — even when the instance
|
||||
# is unreachable OR the API token never reached this hook. The latter is a known
|
||||
# Claude Code gap: sensitive userConfig values aren't always exported to the
|
||||
# hook subprocess, so the dynamic tier can silently get nothing. The static tier
|
||||
# is the load-bearing floor that does not depend on the key or the network.
|
||||
# is unreachable or unconfigured. The static tier is the load-bearing floor that
|
||||
# does not depend on the key or the network.
|
||||
#
|
||||
# Tier 2 (DYNAMIC, best-effort enrichment): curls the operator's Scribe instance
|
||||
# for always-on rules + active-project context and appends it. Config comes from
|
||||
# the plugin's userConfig, exported to hooks as:
|
||||
# CLAUDE_PLUGIN_OPTION_api_endpoint base URL, no trailing slash
|
||||
# CLAUDE_PLUGIN_OPTION_api_token fmcp_ API key (sensitive)
|
||||
# CLAUDE_PLUGIN_OPTION_API_ENDPOINT base URL, no trailing slash
|
||||
# CLAUDE_PLUGIN_OPTION_API_TOKEN fmcp_ API key (sensitive)
|
||||
# NOTE THE CASE: Claude Code uppercases the userConfig key when exporting it, so
|
||||
# the `api_token` option arrives as CLAUDE_PLUGIN_OPTION_API_TOKEN. Reading the
|
||||
# lowercase spelling silently yields nothing — that was issue #2198, and it
|
||||
# disabled the dynamic tier, auto-inject, and the write-path trigger at once.
|
||||
# The active project is resolved server-side from the working repo's git remote
|
||||
# (see services/repo_bindings); bind each repo once with the bind_repo MCP tool.
|
||||
#
|
||||
@@ -25,16 +27,16 @@
|
||||
# can't make the model flush, and can't know the in-flight task ids; the durable
|
||||
# path is record-as-you-go + this post-compaction reload.)
|
||||
#
|
||||
# IMPORTANT: do NOT pass config via `${user_config.*}` substitution in
|
||||
# hooks.json — sensitive values are kept in the keychain and never spliced into
|
||||
# a hook command line, so the placeholder arrives unexpanded. The harness env
|
||||
# vars above are the supported channel; SCRIBE_URL / SCRIBE_TOKEN override for
|
||||
# the settings.json dogfooding path.
|
||||
# IMPORTANT: do NOT pass config via `${user_config.*}` substitution in a
|
||||
# shell-form hooks.json command — Claude Code rejects that outright (splicing a
|
||||
# configured value into a shell command line would let the shell run whatever it
|
||||
# contains). The env vars above are the supported channel; SCRIBE_URL /
|
||||
# SCRIBE_TOKEN override for the settings.json dogfooding path.
|
||||
#
|
||||
# FAIL-OPEN, BUT NOT SILENT: the dynamic tier never blocks a session. A *failed*
|
||||
# dynamic fetch is surfaced as a short status line (not swallowed). A fully
|
||||
# unconfigured install (no url AND no token) is the intended static-only mode
|
||||
# and stays quiet.
|
||||
# FAIL-OPEN, BUT NOT SILENT: the dynamic tier never blocks a session, and every
|
||||
# way it can come up empty produces a short status line — failed fetch, missing
|
||||
# token, and missing-everything alike. Nothing about the credential path is
|
||||
# allowed to fail quietly; see the #2198 comment at the status block below.
|
||||
set -uo pipefail
|
||||
|
||||
command -v jq >/dev/null 2>&1 || exit 0 # needed to emit the JSON envelope safely
|
||||
@@ -55,8 +57,8 @@ prepend() { if [ -n "$out" ]; then out="$1"$'\n\n---\n\n'"${out}"; else out="$1"
|
||||
[ -f "$here/scribe_static_context.md" ] && out=$(cat "$here/scribe_static_context.md")
|
||||
|
||||
# --- Tier 2: dynamic rules + active-project context (best-effort) ---
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_api_endpoint:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_api_token:-}}
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_API_ENDPOINT:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_API_TOKEN:-}}
|
||||
|
||||
# Guard against an unexpanded `${...}` placeholder reaching us as a literal — it
|
||||
# would otherwise be sent as a garbage Bearer token and 401. Treat as unset.
|
||||
@@ -71,7 +73,7 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then
|
||||
repo=$(git -C "$repo_dir" remote get-url origin 2>/dev/null || true)
|
||||
q=""
|
||||
if [ -n "$repo" ]; then
|
||||
enc=$(printf '%s' "$repo" | jq -rR '@uri' 2>/dev/null) || enc=""
|
||||
enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc=""
|
||||
[ -n "$enc" ] && q="?repo=${enc}"
|
||||
fi
|
||||
body=$(curl -fsS --max-time 8 \
|
||||
@@ -80,9 +82,16 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then
|
||||
[ -n "$body" ] && dyn=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null)
|
||||
[ -z "$dyn" ] && status="> ⚠️ Scribe: live rules/project context could not be loaded this session (instance unreachable or request failed). The standing guidance above still applies — pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\` as needed."
|
||||
elif [ -n "$url" ] && [ -z "$token" ]; then
|
||||
# Endpoint configured but token absent: the signature of the known Claude Code
|
||||
# userConfig export gap (sensitive values not always reaching the hook).
|
||||
status="> ⚠️ Scribe: live context disabled this session — the API token did not reach this hook (a known Claude Code plugin-config gap). Tools still work; pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\`."
|
||||
status="> ⚠️ Scribe: live context disabled this session — the API key is not configured (Scribe base URL is). Set it with \`/plugin\` → Scribe → configure, or export SCRIBE_TOKEN. Tools still work; pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\`."
|
||||
elif [ -z "$url" ] && [ -z "$token" ]; then
|
||||
# NEITHER value arrived. Previously this case stayed silent as "an unconfigured
|
||||
# install", which made issue #2198 invisible for weeks: a *casing* bug here
|
||||
# (reading CLAUDE_PLUGIN_OPTION_api_token when Claude Code exports the key
|
||||
# UPPERCASED) looks identical to never having configured the plugin, and
|
||||
# silently disabled auto-inject and the write-path trigger too. It is not a
|
||||
# benign state — the plugin prompts for both values at enable time, so if
|
||||
# neither reached the hook, something is wrong. Say so.
|
||||
status="> ⚠️ Scribe: live context disabled this session — neither the Scribe base URL nor the API key reached this hook. Configure the plugin (\`/plugin\` → Scribe), or export SCRIBE_URL + SCRIBE_TOKEN. Note this also disables prompt auto-inject and the write-path prior-art trigger. Tools still work; pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\`."
|
||||
fi
|
||||
|
||||
[ -n "$dyn" ] && append "$dyn"
|
||||
|
||||
@@ -18,14 +18,16 @@
|
||||
# FAIL-OPEN & SILENT: never blocks a session; emits NOTHING on stdout (so it's
|
||||
# safe as a second SessionStart hook). On any fetch failure it exits without
|
||||
# touching existing stubs — a transient outage must not wipe the user's skills.
|
||||
# Config mirrors the context hook (CLAUDE_PLUGIN_OPTION_* / SCRIBE_* override).
|
||||
# Config mirrors the context hook: CLAUDE_PLUGIN_OPTION_API_ENDPOINT /
|
||||
# CLAUDE_PLUGIN_OPTION_API_TOKEN (userConfig key UPPERCASED by Claude Code — see
|
||||
# #2198), with SCRIBE_URL / SCRIBE_TOKEN as the override.
|
||||
set -uo pipefail
|
||||
|
||||
command -v jq >/dev/null 2>&1 || exit 0
|
||||
command -v curl >/dev/null 2>&1 || exit 0
|
||||
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_api_endpoint:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_api_token:-}}
|
||||
url=${SCRIBE_URL:-${CLAUDE_PLUGIN_OPTION_API_ENDPOINT:-}}
|
||||
token=${SCRIBE_TOKEN:-${CLAUDE_PLUGIN_OPTION_API_TOKEN:-}}
|
||||
# Guard against an unexpanded `${...}` placeholder arriving as a literal.
|
||||
case "$url" in *'${'*) url="" ;; esac
|
||||
case "$token" in *'${'*) token="" ;; esac
|
||||
|
||||
@@ -256,6 +256,9 @@ _READ_ONLY_TOOLS = frozenset({
|
||||
"list_rules", "list_tags", "list_tasks", "list_topics", "list_trash",
|
||||
"list_always_on_rules", "search",
|
||||
"get_system", "list_systems", "list_system_records",
|
||||
# Reports on the snippet corpus. Reads only — the merge it suggests is a
|
||||
# separate, explicitly-called write.
|
||||
"find_duplicate_snippets",
|
||||
})
|
||||
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ from scribe.services import dedup as dedup_svc
|
||||
from scribe.services import notes as notes_svc
|
||||
from scribe.services import systems as systems_svc
|
||||
from scribe.services import trash as trash_svc
|
||||
from scribe.services.note_usage import record_pulled
|
||||
|
||||
|
||||
async def list_notes(
|
||||
@@ -68,6 +69,11 @@ async def get_note(note_id: int) -> dict:
|
||||
raise ValueError(f"note {note_id} not found")
|
||||
out = note.to_dict()
|
||||
out.update(await access_svc.describe_provenance(uid, note))
|
||||
# Records the pull for ANY note kind, not just snippets: the auto-inject
|
||||
# menu surfaces notes, tasks and processes too, so restricting this to
|
||||
# snippets would leave those permanently at zero pulls and make them look
|
||||
# like dead weight next to snippets that merely had a counter (#2085).
|
||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_note")
|
||||
return out
|
||||
|
||||
|
||||
|
||||
@@ -14,12 +14,13 @@ from scribe.mcp._context import current_user_id
|
||||
from scribe.services import access as access_svc
|
||||
from scribe.services import dedup as dedup_svc
|
||||
from scribe.services import snippets as snippets_svc
|
||||
from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
|
||||
from scribe.services import systems as systems_svc
|
||||
|
||||
|
||||
async def list_snippets(
|
||||
q: str = "", tag: str = "", limit: int = 50, project_id: int = 0,
|
||||
repo: str = "", path: str = "", symbol: str = "",
|
||||
repo: str = "", path: str = "", symbol: str = "", verification: str = "",
|
||||
) -> dict:
|
||||
"""List recorded snippets (reusable functions/components).
|
||||
|
||||
@@ -43,13 +44,27 @@ async def list_snippets(
|
||||
OR anything beneath it, so "frontend/src" finds
|
||||
"frontend/src/lib/x.ts" as well as itself.
|
||||
symbol: Narrow to snippets recorded under this symbol name, exactly.
|
||||
verification: Narrow on the drift check (see verify_snippet).
|
||||
"attention" is the one to reach for — everything whose recorded
|
||||
location or code no longer checks out, plus everything whose verdict
|
||||
expired because the snippet was edited after it was checked. Also
|
||||
accepts "ok", "unverified", "drifted", or a specific failure:
|
||||
"missing", "moved", "changed".
|
||||
|
||||
`repo`/`path`/`symbol` must all match the SAME recorded location, so a
|
||||
snippet that lives in repo A and, separately, at path B in another repo is
|
||||
not returned for repo=A + path=B.
|
||||
|
||||
Returns {"snippets": [{id, title, tags, preview}], "total": int}. The title
|
||||
reads "name — when to reach for it"; open one in full with get_snippet(id).
|
||||
Returns {"snippets": [{id, title, tags, preview, usage}], "total": int}. The
|
||||
title reads "name — when to reach for it"; open one in full with
|
||||
get_snippet(id).
|
||||
|
||||
`usage` is {surfaced_count, pull_count, last_surfaced_at, last_pulled_at}:
|
||||
how often the entry has been put in front of an agent versus actually
|
||||
opened. Treat a high surfaced_count with a zero pull_count as a prompt to
|
||||
fix the record — usually its "when to reach for it" doesn't say when — or to
|
||||
delete it. Such an entry is not harmless: it takes a slot in every future
|
||||
auto-inject menu and crowds out something useful.
|
||||
|
||||
An entry marked `shared: true` with an `owner` belongs to someone else — one
|
||||
person's suggestion, not settled practice here. Weigh it on its merits and
|
||||
@@ -61,12 +76,13 @@ async def list_snippets(
|
||||
items, total = await snippets_svc.list_snippets(
|
||||
uid, q=q or None, tag=tag, limit=max(1, min(limit, 100)),
|
||||
project_id=project_id or None,
|
||||
repo=repo, path=path, symbol=symbol,
|
||||
repo=repo, path=path, symbol=symbol, verification=verification,
|
||||
)
|
||||
return {
|
||||
"snippets": await access_svc.label_shared_items(uid, items),
|
||||
"total": total,
|
||||
}
|
||||
labeled = await access_svc.label_shared_items(uid, items)
|
||||
usage = await usage_for_notes([int(it["id"]) for it in labeled])
|
||||
for it in labeled:
|
||||
it["usage"] = usage.get(int(it["id"]), empty_usage())
|
||||
return {"snippets": labeled, "total": total}
|
||||
|
||||
|
||||
async def create_snippet(
|
||||
@@ -166,9 +182,144 @@ async def get_snippet(snippet_id: int) -> dict:
|
||||
raise ValueError(f"snippet {snippet_id} not found")
|
||||
data = snippets_svc.snippet_to_dict(note)
|
||||
data.update(await access_svc.describe_provenance(uid, note))
|
||||
# A "pull" is an explicit open, so it's recorded HERE rather than in
|
||||
# snippets_svc.get_snippet — the service is also reached by update/merge
|
||||
# paths, and counting those would inflate exactly the number that is
|
||||
# supposed to mean "someone chose to look at this" (#2085).
|
||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_snippet")
|
||||
return data
|
||||
|
||||
|
||||
async def unmerge_snippet(survivor_id: int, source_id: int) -> dict:
|
||||
"""Reverse ONE source out of a merged snippet — the inverse of merge_snippets.
|
||||
|
||||
Restores the source record and strips exactly what it contributed from the
|
||||
survivor: the locations and tags it ADDED at merge time, never the ones the
|
||||
survivor already had. Reach for it when a merge turns out to have unified two
|
||||
things that only looked alike.
|
||||
|
||||
Also the fix for a half-undone merge. Restoring a merged-in source from the
|
||||
trash by hand brings the record back but leaves the survivor still claiming
|
||||
its call sites, so both records claim the same places and the reverse lookup
|
||||
reads the duplicate claims as real. Running this on an already-restored
|
||||
source repairs that: it skips the restore and does the subtraction.
|
||||
|
||||
Args:
|
||||
survivor_id: The snippet that absorbed the other.
|
||||
source_id: The snippet to pull back out of it.
|
||||
|
||||
Returns {"survivor": {...}, "restored": {...}}.
|
||||
|
||||
Refuses, with the reason, when: the survivor has no record of absorbing that
|
||||
id; the source was purged from the trash; or the merge predates per-source
|
||||
provenance, in which case what it contributed isn't known and subtracting a
|
||||
guess could strip call sites the survivor genuinely owns — restore it from
|
||||
the trash and adjust both records by hand instead.
|
||||
"""
|
||||
uid = current_user_id()
|
||||
try:
|
||||
result = await snippets_svc.unmerge_snippet(uid, survivor_id, source_id)
|
||||
except snippets_svc.UnmergeError as exc:
|
||||
raise ValueError(str(exc)) from exc
|
||||
if result is None:
|
||||
raise ValueError(f"snippet {survivor_id} not found")
|
||||
survivor, restored = result
|
||||
return {
|
||||
"survivor": snippets_svc.snippet_to_dict(survivor),
|
||||
"restored": snippets_svc.snippet_to_dict(restored) if restored else None,
|
||||
}
|
||||
|
||||
|
||||
async def find_duplicate_snippets(threshold: float = 0.0) -> dict:
|
||||
"""Find snippets already recorded that look like duplicates of each other.
|
||||
|
||||
The create gate PREVENTS a new duplicate and merge_snippets CURES one you
|
||||
point it at — this is the missing third piece: it FINDS the ones already in
|
||||
the record, so nobody has to notice them by hand.
|
||||
|
||||
Results are grouped into candidate merge SETS, not just pairs. Grouping is
|
||||
transitive: if A resembles B and B resembles C, all three land in one set
|
||||
even when A and C don't directly clear the bar. That mirrors what merge does
|
||||
(it folds every source into one survivor), but it means a chain of mild
|
||||
resemblances can rope in a member that isn't really alike — so read a set as
|
||||
a proposal and check the members before acting.
|
||||
|
||||
Reports only YOUR snippets. merge_snippets requires one owner across the
|
||||
whole set, so surfacing someone else's would propose a merge that can't be
|
||||
performed.
|
||||
|
||||
Acting on a group: pick the best record as the canonical target, then
|
||||
`merge_snippets(target_id, [other ids])`. Merge unions the fields and folds
|
||||
every source's location in, so the survivor is findable at all their call
|
||||
sites; the sources are trashed, recoverably. Prefer as target the one with
|
||||
the clearest "when to reach for it" — merge keeps the target's title.
|
||||
|
||||
Args:
|
||||
threshold: Similarity floor, 0-1. 0 (default) uses the configured
|
||||
setting. Raise it if the report is noisy, lower it to catch more.
|
||||
|
||||
Returns {"groups": [{"note_ids", "snippets", "top_score"}], "pairs",
|
||||
"threshold"}. An empty `groups` means nothing resembles anything else that
|
||||
closely — the common and desirable case.
|
||||
"""
|
||||
uid = current_user_id()
|
||||
return await dedup_svc.find_duplicate_snippets(
|
||||
uid, threshold=threshold if threshold > 0 else None
|
||||
)
|
||||
|
||||
|
||||
async def verify_snippet(
|
||||
snippet_id: int, status: str, detail: str = "", path: str = "",
|
||||
) -> dict:
|
||||
"""Record whether a snippet's recorded location and code still match source.
|
||||
|
||||
YOU do the checking — Scribe has no copy of the repo and deliberately never
|
||||
gets one. This tool only remembers your verdict so it becomes queryable and
|
||||
so the operator can see what has rotted.
|
||||
|
||||
The procedure, once per snippet you're checking:
|
||||
1. `get_snippet(id)` — read its `snippet.locations` and `snippet.code`.
|
||||
2. Does the recorded path still exist in the working tree? If not →
|
||||
status="missing".
|
||||
3. Does the recorded symbol still appear in that file? If not →
|
||||
status="moved" (the file is there, the thing isn't).
|
||||
4. Does the source still match the recorded code, allowing for formatting?
|
||||
Judge whether it still does the same thing — an added parameter or a
|
||||
changed branch is "changed"; a reindent is not. If it diverged →
|
||||
status="changed".
|
||||
5. All three hold → status="ok".
|
||||
|
||||
Put what you actually found in `detail` ("renamed to parse_location_str",
|
||||
"moved to services/knowledge.py"). It's what makes the record fixable later
|
||||
by someone who wasn't here, so write it for them, not as a status echo.
|
||||
|
||||
A verdict expires automatically if the snippet is edited afterwards: it is
|
||||
stamped with a hash of the code it was checked against, so it can never go
|
||||
on vouching for code nobody checked. Re-verify after fixing a record.
|
||||
|
||||
Args:
|
||||
snippet_id: The snippet you checked.
|
||||
status: "ok" | "missing" | "moved" | "changed".
|
||||
detail: What you found — free text, shown to the operator.
|
||||
path: The path you actually checked, if it differs from the recorded
|
||||
one (e.g. you found the symbol at its new home). Defaults to the
|
||||
recorded path.
|
||||
|
||||
Requires write access: a verdict changes how the record is presented, so
|
||||
being able to read a snippet someone shared with you doesn't let you mark
|
||||
it broken.
|
||||
"""
|
||||
uid = current_user_id()
|
||||
note = await snippets_svc.record_verification(
|
||||
uid, snippet_id, status=status, detail=detail, path=path,
|
||||
)
|
||||
if note is None:
|
||||
raise ValueError(
|
||||
f"snippet {snippet_id} not found, or you don't have write access to it"
|
||||
)
|
||||
return snippets_svc.snippet_to_dict(note)
|
||||
|
||||
|
||||
async def update_snippet(
|
||||
snippet_id: int,
|
||||
name: str | None = None,
|
||||
@@ -265,6 +416,10 @@ async def merge_snippets(target_id: int, source_ids: list[int]) -> dict:
|
||||
fields and as a "Merged from: #ids" line in the body), so a variant that got
|
||||
folded in leaves a trace outside the trash. It accumulates across merges.
|
||||
|
||||
Reversible: each entry also records what that source contributed, so
|
||||
`unmerge_snippet(target_id, source_id)` can restore it and strip exactly
|
||||
those locations back off — never the ones the target already had.
|
||||
|
||||
Args:
|
||||
target_id: The snippet to keep (the canonical record).
|
||||
source_ids: Snippet ids to fold into the target and retire. Ids that
|
||||
@@ -292,6 +447,7 @@ async def merge_snippets(target_id: int, source_ids: list[int]) -> dict:
|
||||
def register(mcp) -> None:
|
||||
for fn in (
|
||||
list_snippets, create_snippet, get_snippet, update_snippet,
|
||||
delete_snippet, merge_snippets,
|
||||
delete_snippet, merge_snippets, verify_snippet, find_duplicate_snippets,
|
||||
unmerge_snippet,
|
||||
):
|
||||
mcp.tool(name=fn.__name__)(fn)
|
||||
|
||||
@@ -27,6 +27,7 @@ from scribe.models.password_reset import PasswordResetToken # noqa: E402, F401
|
||||
from scribe.models.invitation import InvitationToken # noqa: E402, F401
|
||||
from scribe.models.embedding import NoteEmbedding # noqa: E402, F401
|
||||
from scribe.models.retrieval_log import RetrievalLog # noqa: E402, F401
|
||||
from scribe.models.note_usage import NoteUsageEvent # noqa: E402, F401
|
||||
from scribe.models.project import Project # noqa: E402, F401
|
||||
from scribe.models.milestone import Milestone # noqa: E402, F401
|
||||
from scribe.models.task_log import TaskLog # noqa: E402, F401
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import DateTime, Index, Integer, Text
|
||||
from sqlalchemy.orm import Mapped, mapped_column
|
||||
|
||||
from scribe.models import Base
|
||||
|
||||
SURFACED = "surfaced"
|
||||
PULLED = "pulled"
|
||||
|
||||
|
||||
class NoteUsageEvent(Base):
|
||||
"""One row per time a note was SURFACED to the agent, or PULLED in full.
|
||||
|
||||
Answers the question RetrievalLog cannot: not "what did the ranker return
|
||||
and with what scores" but "did anyone ever actually open this?" A snippet
|
||||
nobody opens is not neutral — it competes for the injection budget on every
|
||||
future turn and dilutes the menu — so the surfaced:pulled ratio is what
|
||||
makes dead weight visible and prunable.
|
||||
|
||||
WHY A SEPARATE TABLE FROM RetrievalLog. RetrievalLog is one row per *call*,
|
||||
keyed on the score distribution it exists to capture; folding un-scored
|
||||
events into it would corrupt exactly the distribution threshold tuning reads
|
||||
(see the KNOWN GAP note this closes in plugin_context.build_write_path_hint).
|
||||
This table is one row per *note per event*, which is the grain the usage
|
||||
readout needs and the grain RetrievalLog's JSONB `result_ids` can't be
|
||||
indexed at. The two are complements: RetrievalLog tunes the threshold, this
|
||||
tunes the corpus.
|
||||
|
||||
WHY NOT IN-SESSION CORRELATION. The original framing was "correlate
|
||||
result_ids against a later get_note in the same session". There is no
|
||||
session identity server-side — the MCP endpoint is stateless and hooks send
|
||||
no session id — and introducing one would mean threading an opaque,
|
||||
client-supplied token through every read path for a signal that does not
|
||||
need it. Two independent counters answer the question without it: a note
|
||||
surfaced 40 times and never pulled is dead weight regardless of which
|
||||
sessions those events fell in.
|
||||
|
||||
Deliberately FK-free on user_id and note_id (mirrors RetrievalLog/AppLog):
|
||||
telemetry outlives the row it describes, and deleting a note should not
|
||||
erase the evidence that it was surfaced 40 times and never once opened.
|
||||
"""
|
||||
|
||||
__tablename__ = "note_usage_events"
|
||||
|
||||
id: Mapped[int] = mapped_column(primary_key=True)
|
||||
created_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True), default=lambda: datetime.now(timezone.utc)
|
||||
)
|
||||
user_id: Mapped[int | None] = mapped_column(Integer, nullable=True)
|
||||
note_id: Mapped[int] = mapped_column(Integer, nullable=False)
|
||||
# 'surfaced' | 'pulled'
|
||||
event: Mapped[str] = mapped_column(Text, nullable=False)
|
||||
# Which surface produced it: 'auto_inject' | 'write_path_place' |
|
||||
# 'write_path_semantic' | 'mcp_get_snippet' | 'mcp_get_note' | 'rest_note'.
|
||||
# Kept granular so the place arm and the semantic arm can be compared —
|
||||
# that comparison is the whole reason the place arm needed logging at all.
|
||||
source: Mapped[str] = mapped_column(Text, nullable=False)
|
||||
|
||||
__table_args__ = (
|
||||
# The readout is always "these note ids, split by event" — a covering
|
||||
# composite beats separate single-column indexes for it.
|
||||
Index("ix_note_usage_note_event", "note_id", "event"),
|
||||
Index("ix_note_usage_created_at", "created_at"),
|
||||
Index("ix_note_usage_user_id", "user_id"),
|
||||
)
|
||||
|
||||
def to_dict(self) -> dict:
|
||||
return {
|
||||
"id": self.id,
|
||||
"created_at": self.created_at.isoformat() if self.created_at else None,
|
||||
"user_id": self.user_id,
|
||||
"note_id": self.note_id,
|
||||
"event": self.event,
|
||||
"source": self.source,
|
||||
}
|
||||
@@ -19,6 +19,7 @@ from scribe.routes.utils import not_found, parse_pagination
|
||||
from scribe.services import dedup as dedup_svc
|
||||
from scribe.services import snippets as snippets_svc
|
||||
from scribe.services import systems as systems_svc
|
||||
from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
|
||||
from scribe.services.access import (
|
||||
can_write_note,
|
||||
describe_provenance,
|
||||
@@ -61,14 +62,22 @@ async def list_snippets_route():
|
||||
repo = request.args.get("repo", "")
|
||||
path = request.args.get("path", "")
|
||||
symbol = request.args.get("symbol", "")
|
||||
# Drift check (#2086). "attention" is what the UI's filter chip sends.
|
||||
verification = request.args.get("verification", "")
|
||||
limit, offset = parse_pagination()
|
||||
items, total = await snippets_svc.list_snippets(
|
||||
uid, q=q, tag=tag, limit=limit, offset=offset, project_id=project_id,
|
||||
repo=repo, path=path, symbol=symbol,
|
||||
repo=repo, path=path, symbol=symbol, verification=verification,
|
||||
)
|
||||
# Mark rows owned by someone else so the UI can show whose they are — an
|
||||
# unmarked row in your own list reads as one you recorded and vetted.
|
||||
items = await label_shared_items(uid, items)
|
||||
# One aggregate for the whole page — a per-row lookup here would be N+1 by
|
||||
# construction. Every row gets the key, zero-filled, so the UI renders
|
||||
# "never pulled" rather than having to treat a missing field as a state.
|
||||
usage = await usage_for_notes([int(it["id"]) for it in items])
|
||||
for it in items:
|
||||
it["usage"] = usage.get(int(it["id"]), empty_usage())
|
||||
return jsonify({"snippets": items, "total": total})
|
||||
|
||||
|
||||
@@ -148,6 +157,13 @@ async def get_snippet_route(snippet_id: int):
|
||||
for s in await systems_svc.list_record_systems(note.user_id, snippet_id)
|
||||
]
|
||||
data.update(await describe_provenance(uid, note))
|
||||
data["usage"] = (await usage_for_notes([snippet_id])).get(
|
||||
snippet_id, empty_usage()
|
||||
)
|
||||
# Opening the detail view IS a pull — the operator chose to look. Tagged
|
||||
# apart from the MCP sources so "the agent reused it" and "a human read it"
|
||||
# stay distinguishable; they mean different things for pruning (#2085).
|
||||
record_pulled(user_id=uid, note_id=snippet_id, source="rest_snippet")
|
||||
return jsonify(data)
|
||||
|
||||
|
||||
@@ -189,6 +205,92 @@ async def update_snippet_route(snippet_id: int):
|
||||
return jsonify(out)
|
||||
|
||||
|
||||
@snippets_bp.route("/<int:snippet_id>/unmerge", methods=["POST"])
|
||||
@login_required
|
||||
async def unmerge_snippet_route(snippet_id: int):
|
||||
"""Pull one source back out of a merged survivor. Body: {"source_id": int}.
|
||||
|
||||
Also the repair for a half-undone merge: restoring a source from the trash by
|
||||
hand leaves the survivor still claiming its call sites, and running this on an
|
||||
already-restored source strips them."""
|
||||
uid = get_current_user_id()
|
||||
data = await request.get_json() or {}
|
||||
source_id = data.get("source_id")
|
||||
if not isinstance(source_id, int):
|
||||
return jsonify({"error": "source_id (int) is required"}), 400
|
||||
try:
|
||||
result = await snippets_svc.unmerge_snippet(uid, snippet_id, source_id)
|
||||
except PermissionError as exc:
|
||||
return jsonify({"error": str(exc)}), 403
|
||||
except snippets_svc.UnmergeError as exc:
|
||||
# 409, not 400: the request is well-formed, the record's state is what
|
||||
# makes it impossible — and the message says which.
|
||||
return jsonify({"error": str(exc)}), 409
|
||||
if result is None:
|
||||
return not_found("Snippet")
|
||||
survivor, restored = result
|
||||
return jsonify({
|
||||
"survivor": snippets_svc.snippet_to_dict(survivor),
|
||||
"restored": snippets_svc.snippet_to_dict(restored) if restored else None,
|
||||
})
|
||||
|
||||
|
||||
@snippets_bp.route("/duplicates", methods=["GET"])
|
||||
@login_required
|
||||
async def duplicate_snippets_route():
|
||||
"""Near-duplicate snippets already recorded, grouped into merge candidates.
|
||||
|
||||
Registered ABOVE the `/<int:snippet_id>` routes on purpose — Quart matches
|
||||
an int converter before a static segment either way, but keeping the literal
|
||||
path first makes the precedence obvious to the next person reading this."""
|
||||
uid = get_current_user_id()
|
||||
try:
|
||||
threshold = float(request.args.get("threshold", 0) or 0)
|
||||
except (TypeError, ValueError):
|
||||
threshold = 0.0
|
||||
return jsonify(await dedup_svc.find_duplicate_snippets(
|
||||
uid, threshold=threshold if threshold > 0 else None
|
||||
))
|
||||
|
||||
|
||||
@snippets_bp.route("/<int:snippet_id>/verify", methods=["POST"])
|
||||
@login_required
|
||||
async def verify_snippet_route(snippet_id: int):
|
||||
"""Record a drift-check verdict. Body: {"status": ..., "detail", "path"}.
|
||||
|
||||
The CHECK itself runs wherever the code is — an agent with the working tree
|
||||
— because Scribe has no checkout and shouldn't have one. This endpoint just
|
||||
stores what was found. It's here for parity with the MCP tool and so the UI
|
||||
can clear a stale marker after the operator fixes a record by hand."""
|
||||
uid = get_current_user_id()
|
||||
if await _load_snippet(uid, snippet_id) is None:
|
||||
return not_found("Snippet")
|
||||
# Distinguished from not-found deliberately: the service returns None for
|
||||
# both, and telling a shared reader "no such snippet" about one they can
|
||||
# plainly see is a confusing lie.
|
||||
if not await can_write_note(uid, snippet_id):
|
||||
return jsonify({"error": "Permission denied"}), 403
|
||||
|
||||
data = await request.get_json() or {}
|
||||
status = (data.get("status") or "").strip()
|
||||
if not status:
|
||||
return jsonify({"error": "status is required"}), 400
|
||||
try:
|
||||
updated = await snippets_svc.record_verification(
|
||||
uid, snippet_id,
|
||||
status=status,
|
||||
detail=data.get("detail") or "",
|
||||
path=data.get("path") or "",
|
||||
)
|
||||
except ValueError as exc:
|
||||
# An unknown status — reject it rather than storing a value the filter
|
||||
# would then never match.
|
||||
return jsonify({"error": str(exc)}), 400
|
||||
if updated is None:
|
||||
return not_found("Snippet")
|
||||
return jsonify(snippets_svc.snippet_to_dict(updated))
|
||||
|
||||
|
||||
@snippets_bp.route("/<int:snippet_id>/merge", methods=["POST"])
|
||||
@login_required
|
||||
async def merge_snippet_route(snippet_id: int):
|
||||
|
||||
@@ -26,11 +26,17 @@ import logging
|
||||
from dataclasses import dataclass
|
||||
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.orm import aliased
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.embedding import NoteEmbedding
|
||||
from scribe.models.note import Note
|
||||
from scribe.models.rulebook import Rule
|
||||
from scribe.services import embeddings as embeddings_svc
|
||||
# Imported rather than redeclared: no service imports this module (the create
|
||||
# gate is called from the routes/tools layer), so there is no cycle to dodge,
|
||||
# and a second copy of the constant is a thing to drift.
|
||||
from scribe.services.snippets import SNIPPET_NOTE_TYPE
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -139,6 +145,174 @@ async def find_duplicate_note(
|
||||
return None
|
||||
|
||||
|
||||
# --- corpus-wide near-duplicate report (#2088) -------------------------------
|
||||
# The gate above PREVENTS a new duplicate; merge_snippets CURES one you point it
|
||||
# at. Neither FINDS the duplicates already sitting in the record — someone had to
|
||||
# notice them by hand, which is the exact failure the Drafter exists to remove.
|
||||
#
|
||||
# WHY OWN SNIPPETS ONLY. merge_snippets requires every record to share the
|
||||
# target's owner (cross-owner merge is out of scope), so a report that surfaced
|
||||
# someone else's snippet would propose a merge that cannot be performed. The
|
||||
# scope here is set by what the operator can actually act on, not by what they
|
||||
# can see.
|
||||
#
|
||||
# WHY A LOWER THRESHOLD THAN THE GATE. The gate BLOCKS a write at 0.90 and has to
|
||||
# be unforgiving of noise. This report only makes a suggestion the operator
|
||||
# reviews, so it can afford to be looser and catch the pairs the gate lets
|
||||
# through — which are precisely the ones that accumulated. It is a setting rather
|
||||
# than a constant (rule #25) because the right value depends on how uniform a
|
||||
# corpus is, and nobody can guess that from here.
|
||||
|
||||
DUPLICATE_THRESHOLD_KEY = "kb_duplicate_threshold"
|
||||
DUPLICATE_DEFAULT_THRESHOLD = 0.82
|
||||
# Hard cap on returned pairs. A pathologically uniform corpus is O(n²) pairs, and
|
||||
# a report nobody can read is not a report.
|
||||
_MAX_DUPLICATE_PAIRS = 200
|
||||
|
||||
|
||||
async def get_duplicate_threshold(user_id: int) -> float:
|
||||
"""The user's near-duplicate similarity floor, clamped to [0, 1]."""
|
||||
from scribe.services.settings import get_setting
|
||||
|
||||
try:
|
||||
value = float(await get_setting(
|
||||
user_id, DUPLICATE_THRESHOLD_KEY, str(DUPLICATE_DEFAULT_THRESHOLD)
|
||||
))
|
||||
except (TypeError, ValueError):
|
||||
value = DUPLICATE_DEFAULT_THRESHOLD
|
||||
return min(1.0, max(0.0, value))
|
||||
|
||||
|
||||
def group_pairs(pairs: list[tuple[int, int, float]]) -> list[list[int]]:
|
||||
"""Collapse similar-pairs into candidate merge SETS (connected components).
|
||||
|
||||
Pure and synchronous so the grouping rule is testable without a database.
|
||||
|
||||
Transitive on purpose: if A~B and B~C, all three land in one set even when
|
||||
A and C fall below the threshold. That matches what merge does — it folds
|
||||
every source into one survivor — and it avoids handing the operator three
|
||||
overlapping pairs to reconcile by hand, which is the chore being removed.
|
||||
The cost is that a chain of mild resemblances can rope in a pair that isn't
|
||||
really alike; the operator sees the members and picks, so a set is a
|
||||
proposal, never an action.
|
||||
"""
|
||||
parent: dict[int, int] = {}
|
||||
|
||||
def find(x: int) -> int:
|
||||
parent.setdefault(x, x)
|
||||
while parent[x] != x:
|
||||
parent[x] = parent[parent[x]]
|
||||
x = parent[x]
|
||||
return x
|
||||
|
||||
def union(a: int, b: int) -> None:
|
||||
ra, rb = find(a), find(b)
|
||||
if ra != rb:
|
||||
parent[rb] = ra
|
||||
|
||||
for left, right, _score in pairs:
|
||||
union(left, right)
|
||||
|
||||
groups: dict[int, list[int]] = {}
|
||||
for node in parent:
|
||||
groups.setdefault(find(node), []).append(node)
|
||||
# Biggest clusters first — the most tangled thing is the most worth fixing.
|
||||
# Ids ascending within a set so the output is stable across runs.
|
||||
return sorted((sorted(g) for g in groups.values() if len(g) > 1),
|
||||
key=lambda g: (-len(g), g[0]))
|
||||
|
||||
|
||||
async def find_duplicate_snippets(
|
||||
user_id: int, *, threshold: float | None = None, limit: int = _MAX_DUPLICATE_PAIRS
|
||||
) -> dict:
|
||||
"""Near-duplicate snippets already in the record, grouped into merge sets.
|
||||
|
||||
One indexed self-join over `note_embeddings` rather than an N² Python scan:
|
||||
pgvector's cosine distance is the same operator semantic search uses, so a
|
||||
similarity floor is a distance ceiling and the work stays in Postgres.
|
||||
|
||||
Returns {"groups": [{"note_ids": [...], "snippets": [...], "top_score": f}],
|
||||
"pairs": [...], "threshold": f}. Fail-open (an empty report) like the rest of
|
||||
this module — a suggestion feature must not be able to break the page it
|
||||
decorates.
|
||||
"""
|
||||
floor = await get_duplicate_threshold(user_id) if threshold is None else threshold
|
||||
floor = min(1.0, max(0.0, floor))
|
||||
max_distance = min(2.0, max(0.0, 1.0 - floor))
|
||||
|
||||
left = aliased(NoteEmbedding, name="left_emb")
|
||||
right = aliased(NoteEmbedding, name="right_emb")
|
||||
left_note = aliased(Note, name="left_note")
|
||||
right_note = aliased(Note, name="right_note")
|
||||
distance = left.embedding.cosine_distance(right.embedding)
|
||||
|
||||
pairs: list[tuple[int, int, float]] = []
|
||||
try:
|
||||
async with async_session() as session:
|
||||
stmt = (
|
||||
select(left.note_id, right.note_id, distance.label("distance"))
|
||||
.select_from(left)
|
||||
# `<` not `!=`: each unordered pair exactly once, and it drops
|
||||
# the self-pair (distance 0) that would otherwise dominate.
|
||||
.join(right, left.note_id < right.note_id)
|
||||
.join(left_note, left_note.id == left.note_id)
|
||||
.join(right_note, right_note.id == right.note_id)
|
||||
.where(
|
||||
left_note.note_type == SNIPPET_NOTE_TYPE,
|
||||
right_note.note_type == SNIPPET_NOTE_TYPE,
|
||||
left_note.deleted_at.is_(None),
|
||||
right_note.deleted_at.is_(None),
|
||||
# Owner-scoped on both sides — see the note above on why the
|
||||
# report is bounded by what merge can actually act on.
|
||||
left_note.user_id == user_id,
|
||||
right_note.user_id == user_id,
|
||||
distance <= max_distance,
|
||||
)
|
||||
.order_by(distance.asc())
|
||||
.limit(max(1, limit))
|
||||
)
|
||||
rows = list((await session.execute(stmt)).all())
|
||||
pairs = [(int(a), int(b), round(1.0 - float(d), 4)) for a, b, d in rows]
|
||||
except Exception:
|
||||
logger.warning("Near-duplicate snippet scan failed", exc_info=True)
|
||||
return {"groups": [], "pairs": [], "threshold": floor}
|
||||
|
||||
if not pairs:
|
||||
return {"groups": [], "pairs": [], "threshold": floor}
|
||||
|
||||
best: dict[tuple[int, int], float] = {(a, b): s for a, b, s in pairs}
|
||||
grouped = group_pairs(pairs)
|
||||
|
||||
# Titles for presentation. One fetch for every id in the report.
|
||||
ids = sorted({n for g in grouped for n in g})
|
||||
titles: dict[int, str] = {}
|
||||
try:
|
||||
async with async_session() as session:
|
||||
rows = (await session.execute(
|
||||
select(Note.id, Note.title).where(Note.id.in_(ids))
|
||||
)).all()
|
||||
titles = {int(i): t for i, t in rows}
|
||||
except Exception:
|
||||
logger.debug("duplicate report titles unavailable", exc_info=True)
|
||||
|
||||
groups = []
|
||||
for members in grouped:
|
||||
scores = [
|
||||
s for (a, b), s in best.items() if a in members and b in members
|
||||
]
|
||||
groups.append({
|
||||
"note_ids": members,
|
||||
"snippets": [
|
||||
{"id": nid, "title": titles.get(nid, "")} for nid in members
|
||||
],
|
||||
# The strongest resemblance in the set — how confident the suggestion
|
||||
# is, and what the list sorts on.
|
||||
"top_score": max(scores) if scores else floor,
|
||||
})
|
||||
groups.sort(key=lambda g: (-g["top_score"], g["note_ids"][0]))
|
||||
return {"groups": groups, "pairs": pairs, "threshold": floor}
|
||||
|
||||
|
||||
async def find_duplicate_rule(
|
||||
title: str,
|
||||
topic_id: int | None = None,
|
||||
|
||||
@@ -18,7 +18,7 @@ in your ambient lists.
|
||||
import json
|
||||
import logging
|
||||
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy import and_, func, or_, select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.note import Note
|
||||
@@ -108,6 +108,83 @@ def _location_clause(parts: dict[str, str]):
|
||||
return Note.data.path_exists(location_jsonpath(parts))
|
||||
|
||||
|
||||
# --- drift-check filter (#2086) ----------------------------------------------
|
||||
# `verification` selects on the drift-check verdict stored in `data.verification`
|
||||
# (see services/snippets.py). Statuses are the service's own constants; the two
|
||||
# composite values are what the operator actually asks for.
|
||||
#
|
||||
# The interesting one is `attention`. A verdict describes the code it was checked
|
||||
# against, so an OK verdict on code that has since been edited is not an OK
|
||||
# record — nobody has checked what's actually there. That is expressible in SQL
|
||||
# only because `data.code_sha` mirrors the current code's fingerprint alongside
|
||||
# the verdict's: jsonpath compares the two fields within the row, so this stays
|
||||
# one index-served predicate rather than a post-filter that would make the
|
||||
# pagination total a lie.
|
||||
|
||||
def verification_matches(data: dict | None, value: str) -> bool:
|
||||
"""Python dialect of the verification predicate.
|
||||
|
||||
Keep in step with _verification_clause — the semantic arm's candidates are
|
||||
already fetched, so there is no query left to narrow and the same rule has
|
||||
to be expressible twice. Same arrangement as location_matches.
|
||||
"""
|
||||
want = (value or "").strip().lower()
|
||||
if not want:
|
||||
return True
|
||||
verdict = (data or {}).get("verification") or {}
|
||||
status = verdict.get("status") or ""
|
||||
if not status:
|
||||
return want == "unverified"
|
||||
expired = verdict.get("code_sha") != (data or {}).get("code_sha")
|
||||
if want == "unverified":
|
||||
return False
|
||||
if want == "drifted":
|
||||
return status != "ok"
|
||||
if want == "attention":
|
||||
return status != "ok" or expired
|
||||
if want == "ok":
|
||||
return status == "ok" and not expired
|
||||
return status == want
|
||||
|
||||
|
||||
_VERIFY_DRIFTED_JSONPATH = '$.verification ? (@.status != "ok")'
|
||||
_VERIFY_EXPIRED_JSONPATH = "$ ? (@.verification.code_sha != @.code_sha)"
|
||||
_VERIFY_ANY_JSONPATH = "$.verification"
|
||||
|
||||
|
||||
def _verification_clause(value: str):
|
||||
"""SQL predicate for one `verification` filter value, or None for no filter."""
|
||||
want = (value or "").strip().lower()
|
||||
if not want:
|
||||
return None
|
||||
has_verdict = Note.data.path_exists(_VERIFY_ANY_JSONPATH)
|
||||
if want == "unverified":
|
||||
# Never checked at all. Rows predating migration 0070 have no `data`
|
||||
# whatsoever and land here correctly — which is right, they haven't been.
|
||||
return ~has_verdict
|
||||
if want == "drifted":
|
||||
return Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH)
|
||||
if want == "attention":
|
||||
# Everything worth looking at: a failing verdict, OR an expired one.
|
||||
return or_(
|
||||
Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH),
|
||||
and_(has_verdict, Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH)),
|
||||
)
|
||||
if want == "ok":
|
||||
# A clean bill of health that still describes the current code. The
|
||||
# `~expired` half matters: without it this would quietly include records
|
||||
# whose blessing has lapsed, which is the exact failure the feature is
|
||||
# meant to catch.
|
||||
return and_(
|
||||
Note.data.path_exists('$.verification ? (@.status == "ok")'),
|
||||
~Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH),
|
||||
)
|
||||
# A specific status: 'missing' | 'moved' | 'changed'.
|
||||
return Note.data.path_exists(
|
||||
f"$.verification ? (@.status == {json.dumps(want)})"
|
||||
)
|
||||
|
||||
|
||||
def _note_to_item(note: Note) -> dict:
|
||||
item: dict = {
|
||||
"id": note.id,
|
||||
@@ -122,6 +199,22 @@ def _note_to_item(note: Note) -> dict:
|
||||
"created_at": note.created_at.isoformat(),
|
||||
"updated_at": note.updated_at.isoformat(),
|
||||
}
|
||||
# Drift verdict (#2086), when one has been recorded. Included here rather
|
||||
# than decorated on by the snippet layer because `current` is derivable from
|
||||
# `data` alone — the verdict's code_sha against the row's — so this needs no
|
||||
# body parsing and stays a plain projection of the column. Omitted entirely
|
||||
# when unchecked, so "no key" and "never verified" don't become two states
|
||||
# the client has to tell apart.
|
||||
verdict = (note.data or {}).get("verification") if note.data else None
|
||||
if verdict and verdict.get("status"):
|
||||
item["verification"] = {
|
||||
"status": verdict["status"],
|
||||
"current": verdict.get("code_sha") == (note.data or {}).get("code_sha"),
|
||||
"checked_at": verdict.get("checked_at"),
|
||||
"detail": verdict.get("detail"),
|
||||
"path": verdict.get("path"),
|
||||
}
|
||||
|
||||
# Task fields — override note_type and add status/priority/due_date
|
||||
if note.is_task:
|
||||
item["note_type"] = "task"
|
||||
@@ -161,6 +254,7 @@ async def query_knowledge(
|
||||
offset: int,
|
||||
project_id: int | None = None,
|
||||
locations: dict[str, str] | None = None,
|
||||
verification: str = "",
|
||||
) -> tuple[list[dict], int]:
|
||||
"""Query knowledge objects (non-task notes) with filters.
|
||||
|
||||
@@ -171,6 +265,10 @@ async def query_knowledge(
|
||||
Today only snippets carry locations, but the column is general, so the filter
|
||||
lives here with the query rather than in one type's service.
|
||||
|
||||
`verification` narrows on the drift-check verdict: 'ok', 'drifted',
|
||||
'unverified', 'attention', or one specific failure ('missing' | 'moved' |
|
||||
'changed'). Empty means no filter.
|
||||
|
||||
Returns (items, total_count).
|
||||
"""
|
||||
# Semantic search path — scores take priority over sort
|
||||
@@ -178,6 +276,7 @@ async def query_knowledge(
|
||||
return await _semantic_knowledge_search(
|
||||
user_id, q, note_type=note_type, tags=tags, limit=limit,
|
||||
offset=offset, project_id=project_id, locations=locations,
|
||||
verification=verification,
|
||||
)
|
||||
|
||||
# No query = browsing. Narrower scope: a record shared directly with the
|
||||
@@ -196,6 +295,10 @@ async def query_knowledge(
|
||||
if locations:
|
||||
base = base.where(_location_clause(locations))
|
||||
|
||||
verify_clause = _verification_clause(verification)
|
||||
if verify_clause is not None:
|
||||
base = base.where(verify_clause)
|
||||
|
||||
# Count before pagination
|
||||
count_stmt = select(func.count()).select_from(base.subquery())
|
||||
total: int = (await session.execute(count_stmt)).scalar_one()
|
||||
@@ -224,6 +327,7 @@ async def _semantic_knowledge_search(
|
||||
offset: int,
|
||||
project_id: int | None = None,
|
||||
locations: dict[str, str] | None = None,
|
||||
verification: str = "",
|
||||
) -> tuple[list[dict], int]:
|
||||
"""Hybrid search: keyword matches first (title/body ILIKE), then semantic results.
|
||||
|
||||
@@ -259,6 +363,9 @@ async def _semantic_knowledge_search(
|
||||
base = base.where(Note.tags.contains([tag]))
|
||||
if locations:
|
||||
base = base.where(_location_clause(locations))
|
||||
verify_clause = _verification_clause(verification)
|
||||
if verify_clause is not None:
|
||||
base = base.where(verify_clause)
|
||||
# Title matches first, then body-only matches, newest first within each
|
||||
base = base.order_by(
|
||||
Note.title.ilike(pattern).desc(),
|
||||
@@ -301,6 +408,8 @@ async def _semantic_knowledge_search(
|
||||
# narrow. See the comment on location_matches.
|
||||
if locations and not location_matches(note.data, locations):
|
||||
continue
|
||||
if verification and not verification_matches(note.data, verification):
|
||||
continue
|
||||
semantic_notes.append(note)
|
||||
except Exception:
|
||||
logger.warning("Semantic search unavailable, using keyword results only", exc_info=True)
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
"""Note usage telemetry — was a surfaced note ever actually pulled?
|
||||
|
||||
Two event streams, deliberately independent:
|
||||
|
||||
- SURFACED: we put this note's title in front of the agent (auto-inject, or
|
||||
either arm of the write-path prior-art trigger).
|
||||
- PULLED: someone then opened it in full (get_snippet / get_note / the REST
|
||||
detail route).
|
||||
|
||||
The ratio between them is the signal. A snippet surfaced forty times and never
|
||||
pulled is not neutral — it occupies the injection budget on every future turn
|
||||
and dilutes the menu — so this is what makes dead weight visible and prunable.
|
||||
|
||||
Design notes (mirrors retrieval_telemetry, for the same reasons):
|
||||
- Writes are fire-and-forget. `record_surfaced` / `record_pulled` extract
|
||||
plain ints synchronously and schedule the insert as a background task, so
|
||||
telemetry never adds latency to — or can break — the surface it observes.
|
||||
- Every failure path is swallowed. Losing a usage row costs a data point;
|
||||
raising would cost the operator their retrieval.
|
||||
- Reads (`usage_for_notes`) are NOT fire-and-forget — a readout the caller
|
||||
awaits, aggregated in one round-trip for a whole page of snippets rather
|
||||
than per row.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
|
||||
from sqlalchemy import func, select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.note_usage import PULLED, SURFACED, NoteUsageEvent
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
async def _insert_events(rows: list[dict]) -> None:
|
||||
"""Persist usage rows. Best-effort: all errors are swallowed."""
|
||||
try:
|
||||
async with async_session() as session:
|
||||
session.add_all([NoteUsageEvent(**row) for row in rows])
|
||||
await session.commit()
|
||||
except Exception:
|
||||
logger.debug("note usage telemetry write skipped", exc_info=True)
|
||||
|
||||
|
||||
def _schedule(rows: list[dict]) -> None:
|
||||
if not rows:
|
||||
return
|
||||
try:
|
||||
asyncio.get_running_loop().create_task(_insert_events(rows))
|
||||
except RuntimeError:
|
||||
# No running loop (sync context outside the app) — skip rather than
|
||||
# block. Every app path runs on the loop.
|
||||
logger.debug("note usage telemetry skipped — no running event loop")
|
||||
|
||||
|
||||
def record_surfaced(
|
||||
*, user_id: int | None, note_ids: list[int] | set[int], source: str
|
||||
) -> None:
|
||||
"""Fire-and-forget: record that these notes were shown to the agent.
|
||||
|
||||
Takes the whole menu at once — one insert per surfacing event, not per note
|
||||
— because a menu is a single decision and its rows should land together.
|
||||
"""
|
||||
try:
|
||||
rows = [
|
||||
{
|
||||
"user_id": user_id,
|
||||
"note_id": int(nid),
|
||||
"event": SURFACED,
|
||||
"source": source,
|
||||
}
|
||||
for nid in note_ids
|
||||
]
|
||||
except Exception:
|
||||
logger.debug("note usage payload build failed", exc_info=True)
|
||||
return
|
||||
_schedule(rows)
|
||||
|
||||
|
||||
def record_pulled(*, user_id: int | None, note_id: int, source: str) -> None:
|
||||
"""Fire-and-forget: record that a note was opened in full."""
|
||||
try:
|
||||
rows = [
|
||||
{
|
||||
"user_id": user_id,
|
||||
"note_id": int(note_id),
|
||||
"event": PULLED,
|
||||
"source": source,
|
||||
}
|
||||
]
|
||||
except Exception:
|
||||
logger.debug("note usage payload build failed", exc_info=True)
|
||||
return
|
||||
_schedule(rows)
|
||||
|
||||
|
||||
def empty_usage() -> dict:
|
||||
"""The zero readout — what a note with no recorded events looks like.
|
||||
|
||||
Callers render this shape unconditionally, so a note predating the table
|
||||
reads as "never surfaced, never pulled" rather than as a missing key.
|
||||
"""
|
||||
return {
|
||||
"surfaced_count": 0,
|
||||
"pull_count": 0,
|
||||
"last_surfaced_at": None,
|
||||
"last_pulled_at": None,
|
||||
}
|
||||
|
||||
|
||||
async def usage_for_notes(note_ids: list[int]) -> dict[int, dict]:
|
||||
"""Aggregate usage for a set of notes: {note_id: {counts + timestamps}}.
|
||||
|
||||
One GROUP BY for the whole page rather than a query per row — this feeds a
|
||||
list view, so the per-row shape would be N+1 by construction. Notes with no
|
||||
events are returned with `empty_usage()` so the caller never has to
|
||||
distinguish "no events" from "not in the result".
|
||||
"""
|
||||
ids = [int(n) for n in note_ids]
|
||||
out: dict[int, dict] = {nid: empty_usage() for nid in ids}
|
||||
if not ids:
|
||||
return out
|
||||
|
||||
try:
|
||||
async with async_session() as session:
|
||||
rows = (
|
||||
await session.execute(
|
||||
select(
|
||||
NoteUsageEvent.note_id,
|
||||
NoteUsageEvent.event,
|
||||
func.count().label("n"),
|
||||
func.max(NoteUsageEvent.created_at).label("last_at"),
|
||||
)
|
||||
.where(NoteUsageEvent.note_id.in_(ids))
|
||||
.group_by(NoteUsageEvent.note_id, NoteUsageEvent.event)
|
||||
)
|
||||
).all()
|
||||
except Exception:
|
||||
# A telemetry readout must not be able to break the list it decorates.
|
||||
logger.debug("note usage readout failed", exc_info=True)
|
||||
return out
|
||||
|
||||
for note_id, event, n, last_at in rows:
|
||||
slot = out.get(int(note_id))
|
||||
if slot is None:
|
||||
continue
|
||||
if event == SURFACED:
|
||||
slot["surfaced_count"] = int(n)
|
||||
slot["last_surfaced_at"] = last_at.isoformat() if last_at else None
|
||||
elif event == PULLED:
|
||||
slot["pull_count"] = int(n)
|
||||
slot["last_pulled_at"] = last_at.isoformat() if last_at else None
|
||||
return out
|
||||
@@ -29,6 +29,7 @@ from scribe.services import rulebooks as rulebooks_svc
|
||||
from scribe.services import snippets as snippets_svc
|
||||
from scribe.services.access import label_shared_items, owner_names_for
|
||||
from scribe.services.embeddings import semantic_search_notes
|
||||
from scribe.services.note_usage import record_surfaced
|
||||
from scribe.services.retrieval_telemetry import record_retrieval
|
||||
from scribe.services.settings import get_setting
|
||||
|
||||
@@ -271,6 +272,12 @@ async def build_autoinject_hint(
|
||||
line += f" — shared by {who}, treat as a suggestion"
|
||||
lines.append(line)
|
||||
|
||||
# Records what SURVIVED the margin gate, not what the ranker returned — the
|
||||
# menu the agent actually saw. retrieval_logs already holds the full
|
||||
# candidate set for threshold tuning; conflating the two would make
|
||||
# "surfaced" mean two different things depending on the surface (#2085).
|
||||
record_surfaced(user_id=user_id, note_ids=note_ids, source="auto_inject")
|
||||
|
||||
return {"context": "\n".join(lines), "note_ids": note_ids, "config": cfg}
|
||||
|
||||
|
||||
@@ -341,12 +348,12 @@ async def build_write_path_hint(
|
||||
arm is logged to retrieval_logs as source='write_path' — its own source, so
|
||||
its precision is tunable separately from auto-inject's.
|
||||
|
||||
KNOWN GAP: only the semantic arm is logged, matching auto-inject's convention
|
||||
of recording the candidate set for threshold tuning. Location hits carry no
|
||||
score, so folding them in would corrupt the score distribution the log exists
|
||||
to capture — but it does mean a snippet surfaced BY PLACE leaves no trace. The
|
||||
usage signal (#2085) correlates retrieval_logs against later pulls, so it will
|
||||
need a home for un-scored surfacing before it can measure this arm.
|
||||
Location hits still carry no score and so stay out of retrieval_logs, whose
|
||||
score distribution they would corrupt. What closed the gap (#2085) is that
|
||||
un-scored surfacing now has its own home: BOTH arms emit note_usage_events,
|
||||
tagged 'write_path_place' vs 'write_path_semantic', so the place arm is
|
||||
finally measurable — and the two arms' pull-through rates are comparable,
|
||||
which is the number that says whether place really does beat meaning here.
|
||||
"""
|
||||
cfg = await get_writepath_config(user_id)
|
||||
empty = {"context": "", "note_ids": [], "config": cfg}
|
||||
@@ -439,6 +446,18 @@ async def build_write_path_hint(
|
||||
owner = owners.get(int(owner_id)) or "another user"
|
||||
lines.append(_prior_art_line(item, marker, owner))
|
||||
|
||||
# Split by arm, which is the whole reason this table exists. The place arm
|
||||
# carries no score and so has no home in retrieval_logs; before #2085 a
|
||||
# snippet surfaced BY PLACE left no trace anywhere, making the arm that
|
||||
# fires on the strongest possible claim ("there is already a canonical
|
||||
# helper in this exact file") the one arm nobody could measure.
|
||||
by_arm: dict[str, list[int]] = {}
|
||||
for marker, item in menu:
|
||||
arm = "write_path_place" if marker in ("here", "nearby") else "write_path_semantic"
|
||||
by_arm.setdefault(arm, []).append(int(item["id"]))
|
||||
for arm, ids in by_arm.items():
|
||||
record_surfaced(user_id=user_id, note_ids=ids, source=arm)
|
||||
|
||||
return {"context": "\n".join(lines), "note_ids": note_ids, "config": cfg}
|
||||
|
||||
|
||||
|
||||
+379
-19
@@ -29,8 +29,10 @@ came from.
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import hashlib
|
||||
import logging
|
||||
import re
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import func, or_, select
|
||||
|
||||
@@ -132,23 +134,58 @@ def _render_location_block(locations: list[dict]) -> str | None:
|
||||
return f"**Locations:**\n{lines}"
|
||||
|
||||
|
||||
def _normalize_merged_from(ids: list[int] | None) -> list[int]:
|
||||
"""Merge provenance as a clean id list: ints only, no dups, order kept.
|
||||
def _normalize_merged_from(entries: list | None) -> list[dict]:
|
||||
"""Merge provenance as `[{"id": int, "locations": [...], "tags": [...]}]`.
|
||||
|
||||
Order is history, not sorting — earlier merges stay first, so the list reads
|
||||
as the sequence of things folded in.
|
||||
|
||||
Each entry records WHAT THAT SOURCE CONTRIBUTED, which is what makes un-merge
|
||||
(#2165) exact. Two problems it solves at once:
|
||||
|
||||
- A location can arrive from a source AND genuinely be the survivor's own.
|
||||
Recording only what the source ADDED means reversing it can never strip a
|
||||
call site the survivor already had.
|
||||
- Two sources can bring the same location. Only the first records it, so
|
||||
un-merging the second leaves it in place — correctly, since the first
|
||||
still claims it.
|
||||
|
||||
A bare int is accepted and normalized to `{"id": n}` with no attribution.
|
||||
That is not legacy tolerance: `snippet_fields` falls back to PARSING THE BODY
|
||||
when a row has no `data`, and the body's `**Merged from:** #ids` line can only
|
||||
ever carry ids. Such an entry still shows provenance; un-merge refuses it
|
||||
rather than guessing, because guessing is exactly the failure above.
|
||||
"""
|
||||
out: list[int] = []
|
||||
for raw in ids or []:
|
||||
out: list[dict] = []
|
||||
seen: set[int] = set()
|
||||
for raw in entries or []:
|
||||
if isinstance(raw, dict):
|
||||
ident, locs, tags = raw.get("id"), raw.get("locations"), raw.get("tags")
|
||||
else:
|
||||
ident, locs, tags = raw, None, None
|
||||
try:
|
||||
i = int(raw)
|
||||
i = int(ident)
|
||||
except (TypeError, ValueError):
|
||||
continue
|
||||
if i > 0 and i not in out:
|
||||
out.append(i)
|
||||
if i <= 0 or i in seen:
|
||||
continue
|
||||
seen.add(i)
|
||||
entry: dict = {"id": i}
|
||||
norm_locs = _normalize_locations(locs) if locs else []
|
||||
if norm_locs:
|
||||
entry["locations"] = norm_locs
|
||||
clean_tags = [t for t in (tags or []) if isinstance(t, str) and t.strip()]
|
||||
if clean_tags:
|
||||
entry["tags"] = clean_tags
|
||||
out.append(entry)
|
||||
return out
|
||||
|
||||
|
||||
def merged_from_ids(entries: list | None) -> list[int]:
|
||||
"""Just the absorbed ids, in history order — for display and containment."""
|
||||
return [e["id"] for e in _normalize_merged_from(entries)]
|
||||
|
||||
|
||||
def compose_body(
|
||||
*,
|
||||
code: str,
|
||||
@@ -186,8 +223,11 @@ def compose_body(
|
||||
# Human-readable mirror of data["merged_from"]. Without it, a merge folds
|
||||
# variants in and the record of what was absorbed lives only in the trash
|
||||
# — recoverable only by someone who already knows to go looking.
|
||||
# Ids only — the body is the human-readable mirror, and per-source
|
||||
# attribution belongs in `data` where it can be queried rather than
|
||||
# re-parsed out of prose.
|
||||
header.append(
|
||||
"**Merged from:** " + ", ".join(f"#{i}" for i in merged)
|
||||
"**Merged from:** " + ", ".join(f"#{e['id']}" for e in merged)
|
||||
)
|
||||
fence_lang = (language or "").strip().lower()
|
||||
code_block = f"```{fence_lang}\n{(code or '').rstrip()}\n```"
|
||||
@@ -303,8 +343,104 @@ def parse_snippet_fields(
|
||||
# and copying a blob into the column we index *around* would be pure weight.
|
||||
_DATA_FIELDS = (
|
||||
"name", "when_to_use", "signature", "language", "locations", "merged_from",
|
||||
"verification",
|
||||
)
|
||||
|
||||
# --- drift check (#2086) -----------------------------------------------------
|
||||
# A recorded snippet points at a repo · path · symbol that WILL rot: files move,
|
||||
# symbols get renamed, implementations diverge from the copy stored here. Left
|
||||
# undetected, the record degrades from "canonical reference" to "confidently
|
||||
# wrong" — which is worse than having no record, because it is surfaced with the
|
||||
# same authority either way.
|
||||
#
|
||||
# WHERE THE CHECK RUNS. Not here. Scribe has no checkout of the operator's repos
|
||||
# and must not acquire one (rule #115 — the instance stays agnostic about where
|
||||
# code lives; giving the server repo access would make every install a
|
||||
# credential problem). The agent already has the working tree, so IT does the
|
||||
# comparing and reports a verdict; the server's job is to remember the verdict,
|
||||
# make it queryable, and know when it has expired.
|
||||
#
|
||||
# WHY THE VERDICT CARRIES A CODE HASH. A stored verdict describes the code it
|
||||
# was checked against. Edit the snippet afterwards and that verdict is no longer
|
||||
# about anything — but invalidating it on write means deciding which edits count
|
||||
# (a `when_to_use` tweak shouldn't void a code check; a code rewrite must). That
|
||||
# rule is fiddly and easy to get subtly wrong. Recording the hash sidesteps it
|
||||
# entirely: a verdict whose `code_sha` no longer matches the body is self-
|
||||
# evidently expired, computed at read time, with no invalidation logic to
|
||||
# maintain and no way for an edit path to forget to call it.
|
||||
|
||||
VERIFY_OK = "ok"
|
||||
VERIFY_MISSING = "missing" # the recorded path is gone
|
||||
VERIFY_MOVED = "moved" # path is there, the symbol isn't in it
|
||||
VERIFY_CHANGED = "changed" # both present, but the source no longer matches
|
||||
VERIFY_STATUSES = (VERIFY_OK, VERIFY_MISSING, VERIFY_MOVED, VERIFY_CHANGED)
|
||||
|
||||
# Everything that isn't a clean bill of health. "Stale" in the UI and the filter
|
||||
# means this set — the operator wants one list of things to look at, not four.
|
||||
VERIFY_DRIFTED = (VERIFY_MISSING, VERIFY_MOVED, VERIFY_CHANGED)
|
||||
|
||||
|
||||
def code_sha(code: str) -> str:
|
||||
"""Stable fingerprint of a snippet's code, for expiring stale verdicts.
|
||||
|
||||
Trailing whitespace per line and leading/trailing blank lines are stripped
|
||||
before hashing: those change when a file is reformatted without the code
|
||||
meaning anything different, and a verdict shouldn't expire over an editor's
|
||||
trailing-newline habit.
|
||||
"""
|
||||
normalized = "\n".join(line.rstrip() for line in (code or "").splitlines()).strip()
|
||||
return hashlib.sha256(normalized.encode("utf-8")).hexdigest()[:32]
|
||||
|
||||
|
||||
def compose_verification(
|
||||
*,
|
||||
status: str,
|
||||
checked_code_sha: str,
|
||||
detail: str = "",
|
||||
path: str = "",
|
||||
checked_at: str = "",
|
||||
) -> dict:
|
||||
"""Build the `data.verification` record. Unknown statuses are rejected here
|
||||
rather than stored, so the filter never has to cope with a typo'd status."""
|
||||
if status not in VERIFY_STATUSES:
|
||||
raise ValueError(
|
||||
f"unknown verification status {status!r} — expected one of {VERIFY_STATUSES}"
|
||||
)
|
||||
out = {
|
||||
"status": status,
|
||||
"code_sha": checked_code_sha,
|
||||
"checked_at": checked_at or datetime.now(timezone.utc).isoformat(),
|
||||
}
|
||||
if (detail or "").strip():
|
||||
out["detail"] = detail.strip()
|
||||
if (path or "").strip():
|
||||
out["path"] = path.strip()
|
||||
return out
|
||||
|
||||
|
||||
def verification_view(note, fields: dict) -> dict:
|
||||
"""The verification readout for one snippet, including whether it's expired.
|
||||
|
||||
`status` is what was last reported; `current` says whether that verdict still
|
||||
describes the code in the record. A verdict that no longer matches reads as
|
||||
unverified, because that is what it is — nobody has checked THIS code.
|
||||
"""
|
||||
stored = (fields.get("verification") or {}) if isinstance(fields, dict) else {}
|
||||
if not stored or not stored.get("status"):
|
||||
return {"status": "unverified", "current": False, "checked_at": None}
|
||||
current = stored.get("code_sha") == code_sha(fields.get("code") or "")
|
||||
return {
|
||||
"status": stored["status"],
|
||||
"current": current,
|
||||
"checked_at": stored.get("checked_at"),
|
||||
"detail": stored.get("detail"),
|
||||
"path": stored.get("path"),
|
||||
# What the operator actually wants to know: is there something to fix?
|
||||
# An expired verdict counts as "needs looking at" even if it said ok,
|
||||
# since the code it blessed is not the code that's there now.
|
||||
"needs_attention": (not current) or stored["status"] in VERIFY_DRIFTED,
|
||||
}
|
||||
|
||||
|
||||
def compose_data(
|
||||
*,
|
||||
@@ -312,8 +448,10 @@ def compose_data(
|
||||
when_to_use: str = "",
|
||||
signature: str = "",
|
||||
language: str = "",
|
||||
code: str = "",
|
||||
locations: list[dict] | None = None,
|
||||
merged_from: list[int] | None = None,
|
||||
verification: dict | None = None,
|
||||
) -> dict:
|
||||
"""Build the `notes.data` mirror of a snippet's structured fields.
|
||||
|
||||
@@ -337,6 +475,19 @@ def compose_data(
|
||||
merged = _normalize_merged_from(merged_from)
|
||||
if merged:
|
||||
out["merged_from"] = merged
|
||||
# Carried, never composed here — like merged_from. An ordinary edit must not
|
||||
# silently drop the last drift check, and it doesn't need to invalidate it
|
||||
# either: the verdict's code_sha expires it on read if the code moved on.
|
||||
if verification:
|
||||
out["verification"] = verification
|
||||
# The current code's fingerprint — NOT the code, which stays in the body
|
||||
# (see _DATA_FIELDS). Its only job is to make "this verdict has expired"
|
||||
# expressible in SQL: a jsonpath can compare `@.verification.code_sha` to
|
||||
# `@.code_sha` within the same row, so "show me everything that needs
|
||||
# looking at" stays one index-served query instead of a post-filter that
|
||||
# would break pagination counts.
|
||||
if (code or "").strip():
|
||||
out["code_sha"] = code_sha(code)
|
||||
return out
|
||||
|
||||
|
||||
@@ -419,6 +570,7 @@ async def backfill_snippet_data(*, batch: int = 500) -> int:
|
||||
when_to_use=fields["when_to_use"],
|
||||
signature=fields["signature"],
|
||||
language=fields["language"],
|
||||
code=fields["code"],
|
||||
locations=fields["locations"],
|
||||
merged_from=fields["merged_from"],
|
||||
)
|
||||
@@ -439,7 +591,13 @@ def snippet_to_dict(note) -> dict:
|
||||
``snippet`` already reports, and shipping both would give API consumers two
|
||||
sources of truth for the same facts."""
|
||||
data = note.to_dict()
|
||||
data["snippet"] = snippet_fields(note)
|
||||
fields = snippet_fields(note)
|
||||
data["snippet"] = fields
|
||||
# Promoted out of `snippet` because it is a computed READOUT, not a recorded
|
||||
# field: `current` and `needs_attention` are derived at read time by hashing
|
||||
# the code, and burying them among the stored fields would invite a caller
|
||||
# to try writing them back.
|
||||
data["verification"] = verification_view(note, fields)
|
||||
return data
|
||||
|
||||
|
||||
@@ -479,7 +637,7 @@ async def create_snippet(
|
||||
# body so the two can never describe different things.
|
||||
data=compose_data(
|
||||
name=name, when_to_use=when_to_use, signature=signature,
|
||||
language=language, locations=locations,
|
||||
language=language, code=code, locations=locations,
|
||||
),
|
||||
)
|
||||
_embed_snippet(note)
|
||||
@@ -513,6 +671,7 @@ async def list_snippets(
|
||||
repo: str = "",
|
||||
path: str = "",
|
||||
symbol: str = "",
|
||||
verification: str = "",
|
||||
) -> tuple[list[dict], int]:
|
||||
"""List snippets (id/title/tags/preview dicts), most-recently-updated first.
|
||||
|
||||
@@ -523,7 +682,11 @@ async def list_snippets(
|
||||
``repo`` / ``path`` / ``symbol`` are the reverse lookup: "what canonical
|
||||
helpers already live here?" They narrow to snippets recorded at a matching
|
||||
location, ANDed within one location entry, with ``path`` also matching as a
|
||||
directory prefix. Combinable with ``q`` — search *and* place."""
|
||||
directory prefix. Combinable with ``q`` — search *and* place.
|
||||
|
||||
``verification`` narrows on the drift check: ``attention`` is the useful one
|
||||
— everything whose recorded location or code no longer checks out, plus
|
||||
everything whose verdict expired because the snippet was edited since."""
|
||||
return await knowledge_svc.query_knowledge(
|
||||
user_id=user_id,
|
||||
note_type=SNIPPET_NOTE_TYPE,
|
||||
@@ -535,6 +698,7 @@ async def list_snippets(
|
||||
project_id=project_id,
|
||||
locations=knowledge_svc.location_parts(repo=repo, path=path, symbol=symbol)
|
||||
or None,
|
||||
verification=verification,
|
||||
)
|
||||
|
||||
|
||||
@@ -615,8 +779,12 @@ async def update_snippet(
|
||||
"data": compose_data(
|
||||
name=merged["name"], when_to_use=merged["when_to_use"],
|
||||
signature=merged["signature"], language=merged["language"],
|
||||
locations=merged_locations,
|
||||
code=merged["code"], locations=merged_locations,
|
||||
merged_from=merged.get("merged_from"),
|
||||
# Carried through the edit rather than cleared. If this edit changed
|
||||
# the code, the verdict's code_sha stops matching and it reads as
|
||||
# unverified from here on — no invalidation branch to get wrong.
|
||||
verification=merged.get("verification"),
|
||||
),
|
||||
}
|
||||
# Recompute tags: keep any non-language, non-marker tags the note already had
|
||||
@@ -639,6 +807,59 @@ async def update_snippet(
|
||||
return updated
|
||||
|
||||
|
||||
async def record_verification(
|
||||
user_id: int,
|
||||
snippet_id: int,
|
||||
*,
|
||||
status: str,
|
||||
detail: str = "",
|
||||
path: str = "",
|
||||
):
|
||||
"""Record the result of a drift check against the snippet's source.
|
||||
|
||||
The CHECK happens agent-side — Scribe has no checkout and shouldn't want one
|
||||
(see the drift-check note above). This just remembers the verdict, stamped
|
||||
with a hash of the code it was checked against so it expires by itself when
|
||||
the snippet is edited.
|
||||
|
||||
Requires WRITE access: a verdict changes how the record is presented and
|
||||
whether it shows up in the operator's "needs attention" list, so being able
|
||||
to read a shared snippet must not let you mark it broken.
|
||||
|
||||
Returns the updated note, or None if the id isn't a snippet this user may
|
||||
write. Raises ValueError on an unknown status.
|
||||
"""
|
||||
note = await get_snippet(user_id, snippet_id)
|
||||
if note is None:
|
||||
return None
|
||||
from scribe.services.access import can_write_note
|
||||
if not await can_write_note(user_id, snippet_id):
|
||||
return None
|
||||
|
||||
fields = snippet_fields(note)
|
||||
verification = compose_verification(
|
||||
status=status,
|
||||
checked_code_sha=code_sha(fields.get("code") or ""),
|
||||
detail=detail,
|
||||
path=path or fields.get("path") or "",
|
||||
)
|
||||
# Rebuilt from the CURRENT stored fields plus the new verdict, so recording a
|
||||
# check can't quietly rewrite anything else about the record. Note the body
|
||||
# is untouched — a verdict is metadata about the snippet, not part of it, and
|
||||
# writing it into the body would put it into the embedding.
|
||||
data = compose_data(
|
||||
name=fields.get("name", ""),
|
||||
when_to_use=fields.get("when_to_use", ""),
|
||||
signature=fields.get("signature", ""),
|
||||
language=fields.get("language", ""),
|
||||
code=fields.get("code", ""),
|
||||
locations=fields.get("locations") or [],
|
||||
merged_from=fields.get("merged_from") or [],
|
||||
verification=verification,
|
||||
)
|
||||
return await notes_svc.update_note(note.user_id, snippet_id, data=data)
|
||||
|
||||
|
||||
async def delete_snippet(user_id: int, snippet_id: int) -> bool:
|
||||
"""Retire a snippet to the trash (recoverable). Returns False if the id isn't
|
||||
a snippet this user may WRITE.
|
||||
@@ -676,15 +897,32 @@ def merge_snippet_fields(
|
||||
"""Pure merge: union locations (target's first, then each source in order)
|
||||
and union extra tags. The target's scalar fields (name/when_to_use/signature/
|
||||
language/code) win — only locations and tags accumulate. ``sources`` is a
|
||||
list of (parsed_fields, tags). Returns (locations, extra_tags)."""
|
||||
locations = list(target_fields.get("locations") or [])
|
||||
list of (parsed_fields, tags).
|
||||
|
||||
Returns (locations, extra_tags, contributions) where `contributions` is one
|
||||
`{"locations": [...], "tags": [...]}` per source, positionally aligned with
|
||||
`sources`, holding ONLY what that source actually added — anything the
|
||||
survivor (or an earlier source) already had is not attributed to it. That is
|
||||
what lets un-merge subtract exactly, without stripping a call site the
|
||||
survivor legitimately owns."""
|
||||
locations = _normalize_locations(target_fields.get("locations") or [])
|
||||
extra = _extra_tags(target_tags, target_fields.get("language", ""))
|
||||
contributions: list[dict] = []
|
||||
for sfields, stags in sources:
|
||||
locations.extend(sfields.get("locations") or [])
|
||||
before = {_location_str(loc) for loc in locations}
|
||||
added_locs = []
|
||||
for loc in _normalize_locations(sfields.get("locations") or []):
|
||||
if _location_str(loc) not in before:
|
||||
before.add(_location_str(loc))
|
||||
locations.append(loc)
|
||||
added_locs.append(loc)
|
||||
added_tags = []
|
||||
for t in _extra_tags(stags, sfields.get("language", "")):
|
||||
if t not in extra:
|
||||
extra.append(t)
|
||||
return _normalize_locations(locations), extra
|
||||
added_tags.append(t)
|
||||
contributions.append({"locations": added_locs, "tags": added_tags})
|
||||
return _normalize_locations(locations), extra, contributions
|
||||
|
||||
|
||||
async def merge_snippets(user_id: int, target_id: int, source_ids: list[int]):
|
||||
@@ -734,15 +972,25 @@ async def merge_snippets(user_id: int, target_id: int, source_ids: list[int]):
|
||||
|
||||
tgt_fields = snippet_fields(target)
|
||||
parsed_sources = [(snippet_fields(s), s.tags) for s in sources]
|
||||
locations, extra_tags = merge_snippet_fields(tgt_fields, target.tags, parsed_sources)
|
||||
locations, extra_tags, contributions = merge_snippet_fields(
|
||||
tgt_fields, target.tags, parsed_sources
|
||||
)
|
||||
|
||||
# Provenance: what this record absorbed, and when it absorbed it, in order.
|
||||
# Merge keeps the target's scalar fields and trashes the sources, so without
|
||||
# this the fact that a variant ever existed survives only in the trash — and
|
||||
# only for someone who already knew to go looking. Accumulated, not replaced:
|
||||
# a target merged twice keeps both histories.
|
||||
#
|
||||
# Each entry also carries what THAT source contributed, which is what makes
|
||||
# un-merge exact (#2165) rather than a blind subtraction that would strip
|
||||
# call sites the survivor legitimately owns.
|
||||
merged_from = _normalize_merged_from(
|
||||
list(tgt_fields.get("merged_from") or []) + [s.id for s in sources]
|
||||
list(tgt_fields.get("merged_from") or [])
|
||||
+ [
|
||||
{"id": s.id, **contrib}
|
||||
for s, contrib in zip(sources, contributions)
|
||||
]
|
||||
)
|
||||
|
||||
# Owner-scoped write, authorised above — same reason as update_snippet.
|
||||
@@ -760,7 +1008,10 @@ async def merge_snippets(user_id: int, target_id: int, source_ids: list[int]):
|
||||
data=compose_data(
|
||||
name=tgt_fields["name"], when_to_use=tgt_fields["when_to_use"],
|
||||
signature=tgt_fields["signature"], language=tgt_fields["language"],
|
||||
locations=locations, merged_from=merged_from,
|
||||
code=tgt_fields["code"], locations=locations, merged_from=merged_from,
|
||||
# No verification carried: the survivor's code is a union of several
|
||||
# sources, so no prior verdict describes it. It reads as unverified,
|
||||
# which is the honest answer — nobody has checked THIS code.
|
||||
),
|
||||
)
|
||||
if updated is None:
|
||||
@@ -776,3 +1027,112 @@ async def merge_snippets(user_id: int, target_id: int, source_ids: list[int]):
|
||||
|
||||
_embed_snippet(updated)
|
||||
return updated, merged_ids
|
||||
|
||||
|
||||
class UnmergeError(Exception):
|
||||
"""Un-merge refused — the reason is the message, meant for the operator."""
|
||||
|
||||
|
||||
async def unmerge_snippet(user_id: int, survivor_id: int, source_id: int):
|
||||
"""Reverse ONE source out of a merged survivor: restore it, and strip exactly
|
||||
what it contributed.
|
||||
|
||||
WHY THIS OWNS THE RESTORE. The obvious alternative was to have trash-restore
|
||||
notice that the record it's reviving was merged into something and offer to
|
||||
reverse. That would make the generic trash path learn snippet semantics for
|
||||
one record type. Instead un-merge performs the restore itself, so the inverse
|
||||
is one operation with one authorization check and the trash path stays
|
||||
ignorant. Restoring from the trash directly is still allowed and still leaves
|
||||
both records claiming the same call sites — which is why this exists — but it
|
||||
is no longer the only way back.
|
||||
|
||||
EXACTNESS. Subtraction uses the contribution recorded at merge time, not the
|
||||
source's current locations. A source that was itself edited after being
|
||||
merged would otherwise strip locations it never contributed, and a location
|
||||
the survivor independently owned would be lost. When an entry carries no
|
||||
attribution (provenance parsed back out of the body, which can only hold
|
||||
ids), this REFUSES rather than guessing.
|
||||
|
||||
Returns (survivor_note, restored_source_note). Raises UnmergeError with a
|
||||
reason the operator can act on; returns None if the survivor isn't a snippet
|
||||
this caller can see.
|
||||
"""
|
||||
from scribe.services.access import can_write_note
|
||||
|
||||
survivor = await get_snippet(user_id, survivor_id)
|
||||
if survivor is None:
|
||||
return None
|
||||
if not await can_write_note(user_id, survivor_id):
|
||||
raise PermissionError(
|
||||
f"snippet {survivor_id} is shared with you read-only — you can't "
|
||||
f"un-merge a record you can't edit"
|
||||
)
|
||||
|
||||
fields = snippet_fields(survivor)
|
||||
entries = _normalize_merged_from(fields.get("merged_from"))
|
||||
entry = next((e for e in entries if e["id"] == int(source_id)), None)
|
||||
if entry is None:
|
||||
raise UnmergeError(
|
||||
f"snippet {survivor_id} has no record of absorbing #{source_id}"
|
||||
)
|
||||
if "locations" not in entry and "tags" not in entry:
|
||||
raise UnmergeError(
|
||||
f"#{source_id} was folded into {survivor_id} before per-source "
|
||||
f"provenance was recorded, so what it contributed isn't known. "
|
||||
f"Restore it from the trash and adjust both records by hand — "
|
||||
f"subtracting a guess could strip call sites {survivor_id} owns."
|
||||
)
|
||||
|
||||
# Bring the source back FIRST: if it can't be revived there is nothing to
|
||||
# un-merge into, and the survivor is better left whole than stripped of
|
||||
# locations whose other claimant never returned.
|
||||
#
|
||||
# An ALREADY-ALIVE source is the common case, not an error — the operator
|
||||
# restored it from the trash themselves, which is precisely the state that
|
||||
# motivated this feature: both records then claim the same call sites, and
|
||||
# nothing had ever stripped the survivor's copy. Skip the revive and go
|
||||
# straight to the subtraction that fixes it.
|
||||
from scribe.services.trash import restore_entity
|
||||
|
||||
restored = await get_snippet(user_id, int(source_id))
|
||||
if restored is None:
|
||||
if await restore_entity(survivor.user_id, "note", int(source_id)) is None:
|
||||
raise UnmergeError(
|
||||
f"#{source_id} could not be restored — it was most likely purged "
|
||||
f"from the trash, and a purged source cannot be brought back"
|
||||
)
|
||||
restored = await get_snippet(user_id, int(source_id))
|
||||
|
||||
drop_locs = {_location_str(loc) for loc in (entry.get("locations") or [])}
|
||||
drop_tags = set(entry.get("tags") or [])
|
||||
kept_locations = [
|
||||
loc for loc in (fields.get("locations") or [])
|
||||
if _location_str(loc) not in drop_locs
|
||||
]
|
||||
kept_extra = [
|
||||
t for t in _extra_tags(survivor.tags, fields.get("language", ""))
|
||||
if t not in drop_tags
|
||||
]
|
||||
remaining = [e for e in entries if e["id"] != int(source_id)]
|
||||
|
||||
updated = await notes_svc.update_note(
|
||||
survivor.user_id, survivor_id,
|
||||
body=compose_body(
|
||||
code=fields["code"], language=fields["language"],
|
||||
signature=fields["signature"], when_to_use=fields["when_to_use"],
|
||||
locations=kept_locations, merged_from=remaining,
|
||||
),
|
||||
tags=compose_tags(fields["language"], kept_extra),
|
||||
data=compose_data(
|
||||
name=fields["name"], when_to_use=fields["when_to_use"],
|
||||
signature=fields["signature"], language=fields["language"],
|
||||
code=fields["code"], locations=kept_locations, merged_from=remaining,
|
||||
# Same reasoning as merge: the survivor's location set just changed,
|
||||
# so any prior drift verdict no longer describes it. Dropped rather
|
||||
# than carried.
|
||||
),
|
||||
)
|
||||
if updated is None:
|
||||
return None
|
||||
_embed_snippet(updated)
|
||||
return updated, restored
|
||||
|
||||
@@ -181,6 +181,36 @@ async def restore(user_id: int, batch_id: str) -> int:
|
||||
return n
|
||||
|
||||
|
||||
async def restore_entity(user_id: int, entity_type: str, entity_id: int) -> int | None:
|
||||
"""Restore ONE trashed entity by id, by reviving the batch that took it.
|
||||
|
||||
The inverse of `delete(user_id, entity_type, entity_id)`, which returns a
|
||||
batch id the caller usually doesn't keep. Callers that need to undo their own
|
||||
soft-delete later — snippet un-merge (#2165) — know the entity id, not the
|
||||
batch, and looking it up here keeps them from reaching into the column set.
|
||||
|
||||
Restores the whole batch on purpose, not just the row: the batch is the
|
||||
entity plus its cascaded descendants, so reviving the parent alone would
|
||||
leave them orphaned in the trash. That is the same thing the trash UI does.
|
||||
|
||||
Returns rows restored, or None if the entity isn't trashed / not owned.
|
||||
"""
|
||||
model = next((m for m, label in _TYPE.items() if label == entity_type), None)
|
||||
if model is None:
|
||||
return None
|
||||
async with async_session() as session:
|
||||
batch = (await session.execute(
|
||||
select(model.deleted_batch_id).where(
|
||||
model.id == entity_id,
|
||||
_owner_clause(model, user_id),
|
||||
model.deleted_at.isnot(None),
|
||||
)
|
||||
)).scalars().first()
|
||||
if not batch:
|
||||
return None
|
||||
return await restore(user_id, batch)
|
||||
|
||||
|
||||
async def purge(user_id: int, batch_id: str) -> int:
|
||||
"""Hard-delete every row in the batch. Irreversible."""
|
||||
from sqlalchemy import delete as sql_delete
|
||||
|
||||
@@ -228,5 +228,6 @@ def test_register_attaches_all_tools():
|
||||
snippets.register(FakeMcp())
|
||||
assert set(names) == {
|
||||
"list_snippets", "create_snippet", "get_snippet", "update_snippet",
|
||||
"delete_snippet", "merge_snippets",
|
||||
"delete_snippet", "merge_snippets", "verify_snippet",
|
||||
"find_duplicate_snippets", "unmerge_snippet",
|
||||
}
|
||||
|
||||
@@ -0,0 +1,185 @@
|
||||
"""Tests for the usage signal (#2085) — was a surfaced record ever pulled?
|
||||
|
||||
Covers the payload shaping and the two contracts that make this safe to leave in
|
||||
the hot path: telemetry never raises, and telemetry never blocks. Plus the thing
|
||||
the feature exists for — that the write-path PLACE arm is now recorded, since
|
||||
before this it surfaced snippets while leaving no trace anywhere.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import note_usage
|
||||
from scribe.services.note_usage import (
|
||||
empty_usage,
|
||||
record_pulled,
|
||||
record_surfaced,
|
||||
usage_for_notes,
|
||||
)
|
||||
|
||||
|
||||
def _note(nid, title, user_id=1):
|
||||
n = MagicMock()
|
||||
n.id, n.title, n.user_id = nid, title, user_id
|
||||
n.note_type = "snippet"
|
||||
return n
|
||||
|
||||
|
||||
# --- recording ------------------------------------------------------------
|
||||
|
||||
|
||||
async def test_record_surfaced_writes_one_row_per_note():
|
||||
with patch.object(note_usage, "_schedule") as sched:
|
||||
record_surfaced(user_id=7, note_ids=[11, 12], source="auto_inject")
|
||||
rows = sched.call_args[0][0]
|
||||
assert [r["note_id"] for r in rows] == [11, 12]
|
||||
assert {r["event"] for r in rows} == {"surfaced"}
|
||||
assert {r["source"] for r in rows} == {"auto_inject"}
|
||||
assert {r["user_id"] for r in rows} == {7}
|
||||
|
||||
|
||||
async def test_record_pulled_writes_a_single_row():
|
||||
with patch.object(note_usage, "_schedule") as sched:
|
||||
record_pulled(user_id=7, note_id=11, source="mcp_get_snippet")
|
||||
rows = sched.call_args[0][0]
|
||||
assert rows == [
|
||||
{"user_id": 7, "note_id": 11, "event": "pulled", "source": "mcp_get_snippet"}
|
||||
]
|
||||
|
||||
|
||||
async def test_empty_menu_schedules_nothing():
|
||||
"""No notes surfaced is not an event — it must not cost a write."""
|
||||
with patch.object(note_usage, "_insert_events") as ins:
|
||||
record_surfaced(user_id=1, note_ids=[], source="auto_inject")
|
||||
ins.assert_not_called()
|
||||
|
||||
|
||||
async def test_recording_never_raises_on_bad_input():
|
||||
"""Telemetry sits in the hot path of every retrieval. A malformed id must
|
||||
cost a data point, never the operator's request."""
|
||||
with patch.object(note_usage, "_schedule"):
|
||||
record_surfaced(user_id=1, note_ids=["not-an-int"], source="auto_inject")
|
||||
record_pulled(user_id=1, note_id=None, source="mcp_get_note")
|
||||
|
||||
|
||||
async def test_recording_without_an_event_loop_is_skipped_not_raised():
|
||||
"""Called from a sync context outside the app (a script, a test helper),
|
||||
there is no loop to schedule on. Skip rather than blow up."""
|
||||
with patch.object(
|
||||
note_usage.asyncio, "get_running_loop", side_effect=RuntimeError
|
||||
):
|
||||
record_pulled(user_id=1, note_id=5, source="mcp_get_note")
|
||||
|
||||
|
||||
# --- readout --------------------------------------------------------------
|
||||
|
||||
|
||||
async def test_usage_for_notes_zero_fills_every_requested_id():
|
||||
"""The caller renders this shape unconditionally, so a note with no events
|
||||
must come back as zeroes, not as a missing key."""
|
||||
session = MagicMock()
|
||||
session.execute = AsyncMock(return_value=MagicMock(all=MagicMock(return_value=[])))
|
||||
ctx = MagicMock()
|
||||
ctx.__aenter__ = AsyncMock(return_value=session)
|
||||
ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
with patch.object(note_usage, "async_session", return_value=ctx):
|
||||
out = await usage_for_notes([3, 4])
|
||||
assert out == {3: empty_usage(), 4: empty_usage()}
|
||||
|
||||
|
||||
async def test_usage_for_notes_splits_counts_by_event():
|
||||
from datetime import datetime, timezone
|
||||
|
||||
ts = datetime(2026, 7, 28, tzinfo=timezone.utc)
|
||||
rows = [(3, "surfaced", 9, ts), (3, "pulled", 2, ts)]
|
||||
session = MagicMock()
|
||||
session.execute = AsyncMock(
|
||||
return_value=MagicMock(all=MagicMock(return_value=rows))
|
||||
)
|
||||
ctx = MagicMock()
|
||||
ctx.__aenter__ = AsyncMock(return_value=session)
|
||||
ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
with patch.object(note_usage, "async_session", return_value=ctx):
|
||||
out = await usage_for_notes([3])
|
||||
assert out[3]["surfaced_count"] == 9
|
||||
assert out[3]["pull_count"] == 2
|
||||
assert out[3]["last_pulled_at"] == ts.isoformat()
|
||||
|
||||
|
||||
async def test_usage_readout_failure_degrades_to_zeroes():
|
||||
"""A telemetry readout must not be able to break the list it decorates."""
|
||||
with patch.object(note_usage, "async_session", side_effect=RuntimeError("boom")):
|
||||
out = await usage_for_notes([3])
|
||||
assert out == {3: empty_usage()}
|
||||
|
||||
|
||||
async def test_no_ids_short_circuits_without_a_query():
|
||||
with patch.object(note_usage, "async_session") as sess:
|
||||
assert await usage_for_notes([]) == {}
|
||||
sess.assert_not_called()
|
||||
|
||||
|
||||
# --- the gap this closes --------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"marker, expected_source",
|
||||
[("here", "write_path_place"), ("nearby", "write_path_place")],
|
||||
)
|
||||
async def test_write_path_place_arm_is_recorded(marker, expected_source):
|
||||
"""The place arm carries no score, so it has no home in retrieval_logs — it
|
||||
surfaced snippets while leaving no trace anywhere. That was the blocker
|
||||
#2082 recorded against this task; this is the assertion that it's closed."""
|
||||
from scribe.services import plugin_context
|
||||
|
||||
here = [{"id": 42, "title": "helper", "user_id": 1, "note_type": "snippet"}]
|
||||
with (
|
||||
patch.object(
|
||||
plugin_context,
|
||||
"get_writepath_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.55, "top_k": 3}),
|
||||
),
|
||||
patch.object(
|
||||
plugin_context.snippets_svc,
|
||||
"list_snippets",
|
||||
AsyncMock(side_effect=[(here, 1), ([], 0)]),
|
||||
),
|
||||
patch.object(
|
||||
plugin_context, "semantic_search_notes", AsyncMock(return_value=[])
|
||||
),
|
||||
patch.object(plugin_context, "owner_names_for", AsyncMock(return_value={})),
|
||||
patch.object(plugin_context, "record_retrieval"),
|
||||
patch.object(plugin_context, "record_surfaced") as surfaced,
|
||||
):
|
||||
out = await plugin_context.build_write_path_hint(
|
||||
1, "src/a.py", code="def f(): pass"
|
||||
)
|
||||
|
||||
assert out["note_ids"] == [42]
|
||||
sources = {c.kwargs["source"] for c in surfaced.call_args_list}
|
||||
assert expected_source in sources
|
||||
|
||||
|
||||
async def test_auto_inject_records_what_survived_the_margin_gate():
|
||||
"""Not what the ranker returned — retrieval_logs already holds that. These
|
||||
two numbers must not silently mean different things per surface."""
|
||||
from scribe.services import plugin_context
|
||||
|
||||
hits = [(0.90, _note(1, "kept")), (0.40, _note(2, "cut by the margin gate"))]
|
||||
with (
|
||||
patch.object(
|
||||
plugin_context,
|
||||
"get_autoinject_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.3, "top_k": 5}),
|
||||
),
|
||||
patch.object(
|
||||
plugin_context, "semantic_search_notes", AsyncMock(return_value=hits)
|
||||
),
|
||||
patch.object(plugin_context, "owner_names_for", AsyncMock(return_value={})),
|
||||
patch.object(plugin_context, "record_retrieval"),
|
||||
patch.object(plugin_context, "record_surfaced") as surfaced,
|
||||
):
|
||||
await plugin_context.build_autoinject_hint(1, "a query")
|
||||
|
||||
assert surfaced.call_args.kwargs["note_ids"] == [1]
|
||||
assert surfaced.call_args.kwargs["source"] == "auto_inject"
|
||||
@@ -20,6 +20,8 @@ def test_snippet_handlers_callable():
|
||||
for name in (
|
||||
"list_snippets_route", "create_snippet_route", "get_snippet_route",
|
||||
"update_snippet_route", "delete_snippet_route", "merge_snippet_route",
|
||||
"verify_snippet_route", "duplicate_snippets_route",
|
||||
"unmerge_snippet_route",
|
||||
):
|
||||
assert callable(getattr(routes, name))
|
||||
|
||||
@@ -29,7 +31,8 @@ def test_service_functions_take_user_id():
|
||||
from scribe.services import snippets as svc
|
||||
for fn_name in (
|
||||
"create_snippet", "list_snippets", "get_snippet", "update_snippet",
|
||||
"delete_snippet", "merge_snippets",
|
||||
"delete_snippet", "merge_snippets", "record_verification",
|
||||
"unmerge_snippet",
|
||||
):
|
||||
fn = getattr(svc, fn_name)
|
||||
assert callable(fn)
|
||||
@@ -45,7 +48,8 @@ def test_agent_and_web_surfaces_stay_at_parity():
|
||||
from scribe.routes import snippets as routes
|
||||
|
||||
# Every write verb the web surface offers, the agent surface offers too.
|
||||
for verb in ("create", "get", "list", "update", "delete", "merge"):
|
||||
for verb in ("create", "get", "list", "update", "delete", "merge", "verify",
|
||||
"unmerge"):
|
||||
assert callable(getattr(tools, f"{verb}_snippets", None) or
|
||||
getattr(tools, f"{verb}_snippet", None)), f"MCP lacks {verb}"
|
||||
|
||||
@@ -61,6 +65,17 @@ def test_agent_and_web_surfaces_stay_at_parity():
|
||||
assert "system_ids" in inspect.getsource(route)
|
||||
assert "find_duplicate_note" in inspect.getsource(routes.create_snippet_route)
|
||||
|
||||
# The drift check (#2086) reaches both: recording a verdict, and filtering
|
||||
# the list by one. A verdict an agent records has to be visible to the human
|
||||
# looking at the same corpus, or the two surfaces disagree about what's rotten.
|
||||
assert "verification" in inspect.signature(tools.list_snippets).parameters
|
||||
assert "verification" in inspect.getsource(routes.list_snippets_route)
|
||||
|
||||
# The near-duplicate report (#2088) reaches both. The web side can only hand
|
||||
# a group to the merge flow if it can get the groups in the first place.
|
||||
assert callable(getattr(tools, "find_duplicate_snippets", None))
|
||||
assert callable(getattr(routes, "duplicate_snippets_route", None))
|
||||
|
||||
|
||||
def test_project_scoping_reaches_every_caller():
|
||||
"""A snippet search has to be narrowable to one project from both surfaces."""
|
||||
|
||||
@@ -148,7 +148,9 @@ def test_merge_snippet_fields_unions_locations_and_tags():
|
||||
["py", "snippet", "helper"])
|
||||
src2 = ({"language": "py", "locations": [{"repo": "a", "path": "a.py", "symbol": "f"}]}, # dup loc
|
||||
["snippet"])
|
||||
locs, extra = s.merge_snippet_fields(target_fields, ["py", "snippet", "core"], [src1, src2])
|
||||
locs, extra, contributions = s.merge_snippet_fields(
|
||||
target_fields, ["py", "snippet", "core"], [src1, src2]
|
||||
)
|
||||
# target location first, then unique source locations; dup dropped.
|
||||
assert locs == [
|
||||
{"repo": "a", "path": "a.py", "symbol": "f"},
|
||||
@@ -157,6 +159,14 @@ def test_merge_snippet_fields_unions_locations_and_tags():
|
||||
# extra tags unioned, language + "snippet" markers excluded.
|
||||
assert extra == ["core", "helper"]
|
||||
|
||||
# Attribution (#2165): only what each source ACTUALLY added. src2's location
|
||||
# is one the target already had, so it contributed nothing — un-merging it
|
||||
# must not strip a call site the survivor owns in its own right.
|
||||
assert contributions[0]["locations"] == [{"repo": "b", "path": "b.py", "symbol": "g"}]
|
||||
assert contributions[0]["tags"] == ["helper"]
|
||||
assert contributions[1]["locations"] == []
|
||||
assert contributions[1]["tags"] == []
|
||||
|
||||
|
||||
# --- the queryable mirror (notes.data, migration 0070) -----------------------
|
||||
|
||||
@@ -253,19 +263,44 @@ def test_data_and_body_round_trip_to_the_same_fields():
|
||||
|
||||
def test_normalize_merged_from_keeps_history_order_and_drops_junk():
|
||||
"""Order is history, not sorting — earlier merges stay first."""
|
||||
assert s._normalize_merged_from([9, 3, 9, "4", None, 0, -2, "x"]) == [9, 3, 4]
|
||||
assert s.merged_from_ids([9, 3, 9, "4", None, 0, -2, "x"]) == [9, 3, 4]
|
||||
assert s._normalize_merged_from(None) == []
|
||||
|
||||
|
||||
def test_normalize_merged_from_carries_per_source_attribution():
|
||||
"""The shape that makes un-merge exact (#2165): each entry holds what THAT
|
||||
source contributed, so reversing one can't strip what the survivor owns."""
|
||||
loc = {"repo": "a", "path": "a.py", "symbol": "f"}
|
||||
out = s._normalize_merged_from([{"id": 7, "locations": [loc], "tags": ["x"]}])
|
||||
assert out == [{"id": 7, "locations": [loc], "tags": ["x"]}]
|
||||
|
||||
|
||||
def test_a_bare_id_normalizes_to_an_entry_with_no_attribution():
|
||||
"""Not legacy tolerance: snippet_fields falls back to PARSING THE BODY when a
|
||||
row has no `data`, and the body's provenance line can only carry ids. Such an
|
||||
entry still shows history; un-merge refuses it rather than guessing."""
|
||||
assert s._normalize_merged_from([5]) == [{"id": 5}]
|
||||
|
||||
|
||||
def test_compose_body_renders_merged_from_and_parse_reads_it_back():
|
||||
body = s.compose_body(code="x = 1", merged_from=[12, 13])
|
||||
assert "**Merged from:** #12, #13" in body
|
||||
got = s.parse_snippet_fields("n — u", body, None)
|
||||
assert got["merged_from"] == [12, 13]
|
||||
# Ids survive the body round-trip; attribution cannot, since the body only
|
||||
# ever renders ids — which is exactly why `data` is the authority for it.
|
||||
assert s.merged_from_ids(got["merged_from"]) == [12, 13]
|
||||
# The code fence still comes last — provenance is header metadata.
|
||||
assert body.rstrip().endswith("```")
|
||||
|
||||
|
||||
def test_compose_body_renders_ids_from_rich_entries():
|
||||
body = s.compose_body(
|
||||
code="x = 1",
|
||||
merged_from=[{"id": 12, "locations": [{"repo": "", "path": "a.py", "symbol": ""}]}],
|
||||
)
|
||||
assert "**Merged from:** #12" in body
|
||||
|
||||
|
||||
def test_compose_body_omits_merged_from_when_there_is_none():
|
||||
"""A snippet that was never merged says nothing about merging."""
|
||||
assert "Merged from" not in s.compose_body(code="x = 1")
|
||||
@@ -273,7 +308,8 @@ def test_compose_body_omits_merged_from_when_there_is_none():
|
||||
|
||||
|
||||
def test_compose_data_mirrors_merged_from():
|
||||
assert s.compose_data(name="f", merged_from=[3, 3, 2])["merged_from"] == [3, 2]
|
||||
got = s.compose_data(name="f", merged_from=[3, 3, 2])["merged_from"]
|
||||
assert s.merged_from_ids(got) == [3, 2]
|
||||
assert "merged_from" not in s.compose_data(name="f")
|
||||
|
||||
|
||||
@@ -283,7 +319,7 @@ def test_snippet_fields_prefers_the_data_columns_merged_from():
|
||||
body = s.compose_body(code="x = 1", merged_from=[12])
|
||||
note = _Note(title="n — u", body=body, tags=["snippet"],
|
||||
data=s.compose_data(name="n", merged_from=[12, 13]))
|
||||
assert s.snippet_fields(note)["merged_from"] == [12, 13]
|
||||
assert s.merged_from_ids(s.snippet_fields(note)["merged_from"]) == [12, 13]
|
||||
|
||||
|
||||
def test_snippet_to_dict_includes_parsed_fields():
|
||||
|
||||
@@ -0,0 +1,251 @@
|
||||
"""Tests for the drift check (#2086) — does a snippet still match its source?
|
||||
|
||||
The check itself runs agent-side (Scribe has no checkout), so what's testable
|
||||
here is everything around it: the verdict's shape, the rule that a verdict
|
||||
expires when the code it blessed is edited away, and the two dialects of the
|
||||
filter predicate — SQL for the browse/keyword arms, Python for the semantic
|
||||
arm's already-fetched candidates.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import knowledge as knowledge_svc
|
||||
from scribe.services import snippets as snippets_svc
|
||||
from scribe.services.snippets import (
|
||||
code_sha,
|
||||
compose_data,
|
||||
compose_verification,
|
||||
verification_view,
|
||||
)
|
||||
|
||||
|
||||
def _note(data=None, body=""):
|
||||
return SimpleNamespace(data=data, body=body, title="n — w", tags=["snippet"])
|
||||
|
||||
|
||||
# --- the verdict record ---------------------------------------------------
|
||||
|
||||
|
||||
def test_unknown_status_is_rejected_not_stored():
|
||||
"""A typo'd status would be stored happily and then never match any filter —
|
||||
invisible rot in the thing built to make rot visible."""
|
||||
with pytest.raises(ValueError):
|
||||
compose_verification(status="broekn", checked_code_sha="abc")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("status", ["ok", "missing", "moved", "changed"])
|
||||
def test_every_documented_status_is_accepted(status):
|
||||
out = compose_verification(status=status, checked_code_sha="abc")
|
||||
assert out["status"] == status
|
||||
assert out["code_sha"] == "abc"
|
||||
assert out["checked_at"]
|
||||
|
||||
|
||||
def test_blank_detail_and_path_are_omitted_not_stored_empty():
|
||||
"""Keeps `data` sparse, and keeps a containment match from tripping on ""."""
|
||||
out = compose_verification(status="ok", checked_code_sha="abc")
|
||||
assert "detail" not in out and "path" not in out
|
||||
|
||||
|
||||
# --- expiry by edit -------------------------------------------------------
|
||||
|
||||
|
||||
def test_code_sha_ignores_reformatting_but_not_meaning():
|
||||
"""A verdict must not expire because an editor stripped trailing whitespace,
|
||||
and must expire when the code actually changes."""
|
||||
assert code_sha("def f():\n return 1") == code_sha("def f(): \n return 1\n")
|
||||
assert code_sha("def f():\n return 1") != code_sha("def f():\n return 2")
|
||||
|
||||
|
||||
def test_verdict_stays_current_while_code_is_unchanged():
|
||||
code = "def f():\n return 1"
|
||||
fields = {"code": code, "verification": compose_verification(
|
||||
status="ok", checked_code_sha=code_sha(code)
|
||||
)}
|
||||
view = verification_view(_note(), fields)
|
||||
assert view["status"] == "ok"
|
||||
assert view["current"] is True
|
||||
assert view["needs_attention"] is False
|
||||
|
||||
|
||||
def test_an_ok_verdict_expires_once_the_code_is_edited():
|
||||
"""The whole reason the verdict carries a hash: an OK verdict must never go
|
||||
on vouching for code nobody checked."""
|
||||
fields = {"code": "def f():\n return 2", "verification": compose_verification(
|
||||
status="ok", checked_code_sha=code_sha("def f():\n return 1")
|
||||
)}
|
||||
view = verification_view(_note(), fields)
|
||||
assert view["current"] is False
|
||||
assert view["needs_attention"] is True
|
||||
|
||||
|
||||
def test_never_checked_reads_as_unverified():
|
||||
view = verification_view(_note(), {"code": "x = 1"})
|
||||
assert view["status"] == "unverified"
|
||||
assert view["current"] is False
|
||||
|
||||
|
||||
def test_a_failing_verdict_needs_attention_even_while_current():
|
||||
code = "x = 1"
|
||||
fields = {"code": code, "verification": compose_verification(
|
||||
status="missing", checked_code_sha=code_sha(code)
|
||||
)}
|
||||
assert verification_view(_note(), fields)["needs_attention"] is True
|
||||
|
||||
|
||||
# --- the data mirror ------------------------------------------------------
|
||||
|
||||
|
||||
def test_compose_data_mirrors_the_current_code_hash():
|
||||
"""`data.code_sha` is what makes "this verdict expired" an SQL-expressible
|
||||
comparison rather than a post-filter that would break pagination totals."""
|
||||
out = compose_data(name="n", code="def f():\n return 1")
|
||||
assert out["code_sha"] == code_sha("def f():\n return 1")
|
||||
|
||||
|
||||
def test_compose_data_omits_the_hash_when_there_is_no_code():
|
||||
assert "code_sha" not in compose_data(name="n")
|
||||
|
||||
|
||||
def test_the_code_itself_is_never_copied_into_data():
|
||||
"""data indexes AROUND the code; duplicating the blob would be pure weight."""
|
||||
out = compose_data(name="n", code="secret_looking_blob")
|
||||
assert "code" not in out
|
||||
assert "secret_looking_blob" not in str(out)
|
||||
|
||||
|
||||
# --- the filter, Python dialect -------------------------------------------
|
||||
|
||||
|
||||
def _data(status, checked_sha, current_sha):
|
||||
return {
|
||||
"verification": {"status": status, "code_sha": checked_sha},
|
||||
"code_sha": current_sha,
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value, expected",
|
||||
[
|
||||
("", True), # no filter
|
||||
("ok", True),
|
||||
("attention", False),
|
||||
("drifted", False),
|
||||
("unverified", False),
|
||||
],
|
||||
)
|
||||
def test_python_dialect_on_a_current_ok_verdict(value, expected):
|
||||
data = _data("ok", "aaa", "aaa")
|
||||
assert knowledge_svc.verification_matches(data, value) is expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value, expected",
|
||||
[("ok", False), ("attention", True), ("drifted", False), ("unverified", False)],
|
||||
)
|
||||
def test_python_dialect_on_an_expired_ok_verdict(value, expected):
|
||||
"""Expired-but-ok is the case that motivated `attention` existing at all: it
|
||||
is not `drifted` (nothing was found wrong) and not `unverified` (a check did
|
||||
happen), yet it plainly needs looking at."""
|
||||
data = _data("ok", "aaa", "bbb")
|
||||
assert knowledge_svc.verification_matches(data, value) is expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value, expected",
|
||||
[("ok", False), ("attention", True), ("drifted", True), ("changed", True),
|
||||
("missing", False)],
|
||||
)
|
||||
def test_python_dialect_on_a_drifted_verdict(value, expected):
|
||||
data = _data("changed", "aaa", "aaa")
|
||||
assert knowledge_svc.verification_matches(data, value) is expected
|
||||
|
||||
|
||||
def test_python_dialect_on_a_row_with_no_data_at_all():
|
||||
"""Rows predating migration 0070 have no `data`. They are unverified, which
|
||||
is true — nobody has checked them."""
|
||||
assert knowledge_svc.verification_matches(None, "unverified") is True
|
||||
assert knowledge_svc.verification_matches(None, "attention") is False
|
||||
assert knowledge_svc.verification_matches(None, "ok") is False
|
||||
|
||||
|
||||
# --- the filter, SQL dialect ----------------------------------------------
|
||||
|
||||
|
||||
def test_no_filter_produces_no_clause():
|
||||
assert knowledge_svc._verification_clause("") is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value", ["ok", "drifted", "unverified", "attention", "missing", "moved", "changed"]
|
||||
)
|
||||
def test_every_filter_value_compiles_to_sql(value):
|
||||
"""The two dialects have to agree, and the SQL one has to be expressible at
|
||||
all — the expired comparison relies on jsonpath reading two fields of the
|
||||
same row, which is easy to get wrong silently."""
|
||||
clause = knowledge_svc._verification_clause(value)
|
||||
assert clause is not None
|
||||
assert str(clause)
|
||||
|
||||
|
||||
def test_a_status_value_is_json_quoted_into_the_jsonpath():
|
||||
"""Statuses are validated on WRITE, but the filter takes caller input
|
||||
directly — a quote must not be able to break out of the jsonpath and turn
|
||||
a narrowing filter into a widening one."""
|
||||
hostile = 'x" || @.status == "ok'
|
||||
clause = knowledge_svc._verification_clause(hostile)
|
||||
params = list(clause.compile().params.values())
|
||||
jsonpath = next(p for p in params if isinstance(p, str) and "status" in p)
|
||||
# The injected quote is escaped, so the whole hostile value stays ONE string
|
||||
# literal rather than closing it and appending a second condition. Asserting
|
||||
# on the exact json.dumps form rather than counting "@.status" — the hostile
|
||||
# value legitimately contains that text inside the literal.
|
||||
import json as _json
|
||||
|
||||
assert jsonpath == f"$.verification ? (@.status == {_json.dumps(hostile)})"
|
||||
assert '\\"' in jsonpath
|
||||
|
||||
|
||||
# --- write path -----------------------------------------------------------
|
||||
|
||||
|
||||
async def test_recording_a_verdict_does_not_touch_the_body():
|
||||
"""A verdict is metadata about the snippet, not part of it. Writing it into
|
||||
the body would put it into the embedding and change what the record recalls
|
||||
against."""
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
note = SimpleNamespace(
|
||||
id=5, user_id=1, note_type="snippet", deleted_at=None,
|
||||
title="n — w", body="```python\nx = 1\n```", tags=["snippet"], data=None,
|
||||
)
|
||||
with (
|
||||
patch.object(snippets_svc, "get_snippet", AsyncMock(return_value=note)),
|
||||
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
||||
patch.object(
|
||||
snippets_svc.notes_svc, "update_note", AsyncMock(return_value=note)
|
||||
) as upd,
|
||||
):
|
||||
await snippets_svc.record_verification(1, 5, status="moved", detail="renamed")
|
||||
|
||||
assert set(upd.call_args.kwargs) == {"data"}
|
||||
stored = upd.call_args.kwargs["data"]["verification"]
|
||||
assert stored["status"] == "moved"
|
||||
assert stored["detail"] == "renamed"
|
||||
|
||||
|
||||
async def test_recording_a_verdict_requires_write_access():
|
||||
"""Being able to read a snippet someone shared must not let you mark it
|
||||
broken — a verdict changes how the record is presented to them."""
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
note = SimpleNamespace(id=5, user_id=2, note_type="snippet", deleted_at=None,
|
||||
title="n — w", body="", tags=[], data=None)
|
||||
with (
|
||||
patch.object(snippets_svc, "get_snippet", AsyncMock(return_value=note)),
|
||||
patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)),
|
||||
patch.object(snippets_svc.notes_svc, "update_note", AsyncMock()) as upd,
|
||||
):
|
||||
assert await snippets_svc.record_verification(1, 5, status="ok") is None
|
||||
upd.assert_not_called()
|
||||
@@ -0,0 +1,99 @@
|
||||
"""Tests for the near-duplicate finder (#2088).
|
||||
|
||||
The SQL half needs a database and lives in the integration lane; what's covered
|
||||
here is the grouping rule — which is where the interesting decisions are — plus
|
||||
the fail-open contract and the threshold resolution.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import dedup as dedup_svc
|
||||
from scribe.services.dedup import (
|
||||
DUPLICATE_DEFAULT_THRESHOLD,
|
||||
get_duplicate_threshold,
|
||||
group_pairs,
|
||||
)
|
||||
|
||||
|
||||
# --- grouping -------------------------------------------------------------
|
||||
|
||||
|
||||
def test_a_simple_pair_becomes_one_set():
|
||||
assert group_pairs([(1, 2, 0.9)]) == [[1, 2]]
|
||||
|
||||
|
||||
def test_grouping_is_transitive():
|
||||
"""A~B and B~C puts all three in one set even though A and C never cleared
|
||||
the bar together. That mirrors merge, which folds every source into one
|
||||
survivor — three overlapping pairs would just be the same chore, unsorted."""
|
||||
assert group_pairs([(1, 2, 0.9), (2, 3, 0.9)]) == [[1, 2, 3]]
|
||||
|
||||
|
||||
def test_disjoint_clusters_stay_separate():
|
||||
groups = group_pairs([(1, 2, 0.9), (3, 4, 0.9)])
|
||||
assert groups == [[1, 2], [3, 4]]
|
||||
|
||||
|
||||
def test_largest_cluster_comes_first():
|
||||
"""The most tangled thing is the most worth fixing."""
|
||||
groups = group_pairs([(5, 6, 0.9), (1, 2, 0.9), (2, 3, 0.9), (3, 4, 0.9)])
|
||||
assert groups[0] == [1, 2, 3, 4]
|
||||
|
||||
|
||||
def test_output_is_stable_across_input_order():
|
||||
"""A report that reshuffles between runs is one nobody can work through."""
|
||||
a = group_pairs([(1, 2, 0.9), (2, 3, 0.9), (7, 8, 0.9)])
|
||||
b = group_pairs([(7, 8, 0.9), (2, 3, 0.9), (1, 2, 0.9)])
|
||||
assert a == b
|
||||
|
||||
|
||||
def test_no_pairs_means_no_groups():
|
||||
assert group_pairs([]) == []
|
||||
|
||||
|
||||
def test_a_node_never_forms_a_group_with_itself():
|
||||
"""The SQL uses `note_id <` so this shouldn't arrive, but a singleton set
|
||||
would render as a "duplicate" of nothing and offer an impossible merge."""
|
||||
assert group_pairs([(1, 1, 1.0)]) == []
|
||||
|
||||
|
||||
# --- threshold ------------------------------------------------------------
|
||||
|
||||
|
||||
async def test_threshold_falls_back_to_the_default_when_unset():
|
||||
with patch("scribe.services.settings.get_setting",
|
||||
AsyncMock(return_value=str(DUPLICATE_DEFAULT_THRESHOLD))):
|
||||
assert await get_duplicate_threshold(1) == DUPLICATE_DEFAULT_THRESHOLD
|
||||
|
||||
|
||||
async def test_a_garbage_setting_falls_back_rather_than_raising():
|
||||
with patch("scribe.services.settings.get_setting",
|
||||
AsyncMock(return_value="not-a-number")):
|
||||
assert await get_duplicate_threshold(1) == DUPLICATE_DEFAULT_THRESHOLD
|
||||
|
||||
|
||||
@pytest.mark.parametrize("stored, expected", [("2.5", 1.0), ("-3", 0.0)])
|
||||
async def test_threshold_is_clamped_to_the_valid_range(stored, expected):
|
||||
with patch("scribe.services.settings.get_setting", AsyncMock(return_value=stored)):
|
||||
assert await get_duplicate_threshold(1) == expected
|
||||
|
||||
|
||||
def test_the_report_threshold_is_looser_than_the_write_gate():
|
||||
"""The gate BLOCKS a create and must be unforgiving of noise; this only
|
||||
suggests a merge the operator reviews, so it has to reach further or it
|
||||
would never surface the pairs the gate already let through."""
|
||||
assert DUPLICATE_DEFAULT_THRESHOLD < dedup_svc._SEMANTIC_THRESHOLD
|
||||
|
||||
|
||||
# --- fail-open ------------------------------------------------------------
|
||||
|
||||
|
||||
async def test_a_failed_scan_returns_an_empty_report_not_an_error():
|
||||
"""A suggestion feature must not be able to break the page it decorates."""
|
||||
with (
|
||||
patch.object(dedup_svc, "get_duplicate_threshold", AsyncMock(return_value=0.8)),
|
||||
patch.object(dedup_svc, "async_session", side_effect=RuntimeError("boom")),
|
||||
):
|
||||
out = await dedup_svc.find_duplicate_snippets(1)
|
||||
assert out == {"groups": [], "pairs": [], "threshold": 0.8}
|
||||
@@ -51,7 +51,7 @@ async def test_merge_records_what_it_absorbed_in_body_and_mirror():
|
||||
_snippet(id=1), [_snippet(id=2), _snippet(id=3)], [2, 3],
|
||||
)
|
||||
assert "**Merged from:** #2, #3" in kwargs["body"]
|
||||
assert kwargs["data"]["merged_from"] == [2, 3]
|
||||
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2, 3]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -60,7 +60,7 @@ async def test_merge_accumulates_across_merges():
|
||||
replace the record of the first."""
|
||||
target = _snippet(id=1, data=s.compose_data(name="f", merged_from=[2]))
|
||||
kwargs = await _run_merge(target, [_snippet(id=3)], [3])
|
||||
assert kwargs["data"]["merged_from"] == [2, 3]
|
||||
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2, 3]
|
||||
assert "**Merged from:** #2, #3" in kwargs["body"]
|
||||
|
||||
|
||||
@@ -74,7 +74,7 @@ async def test_merge_does_not_record_a_skipped_cross_owner_source():
|
||||
[_snippet(id=2, owner=9), _snippet(id=3, owner=42)],
|
||||
[2, 3],
|
||||
)
|
||||
assert kwargs["data"]["merged_from"] == [2]
|
||||
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2]
|
||||
assert "#3" not in kwargs["body"]
|
||||
|
||||
|
||||
@@ -93,7 +93,7 @@ async def test_an_ordinary_edit_carries_provenance_forward():
|
||||
await s.update_snippet(7, 1, signature="f(ms) -> string")
|
||||
|
||||
kwargs = mock_update.await_args.kwargs
|
||||
assert kwargs["data"]["merged_from"] == [2, 3]
|
||||
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2, 3]
|
||||
assert "**Merged from:** #2, #3" in kwargs["body"]
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
"""Tests for un-merge (#2165) — reversing one source out of a merged survivor.
|
||||
|
||||
The design question this task said to settle first was how to subtract without
|
||||
stripping call sites the survivor legitimately owns. The answer is per-source
|
||||
attribution recorded AT MERGE TIME: each `merged_from` entry holds only what
|
||||
that source actually added. These tests pin that rule, and the refusal that
|
||||
guards the case where the attribution isn't there.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import snippets as s
|
||||
|
||||
|
||||
def _loc(path, repo="r", symbol=""):
|
||||
return {"repo": repo, "path": path, "symbol": symbol}
|
||||
|
||||
|
||||
def _survivor(locations, merged_from, tags=None, owner=7):
|
||||
"""A survivor note whose `data` carries locations + merge provenance."""
|
||||
return SimpleNamespace(
|
||||
id=1, user_id=owner, note_type="snippet", deleted_at=None,
|
||||
title="f — does a thing", tags=tags or ["python", "snippet"],
|
||||
body=s.compose_body(code="x = 1", language="python", locations=locations,
|
||||
merged_from=merged_from),
|
||||
data=s.compose_data(name="f", when_to_use="does a thing", language="python",
|
||||
code="x = 1", locations=locations,
|
||||
merged_from=merged_from),
|
||||
)
|
||||
|
||||
|
||||
async def _run_unmerge(survivor, *, source_alive=None, restore=1):
|
||||
"""Drive unmerge_snippet with the DB stubbed; return update_note's kwargs."""
|
||||
async def fake_get(_uid, sid):
|
||||
if sid == survivor.id:
|
||||
return survivor
|
||||
return source_alive
|
||||
|
||||
with (
|
||||
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
|
||||
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
||||
patch("scribe.services.trash.restore_entity", AsyncMock(return_value=restore)),
|
||||
patch.object(s.notes_svc, "update_note",
|
||||
AsyncMock(return_value=survivor)) as upd,
|
||||
patch.object(s, "_embed_snippet", MagicMock()),
|
||||
):
|
||||
await s.unmerge_snippet(7, 1, 2)
|
||||
return upd.await_args.kwargs
|
||||
|
||||
|
||||
# --- the subtraction ------------------------------------------------------
|
||||
|
||||
|
||||
async def test_unmerge_strips_only_what_the_source_contributed():
|
||||
"""The survivor's own location survives; the source's is removed."""
|
||||
mine, theirs = _loc("mine.py"), _loc("theirs.py")
|
||||
survivor = _survivor(
|
||||
[mine, theirs],
|
||||
[{"id": 2, "locations": [theirs], "tags": ["helper"]}],
|
||||
tags=["python", "snippet", "helper", "core"],
|
||||
)
|
||||
kwargs = await _run_unmerge(survivor)
|
||||
assert kwargs["data"]["locations"] == [mine]
|
||||
assert "helper" not in kwargs["tags"]
|
||||
assert "core" in kwargs["tags"]
|
||||
|
||||
|
||||
async def test_a_location_the_survivor_also_owned_is_never_stripped():
|
||||
"""The central hazard the task named. If a source brought a location the
|
||||
survivor ALREADY had, merge attributes nothing to it — so reversing must
|
||||
leave that call site in place."""
|
||||
shared = _loc("shared.py")
|
||||
survivor = _survivor([shared], [{"id": 2, "locations": [], "tags": ["t"]}])
|
||||
kwargs = await _run_unmerge(survivor)
|
||||
assert kwargs["data"]["locations"] == [shared]
|
||||
|
||||
|
||||
async def test_the_reversed_entry_leaves_the_provenance_list():
|
||||
survivor = _survivor(
|
||||
[_loc("a.py"), _loc("b.py")],
|
||||
[{"id": 2, "locations": [_loc("b.py")]}, {"id": 3, "locations": []}],
|
||||
)
|
||||
kwargs = await _run_unmerge(survivor)
|
||||
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [3]
|
||||
assert "#2" not in kwargs["body"]
|
||||
# The other source's history is untouched — un-merge reverses one thing.
|
||||
assert "**Merged from:** #3" in kwargs["body"]
|
||||
|
||||
|
||||
async def test_unmerging_the_last_source_clears_the_provenance_line():
|
||||
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [], "tags": ["t"]}])
|
||||
kwargs = await _run_unmerge(survivor)
|
||||
assert "Merged from" not in kwargs["body"]
|
||||
|
||||
|
||||
# --- restoring the source -------------------------------------------------
|
||||
|
||||
|
||||
async def test_an_already_restored_source_is_repaired_not_refused():
|
||||
"""The scenario that motivated the feature: the operator restored the source
|
||||
from the trash by hand, so both records claim its call sites and nothing ever
|
||||
stripped the survivor's copy. Un-merge must fix that, not reject it."""
|
||||
theirs = _loc("theirs.py")
|
||||
survivor = _survivor([_loc("mine.py"), theirs],
|
||||
[{"id": 2, "locations": [theirs]}])
|
||||
alive = SimpleNamespace(id=2, user_id=7, note_type="snippet", deleted_at=None,
|
||||
title="g — x", tags=["snippet"], body="", data=None)
|
||||
with patch("scribe.services.trash.restore_entity", AsyncMock()) as revive:
|
||||
kwargs = await _run_unmerge(survivor, source_alive=alive)
|
||||
# Nothing to revive — it's already alive — but the subtraction still happens.
|
||||
revive.assert_not_called()
|
||||
assert kwargs["data"]["locations"] == [_loc("mine.py")]
|
||||
|
||||
|
||||
async def test_a_purged_source_is_refused_and_the_survivor_is_untouched():
|
||||
"""If the source can't come back, stripping the survivor would lose the
|
||||
locations entirely — no record would claim them."""
|
||||
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [_loc("a.py")]}])
|
||||
|
||||
async def fake_get(_uid, sid):
|
||||
return survivor if sid == 1 else None
|
||||
|
||||
with (
|
||||
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
|
||||
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
||||
patch("scribe.services.trash.restore_entity", AsyncMock(return_value=None)),
|
||||
patch.object(s.notes_svc, "update_note", AsyncMock()) as upd,
|
||||
patch.object(s, "_embed_snippet", MagicMock()),
|
||||
):
|
||||
with pytest.raises(s.UnmergeError, match="purged"):
|
||||
await s.unmerge_snippet(7, 1, 2)
|
||||
upd.assert_not_called()
|
||||
|
||||
|
||||
# --- refusals -------------------------------------------------------------
|
||||
|
||||
|
||||
async def test_an_entry_without_attribution_is_refused_not_guessed():
|
||||
"""Bare-id provenance comes from parsing the body, which can only hold ids.
|
||||
Subtracting a guess could strip call sites the survivor owns — so refuse and
|
||||
say what to do instead."""
|
||||
survivor = _survivor([_loc("a.py")], [2])
|
||||
|
||||
async def fake_get(_uid, sid):
|
||||
return survivor if sid == 1 else None
|
||||
|
||||
with (
|
||||
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
|
||||
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
||||
patch.object(s.notes_svc, "update_note", AsyncMock()) as upd,
|
||||
patch.object(s, "_embed_snippet", MagicMock()),
|
||||
):
|
||||
with pytest.raises(s.UnmergeError, match="provenance"):
|
||||
await s.unmerge_snippet(7, 1, 2)
|
||||
upd.assert_not_called()
|
||||
|
||||
|
||||
async def test_unmerging_something_never_absorbed_is_refused():
|
||||
survivor = _survivor([_loc("a.py")], [{"id": 99, "locations": []}])
|
||||
with (
|
||||
patch.object(s, "get_snippet", AsyncMock(return_value=survivor)),
|
||||
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
||||
):
|
||||
with pytest.raises(s.UnmergeError, match="no record of absorbing"):
|
||||
await s.unmerge_snippet(7, 1, 2)
|
||||
|
||||
|
||||
async def test_unmerge_requires_write_access():
|
||||
"""Same rule as merge: a read-only share can see the record, not rearrange it."""
|
||||
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": []}])
|
||||
with (
|
||||
patch.object(s, "get_snippet", AsyncMock(return_value=survivor)),
|
||||
patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)),
|
||||
):
|
||||
with pytest.raises(PermissionError):
|
||||
await s.unmerge_snippet(7, 1, 2)
|
||||
|
||||
|
||||
async def test_unmerge_on_a_missing_survivor_returns_none():
|
||||
with patch.object(s, "get_snippet", AsyncMock(return_value=None)):
|
||||
assert await s.unmerge_snippet(7, 1, 2) is None
|
||||
Reference in New Issue
Block a user