Skip to content

dogfood: ignore comments for owner inference#177

Merged
EffortlessSteven merged 1 commit into
mainfrom
dogfood/comment-owner-inference-v1
May 18, 2026
Merged

dogfood: ignore comments for owner inference#177
EffortlessSteven merged 1 commit into
mainfrom
dogfood/comment-owner-inference-v1

Conversation

@EffortlessSteven
Copy link
Copy Markdown
Member

Summary

  • ignore comment lines while scanning backward for card owner inference
  • prevent prose like Drop impl would drop from becoming owner would
  • add scanner regression coverage
  • record
    ust-smallvec#407 PR-diff dogfood before/after in the dogfood handoff/status docs

Validation

tk cargo fmt --check

tk cargo test -p unsafe-review-core scanner --locked

tk cargo test -p unsafe-review-core fixture_card_goldens_match_rendered_json --locked

tk cargo run --locked -p xtask -- check-pr

tk git diff --check

tk cargo check --workspace --all-targets --locked

tk cargo clippy --workspace --all-targets --locked -- -D warnings

  • CARGO_TARGET_DIR=target/validation-test-target rtk cargo test --workspace --locked (172 passed)

Dogfood

Self-review

  • One semantic objective: fix comment-derived owner inference found by real PR-diff dogfood.
  • No soundness, UB-free, Miri-clean, site-execution, calibrated-rate, or support-tier promotion claim.
  • No witness execution, automatic comments, default blocking, baseline, badge, or policy authority added.
  • ReviewCard remains the source of truth.

@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

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

Run ID: 8ac6460b-c797-4dcd-98f5-f0ddd99b4f14

📥 Commits

Reviewing files that changed from the base of the PR and between 521db55 and 0c72e97.

📒 Files selected for processing (4)
  • crates/unsafe-review-core/src/analysis/scanner.rs
  • docs/handoffs/2026-05-18-real-crate-dogfood-v0.6.md
  • docs/status/OBJECTIVE_AUDIT.md
  • docs/status/SUPPORT_TIERS.md

Summary by CodeRabbit

  • Bug Fixes

    • Resolved an issue where comments inside unsafe blocks could incorrectly interfere with owner inference, improving code analysis accuracy.
  • Documentation

    • Updated testing documentation and support tier information to reflect recent improvements and expanded test coverage.

Walkthrough

This PR fixes owner inference by skipping comment lines during backward scanning, preventing comment prose from being misinterpreted as function/trait/impl declarations. The fix is validated against real-world servo/rust-smallvec#407 dogfood results, expanding measured support tiers from one to two PR-diff samples.

Changes

Owner Inference Robustness

Layer / File(s) Summary
Comment-aware backward scanning in owner inference
crates/unsafe-review-core/src/analysis/scanner.rs
New is_comment_line helper classifies trimmed lines as comments. find_owner and find_owner_declaration_index now skip comment lines during backward scanning to avoid parsing comment text as declarations. Unit test verifies correct owner detection when comments appear in unsafe blocks.
Dogfood measurement and support tier updates
docs/handoffs/2026-05-18-real-crate-dogfood-v0.6.md, docs/status/OBJECTIVE_AUDIT.md, docs/status/SUPPORT_TIERS.md
Handoff doc adds rust-smallvec#407 PR-diff analysis with before/after owner-inference behavior and proof commands. Support tier claims expand from one to two measured PR diffs. Objective audit records comment-derived owner false positives as a captured fix. Scope of documented fixes now includes import/declaration, cfg targets, capped-scan behavior, safety-contract inheritance, and comment-ignoring during owner inference.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A scanner scans, and comments it found,
Now gracefully skips them, with nary a sound,
No fn in a quip, no impl in prose—
Real owners emerge, as everyone knows!
🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: ignoring comments during backward scanning for owner inference, which aligns with the core objective and file changes.
Description check ✅ Passed The description is clearly related to the changeset, covering the summary of changes, validation steps, dogfood results, and self-review of the PR.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 dogfood/comment-owner-inference-v1

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.

@EffortlessSteven EffortlessSteven merged commit 7043ded into main May 18, 2026
4 checks passed
@EffortlessSteven EffortlessSteven deleted the dogfood/comment-owner-inference-v1 branch May 18, 2026 10:31
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.

1 participant