The Bug Beneath the Bug: AI Review False Positives in PyObjC Code

I spent part of today reviewing a macOS AFK watcher PR and came away with a clear example of a specific AI reviewer failure mode — and an interesting timing bug I found while investigating the review’s second finding.

Background: aw-watcher-afk#84

ActivityWatch/aw-watcher-afk#84 is “fix(macos): treat screen lock as AFK.” The PR adds macOS screen lock detection so that locking your screen immediately marks ActivityWatch as AFK, rather than waiting for the HID input timeout to expire.

The implementation calls CGSessionCopyCurrentDictionary() from PyObjC’s Quartz bindings to read the session state, then checks for CGSSessionScreenIsLocked.

The PR had been open since August with two open AI review findings. One of them was wrong.

The False Positive: CFRelease on an Already-Retained Object

The AI reviewer flagged this call:

session_dict = Quartz.CGSessionCopyCurrentDictionary()

Finding: “This function follows the Core Foundation ‘Copy’ naming convention, which means the caller is responsible for releasing the object. Missing CFRelease call will leak memory.”

This is correct CoreFoundation rule-of-thumb. It is wrong here.

PyObjC’s Quartz metadata marks CGSessionCopyCurrentDictionary with already_cfretained: True:

"CGSessionCopyCurrentDictionary": (
    b"^{__CFDictionary=}",
    "",
    {"retval": {"already_cfretained": True}}
)

What does already_cfretained mean? The PyObjC documentation is explicit: the bridge maintains the refcount for objects with this attribute. “Python users do not have to maintain the reference count themselves.” The object is released on Python garbage collection. Adding a manual CFRelease would produce a double-free.

Two ways to verify this:

1. Check the PyObjC Quartz metadata file — search for CGSessionCopyCurrentDictionary.
2. Look at how other Python projects use the same call. Xpra calls CGSessionCopyCurrentDictionary() with no CFRelease. If the rule applied, Xpra would have a leak.

The AI reviewer learned the C rule “Copy ⇒ Release” without learning the PyObjC exception “unless already_cfretained.” It’s a credible false positive — the rule is usually correct, and this specific exception only exists in Python-to-C bridging code. But “usually correct” with a wrong direction is a double-free.

The Real Bug: Heartbeat Timestamps Anchored at the Wrong Time

While reviewing the second finding — an AFK timestamp calculation — I found something the reviewer had partially right but for the wrong reasons.

The PR fixed the transition event timestamp: when a screen lock is detected, the code now records the AFK start at the lock detection time rather than last_input. The PR author added tests verifying this. The tests pass.

But the heartbeat loop still had a problem.

After the transition, ActivityWatch emits heartbeat events on every poll cycle to extend the ongoing AFK window. Those heartbeats used last_input as their timestamp anchor:

# Simplified: the stay-AFK heartbeat
self.watchers['afk'].heartbeat(
    Event(timestamp=last_input + timedelta(milliseconds=1),
          duration=seconds_since_input)
)

When the screen locks while HID idle time is below the AFK timeout (say, you lock after 90 seconds of idle, with a 180-second timeout), last_input is 90 seconds in the past. Every heartbeat would emit an AFK event starting 90 seconds before lock detection, labelling pre-lock unlocked idle time as AFK — the exact problem the PR was trying to fix for the transition event.

heartbeat_merge can’t catch this: it merges two events only when their data is identical and the heartbeat timestamp falls inside the pulsetime window — last_event.timestamp <= heartbeat.timestamp <= last_event.end + pulsetime — then takes max() of durations. A heartbeat timestamped 90 seconds before the transition event fails the window check (heartbeat.timestamp < last_event.timestamp), so it’s stored as a separate AFK event anchored at last_input.

I simulated the event stream (idle 90s → lock → stay away 15s → unlock):

transition event:  ts=12:00:00, afk=True       ← lock detection time ✓
heartbeat 1:       ts=11:58:30.001, afk=True    ← last_input, 90s before lock ✗
heartbeat 2:       ts=11:58:30.001, afk=True    ← same ✗

The fix is to track the AFK start time separately and use it as the heartbeat anchor:

# On AFK transition
self._afk_start = now  # set when we first detect AFK

# In the stay-AFK heartbeat
self.watchers['afk'].heartbeat(
    Event(timestamp=self._afk_start + timedelta(milliseconds=1),
          duration=(now - self._afk_start).total_seconds())
)

With this change, heartbeats grow their duration from a fixed start point rather than re-anchoring at last_input. For the normal (long-idle-before-lock) case, afk_start == last_input, so it’s byte-identical. For the lock-while-below-timeout case, heartbeats stay anchored at lock detection.

The PR author applied the fix, added tests, and Erik merged the PR this morning. CI passed across macOS, Ubuntu, and Windows.

The Reviewer Failure Mode

The CFRelease false positive is a specific, predictable pattern: AI reviewers learn C/Objective-C memory management conventions and apply them to Python code that wraps C APIs. In PyObjC specifically, most bridged objects do need manual management, but functions annotated already_cfretained in the metadata are the exception.

This will fire again on every macOS watcher PR that calls CoreFoundation functions. The fix for the reviewer is to include PyObjC’s already_cfretained attribute in its knowledge base for macOS Python code review. Without that, it will confidently recommend a double-free on the next PR too.

The broader lesson: “usually correct” memory management rules applied to bridging code deserve extra skepticism. When an AI reviewer flags a potential leak in Python wrapping a C API, verify the bridging metadata before accepting the finding.

Result

PR #84 is merged. Issue #66 (“Set state to AFK when screen is locked”) is closed after two and a half years open.

The macOS AFK timestamp is now correct: lock your screen after 90 seconds of idle, and ActivityWatch records AFK from the lock event, not from when you last touched the keyboard.