ADR-005: Separation of Scheduling/Collection/Rendering in daily_report.py

Status: Accepted
Date: 2026-05-10
Author: Dev-Core Sprint 2

---

Context

The run() function in jobs/daily_report.py was approximately 400 lines before refactoring, mixing four categories of responsibilities:

1. Price Collection: _get_last_close() fetches yfinance / DB market data
2. Staleness Judgment: staleness classification + hard circuit breaker logic
3. LLM Calls: run_macro_view() + run_committee()
4. Report Rendering: Assembles committee results into markdown + email

This coupling led to:

• The report rendering logic could not be unit tested independently—any test required mocking LLM and yfinance
• _portfolio_summary() and _format_staleness() are valuable pure functions, but were hidden within cron logic and could not be reused
• Adding new rendering fields (e.g., tracking position display logic) required locating context within a 400-line function

Decision

Do not perform a "one-size-fits-all" split, for the following reasons:

• The price collection and staleness judgment logic in daily_report.py are deeply coupled (the same asset_freshness dict is read and written in two places)
• Complete decoupling would require introducing an intermediate data structure (a "PriceContext" dataclass), with an effort equivalent to a rewrite
• The current pain point is concentrated on "the rendering layer cannot be unit tested," making a precise surgical approach more appropriate

Actual approach: Extract pure functions into jobs/daily_report_builder.py

Responsibilities of the new file jobs/daily_report_builder.py

Function	Description
classify_asset_freshness()	Stateless: (price, age_days) → fresh/stale/very_stale/missing
format_staleness_warning()	Stateless: Generates staleness warning string for LLM context
portfolio_summary_text()	Pure function: Given pm + prices + total assets → multi-line string
assemble_full_report()	Pure function: Given committee results → complete markdown report

Responsibilities retained in jobs/daily_report.py

• _get_last_close(): Market data collection (has network IO)
• _gather_relevant_insights(): File IO
• run(): Cron trigger + price collection + staleness judgment + LLM calls + email sending
• _classify_freshness(): Inline closure (delegates to builder.classify_asset_freshness)

Separation Boundary Design

daily_report.py (IO + Scheduling)
  └─→ Price Collection: _get_last_close(), get_gold_snapshot()
  └─→ Staleness Judgment: asset_freshness dict + hard circuit breaker thresholds
  └─→ LLM Calls: run_macro_view(), run_committee()
  └─→ Calls builder: _portfolio_summary() → portfolio_summary_text()
  └─→ Calls builder: assemble_full_report()

daily_report_builder.py (Pure functions, zero IO)
  └─→ classify_asset_freshness()
  └─→ format_staleness_warning()
  └─→ portfolio_summary_text()
  └─→ assemble_full_report()

Why Not a More Thorough Split

Introducing a DailyReportContext dataclass to pass "intermediate results from the collection phase" to the builder was considered, but:

1. The three collections asset_freshness, skipped_assets, and data_warnings are mutable during the collection phase (updated within a for loop); their final state is only known after run() finishes
2. Encapsulating these three dicts/sets into a dataclass would introduce a stateful intermediate layer, and the cost of constructing fixtures for testing would be similar to the current approach
3. The current pain point (report rendering cannot be unit tested) has already been resolved by extracting pure functions; decoupling the scheduling and collection layers is deferred to a subsequent sprint

Test Coverage

Added tests/test_daily_report_builder.py, covering:

• All 7 branches of classify_asset_freshness()
• 5 scenarios for format_staleness_warning()
• 7 scenarios for portfolio_summary_text() (including position unrealized P&L, tracking position filtering, emergency fund deduction)
• 4 scenarios for assemble_full_report() (including placeholder text when all sections are skipped)

A total of 22 new tests were added, all with zero mocking (no yfinance / LLM required).