Skip to content

Fix proof-length counter including unrelated declarations - #712

Merged
Vilin97 merged 7 commits into
LeanPool:mainfrom
loefflerd:CI-theorem-lines
Nov 6, 2026
Merged

Vilin97 merged 7 commits into
LeanPool:mainfrom
loefflerd:CI-theorem-lines

Conversation

@loefflerd

@loefflerd loefflerd commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The 200-line proof-size check currently treats everything up to the next theorem or lemma as part of the preceding proof. Consequently, a two-line proof followed by definitions or instances can be reported as hundreds of lines long. It also mistakes assignments in theorem statements for the start of the proof.

This change replaces the existing Python-based parser with a Lean linter, enforcing the same 200-line maximum but using Lean's own syntax trees to reliably count lines.

No mathematical content or project metadata is changed.

See #723 for an issue tracking over-long proofs in extant LeanPool code which were overlooked due to bugs in the existing checker.

@github-actions

Copy link
Copy Markdown
Contributor

Proof profile (new / modified Lean files)

No added or modified Lean files under LeanPool/ in this PR. No compile-cost measurements are needed.

Advisory only — never blocks merge.

@greptile-apps

greptile-apps Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Advisory cross-file review against Lean Pool's supervisory rubric. This is not by itself a significance, source-verification, or merge verdict.

RetriggerView in Greptile

[Medium impact] No confirmed issue blocks merging this PR.

Summary

Replaces the Python proof-length scan with a Lean compiler plugin that checks proof syntax and elaborated types.

  • Lean measures each proof against the 200-line limit.

Review used the full PR changes because the previous and current review commits have different merge bases with the target commit. No tests were executed.

Reviews (7) · Last reviewed commit: "Merge main into CI-theorem-lines" · Reviewed by Greptile

Comment thread python/lean_pool/quality.py Resolved
Comment thread python/lean_pool/quality.py Resolved
Comment thread python/lean_pool/quality.py Resolved
@loefflerd

Copy link
Copy Markdown
Contributor Author

Aargh. It looks like this proposed Python script change just exchanges the quirks and corner cases of the existing script for a different set. Probably the only tool that can reliably parse Lean code is Lean itself; I'll look into writing a Lean proof-length linter (analogous to the existing line-length linter).

@loefflerd
loefflerd marked this pull request as draft October 24, 2026 07:59
@loefflerd
loefflerd marked this pull request as ready for review October 28, 2026 06:36

@Vilin97 Vilin97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all nine changed files and all 15 hunks at 4cf748e, including the complete plugin and compiler fixtures. Original source ranges, semantic proof classification, comment handling, overlapping ranges and the production 200/201 boundary have coherent coverage. I found no isolated-head correctness defect from this static review.

The policy decision is broader than replacing the parser: blank lines now count, and proof terms inside definitions, instances and examples become subject to the cap. Moving enforcement into the compiler/warning gate also changes protected tooling, workflow and cache inputs. These changes need direct scoped authorization under the repository rules; the default cap remaining 200 does not by itself preserve the old policy.

There is a small integration fix when composing this with #719: test_proof_length_plugin_has_pinned_ci_coverage requires the exact substring -k 'test_lake_', while #719 extends it to -k 'test_lake_ or test_compiled_axiom_audit'. Check the selector semantically and retain both fixture suites. This is a combined-tree issue, not a claimed failure of this head. Compiler fixtures and current CI were not independently run here.

@Vilin97

Vilin97 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Integration status at 4cf748ef8998b66e6ec6368aacc7eb5189cdda10: the favorable implementation review is retained. It does not resolve the existing-content repairs tracked in #723. Please address those over-limit proof terms in separate proof-preserving content changes: factor the reported AndersonConjecture PLift proofs and PLAcceleratedNesterovLean private abbrev proofs into meaningful intermediate lemmas, preserving the mathematical contracts. Keep #723 open until the repaired source passes the semantic checker and normal validation; changing declaration spelling or adding a waiver is not a repair. I have not reproduced fresh compiler counts for those project files.

The current direct gate-change authorization covers #719 only. This PR still needs its own scoped authorization. It follows #719; when composing them, retain both fixture suites and update this PR’s exact-selector assertion. These are separate integration conditions, with the nine-file source review already complete.

Compose with LeanPool#719: keep both compiled-fixture suites in the three pinned-Lean
pytest steps with LeanPool#719's selector, and make the proof-length coverage test
check that the selector includes test_lake_ instead of matching it exactly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Vilin97

Vilin97 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this — the syntax-aware checker is a clear improvement, and it found real loopholes. To get it merged:

Once #756 and #757 are in, CI here should be green and this will go into the merge queue.

🤖 Generated with Claude Code

@Vilin97 Vilin97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
"summary": "Reviewed current ff2d4fc and all nine changed files against current main and the previously reviewed implementation. The proof-length plugin and Lean fixtures are unchanged; the three pinned-Lean pytest steps retain both fixture suites, and the selector test now parses argv and checks test_lake_ membership. The existing axiom auditor is preserved. The current worker assignment supplies the direct scoped gate-change authorization. Source review is favorable; PR756 and PR757 must land first, and this head must then pass its own applicable protected checks before queue admission.",
"assessment": {
"scope": "Syntax-aware proof-length enforcement and PR719 fixture composition",
"introduces_tech_debt": false,
"maintainability": "improved",
"brittleness": "more_robust",
"risk": "medium",
"assessment_one_sentence": "The selector composition is fixed without changing the already validated plugin semantics or weakening the existing audit."
},
"verdict": "approve",
"findings": []
}

@loefflerd

Copy link
Copy Markdown
Contributor Author

See also leanprover-community/mathlib4#45388, a Mathlib PR contributing a descendant of the long-proofs linter from this PR to Mathlib's standard linter set (which will eventually render this LeanPool PR redundant).

Brings in the long-proof splits from LeanPool#756 and LeanPool#757 so the new linter passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Vilin97 added a commit to loefflerd/lean-pool that referenced this pull request Nov 5, 2026
* Split over-long proofs in AndersonConjecture

Refactor every proof over 200 lines (as measured by the syntax-aware
long-proof linter in LeanPool#712) into named helper lemmas. Proof carriers
disguised as PLift-valued defs or private abbrevs become theorems.
Statements and public declarations are unchanged.

Refs LeanPool#723

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Remove the now-unused smul_lift_of_filtration

Its only callers were replaced by the coefficient-based argument.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Preserve the public filtration lifting lemma

Restore the existing exported smul_lift_of_filtration contract and its bounded proof while retaining the long-proof decomposition.

The lemma remains available to downstream users even though current internal callers now use coefficient decomposition. Refs LeanPool#723.

* Put theorem headers on their own line after include/omit

Main's text-based proof-size check only recognizes declarations that start
a line, so one-line 'include T in theorem' headers merged neighbouring
blocks into apparent 264/364-line proofs. Splitting the header is the
standard layout and keeps both checkers accurate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Vilin97 added a commit to loefflerd/lean-pool that referenced this pull request Nov 5, 2026
* Split over-long proofs in PLAcceleratedNesterovLean

Refactor every proof over 200 lines (as measured by the syntax-aware
long-proof linter in LeanPool#712) into named helper lemmas. Proof carriers
disguised as PLift-valued defs or private abbrevs become theorems.
Statements and public declarations are unchanged.

Refs LeanPool#723

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Extract fderiv_kills_normal_of_eqOn from the local convergence proof

Main's text-based proof-size check counted the statement's let-bindings as
proof and reached 201 lines; extracting this self-contained step brings it
comfortably under the limit for both checkers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

@Vilin97 Vilin97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed published head 2a50c58ef221eb6dc2e8c8a1d161e9d1fa7cd2e1.

All nine changed source files are byte-identical to the previously reviewed ff2d4fc59 implementation. I checked the published two-parent merge against current main 6814773, including the complete forward/inverse tree reconstruction. The proof-length plugin, regression fixtures, all three pinned-Lean selectors, and the existing axiom auditor retain the reviewed composition; unrelated projects and controls are preserved.

PR756 and PR757 have now both merged. The direct scoped authorization for this PR is resolved, and my source verdict remains favorable at this exact new head. The current-head CI run and applicable current-main validation still need to complete before normal protected queue admission. Earlier-head CI and the targeted compiler checkpoint have not been credited to this revision.

@Vilin97
Vilin97 added this pull request to the merge queue Nov 6, 2026
Merged via the queue into LeanPool:main with commit 906301d Nov 6, 2026
33 checks passed
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.

2 participants