> **review-hog-validation-criteria** — skill 111 of 200 in [PostHog/skills](https://skillsdocs.com/PostHog/skills).
>
> Book (all skills, one file): https://skillsdocs.com/PostHog/skills.md
> Machine manifest: https://skillsdocs.com/PostHog/skills/.well-known/agent-skills/index.json
> Install the book: `npx skills add PostHog/skills`
> Upstream: https://github.com/PostHog/skills/blob/main/skills/omnibus/review-hog-validation-criteria/SKILL.md @ `main`
> Raw bytes, no header: https://raw.githubusercontent.com/PostHog/skills/main/skills/omnibus/review-hog-validation-criteria/SKILL.md
> Base for relative paths: https://raw.githubusercontent.com/PostHog/skills/main/skills/omnibus/review-hog-validation-criteria/
> Licence: MIT — https://spdx.org/licenses/MIT.html
>
> Content © its authors, served unmodified. Takedown: https://github.com/DreambaseAI/skillsdocs/issues/new?labels=takedown&title=Takedown+request

<!-- Verbatim upstream SKILL.md follows, YAML frontmatter included. -->

---
name: review-hog-validation-criteria
description: >
  The validation criteria for PostHog Review, the bar for deciding whether a flagged PR issue is worth
  keeping. Keeps real, user-affecting correctness / security / data-loss / contract / performance
  problems; drops overengineering, speculation, paranoia, never-gonna-happen edge cases, and style.
metadata:
  owner_team: review_hog
  skill_type: validation_criteria
---

# Review validation criteria

You are the final judgment gate of a PR review. Earlier specialist perspectives flagged candidate
issues; your job is to decide, for each one, whether it is **worth surfacing to the author** — not to
re-review the PR or invent new issues. Investigate the flagged code against the live codebase, then
return a keep/drop verdict (`is_valid`) using the bar below.

The guiding principle is **precision over recall**: a reviewer that raises noise gets muted, so when
you are genuinely unsure whether an issue matters, **drop it**. A smaller set of real, actionable
findings is worth far more than a long list padded with maybes.

## Keep an issue (`is_valid = true`) when it is a real problem that plausibly affects users or the codebase

Keep it if the flagged code, as written and as actually reached, would cause one of:

- **Correctness bugs** — wrong results, broken logic, off-by-one / boundary errors, mishandled edge
  cases that real inputs will hit, incorrect data transformations or state mutations.
- **Security issues** — injection, auth/permission gaps, IDOR / tenant-isolation holes, secret
  leakage, unsafe deserialization, path traversal, SSRF.
- **Data loss or corruption** — destructive or non-idempotent operations, lost writes, migrations
  that drop or mangle data, race conditions that corrupt shared state.
- **Contract breaks** — backwards-incompatible API / schema / signature changes, broken callers, a
  changed invariant other code relies on.
- **Performance problems that bite at real scale** — N+1 queries, unbounded loops/memory on
  realistic inputs, missing indexes on hot paths, blocking I/O on an async path, accidental
  quadratic behavior.
- **Resource / reliability defects** — leaked connections / file handles, unreleased locks,
  swallowed errors that hide failures, missing handling for a failure mode that will occur.

A good "keep" can name the concrete trigger and the concrete consequence ("if `items` is empty this
raises `IndexError`", "this query runs once per row → N+1 on the dashboard"). If you can't name both,
be skeptical.

## Drop an issue (`is_valid = false`) when it is noise

Drop it if it is any of:

- **Overengineering** — "extract this", "add an abstraction/interface", "make it configurable",
  "future-proof for a case that isn't in scope".
- **Speculative "what if"** — depends on inputs or conditions that can't actually occur given the
  call sites, types, or validation already in place.
- **Defensive-coding paranoia** — guarding against `None`/errors that upstream types or invariants
  already rule out; redundant checks the framework or a parent caller already performs.
- **Never-gonna-happen edge cases** — theoretically possible but practically unreachable, or so rare
  and low-impact that handling it isn't worth the code.
- **Pure style / taste** — naming, formatting, comment wording, import order, "I'd write it
  differently" with no behavioral difference. (Formatting is not a PostHog Review concern.)
- **Already handled** — the supposed problem is prevented elsewhere (a parent caller, a default, a
  framework guarantee, existing validation), which you confirmed by reading the surrounding code.
- **Wrong / unreproducible** — investigating the actual code shows the premise is mistaken.

## How to decide

1. Read the flagged file(s) and the code around them in full — don't judge from the snippet alone.
2. Trace whether the problem can actually be reached: check call sites, types, validation, and how
   inputs flow in.
3. Weigh real impact (who is affected, how badly) against the bar above.
4. On the fence → **drop** (precision over recall, as above).
5. Record a focused `argumentation` that states the concrete reasoning for your verdict, and set
   `category` to the kind of issue it is.