shoc-frontend-new/REVIEW_AND_PR_FRAMEWORK.md

REVIEW_AND_PR_FRAMEWORK.md — PR review contract

Every PR is reviewed against this contract, by humans and by review agents. The goal is high-signal review: catch real behavior, security, performance, and regression problems — not re-lint what the gates already enforce.

1. Review the exact head

• MUST review the diff at the current PR head, not a stale checkout. Re-pull before reviewing if new commits landed; stale approvals are dismissed on push by branch protection.
• MUST read the full diff of every changed file, including renames and generated/mapper code, not only the "interesting" components.

2. Board-backed regression inventory

• MUST check the change against the Seahaven Jira SH board inventory — all visible tickets for this area, not only the current ticket. Behavior that is Done / QA-approved / released is protected scope.
• MUST treat a plausible regression against protected behavior as a Blocker until disproven with repo evidence (the accepting tests, the linked PR/release, and a targeted check of the changed code paths).
• MUST NOT approve if board access or the relevant inventory is missing — say so and block rather than infer a pass. A missing ticket link alone is not a blocker, but known acceptance criteria must still be traced.

The current Jira workflow has no Ready for QA transition. Merged work remains in the documented pre-QA status until QA evidence supports Done; do not invent a status or mark unverified work Done.

3. Behavior-based testing

• MUST test through public behavior (rendered output, user interactions, query/mutation outcomes), not internal implementation details. Prefer @testing-library queries and user-event flows; assert what users observe.
• MUST NOT add tests whose only purpose is to prove a tool (ESLint, the governance script) executes — validate tooling by running the real gates (npm run verify), not with assertion-free unit tests.
• SHOULD cover the meaningful branches of new logic: the happy path, the error/empty state, and any boundary the change introduces.

4. Security review

• MUST confirm no raw server error payloads, stack traces, or internal IDs leak to the UI (see ARCHITECTURE_AND_CODE_QUALITY.md §Security).
• MUST confirm no secrets/tokens are introduced into the build, and that no VITE_* variable carries a secret (it is baked into the bundle).
• SHOULD check untrusted input is validated (Zod) before use and that dangerously-set HTML / unescaped server strings are not introduced.

5. Performance and Big-O review

• MUST flag algorithmic regressions in hot paths: O(n²)+ loops over server collections, re-filtering/sorting on every render, unbounded list rendering without virtualization.
• MUST confirm TanStack Query keys are stable and that mutations invalidate the correct keys (no stale cache, no redundant refetch storms).
• SHOULD question speculative useMemo/useCallback (add when measured) and unstable identities passed to memoized children.

6. No style-only comments

• MUST NOT leave comments that only restate what Prettier or ESLint already enforces (formatting, naming nits the linter catches). Style is settled by the gates; review is for behavior, correctness, security, and architecture.
• MUST NOT put Jira issue keys or ticket titles in source comments, JSDoc, or test names (for example (SH-183)). Ticket identity belongs in the PR, commit message, and branch — not in the code. Flag and request removal if a diff adds them.
• MUST make every comment actionable: tie it to a behavior, a risk, or an evidence-based convention in these docs, and offer a concrete fix or a targeted question. Use GitHub suggestion blocks when safe.

7. Review close-out

A review is complete when it records, briefly:

1. Findings ordered by severity (Blocker / Needs-change / Suggestion), or "no findings".
2. Open questions and their owner.
3. Which of the above checks were run, and any that were skipped (and why).
4. Residual risk, if approving.

Do not write a monolithic review body or a validation transcript into the PR surface; keep comments inline and high-signal.