Skip to content

refactor(core): split monolithic test and CLI modules into focused files#35

Merged
m2papierz merged 4 commits into
masterfrom
refactor/core-cleanup
Jun 19, 2026
Merged

refactor(core): split monolithic test and CLI modules into focused files#35
m2papierz merged 4 commits into
masterfrom
refactor/core-cleanup

Conversation

@m2papierz

Copy link
Copy Markdown
Contributor

What

Split the monolithic engine_integration.rs (2364 lines, 53 tests) into 10 focused test modules, extract CLI command handlers
and error type into dedicated files, move PhysicalQubitEstimate re-export to the correct crate-root location, and add unit
tests for classify_rz_angle.

Why

engine_integration.rs had grown into a single file covering 10 unrelated concerns (abort, decoder, demand, hooks,
multi-pool, etc.), making it hard to locate tests and reason about coverage. The CLI main.rs (742 lines) mixed argument
parsing with command execution logic, violating the thin-composition-root pattern. PhysicalQubitEstimate was re-exported
through analysis/mod.rs despite originating from pirx_hw — misleading for readers. classify_rz_angle had zero unit tests
despite being the single source of truth for angle classification.

How

  • Test split: 53 tests moved 1:1 into engine_{basic,abort,decoder,demand,error_budget,hooks,multi_pool,rz,timing,trace_ids}.rs. Shared TOML fixtures and helpers extracted to test_support/mod.rs. Zero test additions, zero test removals — verified by diffing function names.
  • CLI extraction: Each subcommand (profile, monte-carlo, compare, convert, sensitivity) gets its own file under
    commands/. CliError moves to error.rs. main.rs reduced to argument parsing + dispatch.
  • Re-export fix: pub use pirx_hw::PhysicalQubitEstimate moved from analysis/mod.rs to lib.rs. Public API (pirx_core::PhysicalQubitEstimate) unchanged.
  • IR tests: 15 unit tests for classify_rz_angle covering T-gate/Clifford/Rotation classification, edge cases (NaN, ±∞), and tolerance boundaries

Testing

  • make ci passes locally (fmt + clippy + test + audit)
  • New behavior has tests
  • Hot-path changes have criterion benchmarks

Checklist

  • PR description explains why, not just what
  • No new unwrap()/expect() in production code
  • No new allocations in the simulation hot loop
  • Crate boundaries respected (pirx-core never imports from pirx-adapters)
  • New dependencies justified (not "it's popular" — what does it replace?)

Move each subcommand (convert, profile, monte_carlo, compare, sensitivity)
into dedicated modules under commands/ and extract CliError into error.rs
to reduce main.rs from ~900 lines to a thin dispatch layer.
Cover all classification branches: T-gate (odd π/4 multiples),
Clifford (even π/4 multiples incl. zero), Rotation (arbitrary,
irrational), non-finite inputs (NaN, ±Inf), and 1e-10 tolerance
boundary (inside at 1e-12, outside at 1e-8).
…anup

- Re-export PhysicalQubitEstimate directly from pirx_hw in lib.rs
  instead of routing through the analysis module (it's a pirx-hw type,
  not an analysis concern)
- Remove unnecessary #[allow(dead_code)] on ManifestMeta::description
- Fix formatting in classify_rz_angle test
Break monolithic 2300-line integration test file into 10 domain-specific
modules (abort, basic, decoder, demand, error_budget, hooks, multi_pool,
rz, timing, trace_ids). Extract shared HW model loading into test_support.
@codspeed-hq

codspeed-hq Bot commented Jun 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 27 untouched benchmarks


Comparing refactor/core-cleanup (b4c5b73) with master (01ceac4)

Open in CodSpeed

@m2papierz m2papierz merged commit 906bf50 into master Jun 19, 2026
11 checks passed
@m2papierz m2papierz deleted the refactor/core-cleanup branch June 19, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant