Modularity Review

Scope: Entire ccgram codebase — post-refactoring pass, 2026-04-16
Date: 2026-04-16 (v2 — follows the morning review in 2026-04-16/)
Previous review: docs/modularity-review/2026-04-16/modularity-review.html

Executive Summary

ccgram is a single-process Python bot (~47k lines) that routes Telegram messages to AI coding agent CLIs running in tmux panes. A targeted refactoring pass addressed all six issues from the morning review: monitor_events.py was extracted to break the transcript_reader ↔ session_monitor import cycle; subagent mutation authority was consolidated into SessionLifecycle; a reset_window_polling_state() facade now provides a single reset contract for the polling subsystem; tmux_manager's module-level dependency on the provider domain layer was removed; WindowView gained a batch_mode field; and ClaudeProvider.scrape_current_mode became testable via an injectable capture_fn.

The overall modularity health has improved from 4.8/10 to 5.4/10, moving from "needs attention" to "improving but significant work remains." The session-state coupling problem was not targeted in this pass and remains the dominant drag: SessionManager is still directly accessed from all 30 handler files at 89 call sites. The claude_task_state write-authority problem was partially resolved — cleanup paths are now consolidated — but active state mutations from hook_events.py and window_tick.py still bypass the designated authority. Both issues affect the most volatile areas of the codebase and remain the highest-priority targets for the next pass.

Dimension Scores

DimensionBeforeAfterDeltaNotes
Encapsulation / Information Hiding4/105/10+1batch_mode on WindowView; subagent authority consolidated; polling reset facade added
Cohesion5/105/10No module splits; large files unchanged
Coupling Discipline4/105/10+1tmux_manager module-level providers import removed; transcript/monitor cycle broken
Contract Stability6/106/10reset_window_polling_state is a new named contract; WindowView gains batch_mode
Testability5/106/10+1Injectable capture_fn; monitor_events.py freely importable without triggering singletons
Volatility Alignment4/105/10+1Coupling density in volatile areas slightly reduced
Module Size Distribution6/106/10No module splits
Dependency Direction4/105/10+1tmux_managerproviders module-level dependency removed
Overall4.8/105.4/10+0.6Meaningful progress; session-state and active mutation authority remain open

What Was Resolved

IssueChangeStatus
Circular dependency: transcript_reader ↔ session_monitorExtracted monitor_events.py with zero internal dependencies✅ Resolved
Subagent mutation authorityhandle_subagent_start/stop added to SessionLifecycle; hook_events routes through it✅ Resolved (cleanup paths)
Polling state facadereset_window_polling_state(window_id) added; command_orchestration uses it✅ Resolved (one bypass remains)
tmux_managerproviders layer violationModule-level import removed; local import inside _scan_session_windows✅ Resolved
WindowView coverage gapbatch_mode field added; _handle_stop uses view.notification_mode✅ Partial
ClaudeProvider.scrape_current_mode hardwires tmuxInjectable capture_fn parameter with default✅ Resolved

Coupling Overview

Integration Strength Distance Volatility Balanced?
30 handler modules → SessionManager (89 call sites)FunctionalSame serviceHighNo — unchanged from prior review
hook_eventsclaude_task_state (5 active mutations)FunctionalSame serviceHighNo — non-cleanup writes have no single authority
window_tickclaude_task_state (3 mutations)FunctionalSame serviceHighNo — active state mutations outside designated authority
window_tickpolling_strategies internals (30 calls)FunctionalSame service (cross-layer)HighNo — core polling loop reaches into strategy internals directly
hook_eventsterminal_poll_state.clear_seen_statusFunctionalSame serviceHighNo — bypasses reset_window_polling_state facade
3 remaining deferred-import cyclesFunctional (bidirectional)Same serviceHighNo — masks true dependency graph
shell_infra (4 functions) → tmux_manager (hardwired)FunctionalSame serviceModerateNo — providers not unit-testable without tmux
monitor_events.py — new moduleContractSame serviceLowYes — no internal dependencies, pure data
AgentProvider Protocol (18 methods)ContractSame serviceModerateMostly — wide surface, but appropriate for this abstraction level
reset_window_polling_state() facadeContractSame serviceHighYes — single reset contract for external callers
hook.pysession_map.py file-lock protocolBehavioralCross-processLowYes — low volatility makes unbalanced strength tolerable

Issue 1: SessionManager Remains a Dependency Hub

Integration: 30 handler modules → session.py:SessionManager (89 call sites)  |  Severity: Significant — unchanged from prior review

Knowledge Leakage

SessionManager is directly imported and called across every handler in the codebase. The WindowView read-only projection introduced in previous work helps for single-field reads, but it only covers the read path and only for handlers that call view_window() first. The remaining 89 call sites acquire functional coupling to the session state model: notification modes, approval modes, batch modes, session IDs, provider names, and cwds are all read through individual getter methods rather than through a stable contract.

The _wire_singletons() initialization pattern (injecting _schedule_save callbacks into five sub-singletons) still exists as a hidden ordering constraint. Any new sub-singleton added to the system inherits this constraint silently.

Complexity Impact

With 89 call sites across 30 files, a change to the session state model still requires auditing the entire handler layer. The WindowView projection reduces the impact for the fields it covers, but it needs to cover all commonly-read fields before it materially reduces the coupling breadth.

Cascading Changes

Adding a new per-window setting (e.g., verbosity_mode) still requires: a field in WindowState, a getter/setter in SessionManager, serialization changes in session.py, and additions in each handler that needs the setting. The WindowView only helps if new fields are consistently read through it rather than via direct session_manager.get_*() calls.

Recommended Improvement

Continue the WindowView expansion already begun in this pass. Add getters currently called directly on session_manager (get_approval_mode, get_session_id_for_window) to WindowView, and migrate handlers that call view_window() to use the view fields. Target the 10 highest-call-count files first: sync_command.py, recovery_callbacks.py, transcript_discovery.py, resume_command.py. The goal is to reduce the number of files that import session_manager at all.


Issue 2: claude_task_state Active Mutations Have No Single Authority

Integration: hook_events.py (5 calls) + window_tick.py (3 calls) → claude_task_state  |  Severity: Significant — partially addressed

Knowledge Leakage

The prior pass successfully consolidated subagent lifecycle mutations and session-end cleanup into SessionLifecycle. However, active state mutations triggered by hook events and poll-cycle transitions still reach claude_task_state directly: set_wait_header (notification), clear_wait_header (stop + active transition), format_completion_text (stop), mark_task_completed (task completed), set_last_status (poll cycle). Five distinct mutation entry points mean that knowledge of the internal ClaudeTaskStateStore API is diffused across two handler modules rather than concentrated in one authority.

Complexity Impact

The two remaining mutation sites have different triggers: hook events are asynchronous external signals; poll-cycle transitions are synchronous internal state machine steps. This means the shared state is written from two different execution contexts, and the consistency model must be reasoned about across those boundaries — a complexity source that grows as new event types are added.

Cascading Changes

Recommended Improvement

Split the remaining mutations by trigger type. Hook-event-driven mutations (set_wait_header, clear_wait_header on Stop, mark_task_completed) belong in session_lifecycle or a thin new task_state_events.py coordinator. Poll-cycle mutations (clear_wait_header on active transition, set_last_status) are legitimately local to window_tick — these are the monitoring loop's own state updates. The key insight: not all writes need the same authority; the goal is one authority per trigger type, not a single global write lock.


Issue 3: hook_events Bypasses the Polling Reset Facade

Integration: hook_events._handle_session_endterminal_poll_state.clear_seen_status directly  |  Severity: Minor

Knowledge Leakage

reset_window_polling_state(window_id) was introduced in this pass to encapsulate the two-step polling reset. command_orchestration correctly uses it. But hook_events._handle_session_end at line 302 still calls terminal_poll_state.clear_seen_status(window_id) directly, bypassing the contract. If a third state cell is added to reset_window_polling_state, hook_events won't pick it up.

Recommended Improvement

Replace the direct call in _handle_session_end with reset_window_polling_state(window_id). One-line change — the import path is already available in the same file.


Issue 4: window_tick Has 30 Direct Polling Strategy Calls

Integration: window_tick.pyterminal_poll_state, lifecycle_strategy, terminal_screen_buffer (30 calls)  |  Severity: Minor

Knowledge Leakage

window_tick.py is the per-window poll cycle executor — architecturally close to the polling strategy objects and legitimately the primary consumer of them. However, 30 direct calls to three different singletons means window_tick has deep functional coupling to the internal structure of polling_strategies.py. The balance rule tolerates this given the low distance, but the 30-call dispersion increases the cost of any polling state refactoring.

Recommended Improvement

Low priority given the low distance. If window_tick.py grows further, consider grouping related state transitions into compound methods on the strategy objects (e.g., lifecycle_strategy.complete_dead_window_transition(user_id, thread_id, window_id) instead of 3–4 separate calls). This reduces external call-site count without introducing a new layer.


Issue 5: Three Deferred-Import Cycles Remain

Integration: session.pysession_resolver.py, session_map.pywindow_state_store.py / thread_router.py  |  Severity: Minor

Knowledge Leakage

Three bidirectional dependency cycles still exist, each suppressed with function-level deferred imports. The transcript_reader ↔ session_monitor cycle from the prior review was broken by monitor_events.py — the same technique applies to the remaining three. The pattern: modules that are both state owners and state consumers create the cycle; extracting shared types into dependency-free modules resolves it. monitor_events.py is now the exemplar of this pattern in the codebase.

Recommended Improvement

Apply the same recipe: identify shared types that both sides of each cycle depend on and extract them. For the session.py ↔ session_resolver cycle, the ClaudeSession dataclass is a candidate for extraction into session_types.py. Each extraction is mechanical and low-risk, following the established monitor_events.py pattern.


Issue 6: shell_infra Still Hardwires tmux_manager

Integration: providers/shell_infra.pytmux_manager (lazy imports in 4 async functions)  |  Severity: Minor — unchanged from prior review

Knowledge Leakage

ClaudeProvider.scrape_current_mode was fixed in this pass — it now accepts an injectable capture_fn. The four shell_infra async functions (has_prompt_marker, detect_pane_shell, _is_interactive_shell, setup_shell_prompt) follow the identical pattern but were not updated. Any test that exercises setup_shell_prompt() must mock the entire tmux_manager module or use a real tmux session.

Recommended Improvement

The same injectable parameter pattern already demonstrated in claude.py: setup_shell_prompt(window_id, *, send_keys_fn=None, capture_fn=None) with defaults pointing to tmux_manager.send_keys and tmux_manager.capture_pane. The pattern is now established in the codebase — apply it to shell_infra in the next pass.