Skip to content

refactor: extract VariableDetector base class for variable detectors#235

Open
viktorbeck98 wants to merge 1 commit into
developmentfrom
feature/variable-detector-base
Open

refactor: extract VariableDetector base class for variable detectors#235
viktorbeck98 wants to merge 1 commit into
developmentfrom
feature/variable-detector-base

Conversation

@viktorbeck98

Copy link
Copy Markdown
Collaborator

Task

Description

Extracts an abstract VariableDetector base class (common/variable_detector.py,
inheriting CoreDetector) that owns the shared lifecycle (train/detect/
configure/post_train/set_configuration) previously duplicated ~90% across the
five variable-value detectors. Each concrete detector — NewValue, Charset,
ValueRange, BigramFrequency, NewValueCombo — is reduced to a config subclass
plus a few override hooks (_check_variable, _prepare_variables, _event_data_kwargs,
_check_event, _description). All detectors stay in the detectors package so
name-based tracker reconstruction keeps working. Net: +439 / −712 lines.
new_event/random/rule are untouched.

Also fixes latent bugs surfaced by the refactor:

  • ValueRangeDetector now registers persistency and preserves the persist config
    across set_configuration (both were previously dropped).
  • SingleStabilityTracker no longer starts a throwaway PersistencySaver when
    reconstructing a detector to recover its add_value closure (it would leak a saver
    thread per variable and clobber the real state file under persist).

Behavior-preserving; the only intended behavior changes are the two value_range fixes.

How Has This Been Tested?

  • Existing per-detector suites are the safety net and stay green (unchanged pass counts).
  • Added tests: value_range persist-registration + persist-preservation, and a TDD
    regression test that reconstruction under persist starts no throwaway saver.
  • Full suite: 545 passed. prek gate (mypy --strict, flake8, vulture, docformatter,
    bandit) clean.

Checklist

  • This Pull-Request goes to the development branch.
  • I have successfully run prek locally.
  • I have added tests to cover my changes.
  • I have linked the issue-id to the task-description.
  • I have performed a self-review of my own code.

Factor the ~90% duplicated lifecycle (train/detect/configure/post_train/
set_configuration) shared by the five variable-value detectors into a new
abstract VariableDetector base in common/variable_detector.py. Each concrete
detector (new_value, charset, value_range, bigram, new_value_combo) is reduced
to a config subclass plus a few override hooks; all remain in the detectors
package so name-based tracker reconstruction keeps working.

Also fixes latent bugs surfaced by the refactor:
- ValueRangeDetector now registers persistency and preserves the persist
  config across set_configuration (both were previously dropped).
- SingleStabilityTracker no longer starts a throwaway PersistencySaver when
  reconstructing a detector to recover its add_value closure (it would leak a
  saver thread per variable and clobber the real state file under persist).

Behavior-preserving refactor guarded by the existing per-detector suites.
Full suite: 545 passed. prek gate (mypy --strict, flake8, vulture, docformatter) clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@viktorbeck98
viktorbeck98 requested a review from ipmach July 21, 2026 15:41
@viktorbeck98 viktorbeck98 self-assigned this Jul 21, 2026
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