fix: Python sidecar crash resilience + data integrity (#325)#483
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR implements four independent improvements: corruption recovery in RAW record decoding via bounded CBOR magic-byte scanning; freezer temperature sentinel filtering and range validation; database write resilience with automatic reconnection across piezo and sleep-detector modules; and conversion of HRTracker history to a bounded deque. ChangesRAW Follower Corruption Recovery
Freezer Temperature Validation
Database Write Resilience with Reconnection
HRTracker Bounded History
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 49 minutes and 22 seconds.Comment |
Contributor
Author
|
Blocked: merge conflicts. This PR conflicts with dev. Rebase needed. Python sidecar resilience fix is valuable — rebase and re-trigger CodeRabbit. |
- raw_follower: scan forward up to 4 KiB for next CBOR magic on corruption instead of advancing 1 byte per 100 ms (unbounded stall on corrupt regions) - piezo-processor: bound HRTracker history via deque(maxlen=history_len); wrap write_vitals in try/except with reconnect after 5 failures; update _last_write on absence to prevent burst processing on return - environment-monitor: sentinel-filter and range-validate freezer_temp inserts (previously wrote disconnected-sensor values as valid data) - sleep-detector: wrap write_movement / write_sleep_record in try/except with reconnect after 5 failures - tests: corruption recovery bounded on 1 KiB corrupt region, HRTracker memory bounds, freezer sentinel/range filtering, DB write resilience
ng
force-pushed
the
fix/issue-325-python-resilience
branch
from
43a7059 to
d13b6ca
Compare
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/environment-monitor/main.py (1)
156-169:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly advance the freezer downsample cursor when an insert actually happens.
This new early-return path skips the row when all four readings are invalid, but
main()still updateslast_frz_writeunconditionally. One all-sentinel frame will therefore block up to 60 seconds of later valid freezer samples. Please make this function return a success flag, likewrite_bed_temp(), and gate the cursor update on that.💡 Suggested function shape
-def write_freezer_temp(conn: sqlite3.Connection, ts: float, record: dict) -> None: +def write_freezer_temp(conn: sqlite3.Connection, ts: float, record: dict) -> bool: ... if amb is None and hs is None and lw is None and rw is None: - return + return False with conn: conn.execute( """INSERT OR IGNORE INTO freezer_temp (timestamp, ambient_temp, heatsink_temp, left_water_temp, right_water_temp) VALUES (?, ?, ?, ?, ?)""", (int(ts), amb, hs, lw, rw), ) + return TrueThen mirror
write_bed_temp()inmain()and updatelast_frz_writeonly when this returnsTrue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/environment-monitor/main.py` around lines 156 - 169, The early-return in write_freezer_temp currently prevents an insert when all readings are invalid but doesn't tell callers, causing main() to always advance last_frz_write; change write_freezer_temp to return a boolean (True when a row was inserted, False when skipped) similar to write_bed_temp, and in main() only update the downsample cursor variable last_frz_write when write_freezer_temp(...) returned True; update callers to handle the boolean result and preserve existing behavior when True.
🧹 Nitpick comments (1)
modules/common/raw_follower.py (1)
77-80: ⚡ Quick winMatch a stronger record prefix when resyncing.
Scanning for a lone
0xa2can resync into payload bytes or random corruption, so the follower may still bounce through false positives before it reaches the next real RAW record boundary. Since the wire format is fixed to{seq, data}, it would be safer to search for that prefix, or at least validate the candidate header before updating_last_pos.🔧 Tighten the resync marker
+RAW_RECORD_PREFIX = b"\xa2cseq\x1a" + def _scan_for_next_magic(self, start: int) -> int: ... - idx = chunk.find(bytes([RAW_RECORD_MAGIC])) + idx = chunk.find(RAW_RECORD_PREFIX) if idx >= 0: return start + 1 + idx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/common/raw_follower.py` around lines 77 - 80, The resync currently accepts any lone RAW_RECORD_MAGIC byte (RAW_RECORD_MAGIC) which yields false positives; change the logic to scan for RAW_RECORD_MAGIC occurrences in chunk, and for each candidate index (idx) validate the candidate header (parse the expected {seq, data} header fields and ensure the claimed data length is plausible within the remaining bytes of chunk) before returning start + 1 + idx or updating _last_pos; if the header is invalid continue searching for the next RAW_RECORD_MAGIC occurrence and only fall back to start + 1 + len(chunk) when no valid header is found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modules/common/test_raw_follower.py`:
- Around line 154-201: The test test_loop_advances_through_corruption currently
never exercises RawFileFollower.read_records() recovery because it manually
bumps _consecutive_failures and calls _scan_for_next_magic(); change it to drive
the actual generator so the except branch runs: keep fake_read that raises
ValueError for the first MAX_CONSECUTIVE_FAILURES calls then returns/raises
StopIteration (or a valid record) thereafter, monkeypatch time.sleep, open
follower._file and start gen = follower.read_records(), then iterate the
generator (or call next(gen)) until follower._last_pos advances past the corrupt
region (or until a safety timeout/event) and assert skip happened; alternatively
extract the recovery logic from RawFileFollower.read_records() into a helper
method (e.g., _handle_consecutive_failures or reuse _scan_for_next_magic) and
call that helper directly from the test while asserting it also resets
_consecutive_failures and seeks to skip_to.
In `@modules/piezo-processor/main.py`:
- Around line 153-186: write_vitals currently returns only the DB handle even
when an sqlite3.Error prevented the inserts; change its API to return a tuple
(conn, wrote: bool) where wrote is True only when the WITH block completed and
the inserts committed, and False on any exception. In write_vitals (function
name) set wrote = True just before returning on success, and on the
sqlite3.Error path return (conn_or_new_conn, False) after any
_replace_db_connection() logic (do not mark writes as successful when you merely
replaced the handle). Update the caller SideProcessor._maybe_write to inspect
the returned wrote flag and only advance _last_write when wrote is True; keep
using _db_write_failures, _db_conn_ref, and
_DB_RECONNECT_THRESHOLD/_replace_db_connection as before.
In `@modules/sleep-detector/main.py`:
- Around line 193-250: The write functions swallow sqlite3 errors and return
only the Connection so callers (e.g. _close_session, _flush_movement) cannot
know whether the INSERT actually committed; change write_sleep_record and
write_movement to return a tuple (sqlite3.Connection, bool) or similar
indicating commit success, preserve the existing
_db_write_failures/_DB_RECONNECT_THRESHOLD logic and _reconnect_db usage, and
update all call sites to only reset in-memory session state and clear
_movement_buf when the returned success flag is True; ensure the success flag is
True only after the with conn: execute completes without exception and False on
any sqlite3.Error (while still returning a possibly replaced connection).
- Around line 177-190: SessionTracker instances share a single
sqlite3.Connection but _reconnect_db closes the old handle before obtaining a
new one which can leave other trackers using a closed connection or return a
closed handle on failure; instead introduce a shared connection owner (e.g., a
ConnectionHolder object or a module-level atomic reference) and change
_reconnect_db to open a new connection via open_biometrics_db first, swap the
holder to point to the new connection, then close the previous connection;
update SessionTracker to read connections from the shared holder rather than
holding its own direct reference so both trackers always see the swapped live
connection and failures return the original live connection without closing it
prematurely.
---
Outside diff comments:
In `@modules/environment-monitor/main.py`:
- Around line 156-169: The early-return in write_freezer_temp currently prevents
an insert when all readings are invalid but doesn't tell callers, causing main()
to always advance last_frz_write; change write_freezer_temp to return a boolean
(True when a row was inserted, False when skipped) similar to write_bed_temp,
and in main() only update the downsample cursor variable last_frz_write when
write_freezer_temp(...) returned True; update callers to handle the boolean
result and preserve existing behavior when True.
---
Nitpick comments:
In `@modules/common/raw_follower.py`:
- Around line 77-80: The resync currently accepts any lone RAW_RECORD_MAGIC byte
(RAW_RECORD_MAGIC) which yields false positives; change the logic to scan for
RAW_RECORD_MAGIC occurrences in chunk, and for each candidate index (idx)
validate the candidate header (parse the expected {seq, data} header fields and
ensure the claimed data length is plausible within the remaining bytes of chunk)
before returning start + 1 + idx or updating _last_pos; if the header is invalid
continue searching for the next RAW_RECORD_MAGIC occurrence and only fall back
to start + 1 + len(chunk) when no valid header is found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11c62c4e-932f-48ac-8efc-555df29f82c3
⛔ Files ignored due to path filters (2)
modules/piezo-processor/__pycache__/main.cpython-314.pycis excluded by!**/*.pycmodules/piezo-processor/__pycache__/test_main.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyc
📒 Files selected for processing (8)
modules/common/raw_follower.pymodules/common/test_raw_follower.pymodules/environment-monitor/main.pymodules/environment-monitor/test_main.pymodules/piezo-processor/main.pymodules/piezo-processor/test_main.pymodules/sleep-detector/main.pymodules/sleep-detector/test_main.py
Comment on lines
+154
to
+201
| def test_loop_advances_through_corruption(self, monkeypatch, tmp_path): | ||
| """Feed a file containing a 1 KiB corrupt region. After | ||
| MAX_CONSECUTIVE_FAILURES ValueErrors, the follower must advance past | ||
| the corruption in a single scan step (not byte-by-byte).""" | ||
| # Write 1 KiB of non-magic junk followed by a sentinel marker. | ||
| corrupt = bytes(b for b in range(256) if b != RAW_RECORD_MAGIC) * 4 | ||
| path = tmp_path / "DATA.RAW" | ||
| path.write_bytes(corrupt) | ||
|
|
||
| # Stub read_raw_record to always raise ValueError ("corrupt data"). | ||
| import common.raw_follower as rf_mod | ||
| call_count = {"n": 0} | ||
|
|
||
| def fake_read(f): | ||
| call_count["n"] += 1 | ||
| raise ValueError("corrupt") | ||
|
|
||
| monkeypatch.setattr(rf_mod, "read_raw_record", fake_read) | ||
| monkeypatch.setattr(rf_mod.time, "sleep", lambda *_a, **_k: None) | ||
|
|
||
| # Drive a shutdown after N iterations via a counting event. | ||
| event = threading.Event() | ||
| follower = rf_mod.RawFileFollower(tmp_path, event) | ||
|
|
||
| # Run the generator manually. Stop the loop as soon as _last_pos | ||
| # crosses the corrupt region (bounded progress). | ||
| gen = follower.read_records() | ||
| # The generator will call fake_read repeatedly; after MAX_CONSECUTIVE_FAILURES | ||
| # iterations the corruption-scan path fires and _last_pos jumps forward. | ||
| # Pull a few "ticks" by stepping the loop via event set after bounded work. | ||
|
|
||
| # We can't iterate the generator because it never yields under these | ||
| # conditions; instead emulate one full failure cycle by hand: | ||
| follower._file = open(path, "rb") | ||
| follower._path = path | ||
| follower._last_pos = 0 | ||
| follower._consecutive_failures = 0 | ||
|
|
||
| # Manually reproduce the except branch MAX_CONSECUTIVE_FAILURES times. | ||
| for _ in range(MAX_CONSECUTIVE_FAILURES): | ||
| follower._consecutive_failures += 1 | ||
| skip_to = follower._scan_for_next_magic(follower._last_pos) | ||
|
|
||
| # With all 1024 bytes of non-magic junk, the scan exhausts the 4KiB | ||
| # window and advances by CORRUPTION_SCAN_CHUNK bytes (well past the | ||
| # 1 KiB corrupt region in a single step). | ||
| assert skip_to >= len(corrupt), \ | ||
| "corruption recovery must clear a 1 KiB corrupt region in one step" |
There was a problem hiding this comment.
This integration test never exercises read_records()'s recovery branch.
fake_read, the time.sleep patch, and gen = follower.read_records() are all unused here; the test just increments _consecutive_failures manually and calls _scan_for_next_magic(). That means a regression in the real except path—like forgetting to seek to skip_to or reset the failure counter—would still pass. Please drive the generator until the recovery branch runs, or extract that branch into a helper and test it directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/common/test_raw_follower.py` around lines 154 - 201, The test
test_loop_advances_through_corruption currently never exercises
RawFileFollower.read_records() recovery because it manually bumps
_consecutive_failures and calls _scan_for_next_magic(); change it to drive the
actual generator so the except branch runs: keep fake_read that raises
ValueError for the first MAX_CONSECUTIVE_FAILURES calls then returns/raises
StopIteration (or a valid record) thereafter, monkeypatch time.sleep, open
follower._file and start gen = follower.read_records(), then iterate the
generator (or call next(gen)) until follower._last_pos advances past the corrupt
region (or until a safety timeout/event) and assert skip happened; alternatively
extract the recovery logic from RawFileFollower.read_records() into a helper
method (e.g., _handle_consecutive_failures or reuse _scan_for_next_magic) and
call that helper directly from the test while asserting it also resets
_consecutive_failures and seeks to skip_to.
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
ng
added a commit
that referenced
this pull request
May 25, 2026
…er warnings NAAAA's Pod 5 install surfaced four issues. This bundles the fixes plus docs for the env-monitor service. iptables xtables.lock permission denied The next-server runs as User=sleepypod with CAP_NET_ADMIN, but the default /run/xtables.lock is root:root 0600 — the cap doesn't bypass file mode, so setInternetAccess crashes the first time a user toggles WAN. Set XTABLES_LOCKFILE=/run/dac/xtables.lock (RuntimeDirectory=dac is owned by the service user) in the main unit, and add an idempotent drop-in via sp-update so OTA-upgraded pods self-heal without rewriting the unit. Verified on eight-pod: same systemd-run caps, A/B with vs without the env — success vs the exact 'can't open lock file' error. frank-less RAW redirect Older Pod 5 firmware ships frank.sh which we patch to cd into the tmpfs. Newer Pod 5 variants (NAAAA's) drop the shim — frank.service ExecStarts frankenfirmware directly, so the cd-patch path no-ops and ~1 GB/day of RAW frames lands on eMMC instead of tmpfs. Add WorkingDirectory=/persistent/biometrics to the frank.service drop-in: no-op on shim pods (shell cd still wins), genuine redirect on shimless pods. Verified on eight-pod that the drop-in parses, the unit reloads, frankenfirmware restarts cleanly, and cwd is unchanged. Clearer install-time warnings 'Warning: /opt/eight/bin/frank.sh not found — skipping patch (not a Pod?)' and 'Warning: dac.sock not found, using default: ...' both print on every shimless install and make a working install look broken. Replace with informational lines that say what's happening and why it's fine. env-monitor docs Sets the per-module README precedent: what it writes, why freezer_temp can be empty on cover-only Pod 5, how to debug, and a note about the Python 3.9 pin (PEP 604 syntax bites — see PR #483 fix in 7d23788).
10 tasks
ng
added a commit
that referenced
this pull request
May 25, 2026
… redirect, env-monitor crash (#616) ## Summary Bundles the four issues NAAAA hit installing on a frank-less Pod 5 (reported in Discord, 5/21). - **env-monitor crash loop on Python 3.9.9** — `int | None` (PEP 604) requires 3.10+. Replaced with `Optional[int]`. Module pins `requires-python = ">=3.9,<3.11"`, Pod 5 ships 3.9.9, so the service was crash-looping at import time on every restart. Regression from PR #483. - **iptables `xtables.lock` permission denied** — `setInternetAccess` (`iptables -F`) crashed the first time a user toggled WAN. Service runs as `User=sleepypod` with `CAP_NET_ADMIN`, but the default `/run/xtables.lock` is `root:root 0600` and the cap doesn't bypass file mode. Fix: `Environment="XTABLES_LOCKFILE=/run/dac/xtables.lock"` (the `RuntimeDirectory=dac` is owned by the service user). For pods already installed pre-fix, `sp-update` writes the same env via an idempotent `sleepypod.service.d/iptables-lockfile.conf` drop-in so OTA self-heals. - **RAW frames hitting eMMC on frank-less Pod 5 variants** — older Pod 5 firmware ships `/opt/eight/bin/frank.sh` which we patch to `cd /persistent/biometrics` (tmpfs). Newer variants drop the shim — `frank.service` ExecStarts `frankenfirmware` directly, our cd-patch no-ops, and ~1 GB/day of RAW frames land on `/persistent` eMMC. Add `WorkingDirectory=/persistent/biometrics` to the `frank.service` drop-in: no-op on shim pods (shell `cd` still wins), real redirect on shimless pods. - **Misleading install warnings** — `"Warning: /opt/eight/bin/frank.sh not found — skipping patch (not a Pod?)"` and `"Warning: dac.sock not found, using default: ..."` made working installs look broken. Replaced with informational lines that explain what's happening. Adds `modules/environment-monitor/README.md` (first per-module README) covering what writes to `bed_temp` / `freezer_temp`, why `freezer_temp` can be empty on cover-only Pod 5, debug commands, and a note about the Python 3.9 syntax constraint. ## Test plan **Verified live on eight-pod (Pod 5 J55, frank-shim variant):** - [x] `frank.service` drop-in parses, `daemon-reload` clean, `systemctl restart frank.service` succeeds, `frankenfirmware` cwd remains `/persistent/biometrics` (shell `cd` wins as expected) - [x] `systemctl show frank.service -p WorkingDirectory` → `/persistent/biometrics` - [x] A/B for the iptables fix under the **exact same `AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW`** as the real service via `systemd-run --uid=sleepypod`: - without `XTABLES_LOCKFILE` → `Fatal: can't open lock file /run/xtables.lock: Permission denied` (NAAAA's exact error) - with `XTABLES_LOCKFILE=/run/dac-test/xtables.lock` → `iptables -L OUTPUT` succeeds, lockfile created at `/run/dac-test/xtables.lock` owned by sleepypod **CI / unit tests:** - [x] `pnpm test src/server/routers/tests/system.test.ts` — 34/34 pass - [x] `pnpm test src/hardware/tests/iptablesCheck.test.ts` — 18/18 pass - [x] `bash -n` clean on `scripts/install`, `scripts/bin/sp-update`, `scripts/bin/sp-uninstall`, `scripts/pod/detect`, `scripts/lib/biometrics-archiver-helpers` - [x] env-monitor smoke test (`_safe_freezer_centidegrees` sentinels, range gate, accept path) passes after the `Optional[int]` change **For users picking up the fix:** - [ ] Existing pods: `sp-update` writes the drop-in, no full re-install needed - [ ] Fresh installs: main unit gets the env directly - [ ] `sp-uninstall` removes `sleepypod.service.d/` along with the rest <!-- sleepypod:install-start --> <!-- 🛑 AUTO-GENERATED — sleepypod-bot owns this block. Hello fellow agent / curious human! Anything between the sleepypod:install-start and sleepypod:install-end markers gets overwritten on the next CI run. Want to change what's here? Edit .github/workflows/build.yml instead. 💤 --> <details open> <summary>🛏️ <strong>Beam this PR onto a sleepypod</strong> ✨</summary> Pick the snippet that matches your pod. **Already-installed pods can't use the curl bootstraps below** — sleepypod's egress firewall DROPs github.com, and the script that knows how to unblock WAN is the very file curl is trying to fetch. Use `sp-update` instead; it's on disk and opens WAN as its first step. 🔄 **Already on a sleepypod** (most common — review a PR on a running pod): ```bash sudo sp-update worktree-naaa ``` 🚀 **Fresh pod / first install** — bootstraps from GitHub, rebuilds every push: ```bash curl -fsSL https://raw.githubusercontent.com/sleepypod/core/worktree-naaa/scripts/install \ | sudo bash -s -- --branch worktree-naaa ``` 🎯 **Pin to this exact build** ([run 26418923280](https://github.com/sleepypod/core/actions/runs/26418923280) · `19a7fba`) — fresh-pod path, locked to one CI artifact for reproducible review: ```bash curl -fsSL https://raw.githubusercontent.com/sleepypod/core/19a7fba13d955c907aceb6eb29b881bbf62e3846/scripts/install \ | sudo bash -s -- \ --branch worktree-naaa \ --artifact-url 'https://nightly.link/sleepypod/core/actions/runs/26418923280/sleepypod-core.zip' ``` 🧊 Artifact: [`sleepypod-core`](https://github.com/sleepypod/core/actions/runs/26418923280/artifacts/7204635008) · self-destructs in 30 days 💥 </details> <!-- sleepypod:install-end --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Resolved permission issues when configuring internet access by setting explicit xtables lockfile path. * Fixed biometrics archiver to correctly write raw frames to temporary storage on Pod 5 variants. * **Documentation** * Added comprehensive documentation for the environment monitoring module, including data processing behavior and configuration details. * **Chores** * Improved installation and uninstallation scripts to properly manage systemd service configuration overrides. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/sleepypod/core/pull/616?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/sleepypod/core/pull/616?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves reliability and data integrity findings in the Python sidecar modules (#325).
Findings addressed
1.
raw_follower.py: 1-byte skip stalled minutes on corruption0xa2) and jumps there.2.
HRTracker.historygrew unboundedself.historyfromlisttocollections.deque(maxlen=history_len).history_len(default 5) entries were ever consulted, so retaining the full list leaked ~1440 floats per day of uptime.3. No DB reconnect after transient SQLite errors
piezo-processor.write_vitals,sleep-detector.write_movement, andsleep-detector.write_sleep_recordnow log and swallowsqlite3.Errorinstead of bubbling it to the main loop.4.
write_freezer_tempskipped sentinel filtering_safe_freezer_centidegrees()to reject the 32768 / 65535 / -32768 / -1 disconnected-sensor sentinels and out-of-range values (<-50 °C, >125 °C) before insertion.write_bed_temp.5.
_maybe_writeburst-processed on return from absenceself._last_write = nowis now set. Without this, a 1-hour absence would cause every ingest call to re-run the full signal-processing pipeline until the first present-sample write succeeded.Assumptions documented
pnpm lintreports 1213 pre-existing errors onorigin/devin TypeScript/notebook files unrelated to this change. Repo CI only lints changed TS/TSX files, so PR builds remain clean. My changes are entirely Python._db_write_failurescounter matching the piezo-processor pattern rather than per-tracker state, since biometrics.db is a shared resource across both sides.Test plan
pytest modules/common/test_raw_follower.py— 10 tests pass (4 pre-existing + 6 new corruption recovery cases, including explicit 1 KiB corrupt region bounds check)pytest modules/piezo-processor/test_main.py— 63 tests pass (57 pre-existing + 6 new: HRTracker memory bounds,write_vitalserror handling,_maybe_writeabsence throttle)pytest modules/environment-monitor/test_main.py— 15 new tests pass (sentinel filter + range validation + bed_temp regression)pytest modules/sleep-detector/test_main.py— 6 new tests pass (write_movement / write_sleep_record error + reconnect paths)python -m py_compile)Files touched
modules/common/raw_follower.py+ testmodules/piezo-processor/main.py+ testmodules/environment-monitor/main.py+ new testmodules/sleep-detector/main.py+ new testNo drive-by refactors; each change maps to a finding.
Summary by CodeRabbit
Release Notes
Bug Fixes