Make Bonk reviews precise - #7552
Merged
Merged
Conversation
Contributor
|
Since last review: 1 resolved, 0 still open, 0 new. Not re-run: design-simplicity (no author changes in their files since the last review) Reviewed commit: fdd7824f · github run |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
Tune Bonk from its first live reviews and from maintainer pushback, so that a finding reads as "oh yeah, that's right" rather than noise. - Keep the rules every review follows in one place, .github/bonk/specialists/SHARED.md: never report what CI checks (builds, lints, formatting, tests) or performance micro-costs, hold test code only to "can it pass while the code is broken, is it flaky", and mark hypothetical or follow-up issues info at most. - Turn off the built-in performance specialist, and add workerd versions of the correctness, tests and docs specialists. - Stop asking for compatibility flags where they are not needed: code behind $experimental flags, Node.js and spec conformance fixes, unobservable changes, and additive exports of import-only modules. - Tighten specialist calibration from the first reviews: copyright headers only where siblings have them, compare with kj before flagging protocol deviations, test-only unsafety capped at info, and verify design suggestions before raising them. - Drop the jokes and boilerplate from review summaries. - Correct src/rust/AGENTS.md: a C++ exception through an infallible extern "C++" shim becomes a Rust panic, not an abort. - Pin danlapid/ask-bonk@4f53fb3 (Cloudflare-Studio/ask-bonk#228), which fixes the pipeline issues those reviews exposed and adds SHARED.md and disabling built-in specialists. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
danlapid
force-pushed
the
dlapid/bonk-precision
branch
from
September 27, 2026 00:06
8d15d72 to
fdd7824
Compare
jasnell
approved these changes
Sep 27, 2026
danlapid
enabled auto-merge (rebase)
September 27, 2026 00:12
danlapid
disabled auto-merge
September 27, 2026 00:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tunes Bonk from its first live reviews (#7532, #7533, #7535, #7540) and from where maintainers pushed back on its findings, so a finding reads as "oh yeah, that's right" rather than noise. Across four PRs, the first reviews posted 6 useful findings, 7 pedantic ones and 3 wrong ones, and dropped about 10 good design findings. About half of all maintainer pushback in the last three weeks was about compatibility flags that weren't needed (e.g. #7529, #7425, #7385).
One set of shared rules (
.github/bonk/specialists/SHARED.md, handed to every specialist and the judge):infoat most. Claims about dependencies, the language or CI need a source.Compatibility flags only where they're needed: not for code behind
$experimentalflags, Node.js or spec conformance fixes, unobservable changes, or additive exports of import-only modules.Specialists: the built-in performance specialist is off, and workerd has its own correctness, tests and docs specialists. Calibration from the first reviews: copyright headers only where siblings have them (not in the cxx fork), compare with kj before flagging protocol deviations, test-only unsafety capped at
info, and design suggestions verified before they're raised.Also: no jokes or boilerplate in summaries, and a correction to
src/rust/AGENTS.md(a C++ exception through an infallibleextern "C++"shim becomes a Rust panic, not an abort).ask-bonk: pins
danlapid/ask-bonk@4f53fb3(Cloudflare-Studio/ask-bonk#228). It fixes the pipeline issues the first reviews exposed: a first specialist review treated as a re-review, a wrong resolved count, duplicate summaries, startup races, and a judge that could raise severities or drop design findings. It also addsSHARED.mdand turning built-in specialists off.🤖 Generated with Claude Code