Skip to content

[RED-01] Electrical widget family: copy-pasted implementation across 6 widgets (~5,000 LOC) #4

Description

@mairas

Severity: high | Confidence: confirmed | Area: dead-code | Source: codebase review 2026-06-30, finding RED-01

Problem

The six electrical widgets (widget-bms, widget-solar-charger, widget-charger, widget-alternator, widget-inverter, widget-ac) share a well-designed data model layer — common interfaces in widgets-interface.ts, the IElectricalTopologySnapshotCore contract, electrical-card-layout.constants, and the getElectricalWidgetFamilyDescriptor factory — but the behavioral layer was never extracted. Private helpers including normalizeOptionalString (duplicated in all 6 widgets), normalizeTrackedDevices (5 widgets), buildIdToDeviceKeysMap (4 widgets), and escapeRegex (verbatim in 3 widgets) are redefined independently in each component. Independent normalized-line analysis of the four "direct card" widgets (charger, alternator, inverter, ac) finds roughly 159–167 lines identical across all four and pairwise Jaccard similarity of 21–37%, covering d3 card rendering, device-tracking maps, unit conversion, and snapshot assembly. This is an incomplete refactor: the model layer was extracted, but the rendering and tracking layer was not.

Impact

Approximately 800–1,200 lines of duplicated behavior must be maintained in lockstep across six files. Any bug fix or behavioral change to shared helpers must be applied manually to each widget; divergence over time is likely. For a SKip deployment where marine electrical monitoring widgets (BMS, solar charger, alternator, inverter) are core UI components, undetected divergence between widgets could produce inconsistent behavior across the electrical dashboard. The duplication also increases the cost of adding new electrical widget types.

Affected code

  • src/app/widgets/widget-bms/widget-bms.component.ts
  • src/app/widgets/widget-solar-charger/widget-solar-charger.component.ts
  • src/app/widgets/widget-charger/widget-charger.component.ts:23
  • src/app/widgets/widget-alternator/widget-alternator.component.ts:24
  • src/app/widgets/widget-inverter/widget-inverter.component.ts:24
  • src/app/widgets/widget-ac/widget-ac.component.ts

Suggested direction

Extract a shared AbstractElectricalCardWidget base class or a rendering helper service that owns normalizeOptionalString, normalizeTrackedDevices, buildIdToDeviceKeysMap, escapeRegex, the device-tracking map management, and the common d3 direct-card draw routine. Each concrete widget would then supply only its snapshot-to-model mapping and card-specific metrics. The finding notes this is best handled as an upstream Kip PR that benefits the broader project rather than as a fork-local change.

Rebase note

This touches high-churn upstream files; the upstream Kip CHANGELOG shows widget-bms, widget-solar-charger, and widget-charger were all shipped recently. Refactoring these files within the SKip fork is fine for experimentation but will produce head-on conflicts when rebasing onto new upstream Kip releases. If pursued in the fork, track conflicts carefully; the preferred path is an upstream contribution.

Verification

Verified against the codebase on branch profiles (confidence: confirmed); all structural and quantitative claims independently cross-checked.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:dead-codeRedundancy, dead code & baggageenhancementNew feature or requestkip-reviewFrom the 2026-06-30 Kip/SKip codebase reviewrebase-riskHigh-churn upstream file; refactoring in fork risks chronic rebase conflictsseverity:highReview finding: high severity

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions