fix(git): classify declaration-language comments so prose-only can see them - #856
Conversation
|
Warning Review limit reachedNext included review available in 53 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
Merge Risk: ⚪ Minimal · up to The change is merge-ready with no concrete current-head risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
260f037 to
34a7d44
Compare
…e them `without_comments` knew four extensions — `.md`, `.rs`, the shell family, and extensionless `mise-tasks/` programs. Those are the languages a consumer writes PROGRAMS in, which is the set CLOUD-827's shell predecessor classified. A declarative consumer keeps most of its prose somewhere else: policy modules, config, pipeline documents, workflows. Every one of those took the unrecognised-extension fall-through, whose remainder is the whole file, so a comment-only change to any of them read as a code change and `prose-only` could not fire over it. Measured 2026-09-04, on this branch's own first commit: a 121-to-71 comment trim of one policy module, nothing else in the diff, `batten check --rule prose-only` exit 0, and a full required matrix spent on a change CI does not judge. That is precisely the instance CLOUD-827 exists to price. `.rego`, `.toml`, `.yml`/`.yaml` are `#`; `.pkl` is `//` — Java-family, where `#` is not a comment marker at all, so one table for everything `#`-ish would have kept real code in the remainder and dropped commented lines from it. The fall-through's direction is unchanged and still right for a genuinely unknown extension: it admits the branch rather than blocking it, and only blocking is unrecoverable by waiting. What was wrong was calling these unknown. Both tiers, because the load-time one cannot see this class: * `git::tests` pins the table, one assertion per extension, so one arm's presence cannot stand in for the rest; * `crates/batten/tests/it/prose_only.rs` drives the compiled engine over a real `origin/main` for all four, which is the tier that proves the remainder comparison the ENGINE builds now reaches them. Every existing case in that file is a `.rs`, `.md`, `.bats` or extensionless program — the same blind spot one layer up, and the reason the gap survived. Refs: CLOUD-827
`clippy::match_same_arms` is `-D warnings` here, so `Some("pkl") => "//"`
beside `Some("rs") => "//"` does not compile, and neither does the
declaration-language arm beside the shell family's. Two arms now, one per
marker.
The grouping a reader wants is not the grouping the lint permits, so it
moves into the comments: `.pkl` sits with Rust because it is Java-family
and `#` is not a comment marker there at all, which is the one thing about
this table worth getting wrong slowly.
Both test doc comments said "removing an arm" where the removable unit is
now a token, and the per-extension cases matter more for it: seven
extensions share two arms, so a case per extension is the only
granularity the table itself no longer provides.
Refs: CLOUD-827
b66ab96 to
1105c21
Compare
|
❌ The last analysis has failed. |
|
/fast-forward |
prose-onlycould not fire over.rego,.toml,.yml/.yamlor.pkl.without_commentsknew four extensions —.md,.rs, the shell family, andextensionless
mise-tasks/programs — the languages a consumer writes PROGRAMSin, which is the set CLOUD-827's shell predecessor classified. This repository
keeps most of its prose somewhere else: ~40 policy modules, the config
authority, the pipeline document, the workflows. All of them took the
unrecognised-extension fall-through, whose remainder is the whole file, so a
comment-only change to any of them read as a code change.
Measured on this branch's own first commit: a 121→71-comment trim of one policy
module, nothing else in the diff,
batten check --rule prose-onlyexit 0, afull required matrix spent on a change CI does not judge. That is precisely the
instance CLOUD-827 exists to price, and it walked past because the classifier
had never been told what a
.regocomment looks like..rego,.toml,.yml/.yamlare#;.pklis//— Java-family, where#is not a comment marker at all. The fall-through's direction is unchangedand still right for a genuinely unknown extension: it admits rather than
blocks, and only blocking is unrecoverable by waiting.
Both tiers, because the load-time one cannot see this class:
git::testspinsthe table one assertion per extension, and
crates/batten/tests/it/prose_only.rsdrives the compiled engine over a real
origin/mainfor all four. Everyexisting case in that suite is a
.rs,.md,.batsor extensionless program— the same blind spot one layer up, and the reason the gap survived.
Also here, from the first commit:
policy/release-provision-parity.regotrimmed235→185 lines, 121→71 comment, because most of the comment was a second copy of
CLOUD-1431. Predicate,
test_rules and verdicts untouched.DO-NOT-CLOSE CLOUD-1431
CLOUD-1431 was closed by #851, which landed that gate. Nothing here completes
it. CLOUD-827's gate is what this repairs, and CLOUD-827 is Done — the row's
mechanism shipped; this is a class of input it could not read.
https://claude.ai/code/session_01Gyg9qwjPd1PeJRS9b3LqxN