Code Review Guidelines

Use these guidelines for technical review and verification of review fixes. Read the sections relevant to the change; follow behavior across boundaries instead of limiting review to the directories in the diff. Contribution scope and test expectations remain in AGENTS.md and CONTRIBUTING.md.

Scope and Evidence

Identify the intended outcome and the candidate being reviewed: base/head commits, plus task-relevant staged, unstaged, and untracked files for local work. Inspect the actual changes and necessary callers; an author's summary is context, not proof that the implementation works.
Check that the change solves the reported problem without breaking adjacent behavior. For fixes, trace the cause and confirm that the verification would expose the original failure. Use the validation guidance for evidence limits.
Separate newly introduced regressions, existing defects, intentional changes, and unresolved hypotheses. Verify a suspected finding against reachable code and supported configurations before presenting it as a defect.
Prioritize correctness and user impact. Omit formatting already enforced by tools, personal style preferences, and speculative future requirements. Suggest simplification only when a concrete duplicate state, side effect, or unnecessary abstraction can be removed while preserving required behavior.

Frontend

Apply frontend architecture rules. Where affected, check:

Whether an older response or event can overwrite newer authoritative state, including after a profile, subscription, or credential change.
Whether navigation, mode switching, remount, reload, and application restart preserve or reset the intended state. Check pending edits and debounced writes.
Whether one user action duplicates commands or refreshes, and whether timers, subscriptions, and cache entries retain the correct lifetime and identity.

Backend

Apply backend architecture rules. Where affected, check:

What remains persisted or active after partial success, failure, or rollback, and whether returned errors and notifications describe that actual state.
Lock ordering, cancellation, retries, and ownership of Service/Core processes and OS settings through startup, shutdown, disconnection, and recovery.
Platform-specific control flow after conditional compilation. A check on one platform does not establish correctness on the others.

Cross-boundary Behavior

Trace changed commands and events from producer to consumer, including generated bindings, payload semantics, ordering, and success/error handling.
Check that UI state follows backend outcomes, including failed mutations and stale cache reads. Do not treat an optimistic display as proof of success.
For DNS, proxy, updater, or service changes, inspect the affected external state and failure path; source or mock evidence alone cannot establish native behavior. Keep unexercised cases explicit.

Findings and Re-review

Report each actionable finding with a file and line, triggering conditions, impact, supporting evidence, and whether this change introduced it. Severity follows impact and reachability. Keep optional simplifications separate from defects; do not turn existing issues into unrelated required fixes.
State plainly when no actionable findings remain, together with material verification gaps. This is a conclusion about the reviewed candidate, not a guarantee that every runtime or platform was exercised.
For each finding, verify the fix or explain why it is incorrect, intentional, or outside scope. Fix confirmed instances of the same defect within the task's scope, then recheck affected behavior.
On re-review, compare the new candidate with the previously reviewed version. Focus on finding closure, the repair diff, and its consequences. Expand only when changes or new evidence affect another boundary. Keep finding identities stable and do not repost resolved findings without new evidence.
Stop when actionable findings are resolved and relevant checks pass; do not require a fixed number of passes. After three rounds without convergence, summarize the remaining evidence or disagreement and reconsider the cause, scope, or approach before continuing. An unresolved finding is not a pass.

Agents own context transfer between implementation, review, repair, and re-review. The issue supplies the human-facing problem, scope, and acceptance criteria; maintenance without an issue uses the authorized task. Humans do not need to prepare agent handoffs or relay findings between agents.

Pass the task reference, candidate revision and scope, relevant source locations and constraints, verification results, and unresolved findings. Keep this in the working session or agent handoff; do not publish internal transcripts or require another repository document.
Separate observations from assumptions. Refresh stale context from source and independently verify the relevant paths instead of inheriting another agent's verdict. Retrieve missing code context within the authorized scope.
The repairing agent maps each repair or non-fix rationale to its original finding and passes the updated revision and verification results to the reviewer.
Escalate missing requirements, unavailable external evidence, or decisions that need human input; do not ask humans to reconstruct the implementation.

5.5 KiB Raw Blame History