refactor(Algebra): use native Fin.castLE order isomorphism#3274
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Blueprint ↔ Lean equivalence and status (Category A): No blueprint .tex files are changed. sum_castLE_extend_zero has no blueprint entry, so there are no \lean{...} tags, \leanok markers, or \notready tags to verify. The theorem statement is unchanged by this PR — only the proof body is refactored. Nothing to flag.
Prose quality (Category B): The diff contains no new or modified docstrings, block comments, line comments, or section/namespace names. Nothing to flag.
0 equivalence mismatches, 0 stale status tags, 0 prose issues.
|
PR cleanup
Conventions: CONTRIBUTING.md |
There was a problem hiding this comment.
Review: refactor(Algebra): use native Fin.castLE order isomorphism
Verdict: APPROVE — no blocker (🔴) or requires-changes (🟡) issues found.
Scope
The PR refactors the proof of Fin.sum_castLE_extend_zero in TNLean/Algebra/FinSum.lean. The theorem statement, its docstring, and all downstream call sites are unchanged; only the proof body and one import are touched.
Verification
- Builds clean.
lake env lean TNLean/Algebra/FinSum.leanexits 0 with no errors, warnings, orsorry/axiom. - Proof integrity (🔴): No
sorry,admit,native_decide,axiom, or kernel bypass. Clean. - Proof correctness (🔴): The first
calcstep now reindexes via(Fin.castLEOrderIso h).toEquivrather than the removed-pathFin.castLEquiv.Fin.castLEOrderIso h : Fin r ≃o { i : Fin s // (i:ℕ) < r }(Mathlib/Order/Fin/Tuple.lean:185) sendsj ↦ ⟨Fin.castLE h j, _⟩, whose underlying value isj.val, so theFintype.sum_equivobligationf j = f ⟨(castLE h j).val, _⟩is discharged bysimp— faithful to the original reindexing. The subtype→filter and filter→universe steps are now singlerws onFinset.sum_subtype_eq_sum_filterandFinset.sum_filterplus smallsum_congrblocks, mathematically equivalent to the prior explicittranschains and a net reduction of 23 lines. - Mathlib style (🟡): Idiomatic — uses the canonical order-isomorphism instead of the more general
Equiv, and the import is correctly narrowed toMathlib.Order.Fin.Tuple. No issues blocking approval. - Type safety (🔴):
.toEquivon theOrderIsoyields the rightFin r ≃ {i : Fin s // …}subtype;classicalsupplies decidability for theif hlt :branches. No coercion or universe concerns. - Performance / Modularity / Documentation: Unaffected — small finite sums, no new lemmas, docstring retained.
Advisory (ℹ️, non-blocking)
- Lines 40–44:
refine Finset.sum_congr ?_ ?_focuses the first goal with a·bullet but lets the second fall through unbulleted. This is accepted Mathlib practice for a trailing main goal; bulleting both would be marginally more uniform. Not required.
Nice cleanup — shorter and more idiomatic with no change to the statement or its consumers.
There was a problem hiding this comment.
Review Summary
This is a clean proof refactoring of sum_castLE_extend_zero in TNLean/Algebra/FinSum.lean. The PR replaces Fin.castLEquiv with the canonical (Fin.castLEOrderIso h).toEquiv and simplifies the proof by using direct rw calls instead of explicit transitivity chains.
All categories pass with no issues found:
- Proof integrity — no
sorry,axiom,native_decide, or other blockers. - Proof correctness — both
Fin.castLEquivandFin.castLEOrderIsohave identicaltoFun/invFundefinitions (both useFin.castLEwithby simp), so the mathematical equivalence is preserved. Thesimp/rwsteps are all sound, and the file compiles cleanly withlake env lean. - Mathlib style —
Mathlib.Order.Fin.Tupleis the canonical module for Fin order isomorphisms; the old import was from the less idiomaticMathlib.Logic.Equiv.Fin.Basic. Indentation and proof structure follow Mathlib conventions. - Type safety — no issues.
- Performance — the proof is cleaner and shorter (40 lines removed, 17 added).
- Modularity — no duplication; using the canonical module.
- Documentation — existing docstrings suffice; no new definitions.
- Paper-gap notes — not applicable.
No inline comments needed. ✅ Approved.
Motivation
sum_castLE_extend_zeropreviously reindexed finite sums viaFin.castLEquivfromMathlib.Logic.Equiv.Fin.Basic; the nativeFin.castLEOrderIsofromMathlib.Order.Fin.Tupleis the canonical order-isomorphism for this purpose and is more idiomatic.rwcalls shorten the proof without altering the result.Description
TNLean/Algebra/FinSum.lean: replaces the finite-index equivalence insum_castLE_extend_zerowith(Fin.castLEOrderIso h).toEquiv, importing fromMathlib.Order.Fin.Tupleinstead ofMathlib.Logic.Equiv.Fin.Basic. Shortens the subtype-sum and filtered-sum comparison steps to directrwcalls onFinset.sum_subtype_eq_sum_filterandFinset.sum_filter, replacing explicitFinset.sum_congrtransitivity chains. The theorem statement and all call sites are unchanged.Testing
lake env lean TNLean/Algebra/FinSum.leangit diff --checklean_action_ci.yml) to validate the full build.Note
Low Risk
Only the Lean proof and imports in
FinSum.leanchange; the theorem statement and call sites are unchanged.Overview
Refactors the proof of
sum_castLE_extend_zerowithout changing its statement or downstream uses (e.g. Kraus padding arguments).The first step now reindexes via
Fin.castLEOrderIso h).toEquivinstead ofFin.castLEquiv, withMathlib.Order.Fin.TuplereplacingMathlib.Logic.Equiv.Fin.Basic. The subtype-to-filter and filter-to-universe sum steps are shortened to directrwcalls onFinset.sum_subtype_eq_sum_filterandFinset.sum_filter, with smallerFinset.sum_congrblocks instead of explicit transitivity chains.Reviewed by Cursor Bugbot for commit 8666bbf. Bugbot is set up for automated code reviews on this repo. Configure here.