[TimeToBuildBob]

Ports aw-core's merge_subwatcher_fields(base, sub, keys, conflict) to aw-transform and registers it as a query2 function. This is the backend half of ActivityWatch/activitywatch#352 (categorize by URL): aw-webui's canonicalEvents() can only call it once the default server has it. Supersedes the 2020 approach in #179, per the discussion there.

Semantics (same as aw-core):

• Base events are split at clipped subwatcher boundaries; each slice gets only the fields present during that slice.
• If several subwatcher events cover a slice, the latest-starting one wins (ties: the one ending last).
• Adjacent segments with equal data collapse, and total base duration is preserved.
• conflict is optional: "base_wins" (default, purely additive) or "sub_wins".

Differences from the Python implementation:

• An invalid conflict returns a query error, not a panic.
• A subwatcher event that only touches a slice boundary doesn't count as covering it (strict overlap).

Tests: all nine test_merge_subwatcher_fields_* cases from aw-core's tests/test_transforms.py, translated, plus a query-level test of the 3- and 4-argument forms and the invalid-conflict error.

cargo test -p aw-transform -p aw-query   # all pass
cargo fmt --all -- --check               # clean

cargo clippy -D warnings fails on two things this PR doesn't touch, and both also fail on master: aw-query/src/interpret.rs:439 (field_reassign_with_default) and the aw-transform bench's deprecated chunk_events_by_key call. Clippy reports nothing in the changed files.

The aw-webui wiring (browser url/$domain into canonicalEvents()) is a follow-up PR.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; all previous findings are addressed and no new actionable issues were found.

Summary

Adds merge_subwatcher_fields to aw-transform and makes it available to queries.

• Queries can add matching subwatcher fields to base events.

No new actionable issues were found. Tests were inspected, not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Base events] --> C[Sort bases and retain original indices]
    B[Subwatcher events] --> D[Sort subwatchers]
    C --> E[Find overlapping events]
    D --> E
    E --> F[Split bases and copy selected fields]
    F --> G[Collapse equal neighboring slices]
    G --> H[Restore original base order]

Loading

_{Reviews (2) · Last reviewed commit: "fix(transform): sweep expired subwatcher..." · Reviewed by Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 97.05882% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.41%. Comparing base (656f3c9) to head (edb3554).
⚠️ Report is 218 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-query/src/functions.rs	90.00%	2 Missing ⚠️
aw-query/tests/query.rs	97.82%	1 Missing ⚠️
aw-transform/src/merge_subwatcher_fields.rs	98.57%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #824       +/-   ##
===========================================
+ Coverage   70.81%   85.41%   +14.60%     
===========================================
  Files          51       88       +37     
  Lines        2916    12613     +9697     
===========================================
+ Hits         2065    10774     +8709     
- Misses        851     1839      +988     

☔ View full report in Codecov by Harness.
📢 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.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

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.

[TimeToBuildBob]

🤖 AI code review

This PR ports aw-core's merge_subwatcher_fields transform to Rust, adding a new module in aw-transform and registering it as a query function in aw-query. It also adds unit tests in aw-transform and an integration test in aw-query covering the 3- and 4-argument forms and invalid conflict handling.

Safe to merge — no P0/P1 findings

Confidence 5/5

_{ℹ️ Consensus was degraded on this run: 1 of 3 passes answered, so findings were filtered at 1-of-1 agreement rather than 2-of-3 — less filtered than usual; 1 of 3 fan-out jobs answered, so the findings above were filtered against less evidence than the threshold assumes.}

✅ No findings in this PR's changed files. What follows is outside them — not about this change, and not scored.

1 out-of-scope finding (outside this PR's changed files)

Each item here is an observation about the PR as a whole — informational only, not scored.

⚠️ P2 medium — PR-level

The sweep-line active-set logic can drop a subwatcher event that starts exactly at a base event's start time when that subwatcher event also ends exactly at the base start. In the loop at lines 62-66, subwatcher events with timestamp <= base_start are added to active and their endings pushed. Then the expiry loop at lines 67-73 pops any ending <= base_start. If a subwatcher event has timestamp == base_start and duration == 0 (or ends at base_start), it is added and immediately removed, so it never appears in overlapping. More importantly, consider a subwatcher event that starts before base_start and ends exactly at base_start: it is added in the first loop (timestamp <= base_start) and then removed in the expiry loop (end <= base_start), which is correct because it doesn't overlap. But the same removal also affects a subwatcher event that starts exactly at base_start and has zero duration — such an event should not contribute anyway. The real issue is that the expiry loop removes events that end at base_start, but the overlap check later uses sub_end <= base_start to skip them, so this is consistent. However, the active set is shared across base events and the expiry loop only removes events whose end <= current base_start. For a later base event, an event that ended before that base_start is removed, but an event that started before and ends after is kept. This seems correct. The potential bug is that the candidates chain includes sub_sorted[next_sub..] with take_while(|(sub, _)| sub.timestamp < base_end). This includes subwatcher events that start after base_start but before base_end, even if they are already in active? No, next_sub is the index after all events with timestamp <= base_start, so the chain only includes events with timestamp > base_start. Those are not in active yet, but they are considered as candidates. For each such event, the overlap check sub_end <= base_start is false (since sub_end > sub.timestamp > base_start), and start = sub.timestamp.max(base_start) = sub.timestamp, end = sub_end.min(base_end). If sub.timestamp < base_end, it overlaps. This is fine. The active set is only used to avoid re-adding events that have already been added for earlier bases. But the expiry loop removes events that ended before the current base_start, which is correct. However, there is a subtle bug: the expiry loop only removes events from active when their end <= base_start, but it does not remove events that end between base_start and base_end. Those remain active for the current base, which is correct. For the next base, they may be removed if their end <= next base_start. This seems correct. I cannot find a concrete bug in the sweep logic. Let me look at the boundary handling: boundaries starts with [base_start, base_end], then for each overlapping sub event, it pushes start and end. After sorting and dedup, windows are consecutive. For each window, the best sub is chosen by checking sub.timestamp.max(start) >= sub_end.min(end). This checks if the sub covers the entire window. If a sub starts after the window start, then sub.timestamp.max(start) = sub.timestamp, and if sub.timestamp >= end, it doesn't cover. If sub ends before window end, sub_end.min(end) = sub_end, and if sub_end <= start, it doesn't cover. This is correct. The best selection prefers later start, then later end. This matches the spec. The collapse logic merges adjacent segments with equal data. This is correct. I don't see a clear bug. Let me check the test for test_unsorted_overlapping_bases_keep_input_order: base events are at 20-30, 0-30, 10-20. Sorted by timestamp: base[1] (0-30), base[2] (10-20), base[0] (20-30). For base[1] (0-30), sub events: a (0-10), b (10-20), c (20-30). All overlap. Boundaries: 0,10,20,30. Windows: 0-10 (a), 10-20 (b), 20-30 (c). Segments: long/a, long/b, long/c. For base[2] (10-20), sub events: b (10-20) overlaps, a (0-10) ends at 10, c (20-30) starts at 20. Only b overlaps. Boundaries: 10,20. One segment: middle/b. For base[0] (20-30), sub c (20-30) overlaps. Segment: late/c. Output order: by_base[0] = late/c, by_base[1] = long/a, long/b, long/c, by_base[2] = middle/b. Flattened: late/c, long/a, long/b, long/c, middle/b. The test expects exactly that. So the algorithm works for that case. I need to find a real bug. Let me think about the case where a subwatcher event starts before base_start and ends after base_end, and another subwatcher event starts after base_start but before base_end. The first is in active, the second is in the chain. Both are in overlapping. For a window that starts after the second's start, the second may win. That's correct. What about a subwatcher event that starts before base_start and ends exactly at base_start? It is added to active and then removed in the expiry loop. It is not in overlapping. That's correct because it doesn't overlap. What about a subwatcher event that starts exactly at base_start and ends after base_start? It is added to active, not removed (end > base_start), and is in overlapping. Correct. What about a subwatcher event that starts before base_start and ends exactly at base_start? Removed, not overlapping. Correct. What about a subwatcher event that starts exactly at base_start and ends exactly at base_start (zero duration)? Added, then removed, not overlapping. Correct. So the sweep seems correct. Let me look at the candidates chain: active.iter().map(|&i| &sub_sorted[i]).chain(sub_sorted[next_sub..].iter().take_while(|(sub, _)| sub.timestamp < base_end)). The take_while stops at the first sub with timestamp >= base_end. But what if there are sub events with timestamp < base_end that are already in active? They are not in next_sub.. because next_sub is the index after all with timestamp <= base_start. So any sub with timestamp > base_start and < base_end is in the chain. That's fine. But what about a sub with timestamp == base_start? It was added to active in the first loop (since <= base_start). It is not in the chain. So it appears only via active. Good. What about a sub with timestamp < base_start but not yet added? All subs with timestamp <= base_start are added in the first loop, so they are in active. So the chain only contains subs with timestamp > base_start. That's correct. I don't see a bug in the sweep. Let me check the overlapping vector: it collects all subs that overlap the base. For each, it pushes start and end into boundaries. But if a sub starts before base_start and ends after base_end, start=base_start, end=base_end, which are already in boundaries. So no new boundaries. That's fine. If a sub starts before base_start and ends in the middle, start=base_start, end=sub_end, so sub_end is added. If a sub starts in the middle and ends after base_end, start=sub_start, end=base_end, so sub_start added. This creates boundaries at all sub start/end points within the base. Correct. The best selection for each window: it iterates over all overlapping subs and checks if the sub covers the entire window. For a sub that starts before the window start and ends after the window end, it covers. For a sub that starts exactly at window start, it covers if it ends after window end. The condition sub.timestamp.max(start) >= sub_end.min(end) — if sub.timestamp <= start and sub_end >= end, then max = start, min = end, so start >= end is false (since start < end), so it covers. If sub.timestamp > start, then max = sub.timestamp, and if sub.timestamp >= end, it doesn't cover. If sub_end < end, then min = sub_end, and if sub_end <= start, it doesn't cover. This is correct. So the algorithm seems correct. I need to find a real issue. Let me look at the query function in aw-query/src/functions.rs. It checks args.len() != 3 && args.len() != 4. That's fine. It converts args[0] and args[1] to Vec<Event> via try_into. It converts args[2] to Vec<String>. It converts args[3] to String if present. It calls aw_transform::merge_subwatcher_fields and maps errors to InvalidFunctionParameters. That's fine. The test in query.rs: the first query uses `flood(query_bucket(

Files changed (4) — the diff as I read it

• aw-query/src/functions.rs — Registers merge_subwatcher_fields as a query function and implements its argument parsing and error mapping.
• aw-query/tests/query.rs — Adds test_merge_subwatcher_fields integration test covering 3-arg, 4-arg, and invalid conflict cases.
• aw-transform/src/lib.rs — Exports the new merge_subwatcher_fields module and function.
• aw-transform/src/merge_subwatcher_fields.rs — Implements the merge_subwatcher_fields transform with sweep-line segmentation and conflict handling, plus unit tests.

Previous review passes

commit	score	findings	engine	when
edb35546a0ea	4/5	1	llm	2026-10-19 13:19 UTC

_{Reviewed edb35546a0ea · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 123s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}