Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,20 @@ Swift, you MUST build the app yourself: `xcodebuild … build` locally, or run `
- **A gate must be able to fail on the change that caused it.** The same rule again, moved out to CI:
when a check cannot see what invalidates it, the failure lands on whoever pushes next and reads as
their fault. Three live instances, all hit in one day. The parity-governance path filter excludes
product source, so ordinary feature merges move the derived sets, the stored authority stops
reproducing, and nothing reports it until the 04:17 schedule fails against a commit that changed
product source, so ordinary feature merges moved the derived sets, the stored authority stopped
reproducing, and nothing reported it until the 04:17 schedule failed against a commit that changed
none of it; that needed hand repair twice in a day, and in between, a contributor running
`parity_ratchet.py --base upstream/main` gets an error belonging to main rather than to their branch.
`parity_ratchet.py --base upstream/main` got an error belonging to main rather than to their branch.
That half is now shut: the repository-acceptance suite, the one test in that workflow which is about
PRODUCT source rather than about the scanner, runs on the unfiltered `tools-python` leg, and a guard
in `test_core_tools_filter_covers_every_governance_tool_path` asserts it stays there. On the day it
landed, a fourth merge had already turned main red by orphaning a function into a test-only call site,
and three more PRs were then found drifting the authority before they merged, each with its own CI
green against a base that no longer existed. The scanner's own unit tests stay path-filtered,
which is correct: those genuinely are about the scanner. Note what the remedy was NOT. Naming product
paths in the filter only moves the trap, because the list is whatever the assertions happen to read
today, which is the #1691 lesson written into that same guard test after a `Tools/**` filter silenced
a suite reading `TodayScreen.kt` and main carried it red through nine merges.
The governance discovery floor is an exact count, so a legitimate future removal of one test prints
"discovery is broken, not the suite" on main against a change that removed nothing. And GitHub's
fork-PR approval gate re-arms on every force-push, parking workflows at `action_required`, which the
Expand Down
72 changes: 72 additions & 0 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ non-negotiable (especially on the Bluetooth path).
- [Ground rules](#ground-rules)
- [Contributor roles & the issue/PR workflow](#contributor-roles--the-issuepr-workflow)
- [Repository layout](#repository-layout)
- [The parity ledger](#the-parity-ledger-one-sided-declarations)
- [Build & test](#build--test)
- [The design system is the law](#the-design-system-is-the-law)
- [Coding conventions](#coding-conventions)
Expand Down Expand Up @@ -174,6 +175,70 @@ Do **not** add `import AppKit`/`import UIKit`/`import CoreBluetooth` to any file
that is what breaks the cross-platform contract. CoreBluetooth lives only in the macOS app's
`Strand/BLE`.

### The parity ledger (one-sided declarations)

The discipline above is also enforced mechanically. `Tools/parity_ledger.py` pairs declarations across
the two platforms and `Tools/parity_ratchet.py` refuses new **one-sided** ones that carry no typed
reason. It reads exactly these roots and nothing else:

| platform | in scope |
| --- | --- |
| Swift | `Packages/{StrandAnalytics,StrandImport,WhoopStore,WhoopProtocol,OuraProtocol}/Sources/**` |
| Kotlin | `android/app/src/main/java/com/noop/{analytics,ingest,data,protocol,oura}/**` |

Note the asymmetry, because it will bite you: the **app targets are out of Swift scope** (`Strand/`,
`StrandiOS/`), but `com/noop/data/**` is *in* Kotlin scope. So a Kotlin declaration can be in scope
while the Swift twin it genuinely has lives in an app target the scanner never reads. That gap is
tracked in #2567.

If you add, rename or delete a declaration under those roots, refresh the derived snapshots:

```bash
python3 Tools/parity_ledger.py --refresh-derived
```

This rewrites them only if the ratchet accepts the result, so a refusal is information, not an
obstacle to route around. One refusal in particular is **not yours to absorb**: if the *base* is
already stale, because some earlier PR moved product source without the gate running, add

```bash
python3 Tools/parity_ledger.py --refresh-derived --repair-stale-base
```

which is accepted only when product source, finding identities, counters and typed dispositions all
match the exact base. By construction it can only ever produce a metadata-only commit. Do **not**
reach for `--migrate-authority` instead: it waives base-manifest reproducibility, and choosing to
lose that is a maintainer call, not a way to get a red branch green.

If your declaration is legitimately one-sided, register it in `Tools/parity_dispositions.json`. Key
sets are exact, extras and omissions are both errors:

| `type` | extra keys | when |
| --- | --- | --- |
| `experimental` | `issue`, `reason`, `expires_on` | temporary, and it must name an issue and a date |
| `platform_specific` | `rationale` | the other platform genuinely cannot carry it |
| `out_of_scope_twin` | `rationale`, `twin_path` | the twin exists, but outside the roots above |

All three also need exact `kind`, `identity`, `identity_sha256` and `platform`, with a rationale of
real substance (a length floor is enforced, but write for the next reader, not the validator).
`kind` is limited to `add-unpaired-{file,function,property,constant}`: a disposition can never waive
a **shared-parity regression or a pair removal**, so if the ratchet names one of those, the answer is
to fix the code.

Two sharp edges on `out_of_scope_twin`: it is **Kotlin-only**, and its `twin_path` must both exist on
disk and match `Strand/…/*.swift`. A twin under `StrandiOS/` does **not** match today, since the
pattern wants a `/` directly after `Strand`, so ask a maintainer rather than mislabelling it
`platform_specific`.

Finally, know what CI does *not* promise you here. `parity-governance.yml` is path-filtered to
`Tools/parity_*`, yet what it measures is the declaration graph in **product** source, so it may not
run on the very PR that moves it. The repository-acceptance suite runs unfiltered on `tools-python`
to cover that, and the workflow additionally runs on a daily schedule against `main` so drift is
found there rather than on the next contributor's PR. Neither closes the window completely: a green
PR can still sit on a base that has since drifted, so rebase before you conclude anything from a
green run, and if `parity-governance` is red on `main` against a commit that changed none of it, that
is the schedule doing its job and a maintainer's repair to make.

---

## Build & test
Expand Down Expand Up @@ -496,6 +561,13 @@ to the Explore / Compare / tile UI. The catalog is the contract.
registered metric with data behind it appears automatically. No screen edits required.
4. **Add a test.** If a new importer/analyzer produces the series, cover the parse/compute in that
package's test suite.
5. **Refresh the parity ledger.** Step 1 lands in a scanned root (step 4 does not, since `Tests/`
is outside them), so run
`python3 Tools/parity_ledger.py --refresh-derived` and read
[The parity ledger](#the-parity-ledger-one-sided-declarations) if it refuses. Watch step 2 in
particular: `MetricCatalog.swift` sits in `Strand/`, which is *outside* Swift scope, while a
Kotlin catalog twin under `com/noop/data/**` is inside Kotlin scope. That pairing is the exact
asymmetry described there, not a real parity gap.

> **Verify the key in three places** before you push: what the importer/analyzer *writes*, the
> `MetricCatalog` `key`, and any SQL `WHERE key = …`. Mismatched keys are a known class of bug here —
Expand Down
Loading