Decompose analysis_result.py into focused modules

[cnicholas]

Context

processbehavior/analysis_result.py is 1572 LoC with a single AnalysisResult class carrying 46 methods plus a FocusedAnalysisResult subclass. The file has accumulated multiple unrelated responsibilities — tabular formatting, signal-detection orchestration, stratification, repr, dict protocol, plotting delegates — and is starting to show the same accretion pattern as the plotter's tick subsystem before its recent extraction.

Preemptive refactor: no specific recurring bug yet, but two recent independent edits both landed in focus() within 48 hours (the focused-stratum tick-label bug + the reset_index follow-up), and chart_table() carries a # noqa: C901 complexity suppression — the same hidden-debt signal that preceded the plotter rewrite. Pre-release status means decomposing now is cheap.

Public API is unchanged. Every existing method (chart_table, detect_signals, focus, plot, etc.) remains callable on AnalysisResult exactly as today. Only the implementation moves to focused modules; methods become thin delegates.

Goal

Split analysis_result.py into 4 focused modules so each has one responsibility:

File	Responsibility	Approx LoC after
analysis_result.py	Core class: construction, summary, properties, data accessors, plot delegates, dict protocol, repr	~700
result_tabular.py (NEW)	build_chart_table() + build_signals_table() as pure functions	~250
result_signals.py (NEW)	detect_signals_for_result() as a pure orchestrator	~180
focused_result.py (NEW)	focus_on(result, stratum) + FocusedAnalysisResult class	~280

Net effect: same total LoC, distributed across single-responsibility modules. The four module entry points are pure-ish functions taking result as their first argument and returning the same types as today — directly unit-testable without instantiating the full class.

Non-Goals

• No public API changes. result.chart_table(...), result.detect_signals(...), result.focus(...) work identically.
• No methodology changes. Chart math, signal detection rules, statistics — untouched.
• No removal of the dict protocol (__getitem__, keys, etc.) in this PR. Keep for backward compat; that's a separate decision.
• No mypy/lint cleanup beyond what the move requires — that's tracked in Tier 3: Maintenance polish — analysis.py refactor, vocabulary, mypy, coverage #82.

Architecture

The refactored AnalysisResult keeps every method on its public surface. Method bodies become 1-3 line delegates:

# in analysis_result.py
class AnalysisResult:
    def chart_table(self, chart=None, include_signal_col=True, signal_symbols=True):
        from .result_tabular import build_chart_table
        return build_chart_table(self, chart=chart,
                                 include_signal_col=include_signal_col,
                                 signal_symbols=signal_symbols)

    def detect_signals(self, chart=None, rules=None, config=None, **kwargs):
        from .result_signals import detect_signals_for_result
        return detect_signals_for_result(self, chart=chart, rules=rules,
                                         config=config, **kwargs)

    def focus(self, stratum: str) -> AnalysisResult:
        from .focused_result import focus_on
        return focus_on(self, stratum)

The lazy imports (inside the method body) keep analysis_result free of new top-level coupling and avoid any risk of a circular import with focused_result.py (which itself imports AnalysisResult for the subclass).

File changes — detailed

NEW: processbehavior/result_tabular.py (~250 LoC)

Move from analysis_result.py:707-925:

• build_chart_table(result, *, chart, include_signal_col, signal_symbols) — pure function version of the current chart_table body (currently analysis_result.py:707-885). The # noqa: C901 goes away once the function is split into:
  • _resolve_value_col(result, chart_data) — value-column inference (currently analysis_result.py:771-810)
  • _extract_signal_column(chart_data, signal_symbols) — signal-symbol mapping
  • build_chart_table(...) — orchestrator that calls both
• build_signals_table(result, *, chart) — pure function version of get_signals (currently analysis_result.py:887-925).

NEW: processbehavior/result_signals.py (~180 LoC)

Move from analysis_result.py:1054-1198 + 1200-1225:

• detect_signals_for_result(result, *, chart, rules, config, **kwargs) — pure function version. Internal split:
  • _build_config(rules, config, kwargs) — config construction
  • _detect_for_chart(detector, result, chart_name, config) — per-chart detection
  • detect_signals_for_result(...) — orchestrator
• _extract_chart_type(chart_name) moves with it as a module-level private helper.

NEW: processbehavior/focused_result.py (~280 LoC)

Move from analysis_result.py:412-555 + 1442-1572:

• focus_on(result, stratum) — pure function version of the focus() body. Includes the per-stratum boundary unpacking + reset_index from the recent Bug A fix.
• class FocusedAnalysisResult(AnalysisResult) — moved verbatim. Backward-compat re-export at bottom of analysis_result.py:

  from .focused_result import FocusedAnalysisResult  # noqa: E402, F401

MODIFIED: processbehavior/analysis_result.py (1572 → ~700 LoC)

Kept on AnalysisResult class:

• __init__, _build_summary, _is_stratified
• All properties: residuals, effects, interactions, summary, has_residuals, has_effects, has_interactions, all_charts, strata, is_stratified
• Data accessors: _resolve_chart_name, get_chart, get_statistics, get_residual, list_strata, iter_charts
• Dict protocol: __getitem__, __contains__, keys, values, items, __len__, __iter__, get
• String forms: __repr__, __str__
• Plot delegates: plot, plot_residuals, plot_effects, report
• Export delegate: to_excel
• Thin delegate methods: chart_table, get_signals, detect_signals, focus (each becomes 1-3 lines)

Removed (moved):

• Body of chart_table (707-885) → result_tabular.py
• Body of get_signals (887-925) → result_tabular.py
• Body of detect_signals (1054-1198) → result_signals.py
• _extract_chart_type (1200-1225) → result_signals.py
• Body of focus (412-555) → focused_result.py
• Entire FocusedAnalysisResult class (1442-1572) → focused_result.py (with re-export shim)

NEW: tests/test_result_tabular.py

Unit tests (~10):

• chart=None uses first chart
• Unknown chart raises ChartNotAvailableError
• Value-col inference: prefers spec.response_var, then known statistic columns, then first non-meta column
• Signal symbol mapping: ↑/↓/None for beyond_limits values
• include_signal_col=False omits signal column
• Stratified chart returns one row per stratum

NEW: tests/test_result_signals.py

Unit tests (~8):

• chart=None returns dict of all charts
• Specific chart returns single SignalResult
• rules='standard' / 'extended' / ['rule_1'] / RuleSet(...) all build correct config
• Unknown chart raises ChartNotAvailableError
• Chart missing metadata raises ProcessBehaviorError

NEW: tests/test_focused_result.py

Move existing test_focus.py::TestFocusLaneBoundaries here alongside new tests for the extracted focus_on function. ~12 tests total. Keep test_focus.py for the public-API surface tests.

UNCHANGED tests

Every existing test using the public API continues to pass without modification:

• tests/test_focus.py (19 tests) — result.focus(...) still works
• tests/test_chart_parsing.py — result.chart_table(...) still works
• tests/test_plotting*.py — result.plot(...) unchanged
• tests/test_analysis_*.py — accessors unchanged
• tests/test_signals*.py — result.detect_signals(...) unchanged

Migration phases

Each phase commits independently with full pytest green.

Phase 1 — result_tabular.py + tests

• Create processbehavior/result_tabular.py with build_chart_table, build_signals_table (verbatim move with result as first arg)
• Create tests/test_result_tabular.py
• Don't touch analysis_result.py yet; pytest tests/test_result_tabular.py passes
• Commit: refactor(result): extract chart_table + signals_table into result_tabular

Phase 2 — delegate chart_table / get_signals

• Update AnalysisResult.chart_table and AnalysisResult.get_signals to 1-3 line delegates
• Delete the old bodies in analysis_result.py
• Full pytest passes
• Commit: refactor(result): delegate chart_table / get_signals to result_tabular

Phase 3 — result_signals.py + delegate detect_signals

• Create processbehavior/result_signals.py with detect_signals_for_result + _extract_chart_type
• Create tests/test_result_signals.py
• Update AnalysisResult.detect_signals to a delegate; delete old body + _extract_chart_type
• Full pytest passes
• Commit: refactor(result): extract detect_signals into result_signals

Phase 4 — focused_result.py + delegate focus

• Create processbehavior/focused_result.py with focus_on and FocusedAnalysisResult
• Add re-export at bottom of analysis_result.py
• Update AnalysisResult.focus to a delegate; delete old body and inline class
• Move TestFocusLaneBoundaries to tests/test_focused_result.py
• Full pytest + e2e validators pass
• Commit: refactor(result): extract focus + FocusedAnalysisResult into focused_result

Phase 5 — verification

cd /Users/nicholas/Documents/projects/processbehavior
.venv/bin/python -m pytest tests/ --quiet            # all green (1487+)
.venv/bin/python validation/e2e_bishop_report.py     # clean
.venv/bin/python validation/e2e_T20_report.py        # clean

# Public API smoke
.venv/bin/python -c "
import pandas as pd
import processbehavior as pb
df = pd.read_csv('validation/PBTESTDATABASE_T100.csv')
r = pb.ProcessBehavior(df).formulate(
    response='PM SDS 3', factors=['FACTOR 1', 'FACTOR 2'], time='PRODUCTION TIME',
).execute(chart='X', by=['FACTOR 1', 'FACTOR 2'], companion=True)
print(r.chart_table('X').head())
print(r.detect_signals('X').summary)
print(r.focus('1_2').strata)
"

# File sizes after refactor
wc -l processbehavior/analysis_result.py processbehavior/result_tabular.py \
      processbehavior/result_signals.py processbehavior/focused_result.py
# Expected: ~700 + ~250 + ~180 + ~280

Invariants to Preserve

• Public API unchanged. All result.<method>(...) calls work identically with the same return types.
• FocusedAnalysisResult importable from analysis_result via re-export shim.
• Methodology unchanged. Chart math, signal detection, statistics — bit-identical output.
• e2e validators (validation/e2e_bishop_report.py, e2e_T20_report.py) pass byte-for-byte.

Risk Areas / Edge Cases

Risk	Mitigation
Circular import: focused_result imports AnalysisResult; analysis_result.focus lazy-imports focused_result	Lazy imports inside method bodies + if TYPE_CHECKING: for type hints. Pattern already used by _build_summary
Pickle of FocusedAnalysisResult — qualified class name changes	Re-export shim in analysis_result.py keeps analysis_result.FocusedAnalysisResult resolvable
External callers reaching result._extract_chart_type	Keep a method delegate as a safety net
Tests that monkey-patch AnalysisResult.chart_table	Delegate design preserves the method; patching still works

Estimate

Phase	Effort
1. result_tabular + tests	2 hrs
2. delegate chart_table / get_signals	0.5 hr
3. result_signals + tests + delegate	2 hrs
4. focused_result + tests + delegate	2.5 hrs
5. verification + e2e	1 hr
Total	~8 hours focused work, spread over 1-2 days

After-refactor improvement opportunities (out of scope here)

• build_chart_table can drop the # noqa: C901 once the inner helpers are extracted
• _extract_chart_type can lose the "common chart type mapping" + prefix loop (25 LoC for what's a 2-line name.split('_')[0])
• The dict protocol (__getitem__, keys, etc.) — documented "for backward compatibility" — could be deprecated in a future PR

Related

• Tier 3: Maintenance polish — analysis.py refactor, vocabulary, mypy, coverage #82 (Tier 3 maintenance polish — covers similar decompositions for analysis.py, study.py, plotter.py, synthetic.py)
• Recent precedent: x-axis layout extraction in processbehavior/plotting/x_axis_layout.py (commits 754d2fd, 98513a6, 65c83c5, ff800f0) followed exactly this delegate-and-extract pattern with property tests at the layout level