From 392564ed6168fdf051a23de3ef98bed132de2dff Mon Sep 17 00:00:00 2001 From: robbond Date: Sun, 2 Aug 2026 11:11:55 +0100 Subject: [PATCH] feat: prioritise follow-up questions by information value --- lib/graph/apply-proposal.js | 50 ++- lib/graph/prompt-builder.js | 16 +- lib/graph/utils.js | 329 +++++++++++++---- tests/graph/apply-proposal.test.js | 93 ++++- tests/graph/orchestrator.test.js | 85 ++++- tests/graph/prompt-builder.test.js | 3 + tests/graph/utils.test.js | 561 +++++++++++++++++++++-------- 7 files changed, 902 insertions(+), 235 deletions(-) diff --git a/lib/graph/apply-proposal.js b/lib/graph/apply-proposal.js index 4be4c14..1d0a233 100644 --- a/lib/graph/apply-proposal.js +++ b/lib/graph/apply-proposal.js @@ -2,8 +2,10 @@ import { describeGraph } from "./builder.js"; import { graphUpdateSchema, situationGraphSchema } from "./schema.js"; import { applyGraphUpdate, + buildDeterministicQuestionForUnknown, detectDuplicateNodeIds, findAffectedNodes, + scoreUnknownCandidate, selectActiveUnknownCandidate, validateGraphReferences, validateGraphUpdate, @@ -195,6 +197,20 @@ function validateSelectedQuestion(graph, proposal) { errors.push("selectedQuestion must be a single non-compound question"); } + const resolvedNodeIds = [ + ...(graph.resolvedNodeIds || []), + ...(proposal.resolvedUnknownNodeIds || []), + ]; + const candidateScore = scoreUnknownCandidate( + { + ...graph, + nodes: [...graph.nodes, ...(proposal.addedNodes || [])], + edges: [...graph.edges, ...(proposal.addedEdges || [])], + }, + node, + resolvedNodeIds, + ); + return { errors, selectedQuestionNodeId: selectedQuestion.nodeId }; } @@ -571,22 +587,32 @@ export function applyValidatedProposal({ situationGraph, proposal }) { )?.nodeId ?? null; } - if ( - validatedProposal.selectedQuestion?.nodeId && - newActiveUnknownNodeId !== validatedProposal.selectedQuestion.nodeId - ) { - return { - success: false, - stage: "proposal_compatibility", - errors: [ - `activeUnknownNodeId and selectedQuestion.nodeId disagree: "${newActiveUnknownNodeId}" vs "${validatedProposal.selectedQuestion.nodeId}"`, - ], - }; + const deterministicSelection = selectActiveUnknownCandidate( + updatedSituationGraph, + updatedSituationGraph.resolvedNodeIds, + ); + + if (deterministicSelection?.nodeId) { + newActiveUnknownNodeId = deterministicSelection.nodeId; } updatedSituationGraph.activeUnknownNodeId = newActiveUnknownNodeId; updatedSituationGraph.currentSummary = describeGraph(updatedSituationGraph); + const finalSelectedQuestion = deterministicSelection + ? { + nodeId: deterministicSelection.nodeId, + question: + deterministicSelection.question || + buildDeterministicQuestionForUnknown( + updatedSituationGraph.nodes.find( + (node) => node.id === deterministicSelection.nodeId, + ), + ), + reason: deterministicSelection.reason, + } + : null; + const resultGraphValidation = situationGraphSchema.safeParse( updatedSituationGraph, ); @@ -636,7 +662,7 @@ export function applyValidatedProposal({ situationGraph, proposal }) { resolvedUnknownNodeIds: validatedProposal.resolvedUnknownNodeIds, previousActiveUnknownNodeId, newActiveUnknownNodeId, - selectedQuestion: validatedProposal.selectedQuestion, + selectedQuestion: finalSelectedQuestion, changesApplied: buildChangesApplied(validatedProposal, affectedNodeIds), graphReferenceValidation: resultReferenceValidation, }; diff --git a/lib/graph/prompt-builder.js b/lib/graph/prompt-builder.js index 0dbccc7..1212567 100644 --- a/lib/graph/prompt-builder.js +++ b/lib/graph/prompt-builder.js @@ -101,15 +101,16 @@ The JSON object must contain exactly these top-level fields: 13a. For every new unknown node, include at least one added edge that connects it to an existing updated/resolved node or to a newly added non-unknown node introduced from the answer. 14. Do not invent evidence. 15. Do not create unsupported causal edges. -16. Select exactly one new active unknown in selectedQuestion when any consequential unresolved unknown exists. +16. If consequential unresolved unknowns exist, selectedQuestion may identify one valid candidate unknown, but the engine will deterministically choose final priority after validation. 17. selectedQuestion.nodeId must reference an unresolved unknown node that exists either already in the graph or in addedNodes. 18. selectedQuestion.question must be one narrow non-compound question about that one unknown. -19. Return selectedQuestion as null only when no consequential unresolved unknown remains. -20. Use empty arrays when there are no changes in a category. -21. Never return null array entries. -22. Never use unknown enum values. -23. Do not change existing IDs. -24. Do not replace the whole graph, and do not restate unchanged graph content inside the proposal. +19. Do not prioritise downstream implementation, pricing, optimisation, or speculative branches ahead of prerequisite definitions, actors, success criteria, constraints, measures, or terminology. +20. Return selectedQuestion as null only when no consequential unresolved unknown remains. +21. Use empty arrays when there are no changes in a category. +22. Never return null array entries. +23. Never use unknown enum values. +24. Do not change existing IDs. +25. Do not replace the whole graph, and do not restate unchanged graph content inside the proposal. ## Additional Guidance - If the answer only clarifies an existing unknown, prefer updatedNodes and resolvedUnknownNodeIds over creating duplicate nodes. @@ -117,6 +118,7 @@ The JSON object must contain exactly these top-level fields: - If the answer creates a more specific decision situation, add the smallest set of new nodes and edges needed to represent that situation and only its most consequential unknowns. - If you add a new unknown, do not leave it floating: connect it with an added edge to the relevant decision/context node created or updated from the answer. - If you add a new unknown, its description must do two jobs in one sentence: what is unknown, and why resolving it matters for the case. +- Treat selectedQuestion as a candidate only; the engine will apply deterministic information-value scoring after validation. - If the answer does not justify a change, return empty arrays for every category. ## Example Constraint Reminder diff --git a/lib/graph/utils.js b/lib/graph/utils.js index 2ee736d..c1fc6cc 100644 --- a/lib/graph/utils.js +++ b/lib/graph/utils.js @@ -5,26 +5,196 @@ * and these utilities apply them safely. */ -import { situationNodeSchema, situationEdgeSchema, situationGraphSchema } from "./schema.js"; +import { + situationNodeSchema, + situationEdgeSchema, + situationGraphSchema, +} from "./schema.js"; + +function normaliseText(value) { + return String(value || "") + .toLowerCase() + .replace(/[^a-z0-9]+/g, " ") + .trim(); +} + +function collectNodeText(node) { + return `${node?.label || ""} ${node?.description || ""}`.trim(); +} + +function countIncomingUnknownDependencies(graph, nodeId, resolvedNodeIds) { + const resolvedSet = new Set(resolvedNodeIds || []); + const nodesById = new Map(graph.nodes.map((node) => [node.id, node])); + const incoming = new Set(); + + for (const dependencyId of nodesById.get(nodeId)?.dependsOn || []) { + const dependencyNode = nodesById.get(dependencyId); + if (dependencyNode?.kind === "unknown" && !resolvedSet.has(dependencyId)) { + incoming.add(dependencyId); + } + } + + for (const edge of graph.edges) { + if (edge.toNodeId !== nodeId) continue; + const dependencyNode = nodesById.get(edge.fromNodeId); + if ( + dependencyNode?.kind === "unknown" && + !resolvedSet.has(edge.fromNodeId) + ) { + incoming.add(edge.fromNodeId); + } + } + + return incoming.size; +} + +function classifyUnknownPriority(text) { + const normalised = normaliseText(text); + + const matches = { + objective: + /\b(objective|goal|outcome|value|problem|job to be done|benefit|commercial value)\b/.test( + normalised, + ), + actor: + /\b(customer|user|buyer|actor|stakeholder|audience|recipient)\b/.test( + normalised, + ), + criteria: + /\b(success criteria|success threshold|threshold|decision criteria|criterion|justify|sufficient)\b/.test( + normalised, + ), + measure: + /\b(metric|measure|measurable|roi|demand|evidence|signal|proof)\b/.test( + normalised, + ), + terminology: /\b(define|definition|meaning|means|term|terminology)\b/.test( + normalised, + ), + constraint: + /\b(constraint|limit|budget|deadline|requirement|regulation)\b/.test( + normalised, + ), + pricing: /\b(price|pricing|price point|subscription|charge|pay for)\b/.test( + normalised, + ), + implementation: + /\b(implementation|build approach|architecture|stack|feature|technical design)\b/.test( + normalised, + ), + optimisation: + /\b(optimisation|optimi[sz]ation|improve|efficiency|performance|scale)\b/.test( + normalised, + ), + speculative: + /\b(maybe|possible|optional|future branch|nice to have|slogan|colour|color|ui)\b/.test( + normalised, + ), + }; + + return matches; +} + +export function scoreUnknownCandidate(graph, node, resolvedNodeIds = []) { + const text = collectNodeText(node); + const matches = classifyUnknownPriority(text); + const downstreamCount = findDependentNodes(graph, node.id).length; + const unresolvedParentUnknownCount = countIncomingUnknownDependencies( + graph, + node.id, + resolvedNodeIds, + ); + + let score = downstreamCount * 4; + + if (matches.objective) score += 12; + if (matches.actor) score += 10; + if (matches.criteria) score += 11; + if (matches.measure) score += 8; + if (matches.terminology) score += 7; + if (matches.constraint) score += 9; + + if (matches.pricing) score -= 8; + if (matches.implementation) score -= 10; + if (matches.optimisation) score -= 9; + if (matches.speculative) score -= 12; + + if ( + matches.pricing && + !matches.objective && + !matches.criteria && + !matches.actor + ) { + score -= 6; + } + + score -= unresolvedParentUnknownCount * 7; + + return { + nodeId: node.id, + label: node.label, + score, + downstreamCount, + unresolvedParentUnknownCount, + matches, + }; +} + +export function buildDeterministicQuestionForUnknown(node) { + const text = normaliseText(collectNodeText(node)); + + if ( + /\b(success criteria|success threshold|threshold|decision criteria|criterion)\b/.test( + text, + ) + ) { + return `What outcome would define success for ${node.label}?`; + } + if ( + /\b(customer|user|buyer|actor|stakeholder|audience|recipient)\b/.test(text) + ) { + return `Who is the key actor or customer for ${node.label}?`; + } + if ( + /\b(define|definition|meaning|means|term|terminology|value)\b/.test(text) + ) { + return `How should ${node.label} be defined for this decision?`; + } + if ( + /\b(metric|measure|measurable|roi|demand|evidence|signal|proof)\b/.test( + text, + ) + ) { + return `What evidence or measure would resolve ${node.label}?`; + } + + return `What would resolve ${node.label}?`; +} // ── Validate that all edge references point to existing nodes ── export function validateGraphReferences(graph) { const errors = []; const nodeIds = new Set(graph.nodes.map((n) => n.id)); - + for (const node of graph.nodes) { if (node.parentId !== null && !nodeIds.has(node.parentId)) { - errors.push(`Node "${node.id}" references parentId "${node.parentId}" which does not exist`); + errors.push( + `Node "${node.id}" references parentId "${node.parentId}" which does not exist`, + ); } for (const cid of node.childIds) { if (!nodeIds.has(cid)) { - errors.push(`Node "${node.id}" references childIds "${cid}" which does not exist`); + errors.push( + `Node "${node.id}" references childIds "${cid}" which does not exist`, + ); } } for (const dep of node.dependsOn) { if (!nodeIds.has(dep)) { - errors.push(`Node "${node.id}" depends on "${dep}" which does not exist`); + errors.push( + `Node "${node.id}" depends on "${dep}" which does not exist`, + ); } } for (const aff of node.affects) { @@ -33,16 +203,20 @@ export function validateGraphReferences(graph) { } } } - + for (const edge of graph.edges) { if (!nodeIds.has(edge.fromNodeId)) { - errors.push(`Edge "${edge.id}" references non-existent fromNodeId "${edge.fromNodeId}"`); + errors.push( + `Edge "${edge.id}" references non-existent fromNodeId "${edge.fromNodeId}"`, + ); } if (!nodeIds.has(edge.toNodeId)) { - errors.push(`Edge "${edge.id}" references non-existent toNodeId "${edge.toNodeId}"`); + errors.push( + `Edge "${edge.id}" references non-existent toNodeId "${edge.toNodeId}"`, + ); } } - + return { valid: errors.length === 0, errors }; } @@ -76,24 +250,31 @@ export function detectDuplicateNodeIds(nodes) { export function detectDuplicateEdges(edges) { const seen = new Set(); const duplicates = []; - + for (const edge of edges) { const key = `${edge.fromNodeId}->${edge.toNodeId}:${edge.relationship}`; if (seen.has(key)) { - duplicates.push({ edgeId: edge.id, fromNodeId: edge.fromNodeId, toNodeId: edge.toNodeId, relationship: edge.relationship }); + duplicates.push({ + edgeId: edge.id, + fromNodeId: edge.fromNodeId, + toNodeId: edge.toNodeId, + relationship: edge.relationship, + }); } seen.add(key); } - + return duplicates; } // ── Find all nodes that depend on a given node (transitive) ── export function findDependentNodes(graph, nodeId) { - const direct = graph.nodes.filter((n) => n.dependsOn.includes(nodeId)).map((n) => n.id); + const direct = graph.nodes + .filter((n) => n.dependsOn.includes(nodeId)) + .map((n) => n.id); const affected = new Set(direct); - + // Also propagate through edges where the relationship is depends_on for (const edge of graph.edges) { if (edge.toNodeId === nodeId && !affected.has(edge.fromNodeId)) { @@ -101,13 +282,13 @@ export function findDependentNodes(graph, nodeId) { affected.add(edge.fromNodeId); } } - + // Transitive propagation — BFS const queue = [...direct]; while (queue.length > 0) { const current = queue.shift(); if (!current || !affected.has(current)) continue; - + for (const node of graph.nodes) { if (node.dependsOn.includes(current) && !affected.has(node.id)) { affected.add(node.id); @@ -115,7 +296,7 @@ export function findDependentNodes(graph, nodeId) { } } } - + return [...affected]; } @@ -124,10 +305,14 @@ export function findDependentNodes(graph, nodeId) { export function findAffectedNodes(graph, nodeId) { // Direct effects: two sources // 1. Nodes that depend on this node (they list it in their dependsOn) - const directFromDepends = graph.nodes.filter((n) => n.id !== nodeId && n.dependsOn.includes(nodeId)).map((n) => n.id); + const directFromDepends = graph.nodes + .filter((n) => n.id !== nodeId && n.dependsOn.includes(nodeId)) + .map((n) => n.id); // 2. Targets of the node's affects relationships (this node directly affects them) - const myAffectedTargets = new Set(graph.nodes.find((n) => n.id === nodeId)?.affects || []); + const myAffectedTargets = new Set( + graph.nodes.find((n) => n.id === nodeId)?.affects || [], + ); // Merge: also add edge targets where this node is the source for (const edge of graph.edges) { @@ -147,7 +332,11 @@ export function findAffectedNodes(graph, nodeId) { if (!current || !affected.has(current)) continue; for (const node of graph.nodes) { - if (node.id !== nodeId && !affected.has(node.id) && (node.dependsOn.includes(current) || node.affects.includes(current))) { + if ( + node.id !== nodeId && + !affected.has(node.id) && + (node.dependsOn.includes(current) || node.affects.includes(current)) + ) { affected.add(node.id); queue.push(node.id); } @@ -164,10 +353,10 @@ export function resolveUnknownNode(graph, nodeId, newStatus, newValue, reason) { if (nodeIdx === -1) { return { success: false, error: `Node "${nodeId}" not found in graph` }; } - + const previousStatus = graph.nodes[nodeIdx].status; const previousValue = graph.nodes[nodeIdx].value; - + return { success: true, previousStatus, @@ -184,26 +373,37 @@ export function resolveUnknownNode(graph, nodeId, newStatus, newValue, reason) { export function selectActiveUnknownCandidate(graph, resolvedNodeIds) { // Skip already resolved nodes const unresolved = graph.nodes.filter( - (n) => n.kind === "unknown" && !resolvedNodeIds.includes(n.id) + (n) => n.kind === "unknown" && !resolvedNodeIds.includes(n.id), ); - + if (unresolved.length === 0) return null; - - // Prioritise: critical unknowns first, then those that are depended upon most - const dependencyCount = unresolved.map((n) => { - const deps = findDependentNodes(graph, n.id).length; - const importanceOrder = { critical: 3, important: 2, supporting: 1, incidental: 0 }; - const impScore = importanceOrder[n.confidence] || 0; - return { node: n, score: deps * 2 + impScore }; + + const scoredCandidates = unresolved.map((node) => ({ + node, + ...scoreUnknownCandidate(graph, node, resolvedNodeIds), + })); + + scoredCandidates.sort((a, b) => { + if (b.score !== a.score) return b.score - a.score; + if (b.downstreamCount !== a.downstreamCount) { + return b.downstreamCount - a.downstreamCount; + } + if (a.unresolvedParentUnknownCount !== b.unresolvedParentUnknownCount) { + return a.unresolvedParentUnknownCount - b.unresolvedParentUnknownCount; + } + return a.node.label.localeCompare(b.node.label); }); - - dependencyCount.sort((a, b) => b.score - a.score); - - // Return the highest-scoring unresolved unknown - const best = dependencyCount[0]; + + const best = scoredCandidates[0]; if (!best) return null; - - return { nodeId: best.node.id, label: best.node.label, score: best.score }; + + return { + nodeId: best.node.id, + label: best.node.label, + score: best.score, + question: buildDeterministicQuestionForUnknown(best.node), + reason: `Selected for highest information value (score ${best.score}) with ${best.downstreamCount} downstream dependency node(s) and ${best.unresolvedParentUnknownCount} unresolved prerequisite unknown(s).`, + }; } // ── Apply a graph update deterministically ── @@ -211,7 +411,7 @@ export function selectActiveUnknownCandidate(graph, resolvedNodeIds) { export function applyGraphUpdate(graph, update) { const errors = []; const updatedNodesMap = new Map(); - + // Validate that update references existing nodes or newly added ones const allNodeIds = new Set(graph.nodes.map((n) => n.id)); for (const added of update.addedNodes) { @@ -221,34 +421,38 @@ export function applyGraphUpdate(graph, update) { } allNodeIds.add(added.id); } - + // Validate updated nodes exist for (const upd of update.updatedNodes) { if (!allNodeIds.has(upd.nodeId)) { errors.push(`Cannot update non-existent node: "${upd.nodeId}"`); } } - + // Validate added edges reference existing or new nodes for (const edge of update.addedEdges) { if (!allNodeIds.has(edge.fromNodeId)) { - errors.push(`Added edge references non-existent fromNodeId: "${edge.fromNodeId}"`); + errors.push( + `Added edge references non-existent fromNodeId: "${edge.fromNodeId}"`, + ); } if (!allNodeIds.has(edge.toNodeId)) { - errors.push(`Added edge references non-existent toNodeId: "${edge.toNodeId}"`); + errors.push( + `Added edge references non-existent toNodeId: "${edge.toNodeId}"`, + ); } } - + if (errors.length > 0) return { success: false, errors }; - + // Build the new nodes list — start with a deep copy of existing const newNodes = graph.nodes.map((n) => ({ ...n })); - + // Apply updated nodes for (const upd of update.updatedNodes) { const idx = newNodes.findIndex((n) => n.id === upd.nodeId); if (idx === -1) continue; // already validated above - + if (upd.newStatus !== undefined && upd.newStatus !== null) { newNodes[idx].status = upd.newStatus; } @@ -257,22 +461,22 @@ export function applyGraphUpdate(graph, update) { } updatedNodesMap.set(upd.nodeId, newNodes[idx]); } - + // Add new nodes for (const newNode of update.addedNodes) { if (!allNodeIds.has(newNode.id)) continue; allNodeIds.add(newNode.id); newNodes.push({ ...newNode }); } - + // Remove edges if requested const removedEdgeSet = new Set(update.removedEdgeIds); const newEdges = graph.edges.filter((e) => !removedEdgeSet.has(e.id)); - + // Add new edges for (const newEdge of update.addedEdges) { newEdges.push({ ...newEdge }); - + // Update dependsOn / affects on the nodes const fromNode = newNodes.find((n) => n.id === newEdge.fromNodeId); const toNode = newNodes.find((n) => n.id === newEdge.toNodeId); @@ -283,10 +487,12 @@ export function applyGraphUpdate(graph, update) { toNode.dependsOn.push(newEdge.fromNodeId); } } - + // Add resolved node IDs - const newResolved = [...new Set([...graph.resolvedNodeIds, ...update.resolvedUnknownNodeIds])]; - + const newResolved = [ + ...new Set([...graph.resolvedNodeIds, ...update.resolvedUnknownNodeIds]), + ]; + return { success: true, nodes: newNodes, @@ -299,7 +505,7 @@ export function applyGraphUpdate(graph, update) { export function validateGraphUpdate(graph, update) { const errors = []; - + // Check for duplicate node IDs against existing and newly added nodes const extendedIds = new Set(graph.nodes.map((n) => n.id)); for (const newNode of update.addedNodes) { @@ -309,7 +515,7 @@ export function validateGraphUpdate(graph, update) { extendedIds.add(newNode.id); } } - + // Check updated nodes exist (in original graph, not newly added ones) const existingIds = new Set(graph.nodes.map((n) => n.id)); for (const upd of update.updatedNodes) { @@ -317,13 +523,13 @@ export function validateGraphUpdate(graph, update) { errors.push(`Cannot update non-existent node: "${upd.nodeId}"`); } } - + // Reject updates with no meaningful change const statusChanged = update.updatedNodes.some( - (u) => u.previousStatus !== null && u.newStatus !== u.previousStatus + (u) => u.previousStatus !== null && u.newStatus !== u.previousStatus, ); const valueChanged = update.updatedNodes.some( - (u) => u.previousValue !== null && u.newValue !== u.previousValue + (u) => u.previousValue !== null && u.newValue !== u.previousValue, ); const hasMeaningfulChange = @@ -336,13 +542,12 @@ export function validateGraphUpdate(graph, update) { if (!hasMeaningfulChange) { errors.push("Update contains no meaningful change"); } - + // Reject oversized input const totalSize = JSON.stringify(update).length; if (totalSize > 100000) { errors.push(`Proposed graph update exceeds 100KB (${totalSize} bytes)`); } - + return { valid: errors.length === 0, errors }; } - diff --git a/tests/graph/apply-proposal.test.js b/tests/graph/apply-proposal.test.js index f93c5d9..bc68e5f 100644 --- a/tests/graph/apply-proposal.test.js +++ b/tests/graph/apply-proposal.test.js @@ -546,7 +546,10 @@ describe("applyValidatedProposal", () => { ), ).toBe(true); expect(result.newActiveUnknownNodeId).toBe("n-commercial-value"); - expect(result.selectedQuestion).toEqual(proposal.selectedQuestion); + expect(result.selectedQuestion?.nodeId).toBe("n-commercial-value"); + expect(result.selectedQuestion?.question).toContain( + "Commercial value definition", + ); }); it("rejects more than 3 added unknowns", () => { @@ -699,4 +702,92 @@ describe("applyValidatedProposal", () => { expect(result.success).toBe(true); expect(result.newActiveUnknownNodeId).toBe(result.selectedQuestion?.nodeId); }); + + it("replaces downstream pricing question with higher-value commercial-value question", () => { + const { graph, ids } = makeApplicationFixture(); + + const proposal = { + addedNodes: [ + makeNode({ + id: "n-commercial-value", + label: "Commercial value definition", + description: + "Need commercial value definition because the decision depends on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }), + makeNode({ + id: "n-pricing", + label: "Target price point", + description: + "Need a price point because revenue assumptions depend on it.", + kind: "unknown", + status: "unknown", + confidence: "medium", + dependsOn: ["n-commercial-value"], + }), + makeNode({ + id: "n-build-decision", + label: "Build Confidence Engine decision", + description: "Decision introduced by the answer.", + kind: "state", + status: "supported", + confidence: "medium", + }), + ], + updatedNodes: [ + { + nodeId: ids.complaintRateUnknown, + previousStatus: "unknown", + newStatus: "resolved", + previousValue: null, + newValue: "Decision whether to build Confidence Engine", + reason: "The answer resolves the original context unknown.", + }, + ], + addedEdges: [ + makeEdge({ + id: "e-build-commercial-value", + fromNodeId: "n-build-decision", + toNodeId: "n-commercial-value", + relationship: "depends_on", + confidence: "medium", + description: "The decision depends on defining commercial value.", + }), + makeEdge({ + id: "e-commercial-value-pricing", + fromNodeId: "n-commercial-value", + toNodeId: "n-pricing", + relationship: "depends_on", + confidence: "medium", + description: "Pricing depends on commercial value definition.", + }), + makeEdge({ + id: "e-build-pricing", + fromNodeId: "n-build-decision", + toNodeId: "n-pricing", + relationship: "depends_on", + confidence: "low", + description: "The decision also references pricing assumptions.", + }), + ], + removedEdgeIds: [], + resolvedUnknownNodeIds: [ids.complaintRateUnknown], + affectedNodeIds: [], + selectedQuestion: { + nodeId: "n-pricing", + question: "What is the target price point?", + reason: "Model chose a downstream leaf.", + }, + }; + + const result = applyValidatedProposal({ situationGraph: graph, proposal }); + + expect(result.success).toBe(true); + expect(result.selectedQuestion?.nodeId).toBe("n-commercial-value"); + expect(result.selectedQuestion?.question.toLowerCase()).not.toContain( + "price", + ); + }); }); diff --git a/tests/graph/orchestrator.test.js b/tests/graph/orchestrator.test.js index 103b41e..6041289 100644 --- a/tests/graph/orchestrator.test.js +++ b/tests/graph/orchestrator.test.js @@ -582,14 +582,89 @@ describe("lib/graph/orchestrator startCase", () => { }); expect(result.success).toBe(true); - expect(result.selectedQuestion).toEqual({ - nodeId: "n-commercial-value", - question: "How should commercial value be defined for this decision?", - reason: "Consequential unresolved uncertainty remains.", - }); + expect(result.selectedQuestion?.nodeId).toBe("n-commercial-value"); expect(result.newActiveUnknownNodeId).toBe("n-commercial-value"); }); + it("deterministically prioritises customer value over pricing follow-up", async () => { + const { updateCase } = await import("@/lib/graph/orchestrator.js"); + const provider = { + generateReconstruction: vi.fn().mockResolvedValue( + makeProposal({ + addedNodes: [ + makeNode({ + id: "n-value", + label: "Customer value", + description: + "Need customer value because purchase decisions depend on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }), + makeNode({ + id: "n-price", + label: "Target price point", + description: + "Need a target price point because revenue assumptions depend on it.", + kind: "unknown", + status: "unknown", + confidence: "medium", + dependsOn: ["n-value"], + }), + makeNode({ + id: "n-decision", + label: "Build Confidence Engine decision", + description: "Decision introduced by the answer.", + kind: "state", + status: "supported", + confidence: "medium", + }), + ], + addedEdges: [ + { + id: "e-decision-value", + fromNodeId: "n-decision", + toNodeId: "n-value", + relationship: "depends_on", + confidence: "medium", + description: "The decision depends on customer value.", + }, + { + id: "e-value-price", + fromNodeId: "n-value", + toNodeId: "n-price", + relationship: "depends_on", + confidence: "medium", + description: "Pricing depends on customer value.", + }, + { + id: "e-decision-price", + fromNodeId: "n-decision", + toNodeId: "n-price", + relationship: "depends_on", + confidence: "low", + description: "The decision references pricing assumptions.", + }, + ], + selectedQuestion: { + nodeId: "n-price", + question: "What is the price point?", + reason: "Model chose pricing.", + }, + }), + ), + }; + + const result = await updateCase(makeUpdateRequest(), { + provider, + config: MOCK_CONFIG, + applyProposal: true, + }); + + expect(result.success).toBe(true); + expect(result.selectedQuestion?.nodeId).toBe("n-value"); + }); + it("defaults to proposal-only mode", async () => { const { updateCase } = await import("@/lib/graph/orchestrator.js"); const applyValidatedProposal = vi.fn(); diff --git a/tests/graph/prompt-builder.test.js b/tests/graph/prompt-builder.test.js index 5920663..8a8f9f1 100644 --- a/tests/graph/prompt-builder.test.js +++ b/tests/graph/prompt-builder.test.js @@ -107,5 +107,8 @@ describe("buildGraphUpdatePrompt", () => { expect(prompt).toContain( "selectedQuestion.question must be one narrow non-compound question", ); + expect(prompt).toContain( + "the engine will deterministically choose final priority after validation", + ); }); }); diff --git a/tests/graph/utils.test.js b/tests/graph/utils.test.js index 0deba4e..2e6a0b8 100644 --- a/tests/graph/utils.test.js +++ b/tests/graph/utils.test.js @@ -3,6 +3,7 @@ import { validateGraphReferences, detectDuplicateNodeIds, detectDuplicateEdges, + scoreUnknownCandidate, findDependentNodes, findAffectedNodes, resolveUnknownNode, @@ -24,12 +25,22 @@ function makeTestGraph() { // n2 depends on n1; n3 depends on n2 (transitive depends on n1) n2.dependsOn.push(n1.id); n3.dependsOn.push(n2.id); - + // n4 is an unknown not depended on // n5 is an unknown depended upon by n3 indirectly - const e1 = makeEdge({ id: "e1", fromNodeId: n1.id, toNodeId: n2.id, relationship: "depends_on" }); - const e2 = makeEdge({ id: "e2", fromNodeId: n3.id, toNodeId: n1.id, relationship: "supports" }); + const e1 = makeEdge({ + id: "e1", + fromNodeId: n1.id, + toNodeId: n2.id, + relationship: "depends_on", + }); + const e2 = makeEdge({ + id: "e2", + fromNodeId: n3.id, + toNodeId: n1.id, + relationship: "supports", + }); return makeGraph({ centralStatement: "Test graph", @@ -55,7 +66,9 @@ describe("validateGraphReferences", () => { graph.nodes[0].parentId = "nonexistent-parent"; const result = validateGraphReferences(graph); expect(result.valid).toBe(false); - expect(result.errors.some(e => e.includes("nonexistent-parent"))).toBe(true); + expect(result.errors.some((e) => e.includes("nonexistent-parent"))).toBe( + true, + ); }); it("detects invalid childIds reference", () => { @@ -84,7 +97,7 @@ describe("validateGraphReferences", () => { graph.edges[0].fromNodeId = "ghost-node"; const result = validateGraphReferences(graph); expect(result.valid).toBe(false); - expect(result.errors.some(e => e.includes("ghost-node"))).toBe(true); + expect(result.errors.some((e) => e.includes("ghost-node"))).toBe(true); }); it("detects edge referencing non-existent toNodeId", () => { @@ -98,7 +111,7 @@ describe("validateGraphReferences", () => { const graph = makeTestGraph(); graph.nodes[0].parentId = "missing"; graph.nodes[1].parentId = "also-missing"; - + const result = validateGraphReferences(graph); expect(result.valid).toBe(false); expect(result.errors.length).toBe(2); @@ -154,9 +167,19 @@ describe("detectDuplicateEdges", () => { it("detects duplicate edge (same from, to, relationship)", () => { const n1 = makeNode({ id: "n1", label: "A" }); const n2 = makeNode({ id: "n2", label: "B" }); - const e1 = makeEdge({ id: "e1", fromNodeId: n1.id, toNodeId: n2.id, relationship: "supports" }); - const e2 = makeEdge({ id: "e2", fromNodeId: n1.id, toNodeId: n2.id, relationship: "supports" }); - + const e1 = makeEdge({ + id: "e1", + fromNodeId: n1.id, + toNodeId: n2.id, + relationship: "supports", + }); + const e2 = makeEdge({ + id: "e2", + fromNodeId: n1.id, + toNodeId: n2.id, + relationship: "supports", + }); + const dups = detectDuplicateEdges([e1, e2]); expect(dups.length).toBe(1); }); @@ -164,9 +187,19 @@ describe("detectDuplicateEdges", () => { it("allows same nodes with different relationship types", () => { const n1 = makeNode({ id: "n1", label: "A" }); const n2 = makeNode({ id: "n2", label: "B" }); - const e1 = makeEdge({ id: "e1", fromNodeId: n1.id, toNodeId: n2.id, relationship: "supports" }); - const e2 = makeEdge({ id: "e2", fromNodeId: n1.id, toNodeId: n2.id, relationship: "weakens" }); - + const e1 = makeEdge({ + id: "e1", + fromNodeId: n1.id, + toNodeId: n2.id, + relationship: "supports", + }); + const e2 = makeEdge({ + id: "e2", + fromNodeId: n1.id, + toNodeId: n2.id, + relationship: "weakens", + }); + const dups = detectDuplicateEdges([e1, e2]); expect(dups.length).toBe(0); }); @@ -174,9 +207,19 @@ describe("detectDuplicateEdges", () => { it("detects reversed direction as different edge", () => { const n1 = makeNode({ id: "n1", label: "A" }); const n2 = makeNode({ id: "n2", label: "B" }); - const e1 = makeEdge({ id: "e1", fromNodeId: n1.id, toNodeId: n2.id, relationship: "supports" }); - const e2 = makeEdge({ id: "e2", fromNodeId: n2.id, toNodeId: n1.id, relationship: "supports" }); - + const e1 = makeEdge({ + id: "e1", + fromNodeId: n1.id, + toNodeId: n2.id, + relationship: "supports", + }); + const e2 = makeEdge({ + id: "e2", + fromNodeId: n2.id, + toNodeId: n1.id, + relationship: "supports", + }); + const dups = detectDuplicateEdges([e1, e2]); expect(dups.length).toBe(0); }); @@ -229,7 +272,7 @@ describe("findDependentNodes (transitive)", () => { for (let i = 2; i <= 10; i++) { nodes[i - 1].dependsOn.push(nodes[0].id); // All depend on n1 } - + const graph = makeGraph({ centralStatement: "Chain", nodes, @@ -270,7 +313,14 @@ describe("findAffectedNodes (transitive)", () => { it("handles empty graph", () => { // build a minimal graph without triggering schema validation for this edge case - const graph = { centralStatement: "Empty", nodes: [], edges: [], resolvedNodeIds: [], currentSummary: "", activeUnknownNodeId: null }; + const graph = { + centralStatement: "Empty", + nodes: [], + edges: [], + resolvedNodeIds: [], + currentSummary: "", + activeUnknownNodeId: null, + }; const affected = findAffectedNodes(graph, "any-node"); expect(affected.length).toBe(0); }); @@ -279,7 +329,13 @@ describe("findAffectedNodes (transitive)", () => { describe("resolveUnknownNode", () => { it("returns success for valid node id", () => { const graph = makeTestGraph(); - const result = resolveUnknownNode(graph, "n4", "resolved", "Confirmed", "User confirmed"); + const result = resolveUnknownNode( + graph, + "n4", + "resolved", + "Confirmed", + "User confirmed", + ); expect(result.success).toBe(true); expect(result.newStatus).toBe("resolved"); expect(result.reason).toBe("User confirmed"); @@ -287,7 +343,13 @@ describe("resolveUnknownNode", () => { it("returns error for non-existent node", () => { const graph = makeTestGraph(); - const result = resolveUnknownNode(graph, "ghost-node", "resolved", null, "reason"); + const result = resolveUnknownNode( + graph, + "ghost-node", + "resolved", + null, + "reason", + ); expect(result.success).toBe(false); expect(result.error).toContain("not found"); }); @@ -297,13 +359,25 @@ describe("resolveUnknownNode", () => { // n5 depends on... actually let's set up properly graph.nodes[3].affects.push("n1"); // Unknown depends on Actor A graph.nodes[3].dependsOn.push("n2"); // Unknown depends on State B - const result = resolveUnknownNode(graph, "n4", "resolved", "Yes", "Clarified"); + const result = resolveUnknownNode( + graph, + "n4", + "resolved", + "Yes", + "Clarified", + ); expect(result.success).toBe(true); }); it("tracks previous status and value", () => { const graph = makeTestGraph(); - const result = resolveUnknownNode(graph, "n4", "known", "confirmed_value", "Evidence found"); + const result = resolveUnknownNode( + graph, + "n4", + "known", + "confirmed_value", + "Evidence found", + ); expect(result.previousStatus).toBe("unknown"); expect(result.newValue).toBe("confirmed_value"); }); @@ -313,7 +387,11 @@ describe("selectActiveUnknownCandidate", () => { it("returns null when no unresolved unknowns", () => { // makeTestGraph nodes default to kind "observation", not "unknown" // Create explicit unknown-kind nodes for this test - const nUnknown = makeNode({ id: "n-unk-x", label: "Unknown X", kind: "unknown" }); + const nUnknown = makeNode({ + id: "n-unk-x", + label: "Unknown X", + kind: "unknown", + }); const graph = makeGraph({ centralStatement: "Test", nodes: [nUnknown], @@ -349,9 +427,21 @@ describe("selectActiveUnknownCandidate", () => { }); it("prioritises nodes with more dependents", () => { - const unknownA = makeNode({ id: "unknown-a", label: "Unknown A", kind: "unknown" }); - const unknownB = makeNode({ id: "unknown-b", label: "Unknown B", kind: "unknown" }); - const dependent = makeNode({ id: "dep", label: "Dependent", kind: "state" }); + const unknownA = makeNode({ + id: "unknown-a", + label: "Unknown A", + kind: "unknown", + }); + const unknownB = makeNode({ + id: "unknown-b", + label: "Unknown B", + kind: "unknown", + }); + const dependent = makeNode({ + id: "dep", + label: "Dependent", + kind: "state", + }); dependent.dependsOn.push("unknown-a"); @@ -370,7 +460,11 @@ describe("selectActiveUnknownCandidate", () => { it("returns one candidate (not array)", () => { const n1 = makeNode({ id: "n1", label: "A", kind: "observation" }); - const nUnknown = makeNode({ id: "n-unk", label: "Pending", kind: "unknown" }); + const nUnknown = makeNode({ + id: "n-unk", + label: "Pending", + kind: "unknown", + }); const graph = makeGraph({ centralStatement: "Test", nodes: [n1, nUnknown], @@ -386,13 +480,152 @@ describe("selectActiveUnknownCandidate", () => { expect(result.label).toBeDefined(); expect(result.score).toBeDefined(); }); + + it("commercial value wins over pricing", () => { + const commercialValue = makeNode({ + id: "n-commercial-value", + label: "Commercial value definition", + description: + "Need to define commercial value because the decision depends on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }); + const pricing = makeNode({ + id: "n-pricing", + label: "Target price point", + description: + "Need a target price point because revenue assumptions depend on it.", + kind: "unknown", + status: "unknown", + confidence: "medium", + dependsOn: ["n-commercial-value"], + }); + const decision = makeNode({ + id: "n-decision", + label: "Build decision", + description: "Decision context", + kind: "state", + status: "supported", + confidence: "medium", + dependsOn: ["n-commercial-value", "n-pricing"], + }); + + const graph = makeGraph({ + centralStatement: "Build decision", + nodes: [commercialValue, pricing, decision], + edges: [], + activeUnknownNodeId: pricing.id, + resolvedNodeIds: [], + currentSummary: "Test", + }); + + const result = selectActiveUnknownCandidate(graph, []); + expect(result.nodeId).toBe("n-commercial-value"); + }); + + it("customer value wins over UI colour", () => { + const customerValue = makeNode({ + id: "n-customer-value", + label: "Customer value", + description: + "Need to know customer value because adoption depends on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }); + const uiColour = makeNode({ + id: "n-ui-colour", + label: "UI colour", + description: "Need a UI colour because presentation choices remain open.", + kind: "unknown", + status: "unknown", + confidence: "low", + }); + const graph = makeGraph({ + centralStatement: "Value question", + nodes: [customerValue, uiColour], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "Test", + }); + + const result = selectActiveUnknownCandidate(graph, []); + expect(result.nodeId).toBe("n-customer-value"); + }); + + it("success criteria wins over marketing slogan", () => { + const successCriteria = makeNode({ + id: "n-success-criteria", + label: "Success criteria", + description: + "Need success criteria because the decision requires a threshold.", + kind: "unknown", + status: "unknown", + confidence: "high", + }); + const slogan = makeNode({ + id: "n-slogan", + label: "Marketing slogan", + description: "Need a slogan because messaging is undecided.", + kind: "unknown", + status: "unknown", + confidence: "low", + }); + const graph = makeGraph({ + centralStatement: "Threshold question", + nodes: [successCriteria, slogan], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "Test", + }); + + const result = selectActiveUnknownCandidate(graph, []); + expect(result.nodeId).toBe("n-success-criteria"); + }); + + it("penalises unknowns with unresolved parent unknowns", () => { + const parentUnknown = makeNode({ + id: "n-parent", + label: "Commercial value definition", + description: + "Need commercial value definition because the decision depends on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }); + const childUnknown = makeNode({ + id: "n-child", + label: "Target price point", + description: "Need price point because revenue assumptions depend on it.", + kind: "unknown", + status: "unknown", + confidence: "medium", + dependsOn: ["n-parent"], + }); + + const graph = makeGraph({ + centralStatement: "Dependency ordering", + nodes: [parentUnknown, childUnknown], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "Test", + }); + + const parentScore = scoreUnknownCandidate(graph, parentUnknown, []); + const childScore = scoreUnknownCandidate(graph, childUnknown, []); + expect(parentScore.score).toBeGreaterThan(childScore.score); + }); }); describe("applyGraphUpdate", () => { it("applies node additions correctly", () => { const graph = makeTestGraph(); const newNode = makeNode({ id: "n-new", label: "New Node" }); - + const update = { addedNodes: [newNode], updatedNodes: [], @@ -401,65 +634,69 @@ describe("applyGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(true); expect(result.nodes.length).toBe(graph.nodes.length + 1); - expect(result.nodes.some(n => n.id === "n-new")).toBe(true); + expect(result.nodes.some((n) => n.id === "n-new")).toBe(true); }); it("applies status updates correctly", () => { const graph = makeTestGraph(); - + const update = { addedNodes: [], - updatedNodes: [{ - nodeId: "n4", - previousStatus: "unknown", - newStatus: "resolved", - previousValue: null, - newValue: "confirmed", - reason: "Answered by user", - }], + updatedNodes: [ + { + nodeId: "n4", + previousStatus: "unknown", + newStatus: "resolved", + previousValue: null, + newValue: "confirmed", + reason: "Answered by user", + }, + ], addedEdges: [], removedEdgeIds: [], resolvedUnknownNodeIds: ["n4"], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(true); - - const updatedNode = result.nodes.find(n => n.id === "n4"); + + const updatedNode = result.nodes.find((n) => n.id === "n4"); expect(updatedNode.status).toBe("resolved"); }); it("rejects update with non-existent nodeId in updatedNodes", () => { const graph = makeTestGraph(); - + const update = { addedNodes: [], - updatedNodes: [{ - nodeId: "ghost-node", - previousStatus: null, - newStatus: "known", - reason: "test", - }], + updatedNodes: [ + { + nodeId: "ghost-node", + previousStatus: null, + newStatus: "known", + reason: "test", + }, + ], addedEdges: [], removedEdgeIds: [], resolvedUnknownNodeIds: [], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(false); - expect(result.errors.some(e => e.includes("ghost-node"))).toBe(true); + expect(result.errors.some((e) => e.includes("ghost-node"))).toBe(true); }); it("removes requested edges", () => { const graph = makeTestGraph(); const edgeIdToRemove = graph.edges[0].id; - + const update = { addedNodes: [], updatedNodes: [], @@ -468,17 +705,21 @@ describe("applyGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(true); expect(result.edges.length).toBe(graph.edges.length - 1); - expect(result.edges.some(e => e.id === edgeIdToRemove)).toBe(false); + expect(result.edges.some((e) => e.id === edgeIdToRemove)).toBe(false); }); it("adds edges and updates node dependsOn/affects", () => { const graph = makeTestGraph(); - const newEdge = makeEdge({ fromNodeId: "n1", toNodeId: "n4", relationship: "supports" }); - + const newEdge = makeEdge({ + fromNodeId: "n1", + toNodeId: "n4", + relationship: "supports", + }); + const update = { addedNodes: [], updatedNodes: [], @@ -487,23 +728,23 @@ describe("applyGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(true); - + // Check the edge was added - expect(result.edges.some(e => e.id === newEdge.id)).toBe(true); - + expect(result.edges.some((e) => e.id === newEdge.id)).toBe(true); + // Check node relationship arrays updated - const fromNode = result.nodes.find(n => n.id === "n1"); - const toNode = result.nodes.find(n => n.id === "n4"); + const fromNode = result.nodes.find((n) => n.id === "n1"); + const toNode = result.nodes.find((n) => n.id === "n4"); expect(fromNode.childIds).toContain("n4"); expect(toNode.dependsOn).toContain("n1"); }); it("accumulates resolved node IDs", () => { const graph = makeTestGraph(); - + const update = { addedNodes: [], updatedNodes: [], @@ -512,7 +753,7 @@ describe("applyGraphUpdate", () => { resolvedUnknownNodeIds: ["n4"], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(true); expect(result.resolvedNodeIds).toContain("n4"); @@ -538,23 +779,25 @@ describe("applyGraphUpdate", () => { it("rejects edges referencing non-existent nodes", () => { const graph = makeTestGraph(); - + const update = { addedNodes: [], updatedNodes: [], - addedEdges: [{ - id: "e-new", - fromNodeId: "missing-node", - toNodeId: "n1", - relationship: "supports", - confidence: "medium", - description: "bad edge", - }], + addedEdges: [ + { + id: "e-new", + fromNodeId: "missing-node", + toNodeId: "n1", + relationship: "supports", + confidence: "medium", + description: "bad edge", + }, + ], removedEdgeIds: [], resolvedUnknownNodeIds: [], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(false); }); @@ -562,7 +805,7 @@ describe("applyGraphUpdate", () => { it("preserves nodes not mentioned in the update", () => { const graph = makeTestGraph(); const unchangedCount = graph.nodes.length; - + const update = { addedNodes: [], updatedNodes: [], @@ -571,7 +814,7 @@ describe("applyGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(true); expect(result.nodes.length).toBe(unchangedCount); @@ -580,21 +823,23 @@ describe("applyGraphUpdate", () => { it("applies multiple operations in one update", () => { const graph = makeTestGraph(); const newNode = makeNode({ id: "n-multi", label: "Multi" }); - + const update = { addedNodes: [newNode], - updatedNodes: [{ - nodeId: "n4", - previousStatus: "unknown", - newStatus: "resolved", - reason: "Multiple ops test", - }], + updatedNodes: [ + { + nodeId: "n4", + previousStatus: "unknown", + newStatus: "resolved", + reason: "Multiple ops test", + }, + ], addedEdges: [makeEdge({ fromNodeId: "n-multi", toNodeId: "n1" })], removedEdgeIds: [graph.edges[0]?.id || ""], resolvedUnknownNodeIds: ["n4"], affectedNodeIds: [], }; - + const result = applyGraphUpdate(graph, update); expect(result.success).toBe(true); }); @@ -604,7 +849,7 @@ describe("validateGraphUpdate", () => { it("accepts a no-op update with added nodes", () => { const graph = makeTestGraph(); const newNode = makeNode({ id: "n-new", label: "New" }); - + const result = validateGraphUpdate(graph, { addedNodes: [newNode], updatedNodes: [], @@ -613,37 +858,39 @@ describe("validateGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }); - + expect(result.valid).toBe(true); }); it("rejects update with no meaningful change", () => { const graph = makeTestGraph(); - + const result = validateGraphUpdate(graph, { addedNodes: [], - updatedNodes: [{ - nodeId: "n1", - previousStatus: null, - newStatus: null, - previousValue: null, - newValue: null, - reason: "No change test", - }], + updatedNodes: [ + { + nodeId: "n1", + previousStatus: null, + newStatus: null, + previousValue: null, + newValue: null, + reason: "No change test", + }, + ], addedEdges: [], removedEdgeIds: [], resolvedUnknownNodeIds: [], affectedNodeIds: [], }); - + expect(result.valid).toBe(false); - expect(result.errors.some(e => e.includes("no meaningful"))).toBe(true); + expect(result.errors.some((e) => e.includes("no meaningful"))).toBe(true); }); it("rejects duplicate node IDs in additions", () => { const graph = makeTestGraph(); const existingNode = graph.nodes[0]; - + const result = validateGraphUpdate(graph, { addedNodes: [existingNode], // Duplicate ID updatedNodes: [], @@ -652,54 +899,58 @@ describe("validateGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }); - + expect(result.valid).toBe(false); }); it("rejects update to non-existent node", () => { const graph = makeTestGraph(); - + const result = validateGraphUpdate(graph, { addedNodes: [], - updatedNodes: [{ - nodeId: "ghost-node", - previousStatus: null, - newStatus: "known", - reason: "test", - }], + updatedNodes: [ + { + nodeId: "ghost-node", + previousStatus: null, + newStatus: "known", + reason: "test", + }, + ], addedEdges: [], removedEdgeIds: [], resolvedUnknownNodeIds: [], affectedNodeIds: [], }); - + expect(result.valid).toBe(false); }); it("accepts valid status change as meaningful", () => { const graph = makeTestGraph(); - + const result = validateGraphUpdate(graph, { addedNodes: [], - updatedNodes: [{ - nodeId: "n4", - previousStatus: "unknown", - newStatus: "known", - reason: "Confirmed", - }], + updatedNodes: [ + { + nodeId: "n4", + previousStatus: "unknown", + newStatus: "known", + reason: "Confirmed", + }, + ], addedEdges: [], removedEdgeIds: [], resolvedUnknownNodeIds: [], affectedNodeIds: [], }); - + expect(result.valid).toBe(true); }); it("rejects oversized update (>100KB)", () => { const graph = makeTestGraph(); const largeDescription = "x".repeat(150000); - + const result = validateGraphUpdate(graph, { addedNodes: [{ label: largeDescription }], // Will create huge JSON updatedNodes: [], @@ -708,14 +959,16 @@ describe("validateGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }); - + expect(result.valid).toBe(false); - expect(result.errors.some(e => e.includes("100KB") || e.includes("exceeds"))).toBe(true); + expect( + result.errors.some((e) => e.includes("100KB") || e.includes("exceeds")), + ).toBe(true); }); it("returns empty errors array for valid update", () => { const graph = makeTestGraph(); - + const result = validateGraphUpdate(graph, { addedNodes: [makeNode({ id: "n-valid", label: "Valid" })], updatedNodes: [], @@ -724,7 +977,7 @@ describe("validateGraphUpdate", () => { resolvedUnknownNodeIds: [], affectedNodeIds: [], }); - + expect(result.valid).toBe(true); expect(result.errors.length).toBe(0); }); @@ -735,47 +988,55 @@ describe("validateGraphUpdate", () => { describe("update lifecycle integration", () => { it("complete update cycle: validate → apply → verify", () => { const graph = makeTestGraph(); - + // Create a meaningful update const newNode = makeNode({ id: "n-new", label: "New Discovery" }); - const newEdge = makeEdge({ fromNodeId: "n1", toNodeId: "n-new", relationship: "supports" }); - + const newEdge = makeEdge({ + fromNodeId: "n1", + toNodeId: "n-new", + relationship: "supports", + }); + // Validate first const validationResult = validateGraphUpdate(graph, { addedNodes: [newNode], - updatedNodes: [{ - nodeId: "n4", - previousStatus: "unknown", - newStatus: "resolved", - reason: "Answered via follow-up question", - }], + updatedNodes: [ + { + nodeId: "n4", + previousStatus: "unknown", + newStatus: "resolved", + reason: "Answered via follow-up question", + }, + ], addedEdges: [newEdge], removedEdgeIds: [], resolvedUnknownNodeIds: ["n4"], affectedNodeIds: [], }); expect(validationResult.valid).toBe(true); - + // Apply const applyResult = applyGraphUpdate(graph, { addedNodes: [newNode], - updatedNodes: [{ - nodeId: "n4", - previousStatus: "unknown", - newStatus: "resolved", - reason: "Answered via follow-up question", - }], + updatedNodes: [ + { + nodeId: "n4", + previousStatus: "unknown", + newStatus: "resolved", + reason: "Answered via follow-up question", + }, + ], addedEdges: [newEdge], removedEdgeIds: [], resolvedUnknownNodeIds: ["n4"], affectedNodeIds: [], }); - + expect(applyResult.success).toBe(true); expect(applyResult.nodes.length).toBe(graph.nodes.length + 1); expect(applyResult.edges.length).toBe(graph.edges.length + 1); expect(applyResult.resolvedNodeIds).toContain("n4"); - + // Verify post-apply integrity const postValidation = validateGraphReferences(applyResult); expect(postValidation.valid).toBe(true); @@ -783,10 +1044,12 @@ describe("update lifecycle integration", () => { it("reject and retry: invalid update should be caught", () => { const graph = makeTestGraph(); - + const invalidUpdate = { addedNodes: [], - updatedNodes: [{ nodeId: "ghost-node", newStatus: "known", reason: "test" }], + updatedNodes: [ + { nodeId: "ghost-node", newStatus: "known", reason: "test" }, + ], addedEdges: [], removedEdgeIds: [], resolvedUnknownNodeIds: [], @@ -795,7 +1058,7 @@ describe("update lifecycle integration", () => { // Validation should catch it expect(validateGraphUpdate(graph, invalidUpdate).valid).toBe(false); - + // Apply should also catch it expect(applyGraphUpdate(graph, invalidUpdate).success).toBe(false); }); @@ -803,21 +1066,23 @@ describe("update lifecycle integration", () => { it("preserve unchanged nodes during update", () => { const graph = makeTestGraph(); const originalNode1 = JSON.parse(JSON.stringify(graph.nodes[0])); - + applyGraphUpdate(graph, { addedNodes: [], - updatedNodes: [{ - nodeId: "n4", - previousStatus: "unknown", - newStatus: "resolved", - reason: "Test preserve", - }], + updatedNodes: [ + { + nodeId: "n4", + previousStatus: "unknown", + newStatus: "resolved", + reason: "Test preserve", + }, + ], addedEdges: [], removedEdgeIds: [], resolvedUnknownNodeIds: ["n4"], affectedNodeIds: [], }); - + // Re-read the graph and check n1 wasn't modified expect(graph.nodes[0].id).toBe("n1"); expect(graph.nodes[0].status).toBe("unknown"); // unchanged