Skip to content

Add observability for inline wrapping token classification (#296) - #303

Merged
leynos merged 8 commits into
mainfrom
issue-296-add-observability-infrastructure-for-inline-wrapping-token-classification
May 29, 2026
Merged

Add observability for inline wrapping token classification (#296)#303
leynos merged 8 commits into
mainfrom
issue-296-add-observability-infrastructure-for-inline-wrapping-token-classification

Conversation

@leynos

@leynos leynos commented May 27, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #296

This branch brings the inline-wrapping token- and fragment-classification
pipeline into compliance with the project observability policy introduced
for GFM footnote-reference handling in #280.

  • Adds the tracing crate and derives Debug on SpanKind so
    classification outcomes appear as structured fields.
  • Emits debug! at high-value decision points (footnote parse, fragment
    classification, span promotion) and trace! at branch-level checks,
    using consistent field names (token, kind, start, end, width).
  • Defers operational metrics counters until a clear per-wrap use case
    emerges; instrumentation remains purely additive with no global
    subscriber or recorder installation.

Test plan

  • make check-fmt
  • make lint
  • make test
  • CodeRabbit review (coderabbit review --agent) — 0 findings

Made with Cursor

Summary by Sourcery

Add structured tracing around inline wrapping tokenization and classification to improve observability without changing wrapping behavior.

Enhancements:

  • Introduce structured debug/trace logging for footnote detection, fragment classification, token span grouping, and link/image parsing in the inline wrap pipeline.
  • Derive Debug for span classification types and add helpers to standardize and truncate logged fields for trace output.

Build:

  • Add the tracing crate as a dependency for observability instrumentation.

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 97d6c758-7cb3-48ed-93a6-4df4a7992656

📥 Commits

Reviewing files that changed from the base of the PR and between 849d518 and d9fe086.

📒 Files selected for processing (2)
  • docs/developers-guide.md
  • src/wrap/tokenize/parsing.rs

Observability Infrastructure for Inline-Wrapping Token Classification

Implements structured observability for the inline-wrapping token- and fragment-classification pipeline to address issue #296. Adds tracing instrumentation at decision points while keeping instrumentation additive (the library does not install any global tracing subscriber or metrics recorder).

Key Changes

  • Dependencies

    • Added tracing = "0.1" to [dependencies].
    • Added tracing-test = "0.2" to [dev-dependencies].
  • Instrumentation

    • src/wrap/tokenize/parsing.rs
      • Added #[tracing::instrument(level = "debug", skip(text), ret)] on parse_link_or_image and emits DEBUG events for parsed footnote references and parsed links/images (fields: token, is_image).
      • Added #[tracing::instrument(level = "trace", skip(text), ret)] on find_footnote_end and emits TRACE events for branch outcomes (fields: start, end, token, reason).
    • src/wrap/inline/fragment.rs
      • InlineFragment::new now logs a DEBUG "fragment classified" event via log_fragment_classification with a UTF‑8-safe token preview produced by trace_text_snippet.
      • trace_text_snippet returns a prefix of at most 80 bytes that never splits multi-byte UTF‑8 characters and a truncated boolean.
      • Logging is guarded with tracing::enabled!(Level::DEBUG).
    • src/wrap/inline/predicates.rs
      • Added #[tracing::instrument(level = "trace", ret)] to looks_like_footnote_ref and ends_with_footnote_ref so predicate invocations and return values are recorded.
    • src/wrap/inline/span_helpers.rs
      • SpanKind now derives Debug to support structured field serialisation.
  • Documentation

    • docs/developers-guide.md: Added an Observability section documenting dependency scope (runtime tracing vs test-only tracing-test), log-level conventions (library uses debug/trace, no info/above), stable structured field names (token, kind, start, end, width, truncated, reason, is_image), performance discipline (guard expensive computations with tracing::enabled!()), and guidance to redact the token field in production.
  • Testing

    • tests/compile.rs: Added a trybuild compile-pass test exercising an instrumented fixture.
    • tests/ui/tracing_instrument_pass.rs: Compile-pass fixture verifying #[tracing::instrument(level = "trace", ret)] is accepted on free functions.
    • tracing_tests modules and unit/proptest coverage:
      • Tests assert that instrumented functions emit events and that structured fields appear (e.g. token, truncated, is_image, reason, start, end).
      • Proptest verifies trace_text_snippet invariants: returns a valid char-boundary slice, length ≤ 80 bytes, and truncated flag correctness.

Design Decisions

  • Instrumentation is purely additive; no global subscriber/metrics recorder is installed by the library.
  • Expensive field computation is guarded with tracing::enabled!() to avoid allocations on hot paths.
  • UTF‑8-safe truncation helper replaces unsafe byte-slicing.
  • Operational metrics were deferred pending a clear per-wrap use case.
  • Follows AGENTS.md guidance on field-based structured logging with a stable core field set.

Follow-up Work

  • Issue #305: Add insta snapshot tests for exact serialized tracing output.
  • Issue #306: Add end-to-end traced wrapping tests and exhaustive find_footnote_end branch coverage.

Walkthrough

Introduce structured tracing across inline-wrap token classification: add tracing deps and docs; instrument parsing, fragment classification and predicate helpers; add compile- and runtime tests asserting emitted logs and attribute acceptance.

Changes

Inline-wrap token classification observability

Layer / File(s) Summary
Dependencies and observability documentation
Cargo.toml, docs/developers-guide.md
Add tracing = "0.1" runtime and tracing-test = "0.2" dev dependencies; add Observability docs, structured field names, performance guard and security guidance.
Parsing instrumentation
src/wrap/tokenize/parsing.rs
Import tracing::debug/trace; add #[tracing::instrument] to parse_link_or_image and find_footnote_end; emit DEBUG/TRACE events for footnote/link parsing, prefix mismatch, label recognised and unterminated bracket; add traced tests asserting messages and fields.
Fragment classification instrumentation
src/wrap/inline/fragment.rs, src/wrap/inline/span_helpers.rs
Add trace_text_snippet (80‑byte UTF‑8‑safe preview) and log_fragment_classification; call logging from InlineFragment::new to emit fragment classified events; derive Debug for SpanKind; add unit tests and a proptest.
Predicate helpers instrumentation
src/wrap/inline/predicates.rs
Annotate looks_like_footnote_ref and ends_with_footnote_ref with #[tracing::instrument(level = "trace", ret)] and add traced tests asserting TRACE output.
Compile and test fixtures
tests/compile.rs, tests/ui/tracing_instrument_pass.rs
Add trybuild compile‑pass fixture verifying #[tracing::instrument(level = "trace", ret)] compiles on free functions and include the UI fixture calling the instrumented helpers.

Sequence Diagram(s)

sequenceDiagram
  participant Tokeniser
  participant parse_link_or_image
  participant find_footnote_end
  participant InlineFragment
  participant trace_text_snippet
  participant Logging
  Tokeniser->>parse_link_or_image: attempt parse link/image or footnote
  parse_link_or_image->>find_footnote_end: delegate footnote label scan
  find_footnote_end->>Logging: emit prefix_mismatch / label recognised / unterminated_bracket
  parse_link_or_image->>Logging: emit footnote reference parsed / link or image parsed (token, is_image)
  InlineFragment->>trace_text_snippet: request UTF-8 snippet (80 bytes)
  trace_text_snippet-->>InlineFragment: return preview + truncated flag
  InlineFragment->>Logging: emit "fragment classified" debug event (token, truncated, kind)
Loading

Possibly related issues

Possibly related PRs

  • leynos/mdtablefix#301 — Instruments the same predicate helpers and adds analogous traced tests.
  • leynos/mdtablefix#280 — Introduced the inline fragment classification and footnote tokenisation paths now instrumented.

Poem

Trace the token, log the kindly cue,
Snip the bytes where UTF‑8 rings true,
Footnote, link or fragment's name,
Debug and trace record the frame,
Observatory lights reveal the view.


Caution

Pre-merge checks failed

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

  • Ignore

❌ Failed checks (1 error, 3 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Log-level requirements documented but untested; performance guards untested because #[traced_test] installs subscriber; parsing tests verify field presence only, not values. Add tests verifying log levels match documentation (debug vs trace); test that performance guards skip allocation when tracing disabled; strengthen parsing tests to verify field values.
Testing (Unit And Behavioural) ⚠️ Warning Tests verify tracing infrastructure emissions but lack end-to-end tests exercising wrapping with observability enabled through functional boundaries. Add end-to-end tests that exercise wrapping workflows and assert via tracing that classification decisions are observable from caller perspective.
Testing (Compile-Time / Ui) ⚠️ Warning Trybuild compile-pass test present, but snapshot tests for structured tracing output were deferred. Tests use logs_contain() string assertions only, not insta snapshots of serialised tracing output. Add insta snapshot tests to verify exact serialised format of tracing events and structured field values, not just presence of field names.
Domain Architecture ⚠️ Warning Domain functions directly import and use vendor-specific tracing framework (tracing::instrument, debug!, enabled!). These infrastructure concerns should be isolated at adapter boundaries. Define domain-level event types (e.g. enum ClassificationEvent) and pass optional observer into classification functions; emit domain events at boundaries, not vendor-specific tracing inside domain logic.
✅ Passed checks (16 passed)
Check name Status Explanation
Title check ✅ Passed The title correctly identifies the primary change—adding observability for inline wrapping token classification—and includes the issue reference (#296) as required.
Description check ✅ Passed The description comprehensively documents the changes, references issue #296, details the tracing instrumentation approach, and outlines the test plan.
Linked Issues check ✅ Passed The PR implements all acceptance criteria from #296: tracing instrumentation at classification decision points, no global subscriber installation, and consistent field-based logging across the inline-wrapping pipeline.
Out of Scope Changes check ✅ Passed All changes align directly with #296 objectives: tracing dependencies, instrumented classification paths, structured field logging, UTF-8 truncation helpers, and observability documentation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
User-Facing Documentation ✅ Passed Tracing instrumentation has no public API changes, behavioural changes, or impact on primary functionality. Appropriate documentation added to developers-guide.md.
Developer Documentation ✅ Passed Observability section properly documents tracing dependencies, logging conventions, field names, performance discipline, security considerations, and instrumented functions in developers-guide.md.
Module-Level Documentation ✅ Passed All six modified Rust modules carry comprehensive module-level docstrings (//!) explaining purpose, utility, and relationships to other components, meeting the custom check requirements.
Testing (Property / Proof) ✅ Passed The PR introduces trace_text_snippet with proptest coverage validating char-boundary slicing, ≤80 bytes, and truncation flag accuracy on arbitrary Unicode. Remaining changes are instrumentation-only.
Unit Architecture ✅ Passed Side-effects guarded via tracing::enabled!; query logic remains pure; logging separated from classification; dependencies transparent, not global.
Observability ✅ Passed Observability correctly implemented: meaningful decision points, bounded fields, UTF-8 safety, performance guards, security guidance, no global subscriber, comprehensive tests.
Security And Privacy ✅ Passed Token logging is safely truncated to 80 bytes, gated with tracing::enabled(), and documented with production redaction guidance. No secrets in code, no global subscriber, no trust boundary changes.
Performance And Resource Use ✅ Passed Logging operations properly guarded with tracing::enabled!() before allocations; no unbounded collections, clones, or blocking work on hot paths; UTF-8 truncation is O(n) with early exit.
Concurrency And State ✅ Passed No concurrency primitives, global mutable state, locks, or async work introduced. Library installs no global subscriber. Tracing guard checks are thread-safe and read-only.
Architectural Complexity And Maintainability ✅ Passed Standard tracing crate used; UTF-8 helper solves real problem; no new modules/traits; changes purely additive; performance guarded; scope matches requirements.
Rust Compiler Lint Integrity ✅ Passed No lint suppression attributes added. All new functions have real callers. Tracing imports used. Tests properly scoped. No artificial appeasement code.
📋 Issue Planner

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

View plan used: #296

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-296-add-observability-infrastructure-for-inline-wrapping-token-classification

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

@sourcery-ai

sourcery-ai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds tracing-based observability to the inline wrapping token, fragment classification, and link/footnote parsing pipeline without changing wrapping behavior, including debug/trace logs with consistent structured fields and a Debug-derived span kind.

Sequence diagram for link and footnote parsing with tracing

sequenceDiagram
    participant Wrapper
    participant parse_link_or_image
    participant find_footnote_end
    participant tracing

    Wrapper->>parse_link_or_image: parse_link_or_image(text, idx)
    parse_link_or_image->>find_footnote_end: find_footnote_end(text, idx)
    alt footnote_prefix_mismatch_or_unterminated
        find_footnote_end->>tracing: trace!(reason = prefix_mismatch or unterminated_bracket)
        find_footnote_end-->>parse_link_or_image: None
    else footnote_label_span_recognised
        find_footnote_end->>tracing: trace!(token, start, end)
        find_footnote_end-->>parse_link_or_image: Some(text_end)
        parse_link_or_image->>tracing: debug!(token)  %% footnote reference parsed
    end

    alt link_or_image_with_url
        parse_link_or_image->>tracing: trace!(token, is_image)
    else no_url_or_unbalanced
        parse_link_or_image-->>Wrapper: (token, next_idx)
    end

    parse_link_or_image-->>Wrapper: (token, next_idx)
Loading

Flow diagram for fragment classification with structured tracing

flowchart TD
    A[Input text] --> B{is_whitespace_token}
    B -- yes --> K[FragmentKind::Whitespace]
    B -- no --> C[trim_end_matches is_trailing_punct]
    C --> D[trim_start_matches is_opening_punct]
    D --> E{is_inline_code_token on variants}
    E -- yes --> L[FragmentKind::InlineCode]
    E -- no --> F{fragment_is_link}
    F -- yes --> M[FragmentKind::Link]
    F -- no --> G{looks_like_footnote_ref or ends_with_footnote_ref}
    G -- yes --> N[FragmentKind::FootnoteRef]
    G -- no --> O[FragmentKind::Plain]

    K --> P[debug! fragment classified]
    L --> P
    M --> P
    N --> P
    O --> P

    P --> Q[Output FragmentKind]
Loading

File-Level Changes

Change Details Files
Introduce structured tracing/logging for inline span classification and promotion in the wrapper.
  • Add tracing dependency and import debug/trace macros in inline wrapping module.
  • Derive Debug for SpanKind so it can be logged as a structured field.
  • Log successful footnote-ref detection in helper predicates using trace! with the token field.
  • Instrument promote_footnote_span with debug! on promotions and trace! when promotion is not applicable, including span metadata.
  • Add helper functions to emit initial and final span classification logs with consistent fields and invoke them at key return points in determine_token_span.
src/wrap/inline.rs
Cargo.toml
Cargo.lock
Add fragment-level observability for inline fragment classification.
  • Introduce fragment_text_for_trace to truncate very long fragment text for logging while preserving signal.
  • Refactor classify_fragment into an expression that computes FragmentKind then emits a debug! log with text and kind.
  • Ensure footnote, link, code, whitespace, and plain fragment paths all result in a structured classification log.
src/wrap/inline/fragment.rs
Instrument link/image and footnote parsing in the tokenizer for better visibility into parse decisions.
  • Add debug! log when a footnote reference token is parsed, emitting the collected token.
  • Add trace! log when a link or image is parsed, including whether it is an image and the parsed token.
  • Extend find_footnote_end to log negative cases (prefix mismatch, unterminated bracket) and positive recognition of a footnote label span, including the token range.
src/wrap/tokenize/parsing.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#296 Add structured tracing (debug/trace) events at all non-trivial classification decision points in the inline wrapping pipeline (src/wrap/tokenize/, src/wrap/inline/, src/wrap/inline/fragment.rs), including footnote and other token/fragment classifications, using field-based logging and without installing a global tracing subscriber.
#296 Evaluate the use of operational metrics (e.g., counters for recognised footnote tokens per wrap/document) and ensure any observability changes remain purely additive, with no global metrics recorder or tracing subscriber installed.
#296 Establish a consistent observability convention for token and fragment classification (including non-footnote paths) so that present and future classification code paths can be instrumented uniformly.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@leynos
leynos marked this pull request as ready for review May 27, 2026 20:46
sourcery-ai[bot]

This comment was marked as resolved.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@coderabbitai coderabbitai Bot added the Issue label May 27, 2026
coderabbitai[bot]

This comment was marked as resolved.

@leynos

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@lodyai
lodyai Bot force-pushed the issue-296-add-observability-infrastructure-for-inline-wrapping-token-classification branch from 9766055 to 3413ce3 Compare May 28, 2026 00:09
coderabbitai[bot]

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@leynos

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@leynos

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@coderabbitai

coderabbitai Bot commented May 28, 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/ui/tracing_instrument_pass.rs ==="
cat tests/ui/tracing_instrument_pass.rs

echo ""
echo "=== tests/compile.rs ==="
cat tests/compile.rs

echo ""
echo "=== src/wrap/inline/fragment.rs - test block ==="
awk '/^#\[cfg\(test\)\]/,0' src/wrap/inline/fragment.rs

echo ""
echo "=== src/wrap/inline/predicates.rs - full content ==="
cat src/wrap/inline/predicates.rs

echo ""
echo "=== logs_contain assertions in fragment.rs ==="
rg -n "logs_contain\|assert!" --type=rust src/wrap/inline/fragment.rs

echo ""
echo "=== logs_contain assertions in predicates.rs ==="
rg -n "logs_contain\|assert!" --type=rust src/wrap/inline/predicates.rs

echo ""
echo "=== traced_test usage ==="
rg -rn "traced_test" --type=rust -A3 -B2

Length of output: 13850



@leynos All seven checks are now resolved. Here is the verified status based on the current branch content.


✅ All checks resolved

Check Evidence
Testing (Overall) Docstring in tests/ui/tracing_instrument_pass.rs reads #[tracing::instrument] is accepted on free functions that return bool. — no mention of Option.
Module-Level Documentation Same docstring fix.
Unit Architecture Same docstring fix.
Testing (Unit And Behavioural) All three tracing_tests functions in fragment.rs wrap logs_contain() in assert!(); predicates.rs has #[traced_test] tests for both looks_like_footnote_ref and ends_with_footnote_ref, each with assert!(logs_contain(...)).
Testing (Compile-Time / Ui) tracing_instrument_attributes_compile test present in tests/compile.rs; fixture docstring accurate; trybuild::TestCases::pass("tests/ui/tracing_instrument_pass.rs") wired up.
Observability Docstring fix eliminates the inaccurate Option claim.
Architectural Complexity And Maintainability Docstring fix removes the documentation debt.

The proptest in fragment.rs additionally asserts all three documented invariants — char-boundary safety, snippet.len() <= 80, and truncated == s.len() > 80 — closing the earlier property-test gap as well.

The sole remaining deferred item is #305 (insta snapshot tests for structured tracing output), which was already filed and is out of scope for this PR.

@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@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 error, 2 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Tests use only string-matching and would not catch incorrect field values, inverted conditions, or logic bypass. Removing tracing::enabled!() guards would not cause test failures. Strengthen tests to assert structured field values via logs_contain patterns or snapshot testing; add tests for tracing::enabled!() guards and log-level changes.
Testing (Unit And Behavioural) ⚠️ Warning Tests verify logging occurs but lack meaningful coverage: only 3 of 5 fragment types tested; assertions check substrings not structured field values. Cover all 5 fragment types; validate structured field values (token, kind, truncated); test truncation edge cases.
Testing (Compile-Time / Ui) ⚠️ Warning Trybuild compile-pass test present for #[tracing::instrument] attributes, but snapshot tests for structured tracing output are absent. Current tests only verify event emission, not output structure. Add insta snapshot tests capturing structured tracing event output (token, kind, truncated fields) to encode meaningful behavioural expectations and prevent incidental output churn.

@coderabbitai

This comment was marked as resolved.

@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@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 error, 2 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Tests use only string-matching and would not catch incorrect field values, inverted conditions, or logic bypass. Removing tracing::enabled!() guards would not cause test failures. Strengthen tests to assert structured field values via logs_contain patterns or snapshot testing; add tests for tracing::enabled!() guards and log-level changes.
Testing (Unit And Behavioural) ⚠️ Warning Tests verify logging occurs but lack meaningful coverage: only 3 of 5 fragment types tested; assertions check substrings not structured field values. Cover all 5 fragment types; validate structured field values (token, kind, truncated); test truncation edge cases.
Testing (Compile-Time / Ui) ⚠️ Warning Trybuild compile-pass test present for #[tracing::instrument] attributes, but snapshot tests for structured tracing output are absent. Current tests only verify event emission, not output structure. Add insta snapshot tests capturing structured tracing event output (token, kind, truncated fields) to encode meaningful behavioural expectations and prevent incidental output churn.

@coderabbitai

coderabbitai Bot commented May 28, 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 "=== src/wrap/inline/fragment.rs - full test block ==="
awk '/^#\[cfg\(test\)\]/,0' src/wrap/inline/fragment.rs

echo ""
echo "=== FragmentKind variants ==="
rg -n "enum FragmentKind" --type=rust -A10 src/wrap/inline/fragment.rs

echo ""
echo "=== InlineFragment struct ==="
rg -n "struct InlineFragment\|pub kind\|pub fn kind\| kind:" --type=rust -A5 src/wrap/inline/fragment.rs

echo ""
echo "=== logs_contain assertions in fragment.rs ==="
rg -n "logs_contain\|assert!\|traced_test" --type=rust src/wrap/inline/fragment.rs

echo ""
echo "=== insta usage ==="
rg -rn "insta\|assert_snapshot" --type=rust
rg -n "insta" Cargo.toml

echo ""
echo "=== open issues ==="
gh issue list --repo leynos/mdtablefix --state open --limit 50

Length of output: 5551



@leynos All three checks are now resolved or properly deferred.


✅ All checks resolved

Check Evidence
Testing (Overall) All five FragmentKind variants are exercised under #[traced_test]; every logs_contain() call is wrapped in assert!(); structured field assertions (kind=<Variant>, token=, truncated=) pin specific field values, not just bare substrings; fragment_classification_does_not_require_subscriber (no #[traced_test]) asserts fragment.kind == FragmentKind::FootnoteRef with no subscriber installed, pinning correctness of classification independent of the tracing infrastructure.
Testing (Unit And Behavioural) All five variants covered; structured field names asserted in every traced test; truncation edge cases covered by trace_snippet_tests (char-boundary on mixed-byte input) and the proptest (all three documented invariants: char boundary, len ≤ 80, truncation-flag accuracy).
Testing (Compile-Time / Ui) Trybuild compile-pass fixture and its wiring in tests/compile.rs are present. Insta snapshot tests are absent but are tracked in the already-open issue #305 — no further action needed in this PR.

@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

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: 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/inline/fragment.rs`:
- Around line 218-266: Replace the five near-identical tests
fragment_classification_logs_footnote_ref,
fragment_classification_logs_inline_code, fragment_classification_logs_link,
fragment_classification_logs_whitespace, and fragment_classification_logs_plain
with a single #[traced_test] parameterised test using rstest that iterates input
strings and their expected kinds (e.g. ("[^1]", "FootnoteRef"), ("`code`",
"InlineCode"), ("[text](...)", "Link"), ("   ", "Whitespace"), ("plain",
"Plain")) and asserts the same log fragments ("fragment classified",
"kind={expected}", "token=", "truncated=") after creating
InlineFragment::new(input.to_string()); keep the existing
fragment_classification_does_not_require_subscriber test separate; add use
rstest::rstest; to the tracing_tests imports so rstest is available.
🪄 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: 21bda48d-40f0-49bb-9870-e7c9a900671c

📥 Commits

Reviewing files that changed from the base of the PR and between 420f3e3 and aada328.

📒 Files selected for processing (1)
  • src/wrap/inline/fragment.rs

Comment thread src/wrap/inline/fragment.rs
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Overall Comments
- The new tracing in `classify_fragment` calls `fragment_text_for_trace`, which allocates a `String` for every fragment even when logging is disabled; consider logging `&str` directly and letting the subscriber handle truncation, or returning a `Cow<'_ ,str>` to avoid allocations on the hot path.
- Several trace/debug statements (e.g., in `find_footnote_end` and `parse_link_or_image`) construct owned `String`s via `collect_range` purely for logging; you may want to log slices (`&str`) or defer allocation behind `debug_enabled!()`/`trace_enabled!()` checks to reduce overhead in tight parsing loops.

## Individual Comments

### Comment 1
<location path="src/wrap/inline/fragment.rs" line_range="122" />
<code_context>
+        text.to_string()
     } else {
-        FragmentKind::Plain
+        format!("{}…", &text[..MAX_TRACE_TEXT])
     }
 }
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a fixed byte index to slice `&str` can panic on non-ASCII input.

`text[..MAX_TRACE_TEXT]` slices by byte index, so if `text` has multi-byte chars and the cutoff falls inside one, this will panic. Please truncate on a character or grapheme boundary instead (e.g., via `char_indices` or `unicode-segmentation`) to ensure valid UTF-8 slices.
</issue_to_address>

### Comment 2
<location path="src/wrap/inline/fragment.rs" line_range="115" />
<code_context>
 /// still recognised as links or code spans. Footnote references also recognise
 /// the `word.[^label]` suffix shape that the wrapper groups to avoid splitting
 /// sentence punctuation from the marker.
-fn classify_fragment(text: &str) -> FragmentKind {
-    if is_whitespace_token(text) {
-        return FragmentKind::Whitespace;
</code_context>
<issue_to_address>
**issue (review_instructions):** `classify_fragment` both performs classification and logging, and still embeds a multi-branch conditional instead of delegating the branchy logic into a predicate helper.

This function is now doing two things: computing the `FragmentKind` and emitting structured tracing. To better follow single-responsibility/CQRS, consider keeping `classify_fragment` focused on determining the kind (pure classification) and moving the logging into a thin wrapper or caller, or into a dedicated helper that takes the already-computed kind.

Additionally, the nested `if / else if` chain that selects the `FragmentKind` has more than two branches. Per the guideline about conditionals with >2 branches, it would be better to extract this branching logic into one or more predicate helpers (e.g. `is_inline_code_fragment(..)`, `is_link_fragment(..)`, etc.) rather than keeping all the cases inline here.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.rs`

**Instructions:**
* Adhere to single responsibility and CQRS
* Move conditionals with >2 branches into a predicate function.

</details>
</issue_to_address>

@leynos

leynos commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai please create a new GitHub issue for the following:

Domain Architecture ⚠️ Warning Domain functions directly import and use vendor-specific tracing framework (tracing::instrument, debug!, enabled!). These infrastructure concerns should be isolated at adapter boundaries. Define domain-level event types (e.g. enum ClassificationEvent) and pass optional observer into classification functions; emit domain events at boundaries, not vendor-specific tracing inside domain logic.

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

gh issue create \
  --repo leynos/mdtablefix \
  --title "Isolate vendor-specific tracing from domain logic behind adapter boundaries" \
  --assignee leynos \
  --body "## Background

Raised during review of PR `#303` by `@leynos`.

Domain functions in \`src/wrap/inline/\` and \`src/wrap/tokenize/\` currently import and use the vendor-specific \`tracing\` framework directly (\`#[tracing::instrument]\`, \`debug!\`, \`trace!\`, \`tracing::enabled!\`). This couples infrastructure concerns to domain logic, making the domain harder to test without a tracing subscriber and harder to migrate to a different observability backend in the future.

## Required work

1. Define domain-level event types — e.g. \`enum ClassificationEvent\` — that capture the observable outcomes of classification and parsing decisions without referencing any external crate.
2. Refactor classification functions (\`classify_fragment_kind\`, \`parse_link_or_image\`, \`find_footnote_end\`, the footnote predicate helpers) to accept an optional observer (e.g. a callback or a trait object implementing an \`Observer\` trait) and emit domain events through it, rather than emitting tracing calls inline.
3. Implement a \`tracing\`-backed adapter that translates \`ClassificationEvent\` values into the appropriate \`debug!\`/\`trace!\` calls. Wire this adapter at the library boundary so existing instrumented behaviour is preserved.
4. Remove direct \`tracing\` imports from all domain modules; retain the dependency only in the adapter layer and in \`[dev-dependencies]\` for tests.

## Acceptance criteria

- No \`use tracing::\` import appears inside domain modules (\`src/wrap/inline/\`, \`src/wrap/tokenize/\`).
- Domain functions can be called without any tracing subscriber installed and without any \`enabled!\` guard (all guards move to the adapter).
- Existing traced-event tests continue to pass via the adapter.
- Linters and tests pass under \`make test\`.

## References

- PR `#303`
- Issue `#296`
"

Length of output: 2075



@leynos Issue #309 — "Isolate vendor-specific tracing from domain logic behind adapter boundaries" — has been created at #309 and assigned to you.

@leynos
leynos merged commit 12993ce into main May 29, 2026
3 checks passed
@leynos
leynos deleted the issue-296-add-observability-infrastructure-for-inline-wrapping-token-classification branch May 29, 2026 12:08
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.

Add observability infrastructure for inline-wrapping token classification

1 participant