Skip to content

Setup tweet cap alert#47

Merged
dewabisma merged 12 commits into
mainfrom
feat/setup-tweet-cap-alert
Jan 4, 2026
Merged

Setup tweet cap alert#47
dewabisma merged 12 commits into
mainfrom
feat/setup-tweet-cap-alert

Conversation

@dewabisma

Copy link
Copy Markdown
Contributor

Summary

When we pull tweets we track the usage and give alert if certain threshold has passed, so we know we are running out of quota.

Changes

  • Add tweet_pull_usage table
  • Add tweet_pull_usage repo
  • Add alert service
  • Update tweet sync with alert
  • Update leaderboard sync with alert

@dewabisma dewabisma requested a review from n13 December 23, 2025 10:52
Comment thread .cursor/rules/rust-rule.mdc Outdated

@n13 n13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review: Feat - Setup Tweet Cap Alert

This PR implements tracking of Twitter API usage and alerting when a threshold is reached. This is crucial for managing API costs/limits.

1. Suggestion: Alert Frequency

In src/services/alert_service.rs, the alert logic triggers whenever the current total exceeds the threshold:

if current_total >= self.config.tweet_sync.alert_threshold {
    // Sends alert...
}

Observation: Once the threshold is reached, every subsequent sync (e.g., every 10 minutes) will trigger another alert until the billing period resets.

Recommendation: Since the interval is 10 minutes, this spam might be acceptable for visibility. However, if you want to reduce noise in the future, you could modify the logic to only alert when the threshold is just crossed, or add a cooldown.

// Optional: Only alert if we crossed the line just now
if current_total >= threshold && previous_total < threshold {
    self.send_twitter_limit_alert(...).await?;
}

2. Period Calculation & Edge Cases

The billing period calculation in TweetPullUsageRepository::calculate_period_for_date is clever in handling short months, but "reset day" logic can be tricky.

  • Observation: If reset_day is 31:
    • Feb 28th (non-leap) -> Period "Jan" (Start Jan 31).
    • March 1st -> Period "Feb" (Start Feb 28/29 effective).
    • This effectively shifts the "month label" by one. As long as this is consistent with your billing provider (e.g., X API billing cycle), it is fine. Just verify that the resulting "Period String" (e.g., "2023-01") matches the billing invoice you want to correlate it with.

3. Usage Tracking Resilience

The increment_usage method uses an atomic DB update (SET tweet_count = ... + EXCLUDED.tweet_count), which is excellent for concurrency.

However, consider the crash scenario in TweetSynchronizerService:

  1. Tweets are pulled and saved.
  2. track_and_alert_usage is called.
  3. If Step 2 fails (DB connection blip), the error is logged but the sync function continues (tracing::error!).
  4. Since the tweets were saved, last_id will be updated on the next run, so these tweets won't be pulled again.
  5. Result: These tweets are never counted in the usage stats.

Recommendation: While perfect accuracy isn't required for an alert system, be aware that this is "at most once" counting behavior in failure modes. If stricter accounting is needed, usage increment should happen in the same transaction as saving the tweets (or relevant_tweets insertion). Given this is for "alerts" and not "billing audits", the current approach is likely acceptable.

4. Minor: Error Handling

In AlertService::send_webhook_alert:

Err(crate::AppError::Server(format!("Webhook alert failed with status {}: {}", status, body)))

This is a good use of the existing AppError. Ensure that AppError::Server is logged appropriately upstream.

Summary

The logic is solid. The alert frequency (every 10 minutes once threshold is met) is noted as acceptable for current operations.

@n13

n13 commented Dec 31, 2025

Copy link
Copy Markdown
Contributor

Note: I don't think alert spamming is critical when it's only every 10 minutes. It should kinda make noise, and every 10 minutes is not so bad.

@n13 n13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR 47 Review: Extract Duplicated Code in Raid Quest Handlers

Summary

This PR refactors src/handlers/raid_quest.rs to extract duplicated logic into a reusable helper function get_active_raid_and_x_association. This function handles the common requirement of fetching the currently active raid and verifying the user's X (Twitter) association.

Changes Identified

  1. Refactoring:
    • Created private helper get_active_raid_and_x_association which returns Result<(RaidQuest, XAssociation), AppError>.
    • This helper encapsulates:
      • Fetching the active raid (state.db.raid_quests.find_active()).
      • Fetching the user's X association (state.db.x_associations.find_by_address()).
      • Standardized error handling for "No active raid" and "User doesn't have X association" (returning RecordNotFound).
  2. Usage:
    • handle_get_active_raid_raider_submissions: Replaced inline checks with the new helper.
    • handle_create_raid_submission: Replaced inline checks with the new helper.

Code Review

Correctness

  • Logic Preservation: The extracted logic accurately preserves the behavior of the original code. It ensures that both an active raid exists and the user is linked to X before proceeding with submission-related actions.
  • Error Handling: The error messages ("No active raid is found", "User doesn't have X association") are consistent with the previous implementation and map to DbError::RecordNotFound, which is appropriate.

Code Quality

  • DRY Principle: The refactoring successfully adheres to the DRY (Don't Repeat Yourself) principle. Logic that was repeated in two handlers is now centralized.
  • Readability: The handlers handle_get_active_raid_raider_submissions and handle_create_raid_submission are now significantly cleaner and focused on their specific business logic rather than validation preconditions.
  • Maintainability: Future changes to how active raids are retrieved or how X association is validated only need to be updated in one place.

Testing

  • Existing Tests: The existing tests in src/handlers/raid_quest.rs (test_create_raid_submission_success, test_create_raid_submission_fails_no_active_raid, etc.) cover the paths that use this new helper.
  • Coverage: Since the refactoring touches core logic for submissions, the passing tests give high confidence that the refactoring didn't break functionality.

Conclusion

The changes are a definite improvement in code quality. The refactoring is clean, correct, and safe.

Status: ✅ Approved

Base automatically changed from feat/configurable-whitelist to main January 4, 2026 10:03
@dewabisma dewabisma merged commit d12ef5f into main Jan 4, 2026
@dewabisma dewabisma deleted the feat/setup-tweet-cap-alert branch January 4, 2026 10:04
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.

2 participants