carverauto/serviceradar
1
1
Fork 0
Code Issues 190 Pull requests 16 Projects Releases 173 Packages Wiki Activity Actions

fix: UI performance updates and mapper fixes #3017

Merged
mfreeman451 merged 3 commits from refs/pull/3017/head into staging 2026-03-06 05:55:35 +00:00
Conversation 5 Commits 3 Files changed 60 +3728 -297
mfreeman451 commented 2026-03-06 01:12:07 +00:00 (Migrated from github.com)
Owner
Copy link

Imported from GitHub pull request.

Original GitHub pull request: #2996
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2996
Original created: 2026-03-06T01:12:07Z
Original updated: 2026-03-06T05:55:46Z
Original head: carverauto/serviceradar:bug/slow-device-details-page-load
Original base: staging
Original merged: 2026-03-06T05:55:35Z by @mfreeman451

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?
Imported from GitHub pull request. Original GitHub pull request: #2996 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2996 Original created: 2026-03-06T01:12:07Z Original updated: 2026-03-06T05:55:46Z Original head: carverauto/serviceradar:bug/slow-device-details-page-load Original base: staging Original merged: 2026-03-06T05:55:35Z by @mfreeman451 --- ## 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?
qodo-code-review[bot] commented 2026-03-06 01:13:05 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2996#issuecomment-4008844671
Original created: 2026-03-06T01:13:05Z

Review Summary by Qodo

Sweep-to-mapper promotion with async device loading and SNMP optimizations

✨ Enhancement 🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description 
• **Async device details page loading**: Refactored flow stats and IP enrichment loading in device
  show page to use background tasks with request deduplication, improving UI responsiveness on slow
  connections
• **Sweep-to-mapper promotion feature**: Implemented automatic promotion of sweep-discovered live
  hosts to mapper discovery jobs with cooldown tracking and deduplication to prevent spam
• **Mapper seed override support**: Extended mapper job dispatch to accept promoted IP seed lists
  with audit trail tracking across agent control stream, mapper service, and discovery components
• **SNMP polling optimizations**: Added interface metric probing budget (5 seconds) and limits (64
  interfaces) to prevent excessive polling overhead
• **IP alias-aware polling**: Enhanced SNMP compiler and credential resolver to prefer private IP
  aliases over public canonical IPs for polling targets
• **Inventory rollup optimization**: Implemented database triggers and refresh functions for
  efficient device inventory statistics maintenance during bulk sync operations
• **Interface query optimization**: Refactored SRQL interface queries to reduce duplication and
  defer error metric joins until after deduplication
• **Comprehensive test coverage**: Added E2E tests for sweep results processing, mapper promotion
  dispatch, SNMP alias resolution, and mapper discovery seed overrides
• **Design documentation**: Provided detailed specifications and design documents for the
  sweep-to-mapper promotion feature including requirements, architecture, and implementation tasks
Diagram 
flowchart LR
  A["Sweep Results<br/>Ingestor"] -->|"promote eligible<br/>live hosts"| B["Mapper Promotion<br/>Orchestrator"]
  B -->|"dispatch with<br/>seed IPs"| C["Agent Command<br/>Bus"]
  C -->|"RunScheduledJobWithSeeds"| D["Mapper Discovery<br/>Service"]
  D -->|"reconcile identity &<br/>probe interfaces"| E["Target Device"]
  F["Device Show<br/>Page"] -->|"async load"| G["Flow Stats &<br/>IP Enrichment"]
  H["SNMP Compiler"] -->|"prefer private<br/>alias"| I["Polling Target<br/>Selection"]
  J["Sync Ingestor"] -->|"bulk upsert"| K["Inventory Rollup<br/>Triggers"]
Grey Divider

File Changes

1. elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex ✨ Enhancement +154/-70

Async flow stats loading with request deduplication

• Refactored flow stats and IP enrichment loading to use background tasks with request refs for
 deduplication
• Added flow_stats_request_ref and flow_ip_request_ref assigns to track async operations
• Implemented handle_info callbacks for :flow_stats_loaded and :flow_ip_enrichment_loaded
 messages
• Extracted flow IP extraction logic into reusable flow_ips/1 helper function
• Modified detect_has_interfaces to use aggregation query and added legacy_detect_has_interfaces
 fallback

elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex

2. elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_flow_e2e_test.exs 🧪 Tests +311/-9

Sweep results E2E tests with mapper promotion

• Updated test data structures to use new sweep result format (available instead of
 icmp_available, port_results instead of tcp_ports_open)
• Added comprehensive tests for provisional device creation from available unknown sweep hosts
• Added tests for mapper promotion dispatch with cooldown suppression and skipping logic
• Added helper function results_from/1 to normalize paginated and list-based results

elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_flow_e2e_test.exs

3. elixir/serviceradar_core/lib/serviceradar/sweep_jobs/mapper_promotion.ex ✨ Enhancement +403/-0

New mapper promotion module for sweep discovery

• New module implementing sweep-to-mapper promotion orchestration with cooldown and deduplication
• Evaluates promotion candidates based on device availability, mapper job eligibility, and cooldown
 state
• Dispatches promotions through AgentCommandBus with seed IP lists and trigger source tracking
• Records promotion decisions in device metadata for auditability and cooldown enforcement

elixir/serviceradar_core/lib/serviceradar/sweep_jobs/mapper_promotion.ex

View more (23)
4. elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_results_ingestor.ex ✨ Enhancement +174/-7

Sweep ingestor mapper promotion integration

• Integrated MapperPromotion.promote/5 into batch processing pipeline after device availability
 updates
• Added create_available_unknown_devices/4 to create provisional devices for live unknown sweep
 hosts
• Added mapper promotion stats tracking (mapper_dispatched, mapper_suppressed, mapper_skipped,
 mapper_failed)
• Removed synchronous flow stats extraction from parallel task building

elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_results_ingestor.ex

5. elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs 🧪 Tests +164/-6

Inventory rollup schema and refresh tests

• Added inventory rollup schema setup with tables for device counts, type counts, and vendor counts
• Implemented refresh_device_inventory_rollups() function and trigger for maintaining rollup
 statistics
• Added test for inventory rollup refresh after sync ingest operations
• Improved unique_ip/0 helper to check for IP conflicts in database

elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs

6. elixir/serviceradar_core/priv/repo/migrations/20260306001000_optimize_device_inventory_rollup_bulk_sync.exs ⚙️ Configuration changes +237/-0

Database migration for inventory rollup optimization

• Created migration for refresh_device_inventory_rollups() function with full rollup recalculation
• Implemented trigger function trg_ocsf_devices_inventory_rollup() for incremental rollup updates
 on device changes
• Supports skipping rollup updates during bulk sync via platform.skip_inventory_rollup session
 variable

elixir/serviceradar_core/priv/repo/migrations/20260306001000_optimize_device_inventory_rollup_bulk_sync.exs

7. elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex ✨ Enhancement +73/-21

Sync ingestor inventory rollup integration

• Added maybe_refresh_inventory_rollups/2 to trigger rollup refresh after successful sync
 operations
• Implemented with_inventory_rollup_bypassed/1 to disable triggers during bulk upserts
• Added inventory_rollups_supported?/0 check for graceful degradation on older schemas
• Removed ip_only_updates/1 filter to include all updates in IP lookup

elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex

8. elixir/serviceradar_core/test/serviceradar/snmp_profiles/credential_resolver_test.exs 🧪 Tests +71/-6

SNMP credential resolver alias tests

• Updated tests to use unique hostnames and target queries instead of default profiles
• Added integration test for credential resolution following confirmed IP aliases back to canonical
 device
• Improved test isolation by using unique identifiers for profiles and devices

elixir/serviceradar_core/test/serviceradar/snmp_profiles/credential_resolver_test.exs

9. elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex ✨ Enhancement +103/-2

SNMP compiler alias-aware polling host resolution

• Enhanced resolve_polling_host/2 to prefer private IP aliases when canonical IP is public
• Added preferred_alias_polling_host/2 to select best alias based on state, sighting count, and
 recency
• Implemented load_active_ip_aliases/2 and pick_best_alias_value/1 for alias selection logic
• Added private IP detection functions for IPv4 and IPv6 addresses

elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex

10. elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_compiler_test.exs 🧪 Tests +89/-18

SNMP compiler alias preference tests

• Added integration test for SNMP target preferring confirmed private IP alias over public canonical
 IP
• Simplified test setup by removing redundant profile creation
• Verified that compiler selects private alias when available for polling

elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_compiler_test.exs

11. elixir/serviceradar_core/lib/serviceradar/snmp_profiles/credential_resolver.ex ✨ Enhancement +39/-10

Credential resolver IP alias fallback lookup

• Refactored device lookup into lookup_device_uid_by_fields/2 for direct field matching
• Added lookup_device_uid_by_ip_alias/2 to resolve devices through confirmed/updated IP aliases
• Falls back to alias lookup when direct field lookup fails

elixir/serviceradar_core/lib/serviceradar/snmp_profiles/credential_resolver.ex

12. elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex ✨ Enhancement +26/-2

Agent command bus mapper seed support

• Enhanced run_mapper_job/2 to accept and normalize seed IP lists via options
• Added trigger_source tracking to mapper job payload for audit trail
• Implemented maybe_put/3 helper to conditionally add non-empty values to maps

elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex

13. elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_ingestor_test.exs 🧪 Tests +1/-1

Sweep ingestor test description clarification

• Updated test description to clarify that device_id is nil for unavailable unknown hosts

elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_ingestor_test.exs

14. go/pkg/mapper/discovery.go ✨ Enhancement +88/-11

Mapper discovery seed override and recursive polling

• Added RunScheduledJobWithSeeds/3 method to trigger scheduled jobs with override seed lists
• Implemented normalizeOverrideSeeds/1 to deduplicate and trim seed values
• Extracted reconcileIdentity/1 as separate method called before topology polling
• Added recursivePollingModes/1 to determine SNMP polling modes based on discovery type

go/pkg/mapper/discovery.go

15. go/pkg/mapper/snmp_polling.go ✨ Enhancement +39/-9

Interface metric probing budget and limits

• Added interfaceMetricProbeBudget (5 seconds) and interfaceMetricProbeMax (64 interfaces)
 constants
• Modified probeInterfaceMetrics/3 to enforce time budget and interface count limits
• Added deadline checking to stop probing when budget is exhausted
• Improved logging with target and progress information

go/pkg/mapper/snmp_polling.go

16. go/pkg/mapper/discovery_recursive_enrichment_test.go 🧪 Tests +89/-0

Mapper discovery recursive enrichment tests

• New test file with TestRecursivePollingModes/1 verifying polling mode selection logic
• Added TestFinalizePublishesInterfacesAddedAfterIdentityReconcile/1 to verify interface
 publishing after identity reconciliation
• Implemented recordingInterfacePublisher mock for testing interface publication

go/pkg/mapper/discovery_recursive_enrichment_test.go

17. go/pkg/agent/control_stream.go ✨ Enhancement +14/-3

Agent control stream mapper seed payload support

• Extended mapperRunPayload struct with optional Seeds field for promoted IP lists
• Updated handleMapperRun/3 to call RunScheduledJobWithSeeds/3 when seeds are provided
• Added seeds to result payload when present for audit trail

go/pkg/agent/control_stream.go

18. go/pkg/mapper/discovery_test.go 🧪 Tests +35/-0

Mapper discovery seed override tests

• Added TestRunScheduledJobWithSeedsOverridesSeeds/1 to verify seed normalization and override
 behavior
• Tests deduplication, whitespace trimming, and empty string filtering

go/pkg/mapper/discovery_test.go

19. go/pkg/agent/mapper_service.go ✨ Enhancement +19/-0

Mapper service seed override method

• Added RunScheduledJobWithSeeds/3 method to expose seed override capability through mapper
 service
• Implements interface assertion pattern for optional feature support

go/pkg/agent/mapper_service.go

20. rust/srql/src/query/interfaces.rs ✨ Enhancement +115/-39

SRQL interface query optimization and refactoring

• Extracted interface_select_columns/1 helper to reduce duplication in SELECT clause construction
• Extracted interface_enrichment_joins/1 helper for consistent join logic across queries
• Refactored latest-only interface queries to defer error metric joins until after DISTINCT ON
 deduplication
• Improved query structure to avoid per-history-row timeseries joins

rust/srql/src/query/interfaces.rs

21. rust/srql/src/query/downsample.rs Formatting +3/-3

Import statement reordering

• Reordered imports for consistency and readability

rust/srql/src/query/downsample.rs

22. openspec/changes/add-sweep-mapper-promotion/design.md 📝 Documentation +39/-0

Sweep mapper promotion design specification

• New design document outlining sweep-to-mapper promotion feature goals and architecture
• Documents decision to promote from sweep ingestion with existing mapper job reuse
• Identifies risks around job selection breadth and agent load
• Provides migration plan and open questions for cooldown window and SNMP profile eligibility

openspec/changes/add-sweep-mapper-promotion/design.md

23. openspec/changes/add-sweep-mapper-promotion/specs/network-discovery/spec.md 📝 Documentation +30/-0

Network discovery mapper promotion requirements

• New specification for mapper discovery accepting sweep-promoted targets
• Defines requirements for agent-specific mapper job preference and idempotent dispatch
• Specifies scenarios for promotion dispatch and suppression window enforcement

openspec/changes/add-sweep-mapper-promotion/specs/network-discovery/spec.md

24. openspec/changes/add-sweep-mapper-promotion/specs/sweep-jobs/spec.md 📝 Documentation +30/-0

Sweep jobs mapper promotion requirements

• New specification for sweep results promoting eligible discovery candidates
• Defines requirements for live host evaluation, unavailable host filtering, and decision visibility
• Specifies scenarios for skipped promotions and cooldown-based suppression

openspec/changes/add-sweep-mapper-promotion/specs/sweep-jobs/spec.md

25. openspec/changes/add-sweep-mapper-promotion/proposal.md 📝 Documentation +14/-0

Sweep-to-Mapper Promotion Feature Design Proposal

• Introduces a new feature to automatically promote sweep-discovered live hosts to mapper discovery
 when they are SNMP-capable candidates
• Reuses existing mapper job assignment and command-bus delivery mechanisms to avoid creating
 parallel discovery paths
• Implements promotion tracking with decision recording and suppression reasons for operator
 visibility
• Ensures promotion is bounded and idempotent to prevent spam from repeated sweep hits

openspec/changes/add-sweep-mapper-promotion/proposal.md

26. openspec/changes/add-sweep-mapper-promotion/tasks.md 📝 Documentation +15/-0

Sweep-to-Mapper Promotion Implementation Task Checklist

• Documents completed design tasks including sweep host eligibility rules, mapper job selection
 rules, and deduplication behavior
• Lists implementation tasks covering promotion orchestration, mapper command-bus dispatch, state
 persistence, and telemetry emission
• Outlines validation tasks for testing eligible host promotion, duplicate suppression, and mapper
 job selection scenarios

openspec/changes/add-sweep-mapper-promotion/tasks.md

Grey Divider

Qodo Logo

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2996#issuecomment-4008844671 Original created: 2026-03-06T01:13:05Z --- <h3>Review Summary by Qodo</h3> Sweep-to-mapper promotion with async device loading and SNMP optimizations <code>✨ Enhancement</code> <code>🐞 Bug fix</code> <code>🧪 Tests</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>Walkthroughs</h3> <details open> <summary>Description</summary> <br/> <pre> • **Async device details page loading**: Refactored flow stats and IP enrichment loading in device show page to use background tasks with request deduplication, improving UI responsiveness on slow connections • **Sweep-to-mapper promotion feature**: Implemented automatic promotion of sweep-discovered live hosts to mapper discovery jobs with cooldown tracking and deduplication to prevent spam • **Mapper seed override support**: Extended mapper job dispatch to accept promoted IP seed lists with audit trail tracking across agent control stream, mapper service, and discovery components • **SNMP polling optimizations**: Added interface metric probing budget (5 seconds) and limits (64 interfaces) to prevent excessive polling overhead • **IP alias-aware polling**: Enhanced SNMP compiler and credential resolver to prefer private IP aliases over public canonical IPs for polling targets • **Inventory rollup optimization**: Implemented database triggers and refresh functions for efficient device inventory statistics maintenance during bulk sync operations • **Interface query optimization**: Refactored SRQL interface queries to reduce duplication and defer error metric joins until after deduplication • **Comprehensive test coverage**: Added E2E tests for sweep results processing, mapper promotion dispatch, SNMP alias resolution, and mapper discovery seed overrides • **Design documentation**: Provided detailed specifications and design documents for the sweep-to-mapper promotion feature including requirements, architecture, and implementation tasks </pre> </details> <details> <summary>Diagram</summary> <br/> > ```mermaid flowchart LR A["Sweep Results<br/>Ingestor"] -->|"promote eligible<br/>live hosts"| B["Mapper Promotion<br/>Orchestrator"] B -->|"dispatch with<br/>seed IPs"| C["Agent Command<br/>Bus"] C -->|"RunScheduledJobWithSeeds"| D["Mapper Discovery<br/>Service"] D -->|"reconcile identity &<br/>probe interfaces"| E["Target Device"] F["Device Show<br/>Page"] -->|"async load"| G["Flow Stats &<br/>IP Enrichment"] H["SNMP Compiler"] -->|"prefer private<br/>alias"| I["Polling Target<br/>Selection"] J["Sync Ingestor"] -->|"bulk upsert"| K["Inventory Rollup<br/>Triggers"] ``` </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>File Changes</h3> <details> <summary>1. elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex <code>✨ Enhancement</code> <code> +154/-70 </code> </summary> <br/> >Async flow stats loading with request deduplication ><pre> >• Refactored flow stats and IP enrichment loading to use background tasks with request refs for > deduplication >• Added <b><i>flow_stats_request_ref</i></b> and <b><i>flow_ip_request_ref</i></b> assigns to track async operations >• Implemented <b><i>handle_info</i></b> callbacks for <b><i>:flow_stats_loaded</i></b> and <b><i>:flow_ip_enrichment_loaded</i></b> > messages >• Extracted flow IP extraction logic into reusable <b><i>flow_ips/1</i></b> helper function >• Modified <b><i>detect_has_interfaces</i></b> to use aggregation query and added <b><i>legacy_detect_has_interfaces</i></b> > fallback ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-44e1802aef19a1badfee332ded1bfa0e83fe2da9340d6ce61fbb5c00d0b055c8'> elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex </a> <hr/> </details> <details> <summary>2. elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_flow_e2e_test.exs <code>🧪 Tests</code> <code> +311/-9 </code> </summary> <br/> >Sweep results E2E tests with mapper promotion ><pre> >• Updated test data structures to use new sweep result format (<b><i>available</i></b> instead of > <b><i>icmp_available</i></b>, <b><i>port_results</i></b> instead of <b><i>tcp_ports_open</i></b>) >• Added comprehensive tests for provisional device creation from available unknown sweep hosts >• Added tests for mapper promotion dispatch with cooldown suppression and skipping logic >• Added helper function <b><i>results_from/1</i></b> to normalize paginated and list-based results ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-d400921d5787a7de52aab03e0725f804a083c2170bf62c4ede876bb4e73a5c70'> elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_flow_e2e_test.exs </a> <hr/> </details> <details> <summary>3. elixir/serviceradar_core/lib/serviceradar/sweep_jobs/mapper_promotion.ex <code>✨ Enhancement</code> <code> +403/-0 </code> </summary> <br/> >New mapper promotion module for sweep discovery ><pre> >• New module implementing sweep-to-mapper promotion orchestration with cooldown and deduplication >• Evaluates promotion candidates based on device availability, mapper job eligibility, and cooldown > state >• Dispatches promotions through <b><i>AgentCommandBus</i></b> with seed IP lists and trigger source tracking >• Records promotion decisions in device metadata for auditability and cooldown enforcement ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-bfe5f8b1954744a4adbd21a93051e60fbc29bb4cacaca75644c807692fe7e801'> elixir/serviceradar_core/lib/serviceradar/sweep_jobs/mapper_promotion.ex </a> <hr/> </details> <details><summary><ins><strong>View more (23)</strong></ins></summary><br/> <details> <summary>4. elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_results_ingestor.ex <code>✨ Enhancement</code> <code> +174/-7 </code> </summary> <br/> >Sweep ingestor mapper promotion integration ><pre> >• Integrated <b><i>MapperPromotion.promote/5</i></b> into batch processing pipeline after device availability > updates >• Added <b><i>create_available_unknown_devices/4</i></b> to create provisional devices for live unknown sweep > hosts >• Added mapper promotion stats tracking (<b><i>mapper_dispatched</i></b>, <b><i>mapper_suppressed</i></b>, <b><i>mapper_skipped</i></b>, > <b><i>mapper_failed</i></b>) >• Removed synchronous flow stats extraction from parallel task building ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-06f4b3bf56e5f1d122b25040ea7f321125d6cae20606811dc0b2a0ddc7a66226'> elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_results_ingestor.ex </a> <hr/> </details> <details> <summary>5. elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs <code>🧪 Tests</code> <code> +164/-6 </code> </summary> <br/> >Inventory rollup schema and refresh tests ><pre> >• Added inventory rollup schema setup with tables for device counts, type counts, and vendor counts >• Implemented <b><i>refresh_device_inventory_rollups()</i></b> function and trigger for maintaining rollup > statistics >• Added test for inventory rollup refresh after sync ingest operations >• Improved <b><i>unique_ip/0</i></b> helper to check for IP conflicts in database ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-9a0dabe8ab58de6380fcc7be4060502a561ba1b2b378366c16b4734a41545026'> elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs </a> <hr/> </details> <details> <summary>6. elixir/serviceradar_core/priv/repo/migrations/20260306001000_optimize_device_inventory_rollup_bulk_sync.exs <code>⚙️ Configuration changes</code> <code> +237/-0 </code> </summary> <br/> >Database migration for inventory rollup optimization ><pre> >• Created migration for <b><i>refresh_device_inventory_rollups()</i></b> function with full rollup recalculation >• Implemented trigger function <b><i>trg_ocsf_devices_inventory_rollup()</i></b> for incremental rollup updates > on device changes >• Supports skipping rollup updates during bulk sync via <b><i>platform.skip_inventory_rollup</i></b> session > variable ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-6b21a3327b2f8f9d24717e3969587f3a84ac479f4b5f82e5e1fd2df4883928b4'> elixir/serviceradar_core/priv/repo/migrations/20260306001000_optimize_device_inventory_rollup_bulk_sync.exs </a> <hr/> </details> <details> <summary>7. elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex <code>✨ Enhancement</code> <code> +73/-21 </code> </summary> <br/> >Sync ingestor inventory rollup integration ><pre> >• Added <b><i>maybe_refresh_inventory_rollups/2</i></b> to trigger rollup refresh after successful sync > operations >• Implemented <b><i>with_inventory_rollup_bypassed/1</i></b> to disable triggers during bulk upserts >• Added <b><i>inventory_rollups_supported?/0</i></b> check for graceful degradation on older schemas >• Removed <b><i>ip_only_updates/1</i></b> filter to include all updates in IP lookup ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-fdf70a310cef758f735fae943c2a33bc7f851a1c3d1ba66499e911fd2bc5611a'> elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex </a> <hr/> </details> <details> <summary>8. elixir/serviceradar_core/test/serviceradar/snmp_profiles/credential_resolver_test.exs <code>🧪 Tests</code> <code> +71/-6 </code> </summary> <br/> >SNMP credential resolver alias tests ><pre> >• Updated tests to use unique hostnames and target queries instead of default profiles >• Added integration test for credential resolution following confirmed IP aliases back to canonical > device >• Improved test isolation by using unique identifiers for profiles and devices ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-44086ae95a2e488cbb69787a6f31f82fae1eed374e65af0a8b21b007a0405810'> elixir/serviceradar_core/test/serviceradar/snmp_profiles/credential_resolver_test.exs </a> <hr/> </details> <details> <summary>9. elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex <code>✨ Enhancement</code> <code> +103/-2 </code> </summary> <br/> >SNMP compiler alias-aware polling host resolution ><pre> >• Enhanced <b><i>resolve_polling_host/2</i></b> to prefer private IP aliases when canonical IP is public >• Added <b><i>preferred_alias_polling_host/2</i></b> to select best alias based on state, sighting count, and > recency >• Implemented <b><i>load_active_ip_aliases/2</i></b> and <b><i>pick_best_alias_value/1</i></b> for alias selection logic >• Added private IP detection functions for IPv4 and IPv6 addresses ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-3f9551d39c5e77c379f701d5dee9c0a3d95e2a7861ba7515c402e10b585c61e0'> elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex </a> <hr/> </details> <details> <summary>10. elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_compiler_test.exs <code>🧪 Tests</code> <code> +89/-18 </code> </summary> <br/> >SNMP compiler alias preference tests ><pre> >• Added integration test for SNMP target preferring confirmed private IP alias over public canonical > IP >• Simplified test setup by removing redundant profile creation >• Verified that compiler selects private alias when available for polling ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-00ebe1280516fdb784057290bda325a48925b8fd56219583641135addc9e6e69'> elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_compiler_test.exs </a> <hr/> </details> <details> <summary>11. elixir/serviceradar_core/lib/serviceradar/snmp_profiles/credential_resolver.ex <code>✨ Enhancement</code> <code> +39/-10 </code> </summary> <br/> >Credential resolver IP alias fallback lookup ><pre> >• Refactored device lookup into <b><i>lookup_device_uid_by_fields/2</i></b> for direct field matching >• Added <b><i>lookup_device_uid_by_ip_alias/2</i></b> to resolve devices through confirmed/updated IP aliases >• Falls back to alias lookup when direct field lookup fails ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-cbe8eb776c03d1b1aeec3aa97ee50f43846c00a77088f4554875378a656f2f92'> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/credential_resolver.ex </a> <hr/> </details> <details> <summary>12. elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex <code>✨ Enhancement</code> <code> +26/-2 </code> </summary> <br/> >Agent command bus mapper seed support ><pre> >• Enhanced <b><i>run_mapper_job/2</i></b> to accept and normalize seed IP lists via options >• Added <b><i>trigger_source</i></b> tracking to mapper job payload for audit trail >• Implemented <b><i>maybe_put/3</i></b> helper to conditionally add non-empty values to maps ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-b56572e2bef2c1df227eb7272cd24835dcd1ebd6e164a23e56d5177fd3dd14b5'> elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex </a> <hr/> </details> <details> <summary>13. elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_ingestor_test.exs <code>🧪 Tests</code> <code> +1/-1 </code> </summary> <br/> >Sweep ingestor test description clarification ><pre> >• Updated test description to clarify that device_id is nil for unavailable unknown hosts ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-e2950ac645d9bd3ed1cb91796ee4391125965c90ce6e34e979af6b4f64e69087'> elixir/serviceradar_core/test/serviceradar/sweep_jobs/sweep_results_ingestor_test.exs </a> <hr/> </details> <details> <summary>14. go/pkg/mapper/discovery.go <code>✨ Enhancement</code> <code> +88/-11 </code> </summary> <br/> >Mapper discovery seed override and recursive polling ><pre> >• Added <b><i>RunScheduledJobWithSeeds/3</i></b> method to trigger scheduled jobs with override seed lists >• Implemented <b><i>normalizeOverrideSeeds/1</i></b> to deduplicate and trim seed values >• Extracted <b><i>reconcileIdentity/1</i></b> as separate method called before topology polling >• Added <b><i>recursivePollingModes/1</i></b> to determine SNMP polling modes based on discovery type ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-2ba7e064eb7e00e4c9fd0d978cf058b900bfaa9dc1a448f3f9e252400f8f7547'> go/pkg/mapper/discovery.go </a> <hr/> </details> <details> <summary>15. go/pkg/mapper/snmp_polling.go <code>✨ Enhancement</code> <code> +39/-9 </code> </summary> <br/> >Interface metric probing budget and limits ><pre> >• Added <b><i>interfaceMetricProbeBudget</i></b> (5 seconds) and <b><i>interfaceMetricProbeMax</i></b> (64 interfaces) > constants >• Modified <b><i>probeInterfaceMetrics/3</i></b> to enforce time budget and interface count limits >• Added deadline checking to stop probing when budget is exhausted >• Improved logging with target and progress information ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-d2c6917ecde1b7092be872ce277e4fd186fb97890e6ec5252208593ef23e5d82'> go/pkg/mapper/snmp_polling.go </a> <hr/> </details> <details> <summary>16. go/pkg/mapper/discovery_recursive_enrichment_test.go <code>🧪 Tests</code> <code> +89/-0 </code> </summary> <br/> >Mapper discovery recursive enrichment tests ><pre> >• New test file with <b><i>TestRecursivePollingModes/1</i></b> verifying polling mode selection logic >• Added <b><i>TestFinalizePublishesInterfacesAddedAfterIdentityReconcile/1</i></b> to verify interface > publishing after identity reconciliation >• Implemented <b><i>recordingInterfacePublisher</i></b> mock for testing interface publication ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-7a1d01467b0d3280c031b7ee5508454b5efaf37e187f35721e33c0989b3ba6eb'> go/pkg/mapper/discovery_recursive_enrichment_test.go </a> <hr/> </details> <details> <summary>17. go/pkg/agent/control_stream.go <code>✨ Enhancement</code> <code> +14/-3 </code> </summary> <br/> >Agent control stream mapper seed payload support ><pre> >• Extended <b><i>mapperRunPayload</i></b> struct with optional <b><i>Seeds</i></b> field for promoted IP lists >• Updated <b><i>handleMapperRun/3</i></b> to call <b><i>RunScheduledJobWithSeeds/3</i></b> when seeds are provided >• Added seeds to result payload when present for audit trail ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-49c885585f5fa1050d4a64dcf16f525a2ba0fd1fad5e280c8080ba54b28635aa'> go/pkg/agent/control_stream.go </a> <hr/> </details> <details> <summary>18. go/pkg/mapper/discovery_test.go <code>🧪 Tests</code> <code> +35/-0 </code> </summary> <br/> >Mapper discovery seed override tests ><pre> >• Added <b><i>TestRunScheduledJobWithSeedsOverridesSeeds/1</i></b> to verify seed normalization and override > behavior >• Tests deduplication, whitespace trimming, and empty string filtering ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-5e1b5db7debaa61fed6b0e04a601d22651121e2c8e752839f125fd73c72690b8'> go/pkg/mapper/discovery_test.go </a> <hr/> </details> <details> <summary>19. go/pkg/agent/mapper_service.go <code>✨ Enhancement</code> <code> +19/-0 </code> </summary> <br/> >Mapper service seed override method ><pre> >• Added <b><i>RunScheduledJobWithSeeds/3</i></b> method to expose seed override capability through mapper > service >• Implements interface assertion pattern for optional feature support ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-e0e5595578abbe491087210177362edb9c0bc8b0669dcd24f28ed6c69edc189e'> go/pkg/agent/mapper_service.go </a> <hr/> </details> <details> <summary>20. rust/srql/src/query/interfaces.rs <code>✨ Enhancement</code> <code> +115/-39 </code> </summary> <br/> >SRQL interface query optimization and refactoring ><pre> >• Extracted <b><i>interface_select_columns/1</i></b> helper to reduce duplication in SELECT clause construction >• Extracted <b><i>interface_enrichment_joins/1</i></b> helper for consistent join logic across queries >• Refactored latest-only interface queries to defer error metric joins until after DISTINCT ON > deduplication >• Improved query structure to avoid per-history-row timeseries joins ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-1ec833d4525fb2806523888b8c57b76ca8dd2ab70539368ccecc4d262769c8c5'> rust/srql/src/query/interfaces.rs </a> <hr/> </details> <details> <summary>21. rust/srql/src/query/downsample.rs <code> Formatting </code> <code> +3/-3 </code> </summary> <br/> >Import statement reordering ><pre> >• Reordered imports for consistency and readability ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-94f68b4684578afa112ab05ff903667b6cd902ad276e17c12af350078b300a6a'> rust/srql/src/query/downsample.rs </a> <hr/> </details> <details> <summary>22. openspec/changes/add-sweep-mapper-promotion/design.md <code>📝 Documentation</code> <code> +39/-0 </code> </summary> <br/> >Sweep mapper promotion design specification ><pre> >• New design document outlining sweep-to-mapper promotion feature goals and architecture >• Documents decision to promote from sweep ingestion with existing mapper job reuse >• Identifies risks around job selection breadth and agent load >• Provides migration plan and open questions for cooldown window and SNMP profile eligibility ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-e7328c9a6e94fb3cee9104eb621b75be39ddbf62bc3afeff6699c52e41cdcf55'> openspec/changes/add-sweep-mapper-promotion/design.md </a> <hr/> </details> <details> <summary>23. openspec/changes/add-sweep-mapper-promotion/specs/network-discovery/spec.md <code>📝 Documentation</code> <code> +30/-0 </code> </summary> <br/> >Network discovery mapper promotion requirements ><pre> >• New specification for mapper discovery accepting sweep-promoted targets >• Defines requirements for agent-specific mapper job preference and idempotent dispatch >• Specifies scenarios for promotion dispatch and suppression window enforcement ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-2c4d7d9b2975acd51b8e291bac896deac0e2136b61e0529859f2e0ca3b9021c2'> openspec/changes/add-sweep-mapper-promotion/specs/network-discovery/spec.md </a> <hr/> </details> <details> <summary>24. openspec/changes/add-sweep-mapper-promotion/specs/sweep-jobs/spec.md <code>📝 Documentation</code> <code> +30/-0 </code> </summary> <br/> >Sweep jobs mapper promotion requirements ><pre> >• New specification for sweep results promoting eligible discovery candidates >• Defines requirements for live host evaluation, unavailable host filtering, and decision visibility >• Specifies scenarios for skipped promotions and cooldown-based suppression ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-2a4b0b6d61a89de69068da88440088ca3ba1d2c4e46b022801f27b975e9a32fb'> openspec/changes/add-sweep-mapper-promotion/specs/sweep-jobs/spec.md </a> <hr/> </details> <details> <summary>25. openspec/changes/add-sweep-mapper-promotion/proposal.md <code>📝 Documentation</code> <code> +14/-0 </code> </summary> <br/> >Sweep-to-Mapper Promotion Feature Design Proposal ><pre> >• Introduces a new feature to automatically promote sweep-discovered live hosts to mapper discovery > when they are SNMP-capable candidates >• Reuses existing mapper job assignment and command-bus delivery mechanisms to avoid creating > parallel discovery paths >• Implements promotion tracking with decision recording and suppression reasons for operator > visibility >• Ensures promotion is bounded and idempotent to prevent spam from repeated sweep hits ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-1b932b1207d8f5ffef745df741fa5d75f8c2d37f3de8b47c7d8f4ef021d255ce'> openspec/changes/add-sweep-mapper-promotion/proposal.md </a> <hr/> </details> <details> <summary>26. openspec/changes/add-sweep-mapper-promotion/tasks.md <code>📝 Documentation</code> <code> +15/-0 </code> </summary> <br/> >Sweep-to-Mapper Promotion Implementation Task Checklist ><pre> >• Documents completed design tasks including sweep host eligibility rules, mapper job selection > rules, and deduplication behavior >• Lists implementation tasks covering promotion orchestration, mapper command-bus dispatch, state > persistence, and telemetry emission >• Outlines validation tasks for testing eligible host promotion, duplicate suppression, and mapper > job selection scenarios ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2996/files#diff-f141e6409bfebfa44e0d4f37f30ec8674a32c19efb7e5e0f0d0dd0159f1e38a8'> openspec/changes/add-sweep-mapper-promotion/tasks.md </a> <hr/> </details> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>
qodo-code-review[bot] commented 2026-03-06 01:13:07 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2996#issuecomment-4008844757
Original created: 2026-03-06T01:13:07Z

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider
Action required
1. ensure_inventory_rollup_schema! runs DDL 📘 Rule violation ⛯ Reliability
Description 
New test code performs schema/DDL changes via Repo.query! (CREATE TABLE, `CREATE OR REPLACE
FUNCTION`) instead of relying on Elixir migrations. This can cause schema drift and violates the
requirement that all database schema changes be managed exclusively via migrations.
Code

elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[R535-605]

+  defp ensure_inventory_rollup_schema! do
+    Repo.query!("""
+    CREATE TABLE IF NOT EXISTS platform.device_inventory_counts (
+      key text PRIMARY KEY,
+      value bigint NOT NULL DEFAULT 0,
+      updated_at timestamptz NOT NULL DEFAULT now()
+    )
+    """)
+
+    Repo.query!("""
+    CREATE TABLE IF NOT EXISTS platform.device_inventory_type_counts (
+      type text PRIMARY KEY,
+      count bigint NOT NULL DEFAULT 0,
+      updated_at timestamptz NOT NULL DEFAULT now()
+    )
+    """)
+
+    Repo.query!("""
+    CREATE TABLE IF NOT EXISTS platform.device_inventory_vendor_counts (
+      vendor_name text PRIMARY KEY,
+      count bigint NOT NULL DEFAULT 0,
+      updated_at timestamptz NOT NULL DEFAULT now()
+    )
+    """)
+
+    Repo.query!("""
+    CREATE OR REPLACE FUNCTION platform.refresh_device_inventory_rollups()
+    RETURNS void
+    LANGUAGE plpgsql
+    AS $$
+    BEGIN
+      TRUNCATE TABLE platform.device_inventory_counts;
+      TRUNCATE TABLE platform.device_inventory_type_counts;
+      TRUNCATE TABLE platform.device_inventory_vendor_counts;
+
+      INSERT INTO platform.device_inventory_counts (key, value, updated_at)
+      SELECT 'total', COUNT(*)::bigint, now()
+      FROM platform.ocsf_devices
+      WHERE deleted_at IS NULL;
+
+      INSERT INTO platform.device_inventory_counts (key, value, updated_at)
+      SELECT 'available', COUNT(*)::bigint, now()
+      FROM platform.ocsf_devices
+      WHERE deleted_at IS NULL
+        AND COALESCE(is_available, false) = true;
+
+      INSERT INTO platform.device_inventory_counts (key, value, updated_at)
+      SELECT 'unavailable', COUNT(*)::bigint, now()
+      FROM platform.ocsf_devices
+      WHERE deleted_at IS NULL
+        AND COALESCE(is_available, false) = false;
+
+      INSERT INTO platform.device_inventory_type_counts (type, count, updated_at)
+      SELECT COALESCE(NULLIF(trim(type), ''), 'Unknown') AS type,
+             COUNT(*)::bigint AS count,
+             now()
+      FROM platform.ocsf_devices
+      WHERE deleted_at IS NULL
+      GROUP BY COALESCE(NULLIF(trim(type), ''), 'Unknown');
+
+      INSERT INTO platform.device_inventory_vendor_counts (vendor_name, count, updated_at)
+      SELECT COALESCE(NULLIF(trim(vendor_name), ''), 'Unknown') AS vendor_name,
+             COUNT(*)::bigint AS count,
+             now()
+      FROM platform.ocsf_devices
+      WHERE deleted_at IS NULL
+      GROUP BY COALESCE(NULLIF(trim(vendor_name), ''), 'Unknown');
+    END;
+    $$;
+    """)
+  end
Evidence 
PR Compliance ID 10 requires schema/DDL changes to be implemented only via migrations. The added
test helper ensure_inventory_rollup_schema!/0 executes DDL statements directly (`CREATE TABLE IF
NOT EXISTS ..., CREATE OR REPLACE FUNCTION ...) via Repo.query!`.

AGENTS.md
elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[535-605]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new test helper (`ensure_inventory_rollup_schema!/0`) executes DDL (`CREATE TABLE`, `CREATE OR REPLACE FUNCTION`) via `Repo.query!`, which violates the requirement that schema changes be managed exclusively by Elixir migrations.
## Issue Context
The PR already adds a migration that creates/replaces `platform.refresh_device_inventory_rollups()` and related trigger function. Tests should not create/alter schema at runtime; instead, ensure migrations are applied for the test DB (or use proper test setup/migration helpers).
## Fix Focus Areas
- elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[535-605]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Invalid SQL: JOIN after WHERE 🐞 Bug ✓ Correctness
Description 
For non-latest interface queries, the builder constructs FROM ... WHERE ... and then appends
LEFT JOIN ..., producing invalid SQL (JOINs cannot appear after WHERE). This will break interface
queries at runtime (likely surfacing as query failures in UI/API paths that fetch interfaces).
Code

rust/srql/src/query/interfaces.rs[R241-270]

+    let mut discovered_interfaces_from = String::from("FROM discovered_interfaces di");
 if !clauses.is_empty() {
-        base_select.push_str(" WHERE ");
-        base_select.push_str(&clauses.join(" AND "));
+        discovered_interfaces_from.push_str(" WHERE ");
+        discovered_interfaces_from.push_str(&clauses.join(" AND "));
 }
 let (sql, binds) = if latest_only {
     let mut inner = String::from("SELECT DISTINCT ON (di.device_id, di.interface_uid) ");
-        inner.push_str(&base_select["SELECT ".len()..]);
+        inner.push_str(&interface_select_columns("di"));
+        inner.push(' ');
+        inner.push_str(&discovered_interfaces_from);
     inner.push_str(
         " ORDER BY di.device_id, di.interface_uid, di.timestamp DESC, di.created_at DESC",
     );
-        let mut outer = format!("SELECT * FROM ({inner}) AS latest");
+        let mut outer = String::from("SELECT ");
+        outer.push_str(&interface_select_columns("latest"));
+        outer.push_str(
+            ", tm_in.value AS in_errors, tm_out.value AS out_errors, \
+            COALESCE(ifs.favorited, false) AS favorited, \
+            COALESCE(ifs.metrics_enabled, false) AS metrics_enabled \
+            FROM (",
+        );
+        outer.push_str(&inner);
+        outer.push_str(") AS latest");
+        outer.push_str(&interface_enrichment_joins("latest"));
+
     if let Some(order_clause) = build_order_clause(&plan.order) {
         outer.push(' ');
         outer.push_str(&order_clause);
Evidence 
discovered_interfaces_from is built including the WHERE clause, and later
interface_enrichment_joins("di") (which begins with LEFT JOIN ...) is concatenated *after* it
for the non-latest query path, yielding FROM ... WHERE ... LEFT JOIN ....

rust/srql/src/query/interfaces.rs[241-246]
rust/srql/src/query/interfaces.rs[278-287]
rust/srql/src/query/interfaces.rs[192-215]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The non-`latest` interfaces SQL builder concatenates `LEFT JOIN` clauses after an already-built `FROM ... WHERE ...` fragment, generating invalid SQL.
### Issue Context
- `interface_enrichment_joins()` returns a string starting with `LEFT JOIN ...`.
- `discovered_interfaces_from` currently includes the `WHERE` clause.
### Fix Focus Areas
- rust/srql/src/query/interfaces.rs[241-246]
- rust/srql/src/query/interfaces.rs[278-287]
- rust/srql/src/query/interfaces.rs[192-215]
### Suggested implementation direction
- Keep a `where_clause` string (e.g., empty or `&amp;amp;amp;quot; WHERE ...&amp;amp;amp;quot;`).
- For non-latest queries, build:
- `FROM discovered_interfaces di`
- append `interface_enrichment_joins(&amp;amp;amp;quot;di&amp;amp;amp;quot;)`
- append `where_clause`
- For latest queries, keep joins only on the outer query (current approach is fine).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Flow stats loading can stall 🐞 Bug ⛯ Reliability
Description 
Flow stats are now loaded in an unlinked background task and flow_stats_loading is only cleared
when a matching :flow_stats_loaded message arrives. If the task crashes (e.g., due to
Jason.encode!/1) or never sends, the view can remain stuck in a perpetual loading state.
Code

elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[R203-226]

+  def handle_info({:flow_stats_loaded, device_uid, request_ref, stats_bundle}, socket) do
+    current_ref = Map.get(socket.assigns, :flow_stats_request_ref)
+
+    if device_uid == socket.assigns.device_uid and request_ref == current_ref do
+      {flow_stats, sparkline_json, proto_json, chart_keys, chart_points, top_talkers_json,
+       top_destinations_json, top_ports_json, top_protocols_json, facets} = stats_bundle
+
+      {:noreply,
+       socket
+       |> assign(:flow_stats, flow_stats)
+       |> assign(:flow_stats_loading, false)
+       |> assign(:flow_sparkline_json, sparkline_json)
+       |> assign(:flow_proto_json, proto_json)
+       |> assign(:flow_chart_keys_json, chart_keys)
+       |> assign(:flow_chart_points_json, chart_points)
+       |> assign(:flow_top_talkers_json, top_talkers_json)
+       |> assign(:flow_top_destinations_json, top_destinations_json)
+       |> assign(:flow_top_ports_json, top_ports_json)
+       |> assign(:flow_top_protocols_json, top_protocols_json)
+       |> assign(:flow_facets, facets)}
+    else
+      {:noreply, socket}
+    end
+  end
Evidence 
begin_flow_stats_refresh/2 sets flow_stats_loading to true and starts a Task.start/1 that must
send :flow_stats_loaded back. The only code path that sets loading back to false is the
handle_info/2 clause for that message. But load_device_flow_stats/4 uses Jason.encode!/1,
which can raise and kill the task before it sends a message.

elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1744-1772]
elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[203-225]
elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1843-1863]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`flow_stats_loading` can remain `true` indefinitely if the `Task.start/1` crashes (e.g., `Jason.encode!/1` raises) or never sends a `:flow_stats_loaded` message.
### Issue Context
- Loading is only cleared in the `handle_info({:flow_stats_loaded, ...})` clause.
- The task is unlinked and not monitored; failures are silent.
### Fix Focus Areas
- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1744-1772]
- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[203-225]
- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1843-1863]
### Suggested implementation direction
- Wrap the task body:
- `try do ... rescue e -&amp;amp;amp;gt; send(parent, {:flow_stats_loaded, uid, request_ref, empty_bundle}) end`
- and/or send a dedicated `{:flow_stats_failed, ...}` message and handle it by setting `flow_stats_loading: false`.
- Add a timeout message keyed by `request_ref` that clears loading if the task never responds.
- Consider using `Task.Supervisor.async_nolink` + monitor to surface failures and keep behavior consistent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Remediation recommended
4. SyncIngestor uses raw SQL ☑ 📘 Rule violation ⛯ Reliability
Description 
New inventory sync logic introduces additional direct Repo.query! calls to manipulate database
session settings and invoke rollup refresh functions. This bypasses Ash patterns and increases
coupling to SQL/session behavior in domain logic.
Code

elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[R514-560]

+  defp with_inventory_rollup_bypassed(fun) when is_function(fun, 0) do
+    Repo.transaction(
+      fn ->
+        Repo.query!("SET LOCAL platform.skip_inventory_rollup = 'on'")
+        fun.()
+      end,
+      timeout: :infinity
+    )
+  end
+
+  defp refresh_inventory_rollups do
+    if inventory_rollups_supported?() do
+      case Repo.transaction(
+             fn ->
+               Repo.query!("SELECT pg_advisory_xact_lock($1)", [
+                 @inventory_rollup_refresh_lock_key
+               ])
+
+               Repo.query!("SELECT platform.refresh_device_inventory_rollups()")
+             end,
+             timeout: :infinity
+           ) do
+        {:ok, _} ->
+          :ok
+
+        {:error, reason} ->
+          Logger.warning("SyncIngestor: Failed to refresh inventory rollups: #{inspect(reason)}")
+          {:error, reason}
+      end
+    else
+      :ok
+    end
+  end
+
+  defp inventory_rollups_supported? do
+    case Repo.query(
+           "SELECT to_regprocedure('platform.refresh_device_inventory_rollups()') IS NOT NULL",
+           []
+         ) do
+      {:ok, %{rows: [[true]]}} ->
+        :ok
+        true
+
+      _ ->
+        false
+    end
+  end
Evidence 
PR Compliance ID 7 asks Elixir domain/data-access changes to use Ash concepts rather than direct
Ecto/SQL usage. The new functions with_inventory_rollup_bypassed/1, refresh_inventory_rollups/0,
and inventory_rollups_supported?/0 use Repo.query!/Repo.query with raw SQL for transactional
settings/locks and function execution.

AGENTS.md
elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[514-560]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New inventory sync code uses raw SQL via `Repo.query!/Repo.query` to set session flags, acquire advisory locks, and call rollup refresh functions, which conflicts with the guideline to use Ash concepts rather than direct Ecto/SQL in Elixir changes.
## Issue Context
This may be necessary for performance, but the compliance requirement prefers Ash patterns. If raw SQL is unavoidable, consider encapsulating it behind an existing DB utility layer (if present) or an Ash action/extension to keep domain code consistent.
## Fix Focus Areas
- elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[514-560]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. Rollup refresh can raise 🐞 Bug ⛯ Reliability
Description 
Inventory rollup refresh uses Repo.query!/2 inside Repo.transaction/2 while relying on tuple
matching ({:ok, _}/{:error, reason}); exceptions from query! will bypass the `{:error,
reason} branch and can crash ingest_updates/2` after processing batches.
Code

elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[R524-542]

+  defp refresh_inventory_rollups do
+    if inventory_rollups_supported?() do
+      case Repo.transaction(
+             fn ->
+               Repo.query!("SELECT pg_advisory_xact_lock($1)", [
+                 @inventory_rollup_refresh_lock_key
+               ])
+
+               Repo.query!("SELECT platform.refresh_device_inventory_rollups()")
+             end,
+             timeout: :infinity
+           ) do
+        {:ok, _} ->
+          :ok
+
+        {:error, reason} ->
+          Logger.warning("SyncIngestor: Failed to refresh inventory rollups: #{inspect(reason)}")
+          {:error, reason}
+      end
Evidence 
The transaction body uses bang queries (Repo.query!/2). The surrounding case only handles tuple
return values from Repo.transaction/2, so raised exceptions won’t be converted into `{:error,
reason}` for logging/controlled return and can take down the caller.

elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[524-542]
elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[101-106]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`refresh_inventory_rollups/0` uses `Repo.query!/2` inside a transaction but only handles `{:ok, _}` / `{:error, reason}` tuples. If a bang query raises, the exception bypasses the error-handling/logging branch and can crash `ingest_updates/2`.
### Issue Context
The intent appears to be “best-effort refresh with logging,” but current implementation can escalate to a crash.
### Fix Focus Areas
- elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[524-542]
### Suggested implementation direction
- Replace `Repo.query!` with `Repo.query`.
- On `{:error, reason}`, call `Repo.rollback(reason)` so the transaction returns `{:error, reason}`.
- Optionally wrap the whole refresh in `try/rescue` and return `{:error, exception}` while logging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. Seeds trim may crash 🐞 Bug ⛯ Reliability
Description 
run_mapper_job/2 trims seeds with String.trim/1 without validating seed types; any non-binary
seed will raise and prevent dispatch. This is especially risky as seeds are now part of the public
payload surface for mapper runs.
Code

elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex[R149-154]

+    seeds =
+      opts
+      |> Keyword.get(:seeds, [])
+      |> Enum.map(&String.trim/1)
+      |> Enum.reject(&(&1 == ""))
+      |> Enum.uniq()
Evidence 
The function unconditionally maps String.trim/1 over opts[:seeds]. String.trim/1 only accepts
binaries; invalid list elements will crash the call.

elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex[148-155]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AgentCommandBus.run_mapper_job/2` can crash if `opts[:seeds]` contains any non-binary values because it calls `String.trim/1` directly.
### Issue Context
Seeds are now propagated into mapper command payloads and may come from various sources (UI input, API, promotion code).
### Fix Focus Areas
- elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex[148-155]
### Suggested implementation direction
- Change normalization to:
- `|&amp;amp;amp;gt; Enum.filter(&amp;amp;amp;amp;is_binary/1)` before `Enum.map(&amp;amp;amp;amp;String.trim/1)`
- (Optional) cap seeds length/count if needed to protect agent workload.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Grey Divider 
ⓘ The new review experience is currently in Beta. Learn more
Grey Divider

Qodo Logo

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2996#issuecomment-4008844757 Original created: 2026-03-06T01:13:07Z --- <h3>Code Review by Qodo</h3> <code>🐞 Bugs (4)</code> <code>📘 Rule violations (2)</code> <code>📎 Requirement gaps (0)</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <br/> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/action-required.png" height="20" alt="Action required"> <details> <summary> 1. <b><i>ensure_inventory_rollup_schema!</i></b> runs DDL <code>📘 Rule violation</code> <code>⛯ Reliability</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >New test code performs schema/DDL changes via <b><i>Repo.query!</i></b> (<b><i>CREATE TABLE</i></b>, `CREATE OR REPLACE >FUNCTION`) instead of relying on Elixir migrations. This can cause schema drift and violates the >requirement that all database schema changes be managed exclusively via migrations. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[R535-605]](https://github.com/carverauto/serviceradar/pull/2996/files#diff-9a0dabe8ab58de6380fcc7be4060502a561ba1b2b378366c16b4734a41545026R535-R605)</code> > >```diff >+ defp ensure_inventory_rollup_schema! do >+ Repo.query!(""" >+ CREATE TABLE IF NOT EXISTS platform.device_inventory_counts ( >+ key text PRIMARY KEY, >+ value bigint NOT NULL DEFAULT 0, >+ updated_at timestamptz NOT NULL DEFAULT now() >+ ) >+ """) >+ >+ Repo.query!(""" >+ CREATE TABLE IF NOT EXISTS platform.device_inventory_type_counts ( >+ type text PRIMARY KEY, >+ count bigint NOT NULL DEFAULT 0, >+ updated_at timestamptz NOT NULL DEFAULT now() >+ ) >+ """) >+ >+ Repo.query!(""" >+ CREATE TABLE IF NOT EXISTS platform.device_inventory_vendor_counts ( >+ vendor_name text PRIMARY KEY, >+ count bigint NOT NULL DEFAULT 0, >+ updated_at timestamptz NOT NULL DEFAULT now() >+ ) >+ """) >+ >+ Repo.query!(""" >+ CREATE OR REPLACE FUNCTION platform.refresh_device_inventory_rollups() >+ RETURNS void >+ LANGUAGE plpgsql >+ AS $$ >+ BEGIN >+ TRUNCATE TABLE platform.device_inventory_counts; >+ TRUNCATE TABLE platform.device_inventory_type_counts; >+ TRUNCATE TABLE platform.device_inventory_vendor_counts; >+ >+ INSERT INTO platform.device_inventory_counts (key, value, updated_at) >+ SELECT 'total', COUNT(*)::bigint, now() >+ FROM platform.ocsf_devices >+ WHERE deleted_at IS NULL; >+ >+ INSERT INTO platform.device_inventory_counts (key, value, updated_at) >+ SELECT 'available', COUNT(*)::bigint, now() >+ FROM platform.ocsf_devices >+ WHERE deleted_at IS NULL >+ AND COALESCE(is_available, false) = true; >+ >+ INSERT INTO platform.device_inventory_counts (key, value, updated_at) >+ SELECT 'unavailable', COUNT(*)::bigint, now() >+ FROM platform.ocsf_devices >+ WHERE deleted_at IS NULL >+ AND COALESCE(is_available, false) = false; >+ >+ INSERT INTO platform.device_inventory_type_counts (type, count, updated_at) >+ SELECT COALESCE(NULLIF(trim(type), ''), 'Unknown') AS type, >+ COUNT(*)::bigint AS count, >+ now() >+ FROM platform.ocsf_devices >+ WHERE deleted_at IS NULL >+ GROUP BY COALESCE(NULLIF(trim(type), ''), 'Unknown'); >+ >+ INSERT INTO platform.device_inventory_vendor_counts (vendor_name, count, updated_at) >+ SELECT COALESCE(NULLIF(trim(vendor_name), ''), 'Unknown') AS vendor_name, >+ COUNT(*)::bigint AS count, >+ now() >+ FROM platform.ocsf_devices >+ WHERE deleted_at IS NULL >+ GROUP BY COALESCE(NULLIF(trim(vendor_name), ''), 'Unknown'); >+ END; >+ $$; >+ """) >+ end >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >PR Compliance ID 10 requires schema/DDL changes to be implemented only via migrations. The added >test helper <b><i>ensure_inventory_rollup_schema!/0</i></b> executes DDL statements directly (`CREATE TABLE IF >NOT EXISTS ...<b><i>, </i></b>CREATE OR REPLACE FUNCTION ...<b><i>) via </i></b>Repo.query!`. ></pre> > > <code>AGENTS.md</code> > <code>[elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[535-605]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs/#L535-L605)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >A new test helper (`ensure_inventory_rollup_schema!/0`) executes DDL (`CREATE TABLE`, `CREATE OR REPLACE FUNCTION`) via `Repo.query!`, which violates the requirement that schema changes be managed exclusively by Elixir migrations. >## Issue Context >The PR already adds a migration that creates/replaces `platform.refresh_device_inventory_rollups()` and related trigger function. Tests should not create/alter schema at runtime; instead, ensure migrations are applied for the test DB (or use proper test setup/migration helpers). >## Fix Focus Areas >- elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[535-605] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 2. Invalid SQL: JOIN after WHERE <code>🐞 Bug</code> <code>✓ Correctness</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >For non-<b><i>latest</i></b> interface queries, the builder constructs <b><i>FROM ... WHERE ...</i></b> and then appends ><b><i>LEFT JOIN ...</i></b>, producing invalid SQL (JOINs cannot appear after WHERE). This will break interface >queries at runtime (likely surfacing as query failures in UI/API paths that fetch interfaces). ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[rust/srql/src/query/interfaces.rs[R241-270]](https://github.com/carverauto/serviceradar/pull/2996/files#diff-1ec833d4525fb2806523888b8c57b76ca8dd2ab70539368ccecc4d262769c8c5R241-R270)</code> > >```diff >+ let mut discovered_interfaces_from = String::from("FROM discovered_interfaces di"); > if !clauses.is_empty() { >- base_select.push_str(" WHERE "); >- base_select.push_str(&clauses.join(" AND ")); >+ discovered_interfaces_from.push_str(" WHERE "); >+ discovered_interfaces_from.push_str(&clauses.join(" AND ")); > } > let (sql, binds) = if latest_only { > let mut inner = String::from("SELECT DISTINCT ON (di.device_id, di.interface_uid) "); >- inner.push_str(&base_select["SELECT ".len()..]); >+ inner.push_str(&interface_select_columns("di")); >+ inner.push(' '); >+ inner.push_str(&discovered_interfaces_from); > inner.push_str( > " ORDER BY di.device_id, di.interface_uid, di.timestamp DESC, di.created_at DESC", > ); >- let mut outer = format!("SELECT * FROM ({inner}) AS latest"); >+ let mut outer = String::from("SELECT "); >+ outer.push_str(&interface_select_columns("latest")); >+ outer.push_str( >+ ", tm_in.value AS in_errors, tm_out.value AS out_errors, \ >+ COALESCE(ifs.favorited, false) AS favorited, \ >+ COALESCE(ifs.metrics_enabled, false) AS metrics_enabled \ >+ FROM (", >+ ); >+ outer.push_str(&inner); >+ outer.push_str(") AS latest"); >+ outer.push_str(&interface_enrichment_joins("latest")); >+ > if let Some(order_clause) = build_order_clause(&plan.order) { > outer.push(' '); > outer.push_str(&order_clause); >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> ><b><i>discovered_interfaces_from</i></b> is built including the <b><i>WHERE</i></b> clause, and later ><b><i>interface_enrichment_joins(&quot;di&quot;)</i></b> (which begins with <b><i>LEFT JOIN ...</i></b>) is concatenated *after* it >for the non-<b><i>latest</i></b> query path, yielding <b><i>FROM ... WHERE ... LEFT JOIN ...</i></b>. ></pre> > > <code>[rust/srql/src/query/interfaces.rs[241-246]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/rust/srql/src/query/interfaces.rs/#L241-L246)</code> > <code>[rust/srql/src/query/interfaces.rs[278-287]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/rust/srql/src/query/interfaces.rs/#L278-L287)</code> > <code>[rust/srql/src/query/interfaces.rs[192-215]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/rust/srql/src/query/interfaces.rs/#L192-L215)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >The non-`latest` interfaces SQL builder concatenates `LEFT JOIN` clauses after an already-built `FROM ... WHERE ...` fragment, generating invalid SQL. >### Issue Context >- `interface_enrichment_joins()` returns a string starting with `LEFT JOIN ...`. >- `discovered_interfaces_from` currently includes the `WHERE` clause. >### Fix Focus Areas >- rust/srql/src/query/interfaces.rs[241-246] >- rust/srql/src/query/interfaces.rs[278-287] >- rust/srql/src/query/interfaces.rs[192-215] >### Suggested implementation direction >- Keep a `where_clause` string (e.g., empty or `&amp;amp;amp;quot; WHERE ...&amp;amp;amp;quot;`). >- For non-latest queries, build: >- `FROM discovered_interfaces di` >- append `interface_enrichment_joins(&amp;amp;amp;quot;di&amp;amp;amp;quot;)` >- append `where_clause` >- For latest queries, keep joins only on the outer query (current approach is fine). >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 3. Flow stats loading can stall <code>🐞 Bug</code> <code>⛯ Reliability</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >Flow stats are now loaded in an unlinked background task and <b><i>flow_stats_loading</i></b> is only cleared >when a matching <b><i>:flow_stats_loaded</i></b> message arrives. If the task crashes (e.g., due to ><b><i>Jason.encode!/1</i></b>) or never sends, the view can remain stuck in a perpetual loading state. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[R203-226]](https://github.com/carverauto/serviceradar/pull/2996/files#diff-44e1802aef19a1badfee332ded1bfa0e83fe2da9340d6ce61fbb5c00d0b055c8R203-R226)</code> > >```diff >+ def handle_info({:flow_stats_loaded, device_uid, request_ref, stats_bundle}, socket) do >+ current_ref = Map.get(socket.assigns, :flow_stats_request_ref) >+ >+ if device_uid == socket.assigns.device_uid and request_ref == current_ref do >+ {flow_stats, sparkline_json, proto_json, chart_keys, chart_points, top_talkers_json, >+ top_destinations_json, top_ports_json, top_protocols_json, facets} = stats_bundle >+ >+ {:noreply, >+ socket >+ |> assign(:flow_stats, flow_stats) >+ |> assign(:flow_stats_loading, false) >+ |> assign(:flow_sparkline_json, sparkline_json) >+ |> assign(:flow_proto_json, proto_json) >+ |> assign(:flow_chart_keys_json, chart_keys) >+ |> assign(:flow_chart_points_json, chart_points) >+ |> assign(:flow_top_talkers_json, top_talkers_json) >+ |> assign(:flow_top_destinations_json, top_destinations_json) >+ |> assign(:flow_top_ports_json, top_ports_json) >+ |> assign(:flow_top_protocols_json, top_protocols_json) >+ |> assign(:flow_facets, facets)} >+ else >+ {:noreply, socket} >+ end >+ end >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> ><b><i>begin_flow_stats_refresh/2</i></b> sets <b><i>flow_stats_loading</i></b> to true and starts a <b><i>Task.start/1</i></b> that must >send <b><i>:flow_stats_loaded</i></b> back. The only code path that sets loading back to false is the ><b><i>handle_info/2</i></b> clause for that message. But <b><i>load_device_flow_stats/4</i></b> uses <b><i>Jason.encode!/1</i></b>, >which can raise and kill the task before it sends a message. ></pre> > > <code>[elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1744-1772]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex/#L1744-L1772)</code> > <code>[elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[203-225]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex/#L203-L225)</code> > <code>[elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1843-1863]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex/#L1843-L1863)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >`flow_stats_loading` can remain `true` indefinitely if the `Task.start/1` crashes (e.g., `Jason.encode!/1` raises) or never sends a `:flow_stats_loaded` message. >### Issue Context >- Loading is only cleared in the `handle_info({:flow_stats_loaded, ...})` clause. >- The task is unlinked and not monitored; failures are silent. >### Fix Focus Areas >- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1744-1772] >- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[203-225] >- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1843-1863] >### Suggested implementation direction >- Wrap the task body: >- `try do ... rescue e -&amp;amp;amp;gt; send(parent, {:flow_stats_loaded, uid, request_ref, empty_bundle}) end` >- and/or send a dedicated `{:flow_stats_failed, ...}` message and handle it by setting `flow_stats_loading: false`. >- Add a timeout message keyed by `request_ref` that clears loading if the task never responds. >- Consider using `Task.Supervisor.async_nolink` + monitor to surface failures and keep behavior consistent. >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <br/> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/review-recommended.png" height="20" alt="Remediation recommended"> <details> <summary> 4. <s><b><i>SyncIngestor</i></b> uses raw SQL</s> ☑ <code>📘 Rule violation</code> <code>⛯ Reliability</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >New inventory sync logic introduces additional direct <b><i>Repo.query!</i></b> calls to manipulate database >session settings and invoke rollup refresh functions. This bypasses Ash patterns and increases >coupling to SQL/session behavior in domain logic. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[R514-560]](https://github.com/carverauto/serviceradar/pull/2996/files#diff-fdf70a310cef758f735fae943c2a33bc7f851a1c3d1ba66499e911fd2bc5611aR514-R560)</code> > >```diff >+ defp with_inventory_rollup_bypassed(fun) when is_function(fun, 0) do >+ Repo.transaction( >+ fn -> >+ Repo.query!("SET LOCAL platform.skip_inventory_rollup = 'on'") >+ fun.() >+ end, >+ timeout: :infinity >+ ) >+ end >+ >+ defp refresh_inventory_rollups do >+ if inventory_rollups_supported?() do >+ case Repo.transaction( >+ fn -> >+ Repo.query!("SELECT pg_advisory_xact_lock($1)", [ >+ @inventory_rollup_refresh_lock_key >+ ]) >+ >+ Repo.query!("SELECT platform.refresh_device_inventory_rollups()") >+ end, >+ timeout: :infinity >+ ) do >+ {:ok, _} -> >+ :ok >+ >+ {:error, reason} -> >+ Logger.warning("SyncIngestor: Failed to refresh inventory rollups: #{inspect(reason)}") >+ {:error, reason} >+ end >+ else >+ :ok >+ end >+ end >+ >+ defp inventory_rollups_supported? do >+ case Repo.query( >+ "SELECT to_regprocedure('platform.refresh_device_inventory_rollups()') IS NOT NULL", >+ [] >+ ) do >+ {:ok, %{rows: [[true]]}} -> >+ :ok >+ true >+ >+ _ -> >+ false >+ end >+ end >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >PR Compliance ID 7 asks Elixir domain/data-access changes to use Ash concepts rather than direct >Ecto/SQL usage. The new functions <b><i>with_inventory_rollup_bypassed/1</i></b>, <b><i>refresh_inventory_rollups/0</i></b>, >and <b><i>inventory_rollups_supported?/0</i></b> use <b><i>Repo.query!/Repo.query</i></b> with raw SQL for transactional >settings/locks and function execution. ></pre> > > <code>AGENTS.md</code> > <code>[elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[514-560]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex/#L514-L560)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >New inventory sync code uses raw SQL via `Repo.query!/Repo.query` to set session flags, acquire advisory locks, and call rollup refresh functions, which conflicts with the guideline to use Ash concepts rather than direct Ecto/SQL in Elixir changes. >## Issue Context >This may be necessary for performance, but the compliance requirement prefers Ash patterns. If raw SQL is unavoidable, consider encapsulating it behind an existing DB utility layer (if present) or an Ash action/extension to keep domain code consistent. >## Fix Focus Areas >- elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[514-560] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 5. Rollup refresh can raise <code>🐞 Bug</code> <code>⛯ Reliability</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >Inventory rollup refresh uses <b><i>Repo.query!/2</i></b> inside <b><i>Repo.transaction/2</i></b> while relying on tuple >matching (<b><i>{:ok, _}</i></b>/<b><i>{:error, reason}</i></b>); exceptions from <b><i>query!</i></b> will bypass the `{:error, >reason}<b><i> branch and can crash </i></b>ingest_updates/2` after processing batches. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[R524-542]](https://github.com/carverauto/serviceradar/pull/2996/files#diff-fdf70a310cef758f735fae943c2a33bc7f851a1c3d1ba66499e911fd2bc5611aR524-R542)</code> > >```diff >+ defp refresh_inventory_rollups do >+ if inventory_rollups_supported?() do >+ case Repo.transaction( >+ fn -> >+ Repo.query!("SELECT pg_advisory_xact_lock($1)", [ >+ @inventory_rollup_refresh_lock_key >+ ]) >+ >+ Repo.query!("SELECT platform.refresh_device_inventory_rollups()") >+ end, >+ timeout: :infinity >+ ) do >+ {:ok, _} -> >+ :ok >+ >+ {:error, reason} -> >+ Logger.warning("SyncIngestor: Failed to refresh inventory rollups: #{inspect(reason)}") >+ {:error, reason} >+ end >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >The transaction body uses bang queries (<b><i>Repo.query!/2</i></b>). The surrounding <b><i>case</i></b> only handles tuple >return values from <b><i>Repo.transaction/2</i></b>, so raised exceptions won’t be converted into `{:error, >reason}` for logging/controlled return and can take down the caller. ></pre> > > <code>[elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[524-542]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex/#L524-L542)</code> > <code>[elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[101-106]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex/#L101-L106)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >`refresh_inventory_rollups/0` uses `Repo.query!/2` inside a transaction but only handles `{:ok, _}` / `{:error, reason}` tuples. If a bang query raises, the exception bypasses the error-handling/logging branch and can crash `ingest_updates/2`. >### Issue Context >The intent appears to be “best-effort refresh with logging,” but current implementation can escalate to a crash. >### Fix Focus Areas >- elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex[524-542] >### Suggested implementation direction >- Replace `Repo.query!` with `Repo.query`. >- On `{:error, reason}`, call `Repo.rollback(reason)` so the transaction returns `{:error, reason}`. >- Optionally wrap the whole refresh in `try/rescue` and return `{:error, exception}` while logging. >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 6. Seeds trim may crash <code>🐞 Bug</code> <code>⛯ Reliability</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> ><b><i>run_mapper_job/2</i></b> trims seeds with <b><i>String.trim/1</i></b> without validating seed types; any non-binary >seed will raise and prevent dispatch. This is especially risky as seeds are now part of the public >payload surface for mapper runs. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex[R149-154]](https://github.com/carverauto/serviceradar/pull/2996/files#diff-b56572e2bef2c1df227eb7272cd24835dcd1ebd6e164a23e56d5177fd3dd14b5R149-R154)</code> > >```diff >+ seeds = >+ opts >+ |> Keyword.get(:seeds, []) >+ |> Enum.map(&String.trim/1) >+ |> Enum.reject(&(&1 == "")) >+ |> Enum.uniq() >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >The function unconditionally maps <b><i>String.trim/1</i></b> over <b><i>opts[:seeds]</i></b>. <b><i>String.trim/1</i></b> only accepts >binaries; invalid list elements will crash the call. ></pre> > > <code>[elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex[148-155]](https://github.com/carverauto/serviceradar/blob/37a6a0fd17f6d8c44de4d7bfa53a3e7a78b5bfdd/elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex/#L148-L155)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >`AgentCommandBus.run_mapper_job/2` can crash if `opts[:seeds]` contains any non-binary values because it calls `String.trim/1` directly. >### Issue Context >Seeds are now propagated into mapper command payloads and may come from various sources (UI input, API, promotion code). >### Fix Focus Areas >- elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex[148-155] >### Suggested implementation direction >- Change normalization to: >- `|&amp;amp;amp;gt; Enum.filter(&amp;amp;amp;amp;is_binary/1)` before `Enum.map(&amp;amp;amp;amp;String.trim/1)` >- (Optional) cap seeds length/count if needed to protect agent workload. >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <pre>ⓘ The new review experience is currently in Beta. <a href="https://docs.qodo.ai/qodo-documentation/code-review">Learn more</a></pre> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>
qodo-code-review[bot] commented 2026-03-06 01:18:06 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2996#discussion_r2893214528
Original created: 2026-03-06T01:18:06Z
Original path: elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs
Original line: 605

Action required

1. ensure_inventory_rollup_schema! runs ddl 📘 Rule violation ⛯ Reliability

New test code performs schema/DDL changes via Repo.query! (CREATE TABLE, `CREATE OR REPLACE
FUNCTION`) instead of relying on Elixir migrations. This can cause schema drift and violates the
requirement that all database schema changes be managed exclusively via migrations.
Agent Prompt 
## Issue description
A new test helper (`ensure_inventory_rollup_schema!/0`) executes DDL (`CREATE TABLE`, `CREATE OR REPLACE FUNCTION`) via `Repo.query!`, which violates the requirement that schema changes be managed exclusively by Elixir migrations.

## Issue Context
The PR already adds a migration that creates/replaces `platform.refresh_device_inventory_rollups()` and related trigger function. Tests should not create/alter schema at runtime; instead, ensure migrations are applied for the test DB (or use proper test setup/migration helpers).

## Fix Focus Areas
- elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[535-605]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2996#discussion_r2893214528 Original created: 2026-03-06T01:18:06Z Original path: elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs Original line: 605 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 1\. <b><i>ensure_inventory_rollup_schema!</i></b> runs ddl <code>📘 Rule violation</code> <code>⛯ Reliability</code> <pre> New test code performs schema/DDL changes via <b><i>Repo.query!</i></b> (<b><i>CREATE TABLE</i></b>, `CREATE OR REPLACE FUNCTION`) instead of relying on Elixir migrations. This can cause schema drift and violates the requirement that all database schema changes be managed exclusively via migrations. </pre> <details> <summary><strong>Agent Prompt</strong></summary> ``` ## Issue description A new test helper (`ensure_inventory_rollup_schema!/0`) executes DDL (`CREATE TABLE`, `CREATE OR REPLACE FUNCTION`) via `Repo.query!`, which violates the requirement that schema changes be managed exclusively by Elixir migrations. ## Issue Context The PR already adds a migration that creates/replaces `platform.refresh_device_inventory_rollups()` and related trigger function. Tests should not create/alter schema at runtime; instead, ensure migrations are applied for the test DB (or use proper test setup/migration helpers). ## Fix Focus Areas - elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs[535-605] ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>
qodo-code-review[bot] commented 2026-03-06 01:18:06 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2996#discussion_r2893214532
Original created: 2026-03-06T01:18:06Z
Original path: rust/srql/src/query/interfaces.rs
Original line: 270

Action required

2. Invalid sql: join after where 🐞 Bug ✓ Correctness

For non-latest interface queries, the builder constructs FROM ... WHERE ... and then appends
LEFT JOIN ..., producing invalid SQL (JOINs cannot appear after WHERE). This will break interface
queries at runtime (likely surfacing as query failures in UI/API paths that fetch interfaces).
Agent Prompt 
### Issue description
The non-`latest` interfaces SQL builder concatenates `LEFT JOIN` clauses after an already-built `FROM ... WHERE ...` fragment, generating invalid SQL.

### Issue Context
- `interface_enrichment_joins()` returns a string starting with `LEFT JOIN ...`.
- `discovered_interfaces_from` currently includes the `WHERE` clause.

### Fix Focus Areas
- rust/srql/src/query/interfaces.rs[241-246]
- rust/srql/src/query/interfaces.rs[278-287]
- rust/srql/src/query/interfaces.rs[192-215]

### Suggested implementation direction
- Keep a `where_clause` string (e.g., empty or `" WHERE ..."`).
- For non-latest queries, build:
  - `FROM discovered_interfaces di`
  - append `interface_enrichment_joins("di")`
  - append `where_clause`
- For latest queries, keep joins only on the outer query (current approach is fine).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2996#discussion_r2893214532 Original created: 2026-03-06T01:18:06Z Original path: rust/srql/src/query/interfaces.rs Original line: 270 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 2\. Invalid sql: join after where <code>🐞 Bug</code> <code>✓ Correctness</code> <pre> For non-<b><i>latest</i></b> interface queries, the builder constructs <b><i>FROM ... WHERE ...</i></b> and then appends <b><i>LEFT JOIN ...</i></b>, producing invalid SQL (JOINs cannot appear after WHERE). This will break interface queries at runtime (likely surfacing as query failures in UI/API paths that fetch interfaces). </pre> <details> <summary><strong>Agent Prompt</strong></summary> ``` ### Issue description The non-`latest` interfaces SQL builder concatenates `LEFT JOIN` clauses after an already-built `FROM ... WHERE ...` fragment, generating invalid SQL. ### Issue Context - `interface_enrichment_joins()` returns a string starting with `LEFT JOIN ...`. - `discovered_interfaces_from` currently includes the `WHERE` clause. ### Fix Focus Areas - rust/srql/src/query/interfaces.rs[241-246] - rust/srql/src/query/interfaces.rs[278-287] - rust/srql/src/query/interfaces.rs[192-215] ### Suggested implementation direction - Keep a `where_clause` string (e.g., empty or `" WHERE ..."`). - For non-latest queries, build: - `FROM discovered_interfaces di` - append `interface_enrichment_joins("di")` - append `where_clause` - For latest queries, keep joins only on the outer query (current approach is fine). ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>
qodo-code-review[bot] commented 2026-03-06 01:18:06 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2996#discussion_r2893214536
Original created: 2026-03-06T01:18:06Z
Original path: elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex
Original line: 226

Action required

3. Flow stats loading can stall 🐞 Bug ⛯ Reliability

Flow stats are now loaded in an unlinked background task and flow_stats_loading is only cleared
when a matching :flow_stats_loaded message arrives. If the task crashes (e.g., due to
Jason.encode!/1) or never sends, the view can remain stuck in a perpetual loading state.
Agent Prompt 
### Issue description
`flow_stats_loading` can remain `true` indefinitely if the `Task.start/1` crashes (e.g., `Jason.encode!/1` raises) or never sends a `:flow_stats_loaded` message.

### Issue Context
- Loading is only cleared in the `handle_info({:flow_stats_loaded, ...})` clause.
- The task is unlinked and not monitored; failures are silent.

### Fix Focus Areas
- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1744-1772]
- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[203-225]
- elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1843-1863]

### Suggested implementation direction
- Wrap the task body:
  - `try do ... rescue e -> send(parent, {:flow_stats_loaded, uid, request_ref, empty_bundle}) end`
  - and/or send a dedicated `{:flow_stats_failed, ...}` message and handle it by setting `flow_stats_loading: false`.
- Add a timeout message keyed by `request_ref` that clears loading if the task never responds.
- Consider using `Task.Supervisor.async_nolink` + monitor to surface failures and keep behavior consistent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2996#discussion_r2893214536 Original created: 2026-03-06T01:18:06Z Original path: elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex Original line: 226 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 3\. Flow stats loading can stall <code>🐞 Bug</code> <code>⛯ Reliability</code> <pre> Flow stats are now loaded in an unlinked background task and <b><i>flow_stats_loading</i></b> is only cleared when a matching <b><i>:flow_stats_loaded</i></b> message arrives. If the task crashes (e.g., due to <b><i>Jason.encode!/1</i></b>) or never sends, the view can remain stuck in a perpetual loading state. </pre> <details> <summary><strong>Agent Prompt</strong></summary> ``` ### Issue description `flow_stats_loading` can remain `true` indefinitely if the `Task.start/1` crashes (e.g., `Jason.encode!/1` raises) or never sends a `:flow_stats_loaded` message. ### Issue Context - Loading is only cleared in the `handle_info({:flow_stats_loaded, ...})` clause. - The task is unlinked and not monitored; failures are silent. ### Fix Focus Areas - elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1744-1772] - elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[203-225] - elixir/web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex[1843-1863] ### Suggested implementation direction - Wrap the task body: - `try do ... rescue e -> send(parent, {:flow_stats_loaded, uid, request_ref, empty_bundle}) end` - and/or send a dedicated `{:flow_stats_failed, ...}` message and handle it by setting `flow_stats_loading: false`. - Add a timeout message keyed by `request_ref` that clears loading if the task never responds. - Consider using `Task.Supervisor.async_nolink` + monitor to surface failures and keep behavior consistent. ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
1week

   
2weeks

   
Failed compliance check

   
IP cameras

   
NATS

NATS JetStream

  
Possible security concern

   
Review effort 1/5

   
Review effort 2/5

   
Review effort 3/5

   
Review effort 4/5

   
Review effort 5/5

   
UI

   
aardvark

   
accessibility

   
amd64

   
api

   
arm64

   
auth

   
back-end

   
bgp

   
blog

   
bug

Something isn't working

  
build

   
checkers

   
ci-cd

continuous integration-continuous deployments

  
cleanup

   
cnpg

cloud-native postgres

  
codex

   
core

core service

  
dependencies

Pull requests that update a dependency file

  
device-management

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
dusk

   
ebpf

   
enhancement

New feature or request

  
eta 1d

   
eta 1hr

   
eta 3d

   
eta 3hr

   
feature

   
fieldsurvey

   
github_actions

Pull requests that update GitHub Actions code

  
go

Pull requests that update Go code

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update Javascript code

  
k8s

   
log-collector

   
mapper

   
mtr

multi traceroute

  
needs-triage

   
netflow

   
network-sweep

   
observability

   
oracle

Oracle Linux related issues

  
otel

opentelemetry logs, traces, metrics

  
plug-in

   
proton

timeplus proton streaming database

  
python

   
question

Further information is requested

  
reddit

   
redhat

   
research

   
rperf

   
rperf-checker

   
rust

Pull requests that update rust code

  
sdk

   
security

   
serviceradar-agent

   
serviceradar-agent-gateway

   
serviceradar-web

   
serviceradar-web-ng

   
siem

   
snmp

   
sysmon

   
topology

   
ubiquiti

   
wasm

   
wontfix

This will not be worked on

  
zen-engine
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
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
carverauto/serviceradar!3017
No description provided.