Skip to content

Raise the per-PR diff target from ~300 to ~1000 lines, and exclude generated files #191

Description

@thecodedrift

The change

CLAUDE.md currently says:

Prefer stacking, and aim to keep an individual diff under ~300 lines. A 900-line PR does not get reviewed, it gets approved. Tests count toward the total but never split from the code they cover — if a unit is oversized because of its tests, that is usually a sign the unit itself should be smaller.

Raise the target to ~1000 lines. 300 is too small for the shape of work this repository actually produces.

Why 300 does not hold here

Tests dominate, and the rule already forbids splitting them out. The guidance counts tests toward the total while requiring they ship with the code they cover. In practice a change with real coverage is majority test code, so the budget is spent on the part the rule will not let you separate. Recent examples:

PR total hand-written, excluding generated test share
#182 (ast-grep 0.45.2) 515 308 181 of 308 insertions
#190 (remote tier boundary) 1387 1387 roughly two thirds
#187 (dogfood house style) 308 most of the remainder is rules and fixtures

Generated and vendored files inflate the count with nothing to review. #182 was 515 lines, of which 346 was pnpm-lock.yaml and a regenerated JSON schema. Measured against the 300 target it looks like a 1.7x overrun; measured against what a human reads, it is comfortably inside. The rule does not say to discount them, so every judgement call starts with an argument about what counts.

Prose changes have no natural 300-line seam. Agent recipes (packages/cli/src/agent/*.txt), CLAUDE.md, and OpenSpec artifacts routinely move several hundred lines in one coherent edit. Splitting them produces PRs that are individually incoherent, which costs more review attention than it saves.

The observed failure mode was never size alone. The stack that motivated the original guidance had a real problem, but it was depth: #103 reached "ready for review" as a ~93-file change having never been linted, typechecked, or tested in CI, because a branches: filter silently stopped matching seven hops from main. That is a CI-coverage failure, and it is separately fixed. A 300-line target would not have caught it.

Why not remove the target entirely

The sentence it justifies is still true: a 900-line PR does not get reviewed, it gets approved. The number should be a real ceiling that a reviewer can hold, not one that is routinely overrun and therefore ignored. A target everyone exceeds teaches people the guidance is decorative, which is worse than a looser number everyone respects.

~1000 lines is roughly where a diff stops being readable in one sitting, and it is above the natural size of the units this repository produces, so exceeding it becomes a real signal again.

Suggested wording

Prefer stacking, and aim to keep an individual diff under ~1000 lines of hand-written change. Generated files (lockfiles, regenerated schemas, vendored artifacts) do not count toward the total — a reviewer does not read them. Tests do count, but never split from the code they cover; if a unit is oversized excluding generated files and its tests are proportionate, it is probably the right size. A diff well past this is not automatically wrong, but it should come with a reason.

Notes

Raised after landing #182, #187 and #190, each of which was a coherent single unit that the 300-line target would have called oversized. Nothing here changes the preference for stacking, the merge-down rules, or the changeset placement rules.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions