Browse Source

fix(energy): recursively strip tz from nested params for insertmanyvalues

  The prior fix (9f643724) added a list branch to _strip_container but only
  walked one level deep. SQLAlchemy's insertmanyvalues feature can pass
  parameters as nested containers (e.g. a list of tuples, or a tuple inside
  a list) depending on the dialect path, so the inner tz-aware datetimes
  still reached asyncpg and the hourly snapshot loop kept failing with:

    asyncpg.DataError: invalid input for query argument $2: ...
    (can't subtract offset-naive and offset-aware datetimes)

  Replaced the two-helper design with a single recursive _strip() that
  walks dict/list/tuple at any depth. One top-level call now handles every
  parameter shape SQLAlchemy may use, regardless of executemany or the
  insertmanyvalues batching path.
maziggy 1 month ago
parent
commit
1516d58118
2 changed files with 14 additions and 18 deletions
  1. 1 1
      CHANGELOG.md
  2. 13 17
      backend/app/core/database.py

+ 1 - 1
CHANGELOG.md

@@ -14,7 +14,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **LDAP Default Fallback Group** — Settings → Authentication → LDAP → Advanced now has a "Default group" selector. When an LDAP user authenticates but is not listed in any mapped LDAP group, they are automatically assigned to this fallback group instead of being left without permissions. Previously such users could log in successfully but landed on empty pages because every permission check failed. Leave the setting empty to preserve the old behavior. A warning is logged each time the fallback is applied so administrators can spot missing group assignments.
 - **LDAP Default Fallback Group** — Settings → Authentication → LDAP → Advanced now has a "Default group" selector. When an LDAP user authenticates but is not listed in any mapped LDAP group, they are automatically assigned to this fallback group instead of being left without permissions. Previously such users could log in successfully but landed on empty pages because every permission check failed. Leave the setting empty to preserve the old behavior. A warning is logged each time the fallback is applied so administrators can spot missing group assignments.
 
 
 ### Fixed
 ### Fixed
-- **Energy Snapshot Capture Crashes on PostgreSQL** — With an external PostgreSQL database configured, the hourly smart-plug energy snapshot loop (introduced with the #941 fix) logged `asyncpg.DataError: invalid input for query argument $2: ... can't subtract offset-naive and offset-aware datetimes` every hour and failed to persist any snapshots, so date-filtered energy statistics in total-consumption mode stayed empty on Postgres installs. The engine already had a `before_cursor_execute` hook that strips `tzinfo` from bound datetime parameters before they reach asyncpg (the `smart_plug_energy_snapshots.recorded_at` column is `TIMESTAMP WITHOUT TIME ZONE` to match the rest of the schema), but the hook's `_strip_container` helper only handled `dict` and `tuple` parameter containers. When SQLAlchemy's `insertmanyvalues` feature batched two or more snapshot rows into a single `INSERT ... SELECT FROM (VALUES ...)` statement, it passed parameters as a flat `list`, which fell through the helper unchanged and reached asyncpg still tz-aware. The helper now also strips lists. SQLite installs were never affected (SQLite ignores tzinfo entirely).
+- **Energy Snapshot Capture Crashes on PostgreSQL** — With an external PostgreSQL database configured, the hourly smart-plug energy snapshot loop (introduced with the #941 fix) logged `asyncpg.DataError: invalid input for query argument $2: ... can't subtract offset-naive and offset-aware datetimes` every hour and failed to persist any snapshots, so date-filtered energy statistics in total-consumption mode stayed empty on Postgres installs. The engine already had a `before_cursor_execute` hook that strips `tzinfo` from bound datetime parameters before they reach asyncpg (the `smart_plug_energy_snapshots.recorded_at` column is `TIMESTAMP WITHOUT TIME ZONE` to match the rest of the schema), but the hook only stripped datetimes one level deep — when SQLAlchemy's `insertmanyvalues` feature batched multiple snapshot rows into a single `INSERT ... SELECT FROM (VALUES ...)` statement, parameters arrived as nested containers (lists of tuples, or a list inside an outer container) and the inner datetimes slipped through untouched. The hook now recursively walks any nesting of dict/list/tuple and strips `tzinfo` at any depth, so every parameter shape SQLAlchemy may use is handled. SQLite installs were never affected (SQLite ignores tzinfo entirely).
 - **Wrong Filament Color Name Shown on Printer Tab AMS Popup** ([#857](https://github.com/maziggy/bambuddy/issues/857)) — PLA Translucent Cherry Pink (and other colors outside a small hand-maintained list) appeared as "Scarlet Red" on the Printer tab AMS slot popup, and was also auto-provisioned into the inventory under the wrong name on the first RFID read. Root cause: both the backend spool auto-provisioner and the frontend AMS popup resolved color names by looking up the Bambu `tray_id_name` code (e.g. `A17-R1`) in a hardcoded table, and when the exact code wasn't listed they fell back to a suffix-only lookup (`R1 → Scarlet Red`). The suffix half of that code is **not** globally unique across material families — `A17-R1` is PLA Translucent Cherry Pink, while `A01-R1` is PLA Matte Scarlet Red — so the fallback was structurally guaranteed to produce wrong names for any color the hand-maintained list didn't happen to cover. The resolver has been rewritten to use the existing `color_catalog` table (seeded from `catalog_defaults.py` plus the FilamentColors.xyz sync) as the single source of truth. Backend lookup is now by hex color against the catalog; the frontend fetches a compact `{hex: name}` map once per session via a new `GET /api/inventory/colors/map` endpoint (available to any authenticated user, not gated on `inventory:read`), stores it in a `ColorCatalogProvider` context, and uses it for all `getColorName()` calls. The hardcoded tables in `backend/app/core/bambu_colors.py`, `frontend/src/utils/colors.ts`, and `frontend/src/pages/PrintersPage.tsx` have been removed entirely. Existing spools that were auto-created with a wrong name before this fix need to be renamed manually — the fix only affects new auto-provisioning and live display. Thanks to @lightmaster for reporting.
 - **Wrong Filament Color Name Shown on Printer Tab AMS Popup** ([#857](https://github.com/maziggy/bambuddy/issues/857)) — PLA Translucent Cherry Pink (and other colors outside a small hand-maintained list) appeared as "Scarlet Red" on the Printer tab AMS slot popup, and was also auto-provisioned into the inventory under the wrong name on the first RFID read. Root cause: both the backend spool auto-provisioner and the frontend AMS popup resolved color names by looking up the Bambu `tray_id_name` code (e.g. `A17-R1`) in a hardcoded table, and when the exact code wasn't listed they fell back to a suffix-only lookup (`R1 → Scarlet Red`). The suffix half of that code is **not** globally unique across material families — `A17-R1` is PLA Translucent Cherry Pink, while `A01-R1` is PLA Matte Scarlet Red — so the fallback was structurally guaranteed to produce wrong names for any color the hand-maintained list didn't happen to cover. The resolver has been rewritten to use the existing `color_catalog` table (seeded from `catalog_defaults.py` plus the FilamentColors.xyz sync) as the single source of truth. Backend lookup is now by hex color against the catalog; the frontend fetches a compact `{hex: name}` map once per session via a new `GET /api/inventory/colors/map` endpoint (available to any authenticated user, not gated on `inventory:read`), stores it in a `ColorCatalogProvider` context, and uses it for all `getColorName()` calls. The hardcoded tables in `backend/app/core/bambu_colors.py`, `frontend/src/utils/colors.ts`, and `frontend/src/pages/PrintersPage.tsx` have been removed entirely. Existing spools that were auto-created with a wrong name before this fix need to be renamed manually — the fix only affects new auto-provisioning and live display. Thanks to @lightmaster for reporting.
 - **LDAP Auto-Provisioning Fails on Upgraded SQLite Installs** ([#794](https://github.com/maziggy/bambuddy/issues/794)) — First LDAP login on an upgraded SQLite install hit `sqlite3.IntegrityError: NOT NULL constraint failed: users.password_hash` and fell through to a 500 response, because the `users` table on disk had been created before LDAP support landed with `password_hash VARCHAR(255) NOT NULL`. The model was already `nullable=True` and the migration to drop the constraint existed, but only ran on PostgreSQL — SQLite was skipped entirely because it has no `ALTER COLUMN ... DROP NOT NULL`. The migration now patches `sqlite_master` directly via `PRAGMA writable_schema` and bumps `PRAGMA schema_version` so the current connection reloads the table definition without requiring a restart. Fresh installs were never affected (they go through `Base.metadata.create_all` which uses the current nullable model). Thanks to @DylanBrass for reporting.
 - **LDAP Auto-Provisioning Fails on Upgraded SQLite Installs** ([#794](https://github.com/maziggy/bambuddy/issues/794)) — First LDAP login on an upgraded SQLite install hit `sqlite3.IntegrityError: NOT NULL constraint failed: users.password_hash` and fell through to a 500 response, because the `users` table on disk had been created before LDAP support landed with `password_hash VARCHAR(255) NOT NULL`. The model was already `nullable=True` and the migration to drop the constraint existed, but only ran on PostgreSQL — SQLite was skipped entirely because it has no `ALTER COLUMN ... DROP NOT NULL`. The migration now patches `sqlite_master` directly via `PRAGMA writable_schema` and bumps `PRAGMA schema_version` so the current connection reloads the table definition without requiring a restart. Fresh installs were never affected (they go through `Base.metadata.create_all` which uses the current nullable model). Thanks to @DylanBrass for reporting.
 - **Energy Statistics Empty for Week/Month/Day in Total Consumption Mode** ([#941](https://github.com/maziggy/bambuddy/issues/941)) — With "Total consumption" selected as the energy tracking mode, the Statistics page showed the correct kWh total for All Time but zero for every time-filtered range (Today, This Week, This Month, …). The backend fell back to summing per-print archive energy whenever a date filter was active, but in total-consumption mode the per-print column was often empty for two reasons: (1) the starting-kWh value was held in an in-memory dict (`_print_energy_start`) that was lost on any backend restart mid-print, so prints that spanned a restart never got an energy delta computed; (2) historical prints from before a smart plug was added had no value at all. The fix replaces the in-memory dict with a persisted `energy_start_kwh` column on the archive row, and adds an hourly snapshot loop (`smart_plug_energy_snapshots` table) that captures each plug's lifetime counter. The `/archives/stats` endpoint now computes date-range totals via per-plug `(last-in-range − baseline)` deltas from those snapshots, clamping counter resets to zero. A warming-up flag is returned (and rendered as a tooltip next to the Energy stats on StatsPage) when the query runs on incomplete snapshot history — e.g. right after upgrade, before the hourly loop has built up a baseline before the selected range — so the "low" values during the first hours after upgrading are explained in-product rather than misread as a bug. Fully localized across all 7 UI languages. Per-print energy tracking is now restart-resilient in all modes as a side-effect. Thanks to Mike (@TheMadMike23) for reporting.
 - **Energy Statistics Empty for Week/Month/Day in Total Consumption Mode** ([#941](https://github.com/maziggy/bambuddy/issues/941)) — With "Total consumption" selected as the energy tracking mode, the Statistics page showed the correct kWh total for All Time but zero for every time-filtered range (Today, This Week, This Month, …). The backend fell back to summing per-print archive energy whenever a date filter was active, but in total-consumption mode the per-print column was often empty for two reasons: (1) the starting-kWh value was held in an in-memory dict (`_print_energy_start`) that was lost on any backend restart mid-print, so prints that spanned a restart never got an energy delta computed; (2) historical prints from before a smart plug was added had no value at all. The fix replaces the in-memory dict with a persisted `energy_start_kwh` column on the archive row, and adds an hourly snapshot loop (`smart_plug_energy_snapshots` table) that captures each plug's lifetime counter. The `/archives/stats` endpoint now computes date-range totals via per-plug `(last-in-range − baseline)` deltas from those snapshots, clamping counter resets to zero. A warming-up flag is returned (and rendered as a tooltip next to the Energy stats on StatsPage) when the query runs on incomplete snapshot history — e.g. right after upgrade, before the hourly loop has built up a baseline before the selected range — so the "low" values during the first hours after upgrading are explained in-product rather than misread as a bug. Fully localized across all 7 UI languages. Per-print energy tracking is now restart-resilient in all modes as a side-effect. Thanks to Mike (@TheMadMike23) for reporting.

+ 13 - 17
backend/app/core/database.py

@@ -48,28 +48,24 @@ def _create_engine():
             if parameters is None:
             if parameters is None:
                 return statement, parameters
                 return statement, parameters
 
 
+            # Recursive strip that walks any nesting of dict/list/tuple. Needed
+            # because SQLAlchemy passes parameters in several shapes depending
+            # on the path: a dict for named binds, a tuple for positional, a
+            # list of dicts/tuples for executemany, and for insertmanyvalues
+            # sometimes a list of tuples inside an outer list. The simplest
+            # correct answer is "strip datetimes at any depth".
             def _strip(val):
             def _strip(val):
                 if isinstance(val, datetime.datetime) and val.tzinfo is not None:
                 if isinstance(val, datetime.datetime) and val.tzinfo is not None:
                     return val.replace(tzinfo=None)
                     return val.replace(tzinfo=None)
+                if isinstance(val, dict):
+                    return {k: _strip(v) for k, v in val.items()}
+                if isinstance(val, list):
+                    return [_strip(v) for v in val]
+                if isinstance(val, tuple):
+                    return tuple(_strip(v) for v in val)
                 return val
                 return val
 
 
-            def _strip_container(params):
-                if isinstance(params, dict):
-                    return {k: _strip(v) for k, v in params.items()}
-                elif isinstance(params, tuple):
-                    return tuple(_strip(v) for v in params)
-                elif isinstance(params, list):
-                    # SQLAlchemy's insertmanyvalues feature sends one flattened
-                    # list of positional params for a single batched INSERT.
-                    return [_strip(v) for v in params]
-                return params
-
-            if executemany and isinstance(parameters, (list, tuple)):
-                # Batch: list of dicts or list of tuples
-                parameters = [_strip_container(row) for row in parameters]
-            else:
-                parameters = _strip_container(parameters)
-            return statement, parameters
+            return statement, _strip(parameters)
 
 
     return eng
     return eng