diff --git a/lib/graph/utils.js b/lib/graph/utils.js index 7aebb83..0cc91a7 100644 --- a/lib/graph/utils.js +++ b/lib/graph/utils.js @@ -900,24 +900,28 @@ export function validateGraphUpdate(graph, update) { errors.push("structuralActionRequired is false but proposal contains meaningful mutations"); } - // Legacy no-op guard: only fires when structuralActionRequired !== false. - // When false → zero-mutation is a valid intentional no-op (contract PASS). - if (!hasMeaningfulChange && update.structuralActionRequired !== false) { + // Legacy no-op guard: only fires when structuralActionRequired is absent (null/undefined). + // When true or false → the new contract owns no-op/mutation consistency. + // The contract checks above already produced authoritative errors for those cases. + + const fieldAbsent = + update.structuralActionRequired === null || + update.structuralActionRequired === undefined; + + if (!hasMeaningfulChange && fieldAbsent) { if (meaningPopulated) { - // Avoid double-error when field absence was already flagged above - if ( - update.structuralActionRequired !== null && - update.structuralActionRequired !== undefined - ) { - errors.push( - "answerMeaning.userSupportedMeaning is populated, but the proposal contains no graph mutation. answerMeaning alone does not constitute graph progress.", - ); - } + // structuralActionRequired was missing while userSupportedMeaning exists. + // Missing-field rejection already added above; skip semantic-only guard to avoid duplicate errors on the same proposal. } else if (!meaningPopulated) { errors.push("Update contains no meaningful change"); } } + // Legacy guard when structuralActionRequired=false: false declares no action needed, + // but userSupportedMeaning populated implies semantic intent for change. However the + // new-contract checks above already produced an error if there IS mutation. If there's + // zero mutation with false + meaning, treat as intentional no-op (contract PASS). + // Reject oversized input const totalSize = JSON.stringify(update).length; if (totalSize > 100000) { diff --git a/tests/graph/utils.test.js b/tests/graph/utils.test.js index f453aad..f7954b9 100644 --- a/tests/graph/utils.test.js +++ b/tests/graph/utils.test.js @@ -1050,6 +1050,222 @@ describe("validateGraphUpdate", () => { }); }); +// ── Structural action contract v0.23 (57J.70) ──────── + +describe("structural-action-contract-v0.23", () => { + // Test 1 — true + zero mutation + populated meaning → exactly one structuralActionRequired error + it("true + zero mutation + populated meaning: exactly one contract error, no legacy duplicate", () => { + const graph = makeTestGraph(); + const update = { + addedNodes: [], + updatedNodes: [ + { nodeId: "n1", previousStatus: null, newStatus: null, previousValue: null, newValue: null, reason: "test" }, + ], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + answerMeaning: { userSupportedMeaning: "User is unsure about savings realism.", possibleInference: null }, + structuralActionRequired: true, + }; + + const result = validateGraphUpdate(graph, update); + + expect(result.valid).toBe(false); + expect(result.errors.length).toBe(1); + expect(result.errors[0]).toBe("structuralActionRequired is true but proposal contains no graph mutation"); + }); + + // Test 2 — false + zero mutation + populated meaning → pass (intentional no-op) + it("false + zero mutation + populated meaning: pass (intentional no-op)", () => { + const graph = makeTestGraph(); + const update = { + addedNodes: [], + updatedNodes: [ + { nodeId: "n1", previousStatus: null, newStatus: null, previousValue: null, newValue: null, reason: "test" }, + ], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + answerMeaning: { userSupportedMeaning: "No structural change needed.", possibleInference: null }, + structuralActionRequired: false, + }; + + const result = validateGraphUpdate(graph, update); + + expect(result.valid).toBe(true); + expect(result.errors.length).toBe(0); + }); + + // Test 3 — true + meaningful mutation → pass + it("true + meaningful mutation: pass", () => { + const graph = makeTestGraph(); + const update = { + addedNodes: [makeNode({ id: "n-new-unknown", label: "New unknown" })], + updatedNodes: [], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + answerMeaning: { userSupportedMeaning: "Evidence needed.", possibleInference: null }, + structuralActionRequired: true, + }; + + const result = validateGraphUpdate(graph, update); + + expect(result.valid).toBe(true); + expect(result.errors.length).toBe(0); + }); + + // Test 4 — false + meaningful mutation → exactly one contradiction error + it("false + meaningful mutation: exactly one contradiction error", () => { + const graph = makeTestGraph(); + const update = { + addedNodes: [makeNode({ id: "n-new-unknown2", label: "New unknown" })], + updatedNodes: [], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + answerMeaning: { userSupportedMeaning: "Evidence needed.", possibleInference: null }, + structuralActionRequired: false, + }; + + const result = validateGraphUpdate(graph, update); + + expect(result.valid).toBe(false); + expect(result.errors.length).toBe(1); + expect(result.errors[0]).toBe("structuralActionRequired is false but proposal contains meaningful mutations"); + }); + + // Test 5 — missing/null + populated meaning → existing specific missing-field rejection + it("null + populated meaning: existing transition rule rejection preserved", () => { + const graph = makeTestGraph(); + const update = { + addedNodes: [makeNode({ id: "n-new-unknown3", label: "New unknown" })], + updatedNodes: [], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + answerMeaning: { userSupportedMeaning: "Evidence needed.", possibleInference: null }, + }; + + const result = validateGraphUpdate(graph, update); + + expect(result.valid).toBe(false); + expect(result.errors.some((e) => e.includes("structuralActionRequired must be present"))).toBe(true); + }); + + // Test 6 — legacy no-meaning no-op preserved + it("null field + no meaning + zero mutation: existing legacy no-op rejection", () => { + const graph = makeTestGraph(); + const update = { + addedNodes: [], + updatedNodes: [ + { nodeId: "n1", previousStatus: null, newStatus: null, previousValue: null, newValue: null, reason: "test" }, + ], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + answerMeaning: null, + }; + + const result = validateGraphUpdate(graph, update); + + expect(result.valid).toBe(false); + expect(result.errors.some((e) => e === "Update contains no meaningful change")).toBe(true); + }); + + // Test 7 — hasMeaningfulChange definition unchanged + it("hasMeaningfulChange: null + no meaning + status change still counts as meaningful", () => { + const graph = makeTestGraph(); + const update = { + addedNodes: [], + updatedNodes: [ + { nodeId: "n4", previousStatus: "unknown", newStatus: "known", reason: "confirmed" }, + ], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + answerMeaning: null, + }; + + const result = validateGraphUpdate(graph, update); + + expect(result.valid).toBe(true); + expect(result.errors.length).toBe(0); + }); + + // Test 8 — no schema change required (import unchanged) + it("schema imports unchanged", () => { + // This test verifies structural integrity: the makeNode/makeEdge helpers used above + // come from the same schema module. If schema shape changed, these would fail to compile/resolve. + const graph = makeTestGraph(); + expect(graph.nodes.length).toBe(5); + }); + + // Test 9 — no prompt change required (validator-level fix only) + it("validator-level fix does not affect apply path", () => { + const graph = makeTestGraph(); + const newNode = makeNode({ id: "n-valid-apply", label: "Valid" }); + const updateResult = validateGraphUpdate(graph, { + addedNodes: [newNode], + updatedNodes: [], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + }); + + expect(updateResult.valid).toBe(true); + }); + + // Test 10 — no semantic keyword logic added (pure boolean + structural checks) + it("true/false gating: purely boolean with structural check, no semantics", () => { + const graph = makeTestGraph(); + + // true without mutation → error (structural only) + let result = validateGraphUpdate(graph, { + addedNodes: [], + updatedNodes: [{ nodeId: "n1", previousStatus: null, newStatus: null, previousValue: null, newValue: null, reason: "t" }], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + structuralActionRequired: true, + }); + expect(result.errors.length).toBe(1); + + // false with mutation → error (structural only) + result = validateGraphUpdate(graph, { + addedNodes: [makeNode({ id: "n-k", label: "New unknown" })], + updatedNodes: [], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + structuralActionRequired: false, + }); + expect(result.errors.length).toBe(1); + + // true with mutation → pass (structural only) + result = validateGraphUpdate(graph, { + addedNodes: [makeNode({ id: "n-l", label: "New unknown" })], + updatedNodes: [], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + structuralActionRequired: true, + }); + expect(result.valid).toBe(true); + + // false without mutation → pass (structural only) + result = validateGraphUpdate(graph, { + addedNodes: [], + updatedNodes: [{ nodeId: "n1", previousStatus: null, newStatus: null, previousValue: null, newValue: null, reason: "t" }], + addedEdges: [], + removedEdgeIds: [], + resolvedUnknownNodeIds: [], + structuralActionRequired: false, + }); + expect(result.valid).toBe(true); + }); +}); + // ── Integration: full update lifecycle ─────────────────── describe("update lifecycle integration", () => { @@ -1168,8 +1384,8 @@ describe("semantic-to-mutation contract", () => { affectedNodeIds: [], }; - // Test 1 — semantic-only no-op - it("REJECTS with specific semantic-only structural-progress error when userSupportedMeaning populated and zero structural mutation", () => { + // Test 1 — semantic-only no-op (v0.23: requires structuralActionRequired to be present) + it("REJECTS when userSupportedMeaning populated and zero structural mutation without structuralActionRequired", () => { const graph = makeTestGraph(); const update = { ...baseUpdate, @@ -1182,11 +1398,14 @@ describe("semantic-to-mutation contract", () => { const result = validateGraphUpdate(graph, update); expect(result.valid).toBe(false); + // Missing-field rejection fires first (transition rule) + expect( + result.errors.some((e) => e.includes("structuralActionRequired must be present")), + ).toBe(true); + // Semantic-only guard does NOT fire when field was absent (to avoid duplicate) expect( result.errors.some((e) => e.includes("userSupportedMeaning") && e.includes("mutation")), - ).toBe(true); - // Should NOT contain only the generic no-op message without the semantic-specific variant - expect(result.errors.some((e) => e === "Update contains no meaningful change")).toBe(false); + ).toBe(false); }); // Test 2 — ordinary no-op (answerMeaning null) @@ -1230,6 +1449,7 @@ describe("semantic-to-mutation contract", () => { const graph = makeTestGraph(); const update = { ...baseUpdate, + structuralActionRequired: true, answerMeaning: { userSupportedMeaning: "The user confirms risk is a hard constraint.", possibleInference: null, @@ -1255,6 +1475,7 @@ describe("semantic-to-mutation contract", () => { const graph = makeTestGraph(); const update = { ...baseUpdate, + structuralActionRequired: true, answerMeaning: { userSupportedMeaning: "The user provides criteria for acceptable opportunity.", possibleInference: null, @@ -1281,6 +1502,7 @@ describe("semantic-to-mutation contract", () => { const graph = makeTestGraph(); const update = { ...baseUpdate, + structuralActionRequired: true, answerMeaning: { userSupportedMeaning: "The user needs evidence for both savings realism and retention impact.", possibleInference: null, @@ -1300,6 +1522,7 @@ describe("semantic-to-mutation contract", () => { const existingNode = graph.nodes[0]; const update = { ...baseUpdate, + structuralActionRequired: true, answerMeaning: { userSupportedMeaning: "There is a new constraint the user identified.", possibleInference: null, @@ -1318,6 +1541,7 @@ describe("semantic-to-mutation contract", () => { const graph = makeTestGraph(); const update = { ...baseUpdate, + structuralActionRequired: true, answerMeaning: { userSupportedMeaning: "The user clarified the constraint is absolute.", possibleInference: null,