refactor(config): extract configMapper, unify build paths, DRY gadget escalation#539
Conversation
|
🤖 On it — checking the refactor/config-repository-and-gadget-utils branch Progress: [███░░░░░░░] 29% (iteration 20/70) 🔍 Code Review Update (1 min) I've completed reviewing the Last updated: iteration 20 · review |
nhopeatall
left a comment
There was a problem hiding this comment.
LGTM — clean extraction refactoring with no behavioral changes.
What I verified:
-
configMapper.ts extraction: All mapping functions, interfaces, and helpers moved faithfully. The key differences from the original are intentional improvements:
IntegrationRownow includesprojectId(needed bybuildRawConfig's indexing logic, previously accessed on the raw DB row before casting to the narrower type)mapProjectRowreturns typedProjectConfigRawinstead ofRecord<string, unknown>MapProjectInputreplaces 8 positional parameters with a structured object- Both
biome-ignoresuppressions inconfigRepository.tsare eliminated (one moves toconfigMapper.tsforGitHubIntegrationConfig, the other for cognitive complexity is no longer needed)
-
buildRawConfig unification: The duplicated extract→index→map pattern from
loadConfigFromDbandfindProjectConfigFromDbis correctly unified. The indexing-by-projectId logic that was previously inline inloadConfigFromDbis now insidebuildRawConfig, andfindProjectConfigFromDbcorrectly wraps its single-project data into the same structures (single-entry Map forprojectAgentConfigsMap, array forprojectRows). -
editEscalation.ts DRY: Byte-for-byte identical code extracted from both
FileSearchAndReplace.tsandFileMultiEdit.ts. Import ofrecordEditFailurecorrectly removed from both files'./shared/index.jsimports since it's now consumed internally byeditEscalation.ts. -
Type compatibility: The manually-defined
ProjectRowinconfigMapper.tsis a structural subset oftypeof projects.$inferSelect, so TypeScript correctly accepts the DB rows.DefaultsRowsimilarly works becauseBuildRawConfigOpts.defaultsRowis only used for mapping, while theorgIdaccess inloadConfigFromDbhappens on the full DB type before passing tobuildRawConfig. -
Tests: 40 new tests cover the extracted functions thoroughly with appropriate fixtures.
|
✅ Review complete — approved with no issues found. |
nhopeatall
left a comment
There was a problem hiding this comment.
LGTM — clean extraction refactoring with no behavioral changes. Verified type compatibility, faithful code movement, and proper test coverage.
Summary
Refactors two areas of the codebase identified as the top candidates for cleanup:
configRepository.ts→configMapper.tsextraction — Separates pure transformation logic from DB query concerns by moving all mapping functions, interfaces, and helpers into a new dedicated module. RefactorsmapProjectRowto use a structuredMapProjectInputobject instead of 8 positional parameters, adds a typedProjectConfigRawreturn type, and extractsbuildTrelloConfig/buildJiraConfig/buildAgentBackendConfigper-integration helpers. Removes bothbiome-ignoresuppressions.Unified
buildRawConfighelper — Eliminates the duplicated extract→split→map→validate pattern shared betweenloadConfigFromDbandfindProjectConfigFromDbby extracting abuildRawConfig(opts)function both callers use.DRY file-editing gadget utilities — Moves the identical
ESCALATION_HINTconstant andwithEscalationHintfunction fromFileSearchAndReplace.tsandFileMultiEdit.tsinto a new shared modulesrc/gadgets/shared/editEscalation.ts.New files
src/db/repositories/configMapper.ts— extracted mapper with typed interfacessrc/gadgets/shared/editEscalation.ts— shared escalation hint utilitytests/unit/db/repositories/configMapper.test.ts— 34 unit tests (Trello-only, JIRA-only, mixed, none integration combos)tests/unit/gadgets/shared/editEscalation.test.ts— 6 unit tests for the shared escalation utilityTest plan
npx tsc --noEmit)Card: https://trello.com/c/U4YWqGN9/111-find-top-candidate-for-refactoring-and-plan-clean-refactoring-of-it-look-for-god-classes-modules-functions-files-code-duplicatio
🤖 Generated with Claude Code