Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,74 @@ transform pipeline sitting in front of `api.anthropic.com`.
unit / integration tests alone can't prove behavior under real
traffic.

## Anti-Bloat Lens (no-directive PRs)

The global baseline's bloat bar is *"larger than the directive's
requirements justify."* **Community PRs have no directive**, so that bar
has nothing to anchor to and the finding defaults to "None" — including
on a PR adding thousands of lines. Measured 2026-07-31 across eight open
community PRs: every review reported `Bloat: None`, one of them on 6,630
lines of new production code.

For a PR with no directive, anchor to **the defect it claims to fix**
instead. Ask: *what is the smallest change that fixes the stated
problem, and how much larger is this?*

Report these numbers in the review, always, even when the verdict is
"proportionate" — a stated number is checkable, "None" is not:

- **production LOC** (excluding `test/`), and **test:production ratio**
- **new files, new exports, new env vars, new on-disk paths**
- **comment:code ratio** on the production diff

Calibration from merged work in this repo. These are all *proportionate*
— use them as the reference for what normal looks like:

| PR | prod LOC | test:prod | shape |
|---|---|---|---|
| #274 header propagation | 7 code + 19 comment | 23x | a wire-path defect fix |
| #277 supervised stop | 9 code + 10 comment | 9.0x | one branch + a watchdog |
| #261 absolute-form | 23 code + 31 comment | 8.4x | one parser + 3 call sites |

Two things that table shows, both deliberate:

- **A high test:prod ratio is a good sign, not bloat.** Do not flag it.
- **Comment lines exceeding code lines is normal here** and is not
bloat when the comment explains *why* — every one of the above
records a defect's mechanism or a non-obvious constraint. Flag
comments that restate the code, not comments that carry history.

Raise a **blocking** finding when any of these hold:

- Production LOC is **an order of magnitude** beyond what the stated
defect requires, and the excess is not itself explained in the PR.
- New **abstraction** with fewer than ~3 call sites and no concrete
near-term reuse case — default is to inline.
- **Dead code**: an export, branch, or option with no call site. Verify
by grepping `origin/main` *after* the merge base, not a local branch —
a stale checkout produces false "unused" findings.
- **Defensive handling for cases that cannot occur** given the
surrounding code's invariants.
- New **env var / on-disk path / config key** that is not required by
the fix. Each one is permanent surface area.

Do **not** flag: test volume, fixtures, comment density explaining
mechanism, or complexity you cannot show is safe to remove. Per the
global rule, never assert a simplification is behavior-preserving when
you have not verified it — say you could not verify instead.

### Missing Non-Functional Requirements

A community PR over roughly **300 production LOC** should carry a
`## Non-Functional Requirements` section (same checklist as a
directive), including the required **Load-bearing?** yes/no. If it is
absent, say so as an attention item and ask for it — do not block on it
alone, and do not treat its absence as licence to skip the size
question. Independently assess load-bearing status yourself: an author's
"no" on something touching a wire contract, shared abstraction, or
security-relevant path is a blocking finding, because that
classification decides whether a human reviews before merge.

## Repo-Specific Labels

In addition to the global review-outcome labels (`reviewed-by-codex-agent`,
Expand Down
83 changes: 83 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Contributing

Thanks for working on this. A proxy that sits in front of everyone's API
traffic has an unusually low tolerance for surprises, so a few things
here are stricter than a typical repo.

## Using an AI agent? Read this part.

Most contributions here — ours included — are written with AI assistance.
That is fine and welcome. But agents reliably satisfy the *functional*
requirement and neglect the non-functional ones: size, complexity, new
surface area. A change that works and is four times larger than it needs
to be still costs every future reader.

If you point an agent at this repo, point it at these files too:

- **`AGENTS.md`** — what our reviewer checks, including the anti-bloat
lens and how we size a change against the defect it fixes.
- **`CLAUDE.md`** — workflow, labels, and the non-functional checklist.

Both are committed at the repo root. They are written for our own agents,
so some sections are internal (bot identities, label ownership) — ignore
those. The review standards apply to every PR regardless of who or what
wrote it.

Two habits that make an agent-written PR much easier to accept:

- **Have it justify the size.** "What is the smallest change that fixes
this?" is a question worth asking before you open the PR, because it is
the first one we ask.
- **Have it check its own claims.** If the PR body says a function is
only called in two places, or a value is bounded, verify it. We
cite-check load-bearing claims against the source, and a confident
wrong claim costs a review round.

## What we check

- **Does it fix what it says it fixes?** Red-first evidence is
persuasive: show the test failing on the merge base and passing at your
head. Several recent PRs did this and it shortened review considerably.
- **Is it proportionate?** See the anti-bloat lens in `AGENTS.md`. A high
test-to-code ratio is a *good* sign — we do not count tests against
you. Comments that explain *why* are likewise welcome.
- **Is it load-bearing?** Anything touching a wire contract, a shared
abstraction, TLS trust, credentials, or the on-disk format needs human
review before merge, not just agent review. Say so in the PR if you
think it applies.
- **Does the test cover the changed path?** Not an adjacent one.

## PRs over ~300 lines of production code

Include a `## Non-Functional Requirements` section in the PR body. Short
answers are fine — a line or two each, `n/a` where it genuinely does not
apply:

- **Size/complexity budget** — roughly how big should this be, and is it?
- **Threat model** — inputs, trust boundaries, what must not leak. This
proxy handles API keys and full request/response bodies; be specific.
- **Maintainability** — new abstractions need a reason (≈3+ call sites or
concrete near-term reuse). Otherwise inline it.
- **Performance/reliability** — only where it applies.
- **Load-bearing?** — required yes/no.

## Practical notes

- **Fork PRs do not get CI.** GitHub gates workflow runs on forks behind
maintainer approval, and our review bot cannot clear that gate. We run
the full suite locally at the merge commit instead and report the
count on the PR. Nothing is required from you — just know that a
missing green check is not a problem with your PR.
- **Rebase rather than merge** when your branch goes stale. We merge one
PR at a time and test between merges, so a clean fast-forward matters.
- **We will not push to your branch.** If something needs changing we
will ask. (We got this wrong once and reverted it — your branch is
yours.)
- **Draft means draft.** We will not flip a PR out of draft for you.

## Reporting a bug you have not fixed

An issue with a reproduction is worth as much as a PR. Wire captures,
`--output-format json` output, and "works with the proxy bypassed"
comparisons are all especially useful — several of the subtlest bugs
here were diagnosed from exactly that.
Loading