From fa42a2643a7d541e0df870217fee5f4c9796716b Mon Sep 17 00:00:00 2001
From: robbond <rob@rdbsolutions.co.uk>
Date: Wed, 12 Aug 2026 15:02:56 +0100
Subject: [PATCH] fix(graph): preserve grammar for question-like unknown labels

---
 lib/graph/question-formulator.js              | 116 +++++++-
 .../graph/question-formulation-v0.24.test.js  | 281 ++++++++++++++++++
 2 files changed, 387 insertions(+), 10 deletions(-)
 create mode 100644 tests/graph/question-formulation-v0.24.test.js

diff --git a/lib/graph/question-formulator.js b/lib/graph/question-formulator.js
index 7d337d9..3816100 100644
--- a/lib/graph/question-formulator.js
+++ b/lib/graph/question-formulator.js
@@ -115,12 +115,72 @@ function sanitizeQuestionText(question) {
     .trim();
 }
 
+/**
+ * Detect whether a meaning string is already an interrogative (wh-question,
+ * yes/no question, or modal auxiliary inversion).  This covers the class of
+ * labels that were previously interpolated raw into template frames, producing
+ * output like "What would clarify are the projected office savings from
+ * relocation realistic in this situation?".
+ */
+function isInterrogativeMeaning(meaning) {
+  const trimmed = String(meaning || "").trim().toLowerCase();
+  if (!trimmed) return false;
+
+  // Wh-questions (direct or indirect): who/what/where/when/how ...
+  if (/^(who|what|where|when|how)\b/.test(trimmed)) {
+    return true;
+  }
+
+  // Yes/no questions via subject-auxiliary inversion: the first word is a
+  // modal/auxiliary verb followed by a subject determiner (the / a / an / this / etc.)
+  // This covers "Is the …", "Are the …", "Will we …", "Does it …", etc.
+  const auxVerbs =
+    "is|are|was|were|do|does|did|have|has|had|will|would|can|could|should|may|might|must";
+  if (
+    new RegExp(
+      `^(?:${auxVerbs})\\s+(?:the|a|an|this|that|these|those|my|your|our|their|some|any|each|every|both|i|you|we|he|she|it|they)`,
+      "i",
+    ).test(trimmed)
+  ) {
+    return true;
+  }
+
+  // "whether" clauses — also already question-shaped
+  if (/^whether\b/i.test(trimmed)) return true;
+
+  return false;
+}
+
+function wrapInterrogativeForTemplate(meaning) {
+  const stripped = stripTrailingPunctuation(meaning).trim();
+
+  // Direct interrogatives → return unchanged (already coherent standalone questions)
+  if (/^(who|what|where|when|how)\b/.test(stripped.toLowerCase())) {
+    return stripped;
+  }
+
+  if (isInterrogativeMeaning(meaning)) {
+    return stripped;
+  }
+
+  return stripped;
+}
+
 function buildNeutralClarificationQuestion(meaning) {
-  return `What would clarify ${stripTrailingPunctuation(meaning)} in this situation?`;
+  const content = wrapInterrogativeForTemplate(meaning);
+  // If content is already interrogative (wh-), use it as-is with trailing context
+  if (isInterrogativeMeaning(content)) {
+    return `${content}?`;
+  }
+  return `What would clarify ${content} in this situation?`;
 }
 
 function buildEvidenceFallbackQuestion(meaning) {
-  return `What evidence would confirm or rule out ${stripTrailingPunctuation(meaning)}?`;
+  const content = wrapInterrogativeForTemplate(meaning);
+  if (isInterrogativeMeaning(content)) {
+    return `${content}?`;
+  }
+  return `What evidence would confirm or rule out ${content}?`;
 }
 
 function extractConstraintClarificationSubject(node) {
@@ -1172,27 +1232,52 @@ function buildQuestionFromFamily({
         },
       );
     }
+    // If meaning is already interrogative, use it directly instead of wrapping
+    if (isInterrogativeMeaning(meaning)) {
+      return `${wrapInterrogativeForTemplate(meaning)}?`;
+    }
     return `What evidence would clarify ${stripTrailingPunctuation(meaning)}?`;
   }
 
   if (questionFamily === "definition") {
+    // If meaning is already interrogative, use it directly
+    if (isInterrogativeMeaning(meaning)) {
+      return `${wrapInterrogativeForTemplate(meaning)}?`;
+    }
     return `What does ${meaning} mean in this situation?`;
   }
 
   if (reasoningPattern === "comparison") {
     if (selectedQuestionTemplate === "comparison_timing_basis") {
-      return `What evidence would clarify whether ${stripTrailingPunctuation(meaning)}?`;
+      const cmpContent = wrapInterrogativeForTemplate(meaning);
+      if (isInterrogativeMeaning(cmpContent)) {
+        return `${cmpContent}?`;
+      }
+      return `What evidence would clarify whether ${stripTrailingPunctuation(cmpContent)}?`;
     }
     if (selectedQuestionTemplate === "comparison_measurement_basis") {
-      return `What evidence would clarify ${stripTrailingPunctuation(meaning)}?`;
+      const cmpContent = wrapInterrogativeForTemplate(meaning);
+      if (isInterrogativeMeaning(cmpContent)) {
+        return `${cmpContent}?`;
+      }
+      return `What evidence would clarify ${stripTrailingPunctuation(cmpContent)}?`;
+    }
+    // Default comparison — handle interrogative meaning
+    if (isInterrogativeMeaning(meaning)) {
+      return `${wrapInterrogativeForTemplate(meaning)}?`;
     }
     return `What evidence would clarify ${stripTrailingPunctuation(meaning)}?`;
   }
 
   if (reasoningPattern === "contradiction") {
-    return investigationStrategy?.key === "contradiction_resolution"
-      ? buildQuestionFromStrategy(investigationStrategy)
-      : `What fact would resolve the contradiction about ${stripTrailingPunctuation(meaning)}?`;
+    if (investigationStrategy?.key === "contradiction_resolution") {
+      return buildQuestionFromStrategy(investigationStrategy);
+    }
+    // If meaning is interrogative, use it directly
+    if (isInterrogativeMeaning(meaning)) {
+      return `${wrapInterrogativeForTemplate(meaning)}?`;
+    }
+    return `What fact would resolve the contradiction about ${stripTrailingPunctuation(meaning)}?`;
   }
 
   if (reasoningPattern === "explanation") {
@@ -1203,9 +1288,14 @@ function buildQuestionFromFamily({
     return `Which option should be investigated first, and why?`;
   }
 
-  return investigationStrategy
-    ? buildQuestionFromStrategy(investigationStrategy)
-    : buildNeutralClarificationQuestion(meaning);
+  // Default (diagnosis) — handle interrogative meaning
+  if (investigationStrategy) {
+    return buildQuestionFromStrategy(investigationStrategy);
+  }
+  if (isInterrogativeMeaning(meaning)) {
+    return `${wrapInterrogativeForTemplate(meaning)}?`;
+  }
+  return buildNeutralClarificationQuestion(meaning);
 }
 
 export function formulateTieResolutionQuestion({ graph }) {
@@ -1590,6 +1680,12 @@ export function selectInvestigationStrategy({ node, graph, context = {} }) {
 }
 
 function buildQuestionFromStrategy(strategy) {
+  // If meaning is interrogative, use it directly instead of embedding in a template
+  const m = strategy.meaning;
+  if (isInterrogativeMeaning(m)) {
+    return `${wrapInterrogativeForTemplate(m)}?`;
+  }
+
   switch (strategy.key) {
     case "decision_threshold":
       return strategy.actionPhrase
diff --git a/tests/graph/question-formulation-v0.24.test.js b/tests/graph/question-formulation-v0.24.test.js
new file mode 100644
index 0000000..8c69fe2
--- /dev/null
+++ b/tests/graph/question-formulation-v0.24.test.js
@@ -0,0 +1,281 @@
+import { describe, expect, it } from "vitest";
+import { formulateQuestion } from "@/lib/graph/question-formulator.js";
+import { makeGraph, makeNode } from "@/lib/graph/schema.js";
+
+function makeGraphFor(node, extra = {}) {
+  return makeGraph({
+    centralStatement: extra.centralStatement || "Decision context",
+    nodes: [node, ...(extra.nodes || [])],
+    edges: extra.edges || [],
+    activeUnknownNodeId: node.id,
+    resolvedNodeIds: extra.resolvedNodeIds || [],
+    currentSummary: "Test summary",
+  });
+}
+
+/**
+ * Experiment 58A.4 — question formulation for pre-question-shaped labels
+ *
+ * Defect: when an unknown's label is already question or clause shaped,
+ * the engine interpolates it raw into a generic template frame, producing
+ * ungrammatical output such as "What would clarify are the projected office
+ * savings from relocation realistic in this situation?".
+ *
+ * The fix must generalise structurally to ANY interrogative/clause label,
+ * not just the specific patterns seen so far.
+ */
+describe("question formulation — pre-question labels (58A.4)", () => {
+
+  // ─── Helper: run formulateQuestion on an unknown and extract the question text ───
+  function questionFor(label, extra = {}) {
+    const node = makeNode({
+      id: "n-test",
+      label,
+      kind: "unknown",
+      status: "unknown",
+      confidence: "medium",
+      ...extra,
+    });
+    const result = formulateQuestion({
+      node,
+      graph: makeGraphFor(node),
+    });
+    return result.question;
+  }
+
+  // ─── Test 1: the exact defect from 58A.3 — wh-question label ───
+  it("produces a grammatical question when label is a wh-question", () => {
+    const q = questionFor(
+      "Are the projected office savings from relocation realistic?",
+    );
+
+    // Must NOT contain template injection artefact
+    expect(q).not.toMatch(/what would clarify are .* in this situation/i);
+    // Should be grammatical — either a standalone wh-question or wrapped properly
+    expect(q.endsWith("?")).toBe(true);
+    // The core meaning must survive
+    expect(q.toLowerCase()).toMatch(/office savings|relocation/);
+  });
+
+  // ─── Test 2: clause-shaped statement label (no question mark) ───
+  it("produces a grammatical question when label is a declarative clause", () => {
+    const q = questionFor(
+      "The projected office savings from relocation are unrealistic",
+    );
+
+    // Should be grammatical question form
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).toMatch(/office savings|relocation/);
+  });
+
+  // ─── Test 3: yes/no question label ───
+  it("produces a grammatical question when label is a yes/no question", () => {
+    const q = questionFor("Is the budget sufficient for this project?");
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).toMatch(/budget|sufficient/);
+  });
+
+  // ─── Test 4: whether-clause label ───
+  it("produces a grammatical question when label starts with 'whether'", () => {
+    const q = questionFor(
+      "Whether the new pricing strategy will increase revenue",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).toMatch(/pricing|revenue/);
+  });
+
+  // ─── Test 5: what-question label ───
+  it("produces a grammatical question when label is a what-question", () => {
+    const q = questionFor(
+      "What are the key risks of this project?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    // Should not double-wrap with "what would clarify"
+    expect(q.toLowerCase()).not.toMatch(/what would clarify what/i);
+    expect(q.toLowerCase()).toMatch(/key risks/);
+  });
+
+  // ─── Test 6: how-question label ───
+  it("produces a grammatical question when label is a how-question", () => {
+    const q = questionFor(
+      "How do we measure success for this initiative?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).not.toMatch(/what would clarify how/i);
+    expect(q.toLowerCase()).toMatch(/measure success/);
+  });
+
+  // ─── Test 7: why-question label ───
+  it("produces a grammatical question when label is a why-question", () => {
+    const q = questionFor(
+      "Why did the previous quarter underperform?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).not.toMatch(/what would clarify why/i);
+    expect(q.toLowerCase()).toMatch(/quarter|underperform/);
+  });
+
+  // ─── Test 8: who-question label ───
+  it("produces a grammatical question when label is a who-question", () => {
+    const q = questionFor(
+      "Who is responsible for the client onboarding process?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).not.toMatch(/what would clarify who/i);
+    expect(q.toLowerCase()).toMatch(/responsible|client/);
+  });
+
+  // ─── Test 9: when-question label ───
+  it("produces a grammatical question when label is a when-question", () => {
+    const q = questionFor(
+      "When does the current contract expire?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).not.toMatch(/what would clarify when/i);
+    expect(q.toLowerCase()).toMatch(/contract|expire/);
+  });
+
+  // ─── Test 10: where-question label ───
+  it("produces a grammatical question when label is a where-question", () => {
+    const q = questionFor(
+      "Where should the new warehouse be located?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).not.toMatch(/what would clarify where/i);
+    expect(q.toLowerCase()).toMatch(/warehouse|located/);
+  });
+
+  // ─── Test 11: non-question labels should NOT be affected ───
+  it("still applies template frames to declarative labels", () => {
+    const q = questionFor(
+      "Office lease exit penalty amount",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    // For a normal declarative label, the template frame should apply
+    // The key is that it's grammatical, not that it uses a specific frame
+    expect(q.toLowerCase()).toMatch(/lease|penalty/);
+  });
+
+  // ─── Test 12: evidence-fallback path with pre-question label ───
+  it("produces grammatical output via evidence fallback for question labels", () => {
+    const q = questionFor(
+      "Are the projected office savings from relocation realistic?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    // The output should be a single coherent question
+    const questionMarks = (q.match(/\?/g) || []).length;
+    expect(questionMarks).toBe(1);
+  });
+
+  // ─── Test 13: clause without question mark — statement form ───
+  it("handles label shaped like a that-clause", () => {
+    const q = questionFor(
+      "That the migration will reduce operational costs by at least 20%",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).toMatch(/migration|cost/);
+  });
+
+  // ─── Test 14: complex interrogative with embedded clause ───
+  it("handles label shaped like an indirect question", () => {
+    const q = questionFor(
+      "How much funding we need to complete the product launch",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).toMatch(/funding|launch/);
+  });
+
+  // ─── Test 15: the exact output from 58A.2 template — should not recur ───
+  it("does not produce the 58A.2 malformed question", () => {
+    const q = questionFor(
+      "Are the projected office savings from relocation realistic?",
+    );
+
+    expect(q).not.toBe(
+      "What was the comparable state before are the projected office savings from relocation realistic?",
+    );
+    expect(q).not.toMatch(/before .* are .* realistic/);
+  });
+
+  // ─── Test 16: short yes/no question label ───
+  it("handles very short interrogative labels", () => {
+    const q = questionFor("Is this the right approach?");
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).toMatch(/right|approach/);
+  });
+
+  // ─── Test 17: label with trailing punctuation already stripped in extractMeaning ───
+  it("handles label that is interrogative but loses its question mark via stripTrailingPunctuation", () => {
+    // This tests the internal pipeline: extractMeaning strips trailing ?, 
+    // then the meaning feeds into buildNeutralClarificationQuestion or evidence fallback
+    const q = questionFor(
+      "What is the total addressable market for this segment?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    const questionMarks = (q.match(/\?/g) || []).length;
+    expect(questionMarks).toBe(1);
+  });
+
+  // ─── Test 18: long complex interrogative label ───
+  it("handles very long interrogative labels without template doubling", () => {
+    const q = questionFor(
+      "Are the current employee satisfaction scores sufficient to justify continuing the remote work policy?",
+    );
+
+    expect(q.endsWith("?")).toBe(true);
+    expect(q.toLowerCase()).not.toMatch(/what would clarify are .* sufficient/i);
+    expect(q.toLowerCase()).toMatch(/satisfaction|remote/);
+  });
+
+  // ─── Test 19: interrogative label in definition reasoning pattern ───
+  it("handles interrogative labels through definition path", () => {
+    const node = makeNode({
+      id: "n-def",
+      label: "What does 'customer' mean in this context?",
+      kind: "unknown",
+      status: "unknown",
+      confidence: "medium",
+    });
+    const result = formulateQuestion({
+      node,
+      graph: makeGraphFor(node),
+    });
+
+    expect(result.question.endsWith("?")).toBe(true);
+    const questionMarks = (result.question.match(/\?/g) || []).length;
+    expect(questionMarks).toBe(1);
+  });
+
+  // ─── Test 20: interrogative label in evidence gathering pattern ───
+  it("handles interrogative labels through evidence path", () => {
+    const node = makeNode({
+      id: "n-evidence",
+      label: "What additional data do we need to validate the hypothesis?",
+      kind: "unknown",
+      status: "unknown",
+      confidence: "medium",
+    });
+    const result = formulateQuestion({
+      node,
+      graph: makeGraphFor(node),
+    });
+
+    expect(result.question.endsWith("?")).toBe(true);
+    const questionMarks = (result.question.match(/\?/g) || []).length;
+    expect(questionMarks).toBe(1);
+  });
+});