Skip to content

Conversation

@MrJarnould
Copy link
Contributor

@MrJarnould MrJarnould commented Nov 1, 2025

Important

Introduces a watch command to stream Apple's Screen Time data into ActivityWatch in real-time, with a comprehensive CLI and enriched event data.

  • Behavior:
    • Implements watch command in __main__.py to stream Screen Time data into ActivityWatch in real-time.
    • Uses watchdog to detect SEGB file changes and processes only new data.
    • Enriches events with app titles using iTunes API.
  • CLI:
    • Adds commands: devices, file, events, and watch in __main__.py.
    • Supports options for device filtering, time zone, and server configuration.
  • Dependencies:
    • Updates pyproject.toml to include ccl-segb, protobuf, rich, typer, dateparser, and watchdog.
  • Protobuf:
    • Defines AppInFocusEvent in app_in_focus_extended.proto and app_in_focus_extended_pb2.pyi.
  • Misc:
    • Updates README.md with installation, usage, and command details.
    • Removes main.py and replaces it with a more modular structure in src/aw_import_screentime.

This description was created by Ellipsis for 090cfc1. You can customize this summary. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to 090cfc1 in 3 minutes and 37 seconds. Click for details.
  • Reviewed 2512 lines of code in 11 files
  • Skipped 3 files when reviewing.
  • Skipped posting 7 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .python-version:1
  • Draft comment:
    The file contains only '3.13'. Consider ensuring the file ends with a newline for POSIX compliance.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
2. README.md:1
  • Draft comment:
    The README is comprehensive; consider adding a brief note about prerequisites (e.g. full disk access on macOS) and possibly linking to detailed setup instructions.
  • Reason this comment was not posted:
    Confidence changes required: 30% <= threshold 50% None
3. pyproject.toml:14
  • Draft comment:
    The dependency for 'typing-extensions' is specified as '>=4.12.2' for python_version >= '3.13', but later in uv.lock it requires version 4.15.0. Consider aligning these version requirements for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment references a lock file (uv.lock) that is not part of the diff being reviewed. The rules explicitly state to "Ignore cross-file issues. Only think about the file you are reviewing." Additionally, the rules say "Do NOT comment on dependency changes, library versions that you don't recognize, or anything else related to dependencies." This comment is about aligning dependency versions between pyproject.toml and a lock file, which is a cross-file concern. Lock files are typically auto-generated and the version in the lock file being higher than the minimum specified in pyproject.toml is normal and expected behavior - the lock file pins to a specific version while pyproject.toml specifies minimum requirements. Could there be a legitimate reason to align these versions? Perhaps having a higher minimum version requirement would prevent compatibility issues. However, the comment doesn't explain why this alignment is necessary or what problem it would solve. Even if there might be a theoretical benefit to alignment, this is fundamentally a cross-file issue involving a lock file that's not in the diff. The rules are clear about ignoring cross-file issues and dependency-related comments. The comment also doesn't demonstrate any actual problem - having a lock file with a higher version than the minimum requirement is standard practice. This comment should be deleted because it's a cross-file issue referencing uv.lock which isn't in the diff, and it's about dependency versions which the rules say not to comment on. There's no evidence of an actual problem.
4. src/aw_import_screentime/__main__.py:327
  • Draft comment:
    In iter_app_in_focus_events, a generic Exception is caught when parsing protobuf data. Consider catching more specific exceptions to avoid masking critical errors.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is about a code quality refactor - suggesting more specific exception handling. According to the rules, "Comments that suggest code quality refactors are good! But only if they are actionable and clear." This comment is somewhat actionable (catch specific exceptions) but not very specific about WHICH exceptions to catch. The author would need to know what specific exceptions ParseFromString can raise. Also, the current pattern of catching Exception and logging at debug level is a reasonable defensive programming approach for parsing potentially malformed binary data. The comment doesn't provide strong evidence that the current approach is wrong - it's more of a stylistic preference. Given the rule "you must see STRONG EVIDENCE that the comment is correct in order to keep it" and that this is more of a preference than a clear issue, I'm leaning toward deleting it. The comment could be valid if catching generic Exception is masking critical errors. However, in this context (parsing binary protobuf data from files), being defensive and continuing on any parse error seems reasonable. The comment doesn't specify which exceptions should be caught, making it less actionable. While the comment has some merit as a code quality suggestion, it lacks specificity about which exceptions to catch and doesn't demonstrate that the current approach is problematic. The defensive pattern used here (catch all, log, continue) is appropriate for parsing potentially malformed binary data. This comment should be deleted. It's a generic code quality suggestion without strong evidence that the current approach is wrong. The comment doesn't specify which exceptions should be caught, and the current defensive pattern is reasonable for parsing binary data.
5. src/aw_import_screentime/__main__.py:482
  • Draft comment:
    The stitch_intervals function deliberately does not close the last open interval, as noted in the docstring. It might help to explicitly mention in the README or comments that open intervals require a subsequent run to be closed.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50% None
6. src/aw_import_screentime/__main__.py:1000
  • Draft comment:
    In the read_new_events function, the logic for handling file rotation (comparing state.last_file with newest) works well, but consider documenting potential race conditions if a file is modified during reading.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
7. src/aw_import_screentime/ccl_segb_cli.py:43
  • Draft comment:
    The ccl_segb_cli CLI decodes protobuf messages using blackboxprotobuf.decode_message without additional error handling. It might be useful to wrap this in a try/except to handle potential decoding errors gracefully.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment suggests adding error handling, which is a "might be useful" suggestion rather than identifying a definite bug or issue. The comment uses the phrase "might be useful" which indicates it's speculative or optional advice, not a required change. According to the rules, comments should not be speculative and should only be made if there's clearly a code change required. This appears to be a code quality suggestion rather than identifying a concrete problem. The code may work fine without the error handling - we don't have evidence that decode_message will definitely fail or that the current behavior is incorrect. This falls into the category of "nice to have" rather than "must fix". However, error handling in CLI tools is generally good practice, and if protobuf decoding can fail on malformed data, this could be a legitimate issue. The comment might be pointing out a real robustness concern rather than just a speculative improvement. While error handling is good practice, the comment uses "might be useful" language which makes it speculative rather than definitive. The rules explicitly state not to make speculative comments. Without evidence that this will definitely cause problems, this is more of a code quality suggestion than a required fix. This comment should be deleted because it's a speculative suggestion ("might be useful") rather than identifying a definite issue. It doesn't meet the threshold of "clearly a code change required" as specified in the rules.

Workflow ID: wflow_YLEhXdVr6BRCNeLM

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant