diff --git a/lib/graph/apply-proposal.js b/lib/graph/apply-proposal.js index 65529df..f64356e 100644 --- a/lib/graph/apply-proposal.js +++ b/lib/graph/apply-proposal.js @@ -1728,6 +1728,52 @@ function selectedQuestionBelongsToChild(graph, selectedQuestion) { return Boolean(findNodeById(graph, selectedQuestion.nodeId)?.parentId); } +function normaliseQuestionText(question) { + return normaliseText(String(question || "").replace(/\?/g, " ")); +} + +function semanticNodeSignature(node) { + return normaliseText(`${node?.label || ""} ${node?.description || ""}`); +} + +function isStructurallyRepeatedQuestion({ + previousQuestion, + previousNode, + nextQuestion, + nextNode, + nextQuestionFamily, + nextInvestigationStrategy, +}) { + if (!previousQuestion || !nextQuestion || !nextNode) { + return false; + } + + const sameQuestion = + normaliseQuestionText(previousQuestion) === + normaliseQuestionText(nextQuestion); + const previousSignature = previousNode + ? semanticNodeSignature(previousNode) + : null; + const nextSignature = semanticNodeSignature(nextNode); + const sameSemanticTarget = previousSignature === nextSignature; + const sharedParent = + previousNode?.parentId && + nextNode?.parentId && + previousNode.parentId === nextNode.parentId; + const sameQuestionFamily = Boolean(previousNode && nextQuestionFamily); + const samePurpose = Boolean(nextQuestionFamily || nextInvestigationStrategy); + + return ( + sameQuestion && + samePurpose && + (sameSemanticTarget || sharedParent || sameQuestionFamily) + ); +} + +function buildRepeatedQuestionDiagnostics(nextNode) { + return `Rejected repeated follow-up for "${nextNode?.label || nextNode?.id || "unknown"}" because the previous answer did not justify asking the same structural question again while other eligible investigations may remain.`; +} + const ALLOWED_NODE_PATTERNS_BY_ACTIVE_PATTERN = { decision: ["decision", "definition"], explanation: ["explanation", "comparison", "definition"], @@ -2169,6 +2215,29 @@ function resolveAmbiguousGraphBackedSelection({ return null; } +function reseatSelectionAfterQuestionRejection({ + graph, + deterministicSelection, + activePattern = null, + excludedNodeIds = [], +}) { + const nextSelection = activePattern + ? selectPatternCompatibleUnknownCandidate({ + graph, + resolvedNodeIds: graph.resolvedNodeIds || [], + activePattern, + excludedNodeIds, + }) + : selectActiveUnknownCandidate(graph, [ + ...(graph.resolvedNodeIds || []), + ...excludedNodeIds, + ]); + + return nextSelection?.status + ? nextSelection + : { status: "none", nodeId: null }; +} + export function determineGraphBackedQuestion({ situationGraph }) { const graphSnapshot = cloneJsonSafe(situationGraph); let updatedSituationGraph = cloneJsonSafe(situationGraph); @@ -2226,6 +2295,26 @@ export function determineGraphBackedQuestion({ situationGraph }) { deterministicSelection, }); + const initialQuestionRejected = + questionResult.selectedQuestion?.question && + questionResult.selectedQuestion?.questionComplexity && + questionResult.selectedQuestion.questionComplexity.acceptable === false; + + if ( + initialQuestionRejected && + deterministicSelection?.status === "selected" + ) { + deterministicSelection = reseatSelectionAfterQuestionRejection({ + graph: updatedSituationGraph, + deterministicSelection, + excludedNodeIds: [deterministicSelection.nodeId], + }); + questionResult = buildSelectedQuestionResult({ + updatedSituationGraph, + deterministicSelection, + }); + } + if ( deterministicSelection?.status === "ambiguous" && !questionResult.selectedQuestion?.question @@ -2779,6 +2868,9 @@ export function applyValidatedProposal({ const graphSnapshot = cloneJsonSafe(situationGraph); const proposalSnapshot = cloneJsonSafe(validatedProposal); const previousActiveUnknownNodeId = graphSnapshot.activeUnknownNodeId ?? null; + const previousActiveUnknownNode = previousActiveUnknownNodeId + ? findNodeById(graphSnapshot, previousActiveUnknownNodeId) + : null; const affectedNodeIds = buildAffectedNodeIds(graphSnapshot, proposalSnapshot); const reasoningResolution = deriveReasoningStateOverride({ graph: graphSnapshot, @@ -3116,6 +3208,22 @@ export function applyValidatedProposal({ newActiveUnknownNodeId = deterministicSelection.nodeId; } else if (deterministicSelection?.status === "ambiguous") { newActiveUnknownNodeId = null; + } else if (eligibleCandidates.length > 0) { + const siblingFallbackNodeId = + nextSelectedSibling || eligibleCandidates[0]?.id || null; + if (siblingFallbackNodeId) { + deterministicSelection = { + status: "selected", + nodeId: siblingFallbackNodeId, + reason: + "Selected an eligible sibling after update-time validation left the original follow-up unavailable.", + }; + newActiveUnknownNodeId = siblingFallbackNodeId; + } else { + newActiveUnknownNodeId = null; + } + } else { + newActiveUnknownNodeId = null; } updatedSituationGraph.activeUnknownNodeId = newActiveUnknownNodeId; @@ -3177,9 +3285,111 @@ export function applyValidatedProposal({ } : null; - const finalSelectionCompatibility = finalSelectedQuestion?.nodeId + const repeatedQuestionRejected = Boolean( + finalSelectedQuestion?.question && + deterministicSelection?.status === "selected" && + isStructurallyRepeatedQuestion({ + previousQuestion, + previousNode: previousActiveUnknownNode, + nextQuestion: finalSelectedQuestion.question, + nextNode: selectedNode, + nextQuestionFamily: finalSelectedQuestion.questionFamily, + nextInvestigationStrategy: finalSelectedQuestion.strategy, + }), + ); + + if (repeatedQuestionRejected) { + const repeatedSelection = reseatSelectionAfterQuestionRejection({ + graph: updatedSituationGraph, + deterministicSelection, + activePattern: decompositionResult.activeReasoningPattern, + excludedNodeIds: [deterministicSelection.nodeId], + }); + + if ( + repeatedSelection?.status === "selected" && + repeatedSelection.nodeId !== deterministicSelection.nodeId + ) { + deterministicSelection = repeatedSelection; + } + } + + const selectedNodeAfterRepetitionCheck = + deterministicSelection?.status === "selected" && + deterministicSelection?.nodeId + ? updatedSituationGraph.nodes.find( + (node) => node.id === deterministicSelection.nodeId, + ) + : null; + const formulatedQuestionAfterRepetitionCheck = + selectedNodeAfterRepetitionCheck + ? formulateQuestion({ + node: selectedNodeAfterRepetitionCheck, + graph: updatedSituationGraph, + context: { + resolvedValues: validatedProposal.updatedNodes + .map((update) => update.newValue) + .filter( + (value) => typeof value === "string" && value.trim().length > 0, + ), + selectionState: deterministicSelection, + }, + }) + : null; + + const effectiveFormulatedQuestion = + formulatedQuestionAfterRepetitionCheck || formulatedQuestion; + const effectiveSelectedNode = + selectedNodeAfterRepetitionCheck || selectedNode; + const effectiveQuestionComplexity = + effectiveFormulatedQuestion?.questionComplexity ?? null; + const effectivePlainLanguageNormalisations = + effectiveFormulatedQuestion?.plainLanguageNormalisations ?? []; + + const effectiveSelectedQuestion = + deterministicSelection?.status === "ambiguous" + ? { + nodeId: null, + tiedCandidateIds: deterministicSelection.tiedCandidateIds, + question: null, + reason: deterministicSelection.reason, + } + : deterministicSelection?.status === "selected" + ? { + nodeId: deterministicSelection.nodeId, + question: + effectiveFormulatedQuestion?.question || + deterministicSelection.question, + reason: + repeatedQuestionRejected && + deterministicSelection?.nodeId !== previousActiveUnknownNodeId + ? buildRepeatedQuestionDiagnostics(effectiveSelectedNode) + : effectiveFormulatedQuestion?.reason || + deterministicSelection.reason, + strategy: effectiveFormulatedQuestion?.strategy, + investigationStrategy: + effectiveFormulatedQuestion?.investigationStrategy, + reasoningPattern: effectiveFormulatedQuestion?.reasoningPattern, + reasoningPatternReason: + effectiveFormulatedQuestion?.reasoningPatternReason, + questionFamily: effectiveFormulatedQuestion?.questionFamily, + allowedQuestionFamilies: + effectiveFormulatedQuestion?.allowedQuestionFamilies, + rejectedQuestionFamilies: + effectiveFormulatedQuestion?.rejectedQuestionFamilies, + selectedQuestionTemplate: + effectiveFormulatedQuestion?.selectedQuestionTemplate, + questionComplexity: effectiveQuestionComplexity, + plainLanguageNormalisations: effectivePlainLanguageNormalisations, + } + : null; + + const finalSelectionCompatibility = effectiveSelectedQuestion?.nodeId ? assessReasoningPatternCompatibility({ - node: findNodeById(updatedSituationGraph, finalSelectedQuestion.nodeId), + node: findNodeById( + updatedSituationGraph, + effectiveSelectedQuestion.nodeId, + ), graph: updatedSituationGraph, activePattern: decompositionResult.activeReasoningPattern, }) @@ -3189,11 +3399,11 @@ export function applyValidatedProposal({ selectedChildNodeId ?? (selectedQuestionBelongsToChild( updatedSituationGraph, - finalSelectedQuestion, + effectiveSelectedQuestion, ) - ? finalSelectedQuestion?.nodeId + ? effectiveSelectedQuestion?.nodeId : null); - const noQuestionReason = finalSelectedQuestion?.question + const noQuestionReason = effectiveSelectedQuestion?.question ? null : deterministicSelection?.status === "ambiguous" ? "Eligible unresolved candidates remain tied after update-time reselection." @@ -3221,7 +3431,8 @@ export function applyValidatedProposal({ !resultReferenceValidation?.valid || resultDuplicateNodeIds.length > 0 || resultDuplicateEdgeIds.length > 0 || - (finalSelectedQuestion?.nodeId && !finalSelectionCompatibility?.compatible) + (effectiveSelectedQuestion?.nodeId && + !finalSelectionCompatibility?.compatible) ) { return { success: false, @@ -3242,9 +3453,9 @@ export function applyValidatedProposal({ `Updated graph contains duplicate edge ID: "${edgeId}" (${count} occurrences)`, ), ...(!finalSelectionCompatibility?.compatible && - finalSelectedQuestion?.nodeId + effectiveSelectedQuestion?.nodeId ? [ - `Active unknown violates reasoning pattern consistency: "${finalSelectedQuestion.nodeId}" is ${finalSelectionCompatibility?.nodePattern} but active pattern is ${finalSelectionCompatibility?.activePattern}`, + `Active unknown violates reasoning pattern consistency: "${effectiveSelectedQuestion.nodeId}" is ${finalSelectionCompatibility?.nodePattern} but active pattern is ${finalSelectionCompatibility?.activePattern}`, ] : []), ], @@ -3279,22 +3490,23 @@ export function applyValidatedProposal({ childQualitySummary, selectedUnknownBefore: decompositionResult.selectedUnknownBefore, selectedUnknownAfter: deterministicSelection?.nodeId ?? null, - questionComplexityAccepted: questionComplexity?.acceptable ?? null, - primaryConceptCount: questionComplexity?.primaryConceptCount ?? null, - cognitiveLoad: questionComplexity?.cognitiveLoad ?? null, - complexityReasons: questionComplexity?.reasons ?? [], + questionComplexityAccepted: effectiveQuestionComplexity?.acceptable ?? null, + primaryConceptCount: + effectiveQuestionComplexity?.primaryConceptCount ?? null, + cognitiveLoad: effectiveQuestionComplexity?.cognitiveLoad ?? null, + complexityReasons: effectiveQuestionComplexity?.reasons ?? [], decompositionTriggeredByQuestionComplexity: decompositionResult.decompositionTriggeredByQuestionComplexity ?? false, decompositionTriggeredByAnswerability: decompositionResult.decompositionTriggeredByAnswerability ?? false, previousQuestion, - finalQuestion: finalSelectedQuestion?.question ?? null, + finalQuestion: effectiveSelectedQuestion?.question ?? null, unresolvedCandidateCount: unresolvedCandidates.length, eligibleCandidateCount: eligibleCandidates.length, candidateNodeIds: eligibleCandidates.map((node) => node.id), resolvedCurrentTurnNodeIds, noQuestionReason, - plainLanguageNormalisations, + plainLanguageNormalisations: effectivePlainLanguageNormalisations, propagationPerformed, resolvedChildNodeId, parentNodeId, @@ -3362,7 +3574,7 @@ export function applyValidatedProposal({ null, previousActiveUnknownNodeId, newActiveUnknownNodeId, - selectedQuestion: finalSelectedQuestion, + selectedQuestion: effectiveSelectedQuestion, changesApplied: buildChangesApplied(proposalSnapshot, affectedNodeIds), graphReferenceValidation: resultReferenceValidation, previousReasoningState: reasoningResolution.previousReasoningState, diff --git a/lib/graph/utils.js b/lib/graph/utils.js index 134c24d..e84a2bd 100644 --- a/lib/graph/utils.js +++ b/lib/graph/utils.js @@ -870,7 +870,7 @@ export function validateGraphUpdate(graph, update) { (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 ?? null), ); const hasMeaningfulChange = diff --git a/tests/graph/apply-proposal.test.js b/tests/graph/apply-proposal.test.js index acddddb..8a00c5c 100644 --- a/tests/graph/apply-proposal.test.js +++ b/tests/graph/apply-proposal.test.js @@ -1443,6 +1443,159 @@ describe("applyValidatedProposal", () => { expect(secondResult.compatibilityFailures).toEqual([]); }); + it("reselects a non-repeated comparison sibling instead of asking the same evidence question again", () => { + const { graph, proposal } = makeComparabilityUpdateFixture(); + + const firstResult = applyValidatedProposal({ + situationGraph: graph, + proposal, + previousQuestion: + "Were these figures measured on the same basis and at the same scale?", + answer: + "Yes. Both figures cover the same accounting period and are taken from the same management accounts.", + }); + + expect(firstResult.success).toBe(true); + expect(firstResult.selectedQuestion?.question).toBe( + "What evidence would clarify how the two observations were measured?", + ); + + const secondResult = applyValidatedProposal({ + situationGraph: firstResult.updatedSituationGraph, + proposal: { + addedNodes: [], + updatedNodes: [ + { + nodeId: firstResult.selectedQuestion.nodeId, + previousStatus: "unknown", + newStatus: "resolved", + previousValue: null, + newValue: + "Both measures come from the same monthly reporting pack and use the same source system.", + reason: "The answer resolves the measurement clarification child.", + }, + ], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [firstResult.selectedQuestion.nodeId], + affectedNodeIds: [], + selectedQuestion: null, + }, + previousQuestion: firstResult.selectedQuestion.question, + answer: + "Both measures come from the same monthly reporting pack and use the same source system.", + }); + + expect(secondResult.success).toBe(true); + expect(secondResult.selectedQuestion?.nodeId).not.toBe( + firstResult.selectedQuestion.nodeId, + ); + expect(secondResult.selectedQuestion?.question).not.toBe( + firstResult.selectedQuestion.question, + ); + expect(secondResult.finalQuestion).not.toBe( + firstResult.selectedQuestion.question, + ); + expect(secondResult.noQuestionReason).toBeNull(); + expect(secondResult.selectedQuestion?.nodeId).toBe( + secondResult.newActiveUnknownNodeId, + ); + }); + + it("rejects a compound selected question before returning it", () => { + const graph = makeCommercialUpdateFixture(); + + const result = applyValidatedProposal({ + situationGraph: graph, + proposal: { + ...makeMeaningfulNoOpProposal(), + selectedQuestion: { + nodeId: "n-commercial-parent", + question: + "What changed during the period that could explain why work is taking longer, and how were the two observations measured?", + reason: "Invalid compound follow-up.", + }, + }, + }); + + expect(result.success).toBe(false); + expect(result.stage).toBe("proposal_compatibility"); + expect(result.errors).toContain( + "selectedQuestion must be a single non-compound question", + ); + }); + + it("allows a legitimate single-concept or-question", () => { + const graph = makeCommercialUpdateFixture(); + + const result = applyValidatedProposal({ + situationGraph: graph, + proposal: { + ...makeMeaningfulNoOpProposal(), + selectedQuestion: { + nodeId: "n-commercial-parent", + question: "Is the problem caused by timing or measurement basis?", + reason: "Single concept contrast.", + }, + }, + }); + + expect(result.success).toBe(true); + }); + + it("returns no question when the graph is truly complete after resolution", () => { + const graph = makeGraph({ + centralStatement: "A single missing fact needs confirmation.", + nodes: [ + makeNode({ + id: "n-only-unknown", + label: "Missing fact", + description: + "Need the missing fact because the conclusion depends on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }), + ], + edges: [], + activeUnknownNodeId: "n-only-unknown", + resolvedNodeIds: [], + currentSummary: "Single unknown fixture", + }); + + const result = applyValidatedProposal({ + situationGraph: graph, + proposal: { + addedNodes: [], + updatedNodes: [ + { + nodeId: "n-only-unknown", + previousStatus: "unknown", + newStatus: "resolved", + previousValue: null, + newValue: "Confirmed.", + reason: "The answer resolves the only unknown.", + }, + ], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: ["n-only-unknown"], + affectedNodeIds: [], + selectedQuestion: null, + }, + previousQuestion: "What is the missing fact?", + answer: "Confirmed.", + }); + + expect(result.success).toBe(true); + expect(result.selectedQuestion).toBeNull(); + expect(result.finalQuestion).toBeNull(); + expect(result.newActiveUnknownNodeId).toBeNull(); + expect(result.noQuestionReason).toBe( + "No unresolved unknown candidates remain after this update.", + ); + }); + it("does not allow a decision-mode active unknown to remain a comparison child", () => { const graph = makeCommercialUpdateFixture(); graph.nodes.push( diff --git a/tests/graph/orchestrator.test.js b/tests/graph/orchestrator.test.js index af784fc..67361f2 100644 --- a/tests/graph/orchestrator.test.js +++ b/tests/graph/orchestrator.test.js @@ -1083,6 +1083,77 @@ describe("lib/graph/orchestrator startCase", () => { expect(result.selectedQuestion?.nodeId).toBe("n-value"); }); + it("keeps a follow-up question when a resolved child still has an eligible sibling", async () => { + const { updateCase } = await import("@/lib/graph/orchestrator.js"); + const initialGraph = makeCommercialUpdateGraph(); + const firstPass = applyValidatedProposal({ + situationGraph: initialGraph, + proposal: { + addedNodes: [ + makeNode({ + id: "n-anchor", + label: "Update anchor", + description: + "Anchor state introduced by the answer because the update must contain a meaningful change.", + kind: "state", + status: "known", + confidence: "low", + }), + ], + updatedNodes: [], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + affectedNodeIds: [], + selectedQuestion: null, + }, + }); + + expect(firstPass.success).toBe(true); + + const provider = { + generateReconstruction: vi.fn().mockResolvedValue({ + addedNodes: [], + updatedNodes: [ + { + nodeId: firstPass.selectedQuestion.nodeId, + previousStatus: "unknown", + newStatus: "resolved", + previousValue: null, + newValue: + "I experience it myself when deciding whether a project or investment is justified.", + reason: "The answer resolves the first child.", + }, + ], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [firstPass.selectedQuestion.nodeId], + affectedNodeIds: [], + selectedQuestion: null, + }), + }; + + const result = await updateCase( + { + situationGraph: firstPass.updatedSituationGraph, + previousQuestion: firstPass.selectedQuestion.question, + answer: + "I experience it myself when deciding whether a project or investment is justified.", + promptVersion: "v0.4", + }, + { + provider, + config: MOCK_CONFIG, + applyProposal: true, + }, + ); + + expect(result.success).toBe(true); + expect(result.selectedQuestion).toBeTruthy(); + expect(result.newActiveUnknownNodeId).toBe(result.selectedQuestion?.nodeId); + expect(result.diagnostics.noQuestionReason).toBeNull(); + }); + it("defaults to proposal-only mode", async () => { const { updateCase } = await import("@/lib/graph/orchestrator.js"); const applyValidatedProposal = vi.fn();