carverauto/serviceradar

Fork 0

fixing srql #2685

Merged

mfreeman451 merged 15 commits from refs/pull/2685/head into staging

2026-01-18 07:02:11 +00:00

mfreeman451 commented

2026-01-18 00:15:03 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2326
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2326
Original created: 2026-01-18T00:15:03Z
Original updated: 2026-01-18T07:02:28Z
Original head: carverauto/serviceradar:bug/srql-broken
Original base: staging
Original merged: 2026-01-18T07:02:11Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

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

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

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

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

PR Type

Bug fix, Enhancement

Description

Fix SRQL query engine by adding proper LIKE operator support with case-insensitive substring matching
Implement dynamic array field detection to route array filters through correct PostgreSQL operators
Add in:devices entity token prefix to device quick filter URLs for parser compliance
Refactor filter application to separate scalar and array field handling logic
Add require Ash.Query directive for proper Ash query compilation

Diagram Walkthrough

flowchart LR
  A["SRQL Query<br/>like/array filters"] --> B["Ash Adapter<br/>ash_adapter.ex"]
  B --> C{Array Field?}
  C -->|Yes| D["apply_array_filter<br/>has_any operator"]
  C -->|No| E["apply_scalar_filter<br/>ilike/comparison"]
  D --> F["PostgreSQL<br/>Array operators"]
  E --> F
  G["Quick Filters<br/>device_live/index.ex"] --> H["Add in:devices<br/>prefix"]
  H --> B

File Walkthrough

Relevant files

Bug fix

ash_adapter.ex

Add array field detection and LIKE operator support

web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex

Added require Ash.Query directive for proper query compilation
Implemented array_field?/2 function to dynamically detect array-typed
fields via Ash resource introspection
Refactored apply_filter_op/5 to accept resource parameter and route to
specialized handlers
Created apply_array_filter/3 to handle array fields using has_any
operator for containment checks
Created apply_scalar_filter/3 to handle scalar fields with support for
LIKE, not_like, not_in operators
Added proper LIKE pattern handling with ilike filter and wildcard
stripping

+100/-34

index.ex

Add entity token prefix to quick filter URLs

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

Updated quick filter URLs to include in:devices entity token prefix
Changed is_available:true filter from q=is_available:true to
q=in:devices is_available:true
Changed is_available:false filter from q=is_available:false to
q=in:devices is_available:false
Changed discovery_sources filter from q=discovery_sources:sweep to
q=in:devices discovery_sources:(sweep)

+3/-3

Documentation

design.md `Design document for SRQL query engine fixes` openspec/changes/fix-srql-query-engine/design.md Documented current SRQL query flow showing failure points in parsing and filtering Specified four key decisions: quick filter URL format, LIKE operator handling, array field detection, and TimeFilterSpec serialization Provided code examples for LIKE and array filter implementations Listed risks and mitigations including LIKE semantics differences and hardcoded array detection Outlined migration plan with six sequential steps from Rust fixes to testing	+148/-0
proposal.md `Change proposal for SRQL query engine fixes` openspec/changes/fix-srql-query-engine/proposal.md Described five critical bugs in SRQL query engine: missing entity tokens, broken LIKE queries, array field errors, TimeFilterSpec serialization, and stale tenant code Outlined specific fixes for device quick filters, LIKE operator support, array field handling, and Rust serialization Listed affected code files and related GitHub issues Provided rationale for each change and impact assessment	+56/-0
spec.md `SRQL specification updates for query engine fixes` openspec/changes/fix-srql-query-engine/specs/srql/spec.md Added modified requirements for filter operations including LIKE, not_like, and not_in operators Specified scenarios for LIKE filtering on text fields with case-insensitive matching Added scenarios for array field filtering without PostgreSQL operator errors Documented entity token requirement with examples of correct and incorrect query formats Specified TimeFilterSpec serialization requirements for RelativeHours and RelativeDays variants Removed tenant context propagation requirement due to multi-tenancy removal	+72/-0
tasks.md `Implementation tasks for SRQL query engine fixes` openspec/changes/fix-srql-query-engine/tasks.md Created seven task groups covering TimeFilterSpec serialization, quick filter URL updates, LIKE operator implementation, array field handling, tenant code removal, testing, and cleanup Provided detailed subtasks for each group with specific file locations and test scenarios Included manual testing procedures for device page filters and search queries Listed cleanup tasks for archiving outdated proposals and closing related GitHub issues	+55/-0

Imported from GitHub pull request. Original GitHub pull request: #2326 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2326 Original created: 2026-01-18T00:15:03Z Original updated: 2026-01-18T07:02:28Z Original head: carverauto/serviceradar:bug/srql-broken Original base: staging Original merged: 2026-01-18T07:02:11Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Bug fix, Enhancement ___ ### **Description** - Fix SRQL query engine by adding proper LIKE operator support with case-insensitive substring matching - Implement dynamic array field detection to route array filters through correct PostgreSQL operators - Add `in:devices` entity token prefix to device quick filter URLs for parser compliance - Refactor filter application to separate scalar and array field handling logic - Add `require Ash.Query` directive for proper Ash query compilation ___ ### Diagram Walkthrough ```mermaid flowchart LR A["SRQL Query like/array filters"] --> B["Ash Adapter ash_adapter.ex"] B --> C{Array Field?} C -->|Yes| D["apply_array_filter has_any operator"] C -->|No| E["apply_scalar_filter ilike/comparison"] D --> F["PostgreSQL Array operators"] E --> F G["Quick Filters device_live/index.ex"] --> H["Add in:devices prefix"] H --> B ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Bug fix</td><td><table> <tr> <td> <details> <summary>ash_adapter.ex<dd><code>Add array field detection and LIKE operator support</code>            </dd></summary> <hr> web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex <ul><li>Added <code>require Ash.Query</code> directive for proper query compilation <li> Implemented <code>array_field?/2</code> function to dynamically detect array-typed fields via Ash resource introspection <li> Refactored <code>apply_filter_op/5</code> to accept <code>resource</code> parameter and route to specialized handlers <li> Created <code>apply_array_filter/3</code> to handle array fields using <code>has_any</code> operator for containment checks <li> Created <code>apply_scalar_filter/3</code> to handle scalar fields with support for LIKE, not_like, not_in operators <li> Added proper LIKE pattern handling with <code>ilike</code> filter and wildcard stripping</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2326/files#diff-b4206b4f7d4604e08471361ad7ddae4b6dbb75f67239cb134d20644633f5eb8b">+100/-34</a></td> </tr> <tr> <td> <details> <summary>index.ex<dd><code>Add entity token prefix to quick filter URLs</code>                          </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/device_live/index.ex <ul><li>Updated quick filter URLs to include <code>in:devices</code> entity token prefix <li> Changed <code>is_available:true</code> filter from <code>q=is_available:true</code> to <code>q=in:devices is_available:true</code> <li> Changed <code>is_available:false</code> filter from <code>q=is_available:false</code> to <code>q=in:devices is_available:false</code> <li> Changed <code>discovery_sources</code> filter from <code>q=discovery_sources:sweep</code> to <code>q=in:devices discovery_sources:(sweep)</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2326/files#diff-261a01f4876e5984e1d9e9b38a3540675dfb0272abc30e6bdb2a4fa610353cc7">+3/-3</a>      </td> </tr> </table></td></tr><tr><td>Documentation</td><td><table> <tr> <td> <details> <summary>design.md<dd><code>Design document for SRQL query engine fixes</code>                            </dd></summary> <hr> openspec/changes/fix-srql-query-engine/design.md <ul><li>Documented current SRQL query flow showing failure points in parsing and filtering <li> Specified four key decisions: quick filter URL format, LIKE operator handling, array field detection, and TimeFilterSpec serialization <li> Provided code examples for LIKE and array filter implementations <li> Listed risks and mitigations including LIKE semantics differences and hardcoded array detection <li> Outlined migration plan with six sequential steps from Rust fixes to testing</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2326/files#diff-d784fd78d73e834ddff230e6ab1aa173568337e17bb318c3608572ab67e941ee">+148/-0</a>  </td> </tr> <tr> <td> <details> <summary>proposal.md<dd><code>Change proposal for SRQL query engine fixes</code>                            </dd></summary> <hr> openspec/changes/fix-srql-query-engine/proposal.md <ul><li>Described five critical bugs in SRQL query engine: missing entity tokens, broken LIKE queries, array field errors, TimeFilterSpec serialization, and stale tenant code <li> Outlined specific fixes for device quick filters, LIKE operator support, array field handling, and Rust serialization <li> Listed affected code files and related GitHub issues <li> Provided rationale for each change and impact assessment</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2326/files#diff-f2766d7d5f6b73de9713ee7b8b88ffbd94eae68d64c7884c531a0afbaa44cb12">+56/-0</a>    </td> </tr> <tr> <td> <details> <summary>spec.md<dd><code>SRQL specification updates for query engine fixes</code>                </dd></summary> <hr> openspec/changes/fix-srql-query-engine/specs/srql/spec.md <ul><li>Added modified requirements for filter operations including LIKE, not_like, and not_in operators <li> Specified scenarios for LIKE filtering on text fields with case-insensitive matching <li> Added scenarios for array field filtering without PostgreSQL operator errors <li> Documented entity token requirement with examples of correct and incorrect query formats <li> Specified TimeFilterSpec serialization requirements for RelativeHours and RelativeDays variants <li> Removed tenant context propagation requirement due to multi-tenancy removal</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2326/files#diff-68b21eaffd5845bf5212b5383471f5334c5eb00e9de1344daf7cb7d9f8114bc8">+72/-0</a>    </td> </tr> <tr> <td> <details> <summary>tasks.md<dd><code>Implementation tasks for SRQL query engine fixes</code>                  </dd></summary> <hr> openspec/changes/fix-srql-query-engine/tasks.md <ul><li>Created seven task groups covering TimeFilterSpec serialization, quick filter URL updates, LIKE operator implementation, array field handling, tenant code removal, testing, and cleanup <li> Provided detailed subtasks for each group with specific file locations and test scenarios <li> Included manual testing procedures for device page filters and search queries <li> Listed cleanup tasks for archiving outdated proposals and closing related GitHub issues</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2326/files#diff-14f07257abe4c25ce3e2e447210143febf6bcf853675bc04264fe11f95f4b534">+55/-0</a>    </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-18 00:16:00 +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/2326#issuecomment-3764516157
Original created: 2026-01-18T00:16:00Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
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: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Silent error swallowing: Multiple broad `rescue _ -> query/false` blocks silently ignore failures (including invalid filter construction/introspection) without logging or actionable context, making debugging and edge-case diagnosis difficult. Referred Code # Check if a field is an array type by introspecting the Ash resource # This is dynamic so we don't need to hardcode array field names defp array_field?(resource, field_name) when is_atom(field_name) do case Ash.Resource.Info.attribute(resource, field_name) do %{type: {:array, _}} -> true %{type: Ash.Type.Array} -> true _ -> false end rescue # If introspection fails, assume it's not an array _ -> false end defp array_field?(_resource, _field_name), do: false @doc """ Check if an entity should be routed through Ash. """ def ash_entity?(entity) when is_binary(entity), do: entity in @ash_entities def ash_entity?(_), do: false ... (clipped 296 lines) 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: Exception message exposure: `build_query/4` returns `Exception.message(e)` in the error tuple which may propagate internal implementation details to user-facing surfaces depending on upstream handling. Referred Code `rescue e -> {:error, {:query_build_error, Exception.message(e)}} end` 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/2326#issuecomment-3764516157 Original created: 2026-01-18T00:16:00Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR: <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td>🟢</td><td><details><summary>No security concerns identified</summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'>Ticket Compliance</td></tr> <tr><td>⚪</td><td><details><summary>🎫 No ticket provided </summary> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'>Codebase Duplication Compliance</td></tr> <tr><td>⚪</td><td><details><summary>Codebase context is not defined </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'>Custom Compliance</td></tr> <tr><td rowspan=4>🟢</td><td> <details><summary>Generic: Comprehensive Audit Trails</summary> **Objective:** To create a detailed and reliable record of critical system actions for security analysis and compliance. **Status:** Passed > 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>Generic: Meaningful Naming and Self-Documenting Code</summary> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code self-documenting **Status:** Passed > 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>Generic: Secure Logging Practices</summary> **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 <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>Generic: Security-First Input Validation and Data Handling</summary> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities **Status:** Passed > 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>Generic: Robust Error Handling and Edge Case Management</summary> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful degradation **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2326/files#diff-b4206b4f7d4604e08471361ad7ddae4b6dbb75f67239cb134d20644633f5eb8bR223-R539'>Silent error swallowing</a>: Multiple broad <code>rescue _ -> query/false</code> blocks silently ignore failures (including invalid filter construction/introspection) without logging or actionable context, making debugging and edge-case diagnosis difficult. <details open><summary>Referred Code</summary> ```elixir # Check if a field is an array type by introspecting the Ash resource # This is dynamic so we don't need to hardcode array field names defp array_field?(resource, field_name) when is_atom(field_name) do case Ash.Resource.Info.attribute(resource, field_name) do %{type: {:array, _}} -> true %{type: Ash.Type.Array} -> true _ -> false end rescue # If introspection fails, assume it's not an array _ -> false end defp array_field?(_resource, _field_name), do: false @doc """ Check if an entity should be routed through Ash. """ def ash_entity?(entity) when is_binary(entity), do: entity in @ash_entities def ash_entity?(_), do: false ... (clipped 296 lines) ``` </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=1>⚪</td> <td><details> <summary>Generic: Secure Error Handling</summary> **Objective:** To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2326/files#diff-b4206b4f7d4604e08471361ad7ddae4b6dbb75f67239cb134d20644633f5eb8bR424-R427'>Exception message exposure</a>: <code>build_query/4</code> returns <code>Exception.message(e)</code> in the error tuple which may propagate internal implementation details to user-facing surfaces depending on upstream handling. <details open><summary>Referred Code</summary> ```elixir rescue e -> {:error, {:query_build_error, Exception.message(e)}} 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 align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant 🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label </details>

qodo-code-review[bot] commented

2026-01-18 00:17:05 +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/2326#issuecomment-3764517078
Original created: 2026-01-18T00:17:05Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ ~~Import Ash.Query macros~~ Suggestion Impact: The commit did not add `import Ash.Query`, but it refactored filter construction to use `Ash.Query.filter_input/2` with map-based predicates (e.g., `contains`, `not_contains`, `not_in`) instead of the `ref/1` and `ilike/2` macros. This removes the need for importing `Ash.Query` macros, addressing the underlying compilation risk the suggestion raised. code diff: Ash.Query.filter_input(query, %{field_atom => %{contains: value}}) "like" -> - # LIKE with wildcards - strip % and use case-insensitive contains + # LIKE with wildcards - strip % and use substring contains pattern = value \|> to_string() \|> String.replace("%", "") - Ash.Query.filter(query, ilike(^ref(field_atom), ^"%#{pattern}%")) + Ash.Query.filter_input(query, %{field_atom => %{contains: pattern}}) "not_like" -> - # Negated LIKE + # Negated LIKE - doesn't contain the pattern pattern = value \|> to_string() \|> String.replace("%", "") - Ash.Query.filter(query, not(ilike(^ref(field_atom), ^"%#{pattern}%"))) + Ash.Query.filter_input(query, %{field_atom => %{not_contains: pattern}}) "in" when is_list(value) -> Ash.Query.filter_input(query, %{field_atom => %{in: value}}) "not_in" when is_list(value) -> - Ash.Query.filter(query, ^ref(field_atom) not in ^value) + Ash.Query.filter_input(query, %{field_atom => %{not_in: value}}) Add `import Ash.Query` to make the `ref`, `ilike`, and `has_any` macros available and prevent a compilation error. web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex [59-525] `require Ash.Query +import Ash.Query ... defp apply_scalar_filter(query, field_atom, op, value) do ... "like" -> # LIKE with wildcards - strip % and use case-insensitive contains pattern = value \|> to_string() \|> String.replace("%", "") Ash.Query.filter(query, ilike(^ref(field_atom), ^"%#{pattern}%")) "not_like" -> # Negated LIKE pattern = value \|> to_string() \|> String.replace("%", "") Ash.Query.filter(query, not(ilike(^ref(field_atom), ^"%#{pattern}%"))) ... end` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a missing `import Ash.Query` that will cause a compilation failure, making it a critical fix for the code to work as intended.	High
General	Log errors in array filter In `apply_array_filter/4`, log any rescued exceptions instead of silently ignoring them to improve debuggability when a filter fails. web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex [469-489] defp apply_array_filter(query, field_atom, op, value) do # Convert scalar to list for array containment check values = if is_list(value), do: value, else: [value] case op do op when op in ["in", "eq", "contains", "like"] -> # For arrays, use "in" to check if array contains any of the values # This translates to array && ARRAY[values] in PostgreSQL Ash.Query.filter_input(query, %{field_atom => %{has_any: values}}) op when op in ["not_in", "neq", "not_like"] -> # Negated array containment - doesn't have any of the values Ash.Query.filter(query, not(has_any(^ref(field_atom), ^values))) _ -> query end rescue - # Fall back to no filter on error - _ -> query + e -> + Logger.error("Failed to apply array filter: #{Exception.format(:error, e, __STACKTRACE__)}") + query end Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that swallowing errors silently is bad practice and proposes adding logging, which significantly improves debuggability without changing the function's behavior.	Medium
General	Log errors in scalar filter In `apply_scalar_filter/4`, log any rescued exceptions instead of silently ignoring them to improve debuggability when a filter fails. web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex [492-539] `defp apply_scalar_filter(query, field_atom, op, value) do # Use filter_input for dynamic filter building (recommended in Ash 3.x) case op do ... end rescue - # Fall back to no filter on error - _ -> query + e -> + Logger.error("Failed to apply scalar filter: #{Exception.format(:error, e, __STACKTRACE__)}") + query end` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that swallowing errors silently is bad practice and proposes adding logging, which significantly improves debuggability without changing the function's behavior.	Medium
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2326#issuecomment-3764517078 Original created: 2026-01-18T00:17:05Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>✅ <s>Import Ash.Query macros</s></summary> ___ <details><summary>Suggestion Impact:</summary>The commit did not add `import Ash.Query`, but it refactored filter construction to use `Ash.Query.filter_input/2` with map-based predicates (e.g., `contains`, `not_contains`, `not_in`) instead of the `ref/1` and `ilike/2` macros. This removes the need for importing `Ash.Query` macros, addressing the underlying compilation risk the suggestion raised. code diff: ```diff Ash.Query.filter_input(query, %{field_atom => %{contains: value}}) "like" -> - # LIKE with wildcards - strip % and use case-insensitive contains + # LIKE with wildcards - strip % and use substring contains pattern = value |> to_string() |> String.replace("%", "") - Ash.Query.filter(query, ilike(^ref(field_atom), ^"%#{pattern}%")) + Ash.Query.filter_input(query, %{field_atom => %{contains: pattern}}) "not_like" -> - # Negated LIKE + # Negated LIKE - doesn't contain the pattern pattern = value |> to_string() |> String.replace("%", "") - Ash.Query.filter(query, not(ilike(^ref(field_atom), ^"%#{pattern}%"))) + Ash.Query.filter_input(query, %{field_atom => %{not_contains: pattern}}) "in" when is_list(value) -> Ash.Query.filter_input(query, %{field_atom => %{in: value}}) "not_in" when is_list(value) -> - Ash.Query.filter(query, ^ref(field_atom) not in ^value) + Ash.Query.filter_input(query, %{field_atom => %{not_in: value}}) ``` </details> ___ **Add <code>import Ash.Query</code> to make the <code>ref</code>, <code>ilike</code>, and <code>has_any</code> macros available and prevent a compilation error.** [web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex [59-525]](https://github.com/carverauto/serviceradar/pull/2326/files#diff-b4206b4f7d4604e08471361ad7ddae4b6dbb75f67239cb134d20644633f5eb8bR59-R525) ```diff require Ash.Query +import Ash.Query ... defp apply_scalar_filter(query, field_atom, op, value) do ... "like" -> # LIKE with wildcards - strip % and use case-insensitive contains pattern = value |> to_string() |> String.replace("%", "") Ash.Query.filter(query, ilike(^ref(field_atom), ^"%#{pattern}%")) "not_like" -> # Negated LIKE pattern = value |> to_string() |> String.replace("%", "") Ash.Query.filter(query, not(ilike(^ref(field_atom), ^"%#{pattern}%"))) ... end ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a missing `import Ash.Query` that will cause a compilation failure, making it a critical fix for the code to work as intended. </details></details></td><td align=center>High </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Log errors in array filter</summary> ___ **In <code>apply_array_filter/4</code>, log any rescued exceptions instead of silently ignoring them to improve debuggability when a filter fails.** [web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex [469-489]](https://github.com/carverauto/serviceradar/pull/2326/files#diff-b4206b4f7d4604e08471361ad7ddae4b6dbb75f67239cb134d20644633f5eb8bR469-R489) ```diff defp apply_array_filter(query, field_atom, op, value) do # Convert scalar to list for array containment check values = if is_list(value), do: value, else: [value] case op do op when op in ["in", "eq", "contains", "like"] -> # For arrays, use "in" to check if array contains any of the values # This translates to array && ARRAY[values] in PostgreSQL Ash.Query.filter_input(query, %{field_atom => %{has_any: values}}) op when op in ["not_in", "neq", "not_like"] -> # Negated array containment - doesn't have any of the values Ash.Query.filter(query, not(has_any(^ref(field_atom), ^values))) _ -> query end rescue - # Fall back to no filter on error - _ -> query + e -> + Logger.error("Failed to apply array filter: #{Exception.format(:error, e, __STACKTRACE__)}") + query end ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly points out that swallowing errors silently is bad practice and proposes adding logging, which significantly improves debuggability without changing the function's behavior. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Log errors in scalar filter</summary> ___ **In <code>apply_scalar_filter/4</code>, log any rescued exceptions instead of silently ignoring them to improve debuggability when a filter fails.** [web-ng/lib/serviceradar_web_ng/srql/ash_adapter.ex [492-539]](https://github.com/carverauto/serviceradar/pull/2326/files#diff-b4206b4f7d4604e08471361ad7ddae4b6dbb75f67239cb134d20644633f5eb8bR492-R539) ```diff defp apply_scalar_filter(query, field_atom, op, value) do # Use filter_input for dynamic filter building (recommended in Ash 3.x) case op do ... end rescue - # Fall back to no filter on error - _ -> query + e -> + Logger.error("Failed to apply scalar filter: #{Exception.format(:error, e, __STACKTRACE__)}") + query end ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly points out that swallowing errors silently is bad practice and proposes adding logging, which significantly improves debuggability without changing the function's behavior. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>

qodo-code-review[bot] commented

2026-01-18 06:36:32 +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/2326#issuecomment-3764977663
Original created: 2026-01-18T06:36:32Z

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: build

Failed stage: Setup Rust [❌]

Failed test name: ""

Failure summary:

The action failed during the step install rustup if needed because the runner could not download the
Rust installer.
- curl failed with OpenSSL SSL_read: Connection reset by peer (errno 104) while
fetching https://static.rust-lang.org/.../rustup-init (log lines 347-349).
- This network/TLS
connection reset caused the downloader command to fail, and the job exited with code 1 (line 350).

Relevant error logs:

1:  Runner name: 'arc-runner-set-hk6mk-runner-ghfn8'
2:  Runner group name: 'Default'
...

139:  ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m
140:  ^[[36;1m  sudo apt-get update^[[0m
141:  ^[[36;1m  sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m
142:  ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m
143:  ^[[36;1m  sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
144:  ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m
145:  ^[[36;1m  sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
146:  ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m
147:  ^[[36;1m  sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
148:  ^[[36;1melse^[[0m
149:  ^[[36;1m  echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m
150:  ^[[36;1m  exit 1^[[0m
151:  ^[[36;1mfi^[[0m
152:  ^[[36;1m^[[0m
153:  ^[[36;1mensure_pkg_config^[[0m
154:  ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m
155:  shell: /usr/bin/bash -e {0}
...

316:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
317:  env:
318:  BUILDBUDDY_ORG_API_KEY: ***
319:  SRQL_TEST_DATABASE_URL: ***
320:  SRQL_TEST_ADMIN_URL: ***
321:  SRQL_TEST_DATABASE_CA_CERT: 
322:  DOCKERHUB_USERNAME: ***
323:  DOCKERHUB_TOKEN: ***
324:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
325:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
326:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
327:  ##[endgroup]
328:  ##[group]Run : install rustup if needed
329:  ^[[36;1m: install rustup if needed^[[0m
330:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
331:  ^[[36;1m  curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m
332:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

334:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
335:  env:
336:  BUILDBUDDY_ORG_API_KEY: ***
337:  SRQL_TEST_DATABASE_URL: ***
338:  SRQL_TEST_ADMIN_URL: ***
339:  SRQL_TEST_DATABASE_CA_CERT: 
340:  DOCKERHUB_USERNAME: ***
341:  DOCKERHUB_TOKEN: ***
342:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
343:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
344:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
345:  CARGO_HOME: /home/runner/.cargo
346:  ##[endgroup]
347:  info: downloading installer
348:  warn: curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 104
349:  error: command failed: downloader https://static.rust-lang.org/rustup/dist/x86_64-unknown-linux-gnu/rustup-init /tmp/tmp.5UNo7PWd3o/rustup-init x86_64-unknown-linux-gnu
350:  ##[error]Process completed with exit code 1.
351:  Post job cleanup.

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2326#issuecomment-3764977663 Original created: 2026-01-18T06:36:32Z --- ## CI Feedback 🧐 A test triggered by this PR failed. Here is an AI-generated analysis of the failure: <table><tr><td> **Action:** build</td></tr> <tr><td> **Failed stage:** [Setup Rust](https://github.com/carverauto/serviceradar/actions/runs/21107330421/job/60700740621) [❌] </td></tr> <tr><td> **Failed test name:** "" </td></tr> <tr><td> **Failure summary:** The action failed during the step <code>install rustup if needed</code> because the runner could not download the Rust installer. - <code>curl</code> failed with <code>OpenSSL SSL_read: Connection reset by peer (errno 104)</code> while fetching <code>https://static.rust-lang.org/.../rustup-init</code> (log lines 347-349). - This network/TLS connection reset caused the <code>downloader</code> command to fail, and the job exited with code <code>1</code> (line 350). </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-ghfn8' 2: Runner group name: 'Default' ... 139: ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m 140: ^[[36;1m sudo apt-get update^[[0m 141: ^[[36;1m sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m 142: ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m 143: ^[[36;1m sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 144: ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m 145: ^[[36;1m sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 146: ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m 147: ^[[36;1m sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 148: ^[[36;1melse^[[0m 149: ^[[36;1m echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m 150: ^[[36;1m exit 1^[[0m 151: ^[[36;1mfi^[[0m 152: ^[[36;1m^[[0m 153: ^[[36;1mensure_pkg_config^[[0m 154: ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m 155: shell: /usr/bin/bash -e {0} ... 316: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 317: env: 318: BUILDBUDDY_ORG_API_KEY: *** 319: SRQL_TEST_DATABASE_URL: *** 320: SRQL_TEST_ADMIN_URL: *** 321: SRQL_TEST_DATABASE_CA_CERT: 322: DOCKERHUB_USERNAME: *** 323: DOCKERHUB_TOKEN: *** 324: TEST_CNPG_DATABASE: serviceradar_web_ng_test 325: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 326: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 327: ##[endgroup] 328: ##[group]Run : install rustup if needed 329: ^[[36;1m: install rustup if needed^[[0m 330: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 331: ^[[36;1m curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m 332: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 334: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 335: env: 336: BUILDBUDDY_ORG_API_KEY: *** 337: SRQL_TEST_DATABASE_URL: *** 338: SRQL_TEST_ADMIN_URL: *** 339: SRQL_TEST_DATABASE_CA_CERT: 340: DOCKERHUB_USERNAME: *** 341: DOCKERHUB_TOKEN: *** 342: TEST_CNPG_DATABASE: serviceradar_web_ng_test 343: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 344: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 345: CARGO_HOME: /home/runner/.cargo 346: ##[endgroup] 347: info: downloading installer 348: warn: curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 104 349: error: command failed: downloader https://static.rust-lang.org/rustup/dist/x86_64-unknown-linux-gnu/rustup-init /tmp/tmp.5UNo7PWd3o/rustup-init x86_64-unknown-linux-gnu 350: ##[error]Process completed with exit code 1. 351: Post job cleanup. ``` </details></td></tr></table>