Repository navigation
Say what actually gates the feature-flag probe on a WHOOP 4.0 - #873
Merged
Merged
Conversation
Follow-up to #872, from its review. Comments only. Both allowlist comments claimed a default install can never form the 117/118 bytes. That is true on 5/MG and only there: the whole send() allowlist sits inside the deviceFamily == .whoop5 branch, and WHOOP 4.0 has no send allowlist at all. On a 4.0 the only thing keeping START_FF_KEY_EXCHANGE and SEND_NEXT_FF off the wire is probeFeatureFlags()'s own Test Centre gate. Same practical outcome, and it matches how every other 4.0 command already works, so this is not a hole. But the 4.0 is the family with a published key dump to reproduce, which makes it the likely first runner, and someone auditing the safety story of an unvalidated opcode should not have to discover that the reassuring sentence only covers the other family. Also notes on the Test Centre guard itself that for 4.0 it is not belt-and-braces — it is the belt. No behaviour change: the gating is exactly what it was, only described accurately.
ryanbr
pushed a commit
that referenced
this pull request
Jul 27, 2026
… read probe (#103) (#890) Read-only follow-up to #872. That probe asked a strap for the feature-flag key NAMES its firmware knows, and on a real WHOOP 5 MG (WS50_r03) it worked: the strap enumerated exactly the 16 keys NOOP already writes, serving indices 1-10 then 13-18 and skipping 11/12 itself. This one asks for a named key's VALUE, and reaches a namespace 117/118 never covered. WHY. NOOP writes config through two verbs into two namespaces and reads neither: SET_FF_VALUE (120) into the feature-flag namespace, SET_DEVICE_CONFIG_VALUE (119) into device-config. The 117/118 enumerate pair covers the feature-flag namespace only; device-config has never been enumerated or read here at all. For #103 that matters — if a config value gates the byte at offset 82, it is far likelier to live in device-config than among the 16 flags NOOP already writes. THE PRIMARY DELIVERABLE IS A PER-VERB VERDICT, not a value dump. A number in the CommandNumber table is not a served verb (opcode 96 is the standing example of one nothing sends), so a clean "neither verb is served by this firmware" is a publishable result that closes a branch of the #103 search, and is treated as a success of the probe rather than a failure. One round-trip per verb establishes that first — 128 against a flag NOOP writes, 121 against the hardware-validated Broadcast-HR key — so a FAILURE is evidence about the VERB, not the key. A dead verb is retired, so both-verbs-dead is two round-trips and ~16s, not one per key. READ-ONLY BY CONSTRUCTION. DeviceConfigReadProbe.isReadOnlyOpcode is the SAME predicate the 5/MG send() allow-list consults on both platforms — not a hardcoded pair repeated in the clause — so the exhaustive test looping all 256 opcodes proves the allow-list itself, not a copy of it. readOnlyOpcodes is disjoint from writeOpcodes, and 119/120 keep their own separate opt-in clauses, untouched. Verified on review, along with: the plan bounded three ways (maxSteps, an attempted set keyed opcode:key, monotonic phases), CRC gating before any field read on both platforms, all nine shared constants matching, 25 tests each side, and the golden report byte-identical (the Kotlin concatenated literal reconstructed and diffed against the Swift heredoc rather than trusting the test exists). NO SUBSTRING FALSE-POSITIVE. The candidate list contains both enable_spo2 and enable_spo2_packets, so the value claim was checked specifically: echoOffset requires the remainder of the 32-byte field to be NUL padding, so the shorter key against a record echoing the longer one returns nil rather than reading a bogus byte at +32. A value is claimed only when the strap echoes the requested key in the SET side's own layout; nil means "no value claimed", never "value is zero". WIRE SHAPE IS A DOCUMENTED GUESS THAT FAILS CLOSED. The request body ([0x01] then the key NUL-padded to 32 bytes) is inferred from the SET side, not observed; a wrong shape draws FAILURE or silence, which the report states plainly. Beyond the 2-byte response header no field layout is assumed — the record is reported as raw hex. The guessed oxygen key names are labelled as GUESSES in the RENDERED report, not only in source comments ("Candidate oxygen keys — GUESSES, never observed on a wire or in any table"), so someone pasting the result into #103 carries the caveat with it. NOT HARDWARE-VALIDATED: opcodes 121 and 128 have never been sent to a strap. That is the deliverable rather than a risk being glossed — the probe exists to establish whether they are served at all. Default-off behind Test Centre → Connection, user-triggered, in-flight allow-list, Test Centre re-check in the sender. Trial-merged before landing: #872's enumerate clause, #875's abort clause, #890's config-read clause and #873's 4.0 note all coexist cleanly, with both gates green on the merged tree.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…#873) Follow-up to ryanbr#872, from its review. Comments only, no behaviour change. Both allow-list comments claimed: allowed ONLY while a probe is actually in flight, so a default install can never form these bytes That is true on 5/MG and only there. The whole send() allow-list sits inside the `deviceFamily == .whoop5` branch, and WHOOP 4.0 has no send allow-list at all — so on a 4.0 the only thing keeping START_FF_KEY_EXCHANGE (117) and SEND_NEXT_FF (118) off the wire is probeFeatureFlags()'s own Test Centre gate. Same practical outcome, and it matches how every other 4.0 command already works, so this is not a hole. But the 4.0 is the family with a published key dump to reproduce, which makes it the likely first runner — and someone auditing the safety story of an opcode nobody has validated on hardware should not have to discover that the reassuring sentence only covers the other family. Also adds a line on the Test Centre guard itself: for 4.0 it is not belt-and-braces, it is the belt. That guard doing its job is load-bearing on that family, and the comment now says so. Both platforms, both comment sites. doc_comment_lint and i18n_audit --ci green. Originally offered as a patch on ryanbr#872, but the push to the contributor's fork was refused (maintainerCanModify is on, the token is not), so it lands here instead. Verified by trial merge onto current main rather than trusting the mergeable flag: ryanbr#875 edited the same allow-list region in both files afterwards, and its abort clause survives intact on both platforms with the merged comment block reading coherently.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
… read probe (ryanbr#103) (ryanbr#890) Read-only follow-up to ryanbr#872. That probe asked a strap for the feature-flag key NAMES its firmware knows, and on a real WHOOP 5 MG (WS50_r03) it worked: the strap enumerated exactly the 16 keys NOOP already writes, serving indices 1-10 then 13-18 and skipping 11/12 itself. This one asks for a named key's VALUE, and reaches a namespace 117/118 never covered. WHY. NOOP writes config through two verbs into two namespaces and reads neither: SET_FF_VALUE (120) into the feature-flag namespace, SET_DEVICE_CONFIG_VALUE (119) into device-config. The 117/118 enumerate pair covers the feature-flag namespace only; device-config has never been enumerated or read here at all. For ryanbr#103 that matters — if a config value gates the byte at offset 82, it is far likelier to live in device-config than among the 16 flags NOOP already writes. THE PRIMARY DELIVERABLE IS A PER-VERB VERDICT, not a value dump. A number in the CommandNumber table is not a served verb (opcode 96 is the standing example of one nothing sends), so a clean "neither verb is served by this firmware" is a publishable result that closes a branch of the ryanbr#103 search, and is treated as a success of the probe rather than a failure. One round-trip per verb establishes that first — 128 against a flag NOOP writes, 121 against the hardware-validated Broadcast-HR key — so a FAILURE is evidence about the VERB, not the key. A dead verb is retired, so both-verbs-dead is two round-trips and ~16s, not one per key. READ-ONLY BY CONSTRUCTION. DeviceConfigReadProbe.isReadOnlyOpcode is the SAME predicate the 5/MG send() allow-list consults on both platforms — not a hardcoded pair repeated in the clause — so the exhaustive test looping all 256 opcodes proves the allow-list itself, not a copy of it. readOnlyOpcodes is disjoint from writeOpcodes, and 119/120 keep their own separate opt-in clauses, untouched. Verified on review, along with: the plan bounded three ways (maxSteps, an attempted set keyed opcode:key, monotonic phases), CRC gating before any field read on both platforms, all nine shared constants matching, 25 tests each side, and the golden report byte-identical (the Kotlin concatenated literal reconstructed and diffed against the Swift heredoc rather than trusting the test exists). NO SUBSTRING FALSE-POSITIVE. The candidate list contains both enable_spo2 and enable_spo2_packets, so the value claim was checked specifically: echoOffset requires the remainder of the 32-byte field to be NUL padding, so the shorter key against a record echoing the longer one returns nil rather than reading a bogus byte at +32. A value is claimed only when the strap echoes the requested key in the SET side's own layout; nil means "no value claimed", never "value is zero". WIRE SHAPE IS A DOCUMENTED GUESS THAT FAILS CLOSED. The request body ([0x01] then the key NUL-padded to 32 bytes) is inferred from the SET side, not observed; a wrong shape draws FAILURE or silence, which the report states plainly. Beyond the 2-byte response header no field layout is assumed — the record is reported as raw hex. The guessed oxygen key names are labelled as GUESSES in the RENDERED report, not only in source comments ("Candidate oxygen keys — GUESSES, never observed on a wire or in any table"), so someone pasting the result into ryanbr#103 carries the caveat with it. NOT HARDWARE-VALIDATED: opcodes 121 and 128 have never been sent to a strap. That is the deliverable rather than a risk being glossed — the probe exists to establish whether they are served at all. Default-off behind Test Centre → Connection, user-triggered, in-flight allow-list, Test Centre re-check in the sender. Trial-merged before landing: ryanbr#872's enumerate clause, ryanbr#875's abort clause, ryanbr#890's config-read clause and ryanbr#873's 4.0 note all coexist cleanly, with both gates green on the merged tree.
dbAbstract
added a commit
to dbAbstract/noop
that referenced
this pull request
Oct 1, 2026
The codebase's first structured-output LLM path. Describe what you ate, the provider fills the macro fields, you check them and log. PROMPT-ONLY, on all four providers. Native structured output differs per provider and one of them cannot be relied on: OpenAI has response_format, Gemini has responseSchema, Anthropic has neither (tool-use only, which returns a dict where the existing parser expects a string, so it would need a second parser), and a Custom endpoint may 400, silently ignore the field, or truncate mid-JSON on a small context window. A native path would mean a second code path per provider and STILL need the tolerant parser as the Custom fallback — so it buys a maintenance burden and no new capability. It can be added provider-by-provider later. MacroEstimateParse is deliberately generous about WRAPPING and strict about CONTENT. Finding the object is a formatting problem — fences, preambles, nested objects, numbers as strings — and worth being forgiving with. Believing the numbers is a correctness problem and is not. Two rejections carry the weight. A truncated reply FAILS rather than being salvaged: its last number is as likely half-written as complete, and a plausible fragment is worse than an honest failure. And a reply whose macros do not add up to its own calorie figure is discarded as a hallucination — caught by NutritionMath.kcalLooksInconsistent, which already existed and was already tested. That check is the whole reason this can be trusted: an estimate that survives has been verified against arithmetic, not merely parsed. Nothing is repaired. Rewriting a model's kcal from its own macros would hand the user a number neither they nor the model ever stated — the exact failure the food log is built to avoid. The engine method mirrors generateBrief: returns nil rather than throwing, and checks the gate stack AT THE EGRESS — master switch, configured, consent, resolved key. Checked there rather than at the button for the reason ryanbr#2254 established: the sheet can already be open when the switch goes off, and that path passes no tab. It also snapshots the provider before the await, so an estimate that lands after the user switched providers is discarded rather than applied (ryanbr#873's race). callProvider gains an optional system-prompt override, defaulted so every existing caller is untouched. Necessary because the coach prompt ends with "No code blocks" and would fight a request for JSON. Provenance rides BOTH the item and the entry snapshot, and is cleared the moment the user hand-edits any field — at that point the numbers are theirs and the marker would be a lie. Fields are pre-filled and EDITABLE rather than applied: tapping Log is what turns an estimate into a figure the user has stated. 18 parse tests, most about refusing. iOS and macOS both build.
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
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.
Follow-up to #872, from its review. Comments only, no behaviour change.
Both allowlist comments claimed:
That is true on 5/MG and only there. The whole
send()allowlist sits inside thedeviceFamily == .whoop5branch, and WHOOP 4.0 has no send allowlist at all — so on a 4.0 the onlything keeping
START_FF_KEY_EXCHANGE(117) andSEND_NEXT_FF(118) off the wire isprobeFeatureFlags()'s own Test Centre gate.Same practical outcome, and it matches how every other 4.0 command already works, so this is not a
hole. But the 4.0 is the family with a published key dump to reproduce, which makes it the likely
first runner — and someone auditing the safety story of an opcode nobody has validated on hardware
should not have to discover that the reassuring sentence only covers the other family.
Also adds a line on the Test Centre guard itself: for 4.0 it is not belt-and-braces, it is the belt.
That guard doing its job is load-bearing on that family, and the comment now says so.
Both platforms, both comment sites.
doc_comment_lintandi18n_audit.py --cigreen.Originally offered as a patch on #872, but the push to the fork was refused (
maintainerCanModifyis on, the token is not), so it lands here instead. Applies unchanged on top of the merge.