Skip to content

refactor(frontend): fourteen non-test files in frontend/src exceed the 500-line ceiling, up to 5481 lines #210

Description

@cristim

Raised by CodeRabbit on LeanerCloud/cloud-commitments-cli#1864 as a Major finding against frontend/src/inventory.ts. Declined there on scope grounds: LeanerCloud/cloud-commitments-cli#1864 is a four-line CSS fix for an empty table column, and a structural extraction 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. Same shape as LeanerCloud/cloud-commitments-cli#1863, which tracks the equivalent condition in internal/purchase/.

What

CLAUDE.md sets a 500-line ceiling per file. Measured on origin/main at 7214c6b22, fourteen non-test TypeScript files under frontend/src/ are over it:

file lines
frontend/src/recommendations.ts 5481
frontend/src/settings.ts 3715
frontend/src/plans.ts 2359
frontend/src/riexchange.ts 2270
frontend/src/history.ts 1847
frontend/src/auth.ts 1536
frontend/src/dashboard.ts 1182
frontend/src/api/types.ts 932
frontend/src/app.ts 802
frontend/src/inventory.ts 634
frontend/src/modules/savings-history.ts 600
frontend/src/groups/groupModals.ts 539
frontend/src/state.ts 530
frontend/src/archera.ts 523

Filter: frontend/src/**/*.ts, excluding __tests__/, *.test.ts and *.d.ts. Reproduce with:

git ls-tree -r --name-only origin/main frontend/src | grep '\.ts$' \
  | grep -v '__tests__/' | grep -v '\.test\.ts$' | grep -v '\.d\.ts$' \
  | while read -r f; do n=$(git show "origin/main:$f" | wc -l | tr -d ' '); \
      [ "$n" -gt 500 ] && printf '%6d  %s\n' "$n" "$f"; done | sort -rn

The guideline as CodeRabbit cites it (**/*.{go,ts,tsx}) also covers tests, and there 26 more files are over the ceiling, 36840 lines between them, led by __tests__/recommendations.test.ts at 8222.

inventory.ts is tenth of fourteen, at 634 lines. It is one instance of a package-wide condition, not a stray file.

Why it matters beyond the rule

recommendations.ts at 5481 lines is a single module holding the Opportunities table, its column filters, the selection model and the bottom action box that starts a purchase. That is the highest-traffic review surface in the frontend and the one where a reviewer most often has to establish which of several overlapping paths a given row takes. File size is not the cause of that difficulty, but it raises the cost of every future review of the code closest to the money path.

Scope note

The cheap half and the risky half are far apart here, and they should not be done in one pass.

Cheap, no production risk:

  • The 26 oversized test files. Splitting a jest file is moving describe blocks into siblings; nothing downstream imports them.
  • api/types.ts (932). Pure interface declarations with no runtime behaviour. Splitting it by domain changes import paths and nothing else.

Middle, one extraction each:

  • inventory.ts (634), modules/savings-history.ts (600), groups/groupModals.ts (539), state.ts (530), archera.ts (523). Each is a few hundred lines over and has an obvious seam. inventory.ts splits cleanly into its three sub-tabs.

Risky, needs care:

  • recommendations.ts (5481), settings.ts (3715), plans.ts (2359), riexchange.ts (2270), history.ts (1847), auth.ts (1536), dashboard.ts (1182). The purchase, authentication and RI-exchange surfaces run through these, and state.ts subscriptions cross most of them.

Suggested order

Test files first, in separate PRs per file, then api/types.ts, then reassess whether the production split is worth doing at all. Explicitly not the order a review queue produces: fixing whichever file a CodeRabbit finding happened to land on leaves the 5481-line module untouched while spending a PR on the 634-line one. That is the least useful order available, and it is the order this will default to if the condition is only ever addressed reactively.

Triage

Matching LeanerCloud/cloud-commitments-cli#1863's triage (p3 / severity-low / impact-internal / urgency-eventually / type-chore) because this is the same kind of debt with the same absence of user-visible impact. Effort is effort/xl rather than LeanerCloud/cloud-commitments-cli#1863's effort/l: forty files here against nine there, and the risky half includes the purchase and auth surfaces.

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