A supply nobody could read is unknown, not empty

plugins/printers/api/asset_routes.py

@@ -2109,7 +2109,9 @@ def _annotate_supply(supply, vendor_name, modelnumberid):
     Waste cartridge direction depends on vendor, so classification lives in
     the supply_parts helper. Part numbers come from the modelsupplies table.
     """
-    level = supply.get('level', 0)
+    # .get('level') with no default: a supply with no reading carries None, and
+    # a 0 default would put back the very substitution this reports around.
+    level = supply.get('level')
     name = supply.get('name', 'Unknown')
     supplytype = derivesupplytype(name)
     color = derivecolor(name, supply.get('color'))
@@ -2117,6 +2119,7 @@ def _annotate_supply(supply, vendor_name, modelnumberid):
     return {
         'name': name,
         'level': level,
+        'lastseen': supply.get('lastseen'),
         'color': color,
         'supplytype': supplytype,
         'status': cls['status'],
@@ -2160,6 +2163,7 @@ def _get_low_supplies_data():
 
     results = []
     total_checked = 0
+    unknown_printers = 0
 
     for printer, asset, comm, vendor, model in unique_printers:
         supplies = service.getsuppliesbyip_cached(comm.ipaddress)
@@ -2178,13 +2182,24 @@ def _get_low_supplies_data():
         # shown it. The whole report exists to answer "what needs replacing".
         annotated = []
         has_low = False
+        has_unknown = False
         for s in supplies:
             item = _annotate_supply(s, vendor_name, modelnumberid)
             if item['status'] == 'ok':
                 continue
+            # A supply nobody could read is not a supply that needs replacing,
+            # so it does not join the list of things to act on. It is counted
+            # instead: a printer that has gone quiet should be visible as that,
+            # not disappear from the report and not masquerade as empty.
+            if item['status'] == 'unknown':
+                has_unknown = True
+                continue
             has_low = True
             annotated.append(item)
 
+        if has_unknown:
+            unknown_printers += 1
+
         if has_low:
             # location name for the report row
             # Via the relationship, the way the printers list does it. The
@@ -2222,7 +2237,12 @@ def _get_low_supplies_data():
         'summary': {
             'total_checked': total_checked,
             'low': low_count,
-            'critical': critical_count
+            'critical': critical_count,
+            # Printers with at least one unreadable supply. They are NOT in
+            # 'printers' above - nothing about them needs ordering - but a
+            # count that only ever went up when a cartridge ran low would hide
+            # a fleet quietly going dark.
+            'unknown': unknown_printers
         }
     }
 

plugins/printers/frontend/views/PrinterDetail.vue

@@ -215,8 +215,14 @@
           <div v-for="supply in supplies" :key="supply.itemid || supply.name" class="supply-item">
             <div class="supply-header">
               <span class="supply-name">{{ supply.name }}</span>
-              <span class="supply-level" :class="supply.status">
+              <!-- N/A is not the same as 0%. A supply Zabbix has no usable
-                {{ supply.level !== null ? `${supply.level}%` : 'N/A' }}
+                   reading for arrives with level null and status 'unknown';
+                   the title says which of the two reasons it is, because
+                   "no data" and "last seen three weeks ago" need different
+                   people to fix them. -->
+              <span class="supply-level" :class="supply.status"
+                    :title="supply.status === 'unknown' ? unknownReason(supply) : null">
+                {{ supply.level !== null && supply.level !== undefined ? `${supply.level}%` : 'N/A' }}
               </span>
             </div>
             <div class="supply-bar">
@@ -227,7 +233,7 @@
               ></div>
             </div>
             <div class="supply-meta">
-              <span>{{ formatSupplyType(supply.supplytype) }}<template v-if="supply.iswaste"> ({{ supply.remaining }}% remaining)</template></span>
+              <span>{{ formatSupplyType(supply.supplytype) }}<template v-if="supply.iswaste && supply.remaining !== null"> ({{ supply.remaining }}% remaining)</template></span>
               <span v-if="supply.partnumbers && supply.partnumbers.length" class="supply-parts">
                 <span
                   v-for="part in supply.partnumbers"
@@ -363,6 +369,15 @@ function formatSupplyType(supplytype) {
   return supplytype.charAt(0).toUpperCase() + supplytype.slice(1)
 }
 
+// Why a supply reads N/A. lastseen is the epoch of the newest reading Zabbix
+// holds: absent means the item has never collected, present means it has but
+// the reading is too old to present as current. Those are different faults and
+// the tooltip is the only place either one is stated.
+function unknownReason(supply) {
+  if (!supply.lastseen) return 'Zabbix has no reading for this supply'
+  return `No reading since ${new Date(supply.lastseen * 1000).toLocaleString()}`
+}
+
 function formatDate(dateStr) {
   if (!dateStr) return '-'
   return new Date(dateStr).toLocaleString()
@@ -414,6 +429,10 @@ function formatDate(dateStr) {
 .supply-level.ok { color: var(--success); }
 .supply-level.low { color: var(--warning); }
 .supply-level.critical { color: var(--danger); }
+/* Muted on purpose. An unknown level is not a severity - colouring it like one
+   would put it in the same reading as critical, which is the confusion this
+   whole change exists to end. The help cursor points at the tooltip. */
+.supply-level.unknown { color: var(--text-light); cursor: help; }
 
 .supply-bar {
   height: 10px;

plugins/printers/plugin.py

@@ -323,7 +323,9 @@ class PrintersPlugin(BasePlugin):
 
                 click.echo(f'Supply levels for {ip}:')
                 for supply in supplies:
-                    click.echo(f"  {supply['name']}: {supply['level']}%")
+                    level = supply.get('level')
+                    shown = 'unknown' if level is None else f'{level}%'
+                    click.echo(f"  {supply['name']}: {shown}")
 
         @printerscli.command('seed-supplies')
         def seedsuppliescommand():

plugins/printers/services/supply_parts.py

@@ -68,19 +68,34 @@ def derivecolor(name: str, tagcolor: Optional[str] = None) -> str:
     return "none"
 
 
-def classifysupply(level: float, name: str, vendor: Optional[str]) -> Dict:
+def classifysupply(level: Optional[float], name: str,
-    """Classify one supply item into ok/low/critical.
+                   vendor: Optional[str]) -> Dict:
+    """Classify one supply item into ok/low/critical, or unknown.
 
     Waste cartridge fill is inverted vs a toner level: a full waste cartridge
     is bad. Standard vendors report waste as percent FULL (high = bad). Xerox
     EC/AltaLink series report waste as percent capacity REMAINING (low = bad),
     same direction as toner. Normalise everything to percent remaining first.
+
+    A level of None means Zabbix has no usable reading; the status is 'unknown'
+    and remaining is None. Callers must not treat that as a low supply.
     """
     lowername = (name or "").lower()
     iswaste = "waste" in lowername
     isdrum = "drum" in lowername or "imaging" in lowername
     isxerox = bool(vendor) and "xerox" in vendor.lower()
 
+    # No reading is its own answer, not a level. It cannot be classified ok,
+    # low or critical without asserting something nobody measured - and every
+    # one of those three is a claim the reader would act on.
+    if level is None:
+        return {
+            "status": "unknown",
+            "remaining": None,
+            "iswaste": iswaste,
+            "isdrum": isdrum,
+        }
+
     if iswaste and not isxerox:
         remaining = 100 - level
     else:

plugins/printers/services/zabbix_service.py

@@ -208,18 +208,64 @@ class ZabbixService:
                 continue
             if str(item.get("state", "0")) != "0":
                 continue
-            try:
+            level, lastseen = self._readlevel(item)
-                level = int(float(item.get("lastvalue", 0)))
-            except (ValueError, TypeError):
-                level = 0
             supplies.append({
                 "name": item.get("name", "Unknown"),
                 "level": level,
+                "lastseen": lastseen,
                 "color": self._extract_color(item),
                 "itemid": item.get("itemid"),
             })
         return supplies
 
+    # How old the newest reading may be and still be presented as the level.
+    # Generous on purpose: supply items are polled in hours, not seconds, and
+    # calling a slow poller "unknown" would be its own lie. This exists for the
+    # host that stopped answering, not for one that answers rarely.
+    STALE_AFTER_SECONDS = 24 * 3600
+
+    @classmethod
+    def _readlevel(cls, item):
+        """(level, lastseen) for one supply item; level is None when unknown.
+
+        An item that has never collected returns lastvalue as an EMPTY STRING,
+        and one whose host went quiet keeps a lastvalue that is arbitrarily old.
+        Both used to become 0 - the empty string because float('') raises and
+        the handler substituted zero, the stale one because nothing ever read
+        lastclock, which was fetched and then ignored.
+
+        Zero is not a safe stand-in for "no reading". It is a legitimate value
+        meaning the cartridge is spent, so the substitution was indistinguishable
+        from a real empty - and it hit every item on the host at once, which is
+        what a printer that has gone dark looks like. Downstream that reached
+        `level <= EMPTY_LEVEL`, so the forecast gave it daysleft 0 and the order
+        list asked purchasing to buy a full set of cartridges for a printer
+        nobody had measured.
+
+        None says "unknown" and every consumer already handles it: the detail
+        page renders N/A, the alert loop skips it, and analyse() falls back to
+        the last real level in history rather than inventing one.
+        """
+        raw = item.get("lastvalue")
+        if raw is None or str(raw).strip() == "":
+            return None, None
+        try:
+            level = int(float(raw))
+        except (ValueError, TypeError):
+            return None, None
+
+        try:
+            lastseen = int(item.get("lastclock") or 0)
+        except (ValueError, TypeError):
+            lastseen = 0
+        if lastseen <= 0:
+            # A value with no clock cannot be aged. Trust the value and say the
+            # age is unknown rather than discarding a reading that may be fine.
+            return level, None
+        if time.time() - lastseen > cls.STALE_AFTER_SECONDS:
+            return None, lastseen
+        return level, lastseen
+
     # Per item, per call. Zabbix applies `limit` to the WHOLE result set, not
     # per item, so the budget is multiplied by the number of items asked for.
     HISTORY_PER_ITEM = 2000

tests/test_plugins/test_supply_unknown_level.py

@@ -0,0 +1,163 @@
+"""A supply nobody could read is unknown, not empty.
+
+Reported as "CSF04 now reports all 0%", on the printer's own page as well as the
+reports. Every supply on one host reading zero at the same moment is not what a
+set of cartridges does; it is what a host that stopped answering looks like.
+
+`getsuppliesbyip` turned every unreadable item into 0:
+
+    try:
+        level = int(float(item.get("lastvalue", 0)))
+    except (ValueError, TypeError):
+        level = 0
+
+An item that has never collected returns lastvalue as an EMPTY STRING, so
+float('') raised and the handler substituted zero. `lastclock` was fetched in
+the same request and never read by anything, so a reading from three weeks ago
+was presented as current.
+
+Zero is a legitimate level meaning "spent", which is what made the substitution
+dangerous rather than merely wrong: it reached `level <= EMPTY_LEVEL` in the
+forecast, which set daysleft to 0, which put the printer on the order list. The
+report asked purchasing to buy a full set of cartridges for a printer nobody had
+measured.
+
+None means unknown, and it has to stay None the whole way out.
+"""
+
+import time
+
+import pytest
+
+from plugins.printers.services.zabbix_service import ZabbixService
+from plugins.printers.services.supply_parts import classifysupply
+from plugins.printers.services.supply_history import analyse, EMPTY_LEVEL
+
+
+def item(lastvalue, lastclock=None, name='Black Cartridge'):
+    """One Zabbix item.get row, shaped as the API returns it."""
+    if lastclock is None:
+        lastclock = int(time.time()) - 600
+    return {'itemid': '1', 'name': name,
+            'lastvalue': lastvalue, 'lastclock': str(lastclock)}
+
+
+# --- reading one item -------------------------------------------------------
+
+def test_an_item_that_never_collected_is_unknown_not_zero():
+    """The reported case. Zabbix returns '' for an item with no data."""
+    level, lastseen = ZabbixService._readlevel(item(''))
+    assert level is None
+    assert lastseen is None
+
+
+def test_a_missing_lastvalue_is_unknown_not_zero():
+    level, _ = ZabbixService._readlevel({'lastvalue': None, 'lastclock': '0'})
+    assert level is None
+
+
+def test_an_unparseable_lastvalue_is_unknown_not_zero():
+    level, _ = ZabbixService._readlevel(item('n/a'))
+    assert level is None
+
+
+def test_a_real_zero_is_still_zero():
+    """The whole point of the distinction: an empty cartridge must survive it.
+
+    If this returned None, an actually-spent cartridge would stop being ordered.
+    """
+    level, _ = ZabbixService._readlevel(item('0'))
+    assert level == 0
+
+
+def test_a_normal_reading_comes_back_with_its_clock():
+    when = int(time.time()) - 300
+    level, lastseen = ZabbixService._readlevel(item('47', lastclock=when))
+    assert level == 47
+    assert lastseen == when
+
+
+def test_a_float_reading_is_truncated_to_an_int():
+    level, _ = ZabbixService._readlevel(item('47.8'))
+    assert level == 47
+
+
+# --- staleness --------------------------------------------------------------
+
+def test_a_reading_older_than_the_window_is_unknown():
+    """lastclock was fetched and then ignored, so a host that went quiet weeks
+    ago kept presenting its last level as current."""
+    stale = int(time.time()) - (ZabbixService.STALE_AFTER_SECONDS + 60)
+    level, lastseen = ZabbixService._readlevel(item('47', lastclock=stale))
+    assert level is None
+    assert lastseen == stale, 'the age must survive, so the UI can say how old'
+
+
+def test_a_reading_inside_the_window_is_kept():
+    recent = int(time.time()) - (ZabbixService.STALE_AFTER_SECONDS - 3600)
+    level, _ = ZabbixService._readlevel(item('47', lastclock=recent))
+    assert level == 47
+
+
+def test_a_value_with_no_clock_is_trusted():
+    """A reading that cannot be aged is not thereby wrong. Discarding it would
+    lose real levels wherever lastclock is absent."""
+    level, lastseen = ZabbixService._readlevel(
+        {'lastvalue': '47', 'lastclock': '0'})
+    assert level == 47
+    assert lastseen is None
+
+
+# --- classification ---------------------------------------------------------
+
+def test_an_unknown_level_classifies_as_unknown():
+    result = classifysupply(None, 'Black Cartridge', 'HP')
+    assert result['status'] == 'unknown'
+    assert result['remaining'] is None
+
+
+def test_an_unknown_level_is_not_critical():
+    """'critical' is a claim someone acts on. It may not be made about a supply
+    nobody measured."""
+    assert classifysupply(None, 'Black Cartridge', 'HP')['status'] != 'critical'
+
+
+def test_a_real_zero_is_still_critical():
+    assert classifysupply(0, 'Black Cartridge', 'HP')['status'] == 'critical'
+
+
+def test_an_unknown_waste_cartridge_does_not_invert_into_full():
+    """Waste is scored as 100 - level. With level 0 standing in for unknown,
+    an unreadable waste cartridge read as 100% full, which is the alarm state."""
+    result = classifysupply(None, 'Waste Toner Container', 'HP')
+    assert result['status'] == 'unknown'
+    assert result['remaining'] is None
+
+
+# --- the forecast -----------------------------------------------------------
+
+def test_an_unknown_level_with_no_history_gets_no_countdown():
+    """0 reached `level <= EMPTY_LEVEL` and became daysleft 0 - the step that
+    put CSF04 on the order list."""
+    result = analyse([], currentlevel=None)
+    assert result['daysleft'] is None
+    assert result['reason'] == 'no history'
+
+
+def test_a_real_zero_with_no_history_still_counts_as_due_now():
+    assert analyse([], currentlevel=0)['daysleft'] == 0
+    assert EMPTY_LEVEL >= 0
+
+
+def test_an_unknown_level_falls_back_to_the_last_real_reading():
+    """History is evidence the live read lacks. Preferring it beats inventing a
+    level, and beats throwing the cartridge off the report.
+
+    Clock is unix seconds and level falls over time, which is the only direction
+    toner goes; a rising series would read as a cartridge change.
+    """
+    now = int(time.time())
+    points = [(now - n * 86400, 50 + n * 2) for n in range(20, -1, -1)]
+    result = analyse(points, currentlevel=None)
+    assert result['currentlevel'] == 50, 'the newest stored reading'
+    assert result['daysleft'] is not None, 'a forecast is still possible'