Repository navigation
perf(Erdos367): narrow imports and replace proof search - #514
Conversation
|
Advisory cross-file review against Lean Pool's supervisory rubric. This is not by itself a significance, source-verification, or merge verdict. [Medium risk] Replaces proof search tactics with explicit proofs in math library. No blocking issue was identified in the four-file PR diff; the full-library build was not independently verified. SummaryThis PR narrows the Reviews (23) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." |
Proof profile (new / modified Lean files)Statement changes✅ No statements changed — all 86 tracked declarations across 4 modified files kept their type; only proofs / definition bodies changed. Statements are compared textually against the merge base (comments and whitespace ignored; Compile cost of modified files — base → headHeartbeats: 349 → 349 (+0, +0.0%). Declarations: 86 → 86. Per-file wall sum: 37.2 s → 35.4 s (-4.7%). Wall clock is measured under parallel load and is noisy; heartbeats are deterministic and are the regression signal. Files: 4 compared — 0 cheaper, 0 costlier, 4 within noise (threshold: |Δ heartbeats| ≥ 1,000 and ≥ 1%). Both sides are elaborated against the merge-base build of the pool — sound when only proofs and definition bodies change; a file whose statements changed may fail there and is flagged ⚠. Heartbeats come from Mathlib's
Per-project compile cost (top 2 of 2 projects by |Δ heartbeats|):
Advisory only — never blocks merge. Full log uploaded as the |
🤖 LLM review — refactor (
|
| Aspect | Value |
|---|---|
| Scope | import cleanup and targeted tactic rewrites |
| Introduces tech debt | ✅ no |
| Maintainability | ✅ improved |
| Brittleness | ✅ more_robust |
| Risk | ✅ low |
The replacements make proof dependencies clearer without sacrificing structure or introducing actionable maintenance debt.
Tokens: 177,984 in / 2,370 out · Tier: codex-subscription · Effort: xhigh · Billing: Codex subscription quota (no API credits) · Estimated cost: $1.8983 (Standard API equivalent; uncached input)
Automated review against .github/REFACTOR_REVIEW_RULES.md. Disagree? Reply on the PR; rules can be updated in a PR of their own.
Exact-head reconciliation\n\nReviewed current head
|
Maintainer decision requiredReviewed exact current head |
|
/review |
Maintainer re-reviewReviewed exact current head Fresh required checks for this exact head are queued. The branch is now current with |
|
Re-reviewed The complete four-file refactor preserves public statements and useful proof structure while replacing proof search and removing six private imports. No actionable source defect remains. Another worker pushed the same integration tree while this cycle was running; the prepared push was stopped, and this exact remote head was independently validated. No duplicate commit was pushed. Targeted warning-free project build, actual The older automation hold is now contained: the existing |
Vilin97
left a comment
There was a problem hiding this comment.
Reviewed head: 2222c9203324d9adad13b495229878a488afdcbf
Verdict: approve. Posted as a comment review because GitHub does not permit the authenticated PR author to submit an approving review.
The Erdos367 refactor removes six private entry-module imports and replaces proof search with explicit nonzero, divisibility, and finite-set witness terms. The omega replacement closes the existing normalized arithmetic step in T3 without removing its documented inequality chain. Full diff and affected-source review found no actionable regression. Exact-head CI passed the full build, index check, linters, style, and repository quality checks, resolving the PR description's older local linker limitations; these checks were inspected rather than rerun locally. The advisory profile reports 349 to 349 heartbeat units and no material deterministic regression. The author's small T3 wall-time increase is a disclosed local tradeoff, not evidence of a significant compile-cost regression.
| Assessment | Result |
|---|---|
| Scope | proof-search replacement and private-import pruning |
| Introduces technical debt | No |
| Maintainability | Improved |
| Brittleness | More robust |
| Risk | Low |
The refactor exposes routine proof dependencies and preserves the argument and public imports, so no source repair is required before merging.
Findings: none.
Validation evidence: full build and gates, proof profile.
Vilin97
left a comment
There was a problem hiding this comment.
Updated to current main cd2e11846a1a8ae3028d9274112200bb199bcc40 at 01d70128c88eaf41948914a390f90595c242e82a after the Puiseux import merged. All four previously reviewed Erdos367 files remain byte-identical; the registry and generated root index exactly match main. The actual pinned mk_all --module --check passes. The previous full green CI and substantive review remain valid for the unchanged source; required checks must rerun on this integration commit before merging.
Vilin97
left a comment
There was a problem hiding this comment.
Carry-forward review at 9528966fcee3fe31999fc5d7eb3eacda42efdab7: incorporated main 854da85a8bc6c9219c2c04ec9d43d4a49f43aef8. The delta from the just-reviewed integration commit 01d70128c88eaf41948914a390f90595c242e82a is only generated NOTICE and README metadata. All Lean source, theorem contracts, registry and index bytes are unchanged, so the prior source review and actual index check carry forward. Fresh required CI remains necessary before normal merge.
Daily disposition: needs maintainer — protected merge operationReviewed exact head
The remaining authority boundary is operational: this repository has |
Vilin97
left a comment
There was a problem hiding this comment.
Independent review of exact head ad19b3018bdf925a4e299c2ea3c37e5e7cac17d8: The four-file import/proof-search refactor preserves the existing conditional bounds and unconditional Pell limsup results. Those are independently significant intermediate theorems; this PR does not completely solve sourced Erdős problem 367 and is not accepted as a solution to it. All material findings are resolved; scoped sources, provenance, licensing and novelty checks remain valid. The proof/interface delta was inspected; documentation-only or main-integration changes do not introduce a new mathematical claim. Current required checks are green and no unresolved human changes request remains. Approved by review comment because GitHub forbids self-approval.
Selected project
This refactors
LeanPool.Erdos367, selected because its six-file development has about 13,800 lines and a transitive closure of roughly 3,700 modules. The pass removes six unused private Mathlib imports from the entry module and replaces opaqueaesopproof search with directomega, nonzero, divisibility, and witness terms inCore139,RFullLowerBound, andPellLimsup.Semantic invariant audit
Base:
38b8ba36899903cb7d3a42bb9f8a3f5c70fdb05fHead:
d4a477e6f35ac625627332f5f2a19467f32d145aI compiled an environment manifest from the base and head with Lean environment inspection. Both contain 236 declarations in the
LeanPool.Erdos367module closure. Every public declaration has unchanged kind, safety, universe parameters, reducibility metadata, type hash, and fully printed type. The only manifest differences are three private generated simp-companion names whose numbering changed as proof elaboration changed; their types and generated private status are unchanged. No public declaration was added, removed, renamed, or retyped.Measurements
Lean profiler (
-Dtrace.profiler=true) on base → head:RFullOdd.rFullPart_pow: 0.226 → 0.189 sRFullOdd.pow_dvd_rFullPart: 0.364 → 0.262 sErdos367.prime_sq_dvd_pellN_plus2: 0.606 → 0.543 sCore139.T3: 1.182 → 1.225 s (a small local tradeoff for deterministic arithmetic closure)A clean project build was measured before editing at 16.70 s with the warm pinned Mathlib cache. The later clean head run rebuilt 3,775 Mathlib/project jobs after the shared cache was invalidated, so that 690.89 s run is not used as a comparable wall-clock result.
Validation
Passed:
lake build LeanPool.Erdos367(warning-free)lake exe runLinter LeanPool.Erdos367#print axiomsfor all three registered main results: onlypropext,Classical.choice, andQuot.soundgit diff --checklake exe mk_all --checkandlake exe lint-style LeanPool.Erdos367could not link in this worktree because the shared package cache lacks nativeCliobjects; the linker reports missinglp_Cli_*symbols. A repository-widelake build LeanPoolwas attempted and reached 8,423/9,058 jobs before interruption because this isolated checkout had to rebuild the broad pool from an empty root cache. No Lean error was reported for the changed project.