Skip to content

refactor(purchase): nine files in internal/purchase exceed the 500-line ceiling, up to 1826 lines #209

Description

@cristim

Raised by CodeRabbit on LeanerCloud/cloud-commitments-cli#1862 as a Major finding. Declined there on scope grounds: LeanerCloud/cloud-commitments-cli#1862 is a store rename plus a data migration on the money path, and a structural refactor of four files riding along with it is the blast radius the project's scope rules exist to prevent. Filing it here so the debt is tracked rather than left in a resolved review thread.

What

CLAUDE.md sets a 500-line ceiling per file. Measured on origin/main at 0d7a45885, internal/purchase/ alone has nine files over it:

file lines
internal/purchase/execution_test.go 1826
internal/purchase/coverage_extra_test.go 1476
internal/purchase/manager_test.go 1262
internal/purchase/execution.go 1261
internal/purchase/approvals_test.go 1101
internal/purchase/manager.go 704
internal/purchase/money_path_regression_test.go 608
internal/purchase/approvals.go 524
internal/purchase/notifications_test.go 524

The four CodeRabbit named are a subset. This is a package-level condition, not a few stray files, which is part of why it should not be fixed incidentally inside an unrelated PR.

Why it matters beyond the rule

execution.go at 1261 lines is where the ramp-advance, per-account fan-out and retry-successor logic all live, and LeanerCloud/cloud-commitments-cli#1669, LeanerCloud/cloud-commitments-cli#1537 and LeanerCloud/cloud-commitments-cli#1861 are all defects in that interaction. Three separate review rounds on LeanerCloud/cloud-commitments-cli#1862 found defects in the fix rather than the original bug, and the reviewer's recurring difficulty was establishing which of several overlapping code paths a given row reaches. File size is not the cause, but it raises the cost of every future review of the most defect-dense area in the codebase.

Scope note

Splitting test files is the cheaper half and can land independently: execution_test.go, coverage_extra_test.go, manager_test.go and approvals_test.go are 5665 lines between them and carry no production risk. Splitting execution.go and manager.go is the part that needs care, since the money path runs through both.

Suggested order: test files first, in separate PRs per file, then reassess whether the production split is still worth doing.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions