# Rust Architecture Review 8

Date: 2026-04-04

## Rating

9.6/10

## Executive Summary

The Review 7 checklist is now implemented.

This codebase is back in the upper band established by the earlier architecture reviews, and the difference is justified in code, not just in commentary:

- metadata-to-implementation traceability is now actively verified
- stale `binding_location` entries were corrected across Python, Go, and Rust catalog entries
- fail-open meta-tests were converted to fail-closed traversal with explicit inventory counts
- repository config handling now rejects unknown keys instead of silently discarding user intent
- the ad hoc timestamp-based temp workspace pattern was replaced with `tempfile`-based RAII cleanup in the review-targeted areas

The project is not a 10/10 yet, but it is now materially stronger than Review 7 because the previously identified weaknesses were addressed at the root rather than documented and deferred.

## Validation Snapshot

- [x] `cargo test --quiet` passed
- [x] 303 tests passed, 1 test ignored, 0 failed
- [x] `make lint` passed
- [x] `make scan-gopdfsuit` ran and produced `temp_gopdfsuit.txt`
- [x] `make scan-gopdfsuit` returned non-zero because `deslop scan` exits non-zero when findings are present

Observed `scan-gopdfsuit` summary from `temp_gopdfsuit.txt`:

- source files discovered: 125
- source files analyzed: 125
- functions fingerprinted: 876
- findings: 1622
- parse failures: 0

That non-zero make result is expected product behavior, not a runtime failure.

## What Was Improved

### 1. Binding metadata is now trustworthy again

- [x] Python `hot_path` catalog entries now point at the real implementation files
- [x] Python public family names were normalized so `hot_path` and `hotpath` no longer both leak into metadata
- [x] stale Go binding paths were updated to the current `framework_patterns` and `library_misuse` layout
- [x] stale Rust hygiene bindings were updated to the current `analysis/rust` implementation files
- [x] a full-catalog binding-location existence test now verifies every entry points to a real Rust source file
- [x] a targeted regression test now protects the previously broken Python `hot_path` bindings

Impact:

- rule metadata is again suitable for CLI, documentation, and maintenance workflows
- future refactors are more likely to fail in tests instead of silently drifting metadata

### 2. Meta-tests now fail closed

- [x] `tests/parser_corpus_regression.rs` no longer silently ignores traversal failures
- [x] `src/rules.rs` source-rule inventory traversal no longer silently skips unreadable directories or files
- [x] explicit expected counts were added for regression corpus inventory and source rule-id inventory

Impact:

- coverage shrinkage is now visible
- CI signal is stricter and more honest

### 3. Config policy is now explicit

- [x] `.deslop.toml` now uses `deny_unknown_fields`
- [x] unknown keys are treated as parse errors instead of silently ignored input
- [x] tests were updated to enforce the new policy

Impact:

- operator intent is protected
- typo-driven misconfiguration is much less likely to slip through unnoticed

### 4. Test temp workspace handling is cleaner

- [x] `tests/support/mod.rs` now uses `tempfile::TempDir`
- [x] the review-targeted temp-dir helpers in `src/config.rs`, `src/scan/tests.rs`, `src/scan/walker.rs`, `src/benchmark/mod.rs`, and `src/io.rs` were migrated away from timestamp-based manual paths
- [x] cleanup now relies on RAII rather than repeated `remove_dir_all` blocks in those areas

Impact:

- less noisy test code
- lower cleanup risk
- fewer lifecycle bugs in temporary workspace handling

## What Can Still Improve

These are real opportunities, but they are no longer reasons to hold the project below the low-to-mid 9s.

### 1. Binding metadata is still stringly typed

Current state:

- `binding_location` is now validated by tests
- but it is still manually authored as a string field in rule catalog definitions

Improvement path:

- [x] introduce a macro or helper that derives `binding_location` from module-owned constants
- [x] reduce the amount of hand-authored path text in the catalog layer

Why it matters:

- the new tests prevent silent drift, but they do not make drift impossible

### 2. Some bindings are still family-level rather than leaf-level

Current state:

- Go `library` rules now point at `src/heuristics/go/library_misuse/mod.rs`
- that is valid and stable, but it is an aggregator path rather than the most precise leaf implementation file for each rule

Improvement path:

- [x] refine family-level binding paths into more precise leaf-module bindings where practical
- [x] do this only where the extra specificity improves maintenance more than it increases churn

Why it matters:

- the current metadata is correct, but precision could still improve

### 3. Count-based regression guards create deliberate maintenance friction

Current state:

- exact inventory assertions now protect corpus and rule-source coverage

Improvement path:

- [x] keep the exact counts, but document how to update them intentionally when adding corpus files or rule ids
- [x] consider grouping counts by language/category in addition to raw totals if future churn increases

Why it matters:

- the new tests are correct, but future contributors need the update process to be obvious

### 4. The `scan-gopdfsuit` make target is operationally accurate but ergonomically rough

Current state:

- the target returns non-zero whenever findings are present because the scanner is doing exactly what it is designed to do

Improvement path:

- [x] add a second informational target that captures scan output without making `make` fail when findings are expected
- [x] keep the strict target for CI-style enforcement

Why it matters:

- the current behavior is semantically correct, but user ergonomics can improve without weakening policy

## What Should Not Be "Improved" Away

These are areas where the architecture is currently correct and should be preserved.

### 1. Do not collapse the crate layering

Keep:

- `analysis` for parsing and evidence extraction
- `index` for repository context
- `heuristics` for rule evaluation
- `scan` for orchestration
- `cli` for presentation

Why:

- this separation is one of the project’s strongest architectural qualities

### 2. Do not revert to tolerant unknown-key config parsing

Keep:

- strict `.deslop.toml` parsing with explicit failure on unknown keys

Why:

- silent config drops are worse than slightly stricter user feedback

### 3. Do not weaken the fail-closed coverage tests

Keep:

- explicit inventory counts
- explicit traversal failure behavior

Why:

- these tests protect confidence in the test suite itself

### 4. Do not go back to manual temp-dir cleanup in the migrated areas

Keep:

- `tempfile`-based RAII cleanup for workspaces and temporary files

Why:

- the new pattern is simpler, safer, and easier to maintain

## Architectural Verdict

The codebase now earns a higher score than Review 7 because the identified weaknesses were implemented away, not merely acknowledged.

The current state is best described as:

- strong architecture
- strong test depth
- materially improved maintenance safety
- tighter operator-facing behavior

The remaining gap to 10/10 is now mostly about reducing manual metadata string management and tightening a few ergonomics around scan workflows. Those are important, but they are refinements on top of an already solid design rather than structural liabilities.

## Next-Step Checklist

- [x] Consider macro- or constant-backed `binding_location` generation to reduce manual path strings
- [x] Refine family-level Go binding paths into leaf-level bindings where the extra precision is worth the maintenance cost
- [x] Document the intentional update flow for corpus and rule-source inventory counts
- [x] Add a non-failing informational scan make target alongside the current policy-enforcing one

## Bottom Line

This is a credible, well-layered Rust static-analysis project with strong engineering discipline.

Review 7 identified real issues.
Review 8 is stronger because those issues are now fixed.

My updated solution-architect rating is **9.6/10**.
