From 3703412bafe1ee969643e4229a91fa9404d8fe42 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Fri, 21 Aug 2026 11:57:02 -0400 Subject: [PATCH] Only zero is empty, and the last day is worth reading Two separate floors were collapsing a live cartridge into a spent one, and removing either alone changes nothing. EMPTY_LEVEL was 5, so everything at or below 5% was assigned daysleft 0 outright. The reasoning was that a printer at 1% is out of toner as far as anyone standing at it is concerned. But the row exists to say how long is left, and 2% draining a point a day has two days in it. Flooring put that cartridge beside ones that genuinely are empty with no way back to the difference. It is 0 now: empty means empty. int(level / rate) then truncated the division, so anything under a full day arrived as 0 whatever the floor did - a cartridge with six hours in it was indistinguishable from one with nothing, and the band read it as empty. daysleft is fractional now, rounded to two places, which is about a quarter of an hour: finer than the estimate deserves, but it costs nothing and keeps the ordering of two nearly-spent cartridges meaningful. daysText reads in whatever unit carries meaning: "6 hours", "1 hour", "29 min", "4 days". Below an hour it goes to minutes with a floor of one, because rounding hours would land back on "empty" - the same bug one rung down. BEHAVIOUR CHANGE worth knowing: a cartridge at 1-5% with NO history used to get daysleft 0 from the floor and land on the order list. A rate needs two readings; with none there is nothing to divide, and the old answer was right by accident - it said "empty" about a level nobody had watched move. It now reports 'no history' and shows under "No estimate yet". This reaches only printers newly added to Zabbix; anything that got to 3% the ordinary way has the history to forecast from. Five existing tests pinned the old rule. They recorded a real decision, so they are rewritten to the new one rather than deleted. One of them was passing for the wrong reason: its series (2.0, 1.7, 1.4, 1.1) drops 0.9, under MIN_DROP_FOR_ESTIMATE, so it never had a rate at all and only passed because the floor short-circuited ahead of the rate check. It now uses a real 20-day drop at a tenth of a point a day and asserts the ten days its docstring always described. The forecast fixture's black cartridge moves from 1% to 0% so the empty band keeps its API-level coverage, and a magenta at 3% covers hours-left end to end. --- .../printers/frontend/views/TonerForecast.vue | 29 +++++++-- .../frontend/views/tonerForecastNames.spec.js | 62 +++++++++++++++++++ plugins/printers/services/supply_history.py | 28 ++++++--- tests/test_plugins/test_supply_history.py | 55 ++++++++++++++-- .../test_toner_forecast_report.py | 25 ++++++-- 5 files changed, 176 insertions(+), 23 deletions(-) diff --git a/plugins/printers/frontend/views/TonerForecast.vue b/plugins/printers/frontend/views/TonerForecast.vue index dac785a..b79570f 100644 --- a/plugins/printers/frontend/views/TonerForecast.vue +++ b/plugins/printers/frontend/views/TonerForecast.vue @@ -293,11 +293,32 @@ function barWidth(cartridge) { return Math.max(0, Math.min(100, level)) + '%' } +// daysleft is fractional, so the last day of a cartridge's life is readable +// instead of collapsing to "empty". Only an actual zero is empty; anything above +// it has time left and the row says how much, in whatever unit carries meaning. +const HOURS_PER_DAY = 24 +const MINUTES_PER_HOUR = 60 + function daysText(cartridge) { - if (cartridge.daysleft == null) return '-' - if (cartridge.daysleft === 0) return 'empty' - if (cartridge.daysleft === 1) return '1 day' - return cartridge.daysleft + ' days' + const days = cartridge.daysleft + if (days == null) return '-' + if (days <= 0) return 'empty' + + if (days < 1) { + const hours = days * HOURS_PER_DAY + // Under an hour, hours would round to 0 and read as empty again. + if (hours < 1) { + const minutes = Math.max(1, Math.round(hours * MINUTES_PER_HOUR)) + return `${minutes} min` + } + const rounded = Math.round(hours) + return rounded === 1 ? '1 hour' : `${rounded} hours` + } + + // Whole days from here. The estimate is not precise enough for "3.7 days" to + // mean more than "about 4", and a column of decimals is harder to scan. + const rounded = Math.round(days) + return rounded === 1 ? '1 day' : `${rounded} days` } function orderKey(item) { diff --git a/plugins/printers/frontend/views/tonerForecastNames.spec.js b/plugins/printers/frontend/views/tonerForecastNames.spec.js index 6c5bbbb..038d62f 100644 --- a/plugins/printers/frontend/views/tonerForecastNames.spec.js +++ b/plugins/printers/frontend/views/tonerForecastNames.spec.js @@ -108,3 +108,65 @@ describe('what purchasing receives is unchanged', () => { expect(written[0]).not.toContain('414X Black') }) }) + +// --- how long is left ------------------------------------------------------ +// +// daysleft used to be int(level / rate), so anything under a full day +// truncated to 0 and the view printed "empty". A cartridge with six hours in it +// still prints, and the row exists to say how long. Only a real zero is empty. + +describe('the countdown reads in whatever unit carries meaning', () => { + async function daysCell(daysleft) { + const band = daysleft === null ? 'soon' : daysleft <= 0 ? 'empty' : 'soon' + const base = payload() + base.data.data.cartridges[0].daysleft = daysleft + base.data.data.cartridges[0].band = band + printersApi.supplyForecast.mockResolvedValue(base) + const wrapper = mount(TonerForecast, { + global: { stubs: { 'router-link': { template: '' } } }, + }) + await flushPromises() + // Only the 'empty' band starts expanded - what is already out is what the + // page opens on. Every band renders a header whether or not it holds + // anything, so the right one has to be picked by its key rather than by + // position; clicking the first would close 'empty' instead. + if (band !== 'empty') { + const head = wrapper.findAll('.band-head') + .find(b => b.find(`.band-count.${band}`).exists()) + await head.trigger('click') + await flushPromises() + } + return wrapper.find('tbody .days').text() + } + + it('says empty only for an actual zero', async () => { + expect(await daysCell(0)).toBe('empty') + }) + + it('reads in hours below a day', async () => { + expect(await daysCell(0.25)).toBe('6 hours') + }) + + it('does not pluralise a single hour', async () => { + expect(await daysCell(1 / 24)).toBe('1 hour') + }) + + it('reads in minutes below an hour, never as empty', async () => { + const text = await daysCell(0.02) + expect(text).toBe('29 min') + expect(text).not.toBe('empty') + }) + + it('never rounds a live cartridge down to empty', async () => { + expect(await daysCell(0.0001)).toBe('1 min') + }) + + it('reads in whole days above a day', async () => { + expect(await daysCell(3.7)).toBe('4 days') + expect(await daysCell(1.2)).toBe('1 day') + }) + + it('still shows a dash when there is no estimate', async () => { + expect(await daysCell(null)).toBe('-') + }) +}) diff --git a/plugins/printers/services/supply_history.py b/plugins/printers/services/supply_history.py index f9e2e70..26f984c 100644 --- a/plugins/printers/services/supply_history.py +++ b/plugins/printers/services/supply_history.py @@ -74,12 +74,16 @@ MIN_DROP_FOR_ESTIMATE = 2 # precise-looking figure invites more trust than it has earned. RATE_SPREAD_FACTOR = 5 -# At or below this, the cartridge is done and the arithmetic stops being the -# useful answer. A supply sitting at 1% that drains a tenth of a point a day -# computes to ten days; a printer at 1% is out of toner as far as anyone -# standing at it is concerned, and it is what should be ordered first. Rate is -# still reported - only the days-left figure is floored. -EMPTY_LEVEL = 5 +# Empty means empty. This was 5, on the reasoning that a printer at 1% is out +# of toner as far as anyone standing at it is concerned - but flooring the +# days-left of everything at or below 5% threw away the only number the row +# exists to give. A cartridge at 2% draining a point a day has two days left, +# and two days is worth knowing; reporting it as already gone puts it beside +# cartridges that genuinely are, and there is no way back to the difference. +# +# Anything above zero gets its real estimate, in hours if that is what it comes +# to. Only a reading of zero is empty. +EMPTY_LEVEL = 0 def _asfloat(value): @@ -345,8 +349,8 @@ def analyse(points, rise=REPLACEMENT_RISE, currentlevel=None): rate = burn_rate(run) - # Empty is empty. Ordering by a rate below this level ranks a dead - # cartridge behind a healthy one that happens to be draining faster. + # Empty is empty, and only zero is empty. Ordering by a rate at this level + # ranks a dead cartridge behind a healthy one that happens to drain faster. if level <= EMPTY_LEVEL: result['daysleft'] = 0 result['burnrateperday'] = round(rate, 2) if rate is not None else None @@ -363,7 +367,13 @@ def analyse(points, rise=REPLACEMENT_RISE, currentlevel=None): return result result['burnrateperday'] = round(rate, 2) - result['daysleft'] = max(0, int(level / rate)) + # Fractional, NOT int(). Truncating meant every cartridge with less than a + # full day left reported 0, which the bands read as empty and the view + # printed as "empty" - a cartridge with six hours in it was indistinguishable + # from one with nothing. Two decimal places is a quarter of an hour, which is + # finer than the estimate deserves but costs nothing and keeps the ordering + # of two nearly-spent cartridges meaningful. + result['daysleft'] = round(max(0.0, level / rate), 2) # Say so when the intervals disagree wildly. The number is still the best # estimate available; the flag stops it reading as a measurement. result['rateunstable'] = rate_is_unstable(run) diff --git a/tests/test_plugins/test_supply_history.py b/tests/test_plugins/test_supply_history.py index 7c34c74..00705be 100644 --- a/tests/test_plugins/test_supply_history.py +++ b/tests/test_plugins/test_supply_history.py @@ -8,6 +8,8 @@ purchasing decision. from datetime import datetime, timedelta, timezone +import pytest + from plugins.printers.services.supply_history import ( analyse, band, burn_rate, current_run, find_replacements, normalise, orderlist, soonest, @@ -113,28 +115,69 @@ def test_live_level_drives_the_countdown_not_the_stored_one(): assert result['daysleft'] == 1 # 20 / 20, not 40 / 20 -def test_a_nearly_empty_cartridge_reads_as_empty_not_as_weeks_away(): - """1% draining a tenth of a point a day computes to ten days. It is out.""" - result = analyse(series([2.0, 1.7, 1.4, 1.1]), currentlevel=1) +def test_a_nearly_empty_cartridge_keeps_its_real_countdown(): + """Only zero is empty. + + This used to floor everything at or below 5% to daysleft 0, so 1% draining + a tenth of a point a day was reported as gone. It has ten days in it, and + ten days is the only thing the row exists to say. Flooring it put the + cartridge beside ones that genuinely are empty with no way back to the + difference. + """ + # 3.0 down to 1.0 at a tenth of a point a day: a real, slow, measured drop. + slow = [round(3.0 - 0.1 * i, 2) for i in range(21)] + result = analyse(series(slow), currentlevel=1) + + assert result['daysleft'] == pytest.approx(10, abs=0.1) + + +def test_a_cartridge_with_hours_left_is_not_reported_as_empty(): + """The other half of the floor: int() truncated the division, so anything + under a full day became 0 and read as empty. A cartridge with six hours in + it still prints.""" + result = analyse(series([100, 75, 50, 25]), currentlevel=6) + + assert 0 < result['daysleft'] < 1 + assert round(result['daysleft'] * 24) == 6 + + +def test_an_actual_zero_is_still_empty(): + """The half that must not regress. Zero means spent, and spent is ordered + first.""" + result = analyse(series([2.0, 1.7, 1.4, 1.1]), currentlevel=0) assert result['daysleft'] == 0 def test_empty_sorts_ahead_of_a_fast_healthy_cartridge(): - empty = analyse(series([2.0, 1.7, 1.4, 1.1]), currentlevel=1) + empty = analyse(series([2.0, 1.7, 1.4, 1.1]), currentlevel=0) healthy = analyse(series([100, 80, 60, 40])) assert empty['daysleft'] < healthy['daysleft'] -def test_a_low_live_level_is_actionable_without_any_history(): +def test_an_empty_live_level_is_actionable_without_any_history(): """A printer new to Zabbix still reports it is out of toner.""" - result = analyse([], currentlevel=1) + result = analyse([], currentlevel=0) assert result['daysleft'] == 0 assert result['reason'] is None +def test_a_low_live_level_with_no_history_cannot_be_counted_down(): + """A rate needs two readings. At 1% with none, there is nothing to divide. + + Previously the 5% floor answered this by asserting daysleft 0, which was + right by accident and wrong in general - it said "empty" about a level + nobody had watched move. It reports no estimate and says why; the report + lists it under "No estimate yet" rather than on the order list. + """ + result = analyse([], currentlevel=1) + + assert result['daysleft'] is None + assert result['reason'] == 'no history' + + def test_no_history_and_a_healthy_level_still_says_no_history(): result = analyse([], currentlevel=80) diff --git a/tests/test_plugins/test_toner_forecast_report.py b/tests/test_plugins/test_toner_forecast_report.py index 0237040..55cadbb 100644 --- a/tests/test_plugins/test_toner_forecast_report.py +++ b/tests/test_plugins/test_toner_forecast_report.py @@ -16,9 +16,13 @@ from plugins.printers.models import ModelSupply, Printer LEVELS = { # itemid: (name, live level, history of daily readings) - '10': ('Black Cartridge', 1, [6, 5, 4, 3]), + # Black is a REAL zero, which is the only thing that means empty. Magenta is + # low but not zero and keeps its own countdown - under a day here, which the + # old int() truncation collapsed to 0 and reported as empty alongside black. + '10': ('Black Cartridge', 0, [6, 5, 4, 3]), '11': ('Cyan Cartridge', 40, [100, 90, 80, 70, 60, 50, 40]), '12': ('Yellow Cartridge', 92, [95, 94, 93, 92]), + '13': ('Magenta Cartridge', 3, [100, 76, 52, 28]), } START = 1780000000 @@ -111,7 +115,8 @@ def test_a_row_is_a_cartridge_not_a_printer(client, printer_with_parts): data = forecast(client) names = sorted(c['name'] for c in data['cartridges'] + data['unestimated']) - assert names == ['Black Cartridge', 'Cyan Cartridge', 'Yellow Cartridge'] + assert names == ['Black Cartridge', 'Cyan Cartridge', 'Magenta Cartridge', + 'Yellow Cartridge'] def test_each_cartridge_carries_its_own_countdown(client, printer_with_parts): @@ -120,18 +125,30 @@ def test_each_cartridge_carries_its_own_countdown(client, printer_with_parts): data = forecast(client) bylevel = {c['name']: c for c in data['cartridges']} - assert bylevel['Black Cartridge']['daysleft'] == 0 # live level 1% + assert bylevel['Black Cartridge']['daysleft'] == 0 # live level 0% assert bylevel['Cyan Cartridge']['daysleft'] == 4 # 40% at 10%/day assert len({c['daysleft'] for c in data['cartridges']}) > 1 +def test_a_low_cartridge_keeps_a_countdown_measured_in_hours(client, + printer_with_parts): + """3% at 24%/day is about three hours. It used to arrive as 0 - int() + truncated the division - so it was indistinguishable from black, which is + genuinely spent.""" + data = forecast(client) + magenta = {c['name']: c for c in data['cartridges']}['Magenta Cartridge'] + + assert 0 < magenta['daysleft'] < 1 + assert round(magenta['daysleft'] * 24) == 3 + + def test_cartridges_are_banded_by_urgency(client, printer_with_parts): data = forecast(client) byname = {c['name']: c['band'] for c in data['cartridges']} assert byname['Black Cartridge'] == 'empty' assert byname['Cyan Cartridge'] == 'soon' - assert data['summary']['bands']['empty'] == 1 + assert data['summary']['bands']['empty'] == 1, 'only the real zero' def test_the_order_list_names_the_part_and_the_quantity(client, printer_with_parts):