Skip to content

Refactor oversized Rust modules and fix reviewability/correctness issues #109

Description

@bsutton

Summary

The current Rust codebase has a few concrete correctness and reliability issues, plus a larger maintainability problem: several core .rs files are far beyond a reviewable size and mix unrelated responsibilities. This makes the code materially harder to audit, test, and evolve safely.

This issue records the work needed to bring the codebase back toward normal Rust reviewability and module hygiene.

Why this needs to be done

From a recent code review pass, the main problems are:

  1. There is a probable correctness bug in dictionary generation.
  2. The CLI still has production panic paths for normal user errors.
  3. Decoder-core unsafe code is not isolated tightly enough for easy review.
  4. Several core files are far too large and mix production code, tuning logic, heuristics, and tests.
  5. Documentation quality is uneven, and some module docs are malformed.

We should target modules that are easy to review in isolation. As a working guideline, aim for roughly 300-500 lines per file where practical, and only exceed that when there is a strong structural reason.

Concrete findings to address

1. Fix the dictionary construction bug

File:

  • ruzstd/src/dictionary/mod.rs

Problems:

  • epoch_size is computed, but the actual buffer is hard-coded to vec![0; 100]
  • a dbg!(...) remains in the production path
  • the epoch loop appears to read into current_epoch but still scores against collection_sample

Expected outcome:

  • verify and fix the raw dictionary generation logic
  • remove debug leftovers
  • add or improve tests that would catch this regression

2. Remove production panic paths from the CLI

Files:

  • cli/src/main.rs
  • cli/src/progress.rs

Problems:

  • unwrap() / expect() are used in user-facing runtime paths
  • malformed module documentation exists in progress.rs

Expected outcome:

  • replace panic paths with proper Result-based error propagation
  • clean up malformed rustdoc
  • keep user-facing failures actionable and non-panicking

3. Isolate and better document decoder-core unsafe

File:

  • ruzstd/src/decoding/decode_buffer.rs

Problems:

  • unsafe is used in a core buffer path
  • invariants are not isolated behind the smallest possible API boundary

Expected outcome:

  • either reduce/remove the unsafe, or isolate it into a minimal, well-documented helper
  • document invariants precisely
  • add focused tests covering overlap/copy invariants

4. Split oversized core modules into logical groups

Highest-priority files:

  • ruzstd/src/encoding/match_generator.rs (~7700 lines)
  • ruzstd/src/encoding/blocks/compressed.rs (~2400 lines)
  • ruzstd/src/encoding/mod.rs (~1500 lines)
  • ruzstd/src/encoding/frame_compressor.rs (~1100 lines)
  • ruzstd/src/huff0/huff0_encoder.rs (~970 lines)
  • ruzstd/src/tests/mod.rs (~1000 lines)

This should be a logical split, not a mechanical one.

Suggested direction:

match_generator.rs

Split into modules such as:

  • matcher core/state
  • file/profile heuristics
  • candidate scoring and tie-breaks
  • recency sidecar tracking
  • lazy-parse logic
  • diagnostics
  • tests by topic

compressed.rs

Split into modules such as:

  • block compression config
  • literal encoding
  • sequence encoding
  • exact table-search logic
  • tests

encoding/mod.rs

Split into modules such as:

  • public encoding API
  • file classification
  • file/profile hints
  • path/data sampling

tests/mod.rs

Split by behavior/topic instead of accumulating all integration-style tests in one file.

Expected outcome:

  • smaller files with clear ownership boundaries
  • easier code review
  • reduced cognitive load when changing a single behavior

5. Move embedded tests out of giant production files where they harm reviewability

Examples:

  • ruzstd/src/encoding/match_generator.rs
  • ruzstd/src/huff0/huff0_encoder.rs

Expected outcome:

  • keep close-to-code tests where useful
  • but move large topic groups into dedicated test modules/files
  • production files should not contain thousands of lines of embedded test code

6. Improve internal documentation at module boundaries

Problems:

  • some modules have useful docs, but many boundaries are still implicit
  • giant files make the docs less useful because responsibilities are blurred

Expected outcome:

  • each major module should state:
    • what it owns
    • what invariants it depends on
    • what it intentionally does not handle
  • document non-obvious tuning/profile logic and candidate-selection rules
  • fix malformed or stale rustdoc while splitting files

Proposed implementation order

  1. Fix the dictionary correctness issue.
  2. Remove CLI panic paths and malformed docs.
  3. Isolate/document decoder unsafe.
  4. Split match_generator.rs.
  5. Split compressed.rs and encoding/mod.rs.
  6. Split remaining oversized modules and test aggregations.
  7. Finish with a documentation pass once module boundaries are stable.

Acceptance criteria

  • dictionary generation bug is fixed and covered by tests
  • no production unwrap() / expect() remain in normal CLI error paths
  • decoder unsafe is either reduced or isolated behind clearly documented invariants
  • match_generator.rs is split into logical submodules
  • compressed.rs is split into logical submodules
  • encoding/mod.rs is reduced to a small module root plus submodules
  • large embedded test bodies are moved into topic-based test modules where appropriate
  • malformed module docs are fixed
  • key internal modules have clear rustdoc describing responsibilities and invariants
  • the resulting structure is materially easier to review than the current layout

Notes

This issue is intentionally about reviewability and maintainability, not just formatting. The goal is to make future compression and decoder changes easier to reason about and safer to review.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions