.coderabbit.yaml

# CodeRabbit configuration — https://docs.coderabbit.ai/guides/configure-coderabbit
#
# The second bot reviewer on this repository, alongside GitHub Copilot. Two independent reviewers
# is the same argument the accuracy work itself runs on: one opinion is a claim, two that agree are
# evidence, and two that disagree are a finding. Copilot has caught overstated doc comments and
# order-dependent tests; this config tells CodeRabbit where the rest of the sharp edges are.

language: en-US
early_access: false

# House style, applied to reviews and chat alike. The repository's own rule, extended to the bot.
tone_instructions: >-
  Write in precise technical prose. No emojis anywhere. State the defect and the input that
  triggers it; do not soften a finding with praise or hedge one you are confident in.

reviews:
  # Thorough rather than gentle. This repository is an accuracy oracle: a review that lets a
  # plausible-but-wrong assertion through costs more than one that asks an unnecessary question.
  profile: assertive
  # Never block a merge on the bot's say-so. Findings are adjudicated in the thread and resolved
  # there; a "changes requested" state that a human then has to clear adds a step and no signal.
  request_changes_workflow: false
  high_level_summary: true
  high_level_summary_instructions: >-
    Say what the change claims and what would make the claim false. For an AccuracySNES change,
    name the dossier assertions added or altered and whether the coverage denominator moved. For an
    emulator-core change, name the observable behavior that differs. Do not restate the diff.
  poem: false
  review_status: true
  collapse_walkthrough: false
  # Post what the review left out — ignored files, suppressed comments, extra context. The default
  # is off, and that silence is exactly how the `**/gen/**` filter below went unnoticed across
  # three pull requests: the bot said nothing because it had been given nothing to say it about.
  review_details: true

  auto_review:
    enabled: true
    drafts: false
    auto_incremental_review: true
    ignore_title_keywords:
      - 'WIP'
      - 'DO NOT REVIEW'
    # Never stop re-reviewing. The default pauses automatic review after five reviewed commits,
    # and a batch here routinely takes more than five pushes while findings are adjudicated —
    # the later pushes are the ones carrying the fixes, so they are the ones worth reading.
    auto_pause_after_reviewed_commits: 0

  # Both of these open follow-up pull requests containing generated code, and both are wrong for
  # this repository. A doc comment here is the argument for why a test is not vacuous, not a
  # restatement of the signature; and a generated unit test asserting current behavior is the
  # precise opposite of an accuracy oracle, which asserts what the *hardware* does.
  finishing_touches:
    docstrings:
      enabled: false
    unit_tests:
      enabled: false
    # Also off, for a third reason: the chip cores' hot paths (`Bus::step`, the PPU dot loop, the
    # SPC700 dispatch) are deliberately allocation-free and abstraction-light. An automatic
    # "simplify" suggestion is more likely to fight that than to help it.
    simplify:
      enabled: false

  path_filters:
    # --- Includes ----------------------------------------------------------------------------
    #
    # These have to be here, and the list has to be complete. CodeRabbit ships a DEFAULT block
    # list that contains `**/gen/**` — which matches `tests/roms/AccuracySNES/gen/`, the test
    # generator, i.e. the most review-worthy source in this repository. The first three PRs under
    # this config had every generator file silently dropped ("Review skipped due to path filters")
    # and the `gen/src/**` instructions below never ran.
    #
    # **The override has to be the identical glob.** Including `tests/**` did not rescue it — the
    # bot's own report then read "excluded by `!**/gen/**` and included by `tests/**`", exclusion
    # winning. Repeating the blocked pattern verbatim is what removes it from the block list, so
    # the first entry below is deliberately `**/gen/**` and not the path it is there to reach.
    #
    # The catch is that these patterns also drive a sparse checkout, so a lone positive pattern
    # can narrow the review to *only* that path. Hence every directory worth reviewing is named
    # rather than just the one being rescued. **Adding a new top-level directory means adding it
    # here** — otherwise it is reviewed by nobody and nothing says so.
    #
    # Four top-level directories are left out deliberately, not by oversight: `assets/`, `images/`,
    # `screenshots/` and `raw/` hold binaries and captures, which a text review cannot say anything
    # useful about. `ref-docs/` and `ref-proj/` are excluded explicitly below, for a different
    # reason — they are immutable, so a finding there has nowhere to go.
    - '**/gen/**'
    - '.cargo/**'
    - '.github/**'
    - 'android/**'
    - 'crates/**'
    - 'docs/**'
    - 'ios/**'
    - 'scripts/**'
    - 'tests/**'
    - 'to-dos/**'
    - 'Cargo.lock'
    - '*.md'
    - '*.toml'
    - '*.yaml'
    - '*.yml'
    - '*.json'
    - '.editorconfig'
    - '.gitignore'
    - '.markdownlintignore'
    # --- Excludes ----------------------------------------------------------------------------
    # Generated artifacts. Reviewing them is reviewing the generator twice, and the ROM is binary.
    - '!tests/roms/AccuracySNES/asm/tests_group_a.s'
    - '!tests/roms/AccuracySNES/asm/scenes.s'
    - '!tests/roms/AccuracySNES/SOURCE_CATALOG.tsv'
    - '!tests/roms/AccuracySNES/ERROR_CODES.md'
    - '!tests/roms/AccuracySNES/build/**'
    - '!docs/accuracysnes-coverage.md'
    # Golden vectors: hashes and captured logs. A diff here is a blessing decision made elsewhere;
    # the file itself carries no argument to review.
    - '!tests/golden/**'
    # Immutable research and vendored study clones.
    - '!ref-docs/**'
    - '!ref-proj/**'
    # Vendored upstream C (rcheevos). Not ours to change; findings there have nowhere to go.
    - '!crates/rustysnes-cheevos/vendor/**'

  path_instructions:
    - path: 'tests/roms/AccuracySNES/gen/src/**'
      instructions: |
        This generates a hardware-accuracy test cartridge. Judge each test by whether it can
        distinguish the behavior it names from the alternatives, not by whether it passes.

        Flag, specifically:
        - **Vacuity.** An assertion whose expected value is also what a broken or absent
          implementation produces (zero, "unchanged", "not $FF") needs a paired control assertion
          that would fail on that implementation. Say which alternative goes uncaught.
        - **Overstated doc comments.** The prose above a test is a claim about what it validates.
          If it names behaviors the emitted program does not exercise, or asserts a rationale that
          is not true of the code, that is a defect even though the test passes.
        - **Shared state.** OAM, CGRAM, VRAM and the S-DSP registers are not reset between tests. A
          test that does not establish its own starting conditions may be measuring the previous
          one; look for an earlier test that leaves the relevant state dirty.
        - **Timing-marginal reads.** Reading a register a few cycles after disturbing it, or
          asserting on a value that is still moving, produces a verdict that flips when unrelated
          code shifts. Prefer a settle, a disarm, or a provably stationary value.
        - **Scanline geometry.** Line 0 is a blanking line; the V counter's low byte aliases on a
          312-line PAL frame; the visible height is 224 or 239 depending on overscan. Constants
          derived from any of these deserve a second look.
        - **Duplicate coverage.** `dossier.rs::MAP` must not claim an assertion another test already
          implements. There is a build gate for this, but flag it in review too.

    - path: 'tests/roms/AccuracySNES/gen/src/scenes.rs'
      instructions: |
        Rendered scenes are hashed framebuffers. A scene proves nothing unless the picture it draws
        differs from every other scene's — a stable hash that three emulators agree on is not
        evidence, because a scene that renders the wrong thing is equally stable and equally agreed.
        Flag any scene whose setup may not be able to express the difference it claims: a periodic
        canvas that hides a scroll, a square sprite where a tall one was meant, a register the mode
        ignores.

    - path: 'tests/roms/AccuracySNES/asm/runtime.s'
      instructions: |
        Hand-written 65816. The battery runs under forced blank throughout; anything that lifts it
        must put it back. Helpers are width-neutral (`php`/`plp`) by convention — flag one that is
        not, because ca65's `.a8`/`.a16` state is file-global and a mismatch emits silently wrong
        instruction lengths.

    - path: 'crates/**'
      instructions: |
        Emulator core. Hot paths are allocation-free; `unsafe` requires a `// SAFETY:` comment
        naming the invariant. Any change to save-stated fields needs a `FORMAT_VERSION` bump and a
        `docs/adr/0006` bump-log entry. Behavior changes must update the matching `docs/<chip>.md`
        in the same change.

    - path: 'docs/**'
      instructions: |
        Docs are the spec, not a history log. Flag claims that contradict the code, counts that
        contradict the generated `docs/accuracysnes-coverage.md`, and any statement of coverage that
        is broader than what the corresponding test actually asserts.

    - path: 'tests/roms/AccuracySNES/asm/runtime.inc'
      instructions: |
        Shared symbol definitions. Every address here is claimed by something; a new variable that
        overlaps an existing block, or a block whose stated width is smaller than what the code
        stores into it, is the failure mode. Check the arithmetic of `VAR_BASE + $nn` offsets
        against their declared sizes and against the neighbouring definitions.

    - path: 'scripts/accuracysnes/**'
      instructions: |
        The cross-validation harness: the same AccuracySNES image is run on snes9x (through a
        libretro host in C) and on Mesen2 (through its test runner and a Lua script), and their
        verdicts are compared with the cart's. Its integrity is the whole argument for the
        battery, so flag anything that could make a reference *appear* to agree — a verdict parsed
        loosely, a missing-file path that degrades to success, a scene comparison that skips
        rather than fails when the golden is absent. A known reference divergence belongs in
        `SNES9X_KNOWN_FAILURES` with a source citation, never in a widened match.

    - path: '.github/workflows/**'
      instructions: |
        Flag third-party actions pinned to a branch rather than a tag or commit SHA, jobs without
        a least-privilege `permissions:` block, and secrets that could reach the log. The
        `ci-success` job is the required check and must depend on every gate it claims to
        aggregate — a new job that is not in its `needs:` list is a gate nothing enforces.

    - path: 'crates/*/Cargo.toml'
      instructions: |
        The frontend pins one compatible GUI tier (currently winit 0.30 / wgpu 29 / egui 0.35 and
        its `egui-wgpu`/`egui-winit` siblings). Flag a bump to any one of those that is not
        accompanied by matching bumps to the others. Optional features are default-off by
        convention so shipped, no_std and wasm builds stay byte-identical; flag a new feature added
        to a `default = [...]` list.

    - path: 'android/**/*.kt'
      instructions: |
        Kotlin/Compose front end over the UniFFI bridge. Flag recomposition footguns (unstable
        lambdas or parameters causing needless recomposition) and any direct file or preferences
        access that bypasses the bridge.

    - path: '**/*.swift'
      instructions: |
        SwiftUI/Metal front end over UniFFI-generated bindings. Flag force-unwraps (`!`) on values
        originating from the Rust bridge or from file I/O.

    - path: 'scripts/**/*.py'
      instructions: |
        One-off developer tooling, not shipped code. Prioritize correctness and legible failure
        over polish; do not ask for packaging, typing or CLI-ergonomics work unless it affects
        whether the script is right.

    - path: '**/*.md'
      instructions: |
        Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
        rather than style nits. The markdownlint gate is pinned to **v0.39.0** via pre-commit —
        do not report rules that version does not have (MD060 in particular).

  # Encodings of gates this repository actually runs, so the bot's verdict and CI's agree.
  pre_merge_checks:
    description:
      mode: warning
    title:
      mode: warning
      requirements: |
        Conventional Commits: `type: subject`, with an optional `(scope)` and an optional `!`
        marking a breaking change — `docs: refresh the coverage table`, `feat(ppu): ...` and
        `feat!: ...` are all valid. Imperative mood, no trailing period, and the type drawn from
        feat, fix, docs, refactor, test, chore, perf, build, ci.
    custom_checks:
      - name: CHANGELOG entry
        mode: warning
        instructions: |
          If the diff changes user-visible behavior — emulator output, a frontend feature, a CLI
          flag, a public API, or the contents of the AccuracySNES cartridge — `CHANGELOG.md` must
          be modified in the same pull request. Pass if the change is purely internal
          (refactoring, tests of existing behavior, comments, CI configuration).

          **Evaluate the whole pull request against its base branch, never the latest commit.** A
          batch here is routinely several pushes: the entry lands with the first, and later commits
          carry review fixes. A head commit with no `CHANGELOG.md` hunk therefore says nothing about
          whether the pull request has one. Report a failure only if the entry is absent from the
          full PR diff.
      - name: Docs-as-spec
        mode: warning
        instructions: |
          Evaluate the whole pull request against its base branch, never the latest commit.

          A behavior change under `crates/rustysnes-<chip>/` must be accompanied by an edit to
          the matching `docs/<chip>.md` in the same pull request, because those documents are the
          specification rather than a history log. Pass only if the crate change does not alter
          observable behavior — say what the change does instead. "The document was already
          correct" is not a pass: if the behavior changed, the specification of it changed, and
          this check and the `crates/**` path instruction have to say the same thing.
      - name: AccuracySNES bookkeeping
        mode: warning
        instructions: |
          Evaluate the whole pull request against its base branch, never the latest commit.

          If the diff adds or removes a test or a scene under `tests/roms/AccuracySNES/gen/src/`,
          check three things: the test appears in `dossier.rs::MAP` (mapped to a dossier assertion
          or to an empty slice with a matching `UNENUMERATED` entry); the regenerated artifacts
          (`asm/tests_group_a.s`, `SOURCE_CATALOG.tsv`, `ERROR_CODES.md`,
          `docs/accuracysnes-coverage.md`, the two `.sfc` images, and `asm/scenes.s` whenever a
          scene changed) are present in the diff, since a source change without them means the
          committed ROM does not match its generator; and the hand-maintained counts in
          `docs/accuracysnes-plan.md` moved by the same amount as the generated coverage table.

          **Every one of those artifacts is excluded by the path filters above**, so their contents
          are invisible to this review and their absence from the reviewed set proves nothing. Judge
          their presence from the "files ignored due to path filters" list, which names them; a file
          in neither list is the failure. Never report an artifact as missing on the strength of not
          having been able to read it.
      - name: No panic on untrusted input
        mode: warning
        instructions: |
          Flag any new `.unwrap()`, `.expect()` or `panic!()` applied to data that came from
          outside the process — ROM and save-state bytes, netplay messages, Lua or scripting
          input, user-supplied paths — outside `#[cfg(test)]` code. Those boundaries return a
          typed error. Pass when the value was just constructed locally or an invariant was
          checked immediately above, and say which.
      - name: SAFETY comment on new unsafe
        mode: warning
        instructions: |
          Every new `unsafe { ... }` block or `unsafe fn` needs an adjacent `// SAFETY:` comment
          naming the invariant relied on and who guarantees it. Fail if a new unsafe block has
          none. `unsafe_code` is a workspace lint here, so unsafe outside the frontend and the FFI
          shims deserves a question about why it is there at all.

  # The stack, stated rather than inherited. CodeRabbit enables roughly fifty linters by default;
  # this repository is Rust, plus Kotlin and Swift front ends, Python and Lua tooling, a little C
  # for the libretro cross-validation host, shell, YAML, Markdown and GitHub Actions. Everything
  # else is off — not because those tools are bad, but because a finding for an ecosystem with no
  # files in the tree is noise that costs attention the real findings need.
  tools:
    clippy:
      enabled: true
    ast-grep:
      essential_rules: true
    semgrep:
      enabled: true
    opengrep:
      enabled: true
    gitleaks:
      enabled: true
    trufflehog:
      enabled: true
    osvScanner:
      enabled: true
    actionlint:
      enabled: true
    zizmor:
      enabled: true
    yamllint:
      enabled: true
    markdownlint:
      enabled: true
    shellcheck:
      enabled: true
    ruff:
      enabled: true
    luacheck:
      enabled: true
    clang:
      enabled: true
    cppcheck:
      enabled: true
    detekt:
      enabled: true
    swiftlint:
      enabled: true
    # Redundant with ruff — two Python linters means two findings for one defect.
    pylint:
      enabled: false
    flake8:
      enabled: false
    # No files of this kind in the tree.
    hadolint:
      enabled: false
    checkmake:
      enabled: false
    fbinfer:
      enabled: false
    pmd:
      enabled: false
    phpstan:
      enabled: false
    phpcs:
      enabled: false
    phpmd:
      enabled: false
    rubocop:
      enabled: false
    brakeman:
      enabled: false
    prismaLint:
      enabled: false
    sqlfluff:
      enabled: false
    squawk:
      enabled: false
    checkov:
      enabled: false
    tflint:
      enabled: false
    buf:
      enabled: false
    regal:
      enabled: false
    oasdiff:
      enabled: false
    circleci:
      enabled: false
    reactDoctor:
      enabled: false
    emberTemplateLint:
      enabled: false
    shopifyThemeCheck:
      enabled: false
    smartyLint:
      enabled: false
    htmlhint:
      enabled: false
    stylelint:
      enabled: false
    biome:
      enabled: false
    oxc:
      enabled: false
    eslint:
      enabled: false
    dotenvLint:
      enabled: false
    psscriptanalyzer:
      enabled: false
    blinter:
      enabled: false
    fortitudeLint:
      enabled: false

knowledge_base:
  # Claims in this repository cite external hardware documentation — SNESdev, fullsnes, anomie's
  # register notes — and a review that can check a cited claim against its source is worth more
  # than one that can only check internal consistency.
  web_search:
    enabled: true
  code_guidelines:
    enabled: true
    # Explicit rather than inherited: the defaults cover AGENTS.md and CLAUDE.md, but this
    # repository's binding rules are as much in CONTRIBUTING.md and the ADRs — ADR 0013 is why
    # on-cart and scene coverage are never summed, ADR 0006 is the save-state version discipline.
    filePatterns:
      - 'AGENTS.md'
      - 'CLAUDE.md'
      - 'CONTRIBUTING.md'
      - 'docs/adr/*.md'
      - 'docs/architecture.md'
      - 'docs/testing-strategy.md'
  learnings:
    scope: auto
  issues:
    scope: auto
  pull_requests:
    scope: auto

chat:
  auto_reply: true