chore: CI, contributor docs, and a Dart-only language bar - #17
Merged
Conversation
Adds the things a public repo needs and this one was missing. CI: there was none. 71 tests existed and nothing ran them on a PR — the only automation anywhere in the org was a release build triggered by a version tag. Adds dart analyze + dart test on every push and PR. The golden capture (whoop_hist.jsonl) is a real band recording kept beside the repo rather than committed to it, so decoder_test hard-failed with a StateError whenever it was absent — which is exactly what CI is. Those cases now SKIP instead, matching what the analytics suite already did. Locally: 71 pass. Without the fixture: 67 pass, 4 skip, green. The 2934-case parity oracle and the 550 dart_header cases are tracked in-repo and run either way, so CI still covers the decoders properly. .gitattributes marks ts/ vendored. It is the frozen oracle that generated those fixtures and is worth keeping, but GitHub was counting it as 13% of the package and the README says "pure Dart, zero deps". Now the language bar says what the package actually is, without deleting the oracle. Also adds CONTRIBUTING.md (which repo a change belongs in, and how to report a protocol finding in a way that can be checked), SECURITY.md for private disclosure, FUNDING.yml, and README badges. Test count in the README corrected 70 -> 71.
|
Warning Review limit reached
Next review available in: 45 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 (1)
📝 WalkthroughWalkthroughRepository governance and validation updates add Dart CI automation, make an optional golden fixture non-blocking, document contribution and security processes, update README project information, and configure repository language-reporting and funding metadata. ChangesRepository maintenance
Estimated code review effort: 2 (Simple) | ~10 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
🤖 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:
- Line 13: Update the actions/checkout@v4 step in the test workflow to set
persist-credentials to false, and add workflow permissions granting contents
read access.
- Around line 3-7: Update the workflow trigger configuration under on.push to
remove the branches filter, ensuring the workflow runs on every push while
preserving the existing pull_request and workflow_dispatch triggers.
In `@SECURITY.md`:
- Around line 44-48: Update the SECURITY.md paragraph around the “computes and
stores everything on-device” statement to qualify it as applying to
default/store builds. Explicitly state that data can leave the device only when
users opt in to the companion worker’s telemetry or health-upload features,
while preserving the existing details about legacy import, update pointers, and
features being disabled by default.
🪄 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: bbc969f4-67d5-4d88-807f-7285b6922ffc
📒 Files selected for processing (7)
.gitattributes.github/FUNDING.yml.github/workflows/test.ymlCONTRIBUTING.mdREADME.mdSECURITY.mdtest/decoder_test.dart
Comment on lines
+3
to
+7
| on: | ||
| push: | ||
| branches: [main] | ||
| pull_request: | ||
| workflow_dispatch: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' .github/workflows/test.ymlRepository: OpenStrap/protocol
Length of output: 1080
Run CI on every push, not only main.
push.branches: [main] skips direct pushes to feature branches. Drop the branch filter if this workflow should run for every push.
🤖 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 3 - 7, Update the workflow trigger
configuration under on.push to remove the branches filter, ensuring the workflow
runs on every push while preserving the existing pull_request and
workflow_dispatch triggers.
Harden the workflow (zizmor `artipacked`): add `permissions: contents: read` and `persist-credentials: false`. The job builds and tests PR-authored code and never pushes back, so leaving GITHUB_TOKEN in .git/config was a real if small exposure. Dropped the "Verify formatting" step rather than keep it. It carried `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. Making it enforcing isn't an option right now either: `dart format` would rewrite 12 of 16 files. Adopting dart format is a reasonable thing to do, but it's a large mechanical diff that would churn every open PR, so it belongs in its own change, not smuggled in here. SECURITY.md overstated the privacy boundary. It said OpenStrap "computes and stores everything on-device", which reads as absolute and isn't: anonymous Firebase crash/performance diagnostics are ON BY DEFAULT in GitHub release builds (absent from store builds, never health data), and health-data contribution uploads the local database when explicitly enabled. Both were already documented in PRIVACY.md — SECURITY.md just didn't reflect them. Now states the boundary exactly. This project's whole pitch is not overclaiming, so the security policy shouldn't either. Not applied — `push: branches: [main]`. CodeRabbit read the branch filter as skipping feature branches, but `pull_request` already covers them; dropping it would just double every PR run.
Contributor
Author
CodeRabbit triage — 2 applied, 1 rejected, 1 resolved differentlyApplied
Resolved differently
Rejected
CI: ✅ 67 pass + 4 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. 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.
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/analytics, OpenStrap/edge, OpenStrap/.github, OpenStrap/backend.
Why
This repo had no CI at all. 71 tests existed and nothing ran them on a pull request — the only automation anywhere in the org was
edge's tag-triggered release build. For a package whose whole job is "these bytes mean this", an untested PR is the one thing you can least afford.What's here
CI —
dart analyze+dart teston every push and PR.Making that green needed one fix.
decoder_test.dartthrew aStateErrorwhenwhoop_hist.jsonlwas missing, and it's always missing in CI: it's a real band capture kept beside the repo rather than committed to it. Those cases now skip, matching what the analytics suite already did.The 2934-case parity oracle and the 550
dart_header.jsoncases are tracked in-repo and run either way, so CI still covers the decoders properly..gitattributesmarksts/aslinguist-vendored. It's the frozen oracle that generated those fixtures and is worth keeping, but GitHub was reporting the package as 13% TypeScript while the README's first line says "pure Dart, zero runtime deps". Now the language bar matches the claim, without deleting the oracle.Contributor docs —
CONTRIBUTING.md,SECURITY.md,FUNDING.yml, README badges.CONTRIBUTING.mdleads with the thing that actually blocks people (which of the three repos a change belongs in), and has a section on reporting a protocol finding in a form that can be checked — raw bytes, proposed offset, and how you convinced yourself. That last part matters more than a unit test for byte-level work.README test count corrected 70 → 71.
Risk
Low. The only non-doc change is test-harness behaviour; no decoder logic is touched.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes