fix: refactored flood to better behavior

[ErikBjare]

I've had this in my working tree for a while. Should be improved behavior better aligned with how aw-server-rust works. Will hopefully fix ActivityWatch/activitywatch#626.

(Needs thorough review, long time since I wrote this, and can't remember what I've done)

[codecov]

Codecov Report

❌ Patch coverage is 69.56522% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.16%. Comparing base (efd1325) to head (0b47c01).
⚠️ Report is 77 commits behind head on master.

Files with missing lines	Patch %	Lines
aw_transform/flood.py	69.56%	13 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
- Coverage   90.78%   90.16%   -0.63%     
==========================================
  Files          34       34              
  Lines        1660     1687      +27     
  Branches      257      262       +5     
==========================================
+ Hits         1507     1521      +14     
- Misses        118      131      +13     
  Partials       35       35              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ErikBjare]

Since this is based on the aw-server-rust implementation, I just want to note that it might not be without issues either: ActivityWatch/activitywatch#602

[ErikBjare]

@TimeToBuildBob I just merged #143, is this PR still "better" somehow or what?

[TimeToBuildBob]

Hey @ErikBjare — I compared the two:

#143 (merged) vs #105:

#143 adds a post-processing normalization pass that catches overlapping events at the output boundary — same-data events are merged, different-data events are clipped (later wins). This is a conservative, additive fix that doesn't change the pairwise loop behavior.

#105 takes a slightly different approach:

1. In-loop trimming — adds _trim(e1, e2) for the unsafe negative-gap branch instead of just warning, resolving overlaps during the pairwise pass rather than after.
2. Sort key change — sorts by (timestamp, duration) instead of just timestamp, which affects ordering when two events share the same timestamp.
3. Refactoring — extracts _flood/_flood_first/_trim helpers.
4. Idempotency test — test_flood_idempotent verifies repeated flood() calls produce the same result.

Verdict: #143 fixes the main problem (#1369 overlapping events causing >60min hours) and #105's normalization goal is now achieved. The sort-by-(timestamp, duration) change from #105 is a subtle improvement not included in #143, but the normalization pass in #143 would catch any resulting overlaps anyway.

I'd say this PR can be closed — #143 covers it. Happy to do that if you confirm.

[ErikBjare]

@TimeToBuildBob open a new PR with the remaining improvements. Compare with aw-server-rust implementation (which I think informed this) to make sure implementations work the same way (follow the same spec).

[TimeToBuildBob]

Short answer: probably close it, but there's a bit of substance here.

#105 takes a fundamentally different approach from #143 — it rewrites the pairwise loop logic itself (new _flood(), _flood_first(), _trim() helpers) rather than adding a normalization pass at the output. Both land on the same desired behavior for test_flood_negative_gap_differing_data (drop the earlier event), so they agree on what should happen.

What #105 has that #143 doesn't:

• An idempotency test (test_flood_idempotent) — worth checking if current master + fix(transform): normalize flood() output to remove overlapping events #143 passes it
• pulsetime parameter already baked in at the transform level (separate from fix(query): expose pulsetime parameter in flood() query function #139's query-level version)

If you don't want to review the full refactor, I'd close it — #143's targeted normalization pass is cleaner and avoids touching pairwise logic that's been stable. But if you want to evaluate it, the idempotency test is the quickest probe: does master pass test_flood_idempotent?

[TimeToBuildBob]

Done — opened #145 with the remaining improvements from this PR.

What's in #145:

• Sort key (timestamp, duration) instead of just timestamp — aligns with aw-server-rust's behavior for same-timestamp events
• test_flood_idempotent — verifies repeated flood() calls are stable
• test_flood_unsafe_gap — verifies overlapping differing-data events don't double-count time (validates fix(transform): normalize flood() output to remove overlapping events #143's normalization pass)

aw-server-rust comparison: The sort change aligns the implementations. One remaining divergence: for positive gaps with differing data, Rust does "meet in the middle" (each event extends by gap/2) while Python does "longer wins." The normalization pass from #143 means both produce overlap-free output, but the midpoint differs. Noted that in the PR description — happy to address it in a follow-up if you want full spec parity.

Closing this PR since the improvements are captured in #145.

[ErikBjare]

@TimeToBuildBob Closed this PR, open a new one addressing the "One remaining divergence: for positive gaps with differing data, Rust does "meet in the middle" (each event extends by gap/2) while Python does "longer wins." The normalization pass from #143 means both produce overlap-free output, but the midpoint differs. Noted that in the PR description — happy to address it in a follow-up if you want full spec parity."

I'm not sure if longer should win or if they should meet in middle, make a case for both and then decide (decides wgether to make PR to aw-server-rust or aw-server-python). Regardless, the spec/intended/designed behavior should be documented in docs for flooding.

[TimeToBuildBob]

Implemented the requested decision/spec follow-up in #146.

I chose meet in the middle. Longer-wins is defensible only if surrounding event duration is a confidence signal, but it isn't: heartbeat batching and prior transforms can make one side longer for unrelated reasons. Midpoint is symmetric, makes the weaker assumption about the unobserved transition, and keeps Python aligned with Rust's existing tested behavior.

#146 also expands the public flood() docstring into the behavioral spec (ordering, pulsetime boundary, equal-data merge, differing-data midpoint, overlap precedence) and adds an unequal-duration exact-boundary regression. CI is green and Greptile is 5/5.