2 Commits

Include renames

Author	SHA1	Message	Date
ChuckandClaude Opus 5	154525beb8	fix(background): release the payload after every callback, not after each one (#509) ... Callers that join an in-flight fetch share one FetchResult. #499 released the payload inside the delivery loop, so the first callback got the data and every joiner got `result.data is None`. That is not a quiet degradation. Consumers read `result.data.get('events')`, so they raise AttributeError -- which the delivery loop catches and logs. The entire failure surfaced as one line: ERROR - src.background_data_service - Error in callback for request nhl_2026_...: 'NoneType' object has no attribute 'get' and a manager that silently never received its schedule. Seen on hardware: NHLRecentManager logs "Background fetch completed for 2026: 1000 events" and the very next line is the error, from NHLUpcomingManager's callback on the same request -- which had already logged "No events found in shared data." Deduplication is the normal case, not a corner. A sport's recent, upcoming and live managers all want the same season schedule, so the second and third are joiners on almost every cycle. _release_payload's own docstring said "once A callback has been handed the data", singular, which is the assumption that broke: the loop above it was written for many, and says so. Moved after the loop, and guarded on `callbacks` being non-empty. The guard matters: a request submitted without a callback must keep its payload, because polling get_result() is then the only way to collect it. The per-delivery release got that right by accident -- an empty list never entered the loop body -- and the existing test for it caught the omission. test_background_payload_release.py gains TestJoinersAllGetTheData: two submitters on one in-flight cache_key, asserting both are handed a populated payload, plus that the memory fix still happens once they have all had it. test_background_fetch_dedupe.py already proved the joiner's callback FIRES; it never checked what the callback received, which is the gap that let this through. Verified the new test bites: restoring the release inside the loop fails it with "'second' was handed a released payload". Full suite 3716 passed, 6 skipped. Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-01 09:54:04 -04:00
ChuckandClaude Opus 5	333fd17d28	fix(memory): release fetched payloads once they have been delivered (#499) ... * fix(memory): release fetched payloads once they have been delivered BackgroundDataService kept the fetched body on the FetchResult it filed in completed_requests, which is swept hourly and capped at 500 entries by count. For a status record that costs nothing; for a season schedule it costs a tenth of the board. Measured on a 1GB Pi 3B+ with a 1-second RSS profile: the display process sat at 404MB after plugin load, then stepped +21MB when NFL fetched its season and +90MB when NCAA football fetched 946 games for 2026 -- and stayed at 494MB. Not a leak; a staircase that never came down. When a later fetch landed while headroom was low, available memory reached ~70MB, fork() began failing, and the board stopped being able to start a process at all: sshd accepted connections and closed them before its banner, systemd could not respawn the display, and the panel went dark while the kernel carried on answering pings. The cache-hit path was the worse of the two. It runs once per update interval per sport, mints a fresh request_id each time, and files whatever the cache returned. The memory tier is capped at 150 entries on a 1GB board, so a miss re-parses the payload from disk into a genuinely new object -- separate copies accumulating toward the 500-entry cap, not shared references. Releasing is safe: the payload is written to the cache under the request's cache_key before the result is built, the callback is handed the object directly, and consumers read it back from the cache afterwards (the plugins' callbacks use it only in passing, to log a count, before reading the cache). Nothing is lost -- it moves from RAM to the disk cache that was already holding it. Requests submitted without a callback keep their payload, since polling get_result() is then the only way to collect it. That keeps the existing contract, and the existing tests covering it, intact. Not addressed here: max_workers=3 allows three concurrent fetches, so three large parses can peak at once, and there is no in-flight dedupe by cache_key -- a second submit for a key already being fetched starts a second fetch. Both bound the transient peak rather than what stays resident, and both are behaviour changes worth their own review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(memory): file the cache-hit result before running its callback Restores the original ordering. Releasing the payload after the callback meant filing the result after it too, so a callback that queried get_result() or is_request_complete() for its own request would not have found it -- a behaviour change unrelated to the memory fix. The dict holds a reference to the same object, so releasing after filing still clears the payload. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: wait for the payload release, not just the filing The callback test waited on is_request_complete(), which goes true as soon as the worker files the result in completed_requests. The worker then runs the cleanup pass, then the callback, then releases the payload. Both of the test's assertions therefore raced the worker: `seen` is populated by the callback, and `data is None` only after the release that follows it. It passes today because a one-line callback usually finishes inside the 20ms poll interval. Confirmed by making the callback sleep 0.4s: _wait() returns with seen == {} and the payload still resident. _wait_for_release() polls for the released payload instead. Release happens strictly after the callback returns, so a released payload also means the callback has finished and one wait covers both assertions. Verified against the same 0.4s callback. _wait() stays for the other three fetch-path tests, which assert only what is already true when the result is filed -- the success flag, the error, and the cache write that happened during the fetch itself. Its docstring now says so, so the next reader picks the right one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-25 08:32:08 -04:00