fix: detect_signals() on phased charts; value='response' (#141, #142)

CHANGELOG.md

@@ -119,6 +119,16 @@ Equation numbers below are that manual's.
   one assignment, which builds the same figure (0.03 s for 800). Above 1,000
   (`plotting.lane_boundaries.MAX_LANE_DIVIDERS`) none are drawn: the dividers would merge into a
   solid band at any chart width.
+- **`detect_signals()` works on phased charts.** It raised `TypeError` on every `phased=True`
+  chart: each phase has its own centre, so the statistics carry `center=None`, and the zone
+  calculation read that scalar. The chart already marks run rules not applicable, but only the
+  plot read the flag. Phased charts now get Rule 1 by default, judged against each phase's
+  limits, with the same points the chart flags; an explicit rule list runs as given, against each
+  row's own centre. The limits and the flagged points on the chart were always correct.
+- **`value='response'` charts the response**, as the `execute()` signature and docstring say.
+  Every chart refused it ("Valid charts for RESPONSE: None"): residuals are told apart by a leading
+  `R`, which `RESPONSE` also has. `execute()`, `supports_calibration()` and `why_not()` now treat
+  it, in any case, exactly as `value=None`.
 - **Xbar and S charts with subgroups of unequal size follow the manual (Eqs 11-5, 11-16,
   11-17).** sigma-hat is the average of each subgroup's S_r / c4(N_r) (was S-bar / c4(N_r)), and each
   subgroup's limits use its own N_r, so they step with subgroup size, as VAS draws them. N_r now

processbehavior/analysis_result.py

@@ -1039,6 +1039,10 @@ def detect_signals(
         ``signals.is_partial`` is True and ``signals.summary`` says which rules
         were evaluated.
 
+        On a phased chart (``phased=True``) each phase has its own centre line, so
+        the default rules are Rule 1 only, judged against each phase's limits. An
+        explicit rule list still runs as given, against each row's own centre.
+
         Access violations:
 
         >>> signals.by_rule['rule_2']  # Rule 2 violations

processbehavior/result_signals.py

@@ -146,4 +146,6 @@ def _detect_for_chart(
         value_col=value_col,
         chart_name=chart_name,
         chart_type=chart_type,
+        # Phased charts mark run rules not applicable; until now only the renderer read it.
+        run_rules_applicable=metadata.get("run_rules_applicable", True),
     )

processbehavior/signals/detector.py

@@ -91,6 +91,7 @@ def detect(
         value_col: str = 'mean',
         chart_name: str = 'Chart',
         chart_type: str = 'Xbar',
+        run_rules_applicable: bool = True,
     ) -> SignalResult:
         """
         Detect signals in control chart data.
@@ -109,6 +110,11 @@ def detect(
             Name of chart for reporting
         chart_type : str, default 'Xbar'
             Chart type ('Xbar', 'S', 'X', 'mR') - determines applicable rules
+        run_rules_applicable : bool, default True
+            False for charts whose centre moves along the series (phased limits,
+            ``metadata['run_rules_applicable']``). The chart-type default rules
+            narrow to Rule 1; an explicit rule list runs as given, against each
+            row's own centre.
 
         Returns
         -------
@@ -126,6 +132,13 @@ def detect(
         # Get applicable rules for this chart type
         applicable_rules = config.get_rules_for_chart(chart_type)
 
+        # Phased limits give each phase its own centre, so the chart marks run rules
+        # 2-8 not applicable: they are not part of the default set, rather than
+        # skipped (a skip would mark the evaluation partial). An explicit rule list is
+        # the analyst's choice and runs as given.
+        if not run_rules_applicable and config.enabled_rules == 'default':
+            applicable_rules = [r for r in applicable_rules if r == 'rule_1']
+
         # Log rule selection
         if logger.isEnabledFor(logging.INFO):
             all_rules = [f'rule_{i}' for i in range(1, 9)]
@@ -291,7 +304,7 @@ def _build_result(
                             'rule_number': int(rule_name.split('_')[1]),
                             'description': self.RULE_DESCRIPTIONS[rule_name],
                             'value': data.loc[idx, value_col],
-                            'center': stats['center'],
+                            'center': self._row_center(data, stats, idx),
                             'upl': ucl,
                             'lpl': lcl,
                         }
@@ -309,6 +322,22 @@ def _limits_vary(self, stats: dict) -> bool:
         """Check if control limits vary (per-row limits)."""
         return bool(stats.get('limits_vary'))
 
+    @staticmethod
+    def _center(data: pd.DataFrame, stats: dict):
+        """The centre line: the scalar, or the per-row column when there is none.
+
+        Phased charts give each phase its own centre, so their statistics carry
+        ``center=None`` and the per-phase value lives on each row.
+        """
+        if stats.get('center') is None and 'center' in data.columns:
+            return data['center']
+        return stats['center']
+
+    def _row_center(self, data: pd.DataFrame, stats: dict, idx):
+        """The centre line at one row."""
+        center = self._center(data, stats)
+        return center.loc[idx] if isinstance(center, pd.Series) else center
+
     def _calculate_per_row_zones(self, data: pd.DataFrame, stats: dict, config: SignalConfig) -> pd.DataFrame:
         """
         Calculate per-row zone boundaries for varying process limits.
@@ -334,7 +363,7 @@ def _calculate_per_row_zones(self, data: pd.DataFrame, stats: dict, config: Sign
         if 'upl' not in data.columns or 'lpl' not in data.columns:
             raise ValueError("Cannot calculate per-row zones: data missing 'upl' or 'lpl' columns")
 
-        center = stats['center']
+        center = self._center(data, stats)
         zone_def = config.zone_definition
 
         # Calculate sigma per row

processbehavior/signals/detectors.py

@@ -173,6 +173,9 @@ def detect_run(data: pd.DataFrame, stats: dict, value_col: str, length: int = 8)
     """
     values = data[value_col]
     center = stats.get('center', stats.get('Mean'))  # Try 'center' first, fallback to 'Mean'
+    if center is None and 'center' in data.columns:
+        # Phased charts: each phase has its own centre, carried on the rows.
+        center = data['center']
 
     # Above or below center
     above = (values > center).astype(int)

processbehavior/study.py

@@ -75,6 +75,17 @@ def _base_residual_code(value: str) -> str:
     return upper[2:] if upper.startswith('RCR') else upper
 
 
+def _normalise_value(value: str | None) -> str | None:
+    """'response' (any case) -> None, the response's own spelling; residuals unchanged.
+
+    Residuals are told apart by their leading 'R', which 'RESPONSE' also has, so the
+    documented ``value='response'`` must become None before anything tests the prefix.
+    """
+    if isinstance(value, str) and value.lower() == 'response':
+        return None
+    return value
+
+
 @dataclass
 class DesignReport:
     """
@@ -1521,6 +1532,8 @@ def why_not(self, chart: str, value: str | None = None) -> str:
         >>> study.why_not('Xbar', value='R2')
         "'Xbar' with value='R2' is not valid for ADS 1.\\nValid charts for R2: S, X"
         """
+        value = _normalise_value(value)
+
         # Handle old fused syntax: why_not('R5_Xbar') -> why_not('Xbar', value='R5')
         if '_' in chart and value is None:
             parts = chart.split('_', 1)
@@ -2018,6 +2031,8 @@ def execute(
                 )
             by = stratify_by
 
+        value = _normalise_value(value)
+
         # 0. Resolve calibration (label string -> attached Calibration object).
         calibration_obj = self._resolve_calibration(calibration)
 
@@ -2114,6 +2129,7 @@ def supports_calibration(
                     'parameter.'
                 )
             by = stratify_by
+        value = _normalise_value(value)
 
         from .analysis import Analysis
         from .formulation_spec import ChartRequest
@@ -2396,7 +2412,10 @@ def _validate_recentered(self, value: str | None) -> None:
             )
         recenterable = set(RESIDUAL_CODES)
         if value is None:
-            raise ValidationError('recentered=True requires a residual value (R1-R6), got value=None')
+            raise ValidationError(
+                'recentered=True requires a residual value (R1-R6); the response '
+                '(value=None or "response") is already on the measurement scale.'
+            )
         if value.upper() not in recenterable:
             raise ValidationError(f"recentered=True requires a residual value (R1-R6), got '{value}'")
 

tests/test_execute_validation.py

@@ -218,3 +218,54 @@ def test_execute_with_accessor(self, sds1_study):
         """End-to-end: study.execute(chart=..., value=study.residuals.R5)."""
         result = sds1_study.execute(chart=sds1_study.charts.Xbar, value=sds1_study.residuals.R5)
         assert result is not None
+
+
+# =============================================================================
+# value='response' is the response (#142)
+# =============================================================================
+
+
+class TestValueResponse:
+    """The documented value='response' charts the response, like value=None.
+
+    Residuals are told apart by a leading 'R', which 'RESPONSE' also has, so the
+    documented spelling was routed down the residual path and refused.
+    """
+
+    @pytest.mark.parametrize(
+        'kwargs',
+        [
+            {'chart': 'Xbar'},
+            {'chart': 'S'},
+            {'chart': 'Xbar', 'companion': True},
+            {'chart': 'X', 'by': []},
+            {'chart': 'mR', 'by': []},
+            {'chart': 'Histogram'},
+        ],
+        ids=['Xbar', 'S', 'Xbar+S', 'X', 'mR', 'Histogram'],
+    )
+    @pytest.mark.parametrize('spelling', ['response', 'Response', 'RESPONSE'])
+    def test_same_result_as_none(self, sds1_study, kwargs, spelling):
+        from pandas.testing import assert_frame_equal
+
+        expected = sds1_study.execute(**kwargs)
+        actual = sds1_study.execute(value=spelling, **kwargs)
+        assert set(actual.charts) == set(expected.charts)
+        for name in expected.charts:
+            assert_frame_equal(actual.get_chart(name), expected.get_chart(name))
+            assert actual.get_statistics(name) == expected.get_statistics(name)
+            assert actual.charts[name]['metadata'] == expected.charts[name]['metadata']
+
+    def test_why_not_agrees(self, sds1_study):
+        for chart in ('Xbar', 'S', 'X', 'mR'):
+            assert sds1_study.why_not(chart, value='response') == sds1_study.why_not(chart)
+
+    def test_supports_calibration_agrees(self, sds1_study):
+        for kwargs in ({'chart': 'X', 'by': []}, {'chart': 'Xbar'}, {'chart': 'X', 'by': ['factor 1']}):
+            assert sds1_study.supports_calibration(value='response', **kwargs) == (
+                sds1_study.supports_calibration(**kwargs)
+            )
+
+    def test_recentered_response_still_refused(self, sds1_study):
+        with pytest.raises(ValidationError, match='recentered.*requires.*residual'):
+            sds1_study.execute(chart='Xbar', value='response', recentered=True)

tests/test_phased_limits.py

@@ -412,3 +412,77 @@ def test_phased_residual_limits_vary(self, sds2_study):
         result = sds2_study.execute(chart='X', by=[], phased=True, value='R2')
         data = result.get_chart('X')
         assert data['center'].nunique() > 1 or data['upl'].nunique() > 1
+
+
+# =============================================================================
+# Phased + signal detection
+# =============================================================================
+
+
+class TestPhasedSignals:
+    """detect_signals() on phased charts: Rule 1 against each phase's own limits.
+
+    Phased statistics carry ``center=None`` (each phase has its own centre) and the
+    chart marks run rules not applicable. detect_signals() used to read the scalar
+    centre and raise TypeError on every phased chart.
+    """
+
+    def test_phased_x_rule_1_against_phase_limits(self, sds1_study):
+        result = sds1_study.execute(chart='X', by=[], phased=True)
+        signals = result.detect_signals(chart='X')
+        assert signals.rules_evaluated == ['rule_1']
+        assert signals.rules_skipped == {}
+        assert not signals.is_partial
+        data = result.get_chart('X')
+        beyond = set(data.index[data['beyond_limits'] != 0])
+        assert beyond, 'fixture should have a point beyond its phase limits'
+        assert signals.flagged_observations == beyond
+
+    def test_violation_records_the_phase_centre(self, sds1_study):
+        result = sds1_study.execute(chart='X', by=[], phased=True)
+        data = result.get_chart('X')
+        violations = result.detect_signals(chart='X').to_dataframe()
+        assert violations['center'].notna().all()
+        for _, row in violations.iterrows():
+            assert row['center'] == data.loc[row['obs_id'], 'center']
+            assert row['upl'] == data.loc[row['obs_id'], 'upl']
+
+    def test_phased_companion_both_charts(self, sds1_study):
+        result = sds1_study.execute(chart='X', by=[], phased=True, companion=True)
+        signals = result.detect_signals()
+        assert set(signals) == {'X', 'mR'}
+        for name, sig in signals.items():
+            data = result.get_chart(name)
+            assert sig.rules_evaluated == ['rule_1']
+            assert sig.flagged_observations == set(data.index[data['beyond_limits'] != 0])
+
+    def test_phased_stratified_focus(self, sds1_study):
+        result = sds1_study.execute(chart='X', by=['factor 1'], phased=True, companion=True)
+        for stratum in result.strata:
+            signals = result.focus(stratum).detect_signals()
+            assert {sig.rules_evaluated[0] for sig in signals.values()} == {'rule_1'}
+
+    def test_explicit_rules_run_against_each_phase_centre(self):
+        """An explicit rule list runs as given; run and zone rules read the row's centre."""
+        from processbehavior.signals import SignalConfig, SignalDetector
+
+        # Phase 1 sits above its centre 0, phase 2 below its centre 10: one run each.
+        # Phase 2's last two points fall in its lower zone A (7 to 8, sigma 1).
+        data = pd.DataFrame({
+            'x': [1.0] * 8 + [9.0] * 6 + [7.5, 7.5],
+            'center': [0.0] * 8 + [10.0] * 8,
+            'upl': [3.0] * 8 + [13.0] * 8,
+            'lpl': [-3.0] * 8 + [7.0] * 8,
+        })
+        stats = {'N': None, 'center': None, 'lpl': None, 'upl': None, 'limits_vary': True}
+        signals = SignalDetector().detect(
+            data, stats, SignalConfig(enabled_rules=['rule_2', 'rule_4'], min_observations=1),
+            value_col='x', chart_type='X', run_rules_applicable=False,
+        )
+        assert signals.rules_evaluated == ['rule_2', 'rule_4']
+        assert set(signals.by_rule['rule_4']['obs_id']) == {7, 15}
+        assert set(signals.by_rule['rule_2']['obs_id']) == {15}
+
+    def test_unphased_x_keeps_every_rule(self, sds1_study):
+        signals = sds1_study.execute(chart='X', by=[]).detect_signals(chart='X')
+        assert signals.rules_evaluated == [f'rule_{i}' for i in range(1, 9)]