Fix what the last round of device fixes broke, and two dips it missed
Some checks failed
CI / backend (push) Failing after 7m15s
CI / naming (push) Failing after 7m14s
CI / migrations-mysql (push) Has been cancelled
CI / frontend (push) Has been cancelled

A review of d60ed60 and 8b9b936 found five things. Three were introduced by
those commits.

REFUSING A NAMED DEVICE MADE A MARKER PC CLAIM ITS OPERATION. _sync_partmarker
returned [] both for "not a marker PC" and for "a marker PC that linked
nothing", and the caller reads [] as the first - so a bay whose asset-id.txt
named something unresolvable fell through to the ordinary machine link, and for
a marker PC the machine number IS the operation. It took an active link to a
record that can only have one holder while several markers share it, and the
warning said "not linked". The previous behaviour minted a twin; this traded
that for a contested operation. None now means "not a marker PC" and is the only
answer that lets the machine link run; the response normalises it away so the
API shape is unchanged.

A DORMANT CHALLENGER WAS PROMOTED BY DELETING A FILE. Recording a challenger
dormant leaves a row that the next cycle finds as `reuse` and reactivated with
no incumbent check - so a second PC took a live device by its enrollment file
becoming unreadable. Both reuse branches re-check incumbency now, which is what
_sync_machine_link always did.

AN INCUMBENT UNDER ANOTHER COLLECTOR LABEL WAS INVISIBLE. Incumbency was queried
on our own label, but on a CMM the instrument IS the reported bay, so the
incumbent's link is the machine sync's row. A second PC naming that instrument
found no incumbent and linked actively: two live holders of one instrument, each
invisible to the other. Incumbency now counts any collector-owned label. A row
made BY HAND carries none of them and is still excluded - a person's link is not
the collector's to archive.

THE NETWORK FORM LOCKED OUT THE ROWS IT NEEDED TO FIX. Asset number is disabled
while editing, correctly, but the payload is built in script so the blank was
still sent - and the new server-side guard rejects it. A device with no asset
number could not be saved at all, and the field could not be typed into. It now
unlocks only for a record that loaded without one, with a hint saying why.

"2 in 4.0d" WAS THE LABEL LYING. Replacements are counted across the whole
history window; basisdays is only how long the current cartridge has been in.
Joining them with "in" claimed two changes inside four days - the exact shape
reported as unbelievable, except here the data was right. Now "2, this one 4.0d".

Two toner dips the same review found:

A MULTI-POLL OUTAGE STILL MINTED A PHANTOM SWAP. Only single readings were
dropped, so 90, 0, 0, 90 survived and 0 -> 90 scored as a change. Dips of any
length are handled now. One bad sample stays a candidate whatever the polling
cadence, because a reading is an instant; several consecutive low ones only
count as one outage when they are close together, since days at zero is a real
empty period. That time bound also separates an outage from a swap, ordinary
consumption, and a second swap, which have the same shape in levels alone.

THE DIP FILTER ATE REAL SWAPS OF NEARLY-FULL CARTRIDGES. Recovery was tested
with an absolute difference, so 95 then 5 then 100 read as a recovery because
100 and 95 are close, and the swap evidence was deleted. Toner only falls: a
recovery comes back at or BELOW where it left, a new cartridge comes back
higher.

The review also proved by reverting each feature that the previous tests did not
pin the median burn rate or the near-full rule - both passed with the bug
restored. Verified by the same method that all four toner behaviours now fail
when reverted, and the burst assertion is tight enough to tell 0.2 from 0.88.
This commit is contained in:
cproudlock
2026-08-21 08:49:38 -04:00
parent 0c0c7be439
commit 875fde9f48
8 changed files with 517 additions and 26 deletions

View File

@@ -211,6 +211,11 @@ def test_an_unknown_device_warns_and_links_nothing(client, db, rig,
# and the test passed because it only checked that PM-TYPO was not created.
assert Asset.query.filter_by(assetnumber='FMARK500-PARTMARKER').first() is None
assert _controlled('FMARK500', 'collector:partmarker') == []
# AND it must not fall through to claiming the OPERATION directly. A marker
# PC reports the operation as its machine number, so "link nothing" that
# let the ordinary machine link run traded a phantom marker for a contested
# operation - several marker PCs share one, and it holds a single link.
assert _controlled('FMARK500', 'collector:machine') == []
def test_a_device_of_the_wrong_type_is_refused(client, db, rig, collector_key):
@@ -222,6 +227,7 @@ def test_a_device_of_the_wrong_type_is_refused(client, db, rig, collector_key):
machinenumber='0617', deviceid='PLAIN-MACHINE')
assert resp.status_code in (200, 201)
assert _controlled('FMARK600', 'collector:partmarker') == []
assert _controlled('FMARK600', 'collector:machine') == []
assert Asset.query.filter_by(assetnumber='FMARK600-PARTMARKER').first() is None
warnings = ' '.join(resp.get_json()['data'].get('warnings', []))
assert 'PLAIN-MACHINE' in warnings
@@ -267,3 +273,54 @@ def test_deviceid_wins_when_both_arrive(client, db, rig, collector_key):
deviceid='MT-9003', measuringtoolid='MT-9004')
assert resp.status_code in (200, 201)
assert _controlled('KEYENCE300', 'collector:measuringtool') == ['MT-9003']
def test_a_dormant_challenger_is_not_promoted_when_its_file_disappears(
client, db, rig, collector_key):
"""Deleting asset-id.txt must not hand a device to the challenger.
The dormant row this feature creates is found as `reuse` on the next cycle.
Reactivating it unconditionally meant a second PC took a live device by
losing a file - no warning, two active holders.
"""
_asset(db, '0613')
_marker(db, 'PM-0613-A')
_report(client, collector_key, 'FMARK100', pctype='gea-shopfloor-partmarker',
machinenumber='0613', deviceid='PM-0613-A')
_report(client, collector_key, 'FMARK200', pctype='gea-shopfloor-partmarker',
machinenumber='0613', deviceid='PM-0613-A')
assert _controlled('FMARK200', 'collector:partmarker') == []
# Next cycle: the file is gone, so no deviceid is sent at all.
_report(client, collector_key, 'FMARK200', pctype='gea-shopfloor-partmarker',
machinenumber='0613')
assert _controlled('FMARK100', 'collector:partmarker') == ['PM-0613-A']
assert _controlled('FMARK200', 'collector:partmarker') == []
def test_an_incumbent_under_another_collector_label_is_still_seen(
client, db, rig, collector_key):
"""On a CMM the instrument IS the bay, so the incumbent's link is the
machine sync's row under collector:machine. Counting only our own label
left two live holders of one instrument, each invisible to the other.
"""
_tool(db, 'CMM4')
# A LIVE incumbent: it reported through the collector, so it has a computer
# row and a recent check-in. A bare asset would correctly be treated as
# yielded, since something that cannot report cannot be holding anything.
_report(client, collector_key, 'WCMM100',
pctype='gea-shopfloor-keyence', deviceid='CMM4')
# Relabel its link to the machine sync's origin, which is the shape a CMM
# really has: there the instrument IS the reported bay.
pc = Asset.query.filter(Asset.assetnumber.ilike('WCMM100')).first()
held = AssetRelationship.query.filter_by(sourceassetid=pc.assetid).first()
held.label = 'collector:machine'
db.session.commit()
resp = _report(client, collector_key, 'WCMM200',
pctype='gea-shopfloor-keyence', deviceid='CMM4')
assert _controlled('WCMM200', 'collector:measuringtool') == []
warnings = ' '.join(resp.get_json()['data'].get('warnings', []))
assert 'CMM4' in warnings

View File

@@ -319,7 +319,9 @@ def test_an_early_burst_does_not_dominate_the_rate_forever():
levels = [100, 90, 80] + [80 - i * 0.2 for i in range(1, 28)]
rate = burn_rate(normalise(series(levels)))
assert rate is not None
assert rate < 1.0, rate
# The old endpoint slope gives ~0.88 for this series, so a threshold of 1.0
# passed with the bug still in. The median gives ~0.2.
assert rate < 0.5, rate
detail = analyse(series(levels))
assert detail['rateunstable'] is True
@@ -362,3 +364,64 @@ def test_a_run_with_enough_days_still_estimates():
detail = analyse(series([84, 83, 82, 81, 80, 79]))
assert detail['burnrateperday'] == 1.0
assert detail['daysleft'] == 79
def test_a_multi_poll_outage_is_one_dip_not_a_cartridge_change():
"""A door open or a supply out of the machine spans several polls.
These items are often polled every few minutes, so an outage covers more
than one reading. Removing only SINGLE readings left the original failure
in place for any cartridge above NEW_CARTRIDGE_LEVEL: 0 -> 90 clears the
rise and lands near full, so it scored as a swap.
"""
points = normalise(series([90, 0, 0, 90, 88, 86], hours=5 / 60))
assert [level for _, level in points] == [90.0, 90.0, 88.0, 86.0]
assert find_replacements(points) == []
def test_a_long_absence_is_not_deleted_as_noise():
"""Days at zero is a real empty period, not a bad poll.
Only a BRIEF multi-reading dip is removed. Deleting a level that stayed
down for days would hide exactly the outage someone needs to see.
"""
points = normalise(series([90, 0, 0, 90, 88, 86]))
assert 0.0 in [level for _, level in points]
def test_a_swap_of_a_nearly_full_cartridge_is_not_eaten_by_the_dip_filter():
"""95, then 5, then 100 is a swap, not a dip that recovered.
Comparing the absolute difference treated it as a recovery, because 100 and
95 are close, and deleted the evidence: the run then spanned two cartridges
and the replacement went uncounted. Toner only falls, so a recovery comes
back at or BELOW where it left - a new cartridge comes back higher.
"""
points = normalise(series([98, 95, 5, 100, 96, 92]))
assert 5.0 in [level for _, level in points]
assert len(find_replacements(points)) == 1
def test_consumption_between_two_swaps_is_not_mistaken_for_a_dip():
"""Swap to full, print for days, swap again: same shape, different thing.
The readings between the swaps are far below both bounding levels, which is
what a dip looks like. What separates them is elapsed time.
"""
points = normalise(series([80, 60, 40, 20, 100, 80, 60, 95, 70]))
assert [level for _, level in points] == [80.0, 60.0, 40.0, 20.0,
100.0, 80.0, 60.0, 95.0, 70.0]
assert len(find_replacements(points)) == 2
def test_a_rise_that_stops_mid_range_is_not_a_cartridge_change():
"""Isolates the near-full rule from the dip filter.
50 -> 75 clears the rise and is NOT a recovering dip (75 comes back higher
than the 60 before it), so the dip filter leaves it alone. Only the rule
that a swap must LAND near full rejects it. A new cartridge does not read
75 percent.
"""
points = normalise(series([60, 50, 75, 73, 71]))
assert 50.0 in [level for _, level in points]
assert find_replacements(points) == []