Documentation and modifiability pass: Severance.Phase + architecture doc - #39
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c8f81137f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @spec phase_for_remaining(integer()) :: :gentle | :aggressive | :final | :shutdown | ||
| def phase_for_remaining(minutes) do |
There was a problem hiding this comment.
Preserve the documented Countdown helpers
Existing code that calls the documented public helpers Severance.Countdown.phase_for_remaining/1 or Severance.Countdown.tick_interval_ms/1 now gets UndefinedFunctionError, because the implementations were moved here but no forwarding wrappers were left in Countdown. This matters for any user scripts/tests compiled against the previous API, and the design note for this pass explicitly says public signatures should not change; keeping deprecated delegations to Phase would preserve compatibility.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not making this change. These were internal helpers exposed only for unit testing, not a public/library contract:
- Severance ships as the
sevbinary; mix.exs has nopackageblock, so it is not published to Hex. No external code can compile againstSeverance.Countdown. - Neither function appears in any user-facing contract — not in
sev help --agent, not indocs/configuration.md. The publisher extension surface isStatus+Tmux.Format+Tmux.publisher. grepfinds zero callers ofCountdown.phase_for_remaining/1orCountdown.tick_interval_ms/1outside the tests that moved tophase_test.
Adding forwarders would be dead code for callers that cannot exist (YAGNI). Delegating tick_interval_ms to Phase.interval_ms would also widen its non_neg_integer() contract, since interval_ms returns nil for non-escalation phases. Phase logic now lives solely in Severance.Phase. Fixed the spec note that wrongly described these as preserved delegations (936e671).
The mix todo agent prompt now tells implementers to update docs/architecture.md in the same PR when a change alters the system. AGENTS.md and CLAUDE.md document the living-docs vs point-in-time distinction so architecture.md stays current as the app evolves.
…untdown phase definitions in a single Severance.Phase module
Summary
Severance.Phaseas the single source of truth for the escalating countdown phases (threshold, tick interval, notification sound, tmux color, blink).Countdown,Notifier,StatusPublisher.Tmux.Format, andStatusnow read phase attributes from it instead of each owning a parallel copy — adding or re-ordering a phase becomes a one-place edit, and it's the seam the roadmap's "configurable escalation phases" would replace.Countdownhelpers that were only exposed for testing (phase_for_remaining/1,tick_interval_ms/1) move intoPhase; their tests move with them.docs/architecture.md— a current-state living doc (distinct from the dateddocs/specs/): supervision tree, CLI-dispatch vs daemon split, countdown state machine, config resolution, RPC seam, system-adapter test seam, publishers, and the newPhasemodule. Linked from the README.Applicationenv, duplicated RPC/distribution setup (daemon vs CLI), and the oversizedSeverance.CLI.Test plan
mix qualitygreen — format, compile (warnings-as-errors), credo, dialyzer (0 warnings), doctor, tests at 63.0% coveragetest/severance/phase_test.exscovers everyPhasefunction + doctestscountdown/notifier/formattests exercise the delegations end-to-end, confirming behavior is preservedImplementation Plan
Full design committed at
docs/specs/2026-06-08-architecture-modifiability-pass-design.md.Deliverable 1:
Severance.PhaseOne ordered phase list is the single source of truth. A phase is active while
minutes_remaining > min_minutes;color_indexis the position in a 3-element tmux palette[waiting_or_gentle, aggressive, final_or_shutdown], preserving the existing custom-palette behavior ofcolor_for_phase/2.Public API:
name/0(type),phase_for_remaining/1,interval_ms/1,sound/1,color/2,blink?/1,default_palette/0.Consumer changes (delegations only):
CountdowncallsPhase.phase_for_remaining/1+Phase.interval_ms/1;Notifier.phase_sound/1keeps the mode-specific:overtimeclause and delegates the rest toPhase.sound/1;Format.color_for_phase/1,2→Phase.color/2,blink_for_phase/1→Phase.blink?/1;Status.@type phase→Severance.Phase.name().Out of scope (stays in
Countdown— countdown timing, not per-phase): T-30 start window, stale-pane threshold, wait/retry intervals, overtime-burst constants. Loading phases from config (the roadmap item) is also out of scope; this pass only centralizes the static data so that work has one place to land.Deviation from spec discovered during implementation: the spec proposed keeping the two
Countdownhelpers as thin delegations. Delegatingtick_interval_mstoPhase.interval_ms(which returnsnilfor non-escalation phases) would have muddied itsnon_neg_integer()contract, so the helpers were internalized instead —CountdowncallsPhasedirectly at the two call sites and the two test blocks moved tophase_test. Dialyzer confirmed clean.Deliverable 2:
docs/architecture.mdCurrent-state architecture doc covering the process model, entry points, countdown state machine, configuration, RPC seam, system adapter, publishers, a module map, and known rough edges. Linked from the README Development section.