Skip to content

Preserve inline GFM footnote reference attachment (#353) - #355

Merged
leynos merged 7 commits into
mainfrom
issue-353-preserve-inline-gfm-footnote-reference-attachment
Jun 9, 2026
Merged

Preserve inline GFM footnote reference attachment (#353)#355
leynos merged 7 commits into
mainfrom
issue-353-preserve-inline-gfm-footnote-reference-attachment

Conversation

@lodyai

@lodyai lodyai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

This branch preserves the attachment between sentence punctuation and immediately following inline GFM footnote references during Markdown wrapping and reflow.

Closes #353.

Review walkthrough

Validation

  • cargo test footnote_ref_spacing: passed.
  • cargo test --test wrap_unit footnote: passed.
  • cargo test --test wrap_cli footnote: passed.
  • make check-fmt: passed.
  • make lint: passed.
  • make test: passed.
  • make markdownlint: passed.
  • coderabbit review --agent --type uncommitted: completed with 0 findings.
  • coderabbit review --agent --type committed --base origin/main: completed with 0 findings after the free CLI rate-limit wait.

Notes

The first full-branch CodeRabbit attempt exceeded the free CLI file limit because local main is stale. The committed review was rerun against origin/main, which matches the pull request diff.

Normalize whitespace between sentence punctuation and immediately following
GFM footnote references before inline wrapping. Keep footnote definitions
excluded so definition lines remain structurally independent.

Add unit, wrap, snapshot, and CLI regressions for same-line spacing, split
references, indented split references, punctuation variants, and definition
exclusion.
@lodyai
lodyai Bot force-pushed the issue-353-preserve-inline-gfm-footnote-reference-attachment branch from 9ad5865 to 86eaef0 Compare June 8, 2026 20:26
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Apply a new inline-token normaliser that removes whitespace between sentence-ending punctuation and immediately following GFM inline footnote references, integrate it into wrap_preserving_code immediately after tokenisation, and validate behaviour with unit, property and CLI regression tests; preserve footnote definition lines unchanged.

Changes

Inline GFM Footnote Reference Attachment Normalisation

Layer / File(s) Summary
Normaliser implementation and tests
src/wrap/inline/normalize.rs
normalize_footnote_ref_spacing scans token streams for (punctuation, whitespace, inline-footnote-ref) windows, collapses matched windows by removing the whitespace, returns a Cow<'_, [String]>, and includes rstest and proptest coverage for matching, idempotence, and definition exclusion.
Integration with wrapping pipeline
src/wrap/inline.rs
Declare mod normalize, import normalize_footnote_ref_spacing, and call it inside wrap_preserving_code immediately after tokenisation before fragment construction and rendering.
Unit tests and helper updates
tests/wrap_unit/footnotes.rs
Enhance assert_footnote_reference_is_intact to detect newline-split footnote markers; add parametrised tests for punctuation cases, split-line normalisation (including indentation), boundary-wrapping snapshot, and preservation of footnote definition lines.
CLI regression test
tests/wrap_cli.rs
Add regression test cli_wrap_in_place_reattaches_split_inline_footnote_reference asserting --wrap --in-place reattaches split inline footnote markers and preserves definitions.
Docs
docs/developers-guide.md, docs/users-guide.md
Document the new normalisation stage and the user-facing wrapping rule that punctuation followed by inline footnote references remains attached; record the new symbol in the internal API table.

Possibly related PRs

  • leynos/mdtablefix#280: Related changes to inline span/fragment classification to keep [^label] atomic and coupled to adjacent punctuation.
  • leynos/mdtablefix#301: Related fixes around coupling inline footnote refs to preceding punctuation and interaction with span-detection (determine_token_span / try_couple_footnote_reference).
  • leynos/mdtablefix#125: Earlier punctuation-handling changes in the inline wrapping pipeline that intersect with this normalisation.

Poem

Footnotes drifted off on a new line, astray,
Attach them to punctuation; bring them to stay.
Collapse the gap where a marker was tossed,
Keep definitions alone, leave none double-crossed.
Now citations cling close — neat, tidy, and A-OK.

🚥 Pre-merge checks | ✅ 19 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Developer Documentation ⚠️ Warning normalize_footnote_ref_spacing documented in developers-guide.md but absent from architecture.md's flowchart and sequence diagrams showing the tokenisation-to-fragment pipeline. Update architecture.md tokeniser flowchart to show normalisation between steps B and C; update sequence diagram to show normalize_footnote_ref_spacing within wrap_preserving_code.
✅ Passed checks (19 passed)
Check name Status Explanation
Title check ✅ Passed The title directly references the main change objective—preserving inline GFM footnote reference attachment during wrapping—and correctly links the closed issue #353.
Description check ✅ Passed The description comprehensively summarises the change, outlines the review walkthrough, documents validation steps, and transparently notes the CodeRabbit rerun against origin/main.
Linked Issues check ✅ Passed The changeset fully implements all acceptance criteria from issue #353: normalising split footnote references back to attachment, preserving existing attachment, excluding definition lines, maintaining markdownlint compliance, and providing comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes align directly with issue #353 objectives: the normaliser, integration point, unit tests, CLI test, snapshot, and documentation updates serve the goal of preserving punctuation–footnote reference attachment.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Testing (Overall) ✅ Passed 22 non-vacuous tests cover normalize_footnote_ref_spacing across unit, property, integration, and CLI layers. All tests would fail if implementation were removed, broken, or inverted.
User-Facing Documentation ✅ Passed docs/users-guide.md adequately documents inline GFM footnote reference attachment with examples, normalisation, and definition exclusion rules for end users.
Module-Level Documentation ✅ Passed New module normalize.rs has module-level documentation explaining purpose (token boundary repair), utility (prevents punctuation-construct separation), and relationship to wrapping pipeline.
Testing (Unit And Behavioural) ✅ Passed 14 unit examples, 5 property tests, 8 behavioural wrap_text tests, and CLI integration test cover spacing, splits, indentation, punctuation, definition exclusion per requirements.
Testing (Property / Proof) ✅ Passed Proptest tests cover five substantive invariants with diverse input strategies: non-whitespace preservation, token count, idempotence, pattern collapse, and definition exclusion.
Testing (Compile-Time / Ui) ✅ Passed Snapshot tests encode meaningful behavioural expectations for wrapped output with punctuation attached to footnote references. Trybuild tests not applicable (internal API).
Unit Architecture ✅ Passed Pure query function, immutable inputs, no side-effects or I/O, correctly positioned in call chain, unit/property/integration tests verify boundaries, no hidden dependencies.
Domain Architecture ✅ Passed New normalize.rs expresses pure domain logic (markdown token normalisation) without leaking HTTP, SQL, CLI, or infrastructure concerns; correct abstraction level within wrap/inline boundary.
Observability ✅ Passed Pure synchronous data-transformation function for offline CLI text processing with no I/O, network, async, or failure modes requiring logging, metrics, or tracing.
Security And Privacy ✅ Passed Pull request introduces only markdown formatting logic with no secrets, injection risks, unsafe operations, or sensitive data exposure in code, tests, fixtures, or documentation.
Performance And Resource Use ✅ Passed Normalise function uses O(n) linear scan with Cow optimisation. Clones only when pattern matched. Called once per paragraph. Property tests verify no regression.
Concurrency And State ✅ Passed No async, locks, shared state, or concurrency primitives introduced. Function uses immutable inputs and clean Cow-based ownership with comprehensive property tests.
Architectural Complexity And Maintainability ✅ Passed New module properly scoped to wrap::inline, reuses existing predicates, no circular dependencies, comprehensive tests, and solves specific problem with clear single responsibility.
Rust Compiler Lint Integrity ✅ Passed No lint suppressions hiding unused code; clones are intentional (Cow optimises fast path, tests reuse proptest values); module boundaries are clean with proper scoping.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #353

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-353-preserve-inline-gfm-footnote-reference-attachment

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @LodyAI[bot], you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@leynos
leynos marked this pull request as ready for review June 8, 2026 21:38
@coderabbitai coderabbitai Bot added the Issue label Jun 8, 2026
Add user and developer documentation for punctuation-attached inline GFM
footnote references and the normalisation pass that preserves them.

Cover the normaliser with property tests for token preservation, matched
spacing removal, definition exclusion, generated pattern handling, and
idempotence. Exclude already parsed footnote reference tokens from the
preceding-punctuation match so repeated normalisation cannot join adjacent
references.
@leynos

leynos commented Jun 8, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Rename the spacing predicate so it describes the matched pattern directly.
Return a borrowed token slice when no collapsible footnote spacing exists,
avoiding an unconditional clone of every paragraph token stream.
@leynos

leynos commented Jun 8, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already.

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
User-Facing Documentation ⚠️ Warning PR implements preservation of sentence punctuation plus inline footnote references, but users-guide.md only documents the code-span/link case, not general punctuation. Document in users-guide.md that inline GFM footnote references immediately following punctuation (periods, commas, etc.) are kept attached as unbreakable units.
Developer Documentation ⚠️ Warning New module src/wrap/inline/normalize.rs with normalize_footnote_ref_spacing lacks documentation in developers-guide.md despite similar modules being documented in Key types and functions table. Add normalize_footnote_ref_spacing function entry to developers-guide.md Key types and functions table referencing src/wrap/inline/normalize.rs.
Testing (Property / Proof) ⚠️ Warning Normalise function maintains invariants over token sequences using only rstest fixtures; proptest—extensively used elsewhere in codebase—should validate these invariants across generated inputs. Add proptest tests verifying: token count preservation, non-whitespace token preservation, pattern matching, definition exclusion, and idempotence.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check 1: users-guide.md for footnote+punctuation documentation
echo "=== users-guide.md (footnote-related sections) ==="
fd -i 'users-guide.md' --exec grep -n -i 'footnote\|punctuation\|unbreakable\|attached' {}

# Check 2: developers-guide.md for normalize_footnote_ref_spacing entry
echo ""
echo "=== developers-guide.md (normalize / footnote sections) ==="
fd -i 'developers-guide.md' --exec grep -n -i 'normalize\|footnote' {}

# Check 3: proptest usage in normalize.rs or footnote test files
echo ""
echo "=== proptest in normalize.rs ==="
fd -e rs --exec grep -l 'proptest' {} | head -20

echo ""
echo "=== src/wrap/inline/normalize.rs contents ==="
fd -e rs -p 'normalize' --exec cat {}

echo ""
echo "=== tests/wrap_unit/footnotes.rs (check for proptest) ==="
fd -e rs -p 'footnotes' --exec grep -n 'proptest\|prop_' {}

Length of output: 16100



@leynos: All three warnings have been resolved.

Check Status Evidence
User-Facing Documentation ✅ Resolved users-guide.md lines 82–87 now document that inline GFM footnote references immediately following sentence punctuation (., ,, ?, ", etc.) are kept attached as unbreakable units, with concrete examples (Sentence.[^ref], Sentence,[^ref], etc.) and explicit exclusion of definition lines.
Developer Documentation ✅ Resolved developers-guide.md contains a normalize_footnote_ref_spacing row in the Key types and functions table, referencing src/wrap/inline/normalize.rs.
Testing (Property / Proof) ✅ Resolved src/wrap/inline/normalize.rs contains a proptest! block with five property tests covering non-whitespace token preservation, token-count invariant, idempotence, pattern matching, and definition exclusion.

No further work is required on these items.

@leynos

leynos commented Jun 8, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/users-guide.md`:
- Line 86: Replace the British spelling "normalised" with the Oxford -ize form
"normalized" in the sentence fragment that reads "or `Sentence.` followed by
`[^ref]` is normalised back to `Sentence.[^ref]`" so the line becomes consistent
with the en-GB-oxendict `-ize` style rule.

In `@tests/wrap_unit/footnotes.rs`:
- Around line 89-95: The test constructs input with embedded "\n" inside one
string, so it doesn't exercise wrap_text's paragraph-joining path; change the
test so input is a Vec<String> of physical lines instead of a single string
containing '\n' (i.e. use lines_vec! with two entries like "Word." and "[^1]"
and the variant with leading spaces "  [^1]") when building the `input` passed
to wrap_text in wrap_text_normalizes_split_footnote_refs_in_same_paragraph; keep
the existing assertions unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 64f50bf5-1e14-4df0-b31a-fed801caf6af

📥 Commits

Reviewing files that changed from the base of the PR and between fec9cef and cb7e153.

⛔ Files ignored due to path filters (1)
  • tests/snapshots/inline_footnote_reference_boundary_wrap.snap is excluded by !**/*.snap
📒 Files selected for processing (6)
  • docs/developers-guide.md
  • docs/users-guide.md
  • src/wrap/inline.rs
  • src/wrap/inline/normalize.rs
  • tests/wrap_cli.rs
  • tests/wrap_unit/footnotes.rs

Comment thread docs/users-guide.md Outdated
Comment thread tests/wrap_unit/footnotes.rs Outdated
@leynos

leynos commented Jun 8, 2026

Copy link
Copy Markdown
Owner

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews resumed.

Replace the British "normalised" variant in users guide prose with
"normalized" to match documented -ize conventions.

Adjust the inline footnote split test to pass physical lines
(`Word.` + `[^1]`) so it exercises paragraph joining behaviour
instead of encoding a newline in one string.
@leynos

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

leynos added 2 commits June 9, 2026 02:36
Refactor `wrap_text_normalizes_split_footnote_refs_in_same_paragraph` to use
`lines_vec!`-based `Vec<String>` test cases directly so the input already
represents physical lines. Remove the implicit newline split in test setup so the
case validates paragraph joining behavior.
Remove the stray blank line in `wrap_text_normalizes_split_footnote_refs_in_same_paragraph`
after the function signature so `cargo fmt --check` is clean.
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== tests/wrap_unit/footnotes.rs (lines 80-120) ==="
fd -e rs -p 'footnotes' --exec sed -n '80,120p' {}

Length of output: 10422



@leynos: Yes, this check is now resolved. The test has been refactored to the required form:

#[rstest]
#[case(lines_vec!["Word.", "[^1]"])]
#[case(lines_vec!["Word.", "  [^1]"])]
fn wrap_text_normalizes_split_footnote_refs_in_same_paragraph(#[case] input: Vec<String>) {
    assert_eq!(wrap_text(&input, 80), lines_vec!["Word.[^1]"]);
}

The embedded \n strings and the runtime .split('\n').map(ToString::to_string).collect() construction are gone. Each case now passes a physical Vec<String> directly, exercising the real paragraph-join path through wrap_text. No further work is required.

@leynos

leynos commented Jun 9, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

)

Add `normalize_footnote_ref_spacing` to the architecture tokeniser flow
between tokenization and fragment grouping, and include the same step in the
`wrap_preserving_code` sequence diagram so the pipeline and sequence view match
current inline wrapping implementation.
@leynos

leynos commented Jun 9, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already.

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Developer Documentation ⚠️ Warning normalize_footnote_ref_spacing documented in developers-guide.md but absent from architecture.md's flowchart and sequence diagrams showing the tokenisation-to-fragment pipeline. Update architecture.md tokeniser flowchart to show normalisation between steps B and C; update sequence diagram to show normalize_footnote_ref_spacing within wrap_preserving_code.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== architecture.md (full) ==="
fd -i 'architecture.md' --exec cat {}

Length of output: 22867



@leynos: Yes, this check is now resolved.

Check Status Evidence
Developer Documentation ✅ Resolved architecture.md tokeniser flowchart now includes step C, "Normalize inline footnote reference spacing", between the tokenisation step and the fragment-grouping step. The descriptive paragraph beneath that flowchart explicitly names normalize_footnote_ref_spacing. The wrap-sequence diagram shows WP->>IH: normalize_footnote_ref_spacing as a call from wrap_preserving_code into the inline helpers. Both required insertion points are present and correct.

No further work is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Preserve Inline GFM Footnote Reference Attachment

1 participant