fix(transform): return merge_events_by_keys groups in first-seen order

[ErikBjare]

merge_events_by_keys returned HashMap::into_values(), so the order of the merged events changed from run to run. Order-sensitive transforms downstream inherit that. In aw-webui's multidevice query (ActivityWatch/aw-webui#1004), Android events are merged by app before union_no_overlap. The merged events overlap, so the union's result depended on the HashMap order. The query parity suite (ActivityWatch/activitywatch#1467) showed it: three runs of the same query gave three different aw-server-rust results.

The groups now come out in the order their first event appears in the input, which is what aw-core (Python) does. This uses a HashMap<String, usize> index into a Vec, so there's no new dependency. Grouping, the kept first payload and the summed durations are unchanged.

Also fixes the doc comment: events missing a key are dropped, but Example 1 listed one ({ "b": 1 }) in the output (noted in ActivityWatch/activitywatch#1466).

Test: merge_keeps_first_seen_order (10 groups plus repeats). It fails on the old code and passes now. cargo test -p aw-transform merge: 7 passed.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-26T21:46:19.097750Z	bd04de3	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Changes event merging to preserve input order instead of hash order.

The PR appears safe to merge, with a non-blocking clarification needed in the transform documentation.

Findings

1.  First-seen order is not chronological ▶

Summary

The PR replaces unordered map output with first-seen group order, corrects the missing-key documentation example, and adds an ordering regression test.

• Grouping, first-event payload, and summed durations remain unchanged.
• The documentation should distinguish deterministic order from the chronological order required by a downstream union.

_{Reviews (1) · Last reviewed commit: "fix(transform): return merge_events_by_k..."}

[greptile-apps]

First-seen order is not chronological

The reference to union_no_overlap suggests that deterministic first-seen order is enough for that transform. If the input events are not chronological, the merged groups will not be chronological either. Passing them directly to union_no_overlap, which processes each input in timestamp order, can produce incorrectly ordered coverage. Please clarify that callers must sort when the union needs chronological input.

Suggested change

	/// the result is deterministic (order-sensitive transforms such as
	/// union_no_overlap depend on it).
	/// the result is deterministic. This is not necessarily timestamp order; sort
	/// by timestamp before passing these events to union_no_overlap.

Knowledge Base Used: Event transformation operators

[ErikBjare]

Agreed, fixed in e47a055: the doc now says first-seen order is deterministic but not necessarily timestamp order, and that callers must sort by timestamp before union_no_overlap (which expects chronological input). Behaviour unchanged: first-seen order matches Python's merge_events_by_keys.

[TimeToBuildBob]

🤖 AI code review

The PR changes merge_events_by_keys in aw-transform/src/merge.rs to return merged events in first-seen order by using a HashMap<String, usize> index into a Vec instead of collecting HashMap values. It also updates the doc comment to clarify that events missing keys are dropped and that output order is first-seen, not timestamp order. A new test merge_keeps_first_seen_order verifies the ordering and duration merging.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No thread-worthy findings. Advisory notes follow; they are retained without opening review threads.

1 advisory finding (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — aw-transform/src/merge.rs

This is a fix(...) PR but no test files are included in the diff. Erik's feedback: 'where is the repro & fixes they are supposed to catch' (gptme#3441), 'that measurement should come with a regression test' (gptme#3446). Add a test that would have caught this bug. (Advisory: Erik merged all such PRs but consistently requested tests.)

Add a test file that reproduces the bug before the fix and passes after it.

_{How this was verified: static preflight: fix-commit + touched-files scan (rule 7)}

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

• aw-transform/src/merge.rs — Replaces HashMap<Event> with HashMap<String, usize> index into Vec to preserve first-seen order; updates doc comment and adds a test for ordering.

Previous review passes

commit	score	findings	engine	when
bd04de3055cf	5/5	0	llm	2026-09-26 22:06 UTC

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

_{Maintainer commands}

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