Move the inline tests out of tally/runtime.rs and tally/connection.rs by lamemustafa · Pull Request #440 · lamemustafa/bridge

lamemustafa · 2026-09-16T09:49:28Z

Follows docs/proposed-rust-module-conventions.md item 2: extract inline test modules first. It's mechanical and doesn't change production code. These were the two largest inline test blocks left in pinned production files.

file	before	after	moved to
`src-tauri/src/tally/runtime.rs`	5,441	3,490	`runtime_tests.rs`: 1,951 lines moved, 1,940 after rustfmt
`src-tauri/src/tally/connection.rs`	3,872	2,024	`connection_tests.rs`: 1,848 lines moved, 1,836 after rustfmt

Each block stays the same child module, #[cfg(test)] #[path = "<stem>_tests.rs"] mod tests;. So private access, test paths and relative include_str!/include_bytes! paths (resolved from the same directory) are all unchanged.

Checked, not asserted

Only indentation moved. The new files, re-indented, match the removed blocks line for line against HEAD. The 8 multi-line string literals were left byte-identical rather than dedented, because their whitespace can be test data.
rustfmt changed layout only. After it re-wrapped lines into the freed columns, the only non-whitespace changes are 4 trailing commas. All 1,057 string literals are identical before and after.
No test lost. I compared test-name lists for everything under tally::runtime:: and tally::connection:: (99 with default features, 110 with voucher-scan). That covers the moved blocks plus 32 tests in sibling modules that did not move, and both lists are identical. The independent review narrowed this to the moved blocks alone (67 default / 78 voucher-scan / 79 live-calibration-harness) and compared whole-lib lists under default, voucher-scan, live-calibration-harness and lab-writes. All identical, and the moved tests pass.
cargo clippy --lib --tests -- -D warnings -A clippy::pedantic is clean.
Reseal: exactly 2 pinned hashes changed (the two parents), and --verify exits 0. The Report pins a branch drops and modules it leaves unsealed, without refusing either #436 coverage report's first real decomposition run reports newly unsealed test-only modules: 2 and lists nothing else, which is what this change should produce. Unpinned test modules are the repo norm.

Every cfg in the moved code is a feature gate (voucher-scan, live-calibration-harness); there is no platform cfg, so a Windows build is unaffected. (Corrected: an earlier version of this body said there were Windows-only test paths. There are none.)

A text-reading JS test depended on the old layout

scripts/outstandings-evidence.test.mjs reads runtime.rs as text and asserted a line of the moved test block. It turned CI's Frontend build red. I had verified the Rust side and never ran the JS suite. Fixed in 6ea54fc8: production patterns still read runtime.rs; the one Rust-test assertion reads runtime_tests.rs. Proven to still bite (changing the moved 6 to 7 fails it), and the full frontend suite passes (239 tests). scripts/check-tally-live-read-boundary.mjs also lists runtime.rs, but only scans for probe feature names, which neither new file contains.

Coverage: a deliberate reduction, stated

These ~3,800 test lines were sealed only because they sat inside pinned files. They no longer are. Editing them won't force a reseal now. That's a choice, not a norm to assume:

By Compatibility surface has zero headroom, which blocks every decomposition of a pinned file #416's rule, a pin needs a named reason: the file decides what Bridge posts or lets leave the machine. Tests decide neither, and the production code they exercise stays pinned.
Four test modules are pinned (agent_presence_tests.rs, agent_tests.rs, agent_voucher_parse_tests.rs and the sibling runtime_trial_balance_tests.rs). Only agent_presence_tests.rs has a named reason: it is the only independent statement of an admission contract. The other three were pinned in feat(tally): expose native Trial Balance through MCP #246 and feat(desktop): investigate selected ledger entries #252, before per-pin reasons were recorded, so they set no precedent with a rationale.
Extract the test modules from tally_mirror.rs and snapshot.rs #395 left its extracted tally_mirror_tests.rs and snapshot_tests.rs unpinned.

What's lost: a change that only weakens one of these tests no longer changes a pinned hash. I haven't assessed each test for an admission contract stated nowhere else. If you want them sealed, pinning both is a two-entry change with a named reason.

🤖 Generated with Claude Code

chatgpt-codex-connector · 2026-09-16T09:49:33Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

docs/proposed-rust-module-conventions.md item 2: extract inline test modules first -- mechanical, no production change. These were the two largest remaining inline test blocks among pinned production files: 1,951 lines in runtime.rs and 1,848 in connection.rs. Production lines are unchanged; the files drop to 3,490 and 2,024 lines. Each block moves to <stem>_tests.rs beside its parent, kept as the same child module (`#[cfg(test)] #[path = "..."] mod tests;`), so private access and test paths are unchanged, and relative include_str!/include_bytes! paths still resolve from the same directory. Checked, not asserted: - The moved bodies differ from the removed blocks only by four columns of indentation, compared against HEAD line by line. The eight multi-line string literals were left byte-identical rather than dedented, since their whitespace can be test data. - rustfmt then re-wrapped lines using the freed columns. Its only non-whitespace changes are four trailing commas; all 1,057 string literals are identical before and after. - Test names are identical before and after: 99 under default features, 110 with voucher-scan, which gates 11 of them. All pass under both. - cargo clippy --lib --tests -D warnings is clean. - Resealing changed exactly two pinned hashes, and the #436 coverage report counts exactly two newly unsealed test-only modules, listing nothing else. Unpinned test modules are the repository norm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… live scripts/outstandings-evidence.test.mjs reads runtime.rs as text and asserted `assert_eq!(statement_open_bills.len(), 6);` in it -- a line of the inline test module the previous commit moved to runtime_tests.rs. CI's Frontend build failed on it. I had verified the Rust test lists, clippy and formatting but never ran the JS suite, which is where the breakage was. The production patterns it checks (the NativeBills and VoucherScan read strategies, and all_open_bill_rows consuming both sources) still read runtime.rs; only the Rust-test assertion reads runtime_tests.rs. Proven to still bite: changing the moved assertion's 6 to 7 fails this test, and the full frontend suite passes with it restored (239 tests). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs/proposed-rust-module-conventions.md item 2, continuing #440. Four inline test modules move to files beside their parents, each kept as the same child module via #[path], so private access, test paths and module paths are unchanged: commands.rs tests -> commands_tests.rs statement_export_tests -> commands_statement_export_tests.rs party_statement_export_tests -> commands_party_statement_export_tests.rs sync/reconciliation.rs tests -> reconciliation_tests.rs commands.rs has production code after its first test block; it stays in place. commands.rs goes from 4,684 to 3,580 lines; reconciliation.rs from 2,578 to 1,708. Checked, not asserted: - Both parents rebuild byte-identical to HEAD from the new parent plus the re-indented children, expanding only the three moved commands.rs modules (the existing commands_native_ledger_tests.rs declaration is untouched). - rustfmt changed layout only: two trailing commas in commands_tests.rs, and in reconciliation_tests.rs two match arms `=> { assignment }` rewritten as `=> assignment,`, which both evaluate to (). The 559 string literals in those two reformatted files are identical; the other two test files and both parents were unchanged by rustfmt. - Whole-lib test lists are identical before and after under default (968), voucher-scan (986), live-calibration-harness (988) and lab-writes (1,068). The 67 moved tests pass. The only compiler warnings are the two existing unused variables in runtime.rs under voucher-scan. - The two #[cfg(unix)] tests moved; unix includes the macOS host. The target_os cfgs in commands.rs are in production code that did not move. - Text readers checked: scripts/client-grouping.test.mjs and ledger-entries-ui.test.mjs read commands.rs and still pass, as does the full frontend suite (239). check-tally-live-read-boundary.mjs scans reconciliation.rs for probe features and still passes. - clippy --lib --tests -D warnings is clean. Resealing changed exactly two pinned hashes; the coverage report counts four newly unsealed test-only modules and lists nothing else. The four new test files are unpinned, like #440's: tests decide nothing about what Bridge posts or emits, so they carry no pin reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…441) docs/proposed-rust-module-conventions.md item 2, continuing #440. Four inline test modules move beside their parents, kept as the same child modules via #[path]: commands.rs tests, statement_export_tests, party_statement_export_tests -> commands_tests.rs, commands_statement_export_tests.rs, commands_party_statement_export_tests.rs sync/reconciliation.rs tests -> reconciliation_tests.rs commands.rs goes from 4,684 to 3,580 lines, reconciliation.rs from 2,578 to 1,708. Production code after commands.rs's first test block stays in place. No production change: both parents rebuild byte-identical to the base with only the moved blocks replaced by #[path] declarations. rustfmt changed layout only -- two trailing commas, and two match arms `=> { x = .. }` rewritten as `=> x = ..,`, both evaluating to (). String and char literals are identical. No test lost: whole-lib test lists are identical under default (968), voucher-scan (986), live-calibration-harness (988) and lab-writes (1,068). The 61 moved tests pass. The frontend suite (node, vitest, Playwright) and the text readers of commands.rs pass. check-tally-live-read-boundary.mjs's production-surface feature check no longer reads the moved reconciliation test code; its per-file identifier scan still does. The four new test files are unpinned, as in #440. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…_write_store.rs (#442) docs/proposed-rust-module-conventions.md item 2, continuing #440 and #441. Three inline test modules move beside their parents via #[path]: bridge-tally-protocol native_outstandings/wire.rs currency_tests -> wire_currency_tests.rs group_tests -> wire_group_tests.rs bridge db/tally_write_store.rs tests -> tally_write_store_tests.rs wire.rs goes from 1,796 to 1,262 lines; tally_write_store.rs from 1,794 to 1,259. No production change: both parents rebuild byte-identical to the base. rustfmt changed whitespace only; every string and char literal, including the eight multi-line ones, is identical. No test lost: the 23 moved wire.rs tests and 6 moved tally_write_store.rs tests pass, and whole-lib test lists are identical for bridge under default, voucher-scan, live-calibration-harness and lab-writes, and for bridge-tally-protocol under default and all features. The only compiler warnings are two existing unused variables in tally/runtime.rs under voucher-scan and live-calibration-harness. check-tally-request-builder-hazards.mjs skips only inline #[cfg(test)] blocks, so it now scans these extracted test files (and #440's and #441's) as production; its results are unchanged. The three new test files are unpinned, as in #440 and #441. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ot by name #440, #441 and #442 moved inline `#[cfg(test)] mod tests { ... }` blocks into sibling files loaded by `#[cfg(test)] #[path = "..."] mod tests;`. check-tally-request-builder-hazards.mjs skipped only the inline form, so it began reading those files as production code. Results did not change, but a test that builds a hazard-shaped string as an expectation would have raised a false alarm. Skipping any file named *_tests.rs was rejected: a safety scanner must not exempt production code for its file name. Instead a file is skipped only when it is actually loaded as a test module: - Edges: a brace-depth-0 out-of-line `mod name;` whose attribute group has a cfg implying test (cfg(test), or test as an argument of cfg(all(...))), with #[path] or bare in a crate root or mod.rs; plus any such declaration made by a file already test-only, iterated to a fixed point. - Vetoes, from every .rs file in the repository and matched by basename case-insensitively: any other `mod name;` at any depth (including macro bodies and inline modules), any `path = "..."` or include!-family string not part of an edge (covering cfg_attr), and every Cargo crate root. - Files whose braces underflow or do not end balanced contribute no edges. - Everything unhandled -- cfg(any(...)), #[path] inside an inline module, r#name -- leaves the file scanned, so an error is a false alarm, never a missed hazard. The design was critiqued before implementation. The critique found four ways the first draft could skip a production file -- a non-test declaration it did not recognise, a Cargo target, a declaration outside the scan roots, case variants -- and that it would still have scanned 24 of the 74 extracted files: 22 declared by other test files, one under cfg(all(test, ...)), one bare in a mod.rs. The veto set, the fixed point and those two cfg/bare shapes are the response. Checked, not asserted: on this repository the skipped set is exactly the 74 files the rustc-validated module graph from #436 marks test-only, path for path; the 10 pinned violations are unchanged. The skipped count is now pinned too, so a lexer regression that quarantines too much fails the gate. Main now runs under `import.meta.main` so the functions can be imported; a test runs the real script as a child process so a main that silently does nothing cannot pass. 15 fixture tests; 15 mutations of the scanner (trust file names, accept any cfg mentioning test, drop each veto, trust unbalanced files, accept nested declarations, drop propagation, drop the crate-root veto, case-sensitive veto, #[path] under the stem, main never running, no bare-mod detection, strings or comments not skipped, only the first attribute, no raw-string path) each fail a named test. One of those first survived because a fixture's production declaration masked the lexer; the fixture was fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs/proposed-rust-module-conventions.md item 2, continuing #440-#442, for the last large inline test block among pinned files: the `tests` module of tools/bridge-tally-compatibility/src/lib.rs -- the tool that validates the compatibility surface -- moves to lib_tests.rs beside it via #[path]. lib.rs goes from 2,853 to 1,686 lines. No production code followed the block. Checked, not asserted: - lib.rs rebuilds byte-identical to HEAD from the new parent plus the re-indented child. - rustfmt's only non-whitespace change is one trailing comma; all 169 string and char literals are identical. - Test lists are identical before and after: 24 lib tests by default, 30 with bills-native-outstandings-probe-receipt. The full crate passes under both, with no warnings. - Before resealing, real_tree_has_complete_migration_and_report_surface_coverage failed with surface_file_changed -- the expected result of changing a pinned file -- and passes after; reseal --verify exits 0 and exactly one pinned hash changed. - clippy -D warnings is clean for CI's tools-workspace step and its probe-receipt feature step. - Text readers: check-tally-live-read-boundary.mjs allowlists this lib.rs for probe identifiers; the moved tests contain none, and it passes. The request-builder hazard scanner now reads lib_tests.rs as production (see #443) and still reports exactly its 10 pinned violations. The other scanning scripts pass, and the frontend suite passes (239 node, 113 vitest, 10 Playwright) on a clean tree -- the merge-driver test refuses to run with uncommitted surface changes. Coverage, stated rather than assumed. lib_tests.rs is unpinned, following #416's rule and #440-#442: tests decide nothing about what Bridge posts or emits. For this file the loss is sharper than before, because real_tree_has_complete_migration_and_report_surface_coverage holds the only executable check of the reserve bound, `MAX_SURFACE_FILES - files.len() <= RESERVED_SURFACE_FILES`. The cap itself stays enforced in the pinned lib.rs. Editing or deleting that assertion no longer changes a pinned hash. The one #[cfg(windows)] test, normalise_surface_path_uses_forward_slashes, could not run on the macOS host that did this move; CI's windows-latest native job runs the tools workspace tests and covers it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…446) docs/proposed-rust-module-conventions.md item 2, continuing #440-#442. The inline `tests` module of tools/bridge-tally-compatibility/src/lib.rs -- the tool that validates the compatibility surface -- moves to lib_tests.rs beside it via #[path]. lib.rs goes from 2,853 to 1,686 lines. No production change: before rustfmt, lib.rs rebuilt byte-identical to the base from the new parent plus the re-indented child; rustfmt then changed layout only, its one non-whitespace change a trailing comma, with every literal identical. Test lists are identical before and after (24 lib tests by default, 30 with bills-native-outstandings-probe-receipt), all pass, and clippy is clean for both CI tools steps. The #[cfg(windows)] test compiles for x86_64-pc-windows-msvc and runs in CI's windows-latest native job. lib_tests.rs is unpinned, following #416's rule and #440-#442. That unseals the only executable check of the reserve bound, `MAX_SURFACE_FILES - files.len() <= RESERVED_SURFACE_FILES`; the cap itself stays enforced by validate_shape in the pinned lib.rs. Inline tests are not done: 70 pinned files still hold 18,384 lines of top-level inline #[cfg(test)] modules, led by source_draft/catalog.rs (1,779) and tally/connector.rs (1,317). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

lamemustafa and others added 2 commits September 16, 2026 15:36

lamemustafa force-pushed the refactor/extract-tally-runtime-connection-tests branch from 6ea54fc to 72afa57 Compare September 16, 2026 10:07

lamemustafa merged commit 7610965 into master Sep 16, 2026
13 checks passed

lamemustafa deleted the refactor/extract-tally-runtime-connection-tests branch September 16, 2026 10:15

lamemustafa mentioned this pull request Sep 16, 2026

Move the inline tests out of commands.rs and sync/reconciliation.rs #441

Merged

lamemustafa mentioned this pull request Sep 16, 2026

Move the inline tests out of native_outstandings/wire.rs and db/tally_write_store.rs #442

Merged

lamemustafa mentioned this pull request Sep 16, 2026

Skip extracted test modules in the hazard scanner by how they load, not by name #443

Closed

lamemustafa mentioned this pull request Sep 16, 2026

Move the compatibility tool's inline tests out of its pinned lib.rs #446

Merged

Navigation Menu

Move the inline tests out of tally/runtime.rs and tally/connection.rs - #440

Move the inline tests out of tally/runtime.rs and tally/connection.rs#440
lamemustafa merged 2 commits into
masterfrom
refactor/extract-tally-runtime-connection-tests

lamemustafa commented Sep 16, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lamemustafa commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checked, not asserted

A text-reading JS test depended on the old layout

Coverage: a deliberate reduction, stated

Uh oh!

chatgpt-codex-connector Bot commented Sep 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lamemustafa commented Sep 16, 2026 •

edited

Loading