Skip to content

Conversation

@rmccorm4
Copy link
Contributor

@rmccorm4 rmccorm4 commented Oct 10, 2025

Overview:

This doctest has been failing intermittently, causing rust test failures on various PRs, for example: https://github.com/ai-dynamo/dynamo/actions/runs/18419132132/job/52489609655?pr=3539

It is relatively rare, maybe 1 out of 10-20 runs that it will fail, but it's enough to make the CI test unreliable. This should be revisited and fixed properly to determine if there is an actual bug in the underlying implementation or just an issue with the doctest example.

Details:

For future reference: I had an attempt to fix here, but I'm not confident in its correctness: #3414

I'm opting to remove the flakiness for now to stabilize CI, and have this revisited independently.

Summary by CodeRabbit

  • Documentation
    • Updated inline documentation by removing an unstable example to prevent intermittent failures in automated checks. No impact on behavior or APIs.
  • Chores
    • Maintenance to improve reliability of documentation-related validation during builds and tests. No functional changes for end-users.

@rmccorm4 rmccorm4 requested a review from a team as a code owner October 10, 2025 22:37
@github-actions github-actions bot added the fix label Oct 10, 2025
@rmccorm4 rmccorm4 enabled auto-merge (squash) October 10, 2025 22:39
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

Walkthrough

Replaced an inline doctest in TaskHandle::cancellation_token() with a FIXME comment citing intermittent doctest failures. No code logic, signature, or control flow was modified.

Changes

Cohort / File(s) Summary
Docs/Doctest adjustment
lib/runtime/src/utils/tasks/tracker.rs
Removed doctest example from TaskHandle::cancellation_token() and added a FIXME comment about flaky doctests; no functional changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

I thump my foot at flaky tests—oh dear!
A doctest hopped away, we’ll fix it here.
With FIXME carrots planted neat,
The code still hums, serene and sweet.
Until the tests return to play—hip hop, hooray! 🥕🐇

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ⚠️ Warning The PR description includes Overview and Details sections as per the template but omits the 'Where should the reviewer start?' and 'Related Issues' sections that are required by the repository’s description template. Without guidance on which files to review and the linkage to an existing issue via an action keyword, reviewers may be uncertain where to focus and how this PR relates to tracked issues. This omission makes the description incomplete and fails to meet the template’s structure. Please add a 'Where should the reviewer start?' section that specifies the key files or areas for review and add a 'Related Issues' section using an action keyword (e.g., closes #xxx) to link this PR to the relevant issue. This will ensure reviewers have guidance on where to focus and that the PR is properly associated with issue tracking.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly states the core change of removing a flaky doctest and aligns directly with the implemented modifications, making it clear and specific.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@rmccorm4 rmccorm4 merged commit 2ba8bdb into main Oct 10, 2025
30 checks passed
@rmccorm4 rmccorm4 deleted the rmccormick/fix_flaky_doctest branch October 10, 2025 23:48
@rmccorm4
Copy link
Contributor Author

CC @ryanolson for viz since you wrote these doctests - just trying to stabilize CI for now, couldn't come up with a confident fix atm.

ziqifan617 pushed a commit that referenced this pull request Oct 20, 2025
nv-tusharma pushed a commit that referenced this pull request Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants