Fix four latent pipeline bugs found in a full-pass review - #35
Merged
Conversation
- discovery.py: Inductive Miner discovery called inductive_miner.apply()
without variant=Variants.IMf, so noise_threshold had zero effect -
noise filtering only exists in the IMf variant. Verified empirically
(identical trees at noise=0.0 vs 0.9 under the default IM variant,
and IMf at noise=0.0 matches plain IM exactly, so this is safe at
the default too). The UI's Noise Threshold slider had been a no-op.
- conformance.py: _fitness_state_equation_alignments() accepted
initial_marking/final_marking but never used them, instead guessing
markings from net topology ("no in-arcs" = initial, "no out-arcs" =
final). Usually coincides with the real markings for a discovered
net, but a place with no arcs at all satisfies both conditions and
gets folded into both guessed markings - and any reference-model
topology (build_structured_reference_model/import_reference_model_
bpmn) where the real start/end doesn't coincide with sourceless/
sinkless places silently corrupts fitness/alignments. Now uses the
real markings, same pattern as the sibling _fitness_token_replay(),
falling back to the topology guess only if none were supplied.
- analytics.py: purchase/cart/research-keyword matching built a regex
via '|'.join(values) with no guard for an empty list - '|'.join([])
is '', and str.contains('') matches every row. An empty
purchase_values/cart_values/research_keywords list (a reachable
business_params/function-argument value) silently classified
everything as a match instead of nothing, across 5 call sites
including this session's classify_sessions(). Extracted into a
shared _contains_any() helper that returns an all-False mask for an
empty list.
- data_manager.py: refine_activity_labels() decided whether to apply
URL-cleaning (strip query string, keep last path segment) based only
on the first matched row's value, then applied that single decision
to the entire column - a column mixing plain and URL-like values
leaked raw slashes/query strings into every non-first-style row's
activity name. The cleaning steps are no-ops for genuinely plain
values, so they're now applied unconditionally per row instead of
gated on a single sampled value.
Added a regression test for each.
BasLinders
marked this pull request as ready for review
August 21, 2026 12:16
BasLinders
added a commit
that referenced
this pull request
Sep 8, 2026
Fix four latent pipeline bugs found in a full-pass review
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
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.
Summary
Four correctness bugs found in a full-pipeline review, each verified empirically/by direct reproduction before fixing, with a regression test added per fix.
prox/discovery.py— Inductive Miner discovery calledinductive_miner.apply()withoutvariant=Variants.IMf, sonoise_thresholdhad zero effect: noise filtering only exists in theIMfvariant, not the defaultIMvariant this code invoked. Verified: identical process trees atnoise_threshold=0.0vs0.9underIM;IMfatnoise_threshold=0.0matches plainIMexactly, so switching is safe at the default too. The UI's "Noise Threshold" slider had been a no-op the whole time.prox/conformance.py—_fitness_state_equation_alignments()acceptedinitial_marking/final_markingarguments but never used them, instead guessing markings from net topology ("place with no in-arcs" = initial, "no out-arcs" = final). Usually coincides with the real markings for a discovered net (which is why this went unnoticed), but a place with no arcs at all satisfies both conditions and gets folded into both guessed markings, and any reference-model topology (build_structured_reference_model/import_reference_model_bpmn) where the real start/end doesn't coincide with sourceless/sinkless places silently corrupts fitness/alignments. Now uses the real markings passed in — same pattern the sibling_fitness_token_replay()already used — falling back to the topology guess only if none were supplied.prox/analytics.py— purchase/cart/research-keyword matching built a regex via'|'.join(values)with no guard for an empty list:'|'.join([])is'', andstr.contains('')matches every row. An emptypurchase_values/cart_values/research_keywordslist (reachable viabusiness_paramsor direct function arguments) silently classified everything as a match instead of nothing, across 5 call sites — includingclassify_sessions()added earlier this session. Extracted into a shared_contains_any()helper that returns an all-False mask for an empty list.prox/data_manager.py—refine_activity_labels()decided whether to apply URL-cleaning (strip query string, keep last path segment) based only on the first matched row's value, then applied that one decision to the entire column. A column mixing plain and URL-like values leaked raw slashes/query strings into every non-first-style row's activity name. The cleaning steps are no-ops for genuinely plain values, so they're now applied unconditionally per row instead of gated on a single sampled value.Test plan
pytest tests/— 140 passed (5 new regression tests, one per fix, each reproducing the original failure against the pre-fix code before confirming the fix)noise_threshold=0.3— completes cleanly, all result keys present, conformance scores in expected range