docs(rf-sensing): Phase 12 spec — HTTP route surface for wizard endpoints#321
docs(rf-sensing): Phase 12 spec — HTTP route surface for wizard endpoints#321
Conversation
…ints Phase 10 deferred HTTP route registration; Phase 11 noted it as the remaining gap before the rf-sensing v2 stack is user-driveable. This spec defines the nine /api/rf/* endpoints needed to expose the wizard / household / familiar / notify modules to the SPA, without regressing the privacy invariants those modules already enforce internally. Specifies auth model (reuses Bearer pattern from PR #309), DP-noised read paths, error envelope, SPA integration sketch, and acceptance criteria. Defers federated mesh transport (Phase 13) and ESP32-C6 port (Phase 1b) explicitly so Phase 12 stays bounded. https://claude.ai/code/session_01NHvYH8ioM2w3DnDXNeeJbt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the specification for Phase 12 of the RF sensing stack, defining the HTTP route surface required for the SPA to interact with the firmware modules. The spec covers endpoints for status monitoring, setup wizard flows, alert management, and conformance testing, while enforcing privacy through DP-noised counters and the omission of raw identifiers. Review feedback highlights the need to align the JSON response schema with existing C++ data structures, correct a contradiction in UTF-8 validation for zone names, and mitigate a potential race condition in the self-test error handling logic.
| { | ||
| "state": "training", | ||
| "zone_name": "Back Door", | ||
| "context": "home", | ||
| "training": { | ||
| "progress_bps": 4200, | ||
| "complete": false | ||
| }, | ||
| "household": { | ||
| "paired_count": 2, | ||
| "enrolling": false, | ||
| "enrollment_ms_remaining": 0 | ||
| }, | ||
| "baseline": { | ||
| "populated_buckets": 91 | ||
| }, | ||
| "activity_noised": { | ||
| "alerts_fired": 4, | ||
| "events_evaluated": 1281, | ||
| "ambient_suppressed": 612, | ||
| "household_suppressed": 38 | ||
| } | ||
| } |
There was a problem hiding this comment.
The proposed JSON structure for /api/rf/status introduces nesting and field renames (e.g., progress_bps vs training_progress_bps) that do not match the wizard::Status struct defined in wizard.h. Since the underlying C++ modules (Phases 4-11) are already implemented with a flat structure and specific field names, this mismatch will require a manual mapping layer in the implementation. Aligning the JSON schema with the existing C++ model simplifies the code and ensures consistency across the stack.
| { | |
| "state": "training", | |
| "zone_name": "Back Door", | |
| "context": "home", | |
| "training": { | |
| "progress_bps": 4200, | |
| "complete": false | |
| }, | |
| "household": { | |
| "paired_count": 2, | |
| "enrolling": false, | |
| "enrollment_ms_remaining": 0 | |
| }, | |
| "baseline": { | |
| "populated_buckets": 91 | |
| }, | |
| "activity_noised": { | |
| "alerts_fired": 4, | |
| "events_evaluated": 1281, | |
| "ambient_suppressed": 612, | |
| "household_suppressed": 38 | |
| } | |
| } | |
| { | |
| "state": "training", | |
| "zone_name": "Back Door", | |
| "context": "home", | |
| "training_progress_bps": 4200, | |
| "training_complete": false, | |
| "household_paired_count": 2, | |
| "household_enrolling": false, | |
| "household_enrollment_ms_remaining": 0, | |
| "baseline_populated_buckets": 91, | |
| "total_alerts_fired": 4, | |
| "total_events_evaluated": 1281, | |
| "total_ambient_suppressed": 612, | |
| "total_household_suppressed": 38 | |
| } |
| - Server trims trailing whitespace and rejects bytes < 0x20 or >= 0x7F | ||
| with `400 bad_request`. UI-only emoji are fine; we accept UTF-8 but | ||
| measure length in bytes against MAX_ZONE_NAME_LEN. |
There was a problem hiding this comment.
There is a contradiction in the validation logic for the zone name. The spec states it "rejects bytes < 0x20 or >= 0x7F" while also claiming to "accept UTF-8" and support emojis. Since UTF-8 multi-byte sequences (including all emojis) use bytes with the high bit set (>= 0x80), the proposed range check would effectively block all non-ASCII characters. The validation should be updated to allow valid UTF-8 sequences while still filtering out ASCII control characters.
| - Server trims trailing whitespace and rejects bytes < 0x20 or >= 0x7F | |
| with `400 bad_request`. UI-only emoji are fine; we accept UTF-8 but | |
| measure length in bytes against MAX_ZONE_NAME_LEN. | |
| - Server trims trailing whitespace and rejects ASCII control characters | |
| (bytes < 0x20 or 0x7F) with 400 bad_request. UI-only emoji are fine; | |
| we accept UTF-8 but measure length in bytes against MAX_ZONE_NAME_LEN. |
| This endpoint runs synchronously on the HTTP task; max wall time | ||
| budget is 2 s under nominal load. If it exceeds, the handler returns | ||
| `500 internal` and the in-progress test continues to completion in the | ||
| background (the next call observes a clean state because each test | ||
| restores state on exit). |
There was a problem hiding this comment.
The error handling for the self-test endpoint introduces a race condition risk. Returning 500 internal while allowing a state-mutating test to "continue to completion in the background" is problematic because these tests modify and then restore global state (NVS, module globals). If a subsequent request or another self-test is processed before the background execution finishes, it could lead to corrupted state or non-deterministic results. It is safer to ensure the test is fully aborted on timeout or to implement a "busy" state that prevents concurrent execution of state-sensitive operations.
| This endpoint runs synchronously on the HTTP task; max wall time | |
| budget is 2 s under nominal load. If it exceeds, the handler returns | |
| `500 internal` and the in-progress test continues to completion in the | |
| background (the next call observes a clean state because each test | |
| restores state on exit). | |
| This endpoint runs synchronously on the HTTP task; max wall time | |
| budget is 2 s under nominal load. If it exceeds, the handler returns | |
| 500 internal and ensures the test is aborted or the system is marked | |
| busy to prevent concurrent state-mutating operations. This prevents | |
| race conditions during global state restoration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4062de54a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Calls `tests::run_all_conformance()` and returns the structured | ||
| report. Schema mirrors `tests::Report`: |
There was a problem hiding this comment.
tests::Report
This spec says /api/rf/self-test “mirrors tests::Report”, but the documented payload (passed/failed/results) does not match the actual tests::Report layout (conformance[], red_team[], per-group counts, total_ms, all_passed in firmware/projects/canary-wap/arduino/canary_wap/tests.h), and tests::run_all_conformance() currently emits only conformance checks (tests.cpp) rather than the red-team entries shown here. If implementation follows this text literally, either the API will diverge from the stated source type or clients will be built against a response shape the firmware does not naturally produce.
Useful? React with 👍 / 👎.
| Calls `wizard::always_ignore_last_decision()`. Returns: | ||
|
|
||
| - `200` `{ "muted": true }` on success | ||
| - `200` `{ "muted": false, "reason": "no_decision_yet" }` if there has |
There was a problem hiding this comment.
The endpoint contract requires two distinct failure reasons (no_decision_yet vs last_decision_fired) while also saying the handler should call wizard::always_ignore_last_decision(). In the current implementation (wizard.cpp), that helper returns only bool and collapses both cases to false, so the documented response reasons cannot be produced without bypassing or duplicating helper logic. This ambiguity will lead to inconsistent handler implementations unless the spec (or helper API) is clarified.
Useful? React with 👍 / 👎.
2 participants
Summary
After Phase 11 (#320) merged, the rf-sensing v2 stack is functionally complete in C++ but unreachable from the SPA — every entry point (
wizard::*,household::*,notify::*, etc.) is callable only fromrf_presence::update. The Phase 10 PR (#318) deliberately deferred the HTTP routing layer; this PR is the spec that closes that gap.This is a docs-only PR. Implementation is a follow-up.
The spec defines:
/api/rf/*endpoints — one read (status), six setup-flow (zone, pair-start, pair-finish, restart-training, context, mute, forget), one self-test, all reusing the existing Bearer auth + rate-limit pattern from PR feat(canary): inject bearer into SPA + gate read endpoints (P3 phase 2.5) #309._for_export(DP-noised) paths are exposed; raw fingerprints / IRKs / MACs / precise timestamps must not leave the device. Lines up withTHREAT_MODEL.mdPrinciple 3.bad_request,unauthorized,conflict,rate_limited,not_initialized,internal) using the existingwap_server::send_json_errorhelper./api/rf/status, "Mute this pattern" button, etc.) under the existing 64 KBweb_ui.hbudget.Phase 13 (federated mesh transport) and Phase 1b (ESP32-C6 port) are explicitly listed as out-of-scope follow-ups so this phase stays bounded.
Housekeeping
household::init()wiring +AlertDecision.fingerprint+wizard::always_ignore_last_decision()changes. Verified against currentmainbefore closing.Test plan
wizard.h,notify.h,household.h,familiar.h,baseline.h,federated.h,dp.h,tests.h— these were read during drafting; flag if anything has drifted.https://claude.ai/code/session_01NHvYH8ioM2w3DnDXNeeJbt
Generated by Claude Code