Skip to content

ci + docs: test gate on PRs, TestFlight install, landing page, contributor on-ramp - #154

Merged
abdulsaheel merged 7 commits into
mainfrom
chore/public-facing
Jul 26, 2026
Merged

ci + docs: test gate on PRs, TestFlight install, landing page, contributor on-ramp#154
abdulsaheel merged 7 commits into
mainfrom
chore/public-facing

Conversation

@abdulsaheel

@abdulsaheel abdulsaheel commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

User description

Part of a pass over the org's public surface. Sibling PRs: OpenStrap/protocol#17, OpenStrap/analytics#28, OpenStrap/.github, OpenStrap/backend.

Two commits, reviewable separately.


1. ci: run analyze + the full suite on every PR

623 tests existed and nothing ran them on a pull request. The only workflow was the tag-triggered release build, so the actual gate on incoming contributions was three AI reviewers and no test run.

Adds flutter analyze + flutter test --concurrency=1 on push and PR. --concurrency=1 is required — the suite runs sqflite_common_ffi against real database files and parallel workers race on them.

Getting the first run green (a red baseline badge is worse than no badge) needed:

  • derivation_pipeline_test hard-failed without whoop_hist.jsonl. That fixture is a real band capture kept beside the repo rather than in it — present locally, absent in CI. Those two tests now skip when it's missing, matching the analytics suite. 623 still pass locally.
  • Eight pre-existing analyze findings, which meant flutter analyze had never exited zero:
    • theme.dart imported CupertinoPageTransitionsBuilder from cupertino.dart, which doesn't declare it. material.dart re-exports it and was already imported — the line was both redundant and wrong.
    • _Stat.valueColor in live_session_screen was never passed by any of its seven call sites; the value always fell through to the default. Removed the dead parameter, not the behaviour.
    • Three underscore-prefixed locals in substrate.dart, renamed.
    • One null-aware map entry in a test.

glassBoxReadiness is deliberately not migrated. It's deprecated in favour of readinessComposite, but switching it changes user-visible scores and needs a kAlgoVersion bump plus a release note. It stays an explicit open decision with an ignore comment explaining why, rather than being silently changed to satisfy a linter.


2. docs: TestFlight, landing page, contributor on-ramp

Install. The README documented iOS as "sideload an unsigned IPA" — but a public TestFlight beta has been live and open with no tester cap, and it wasn't mentioned anywhere. That's a normal install with no computer involved:

iOShttps://testflight.apple.com/join/2BVSwq65

Now the primary iOS path; the sideload guide stays for anyone who prefers it. Drops the stale "sideload only" line and flags WHOOP 5 / MG as detected-but-unvalidated, which is the honest status.

Landing page. https://openstrap.github.io/edge/ served a page titled "Edge — Legal" — the repo's homepage link pointed at the privacy policy. Adds a real landing page at docs/index.html and moves the legal index to docs/legal.html, which every legal page now links. Reuses the existing CSS tokens; no framework, no build step, light and dark.

Contributor on-ramp. 53 forks, one regular outside contributor, and no CONTRIBUTING, SECURITY, issue templates or funding config. Adds all four.

CONTRIBUTING.md leads with what actually blocks people — which of the three repos a change belongs in — then the two rules that matter (never fabricate a number when the input is missing; cite the published method) and the honest ceilings (PRV not ECG-grade HRV, relative-only SpO₂/temp) that read like bugs but aren't.

Two issue forms. The second, "a metric looks wrong", exists because "it doesn't match the WHOOP app" is the most common report and isn't by itself a bug — the form says so up front and asks what kind of wrong instead.

DONATE.md carries the BTC and EVM addresses, with FUNDING.yml pointing at it (GitHub's config takes URLs, not wallet addresses). It states plainly that nothing is gated behind paying and that bug reports from real bands are worth more — which is true, since otherwise there's exactly one person's physiology in the test data.


Verification

  • flutter analyzeNo issues found!
  • flutter test --concurrency=1623 pass
  • Landing page rendered and checked in light and dark at desktop and mobile widths.

PR Type

Enhancement, Bug fix, Documentation, tests


Description

  • Adds GitHub Actions CI workflow running flutter analyze + flutter test on every PR

  • Fixes eight pre-existing analyzer errors (dead import, dead parameter, underscore-prefixed locals, null-aware map entry) to get a green baseline

  • Adds CONTRIBUTING.md, SECURITY.md, DONATE.md, GitHub issue templates, and funding config for contributor on-ramp

  • Rewrites docs/index.html into a real landing page with install CTAs, screenshots, and feature cards; extracts legal index to docs/legal.html


Diagram Walkthrough

flowchart LR
  pr["Pull Request / Push"]
  ci[".github/workflows/test.yml"]
  analyze["flutter analyze"]
  test["flutter test --concurrency=1"]
  skip["Skip fixture-dependent tests\n(whoop_hist.jsonl absent in CI)"]

  pr -- "triggers" --> ci
  ci -- "runs" --> analyze
  ci -- "runs" --> test
  test -- "graceful skip" --> skip

  fixes["Analyzer fixes\n(theme.dart, live_session_screen,\nsubstrate.dart, readiness_flash_test)"]
  analyze -- "now passes due to" --> fixes

  docs["Public surface\n(README, CONTRIBUTING.md,\nSECURITY.md, DONATE.md,\ndocs/index.html, issue templates)"]
  pr -- "adds" --> docs
Loading

File Walkthrough

Relevant files
Configuration changes
3 files
test.yml
New CI workflow: analyze and test on every PR                       
+40/-0   
FUNDING.yml
Add GitHub funding config pointing to DONATE.md                   
+4/-0     
config.yml
Add issue template config with contact links                         
+25/-0   
Documentation
11 files
crossday_pipeline.dart
Add explanatory comment and suppress deprecated_member_use warning
+9/-0     
CONTRIBUTING.md
New contributor guide: repo split, rules, and workflow     
+116/-0 
DONATE.md
Add donation page with BTC and EVM addresses                         
+46/-0   
SECURITY.md
Add security policy with private reporting instructions   
+52/-0   
README.md
Add CI badge, TestFlight install section, and contribution links
+38/-5   
bug_report.yml
Add structured bug report issue template                                 
+90/-0   
wrong_number.yml
Add structured wrong-metric issue template                             
+69/-0   
legal.html
Extract legal index page from former index.html                   
+48/-0   
notice.html
Add Legal nav link to site header                                               
+1/-0     
privacy.html
Add Legal nav link to site header                                               
+1/-0     
terms.html
Add Legal nav link to site header                                               
+1/-0     
Bug fix
4 files
substrate.dart
Rename underscore-prefixed locals to fix analyzer warnings
+13/-13 
theme.dart
Remove redundant and incorrect cupertino.dart import         
+3/-1     
live_session_screen.dart
Remove dead valueColor parameter never passed by call sites
+2/-4     
readiness_flash_test.dart
Fix null-aware map entry analyzer warning in test helper 
+1/-1     
Tests
1 files
derivation_pipeline_test.dart
Skip fixture-dependent tests when whoop_hist.jsonl is absent
+9/-2     
Enhancement
2 files
index.html
Rewrite as real landing page with install CTAs and feature cards
+153/-24
style.css
Add landing page styles: hero, cards, shots, CTA, addr     
+185/-0 

Summary by CodeRabbit

  • New Features

    • Added a refreshed project website with installation links, screenshots, feature guidance, and support information.
    • Added clearer documentation for local data handling, privacy, legal terms, contributions, and security reporting.
    • Added structured templates for bug reports and incorrect metric reports.
  • Improvements

    • Standardized GPS statistic styling for a more consistent display.
    • Clarified current device support, background syncing, health exports, and experimental WHOOP 5.0/MG support.
  • Documentation

    • Added donation and contributor guidance, including available support options.

623 tests existed and nothing ran them on a pull request. The only
workflow in the repo was the tag-triggered release build, so the actual
review gate on contributions was three AI reviewers and no test run.

Adds flutter analyze + flutter test --concurrency=1 on push and PR.
--concurrency=1 is required: the suite runs sqflite_common_ffi against
real database files and parallel workers race on them.

Two things had to be fixed for a first run to be green rather than red,
because a red baseline badge is worse than no badge:

* derivation_pipeline_test hard-failed without whoop_hist.jsonl. That
  fixture is a real band capture kept beside the repo rather than in it,
  so it is present locally and absent in CI. Those two tests now skip
  when it is missing, matching the analytics suite. 623 pass locally.

* eight pre-existing analyze findings. Individually trivial, but they
  meant `flutter analyze` had never exited zero:
    - theme.dart imported CupertinoPageTransitionsBuilder from
      cupertino.dart, which does not declare it. material.dart
      re-exports it, and that import was already in scope — so the line
      was both redundant and wrong.
    - _Stat.valueColor in live_session_screen was never passed by any
      of its seven call sites; the value always fell through to the
      default. Removed the dead parameter, not the behaviour.
    - three underscore-prefixed locals in substrate.dart, renamed.
    - one null-aware map entry in a test.

glassBoxReadiness in crossday_pipeline is deliberately NOT migrated. It
is deprecated in favour of readinessComposite, but switching it changes
user-visible scores and needs a kAlgoVersion bump and a release note, so
it stays an explicit open decision with an ignore comment explaining why,
rather than being silently changed to make a linter happy.
The public surface was lagging well behind the app.

Install. iOS was documented as "sideload an unsigned IPA", but a public
TestFlight beta has been live and open for a while with no cap on
testers. That is a normal install with no computer involved, and it was
nowhere in the README. Adds it as the primary iOS path, keeps the
sideload guide for anyone who prefers it, and drops the stale "sideload
only" line. Also flags WHOOP 5 / MG as detected-but-unvalidated, which is
the honest status.

Landing page. https://openstrap.github.io/edge/ served a page titled
"Edge — Legal" — the repo's own homepage link pointed at the privacy
policy. Adds a real landing page at docs/index.html (screenshots, both
install paths, what the app is and what it deliberately does not claim)
and moves the legal index to docs/legal.html, which every legal page now
links. Reuses the existing CSS tokens; no framework, no build step.

Contributor on-ramp. 53 forks, one regular outside contributor, and no
CONTRIBUTING, SECURITY, issue templates or funding config. Adds:
  - CONTRIBUTING.md, leading with the thing that actually blocks people:
    which of the three repos a change belongs in. Plus the two rules that
    matter — never fabricate a number when the input is missing, and cite
    the published method — and the honest ceilings (PRV not ECG HRV,
    relative-only SpO2/temp) that read like bugs but are not.
  - SECURITY.md for private disclosure, scoped to where the data actually
    is: the phone, the BLE link, the local database. Not a cloud backend,
    because there isn't one.
  - Two issue forms. The second, "a metric looks wrong", exists because
    "it doesn't match the WHOOP app" is the most common report and is not
    by itself a bug — the form says so up front and asks for the kind of
    wrong instead.

DONATE.md carries the BTC and EVM addresses, with FUNDING.yml pointing at
it (GitHub's config takes URLs, not wallet addresses). It says plainly
that nothing is gated behind paying and that bug reports from real bands
are worth more — which is true, since otherwise there is exactly one
person's physiology in the test data.
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@abdulsaheel, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04858fce-26a9-4291-9ef1-3d0312328f91

📥 Commits

Reviewing files that changed from the base of the PR and between d1ac6fa and 94d04ad.

📒 Files selected for processing (2)
  • .github/ISSUE_TEMPLATE/wrong_number.yml
  • README.md
📝 Walkthrough

Walkthrough

The PR adds repository contribution, security, funding, issue, and CI configuration; rebuilds the Edge static homepage and legal navigation; and applies targeted Flutter, decoding, UI, and test-fixture maintenance changes.

Changes

Project operations and contributor guidance

Layer / File(s) Summary
Issue intake and routing
.github/ISSUE_TEMPLATE/*
Adds structured bug and metric-report forms, blank issue support, and contact links for specialized reports and questions.
Project guidance and support
CONTRIBUTING.md, SECURITY.md, DONATE.md, README.md, .github/FUNDING.yml
Documents contribution rules, security reporting, project support, installation details, funding, and donation addresses.
Automated Flutter validation
.github/workflows/test.yml
Adds Flutter setup, dependency retrieval, static analysis, and serialized test execution for pushes, pull requests, and manual runs.

Static website and legal navigation

Layer / File(s) Summary
Landing page content and presentation
docs/index.html, docs/style.css
Rebuilds the homepage with installation CTAs, screenshots, project information, pipeline links, support details, and dedicated responsive styles.
Legal page routing
docs/legal.html, docs/notice.html, docs/privacy.html, docs/terms.html
Adds a legal index page and links existing legal pages to it through their navigation headers.

Application maintenance and test fixtures

Layer / File(s) Summary
Compute and theme maintenance
lib/compute/crossday_pipeline.dart, lib/compute/substrate.dart, lib/theme/theme.dart
Documents and suppresses a retained deprecated readiness call, renames JSON decoding helpers, and removes a redundant Flutter import.
UI and regression test adjustments
lib/ui/activity/live_session_screen.dart, test/*
Standardizes GPS statistic colors, skips fixture-dependent integration tests when needed, and preserves null readiness fields in test payloads.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main CI gating and documentation updates in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/public-facing

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 94d04ad)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Null-entry map change

The helper changed from {if (readiness != null) 'readiness': readiness} to {'readiness': ?readiness}. The null-aware spread ?readiness inserts the key with a null value when readiness is null, whereas the old form omitted the key entirely. If TodayData.fromJson distinguishes between a missing key and a key present with a null value (e.g. via containsKey or a non-null default), the tests that pass readiness: null now exercise a different code path than before, potentially masking the flash regression they were written to pin.

'daily': {'readiness': ?readiness},

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Latest suggestions up to 94d04ad

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix invalid null-aware map value syntax

The null-aware spread ?readiness is not valid Dart syntax in a map literal — the
null-aware element operator (?) applies to collection elements in list/set literals,
not to map values. This will fail to compile. To conditionally include the key only
when readiness is non-null (preserving the original intent of the removed if
(readiness != null) guard), use a conditional spread or the original pattern.

test/readiness_flash_test.dart [24]

-    'daily': {'readiness': ?readiness},
+    'daily': {if (readiness != null) 'readiness': readiness},
Suggestion importance[1-10]: 7

__

Why: The ?readiness syntax in a map literal value position is not valid Dart — the null-aware element operator only works in list/set literals. This would cause a compile error. The original if (readiness != null) guard was the correct approach and the suggestion correctly restores it.

Medium
Remove redundant expect that fires before skip takes effect

When skipNoFixture is non-null (fixture absent), the test body still runs up to the
first expect, which calls fixtureFile() again and then expect(f, isNotNull, ...)
this expect will throw before the skip mechanism suppresses the test, causing a test
failure rather than a skip. The expect(f, isNotNull) guard inside the body should be
removed since skipNoFixture already guarantees f is non-null when the test runs.

test/derivation_pipeline_test.dart [44-46]

-  test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
-    final f = fixtureFile();
-    expect(f, isNotNull, reason: 'whoop_hist.jsonl fixture not found');
-  
-  ...
-  }, skip: skipNoFixture);
+    test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
+      final f = fixtureFile()!;
Suggestion importance[1-10]: 5

__

Why: In Dart's test framework, skip is evaluated before the test body runs, so the expect(f, isNotNull) inside the body would not execute when the test is skipped. The suggestion's concern about the expect firing before skip is incorrect for Dart's test runner. However, removing the redundant expect and using fixtureFile()! is still a minor improvement for clarity.

Low

Previous suggestions

Suggestions up to commit d1ac6fa
CategorySuggestion                                                                                                                                    Impact
Possible issue
Restore absent-key semantics for missing readiness

The null-aware spread ?readiness unconditionally inserts the key 'readiness' with a
null value when readiness is null, whereas the original if (readiness != null)
omitted the key entirely. Downstream code that distinguishes "key absent" from "key
present with null value" (e.g. Metric.absent vs a null metric) will behave
differently, potentially causing the flash test to pass for the wrong reason — a
fabricated-absent metric instead of a truly absent one.

test/readiness_flash_test.dart [24]

-    'daily': {'readiness': ?readiness},
+    'daily': {if (readiness != null) 'readiness': readiness},
Suggestion importance[1-10]: 7

__

Why: The null-aware element ?readiness inserts 'readiness': null when readiness is null, while the original if (readiness != null) omitted the key entirely. If TodayData.fromJson distinguishes between a missing key and a null value, this behavioral change could cause tests to pass for the wrong reason.

Medium
General
Remove redundant expect that defeats skip guard

The first test still calls expect(f, isNotNull, ...) before the skip guard can take
effect. When the fixture is absent in CI the test body runs, hits that expect, and
fails — defeating the purpose of skipNoFixture. The expect inside the body should be
removed (the skip already communicates the reason), or the body should guard with an
early return.

test/derivation_pipeline_test.dart [44-46]

   test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
-    final f = fixtureFile();
-    expect(f, isNotNull, reason: 'whoop_hist.jsonl fixture not found');
+    final f = fixtureFile()!;
Suggestion importance[1-10]: 6

__

Why: When skipNoFixture is non-null, the skip parameter prevents the test body from running, so the expect(f, isNotNull, ...) inside the body would not execute. However, the improved_code changes fixtureFile() to fixtureFile()! which would throw a null dereference if the fixture is absent — this doesn't fully address the concern and the suggestion's fix is incomplete.

Low
Suggestions up to commit 2f93bcc
CategorySuggestion                                                                                                                                    Impact
Possible issue
Absent-key semantics changed by null-aware spread

The null-aware spread ?readiness unconditionally inserts the key 'readiness' with a
null value when readiness is null, whereas the original if (readiness != null)
omitted the key entirely. Downstream code that distinguishes "key absent" from "key
present with null value" (e.g. Metric.absent vs a fabricated null metric) will
behave differently, potentially masking the exact flash/fabrication bug this test is
meant to pin. Restore the conditional entry to preserve the original semantics.

test/readiness_flash_test.dart [24]

-    'daily': {'readiness': ?readiness},
+    'daily': {if (readiness != null) 'readiness': readiness},
Suggestion importance[1-10]: 7

__

Why: The change from {if (readiness != null) 'readiness': readiness} to {'readiness': ?readiness} alters semantics: the null-aware spread inserts the key with a null value when readiness is null, whereas the original omitted the key entirely. If downstream TodayData.fromJson distinguishes between a missing key and a null-valued key, this could mask the exact behavior the test is meant to verify.

Medium
General
Redundant expect inside skipped test still throws

When skipNoFixture is non-null the test is skipped, but the body still calls
expect(f, isNotNull, ...) before the skip takes effect — in the test() callback the
skip is applied by the framework after the callback is registered, not before it
runs, so the expect inside will throw when the fixture is absent rather than
skipping cleanly. Remove the now-redundant expect(f, isNotNull) guard inside the
test body (or replace it with an early return) since skipNoFixture already handles
the absent-fixture case.

test/derivation_pipeline_test.dart [44-46]

-  test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
-    final f = fixtureFile();
-    expect(f, isNotNull, reason: 'whoop_hist.jsonl fixture not found');
-  ...
-  }, skip: skipNoFixture);
+    test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
+      final f = fixtureFile()!; // safe: test is skipped when fixture absent
+      // ... rest of test using f directly
+    }, skip: skipNoFixture);
Suggestion importance[1-10]: 4

__

Why: The skip parameter in test() prevents the test body from running when the fixture is absent, so the expect(f, isNotNull) inside the body would not execute during a skip. The suggestion's premise that the expect throws before the skip takes effect is incorrect — the framework skips the entire callback. The improvement is minor style cleanup rather than a real bug fix.

Low
Suggestions up to commit 3ba9b4b
CategorySuggestion                                                                                                                                    Impact
Possible issue
Absent-readiness test case fabricates null entry

The null-aware spread ?readiness unconditionally inserts the key 'readiness' with a
null value when readiness is null, whereas the original if (readiness != null)
'readiness': readiness omitted the key entirely. TodayData.fromJson likely
distinguishes between a missing key and a null value, so this change fabricates a
null entry for the absent-readiness test cases and may cause those cases to behave
differently than a real absent-readiness scenario, breaking the flash/freeze
regression tests this file is meant to pin.

test/readiness_flash_test.dart [24]

-    'daily': {'readiness': ?readiness},
+    'daily': {if (readiness != null) 'readiness': readiness},
Suggestion importance[1-10]: 8

__

Why: The change from {if (readiness != null) 'readiness': readiness} to {'readiness': ?readiness} is semantically different: the null-aware spread operator ?readiness still inserts the key 'readiness' with a null value when readiness is null, whereas the original omitted the key entirely. This could cause TodayData.fromJson to behave differently for absent-readiness test cases, potentially breaking the flash/freeze regression tests this file is meant to pin.

Medium
General
Fixture resolved twice with inconsistent results

fixtureFile() is called twice before the test body runs — once to compute
skipNoFixture and once inside the test. If the function has any side-effect or the
file appears between the two calls the results could diverge, but more importantly
the expect(f, isNotNull, ...) inside the test body will still hard-fail CI if the
fixture is somehow present for skipNoFixture but absent when the test runs. Since
the test is now guarded by skip: skipNoFixture, the inner expect on f is redundant
and should be removed to avoid a confusing double-failure path; use the
already-resolved f from the outer call or remove the inner guard.

test/derivation_pipeline_test.dart [40-46]

-  final skipNoFixture = fixtureFile() == null
-      ? 'whoop_hist.jsonl fixture not found beside the repo'
-      : null;
+    final fixture = fixtureFile();
+    final skipNoFixture = fixture == null
+        ? 'whoop_hist.jsonl fixture not found beside the repo'
+        : null;
 
-  test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
-    final f = fixtureFile();
-    expect(f, isNotNull, reason: 'whoop_hist.jsonl fixture not found');
+    test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
+      final f = fixture;
+      // skip guard above ensures f is non-null here
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies that fixtureFile() is called twice, but the practical risk is low since the file system state is unlikely to change between calls. The improved_code also moves the fixture resolution outside the main() scope incorrectly, and the inner expect(f, isNotNull) is indeed redundant but not harmful since the skip guard prevents the test from running when the fixture is absent.

Low
Suggestions up to commit 1190386
CategorySuggestion                                                                                                                                    Impact
Possible issue
Absent-readiness key fabricated instead of omitted

The null-aware spread ?readiness unconditionally inserts the key 'readiness' with a
null value when readiness is null, whereas the original if (readiness != null)
'readiness': readiness omitted the key entirely. TodayData.fromJson likely
distinguishes between a missing key and an explicit null, so this change alters the
behavior of every call site that passes readiness: null and may cause the flash test
to stop exercising the absent-readiness path it was written to pin.

test/readiness_flash_test.dart [24]

-    'daily': {'readiness': ?readiness},
+    'daily': {if (readiness != null) 'readiness': readiness},
Suggestion importance[1-10]: 7

__

Why: The change from {if (readiness != null) 'readiness': readiness} to {'readiness': ?readiness} alters semantics: the new form always inserts the key with a null value when readiness is null, while the original omitted the key entirely. If TodayData.fromJson distinguishes between a missing key and an explicit null, this could break the absent-readiness test path.

Medium
General
Redundant fixture null-check after skip guard

fixtureFile() is called twice: once to compute skipNoFixture and again inside the
test body. If the fixture is absent, the test is skipped via skipNoFixture, but the
expect(f, isNotNull, ...) assertion inside the body is now unreachable dead code
that could mislead future readers into thinking the fixture is guaranteed present.
More importantly, if fixtureFile() has any side-effect or the file appears between
the two calls, the skip guard and the body can disagree. Remove the redundant expect
inside the body since the skip already guards it.

test/derivation_pipeline_test.dart [40-46]

-  final skipNoFixture = fixtureFile() == null
-      ? 'whoop_hist.jsonl fixture not found beside the repo'
-      : null;
+    final skipNoFixture = fixtureFile() == null
+        ? 'whoop_hist.jsonl fixture not found beside the repo'
+        : null;
 
-  test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
-    final f = fixtureFile();
-    expect(f, isNotNull, reason: 'whoop_hist.jsonl fixture not found');
+    test('decodeRecTs reads the frame\'s real ts, not the fallback', () {
+      final f = fixtureFile()!;
Suggestion importance[1-10]: 3

__

Why: The expect(f, isNotNull, ...) inside the test body is indeed dead code when the skip guard is active, but the improved_code changes indentation and uses fixtureFile()! without removing the expect, making the suggestion incomplete and slightly inaccurate. The issue is minor and the fix as shown doesn't fully address the concern.

Low

The first CI run failed, and the failure is worth keeping the note about.

`channel: stable` resolved to Flutter 3.44.8 on the runner while local dev
is on 3.41.6. CupertinoPageTransitionsBuilder moved between cupertino.dart
and material.dart across those versions, so the same theme.dart is
analyze-clean on one and five hard errors on the other. I'd "fixed" a
local unused-import warning by deleting the cupertino import, which was
correct for 3.41.6 and broke the build on 3.44.8.

Reverted that import and pinned flutter-version: 3.44.8, with a comment on
the import warning the next person not to clean it up without checking the
pinned toolchain.

This is the same reasoning as the commit-SHA pins for sibling packages in
pubspec.yaml: a floating ref means the toolchain changes underneath an
already-reviewed commit. CI is now deterministic.

Note this leaves local dev on 3.41.6 reporting two warnings that CI does
not see. Upgrading local Flutter to the pinned version makes both agree.
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 1190386

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (3)
.github/ISSUE_TEMPLATE/wrong_number.yml (1)

39-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Capture the band model for metric reports.

README.md Lines 88-90 state that WHOOP 5.0/MG is experimental while WHOOP 4.0 is validated. Without a required band field, reports cannot distinguish device-specific behavior.

Add a band dropdown, reusing the options from .github/ISSUE_TEMPLATE/bug_report.yml.

🤖 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 @.github/ISSUE_TEMPLATE/wrong_number.yml around lines 39 - 59, Add a required
band dropdown to the wrong-number issue template near the existing kind and
version fields, using the same options and labeling conventions as the band
field in bug_report.yml. Preserve the current validations and ensure reporters
must select their WHOOP device band.
DONATE.md (1)

10-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fix the Markdown heading and fence warnings.

The document jumps from h1 to h3 and both wallet-address fences lack a language, triggering MD001 and MD040.

Proposed fix
-### Bitcoin
+## Bitcoin

-```
+```text
 bc1qvtcch38dcwp967ar764uu6eetw7tf907844wfq

-### EVM — Ethereum, Base, Arbitrum, Optimism, Polygon
+## EVM — Ethereum, Base, Arbitrum, Optimism, Polygon

- +text
0x8310C89393366b7eBCD47ABa82e1dfB5ECeFFbD9

🤖 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 `@DONATE.md` around lines 10 - 19, Update the Markdown headings in DONATE.md so
the Bitcoin section uses the appropriate level without skipping from h1 to h3,
and change both wallet-address code fences under the Bitcoin and EVM sections to
use the text language identifier. Keep the wallet addresses unchanged.

Source: Linters/SAST tools

docs/style.css (1)

216-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Scope the landing-page selectors.

The comment says these rules are used only by index.html, but selectors such as .fineprint, .card, and .pipeline are global while style.css is shared with legal pages. Add a landing-page body class and prefix these selectors, or split the stylesheet, to prevent cross-page style collisions.

🤖 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 `@docs/style.css` around lines 216 - 218, Scope the landing-page selectors in
the section beginning with the landing-page comment so they cannot affect legal
pages. Add a landing-page class to the index.html body and prefix selectors such
as .fineprint, .card, and .pipeline with that class, preserving their existing
landing-page styling while leaving shared legal-page rules unchanged.
🤖 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 @.github/ISSUE_TEMPLATE/bug_report.yml:
- Around line 74-80: Update the screenshots textarea description in the bug
report template to explicitly warn that issues are public and reporters must
redact health information and personal identifiers before uploading images.
Replace the current vague redaction wording while preserving the existing field
and validation settings.

In @.github/ISSUE_TEMPLATE/config.yml:
- Around line 2-25: Add a contact_links entry in the config.yml contact_links
list for private vulnerability reporting, using the repository’s GitHub private
security advisory/reporting URL and clearly directing reporters not to open
public issues. Preserve the existing category links and formatting.

In @.github/workflows/test.yml:
- Line 17: Harden the checkout step using actions/checkout@v4 by setting
persist-credentials to false and configure the workflow job’s repository
permissions as read-only. Apply these changes to the checkout and job
permissions configuration without altering the dependency installation,
analysis, or test steps.

In `@docs/index.html`:
- Line 7: Update the homepage description metadata and corresponding visible “No
cloud” claims in docs/index.html to qualify that health data stays off the cloud
or disclose the Firebase crash/performance diagnostics exception. Keep the
wording consistent across the referenced locations and aligned with the
diagnostics disclosure in docs/privacy.html.

In `@docs/legal.html`:
- Line 15: Update the Legal navigation link in legal.html to include
aria-current="page", preserving its existing href and label.

In `@lib/theme/theme.dart`:
- Around line 28-30: Add an explicit package:flutter/cupertino.dart import in
the theme file so CupertinoPageTransitionsBuilder usages resolve, while
retaining the existing material.dart import for other symbols.

In `@SECURITY.md`:
- Around line 42-48: Update the privacy-boundary descriptions in SECURITY.md and
README.md to consistently enumerate all network-capable features, including
optional diagnostics and health-app integrations, alongside legacy import and
the update pointer. Ensure both documents clearly state that opt-in features are
disabled by default and compiled out of store builds, and remove or revise
README.md’s incomplete “only three network calls” wording.

---

Nitpick comments:
In @.github/ISSUE_TEMPLATE/wrong_number.yml:
- Around line 39-59: Add a required band dropdown to the wrong-number issue
template near the existing kind and version fields, using the same options and
labeling conventions as the band field in bug_report.yml. Preserve the current
validations and ensure reporters must select their WHOOP device band.

In `@docs/style.css`:
- Around line 216-218: Scope the landing-page selectors in the section beginning
with the landing-page comment so they cannot affect legal pages. Add a
landing-page class to the index.html body and prefix selectors such as
.fineprint, .card, and .pipeline with that class, preserving their existing
landing-page styling while leaving shared legal-page rules unchanged.

In `@DONATE.md`:
- Around line 10-19: Update the Markdown headings in DONATE.md so the Bitcoin
section uses the appropriate level without skipping from h1 to h3, and change
both wallet-address code fences under the Bitcoin and EVM sections to use the
text language identifier. Keep the wallet addresses unchanged.
🪄 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 Plus

Run ID: 7e9a54dd-03e0-4637-85cd-5c4733bc9aa9

📥 Commits

Reviewing files that changed from the base of the PR and between 8aebe08 and e0aa467.

📒 Files selected for processing (21)
  • .github/FUNDING.yml
  • .github/ISSUE_TEMPLATE/bug_report.yml
  • .github/ISSUE_TEMPLATE/config.yml
  • .github/ISSUE_TEMPLATE/wrong_number.yml
  • .github/workflows/test.yml
  • CONTRIBUTING.md
  • DONATE.md
  • README.md
  • SECURITY.md
  • docs/index.html
  • docs/legal.html
  • docs/notice.html
  • docs/privacy.html
  • docs/style.css
  • docs/terms.html
  • lib/compute/crossday_pipeline.dart
  • lib/compute/substrate.dart
  • lib/theme/theme.dart
  • lib/ui/activity/live_session_screen.dart
  • test/derivation_pipeline_test.dart
  • test/readiness_flash_test.dart

Comment thread .github/ISSUE_TEMPLATE/bug_report.yml
Comment thread .github/ISSUE_TEMPLATE/config.yml
Comment thread .github/workflows/test.yml
Comment thread docs/index.html
Comment thread docs/legal.html Outdated
Comment thread lib/theme/theme.dart Outdated
Comment thread SECURITY.md Outdated
Comment on lines +42 to +48
## Where your data actually is

Worth knowing before you go looking: OpenStrap computes and stores everything
on-device. There's no account and no server holding your health data. The
optional companion worker exists for legacy import, an update pointer, and two
opt-in features that are off by default and compiled out of store builds
entirely. See [PRIVACY.md](PRIVACY.md).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== SECURITY.md (around cited lines) ==\n'
nl -ba SECURITY.md | sed -n '35,60p'

printf '\n== README.md (around cited lines) ==\n'
nl -ba README.md | sed -n '120,155p'

printf '\n== PRIVACY.md (top and relevant matches) ==\n'
nl -ba PRIVACY.md | sed -n '1,220p'

printf '\n== Search for related terms ==\n'
rg -n --hidden --glob '!**/.git/**' \
  -e 'legacy import|update pointer|opt-in feature|store build|compiled out|BYOK|LLM proxy|telemetry|health-upload|health data|network inventory|companion worker|update.*pointer|privacy boundary' .

Repository: OpenStrap/edge

Length of output: 232


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== SECURITY.md (around cited lines) ==\n'
awk 'NR>=35 && NR<=60 {printf "%4d %s\n", NR, $0}' SECURITY.md

printf '\n== README.md (around cited lines) ==\n'
awk 'NR>=120 && NR<=155 {printf "%4d %s\n", NR, $0}' README.md

printf '\n== PRIVACY.md (top and relevant matches) ==\n'
awk 'NR>=1 && NR<=220 {printf "%4d %s\n", NR, $0}' PRIVACY.md

printf '\n== Search for related terms ==\n'
rg -n --hidden --glob '!**/.git/**' \
  -e 'legacy import|update pointer|opt-in feature|store build|compiled out|BYOK|LLM proxy|telemetry|health-upload|health data|network inventory|companion worker|update.*pointer|privacy boundary' .

Repository: OpenStrap/edge

Length of output: 24853


Align the privacy boundary wording across SECURITY.md and README.md. README.md’s “only three network calls” line is incomplete; it needs to account for the optional diagnostics and health-app integrations, and the store-build exclusions should be stated consistently in both docs.

🤖 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 `@SECURITY.md` around lines 42 - 48, Update the privacy-boundary descriptions
in SECURITY.md and README.md to consistently enumerate all network-capable
features, including optional diagnostics and health-app integrations, alongside
legacy import and the update pointer. Ensure both documents clearly state that
opt-in features are disabled by default and compiled out of store builds, and
remove or revise README.md’s incomplete “only three network calls” wording.

Correcting the previous commit, which pinned 3.44.8. That was the wrong
direction and CI caught it.

The repo genuinely cannot build on Flutter 3.44.x: phosphor_flutter 2.1.0
extends IconData, which became a `final class` in 3.44, so every widget
test fails to compile with

  The class 'IconData' can't be extended outside of its library because
  it's a final class.

That is a real forward-compatibility problem worth knowing about
independently of CI — moving past 3.41.x needs phosphor_flutter bumped or
replaced first. Left as-is here rather than bundled into a docs pass.

So CI pins 3.41.6, which is what local development actually uses and what
the release build produces. With that pin the cupertino import in
theme.dart is genuinely redundant (material.dart re-exports the symbol on
this version), so it goes back out, with a comment saying what to do if
the pin is ever bumped.

Local and CI now agree: analyze clean, 623 tests.
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 3ba9b4b

@abdulsaheel

Copy link
Copy Markdown
Collaborator Author

CI is green — and it caught a real forward-compat problem on its first run

Worth calling out separately from the diff, because it's a finding rather than a change.

The repo does not build on current Flutter stable. phosphor_flutter 2.1.0 extends IconData, which became a final class in Flutter 3.44:

The class 'IconData' can't be extended outside of its library because it's a final class.

Every widget test fails to compile on 3.44.8. Local dev and the release build are on 3.41.6, so nothing had surfaced it — this is exactly the class of thing a PR gate exists to find, and it found it within an hour of existing.

I've pinned CI to flutter-version: 3.41.6 (what actually works) rather than channel: stable, with the reasoning in a comment on the workflow. Moving past 3.41.x will need phosphor_flutter bumped or replaced first — I've deliberately left that out of this PR rather than bundling a dependency migration into a docs pass. Probably worth its own issue.

Same reasoning as the commit-SHA pins for sibling packages in pubspec.yaml: a floating ref changes the toolchain underneath an already-reviewed commit.

Final state — local and CI now agree:

flutter analyze No issues found
flutter test --concurrency=1 623 pass

The one comment the HN submission got (item 48939059) said the app
"doesn't appear to be any Apple Health integration (yet)" and that there's
"no background sync on iOS -- you have to open the app to sync."

Both are wrong, and the README is why:

* HealthKit and Health Connect export has been shipping (health_export.dart,
  wired through app_state and the profile screen: sleep stages, resting HR,
  HRV SDNN/RMSSD, respiratory rate, active energy, steps, workouts,
  idempotent per day). The README did not contain the string "Apple
  Health" or "HealthKit" anywhere at all.

* iOS background sync exists -- a BGProcessingTask, a lighter
  BGAppRefreshTask, and a separate restore CBCentral that relaunches the
  app when the band reconnects, plus a foreground service and a 15-minute
  watchdog worker on Android. Its ONLY mention in the README was a bullet
  under "What doesn't work", so a reader skimming for it concluded it
  wasn't there.

That second one is the more instructive failure. The honesty framing is
worth keeping, but listing genuinely hard work exclusively under its
caveat reads as an absence. Now stated under "What works" with the
limitation kept immediately below it, sharpened to say what is actually
true: the tasks work, iOS decides when they run.

Also mentions the local .db export and the smart alarm, neither of which
appeared anywhere either.
@abdulsaheel

Copy link
Copy Markdown
Collaborator Author

One more commit: fixing what the HN thread revealed

The HN submission got exactly one comment. It was positive, but it made three claims — and two of them are factually wrong:

Requires being sideloaded (but no jailbreak) on iOS. No background sync on iOS yet — you have to open the app to sync. Doesn't appear to be any Apple Health integration (yet).

  • Sideload — fixed by the TestFlight link in this PR.
  • "No Apple Health integration"lib/health/health_export.dart has been shipping HealthKit and Health Connect export: sleep stages, resting HR, HRV (SDNN/RMSSD), respiratory rate, active energy, steps, workouts, idempotent per day. The README did not contain the string "Apple Health" or "HealthKit" anywhere.
  • "No background sync on iOS" — there's a BGProcessingTask, a lighter BGAppRefreshTask, and a separate restore CBCentral that relaunches the app when the band reconnects. Its only mention in the README was a bullet under "What doesn't work."

That last one is the instructive failure. The honesty framing is a real asset and worth keeping — but listing genuinely hard engineering exclusively under its own caveat reads to a skimmer as an absence. A technically sophisticated reader looked for both features, couldn't find them, and said so publicly on the project's only HN thread. Nobody corrected it, because the submitter wasn't the author (guidoiaquinti, not you) so there was no one in the thread to answer.

Now both are under "What works", with the iOS limitation kept immediately below and sharpened to what's actually true: the tasks work, iOS decides when they run. Also added the local .db export and the smart alarm — neither appeared anywhere either.

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 2f93bcc

The substantive one: the landing page said "No account, no cloud, nothing
to pay", and the README said "the only network calls this app ever makes
are three narrow, optional things". Both are wrong, and PRIVACY.md already
said so.

Anonymous Firebase crash/performance diagnostics are ON BY DEFAULT in
GitHub release builds — which is what almost everyone is running right now,
since store builds don't exist yet. They're switchable off in-app, absent
entirely from App Store/Play Store builds, and never carry health data.
Separately, opt-in health-data contribution does upload the local database
when enabled. Neither appeared in the README's list of network calls.

The health-data claim was always true and stays: nothing sends your health
data anywhere unless you turn it on. But "no cloud", unqualified, is the
kind of thing this project exists not to do — the whole pitch is that the
numbers admit what they don't know. Overclaiming on privacy while doing
that would be indefensible, and it's the one claim a hostile reader would
check first.

So:
* Landing page hero says "no server that ever receives your health data"
  instead of "no cloud", with the diagnostics exception stated in the card
  that makes the claim, linked to the full policy.
* README replaces the "three narrow things" line with all five network
  paths, each marked required/optional and store-vs-GitHub.
* SECURITY.md states the same boundary exactly, rather than "computes and
  stores everything on-device".

Also from the review:
* Both issue forms now say the issue is PUBLIC and ask for health data,
  dates and identifiers to be redacted before uploading screenshots. This
  is a health app; "redact whatever you want to" wasn't good enough.
* Issue chooser gets a private security-reporting link, so the router
  doesn't send every category to a public issue while SECURITY.md says not
  to file security problems publicly.
* Workflow: permissions: contents: read + persist-credentials: false.
* aria-current="page" on each legal page's own nav link.

Not applied — CodeRabbit flagged the removed cupertino import in theme.dart
as critical, claiming material.dart no longer exports
CupertinoPageTransitionsBuilder. That's true on Flutter 3.44 and false on
3.41.6, which is what this repo pins and builds against. CI is green on the
pinned toolchain; re-adding the import would break it.
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit d1ac6fa

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@README.md`:
- Around line 161-165: Update the README privacy comparison so the BYOK AI
assistant entry explicitly acknowledges that prompts may include health-related
data, and revise the Health-data contribution statement to claim only that it is
the sole feature uploading the local database wholesale.
🪄 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 Plus

Run ID: 846febe5-832c-4d1b-8a23-4e97ea8fefb8

📥 Commits

Reviewing files that changed from the base of the PR and between e0aa467 and d1ac6fa.

📒 Files selected for processing (10)
  • .github/ISSUE_TEMPLATE/bug_report.yml
  • .github/ISSUE_TEMPLATE/config.yml
  • .github/ISSUE_TEMPLATE/wrong_number.yml
  • .github/workflows/test.yml
  • README.md
  • SECURITY.md
  • docs/index.html
  • docs/legal.html
  • docs/style.css
  • lib/theme/theme.dart
🚧 Files skipped from review as they are similar to previous changes (8)
  • docs/legal.html
  • .github/ISSUE_TEMPLATE/config.yml
  • .github/ISSUE_TEMPLATE/wrong_number.yml
  • SECURITY.md
  • lib/theme/theme.dart
  • .github/workflows/test.yml
  • docs/index.html
  • docs/style.css

Comment thread README.md Outdated
@abdulsaheel

Copy link
Copy Markdown
Collaborator Author

CodeRabbit triage — 5 applied, 1 rejected

Applied

Finding Verdict
docs/index.html — qualify "No cloud" Correct, and the important one. See below.
SECURITY.md / README.md — privacy boundary inconsistent Correct, same root cause.
bug_report.yml — explicit public-screenshot warning Correct. This is a health app; "redact whatever you want to" wasn't good enough. Applied to both issue forms.
config.yml — add private security-reporting link Correct. The router sent every category to a public issue while SECURITY.md says not to file security problems publicly.
workflows/test.ymlpersist-credentials: false + permissions: contents: read Correct. The job runs PR-authored code and never pushes back.
legal.htmlaria-current="page" Correct, trivial.

The "No cloud" one deserves calling out, because CodeRabbit caught a genuine honesty failure and it's exactly the kind this project can least afford.

The landing page said "No account, no cloud, nothing to pay." The README said "the only network calls this app ever makes are three narrow, optional things." Both are wrong — and PRIVACY.md already said so:

This is on by default in GitHub releases; you can turn it off at any time in your profile… In App Store and Play Store releases, this data collection does not occur.

Anonymous Firebase crash/performance diagnostics are on by default in GitHub release builds — which is what essentially every current user is running, since store builds don't exist yet. Never health data, absent from store builds, switchable off. Separately, opt-in health-data contribution does upload the local DB when enabled. Neither appeared in the README's list.

The health-data claim was always true and stands: nothing sends your health data anywhere unless you turn it on. But "no cloud" unqualified is precisely the kind of overclaim this project exists not to make. The entire pitch is metrics that admit what they don't know — overclaiming on privacy while doing that would be indefensible, and it's the first claim a hostile reader checks.

Hero now reads "no server that ever receives your health data", with the diagnostics exception stated in the card that makes the claim. README lists all five network paths, each marked required/optional and store-vs-GitHub.

Rejected

  • theme.dart — "add the missing Cupertino import" (flagged 🔴 Critical). CodeRabbit says material.dart no longer exports CupertinoPageTransitionsBuilder. True on Flutter 3.44, false on 3.41.6, which is what this repo pins and builds against — and 3.44 can't build this repo at all (phosphor_flutter extends IconData, now final). Re-adding the import breaks the pinned toolchain. CI is green; that's authoritative.

CI: ✅ analyze clean, 623 tests.

Second CodeRabbit pass on the privacy rewrite, and it caught a real soft
spot in my own wording.

The BYOK AI assistant bullet said only "your key, your account, prompts go
to whoever you chose" — which quietly implies the prompts are about health
rather than made of it. They are made of it: coach_prompt.dart hands the
model read access to v_metric (rhr, rmssd, sdnn, readiness, strain, sleep,
and the rest), because that is the only way it can answer "why is my
recovery low". Anyone enabling that is sending their metrics to OpenAI or
whoever they configured, and the README should say so in those words.

Also tightened the health-data-contribution line. It claimed to be "the
only thing here that does", which read as "the only thing that sends health
data" when what is actually true is narrower: it's the only thing that
sends the whole database rather than a slice. The AI assistant sends a
slice.

Separately, added a band dropdown to the wrong-number issue form, matching
the bug form. WHOOP 5.0/MG feeds a thinner record than 4.0, so a metric can
be genuinely wrong on one and fine on the other — without that field the
first reply is always "which band?".
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 94d04ad

@abdulsaheel
abdulsaheel merged commit 70654db into main Jul 26, 2026
3 checks passed
@abdulsaheel
abdulsaheel deleted the chore/public-facing branch July 26, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant