Skip to content

Extend tracing-test coverage for footnote branches and public API (#306) - #417

Open
lodyai[bot] wants to merge 2 commits into
mainfrom
issue-306-extend-tracing-tests-cover-all-find-footnote-end-branches-and-add-end-to-end-traced-wrapping-tests
Open

Extend tracing-test coverage for footnote branches and public API (#306)#417
lodyai[bot] wants to merge 2 commits into
mainfrom
issue-306-extend-tracing-tests-cover-all-find-footnote-end-branches-and-add-end-to-end-traced-wrapping-tests

Conversation

@lodyai

@lodyai lodyai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #306.

Post-merge review of PR #303 (which added tracing instrumentation to the
inline-wrapping token- and fragment-classification pipeline) identified two
testing gaps that were out of scope for that PR. This change closes both.

Gap 1 — find_footnote_end branch coverage

find_footnote_end emits three distinct trace! events; the existing
tracing_tests module in src/wrap/tokenize/parsing.rs only covered the
prefix_mismatch path directly, and the unterminated_bracket path was
already covered. This adds a direct #[traced_test] for the remaining
footnote_label_span_recognized branch, calling find_footnote_end with a
well-formed [^4] input and asserting the emitted event message plus the
structured start=/end=/token_length=4 fields, while upholding the
content-free logging convention (!logs_contain("[^4]")).

The assertion uses the exact emitted spelling recognized (Oxford -ize),
matching production code, rather than the -ised variant.

Gap 2 — end-to-end tracing through the public API

All prior tracing tests exercised internal helpers directly. This adds the
first integration-level tracing test, in tests/wrap_unit/footnotes.rs, that
drives the public wrap_text API with "Some text.[^1]" and asserts the
fragment classified / kind=FootnoteRef DEBUG event fires. This confirms
the instrumentation is reachable through the public API and not accidentally
gated away.

Note on tracing-test configuration

Enabling tracing-test's no-env-filter feature was required for Gap 2: by
default #[traced_test] installs a per-crate env filter that only records the
test crate's own events, which would silently hide events emitted from the
mdtablefix library crate in an integration test. This was not anticipated in
the coding plan but is necessary for the end-to-end test to observe the
library's instrumentation.

Validation

  • make check-fmt, make lint, make typecheck, make test — all green
    (lib 707/707, wrap_unit 84/84 including both new tests).
  • coderabbit review --agent — 0 findings.

References

🤖 Generated with Claude Code

Close two post-merge testing gaps identified after PR #303 added tracing
instrumentation to the inline-wrapping classification pipeline.

- Add a direct `#[traced_test]` for `find_footnote_end`'s
  `footnote_label_span_recognized` branch, asserting the emitted event and
  structured fields while upholding the content-free logging convention.
- Add the first integration-level tracing test driving the public
  `wrap_text` API with a footnote reference, proving the `fragment
  classified` / `FootnoteRef` DEBUG event survives through the pipeline.

Enable `tracing-test`'s `no-env-filter` feature so the integration test crate
captures events emitted from the `mdtablefix` library crate; the default
per-crate env filter would otherwise hide the library's instrumentation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@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 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 26, 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

The change enables crate-wide tracing capture in tests and adds assertions for footnote label recognition and FootnoteRef classification through both internal parsing and the public wrap_text API.

Changes

Tracing coverage

Layer / File(s) Summary
Tracing test capture setup
Cargo.toml
Enable the no-env-filter feature for tracing-test and document its purpose.
Footnote tracing assertions
src/wrap/tokenize/parsing_tests.rs, tests/wrap_unit/footnotes.rs
Assert structured footnote label-span events and FootnoteRef classification events from find_footnote_end and wrap_text.

Possibly related issues

  • #305 — Both changes add tracing assertions for footnote and fragment parsing paths.

Possibly related PRs

Suggested labels: Issue

Suggested reviewers: leynos

Poem

Footnotes whisper, traces glow,
Through parser paths the signals flow.
[^4] leaves fields in line,
FootnoteRef proves the sign.
Tests catch each event in time.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 2 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error find_footnote_end’s new test still only checks start=/end= presence, so a wrong span would pass. Assert the exact span values, e.g. start=0 and end=4, and keep the return value/oracle tied to the parsed footnote.
Developer Documentation ⚠️ Warning Cargo.toml adds tracing-test's no-env-filter feature, but no developer-guide/ADR update records this new test/build requirement; the guide only names tracing-test generically. Add a note in docs/developers-guide.md (or an ADR) explaining the no-env-filter requirement for integration tests, and tick off any linked roadmap item if one exists.
Testing (Unit And Behavioural) ⚠️ Warning The new direct find_footnote_end test only checks start=/end= exist, so it misses the span invariant and can pass on wrong offsets. Assert the exact fields (start=0, end=4) and keep the !logs_contain("[^4]") check so the test fails on incorrect span metadata.
✅ Passed checks (17 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the PR scope and includes the linked issue reference (#306).
Description check ✅ Passed The description stays on-topic and describes the tracing test coverage changes clearly.
Linked Issues check ✅ Passed The PR adds the requested branch tests and public API tracing test for #306.
Out of Scope Changes check ✅ Passed The Cargo.toml feature change supports the new integration test and adds no unrelated scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
User-Facing Documentation ✅ Passed No user-facing behaviour changed; the PR only adds tracing-test coverage and a dev-dependency feature, so docs/users-guide.md needs no update.
Module-Level Documentation ✅ Passed PASS: Both changed Rust modules have clear file-level docstrings, and the nested tracing_tests module is documented too.
Testing (Property / Proof) ✅ Passed The patch only adds branch-specific tracing tests and a tracing-test config; it introduces no new range-wide invariant or proof obligation.
Testing (Compile-Time / Ui) ✅ Passed PASS: No compile-time/UI behaviour is introduced; the change adds runtime tracing assertions, and the repo already uses focused snapshots for wrap output where needed.
Unit Architecture ✅ Passed Only dev-dependency and tests changed; production paths stay pure, with tracing exercised explicitly at the test boundary.
Domain Architecture ✅ Passed Only dev-dependencies and test files changed; no domain, transport, persistence, or adapter code was altered.
Observability ✅ Passed PASS: The PR only adds tracing tests and a dev-only tracing-test flag; no production observability, metrics, or alerting behaviour changes.
Security And Privacy ✅ Passed Only tracing tests and a dev-dependency feature changed; no secrets, auth, permissions, or unsafe input handling were introduced, and test inputs are fake.
Performance And Resource Use ✅ Passed PASS: keep changes test-only; no production hot paths or unbounded work were added, and the new tracing-test feature is dev-only.
Concurrency And State ✅ Passed PASS: The diff is test-only plus a dev-dependency flag; it adds no shared mutable state, locks, async tasks, or ordering-sensitive runtime behaviour.
Architectural Complexity And Maintainability ✅ Passed Only test coverage and a dev-dependency feature flag were added; no new runtime abstractions, layers, registries, or dependency cycles appear in the diff.
Rust Compiler Lint Integrity ✅ Passed No new #[allow]/#[expect] suppressions or fake usage anchors were added, and the touched tests add no suspicious clones or unused imports.
📋 Issue Planner

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

View plan used: #306

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-306-extend-tracing-tests-cover-all-find-footnote-end-branches-and-add-end-to-end-traced-wrapping-tests

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

@leynos
leynos marked this pull request as ready for review July 26, 2026 22:45
@pandalump

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 27, 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 added the Issue label Jul 27, 2026

@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: 1

🤖 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 `@src/wrap/tokenize/parsing_tests.rs`:
- Around line 281-282: In the test covering find_footnote_end("[^4] tail", 0),
replace the broad logs_contain checks for "start=" and "end=" with exact
assertions for "start=0" and "end=4", preserving the existing log validation.
🪄 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: d0d1332f-aaf3-448f-8043-05a595b66bba

📥 Commits

Reviewing files that changed from the base of the PR and between cbf7cb0 and fa76670.

📒 Files selected for processing (3)
  • Cargo.toml
  • src/wrap/tokenize/parsing_tests.rs
  • tests/wrap_unit/footnotes.rs

Comment thread src/wrap/tokenize/parsing_tests.rs Outdated
Comment on lines +281 to +282
assert!(logs_contain("start="));
assert!(logs_contain("end="));

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the exact span boundaries.

logs_contain("start=") and logs_contain("end=") only verify that the fields exist. For find_footnote_end("[^4] tail", 0), assert start=0 and end=4 so the test catches incorrect span metadata.

As per coding guidelines, tests must be substantive and fail for plausible incorrect implementations.

Suggested assertion tightening
-        assert!(logs_contain("start="));
-        assert!(logs_contain("end="));
+        assert!(logs_contain("start=0"));
+        assert!(logs_contain("end=4"));
🤖 Prompt for 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.

In `@src/wrap/tokenize/parsing_tests.rs` around lines 281 - 282, In the test
covering find_footnote_end("[^4] tail", 0), replace the broad logs_contain
checks for "start=" and "end=" with exact assertions for "start=0" and "end=4",
preserving the existing log validation.

Source: Coding guidelines

Address post-review feedback on the tracing-test additions.

- Assert exact span metadata (`start=0`, `end=4`) in the
  `find_footnote_end` label-span traced test so an incorrect offset fails
  the test, rather than only checking the fields are present.
- Document in the developers' guide why `tracing-test` is enabled with the
  `no-env-filter` feature: the default per-crate filter hides library
  events from integration tests under `tests/`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Extend tracing tests: cover all find_footnote_end branches and add end-to-end traced wrapping tests

1 participant