Skip to content

DELIVERY: two gates from Verdict 3.0, and a sharper mutation check - #14

Open
arnelirobles wants to merge 2 commits into
mainfrom
delivery-verdict-3
Open

DELIVERY: two gates from Verdict 3.0, and a sharper mutation check#14
arnelirobles wants to merge 2 commits into
mainfrom
delivery-verdict-3

Conversation

@arnelirobles

@arnelirobles arnelirobles commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Three additions, all cited to something that got through during Verdict 3.0.

A file on your disk is not a file in the repository. .gitignore had an
unanchored packages/ rule, meant for a NuGet folder, which also matched
docs/packages/. Eight guides were written, the docs index linked all eight, and
every test passed locally because the files were on the laptop. git add -A
skipped them silently. The link test failed on the first CI run, on a clean
clone, which is the only place that difference is visible.

Assert the middle, not just the edges. A pooled buffer was returned with
clearArray: true one line before it was copied out of, so every item collected
before the first doubling came back blank: 16 of 17, 512 of 2000. Two tests
covered that path and both were green, because one asserted the count and the
final element, which is written after the loss, and the other used a collection
type that took a different branch.

The mutation check now says to check the mutation. Proving an allocation gate
could fail meant making the code allocate, and boxing a bool was the obvious
way. The gate stayed green and briefly looked broken. The JIT had elided the box,
so nothing allocated and the gate was correct. GC.KeepAlive(new object()) turned
it red at once.

The first two are new sections. The third is a paragraph on the existing mutation
check, since it is the same trap one level up rather than a separate gate.

Summary by CodeRabbit

  • Documentation
    • Expanded guidance on mutation checks, including compiler-optimized-away mutations.
    • Added recommendations to validate against a fresh repository checkout rather than the working tree.
    • Documented the importance of testing intermediate state, including pooled buffer handling and clearing.

A file on your disk is not a file in the repository. An unanchored `packages/`
rule in .gitignore also matched docs/packages/, so eight written and linked guides
were skipped by git add -A without a word. Every local test passed because the
files were there. CI caught it on a clean clone.

Assert the middle, not just the edges. A pooled buffer was cleared one line before
it was copied out of, so 16 of 17 items came back blank. One test checked the
count and the final element, which is written after the loss; the other took a
different branch. Both green.

The mutation check now says to check the mutation too. Boxing a bool to prove an
allocation gate could fail did not allocate, because the JIT elided it, and the
gate looked broken when it was right.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 54 minutes.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 716fea1f-d9d4-4601-b4dc-c68ddb36efdb

📥 Commits

Reviewing files that changed from the base of the PR and between 6715e49 and b07fe59.

📒 Files selected for processing (1)
  • DELIVERY.md

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5512b1de-d300-42f7-b004-b58c17ebe021

📥 Commits

Reviewing files that changed from the base of the PR and between 9a6cdb0 and 6715e49.

📒 Files selected for processing (1)
  • DELIVERY.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

DELIVERY.md adds three validation lessons covering compiler-elided mutations, fresh-checkout checks, and assertions within pooled-buffer contents.

Changes

Delivery guidance

Layer / File(s) Summary
Validation lesson updates
DELIVERY.md
The mutation-check guidance covers compiler-elided mutations. A new subsection requires checks against a fresh checkout. Another subsection documents checking intermediate pooled-buffer contents, not only count and final elements.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 6715e

This documentation-only change adds guidance for repository cleanliness, middle-state assertions, and reliable mutation checks; it introduces no production behavior or merge-blocking risk beyond normal review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the documentation changes: it identifies two new gates from Verdict 3.0 and the improved mutation check.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch delivery-verdict-3

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.

A CI step printed dotnet --info as evidence that a matrix leg ran on .NET 10. Its
sed pattern was case-sensitive and matched nothing, so grepping the empty output
for a .NET 8 runtime found none, which read as confirmation. Both legs had been
running the net8.0 assets on .NET 8 the whole time. Replacing the print with an
assertion caught it on the first run.
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