# Code review: the automated part

Four gates existed before this document (the test suites, `apps/studio/tools/ui_check.py`, the
real-GL smoke tests, the binding-purity test) and none of them is a linter. This adds the missing
*static* layer, deliberately small.

## Why it exists, in one measured fact

Until now the three repos had **no Ruff configuration at all**. `ruff check .` therefore ran
whatever the installed version enables by default — Ruff 0.16 enables a very broad set
(`UP`/`RUF`/`PLR`/`S`/`TRY`/`PERF`/`FURB`/`YTT`/`ASYNC`…) — so the same tree produced **1038 /
438 / 117** findings in `orisys-app` / `orisys-sdk` / `orisys-sensor-design`, and a different
Ruff version produced different ones. A gate built on that is not a gate. `ruff.toml` now pins it.

## What is gated, and what is deliberately not

| rule | meaning | gated? | why |
|---|---|---|---|
| `BLE001` | broad `except Exception` | **no** | A broad catch is the *right* choice around device I/O, GL teardown and camera probing. This codebase already records that choice inline as `# noqa: BLE001 - <reason>`. Gating it would produce ~100 findings whose only correct fix is "add a reasoned suppression". |
| `S110`, `S112` | handler is only `pass` / `continue` | **yes** | The complaint is not the breadth of the catch, it is that the failure becomes **invisible**. Silence is acceptable only with a reasoned suppression or a handler that reports/re-raises. |
| `E722` | bare `except:` | **yes** | Strictly worse than `except Exception:` — it also swallows `KeyboardInterrupt`/`SystemExit`. |
| `F` | unused import, redefinition, undefined name, unused variable | **yes** | Real defects: `F821`/`F822` are latent `NameError`s, `F811` can be a shadowed definition. |
| `B023` | closure over a loop variable | **yes** | Late binding silently produces wrong geometry in the headless Blender scripts. |
| `PLW1510` | `subprocess.run` without `check` | **yes** | A swallowed tool failure. |
| `RUF100` | unused suppression | reported, **never auto-fixed** | The `- reason` in a `noqa` comment is documentation. A machine must not delete it. Triage by hand. |
| `UP`, `I`, `PIE`, `RET501`, `PLR1711`, `SIM114`, `RUF022`, `RUF023` | mechanical modernisation | in the lint config, **not** in the gate | Applied once by `--fix`; gating them would only churn AI-authored diffs. |

`E402` (module import not at top of file) is **selected but not gated**. `import` after
`os.environ[...]` is deliberate in this codebase — the environment must be set before Qt/cv2/the SDK
load — so gating it would be pure noise. But it is the only rule that would catch an *accidental*
import moved above that setup, so it stays visible. When you touch one of those files, record the
intent as `# noqa: E402 - <reason>`; `E701`/`E702` are style, which `ruff format` owns. A gate that
cries wolf gets ignored, which is worse than no gate.

## The three commands

```powershell
# 1. The agent loop, after every edit — mechanical, no review needed.
ruff check <changed files> --fix            # never --unsafe-fixes
ruff check <changed files> --output-format json   # what remains, machine-readable

# 2. The gate before a change lands. Changed files only, machine-readable, non-zero on findings.
python tools/lint_gate.py                   # vs HEAD
python tools/lint_gate.py --base origin/main
python tools/lint_gate.py --all             # gate rules over the whole repo
python tools/lint_gate.py --json

# 3. Formatting, changed files only — never a whole-tree rewrite mid-diff.
ruff format <changed files>
```

`$RUFF` may point at a specific ruff binary; otherwise `tools/lint_gate.py` uses `PATH` or the one
beside the interpreter. **Pin the version**: the gate prints the version it ran, and findings differ
between versions, so a runner must install the same one the developers use (currently 0.16.9).

## What this found when it was introduced

Filtering to rules that can indicate a defect produced ~18 candidates rather than style noise —
which is the argument for the narrow set:

- six latent `NameError`s (`np`, `sys`, `Callable`, `FrameIdentity`, `tessellate_polygon`, and an
  `__all__` entry naming a function that does not exist) — none caught by any test;
- a **duplicated** function definition that silently shadowed the first;
- `lower == lower` / `upper == upper` in a range check, and the same pattern inside a **test
  assertion that could never fail**;
- 13 closures over loop variables in the headless Blender/patch scripts;
- two `subprocess.run` calls whose failure was silently ignored.

## Follow-up deliberately not done yet

`PLR0124` ("name compared with itself") is not in `select`. Eleven of its sites are the deliberate
`value != value` NaN idiom and must become `math.isnan(...)` (or carry a reasoned suppression)
before the rule can be gated — otherwise it is noise. Once those are converted, add `PLR0124` to
`select` and to `GATE_SELECT` in `tools/lint_gate.py`.