diff --git a/PRODUCTION_REVIEW_2026-03-20.md b/PRODUCTION_REVIEW_2026-03-20.md new file mode 100644 index 00000000..25338623 --- /dev/null +++ b/PRODUCTION_REVIEW_2026-03-20.md @@ -0,0 +1,183 @@ +# SolverForge production review — 2026-03-20 + +## Scope + +This review audits every workspace crate individually for completeness, coherence, production readiness, and obvious dead code / wrappers / stubs / duplicates. + +## Workspace-level assessment + +The workspace architecture is coherent: the top-level manifest cleanly separates core types, proc macros, scoring, solver engine, config, console output, façade crate, CVRP helpers, shared test fixtures, and the CLI. As a codebase, it feels organized and test-heavy. As a product, it is close, but not uniformly production-ready across all crates. + +### Global strengths +- Clear crate boundaries and layered architecture. +- Strong automated test coverage in the core solver/scoring crates. +- Passing workspace tests and doctests. + +### Global blockers +1. Strict Clippy does not currently pass because of `module_inception` warnings in two CLI test modules. +2. The CLI still exposes intentionally incomplete commands and templates. +3. Generated code still contains `todo!()` placeholders/stubs that require manual cleanup. +4. Some smaller/public helper crates are lightly or indirectly tested compared with the core runtime crates. + +## Crate-by-crate audit + +### 1. `solverforge-core` +**Role:** foundational domain traits, descriptors, errors, and score types. + +**Assessment:** strongest crate in the workspace; looks production-ready from a structure and validation standpoint. + +**Why:** +- Small, focused public surface. +- Broad internal test coverage under the crate. +- No obvious stubs or placeholder logic found. + +**Suggested follow-up:** keep this crate as the quality bar for the rest of the workspace. + +### 2. `solverforge-macros` +**Role:** attribute/derive macros for planning entities, facts, and solutions. + +**Assessment:** coherent and likely usable, but less directly validated than the runtime crates. + +**Why:** +- Public API is compact and understandable. +- No obvious placeholders/stubs in the macro entrypoints. +- Validation appears to come mostly from downstream tests rather than macro-crate-local tests. + +**Suggested follow-up:** add direct macro-focused golden tests / compile-fail tests to reduce regression risk. + +### 3. `solverforge-scoring` +**Role:** incremental scoring engine and fluent constraint stream API. + +**Assessment:** high-confidence crate; appears close to production-ready. + +**Why:** +- Clear zero-erasure design and public exports. +- Large number of focused tests across constraints, directors, streams, analysis, and collectors. +- No placeholder/stub code found. + +**Suggested follow-up:** mostly documentation/performance hardening, not architectural cleanup. + +### 4. `solverforge-solver` +**Role:** solver engine, phases, heuristics, selectors, runtime orchestration. + +**Assessment:** technically strong and heavily exercised; likely production-capable, though large and therefore deserving continued hardening. + +**Why:** +- Broad feature set with a correspondingly large test surface. +- Good modular separation between moves, selectors, phases, terminations, manager/builder layers, and realtime hooks. +- No obvious placeholder/stub logic found. + +**Suggested follow-up:** keep investing in docs/examples/benchmarks because this is the highest-complexity crate. + +### 5. `solverforge-config` +**Role:** TOML/YAML-driven solver configuration. + +**Assessment:** small, coherent, and likely production-ready for its scope. + +**Why:** +- Good crate fit: narrowly scoped and easy to reason about. +- Has examples and tests. +- No placeholder/stub logic found. + +**Suggested follow-up:** add more malformed-config cases over time, but no major red flags. + +### 6. `solverforge-console` +**Role:** tracing-based console/log formatting layer. + +**Assessment:** useful but under-validated. + +**Why:** +- Public surface is tiny and conceptually coherent. +- The crate has no visible test module/files. +- `init()` configures global subscriber state, which is exactly the sort of code that benefits from focused smoke tests. + +**Suggested follow-up:** add tests for idempotent init, filter wiring, and formatting smoke coverage. + +### 7. `solverforge` +**Role:** façade crate that re-exports the main user-facing API. + +**Assessment:** coherent and convenient, but inherits the uneven readiness of the crates beneath it. + +**Why:** +- Strong façade design: re-exports macros, scores, stream API, solver types, optional console, and CVRP helpers. +- Includes user-facing docs and integration tests. +- Its production readiness is constrained by weaker subcomponents such as CLI-generated experience and CVRP helpers. + +**Suggested follow-up:** keep tightening the public contract and document which modules/features are stable. + +### 8. `solverforge-cvrp` +**Role:** domain helpers for list-based vehicle-routing examples/solvers. + +**Assessment:** weakest runtime/helper crate in the workspace; not yet at the same production bar as core/scoring/solver. + +**Why:** +- Entire crate is a single file with no visible test module/files. +- Uses `*const ProblemData` plus repeated `unsafe` dereferences based on trait-level guarantees. +- Contains duplicate thin wrappers: `assign_route` and `set_route` do the same thing; `get_route` is also a minimal convenience wrapper. + +**Suggested follow-up:** +- Add direct unit tests. +- Consolidate duplicate route helpers. +- Consider a safer ownership/reference pattern or, at minimum, much stronger safety documentation. + +### 9. `solverforge-test` +**Role:** shared fixtures for testing other crates. + +**Assessment:** coherent internal-support crate; fit for purpose. + +**Why:** +- Clear purpose and narrow scope. +- Contains self-tests for fixture behavior. +- No obvious dead code/stub issues. + +**Suggested follow-up:** low priority unless fixture complexity grows. + +### 10. `solverforge-cli` +**Role:** scaffolding and project-management CLI. + +**Assessment:** least production-ready crate in the workspace. + +**Why:** +- Strict Clippy fails because of `module_inception` warnings in two test modules. +- The CLI advertises an interactive console command that is explicitly a stub. +- The generic `--list` scaffold is advertised as “coming soon” and intentionally unavailable. +- Generated constraint/domain code includes multiple `todo!()` placeholders and a stub data loader. + +**Dead code / duplication / polish issues:** +- The nested `mod tests` pattern is a lint-only issue but is easy cleanup. +- The public CLI surface currently includes unfinished paths that should be hidden, gated, or finished. +- Generated output is useful for scaffolding but not yet polished enough to feel production-grade. + +**Suggested follow-up:** +- Make Clippy pass. +- Remove/gate unfinished commands. +- Improve generated code so new projects compile cleanly or fail with much more guided placeholders. + +## Production-readiness ranking + +### Ready or nearly ready +- `solverforge-core` +- `solverforge-scoring` +- `solverforge-solver` +- `solverforge-config` +- `solverforge-test` + +### Usable but needs more validation/polish +- `solverforge-macros` +- `solverforge-console` +- `solverforge` + +### Not yet at the same production bar +- `solverforge-cvrp` +- `solverforge-cli` + +## Highest-value improvements + +1. Fix the existing Clippy failures in `solverforge-cli`. +2. Remove or gate incomplete CLI functionality (`console`, generic list scaffold). +3. Upgrade scaffolding output so generated projects are closer to compile-and-run quality. +4. Add direct tests for `solverforge-console`, `solverforge-cvrp`, and macro expansion edge cases. +5. Reduce unnecessary API surface in `solverforge-cvrp` by consolidating duplicate wrappers. +6. Publish a stability/readiness matrix for crates and major features. + +See also `PRODUCTION_REVIEW_ISSUES_2026-03-20.md` for the issue-by-issue breakdown that can be opened and tracked. diff --git a/PRODUCTION_REVIEW_ISSUES_2026-03-20.md b/PRODUCTION_REVIEW_ISSUES_2026-03-20.md new file mode 100644 index 00000000..55252fdc --- /dev/null +++ b/PRODUCTION_REVIEW_ISSUES_2026-03-20.md @@ -0,0 +1,153 @@ +# Suggested issues from the production review — 2026-03-20 + +This file turns the findings from `PRODUCTION_REVIEW_2026-03-20.md` into a concrete set of issues that can be opened and tracked. + +--- + +## Issue 1 — Make `solverforge-cli` pass strict Clippy + +- **Priority:** P0 +- **Affected crates:** `solverforge-cli` +- **Why this matters:** the documented strict lint gate currently fails, which weakens confidence in release readiness and CI signal quality. +- **Problem statement:** `cargo clippy --workspace --all-targets -- -D warnings` fails because two test modules trigger `clippy::module_inception`. +- **Suggested work:** + - Rename the nested CLI test modules or flatten them. + - Run Clippy again and make the workspace lint-clean. + - Add a CI/local validation step that keeps this from regressing. +- **Acceptance criteria:** + - `cargo clippy --workspace --all-targets -- -D warnings` passes. + - No `#[allow(clippy::module_inception)]` escape hatch is added unless documented and justified. + +## Issue 2 — Gate or implement the `solverforge console` command + +- **Priority:** P1 +- **Affected crates:** `solverforge-cli`, possibly `solverforge-console` +- **Why this matters:** users see a public command that explicitly says it is not available yet. +- **Problem statement:** `solverforge console` is exposed in the CLI but currently only prints that the feature is not yet available. +- **Suggested work:** + - Either implement a minimal supported interactive workflow, or remove/gate the command until it exists. + - Align CLI help text and generated documentation with the actual support level. +- **Acceptance criteria:** + - The command is either functional and supported, or no longer exposed as a normal user-facing command. + - Help output does not advertise unfinished functionality as if it were complete. + +## Issue 3 — Finish or hide the generic list-variable scaffold + +- **Priority:** P1 +- **Affected crates:** `solverforge-cli` +- **Why this matters:** `--list` is advertised to users but intentionally errors at runtime. +- **Problem statement:** the generic list-variable template is marked “coming soon” and is not actually available. +- **Suggested work:** + - Implement the generic list scaffold, or + - Hide/remove the flag until the template exists. +- **Acceptance criteria:** + - `solverforge new --list` either succeeds with a supported template or is not presented as an available option. + - CLI docs/examples match runtime behavior. + +## Issue 4 — Make generated constraints compile without raw `todo!()` placeholders + +- **Priority:** P1 +- **Affected crates:** `solverforge-cli` +- **Why this matters:** generated code that panics or does not compile cleanly feels unfinished and slows first-time users. +- **Problem statement:** generated constraint skeletons include multiple `todo!()` placeholders in executable code paths. +- **Suggested work:** + - Replace raw `todo!()` calls with safer compile-time placeholders, explicit `unimplemented!()` markers with better guidance, or commented examples that keep the generated crate buildable. + - Improve post-generation instructions. +- **Acceptance criteria:** + - Newly generated constraints are clearer and safer for end users. + - The generated code path is intentionally designed, documented, and tested. + +## Issue 5 — Replace scaffolded data-loader stubs with compile-safe templates + +- **Priority:** P1 +- **Affected crates:** `solverforge-cli` +- **Why this matters:** generated projects should be as close as possible to compile-and-run quality. +- **Problem statement:** domain generation rewrites `src/data/mod.rs` with a `todo!("Implement data loading")` stub. +- **Suggested work:** + - Generate a compile-safe placeholder implementation. + - Add clear comments for where users should insert domain-specific loading code. + - Add scaffold smoke tests covering this path. +- **Acceptance criteria:** + - Generated projects compile cleanly after domain generation, or failures are deliberate, obvious, and well-explained. + - Scaffold tests cover the generated data module. + +## Issue 6 — Add direct unit tests for `solverforge-cvrp` + +- **Priority:** P1 +- **Affected crates:** `solverforge-cvrp` +- **Why this matters:** the crate exposes public helpers used in routing scenarios but currently appears untested directly. +- **Problem statement:** `solverforge-cvrp` has no visible direct test module/files despite containing nontrivial routing/time-feasibility logic. +- **Suggested work:** + - Add unit tests for distance helpers, route assignment helpers, and time-window feasibility. + - Add edge-case tests for empty fleets, invalid route positions, and time-window boundaries. +- **Acceptance criteria:** + - `solverforge-cvrp` has direct tests in the crate. + - Core helper behavior is covered by deterministic unit tests. + +## Issue 7 — Reduce unsafe surface and duplicate wrappers in `solverforge-cvrp` + +- **Priority:** P1 +- **Affected crates:** `solverforge-cvrp`, `solverforge` +- **Why this matters:** raw-pointer-based helpers and redundant wrappers raise maintenance and safety risk in a public helper API. +- **Problem statement:** the crate relies on `*const ProblemData` and repeated `unsafe` dereferences, while also exposing duplicate thin wrappers such as `assign_route` and `set_route`. +- **Suggested work:** + - Consolidate duplicate route helpers. + - Document safety invariants much more rigorously. + - Investigate safer ownership/reference patterns where possible. + - Re-evaluate what should be publicly re-exported from the façade crate. +- **Acceptance criteria:** + - Duplicate wrapper surface is reduced. + - Safety invariants are explicit and tested. + - The public API is easier to reason about. + +## Issue 8 — Add direct tests for `solverforge-console` + +- **Priority:** P2 +- **Affected crates:** `solverforge-console` +- **Why this matters:** global tracing/subscriber setup is subtle and easy to regress even in a small crate. +- **Problem statement:** `solverforge-console` appears to have no direct tests. +- **Suggested work:** + - Add smoke tests for idempotent initialization. + - Add tests for filter defaults and formatting behavior where practical. +- **Acceptance criteria:** + - The crate has at least a minimal direct test suite. + - Repeated initialization behavior is explicitly covered. + +## Issue 9 — Add compile-fail / golden tests for `solverforge-macros` + +- **Priority:** P2 +- **Affected crates:** `solverforge-macros`, possibly `solverforge` +- **Why this matters:** proc macros benefit from direct expansion/regression tests rather than relying only on downstream behavior. +- **Problem statement:** macro validation appears to be mostly indirect. +- **Suggested work:** + - Add compile-pass and compile-fail tests for key macro attributes. + - Add snapshot/golden tests for expected expansion behavior where practical. +- **Acceptance criteria:** + - Macro-specific regression coverage exists. + - Common authoring mistakes produce stable, intentional diagnostics. + +## Issue 10 — Publish a crate stability matrix and release checklist + +- **Priority:** P2 +- **Affected crates:** workspace-wide +- **Why this matters:** users need to know which crates/features are mature, experimental, or template-oriented. +- **Problem statement:** readiness varies significantly across crates, but that variance is not captured in a standard release/stability document. +- **Suggested work:** + - Add a workspace-level matrix for stable/experimental/template-only surfaces. + - Define a release checklist that includes `fmt`, `clippy`, `test`, and scaffold smoke validation. +- **Acceptance criteria:** + - Stability/readiness expectations are documented. + - Release validation is repeatable and explicit. + +## Issue 11 — Add scaffold smoke tests for generated projects and workflows + +- **Priority:** P2 +- **Affected crates:** `solverforge-cli` +- **Why this matters:** the CLI is user-facing and should be validated end-to-end, not only by string/template unit tests. +- **Problem statement:** current validation catches many pieces, but there is still a gap between generated output and polished compile-ready projects. +- **Suggested work:** + - Add smoke tests for `solverforge new`, entity/fact/constraint generation, and follow-on `cargo check` in representative cases. + - Cover both basic and list-oriented project flows as they mature. +- **Acceptance criteria:** + - Representative generated projects are exercised in automated tests. + - Regressions in scaffolding behavior are caught before release.