fix: key the dropped-filter ledger on the declaring filter - #1096
Open
TKaltofen wants to merge 2 commits into
Open
fix: key the dropped-filter ledger on the declaring filter#1096TKaltofen wants to merge 2 commits into
TKaltofen wants to merge 2 commits into
Conversation
GlobalFilter.dropped_filters keyed on (feature group, filter feature name), so two filters declared over one column collapsed into one entry and which fact survived rode set iteration order. The key now carries the declaring filter's uuid, mirroring the probes ledger, and the two report-dedupe sets follow it. _nearest_miss no longer withholds a reason when two filters share a column, and the unmatched-filter warning emits its messages sorted, since the two now differ only in their suffix and filters is a set.
The report lines render from the feature group, the reason and the filter feature name alone, so two declarations on one column produce byte-identical warnings. The dedupe goes back to the column key while the ledger keeps its per-declaration fact, which it now records unconditionally. The unmatched warning sorts on the filter feature name first and uses the rendered message only as the tie-break, so a name that prefixes another keeps its old position. criteria() states that the filter it is handed must be a declared filter or a per-match copy, since the ledger keys on its uuid, and the docs name SingleFilter.uuid as the join back to the declaration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1074
GlobalFilter.dropped_filterswas keyed by(FeatureGroup, filter feature name). A filter's identity is its uuid, so two filters declared over one column collapsed into a single ledger entry, and which of the two facts survived rodesetiteration order overGlobalFilter.filters. Only one of the two declarations could be explained, andwarn_on_unmatched_filterswithheld the nearest miss entirely whenever a name was shared rather than attributing a fact to the wrong filter.What changed
The ledger key carries the declaring filter's uuid:
(FeatureGroup, filter feature name, filter uuid), mirroring theprobeskey shape in the same class. The recorders take theSingleFilterand derive the key themselves; the per-matchdeepcopypreserves the declaration's uuid, which is pinned by a test._nearest_missselects by uuid, so the shared-name guard is gone and each declared filter is told the gate it lost at. The unmatched warning sorts on the filter feature name first, with the rendered message as the tie-break, so two filters on one name emit in a deterministic order and a name that prefixes another keeps its position._record_dropped_filterno longer derives its ledger write from the WARNING dedupe set: a defect still takes the key from a stored near-miss and the first defect's reason still survives later ones, but the fact is recorded independently of the report. The two report-dedupe sets stay keyed on the column, since both report lines render from the feature group, the reason and the filter feature name alone, so a second declaration would only repeat a byte-identical line.criterianow states that the filter it is handed must be a declared filter or a per-match copy of one, because the ledger keys on its uuid.Public shape
dropped_filtersis exported throughmloda.userand documented. The key grew from two parts to three, so code unpacking it asfor (feature_group, name), elimination in ...must now unpack three; the group stays at index 0 and the name at index 1. The docs nameSingleFilter.uuidas the join back to the declaration.Tests
tests/test_core/test_filter/test_filter_elimination_reasons.pycovers the two-filters-on-one-column case end to end: one fact per declaration, each warning naming its own nearest miss, a stable emission order across runs, and the deepcopy preserving filter identity. Three neighbouring drivers built a freshSingleFilterper loop iteration, which only looked like one ledger key because the old key collapsed names; they now mirror the engine and probe a per-match deepcopy of one declaration.toxpasses: 8976 passed, 170 skipped.