carverauto/serviceradar

Fork 0

2322 cleanup fix device details page #2683

Merged

mfreeman451 merged 4 commits from refs/pull/2683/head into staging

2026-01-17 23:38:24 +00:00

mfreeman451 commented

2026-01-17 22:04:21 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2324
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2324
Original created: 2026-01-17T22:04:21Z
Original updated: 2026-01-17T23:38:31Z
Original head: carverauto/serviceradar:2322-cleanup-fix-device-details-page
Original base: staging
Original merged: 2026-01-17T23:38:24Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

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

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

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

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

PR Type

Enhancement, Bug fix

Description

Gate sysmon metrics panels to only show for devices with sysmon data
Render sysmon CPU/memory/disk metrics as graphs instead of tables
Remove low-value auto-generated category visualizations
Improve device identity detection using agent_id and device_uid
Add timeseries inference support for metric visualization fallback

Diagram Walkthrough

flowchart LR
  A["Device Details Page"] --> B["Load Sysmon Identity"]
  B --> C["Check Sysmon Presence"]
  C --> D{Has Sysmon Data?}
  D -->|Yes| E["Show Sysmon Panels"]
  D -->|No| F["Hide Sysmon Panels"]
  E --> G["Render as Graphs"]
  A --> H["Build Panels"]
  H --> I["Filter Low-Value Categories"]
  H --> J["Infer Timeseries Viz"]
  J --> G

File Walkthrough

Relevant files

Enhancement

timeseries.ex

Add timeseries inference support for results

web-ng/lib/serviceradar_web_ng_web/dashboard/plugins/timeseries.ex

Add Viz.infer/1 alias import for visualization inference
Add supports?/1 clause to detect timeseries from raw results
Add build/1 clause to handle results without explicit viz suggestions
Add infer_timeseries_spec/1 helper to infer timeseries structure

+24/-0

show.ex

Refactor sysmon metrics gating and visualization rendering

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

Add sysmon identity detection from device row (device_uid and
agent_id)
Add sysmon presence flag to track if device has sysmon metrics
Gate sysmon metric panels visibility based on presence or summary data
Filter out low-value category visualizations (type_id by modified)
Refactor metric loading to use identity map instead of escaped
device_uid
Add timeseries inference fallback for metric visualization
Update sysmon summary/CPU/memory/disk loaders to filter by identity

+277/-66

Formatting

index.ex

Minor whitespace formatting fix

web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex

Fix whitespace formatting in hosts summary display text

+1/-1

Documentation

proposal.md `Add change proposal documentation` openspec/changes/fix-device-details-ui/proposal.md Document the problem: sysmon metrics showing for non-sysmon devices and poor table rendering List three main changes: gating sysmon panels, rendering as graphs, removing low-value visualizations Specify affected specs and code areas	+13/-0
spec.md `Add formal specification requirements` openspec/changes/fix-device-details-ui/specs/build-web-ui/spec.md Define requirement for sysmon metrics visibility gating Define requirement for graph rendering of sysmon metrics Define requirement to suppress type_id by modified visualization Include scenarios for each requirement with given/when/then format	+40/-0
tasks.md `Add implementation and validation tasks` openspec/changes/fix-device-details-ui/tasks.md List implementation tasks with completion status List validation tasks for testing the changes	+11/-0

Imported from GitHub pull request. Original GitHub pull request: #2324 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2324 Original created: 2026-01-17T22:04:21Z Original updated: 2026-01-17T23:38:31Z Original head: carverauto/serviceradar:2322-cleanup-fix-device-details-page Original base: staging Original merged: 2026-01-17T23:38:24Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Enhancement, Bug fix ___ ### **Description** - Gate sysmon metrics panels to only show for devices with sysmon data - Render sysmon CPU/memory/disk metrics as graphs instead of tables - Remove low-value auto-generated category visualizations - Improve device identity detection using agent_id and device_uid - Add timeseries inference support for metric visualization fallback ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Device Details Page"] --> B["Load Sysmon Identity"] B --> C["Check Sysmon Presence"] C --> D{Has Sysmon Data?} D -->|Yes| E["Show Sysmon Panels"] D -->|No| F["Hide Sysmon Panels"] E --> G["Render as Graphs"] A --> H["Build Panels"] H --> I["Filter Low-Value Categories"] H --> J["Infer Timeseries Viz"] J --> G ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>timeseries.ex</strong><dd><code>Add timeseries inference support for results</code>                          </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/dashboard/plugins/timeseries.ex <ul><li>Add <code>Viz.infer/1</code> alias import for visualization inference<br> <li> Add <code>supports?/1</code> clause to detect timeseries from raw results<br> <li> Add <code>build/1</code> clause to handle results without explicit viz suggestions<br> <li> Add <code>infer_timeseries_spec/1</code> helper to infer timeseries structure</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2324/files#diff-2dd46da2ac00979257fc2b79ad1c72441c867402317f0f903c6d76ae3c8faecd">+24/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>show.ex</strong><dd><code>Refactor sysmon metrics gating and visualization rendering</code></dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex <ul><li>Add sysmon identity detection from device row (device_uid and <br>agent_id)<br> <li> Add sysmon presence flag to track if device has sysmon metrics<br> <li> Gate sysmon metric panels visibility based on presence or summary data<br> <li> Filter out low-value category visualizations (type_id by modified)<br> <li> Refactor metric loading to use identity map instead of escaped <br>device_uid<br> <li> Add timeseries inference fallback for metric visualization<br> <li> Update sysmon summary/CPU/memory/disk loaders to filter by identity</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2324/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24">+277/-66</a></td> </tr> </table></td></tr><tr><td><strong>Formatting</strong></td><td><table> <tr> <td> <details> <summary><strong>index.ex</strong><dd><code>Minor whitespace formatting fix</code>                                                    </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex - Fix whitespace formatting in hosts summary display text </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2324/files#diff-b2127e71582033bc6dfd2d7397f56bf43c1f7c613defffc504b3d8ee1e7406c4">+1/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Documentation</strong></td><td><table> <tr> <td> <details> <summary><strong>proposal.md</strong><dd><code>Add change proposal documentation</code>                                                </dd></summary> <hr> openspec/changes/fix-device-details-ui/proposal.md <ul><li>Document the problem: sysmon metrics showing for non-sysmon devices <br>and poor table rendering<br> <li> List three main changes: gating sysmon panels, rendering as graphs, <br>removing low-value visualizations<br> <li> Specify affected specs and code areas</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2324/files#diff-e1fd5ff0cd72e0fee1795227b1f30abe3a8704554de4b6a7e16c16aa650fdf41">+13/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>spec.md</strong><dd><code>Add formal specification requirements</code>                                        </dd></summary> <hr> openspec/changes/fix-device-details-ui/specs/build-web-ui/spec.md <ul><li>Define requirement for sysmon metrics visibility gating<br> <li> Define requirement for graph rendering of sysmon metrics<br> <li> Define requirement to suppress type_id by modified visualization<br> <li> Include scenarios for each requirement with given/when/then format</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2324/files#diff-e45a3611edb0f820076d71239c87339969bb50c06692a26ae373ac6b0b53b769">+40/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>tasks.md</strong><dd><code>Add implementation and validation tasks</code>                                    </dd></summary> <hr> openspec/changes/fix-device-details-ui/tasks.md <ul><li>List implementation tasks with completion status<br> <li> List validation tasks for testing the changes</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2324/files#diff-1cb6c954f90f6afcedfe3a8c605de0ebc4c5fd94f22e819de9ca37cf17504bd7">+11/-0</a>    </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-17 22:05:07 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2324#issuecomment-3764394799
Original created: 2026-01-17T22:05:07Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

⚪

Query injection risk

Description: SRQL query strings are built by concatenating user/device-sourced identity fields (e.g.,
agent_id and device_uid) into tokens via sysmon_filter_tokens/1, but escaping only handles
backslashes and quotes, so crafted values containing spaces or SRQL operators (e.g.,
agent_id like foo" time:last_7d in:secrets) could potentially alter the query semantics. show.ex [1225-1244]

Referred Code

defp sysmon_filter_tokens(identity) when identity == %{} or identity == nil, do: []

defp sysmon_filter_tokens(identity) do
  tokens = []
  device_uid = Map.get(identity, :device_uid)
  agent_id = Map.get(identity, :agent_id)

  tokens =
    if is_binary(device_uid) and device_uid != "" do
      tokens ++ ["device_id:\"#{escape_value(device_uid)}\""]
    else
      tokens
    end

  if is_binary(agent_id) and agent_id != "" do
    tokens ++ ["agent_id:\"#{escape_value(agent_id)}\""]
  else
    tokens
  end
end

Ticket Compliance

🟡

🎫 #2322

⚪	Device details page must not show sysmon stats for devices that do not have sysmon data (e.g., the referenced non-sysmon device).
	CPU metrics on the device details page should be shown as a graph (not rendered as a table).
	Memory metrics on the device details page should be shown as a graph (not rendered as a table).
	Disk metrics on the device details page should be shown as a graph (not rendered as a table).
	Do not auto-create the low-value visualization "Categories: type_id by modified" on the device details page.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Comprehensive Audit Trails

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

Status: Passed

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

Generic: Meaningful Naming and Self-Documenting Code

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

Status: Passed

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

Generic: Secure Logging Practices

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

Status: Passed

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

🔴

Generic: Secure Error Handling

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

Status:
Leaky error messages: The new error strings embed inspect(other) / format_error(reason) which can expose
internal response structures or sensitive details to end-users if rendered in the UI.

Referred Code

  {:ok, other} ->
    %{base | error: "unexpected SRQL response: #{inspect(other)}"}

  {:error, reason} ->
    %{base | error: "SRQL error: #{format_error(reason)}"}
end

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

⚪

Generic: Robust Error Handling and Edge Case Management

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

Status:
Error handling context: The new SRQL error paths convert failures into user-consumable strings and it is unclear
from the diff whether these errors are logged/monitored or handled in a way that provides
actionable internal context without leaking details.

Referred Code

  {:ok, other} ->
    %{base | error: "unexpected SRQL response: #{inspect(other)}"}

  {:error, reason} ->
    %{base | error: "SRQL error: #{format_error(reason)}"}
end

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

Generic: Security-First Input Validation and Data Handling

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

Status:
Query injection risk: The PR builds SRQL query fragments by string concatenation from device_uid/agent_id using
escape_value/1, but it is not verifiable from the diff alone that this escaping is
sufficient to prevent SRQL injection across all parser edge-cases.

Referred Code

tokens =
  if is_binary(device_uid) and device_uid != "" do
    tokens ++ ["device_id:\"#{escape_value(device_uid)}\""]
  else
    tokens
  end

if is_binary(agent_id) and agent_id != "" do
  tokens ++ ["agent_id:\"#{escape_value(agent_id)}\""]
else

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

Update

Compliance status legend

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

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2324#issuecomment-3764394799 Original created: 2026-01-17T22:05:07Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td rowspan=1>⚪</td> <td><details><summary><strong>Query injection risk </strong></summary><br> <b>Description:</b> SRQL query strings are built by concatenating user/device-sourced identity fields (e.g., <br><code>agent_id</code> and <code>device_uid</code>) into tokens via <code>sysmon_filter_tokens/1</code>, but escaping only handles <br>backslashes and quotes, so crafted values containing spaces or SRQL operators (e.g., <br><code>agent_id</code> like <code>foo" time:last_7d in:secrets</code>) could potentially alter the query semantics. <strong><a href='https://github.com/carverauto/serviceradar/pull/2324/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1225-R1244'>show.ex [1225-1244]</a></strong><br> <details open><summary>Referred Code</summary> ```elixir defp sysmon_filter_tokens(identity) when identity == %{} or identity == nil, do: [] defp sysmon_filter_tokens(identity) do tokens = [] device_uid = Map.get(identity, :device_uid) agent_id = Map.get(identity, :agent_id) tokens = if is_binary(device_uid) and device_uid != "" do tokens ++ ["device_id:\"#{escape_value(device_uid)}\""] else tokens end if is_binary(agent_id) and agent_id != "" do tokens ++ ["agent_id:\"#{escape_value(agent_id)}\""] else tokens end end ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2322>#2322</a></summary> <table width='100%'><tbody> <tr><td rowspan=5>⚪</td> <td>Device details page must not show sysmon stats for devices that do not have sysmon data <br>(e.g., the referenced non-sysmon device).<br></td></tr> <tr><td>CPU metrics on the device details page should be shown as a graph (not rendered as a <br>table).<br></td></tr> <tr><td>Memory metrics on the device details page should be shown as a graph (not rendered as a <br>table).<br></td></tr> <tr><td>Disk metrics on the device details page should be shown as a graph (not rendered as a <br>table).<br></td></tr> <tr><td>Do not auto-create the low-value visualization "Categories: type_id by modified" on the <br>device details page.<br></td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=3>🟢</td><td> <details><summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=1>🔴</td> <td><details> <summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2324/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1115-R1120'><strong>Leaky error messages</strong></a>: The new error strings embed <code>inspect(other)</code> / <code>format_error(reason)</code> which can expose <br>internal response structures or sensitive details to end-users if rendered in the UI.<br> <details open><summary>Referred Code</summary> ```elixir {:ok, other} -> %{base | error: "unexpected SRQL response: #{inspect(other)}"} {:error, reason} -> %{base | error: "SRQL error: #{format_error(reason)}"} end ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=2>⚪</td> <td><details> <summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2324/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1115-R1120'><strong>Error handling context</strong></a>: The new SRQL error paths convert failures into user-consumable strings and it is unclear <br>from the diff whether these errors are logged/monitored or handled in a way that provides <br>actionable internal context without leaking details.<br> <details open><summary>Referred Code</summary> ```elixir {:ok, other} -> %{base | error: "unexpected SRQL response: #{inspect(other)}"} {:error, reason} -> %{base | error: "SRQL error: #{format_error(reason)}"} end ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2324/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1232-R1241'><strong>Query injection risk</strong></a>: The PR builds SRQL query fragments by string concatenation from <code>device_uid</code>/<code>agent_id</code> using <br><code>escape_value/1</code>, but it is not verifiable from the diff alone that this escaping is <br>sufficient to prevent SRQL injection across all parser edge-cases.<br> <details open><summary>Referred Code</summary> ```elixir tokens = if is_binary(device_uid) and device_uid != "" do tokens ++ ["device_id:\"#{escape_value(device_uid)}\""] else tokens end if is_binary(agent_id) and agent_id != "" do tokens ++ ["agent_id:\"#{escape_value(agent_id)}\""] else ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2026-01-17 22:06:50 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2324#issuecomment-3764396206
Original created: 2026-01-17T22:06:50Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Refine query logic for correctness Refactor `sysmon_filter_tokens/1` to build a more accurate query using an `OR` for device identifiers (`device_id` or `uid`) and an `AND` for the `agent_id`, improving query correctness. web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [1227-1244] defp sysmon_filter_tokens(identity) do - tokens = [] device_uid = Map.get(identity, :device_uid) agent_id = Map.get(identity, :agent_id) - tokens = + device_filter = if is_binary(device_uid) and device_uid != "" do - tokens ++ ["device_id:\"#{escape_value(device_uid)}\""] + escaped_uid = escape_value(device_uid) + "(device_id:\"#{escaped_uid}\" or uid:\"#{escaped_uid}\")" else - tokens + nil end - if is_binary(agent_id) and agent_id != "" do - tokens ++ ["agent_id:\"#{escape_value(agent_id)}\""] - else - tokens - end + agent_filter = + if is_binary(agent_id) and agent_id != "" do + "agent_id:\"#{escape_value(agent_id)}\"" + else + nil + end + + [device_filter, agent_filter] + \|> Enum.filter(&is_binary/1) end Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a logic flaw where the generated query is too restrictive and does not match the filtering logic in `row_matches_identity?/2`, which the PR relies on.	Medium
General	Use Enum.find for first map Replace `List.first(Enum.filter(results, &is_map/1))` with `Enum.find(results,` `&is_map/1)` to simplify finding the first map in the list. web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [114] `-device_row = List.first(Enum.filter(results, &is_map/1)) +device_row = Enum.find(results, &is_map/1)` Apply / Chat Suggestion importance[1-10]: 2 __ Why: The suggestion offers a minor improvement by replacing `List.first(Enum.filter(...))` with the more direct and slightly more efficient `Enum.find/2`.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2324#issuecomment-3764396206 Original created: 2026-01-17T22:06:50Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Refine query logic for correctness</summary> ___ **Refactor <code>sysmon_filter_tokens/1</code> to build a more accurate query using an <code>OR</code> for <br>device identifiers (<code>device_id</code> or <code>uid</code>) and an <code>AND</code> for the <code>agent_id</code>, improving <br>query correctness.** [web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [1227-1244]](https://github.com/carverauto/serviceradar/pull/2324/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1227-R1244) ```diff defp sysmon_filter_tokens(identity) do - tokens = [] device_uid = Map.get(identity, :device_uid) agent_id = Map.get(identity, :agent_id) - tokens = + device_filter = if is_binary(device_uid) and device_uid != "" do - tokens ++ ["device_id:\"#{escape_value(device_uid)}\""] + escaped_uid = escape_value(device_uid) + "(device_id:\"#{escaped_uid}\" or uid:\"#{escaped_uid}\")" else - tokens + nil end - if is_binary(agent_id) and agent_id != "" do - tokens ++ ["agent_id:\"#{escape_value(agent_id)}\""] - else - tokens - end + agent_filter = + if is_binary(agent_id) and agent_id != "" do + "agent_id:\"#{escape_value(agent_id)}\"" + else + nil + end + + [device_filter, agent_filter] + |> Enum.filter(&is_binary/1) end ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a logic flaw where the generated query is too restrictive and does not match the filtering logic in `row_matches_identity?/2`, which the PR relies on. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Use Enum.find for first map</summary> ___ **Replace <code>List.first(Enum.filter(results, &is_map/1))</code> with <code>Enum.find(results, </code><br><code>&is_map/1)</code> to simplify finding the first map in the list.** [web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [114]](https://github.com/carverauto/serviceradar/pull/2324/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R114-R114) ```diff -device_row = List.first(Enum.filter(results, &is_map/1)) +device_row = Enum.find(results, &is_map/1) ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 2</summary> __ Why: The suggestion offers a minor improvement by replacing `List.first(Enum.filter(...))` with the more direct and slightly more efficient `Enum.find/2`. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>