From 35597ab5f2e6c7b3717f0210aab2349b1137ffd7 Mon Sep 17 00:00:00 2001 From: Chris Nicholas <4948774+cnicholas@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:47:24 -0400 Subject: [PATCH] fix(signals): a short series no longer returns an unqualified all-clear (#114) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - **What:** wire SignalConfig.min_observations into detection as an advisory threshold, and make SignalResult.summary report partial evaluation. - **Why:** on a four-point X chart six of eight rules cannot fire, yet the summary printed "✓ No signals detected" with no warning. The config's min_observations was never read. GHSA-hw63-2x95-fmpv / #114. - **Scope:** signals/detector.py, signals/result.py, docstrings, weco-rules docs, api.md, CHANGELOG, SECURITY.md version table, tests. ## Contract / Invariants (must remain true) - Which rules run, and what they flag, is unchanged: the per-rule table is now SignalDetector.RULE_MIN_OBSERVATIONS and still gates rule execution. - rules_skipped reason text unchanged. - SignalResult constructor additions are keyword-with-default; 4-arg positional construction still works and reads as complete. - to_json output format unchanged (records array). - No chart math touched; validation/e2e_bishop_report.py 280/280. ## Behavior Changes (explicit) - detect_signals() on fewer than min_observations rows emits one ProcessBehaviorWarning per chart naming both numbers (stacklevel reaches the caller of AnalysisResult.detect_signals). - is_partial / evaluation_status are also True/'partial' when the series is below min_observations, even if every runnable rule ran. - summary for a partial evaluation starts with "⚠ Partial evaluation in ..." and never contains "No signals detected"; the ✓ line is reserved for a complete evaluation. With violations present, a partial evaluation adds an "Evaluation: partial (...)" line. - New SignalResult attributes: rules_evaluated, rules_applicable, n_observations, min_observations, below_min_observations, evaluation_note. repr gains evaluation='partial'. Excel Summary sheet gains two rows. ## Tests - test_signals: partial test extended with denominator and summary asserts; below-threshold warns and is partial; at-threshold is complete and silent; threshold never changes which rules run; violations + partial shows the evaluation line. - test_signal_result_surface: partial summary is not an all-clear; positional construction reads complete. - test_documented_rule_coverage: docs minimum-observations table pinned to RULE_MIN_OBSERVATIONS; the advisory's four-point ramp reproducer. ## Manual Verification - pytest tests/: 2335 passed, 10 skipped - ruff check .: clean; mypy processbehavior/signals: clean - validation/e2e_bishop_report.py: exit 0, 280 assertions pass --- CHANGELOG.md | 30 ++++++++ SECURITY.md | 8 +-- docs/reference/api.md | 3 +- docs/reference/weco-rules.md | 57 +++++++++++++++- processbehavior/analysis_result.py | 7 +- processbehavior/signals/config.py | 6 +- processbehavior/signals/detector.py | 76 +++++++++++++++------ processbehavior/signals/result.py | 95 +++++++++++++++++++++++--- tests/test_documented_rule_coverage.py | 30 ++++++++ tests/test_signal_result_surface.py | 30 ++++++++ tests/test_signals.py | 76 +++++++++++++++++++++ 11 files changed, 377 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b8f9e0..e84a4fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **A short series no longer returns an unqualified all-clear from + ``detect_signals``.** ``SignalConfig.min_observations`` (default 20) was + unreachable: ``SignalDetector`` read a hardcoded per-rule table and never + consulted the config, so ``SignalConfig(min_observations=30)`` changed nothing. + Separately, the detector already recorded which rules it skipped for length + (``rules_skipped``, added in 0.2.0) but ``SignalResult.summary`` ignored that and + printed ``✓ No signals detected`` regardless. On a four-point X chart six of the + eight rules cannot fire, and the one that can is weakened by any trend in the + data, so the checkmark described an examination that had mostly not happened. + + Now: the per-rule table is the *structural* minimum (``RULE_MIN_OBSERVATIONS``, + a rule is skipped below it, as before) and ``min_observations`` is the *advisory* + minimum. Below it the detector emits a ``ProcessBehaviorWarning`` naming both + numbers (one per chart evaluated; silence it with + ``warnings.simplefilter('ignore', ProcessBehaviorWarning)`` once a pipeline has + accounted for it) and the result is marked partial even when every runnable + rule ran. + ``SignalResult`` gains ``rules_evaluated``, ``rules_applicable``, + ``n_observations``, ``min_observations``, ``below_min_observations`` and + ``evaluation_note``; ``is_partial`` / ``evaluation_status`` now also reflect the + advisory threshold. A partial evaluation prints + ``⚠ Partial evaluation in X: 2 of 8 rules applicable at n=4 (...). No signals + from the rules evaluated.`` and the checkmark line is reserved for a complete + evaluation. Which rules run, and what they flag, is unchanged. Excel export's + Summary sheet adds evaluation status. Reported privately (GHSA-hw63-2x95-fmpv) + and in #114. +- ``SECURITY.md`` supported-versions table said 0.1.x while the prose said 0.2.x. + The table now matches the prose. + ### Removed - **Seven never-called utilities from ``datasets.synthetic``** — ``make_edge_cases`` (unseeded, non-deterministic), ``compare_sds_characteristics`` (its docstring diff --git a/SECURITY.md b/SECURITY.md index 6e4adf3..4d9a947 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,13 +2,13 @@ ## Supported versions -processbehavior is in **alpha** (0.1.x). Only the most recent 0.1.x release -receives security fixes. 0.2.x is the supported line; 0.1.x is not patched. +processbehavior is in **alpha**. Only the most recent 0.2.x release receives +security fixes; 0.1.x is not patched. | Version | Supported | | ------- | ------------------ | -| 0.1.x | :white_check_mark: | -| < 0.1 | :x: | +| 0.2.x | :white_check_mark: | +| < 0.2 | :x: | ## Reporting a vulnerability diff --git a/docs/reference/api.md b/docs/reference/api.md index 45ec95d..2dcca3d 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -451,7 +451,8 @@ from processbehavior.signals import SignalConfig, RuleSet, SignalResult Returned by `result.detect_signals()`. - Status: `.count`, `.has_signals`, `.is_partial`, `.evaluation_status`, - `.rules_skipped` + `.evaluation_note`, `.rules_evaluated`, `.rules_skipped`, `.rules_applicable`, + `.n_observations`, `.min_observations`, `.below_min_observations` - Views: `.violations` (list), `.by_rule` (dict), `.by_observation` (dict), `.flagged_observations`, `.summary` - Filters: `.get_rule_violations(rule_name)`, `.get_observation_violations(obs_id)` diff --git a/docs/reference/weco-rules.md b/docs/reference/weco-rules.md index 83b7ba9..894d1ab 100644 --- a/docs/reference/weco-rules.md +++ b/docs/reference/weco-rules.md @@ -263,7 +263,12 @@ Not all rules apply to all chart types: ## Minimum Observations -Each rule requires a minimum number of observations: +There are two minimums, and they answer different questions. + +### Structural minimum, per rule + +Each rule needs a certain number of points before its pattern can occur at all. +A rule below its minimum is skipped, not failed: | Rule | Minimum Observations | |------|---------------------| @@ -276,7 +281,55 @@ Each rule requires a minimum number of observations: | 7 | 15 | | 8 | 8 | -ProcessBehavior automatically skips rules that can't be evaluated due to insufficient data. +These live in `SignalDetector.RULE_MIN_OBSERVATIONS` and are not configurable: +a run of eight cannot exist in six points. + +### Advisory minimum, per evaluation + +`SignalConfig.min_observations` (default 20) is the series length below which the +evaluation as a whole is reported as **partial**, even if every rule that could run +did run. At four points, six of the eight X/mR rules are structurally inert and the +remaining two have very little to work with. Below the threshold the detector emits +a `ProcessBehaviorWarning` naming both numbers, and the result carries the detail: + +```python +signals = result.detect_signals(chart='X') +signals.is_partial # True +signals.evaluation_status # 'partial' +signals.rules_evaluated # ['rule_1', 'rule_2'] +signals.rules_skipped # {'rule_3': 'needs 5 observations, have 4', ...} +signals.evaluation_note # '2 of 8 rules applicable at n=4 (below min_observations=20); skipped: ...' +``` + +### Reading the summary + +A partial evaluation never prints the clean-series checkmark. Compare: + +``` +✓ No signals detected in X +``` + +which means every applicable rule ran on an adequate series and none fired, with: + +``` +⚠ Partial evaluation in X: 2 of 8 rules applicable at n=4 (below min_observations=20); +skipped: rule_3 (needs 5), rule_4 (needs 8), rule_5 (needs 6), rule_6 (needs 14), +rule_7 (needs 15), rule_8 (needs 8). No signals from the rules evaluated. +``` + +which means most of the examination could not be performed. The second is not an +all-clear. On a short series that is also trending, the one rule that can still fire +(Rule 1) is weakened by the trend itself, because the trend inflates the moving range +that sets the limits. Treat a partial result as "not yet examined", and see the +series-length guidance in the user guide before quoting limits from a short series. + +To silence the warning in a pipeline that has already accounted for it: + +```python +import warnings +from processbehavior import ProcessBehaviorWarning +warnings.simplefilter('ignore', ProcessBehaviorWarning) +``` ## Interpreting Results diff --git a/processbehavior/analysis_result.py b/processbehavior/analysis_result.py index be805bb..9bf8619 100644 --- a/processbehavior/analysis_result.py +++ b/processbehavior/analysis_result.py @@ -1029,11 +1029,16 @@ def detect_signals( >>> from processbehavior.signals import SignalConfig >>> config = SignalConfig( ... enabled_rules=['rule_1', 'rule_2'], - ... min_observations=30, + ... min_observations=30, # below 30 points the result is marked partial ... ignore_first_n=5 ... ) >>> signals = result.detect_signals(config=config) + A short series never returns an unqualified all-clear: when rules were + skipped for length, or the series is below ``min_observations``, + ``signals.is_partial`` is True and ``signals.summary`` says which rules + were evaluated. + Access violations: >>> signals.by_rule['rule_2'] # Rule 2 violations diff --git a/processbehavior/signals/config.py b/processbehavior/signals/config.py index 8e0e141..78c5d23 100644 --- a/processbehavior/signals/config.py +++ b/processbehavior/signals/config.py @@ -97,7 +97,11 @@ class SignalConfig: zone_definition : ZoneDefinition, optional Custom zone definitions min_observations : int, default 20 - Minimum observations required + Advisory minimum series length for a complete evaluation. Rules that + structurally need more points than are present are always skipped (see + ``SignalDetector.RULE_MIN_OBSERVATIONS``); below this threshold the + result is additionally marked partial and a ``ProcessBehaviorWarning`` + is emitted naming both numbers. It does not change which rules run. ignore_first_n : int, default 0 Ignore first N observations (startup period) ignore_last_n : int, default 0 diff --git a/processbehavior/signals/detector.py b/processbehavior/signals/detector.py index d1b37c0..414f056 100644 --- a/processbehavior/signals/detector.py +++ b/processbehavior/signals/detector.py @@ -7,11 +7,12 @@ from __future__ import annotations import logging +import warnings from typing import TYPE_CHECKING import pandas as pd -from ..exceptions import ValidationError +from ..exceptions import ProcessBehaviorWarning, ValidationError from .config import SignalConfig from .detectors import ( detect_avoiding_center, @@ -67,6 +68,21 @@ class SignalDetector: 'rule_8': '8+ consecutive points avoiding Zone C', } + # Structural minimum for each rule: the fewest points at which the pattern + # can occur at all. A rule is skipped (not failed) below its minimum. This is + # distinct from ``SignalConfig.min_observations``, the advisory series-length + # threshold below which the whole evaluation is reported as partial. + RULE_MIN_OBSERVATIONS = { + 'rule_1': 1, + 'rule_2': 3, + 'rule_3': 5, + 'rule_4': 8, + 'rule_5': 6, + 'rule_6': 14, + 'rule_7': 15, + 'rule_8': 8, + } + def detect( self, data: pd.DataFrame, @@ -146,9 +162,12 @@ def detect( # Detect violations for each applicable rule all_violations = pd.DataFrame(index=filtered_data.index) + n_obs = len(filtered_data) # Applicable rules skipped for too few observations (rule -> reason), so the # result can report partial evaluation instead of raising. rules_skipped: dict[str, str] = {} + # Applicable rules that actually ran — the numerator of "k of n rules". + rules_evaluated: list[str] = [] for rule_name in applicable_rules: if rule_name not in self.RULE_DETECTORS: @@ -157,12 +176,11 @@ def detect( # Check minimum observations — skip (don't abort) rules the group is too small for. min_obs = self._get_min_observations(rule_name) - if len(filtered_data) < min_obs: - logger.debug( - f'Skipping {rule_name}: insufficient observations (need {min_obs}, have {len(filtered_data)})' - ) - rules_skipped[rule_name] = f'needs {min_obs} observations, have {len(filtered_data)}' + if n_obs < min_obs: + logger.debug(f'Skipping {rule_name}: insufficient observations (need {min_obs}, have {n_obs})') + rules_skipped[rule_name] = f'needs {min_obs} observations, have {n_obs}' continue + rules_evaluated.append(rule_name) # Apply detector detector = self.RULE_DETECTORS[rule_name] @@ -184,10 +202,30 @@ def detect( logger.error(f'Error detecting {rule_name}: {e}') all_violations[rule_name] = False + # Below the advisory threshold the evaluation is reported as partial even + # when every rule that *could* run did run: at short lengths most run-rules + # are structurally inert, and a bare "no signals" would read as an + # all-clear the series has not earned. One warning per detect() call; a + # pipeline that has accounted for this silences it with + # warnings.simplefilter('ignore', ProcessBehaviorWarning). + n_applicable = len(rules_evaluated) + len(rules_skipped) + if n_obs < config.min_observations: + warnings.warn( + f'Signal detection on {n_obs} observations is below min_observations=' + f'{config.min_observations}: {len(rules_evaluated)} of {n_applicable} ' + f'applicable rules could be evaluated. The result is marked partial; ' + f'do not read "no signals" as an all-clear.', + ProcessBehaviorWarning, + # detect() <- _detect_for_chart <- detect_signals_for_result + # <- AnalysisResult.detect_signals <- caller + stacklevel=5, + ) + # Build result return self._build_result( data=filtered_data, violations=all_violations, stats=stats, value_col=value_col, - chart_name=chart_name, rules_skipped=rules_skipped, + chart_name=chart_name, rules_skipped=rules_skipped, rules_evaluated=rules_evaluated, + n_observations=n_obs, min_observations=config.min_observations, ) def _validate_inputs(self, data: pd.DataFrame, stats: dict): @@ -218,22 +256,15 @@ def _filter_data(self, data: pd.DataFrame, config: SignalConfig) -> pd.DataFrame return filtered def _get_min_observations(self, rule_name: str) -> int: - """Get minimum observations for a rule.""" - minimums = { - 'rule_1': 1, - 'rule_2': 3, - 'rule_3': 5, - 'rule_4': 8, - 'rule_5': 6, - 'rule_6': 14, - 'rule_7': 15, - 'rule_8': 8, - } - return minimums.get(rule_name, 1) + """Structural minimum observations for a rule (see ``RULE_MIN_OBSERVATIONS``).""" + return self.RULE_MIN_OBSERVATIONS.get(rule_name, 1) def _build_result( self, data: pd.DataFrame, violations: pd.DataFrame, stats: dict, value_col: str, chart_name: str, rules_skipped: dict[str, str] | None = None, + rules_evaluated: list[str] | None = None, + n_observations: int | None = None, + min_observations: int | None = None, ) -> SignalResult: """Build SignalResult from violation matrix.""" # Create violation records @@ -268,8 +299,11 @@ def _build_result( violation_df = pd.DataFrame(records) if records else pd.DataFrame() - return SignalResult(violations=violation_df, chart_name=chart_name, data=data, stats=stats, - rules_skipped=rules_skipped) + return SignalResult( + violations=violation_df, chart_name=chart_name, data=data, stats=stats, + rules_skipped=rules_skipped, rules_evaluated=rules_evaluated, + n_observations=n_observations, min_observations=min_observations, + ) def _limits_vary(self, stats: dict) -> bool: """Check if control limits vary (per-row limits).""" diff --git a/processbehavior/signals/result.py b/processbehavior/signals/result.py index 4012e38..f2c5ac2 100644 --- a/processbehavior/signals/result.py +++ b/processbehavior/signals/result.py @@ -37,7 +37,10 @@ class SignalResult: """ def __init__(self, violations: pd.DataFrame, chart_name: str, data: pd.DataFrame, stats: dict, - rules_skipped: dict[str, str] | None = None): + rules_skipped: dict[str, str] | None = None, + rules_evaluated: list[str] | None = None, + n_observations: int | None = None, + min_observations: int | None = None): self.violations = violations self.chart_name = chart_name self.data = data @@ -46,6 +49,14 @@ def __init__(self, violations: pd.DataFrame, chart_name: str, data: pd.DataFrame # points (rule_name -> reason). Lets callers distinguish "no signals found" # from "not fully evaluated" (e.g. a small stratified subgroup). self.rules_skipped = rules_skipped or {} + # Applicable rules that did run. Together with ``rules_skipped`` this gives + # the "k of n rules applicable" denominator the summary reports. + self.rules_evaluated = list(rules_evaluated or []) + # Post-filter observation count the rules were evaluated on, and the + # advisory threshold from ``SignalConfig.min_observations``. Either may be + # None when a result is constructed directly rather than by the detector. + self.n_observations = n_observations + self.min_observations = min_observations @property def count(self) -> int: @@ -57,15 +68,56 @@ def has_signals(self) -> bool: """Whether any signals were detected.""" return self.count > 0 + @property + def rules_applicable(self) -> int: + """Number of rules that applied to this chart (evaluated + skipped).""" + return len(self.rules_evaluated) + len(self.rules_skipped) + + @property + def below_min_observations(self) -> bool: + """True when the series is shorter than the configured advisory minimum. + + Rules that could run still ran; this flags that the evaluation as a whole + is not one the analyst should treat as complete. + """ + return ( + self.n_observations is not None + and self.min_observations is not None + and self.n_observations < self.min_observations + ) + @property def is_partial(self) -> bool: - """True when some applicable rules were skipped for too few observations.""" - return bool(self.rules_skipped) + """True when some applicable rules were skipped, or the series is below + ``min_observations``. Either way, "no signals" is not an all-clear.""" + return bool(self.rules_skipped) or self.below_min_observations @property def evaluation_status(self) -> str: - """'complete' when every applicable rule ran, else 'partial'.""" - return 'partial' if self.rules_skipped else 'complete' + """'complete' when every applicable rule ran on an adequate series, else 'partial'.""" + return 'partial' if self.is_partial else 'complete' + + @property + def evaluation_note(self) -> str: + """One-line account of what was and was not evaluated. + + Empty when the evaluation is complete. Otherwise, e.g.:: + + 2 of 8 rules applicable at n=4 (below min_observations=20); skipped: + rule_3 (needs 5), rule_4 (needs 8), ... + """ + if not self.is_partial: + return '' + n_txt = f'n={self.n_observations}' if self.n_observations is not None else 'this series length' + parts = [f'{len(self.rules_evaluated)} of {self.rules_applicable} rules applicable at {n_txt}'] + if self.below_min_observations: + parts[0] += f' (below min_observations={self.min_observations})' + if self.rules_skipped: + skipped = ', '.join( + f'{rule} ({reason.split(" observations")[0]})' for rule, reason in self.rules_skipped.items() + ) + parts.append(f'skipped: {skipped}') + return '; '.join(parts) @property def flagged_observations(self) -> set: @@ -130,8 +182,17 @@ def get_observation_violations(self, obs_id: Any) -> pd.DataFrame: @property def summary(self) -> str: - """Human-readable summary of detected signals.""" + """Human-readable summary of detected signals. + + A partial evaluation never reads as an all-clear: the clean-series + checkmark line is reserved for a complete evaluation. + """ if not self.has_signals: + if self.is_partial: + return ( + f'⚠ Partial evaluation in {self.chart_name}: {self.evaluation_note}. ' + f'No signals from the rules evaluated.' + ) return f'✓ No signals detected in {self.chart_name}' lines = [ @@ -140,8 +201,10 @@ def summary(self) -> str: f'{"=" * 70}', f'Total violations: {self.count}', f'Flagged observations: {len(self.flagged_observations)}', - '', ] + if self.is_partial: + lines.append(f'Evaluation: partial ({self.evaluation_note})') + lines.append('') # Breakdown by rule rule_counts = self.violations['rule_name'].value_counts() @@ -198,8 +261,14 @@ def to_excel(self, filepath: str): # Summary sheet summary_data = { - 'Metric': ['Total Violations', 'Flagged Observations', 'Chart Name'], - 'Value': [self.count, len(self.flagged_observations), self.chart_name], + 'Metric': [ + 'Total Violations', 'Flagged Observations', 'Chart Name', + 'Evaluation Status', 'Rules Evaluated / Applicable', + ], + 'Value': [ + self.count, len(self.flagged_observations), self.chart_name, + self.evaluation_status, f'{len(self.rules_evaluated)} / {self.rules_applicable}', + ], } pd.DataFrame(summary_data).to_excel(writer, sheet_name='Summary', index=False) @@ -207,7 +276,10 @@ def to_excel(self, filepath: str): def to_json(self, filepath: str): """ - Export violations to JSON. + Export violations to JSON (a list of violation records). + + Evaluation status is not part of this file; read ``evaluation_status``, + ``rules_evaluated`` and ``rules_skipped`` on the result instead. Parameters ---------- @@ -222,10 +294,11 @@ def to_json(self, filepath: str): logger.info(f'✓ Exported violations to: {filepath}') def __repr__(self): + partial = ", evaluation='partial'" if self.is_partial else '' return ( f'SignalResult(violations={self.count}, ' f'flagged_obs={len(self.flagged_observations)}, ' - f"chart='{self.chart_name}')" + f"chart='{self.chart_name}'{partial})" ) def __str__(self): diff --git a/tests/test_documented_rule_coverage.py b/tests/test_documented_rule_coverage.py index cb792c9..bb0536a 100644 --- a/tests/test_documented_rule_coverage.py +++ b/tests/test_documented_rule_coverage.py @@ -76,3 +76,33 @@ def test_readme_does_not_claim_rule_1_only(): assert '**Signal detection**' in text, 'the Features bullet was renamed; update this test' line = next(ln for ln in text.splitlines() if '**Signal detection**' in ln) assert 'eight' in line.lower(), f'README understates rule coverage: {line}' + + +def test_docs_minimum_observations_table_matches_detector(): + """``weco-rules.md`` restates ``RULE_MIN_OBSERVATIONS``; keep the two identical.""" + import re + + from processbehavior.signals.detector import SignalDetector + + doc = (Path(__file__).resolve().parent.parent / 'docs' / 'reference' / 'weco-rules.md').read_text( + encoding='utf-8' + ) + table = re.findall(r'^\| (\d) \| (\d+) \|$', doc, flags=re.MULTILINE) + assert {f'rule_{r}': int(n) for r, n in table} == SignalDetector.RULE_MIN_OBSERVATIONS + + +def test_four_point_ramp_is_not_an_all_clear(): + """The GHSA-hw63-2x95-fmpv reproducer: a pure ramp at T=4 must not print ✓.""" + + from processbehavior import ProcessBehaviorWarning + + df = pd.DataFrame({'t': range(4), 'y': [10_000.0, 10_400.0, 10_800.0, 11_200.0]}) + st = pb.formulate(df, response='y', time='t') + r = st.execute(chart='X', companion=True) + with pytest.warns(ProcessBehaviorWarning, match='below min_observations=20'): + signals = r.detect_signals()['X'] + assert signals.is_partial + assert signals.rules_evaluated == ['rule_1', 'rule_2'] + assert signals.rules_applicable == 8 + assert not signals.summary.startswith('✓') + assert 'No signals detected' not in signals.summary diff --git a/tests/test_signal_result_surface.py b/tests/test_signal_result_surface.py index 356e1c8..60715a6 100644 --- a/tests/test_signal_result_surface.py +++ b/tests/test_signal_result_surface.py @@ -87,6 +87,36 @@ def test_summary_with_signals(self, signals): def test_summary_clean(self, clean): assert 'No signals' in clean.summary + def test_summary_partial_is_not_an_all_clear(self): + """Skipped rules, or a series under min_observations, never yield the ✓ line.""" + data = pd.DataFrame({'mean': [100.0] * 4}) + stats = {'center': 100.0, 'upl': 115.0, 'lpl': 85.0} + partial = SignalResult( + pd.DataFrame(), + 'Short Chart', + data, + stats, + rules_skipped={'rule_4': 'needs 8 observations, have 4'}, + rules_evaluated=['rule_1', 'rule_2'], + n_observations=4, + min_observations=20, + ) + text = partial.summary + assert text.startswith('⚠ Partial evaluation in Short Chart') + assert '2 of 3 rules applicable at n=4 (below min_observations=20)' in text + assert 'skipped: rule_4 (needs 8)' in text + assert 'No signals detected' not in text + assert '✓' not in text + assert partial.evaluation_status == 'partial' + + def test_positional_construction_still_complete(self, clean): + """The 4-arg form (no evaluation metadata) reads as a complete evaluation.""" + assert clean.rules_evaluated == [] + assert clean.rules_applicable == 0 + assert clean.n_observations is None + assert not clean.below_min_observations + assert not clean.is_partial + def test_repr_and_str(self, signals, clean): assert 'SignalResult' in repr(signals) assert isinstance(str(signals), str) and str(signals) diff --git a/tests/test_signals.py b/tests/test_signals.py index cfa0cc8..054c481 100644 --- a/tests/test_signals.py +++ b/tests/test_signals.py @@ -5,10 +5,13 @@ detection, and result handling. """ +import warnings + import numpy as np import pandas as pd import pytest +from processbehavior.exceptions import ProcessBehaviorWarning from processbehavior.signals import RuleSet, SignalConfig, SignalDetector, ZoneDefinition @@ -202,6 +205,79 @@ def test_insufficient_data_partial_evaluation(self, simple_stats): assert 'rule_1' not in result.rules_skipped # Flat, in-limits data → no violations among the rules that ran. assert not result.has_signals + # The result carries the denominator and says so in the summary. + assert result.rules_evaluated == ['rule_1', 'rule_2', 'rule_3'] + assert result.rules_applicable == 8 + assert result.n_observations == 5 + assert result.summary.startswith('⚠ Partial evaluation') + assert '3 of 8 rules applicable at n=5' in result.summary + assert 'No signals detected' not in result.summary + + def test_below_min_observations_is_partial_and_warns(self, simple_stats): + """Every runnable rule ran, but the series is under the advisory threshold. + + GHSA-hw63-2x95-fmpv / #114: ``min_observations`` was unreachable and a + short series printed an unqualified all-clear. Below the threshold the + detector now warns, naming both numbers, and the result is partial. + """ + data = pd.DataFrame({'mean': [100.0] * 10, 'obs_id': range(10)}) + detector = SignalDetector() + config = SignalConfig(enabled_rules=['rule_1'], min_observations=20) + + with pytest.warns(ProcessBehaviorWarning, match='10 observations is below min_observations=20'): + result = detector.detect(data, simple_stats, config, chart_type='X') + + assert result.rules_skipped == {} # rule_1 needs 1 point; it ran + assert result.rules_evaluated == ['rule_1'] + assert result.below_min_observations + assert result.is_partial + assert result.evaluation_status == 'partial' + assert '1 of 1 rules applicable at n=10 (below min_observations=20)' in result.evaluation_note + assert not result.summary.startswith('✓') + assert "evaluation='partial'" in repr(result) + + def test_at_min_observations_is_complete_and_silent(self, simple_data, simple_stats): + """An adequate series with every rule run is complete: no warning, checkmark summary.""" + detector = SignalDetector() + config = SignalConfig(enabled_rules=['rule_1'], min_observations=30) # fixture has 30 rows + + with warnings.catch_warnings(): + warnings.simplefilter('error', ProcessBehaviorWarning) + result = detector.detect(simple_data, simple_stats, config, chart_type='X') + + assert not result.below_min_observations + assert not result.is_partial + assert result.evaluation_status == 'complete' + assert result.evaluation_note == '' + assert result.summary == '✓ No signals detected in Chart' + + def test_min_observations_does_not_change_which_rules_run(self, simple_data, simple_stats): + """The advisory threshold marks the result; it never suppresses a rule.""" + detector = SignalDetector() + loose = SignalConfig(min_observations=1) + strict = SignalConfig(min_observations=1000) + + with warnings.catch_warnings(): + warnings.simplefilter('ignore', ProcessBehaviorWarning) + r_loose = detector.detect(simple_data, simple_stats, loose, chart_type='X') + r_strict = detector.detect(simple_data, simple_stats, strict, chart_type='X') + + assert r_loose.rules_evaluated == r_strict.rules_evaluated + assert r_loose.rules_skipped == r_strict.rules_skipped + assert r_loose.count == r_strict.count + assert not r_loose.is_partial and r_strict.is_partial + + def test_signals_present_and_partial_shows_evaluation_line(self, simple_stats): + """With violations AND a short series, the banner summary says it was partial.""" + data = pd.DataFrame({'mean': [100.0, 100.0, 130.0, 100.0], 'obs_id': range(4)}) + detector = SignalDetector() + with warnings.catch_warnings(): + warnings.simplefilter('ignore', ProcessBehaviorWarning) + result = detector.detect(data, simple_stats, SignalConfig(), chart_type='X') + + assert result.has_signals + assert result.is_partial + assert 'Evaluation: partial (2 of 8 rules applicable at n=4' in result.summary def test_empty_data_raises_validation_error(self, simple_stats): """Empty data is still a genuine error — now a ValidationError."""