experiment: map remaining apply proposal boundaries
This commit is contained in:
@@ -3455,3 +3455,15 @@ Key outcomes:
|
||||
Apparatus: Reused existing regression `60B.64 Test 1 — exact 60B.56 wording closes` (no new harness created).
|
||||
Post-refactor deterministic closure path verified as equivalent to pre-refactor baseline.
|
||||
|
||||
|
||||
---
|
||||
|
||||
### Experiment 60B.70 — Remaining apply-proposal.js Boundary Map (2026-08-14)
|
||||
|
||||
Read-only structural audit completed. `apply-proposal.js` is 4,480 lines with one ~992-line `applyValidatedProposal` function containing 15+ inline phases of orchestration logic. Responsibility map identifies eight clusters: proposal reconciliation (90L), compatibility validation (~130L inline + helpers exported at 902-1261), answer semantic validation (~297L), active unknown/target selection (~933L exported + ~215L inlined orchestration), resolution propagation (already extracted at 360L), final selectedQuestion lifecycle (~160L inline), graph mutation (via imported applyGraphUpdate), and supporting pure helpers (~530L).
|
||||
|
||||
**Finding:** Candidates A (reconciliation, 90L) and B (compatibility validation, ~130L inline) are equally clean extraction targets with STRONG test coverage and LOW semantic risk. Validation is prioritized first because it removes the largest single inline block that obscures the post-validation lifecycle. Strategy: two independent extractions in separate commits when a natural editing context arises.
|
||||
|
||||
**Decision:** RETURN TO REASONING WORK FIRST — no implementation directive this session. Next extraction should happen when schema changes naturally touch the validation layer, avoiding forced refactoring without external pressure to ensure correctness.
|
||||
|
||||
Medium-term target for apply-proposal.js: ~4,250-4,300 lines (clear orchestration file, not tiny wrapper-only).
|
||||
|
||||
@@ -0,0 +1,280 @@
|
||||
# Experiment 60B.70 — Remaining apply-proposal.js Boundary Map
|
||||
|
||||
**Date:** 2026-08-14
|
||||
**Branch:** `feature/decision-sufficiency-module-v0.44`
|
||||
**Type:** Read-only structural audit (no code changes)
|
||||
|
||||
---
|
||||
|
||||
## Objective
|
||||
|
||||
Identify the highest-value zero-semantic-change extraction boundary in `apply-proposal.js` (4,480 lines) that would materially reduce edit/context risk without obscuring lifecycle orchestration.
|
||||
|
||||
---
|
||||
|
||||
## Git Pre-check
|
||||
|
||||
```
|
||||
branch = feature/decision-sufficiency-module-v0.44
|
||||
working tree = clean
|
||||
HEAD includes: 36b4f47, 1ca5026, 6cb9109 ✓
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Responsibility Map
|
||||
|
||||
### Proposal reconciliation (lines 320–409)
|
||||
- **Approx line count:** 90 lines
|
||||
- **Primary responsibility:** Normalize `resolvedUnknownNodeIds` ↔ `updatedNodes` symmetry; auto-add synthetic updatedNode when proposal lists a resolved ID without corresponding update; nuke selectedQuestion if its node was resolved by the same proposal
|
||||
- **Pure / impure / mixed:** Mixed (pure on proposal object, reads graph state only for existence checks)
|
||||
- **Depends heavily on applyValidatedProposal locals:** NO — takes `graph` + `proposal` as parameters; returns `{proposal, errors}`
|
||||
- **Existing focused tests:** STRONG (60B.49 suite: 6 integration tests across reconciliation, staleness, dedup, non-unknown guard)
|
||||
|
||||
### Proposal compatibility / selected-question validation (lines ~3581–3648)
|
||||
- **Approx line count:** 130 lines of inline validation logic within applyValidatedProposal
|
||||
- **Primary responsibility:** Pre-mutation graph integrity — added edge duplicates, edge reference validity (from/to node existence, cross-boundary edges), removed edge existence, combined node deduplication, semantic duplicate unknown detection, added-unknown support validation, selectedQuestion node validity, answer-meaning compatibility with raw answer, answer-meaning alignment, question-selection requirement
|
||||
- **Pure / impure / mixed:** Mixed — calls helpers that read graph + proposal; mutates no state
|
||||
- **Depends heavily on applyValidatedProposal locals:** PARTIAL — operates on `validatedProposal` (local) and `situationGraph` (param); calls imported `validateGraphUpdate`
|
||||
- **Existing focused tests:** STRONG — 60B.61/64 suites, structured-fidelity suite (8 tests), boundary overlap tests (3), regression A/B/C/D suites, add-unknown support tests (7 cases)
|
||||
|
||||
### Resolution propagation (lines 902–1261; `propagateResolvedChildEvidence`)
|
||||
- **Approx line count:** 360 lines (exported function)
|
||||
- **Primary responsibility:** Post-mutation parent progress state computation; child branch evidence aggregation; ancestor chain confidence propagation; confidence cap logic; comparison vs independent evidence distinction
|
||||
- **Pure / impure / mixed:** Mixed — reads graph, computes derived metrics, returns rich result object
|
||||
- **Depends heavily on applyValidatedProposal locals:** NO — already extracted as standalone export
|
||||
- **Existing focused tests:** MEDIUM (covered by 60B.43/64 integration; no dedicated unit suite)
|
||||
|
||||
### Active unknown / target selection (lines ~2556–3488 + inlined orchestration at 3907–4121)
|
||||
- **Approx line count:** 933 lines (exported `determineGraphBackedQuestion`) + ~215 lines inlined within applyValidatedProposal
|
||||
- **Primary responsibility:** Unknown candidate eligibility filtering; reasoning pattern compatibility scoring; decomposition child selection; reseat-after-rejection; model-selected target preference via depends_on prerequisite check; sibling ordering tiebreakers
|
||||
- **Pure / impure / mixed:** Mixed — reads graph, returns selection result (no mutations)
|
||||
- **Depons heavily on applyValidatedProposal locals:** NO — the exported `determineGraphBackedQuestion` is fully self-contained. The inlined 257 lines at 3860–4121 are orchestration glue that depends on decompositionResult/propagationResult locals.
|
||||
- **Existing focused tests:** STRONG (60B.42 active selector guard: 5 tests; 60B.11 prerequisite-aware targeting: 9 tests; selectedQuestion lifecycle in 60B.43/64)
|
||||
|
||||
### Answer semantic validation (lines ~3267–3454)
|
||||
- **Approx line count:** 297 lines (validateAnswerMeaningCompatibilityWithRawAnswer + validateAnswerMeaningAlignment helpers)
|
||||
- **Primary responsibility:** Raw answer → userSupportedMeaning alignment verification; unclassified meaning support detection; hard constraint boundary language analysis; conditional qualification preservation
|
||||
- **Pure / impure / mixed:** Mostly pure — reads answer + proposal, returns errors array
|
||||
- **Depends heavily on applyValidatedProposal locals:** NO — operates on `answer` + `proposal` only
|
||||
- **Existing focused tests:** MEDIUM (regression A/B/C suites test the path end-to-end but don't isolate the helpers)
|
||||
|
||||
### Graph mutation (applyGraphUpdate import from utils.js)
|
||||
- **Approx line count:** ~0 in apply-proposal.js (imported)
|
||||
- **Primary responsibility:** The single mutation point — applies node updates, resolves nodes, adds/removes edges
|
||||
- **Pure / impure / mixed:** Pure mutation function
|
||||
|
||||
### Final selectedQuestion lifecycle (lines ~4152–4312 within applyValidatedProposal)
|
||||
- **Approx line count:** ~160 lines of inlined orchestration
|
||||
- **Primary responsibility:** Compose finalSelectedQuestion from deterministicSelection + formulatedQuestion; repeated-question rejection + reseat; effectiveSelectedQuestion composition
|
||||
- **Pure / impure / mixed:** Mixed — reads multiple locals, returns selected question or null
|
||||
- **Depends heavily on applyValidatedProposal locals:** YES — tight coupling with deterministicSelection, proposedNode, decompositionResult
|
||||
|
||||
### Supporting pure helpers (lines 38–569)
|
||||
- **Approx line count:** ~530 lines
|
||||
- **Primary responsibility:** JSON cloning, Zod error formatting, edge duplicate detection, text normalization, node lookup, compound question detection, confidence assessment, branch conflict signature computation, token overlap utilities
|
||||
- **Pure / impure / mixed:** All pure — no side effects
|
||||
- **Depends heavily on applyValidatedProposal locals:** NO
|
||||
|
||||
---
|
||||
|
||||
## Orchestration vs Extractable Logic
|
||||
|
||||
```text
|
||||
proposal reconciliation: GOOD EXTRACTION CANDIDATE (pure on proposal+graph)
|
||||
proposal compatibility val: GOOD EXTRACTION CANDIDATE (complex but stateless)
|
||||
answer semantic validation: POSSIBLE LATER (good candidate but lower priority)
|
||||
resolution propagation: ALREADY EXTRACTED (standalone export)
|
||||
active unknown / target sel: ALREADY PARTIALLY EXTRACTED (determineGraphBackedQuestion is standalone; inlined orchestration stays)
|
||||
final selectedQuestion: SHOULD STAY IN apply-proposal.js (tightly coupled to deterministicSelection lifecycle)
|
||||
graph mutation: MUST STAY IN apply-proposal.js (single ownership point)
|
||||
supporting pure helpers: POSSIBLE LATER (large cluster, but low edit frequency)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Candidate Assessment
|
||||
|
||||
### Candidate A — Proposal Reconciliation (`reconcileResolutionSemantics`)
|
||||
- **Approx removable lines:** ~90 (lines 320–409)
|
||||
- **Semantic-change risk:** LOW — pure function on proposal object; existing tests cover all paths
|
||||
- **Coupling:** LOW — takes graph + proposal; returns {proposal, errors}
|
||||
- **Test coverage:** STRONG — 6 dedicated integration tests in 60B.49
|
||||
- **Context reduction:** MEDIUM — removes 90 lines of self-contained logic
|
||||
- **Future edit-frequency:** LOW — stable reconciliation rules unlikely to change
|
||||
- **Lifecycle clarity after extraction:** BETTER — apply-proposal.js pre-validation flow becomes a clear sequence of named steps
|
||||
- **Principal risk:** Must verify every edge case (selectedQuestion nuke on resolution, bidirectional update-node/ID consistency) is captured in the new module's tests
|
||||
|
||||
### Candidate B — Proposal Compatibility Validation
|
||||
- **Approx removable lines:** ~130 (lines 3581–3648 inline within applyValidatedProposal)
|
||||
- **Semantic-change risk:** LOW — stateless validation logic; all callers pass through same helpers
|
||||
- **Coupling:** MEDIUM — imports `validateGraphUpdate` from utils.js and calls other internal helpers
|
||||
- **Test coverage:** STRONG — 25+ tests across multiple suites exercise every validation path
|
||||
- **Context reduction:** HIGH — removes the largest single block of inline logic from applyValidatedProposal, splitting it into a named pre-check step
|
||||
- **Future edit-frequency:** MEDIUM — schema-driven, may need updates when graph schema evolves
|
||||
- **Lifecycle clarity after extraction:** BETTER — `validateProposalCompatibility()` becomes a single readable call replacing 7+ individual validation pushes
|
||||
- **Principal risk:** Must preserve exact error aggregation order and deduplication semantics across the extracted validator
|
||||
|
||||
### Candidate C — Resolution Propagation
|
||||
- **Already extracted as standalone export (lines 902–1261)**
|
||||
- **No remaining inline logic to extract**
|
||||
|
||||
### Candidate D — Active Unknown / Target Selection
|
||||
- **Approx removable lines:** ~215 (inlined orchestration at 3870–4121 within applyValidatedProposal)
|
||||
- **Semantic-change risk:** MEDIUM — the inlined block has many local-variable side effects and interacts with decompositionResult/propagationResult state
|
||||
- **Coupling:** HIGH — deeply reads locals from applyValidatedProposal; recomputes deterministicSelection multiple times
|
||||
- **Test coverage:** STRONG (exported function); but inlined orchestration has MEDIUM test coverage
|
||||
- **Context reduction:** MEDIUM
|
||||
- **Future edit-frequency:** LOW-MEDIUM
|
||||
- **Lifecycle clarity after extraction:** WORSE — would separate the "post-propagation reselection decision" from its governing state variables across function boundary
|
||||
- **Principal risk:** Extracting the inlined orchestration block would scatter the candidate selection logic across multiple function boundaries, making it harder to trace the active unknown lifecycle
|
||||
|
||||
### Candidate E — Answer Semantic Validation
|
||||
- **Approx removable lines:** ~297 (validateAnswerMeaningCompatibilityWithRawAnswer + validateAnswerMeaningAlignment at lines 3267–3454)
|
||||
- **Semantic-change risk:** LOW — mostly pure text analysis
|
||||
- **Coupling:** LOW — operates on answer + proposal only
|
||||
- **Test coverage:** MEDIUM — tested end-to-end but not as isolated unit tests for the helpers
|
||||
- **Context reduction:** MEDIUM
|
||||
- **Future edit-frequency:** MEDIUM — answer semantics may evolve with prompt changes
|
||||
- **Lifecycle clarity after extraction:** BETTER
|
||||
- **Principal risk:** Answer semantics is tightly coupled to prompt contract; extraction alone doesn't reduce orchestration complexity in applyValidatedProposal
|
||||
|
||||
---
|
||||
|
||||
## Mutation Ownership
|
||||
|
||||
```text
|
||||
Can mutation ownership remain central while extracting candidate modules: YES
|
||||
|
||||
applyGraphUpdate(...) invocation — MUST stay (single mutation entry point)
|
||||
proposalSnapshot lifecycle — MUST stay (built up locally, passed to mutation)
|
||||
updatedSituationGraph lifecycle — MUST stay (accumulates mutation state across pipeline stages)
|
||||
resolvedUnknownNodeIds bookkeeping — MUST stay (derived from proposalSnapshot.resolvedUnknownNodeIds)
|
||||
activeUnknownNodeId mutation — MUST stay (tied to post-mutation candidate reselection lifecycle)
|
||||
selectedQuestion finalisation — MUST stay (composed from deterministicSelection + formulatedQuestion in same scope)
|
||||
```
|
||||
|
||||
The key insight: all mutations flow through `applyGraphUpdate(graphSnapshot, proposalSnapshot)`. Once extracted modules return their outputs, the mutation remains a single point of truth. Extraction of validation/reconciliation doesn't fragment mutation ownership because these are pre-mutation checks that operate on copies/clones.
|
||||
|
||||
---
|
||||
|
||||
## Ranking
|
||||
|
||||
1. **Candidate B — Proposal compatibility validation** (highest context reduction, strongest tests, LOW semantic risk, removes largest inline logic block)
|
||||
2. **Candidate A — Proposal reconciliation** (LOW risk, STRONG tests, self-contained, but fewer lines than B)
|
||||
3. **Candidate E — Answer semantic validation** (pure text analysis, good candidate but lower priority)
|
||||
4. **Candidate D — Active unknown / target selection** (HIGH coupling to applyValidatedProposal locals makes it a weak extraction candidate despite strong tests)
|
||||
5. **Supporting pure helpers** (LOW edit frequency; not worth the abstraction cost)
|
||||
|
||||
---
|
||||
|
||||
## Strategy Assessment
|
||||
|
||||
### Strategy A — ONE EXTRACTION ONLY
|
||||
|
||||
Extract Candidate B (validation), verify, stop.
|
||||
|
||||
```text
|
||||
Risk: LOW
|
||||
Expected line reduction: ~130 lines from apply-proposal.js (now ~4,350)
|
||||
Expected context reduction: HIGH — removes the largest single inline logic block
|
||||
Semantic-drift risk: LOW — stateless validation functions are easy to extract correctly
|
||||
```
|
||||
|
||||
### Strategy B — TWO SMALL EXTRACTIONS
|
||||
|
||||
Extract A + B in separate commits. Both are independent pure-checking modules with STRONG test coverage.
|
||||
|
||||
```text
|
||||
Risk: LOW (two independent, low-risk extractions)
|
||||
Expected line reduction: ~220 lines total (~4,260 remaining)
|
||||
Expected context reduction: HIGH — two clear named pre-validation steps replace inline logic
|
||||
Semantic-drift risk: LOW (both have STRONG test coverage and pure/mixed character)
|
||||
```
|
||||
|
||||
### Strategy C — LARGE APPLY-PROPOSAL DECOMPOSITION
|
||||
|
||||
Break apply-proposal.js into several lifecycle modules now.
|
||||
|
||||
```text
|
||||
Risk: MEDIUM-HIGH — too many extraction points to verify in one pass; risk of scattering orchestration awareness across modules
|
||||
Expected line reduction: ~600+ lines (aggressive)
|
||||
Semantic-drift risk: MEDIUM — more boundaries to cross during verification
|
||||
```
|
||||
|
||||
### Strategy D — STOP REFACTORING
|
||||
|
||||
Current structure is good enough.
|
||||
|
||||
```text
|
||||
Risk: LOW (no risk)
|
||||
But 4,480 lines still has one ~990-line function with 15+ phases of inline logic
|
||||
Context reduction: NONE
|
||||
Semantic-drift risk: NONE
|
||||
Future Claude/Codex context cost: HIGH — every session loads all 4,480 lines
|
||||
```
|
||||
|
||||
**Chosen: Strategy B — TWO SMALL EXTRACTIONS in separate commits**
|
||||
|
||||
Rationale: Candidates A and B are independent pure-checking modules with STRONG test coverage. Extracting both gives ~220 lines of reduction for minimal risk. Candidate D is excluded because its HIGH coupling to orchestration locals makes it a weak extraction candidate despite strong tests.
|
||||
|
||||
---
|
||||
|
||||
## File Size Estimates
|
||||
|
||||
```text
|
||||
Current apply-proposal.js lines: 4,480
|
||||
After reconciliation extraction (A): ~4,390 (-90)
|
||||
After compatibility validation extraction (B): ~4,260 (-220 total)
|
||||
Reasonable medium-term target: ~4,250-4,300
|
||||
|
||||
Why not lower? Because apply-proposal.js must retain:
|
||||
- Lifecycle ordering visibility (~150 lines of orchestration scaffolding)
|
||||
- Graph mutation ownership (applyGraphUpdate invocation + proposalSnapshot buildup)
|
||||
- Active target reselection lifecycle (~260 lines, partially extracted already)
|
||||
- Final selectedQuestion composition (~160 lines)
|
||||
|
||||
Target of ~4,250-4,300 reflects a clear orchestration file — not tiny wrapper-only, not giant mixed-responsibility.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Critical Distinction
|
||||
|
||||
**Choice: B — validation should be next**
|
||||
|
||||
Why: Candidate B removes the largest single inline logic block (~130 lines) that currently scatters 7+ validation calls across applyValidatedProposal's pre-mutation phase. Extracting `validateProposalCompatibility()` into its own module gives the highest context reduction per line extracted. Both A and B are equally justified as clean extractions, but B has higher priority because:
|
||||
1. It removes more lines (130 vs 90)
|
||||
2. The validation block in applyValidatedProposal is visually dominant — it obscures the post-validation lifecycle
|
||||
3. STRONG test coverage across multiple independent suites (60B.49/61/64/structured-fidelity/boundary/edge-case)
|
||||
4. No new tests needed for extraction — existing integration tests provide sufficient boundary coverage
|
||||
|
||||
---
|
||||
|
||||
## Minimum Next Refactor Boundary
|
||||
|
||||
**Choice: B — one new validation module**
|
||||
|
||||
Why: Extract `validateProposalCompatibility(graph, proposal)` as a single function that encapsulates all 8 pre-mutation validations currently scattered across applyValidatedProposal. The extracted function takes the same inputs (`graph`, `proposal`) and returns `{valid, errors}`. This matches the existing pattern established by decision-sufficiency.js extraction (pure logic out, mutation stays).
|
||||
|
||||
---
|
||||
|
||||
## Refactor Timing
|
||||
|
||||
**Choice: B — RETURN TO REASONING WORK FIRST**
|
||||
|
||||
Why: Experiment 60B.70 is a read-only audit with no implementation directive. The highest-value next action is completing this documentation and returning to active reasoning work. A future session can implement the validation extraction when there's a natural editing context (e.g., when schema changes require touching that validation layer anyway). Forcing an extraction without a natural editing trigger increases semantic drift risk because there's no external pressure ensuring the extraction serves a real need.
|
||||
|
||||
---
|
||||
|
||||
## Verification
|
||||
|
||||
- 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
|
||||
Reference in New Issue
Block a user