Decompose .build_suggestions() (106 lines) into named helpers #33

New Issue

2026-08-05T08:22:43-04:00

jared commented

2026-08-05 08:22:43 -04:00

Split out of the uscogdata#9 work (PR #32). Owner ruled at review time: accept the length as-is there, track the cleanup here.

.build_suggestions() in R/suggestions.R is 106 lines against the project's "functions under 50 lines" convention. It was already 79 lines before #9 — this is not a regression introduced by that branch, it is a pre-existing violation that #9 made more visible.

The reviewer's read: the function is linear (max ~3 levels of nesting), well-commented per phase, and fully covered by tests, so it is not fragile today. But it mixes four distinct responsibilities:

gap-year computation
the row-absence covered query
the recipe metadata lookup
suggestion-list construction and the two-path merge

Suggested extraction: .query_covered_years(), .query_recipe_meta(), and a merge helper — leaving the orchestrator under 50 lines and making each piece independently testable.

Note .suppressed_components() already moved to R/suppression.R during #9 (for the 400-line file limit), so there is precedent for splitting this cluster further.

Split out of the uscogdata#9 work (PR #32). Owner ruled at review time: accept the length as-is there, track the cleanup here. `.build_suggestions()` in `R/suggestions.R` is 106 lines against the project's "functions under 50 lines" convention. It was already 79 lines before #9 — this is not a regression introduced by that branch, it is a pre-existing violation that #9 made more visible. The reviewer's read: the function is linear (max ~3 levels of nesting), well-commented per phase, and fully covered by tests, so it is not fragile today. But it mixes four distinct responsibilities: 1. gap-year computation 2. the row-absence `covered` query 3. the recipe metadata lookup 4. suggestion-list construction and the two-path merge Suggested extraction: `.query_covered_years()`, `.query_recipe_meta()`, and a merge helper — leaving the orchestrator under 50 lines and making each piece independently testable. Note `.suppressed_components()` already moved to `R/suppression.R` during #9 (for the 400-line file limit), so there is precedent for splitting this cluster further.

jared referenced this issue

2026-08-09 09:18:47 -04:00

Performance pass over the reader before final release #56

jared referenced this issue

2026-08-10 19:17:11 -04:00

Performance pass over the reader before final release #56

jared added the