Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 183 additions & 0 deletions PRODUCTION_REVIEW_2026-03-20.md
Original file line number Diff line number Diff line change
@@ -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.
153 changes: 153 additions & 0 deletions PRODUCTION_REVIEW_ISSUES_2026-03-20.md
Original file line number Diff line number Diff line change
@@ -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.
Loading