Skip to content

Limit sync to one run/day, prevent overlap, and migrate batching to /v1/movies + /v1/shows#22

Merged
AbandonedCart merged 2 commits intomainfrom
copilot/limit-sync-process-once-per-day
Apr 21, 2026
Merged

Limit sync to one run/day, prevent overlap, and migrate batching to /v1/movies + /v1/shows#22
AbandonedCart merged 2 commits intomainfrom
copilot/limit-sync-process-once-per-day

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

This updates the sync pipeline to enforce one successful run per 24 hours and prevent concurrent executions. It also migrates ingestion from legacy per-item endpoints to batched POST /v1/movies and POST /v1/shows, including 100MB request-size handling.

  • Rate limiting and overlap control

    • Added a process-wide execution gate to skip overlapping sync runs.
    • Added persisted last-successful-sync tracking in SQLite (Metadata table) and daily run suppression.
    • Daily suppression logs explicitly as exceeding the rate limit.
  • API migration to new batch endpoints

    • Replaced legacy single-item fetch flow with batch POST methods:
      • GetByMoviesBatchAsync(...)/v1/movies
      • GetByShowsBatchAsync(...)/v1/shows
    • Added typed request payload models:
      • MovieLookupRequest
      • ShowLookupRequest
    • Preserved request/response positional mapping for deterministic item association.
  • Batch construction and size enforcement

    • Introduced request chunking by serialized payload size with a hard 100MB ceiling.
    • Added guard to reject any single lookup item that would exceed the maximum request size.
  • Sync task refactor

    • Reworked sync planning into two batched streams:
      • Movies + non-series episode fallbacks → /v1/movies
      • Series-backed episode lookups → /v1/shows
    • Kept existing segment filtering behavior (duration tolerance, segment validity, first timestamp extraction).
if (lastSuccessfulSync.HasValue && (now - lastSuccessfulSync.Value) < MinimumSyncInterval)
{
    _logger.LogWarning(
        "Skipping SkipMe.db sync: exceeding the rate limit of one run per day (last successful run at {LastRunUtc:o}).",
        lastSuccessfulSync.Value);
    return;
}

Copilot AI and others added 2 commits April 21, 2026 12:48
@AbandonedCart AbandonedCart marked this pull request as ready for review April 21, 2026 12:56
@AbandonedCart AbandonedCart merged commit e2421e4 into main Apr 21, 2026
2 checks passed
@AbandonedCart AbandonedCart deleted the copilot/limit-sync-process-once-per-day branch April 21, 2026 12:57
@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Apr 21, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

The PR introduces rate limiting, overlap prevention, and batch API migration to the SkipMe.db sync pipeline. The changes are well-structured and follow existing codebase patterns.

Design Pattern Observations

Pattern Implementation Notes
Repository SegmentStore SQLite-backed persistence with proper semaphore locking
Provider SkipMeApiClient HTTP client with batch API integration
Dependency Injection Constructor injection Used throughout for ILogger, IHttpClientFactory

Architecture

  • Proper separation between Models, Services, and Tasks
  • New batch request models (MovieLookupRequest, ShowLookupRequest) follow existing JSON serialization patterns
  • Metadata table schema added with proper upsert logic (line 234-238 in SegmentStore.cs)

.NET Best Practices

  • Async/await with ConfigureAwait(false) used consistently
  • Structured logging with Microsoft.Extensions.Logging
  • Proper semaphore-based concurrency control

Key Changes Reviewed

  1. Rate limiting (SyncSegmentsTask.cs:101-107) - Correctly enforces 24-hour minimum interval
  2. Overlap prevention (line 92-95) - Uses WaitAsync(0, ...) for non-blocking gate acquisition
  3. Batch API migration - PostBatchAsync handles response count validation and error handling
  4. Request chunking - ChunkByMaxRequestSize with proper 100MB limit enforcement

Potential Improvement (Not a Bug)

  • GetLastSuccessfulSyncUtc() uses synchronous semaphore Wait() in an async context (SegmentStore.cs:200). This matches the existing pattern in GetSegments() but could benefit from an async variant in future PRs.
Files Reviewed (7 files)
  • SkipMe.Db.Plugin/Models/MediaResponse.cs - Doc update
  • SkipMe.Db.Plugin/Models/MovieLookupRequest.cs - New file
  • SkipMe.Db.Plugin/Models/SeriesResponse.cs - Doc update
  • SkipMe.Db.Plugin/Models/ShowLookupRequest.cs - New file
  • SkipMe.Db.Plugin/Services/SegmentStore.cs - Metadata + methods
  • SkipMe.Db.Plugin/Services/SkipMeApiClient.cs - Batch API
  • SkipMe.Db.Plugin/Tasks/SyncSegmentsTask.cs - Rate limiting + batching

Reviewed by minimax-m2.5-20260211 · 610,952 tokens

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