Fix Custom Format score syncer not being scheduled accordingly#584
Conversation
|
@greptileai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Greptile SummaryThis PR fixes the Confidence Score: 5/5Safe to merge — changes are logically correct and the only finding is a minor style inconsistency. Both the startup registration path and the dynamic toggle path now consistently gate on No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[UpdateSeekerConfig called] --> B[Capture previous state\npreviousProactiveSearchEnabled\npreviousAnyUseCustomFormatScore]
B --> C[Apply request & save changes]
C --> D[Re-query anyUseCustomFormatScore from DB]
D --> E{Compute states}
E --> F[syncerShouldBeRunning =\nanyUseCustomFormatScore &&\nconfig.ProactiveSearchEnabled]
E --> G[syncerWasRunning =\npreviousAnyUseCustomFormatScore &&\npreviousProactiveSearchEnabled]
F --> H{syncerShouldBeRunning\n!= syncerWasRunning?}
G --> H
H -- Yes + should run --> I[StartJob + TriggerJobOnce\nCustomFormatScoreSyncer]
H -- Yes + should stop --> J[StopJob\nCustomFormatScoreSyncer]
H -- No change --> K[No action]
subgraph Startup [BackgroundJobManager.StartAsync]
L[Query seekerConfig &\nanyUseCustomFormatScore] --> M{ProactiveSearchEnabled\n&& anyUseCustomFormatScore?}
M -- Yes --> N[AddTriggersForJob\nCustomFormatScoreSyncer]
M -- No --> O[Register job without trigger]
end
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes consolidate the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@sourcery-ai review |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAligns the CustomFormatScoreSyncer job scheduling with both instance-level UseCustomFormatScore and global ProactiveSearchEnabled flags, and centralizes its registration logic to use configuration-derived state instead of re-querying. Sequence diagram for CustomFormatScoreSyncer scheduling on seeker config updatesequenceDiagram
actor AdminUser
participant SeekerConfigController
participant JobManagementService
participant Logger
AdminUser->>SeekerConfigController: UpdateSeekerConfig(newConfig)
activate SeekerConfigController
SeekerConfigController->>SeekerConfigController: Load previousAnyUseCustomFormatScore
SeekerConfigController->>SeekerConfigController: Load previousProactiveSearchEnabled
SeekerConfigController->>SeekerConfigController: Compute anyUseCustomFormatScore
SeekerConfigController->>SeekerConfigController: syncerShouldBeRunning = anyUseCustomFormatScore && newConfig.ProactiveSearchEnabled
SeekerConfigController->>SeekerConfigController: syncerWasRunning = previousAnyUseCustomFormatScore && previousProactiveSearchEnabled
alt syncerShouldBeRunning != syncerWasRunning
alt syncerShouldBeRunning
SeekerConfigController->>Logger: LogInformation(CustomFormatScoreSyncer conditions met, starting job)
SeekerConfigController->>JobManagementService: StartJob(CustomFormatScoreSyncer, null, CustomFormatScoreSyncerCron)
SeekerConfigController->>JobManagementService: TriggerJobOnce(CustomFormatScoreSyncer)
else syncerShouldBeRunning is false
SeekerConfigController->>Logger: LogInformation(CustomFormatScoreSyncer conditions no longer met, stopping job)
SeekerConfigController->>JobManagementService: StopJob(CustomFormatScoreSyncer)
end
else no change in scheduling conditions
SeekerConfigController->>SeekerConfigController: Do not modify CustomFormatScoreSyncer scheduling
end
SeekerConfigController-->>AdminUser: Ok(seeker configuration updated)
deactivate SeekerConfigController
Updated class diagram for BackgroundJobManager and SeekerConfigController scheduling logicclassDiagram
class SeekerConfigController {
- ILogger logger
- IJobManagementService jobManagementService
+ Task UpdateSeekerConfig(UpdateSeekerConfigRequest config)
}
class BackgroundJobManager {
- IJobManagementService jobManagementService
- DataContext dataContext
+ Task InitializeJobsFromConfiguration(CancellationToken cancellationToken)
+ Task RegisterQueueCleanerJob(QueueCleanerConfig config, CancellationToken cancellationToken)
+ Task RegisterArrConfigJob(ArrConfig config, CancellationToken cancellationToken)
+ Task RegisterDownloadCleanerJob(DownloadCleanerConfig config, CancellationToken cancellationToken)
+ Task RegisterBlacklistSyncJob(BlacklistSyncConfig config, CancellationToken cancellationToken)
+ Task RegisterSeekerJob(SeekerConfig config, CancellationToken cancellationToken)
+ Task RegisterCustomFormatScoreSyncJob(SeekerConfig seekerConfig, bool anyUseCustomFormatScore, CancellationToken cancellationToken)
+ Task AddJobWithoutTrigger~CustomFormatScoreSyncer~(CancellationToken cancellationToken)
+ Task AddTriggersForJob~CustomFormatScoreSyncer~(string cronExpression, CancellationToken cancellationToken)
}
class SeekerConfig {
+ bool ProactiveSearchEnabled
+ bool SearchEnabled
}
class SeekerInstanceConfig {
+ bool Enabled
+ bool UseCustomFormatScore
}
class ArrInstance {
+ bool Enabled
}
class IJobManagementService {
+ Task StartJob(JobType jobType, object state, string cronExpression)
+ Task StopJob(JobType jobType)
+ Task TriggerJobOnce(JobType jobType)
}
class DataContext {
+ DbSet~SeekerConfig~ SeekerConfigs
+ DbSet~SeekerInstanceConfig~ SeekerInstanceConfigs
}
class Logger {
+ void LogInformation(string message)
}
SeekerConfigController --> IJobManagementService : uses
SeekerConfigController --> Logger : logs to
BackgroundJobManager --> IJobManagementService : manages jobs via
BackgroundJobManager --> DataContext : reads configuration from
SeekerInstanceConfig --> ArrInstance : has ArrInstance
DataContext --> SeekerConfig : stores
DataContext --> SeekerInstanceConfig : stores
BackgroundJobManager --> SeekerConfig : reads
SeekerConfigController --> SeekerConfig : evaluates flags
Flow diagram for CustomFormatScoreSyncer registration in BackgroundJobManagerflowchart TD
A[InitializeJobsFromConfiguration] --> B[Load SeekerConfig from DataContext]
B --> C[Query anyUseCustomFormatScore from SeekerInstanceConfigs]
C --> D[RegisterQueueCleanerJob]
D --> E[RegisterArrConfigJob]
E --> F[RegisterDownloadCleanerJob]
F --> G[RegisterBlacklistSyncJob]
G --> H[RegisterSeekerJob]
H --> I[RegisterCustomFormatScoreSyncJob seekerConfig and anyUseCustomFormatScore]
subgraph RegisterCustomFormatScoreSyncJob
I --> J[AddJobWithoutTrigger CustomFormatScoreSyncer]
J --> K{seekerConfig.ProactiveSearchEnabled && anyUseCustomFormatScore}
K -- true --> L[AddTriggersForJob CustomFormatScoreSyncer with CustomFormatScoreSyncerCron]
K -- false --> M[Do not add triggers]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The condition for when the CustomFormatScoreSyncer should run (
anyUseCustomFormatScore && ProactiveSearchEnabled) is now duplicated between the controller andBackgroundJobManager; consider extracting this into a shared helper/service method to keep the behavior consistent over time. - In
InitializeJobsFromConfiguration, you now perform two separate queries forSeekerConfigandanyUseCustomFormatScore; consider combining these (e.g., via a projection or navigation property) to avoid an extra DB round-trip on startup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The condition for when the CustomFormatScoreSyncer should run (`anyUseCustomFormatScore && ProactiveSearchEnabled`) is now duplicated between the controller and `BackgroundJobManager`; consider extracting this into a shared helper/service method to keep the behavior consistent over time.
- In `InitializeJobsFromConfiguration`, you now perform two separate queries for `SeekerConfig` and `anyUseCustomFormatScore`; consider combining these (e.g., via a projection or navigation property) to avoid an extra DB round-trip on startup.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary by Sourcery
Align CustomFormatScoreSyncer scheduling with seeker configuration so it only runs when custom format scores are in use and proactive search is enabled.
Bug Fixes:
Enhancements: