feat: add port option

[BelKed]

Important

Adds --port option to main and start in aw_notify/main.py for custom ActivityWatch server port, with default 5600 or 5666 for testing.

• Behavior:
  • Adds --port option to main and start functions in aw_notify/main.py to specify custom ActivityWatch server port.
  • Default port is 5600, or 5666 for testing mode.
• Functions:
  • Introduces common_options decorator to consolidate shared Click options.
  • Updates ActivityWatchClient initialization to use specified port in start function.

^{This description was created by for d8e96c5. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to dbb5668 in 2 minutes and 1 seconds

More details

• Looked at 42 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. aw_notify/main.py:329

• Draft comment:
  Annotate 'port' as Optional[int] since it defaults to None.
• Reason this comment was not posted:
  Marked as duplicate.

2. aw_notify/main.py:331

• Draft comment:
  The default port assignment (5666 if testing else 5600) appears in 'main'; consider extracting it to a helper or constant to avoid duplicated logic.
• Reason this comment was not posted:
  Marked as duplicate.

3. aw_notify/main.py:352

• Draft comment:
  Duplicate default port logic in 'start'; consider reusing a common helper to determine the default port.
• Reason this comment was not posted:
  Marked as duplicate.

4. aw_notify/main.py:328

• Draft comment:
  Enhance the '--port' option help text to mention that the default is 5666 in testing and 5600 otherwise.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

5. aw_notify/main.py:238

• Draft comment:
  Typo: 'wether' should be corrected to 'whether' for clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. aw_notify/main.py:540

• Draft comment:
  Typo in docstring: 'on a new days' should be changed to 'on a new day' for proper grammar.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_RLfKji20ZoCtms09

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to daa59fe in 1 minute and 22 seconds

More details

• Looked at 35 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. aw_notify/main.py:343

• Draft comment:
  Update the start() function docstring to document the new 'port' argument, and consider consolidating shared options between commands.
• 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 suggesting a reasonable improvement - documenting a new parameter in the docstring. The docstring is quite minimal currently. However, the port parameter already has a clear help text in the click.option decorator. In a CLI tool like this, the click help text is the primary documentation that users see. Duplicating it in the docstring provides little additional value.
  The comment also suggests "consolidating shared options between commands" which is a broader refactoring suggestion that isn't clearly actionable. The docstring update itself may be redundant with the click help text.
  While documenting parameters is generally good practice, in this case the click.option already provides clear documentation that is more useful to users than a docstring would be. The consolidation suggestion is too vague.
  This comment should be deleted. The port parameter is already well-documented via click.option and duplicating it in the docstring provides little value. The consolidation suggestion is not actionable enough.

2. aw_notify/main.py:328

• Draft comment:
  Added new --port option to the main command. Consider documenting the expected default behavior when no port is provided (None) and ensuring that ActivityWatchClient handles it appropriately.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

3. aw_notify/main.py:343

• Draft comment:
  For consistency and improved type-safety, annotate the 'port' parameter in the start() function as Optional[int] (e.g., def start(testing: bool = False, port: Optional[int] = None):).
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

4. aw_notify/main.py:238

• Draft comment:
  Typo: Replace 'wether' with 'whether' in the comment regarding the alert being 'positive'.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. aw_notify/main.py:534

• Draft comment:
  Grammatical suggestion: In the docstring for start_new_day, change 'on a new days' to 'on a new day' for clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_H7wyWCNfBJtcirSe

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

Consider updating the main() docstring/comment to mention the new 'port' option. Also, sharing common options (like testing and port) via a decorator could reduce duplication.

[ErikBjare]

Not sure I agree with this, but I'll accept it.

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to d8e96c5 in 2 minutes and 8 seconds

More details

• Looked at 61 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 7 drafted comments based on config settings.

1. aw_notify/main.py:327

• Draft comment:
  The port option help text mentions a default (5600 or 5666 for testing) but the option is defined with default=None. Consider having logic to set the default based on the testing flag, or document that the default is handled later.
• 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 raises a valid point about documentation accuracy. However, looking at the code, the default port handling is likely done inside ActivityWatchClient, which is a reasonable design. The help text accurately describes the end behavior even though the default is technically None in this layer. The comment suggests either changing the implementation or documentation, but neither change would improve the code meaningfully.
  The comment identifies a real technical discrepancy. Maybe the current design of delegating default port selection to ActivityWatchClient could be problematic in some way I'm not seeing?
  While technically correct, the current design is clean - it delegates port selection to the client library rather than duplicating that logic. The help text describes the end behavior correctly which is what matters for users.
  Delete the comment. While technically accurate, the current design is reasonable and the help text correctly describes the behavior users will experience. Making changes based on this comment would not meaningfully improve the code.

2. aw_notify/main.py:340

• Draft comment:
  Good reuse of a common options decorator for main and start commands. Consider using the same decorator for all commands (e.g., checkin) for consistency.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

3. aw_notify/main.py:324

• Draft comment:
  The common_options decorator is a nice reuse. However, the --port option has default=None while its help text mentions default values (5600/5666). Consider applying a concrete default value or handling None explicitly to align with the help message.
• 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%
  The comment appears to be incorrect. The help text accurately describes the defaults that will be used by ActivityWatchClient, while keeping the port parameter optional (None) allows the client to handle the default logic. This is actually a good pattern - it allows the client to handle the default port selection based on the testing flag. Setting a concrete default here would make it harder to have different defaults for testing vs non-testing mode.
  Maybe I'm missing something about Click's option handling - could there be a benefit to having the default value explicit in the CLI layer rather than delegating to the client?
  No, delegating default handling to the client is the right approach here since the default depends on the testing flag. The help text properly documents the defaults while keeping the parameter optional.
  The comment should be deleted. The current implementation correctly delegates default port handling to the client while documenting the defaults in the help text.

4. aw_notify/main.py:358

• Draft comment:
  When invoking the start command, the port parameter is passed through ctx.invoke. Ensure that if port is None, the intended default behavior is correctly applied downstream.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the PR author to ensure that the default behavior is correctly applied if the port is None. This falls under asking the author to ensure behavior, which is against the rules.

5. aw_notify/main.py:362

• Draft comment:
  In the start command, the ActivityWatchClient is created with the port parameter. Verify that the client handles a None port appropriately or consider setting a default value based on the testing flag.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the author to verify the behavior of the ActivityWatchClient when a None port is passed. This is a request for confirmation of behavior, which violates the rule against asking the author to confirm their intention or ensure behavior is intended. The comment does not provide a specific code suggestion or ask for a specific test to be written, which would have been acceptable.

6. aw_notify/main.py:238

• Draft comment:
  Typo detected: 'wether' should be corrected to 'whether' in the comment describing if the alert is positive.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. aw_notify/main.py:554

• Draft comment:
  Typo detected: In the docstring for 'start_new_day()', change 'on a new days' to 'on a new day' for proper grammar.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_k5JYp3CkOojybt9r

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ErikBjare]

Thanks! :)