Skip to content

refactor(progress-monitor): extract 4 single-responsibility modules from god class#538

Merged
zbigniewsobiecki merged 1 commit intodevfrom
refactor/progress-monitor-extract-modules
Feb 24, 2026
Merged

refactor(progress-monitor): extract 4 single-responsibility modules from god class#538
zbigniewsobiecki merged 1 commit intodevfrom
refactor/progress-monitor-extract-modules

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Feb 24, 2026

Summary

Refactors src/backends/progressMonitor.ts from a 423-line god class with 5+ mixed responsibilities into a thin orchestrator (~245 lines) that composes four single-responsibility modules.

Card: https://trello.com/c/699dd8e10c3df6b4b67541ec

Extracted modules

  • src/backends/progressState/accumulator.ts — Ring buffer state accumulation (ProgressAccumulator): tool calls, text snippets, completed tasks, iteration tracking, plus summarizeToolParams() and getSnapshot().
  • src/backends/progressState/scheduler.ts — Progressive timer scheduling (ProgressScheduler): fires ticks at [1, 3, 5] min, then intervalMinutes steady-state. Pure scheduling — no business logic.
  • src/backends/progressState/pmPoster.ts — PM comment lifecycle (PMProgressPoster): create-once / update-in-place / fallback-to-new pattern with state file coordination.
  • src/backends/progressState/githubPoster.ts — GitHub PR comment updates (GitHubProgressPoster): session state lookup, comment formatting, and updatePRComment() call.

Changes to progressMonitor.ts

  • Composes the 4 extracted classes — only orchestrates the tick flow (model call + posting + checklist sync)
  • Public API unchanged: ProgressMonitorConfig, ProgressMonitor, ProgressReporter interface, start(), stop(), getProgressCommentId() all preserved
  • All 44 pre-existing tests continue to pass without modification

New unit tests (46 tests across 4 files)

  • accumulator.test.ts — Ring buffer bounds, param summarization, snapshot copies
  • scheduler.test.ts — Progressive schedule, steady-state fallback, stop-during-tick safety
  • pmPoster.test.ts — Create/update/fallback lifecycle, state file coordination
  • githubPoster.test.ts — Session state lookup, comment body substitution

Test plan

  • All existing tests pass (npm test → 3185 tests passing)
  • 46 new focused unit tests added for each extracted class
  • TypeScript type check passes (npm run typecheck)
  • Lint passes (npm run lint) — pre-existing adapter.ts warning is unchanged

🤖 Generated with Claude Code

@nhopeatall
Copy link
Copy Markdown
Collaborator

nhopeatall commented Feb 24, 2026

✨ On it — checking the progress monitor refactor


Progress: [███░░░░░░░] 30% (iteration 21/70)

🔍 Code Review Update (1 min)

I've completed a thorough review of the refactor/progress-monitor-extract-modules branch, examining the new progressState modules and confirming direct imports. I've also reviewed existing tests in tests/unit/backends/progress.test.ts to ensure continued functionality. My current focus is on understanding the initialization of ProgressAccumulator.startTime and how the start() method is called to ensure correct timing behavior.

Last updated: iteration 21 · review

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — Clean extraction of a god class into four focused, single-responsibility modules. The decomposition preserves the original behavior faithfully, the public API is unchanged, and the new module boundaries (accumulation, scheduling, PM posting, GitHub posting) are well-chosen.

One minor observation (not blocking): ProgressAccumulator.startTime is now set at construction time (readonly startTime = Date.now()) rather than being reset in ProgressMonitor.start() as the original code did. In practice this is negligible — the gap between constructor and start() is milliseconds, and elapsed time is reported in minutes — but worth noting for completeness.

The 46 new tests cover each extracted module's key behaviors well, and all 44 existing integration-level tests continue to pass unchanged, which is strong evidence the refactoring is behavioral-preserving.

@zbigniewsobiecki zbigniewsobiecki merged commit 0b581e3 into dev Feb 24, 2026
5 checks passed
@zbigniewsobiecki zbigniewsobiecki mentioned this pull request Feb 25, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants