carverauto/serviceradar

Fork 0

fixing sweep job / oban scheduler issue #2690

Merged

mfreeman451 merged 2 commits from refs/pull/2690/head into staging

2026-01-18 08:36:43 +00:00

mfreeman451 commented

2026-01-18 08:31:02 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2340
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2340
Original created: 2026-01-18T08:31:02Z
Original updated: 2026-01-18T08:36:44Z
Original head: carverauto/serviceradar:2337-bugweb-ng-unable-to-create-new-sweep-group
Original base: staging
Original merged: 2026-01-18T08:36:43Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Bug fix, Enhancement

Description

Add resilient Oban job scheduling to prevent sweep group creation failures
- New ObanSupport module safely detects Oban availability and handles insert errors
- Sweep workers return :oban_unavailable error instead of crashing when Oban missing
Implement SweepScheduleReconciler GenServer to reconcile deferred schedules
- Periodically ensures sweep workers are enqueued once Oban becomes available
- Runs at app startup and on configurable interval (default 60 seconds)
Update web-ng UI to provide clear feedback when scheduling is deferred
- Display contextual messages indicating scheduler unavailability
- Inform users that scheduling will resume automatically
Add comprehensive tests and design documentation for job scheduling resilience

Diagram Walkthrough

flowchart LR
  A["Sweep Group<br/>Create/Update"] -->|calls| B["ScheduleSweepMonitor<br/>Change"]
  B -->|uses| C["ObanSupport<br/>safe_insert"]
  C -->|checks| D{"Oban<br/>Available?"}
  D -->|yes| E["Insert Job<br/>Success"]
  D -->|no| F["Return Error<br/>oban_unavailable"]
  E --> G["User Sees<br/>Success Message"]
  F --> H["User Sees<br/>Deferred Message"]
  I["SweepScheduleReconciler<br/>GenServer"] -->|periodic| J["Check Enabled<br/>Groups"]
  J -->|retry| C
  C -->|now available| E

File Walkthrough

Relevant files

Enhancement

4 files

application.ex `Add SweepScheduleReconciler child to supervision tree`	+11/-0
oban_support.ex `New module for safe Oban availability checks and inserts`	+35/-0
sweep_schedule_reconciler.ex `New GenServer to reconcile deferred sweep schedules`	+83/-0
index.ex `Add contextual messaging for deferred sweep scheduling`	+38/-6

Bug fix

3 files

sweep_monitor_worker.ex `Guard Oban inserts with availability check and error handling`	+19/-8
sweep_data_cleanup_worker.ex `Guard Oban inserts with availability check and error handling`	+18/-7
schedule_sweep_monitor.ex `Add error handling for Oban unavailability conditions`	+22/-0

Tests

1 files

oban_support_test.exs
New tests for Oban support and reconciliation behavior +23/-0

Documentation

5 files

proposal.md `Design proposal for Oban availability resilience`	+13/-0
design.md `Detailed design decisions and migration plan`	+29/-0
spec.md `New job scheduling resilience requirements`	+12/-0
spec.md `Updated sweep group management requirements`	+44/-0
tasks.md `Implementation task checklist and completion status`	+5/-0

Imported from GitHub pull request. Original GitHub pull request: #2340 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2340 Original created: 2026-01-18T08:31:02Z Original updated: 2026-01-18T08:36:44Z Original head: carverauto/serviceradar:2337-bugweb-ng-unable-to-create-new-sweep-group Original base: staging Original merged: 2026-01-18T08:36:43Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Bug fix, Enhancement ___ ### **Description** - Add resilient Oban job scheduling to prevent sweep group creation failures - New `ObanSupport` module safely detects Oban availability and handles insert errors - Sweep workers return `:oban_unavailable` error instead of crashing when Oban missing - Implement `SweepScheduleReconciler` GenServer to reconcile deferred schedules - Periodically ensures sweep workers are enqueued once Oban becomes available - Runs at app startup and on configurable interval (default 60 seconds) - Update web-ng UI to provide clear feedback when scheduling is deferred - Display contextual messages indicating scheduler unavailability - Inform users that scheduling will resume automatically - Add comprehensive tests and design documentation for job scheduling resilience ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Sweep Group<br/>Create/Update"] -->|calls| B["ScheduleSweepMonitor<br/>Change"] B -->|uses| C["ObanSupport<br/>safe_insert"] C -->|checks| D{"Oban<br/>Available?"} D -->|yes| E["Insert Job<br/>Success"] D -->|no| F["Return Error<br/>oban_unavailable"] E --> G["User Sees<br/>Success Message"] F --> H["User Sees<br/>Deferred Message"] I["SweepScheduleReconciler<br/>GenServer"] -->|periodic| J["Check Enabled<br/>Groups"] J -->|retry| C C -->|now available| E ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><details><summary>4 files</summary><table> <tr> <td><strong>application.ex</strong><dd><code>Add SweepScheduleReconciler child to supervision tree</code>        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-a9ffbf400b7f9b22cd8980c41286c54fe373f1f1a8684bb6a344a5fb39b178d0">+11/-0</a>    </td> </tr> <tr> <td><strong>oban_support.ex</strong><dd><code>New module for safe Oban availability checks and inserts</code>  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-c7ada969c31cb0f727ed28dfc5f874328264ef4ec6987770fb29ad1cbf1e96e8">+35/-0</a>    </td> </tr> <tr> <td><strong>sweep_schedule_reconciler.ex</strong><dd><code>New GenServer to reconcile deferred sweep schedules</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-08cc9510c5b0889be570924cefe2965349240dd696a04b28fc3171bde49edbc4">+83/-0</a>    </td> </tr> <tr> <td><strong>index.ex</strong><dd><code>Add contextual messaging for deferred sweep scheduling</code>      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-b2127e71582033bc6dfd2d7397f56bf43c1f7c613defffc504b3d8ee1e7406c4">+38/-6</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Bug fix</strong></td><td><details><summary>3 files</summary><table> <tr> <td><strong>sweep_monitor_worker.ex</strong><dd><code>Guard Oban inserts with availability check and error handling</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-c25f50cc2e496fc7f8f883a12098f278c56ae1d2d42d4d2af4200d364e2ef9a8">+19/-8</a>    </td> </tr> <tr> <td><strong>sweep_data_cleanup_worker.ex</strong><dd><code>Guard Oban inserts with availability check and error handling</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-b8660522af1e1ad3ba8da56f754567fdae03d09fa9e18749a3a49435893073fe">+18/-7</a>    </td> </tr> <tr> <td><strong>schedule_sweep_monitor.ex</strong><dd><code>Add error handling for Oban unavailability conditions</code>        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-aea351916ba684aaab2941779c2750ebd099e2d30d0c929358f53aebc674b31f">+22/-0</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Tests</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>oban_support_test.exs</strong><dd><code>New tests for Oban support and reconciliation behavior</code>      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-96fb2e19b05c320f0b95cbcd87d6990d6c1009ba4f60c56a41cd8f487df25c2c">+23/-0</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Documentation</strong></td><td><details><summary>5 files</summary><table> <tr> <td><strong>proposal.md</strong><dd><code>Design proposal for Oban availability resilience</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-37ddf81769aac3b6c9033e86a6d22d2ae3607689f3adb6d0c95ab4ddc7b720d3">+13/-0</a>    </td> </tr> <tr> <td><strong>design.md</strong><dd><code>Detailed design decisions and migration plan</code>                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-e1d0994ee9e33390ecacaa5afd07f2e3d0a61d0204ecb3bd9b2510b36c75f1f3">+29/-0</a>    </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>New job scheduling resilience requirements</code>                              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-da22bc36ffaf756f8b971fee18c979bd934307de0ca5e34520f67c6febc18b22">+12/-0</a>    </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Updated sweep group management requirements</code>                            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-3b709e359a6ffb6d9a047087ec55fc9c75cd210a677875eb4c144571e343ba39">+44/-0</a>    </td> </tr> <tr> <td><strong>tasks.md</strong><dd><code>Implementation task checklist and completion status</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-facb4f3998adc8e5ecb4bc2331453c4336924cc967f171cafa83b607e5c81a45">+5/-0</a>      </td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-18 08:31:44 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2340#issuecomment-3765055831
Original created: 2026-01-18T08:31:44Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

🟢

No security concerns identified

No security vulnerabilities detected by AI analysis. Human verification advised for critical code.

Ticket Compliance

🟡

🎫 #2337

🟢	Prevent sweep group create/update from crashing when Oban is not running/configured (the current flow raises `RuntimeError` about missing Oban instance).
🟢	Allow users in web-ng (Settings > Networks) to create a new sweep group without the LiveView/GenServer terminating due to Oban scheduling.
⚪	Confirm via manual UI reproduction (Settings > Networks -> create sweep group) that the action succeeds in an environment where Oban is not running/configured, and that the original LiveView crash no longer occurs.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing user context: New scheduling audit logs include sweep_group_id and outcome but do not include any
initiating user identifier, so it may be insufficient as an audit trail if sweep group
changes are considered critical actions.

Referred Code

  Logger.info("Scheduled sweep monitor",
    sweep_group_id: record.id
  )

{:error, :oban_unavailable} ->
  Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)",
    sweep_group_id: record.id
  )

{:error, {:oban_unavailable, message}} ->
  Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)",
    sweep_group_id: record.id,
    reason: message
  )

{:error, reason} ->
  Logger.error("Failed to schedule sweep monitor",
    sweep_group_id: record.id,
    reason: inspect(reason)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Interval not validated: The reconciler reads :sweep_schedule_reconcile_interval_seconds from config without
validating it is a non-negative integer, which could cause crashes or unexpected behavior
when used in Process.send_after/3.

Referred Code

  interval = Application.get_env(:serviceradar_core, :sweep_schedule_reconcile_interval_seconds,
    @default_interval_seconds
  )

  schedule_reconcile(0)

  {:ok, %{interval: interval}}
end

@impl true
def handle_info(:reconcile, %{interval: interval} = state) do
  reconcile()
  schedule_reconcile(interval)
  {:noreply, state}
end

defp schedule_reconcile(delay_seconds) do
  Process.send_after(self(), :reconcile, delay_seconds * 1000)
end

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Exception message logged: The new warning logs can include reason: message derived from Exception.message/1, which
may leak internal scheduler/config details depending on the raised error content.

Referred Code

{:error, :oban_unavailable} ->
  Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)",
    sweep_group_id: record.id
  )

{:error, {:oban_unavailable, message}} ->
  Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)",
    sweep_group_id: record.id,
    reason: message
  )

Learn more about managing compliance generic rules or creating your own custom rules

Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2340#issuecomment-3765055831 Original created: 2026-01-18T08:31:44Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2337>#2337</a></summary> <table width='100%'><tbody> <tr><td rowspan=2>🟢</td> <td>Prevent sweep group create/update from crashing when Oban is not running/configured (the <br>current flow raises <code>RuntimeError</code> about missing Oban instance).<br></td></tr> <tr><td>Allow users in web-ng (Settings > Networks) to create a new sweep group without the <br>LiveView/GenServer terminating due to Oban scheduling.<br></td></tr> <tr><td rowspan=1>⚪</td> <td>Confirm via manual UI reproduction (Settings > Networks -> create sweep group) that the <br>action succeeds in an environment where Oban is not running/configured, and that the <br>original LiveView crash no longer occurs.<br></td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=3>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=3>⚪</td> <td><details> <summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2340/files#diff-aea351916ba684aaab2941779c2750ebd099e2d30d0c929358f53aebc674b31fR41-R59'><strong>Missing user context</strong></a>: New scheduling audit logs include <code>sweep_group_id</code> and outcome but do not include any <br>initiating user identifier, so it may be insufficient as an audit trail if sweep group <br>changes are considered critical actions.<br> <details open><summary>Referred Code</summary> ```elixir Logger.info("Scheduled sweep monitor", sweep_group_id: record.id ) {:error, :oban_unavailable} -> Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)", sweep_group_id: record.id ) {:error, {:oban_unavailable, message}} -> Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)", sweep_group_id: record.id, reason: message ) {:error, reason} -> Logger.error("Failed to schedule sweep monitor", sweep_group_id: record.id, reason: inspect(reason) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2340/files#diff-08cc9510c5b0889be570924cefe2965349240dd696a04b28fc3171bde49edbc4R25-R43'><strong>Interval not validated</strong></a>: The reconciler reads <code>:sweep_schedule_reconcile_interval_seconds</code> from config without <br>validating it is a non-negative integer, which could cause crashes or unexpected behavior <br>when used in <code>Process.send_after/3</code>.<br> <details open><summary>Referred Code</summary> ```elixir interval = Application.get_env(:serviceradar_core, :sweep_schedule_reconcile_interval_seconds, @default_interval_seconds ) schedule_reconcile(0) {:ok, %{interval: interval}} end @impl true def handle_info(:reconcile, %{interval: interval} = state) do reconcile() schedule_reconcile(interval) {:noreply, state} end defp schedule_reconcile(delay_seconds) do Process.send_after(self(), :reconcile, delay_seconds * 1000) end ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2340/files#diff-aea351916ba684aaab2941779c2750ebd099e2d30d0c929358f53aebc674b31fR45-R54'><strong>Exception message logged</strong></a>: The new warning logs can include <code>reason: message</code> derived from <code>Exception.message/1</code>, which <br>may leak internal scheduler/config details depending on the raised error content.<br> <details open><summary>Referred Code</summary> ```elixir {:error, :oban_unavailable} -> Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)", sweep_group_id: record.id ) {:error, {:oban_unavailable, message}} -> Logger.warning("Sweep monitor scheduling deferred (Oban unavailable)", sweep_group_id: record.id, reason: message ) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2026-01-18 08:32:57 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2340#issuecomment-3765056765
Original created: 2026-01-18T08:32:57Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consider a more direct reconciliation trigger Instead of having the `SweepScheduleReconciler` poll periodically, consider an event-driven approach. The reconciliation process should be triggered directly by the component that starts Oban, making job scheduling more immediate. Examples: elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_schedule_reconciler.ex [24-43] `def init(_opts) do interval = Application.get_env(:serviceradar_core, :sweep_schedule_reconcile_interval_seconds, @default_interval_seconds ) schedule_reconcile(0) {:ok, %{interval: interval}} end ... (clipped 10 lines)` Solution Walkthrough: Before: `# elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_schedule_reconciler.ex defmodule SweepScheduleReconciler do use GenServer def init(_opts) do # ... schedule_reconcile(0) # Run immediately on start {:ok, %{interval: interval}} end def handle_info(:reconcile, state) do reconcile() schedule_reconcile(state.interval) # Schedule next poll {:noreply, state} end defp schedule_reconcile(delay_seconds) do Process.send_after(self(), :reconcile, delay_seconds * 1000) end ... end` After: # Conceptual change: # 1. The component starting Oban would notify the reconciler. # e.g., GenServer.cast(SweepScheduleReconciler, :reconcile) # # 2. The reconciler would react to events instead of polling. defmodule SweepScheduleReconciler do use GenServer def init(_opts) do # Optionally reconcile on startup as a fallback reconcile() # No longer needs to manage polling interval state {:ok, %{}} end # Triggered by an external event when Oban is confirmed to be up def handle_cast(:reconcile, state) do reconcile() {:noreply, state} end end Suggestion importance[1-10]: 7 __ Why: This is a strong architectural suggestion that correctly identifies the new polling mechanism and proposes a more efficient, event-driven alternative which would reduce scheduling delays and system overhead.	Medium
Possible issue	Ensure essential jobs run always Modify `handle_groups/1` to always schedule the `SweepMonitorWorker` and `SweepDataCleanupWorker` during reconciliation, regardless of whether any sweep groups exist. elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_schedule_reconciler.ex [58-63] `-defp handle_groups({:ok, []}), do: :ok - defp handle_groups({:ok, _groups}) do + # Always schedule workers if Oban is available, regardless of whether + # there are enabled sweep groups. These workers handle general monitoring + # and cleanup. schedule_worker(SweepMonitorWorker, "sweep monitor") schedule_worker(SweepDataCleanupWorker, "sweep data cleanup") end` Apply / Chat Suggestion importance[1-10]: 7 __ Why: This is a valid logical correction. The monitoring and cleanup workers should run even if no sweep groups are currently enabled, to handle historical data and ensure the system is ready for future groups.	Medium
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2340#issuecomment-3765056765 Original created: 2026-01-18T08:32:57Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Consider a more direct reconciliation trigger</summary> ___ **Instead of having the <code>SweepScheduleReconciler</code> poll periodically, consider an <br>event-driven approach. The reconciliation process should be triggered directly <br>by the component that starts Oban, making job scheduling more immediate.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2340/files#diff-08cc9510c5b0889be570924cefe2965349240dd696a04b28fc3171bde49edbc4R24-R43">elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_schedule_reconciler.ex [24-43]</a> </summary> ```elixir def init(_opts) do interval = Application.get_env(:serviceradar_core, :sweep_schedule_reconcile_interval_seconds, @default_interval_seconds ) schedule_reconcile(0) {:ok, %{interval: interval}} end ... (clipped 10 lines) ``` </details> ### Solution Walkthrough: #### Before: ```elixir # elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_schedule_reconciler.ex defmodule SweepScheduleReconciler do use GenServer def init(_opts) do # ... schedule_reconcile(0) # Run immediately on start {:ok, %{interval: interval}} end def handle_info(:reconcile, state) do reconcile() schedule_reconcile(state.interval) # Schedule next poll {:noreply, state} end defp schedule_reconcile(delay_seconds) do Process.send_after(self(), :reconcile, delay_seconds * 1000) end ... end ``` #### After: ```elixir # Conceptual change: # 1. The component starting Oban would notify the reconciler. # e.g., GenServer.cast(SweepScheduleReconciler, :reconcile) # # 2. The reconciler would react to events instead of polling. defmodule SweepScheduleReconciler do use GenServer def init(_opts) do # Optionally reconcile on startup as a fallback reconcile() # No longer needs to manage polling interval state {:ok, %{}} end # Triggered by an external event when Oban is confirmed to be up def handle_cast(:reconcile, state) do reconcile() {:noreply, state} end end ``` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: This is a strong architectural suggestion that correctly identifies the new polling mechanism and proposes a more efficient, event-driven alternative which would reduce scheduling delays and system overhead. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Ensure essential jobs run always</summary> ___ **Modify <code>handle_groups/1</code> to always schedule the <code>SweepMonitorWorker</code> and <br><code>SweepDataCleanupWorker</code> during reconciliation, regardless of whether any sweep <br>groups exist.** [elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_schedule_reconciler.ex [58-63]](https://github.com/carverauto/serviceradar/pull/2340/files#diff-08cc9510c5b0889be570924cefe2965349240dd696a04b28fc3171bde49edbc4R58-R63) ```diff -defp handle_groups({:ok, []}), do: :ok - defp handle_groups({:ok, _groups}) do + # Always schedule workers if Oban is available, regardless of whether + # there are enabled sweep groups. These workers handle general monitoring + # and cleanup. schedule_worker(SweepMonitorWorker, "sweep monitor") schedule_worker(SweepDataCleanupWorker, "sweep data cleanup") end ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: This is a valid logical correction. The monitoring and cleanup workers should run even if no sweep groups are currently enabled, to handle historical data and ensure the system is ready for future groups. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>