diff --git a/AGENTS.md b/AGENTS.md index b58b8fec01..9c405ef56e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 4b6f001bc3..4acd488010 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -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) @@ -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 @@ -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 —