Fix: improve tracking accuracy

[zbw8388]

Hi, thank you for making this extension! I recently noticed this, and it is especially dominant when I set a higher poll time:

Upon investigation, it seems like the zero duration caused the issue. If window changes within one polling block, that entry will have a total time of zero, resulting in inaccuracies.

This zero duration was used in other places such as web tracker. However, they hook onto events (such as tab changes) making the duration unknown to them when generating events. They were able to patch up the event time by sending two consecutive events: https://github.com/ActivityWatch/aw-watcher-web/blob/0b289c4208f308050979fc729a69cf0c57dd7d8f/src/eventPage.js#L58-L63

In our case, however, it is not necessary to store the previous event as we know approximately how long is each event. If you were to look at database closely, you will find that there's still minor inaccuracies as we used await after the timer fires. This is much minor and I'll be happy to take such a compromise. (it should be easy to fix it by removing the await, though we need to deal with duration computation more carefully).

Here's the result after the fix:

[2e3s]

Hello, good research! I also noticed some shady juggling but it was in AFK watcher.
Does this problem exist in the original Python window watcher? I have mostly replicated its logic https://github.com/ActivityWatch/aw-watcher-window/blob/master/aw_watcher_window/main.py#L141
I also noticed some timing warnings from AW server, I'll check it a bit later this week.

[zbw8388]

I tested on a windows machine and yes the issue is still there. It only becomes apparent when you have a higher window poll time (5s in my case). I did it initially to reduce the database size as the database can get quite large... The way to reproduce this bug is by having a different window every time it polls (which is quite common when going through websites).

They explained the zero part here: https://github.com/ActivityWatch/aw-server/blob/90c8b30f3becd622e982006901b5e1df3a76f8e0/aw_server/api.py#L256, but they'll need to consider the duration of the previous event when seeing a new event as having a duration of zero does not make sense. Changing the duration is one way of fixing it when we don't want to touch code for handling the heartbeat.

Interestingly, they implemented the afk module with duration. https://github.com/ActivityWatch/aw-watcher-afk/blob/caa6e792370801554100348db4ceb9c960aaef1f/aw_watcher_afk/afk.py#L102 (though it is true that the timestamp is not now)

[2e3s]

I did it initially to reduce the database size as the database can get quite large

The database doesn't grow if the heartbeat happens on the same app+title. Instead, the event is supposed to be updated to a longer duration: calculate -> merge. So if everything goes correctly and no debug messages are posted about inability to merge, you can't reduce the database with longer loops otherwise than by skipping shorter events.
Skipping short events may be useful to slim down DB, but it could be more universal as a cleaning routine of the database, maybe a part of the server or the bundle. It would also avoid overcounting more than your proposed solution.

One other idea to reduce the DB is to intersect AFK with other events after some interval, like here. But it would reduce DB only if titles typically change when you're afk.

I tested on a windows machine and yes the issue is still there

So it's intentional. Fighting an original intention of a framework often ends up with hacks and problems.
It cannot be done on the server side because it doesn't know the poll times.
If there are a few different app+title within 1 loop, e.g. 5s loop starting from 100s firefox, heartbeat, 1s firefox, 3s vscode, 100s dolphin, as I understand,

• currently there will be a hole for 5s
• after your change firefox gets +4s of extra usage

which isn't inherently more correct than 0s, albeit the timeline may look better indeed.

So I'm split on this issue. It would be much clearer if aw-watcher-window had this too. The authors are on it for many years, they should know this domain better.

Some watchers are reactive like aw-watcher-web, namely Wayland and KWin (theoretically, Gnome and even X11 could provide it too). It would be possible to make them send the heartbeats on app+title changes by a cost of higher complexity.

[zbw8388]

Thanks for explaining! I do agree this is a rather crude way of fixing this issue, so I'll close the pr. Thanks a lot for looking into this! Here're my responses:

you can't reduce the database with longer loops otherwise than by skipping shorter events

Yes that is true. I intended to skip those shorter events. I found lots of less-than-one-second events for some reason when I first used this software. However, as I logged more hours, it seems like most events last over that so it should log things semi-correctly.

So it's intentional. Fighting an original intention of a framework often ends up with hacks and problems.

Yes it was intentional for aw-watcher-window. However, they opted for a different approach in aw-watcher-web and were trying to fight the framework to some degree by sending two messages for one heartbeat: ActivityWatch/aw-watcher-web@f526f38#diff-6e815714d80f53fd92e3e15783661976fcfd4dd7ef82410c2e31f3f8cdd5961bR51. They send a heartbeat to finish the event first, and then open a new event entry, resulting in an accurate time logging. We could do that in aw-watcher-window, but it would be more complicated than proposed approach and both achieve a similar result.

It cannot be done on the server side because it doesn't know the poll times.

The server can just do what the web watcher is doing: make the past event end one microsecond before the current event. We don't have to worry about it making the past event too long, as the server already has a timeout-ish config, pulsetime, that prevents events from merging when they are too distant apart: https://github.com/ActivityWatch/aw-server/blob/90c8b30f3becd622e982006901b5e1df3a76f8e0/aw_server/rest.py#L280
It could be changing the signature of heartbeat function to make sure it returns different things for 1. merge-able events 2. non-merge-able events but the previous event is extendable and 3. non-merge-able events here: https://github.com/ActivityWatch/aw-server-rust/blob/75381632406b1e442fed77940da718a338e2923f/aw-transform/src/heartbeat.rs#L53

If there are a few different app+title within 1 loop, e.g. 5s loop starting from 100s firefox, heartbeat, 1s firefox, 3s vscode, 100s dolphin, as I understand,

In my situation, since I don't filter out website titles, it will be more like

5s website A in firefox,
5s website B in firefox,
5s file C in vscode,
5s website D in firefox etc.

so there will be patches of emptiness on the screen

[2e3s]

Thanks anyway! There was an actual small fix on 98 line, maybe you could've kept it alone, if not, I'll do it myself later.

However, they opted for a different approach in aw-watcher-web and were trying to fight the framework

Not really, this logic is appropriate there because it happens on tabs switch, so we know when the last event ends for sure. Therefore, no excessive time is counted in. Look at the next condition branch too by your link. This is how I would do it with a reactive (event-based) approach, I'll possibly try to implement it for Wayland and KWin later within a few weeks. I thought about it when coding the watchers, but not much since aw-watcher-window was the guide.

We could do that in aw-watcher-window, but it would be more complicated

Not sure it's even realistic for X11. Certainly more complex.

I do agree this is a rather crude way of fixing this issue, so I'll close the pr.

I would encourage you, if you have time and interest, to ask about this issue in aw-watcher-window or aw-server-rust and suggest your proposal to see if it's okay. Maybe your treatment of these gaps in the timed polling based approach is better than it is now. I'm just not sure about it.