robbond/confidence-engine
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

experiment: define decision sufficiency module boundary

Browse Source
This commit is contained in:
robbond
2026-08-14 15:00:48 +01:00
parent bce05f779b
commit 983ebcc836
2 changed files with 447 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/current-handoff.md
+94
View File
@@ -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
docs/experiment-60b65.md
+353
View File
@@ -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
```