[Merged by Bors] - feat(Tactic): add #defeq_abuse tactic combinator#35750
[Merged by Bors] - feat(Tactic): add #defeq_abuse tactic combinator#35750
#defeq_abuse tactic combinator#35750Conversation
`check_defeq_abuse tac` runs `tac` with `backward.isDefEq.respectTransparency` both `true` and `false`. If the tactic succeeds with `false` but fails with `true`, it identifies the specific `isDefEq` checks that are the root causes of the failure. This is a debugging tool for diagnosing where Mathlib relies on definitional equality that isn't available at instance transparency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PR summary d300915bf1Import changes for modified filesNo significant changes to the import graph Import changes for all files
|
| Current number | Change | Type |
|---|---|---|
| 11957 | 1 | backward.isDefEq |
Current commit 93584a9453
Reference commit d300915bf1
You can run this locally as
./scripts/technical-debt-metrics.sh pr_summary
- The
relativevalue is the weighted sum of the differences with weight given by the inverse of the current value of the statistic. - The
absolutevalue is therelativevalue divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).
- Filter trace analysis to Meta.isDefEq trace class (Name.isPrefixOf) - Replace foldlM with for loops for better array mutation optimization - Use Std.HashSet for dedup instead of linear Array.contains scan - Rethrow internal exceptions (interrupts, heartbeat) instead of swallowing them - Restore trace state before rethrowing in the tracing catch block Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a command-mode `#check_defeq_abuse` combinator that diagnoses `backward.isDefEq.respectTransparency` failures in commands like `instance` declarations, where type class synthesis failures occur during elaboration rather than within a tactic. Groups isDefEq root causes by synthesis application for structured output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…'s test - Command mode syntax is now `#check_defeq_abuse in cmd` (with `in`) - Tactic mode stays as `check_defeq_abuse tac` (no `#`, since `#` conflicts with Lean's `#check` parsing in tactic position) - Add Weiyi's `Module.Free ℝ l` example as a command-mode test that actually detects synthesis failures Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…command) Shorter names. The tactic form can't use `#` because Lean's parser treats `#` as a command prefix even in tactic position. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use `elab` instead of `syntax` + `elab_rules` for the tactic form, following the `#adaptation_note` pattern, so that `#` works in tactic position. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lures The command mode was reporting all instance applications that fail with strict transparency, including ones that would also fail with permissive (e.g., `Submodule =?= AddSubgroup`, `↥l =?= ℝ`). Now we also trace the permissive run and only report applications that succeed there but fail with strict transparency — the actual transparency-related failures. For Weiyi's `Module.Free ℝ l` example, this reduces the output from 8 instance applications to just 1 (`@Submodule.module`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace custom `hasSubstr` with `String.contains`/`startsWith` - Extract `foldTraces` to handle MessageData structural recursion - Extract `extractInstName` (was duplicated in two places) - Make all trace-walking helpers `private` - Add comment explaining why string matching is necessary (TraceData has no structured success/failure field) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Combine the "check" and "trace" passes by enabling tracing from the start. The old runAndCheck becomes runAndCapture, returning both the result and the new messages generated during the run. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add two self-contained synthetic test cases that exercise `#defeq_abuse` without depending on library defeq abuses (which will be fixed upstream): 1. **Tactic mode (MyPred/Pi diamond):** A type alias `MyPred α := α → Prop` with a `DistribLattice` instance inheriting from Pi, a lemma elaborated with that instance, and a later `CompleteLattice` instance creating an instance diamond. The `rw` tactic fails under strict transparency because the lemma and goal use different paths to `PartialOrder`. 2. **Command mode (MySub₂/virtual parent):** A structure extending `AddSubmonoid` with a plain `def toAddSubgroup` (virtual parent pattern), creating a diamond between `AddCommMonoid` from `toAddSubmonoid` (projection) and from `toAddSubgroup` (opaque def). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Find "transition points" (isDefEq checks that fail strict but succeed permissive) instead of all leaf failures. This filters out false positives from simp exploration. Also rename CheckDefEqAbuse → DefEqAbuse and import from Tactic.Common. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…in #defeq_abuse Reduce tactic-mode elaboration from 5 passes to 3 (matching command mode): strict+trace, permissive+trace, final permissive run. Improve transition-point detection to filter out false positives by also collecting permissive failures. A check is only reported as a transition point if it succeeds in the permissive trace AND does not also fail in the permissive trace (which would indicate context-dependent, not transparency-dependent, behavior). Add test for the "fails regardless of transparency" branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Since this is specific to Perhaps |
- Remove unnecessary `tk` from `getRef`/`%$tk`, use `logInfo`/`logWarning` instead of `logInfoAt tk`/`logWarningAt tk` - Use `s.restore (restoreInfo := true)` to discard stale info trees from diagnostic trial runs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the three test cases that depend on current Mathlib defeq abuses (Set.disjoint_singleton_right, Module.Free Submodule, sorriedResult/simp) since these will break once the upstream fixes land. Replace with module-docs showing the original Mathlib code that triggered each issue, followed by self-contained test cases that reproduce the same patterns. Add Matthew Jasper's Mathlib-free minimization of the sub_nonneg zero instance mismatch. Add a test showing that `@[implicit_reducible]` on the virtual parent def fixes the synthesis failure. Imports reduced from 7 to 3. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show that `@[implicit_reducible]` on the virtual parent def resolves the synthesis failure, and that a `unif_hint` equating `Num'.fromNat` across different instances resolves the zero mismatch during `rw`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
check_defeq_abuse tactic combinator#defeq_abuse tactic combinator
Align naming with lean4 PR #12698 (TraceResult in TraceData): - TraceStatus → TraceResult - traceStatusOf → traceResultOf (now returns Option TraceResult) - stripTraceStatusPrefix → stripTraceResultPrefix - Add .error case for bomb emoji (exceptions ≠ failures) - Add source comments noting future simplifications from lean4#12698/#12699 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Without this, `theorem` proofs with `by` blocks are elaborated asynchronously, so `elabCommand` returns before errors appear in `messages`, causing false negatives. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing `import Mathlib.Init` to `Mathlib.Lean.MessageData.Trace` and replace bare `sorry` with `test_sorry` axiom in test file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…35833) * feat: refactor for readability * chore: fix up * chore: fix up more * choreL remove stray comment
|
For maintainers without context: this PR contributes a diagnostic tool from Kim & their Claude to assist with managing the breaking changes associated with I recommend reading the recent messages in the thread on Zulip for context, but in short: This is not intended as a feature so much as an adaptation tool for immediate use. So, it’s being held to different standards—but with clear disclaimers to users about this difference-in-standards in the module docstrings and hovers (see the review PR #35843 if it has not been merged into this branch yet), so that it does get confused for a stable user-facing tool. That difference makes this
Though, to balance this out for the circumstances, I have:
Given that it fulfills different obligations than an ordinary meta PR, has time-sensitive value (i.e. it is most useful immediately, while this breakage is still affecting Mathlib), and is flagged to users as experimental/subject to change (and therefore does not make the same promises about behavior or maintainability as ordinary meta code), I think—pending merging #35843 into this branch—that it's reasonable to: maintainer delegate? |
|
🚀 Pull request has been placed on the maintainer queue by thorimur. |
|
Also, please remember to update the PR description and title before merging :) |
Vierkantor
left a comment
There was a problem hiding this comment.
A cursory glance and this looks good, once the changes of #35843 are merged.
bors d+
|
✌️ kim-em can now approve this pull request. To approve and merge a pull request, simply reply with |
* chore: warnings * chore: docs, tidying, move declarations * chore: make decls private by default * chore: todo, remove unnecessary non-async
|
bors merge |
This PR adds `#defeq_abuse in`, a tactic/command combinator for diagnosing `backward.isDefEq.respectTransparency` failures. It runs the given tactic or command with the option both `true` and `false`, and when the strict setting fails, walks the trace tree to identify the specific `isDefEq` checks that are root causes. Reusable trace-tree analysis utilities are extracted into `Mathlib.Lean.MessageData.Trace`: - `TraceResult` / `traceResultOf` — structured access to the ✅/❌/💥 status encoding - `foldTraceNodes` — generic fold over `MessageData` trace trees - `stripTraceResultPrefix` / `dedupByString` — helpers for trace content comparison Companion lean4 PRs that will simplify this further: - leanprover/lean4#12698 — structured `TraceResult` field on `TraceData` - leanprover/lean4#12699 — `Meta.synthInstance.apply` trace class See https://leanprover.zulipchat.com/#narrow/channel/113488-general/topic/backward.2EisDefEq.2ErespectTransparency 🤖 Prepared with Claude Code Co-authored-by: Thomas R. Murrills <68410468+thorimur@users.noreply.github.com>
This PR gives the `generate` function's "apply @foo to Goal" trace nodes their own trace sub-class `Meta.synthInstance.apply` instead of sharing the parent `Meta.synthInstance` class. This allows metaprograms that walk synthesis traces to distinguish instance application attempts from other synthesis nodes by checking `td.cls` rather than string-matching on the header text. The new class is registered with `inherited := true`, so `set_option trace.Meta.synthInstance true` continues to show these nodes. Motivated by mathlib's `#defeq_abuse` diagnostic tactic (leanprover-community/mathlib4#35750) which currently checks `headerStr.contains "apply"` to identify these nodes. See https://leanprover.zulipchat.com/#narrow/channel/113488-general/topic/backward.2EisDefEq.2ErespectTransparency 🤖 Prepared with Claude Code --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|
Pull request successfully merged into master. Build succeeded:
|
#defeq_abuse tactic combinator#defeq_abuse tactic combinator
This PR marks `Submodule.toAddSubgroup` as `@[reducible]`, matching the reducibility that a real structure parent projection would have (Lean marks structure parent projections `[reducible]`). This resolves `backward.isDefEq.respectTransparency` failures where Lean needs to see that `s.toAddSubgroup.toAddSubmonoid = s.toAddSubmonoid`.
The motivating example is:
```lean
instance {V : Type} [AddCommGroup V] [Module ℝ V] {l : Submodule ℝ V} :
Module.Free ℝ l := Module.Free.of_divisionRing ℝ l
```
which fails with `backward.isDefEq.respectTransparency` because synthesis of `Module ℝ ↥l` needs `Submodule.module`, whose `AddCommMonoid` comes through `toAddSubgroup` (a virtual parent), while the `Module` prerequisite expects `AddCommMonoid` from `toAddSubmonoid` (a real projection). Lean must unfold `toAddSubgroup` to see these agree, but as a plain `def` it was `[semireducible]` and invisible at instance transparency.
See [#general > backward.isDefEq.respectTransparency](https://leanprover.zulipchat.com/#narrow/channel/113488-general/topic/backward.2EisDefEq.2ErespectTransparency/near/575748939) and the synthetic test case in #35750.
…ity#35750) This PR adds `#defeq_abuse in`, a tactic/command combinator for diagnosing `backward.isDefEq.respectTransparency` failures. It runs the given tactic or command with the option both `true` and `false`, and when the strict setting fails, walks the trace tree to identify the specific `isDefEq` checks that are root causes. Reusable trace-tree analysis utilities are extracted into `Mathlib.Lean.MessageData.Trace`: - `TraceResult` / `traceResultOf` — structured access to the ✅/❌/💥 status encoding - `foldTraceNodes` — generic fold over `MessageData` trace trees - `stripTraceResultPrefix` / `dedupByString` — helpers for trace content comparison Companion lean4 PRs that will simplify this further: - leanprover/lean4#12698 — structured `TraceResult` field on `TraceData` - leanprover/lean4#12699 — `Meta.synthInstance.apply` trace class See https://leanprover.zulipchat.com/#narrow/channel/113488-general/topic/backward.2EisDefEq.2ErespectTransparency 🤖 Prepared with Claude Code Co-authored-by: Thomas R. Murrills <68410468+thorimur@users.noreply.github.com>
…y#35761) This PR marks `Submodule.toAddSubgroup` as `@[reducible]`, matching the reducibility that a real structure parent projection would have (Lean marks structure parent projections `[reducible]`). This resolves `backward.isDefEq.respectTransparency` failures where Lean needs to see that `s.toAddSubgroup.toAddSubmonoid = s.toAddSubmonoid`. The motivating example is: ```lean instance {V : Type} [AddCommGroup V] [Module ℝ V] {l : Submodule ℝ V} : Module.Free ℝ l := Module.Free.of_divisionRing ℝ l ``` which fails with `backward.isDefEq.respectTransparency` because synthesis of `Module ℝ ↥l` needs `Submodule.module`, whose `AddCommMonoid` comes through `toAddSubgroup` (a virtual parent), while the `Module` prerequisite expects `AddCommMonoid` from `toAddSubmonoid` (a real projection). Lean must unfold `toAddSubgroup` to see these agree, but as a plain `def` it was `[semireducible]` and invisible at instance transparency. See [#general > backward.isDefEq.respectTransparency](https://leanprover.zulipchat.com/#narrow/channel/113488-general/topic/backward.2EisDefEq.2ErespectTransparency/near/575748939) and the synthetic test case in leanprover-community#35750.
This PR adds
#defeq_abuse in, a tactic/command combinator for diagnosingbackward.isDefEq.respectTransparencyfailures. It runs the given tactic or command with the option bothtrueandfalse, and when the strict setting fails, walks the trace tree to identify the specificisDefEqchecks that are root causes.Reusable trace-tree analysis utilities are extracted into
Mathlib.Lean.MessageData.Trace:TraceResult/traceResultOf— structured access to the ✅/❌/💥 status encodingfoldTraceNodes— generic fold overMessageDatatrace treesstripTraceResultPrefix/dedupByString— helpers for trace content comparisonCompanion lean4 PRs that will simplify this further:
TraceResultfield onTraceDataMeta.synthInstance.applytrace classSee https://leanprover.zulipchat.com/#narrow/channel/113488-general/topic/backward.2EisDefEq.2ErespectTransparency
🤖 Prepared with Claude Code