Skip to content

refactor(report): drop dead include_changed_files knob, fix config_rule stored_range docs - #78

Open
cargo-affected-bot wants to merge 2 commits into
mainfrom
docs/report-config-rule-stored-range
Open

refactor(report): drop dead include_changed_files knob, fix config_rule stored_range docs#78
cargo-affected-bot wants to merge 2 commits into
mainfrom
docs/report-config-rule-stored-range

Conversation

@cargo-affected-bot

@cargo-affected-bot cargo-affected-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Two small accuracy fixes in the report layer, found by the nightly survey of docs/report-json.md (today's rotation bucket) read against src/report.rs.

config_rule reasons also have a null stored_range. The schema doc already says so — docs/report-json.md lists stored_range as "null for structural_backstop and config_rule" — but both Rust doc comments named only structural_backstop. selection.rs builds ConfigRule reasons with stored_range: None (a config rule matches an input path, not a coverage row), so the markdown was right and the doc comments on HitReason::stored_range and ReasonEntry::stored_range were incomplete. Corrected both to match.

SelectionInputs::include_changed_files has never been anything but true. It was introduced in #16 with two call sites (run and status), both hardcoded true; #71 folded those into the single plan.rs call site, still true. git log -S over the field's whole history shows no false anywhere. Its doc claimed "false collapses to selection.changed_files = None (no diff anchor was usable)" — but a run with no usable diff anchor is miss-no-reachable-sha, which routes to build_full_suite instead and never constructs SelectionInputs at all. So the branch described a state the type can't reach. Removed the field and the dead arm, leaving a comment on why the Some is unconditional. No behavior change.

Test. Neither change is a bug fix, so there's no failing-before regression test to add. What was missing is a pin on the encoding the corrected docs describe: selection.rs fills collect_sha, matched_hunk and stored_range for a ConfigRule reason by hand, and nothing asserted the resulting JSON. config_rule_reason_carries_no_coverage_anchor covers that — kind: "config_rule", stored_range: null, collect_sha: "", matched_hunk: [0, 0] — so a future edit that starts populating a range has to update the v1 schema doc deliberately rather than silently.

cargo clippy --all-targets, cargo fmt --check and the 113 unit tests pass locally. The functional suite runs in CI.

@cargo-affected-bot cargo-affected-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Both doc corrections check out against the code: selection.rs builds ConfigRule reasons with stored_range: None / collect_sha: String::new() / matched_hunk: (0, 0), and docs/report-json.md already documented the config_rule case, so the Rust comments were the stale side. The include_changed_files removal is safe — SelectionInputs has exactly one construction site (plan.rs::write_selection_report), which is only reached from the selection path in run.rs/status.rs; the no-anchor case goes through build_full_suite, which hardcodes changed_files: None.

One nit inline. cargo clippy --all-targets is clean and the 113 unit tests pass here (the description says 114 — that's the count including the test result: summary line).

Comment thread src/report.rs Outdated
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