diff --git a/AGENTS.md b/AGENTS.md index 66668c0..f55aa4b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,9 +45,13 @@ auxiliary bot's PRAW instance (`get_comment_with_different_praw_instance`). All - Trigger: Comment links to a target subreddit that already contains a post with the exact same URL/content or visually matching image. - Behavior: Detects existing posts via Reddit search and self-hosted perceptual image hashing (`imagehash`), replying with a link to the prior post. -> [!WARNING] -> Auxiliary bots reply to matching comments at **any depth**. The top-level-only check -> (`comment.parent_id == comment.link_id`) runs **after** the auxiliary dispatch and gates only the DB insert. +> [!NOTE] +> Only **top-level** comments (`comment.parent_id == comment.link_id`, a direct answer to the post) count as +> recommendations and get a reply from `same_subreddit_bot` or `same_post_bot`. `sub_doesnt_exist_bot` is the +> one bot that also answers replies to other comments. For those, Phase 1 only checks whether the target exists +> (`get_posts_with_same_content(..., existence_only=True)`: the URL search, no duplicate scan). +> Every bot requires the **whole** body to be the subreddit reference (the matching rule above). A comment +> edited after the bot answered (e.g. `r/name` later extended with `\n\nWait...`) keeps that answer. --- @@ -89,14 +93,18 @@ flowchart TD T -->|Yes| X3["ignore"] T -->|No| BL{"Source or target in\nSUB_BLACKLIST?\n(case-insensitive)"} BL -->|Yes| X4["ignore"] - BL -->|No| SS{"target == source\n(case-insensitive)?"} + BL -->|No| TL{"Top-level comment?"} + TL -->|No| SSN{"target == source\n(case-insensitive)?"} + SSN -->|Yes| X5["ignore"] + SSN -->|No| EX["get_posts_with_same_content(existence_only=True)\n1 URL search"] + EX -->|SUBREDDIT_DOES_NOT_EXIST| A3 + EX -->|otherwise| X6["ignore"] + TL -->|Yes| SS{"target == source\n(case-insensitive)?"} SS -->|Yes| A1["same_subreddit_bot reply"] SS -->|No| DUP["get_posts_with_same_content()\n1 URL search + 1 other-discussions call\n+ image/video-hash scan of\n100 newest + 100 all-time top posts"] DUP -->|posts_found| A2["same_post_bot reply"] DUP -->|SUBREDDIT_DOES_NOT_EXIST| A3["sub_doesnt_exist_bot reply"] - DUP -->|otherwise| TL{"Top-level comment?"} - TL -->|No| X5["ignore"] - TL -->|Yes| DB[("insert_scraped_comment(permalink, target)")] + DUP -->|otherwise| DB[("insert_scraped_comment(permalink, target)")] ``` Note: a private target (`SUBREDDIT_IS_PRIVATE`) is **still inserted** in Phase 1. Phase 2 deletes it later. @@ -106,7 +114,7 @@ A banned target (a 404 whose body says `"reason": "banned"`, `SUBREDDIT_IS_BANNE ### Phase Details - **Phase 1 ([`src/racb/phases/phase1.py`](./src/racb/phases/phase1.py)):** - Continuous streaming from `reddit.subreddit('all').stream.comments(skip_existing=True, pause_after=-1)`. - - Filters: Discards moderator comments, submissions with meta-phrases (`sub`, `subreddit`), and blacklisted subreddits ([`src/racb/constants.py`](./src/racb/constants.py), compared case-insensitively by `is_crosspost_blocked`). Non-top-level comments (`comment.parent_id != comment.link_id`) are discarded only **after** auxiliary-bot dispatch (see the warning in section 1). + - Filters: Discards moderator comments, submissions with meta-phrases (`sub`, `subreddit`), and blacklisted subreddits ([`src/racb/constants.py`](./src/racb/constants.py), compared case-insensitively by `is_crosspost_blocked`). Non-top-level comments (`comment.parent_id != comment.link_id`) can only get a `sub_doesnt_exist_bot` reply and are never stored (see the note in section 1). - Qualified suggestions are stored via stored procedure `insert_scraped_comment(permalink, target_subreddit)`. - Phases 2 and 3 re-parse the target from the live comment body and reject the row (`TARGET_SUBREDDIT_CHANGED`) if it no longer equals the stored target, case-insensitively. This blocks diff --git a/changelog.d/+top-level-only-replies.fixed.md b/changelog.d/+top-level-only-replies.fixed.md new file mode 100644 index 0000000..255153f --- /dev/null +++ b/changelog.d/+top-level-only-replies.fixed.md @@ -0,0 +1 @@ +**Bots No Longer Answer Nested Comments:** `same_subreddit_bot` and `same_post_bot` replied to a subreddit reference anywhere in a comment thread, not only to comments answering the post. They now answer top-level comments only, like the crosspost recommendation itself. `sub_doesnt_exist_bot` still answers at any depth, and for those replies Phase 1 now only checks that the target exists instead of running the full duplicate scan. diff --git a/src/racb/phases/phase1.py b/src/racb/phases/phase1.py index d614968..5e0917a 100644 --- a/src/racb/phases/phase1.py +++ b/src/racb/phases/phase1.py @@ -31,7 +31,13 @@ def handle_incoming_comment(comment, metrics=None): if is_crosspost_blocked(source_subreddit, target_subreddit): return + # Only sub_doesnt_exist_bot answers replies to other comments. Every other bot, and the crosspost + # recommendation itself, only counts comments that answer the post directly + top_level = is_top_level_comment(comment) + if target_subreddit.lower() == source_subreddit: + if not top_level: + return None if not config.is_bot_enabled(reddit.SAME_SUBREDDIT_BOT_NAME): logger.info(f'Found "same subreddit" comment ({reddit.full_url(comment.permalink)}); same_subreddit_bot is disabled.') return None @@ -41,8 +47,9 @@ def handle_incoming_comment(comment, metrics=None): metrics.record_aux() return reply - result_obj = get_posts_with_same_content(comment, target_subreddit) - if result_obj.posts_found: + # A reply only needs to know whether the target exists, which the first search tells + result_obj = get_posts_with_same_content(comment, target_subreddit, existence_only=not top_level) + if top_level and result_obj.posts_found: if not config.is_bot_enabled(reddit.SAME_POST_BOT_NAME): logger.info(f'Found post with duplicate content in r/{target_subreddit} ({reddit.full_url(comment.permalink)}); same_post_bot is disabled.') return None @@ -108,7 +115,8 @@ def check_pattern(comment): def get_posts_with_same_content(comment, subreddit, media_scan_since=None, media_scan_limit=duplicate_detector.QUICK_SCAN_NEWEST_LIMIT, media_scan_top_limit=duplicate_detector.QUICK_SCAN_TOP_LIMIT, - media_scan_top_time_filters=('all',), search_titles=False): + media_scan_top_time_filters=('all',), search_titles=False, + existence_only=False): """Looks for posts in `subreddit` that link to the same URL or, for an image or video post, look the same. Also counts as the same content: a post in `subreddit` that Reddit lists among the submission's "other @@ -118,6 +126,9 @@ def get_posts_with_same_content(comment, subreddit, media_scan_since=None, The picture comparison reads at most `media_scan_limit` of the newest posts, stopping at the first one older than `media_scan_since` (a Unix timestamp), plus the `media_scan_top_limit` top posts of each period in `media_scan_top_time_filters`. Its outcome is kept in `media_scan` (None if nothing was compared). + + With `existence_only`, it stops after the URL search, which is enough to fill `unable_to_search` and + `unable_to_search_reason`; `posts_found` then stays False. """ class Result: posts_found = False @@ -166,6 +177,9 @@ class Result: else: raise + if existence_only: + return result + if len(submissions) > 0: result.posts_found = True result.posts = submissions diff --git a/tests/unit/test_phase1.py b/tests/unit/test_phase1.py index 8f910c7..9932d1b 100644 --- a/tests/unit/test_phase1.py +++ b/tests/unit/test_phase1.py @@ -22,6 +22,13 @@ def test_check_pattern(make_mock_comment): assert phase1.check_pattern(make_mock_comment(body='r/this_subreddit_name_is_way_too_long_for_reddit')) is None assert phase1.check_pattern(make_mock_comment(body='just some text')) is None + # Anything besides the reference, on the same line or another one, disqualifies the comment + assert phase1.check_pattern(make_mock_comment(body='r/brothrewinafunfact\n\nWait...')) is None + assert phase1.check_pattern(make_mock_comment(body='Wait...\n\nr/brothrewinafunfact')) is None + assert phase1.check_pattern(make_mock_comment(body='r/funny\nr/pics')) is None + assert phase1.check_pattern(make_mock_comment(body='r/funny.')) is None + assert phase1.check_pattern(make_mock_comment(body=' r/funny')) is None + @pytest.mark.unit def test_is_mod_post(make_mock_comment): @@ -469,3 +476,92 @@ def test_banned_target_gets_no_sub_doesnt_exist_reply(make_mock_comment): patch('racb.phases.phase1.db.add_comment'): phase1.handle_incoming_comment(comment) mock_reply.assert_not_called() + + +NESTED = dict(parent_id='t1_c999', link_id='t3_sub123') + + +@pytest.mark.unit +def test_same_subreddit_bot_ignores_nested_comments(make_mock_comment): + comment = make_mock_comment(body='r/test_source', subreddit='test_source', **NESTED) + with patch('racb.phases.phase1.reply_to_source_equals_target_comment') as mock_reply, \ + patch('racb.phases.phase1.get_posts_with_same_content') as mock_search, \ + patch('racb.phases.phase1.db.add_comment') as mock_add: + assert phase1.handle_incoming_comment(comment) is None + mock_reply.assert_not_called() + mock_search.assert_not_called() + mock_add.assert_not_called() + + +@pytest.mark.unit +def test_nested_comment_only_checks_that_the_target_exists(make_mock_comment): + comment = make_mock_comment(body='r/target_sub', subreddit='test_source', **NESTED) + mock_reddit = MagicMock() + mock_reddit.subreddit.return_value.search.return_value = [MagicMock(permalink='/r/target_sub/comments/dup1/')] + with patch('racb.core.reddit.get_reddit_instance', return_value=mock_reddit), \ + patch('racb.phases.phase1.find_other_discussions') as mock_other_discussions, \ + patch('racb.core.duplicate_detector.scan_subreddit_for_duplicates') as mock_scan, \ + patch('racb.phases.phase1.reply_to_same_content_post_comment') as mock_reply, \ + patch('racb.phases.phase1.db.add_comment') as mock_add: + assert phase1.handle_incoming_comment(comment) is None + mock_reply.assert_not_called() + mock_other_discussions.assert_not_called() + mock_scan.assert_not_called() + mock_add.assert_not_called() + + +@pytest.mark.unit +def test_same_post_bot_ignores_nested_comments(make_mock_comment): + comment = make_mock_comment(body='r/target_sub', subreddit='test_source', **NESTED) + mock_res = MagicMock(posts_found=True, posts=[MagicMock(permalink='/r/target_sub/comments/dup1/')]) + with patch('racb.phases.phase1.get_posts_with_same_content', return_value=mock_res) as mock_search, \ + patch('racb.phases.phase1.reply_to_same_content_post_comment') as mock_reply, \ + patch('racb.phases.phase1.db.add_comment') as mock_add: + assert phase1.handle_incoming_comment(comment) is None + assert mock_search.call_args.kwargs['existence_only'] is True + mock_reply.assert_not_called() + mock_add.assert_not_called() + + +@pytest.mark.unit +def test_sub_doesnt_exist_bot_still_answers_nested_comments(make_mock_comment): + comment = make_mock_comment(body='r/nonexistent_sub_xyz', subreddit='test_source', **NESTED) + mock_res = MagicMock(posts_found=False, unable_to_search=True, unable_to_search_reason='SUBREDDIT_DOES_NOT_EXIST') + with patch('racb.phases.phase1.get_posts_with_same_content', return_value=mock_res), \ + patch('racb.phases.phase1.reply_to_nonexistent_target_subreddit_comment') as mock_reply, \ + patch('racb.phases.phase1.db.add_comment') as mock_add: + phase1.handle_incoming_comment(comment) + mock_reply.assert_called_once_with(comment, 'nonexistent_sub_xyz') + mock_add.assert_not_called() + + +@pytest.mark.unit +def test_nested_recommendation_is_not_stored(make_mock_comment): + comment = make_mock_comment(body='r/target_sub', subreddit='test_source', **NESTED) + mock_res = MagicMock(posts_found=False, unable_to_search=False) + with patch('racb.phases.phase1.get_posts_with_same_content', return_value=mock_res), \ + patch('racb.phases.phase1.db.add_comment') as mock_add: + assert phase1.handle_incoming_comment(comment) is None + mock_add.assert_not_called() + + +@pytest.mark.unit +def test_top_level_comment_gets_the_full_duplicate_check(make_mock_comment): + comment = make_mock_comment(body='r/target_sub', subreddit='test_source') + mock_res = MagicMock(posts_found=False, unable_to_search=False) + with patch('racb.phases.phase1.get_posts_with_same_content', return_value=mock_res) as mock_search, \ + patch('racb.phases.phase1.db.add_comment'): + phase1.handle_incoming_comment(comment) + assert mock_search.call_args.kwargs['existence_only'] is False + + +@pytest.mark.unit +def test_comment_with_extra_text_gets_no_reply(make_mock_comment): + comment = make_mock_comment(body='r/test_source\n\nWait...', subreddit='test_source') + with patch('racb.phases.phase1.reply_to_source_equals_target_comment') as mock_reply, \ + patch('racb.phases.phase1.get_posts_with_same_content') as mock_search, \ + patch('racb.phases.phase1.db.add_comment') as mock_add: + assert phase1.handle_incoming_comment(comment) is None + mock_reply.assert_not_called() + mock_search.assert_not_called() + mock_add.assert_not_called()