chore: make the repo cloneable, add CI, drop the dead TypeScript port - #28
Merged
Conversation
Three things, all of which made this repo look and behave worse in public than the code in it deserves. 1. A fresh clone could not `pub get` at all. pubspec.yaml pinned openstrap_protocol to `path: ../openstrap-protocol-dart`, a directory name that no longer exists anywhere; it only ever resolved on one machine, via a gitignored pubspec_overrides.yaml. Anyone who cloned this repo hit a version-solve failure on the first command in the README. Now pinned to a protocol commit SHA over git, matching the convention edge already uses (SHAs, never floating branch refs). The package description was also still the pre-rewrite one, describing "analytics over minute rollups" — the backend-cron architecture that was replaced by the 1 Hz-native rewrite. 2. No CI. 290 tests and nothing ran them on a PR. Adds dart analyze + dart test on push and PR. Fixed one pre-existing unused import that would have made the first run red. 3. Removed src/ — 35 TypeScript files, ~286KB, last touched before the 1 Hz rewrite. GitHub counted it as 28% of a repo whose first line says "pure Dart". Its package.json also declared `"license": "ISC"` while LICENSE is MIT, which is a licensing contradiction sitting in public. It is the dead pre-1Hz backend port, it is not the parity oracle (that lives in protocol/ts), nothing references it, and git history keeps it. Also adds CONTRIBUTING.md, SECURITY.md, FUNDING.yml and README badges. Test count in the README corrected 282 -> 290.
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR removes the TypeScript analytics implementation and public exports, removes its tests and tooling configuration, and adds Dart-focused package metadata, CI validation, contribution and security documentation, funding details, and a pinned development dependency. ChangesAnalytics surface removal
Repository setup and governance
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced Jul 26, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
14-16: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the declared minimum Dart SDK in CI.
pubspec.yamldeclaressdk: ^3.5.0, but this workflow only testsstable; add a 3.5.x job or narrow the SDK constraint to versions actually covered by CI.🤖 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/workflows/test.yml around lines 14 - 16, Update the CI workflow’s Dart setup around dart-lang/setup-dart so it tests the declared minimum supported SDK, Dart 3.5.x, by adding a corresponding job or matrix entry while retaining stable coverage; alternatively, narrow the pubspec.yaml SDK constraint to match the versions tested by the workflow.
🤖 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/workflows/test.yml:
- Around line 21-23: Remove continue-on-error from the “Verify formatting” step
so dart format changes cause the workflow to fail; keep the existing formatting
command unchanged.
- Line 13: Harden the workflow job using actions/checkout@v4 by adding
permissions with contents: read and configuring persist-credentials: false. Keep
the existing checkout and analysis/test steps unchanged.
- Around line 13-16: Update the workflow steps using actions/checkout and
dart-lang/setup-dart to reference immutable commit SHAs instead of mutable
version tags, preserving their current action versions. Add persist-credentials:
false to the actions/checkout configuration unless this job explicitly requires
repository write-back.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 14-16: Update the CI workflow’s Dart setup around
dart-lang/setup-dart so it tests the declared minimum supported SDK, Dart 3.5.x,
by adding a corresponding job or matrix entry while retaining stable coverage;
alternatively, narrow the pubspec.yaml SDK constraint to match the versions
tested by the workflow.
🪄 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: 4eb9366b-e381-4829-b4f9-85138e36c6fe
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (45)
.github/FUNDING.yml.github/workflows/test.yml.gitignoreCONTRIBUTING.mdREADME.mdSECURITY.mdpackage.jsonpubspec.yamlsrc/__tests__/_harness.tssrc/__tests__/analytics.test.tssrc/activity.tssrc/advanced.tssrc/arousal.tssrc/baselines.tssrc/calories.tssrc/circadian.tssrc/coach.tssrc/cycle.tssrc/cycles.tssrc/fitness.tssrc/har.tssrc/hrv.tssrc/illness.tssrc/index.tssrc/nocturnal.tssrc/notify.tssrc/readiness.tssrc/readiness_index.tssrc/recovery.tssrc/regularity.tssrc/resting.tssrc/restlessness.tssrc/sessions.tssrc/sleep.tssrc/spo2.tssrc/steps.tssrc/strain.tssrc/stress.tssrc/trends.tssrc/types.tssrc/util.tssrc/wake.tssrc/zones.tstest/onehz/real_night_cardio_stager_test.darttsconfig.json
💤 Files with no reviewable changes (38)
- package.json
- tsconfig.json
- src/spo2.ts
- src/calories.ts
- src/tests/_harness.ts
- src/index.ts
- src/zones.ts
- src/nocturnal.ts
- test/onehz/real_night_cardio_stager_test.dart
- src/restlessness.ts
- src/sessions.ts
- src/recovery.ts
- src/fitness.ts
- src/trends.ts
- src/regularity.ts
- src/readiness_index.ts
- src/stress.ts
- src/resting.ts
- src/cycle.ts
- src/coach.ts
- src/arousal.ts
- src/util.ts
- src/baselines.ts
- src/tests/analytics.test.ts
- src/sleep.ts
- src/illness.ts
- src/activity.ts
- src/circadian.ts
- src/cycles.ts
- src/readiness.ts
- src/steps.ts
- src/wake.ts
- src/types.ts
- src/notify.ts
- src/strain.ts
- src/har.ts
- src/hrv.ts
- src/advanced.ts
Comment on lines
+13
to
+16
| - uses: actions/checkout@v4 | ||
| - uses: dart-lang/setup-dart@v1 | ||
| with: | ||
| sdk: stable |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workflow ==\n'
nl -ba .github/workflows/test.yml | sed -n '1,200p'
printf '\n== pubspecs ==\n'
for f in pubspec.yaml pubspec.yml; do
if [ -f "$f" ]; then
echo "--- $f ---"
nl -ba "$f" | sed -n '1,120p'
fi
done
printf '\n== search for checkout/setup-dart/permissions/formatter ==\n'
rg -n "actions/checkout|setup-dart|continue-on-error|permissions:|dart format|pubspec" .github/workflows . -g '!**/.git/**'Repository: OpenStrap/analytics
Length of output: 213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/test.yml')
print(p.exists())
PYRepository: OpenStrap/analytics
Length of output: 162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files .github/workflows/test.yml pubspec.yaml pubspec.yml | catRepository: OpenStrap/analytics
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "workflow:"
nl -ba .github/workflows/test.yml | sed -n '1,120p'
echo
echo "pubspec:"
nl -ba pubspec.yaml | sed -n '1,80p'Repository: OpenStrap/analytics
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/test.yml =="
sed -n '1,220p' .github/workflows/test.yml
echo
echo "== pubspec.yaml =="
sed -n '1,140p' pubspec.yamlRepository: OpenStrap/analytics
Length of output: 2191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow action refs and formatter settings =="
grep -nE 'actions/checkout|setup-dart|persist-credentials|continue-on-error|dart format|permissions:' .github/workflows/test.yml || trueRepository: OpenStrap/analytics
Length of output: 393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path in [Path('.github/workflows/test.yml'), Path('pubspec.yaml')]:
print(f'--- {path} ---')
with path.open() as f:
for i, line in enumerate(f, 1):
if i > 220:
break
print(f'{i:4d}: {line.rstrip()}')
PYRepository: OpenStrap/analytics
Length of output: 2548
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
text = Path('.github/workflows/test.yml').read_text()
for pat in ['actions/checkout', 'setup-dart', 'persist-credentials', 'continue-on-error', 'permissions:']:
print(f'PATTERN {pat}:', bool(re.search(pat, text)))
PYRepository: OpenStrap/analytics
Length of output: 308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import yaml
data = yaml.safe_load(Path('.github/workflows/test.yml').read_text())
print(data.get('permissions'))
print(data.get('jobs', {}).keys())
PYRepository: OpenStrap/analytics
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/test.yml =="
sed -n '1,220p' .github/workflows/test.yml
echo
echo "== key workflow lines =="
grep -nE 'actions/checkout|setup-dart|persist-credentials|continue-on-error|permissions:' .github/workflows/test.yml || true
echo
echo "== pubspec.yaml =="
sed -n '1,120p' pubspec.yamlRepository: OpenStrap/analytics
Length of output: 2330
Pin workflow actions and disable credential persistence. actions/checkout@v4 and dart-lang/setup-dart@v1 are mutable tags; pin both to immutable commit SHAs. Add persist-credentials: false to actions/checkout unless this job needs to push back to the repo.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 13-13: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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/workflows/test.yml around lines 13 - 16, Update the workflow steps
using actions/checkout and dart-lang/setup-dart to reference immutable commit
SHAs instead of mutable version tags, preserving their current action versions.
Add persist-credentials: false to the actions/checkout configuration unless this
job explicitly requires repository write-back.
Comment on lines
+21
to
+23
| - name: Verify formatting | ||
| run: dart format --output=none --set-exit-if-changed lib test | ||
| continue-on-error: true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the formatting check enforceable.
continue-on-error: true allows the workflow to pass when formatting changes are detected. Remove it, or explicitly label this step as informational.
Proposed fix
- name: Verify formatting
run: dart format --output=none --set-exit-if-changed lib test
- continue-on-error: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Verify formatting | |
| run: dart format --output=none --set-exit-if-changed lib test | |
| continue-on-error: true | |
| - name: Verify formatting | |
| run: dart format --output=none --set-exit-if-changed lib test |
🤖 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/workflows/test.yml around lines 21 - 23, Remove continue-on-error
from the “Verify formatting” step so dart format changes cause the workflow to
fail; keep the existing formatting command unchanged.
Same three changes as the protocol PR. Workflow hardening (zizmor `artipacked`): `permissions: contents: read` plus `persist-credentials: false`. This job runs `dart analyze`/`dart test` on PR-authored code and never pushes back, so it has no business holding a writable token in .git/config. Dropped the "Verify formatting" step. It had `continue-on-error: true`, so it could never fail — a check that cannot fail is worse than no check, because the green tick implies coverage it doesn't have. Enforcing it isn't currently possible either: `dart format` would rewrite 51 of 79 files. That's a worthwhile change but it's a large mechanical diff that would churn every open PR, so it belongs on its own. SECURITY.md overstated the privacy boundary — "computes and stores everything on-device" reads as absolute, and Firebase diagnostics are on by default in GitHub release builds (never health data, absent from store builds), with health-data contribution uploading the local DB when explicitly enabled. Both were already in PRIVACY.md; the security policy now matches it. Not applied — pinning actions to commit SHAs. Sound in principle, but with no Dependabot config to keep them current, pinned SHAs rot into known-vulnerable versions that nobody notices, which is a worse failure mode than tracking a maintained major tag. Worth revisiting together with a Dependabot config.
Contributor
Author
CodeRabbit triage — 2 applied, 1 rejected, 1 resolved differentlyApplied
Resolved differently
Rejected
CI: ✅ 284 pass + 6 skip (fixture-dependent), analyze clean. |
pubspec declares `sdk: ^3.5.0`, which is a promise this package makes to anyone depending on it, but CI only ever ran whatever `stable` happened to be that week. So the lower bound was an untested guess. (CodeRabbit nitpick, and a fair one.) Adds a matrix over ['3.5.0', 'stable'] with fail-fast: false, so a break on the floor is distinguishable from a break on current. If 3.5.0 turns out not to work, the honest fix is to raise the constraint to what does — but either way it should be measured rather than assumed.
The SDK matrix added in the previous commit did its job immediately: this package does NOT pass on the Dart 3.5.0 it claims to support, while protocol (same declared constraint) does. The cause isn't a language incompatibility, which is the interesting part. Dart 3.5.0's analyzer reports `deprecated_member_use_from_same_package` where current stable doesn't, and CI runs `--fatal-infos`, so six intentional usages became six build failures. The usages are correct: sleep_test covers `autonomicStager` and human_test covers `glassBoxReadiness`, both deprecated but still EXPORTED, and dropping coverage of shipped API because it's deprecated would be backwards. So the notices are suppressed at the six sites rather than the tests changed. sleep_test uses a file-level ignore because the usage there is inside a multi-line `show` clause, where an inline ignore anchors unreliably across analyzer versions; human_test uses three inline ignores. Both carry a comment saying the deprecation is deliberate. `^3.5.0` is now a tested claim rather than an assumed one.
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.
Part of a pass over the org's public surface. Sibling PRs: OpenStrap/protocol#17, OpenStrap/edge, OpenStrap/.github, OpenStrap/backend.
1. A fresh clone couldn't
pub get— at allThat path is from the old
whoop-masterlayout. It resolved on exactly one machine, via a gitignoredpubspec_overrides.yaml. Anyone else who cloned this repo hit a version-solve failure on the first command in the README:Now pinned to a protocol commit SHA over git, matching the convention
edgealready uses (SHAs, never floating branch refs, for the reason documented in edge's pubspec). Verified against a clean clone with the override deleted: resolves, 284 pass + 6 skip.Also fixed the package
description:, which still described "analytics over minute rollups" — the backend-cron architecture the 1 Hz-native rewrite replaced.2. No CI
290 tests, nothing running them on a PR. Adds
dart analyze+dart test. One pre-existing unused import would have made the first run red; fixed.3. Removed the dead
src/TypeScript35 files, ~286KB, last touched before the 1 Hz rewrite. Three problems, all public-facing:
package.jsondeclared"license": "ISC"whileLICENSEis MIT — a licensing contradiction sitting in a public repo.It is not the parity oracle — that's
protocol/ts, which this PR's sibling keeps and marks vendored. Nothing referencessrc/, and git history retains it.Also
CONTRIBUTING.md(leading with the two rules: never fabricate a value when the input is absent, and cite the published method),SECURITY.md,FUNDING.yml, README badges.pubspec_overrides.yamladded to.gitignoreso the local-dev override can't be committed again.README test count corrected 282 → 290.
Risk
No
lib/logic changes — 290 tests still pass, unchanged. The dependency change is dev-only (openstrap_protocolis a dev dependency; nothing underlib/imports it, which is what keeps this package zero-runtime-dep).Summary by CodeRabbit
Documentation
Chores
Tests
Breaking Changes