refactor(schedule): PR #177 second-pass P3 follow-ups (data clump, change types, and dead code) #214

New issue

Open

opened 2026-10-02 18:08:34 +00:00 by gabogg · 0 comments

gabogg commented

2026-10-02 18:08:34 +00:00

Owner

Follow-up for PR #177 (next-day settings activation, #114) addressing second-pass review findings P3-6, P3-7, P3-8, P3-9, and P3-11:

Scope

Data Clump (P3-6): _stage_pending_change_async returns a 6-tuple, unpacked 7 times across service callers. Introduce a typed StagedChange Pydantic model.
Repeated Switch / Shotgun Surgery (P3-7): Change types are duplicated across PendingChangeType, SQL CHECK, repository if/elif, and notice switch, while repository methods still take change_type: str.
Redundant hasattr (P3-8): hasattr(reset, "reset_for") on a value already typed ResetSchedule.
Speculative Generality (P3-9): The baseline rewrite in repository's update_config_async has no callers.
Merged Dict Settings (P3-11): pending_settings is still populated as a merged dict alongside explicit structured fields in notices.

Acceptance Criteria

_stage_pending_change_async returns a strongly typed StagedChange model instead of an unstructured 6-tuple.
Repository methods accept PendingChangeType enum/Literal consistently without fallback to untyped str.
Redundant hasattr calls on typed ResetSchedule instances are replaced with direct method invocations.
Dead code / unused baseline rewrite in occupancy_repository.update_config_async is removed.
Notice payloads cleanly separate domain-specific pending structures without ambiguous merged dictionary fallback.

Refs #177, #114.

Follow-up for PR #177 (next-day settings activation, #114) addressing second-pass review findings P3-6, P3-7, P3-8, P3-9, and P3-11: ### Scope 1. **Data Clump (P3-6)**: `_stage_pending_change_async` returns a 6-tuple, unpacked 7 times across service callers. Introduce a typed `StagedChange` Pydantic model. 2. **Repeated Switch / Shotgun Surgery (P3-7)**: Change types are duplicated across `PendingChangeType`, SQL `CHECK`, repository `if/elif`, and notice switch, while repository methods still take `change_type: str`. 3. **Redundant hasattr (P3-8)**: `hasattr(reset, "reset_for")` on a value already typed `ResetSchedule`. 4. **Speculative Generality (P3-9)**: The baseline rewrite in repository's `update_config_async` has no callers. 5. **Merged Dict Settings (P3-11)**: `pending_settings` is still populated as a merged dict alongside explicit structured fields in notices. ### Acceptance Criteria - `_stage_pending_change_async` returns a strongly typed `StagedChange` model instead of an unstructured 6-tuple. - Repository methods accept `PendingChangeType` enum/Literal consistently without fallback to untyped `str`. - Redundant `hasattr` calls on typed `ResetSchedule` instances are replaced with direct method invocations. - Dead code / unused baseline rewrite in `occupancy_repository.update_config_async` is removed. - Notice payloads cleanly separate domain-specific pending structures without ambiguous merged dictionary fallback. Refs #177, #114.

gabogg added this to the Historical passenger-flow backfill milestone

2026-10-02 18:08:34 +00:00

gabogg added the

ready-for-agent

label

2026-10-02 18:08:34 +00:00

gabogg referenced this issue

2026-10-02 18:10:02 +00:00

feat(schedule): activate manual calibration and time changes next day #177

gabogg referenced this issue

2026-10-03 07:23:57 +00:00

feat(schedule): activate manual calibration and time changes next day #177