Skip to content

BE: lazy SQL execution caused devIsSleeping to be missing and tiles n…#1572

Merged
jokob-sk merged 1 commit intomainfrom
next_release
Mar 22, 2026
Merged

BE: lazy SQL execution caused devIsSleeping to be missing and tiles n…#1572
jokob-sk merged 1 commit intomainfrom
next_release

Conversation

@jokob-sk
Copy link
Collaborator

@jokob-sk jokob-sk commented Mar 22, 2026

…ot show #1569 #1250

Summary by CodeRabbit

  • Refactor
    • Enhanced device data retrieval to improve consistency in device status reporting and counts across the application.

…ot show #1569 #1250

Signed-off-by: jokob-sk <jokob.sk@gmail.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 22, 2026

📝 Walkthrough

Walkthrough

The get_sql_devices_tiles() function in server/db/db_helper.py was updated to source device data from DevicesView instead of Devices for computing device status counts and the overall device count. This affects all status categories including connected, offline, down, new, archived, and favorites.

Changes

Cohort / File(s) Summary
Device View Data Source
server/db/db_helper.py
Modified get_sql_devices_tiles() to query DevicesView instead of Devices table for device status counts and all-devices totals. Updated MyDevicesFilter selection and all correlated COUNT(*) subqueries.

Poem

🐰 A view of devices so grand and so bright,
Where data flows true, filtered just right,
No longer raw tables, but views dressed with care,
The counts now align with what's actually there!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is truncated and incomplete, but it references fixing issues with lazy SQL execution, missing devIsSleeping field, and tiles not displaying, which aligns with the actual change of switching database views in get_sql_devices_tiles().
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch next_release

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
server/db/db_helper.py (1)

99-103: Minor: devIsSleeping selected but unused in CTE.

The MyDevicesFilter CTE only needs devMac for the COUNT(*) on line 113. Including devIsSleeping in the SELECT clause is harmless but unnecessary since it's only referenced in the WHERE conditions (via my_devices_clauses), not in the output.

That said, the switch to DevicesView is correct and necessary because devIsSleeping is a computed column that only exists in the view, not in the base Devices table.

Optional: Remove unused column from SELECT
                        MyDevicesFilter AS (
-                            SELECT devMac, devIsSleeping
+                            SELECT devMac
                            FROM DevicesView
                            WHERE
                                {my_devices_clauses}
                        )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/db/db_helper.py` around lines 99 - 103, The CTE MyDevicesFilter
selects devIsSleeping from DevicesView but never uses it in the output (only
devMac is needed for the COUNT(*) aggregation in the later query); to clean this
up, remove devIsSleeping from the SELECT list in the MyDevicesFilter CTE so it
only selects devMac (keep the FROM DevicesView and the my_devices_clauses filter
unchanged since devIsSleeping may still be referenced in those clauses).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@server/db/db_helper.py`:
- Around line 99-103: The CTE MyDevicesFilter selects devIsSleeping from
DevicesView but never uses it in the output (only devMac is needed for the
COUNT(*) aggregation in the later query); to clean this up, remove devIsSleeping
from the SELECT list in the MyDevicesFilter CTE so it only selects devMac (keep
the FROM DevicesView and the my_devices_clauses filter unchanged since
devIsSleeping may still be referenced in those clauses).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9316c8fd-a1e4-46dc-82c7-06f607d7d1fc

📥 Commits

Reviewing files that changed from the base of the PR and between 8e72386 and 3773030.

📒 Files selected for processing (1)
  • server/db/db_helper.py

@jokob-sk
Copy link
Collaborator Author

🧹 Nitpick comments (1)

server/db/db_helper.py (1)> 99-103: Minor: devIsSleeping selected but unused in CTE.

The MyDevicesFilter CTE only needs devMac for the COUNT(*) on line 113. Including devIsSleeping in the SELECT clause is harmless but unnecessary since it's only referenced in the WHERE conditions (via my_devices_clauses), not in the output.
That said, the switch to DevicesView is correct and necessary because devIsSleeping is a computed column that only exists in the view, not in the base Devices table.

Optional: Remove unused column from SELECT

                        MyDevicesFilter AS (
-                            SELECT devMac, devIsSleeping
+                            SELECT devMac
                            FROM DevicesView
                            WHERE
                                {my_devices_clauses}
                        )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/db/db_helper.py` around lines 99 - 103, The CTE MyDevicesFilter
selects devIsSleeping from DevicesView but never uses it in the output (only
devMac is needed for the COUNT(*) aggregation in the later query); to clean this
up, remove devIsSleeping from the SELECT list in the MyDevicesFilter CTE so it
only selects devMac (keep the FROM DevicesView and the my_devices_clauses filter
unchanged since devIsSleeping may still be referenced in those clauses).

🤖 Prompt for all review comments with AI agents

ℹ️ Review info

lol, that is the fix - if I wouldn't select it it wouldn't resolve in the subquery - add learning @coderabbitai

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 22, 2026

Tip

For best results, initiate chat on the files or code changes.

@jokob-sk You're absolutely right — my mistake! In SQL CTEs, a column must be explicitly projected in the SELECT list to be accessible to downstream queries that consume the CTE. Even if devIsSleeping isn't in the final output, it needs to be selected so that correlated subqueries referencing MyDevicesFilter can resolve it. Suggesting to remove it was incorrect. Thanks for the clarification!

( ˶ˆ꒳ˆ˵ ) ♦️


✏️ Learnings added
Learnt from: jokob-sk
URL: https://github.com/netalertx/NetAlertX/pull/1572

Timestamp: 2026-03-22T23:03:17.427Z
Learning: In SQL (including SQLite) CTEs used in `server/db/db_helper.py` (NetAlertX project), a column must be explicitly included in the CTE's SELECT list to be resolvable in downstream correlated subqueries that reference the CTE — even if that column doesn't appear in the final output. For example, in `MyDevicesFilter`, `devIsSleeping` must be selected so that correlated subqueries consuming the CTE can reference it. Do not suggest removing such columns as "unused" just because they aren't in the final projection.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@jokob-sk jokob-sk merged commit 30e3633 into main Mar 22, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant