Merge pull request 'Bescherm excluded en locked topics tegen AI-deletion/merge' (#15) from fix/protect-excluded-locked-topics into main

Reviewed-on: #15

src/components/admin/KnowledgeGraph.jsx

@@ -5,6 +5,7 @@ import { callLLM } from '../../lib/llm';
 import { EMIT_GRAPH_ACTIONS_TOOL } from '../../lib/llmTools';
 import { Network, Table2 } from 'lucide-react';
 import { useGraphData } from '../../hooks/useGraphData';
+import { filterAiActions } from '../../lib/graphGuard';
 import GraphControls from './graph/GraphControls';
 import NodeDetailPanel from './graph/NodeDetailPanel';
 import GraphTable from './graph/GraphTable';
@@ -25,11 +26,21 @@ const SYSTEM_PROMPTS = {
   full: `You are a strict Data Quality AI maintaining a Knowledge Graph for Respellion.
 Evaluate the provided topics and relations and emit the actions to take via the emit_graph_actions tool.
 
+PROTECTED TOPICS — NEVER include these in merges (as deleteId) or in deletions:
+  • Topics with learning_relevance="exclude" are REFERENCE MATERIAL, kept on purpose
+    so R42 can still answer questions about them. They are intentionally outside
+    the theme-learning flow. Do not propose deleting or merging them away.
+  • Topics with relevance_locked=true are admin-pinned. Do not change their
+    learning_relevance and do not delete or merge them.
+These topics may appear as the keepId in a merge (others fold into them), and
+may appear as source/target of newRelations, but their own row must survive.
+
 Rules:
 1. Identify topics that mean exactly the same thing. Choose one to keep, one to delete (merges).
 2. Identify topics that are too vague, irrelevant, or malformed (deletions).
 3. Identify missing logical relations (depends_on, part_of, related_to, executed_by) between conceptually linked topics (newRelations).
 4. Evaluate learning_relevance. Mark purely operational topics (printer guides, etc.) as "exclude"; low-priority as "peripheral" (relevanceUpdates).
+   Skip topics where relevance_locked=true — omit them from relevanceUpdates entirely.
 
 Do not return the entire graph — only the actions to take.`,
 
@@ -73,6 +84,7 @@ const KnowledgeGraph = () => {
   const [showExcludeNodes, setShowExcludeNodes] = useState(false);
   const [isAnalyzing, setIsAnalyzing] = useState(false);
   const [analyzeError, setAnalyzeError] = useState(null);
+  const [analyzeNotice, setAnalyzeNotice] = useState(null); // info-level message from last analyze
   const [isRestoring, setIsRestoring] = useState(false);
   const [viewMode, setViewMode] = useState('graph'); // 'graph' | 'table'
 
@@ -305,6 +317,7 @@ const KnowledgeGraph = () => {
 
     setIsAnalyzing(true);
     setAnalyzeError(null);
+    setAnalyzeNotice(null);
 
     try {
       const [currentTopics, currentRelations] = await Promise.all([
@@ -314,13 +327,16 @@ const KnowledgeGraph = () => {
 
       const tier = scope === 'full' ? 'reasoning' : 'standard';
 
-      // For relevance scope, include relevance_locked so the model can skip those.
+      // For full + relevance scopes the model needs to see relevance_locked so
+      // it can honor the "do not touch protected topics" rule. (relations-scope
+      // only emits newRelations, so the flag is irrelevant there.)
+      const sendLocked = scope === 'full' || scope === 'relevance';
       const compactTopics = currentTopics.map(t => ({
         id: t.id,
         label: t.label,
         type: t.type,
         learning_relevance: t.learning_relevance,
-        ...(scope === 'relevance' && t.relevance_locked ? { relevance_locked: true } : {}),
+        ...(sendLocked && t.relevance_locked ? { relevance_locked: true } : {}),
       }));
       const compactRelations = currentRelations.map(({ source, target, type }) => ({
         source, target, type,
@@ -336,8 +352,26 @@ const KnowledgeGraph = () => {
         maxTokens: scope === 'full' ? 4096 : 2048,
       });
 
-      const actions = llmResult.toolUses[0]?.input;
-      if (!actions) throw new Error('Graph analysis did not emit a tool result.');
+      const rawActions = llmResult.toolUses[0]?.input;
+      if (!rawActions) throw new Error('Graph analysis did not emit a tool result.');
+
+      // ── Safety guard ─────────────────────────────────────────────────────
+      // Strip merges/deletions that target excluded or locked topics. Excluded
+      // topics are reference material kept on purpose for R42; locked topics
+      // are admin-pinned. The AI may suggest removing them anyway — we drop
+      // those suggestions before they reach bulkSave.
+      const { filtered: actions, dropped } = filterAiActions(currentTopics, rawActions);
+      const droppedCount = dropped.deletions.length + dropped.merges.length;
+      if (droppedCount > 0) {
+        // Visible feedback so the admin knows the AI tried to touch protected rows.
+        console.warn(
+          `[graph guard] Blocked ${droppedCount} destructive action(s) on protected topics`,
+          dropped,
+        );
+        setAnalyzeNotice(
+          `Guard blocked ${droppedCount} destructive AI action${droppedCount === 1 ? '' : 's'} on excluded or locked topics. ${dropped.deletions.length} deletion${dropped.deletions.length === 1 ? '' : 's'}, ${dropped.merges.length} merge${dropped.merges.length === 1 ? '' : 's'}.`,
+        );
+      }
 
       let updatedTopics = [...currentTopics];
       let updatedRelations = [...currentRelations];
@@ -460,9 +494,11 @@ const KnowledgeGraph = () => {
         <GraphControls
           showExcludeNodes={showExcludeNodes}
           onShowExcludeChange={setShowExcludeNodes}
+          excludedCount={topics.filter(t => t.learning_relevance === 'exclude').length}
           onAnalyze={analyzeGraph}
           isAnalyzing={isAnalyzing}
           analyzeError={analyzeError}
+          analyzeNotice={analyzeNotice}
           disabled={topics.length === 0}
           onApplied={reload}
           snapshotMeta={snapshotMeta}

src/components/admin/graph/GraphControls.jsx

@@ -1,5 +1,5 @@
 import { useState } from 'react';
-import { RotateCcw, RefreshCw, AlertCircle, ChevronDown } from 'lucide-react';
+import { RotateCcw, RefreshCw, AlertCircle, ChevronDown, ShieldCheck } from 'lucide-react';
 import Button from '../../ui/Button';
 import SuggestionsQueue from '../SuggestionsQueue';
 
@@ -24,9 +24,11 @@ const SCOPE_OPTIONS = [
 export default function GraphControls({
   showExcludeNodes,
   onShowExcludeChange,
+  excludedCount = 0,
   onAnalyze,
   isAnalyzing,
   analyzeError,
+  analyzeNotice,
   disabled,
   onApplied,
   snapshotMeta,
@@ -50,7 +52,14 @@ export default function GraphControls({
           onChange={e => onShowExcludeChange(e.target.checked)}
           className="rounded bg-bg-warm border-transparent focus:ring-0 text-teal"
         />
+        <span>
           Show Excluded Nodes (Reference Material)
+          {excludedCount > 0 && (
+            <span className="ml-1 text-xs text-fg-muted/80">
+              · {excludedCount} {showExcludeNodes ? 'visible' : 'hidden'}
+            </span>
+          )}
+        </span>
       </label>
 
       <div className="space-y-2">
@@ -106,6 +115,16 @@ export default function GraphControls({
             {analyzeError}
           </p>
         )}
+
+        {analyzeNotice && (
+          <p
+            className="text-xs text-teal flex items-start gap-1 bg-teal/5 border border-teal/20 rounded-[var(--r-sm)] p-2"
+            title="The AI suggested removing excluded/locked topics. Those suggestions were dropped client-side before saving."
+          >
+            <ShieldCheck size={14} className="shrink-0 mt-0.5" />
+            {analyzeNotice}
+          </p>
+        )}
       </div>
 
       <SuggestionsQueue onApplied={onApplied} />

src/lib/__tests__/graphGuard.test.js

@@ -0,0 +1,125 @@
+import { describe, expect, it } from 'vitest';
+import { isProtectedTopic, filterAiActions } from '../graphGuard';
+
+const topic = (id, extras = {}) => ({
+  id,
+  label: `Topic ${id}`,
+  type: 'concept',
+  learning_relevance: 'standard',
+  relevance_locked: false,
+  ...extras,
+});
+
+describe('isProtectedTopic', () => {
+  it('flags excluded topics', () => {
+    expect(isProtectedTopic(topic('a', { learning_relevance: 'exclude' }))).toBe(true);
+  });
+
+  it('flags locked topics', () => {
+    expect(isProtectedTopic(topic('a', { relevance_locked: true }))).toBe(true);
+  });
+
+  it('flags topics that are both excluded and locked', () => {
+    expect(
+      isProtectedTopic(topic('a', { learning_relevance: 'exclude', relevance_locked: true })),
+    ).toBe(true);
+  });
+
+  it('does not flag standard / core / peripheral topics', () => {
+    expect(isProtectedTopic(topic('a', { learning_relevance: 'core' }))).toBe(false);
+    expect(isProtectedTopic(topic('a', { learning_relevance: 'standard' }))).toBe(false);
+    expect(isProtectedTopic(topic('a', { learning_relevance: 'peripheral' }))).toBe(false);
+  });
+
+  it('treats null/undefined as not protected', () => {
+    expect(isProtectedTopic(null)).toBe(false);
+    expect(isProtectedTopic(undefined)).toBe(false);
+  });
+});
+
+describe('filterAiActions', () => {
+  const topics = [
+    topic('safe'),
+    topic('excluded', { learning_relevance: 'exclude' }),
+    topic('locked', { relevance_locked: true }),
+    topic('both', { learning_relevance: 'exclude', relevance_locked: true }),
+    topic('other'),
+  ];
+
+  it('drops deletions that target excluded topics', () => {
+    const { filtered, dropped } = filterAiActions(topics, {
+      deletions: ['safe', 'excluded', 'other'],
+      merges: [],
+    });
+    expect(filtered.deletions).toEqual(['safe', 'other']);
+    expect(dropped.deletions).toEqual(['excluded']);
+  });
+
+  it('drops deletions that target locked topics', () => {
+    const { filtered, dropped } = filterAiActions(topics, {
+      deletions: ['locked'],
+      merges: [],
+    });
+    expect(filtered.deletions).toEqual([]);
+    expect(dropped.deletions).toEqual(['locked']);
+  });
+
+  it('drops merges whose deleteId is protected', () => {
+    const merges = [
+      { keepId: 'safe',  deleteId: 'other'    },   // ok
+      { keepId: 'safe',  deleteId: 'excluded' },   // drop — excluded
+      { keepId: 'other', deleteId: 'locked'   },   // drop — locked
+      { keepId: 'safe',  deleteId: 'both'     },   // drop — both flags
+    ];
+    const { filtered, dropped } = filterAiActions(topics, { merges, deletions: [] });
+    expect(filtered.merges).toEqual([{ keepId: 'safe', deleteId: 'other' }]);
+    expect(dropped.merges).toHaveLength(3);
+    expect(dropped.merges.map(m => m.deleteId)).toEqual(['excluded', 'locked', 'both']);
+  });
+
+  it('keeps merges where a protected topic is the keepId (canonical survivor)', () => {
+    const merges = [
+      { keepId: 'excluded', deleteId: 'safe'  },   // ok — protected is survivor
+      { keepId: 'locked',   deleteId: 'other' },   // ok — protected is survivor
+    ];
+    const { filtered, dropped } = filterAiActions(topics, { merges, deletions: [] });
+    expect(filtered.merges).toEqual(merges);
+    expect(dropped.merges).toEqual([]);
+  });
+
+  it('passes through other action keys untouched', () => {
+    const actions = {
+      deletions: [],
+      merges: [],
+      newRelations: [{ source: 'a', target: 'b', type: 'related_to' }],
+      relevanceUpdates: [{ id: 'safe', learning_relevance: 'peripheral' }],
+    };
+    const { filtered } = filterAiActions(topics, actions);
+    expect(filtered.newRelations).toEqual(actions.newRelations);
+    expect(filtered.relevanceUpdates).toEqual(actions.relevanceUpdates);
+  });
+
+  it('tolerates missing actions keys', () => {
+    const { filtered, dropped } = filterAiActions(topics, {});
+    expect(filtered.deletions).toEqual([]);
+    expect(filtered.merges).toEqual([]);
+    expect(dropped.deletions).toEqual([]);
+    expect(dropped.merges).toEqual([]);
+  });
+
+  it('tolerates null actions', () => {
+    const { filtered, dropped } = filterAiActions(topics, null);
+    expect(filtered.deletions).toEqual([]);
+    expect(filtered.merges).toEqual([]);
+    expect(dropped.deletions).toEqual([]);
+    expect(dropped.merges).toEqual([]);
+  });
+
+  it('drops references to unknown ids by treating them as not protected (delete pass-through)', () => {
+    // If the AI hallucinates an id, the deletion is harmless downstream (no
+    // topic by that id exists). We do NOT block on unknown ids — that would
+    // mask real bugs. Verify pass-through behavior.
+    const { filtered } = filterAiActions(topics, { deletions: ['ghost-id'], merges: [] });
+    expect(filtered.deletions).toEqual(['ghost-id']);
+  });
+});

src/lib/graphGuard.js

@@ -0,0 +1,81 @@
+/**
+ * Knowledge-graph safety guards.
+ *
+ * The "Full Analysis" pass lets the LLM propose `merges` (collapse two topics
+ * into one) and `deletions` (drop a topic entirely). Both are destructive and
+ * persisted via bulkSave → db.saveTopics, which wipes-and-rewrites the
+ * `topics` collection.
+ *
+ * Two classes of topic must NEVER be removed by an AI suggestion:
+ *
+ *   1. `learning_relevance === 'exclude'`
+ *        Reference material — kept on purpose, surfaced only to R42 search,
+ *        intentionally excluded from theme topics. An admin made this choice;
+ *        the AI's "irrelevant" judgement must not override it.
+ *
+ *   2. `relevance_locked === true`
+ *        Admin pinned this row's relevance. By extension the row itself is
+ *        also pinned — deleting it would be a louder change than re-scoring it.
+ *
+ * `filterAiActions` strips any merge/deletion that would touch a protected id
+ * BEFORE it reaches bulkSave. The model still gets to suggest them (we can't
+ * stop it generating), but those suggestions are dropped client-side.
+ *
+ * The function is intentionally pure so it is trivially unit-testable.
+ */
+
+/**
+ * @param {{ learning_relevance?: string, relevance_locked?: boolean }} topic
+ */
+export function isProtectedTopic(topic) {
+  if (!topic) return false;
+  if (topic.learning_relevance === 'exclude') return true;
+  if (topic.relevance_locked === true) return true;
+  return false;
+}
+
+/**
+ * Strip protected topics out of `actions.deletions` and `actions.merges`.
+ *
+ * Merges have two ids:
+ *   - `keepId`    — the survivor
+ *   - `deleteId`  — gets removed and its relations re-pointed
+ *
+ * If `deleteId` is protected we drop the entire merge (we will not delete a
+ * protected topic, even to consolidate it). If `keepId` is protected we keep
+ * the merge — folding others into a protected canonical topic is fine.
+ *
+ * @param {object[]} currentTopics
+ * @param {{ merges?: {keepId: string, deleteId: string}[], deletions?: string[], newRelations?: object[], relevanceUpdates?: object[] }} actions
+ * @returns {{ filtered: object, dropped: { deletions: string[], merges: object[] } }}
+ */
+export function filterAiActions(currentTopics, actions) {
+  const byId = new Map(currentTopics.map(t => [t.id, t]));
+  const isProtected = (id) => isProtectedTopic(byId.get(id));
+
+  const droppedDeletions = [];
+  const keptDeletions = [];
+  for (const id of actions?.deletions ?? []) {
+    if (isProtected(id)) droppedDeletions.push(id);
+    else keptDeletions.push(id);
+  }
+
+  const droppedMerges = [];
+  const keptMerges = [];
+  for (const m of actions?.merges ?? []) {
+    if (isProtected(m?.deleteId)) droppedMerges.push(m);
+    else keptMerges.push(m);
+  }
+
+  return {
+    filtered: {
+      ...(actions || {}),
+      deletions: keptDeletions,
+      merges: keptMerges,
+    },
+    dropped: {
+      deletions: droppedDeletions,
+      merges: droppedMerges,
+    },
+  };
+}