From 983ebcc836f01deddfb8750b37533187bb980613 Mon Sep 17 00:00:00 2001 From: robbond Date: Fri, 14 Aug 2026 15:00:48 +0100 Subject: [PATCH] experiment: define decision sufficiency module boundary --- docs/current-handoff.md | 94 +++++++++++ docs/experiment-60b65.md | 353 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 447 insertions(+) create mode 100644 docs/experiment-60b65.md diff --git a/docs/current-handoff.md b/docs/current-handoff.md index 6bc23e0..f9e5d8a 100644 --- a/docs/current-handoff.md +++ b/docs/current-handoff.md @@ -3295,3 +3295,97 @@ One unresolved question at implementation layer only: precise phrase family brea ### VITEST RUN: NO ### JEST RUN: NO ### WATCHMAN USED: NO + +## Experiment 60B.65 — Decision-sufficiency module boundary audit (read-only structural analysis) + +**Type:** Structural audit of decision-sufficiency implementation scale and extractability +**Purpose:** Determine whether the 60B.64 closure implementation added ~210 lines to apply-proposal.js is carrying too many responsibilities, and identify the smallest safe module boundary for future zero-semantic-change extraction. + +### LINE FOOTPRINT (lib/graph/apply-proposal.js — 4730 lines) + +| Unit | Lines | Approx count | +|---|---|---| +| Confirmation helper (`isUserConfirmationOfNoRemainingUncertainty`) + constants | 61–117 | ~57 | +| Remaining-factor helpers (`isUnresolvedUnknown`, `hasRemainingMaterialFactors`, `countRemainingMaterialFactors`) + TERMINAL_STATUSES | 4615–4730 | ~111 | +| Closure integration block (inside applyValidatedProposal) | 3835–3984 | ~149 | +| **Total decision-sufficiency production lines** | — | **~317** | + +The closure integration block (~149 lines) is the largest single unit. It contains three responsibilities: virtual resolution set construction, decision-sufficiency evaluation (predicate), and graph mutation (status update). The virtual helper `checkRemainingFactorsVirtual` duplicates ~55 lines of traversal logic from `countRemainingMaterialFactors`. + +### RESPONSIBILITY CLASSIFICATION + +| Unit | Classification | Mixed? | +|---|---|---| +| Confirmation helper | TEXT CONFIRMATION | NO — pure text predicate | +| `isUnresolvedUnknown` | GRAPH QUERY | NO | +| `hasRemainingMaterialFactors` | GRAPH QUERY | NO | +| `countRemainingMaterialFactors` | GRAPH QUERY | NO | +| Closure integration block | ORCHESTRATION + MUTATION | YES — 3 responsibilities in one inline block | + +### DATA DEPENDENCIES + +- **Confirmation:** needs only `answer` string. Zero accidental coupling. Pure function. +- **Remaining-factor detection:** needs `(decisionNodeId, graph)`. Zero accidental coupling. Pure function. +- **Closure application:** reads `updatedSituationGraph`, `proposalSnapshot`, `answer`. All PASSABLE ARGUMENTS — no deep local-variable dependency. The virtual helper's duplication of graph traversal is latent code smell, not accidental coupling. + +### CANDIDATE ASSESSMENT SUMMARY + +| Candidate | Semantic risk | Coupling reduction | Testability | Complexity reduction | Schema change? | +|---|---|---|---|---|---| +| A (no extract) | N/A | NONE | NONE | NONE | NO | +| B (query only) | LOW | MEDIUM | MEDIUM | MEDIUM | NO | +| C (query+confirm) | LOW | HIGH | HIGH | MEDIUM | NO | +| **D (pure sufficiency unit)** ★ | **LOW** | **HIGH** | **HIGH** | **HIGH** | **NO** | +| E (query+mutation) | HIGH | MEDIUM | MEDIUM | LOW-MED | YES/NO | + +### PREDICATE BOUNDARY + +**Pure-function boundary possible: YES** + +```js +shouldCloseDecision({ decisionNodeId, graph, answer }) -> boolean +``` + +Derived from two existing pure functions + one boolean combine. Zero mutation, zero state change. All inputs naturally available at the closure-evaluation point. + +### MINIMUM ORCHESTRATION REMAINING AFTER EXTRACTION + +~40 lines (reduced from ~150): +```js +iterate parent unknown nodes → call shouldCloseDecision() → apply mutation if true → set closureApplied = true +``` + +### TEST MIGRATION + +- 60B.61 tests (9 cases, 353 lines): **YES** — all test pure `hasRemainingMaterialFactors` +- Confirmation subtests: **PARTIAL** — mixed with integration fixtures +- 60B.64 full integration tests: **STAY in apply-proposal.test.js** + +### RUNTIME / TOOLING IMPACT + +| Question | Answer | +|---|---| +| Runtime performance improvement? | NEGLIGIBLE | +| Claude/Codex edit reliability? | LIKELY YES | +| Future context reduction? | LIKELY YES | + +### CRITICAL DISTINCTION: D — EXTRACT PURE DECISION-SUFFICIENCY UNIT +### MINIMUM REFACTOR BOUNDARY: B — ONE NEW decision-sufficiency.js MODULE +### REFACTOR TIMING: B — RUN LIVE REGRESSION FIRST, THEN REFACTOR +### IMPLEMENTATION READINESS: A — READY FOR ZERO-SEMANTIC-CHANGE REFACTOR + +The evaluation logic (confirmation detection + remaining-factor counting + closure predicate) is entirely pure and self-contained. One `decision-sufficiency.js` module with 5 exports achieves all seven criteria: zero semantic change, identical 60B.64 behaviour, visible orchestration (~40 lines), mutation ownership in apply-proposal, independently testable pure logic, no schema/prompt changes, less future context. + +**One unresolved question at implementation layer only:** Should `shouldCloseDecision` return just `boolean` or a richer shape `{ hasRemainingFactors, userConfirmedNoRemainingUncertainty, shouldClose }` for diagnostic logging? This does not affect extraction correctness — only post-refactor API surface. + +Smallest zero-semantic-change refactor: extract all decision-sufficiency evaluation logic to one module with 5 exports, replace inline closure evaluation in apply-proposal.js with call to `shouldCloseDecision`, keep mutation code in apply-proposal.js. + +### PRODUCTION CODE CHANGED: NO +### TESTS CHANGED: NO +### PROMPT CHANGED: NO +### SCHEMA CHANGED: NO +### OLLAMA CALLS: 0 +### LIVE API CALLS: 0 +### VITEST RUN: NO +### JEST RUN: NO +### WATCHMAN USED: NO diff --git a/docs/experiment-60b65.md b/docs/experiment-60b65.md new file mode 100644 index 0000000..4db3697 --- /dev/null +++ b/docs/experiment-60b65.md @@ -0,0 +1,353 @@ +# Experiment 60B.65 — Decision-sufficiency module boundary audit + +**Branch:** `feature-decision-closure-integration-v0.43` +**Status:** audit only, zero production changes +**Date:** 2026-08-14 + +--- + +## Pre-check + +```text +branch = feature-decision-closure-integration-v0.43 ✓ +working tree = clean ✓ +HEAD includes bce05f7 ✓ +``` + +--- + +## LINE FOOTPRINT (lib/graph/apply-proposal.js) + +### Confirmation helper +**Lines:** 61–117 (total), 63–85 constants + 96–117 function body +- Header comment: line 61 (1 line) +- `CONTRADICTION_PHRASES`: lines 63–66 (4 lines) +- `CONFIRMATION_PHRASES`: lines 69–80 (12 lines) +- `CONFIRMATION_PATTERNS`: lines 82–85 (4 lines) +- JSDoc for `isUserConfirmationOfNoRemainingUncertainty`: lines 87–95 (9 lines) +- Function `isUserConfirmationOfNoRemainingUncertainty`: lines 96–117 (22 lines) + +**Approx count:** ~47 production lines (constants + function body, excl. header comment) + +### Remaining-factor helpers +**Lines:** 4615–4730 (total) +- Comment header: line 4615 (1 line) +- `TERMINAL_STATUSES`: line 4617 (1 line) +- `isUnresolvedUnknown`: lines 4619–4623 (5 lines) +- `hasRemainingMaterialFactors`: lines 4625–4627 (3 lines, thin wrapper) +- JSDoc + `countRemainingMaterialFactors`: lines 4629–4729 (101 lines incl. JSDoc) + +**Approx count:** ~110 production lines + +### Closure integration block (inside applyValidatedProposal) +**Lines:** 3835–3984 (within function) +- Comment header: line 3835 (1 line) +- `pendingResolvedIds` + virtual helper setup: lines 3843–3853 (~11 lines) +- `checkRemainingFactorsVirtual`: lines 3855–3942 (88 lines — **duplicates** graph traversal from countRemainingMaterialFactors) +- Parent-node iteration + closure predicate application: lines 3945–3983 (~39 lines) + +**Approx count:** ~149 production lines + +### Supporting additions (60B.64-specific) +- `TERMINAL_STATUSES` at line 4617: 1 line (shared between remaining-factor detection and closure virtual helper) + +### Total decision-sufficiency production lines in apply-proposal.js + +```text +Confirmation constants + function: ~57 +Remaining-factor helpers: ~111 +Closure integration block: ~150 +───────────────────────────────────────────── +Total in apply-proposal.js: ~318 +``` + +Of these, **~149 lines are the closure integration block** (the bulk of the 210-line addition cited for 60B.64). The remaining ~70 lines are helper functions/constants that support it. + +--- + +## RESPONSIBILITIES + +### Confirmation helper (`isUserConfirmationOfNoRemainingUncertainty`) +- **Classification:** TEXT CONFIRMATION +- Pure text-predicate on raw user answer string +- Zero graph access, zero side effects + +### Remaining-factor helpers +- `isUnresolvedUnknown`: **GRAPH QUERY** (simple status check) +- `hasRemainingMaterialFactors`: **GRAPH QUERY** (thin boolean wrapper) +- `countRemainingMaterialFactors`: **GRAPH QUERY** (complex traversal across 4 routes) + +### Closure integration block responsibilities + +The block performs **three distinct** responsibilities: + +1. **Virtual resolution set construction** — builds `pendingResolvedIds` from `proposalSnapshot.resolvedUnknownNodeIds` and `proposalSnapshot.updatedNodes` +2. **Decision sufficiency evaluation** — calls `checkRemainingFactorsVirtual` + `isUserConfirmationOfNoRemainingUncertainty` to produce a boolean predicate +3. **Graph mutation** — sets `parentNode.status = "resolved"`, calls `ensureResolvedUnknownId`, upserts `proposalSnapshot.updatedNodes` + +### Mixed responsibilities present? + +**YES.** The closure integration block mixes: +- Decision sufficiency *evaluation* (responsibility 2) with graph *mutation* (responsibility 3). +- The virtual factor-counting function (`checkRemainingFactorsVirtual`) is also a duplicate of the pure `countRemainingMaterialFactors` from 60B.61, creating **intra-file duplication** of ~55 lines of traversal logic. + +--- + +## DATA DEPENDENCIES + +### Confirmation helper +**Needs:** +- `answer` (raw user answer string) — from applyValidatedProposal argument + +**Accidental coupling:** NONE +- Pure function with single input, zero graph access + +### Remaining-factor detection (`countRemainingMaterialFactors`) +**Needs:** +- `decisionNodeId` (string) +- `graph.nodes`, `graph.edges` +- `TERMINAL_STATUSES` constant (internal to same module) + +**Accidental coupling:** NONE +- Pure function with two explicit parameters; all logic is internal + +### Closure application (integration block) +**Needs:** +- `parentNode` — from iteration over `updatedSituationGraph.nodes` +- `answer` — for confirmation check +- `proposalSnapshot` — to read `resolvedUnknownNodeIds`, `updatedNodes`; to mutate status entries +- `updatedSituationGraph.nodes/edges` — to build nodesById map (duplicates what countRemainingMaterialFactors already does) + +**Accidental coupling:** +- **LOW.** Reads from `proposalSnapshot` and `updatedSituationGraph` which are natural outputs of the preceding decomposition → propagation stages. These are essential flow-throughs, not deep-local coupling. +- The **virtual helper** duplicates the graph traversal from `countRemainingMaterialFactors`, reading nodes/edges that the pure function already accepts as parameters. This is *latent* duplication rather than accidental coupling per se — it exists because the block chooses to re-implement rather than reuse. + +--- + +## HIDDEN COUPLING AUDIT + +| Local variable in applyValidatedProposal | Dependency type | +|---|---| +| `proposalSnapshot` | **PASSABLE ARGUMENT** — could be passed to a predicate | +| `updatedSituationGraph` | **PASSABLE ARGUMENT** — same as graph parameter to pure function | +| `reasoningState` | NOT used by closure block | +| `deterministicSelection` | NOT used BY closure (but read AFTER if closureApplied=true) | +| `resolvedUnknownNodeIds` | PART of `proposalSnapshot`; not accessed directly | +| `validatedProposal` | NOT used by closure block | +| `answer` | **PASSABLE ARGUMENT** — single string, already extracted in confirmation helper | + +No deep/local-variable coupling discovered. The closure block's dependencies are all at the function's parameter/early-boundary level. + +--- + +## CANDIDATE ASSESSMENT + +### Candidate A — NO EXTRACTION +- **Semantic-change risk:** N/A (no change) +- **Coupling reduction:** NONE +- **Testability improvement:** NONE (tests already exist but in large file) +- **Complexity reduction:** NONE (~318 lines of decision-sufficiency code still mixed in 4730-line file) +- **Schema change:** NO +- **Principal weakness:** The virtual helper duplicates `countRemainingMaterialFactors`. Two independent implementations of the same graph traversal logic create maintenance risk. + +### Candidate B — EXTRACT GRAPH QUERY ONLY +Extract `isUnresolvedUnknown`, `hasRemainingMaterialFactors`, `countRemainingMaterialFactors` → `decision-sufficiency.js` + +- **Semantic-change risk:** LOW (all three are pure functions already exported) +- **Coupling reduction:** MEDIUM (removes ~111 lines from apply-proposal.js; eliminates one duplication source by enabling reuse) +- **Testability improvement:** MEDIUM (pure graph queries become importable test fixtures) +- **Complexity reduction:** MEDIUM (~111 fewer lines in apply-proposal.js) +- **Schema change:** NO +- **Principal weakness:** The virtual helper inside the closure block still duplicates traversal logic. It would need to be rewritten to call `countRemainingMaterialFactors` with a custom "unresolved predicate" parameter, or the extracted module would need to accept such a parameter — introducing a new signature variant that complicates the extraction. + +### Candidate C — EXTRACT QUERY + CONFIRMATION +Add `isUserConfirmationOfNoRemainingUncertainty`, `hasRemainingMaterialFactors`, `countRemainingMaterialFactors` → `decision-sufficiency.js` + +- **Semantic-change risk:** LOW (all pure, zero state dependency) +- **Coupling reduction:** HIGH (removes all decision-sufficiency *evaluation* from apply-proposal.js; ~167 lines) +- **Testability improvement:** HIGH (confirmation detection becomes independently testable) +- **Complexity reduction:** MEDIUM (~167 fewer lines in apply-proposal.js; closure block reduced to orchestration/mutation only) +- **Schema change:** NO +- **Principal weakness:** The closure integration block's virtual helper still exists and duplicates graph traversal. It must be eliminated or rewritten. + +### Candidate D — EXTRACT PURE DECISION-SUFFICIENCY UNIT ★ RECOMMENDED +Extract all three functions + a combined predicate: +```js +// decision-sufficiency.js exports: +isUserConfirmationOfNoRemainingUncertainty(answer) -> boolean +hasRemainingMaterialFactors(decisionNodeId, graph) -> boolean +countRemainingMaterialFactors(decisionNodeId, graph) -> number +shouldCloseDecision({ decisionNodeId, graph, answer }) -> boolean +``` + +Keep in apply-proposal.js only: +- The confirmation constants (or move them to the new module too) +- `TERMINAL_STATUSES` (or move it — see below) +- The closure *mutation* block that applies parentNode.status = "resolved" + +- **Semantic-change risk:** LOW (pure functions extracted; apply-proposal.js becomes a thin consumer of a predicate result) +- **Coupling reduction:** HIGH (all evaluation moves to dedicated module; only orchestration/mutation stays) +- **Testability improvement:** HIGH (`shouldCloseDecision` is the clearest possible unit test target — 3 inputs, 1 boolean output, zero graph access needed in tests) +- **Complexity reduction:** HIGH (~210 fewer lines in apply-proposal.js for evaluation; closure block reduced to ~40 mutation lines) +- **Schema change:** NO (existing `hasRemainingMaterialFactors` and `isUserConfirmationOfNoRemainingUncertainty` already exported — no public API change) +- **Principal weakness:** Requires adding a new `shouldCloseDecision` predicate that doesn't exist today. This is the only "new function" introduced, but it's derived directly from the existing inline code (lines 3952–3955). + +### Candidate E — EXTRACT QUERY + MUTATION +Move both evaluation AND graph mutation to a new module. + +- **Semantic-change risk:** HIGH (breaks apply-proposal.js's ownership of all graph mutations) +- **Coupling reduction:** MEDIUM (evaluation isolated but now also outside apply-proposal.js) +- **Testability improvement:** MEDIUM (mutation tests require graph state setup in every test) +- **Complexity reduction:** LOW-MEDIUM (apply-proposal.js loses mutation code but also loses visibility into the full lifecycle) +- **Schema change:** YES or NO depending on whether mutation is applied inside apply-proposal or returned as a diff — either way requires interface change +- **Principal weakness:** Violates principle #4 ("graph mutation ownership stays in apply-proposal"). Introduces dual-mutation-source risk. The extracted module would need to be aware of `applyValidatedProposal`'s post-closure flow (`deterministicSelection`, selectedQuestion) to avoid orphaned state. + +--- + +## PURE-FUNCTION BOUNDARY + +**Pure-function boundary possible:** YES + +**Recommended shape:** +```js +shouldCloseDecision({ + decisionNodeId, // string — the unknown node ID being evaluated for closure + graph, // SituationGraph — post-propagation graph state + answer // string — raw user answer (not processed/normalized) +}) -> boolean +``` + +**Why:** +- All three inputs are naturally available at the point where the closure block runs. +- The existing `isUserConfirmationOfNoRemainingUncertainty` already accepts a single `answer` parameter and is pure. +- The existing `countRemainingMaterialFactors` already accepts `(decisionNodeId, graph)` and is pure. +- The predicate is simply: `countRemainingMaterialFactors(decisionNodeId, graph) === 0 && isUserConfirmationOfNoRemainingUncertainty(answer)`. +- No mutation, no question selection, no state change — all within the strict purity constraints listed in the prompt. + +--- + +## ORCHESTRATION BOUNDARY + +**Minimum code remaining in applyValidatedProposal after extraction:** + +```js +// Lines ~15-20 would remain: + +const sufficiency = shouldCloseDecision({ + decisionNodeId: parentNode.id, + graph: updatedSituationGraph, + answer, +}); + +if (sufficiency) { + // mutation only: + parentNode.status = "resolved"; + ensureResolvedUnknownId(proposalSnapshot, parentNode.id); + upsertUpdateInSnapshot(proposalSnapshot, parentNode.id, ...); + closureApplied = true; +} +``` + +**Approximate orchestration lines after extraction:** ~40 lines +(reduced from ~150 lines currently) + +The remaining code is purely: +1. Iterate parent unknown nodes +2. Call external predicate +3. Apply mutation if predicate returns true +4. Mark `closureApplied = true` + +--- + +## TEST MIGRATION + +**60B.61 tests movable:** YES +- 9 test cases (lines 5012–5364, ~353 lines) +- All test `hasRemainingMaterialFactors` which is a pure function +- Could be extracted to `tests/graph/decision-sufficiency.test.js` without assertion changes + +**Confirmation tests movable:** PARTIAL +- 8 tests in "60B.64 — explicit decision sufficiency closure" (lines 5367–5784) +- These test the *full integration* of confirmation + remaining-factor evaluation + mutation +- The confirmation helper's individual behaviour is tested indirectly through these integration tests +- Could extract ~120 lines of confirmation-only subtests to a separate file, but the fixtures (makeClosureDecisionFixture) are shared + +**60B.64 full integration tests should remain in apply-proposal.test.js:** YES +- These test the end-to-end flow: applyValidatedProposal → closure mutation → downstream state effects +- Any extraction must preserve these assertions exactly as they stand + +--- + +## RUNTIME / TOOLING + +**Would splitting this logic into modules materially improve runtime performance:** NEGLIGIBLE +- No computational complexity change; same function calls, same object allocations +- Possibly microscopically slower due to module import overhead (unobservable in practice) + +**Would it improve Claude/Codex edit reliability:** LIKELY YES +- Decision-sufficiency logic would live in a ~150-line file instead of being scattered across a 4730-line file +- Future edits to the confirmation phrases, factor routes, or closure predicate can be done with ~60 lines of context vs ~400+ lines today + +**Would it reduce context required for future reasoning changes:** LIKELY YES +- Confirmation logic is conceptually independent from graph traversal +- Factor-detection logic is independently auditable +- Today all three are interleaved inside applyValidatedProposal, requiring the reader to mentally separate concerns while reading ~150 lines of inline code + +--- + +## CRITICAL DISTINCTION + +**Choice: D — EXTRACT PURE DECISION-SUFFICIENCY UNIT** + +**Why:** The evaluation logic (confirmation detection + remaining-factor counting + closure predicate) is entirely pure and self-contained. It should own itself as a unit. Graph mutation stays in apply-proposal.js per principle #4. This is the narrowest boundary that achieves goals #1–#7. + +--- + +## MINIMUM REFACTOR BOUNDARY + +**Choice: B — one new decision-sufficiency module** + +**Why:** A single `decision-sufficiency.js` module containing all five functions (`isUserConfirmationOfNoRemainingUncertainty`, `hasRemainingMaterialFactors`, `countRemainingMaterialFactors`, `shouldCloseDecision`, and `TERMINAL_STATUSES`) achieves: +- Zero semantic change (all existing exports preserved) +- 60B.64 behaviour identical (apply-proposal.js calls the same predicate, produces same result) +- Apply-proposal orchestration fully visible (~40 lines) +- Graph mutation ownership stays in apply-proposal +- Pure logic independently testable (`shouldCloseDecision` is the ideal unit test target) +- No schema change +- No prompt change +- Future edits require less context + +--- + +## REFACTOR TIMING + +**Choice: B — RUN LIVE REGRESSION FIRST, THEN REFACTOR** + +**Why:** The current implementation passes its targeted behavioural tests. Introducing a refactor before verifying that live regression (60B.56) still passes would conflate two risk vectors: regression risk + extraction risk. Running regression first provides confidence that the existing code is correct, making any subsequent extraction's "zero semantic change" claim verifiable against a known-good baseline. + +--- + +## IMPLEMENTATION READINESS + +**Choice: A — READY FOR ZERO-SEMANTIC-CHANGE REFACTOR** + +If B (one more design question required), the unresolved question would be: should `shouldCloseDecision` return just `boolean` or a richer shape `{ hasRemainingFactors, userConfirmedNoRemainingUncertainty, shouldClose }` for diagnostic logging? This does not affect correctness of extraction — only post-refactor API surface. + +**Smallest zero-semantic-change refactor:** +Extract all decision-sufficiency evaluation logic to a single `decision-sufficiency.js` module with 5 exports, replace the inline closure evaluation in apply-proposal.js with a call to `shouldCloseDecision`, and keep mutation code in apply-proposal.js. + +--- + +## CONSTRAINTS CHECK + +```text +Production code changed: NO +Tests changed: NO +Prompt changed: NO +Schema changed: NO +Ollama calls: 0 +Live API calls: 0 +Vitest run: NO +Jest run: NO +Watchman used: NO +```