fix(graph): make structural action contract authoritative

2026-08-12 08:15:41 +01:00
parent c899ad620c
commit bd3c7d59ae
2 changed files with 245 additions and 17 deletions
@@ -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) {
@@ -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,