Skip to content

engine.py's filter-feature intake call is not the identity no-op its comment claims #904

Description

@TKaltofen

Summary

The comment at mloda/core/core/engine.py:337-339 asserts that the add_feature_to_collection call for a matched filter feature "must remain an identity no-op (no rebind) to keep the SingleFilter set hash stable". That premise does not hold when the filter feature declares a defaulted PROPERTY_MAPPING key explicitly as None.

Reproduction

GlobalFilter.unify_options copies the resolved feature's options only for keys the filter feature does not already set, and never calls options_with_defaults on the filter feature itself. So an explicit None on the filter feature survives unification. options_with_defaults then treats a present None as absent (unless the spec sets allow_explicit_none=True) and fills the declared default.

With a spec declaring default="D" for key k:

host feature effective        : {'k': 'D'}
filter feature declares       : Options(context={'k': None})
after unify_options           : {'k': None}      # warns: "k is different. None != D"
options_with_defaults of that : {'k': 'D'}       # so intake WOULD rebind

GlobalFilter.add_filter accepts a Feature, so a caller can construct this through the public API.

Code pointers

  • mloda/core/core/engine.py:337-339 (the comment and the call)
  • mloda/core/core/engine.py:367 (the unconditional rebind)
  • mloda/core/filter/global_filter.py:103-115 (unify_options, absent-key-only copy)

Definition of done

Either the comment is corrected and the resulting hash shift is shown to be harmless, or the rebind is made genuinely conditional for filter features. A test covering a filter feature that declares a defaulted key explicitly as None either way.

Found while documenting that match_feature_group_criteria's options view depends on the caller (#903).

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions