Treat date-like sequences as atomic inline fragments (#346) - #350
Conversation
|
Warning Review limit reached
More reviews will be available in 33 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR extends the inline text-wrapping pipeline to recognise and preserve English date sequences (e.g., ChangesDate-sequence atomicity in text wrapping
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 6 warnings)
✅ Passed checks (12 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideTreat common English date-like token sequences as single atomic inline fragments during wrapping by adding date component predicates, a date sequence matcher in span helpers, integrating it into span determination, and covering behavior with new unit and property tests. Flow diagram for date sequence handling in determine_token_spanflowchart TD
A_determine[determine_token_span]
B_try_date[try_match_date_sequence]
C_date_found[[date_end found]]
D_no_date[[no date sequence]]
E_width[sum token widths]
F_footnote[try_couple_footnote_reference]
G_return_date[(return date_end, date_width)]
H_return_footnote[(return footnote_end, date_width)]
I_general[existing span heuristics]
A_determine --> B_try_date
B_try_date -->|"Some(date_end)"| C_date_found
B_try_date -->|"None"| D_no_date
C_date_found --> E_width
E_width --> F_footnote
F_footnote -->|"Some(footnote_end)"| H_return_footnote
F_footnote -->|"None"| G_return_date
D_no_date --> I_general
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65fb3b8b18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai please propose a property test design for this functionality and provide an AI coding agent prompt for this. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@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. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check the current state of the relevant files on the PR branch
cat tests/wrap_unit/dates.rs
echo "---FILE_SEPARATOR---"
cat src/wrap/inline/span_helpers.rs
echo "---FILE_SEPARATOR---"
cat src/wrap/inline/predicates.rsLength of output: 21797
🧠 Learnings used |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 465ee74048
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 465ee74048
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@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 (2 errors, 6 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/adrs/0003-date-sequences-as-inline-fragments.md`:
- Around line 18-35: Rewrite the paragraph to remove first-person phrasing
(e.g., replace "we decided for matching" with an impersonal construction like
"The implementation matches" or "The matcher recognises"), normalize to
en-GB-oxendict -ize spellings (use "recognizes" → "recognises", "parenthesized"
→ "parenthesised" if following en‑GB-oxendict) and keep references to the
implementation (mention src/wrap/inline/span_helpers.rs and the InlineFragment
grouping model) so the text reads objectively and follows repository docstyle
rules.
In `@src/wrap/inline/date_strategies.rs`:
- Around line 3-5: Update the module doc comment in date_strategies.rs to use
en-GB-oxendict `-ize` spelling by replacing "recognises" with "recognizes" in
the top comment block; locate the module-level doc string at the start of the
file (the triple-slash //! comment) and make the single-word spelling change so
documentation follows the project's `-ize` guideline.
In `@src/wrap/inline/month_names.rs`:
- Around line 8-11: Doc comment in month_names.rs uses British spelling
"recognised"; update the Rust doc comment to the repository's en-GB-oxendict
`-ize` form by replacing "recognised" with "recognized" in the paragraph that
describes Full and abbreviated English month names (the doc block above the
month names and the comment referencing `is_month_name`).
🪄 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: aa59ccdb-b3bc-4a6c-b1a3-747da344b0f8
⛔ Files ignored due to path filters (8)
tests/snapshots/date_month_day_year_punctuation_wrap.snapis excluded by!**/*.snaptests/snapshots/date_ordinal_day_month_year_wrap.snapis excluded by!**/*.snaptests/snapshots/date_parenthesised_and_footnote_wrap.snapis excluded by!**/*.snaptests/snapshots/date_wrap_month_day_year_with_comma.snapis excluded by!**/*.snaptests/snapshots/date_wrap_numeric_day_month_year.snapis excluded by!**/*.snaptests/snapshots/date_wrap_ordinal_day_month_year.snapis excluded by!**/*.snaptests/snapshots/date_wrap_over_width_fallback.snapis excluded by!**/*.snaptests/snapshots/date_wrap_parenthesised_with_footnote.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
CHANGELOG.mdREADME.mddocs/adrs/0003-date-sequences-as-inline-fragments.mddocs/developers-guide.mdsrc/wrap/inline.rssrc/wrap/inline/date_strategies.rssrc/wrap/inline/month_names.rssrc/wrap/inline/predicates.rssrc/wrap/inline/span_helpers.rstests/wrap/date_snapshots.rstests/wrap/mod.rstests/wrap_unit/date_strategies.rstests/wrap_unit/dates.rstests/wrap_unit/dates_prop.rs
Remove first-person phrasing from the date atomicity ADR and align the reviewed doc comments with the requested spelling forms.
Document the instrumented date span helpers and add traced tests that verify both `date_token_span` and `try_match_date_sequence` emit TRACE events when date grouping is exercised.
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/adrs/0003-date-sequences-as-inline-fragments.md (1)
24-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize Oxford spelling to
-izeforms in ADR prose.Replace
recogniseswithrecognizes, and replaceparenthesisedwith
parenthesizedto align with repository en-GB-oxendict conventions.Triage:
[type:spelling][type:docstyle]As per coding guidelines, documentation must use en-GB-oxendict with
-ize
forms outside API literals.🤖 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 `@docs/adrs/0003-date-sequences-as-inline-fragments.md` around lines 24 - 34, The ADR prose uses en-GB spellings "recognises" and "parenthesised"; update the text in docs/adrs/0003-date-sequences-as-inline-fragments.md by replacing "recognises" with "recognizes" and "parenthesised" with "parenthesized" so the document follows the repository en-GB-oxendict `-ize` convention (search for the sentence beginning "The matcher recognises..." to locate the tokens to change).Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@docs/adrs/0003-date-sequences-as-inline-fragments.md`:
- Around line 24-34: The ADR prose uses en-GB spellings "recognises" and
"parenthesised"; update the text in
docs/adrs/0003-date-sequences-as-inline-fragments.md by replacing "recognises"
with "recognizes" and "parenthesised" with "parenthesized" so the document
follows the repository en-GB-oxendict `-ize` convention (search for the sentence
beginning "The matcher recognises..." to locate the tokens to change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 02caa539-3ca6-4180-90d7-37977a6dee71
📒 Files selected for processing (7)
docs/adrs/0003-date-sequences-as-inline-fragments.mddocs/developers-guide.mdsrc/wrap/inline.rssrc/wrap/inline/date_strategies.rssrc/wrap/inline/inline_tracing_tests.rssrc/wrap/inline/month_names.rssrc/wrap/inline/span_helpers.rs
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Add the missing `make typecheck` guidance and normalize the remaining Oxford spelling violations in the date-sequence ADR and span helper module documentation.
Emit DEBUG events for matched date sequence patterns and INFO events when an over-width date span falls back to long-token wrapping. Expand the date snapshot module documentation and keep the date span tracing tests colocated with the span helper implementation.
|
@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, 1 warning)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== tests/wrap/date_snapshots.rs top ==="
fd -e rs date_snapshots --exec sh -c 'echo "--- {} ---"; sed -n "1,80p" "{}"'
echo
echo "=== try_match_date_sequence and logging in span_helpers.rs ==="
fd -e rs span_helpers --exec sh -c 'echo "--- {} ---"; rg -n -C 4 "try_match_date_sequence|debug!|info!|tracing::debug|tracing::info|matched pattern|date.*pattern" "{}"'
echo
echo "=== date_token_span / fallback logging in inline.rs ==="
sed -n '1,140p' src/wrap/inline.rs
echo
rg -n -C 4 "date_token_span|try_match_date_sequence|debug!|info!|tracing::debug|tracing::info|fallback|width|date_width" src/wrap/inline.rs src/wrap/inline/*.rs
echo
echo "=== tracing imports ==="
rg -n "use tracing|tracing::|debug!|info!" src/wrap/inline.rs src/wrap/inline/span_helpers.rs
echo
echo "=== relevant tracing tests ==="
rg -n -C 3 "try_match_date_sequence|date_token_span|debug|info|logs_contain|traced_test" src tests | head -120Length of output: 77672
No further AI coding agent prompt is required. 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Makefile typecheck target ==="
rg -n "^(typecheck:|\.PHONY:|.*cargo check --all-targets --all-features|.*check --all-targets --all-features)" Makefile || true
echo
echo "=== developers-guide typecheck documentation ==="
rg -n -C 3 "\btypecheck\b|make typecheck|cargo check --all-targets --all-features|make (fmt|lint|test|check-fmt)" docs/developers-guide.md || true
echo
echo "=== spelling checks in named files ==="
for f in src/wrap/inline/month_names.rs src/wrap/inline/date_strategies.rs docs/adrs/0003-date-sequences-as-inline-fragments.md src/wrap/inline/span_helpers.rs; do
echo "--- $f ---"
rg -n "recognised|recognises|parenthesised|recognized|recognizes|parenthesized" "$f" || true
done
echo
echo "=== ADR line 20 context ==="
nl -ba docs/adrs/0003-date-sequences-as-inline-fragments.md | sed -n '16,24p'
echo
echo "=== tracing instrumentation declarations ==="
rg -n -C 2 "#\[tracing::instrument\(level\s*=\s*\"trace\",\s*ret\)\]|fn date_token_span|fn try_match_date_sequence" src/wrap/inline.rs src/wrap/inline/span_helpers.rs || true
echo
echo "=== developers-guide instrumented functions section ==="
rg -n -A 18 -B 4 "Instrumented functions|date_token_span|try_match_date_sequence|is_month_name|is_ordinal_day|is_numeric_day|is_year" docs/developers-guide.md || true
echo
echo "=== traced_test coverage for date_token_span / try_match_date_sequence ==="
rg -n -C 3 "traced_test|logs_contain|date_token_span|try_match_date_sequence" src tests || true
echo
echo "=== likely tracing test files ==="
fd -e rs 'tracing|span_helpers|inline' src tests | sortLength of output: 3355 🏁 Script executed: #!/bin/bash
set -u
echo "=== ADR line 20 context ==="
awk 'NR>=16 && NR<=24 { printf "%d:%s\n", NR, $0 }' docs/adrs/0003-date-sequences-as-inline-fragments.md || true
echo
echo "=== tracing instrumentation declarations ==="
rg -n -C 2 '#\[tracing::instrument\(level\s*=\s*"trace",\s*ret\)\]|fn date_token_span|fn try_match_date_sequence' src/wrap/inline.rs src/wrap/inline/span_helpers.rs || true
echo
echo "=== developers-guide instrumented functions section ==="
rg -n -A 22 -B 4 'Instrumented functions|date_token_span|try_match_date_sequence|is_month_name|is_ordinal_day|is_numeric_day|is_year' docs/developers-guide.md || true
echo
echo "=== traced_test coverage for date_token_span / try_match_date_sequence ==="
rg -n -C 4 'traced_test|logs_contain|date_token_span|try_match_date_sequence' src tests || true
echo
echo "=== likely tracing test files ==="
fd -e rs 'tracing|span_helpers|inline' src tests | sort || trueLength of output: 34205
No further work is required, so no AI agent prompt is needed. 🧠 Learnings used |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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/developers-guide.md`:
- Around line 543-544: The table row for date_token_span contradicts the logging
policy ("Never emit at info! or above from library code"); update the docs to
remove the "info" level from the `date_token_span` entry so it reads e.g.
"trace" (or otherwise match the normative rule), keeping
`try_match_date_sequence` unchanged; ensure the table note or description still
documents the fallback behavior for `date_token_span` but does not advertise
emitting at info, or if you choose to keep an exception instead, update the
policy section text to justify and document why `date_token_span` is allowed to
log at info and reference the `date_token_span` symbol explicitly.
In `@src/wrap/inline.rs`:
- Around line 217-233: build_fragments currently accepts a wrap_width and emits
tracing::info! when a date span exceeds it; remove the info! and the wrap_width
parameter so the helper is behaviour-only: change fn build_fragments(tokens:
&[String], wrap_width: usize) -> Vec<InlineFragment> to fn
build_fragments(tokens: &[String]) -> Vec<InlineFragment>, drop the
tracing::info! block, update the determine_token_span usage to ignore
group_width (e.g., let (group_end, _group_width) = determine_token_span(tokens,
i)), and update all call sites (e.g., where build_fragments(&tokens, width) is
invoked) to call build_fragments(&tokens) instead; if diagnostics are needed
keep them at debug!/trace! in try_match_date_sequence or its matcher path.
🪄 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: bfd6c224-0265-432e-b24b-6e942af401fe
📒 Files selected for processing (7)
docs/adrs/0003-date-sequences-as-inline-fragments.mddocs/developers-guide.mdsrc/wrap/inline.rssrc/wrap/inline/date_strategies.rssrc/wrap/inline/month_names.rssrc/wrap/inline/span_helpers.rstests/wrap/date_snapshots.rs
Remove the INFO-level over-width date fallback event from library code and restore `build_fragments` to behaviour-only grouping. Keep pattern match diagnostics on `try_match_date_sequence` at DEBUG and document `date_token_span` as TRACE-only.
|
@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 (2 errors, 6 warnings)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== src/wrap/inline.rs: build_fragments, calls, date matching, logging ==="
rg -n -C 6 'fn build_fragments|build_fragments\(|wrap_width|tracing::info!|info!\(|tracing::debug!|debug!\(|try_match_date_sequence|date_token_span' src/wrap/inline.rs || true
echo
echo "=== all info! usage under src/wrap ==="
rg -n 'tracing::info!|info!\(' src/wrap src/fences.rs src/process.rs src/lib.rs 2>/dev/null || true
echo
echo "=== try_match_date_sequence call sites ==="
rg -n -C 3 '\btry_match_date_sequence\s*\(' src tests || true
echo
echo "=== date_token_span docs/instrumented-functions table ==="
rg -n -A 18 -B 6 'Instrumented functions|date_token_span|try_match_date_sequence|is_month_name|is_ordinal_day|is_numeric_day|is_year|info|debug|trace' docs/developers-guide.md || true
echo
echo "=== function signatures around build_fragments and wrap_preserving_code ==="
sed -n '190,245p' src/wrap/inline.rs
sed -n '320,355p' src/wrap/inline.rsLength of output: 26260
No further AI coding agent prompt is required. 🧠 Learnings used |
Summary
This branch treats common English date-like prose sequences as atomic inline fragments before
wrap_first_fitreceives them, somdtablefix --wrapavoids splitting dates such as25th December 2025andJuly 4, 2008across lines when they fit.Closes #346.
Review walkthrough
Validation
cargo test --test wrap_unit dates: passedmake check-fmt: passedmake lint: passedmake test: passedcoderabbit review --agent: final pass completed with 0 findingsSummary by Sourcery
Treat common English date-like sequences as atomic inline fragments during wrapping to prevent them being split across lines.
Enhancements:
Tests: