diff --git a/lib/graph/apply-proposal.js b/lib/graph/apply-proposal.js index 0ad7b78..fcfe266 100644 --- a/lib/graph/apply-proposal.js +++ b/lib/graph/apply-proposal.js @@ -1387,6 +1387,12 @@ function describeObservationFocus(context, which) { return which === "first" ? "the first observation" : "the second observation"; } +function parentSupportsComparisonDecomposition(parentText) { + return /\b(compare|comparison|comparable|comparability|different timing|timing|measured|measurement|basis|scale|same period|timing or measurement basis|two observations)\b/.test( + parentText, + ); +} + function buildDecompositionTemplates(parentNode, graph, depth = 0) { const parentText = normaliseText( `${parentNode?.label || ""} ${parentNode?.description || ""}`, @@ -1468,7 +1474,7 @@ function buildDecompositionTemplates(parentNode, graph, depth = 0) { ]; } - if (/\btiming or measurement basis\b/i.test(parentNode.label)) { + if (parentSupportsComparisonDecomposition(parentText)) { return [ { label: "Whether the two observations reflect different timing", @@ -1481,38 +1487,27 @@ function buildDecompositionTemplates(parentNode, graph, depth = 0) { ]; } - return [ - { - label: "Whether the two observations reflect different timing", - description: `Need to know whether the two observations reflect different timing, because that could help explain ${context.centralStatement}.`, - }, - { - label: "How the two observations were measured", - description: `Need evidence about the measure used for each observation, because that could help explain ${context.centralStatement}.`, - }, - { - label: `Possible change mainly affecting ${firstFocus}`, - description: `Need to know whether a possible change mainly affected ${firstFocus}, because that could help explain ${context.centralStatement}.`, - }, - { - label: `Possible change mainly affecting ${secondFocus}`, - description: `Need to know whether a possible change mainly affected ${secondFocus}, because that could help explain ${context.centralStatement}.`, - }, - depth === 0 - ? { - label: "Possible one-off event during the period", - description: `Need to know whether a possible one-off event happened during the period, because that could help explain ${context.centralStatement}.`, - } - : { - label: "Mix shift during the period", - description: `Need to know whether the mix of cases, customers, or items shifted during the period, because that could help explain ${context.centralStatement}.`, - }, - ]; + return []; } function buildCompositeUnknownChildren(parentNode, graph, depth = 0) { const templates = buildDecompositionTemplates(parentNode, graph, depth); + if (templates.length === 0) { + return { + accepted: false, + childNodes: [], + childEdges: [], + childNodeIds: [], + proposedChildCount: 0, + acceptedChildCount: 0, + rejectedChildren: [], + childQualitySummary: [], + reason: + "Decomposition stopped because no meaning-preserving child family was justified for this parent.", + }; + } + const labelToId = new Map( templates.map((template) => [ template.label, diff --git a/tests/graph/apply-proposal.test.js b/tests/graph/apply-proposal.test.js index 0b22293..5aa0d2d 100644 --- a/tests/graph/apply-proposal.test.js +++ b/tests/graph/apply-proposal.test.js @@ -278,6 +278,70 @@ function makeRiskClarificationFixture() { return { graph, riskUnknownId: riskUnknown.id }; } +function makeGenericExplanationFixture() { + const explanationUnknown = makeNode({ + id: "n-generic-explanation", + label: + "Explanation for why Should I relocate my engineering team from London to Manchester", + description: + "Need to understand what change or event could explain why these observations differ, because that is needed to investigate their relationship.", + kind: "unknown", + status: "unknown", + confidence: "medium", + }); + + const observationA = makeNode({ + id: "n-london-team-state", + label: "Engineering team is currently operational in London.", + description: "Engineering team is currently operational in London.", + kind: "observation", + status: "supported", + confidence: "high", + }); + + const observationB = makeNode({ + id: "n-manchester-relocation-eval", + label: + "Relocation to Manchester is actively being evaluated by the decision-maker.", + description: + "Relocation to Manchester is actively being evaluated by the decision-maker.", + kind: "observation", + status: "supported", + confidence: "high", + }); + + const graph = makeGraph({ + centralStatement: + "Should I relocate my engineering team from London to Manchester?", + nodes: [explanationUnknown, observationA, observationB], + edges: [ + makeEdge({ + id: "e-london-explanation", + fromNodeId: observationA.id, + toNodeId: explanationUnknown.id, + relationship: "supports", + confidence: "medium", + description: + "The current London operating state contributes to the broad explanation unknown.", + }), + makeEdge({ + id: "e-manchester-explanation", + fromNodeId: observationB.id, + toNodeId: explanationUnknown.id, + relationship: "supports", + confidence: "medium", + description: + "The Manchester relocation evaluation contributes to the broad explanation unknown.", + }), + ], + activeUnknownNodeId: explanationUnknown.id, + resolvedNodeIds: [], + currentSummary: "Generic explanation fixture", + }); + + return { graph, explanationUnknownId: explanationUnknown.id }; +} + function makeMeaningfulNoOpProposal() { return { addedNodes: [ @@ -2114,86 +2178,38 @@ describe("applyValidatedProposal", () => { ).toHaveLength(1); }); - it("decomposes a composite selected unknown before asking the next question", () => { - const { graph, proposal } = makeComparabilityUpdateFixture(); + it("does not decompose a generic explanation unknown into unsupported comparison children", () => { + const { graph, explanationUnknownId } = makeGenericExplanationFixture(); const result = 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.", + proposal: makeMeaningfulNoOpProposal(), }); expect(result.success).toBe(true); expect(result.atomicityAssessment).toBe("composite"); - expect(result.decompositionPerformed).toBe(true); - expect(result.childUnknownCount).toBe(5); - expect(result.childNodeIds).toHaveLength(5); - expect(result.atomicityReason).toBeTruthy(); - expect(result.selectedQuestion?.nodeId).toBe(result.newActiveUnknownNodeId); - expect(result.selectedQuestion?.nodeId).not.toBe( - result.emergentReasoningNodeId, - ); - expect(result.selectedQuestion?.question.toLowerCase()).not.toMatch( - /dso|working capital|receivables|capex/, - ); - - const parentNode = result.updatedSituationGraph.nodes.find( - (node) => node.id === result.emergentReasoningNodeId, - ); - expect(parentNode?.status).toBe("unknown"); - - const childNodes = result.updatedSituationGraph.nodes.filter((node) => - result.childNodeIds.includes(node.id), - ); - expect(childNodes).toHaveLength(5); - expect(childNodes.every((node) => node.parentId === parentNode.id)).toBe( - true, - ); + expect(result.decompositionPerformed).toBe(false); + expect(result.childUnknownCount).toBe(0); + expect(result.childNodeIds).toHaveLength(0); expect( - result.updatedSituationGraph.edges.filter( - (edge) => - result.childNodeIds.includes(edge.fromNodeId) && - edge.toNodeId === parentNode.id && - edge.relationship === "depends_on", + result.updatedSituationGraph.nodes.some( + (node) => node.label === "How the two observations were measured", ), - ).toHaveLength(5); - }); - - it("reuses existing decomposition children instead of duplicating them", () => { - 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); - - const secondResult = 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(secondResult.success).toBe(true); - expect(secondResult.atomicityAssessment).toBe("composite"); - const uniqueChildIds = new Set(firstResult.childNodeIds); - expect(uniqueChildIds.size).toBe(firstResult.childNodeIds.length); + ).toBe(false); expect( - secondResult.updatedSituationGraph.nodes.filter((node) => - firstResult.childNodeIds.includes(node.id), + result.updatedSituationGraph.nodes.some( + (node) => + node.label === + "Whether the two observations reflect different timing", ), - ).toHaveLength(firstResult.childNodeIds.length); + ).toBe(false); + expect(result.selectedQuestion?.question || "").not.toContain( + "two observations", + ); + expect(result.decompositionStoppedReason).toBe( + "Decomposition stopped because no meaning-preserving child family was justified for this parent.", + ); + expect(result.selectedUnknownBefore).toBe(explanationUnknownId); }); it("reselects a remaining commercial sibling after resolving the first child", () => { diff --git a/tests/graph/decomposition-quality.test.js b/tests/graph/decomposition-quality.test.js index 32eb45b..93604d7 100644 --- a/tests/graph/decomposition-quality.test.js +++ b/tests/graph/decomposition-quality.test.js @@ -225,7 +225,7 @@ describe("decomposition stopping conditions", () => { ); }); - it("stops once a directly answerable child is selected", () => { + it("does not manufacture comparison children for a generic explanatory parent", () => { const { parent, graph } = makeParentGraph({ centralStatement: "Traffic increased, but sales stayed flat.", parentLabel: @@ -259,15 +259,82 @@ describe("decomposition stopping conditions", () => { expect(result.success).toBe(true); expect(result.decompositionAttempted).toBe(true); - expect(result.decompositionAccepted).toBe(true); - expect(result.selectedQuestion).toMatchObject({ - nodeId: expect.any(String), - question: - "What evidence would clarify how the two observations were measured?", - }); - expect(result.selectedChildNodeId).toBe(result.selectedQuestion?.nodeId); + expect(result.decompositionAccepted).toBe(false); + expect(result.childUnknownCount).toBe(0); + expect( + result.updatedSituationGraph.nodes.some( + (node) => node.parentId === parent.id, + ), + ).toBe(false); + expect( + result.updatedSituationGraph.nodes.some( + (node) => node.label === "How the two observations were measured", + ), + ).toBe(false); + expect( + result.updatedSituationGraph.nodes.some( + (node) => + node.label === + "Whether the two observations reflect different timing", + ), + ).toBe(false); expect(result.decompositionStoppedReason).toBe( - "Selected child is atomic and directly answerable.", + "Decomposition stopped because no meaning-preserving child family was justified for this parent.", + ); + expect(result.selectedQuestion?.question || "").not.toContain( + "two observations", + ); + }); + + it("still decomposes a genuine comparison/measurement parent into valid comparison children", () => { + const { graph } = makeParentGraph({ + centralStatement: + "Revenue increased by 18%, but cash in the bank fell over the same period.", + parentLabel: "Timing or measurement basis", + parentDescription: + "Need evidence about whether a timing or measurement-basis difference could explain the observations, because that would change how they should be interpreted.", + observations: [ + makeNode({ + id: "n-revenue", + label: "Revenue increased by 18%.", + description: "Revenue increased by 18%.", + kind: "observation", + status: "supported", + confidence: "high", + }), + makeNode({ + id: "n-cash", + label: "Cash in the bank decreased over the same period.", + description: "Cash in the bank decreased over the same period.", + kind: "observation", + status: "supported", + confidence: "high", + }), + ], + }); + + const result = applyValidatedProposal({ + situationGraph: graph, + proposal: makeMeaningfulNoOpProposal(), + }); + + expect(result.success).toBe(true); + expect(result.decompositionPerformed).toBe(true); + expect(result.childUnknownCount).toBe(2); + expect( + result.updatedSituationGraph.nodes.some( + (node) => node.label === "How the two observations were measured", + ), + ).toBe(true); + expect( + result.updatedSituationGraph.nodes.some( + (node) => + node.label === + "Whether the two observations reflect different timing", + ), + ).toBe(true); + expect(result.selectedQuestion?.question).toBe( + "What evidence would clarify how the two observations were measured?", ); });