Slipstream/auto_review
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases 27 Packages Activity Actions

fix(gateway): deduplicate chat mentions #62

Merged
jwilger merged 2 commits from fix/deduplicate-chat-mentions-50 into main 2026-05-03 17:41:46 -07:00
Conversation 3 Commits 2 Files changed 6 +373 -6
jwilger commented 2026-05-03 15:51:19 -07:00
Owner
Copy link

Summary

  • Fixes #50 by sharing chat mention cursor state between the issue_comment webhook path and background poller.
  • Claims Forgejo comment IDs before dispatch so webhook→poller, poller→webhook, and first-sight seeding cases do not duplicate forced re-review jobs.
  • Adds regression coverage and changelog entry while preserving poller fallback for missed inline-thread mentions.

Verification

  • nix develop --command cargo nextest run -p ar-gateway
  • nix develop --command cargo clippy -p ar-gateway --all-targets -- -D warnings
  • nix develop --command cargo fmt --all -- --check
  • rtk git diff --check
## Summary - Fixes #50 by sharing chat mention cursor state between the issue_comment webhook path and background poller. - Claims Forgejo comment IDs before dispatch so webhook→poller, poller→webhook, and first-sight seeding cases do not duplicate forced re-review jobs. - Adds regression coverage and changelog entry while preserving poller fallback for missed inline-thread mentions. ## Verification - `nix develop --command cargo nextest run -p ar-gateway` - `nix develop --command cargo clippy -p ar-gateway --all-targets -- -D warnings` - `nix develop --command cargo fmt --all -- --check` - `rtk git diff --check`
fix(gateway): deduplicate chat mentions
All checks were successful
auto_review auto_review: no findings
CI / Nix flake check (pull_request) Successful in 1m56s
Details
b77068f8c6
auto-review requested changes 2026-05-03 15:51:34 -07:00
Dismissed
auto-review left a comment
Copy link

This PR addresses issue #50 by implementing deduplication of chat mentions between the webhook and poller paths. The changes introduce shared comment cursors to manage state and prevent duplicate forced re-review jobs. The implementation appears sound, with appropriate tests added for regression coverage.

Walkthrough

  • CHANGELOG.md:

    • Added an entry documenting the deduplication of chat mentions, which is a significant change.

  • crates/ar-gateway/src/lib.rs:

    • Introduced SharedCommentCursors to manage comment cursors.
    • Added with_chat_comment_cursors method to AppState for setting up shared cursors.

  • crates/ar-gateway/src/main.rs:

    • Initialized SharedCommentCursors in the main function to be used by the poller and webhook.

  • crates/ar-gateway/src/poller.rs:

    • Introduced SharedCommentCursors type and claim_chat_comment function for deduplication.
    • Added with_cursors method to ChatPoller for setting shared cursors.

  • crates/ar-gateway/src/webhook.rs:

    • Updated handle_issue_comment to use shared cursors for deduplication logic.

  • Tests:

    • Added tests to ensure that deduplication logic works correctly and that comments are not processed multiple times.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — source changed but no test file appears in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR addresses issue #50 by implementing deduplication of chat mentions between the webhook and poller paths. The changes introduce shared comment cursors to manage state and prevent duplicate forced re-review jobs. The implementation appears sound, with appropriate tests added for regression coverage. ## Walkthrough - **CHANGELOG.md**: - Added an entry documenting the deduplication of chat mentions, which is a significant change. - **crates/ar-gateway/src/lib.rs**: - Introduced `SharedCommentCursors` to manage comment cursors. - Added `with_chat_comment_cursors` method to `AppState` for setting up shared cursors. - **crates/ar-gateway/src/main.rs**: - Initialized `SharedCommentCursors` in the main function to be used by the poller and webhook. - **crates/ar-gateway/src/poller.rs**: - Introduced `SharedCommentCursors` type and `claim_chat_comment` function for deduplication. - Added `with_cursors` method to `ChatPoller` for setting shared cursors. - **crates/ar-gateway/src/webhook.rs**: - Updated `handle_issue_comment` to use shared cursors for deduplication logic. - **Tests**: - Added tests to ensure that deduplication logic works correctly and that comments are not processed multiple times. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [ ] Tests touched — source changed but no test file appears in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
test(gateway): cover chat mention claims
All checks were successful
auto_review auto_review: no findings
CI / Nix flake check (pull_request) Successful in 1m58s
Details
e76e8be53c
jwilger commented 2026-05-03 17:36:49 -07:00
Author
Owner
Copy link

Addressed the review feedback about test-file coverage.

Reflection/classification: this was a one-off mismatch between the existing adjacent unit-test coverage and the pre-merge heuristic, which expects a changed test file path. The implementation already had behavioral regression tests, but I added an integration test file so the PR surface clearly shows tests were touched.

Added:

  • crates/ar-gateway/tests/chat_comment_dedup.rs
  • coverage for shared chat comment claims deduplicating by (owner, repo, PR, comment_id) while allowing distinct PRs and newer comment IDs.

Verification:

  • nix develop --command cargo nextest run -p ar-gateway shared_chat_comment_claims_deduplicate_by_pr_and_comment_id
  • nix develop --command cargo nextest run -p ar-gateway
  • nix develop --command cargo clippy -p ar-gateway --all-targets -- -D warnings
  • nix develop --command cargo fmt --all -- --check
Addressed the review feedback about test-file coverage. Reflection/classification: this was a one-off mismatch between the existing adjacent unit-test coverage and the pre-merge heuristic, which expects a changed test file path. The implementation already had behavioral regression tests, but I added an integration test file so the PR surface clearly shows tests were touched. Added: - `crates/ar-gateway/tests/chat_comment_dedup.rs` - coverage for shared chat comment claims deduplicating by `(owner, repo, PR, comment_id)` while allowing distinct PRs and newer comment IDs. Verification: - `nix develop --command cargo nextest run -p ar-gateway shared_chat_comment_claims_deduplicate_by_pr_and_comment_id` - `nix develop --command cargo nextest run -p ar-gateway` - `nix develop --command cargo clippy -p ar-gateway --all-targets -- -D warnings` - `nix develop --command cargo fmt --all -- --check`
auto-review approved these changes 2026-05-03 17:36:55 -07:00
auto-review left a comment
Copy link

This PR addresses issue #50 by implementing a deduplication mechanism for chat mentions in the ar-gateway component. The changes appear well-structured and include necessary tests to ensure functionality. The PR is ready for merging, assuming all tests pass.

Walkthrough

  • CHANGELOG.md:

    • Added a clear entry describing the deduplication of chat mentions, which helps users understand the change.

  • crates/ar-gateway/src/lib.rs:

    • Introduced SharedCommentCursors to manage shared state for comment cursors.
    • Added with_chat_comment_cursors method to AppState for setting up shared cursors.

  • crates/ar-gateway/src/main.rs:

    • Initialized SharedCommentCursors in the main function to ensure shared state is available for the poller and webhook.

  • crates/ar-gateway/src/poller.rs:

    • Added claim_chat_comment and seed_chat_cursor_monotonic functions to manage comment deduplication.
    • Updated ChatPoller to use shared cursors for deduplication.

  • crates/ar-gateway/src/webhook.rs:

    • Integrated claim_chat_comment to ensure webhook comments are deduplicated.

  • crates/ar-gateway/tests/chat_comment_dedup.rs:

    • Added tests to verify that comments are deduplicated correctly by PR and comment ID.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR addresses issue #50 by implementing a deduplication mechanism for chat mentions in the `ar-gateway` component. The changes appear well-structured and include necessary tests to ensure functionality. The PR is ready for merging, assuming all tests pass. ## Walkthrough - **CHANGELOG.md**: - Added a clear entry describing the deduplication of chat mentions, which helps users understand the change. - **crates/ar-gateway/src/lib.rs**: - Introduced `SharedCommentCursors` to manage shared state for comment cursors. - Added `with_chat_comment_cursors` method to `AppState` for setting up shared cursors. - **crates/ar-gateway/src/main.rs**: - Initialized `SharedCommentCursors` in the main function to ensure shared state is available for the poller and webhook. - **crates/ar-gateway/src/poller.rs**: - Added `claim_chat_comment` and `seed_chat_cursor_monotonic` functions to manage comment deduplication. - Updated `ChatPoller` to use shared cursors for deduplication. - **crates/ar-gateway/src/webhook.rs**: - Integrated `claim_chat_comment` to ensure webhook comments are deduplicated. - **crates/ar-gateway/tests/chat_comment_dedup.rs**: - Added tests to verify that comments are deduplicated correctly by PR and comment ID. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
jwilger deleted branch fix/deduplicate-chat-mentions-50 2026-05-03 17:41:46 -07:00
jwilger referenced this pull request from a commit 2026-05-05 12:18:07 -07:00
chore: release v0.1.0 (#94)
Sign in to join this conversation.
Reviewers
No reviewers
auto-review
Labels
Clear labels   
bug

   
discussion

   
enhancement
  
bug
Something is not working

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
bug
discussion
enhancement
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Slipstream/auto_review!62
No description provided.