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.
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.mdsets a 500-line ceiling per file. Measured onorigin/mainat0d7a45885,internal/purchase/alone has nine files over it:internal/purchase/execution_test.gointernal/purchase/coverage_extra_test.gointernal/purchase/manager_test.gointernal/purchase/execution.gointernal/purchase/approvals_test.gointernal/purchase/manager.gointernal/purchase/money_path_regression_test.gointernal/purchase/approvals.gointernal/purchase/notifications_test.goThe 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.goat 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.goandapprovals_test.goare 5665 lines between them and carry no production risk. Splittingexecution.goandmanager.gois 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.