fix(transform): normalize flood() output to remove overlapping events - #143
Merged
ErikBjare merged 1 commit intoJul 22, 2026
Merged
Conversation
2 tasks
Greptile SummaryThis PR normalizes
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (1): Last reviewed commit: "fix(transform): normalize flood output o..." | Re-trigger Greptile |
Contributor
Author
|
CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click. This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted. |
This was referenced Jul 22, 2026
ErikBjare
pushed a commit
that referenced
this pull request
Jul 22, 2026
…cy tests (#145) Port two improvements from #105 that weren't included in #143: 1. Sort key change: sort by (timestamp, duration) instead of just timestamp. When multiple events share the same timestamp, shorter events now sort first. This matches aw-server-rust's sort_by_timestamp behavior and gives deterministic, spec-aligned ordering. 2. Two new tests from #105: - test_flood_idempotent: verifies repeated flood() calls produce the same result (important correctness property) - test_flood_unsafe_gap: verifies overlapping differing-data events don't double-count time (validates the normalization pass from #143)
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.
Problem
flood()can return overlapping positive-duration events when the pairwiseflooding loop mutates events that have already been processed. Downstream
duration aggregation counts each event independently, so overlapping events
cause hourly totals to exceed wall-clock time — reported as 60–71 minute hours
in the ActivityWatch Summary timeline.
Minimal reproducer from ActivityWatch/activitywatch#1369:
The root cause: when
gap < -negative_gap_trim_thresand the two events havedifferent data, the existing code only logs a warning and leaves both events
unchanged. Because the pairwise loop has already advanced past the earlier pair,
those overlaps survive the final zero-duration filter.
Fix
Replace the final zero-duration filter with a normalization pass that guarantees
a non-overlapping stream at the output boundary:
the later event's start. Drop the earlier event if clipping reduces it to zero
duration.
This is the same policy the reporter's local fix used and is consistent with the
earlier-event-trimming direction in #105.
The normalization pass is a purely additive safety net — it does not change the
pairwise flooding logic, so existing behaviour for non-overlapping inputs is
preserved.
Tests
test_flood_negative_gap_differing_datato assert the new (correct)behaviour: the earlier event is clipped away, leaving only the later event.
test_flood_zero_duration_chain_does_not_leave_overlaps(the exactreproducer from the bug report).
test_flood_normalization_preserves_non_overlapping_tail— ensures thepass does not disturb events that follow the overlap.
test_flood_normalization_merges_same_data_after_zero_duration_event—covers the equal-data merge path.
All 174 tests pass; Ruff clean; targeted mypy clean.
Related