Describe the bug
{{ restock.previous_price }} returns the first-ever price in the watch history, not the price at the previous check. It is only correct while a watch has exactly two snapshots, which is why the existing test passes.
Watch.extra_notification_token_values() in changedetectionio/processors/restock_diff/__init__.py:100-103 sorts the history keys oldest-first, reverses them to newest-first, and then reads index [-1]:
sorted_keys = sorted(list(history), key=lambda x: int(x))
sorted_keys.reverse()
price_str = self.get_history_snapshot(timestamp=sorted_keys[-1])
After .reverse(), [-1] is the oldest snapshot, so the .reverse() has no effect on the result. It appears the index was meant to be [1] (the second-newest, i.e. the previous check), since worker.py:537 saves the new snapshot before worker.py:582 sends the notification, so the newest key is the current check.
The practical effect is that on a watch checked a few times, a price-change notification reports a stale price that may be months old, rather than the value requested in #3987 ("the price value just before it is set with the new price").
Version
0.55.8 (master, commit 5eb2638)
How did you install?
From source, running in Docker.
To Reproduce
This is in the notification token layer rather than a specific website, so here is a standalone repro against the real Watch model (no network, no scraping). Save as repro.py in the repo root and run python repro.py:
import tempfile, uuid as uuid_builder
from changedetectionio.processors.restock_diff import Watch
def build(prices):
w = Watch(datastore_path=tempfile.mkdtemp(),
__datastore={'settings': {'application': {}}, 'watching': {}},
default={'url': 'http://example.com'})
w.ensure_data_dir_exists()
for i, p in enumerate(prices):
w.save_history_blob(contents=f"Product Title\nPrice: {p}\nIn stock\n",
timestamp=1700000000 + i * 3600,
snapshot_id=str(uuid_builder.uuid4()))
return w
for prices in ([960.45, 1950.45], [960.45, 1950.45, 2500.00], [960.45, 1950.45, 2500.00, 3100.00]):
got = build(prices).extra_notification_token_values()['restock']['previous_price']
print(f"history={prices} current={prices[-1]} expected_previous={prices[-2]} got={got}")
Output on 5eb2638:
history=[960.45, 1950.45] current=1950.45 expected_previous=960.45 got=960.45
history=[960.45, 1950.45, 2500.0] current=2500.0 expected_previous=1950.45 got=960.45
history=[960.45, 1950.45, 2500.0, 3100.0] current=3100.0 expected_previous=2500.0 got=960.45
The two-snapshot case is right only by coincidence: with two entries the oldest snapshot is the previous check. From the third check onward the token is stuck on the first price ever recorded.
Expected behavior
With history 960.45 -> 1950.45 -> 2500.00, a notification for the change to 2500.00 should report previous price 1950.45. It reports 960.45.
Additional context
Changing sorted_keys[-1] to sorted_keys[1] makes all three cases above correct and leaves the two-snapshot case byte-identical, so the common path does not change behavior. I have not sent that as a PR because I have two PRs open here already (#4256, #4259) and did not want to stack a third, and because you may prefer a different shape (for example dropping the now-redundant .reverse() and indexing [-2] on the ascending list). Happy to send a PR with a regression test if useful, just say which shape you want.
One more thing for whoever picks this up: changedetectionio/tests/test_restock_itemprop.py:405 asserts previous price 960.45 and passes only because that test creates exactly two snapshots (960.45 then 1950.45), which is the one case where the index coincides. Extending it with a third price change looks like the natural regression test. I have not run that test myself (it needs the live server fixture), so that part is from reading it rather than executing it; the standalone repro above is what I actually ran.
AI assistance was used to help write this report; I reproduced and verified it as described above.
Describe the bug
{{ restock.previous_price }}returns the first-ever price in the watch history, not the price at the previous check. It is only correct while a watch has exactly two snapshots, which is why the existing test passes.Watch.extra_notification_token_values()inchangedetectionio/processors/restock_diff/__init__.py:100-103sorts the history keys oldest-first, reverses them to newest-first, and then reads index[-1]:After
.reverse(),[-1]is the oldest snapshot, so the.reverse()has no effect on the result. It appears the index was meant to be[1](the second-newest, i.e. the previous check), sinceworker.py:537saves the new snapshot beforeworker.py:582sends the notification, so the newest key is the current check.The practical effect is that on a watch checked a few times, a price-change notification reports a stale price that may be months old, rather than the value requested in #3987 ("the
pricevalue just before it is set with the new price").Version
0.55.8 (master, commit 5eb2638)
How did you install?
From source, running in Docker.
To Reproduce
This is in the notification token layer rather than a specific website, so here is a standalone repro against the real
Watchmodel (no network, no scraping). Save asrepro.pyin the repo root and runpython repro.py:Output on 5eb2638:
The two-snapshot case is right only by coincidence: with two entries the oldest snapshot is the previous check. From the third check onward the token is stuck on the first price ever recorded.
Expected behavior
With history
960.45 -> 1950.45 -> 2500.00, a notification for the change to 2500.00 should reportprevious price 1950.45. It reports960.45.Additional context
Changing
sorted_keys[-1]tosorted_keys[1]makes all three cases above correct and leaves the two-snapshot case byte-identical, so the common path does not change behavior. I have not sent that as a PR because I have two PRs open here already (#4256, #4259) and did not want to stack a third, and because you may prefer a different shape (for example dropping the now-redundant.reverse()and indexing[-2]on the ascending list). Happy to send a PR with a regression test if useful, just say which shape you want.One more thing for whoever picks this up:
changedetectionio/tests/test_restock_itemprop.py:405assertsprevious price 960.45and passes only because that test creates exactly two snapshots (960.45 then 1950.45), which is the one case where the index coincides. Extending it with a third price change looks like the natural regression test. I have not run that test myself (it needs the live server fixture), so that part is from reading it rather than executing it; the standalone repro above is what I actually ran.AI assistance was used to help write this report; I reproduced and verified it as described above.