Skip to content

Commit 81f47a5

Browse files
vsits-proxy-builder[bot]vsits-proxy-builder[bot]claude
authored
docs: anchor the anti-bloat lens for PRs that have no directive (cnighswonger#287)
The global reviewer bar is "larger than the directive's requirements justify." Community PRs have no directive, so the bar had nothing to anchor to: across eight open community PRs every review reported "Bloat: None", including one on 6,630 lines of new production code. AGENTS.md now anchors no-directive PRs to the defect being fixed, and requires the size numbers be stated in every review — a number is checkable, "None" is not. Calibrated against cnighswonger#274/cnighswonger#277/cnighswonger#261, all merged and all proportionate, so the reference is real work rather than a guess. Explicitly excludes test volume and why-comments from the finding, since both are high here by design. CONTRIBUTING.md is new: AGENTS.md and CLAUDE.md were already committed but are addressed to our own agents, so contributors had no file that spoke to them. Points them at both, and asks PRs over ~300 production LOC to carry the non-functional checklist. Claude-Session: https://claude.ai/code/session_01GvZKP1JeXgHFCovTaAPT5B Co-authored-by: vsits-proxy-builder[bot] <223447982+vsits-proxy-builder[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8b25dc9 commit 81f47a5

2 files changed

Lines changed: 151 additions & 0 deletions

File tree

AGENTS.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,74 @@ transform pipeline sitting in front of `api.anthropic.com`.
101101
unit / integration tests alone can't prove behavior under real
102102
traffic.
103103

104+
## Anti-Bloat Lens (no-directive PRs)
105+
106+
The global baseline's bloat bar is *"larger than the directive's
107+
requirements justify."* **Community PRs have no directive**, so that bar
108+
has nothing to anchor to and the finding defaults to "None" — including
109+
on a PR adding thousands of lines. Measured 2026-07-31 across eight open
110+
community PRs: every review reported `Bloat: None`, one of them on 6,630
111+
lines of new production code.
112+
113+
For a PR with no directive, anchor to **the defect it claims to fix**
114+
instead. Ask: *what is the smallest change that fixes the stated
115+
problem, and how much larger is this?*
116+
117+
Report these numbers in the review, always, even when the verdict is
118+
"proportionate" — a stated number is checkable, "None" is not:
119+
120+
- **production LOC** (excluding `test/`), and **test:production ratio**
121+
- **new files, new exports, new env vars, new on-disk paths**
122+
- **comment:code ratio** on the production diff
123+
124+
Calibration from merged work in this repo. These are all *proportionate*
125+
— use them as the reference for what normal looks like:
126+
127+
| PR | prod LOC | test:prod | shape |
128+
|---|---|---|---|
129+
| #274 header propagation | 7 code + 19 comment | 23x | a wire-path defect fix |
130+
| #277 supervised stop | 9 code + 10 comment | 9.0x | one branch + a watchdog |
131+
| #261 absolute-form | 23 code + 31 comment | 8.4x | one parser + 3 call sites |
132+
133+
Two things that table shows, both deliberate:
134+
135+
- **A high test:prod ratio is a good sign, not bloat.** Do not flag it.
136+
- **Comment lines exceeding code lines is normal here** and is not
137+
bloat when the comment explains *why* — every one of the above
138+
records a defect's mechanism or a non-obvious constraint. Flag
139+
comments that restate the code, not comments that carry history.
140+
141+
Raise a **blocking** finding when any of these hold:
142+
143+
- Production LOC is **an order of magnitude** beyond what the stated
144+
defect requires, and the excess is not itself explained in the PR.
145+
- New **abstraction** with fewer than ~3 call sites and no concrete
146+
near-term reuse case — default is to inline.
147+
- **Dead code**: an export, branch, or option with no call site. Verify
148+
by grepping `origin/main` *after* the merge base, not a local branch —
149+
a stale checkout produces false "unused" findings.
150+
- **Defensive handling for cases that cannot occur** given the
151+
surrounding code's invariants.
152+
- New **env var / on-disk path / config key** that is not required by
153+
the fix. Each one is permanent surface area.
154+
155+
Do **not** flag: test volume, fixtures, comment density explaining
156+
mechanism, or complexity you cannot show is safe to remove. Per the
157+
global rule, never assert a simplification is behavior-preserving when
158+
you have not verified it — say you could not verify instead.
159+
160+
### Missing Non-Functional Requirements
161+
162+
A community PR over roughly **300 production LOC** should carry a
163+
`## Non-Functional Requirements` section (same checklist as a
164+
directive), including the required **Load-bearing?** yes/no. If it is
165+
absent, say so as an attention item and ask for it — do not block on it
166+
alone, and do not treat its absence as licence to skip the size
167+
question. Independently assess load-bearing status yourself: an author's
168+
"no" on something touching a wire contract, shared abstraction, or
169+
security-relevant path is a blocking finding, because that
170+
classification decides whether a human reviews before merge.
171+
104172
## Repo-Specific Labels
105173

106174
In addition to the global review-outcome labels (`reviewed-by-codex-agent`,

CONTRIBUTING.md

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# Contributing
2+
3+
Thanks for working on this. A proxy that sits in front of everyone's API
4+
traffic has an unusually low tolerance for surprises, so a few things
5+
here are stricter than a typical repo.
6+
7+
## Using an AI agent? Read this part.
8+
9+
Most contributions here — ours included — are written with AI assistance.
10+
That is fine and welcome. But agents reliably satisfy the *functional*
11+
requirement and neglect the non-functional ones: size, complexity, new
12+
surface area. A change that works and is four times larger than it needs
13+
to be still costs every future reader.
14+
15+
If you point an agent at this repo, point it at these files too:
16+
17+
- **`AGENTS.md`** — what our reviewer checks, including the anti-bloat
18+
lens and how we size a change against the defect it fixes.
19+
- **`CLAUDE.md`** — workflow, labels, and the non-functional checklist.
20+
21+
Both are committed at the repo root. They are written for our own agents,
22+
so some sections are internal (bot identities, label ownership) — ignore
23+
those. The review standards apply to every PR regardless of who or what
24+
wrote it.
25+
26+
Two habits that make an agent-written PR much easier to accept:
27+
28+
- **Have it justify the size.** "What is the smallest change that fixes
29+
this?" is a question worth asking before you open the PR, because it is
30+
the first one we ask.
31+
- **Have it check its own claims.** If the PR body says a function is
32+
only called in two places, or a value is bounded, verify it. We
33+
cite-check load-bearing claims against the source, and a confident
34+
wrong claim costs a review round.
35+
36+
## What we check
37+
38+
- **Does it fix what it says it fixes?** Red-first evidence is
39+
persuasive: show the test failing on the merge base and passing at your
40+
head. Several recent PRs did this and it shortened review considerably.
41+
- **Is it proportionate?** See the anti-bloat lens in `AGENTS.md`. A high
42+
test-to-code ratio is a *good* sign — we do not count tests against
43+
you. Comments that explain *why* are likewise welcome.
44+
- **Is it load-bearing?** Anything touching a wire contract, a shared
45+
abstraction, TLS trust, credentials, or the on-disk format needs human
46+
review before merge, not just agent review. Say so in the PR if you
47+
think it applies.
48+
- **Does the test cover the changed path?** Not an adjacent one.
49+
50+
## PRs over ~300 lines of production code
51+
52+
Include a `## Non-Functional Requirements` section in the PR body. Short
53+
answers are fine — a line or two each, `n/a` where it genuinely does not
54+
apply:
55+
56+
- **Size/complexity budget** — roughly how big should this be, and is it?
57+
- **Threat model** — inputs, trust boundaries, what must not leak. This
58+
proxy handles API keys and full request/response bodies; be specific.
59+
- **Maintainability** — new abstractions need a reason (≈3+ call sites or
60+
concrete near-term reuse). Otherwise inline it.
61+
- **Performance/reliability** — only where it applies.
62+
- **Load-bearing?** — required yes/no.
63+
64+
## Practical notes
65+
66+
- **Fork PRs do not get CI.** GitHub gates workflow runs on forks behind
67+
maintainer approval, and our review bot cannot clear that gate. We run
68+
the full suite locally at the merge commit instead and report the
69+
count on the PR. Nothing is required from you — just know that a
70+
missing green check is not a problem with your PR.
71+
- **Rebase rather than merge** when your branch goes stale. We merge one
72+
PR at a time and test between merges, so a clean fast-forward matters.
73+
- **We will not push to your branch.** If something needs changing we
74+
will ask. (We got this wrong once and reverted it — your branch is
75+
yours.)
76+
- **Draft means draft.** We will not flip a PR out of draft for you.
77+
78+
## Reporting a bug you have not fixed
79+
80+
An issue with a reproduction is worth as much as a PR. Wire captures,
81+
`--output-format json` output, and "works with the proxy bypassed"
82+
comparisons are all especially useful — several of the subtlest bugs
83+
here were diagnosed from exactly that.

0 commit comments

Comments
 (0)