Skip to content

fix(signals): a short series no longer returns an unqualified all-clear (#114) - #118

Merged
cnicholas merged 1 commit into
mainfrom
fix/signal-partial-evaluation-114
Sep 10, 2026
Merged

fix(signals): a short series no longer returns an unqualified all-clear (#114)#118
cnicholas merged 1 commit into
mainfrom
fix/signal-partial-evaluation-114

Conversation

@cnicholas

Copy link
Copy Markdown
Owner

Summary

  • What: SignalConfig.min_observations becomes a real advisory threshold, and SignalResult.summary reports partial evaluation instead of printing ✓ No signals detected on a series it could barely examine.
  • Why: 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. The summary still printed the checkmark, and min_observations was never read by the detector. Reported in Series length (T) as an axis in design-state admissibility — and a saturation question #114 and privately as GHSA-hw63-2x95-fmpv.
  • Scope: signals/detector.py, signals/result.py, two docstrings, docs/reference/weco-rules.md, docs/reference/api.md, CHANGELOG.md, SECURITY.md version table, tests.

Contract / invariants

  • Defaults unchanged — which rules run, and what they flag, is identical. The per-rule table is now SignalDetector.RULE_MIN_OBSERVATIONS and still gates rule execution. validation/e2e_bishop_report.py exits 0 with 280 of 280.
  • Residuals unaffected — signals only.
  • Row/index alignment preserved — no data path touched.
  • Output schema compatibleto_json still writes a records array. SignalResult constructor additions are keyword-with-default; 4-arg positional construction still works and reads as complete.
  • Pinned error strings unchangedrules_skipped reason text is untouched. The clean-series ✓ No signals detected in X line is unchanged for a complete evaluation.

Behaviour changes

  • detect_signals() on fewer than min_observations rows emits one ProcessBehaviorWarning per chart naming both numbers. Silence with warnings.simplefilter('ignore', ProcessBehaviorWarning).
  • is_partial / evaluation_status are also partial when the series is below min_observations, even if every runnable rule ran.
  • A partial evaluation with no violations prints ⚠ Partial evaluation in X: 2 of 8 rules applicable at n=4 (below min_observations=20); skipped: rule_3 (needs 5), ... No signals from the rules evaluated. With violations present, the banner summary gains an Evaluation: partial (...) line.
  • New SignalResult attributes: rules_evaluated, rules_applicable, n_observations, min_observations, below_min_observations, evaluation_note. repr gains evaluation='partial'. The Excel Summary sheet gains Evaluation Status and Rules Evaluated / Applicable rows.

Methodology

Tests

  • test_signals::test_insufficient_data_partial_evaluation — extended: carries the 3 of 8 denominator and the summary is not an all-clear.

  • test_signals::test_below_min_observations_is_partial_and_warns — every runnable rule ran, series under threshold: warns naming both numbers, result partial.

  • test_signals::test_at_min_observations_is_complete_and_silent — adequate series: no warning, checkmark summary unchanged.

  • test_signals::test_min_observations_does_not_change_which_rules_run — threshold 1 vs 1000 give identical rules and violations.

  • test_signals::test_signals_present_and_partial_shows_evaluation_line.

  • test_signal_result_surface::test_summary_partial_is_not_an_all_clear, ::test_positional_construction_still_complete.

  • test_documented_rule_coverage::test_docs_minimum_observations_table_matches_detector — the docs table is pinned to RULE_MIN_OBSERVATIONS.

  • test_documented_rule_coverage::test_four_point_ramp_is_not_an_all_clear — the advisory's reproducer, end to end through pb.formulate.

  • pytest tests/ — 2335 passed, 10 skipped

  • ruff check . — clean

  • mypy processbehavior/signals — clean

  • Golden masters untouched

Notes

  • RuleSet.custom(min_observations=) is still dropped by to_config(). Pre-existing, separate, not addressed here.
  • run_rules_viz.py early-returns on has_signals, so a partial result draws no annotation. Not addressed here.
  • The warning does not collapse across repeated calls under the default filter, because pandas resets the warning registry internally. One warning per chart per call is the honest description.
  • Closes the advisory once it is published. Second half of Series length (T) as an axis in design-state admissibility — and a saturation question #114 (series-length advisory beside the design state) follows in a separate PR after the methodology discussion.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BKphCi1LC1hjGYteWEK9JV

…ar (#114)

## 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
@cnicholas
cnicholas merged commit 0c6dd08 into main Sep 10, 2026
17 checks passed
@cnicholas
cnicholas deleted the fix/signal-partial-evaluation-114 branch September 10, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant