fix(hardware): stop pump stall guard false-tripping on firmware-commanded stops#663
Merged
Conversation
📝 WalkthroughWalkthrough
ChangesPump stall suppression
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DeviceStatus
participant DeviceStateSync
participant pumpStallGuard
DeviceStatus->>DeviceStateSync: provide firmware status and priming state
DeviceStateSync->>DeviceStateSync: cache stop signals
DeviceStateSync->>pumpStallGuard: pass RPM, duty, and expectedActive
pumpStallGuard-->>DeviceStateSync: record stall frame result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/hardware/tests/deviceStateSync.stallSuppression.test.ts (1)
162-165: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd test for
duty > 0combined with session-end graceThis test passes only because
lastSideStatusis null (nosync()called). It doesn't cover the interaction betweenduty > 0and the fallback suppression checks. A test that callssync()with a near-end session and then sendsduty > 0withrpm: 0would expose the major issue inisExpectedPumpStopwhere session-end grace suppresses despite the pump being actively driven.🧪 Suggested additional test case
it('does not suppress duty > 0 near session end (stalled pump still driven)', async () => { await sync.sync(status({ targetLevel: 5, heatingDuration: 60 })) sync.recordFlowData(frame({ rpm: 0, duty: 65 })) expect((await lastGuardInput('left'))?.expectedActive).toBe(true) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hardware/tests/deviceStateSync.stallSuppression.test.ts` around lines 162 - 165, Add a regression test alongside the existing duty-only case that first calls sync.sync with a near-end session status, then records a frame with rpm 0 and duty greater than 0, and asserts left.expectedActive remains true. Use the existing status, frame, and lastGuardInput helpers to cover the interaction with session-end grace.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0022-pump-stall-safety.md`:
- Around line 360-361: Update the ADR addendum’s session countdown reference
from “heatTime” to “heatingDuration” so it matches the implementation and
DeviceStatus type, while preserving the existing 90-second condition and
surrounding wording.
In `@src/hardware/deviceStateSync.ts`:
- Around line 271-294: Update isExpectedPumpStop so any present duty value
greater than zero immediately returns false, preserving stall detection; only
duty === 0 should return true and only duty === null should proceed to priming,
targetLevel, and session-end fallback checks. Keep the existing fallback
behavior unchanged for absent duty values.
---
Nitpick comments:
In `@src/hardware/tests/deviceStateSync.stallSuppression.test.ts`:
- Around line 162-165: Add a regression test alongside the existing duty-only
case that first calls sync.sync with a near-end session status, then records a
frame with rpm 0 and duty greater than 0, and asserts left.expectedActive
remains true. Use the existing status, frame, and lastGuardInput helpers to
cover the interaction with session-end grace.
🪄 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: 11ac3caa-72e2-46c6-baf6-8dc7762c1527
📒 Files selected for processing (3)
docs/adr/0022-pump-stall-safety.mdsrc/hardware/deviceStateSync.tssrc/hardware/tests/deviceStateSync.stallSuppression.test.ts
ng
added a commit
that referenced
this pull request
Jul 12, 2026
…ilot cap frames, Node 24) (#664) ## Release summary 50 commits from `dev`. Highlights: ### Fixes - **Pump stall guard false trips (#663)** — the guard no longer trips on firmware-commanded pump stops (session duration expiry, daily prime end). Field-reported: daily bilateral `power_off` stall alerts on a healthy Pod 4. Pump duty is now authoritative when present; priming, firmware target level, and a bounded session-end countdown cover pods that omit it. - **Biometrics archive pruner SIGPIPE (#640)** — pruner no longer dies mid-run, letting `/persistent` fill past target. - **piezoStream file-tailing race (#652)** — CI flake eliminated. - **RAW streaming (#608)** — skip SEQNO, prefer biometrics RAW dir. - **Install (#662)** — service user granted journal read so the log viewer works on all pods. - **2026-07-01 review batches 1–4** — ~20 low/medium-severity fixes across hardware, scheduler, automation, homekit, server, UI, db, and export paths (optimistic UI state, sideLock mutex on globalThis, RAW-follower rotation survival, DAC command/response realignment, archive-export unwedge, timezone propagation, per-side pump movement gate, and more). ### Features - **Autopilot cap-frame replay (#645, #642)** — persist downsampled capacitive frames; backtests consume them for zone reports; legible backtest chart with editable number fields. ### Chores - Node 24 across CI and pod runtime (#416, #650), TypeScript 6 (#315), dependency updates. ## Test plan - CI green on `dev` (lint, typecheck, unit tests, build) — every commit landed through a PR with its own checks. - Post-deploy on a pod: run `sp-update`, verify `.git-info` shows the new commit; confirm biometrics writing, `/debug` console clean, and no `pump_alerts` rows after a session-duration expiry and a scheduled prime. **Merge with a merge commit (not squash)** — preserves semver commit messages for the release tooling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- 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 dev ``` 🚀 **Fresh pod / first install** — bootstraps from GitHub, rebuilds every push: ```bash curl -fsSL https://raw.githubusercontent.com/sleepypod/core/dev/scripts/install \ | sudo bash -s -- --branch dev ``` 🎯 **Pin to this exact build** ([run 29174365542](https://github.com/sleepypod/core/actions/runs/29174365542) · `a3d885f`) — fresh-pod path, locked to one CI artifact for reproducible review: ```bash curl -fsSL https://raw.githubusercontent.com/sleepypod/core/a3d885f6730ad28a417880eff603551253ba1773/scripts/install \ | sudo bash -s -- \ --branch dev \ --artifact-url 'https://nightly.link/sleepypod/core/actions/runs/29174365542/sleepypod-core.zip' ``` 🧊 Artifact: [`sleepypod-core`](https://github.com/sleepypod/core/actions/runs/29174365542/artifacts/8254427229) · self-destructs in 30 days 💥 </details> <!-- sleepypod:install-end -->
|
🎉 This PR is included in version 2.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Problem
Field report (Pod 4, 2026-07-11):
stall_left/stall_rightalerts fired daily on both sides within seconds of each other, always withrpm = 0; the two most recent ended inpower_off, leaving the bed guard-blocked. The pumps are healthy — 3,477 of 5,881 flow readings in the last 7 days show normal running RPM, and the daily prime ran fine at ~3,000 RPM after the trip.Every alert lines up with a firmware-commanded pump stop, not a stall:
Root cause
runStallGuardderivedexpectedActivesolely fromdevice_state, which mirrors firmware through thedurationExpiredheuristic (targetLevel === 0 && heatingDuration === 0, elsecurrentLevel !== 0). After the firmware ends a session,currentLevelstays non-zero while the water equalizes, so the mirror reports the side as powered for minutes while the pump correctly reads 0 RPM. With frzHealth frames arriving ~every 10s (not the 60s cadence ADR 0022 assumed) anddwellSamples = 2, the guard tripped ~20s after every natural session end.Fix
DeviceStateSyncnow suppresses stall counting when the stop is explainable by lag-free firmware-side signals (isExpectedPumpStop):pump.duty === 0— firmware isn't driving the pump. A genuinely stalled pump under closed-loop control shows duty > 0 while RPM reads 0, so real stalls are unaffected. Falls back cleanly when the frame omitsduty.targetLevel === 0— commanded neutral, pump stop expected even whiledevice_statelags.heatTime) within 90s of its natural end, projected forward from the last poll. Gated onheatingDuration > 0so firmware variants that report no countdown during an active session keep the previous behavior.A zero-RPM frame mid-session with the pump still driven reaches the guard as
expectedActive = trueexactly as before.pumpStallGuard.tsitself is unchanged. Also documents the failure mode as an addendum in ADR 0022.Test plan
vitest run src/hardware/tests/— 71 tests pass, including a new suite (deviceStateSync.stallSuppression.test.ts) covering: duty=0 suppression, duty>0 non-suppression, missing-duty fallback, priming + post-prime grace expiry, firmware-neutral-while-DB-powered (the field-observed case), session-end grace, countdown projection across a stalled status stream, indefinite-session (heatingDuration=0) non-suppression, and mid-session zero-RPM still tripping.tsc --noEmitandeslintclean.pump_alertsrows should appear; then verify a real stall still trips by unplugging a pump mid-session.Refs: ygg
sleepypod-core-1, ADR 0022.🤖 Generated with Claude Code
🛏️ Beam this PR onto a sleepypod ✨
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-updateinstead; it's on disk and opens WAN as its first step.🔄 Already on a sleepypod (most common — review a PR on a running pod):
🚀 Fresh pod / first install — bootstraps from GitHub, rebuilds every push:
curl -fsSL https://raw.githubusercontent.com/sleepypod/core/fix/pump-stall-false-trip/scripts/install \ | sudo bash -s -- --branch fix/pump-stall-false-trip🎯 Pin to this exact build (run 29172327550 ·
a45903d) — fresh-pod path, locked to one CI artifact for reproducible review:🧊 Artifact:
sleepypod-core· self-destructs in 30 days 💥Summary by CodeRabbit
Bug Fixes
Documentation