[isPANN]

Migrates all five workspace crates to Rust 2024 with rust-version = "1.88", including the new WASM crate. The delta against the reviewed semantic stack contains formatting, explicit reference patterns, equivalent let-chains, and the existing checked instruction-count and pipeline-parser fixes.

An independent review checked the remaining nonmechanical token changes, all merge conflicts, expression macros, temporary/drop scopes, and WASM exports. No unintended behavior change or lost test assertion was found. The instruction-count helper retains checked arithmetic and now has exact-limit/overflow tests; the STORE regression verifies an exact instruction count.

Validation on the integrated candidate:

• make check: 7,194 tests passed with none skipped, formatting and all-target/all-feature Clippy passed.
• make paper and 23 Python pipeline tests passed.
• Production website/WASM verification passed with 336 Chromium/Firefox/WebKit cases and artifact/provenance integrity checks. A subsequent independently reviewed WebKit math-paint fix passes focused three-engine tests. The fresh complete verifier passed all 339 browser cases at 4f57c65928759e2a04933856d64f178eea4119c2, with source-unchanged, artifact, and provenance checks passed. The final published integration has the identical source tree.
• All five manifests and locked dependency metadata are consistent with the declared Rust minimum. The final five-crate integration was built with the installed stable toolchain; no new Rust 1.88 execution result is claimed.

This PR was merged through the reviewed feature stack in #1196, preserving its separate merge record. All nine final aggregate CI checks passed, including Codecov patch and project checks. Reported aggregate patch coverage is 97.27654%; project coverage is 97.18%. Main merge a10ee83007296e686c4c7f94f70f70bddb92b078 preserves the exact reviewed and website-verified source tree.

[isPANN]

Agentic Review Report

PR #1207: Migrate workspace to Rust 2024 edition (head 81c2e1be). This is a generic, toolchain-only PR: 54 files, +255/−266, with no models or rules added or removed.

Structural and semantic

• Hard fails: none. The diff has no generated exports, no unrelated edits, and no trait or macro behavior changes.
• Edition bump: all four crates (problemreductions, -cli, -expr, -macros) and rustfmt.toml are set to 2024. style_edition = "2021" keeps formatting unchanged, and make fmt-check passes.
• Closure patterns (|(_, &v)| → |&(_, &v)|): these are the mechanical cargo fix rewrites required by the 2024 match-ergonomics rules. Every call site is .iter().enumerate().filter(...) over Copy items (bool, i64, tuples of usize), so the binding types stay the same.
• Let-chain collapses (all 39 hunks checked by hand):
  • Each rewrite merges an outer if / if let that has no else with an inner if / if let that also has no else. The new if A && let B = … && C { body } evaluates A, then B, then C, left to right with short-circuiting. That is the same order as the nested form. No else branch was added, removed or moved.
  • Statements after the inner block stayed outside the chain: for example, return; in cli.rs:392ff and normalized.push(...) after continue in ilp.rs:585ff/625ff.
  • ? inside a condition (rules/graph.rs:1581 aggregate_witness_evaluation(..)?, and solvers/decision_search.rs:39,79 is_satisfiable(..)?) runs at the same point and under the same guard as before. The mutable borrow from normalized.last_mut() is still scoped to the body.
  • The parenthesized || in mcp/tools.rs:254 keeps the original grouping.
• Edition-2024 drop order (if_let_rescope, tail_expr_drop_order), including sites the lint did not flag:
  • I searched the whole workspace (src, the three sub-crates, tests, examples). It has no Drop impls, no RefCell / Mutex / RwLock / .lock() / .borrow_mut() guards, and no ManuallyDrop / mem::forget or tempfile guards.
  • The only interior mutability is Cell counters (unit_tests/solvers/brute_force.rs:238, unit_tests/rules/traits.rs:513), the OnceLock registries, and the allocation-counting GlobalAlloc test (unit_tests/models/graph/bounded_component_spanning_forest.rs). That test counts only alloc and realloc, not dealloc, so earlier drops cannot change its count.
  • So the rescoping changes only when memory is freed. No observable behavior changes.
• Reverting expr_2021 to expr: this is safe. In 2024, $x:expr also accepts const { … } and _. That can change behavior only when an earlier arm with expr would now capture input meant for a later arm.
  • I checked all 27 :expr fragments across the 35 macro_rules!. No arm order lets _ or const {} reach a different arm.
  • impl_variant_param! arms are distinguished by , k:. The @declare arms of register_decision_variant! are distinguished by a trailing random. impl_variant_reduction!, impl_sat_to_ksat! and the test macros have one arm each.
  • Fragment semantics follow the edition of the crate that defines the macro, so the exported macros now accept slightly more input downstream. They never accept less.
• Other 2024 changes:
  • Box<[T]>::into_iter: the Box<[Plan]> / Box<[Expr]> fields are only iterated with .iter(), and SerializedNode uses Vec.
  • The gen keyword appears only as a string in the expr reserved-word list.
  • unsafe extern and set_var changes don't apply. The unsafe {} blocks in the global allocator were already there.
  • Merged doctests and never-type fallback compile and pass under -D warnings.
• Doc fence ```ignore → ```text in problemreductions-macros/src/lib.rs:211: this is correct, because the block is illustrative syntax and cannot compile under --include-ignored.
• Issue compliance: there is no linked issue. The PR body matches the diff.

Quality

• No production logic changed, so no new tests are needed. The existing suite exercises every touched model, rule and solver path.
• Local gates on the PR head (rustc 1.99.0):
  • make fmt-check: OK
  • make clippy (--all-targets --all-features -D warnings): OK
  • make test (workspace, --include-ignored): all pass, including 6241 lib unit tests, 344 integration tests and the MCP integration tests, with 0 failures.
• cargo doc prints 4 rustdoc warnings (URL is not a hyperlink, unclosed HTML tag, unresolved link). They are in doc comments this PR does not touch.

Feature test (make cli, scratch directory)

• pred show MIS prints the schema description line (commands/graph.rs let-chain).
• pred create --example MaximumMatching → pred solve returns Max(2) via ILP (maximum_matching.rs let-chains).
• pred create --example MinimumSetCovering → pred solve --solver brute-force returns Min(2) (covered_elements let-chain).
• pred path MIS QUBO → pred reduce --via → pred solve returns Max(4) with QUBO intermediate Min(-394). This recovers through recover_completed_result (rules/graph.rs let-chain).
• pred solve with a missing argument gives the usual clap error and usage text.

Findings

Critical: none.

Important: none.

Minor

1. No declared MSRV (Cargo.toml:12, problemreductions-{cli,expr,macros}/Cargo.toml:4). Edition 2024 requires Rust ≥ 1.85, and the PR body says so, but no crate sets rust-version. Without it, crates.io users on an older toolchain get a less helpful error, and the implicit resolver = "3" that 2024 brings has no MSRV to resolve against. Fix: add rust-version = "1.85" to each [package], or to a [workspace.package] that the crates inherit.
2. Misaligned indentation after the collapse. At problemreductions-cli/src/mcp/tools.rs:261-265, the anyhow::bail! arguments still sit at the old, deeper nesting level. At src/models/algebraic/ilp.rs:597, the format! string in normalize_constraint no longer lines up with its format!(. rustfmt skips these blocks because the string literal is longer than max_width, so fmt-check doesn't catch it. Fix: dedent them by hand, or split the long strings so rustfmt can format the blocks. This is cosmetic only.
3. PR body rationale for reverting expr_2021. The body says "no caller passes const {} or _", but the stronger reason is that no macro has an arm that such input could be redirected to. That also covers future downstream callers of the exported macros. Fix: optionally reword the body; no code change is needed.

Verdict: the change preserves behavior. Every let-chain is equivalent to the nested form, the drop-order changes have no observable effect because the workspace has no guard-type destructors, and reverting to expr is safe. It can be merged once a human approves; adding rust-version is recommended.

[codecov]

Codecov Report

❌ Patch coverage is 95.55556% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.12%. Comparing base (48a9571) to head (e638958).

Files with missing lines	Patch %	Lines
src/unit_tests/example_db.rs	82.35%	6 Missing ⚠️
src/rules/ksatisfiability_cyclicordering.rs	50.00%	4 Missing ⚠️
.../rules/consistencyofdatabasefrequencytables_ilp.rs	0.00%	2 Missing ⚠️
src/rules/stringtostringcorrection_ilp.rs	0.00%	2 Missing ⚠️
...s/decisionminimumvertexcover_hamiltoniancircuit.rs	0.00%	1 Missing ⚠️
src/rules/hamiltoniancircuit_hamiltonianpath.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1207      +/-   ##
==========================================
+ Coverage   97.03%   97.12%   +0.09%     
==========================================
  Files        1103     1103              
  Lines      145876   146230     +354     
==========================================
+ Hits       141545   142030     +485     
+ Misses       4331     4200     -131     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[isPANN]

Addressed the Minor findings of the Agentic Review Report in 57ebb23:

1. MSRV: declared rust-version = "1.88", not 1.85. The let-chains this migration introduces need 1.88. Verified with a real 1.88 toolchain across the whole workspace.
2. Indentation: tools.rs was already fixed by the 2024 style reformat in 16ee25f. ilp.rs:597 was realigned by hand.
3. PR body: now gives the stronger reason the expr revert is safe (no macro arm can be re-matched by the new input) and maps each commit to a step of the edition guide.

[GiggleLiu]

Agentic Review Report

Reviewed PR #1207 at 4152d3eca94c8cc4098ce06b90a1f0aecd13d877 against merge base 875b6e86d5a87e5f66c2f6743be9d22abe917a09.

Structural and semantic

No blocking findings. Inspected the migration's semantic changes separately from its Rustfmt commit. The reference-pattern rewrites preserve the filter closure bindings. The let-chain rewrites preserve condition order, short-circuiting, and guarded indexing. In particular, the error propagation in decision search and result recovery stays under the same guards, and ILP normalization retains its continue and following insertion behavior. Production sources contain no Drop implementations or Mutex/RefCell guard patterns affected by temporary rescoping.

All four crates declare edition 2024 and Rust 1.88, matching the new let-chain requirement. Later Rust changes add error-path tests and indentation only. The Python change supplies the missing wait-ci --format parser field, and its default and explicit values are tested.

Quality

Local verification on this exact head passed:

• cargo fmt --all -- --check
• git diff --check
• python3 -m unittest discover -s scripts -p test_pipeline_pr.py, 23 tests

The exact PR head also has nine successful GitHub CI checks, including workspace tests, Clippy, coverage, and platform builds. Existing review comments and follow-ups were read. Full Rust tests and the stated Rust 1.88 build were not rerun locally by this reviewer.

Feature test

No new feature is introduced. No CLI installation or runtime walkthrough was performed in this review. The previous review documents successful CLI workflows, while current-head CI independently covers compilation and tests.

Findings

None that block merging. The one-line pipeline parser repair is outside the edition migration's core scope but is disclosed, tested, and does not add material risk. Safe to merge this reviewed head after the parent verifies it remains current.

GitHub PR audit

GATE: PASS

• harm.secrets: OK. No credential material in the diff.
• harm.dangerous-files: OK. Existing source, manifests, and tests only.
• harm.generated-artifacts: OK. No generated exports or build outputs.
• harm.cache: OK. No cache directories.
• harm.debug-leftovers: OK. No debugging probes introduced.
• pr.description-quality: OK. The body explains commit scope, compatibility, and validation.
• pr.traceable: OK. The body states the migration purpose and toolchain requirement.

Combined integration validation

The reviewed change was also validated with the other safe candidates at Rust 2024 integration head 3e768b37bc54efc5af4d133b3162efdc7ecfb27f: workspace/all-feature tests and the ignored-only follow-up passed all 7,161 tests; all-target/all-feature Clippy with warnings denied, formatting, diff check, and 23 Python tests passed. Independent semantic-baseline token comparison verified the merge resolutions. Details and logs are in review-1207-integration.md and the adjacent *-1207-integrated*.log artifacts.

Final integration review and validation

Current reviewed head is 62da60439b1598a5c627ede9b669718c06b628c8. Its tree e7bf8f4fe5d136c01cafdd116a5fb4a398680599 is byte-identical to tested head 3e768b37bc54efc5af4d133b3162efdc7ecfb27f; the last commit records current main/NAE ancestry without changing files.

Resolved the overlapping graph, LCS, numeric-cast and NAE changes explicitly. A second reviewer independently checked the combined edition diff against a semantic-only baseline. No reviewed implementation was lost, no function or test declaration was removed, and the remaining semantic edits match the reviewed Rust 2024 migration. LCS differs only in import order. The parameter lookup retains the equivalent short-circuit guard order.

Verified the full logs on this exact tree: 7,158 ordinary workspace/all-feature tests passed, followed by all three ignored doctests, for 7,161 passed and zero failed. All-target/all-feature Clippy with warnings denied, formatting, diff-whitespace checks, and 23 Python pipeline tests also passed. Workspace crates were rebuilt after clearing only their generated cache artifacts.

The harm gate remains PASS. Final merge requires current-head CI to pass and #1210 to land first.

[GiggleLiu]

Agentic Review Report

Final merge gate: hold

The current reviewed head is 62da60439b1598a5c627ede9b669718c06b628c8. This update supersedes the earlier conditional approval for merging.

Important: codecov/patch fails. The current Codecov report reports patch coverage of 94.41489%, below its 95.00% target, with 21 changed lines missing coverage. Overall project coverage is 97.11%; that does not satisfy the separate patch gate.

The missing lines are reported in the example database tests, minimum-code-generation model, cyclic-ordering reduction, database-frequency-table ILP reduction, string-correction ILP reduction, two Hamiltonian reductions, and NAE-to-SAT reduction. Inspect the file-specific missing-line links in Codecov and cover the relevant branches without lowering or excluding the coverage requirement. Recalculate against current main after #1210 lands.

Structural, quality and feature verification

The independent semantic and integration reviews remain valid. All 7,161 local workspace tests passed, including the ignored doctests; Clippy, formatting and Python pipeline tests passed. All seven GitHub Actions jobs also passed. No new behavioral defect was found during integration review, but the final patch-coverage check is not green.

The user requested review and merge only, with blockers left open. Therefore this PR remains open for the coverage gap to be addressed and reviewed. No coverage threshold or exclusion was changed. The previously reported harm-signals gate remains PASS.

[GiggleLiu]

Addressed the one-register instruction-counter coverage gap in fcd7073. The five identical checked additions now share one private helper, with tests for the exact i64::MAX boundary and overflow for one- and two-instruction increments. The existing STORE test now verifies both evaluation orders and the exact instruction count. Behavior and coverage thresholds are unchanged.

Validation passed: 13 focused tests, all 7,162 workspace/all-feature tests including ignored tests, Clippy across all targets/features with warnings denied, formatting, and diff checks. The final main merge is ancestry-only and preserves the tested tree. Current head: e638958.

Fresh Codecov must confirm the patch gate before merging.

[GiggleLiu]

Reviewed the coverage repair at e638958. The shared checked instruction counter preserves behavior and has exact-limit and overflow regressions; the STORE test now verifies the instruction count. All 7,162 local tests, Clippy, formatting, and all nine CI checks pass. Fresh patch coverage is 95.55556%. The prior coverage blocker is resolved. Merge will follow the reviewed feature stack to avoid repeated edition-integration conflicts.

[GiggleLiu]

Agentic Review Report: final edition integration

Approved head 17dfcc17eeb7d40dd46142fc126e5bd69289f2e9 for merging into #1196. Its tree exactly matches tested candidate 4f57c65928759e2a04933856d64f178eea4119c2; the last merge adds only the recorded feature-PR ancestry.

Fresh independent review covered all five manifests, nonmechanical token changes, integration conflicts, expression macros, temporary/drop scopes, and WASM exports. Quality and harm-signal gates passed. The checked instruction counter retains behavior and exact-boundary coverage. All five crates declare Rust 2024 and minimum Rust 1.88; final local compilation used installed stable, not a new Rust 1.88 run.

The combined candidate passed 7,194 native tests, fmt, all-target/all-feature Clippy, make paper, and 23 pipeline tests. Later changes are limited to website CSS, browser/provenance tests, and a dev dependency lockfile. Focused WebKit correction and independent review pass; the final 339-case artifact verifier is running. The aggregate must pass that verifier and fresh required main CI/Codecov before landing. #1219, #1220, and #1221 remain excluded.