Fix recall pretty-print in proof lists (Refs #931)#984
Merged
Conversation
`PRecall.__str__` produces `recall <fact1>, <fact2>, ...` which is term-greedy at the parser level: its `parse_nonempty_term_list` is comma-greedy and the embedded `parse_term` happily reads `|` (an add-level operator aliased to `∪`) and `in` (a compare-level operator). So a pretty-printed `simplify with recall A | recall B`, `apply f to recall A, recall B`, or `replace symmetric recall E in ...` re-parses incorrectly (or fails entirely with `expected a term, not "recall"`). Add a `_proof_list_item_str` helper that wraps such proofs in parens when emitted as an element of a `|`-separated proof list (`simplify with`, `replace`), a `,`-separated proof tuple (`apply f to ...`), or just before an `in` keyword (`replace ... in ...`). The helper recurses through `PSymmetric`/`PTransitive`/`PHelpUse` so a proof whose trailing sub-proof is `PRecall` is also wrapped. Parens are a no-op for the parser, so the canonical AST is unaffected. Four `lib/` files (`IntAddSub.pf`, `Maps.pf`, `NatLess.pf`, `UIntLess.pf`) move from parse-fail to OK on the round-trip sweep and join `PARSER_ROUND_TRIP_FILES`, together with a synthetic `recall_list_roundtrip.pf` covering the three surface shapes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The `--passable` test mode prewarms `lib/` but does not auto-import its names into the file's scope (that's a `--no-stdlib`-style contract). Add an explicit `import UInt` so the fixture validates under the test harness, not just under the `deduce.py` CLI. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #931
Summary
Picking up the next sub-bug from the remaining
lib/parse-fail set. A fresh local sweep oflib/*.pfround-trip on today'smainshowed 7 parse-fails, of which 5 share a root cause inPRecall's pretty-printer being too lax.PRecall.__str__producesrecall <fact1>, <fact2>, ...which is term-greedy at the parser level: itsparse_nonempty_term_listis comma-greedy, and the embeddedparse_termhappily reads|(an add-level operator aliased to∪) andin(a compare-level operator, used for set membership). So when the pretty-printer emits aPRecallinside a|-separated proof list, a,-separated proof tuple, or just before aninkeyword, the next separator gets consumed by the inner term parse and reparsing trips withexpected a term, not "recall". Concrete repros fromlib/:lib/IntAddSub.pf: sourcesimplify with (recall y < x) | (recall not (x < y))was pretty-printed without the parens.lib/NatDiv.pf: sourceapply less_trans to (recall k' < m), (recall m < j')was pretty-printed as bareapply less_trans to recall k' < m, recall m < j'.lib/NatDiv.pf(later):replace symmetric (recall k = j') in recall P(j')lost the parens around thesymmetric (recall ...)argument, so theinwas consumed by the inner term parse at compare level.Fix: add a
_proof_list_item_strhelper inabstract_syntax/proofs.pythat wraps such proofs in parens when emitted as an element of a|-separated proof list (SimplifyGoal/SimplifyFact/RewriteGoal/RewriteFact), a,-separated proof tuple (PTuple), or just before aninkeyword (replace ... in ...already wraps the equations list). The helper recurses throughPSymmetric/PTransitive/PHelpUseso a proof whose trailing sub-proof isPRecallis also wrapped. Parens are a no-op for the parser (LPAR proof RPARreturns the inner proof unchanged), so the canonical AST is unaffected.Four
lib/files (IntAddSub.pf,Maps.pf,NatLess.pf,UIntLess.pf) move from parse-fail to OK on the round-trip sweep and joinPARSER_ROUND_TRIP_FILES, together with a syntheticrecall_list_roundtrip.pffixture covering the three surface shapes (replace ... | ...,apply ... to ..., ...,replace symmetric ... in ...).Remaining
lib/parse-fails after this PR:IntMult.pf(aCasesproof falls back to the dataclassreprfor lack of a__str__) andNatDiv.pf(anobtain ... where ... from ...; choose ...block is being flattened with semicolons by the pretty-printer). Both are independent pretty-printer bugs that #931 already tracks; this PR doesn't try to address them.UIntAdd.pftransitions from parse-fail to AST-drift (itsapply ... to (X), (Y)PTuple is now pretty-printable but a separate ModusPonens-vs-PTuple bug surfaces — also a follow-up).Test plan
python3.13 deduce.py test/should-validate/recall_list_roundtrip.pfvalidatespython3.13 test-deduce.py --equiv --serialpasses (parser equivalence + round-trip)make staticpassespython3.13 test-deduce.pypasseslib/round-trip parse-fails drop from 7 to 2Claude session — fallback UUID:
fb5a58da-9dd0-4c1f-a373-595087fe7edb🤖 Generated with Claude Code