Skip to content

Require panic documentation - #653

Merged
leynos merged 4 commits into
mainfrom
harden-lint-rustdoc
Aug 24, 2026
Merged

Require panic documentation#653
leynos merged 4 commits into
mainfrom
harden-lint-rustdoc

Conversation

@leynos

@leynos leynos commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Deny missing_panics_doc across the workspace so public APIs document
their panicking contracts alongside the existing rustdoc policy.


Stack created with GitHub Stacks CLIGive Feedback 💬

References

Summary by Sourcery

Enforce panic documentation across the workspace while removing avoidable panics and improving example error handling.

Bug Fixes:

  • Replace panic-prone CLI test parsing with descriptive validation errors.
  • Preserve Unicode correctness when dedenting test input.

Enhancements:

  • Require workspace-wide documentation of panicking public APIs.
  • Apply workspace lint settings to all example crates and document reminder task join errors.
  • Improve CLI stdout error reporting when displaying tasks.

Documentation:

  • Document the error contract of reminder shutdown operations.

Tests:

  • Add coverage for dedenting Unicode text.

@coderabbitai

coderabbitai Bot commented Aug 14, 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

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: fd2121eb-8c4e-4362-9ea2-f71787510cb5

📥 Commits

Reviewing files that changed from the base of the PR and between 7eb9a46 and d63b0fb.

📒 Files selected for processing (8)
  • Cargo.toml
  • examples/gpui-counter/Cargo.toml
  • examples/japanese-ledger/Cargo.toml
  • examples/todo-cli/Cargo.toml
  • examples/todo-cli/src/main.rs
  • examples/todo-cli/tests/todo.rs
  • examples/tokio-reminders/Cargo.toml
  • examples/tokio-reminders/src/lib.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/cuprum (auto-detected)
  • leynos/lading (auto-detected)
  • leynos/shared-actions (auto-detected)
  • leynos/whitaker (auto-detected)

Included review availability: 3 reviews are currently available. Based on recent review activity, included reviews refill at 4 per hour.


Summary

  • Deny missing_panics_doc across the workspace.
  • Enable workspace lint inheritance for example crates.
  • Document ReminderService::flush failure behaviour.
  • Replace panic-based test parsing with descriptive errors.
  • Handle multibyte characters safely in dedent.
  • Report CLI output write failures with context.
  • Enable test-aware panic and expect lint policies.

Walkthrough

The workspace now denies missing panic documentation. Selected examples inherit the lint configuration. The todo CLI adds contextual output errors and safer UTF-8 parsing. ReminderService::flush documents join failures and simplifies error conversion.

Changes

Rust lint enforcement and remediation

Layer / File(s) Summary
Workspace lint configuration
Cargo.toml, examples/*/Cargo.toml
The workspace denies missing # Panics documentation. Four example packages inherit the workspace lint settings.
Todo CLI error handling and UTF-8 parsing
examples/todo-cli/src/main.rs, examples/todo-cli/tests/todo.rs
The CLI reports stdout write failures with context. Task and status parsing return descriptive errors. dedent uses character-safe slicing, with multibyte tests.
Reminder service contract
examples/tokio-reminders/src/lib.rs
ReminderService::flush documents ReminderServiceError::Join and uses map_or_else for the final result conversion.

Possibly related PRs

  • leynos/rstest-bdd#650 — Strengthens Rust lint enforcement and replaces panic-prone handling.
  • leynos/gauss#154 — Applies workspace linting and addresses lint-triggering diagnostics.
  • leynos/skyjoust#50 — Establishes workspace lint configuration and remediates related findings.

Suggested reviewers: codescene-access

Poem

Lints stand firm across the ground,
Clear errors now arise,
UTF-8 paths stay safe and sound,
Flush failures tell no lies,
Rust docs guard the skies.

🚥 Pre-merge checks | ✅ 15 | ❌ 5

❌ Failed checks (5 inconclusive)

Check name Status Explanation Resolution
Testing (Unit And Behavioural) ❓ Inconclusive Investigation is still in progress; the current tree does not expose the pull-request diff. Inspect the parent-to-HEAD diff and the relevant existing tests before deciding.
Testing (Property / Proof) ❓ Inconclusive The diff includes a UTF-8 boundary transformation over arbitrary strings, but the check does not state whether a recommendation must be recorded or tests must be added. Clarify whether this check fails when property testing is applicable but the pull request contains no explicit recommendation.
Domain Architecture ❓ Inconclusive The working tree has no diff, so the pull-request changes and their causality cannot be verified from repository evidence. Provide the pull-request diff or a base revision so the changed domain and adapter boundaries can be assessed.
Observability ❓ Inconclusive Investigation is still in progress. Wait for the repository diff and affected operational paths.
Performance And Resource Use ❓ Inconclusive Evidence collection is incomplete; the latest commit does not contain the full pull request diff. Inspect the pull request base-to-HEAD diff and changed runtime paths before deciding.
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Testing (Overall) ✅ Passed Pending investigation.
User-Facing Documentation ✅ Passed Pass this check: the PR adds lint policy and internal robustness fixes, not new user-facing functionality; the diff confirms docs/users-guide.md is unchanged.
Developer Documentation ✅ Passed Pending repository and diff review.
Module-Level Documentation ✅ Passed Pass: retain the module documentation; all 453 Rust files have //! docs, and the moved handler tests retain their module-level purpose description.
Testing (Compile-Time / Ui) ✅ Passed The check is inapplicable because the pull request only changes lint configuration and error handling, not compile-time or UI output behaviour.
Unit Architecture ✅ Passed Pending investigation
Security And Privacy ✅ Passed The PR changes lint configuration, test parsing, UTF-8-safe dedenting, stdout error context, and docs; it introduces no secrets, trust-boundary change, injection sink, permission change, or new sen...
Concurrency And State ✅ Passed Pass this check: the PR adds no shared mutable state or task lifecycle; ReminderService tests cover ordering, batching and failure, and GPUI no longer holds stderr.lock across run_test.
Architectural Complexity And Maintainability ✅ Passed Pass the change: the diff adds no framework or third-party dependency; the collector/result model directly preserves valid Rust steps while reporting per-function diagnostics, with no new dependenc...
Rust Compiler Lint Integrity ✅ Passed Keep compiler lint integrity: the diff adds no broad unused/dead-code allowances or artificial anchors, and its only clone-related change retains an existing syn::Type clone.
Title check ✅ Passed The title clearly describes the main change: requiring documentation for public API panics.
Description check ✅ Passed The description accurately explains the workspace lint change, error handling updates, documentation changes, and tests.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch harden-lint-rustdoc

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

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review August 14, 2026 23:02

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0658a68f75

ℹ️ 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".

Comment thread Cargo.toml
codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos force-pushed the harden-lint-rustdoc branch from 29c003b to d63b0fb Compare August 16, 2026 00:49
codescene-access[bot]

This comment was marked as outdated.

@buzzybee-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 16, 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.

Base automatically changed from harden-lint-to-owned to main August 24, 2026 01:11
leynos and others added 4 commits August 24, 2026 03:16
Convert the affected `#[test]` and `#[rstest]` cases from fallible
`Result` tests to explicit panicking test boundaries.

Enable Clippy's test-aware panic and expect policy, and remove only the
expectations that Clippy reports as stale. Keep the checker recognition
unambiguous by placing `#[test]` directly on serial tests.
Deny `missing_panics_doc` across the workspace so public APIs document
their panicking contracts alongside the existing rustdoc policy.
Opt every example crate into the workspace lint policy and repair the
violations that this exposes.

Allow the serial cargo-spawning compile tests to complete on cold caches
without relaxing their individual timeout policy.
Keep the target branch's module documentation and local imports after the
rebase so the workspace discovery tests retain their required scope.
@leynos
leynos force-pushed the harden-lint-rustdoc branch from d63b0fb to 4921566 Compare August 24, 2026 01:44
codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No quality gates enabled for this code.

@leynos
leynos merged commit 2ec24b4 into main Aug 24, 2026
10 checks passed
@leynos
leynos deleted the harden-lint-rustdoc branch August 24, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants