From 34eb0cd4e38bfe30614f662c66403d4f5560a3a7 Mon Sep 17 00:00:00 2001 From: robbond Date: Fri, 14 Aug 2026 09:02:08 +0100 Subject: [PATCH] fix(reasoning): exclude terminal nodes from active selector --- docs/current-handoff.md | 8 ++ docs/experiment-60b42.md | 92 ++++++++++++++++ lib/graph/utils.js | 5 +- tests/graph/apply-proposal.test.js | 162 ++++++++++++++++++++++++++++- 4 files changed, 265 insertions(+), 2 deletions(-) create mode 100644 docs/experiment-60b42.md diff --git a/docs/current-handoff.md b/docs/current-handoff.md index 87dd149..1ce54f5 100644 --- a/docs/current-handoff.md +++ b/docs/current-handoff.md @@ -226,6 +226,14 @@ Experiment 54N tested whether an interpretation disagreement can be judged for m - £700k figure preserved semantically (in reason text, not dedicated value field). No directional recommendation recorded for the decision. - Status pending Rob's review. +### 60B.42 active selector terminal-status guard + +- 60B.41 confirmed `selectActiveUnknownCandidate(...)` filtered only by `kind === "unknown"` and `resolvedNodeIds`, so terminal-status unknown nodes could still enter scoring when absent from `resolvedNodeIds`. +- Added a bounded selector-only guard in `lib/graph/utils.js`: active-selector candidates now exclude `status in ["known", "resolved", "contradicted"]` before scoring. +- Focused tests added in `tests/graph/apply-proposal.test.js` cover known-only null return, known/resolved/contradicted exclusion, and unchanged unresolved ranking. +- Focused preservation run passed via `npx vitest run tests/graph/apply-proposal.test.js -t "60B.42|60B.11|replaces downstream pricing"`. +- Broader duplicated post-mutation eligibility checks remain unchanged; this fixes the selector contract only, not the full stale-question lifecycle. + ### When This Knowledge-Management Phase Is Complete Provisional criteria for review (all confirmed met by Experiment 38 cold-start test): diff --git a/docs/experiment-60b42.md b/docs/experiment-60b42.md new file mode 100644 index 0000000..4e2e362 --- /dev/null +++ b/docs/experiment-60b42.md @@ -0,0 +1,92 @@ +# Experiment 60B.42 — Active selector terminal-status guard + +**Date:** 2026-08-14 +**Branch:** `feature/active-selector-terminal-guard-v0.37` + +## Purpose + +Implement the narrow selector-only fix established by 60B.41 so `selectActiveUnknownCandidate(...)` never scores or returns terminal-status unknown nodes. + +## 60B.41 diagnosis + +60B.41 confirmed that `selectActiveUnknownCandidate(graph, resolvedNodeIds)` filtered candidates using only: + +- `kind === "unknown"` +- `!resolvedNodeIds.includes(node.id)` + +It applied **no status filter at all**. Because `scoreUnknownCandidate(...)` also ignores node status, nodes with: + +- `status = known` +- `status = resolved` +- `status = contradicted` + +could enter scoring whenever their IDs were absent from `resolvedNodeIds`. + +That meant the active-selector fallback path could still select terminal nodes, including the exact known-decision stale-target risk seen in the 60B.37 lifecycle. + +## Exact selector filter change + +Changed only the candidate filter inside `selectActiveUnknownCandidate(...)` in `lib/graph/utils.js`. + +Before: + +```js +(n) => n.kind === "unknown" && !resolvedNodeIds.includes(n.id) +``` + +After: + +```js +(n) => + n.kind === "unknown" && + !["known", "resolved", "contradicted"].includes(n.status) && + !resolvedNodeIds.includes(n.id) +``` + +No scoring weights, ordering rules, prerequisite logic, or other eligibility predicates were changed. + +## Terminal-state tests + +Added focused tests in `tests/graph/apply-proposal.test.js` under: + +- `60B.42 — active selector terminal-status guard` + +Covered cases: + +1. known node excluded when a genuine unresolved node exists +2. known-only graph returns `null` +3. resolved node excluded even when absent from supplied `resolvedNodeIds` +4. contradicted node excluded even when absent from supplied `resolvedNodeIds` +5. unresolved ranking remains unchanged when both candidates are genuinely unresolved + +## Unresolved-ranking preservation + +The selector still chooses the same higher-priority unresolved candidate when both candidates remain valid (`status = unknown`). + +This confirms the change acts only as a pre-scoring terminal-state gate and does not alter ranking semantics. + +## 60B.11 / pricing preservation + +The same focused run preserved: + +- 60B.11 preferred-target behaviour +- prerequisite-first behaviour +- pricing regression selecting `n_commercial_value` instead of downstream `n_pricing` + +## Validation + +Command run: + +```bash +npx vitest run tests/graph/apply-proposal.test.js -t "60B.42|60B.11|replaces downstream pricing" +``` + +Result: + +- PASS — `16 passed | 71 skipped` + +## Remaining boundary + +This experiment does **not** solve the broader duplicated eligibility problem. + +Other post-mutation eligibility checks still exist elsewhere and remain unchanged in this task. This selector guard closes one specific fallback risk, but the broader shared-eligibility cleanup still remains to be handled separately before declaring the 60B.37 stale-question lifecycle fully fixed. \ No newline at end of file diff --git a/lib/graph/utils.js b/lib/graph/utils.js index 0cc91a7..22b0760 100644 --- a/lib/graph/utils.js +++ b/lib/graph/utils.js @@ -593,7 +593,10 @@ export function resolveUnknownNode(graph, nodeId, newStatus, newValue, reason) { export function selectActiveUnknownCandidate(graph, resolvedNodeIds) { // Skip already resolved nodes const unresolved = graph.nodes.filter( - (n) => n.kind === "unknown" && !resolvedNodeIds.includes(n.id), + (n) => + n.kind === "unknown" && + !["known", "resolved", "contradicted"].includes(n.status) && + !resolvedNodeIds.includes(n.id), ); if (unresolved.length === 0) return null; diff --git a/tests/graph/apply-proposal.test.js b/tests/graph/apply-proposal.test.js index 402b0d0..30138b6 100644 --- a/tests/graph/apply-proposal.test.js +++ b/tests/graph/apply-proposal.test.js @@ -1,7 +1,10 @@ import { describe, expect, it } from "vitest"; import { applyValidatedProposal } from "@/lib/graph/apply-proposal.js"; import { makeEdge, makeGraph, makeNode } from "@/lib/graph/schema.js"; -import { validateGraphReferences } from "@/lib/graph/utils.js"; +import { + selectActiveUnknownCandidate, + validateGraphReferences, +} from "@/lib/graph/utils.js"; function makeComparabilityUpdateFixture() { const comparabilityUnknown = makeNode({ @@ -3660,6 +3663,163 @@ describe("applyValidatedProposal", () => { // 60B.11 — prerequisite-aware question targeting // ============================================ +describe("60B.42 — active selector terminal-status guard", () => { + it("excludes known nodes and returns a genuine unresolved candidate", () => { + const knownDecision = makeNode({ + id: "n-known-decision", + label: "Known decision node", + description: "This unknown-shaped node is already known and must be excluded.", + kind: "unknown", + status: "known", + confidence: "high", + }); + const unresolvedCandidate = makeNode({ + id: "n-unresolved-candidate", + label: "Unresolved candidate", + description: "Need customer evidence because the decision depends on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }); + + const graph = makeGraph({ + centralStatement: "Test graph", + nodes: [knownDecision, unresolvedCandidate], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "Known and unresolved candidates present.", + }); + + const result = selectActiveUnknownCandidate(graph, []); + + expect(result?.nodeId).toBe("n-unresolved-candidate"); + expect(result?.nodeId).not.toBe("n-known-decision"); + }); + + it("returns null when the graph contains only known-status unknown nodes", () => { + const knownOnly = makeNode({ + id: "n-known-only", + label: "Known-only node", + description: "This unknown-shaped node is already terminal.", + kind: "unknown", + status: "known", + confidence: "medium", + }); + + const graph = makeGraph({ + centralStatement: "Known-only graph", + nodes: [knownOnly], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "No unresolved candidates remain.", + }); + + expect(selectActiveUnknownCandidate(graph, [])).toBeNull(); + }); + + it("excludes resolved nodes even when their IDs are absent from resolvedNodeIds", () => { + const resolvedUnknown = makeNode({ + id: "n-resolved-terminal", + label: "Resolved terminal node", + description: "This node is resolved and must not enter scoring.", + kind: "unknown", + status: "resolved", + confidence: "high", + }); + const unresolvedCandidate = makeNode({ + id: "n-unresolved-fallback", + label: "Fallback unresolved node", + description: "Need unresolved evidence to continue the investigation.", + kind: "unknown", + status: "unknown", + confidence: "medium", + }); + + const graph = makeGraph({ + centralStatement: "Resolved status guard graph", + nodes: [resolvedUnknown, unresolvedCandidate], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "Resolved node omitted from resolvedNodeIds on purpose.", + }); + + const result = selectActiveUnknownCandidate(graph, []); + + expect(result?.nodeId).toBe("n-unresolved-fallback"); + expect(result?.nodeId).not.toBe("n-resolved-terminal"); + }); + + it("excludes contradicted nodes even when their IDs are absent from resolvedNodeIds", () => { + const contradictedUnknown = makeNode({ + id: "n-contradicted-terminal", + label: "Contradicted terminal node", + description: "This node is contradicted and must not enter scoring.", + kind: "unknown", + status: "contradicted", + confidence: "high", + }); + const unresolvedCandidate = makeNode({ + id: "n-unresolved-after-contradiction", + label: "Remaining unresolved node", + description: "Need remaining unresolved evidence after contradiction.", + kind: "unknown", + status: "unknown", + confidence: "medium", + }); + + const graph = makeGraph({ + centralStatement: "Contradicted status guard graph", + nodes: [contradictedUnknown, unresolvedCandidate], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "Contradicted node omitted from resolvedNodeIds on purpose.", + }); + + const result = selectActiveUnknownCandidate(graph, []); + + expect(result?.nodeId).toBe("n-unresolved-after-contradiction"); + expect(result?.nodeId).not.toBe("n-contradicted-terminal"); + }); + + it("preserves existing unresolved ranking when both candidates remain genuinely unresolved", () => { + const higherPriority = makeNode({ + id: "n-higher-priority", + label: "Customer evidence priority", + description: + "Need customer evidence and success threshold clarity because the decision depends on it.", + kind: "unknown", + status: "unknown", + confidence: "high", + }); + const lowerPriority = makeNode({ + id: "n-lower-priority", + label: "Implementation detail", + description: + "Need technical implementation detail for a possible future feature.", + kind: "unknown", + status: "unknown", + confidence: "medium", + }); + + const graph = makeGraph({ + centralStatement: "Ranking preservation graph", + nodes: [higherPriority, lowerPriority], + edges: [], + activeUnknownNodeId: null, + resolvedNodeIds: [], + currentSummary: "Two unresolved candidates remain.", + }); + + const result = selectActiveUnknownCandidate(graph, []); + + expect(result?.nodeId).toBe("n-higher-priority"); + }); +}); + describe("60B.11 — prerequisite-aware question targeting", () => { // Test 1: same-proposal selected unknown with no prerequisite is preferred it("prefers a same-proposal added unknown when model selects it and it has no unresolved depends_on prerequisite", () => {