Repository navigation
quality: read an axiom list that Lean wrapped over several lines - #564
deadczarvc wants to merge 12 commits into
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] Adjusts axiom parsing to handle Lean's line wrapping. The PR appears safe to merge based on the reviewed changes. SummaryThe PR updates the axiom-output parser to read Lean’s multiline lists and adds a regression test that rejects Reviews (12) · Last reviewed commit: "Merge branch 'main' into fix/axiom-audit..." |
… in 120 columns The audit parser on main reads the axiom list on one line; Lean wraps it past 120 columns, and the long name pushed it over (fix for the parser: LeanPool#564).
Vilin97
left a comment
There was a problem hiding this comment.
The multiline capture fixes the reported wrapped sorryAx output, and the new test correctly fails on the old parser. One small change is needed before this gate update merges:
- Retain the existing end-of-line
$after the closing\]. The new[^\]]*already spans continuation lines, and$underre.MULTILINEstill matches the closing bracket on its final line. Without$, the parser accepts a line such as'Challenge.Widget.widget_exists' depends on axioms: [sorryAx] unexpected textand silently ignores the trailing content. The gate should continue requiring the complete#print axiomslist line. - Extend the regression with a neighboring declaration after the wrapped registered statement: an unregistered helper whose list contains
sorryAxmust still produce the existing rejection. This checks that the multiline match ends at its own bracket and the next result is audited independently.
I checked both regexes against the wrapped sample and the trailing-text case in memory: retaining $ passes the valid wrapped sample and rejects the trailing-text case. With the exact PR parser, a wrapped registered statement followed by an unregistered helper still raises the helper error, while the trailing-text input is accepted without error. In an isolated exact-head checkout, focused test_challenge.py and test_quality.py passed (75 tests), and Ruff check/format checks passed on the two changed files. Hosted Python CI is still queued, so these are local results only.
Vilin97
left a comment
There was a problem hiding this comment.
Reviewed exact head 36db76fabc7809e1747c3704934062f7cc8c2ee5. The follow-up restores the parser's terminal $ while [^\]]* reads Lean's wrapped axiom list across lines. The regression now places a wrapped registered sorryAx statement immediately before a wrapped unregistered helper and requires the helper to be rejected. This resolves both findings in my prior changes-requested review.
In an isolated exact-head checkout, focused axiom-audit pytest passed (4 tests), the new regression passed individually, and Ruff check and format checks passed for the two changed files. The checkout diff is limited to python/lean_pool/quality.py and python/tests/test_challenge.py.
Approved as a gate-code review. Merge still awaits explicit user authorization and protected CI at this head.
* feat(challenge): Ville's inequality for testing by betting * fix(challenge): shorter statement name so the #print axioms line fits in 120 columns The audit parser on main reads the axiom list on one line; Lean wraps it past 120 columns, and the long name pushed it over (fix for the parser: #564). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Vasily Ilin <vasilin97@gmail.com>
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Needs maintainer: explicitly authorize this protected axiom-audit repair. Re-reviewed exact head Both inline review threads are now resolved. The existing code approval nevertheless explicitly reserves authorization. Live CONTRIBUTING.md says “Never change the checks or gates” and allows changes only “unless explicitly asked.” I found no recorded explicit grant for changing protected Continuation: once explicitly authorized, re-read the head and current main, revalidate any integration changes, and use normal protected merging after required checks pass. This is an external fork, so this cycle made no branch push. Updated this existing comment to avoid a duplicate discussion entry. Final cycle reconciliation: GitHub records this PR as closed by the |
_parse_axiom_outputexpects the whole#print axiomsresult on one line. Past 120 columns Lean breaks the list after each comma, and a challenge statement with a long name reaches that easily, because the open declaration's list carriessorryAxas well. Verbatim output for the statement in #557 (Lean 4.35.0-rc3,lake env lean):The pattern
\[(.*)\]$finds no closing bracket on the first line, so the declaration is never marked as seen._axiom_audit_resolvedstill counts it as resolved, since it only needs the prefix, and the audit then reports a false error: the open statement "does not depend onsorryAx". The same miss would let a wrapped list with an unallowlisted axiom through unchecked.The fix lets the list span lines (
\[([^\]]*)\], stillre.MULTILINEfor the anchor); the existing split on commas already strips the line breaks and indents. A regression test feeds the verbatim output above and expects no errors (it fails onmain).Checks:
ruff checkandruff format --checkclean on both files;pytest tests/test_challenge.py tests/test_quality.py: 75 passed. The fulltests/run on Windows has the same 28 environment failures onmainwith and without this change (aggregator cloner, codex worker, exposition and similar), 533 passed vs 532 onmain.