|
| 1 | +# Typed Contract Audit and Unification Plan |
| 2 | + |
| 3 | +## Goal |
| 4 | + |
| 5 | +Audit all `Typed*` / `typed_*` contracts and evaluate whether we can move to a unified model with neutral names (no type-signaling prefixes/suffixes). |
| 6 | + |
| 7 | +## Scope audited |
| 8 | + |
| 9 | +- `crates/solverforge-core` |
| 10 | +- `crates/solverforge-solver` |
| 11 | +- `crates/solverforge-scoring` |
| 12 | +- Macro-generated call sites in `crates/solverforge-macros` |
| 13 | +- Public re-exports in `crates/solverforge` |
| 14 | + |
| 15 | +## What exists today |
| 16 | + |
| 17 | +### 1) `TypedEntityExtractor` (core) |
| 18 | + |
| 19 | +- Concrete implementation of the dynamic `EntityExtractor` boundary. |
| 20 | +- Exists to bridge: |
| 21 | + - runtime descriptor storage (`Box<dyn EntityExtractor>`), and |
| 22 | + - concrete solution/entity access via `fn(&S) -> &Vec<E>` and `fn(&mut S) -> &mut Vec<E>`. |
| 23 | +- This is an **intentional type-erasure boundary** in current architecture, not accidental abstraction. |
| 24 | + |
| 25 | +### 2) `typed_value.rs` (solver) |
| 26 | + |
| 27 | +- Defines: |
| 28 | + - `TypedValueSelector<S, V>` |
| 29 | + - `StaticTypedValueSelector<S, V>` |
| 30 | + - `FromSolutionTypedValueSelector<S, V>` |
| 31 | + - `RangeValueSelector<S>` |
| 32 | +- Functionally this is the default value selector model in the hot path. The `Typed` prefix is mostly naming noise. |
| 33 | + |
| 34 | +### 3) `typed_move_selector.rs` (solver) |
| 35 | + |
| 36 | +- File/module name contains `typed_`, but the key trait is already neutral (`MoveSelector<S, M>`). |
| 37 | +- This is already aligned with a unified model semantically; naming drift remains in module/file names and comments. |
| 38 | + |
| 39 | +### 4) Scoring references to “typed” |
| 40 | + |
| 41 | +- Mostly descriptive text/comments (`typed director`, `typed undo`) rather than separate prefixed public contract types. |
| 42 | +- This is conceptual language and can be normalized independently from API renaming. |
| 43 | + |
| 44 | +## Assessment of the claim |
| 45 | + |
| 46 | +Your claim is directionally correct for naming and public ergonomics: |
| 47 | + |
| 48 | +- The “typed” prefix is largely redundant in a generic Rust API where static typing is already explicit in signatures. |
| 49 | +- Prefix-heavy naming obscures the true model: there is one selector/extractor model with an intentional erased boundary where needed. |
| 50 | + |
| 51 | +However, one caveat is important: |
| 52 | + |
| 53 | +- We **cannot remove all wrappers** outright. `EntityDescriptor` requires an object-safe boundary (`Box<dyn EntityExtractor>`). A concrete adapter type is still needed; only its name/placement should change. |
| 54 | + |
| 55 | +## Recommended unified naming |
| 56 | + |
| 57 | +### Core |
| 58 | + |
| 59 | +- `TypedEntityExtractor<S, E>` → `EntityCollectionExtractor<S, E>` |
| 60 | + |
| 61 | +Rationale: describes the concrete responsibility (extracting an entity collection) without repeating Rust's type system. |
| 62 | + |
| 63 | +### Solver value selectors |
| 64 | + |
| 65 | +- `TypedValueSelector<S, V>` → `ValueSelector<S, V>` |
| 66 | +- `StaticTypedValueSelector<S, V>` → `StaticValueSelector<S, V>` |
| 67 | +- `FromSolutionTypedValueSelector<S, V>` → `SolutionValueSelector<S, V>` |
| 68 | +- `typed_value.rs` → `value_selector.rs` |
| 69 | + |
| 70 | +### Solver move selector module |
| 71 | + |
| 72 | +- `typed_move_selector.rs` → `move_selector.rs` |
| 73 | +- Keep trait names already neutral (`MoveSelector`, `ChangeMoveSelector`, `SwapMoveSelector`, etc.). |
| 74 | + |
| 75 | +## Migration strategy (no compatibility shims) |
| 76 | + |
| 77 | +Because repository policy disallows transitional APIs, do this in a single coherent refactor: |
| 78 | + |
| 79 | +1. Rename files/modules first (`typed_value` → `value_selector`, `typed_move_selector` → `move_selector`). |
| 80 | +2. Rename public types above and update all imports/usages. |
| 81 | +3. Update macro expansion references in `planning_solution.rs` from `TypedEntityExtractor` to the new core name. |
| 82 | +4. Update facade re-exports (`solverforge/src/lib.rs`). |
| 83 | +5. Update wireframes: |
| 84 | + - `crates/solverforge-core/WIREFRAME.md` |
| 85 | + - `crates/solverforge-solver/WIREFRAME.md` |
| 86 | + - `crates/solverforge/WIREFRAME.md` |
| 87 | +6. Sweep docs/comments to remove wording that suggests two separate models (“typed vs non-typed”) where there is only one runtime path. |
| 88 | + |
| 89 | +## Risk profile |
| 90 | + |
| 91 | +- **Compile-time breakage:** high during rename, low after full migration. |
| 92 | +- **Runtime behavior risk:** low if mechanical rename only. |
| 93 | +- **API break:** intentional and acceptable per change policy (no shims). |
| 94 | + |
| 95 | +## Conclusion |
| 96 | + |
| 97 | +- A unified, prefix-free surface is feasible and aligns with the architecture. |
| 98 | +- Keep the concrete extractor adapter, but rename it to reflect purpose rather than “typedness”. |
| 99 | +- The largest value comes from renaming solver selector modules/types and updating wireframes in lockstep. |
0 commit comments