[vchamarthi]

Summary

Adds a review guide and a pre-PR self-review checklist, so that most findings are caught before a PR is opened and each review round closes more of them. Text only; no code, CI or gate changes.

 /
+├── REVIEW.md                         # what a defect is here, how to raise and close findings, rounds
 ├── AGENTS.md
+│   └── "Before opening a PR"         # self-review: mutation check, sibling sites, bad inputs, docs match code
 └── .github/
+    ├── copilot-instructions.md       # points Copilot review at REVIEW.md
     └── pull_request_template.md
+        ├── "Revert" section
+        └── "Self-reviewed" checkbox

Why: what the data shows

The figures below are self-measured and keyword-matched, not independently audited. REVIEW.md is read by review bots; coding agents read AGENTS.md.

PR history of intel/dokimi (GitHub API, all 185 PRs, 13 to 23 Oct 2026):

Measure	Value
PRs	146 merged, 32 open, 7 closed
Time to merge	median 13.7 h, p90 64.9 h
Commits per merged PR	median 4, p90 22
Review threads	1,416 on 127 PRs
Threads on the 25 PRs with 15+ commits	603 (43%)
Threads unresolved at merge	346 of 1,141, on 76 of 146 merged PRs

Recurring themes in thread openers (keyword match, overlapping): tests that cannot fail 680, nits 387, docs or docstrings that disagree with code 246, timing or flakiness 161, error handling 121, changelog 101, exit codes 88, Windows paths 35. Several are the same bug class found one command at a time, for example the unreadable-input exit code across #189 to #194, a docstring claiming exit 1 where the code exits 2 (#190), None from os.cpu_count() (#112), and backslash paths in generated config (#116).

One week of agent review on this repo (self-measured audit of 301 review points on 65 PRs, 14 to 20 Oct 2026):

• 57 of 62 must-fix findings came from running code (a repro, a differential against main, a mutation check), not from reading it.
• A pushed fix with a test landed 99% of the time; a comment-only point landed 62%.
• 21 of 83 author fixes needed another round; 5 of 13 must-fix fixes made by authors regressed.
• 3 of the 7 must-fix issues missed were in the reviewer's own fix commits.
• Misses clustered on the largest PRs (repo inspect: static look at what a repository offers an agent (repo_environment.v1) #66, refactor(eval): give answer and eval records one reader (eval/records.py) #80, docs, tests: user-journey catalog (J1–J15) and criterion evidence trace #102, refactor(harnesses): docker-* solver, sandbox and probe on the process runner #104).

How the Claude Code team runs review (Solving code review at Anthropic):

• Target the least human time per change with no rise in reverts. On their repo, merged PRs more than doubled while the share needing a revert fell by more than 40%.
• Verify before the PR: tests, build and a fresh-context review of the diff. Giving the agent a way to check its work is estimated to make results two to three times better.
• A REVIEW.md per repo: say what failure looks like there, cap nits, require file and line as evidence, open with a count and say plainly when there is nothing to fix.
• Converge: re-review the diff only after early rounds, flag the bug class so every instance is fixed, never repeat a posted finding, and after three rounds reconsider the approach. This took PRs passing in one round from 65% to 78% and mean reviews to pass from 2.08 to 1.32.
• Every finding is fixed or closed with a reason. Repeated comments move upstream into a lint rule, skill or instruction file.

Changes

• REVIEW.md: defects defined for this repo (wrong number, verdict or exit code; a test that cannot fail; docs that say what the code does not; schema drift; Windows or None behaviour). Findings name file, line and a breaking input, name the bug class and its other sites, are pushed as fix plus test when short, are marked must-fix or nit (nits only after must-fix findings, three at most), and are not repeated unless the fix did not hold. Bugs in unchanged code are noted once as pre-existing. Later rounds review the new diff and its callers and re-run the original input. After three rounds of must-fix findings, agree the approach in the thread. Threads close with the fixing commit or a reason. Repeated comments become a test, a make ci-local check or an AGENTS.md line.
• AGENTS.md, "Before opening a PR": mutate the code under each test of changed behaviour and revert; fix sibling sites in scope and list the rest; run bad inputs where they apply and say what was not run; check the title, docs and changelog against the code; re-review your own fix commits.
• .github/copilot-instructions.md: Copilot review follows REVIEW.md.
• PR template: "Revert" section and a "Self-reviewed" checkbox.

How this helps later PRs

Seen in the data	What changes
Must-fix found by running code, late	The author runs the mutation and bad-input checks before opening
Same bug found one command at a time (#189 to #194)	Author and reviewer name the class and its sites in one round
Author fixes regress, reviewer fixes miss	Every fix, either side's, is re-run on the original input
387 nit threads, each a commit and a round	Three nits per review at most
346 threads open at merge	Each thread closes with a commit or a reason
Docs and exit codes disagree with code	Checked by the author before review
The same comment on PR after PR	Moved into a test or AGENTS.md

Nothing here adds a required check or blocks a merge.

Risk

Two-way door: text only, reverted in one commit. Blast radius: contributor and reviewer workflow. Agents that load AGENTS.md or REVIEW.md change how they self-review and comment; CI, the package and stored records are unaffected.

Testing

• pytest tests/test_docs_code_refs.py: 40 passed (paths and commands named in AGENTS.md resolve).
• New and changed lines are ASCII.

Revert

git revert of this PR. Nothing is left behind.

Checklist

• Tests added/updated and pytest passes (no code change; doc reference test run)
• ruff, mypy, and schema checks pass (N/A, no code)
• Docs updated (or N/A)
• Self-reviewed per "Before opening a PR" in AGENTS.md
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

[Copilot]

🟢 Approval recommended

The four documentation changes are consistent with the stated scope, with no unresolved defects identified.

No open issues 🎉

What changed in this PR

Adds contributor and reviewer guidance to catch defects before PR submission and reduce repeated review rounds. The changes affect workflow documentation only; application code, CI, and merge checks are unchanged.

Changes:

• Defines actionable findings, review rounds, and thread-resolution expectations.
• Adds pre-PR self-review checks for test effectiveness, related bug sites, bad inputs, and documentation accuracy.
• Connects Copilot to the guidance and adds rollback and self-review prompts to the PR template.

File	Description
REVIEW.md	Defines defects, finding expectations, and review follow-up.
AGENTS.md	Adds the pre-PR self-review checklist.
.github/​pull_request_template.md	Adds a Revert section and self-review checkbox.
.github/​copilot-instructions.md	Points Copilot to review and coding guidance.

_{Review effort: Balanced}

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.81%. Comparing base (cdb58cb) to head (eb77363).
⚠️ Report is 42 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #206      +/-   ##
==========================================
+ Coverage   91.85%   92.81%   +0.96%     
==========================================
  Files         223      241      +18     
  Lines       27712    30516    +2804     
  Branches     4563     5039     +476     
==========================================
+ Hits        25455    28324    +2869     
+ Misses       1600     1487     -113     
- Partials      657      705      +48     

see 82 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[napetrov]

I ran dokimi's own repo eval on this change, with and without the PR. It is safe to merge as text, but at this sample size it doesn't change what agents do, and agents never open REVIEW.md.

Static (dokimi repo budget, no LLM): start context goes from 7,377 to 8,182 chars (+11%), all of it the new AGENTS.md section. REVIEW.md is not loaded when a session starts. Agents reach it only through the one-line pointer.

Agent run (dokimi repo run): 4 .agent-eval tasks × {guidance:raw, this PR} × 3 reps, Sonnet 5.5, 24 cells, $2.18. The task base (16e319f) has only CLAUDE.md, so I ported the change: same REVIEW.md, and the same section appended to CLAUDE.md.

task	without PR	with PR
fix-degrade-pre	3/3 pass, 7.7 turns, $0.122	3/3, 11.3 turns, $0.143
fix-docker-reward	3/3, 10.7 turns	3/3, 11.7 turns
rename-cli-flag	3/3, 4.3 turns, $0.027	3/3, 6.0 turns, $0.042
review-pr-conventions	recall 0.93	recall 1.00 (one extra off-by-one find, noise at n=3)

• regress, ruff, mypy and diffcov were all 100% in both arms.
• repo report says the arm's additions were used in 0/3 cells for every task.
• 1 of 12 cells did a real "test fails without the fix" check (git stash, then rerun the tests). No cell ran a bad input or make ci-local.
• Net effect: +1 to 2 turns per fix/docs task (+3.6 on fix-degrade-pre) and more spend per task (+3% to +55%), with no change in quality.

Suggestions:

1. REVIEW.md is read by review bots (Copilot, the Claude Code Review app) but not by coding agents. If the goal is author-side self-review, the few rules that matter need to be in AGENTS.md itself, or the PR should say plainly that REVIEW.md is for review bots.
2. The checks that can be automated would land more reliably as gates than as prose, which is what REVIEW.md's last section recommends. Examples: docstring exit codes against the real ones, and None/Windows-path inputs for each command. These could go in make ci-local or the test-quality gate.
3. "At most three nits per review" fits a bot, but may suppress real low-severity findings from a person. Maybe: "nits only after must-fix findings; at most three."
4. The numbers in the description (57/62, the theme counts) are self-measured and keyword-matched. Worth saying so next to them.

Caveats: n=3, one model, guidance ported onto an older base. A stronger test would re-pin the tasks to a base that has AGENTS.md and add a task with a test that can't fail. Runs are available if useful.

[antonwolfy]

LGTM. The guide's defect list matches the repo's real failure classes, and every path, command and CI gate it names exists in the tree. Two nits inline.

What I ran, on a Linux review host at a54f76c:

• make ci-local: 26 passed, 7 skipped, 3 failed. The failures are the gitleaks tool download (HTTP 504), the gitleaks scan that depends on it, and pip-audit; none touch files in this PR. A manual gitleaks detect over origin/main..HEAD found no leaks. Ruff, mypy, deptry, bandit and 7,610 tests pass.
• Added lines are ASCII (grep for non-ASCII bytes on the diff returns nothing). No changelog fragment is needed: no path under dokimi/dokimi/, dokimi/data/ or pyproject.toml changes.

Checked, no issues

• Paths and gates: dokimi/changelog.d/, make ci-local, .github/.licenserc.yaml and the CI gates named in REVIEW.md all exist and match AGENTS.md.
• License headers: REVIEW.md and copilot-instructions.md carry the SPDX header; AGENTS.md and the PR template are exempt in .licenserc.yaml.

@napetrov's question on whether REVIEW.md should reach coding agents (it is only read by review bots) and on the three-nit cap for human reviewers waits for the author.

[napetrov]

Re-review at 97676db. Nothing to fix: no new findings, and none still open.

Earlier threads: all three of antonwolfy's inline nits (repeat vs count rule, REVIEW.md not scanned, nit definition) are addressed in d925913 and read correctly at head. My earlier eval comment (no measured lift at n=3, REVIEW.md not opened by agents) still stands as context, not as a blocker; this is a text-only, one-commit revert.

What I ran at 97676db

• uv run pytest tests/test_docs_code_refs.py: 41 passed (one new case, [REVIEW.md]).
• Mutation check on the new scan: appending a bogus path and make target to REVIEW.md makes test_doc_code_references_resolve[REVIEW.md] fail; reverted, passes again. So the added line is a real check.
• CI at head: 16 success, 2 skipped. Mergeable.

Fixes pushed: none.

[antonwolfy]

LGTM. All three nits from my earlier review are fixed at 97676db, and the new text reads correctly. Nothing new to fix.

What I ran, on a Linux review host at 97676db:

• make ci-local: 28 passed, 1 failed, 7 skipped. The failure is pip-audit: filelock 3.32.7 has three new advisories, fixed in 4.2.1. main pins the same version and this PR touches no lock file, so it is not from this change. Ruff, mypy, deptry, bandit, gitleaks, the mkdocs build and 7,611 tests pass.
• tests/test_docs_code_refs.py: 41 passed, including the new [REVIEW.md] case. Changing dokimi/changelog.d/ to dokimi/changelogs.d/ in REVIEW.md makes it fail, so the added scan is a real check.

Checked, no issues

• Earlier nits: the repeat and count rules no longer combine into a false "nothing to fix" (REVIEW.md:39-42). The nit definition is at REVIEW.md:34-36. REVIEW.md is scanned.
• Claims: the ruff statement (E501, quotes and import order not selected) and the growth-baseline rule match pyproject.toml and AGENTS.md. The CI gates, make ci-local and .github/.licenserc.yaml exemptions named in the new text exist.

@napetrov's question on whether REVIEW.md should reach coding agents and on the three-nit cap for human reviewers waits for the author.

[vchamarthi]

@napetrov @antonwolfy suggestions applied at eb77363, ready for another review.

• Nit rule now reads "Post nits only after the must-fix findings, at most three" (napetrov point 3).
• REVIEW.md now says review bots read it and coding agents read AGENTS.md, whose "Before opening a PR" section holds the author-side checks (point 1).
• The description now says the figures are self-measured and keyword-matched (point 4).
• Point 2 (docstring exit codes and None/Windows-path inputs as make ci-local gates) is code, so it is left for a follow-up PR.

tests/test_docs_code_refs.py: 41 passed.

[antonwolfy]

LGTM. The two changes since 97676db, the new reader note and the reworded nit bullet, are correct, and nothing new needs fixing.

What I ran, on a Linux review host at eb77363:

• make ci-local: 28 passed, 1 failed, 7 skipped. The failure is pip-audit: three known advisories in filelock 3.32.7. This PR touches no lock file, so it is not from this change. Ruff, mypy, deptry, bandit, the build and 7,611 tests pass.
• tests/test_docs_code_refs.py: 41 passed with [REVIEW.md] in the scan. Renaming dokimi/changelog.d/ to dokimi/changelogs.d/ in REVIEW.md makes it fail with missing path, and the unmutated file is clean. The lines added since 97676db introduce no false positive.

Checked, no issues

• Reader note (REVIEW.md:12-14): .github/copilot-instructions.md points review at REVIEW.md and code writing at AGENTS.md, and AGENTS.md has the "Before opening a PR" section it names. This takes @napetrov's first point.
• Nit rule (REVIEW.md:36-41): "nits only after the must-fix findings, at most three" does not conflict with the nit definition or the repeat and count rules. This takes @napetrov's third point.
• Ruff and growth claims: pyproject.toml selects no E501, Q or I, and the growth-baseline rule matches AGENTS.md.
• Repo rules: SPDX headers are present or exempt in .github/.licenserc.yaml, there are no added non-ASCII bytes, no generated artifacts, no nav change and no changelog fragment is needed.

@napetrov's second point (docstring exit codes and None/Windows-path inputs as make ci-local gates) is code and waits for the author's follow-up PR.