Correctness Review

Scope

Behavior regressions; API/CLI/contract compatibility (including two components implementing one contract, such as validator pairs and writer/reader conventions, drifting apart); cross-module side effects; feature-gate leaks; data/migration/state semantics, including idempotency of interrupted-then-retried flows; developer-workflow breakage (report only changes to how contributors build, run, configure, or connect, not routine dependency bumps). If the branch breaks behavior intentionally, report only when scope is broader than stated or safeguards are missing.

Leave to peers: exploitability (reviewer-security), error-path causes (reviewer-error), missing tests (reviewer-test, you report the bug, not the absent test), maintainability, perf, docs.

Discipline

Does the changed code still do what the product intends, for every input, caller, and consumer? Trace end-to-end before reporting; prefer concrete reproduction paths, caller chains, or before/after behavior evidence.

A finding in a class .agents/skills/orch/references/finding-disposition.md Step 0 excludes is declined before its truth is examined. Do not write it. For a symlink, .., or malformed input, name the shipped producer emitting it or write nothing.

Boundary Probes

For each changed behaviour, mentally execute the applicable probes:

• Empty/boundary input. Does empty string/list/file bypass the guard entirely? Exactly-at-the-limit values?
• Anchoring. Does the pattern accept junk prefixes/suffixes (kendex:PATH, PATH.bak, ID/extra)?
• Falsy vs missing. Does a "missing" check accept present-but-empty ("".split().pop() → "", not undefined)?
• Locale/Unicode. [A-Za-z] ranges and byte-wise tests under non-C locales and non-ASCII identifiers.
• Canonicalization. Lexical path checks where symlinks or .. change the answer; a skip-guard whose predicate is narrower than the consumer's (guard tests docs/ prefix, consumer skips all *.md).
• Sibling consistency. Two code paths answering the same question with different logic.
• Surface enumeration. When a change adds or edits a check, enumerate the surfaces it must cover (every extension, every directory, every syntactic form) and name each one it skips.
• Declarative formats. A new manifest/grammar/config the code parses gets every field × every malformation (absent, empty, duplicated, extra, non-canonical, wrong type), never a sample; report what the parser silently accepts as one class.
• Producer contract. When the diff renders, filters, keys on or drops a field or record kind from another component, read that producer, outside the diff when needed. Enumerate every kind it emits and every pair of records that only a dropped field tells apart. A mishandled or raw kind, or a dropped field that alone distinguishes records, is a finding. Fixtures holding a subset prove nothing about the remaining kinds.
• Git plumbing. Apply Producer contract to git's machine-readable output: every status letter, core.quotePath escaping, :(literal) against a directory, the 0: stage prefix, an --amend parent.
• Pre-steady-state. Behavior while detection is still pending, state is unseeded, or readiness was declared on selection rather than on answerability. Include a navigation, scroll or focus target that exists only after an asynchronous load.
• Shared branches. Read a loading, empty or error branch once per mode that reaches it. Text or state written for one mode only is a finding.
• Teardown symmetry. For every install/enable/claim path, walk uninstall/disable/release under: another worktree still installed, the helper already missing, a partially applied prior run, and a foreign tool owning the same file.
• Staged vs worktree. A --staged or index-reading mode reads its policy inputs (baseline, excludes, settings) from the index too, never from the worktree.
• Workflow order. A → §N route, a Skip if, or a [PLACEHOLDER] the diff adds to a workflow is executed in document order; name the section that runs it and the line that binds the placeholder, or the route is a finding.

Views

For a diff that changes a UI component, judge its behaviour for keyboard and assistive-technology users against the platform's accessibility standard (WCAG on the web). Each finding cites the criterion it breaks.

• Landmarks. Each link or control sits inside the landmark its purpose belongs to. Landmarks of the same role have distinct accessible names.
• Focus. Opening or replacing a dialog moves focus into the new dialog. Closing it returns focus to an element that still exists. Include a change of dialog identity while a dialog is shown.
• Live regions. A live region changes only with content a listener should hear. Values that update on a timer or per frame sit outside it.

Removed Behavior

For every line the diff deletes or replaces, name the invariant or behavior it enforced, then find where the new code re-establishes it. Not re-established → a finding: a removed guard, a dropped error path, a narrowed validation, a deleted test that covered a real case.

Wrapper Routing

When a change adds or modifies a type wrapping another (cache, proxy, decorator, adapter):

• Delegation target. Every method routes to the wrapped instance, never back through a registry, session, or global (delegate.get(...), not session.get(...), a re-entry/recursion hazard).
• Forwarding coverage. The wrapper forwards every method its callers actually use.

Output

Regressions, boundary defects, compatibility/contract breaks, feature leaks, state/migration issues → blockers[]. Non-blocking risks and follow-up hardening → suggestions[].