From 9681d2eac504fc6e0ab3810d3097a9ba88cd09e3 Mon Sep 17 00:00:00 2001 From: Kyle King Date: Mon, 31 Aug 2026 20:23:46 -0600 Subject: [PATCH] fix: footnote reorder is not idempotent for nested footnotes 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. --- mdformat_footnote/_reorder.py | 37 ++++++++++++++++++++---- tests/fixtures/footnote.md | 54 +++++++++++++++++++++++++---------- tests/test_fixtures.py | 33 +++++++++++++++++++++ 3 files changed, 103 insertions(+), 21 deletions(-) diff --git a/mdformat_footnote/_reorder.py b/mdformat_footnote/_reorder.py index 71c1bbf..9d27b2e 100644 --- a/mdformat_footnote/_reorder.py +++ b/mdformat_footnote/_reorder.py @@ -107,10 +107,27 @@ def _collect_nested_refs(token, ref_set: set[str]) -> None: _collect_nested_refs(child, ref_set) +def _collect_body_ref_order(body_refs: list) -> dict[str, int]: + """Map each label to the index of its first genuine body-level reference. + + Only `body_refs` (references outside any footnote definition) are + considered: a footnote's raw parse-assigned id can be contaminated by an + earlier *nested* occurrence (e.g. referenced from within another + footnote's definition, which may sit before the real body reference in + source), so it isn't safe to sort by id directly. + """ + order: dict[str, int] = {} + for token in body_refs: + label = token.meta["label"] + order.setdefault(label, len(order)) + return order + + def _categorize_footnotes( refs: dict, footnote_deps: dict[str, set[str]], refs_in_fences: list[str], + body_ref_order: dict[str, int], ) -> _FootnoteCategories: """Categorize footnotes.""" referenced_by_footnotes: set[str] = set() @@ -127,7 +144,7 @@ def _categorize_footnotes( for label_key, old_id in refs.items(): label = label_key[1:] match ( - old_id >= 0, + label in body_ref_order, label in referenced_by_footnotes, label in refs_in_fences_set, ): @@ -140,7 +157,10 @@ def _categorize_footnotes( case _: true_orphans.append(label_key) - body_referenced.sort(key=lambda x: x[0]) + # 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] return _FootnoteCategories(body_referenced, nested_only, fence_only, true_orphans) @@ -226,9 +246,10 @@ def _assign_subids_to_refs(ref_tokens: list, counters: dict[int, int]) -> None: counters[fn_id] = counters.get(fn_id, 0) + 1 -def _reassign_subids(tokens: list, refs: dict, footnote_list: dict) -> None: +def _reassign_subids( + body_refs: list, def_refs: dict[str, list], refs: dict, footnote_list: dict +) -> None: """Reassign subIds based on output order: body refs first, then definition refs.""" - body_refs, def_refs = _partition_refs_by_context(tokens) subid_counters: dict[int, int] = {} _assign_subids_to_refs(body_refs, subid_counters) @@ -270,7 +291,11 @@ def reorder_footnotes_by_definition( refs, old_list = data footnote_deps = _build_dependency_graph(state.tokens) refs_in_fences = _collect_refs_in_fences(state.tokens) - categories = _categorize_footnotes(refs, footnote_deps, refs_in_fences) + body_refs, def_refs = _partition_refs_by_context(state.tokens) + body_ref_order = _collect_body_ref_order(body_refs) + categories = _categorize_footnotes( + refs, footnote_deps, refs_in_fences, body_ref_order + ) if not keep_orphans: for orphan_key in categories.true_orphans: @@ -282,4 +307,4 @@ def reorder_footnotes_by_definition( state.env["footnotes"]["list"] = reorder_state.new_list _update_token_ids(state.tokens, reorder_state.old_to_new_id) - _reassign_subids(state.tokens, refs, reorder_state.new_list) + _reassign_subids(body_refs, def_refs, refs, reorder_state.new_list) diff --git a/tests/fixtures/footnote.md b/tests/fixtures/footnote.md index 49fc307..f7758b3 100644 --- a/tests/fixtures/footnote.md +++ b/tests/fixtures/footnote.md @@ -241,11 +241,35 @@ Start [^a] . Start [^a] +[^a]: References B [^b] + [^b]: References C [^c] [^c]: Final one +. -[^a]: References B [^b] + +Nested footnote followed by an independent footnote (idempotency regression) +. +Nesting.[^outer] + +[^outer]: References [^inner]. + +[^inner]: Independent. + +Later.[^c] + +[^c]: C. +. +Nesting.[^outer] + +Later.[^c] + +[^outer]: References [^inner]. + +[^inner]: Independent. + +[^c]: C. . @@ -293,12 +317,12 @@ Text [^outer] . Text [^outer] -[^inner]: Inner content - [^outer]: List item: - Item with [^inner] reference - Another item + +[^inner]: Inner content . @@ -314,11 +338,11 @@ Body: [^3] [^2] [^1] . Body: [^3] [^2] [^1] -[^1]: First +[^3]: Third with [^2] and [^1] [^2]: Second with [^1] -[^3]: Third with [^2] and [^1] +[^1]: First . @@ -334,11 +358,11 @@ Body [^a] [^b] [^c] . Body [^a] [^b] [^c] -[^c]: Defined first +[^a]: Defined third [^b] [^b]: Defined second [^c] -[^a]: Defined third [^b] +[^c]: Defined first . @@ -370,10 +394,10 @@ Body [^m] [^a] [^z] [^m] . Body [^m] [^a] [^z] [^m] -[^a]: First defined [^m] - [^m]: Middle defined +[^a]: First defined [^m] + [^z]: Last defined [^a] . @@ -389,11 +413,11 @@ Text [^outer] . Text [^outer] -[^inner]: Inner note - [^outer]: Quote: > Blockquote with [^inner] + +[^inner]: Inner note . @@ -409,11 +433,11 @@ Start [^3] . Start [^3] -[^1]: Level 1 +[^3]: Level 3 [^2] [^2]: Level 2 [^1] -[^3]: Level 3 [^2] +[^1]: Level 1 . @@ -431,9 +455,9 @@ Body [^used-second] [^used-first] . Body [^used-second] [^used-first] -[^used-first]: Used - [^used-second]: Also used [^used-first] + +[^used-first]: Used . diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py index 9e9a26d..1bb486b 100644 --- a/tests/test_fixtures.py +++ b/tests/test_fixtures.py @@ -43,3 +43,36 @@ def test_fixtures( ): output = mdformat.text(text, extensions={"footnote"}, options=options) assert output.rstrip() == expected.rstrip(), output + + +# A footnote referenced only from within another footnote that is itself an +# orphan (issue #7) survives one formatting pass but is a genuine orphan by +# the next, since its only referrer is gone. Pre-existing, unrelated to +# nested-footnote reordering; not covered by the idempotency guarantee below. +_IDEMPOTENCY_EXCLUDED_TITLES = { + "footnote-ref-inside-footnote (issue #7)", + "Issue 7: footnote ref inside footnote without body reference", +} + +IDEMPOTENCY_CASES = [ + tc for tc in TEST_CASES if tc[2] not in _IDEMPOTENCY_EXCLUDED_TITLES +] + + +@pytest.mark.parametrize( + "filename,line,title,text,expected,options", + IDEMPOTENCY_CASES, + ids=[f"{tc[0].replace('.md', '')}::{tc[2]}" for tc in IDEMPOTENCY_CASES], +) +def test_fixtures_are_idempotent( + filename: str, + line: int, + title: str, + text: str, + expected: str, + options: dict, +): + """Formatting already-formatted output must be a no-op.""" + once = mdformat.text(text, extensions={"footnote"}, options=options) + twice = mdformat.text(once, extensions={"footnote"}, options=options) + assert twice.rstrip() == once.rstrip(), twice