← All decisions

#0039 · 2026-08-09 · accepted · architecture

Formal modularization thresholds (ARCHITECTURE.md), grounded in SRP/cohesion research not line-count alone -- and one real extraction the audit found: pe/collision_sweep.gleam

architecturemodularizationsrpdocumentationrefactor

Context

User feedback: large files (~1000+ lines) are a code smell, but a hard line-count cap alone is insufficient and shouldn't be the primary tool -- asked for formal, researched modularization thresholds grounded in real software-engineering principles (SRP, cognitive load, cohesion, coupling), tailored to this project's actual architecture and language (Gleam, functional core/imperative shell), documented in the repo, then the current codebase audited against them. Measured the real current distribution first rather than guessing: pe/* domain modules run 72-177 lines; pe.gleam (413) and game.gleam (291) are the two 'composer' files; the largest file in the whole repo by far is test/glemy/pe_test.gleam at 1615 lines (mirroring pe.gleam per the project's own 'testing mirrors source' rule).

Options considered

A hard line-count cap (e.g. 'no file over 1000 lines') as the primary, enforced rule — rejected
Researched directly rather than assumed: Credo, the standard static analyzer for Elixir (the closest sibling ecosystem to Gleam -- same BEAM VM, same module-as-function-namespace shape), has no default check for total file length at all -- only function-level signals (CyclomaticComplexity, default max 9; FunctionArity, default max 8 params). This matches the Single Responsibility Principle literature directly: SRP counts responsibilities, not lines. A line-count-first rule would have missed the actual finding of this audit (a 413-line pe.gleam secretly housing a fully self-contained, zero-Model-dependency collision-sweep algorithm) while potentially flagging an appropriately-scoped, single-responsibility file for no real reason.
Adopt generic OOP-style class-size guidance (e.g. a fixed 'max 300 lines per class' rule from an unrelated ecosystem) without adapting it — rejected
Gleam has no classes; a module is a plain namespace of functions and types, and this project's own established convention (already in ARCHITECTURE.md, Gleam's own docs) draws module boundaries around business-domain nouns, not generic layering vocabulary. Porting an unrelated language's class-size number wholesale would not reflect either Gleam's actual unit of modularity or this project's specific Core/Shell split.
Responsibility-count and coupling-shape as the primary signals (does a group of functions share a primary type the file isn't named after; how many sibling pe/* modules does a domain module import), with line count kept only as a soft, file-category-specific trip-wire prompting a manual SRP check -- documented in a new ARCHITECTURE.md section, then applied to a real audit of the current codebase — chosen
Grounded in real, cited research (SRP literature, Credo's actual BEAM-ecosystem defaults, SonarQube's cognitive-complexity default of 15) rather than an arbitrary number, and tailored to this project's real, measured file-size distribution rather than an imported convention. Directly found one genuine violation this audit is not hypothetical about: pe.gleam's private resolve_all_collisions/resolve_target_against_rest/ResolveOutcome operated purely on List(Entity), never touching pe.gleam's own Model type -- the concrete tell that they were never actually coupled to pe.gleam's stated responsibility, just co-located with it.

Decision

Added a 'Modularization thresholds' section to ARCHITECTURE.md: the why-not-line-count-alone research, then five prioritized signals (responsibility count; coupling shape/sibling-import count; shared-primary-type mismatch; line count as a soft per-category trip-wire only -- ~250 lines for a pe/* domain module, ~400 for an orchestrator like pe.gleam/game.gleam, no separate cap for test files since they mirror source exactly; function-level branching complexity, adapted from Credo/SonarQube's ~9-15 range). Applied it immediately, not left as a document nobody acts on: extracted the collision-and-merge sweep out of pe.gleam into a new src/glemy/pe/collision_sweep.gleam (99 lines), matching the existing pe/vector2, pe/entity, pe/bounds, pe/collision, pe/tier pattern exactly. pe.gleam dropped from 413 to 334 lines as a direct consequence of removing what was never really its own responsibility, not as a goal in itself. test/glemy/pe/collision_sweep_test.gleam was created to mirror it (the project's own standing rule), moving five merge/collision-sweep-specific tests out of pe_test.gleam and rewriting them to call collision_sweep.resolve_all_collisions directly -- a real testability improvement the extraction enabled, since that function was previously private and only reachable indirectly through pe.update's own tests. Two new base-case unit tests (empty list, single entity) were added at the same time, now possible for the same reason.

Verification

164 Erlang / 187 JavaScript gleam test cases passing on both targets (net +2 over the pre-refactor 162/185: 7 new/moved collision_sweep tests minus 5 removed from pe_test.gleam). tools/check_warnings.ts (decision 0038) still passes against the existing 49-warning baseline unchanged -- confirming the new pure, dual-target collision_sweep.gleam/collision_sweep_test.gleam introduced zero new warnings, exactly as a properly-scoped Core module should. tools/browser_check.ts re-run against a real headless-Chromium session after the refactor: still passes cleanly with zero page errors, confirming the extraction didn't change any observable game behavior. Audited every other file in the codebase against the new thresholds (pe/* modules 72-177 lines, well under the ~250 trip-wire; game.gleam 291 and pe.gleam now 334, both under the ~400 orchestrator trip-wire; render.gleam 163) and found no other genuine SRP violation -- pe_test.gleam remains large (1384 lines after this change) but was confirmed, not assumed, to be large because it thoroughly tests pe.gleam's now-appropriately-scoped single responsibility (Model state and its tick/update composition), not because of a hidden second violation; forcing an artificial secondary split there would contradict the very 'responsibility count, not lines' principle this decision is grounded in.

Consequences

ARCHITECTURE.md's 'Modularization thresholds' section is now the standing reference for any future 'should this be its own module' question, with concrete, cited numbers rather than only the prior qualitative guidance. The collision-sweep algorithm is now independently unit-tested for the first time, closing a real testability gap the extraction surfaced rather than just moving code around. Future growth of pe.gleam or game.gleam past their documented trip-wires should prompt the same responsibility-count analysis applied here, not an automatic split.

References