fix: footnote reorder is not idempotent for nested footnotes - #47
Draft
KyleKing wants to merge 1 commit into
Draft
fix: footnote reorder is not idempotent for nested footnotes#47KyleKing wants to merge 1 commit into
KyleKing wants to merge 1 commit into
Conversation
Categorization used old_id (raw first-encounter parse order) as a proxy for 'referenced from real body text'. That proxy breaks once a document has already been formatted once: a nested footnote's only textual occurrence moves into the trailing block, so a second formatting pass assigns it a later old_id than footnotes defined after it in the original source, silently reordering the output on every other run. Categorize and sort by genuine body-reference position instead (from _partition_refs_by_context, which already distinguishes real body refs from references nested inside another footnote's definition), so the order no longer depends on where the previous pass happened to leave each definition. Adds a blanket idempotency check across the existing fixture set and a fixture for the specific nested-plus-later-footnote pattern that exposed this. One pre-existing, unrelated idempotency gap (issue #7: a footnote referenced only from within an about-to-be-orphan-deleted footnote) is excluded with a documented reason rather than papered over.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a non-idempotent footnote reordering edge case for nested footnotes by switching the “body-referenced” ordering heuristic from parse-assigned IDs to the first body-level reference position, so formatting an already-formatted document becomes a no-op.
Changes:
- Compute a stable “first body reference” order (
body_ref_order) and use it to sort body-referenced footnotes instead of relying on parse order. - Add an idempotency test suite over the existing fixtures, excluding known pre-existing non-idempotent cases (issue #7).
- Extend/update footnote fixtures to cover the regression and reflect the new ordering behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
mdformat_footnote/_reorder.py |
Switches body-reference ordering from parse IDs to a computed body-reference position map; adjusts subId reassignment wiring. |
tests/test_fixtures.py |
Adds an idempotency test pass over fixtures (with explicit exclusions for known legacy behavior). |
tests/fixtures/footnote.md |
Updates/extends fixtures to reproduce and lock in the nested-footnote idempotency regression behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+160
to
164
| # Sort by genuine body-reference position, not old_id: old_id reflects raw | ||
| # parse order, which a nested pre-occurrence can put ahead of the real | ||
| # body reference. | ||
| body_referenced.sort(key=lambda x: body_ref_order[x[2]]) | ||
| fence_only = [label for label in refs_in_fences if label in fence_only_set] |
KyleKing
marked this pull request as draft
September 1, 2026 03:01
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.
Categorization used to rely on
old_id(parse order) as a proxy for 'referenced from real body text,' but that breaks when a document has already been formatted, which breaks idempotencyInstead, this PR introduces using the body-reference position