Skip to content

analytics(android): Kotlin LiftMetrics twin of the Swift engine (#2099) - #2232

Merged
ryanbr merged 3 commits into
mainfrom
liftmetrics-kotlin-twin
Sep 15, 2026
Merged

ryanbr merged 3 commits into
mainfrom
liftmetrics-kotlin-twin

Conversation

@ryanbr

@ryanbr ryanbr commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Lift Log (#2099) shipped its analytics on Swift only. This adds the Kotlin twin,
and two re-review passes over it found work worth doing.

What is here

LiftMetrics.kt mirrors StrandAnalytics.LiftMetrics: volumeLoadKg,
sessionLoad, estimatedOneRepMaxKg, perExercise, rpeProfile,
muscleCounts, plus the ReferenceDose constants.

LiftMuscle.kt carries the 20 muscle tokens in Swift's order and spelling,
compared mechanically rather than by eye. The encoded list crosses a .noopbak
boundary, so a reordering would silently remap stored data on restore.

Two oracle tests follow the CLAUDE.md method: the Swift side was compiled
standalone with swiftc -O and its stdout pasted in verbatim. No expected value
in either test was written by hand.

A parity bug the second re-review found

Swift's LiftSetRow.init normalises a row's secondary list at construction:

self.secondaryMuscles = LiftMuscle.decodeList(
    LiftMuscle.encodeList(secondaryMuscles, excluding: primaryMuscle))

so LiftMetrics never sees a repeated muscle on that side. The Kotlin Row
carried the list through raw, and muscleCounts strips only the primary, never
duplicates. A set naming a muscle twice was counted twice for it. Measured on
both sides, for one working set with primary chest and secondaries
[triceps, triceps, chest]:

fractional triceps indirect sets
Swift 0.500000 1
Kotlin, before the fix 1.000000 2

Double fractional credit, compared against reference doses derived under the
fractional method, so the comparison those doses exist for stops holding.

Row now applies the same decode-of-encode at construction, where Swift applies
it, so any later reader sees the list Swift would rather than only muscleCounts
being patched. It is no longer a data class, because the generated copy()
bypasses the constructor and would hand back an unnormalised row.

The first oracle was blind to this by construction. Its Swift stub declared
LiftSetRow as a plain memberwise struct, modelling LiftMetrics in isolation
rather than how it is actually fed, so the first pass mutation-tested the wrong
thing and passed. The stub now carries the real init's normalisation verbatim,
the fixture gains a Dip row naming triceps three times plus the primary, and a
normalisedSecondaries block pins the per-row list itself so a regression names
the cause rather than a stray sum.

The stored-data codec had no test

LiftMuscle.encodeList and decodeList had zero Kotlin coverage, while both
now claim a twin, which made the ledger treat them as guarded when nothing
guarded them. They are the one part of the Lift Log that is a stored-data
contract, so a divergence is silent corruption on restore rather than a wrong
number on a screen.

LiftMuscleParityOracleTest pins them where the two languages genuinely differ:

  • Swift's split(separator:) omits empty subsequences; Kotlin's split(",")
    keeps them. "chest,,biceps", ",", "chest," and ",chest" pin that the
    two agree anyway, an empty token resolving to no case on either side. That was
    reasoned to be safe when the twin was written; now it is measured.
  • decoding does not deduplicate on either side while encoding does, so
    "chest,chest" pins the asymmetry as deliberate
  • tokens match exactly: " chest" is not trimmed, "CHEST" is not case-folded

Two smaller findings from the same passes

  • Both oracle tests formatted doubles with the default locale, so they would
    fail for anyone running the suite on a comma-decimal machine while passing in
    CI. Pinned to Locale.ROOT.
  • perExercise assumes ord is unique. It is, because ord is 0-based within
    a session and the only caller passes one session's sets. Recorded in the
    source because if that changes the platforms disagree on ties: Kotlin's
    sortedBy is stable where Swift's sorted(by:) is not, so Swift would also
    stop agreeing with itself. The fix then is to make ord unique, not to
    imitate an unspecified order.

Verification

  • both oracles green under --no-build-cache, result XML mtime checked against
    the clock each time
  • mutation-tested, each turning the relevant oracle red: dropping Row's
    normalisation, adding .distinct() to decodeList, removing the primary
    exclusion in encodeList, and inverting the maxWithOrNull comparator
  • the Swift side of this diff is 12 /// lines and zero code lines

Ledger effect, measured

Measured by running the ledger on a clean worktree and on this branch at the
same base, then set-diffing requirement identities rather than comparing totals
between runs. Re-measured at branch HEAD after the Row change, not carried
over from the first commit:

kind main this branch
add-unpaired-function 32 27
add-unpaired-constant 24 19
add-unpaired-file 3 2
add-unpaired-property 2 2
add-finding 1 5
total 62 55

Twelve Swift-side requirements retire: six functions, five constants, and the
file entry, which follows from the function pairs. Five arrive: fromRaw,
Kotlin-only by design because Swift gets the equivalent from
RawRepresentable.init(rawValue:), and four test-only-callsite findings.

Twin claims are directional, which is the part that is easy to miss. Claims
written only in the Kotlin files retired Kotlin-side entries and left all twelve
Swift-side entries standing; the reciprocal claims in LiftMetrics.swift are
what retired those. The same asymmetry CLAUDE.md notes for oracles.

The five that arrive fold into the #2229 authority decision rather than being
declarable here.

What this does not do

Android has LiftEntities.kt and LiftingImporter.kt but no Lift Log screen,
so nothing in the app calls this engine yet. volumeLoadKg and
estimatedOneRepMaxKg avoid the test-only-callsite finding only because
perExercise calls them internally, which is a fact about the call graph rather
than evidence they are wired up. The engine is deliberately ported ahead of its
consumer, and this does not make the board green.

Lift Log shipped its analytics on Swift only. This adds the Kotlin twin so
both sides compute identically, with an oracle test whose expected values are
the Swift build's own stdout pasted verbatim rather than hand-written.

LiftMuscle.kt carries the 20 muscle tokens in Swift's order and spelling. That
matters because the encoded list crosses a .noopbak boundary, so a reordering
would silently remap stored data.

Reciprocal twin claims on both sides let the parity ledger pair the
declarations. Measured against a clean worktree at the same base, total drift
goes from 62 to 55: twelve Swift-side requirements retire, five arrive.

The five that arrive fold into the #2229 authority decision:
  * fromRaw, Kotlin-only by design, since Swift gets the same thing free from
    RawRepresentable.init(rawValue:)
  * four test-only-callsite findings, because Android has Room lift entities
    and an importer but no Lift Log screen, so nothing in the app calls the
    engine yet

That last point is deliberate and worth stating plainly: the engine is ported
ahead of its consumer. It is verified against Swift, and it is not yet wired
to any Android surface.
Re-review of the twin found `encodeList` and `decodeList` had no Kotlin test at
all. They are the one part of the Lift Log that is a stored-data contract: the
output lands in the `secondaryMuscles` column and crosses the .noopbak boundary
to an Apple install, so a divergence is silent corruption on restore rather than
a wrong number on a screen. Both also now claim a twin, which makes the ledger
treat them as guarded when nothing guarded them.

`LiftMuscleParityOracleTest` pins them against Swift by the same oracle method,
including the cases where the two languages genuinely differ:

  * Swift's split(separator:) omits empty subsequences and Kotlin's split(",")
    keeps them, so "chest,,biceps", ",", "chest," and ",chest" pin that the two
    agree anyway, an empty token resolving to no case on either side. That was
    reasoned to be safe when the twin was written; now it is measured.
  * decoding does not deduplicate on either side while encoding does, so
    "chest,chest" pins the asymmetry as deliberate
  * tokens match exactly: " chest" is not trimmed, "CHEST" is not case-folded

Mutation-tested rather than assumed: adding .distinct() to decodeList turns it
red, and dropping the primary exclusion in encodeList turns it red.

Two more from the same pass:

  * both oracle tests formatted doubles with the default locale, so they would
    fail for anyone running the suite on a comma-decimal machine while passing
    in CI. Pinned to Locale.ROOT.
  * perExercise assumes `ord` is unique. It is, because ord is 0-based within a
    session and the only caller passes one session. Recorded in the source
    because if that changes the platforms disagree on ties, Kotlin's sortedBy
    being stable where Swift's sorted(by:) is not.
… does

Second re-review pass found a real parity bug, not a style point.

Swift's `LiftSetRow.init` normalises the secondary list at construction:

    self.secondaryMuscles = LiftMuscle.decodeList(
        LiftMuscle.encodeList(secondaryMuscles, excluding: primaryMuscle))

so `LiftMetrics` on that side never sees a repeated muscle. `LiftMetrics.Row`
carried the list through raw, and `muscleCounts` strips only the primary, never
duplicates. A set naming a muscle twice was therefore counted twice for it.

Measured on both sides rather than argued. For one working set with primary
`chest` and secondaries `[triceps, triceps, chest]`:

| | fractional triceps | indirect sets |
|---|---|---|
| Swift | 0.500000 | 1 |
| Kotlin, before | 1.000000 | 2 |

Double fractional credit for that muscle, against reference doses derived under
the fractional method, so the comparison the doses exist for stops holding.

`Row` now applies the same decode-of-encode at construction, which is where
Swift applies it, so any later reader of `secondaryMuscles` sees the same list
Swift would rather than only `muscleCounts` being patched. It is no longer a
`data class`: the generated `copy()` bypasses the constructor and would hand
back an unnormalised row.

The oracle was blind to this by construction. Its Swift stub declared
`LiftSetRow` as a plain memberwise struct, so it modelled `LiftMetrics` in
isolation rather than how it is actually fed. The stub now carries the real
init's normalisation line verbatim, the fixture gains a `Dip` row naming
triceps three times plus the primary, and a `normalisedSecondaries` block pins
the per-row list itself so a regression names the cause rather than a stray sum.

Mutation-tested: dropping the normalisation turns the oracle red.
@ryanbr
ryanbr merged commit eb34f3c into main Sep 15, 2026
16 checks passed
@ryanbr
ryanbr deleted the liftmetrics-kotlin-twin branch September 15, 2026 09:35
UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Sep 15, 2026
… LiftMetrics twin

The Swift side of this change leaves a set with zero reps (how a discarded set
is saved) out of every figure. ryanbr#2232 made LiftMetrics.kt its twin, so it has to
follow: `isPerformed` is added with the same rule, `reps != 0`, and
`perExercise`, `rpeProfile` and `muscleCounts` skip such a set exactly as Swift
does. A set with no rep count still counts on both.

The oracle fixture gains a bench set at zero reps that carries muscles and an
RPE, so a port that skips the rule in `muscleCounts` or `rpeProfile` shows up
as an extra chest set or rating rather than only a missing row, and an
`isPerformed` section. The expected block is the stdout of the Swift build,
regenerated from the real StrandAnalytics and WhoopStore packages rather than a
stub; every section this change does not touch came back byte-identical to
the previous oracle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Sep 15, 2026
…-up rebased with the Kotlin twin (a7f2ad0), build 775a181, next PR drafted
ryanbr added a commit that referenced this pull request Sep 15, 2026
)

`parity-governance` had been red on `main` since Lift Log landed, and the
documented recovery refused. This makes the recovery work, then runs it.
`RepositoryBaselineTests` goes from two failures to ten passes.

## The remedy

The refusal on a stale base said "migration required" and no migration existed.
`--migrate-authority` re-bases the comparison onto a freshly derived base
authority when the base's stored manifest cannot be reproduced.
`--repair-stale-base` cannot serve that case by design: repair is for a base
whose governed state is unchanged, which a tree carrying merged product files is
not.

Migration waives one thing, the reproducibility of the base's stored manifest.
It waives no semantic debt: new one-sided declarations are still computed from
the freshly derived base and current sets, so an undeclared one still fails
closed. That guard is mutation-proven, and the refusal now names the flag.

## Corrections to what #2229 recorded

  * `--bootstrap-map --write-baseline` was never an available lever. It refuses
    when an authority exists.
  * it could not have discarded reviewed dispositions in any case. No refresh
    path writes `parity_dispositions.json`, which the governance doc already
    said.
  * the "55 requirements" were an artifact of the base I measured against.
    Against `origin/main` the delta is zero, so there was nothing to
    disposition, and the "decide which spreadsheet internals are Apple-only"
    step was never a prerequisite for a green board.

## What the refresh adopts

Roughly 56 one-sided declarations already on `main` enter the authority
undispositioned, because the ratchet governs debt relative to base and merged
debt is the base. The same maintenance action as fe0f5c9 after the Oura
merges. Four of the accepted findings come from code I wrote in #2232.

## Two tests that were wrong

The hand-edited-authority test passed for the wrong reason: it tampered in a way
the pre-existing guard already caught, so the migration check went unexercised.
The sharp shape, a current authority equal to the STALE base, only warns under
the general guard and would otherwise adopt an unrefreshed authority. Pinned.

The baseline was first derived in a working tree carrying gitignored `.build/`
directories, which supply a callsite the repository does not have. That recorded
43 test-only findings for `StrandAnalytics` where the repository has 44. CI
caught it after three local re-review passes could not, because all three ran
against the same wrong tree. Re-derived in a pristine worktree.

Governance job: 122 tests, floor raised 115 to 122, measured rather than
incremented.
UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Sep 15, 2026
… LiftMetrics twin

The Swift side of this change leaves a set with zero reps (how a discarded set
is saved) out of every figure. ryanbr#2232 made LiftMetrics.kt its twin, so it has to
follow: `isPerformed` is added with the same rule, `reps != 0`, and
`perExercise`, `rpeProfile` and `muscleCounts` skip such a set exactly as Swift
does. A set with no rep count still counts on both.

The oracle fixture gains a bench set at zero reps that carries muscles and an
RPE, so a port that skips the rule in `muscleCounts` or `rpeProfile` shows up
as an extra chest set or rating rather than only a missing row, and an
`isPerformed` section. The expected block is the stdout of the Swift build,
regenerated from the real StrandAnalytics and WhoopStore packages rather than a
stub; every section this change does not touch came back byte-identical to
the previous oracle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Sep 16, 2026
`WhoopStore.deleteLiftSets`, which editing a finished session calls when a set
is removed, was Swift-only. The parity ratchet names that as a new one-sided
declaration, and `PARITY_GOVERNANCE.md` asks for the twin rather than debt: a
disposition cannot settle `add-unpaired-function` anyway (ryanbr#2163).

`DeviceRegistryDao` already holds the lift tables' delete queries, so the
by-id delete sits with them. It is ported ahead of its consumer, as ryanbr#2232 did
for `LiftMetrics.kt`: Android has no Lift Log screen yet, so nothing calls it.
Both sides now declare the pair in their doc comments.

Measured, not assumed: with this the ratchet reports no error and the ledger no
new finding, so the branch needs no authority refresh and touches neither
`parity_twin_map.json` nor `parity_ledger_baseline.json`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Sep 16, 2026
`WhoopStore.deleteLiftSets`, which editing a finished session calls when a set
is removed, was Swift-only. The parity ratchet names that as a new one-sided
declaration, and `PARITY_GOVERNANCE.md` asks for the twin rather than debt: a
disposition cannot settle `add-unpaired-function` anyway (ryanbr#2163).

`DeviceRegistryDao` already holds the lift tables' delete queries, so the
by-id delete sits with them. It is ported ahead of its consumer, as ryanbr#2232 did
for `LiftMetrics.kt`: Android has no Lift Log screen yet, so nothing calls it.
Both sides now declare the pair in their doc comments.

Measured, not assumed: with this the ratchet reports no error and the ledger no
new finding, so the branch needs no authority refresh and touches neither
`parity_twin_map.json` nor `parity_ledger_baseline.json`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ryanbr added a commit that referenced this pull request Sep 19, 2026
#2328)

docs(changelog): say which platforms the 11.8.0 lift log actually shipped on

An Android user went looking for the gym log book the release notes announced and
could not find it. They were right that there is no entry point: #2099 added the
Lift Log to Strand only. Android got #2098's schema and Room twin and #2232's
LiftMetrics twin, which is the storage and the maths, and no UI at all.

The entry said:

    An on-device gym log book: build a session, then move between sets with a
    double-tap on the strap instead of reaching for the phone. Stored in a new
    schema with a matching Room twin, and the set metrics are computed by the same
    engine on both platforms.

Every clause is true, which is what made it misleading rather than wrong. Both
platform phrases in it, "a matching Room twin" and "on both platforms", attach to
the two halves Android DID get, so on Android the whole thing reads as an
announcement of a feature you have. The same text ships in the in-app changelog on
both platforms and in the release doc.

Corrected in all four places it appears: the two AppChangelog copies, and the item
plus the prose section in docs/releases/v11.8.0.md. It now names iPhone and Mac for
the log book, says plainly that Android has the groundwork and not the book, and
points at #2327, which tracks the Android UI.

Platform-scoping an entry is already the house convention: "iPhone only",
"Android only" and "(iPhone and Mac)" all appear in existing entries.

The release HEADLINE is deliberately unchanged. Its localized key is derived from
its own English text (sha1("A gym log book on your wrist, ...")[:8] = e0f00272),
so rewording it would re-key the string and orphan the translation in all eight
Android locales plus the xcstrings side, for a headline that is not itself false:
the release did ship a gym log book, on two of the three platforms.

Changelog item strings are not localized (only the release title is), so this needs
no translation work. No behavioural change.
UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Sep 21, 2026
… LiftMetrics twin

The Swift side of this change leaves a set with zero reps (how a discarded set
is saved) out of every figure. ryanbr#2232 made LiftMetrics.kt its twin, so it has to
follow: `isPerformed` is added with the same rule, `reps != 0`, and
`perExercise`, `rpeProfile` and `muscleCounts` skip such a set exactly as Swift
does. A set with no rep count still counts on both.

The oracle fixture gains a bench set at zero reps that carries muscles and an
RPE, so a port that skips the rule in `muscleCounts` or `rpeProfile` shows up
as an extra chest set or rating rather than only a missing row, and an
`isPerformed` section. The expected block is the stdout of the Swift build,
regenerated from the real StrandAnalytics and WhoopStore packages rather than a
stub; every section this change does not touch came back byte-identical to
the previous oracle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Sep 21, 2026
`WhoopStore.deleteLiftSets`, which editing a finished session calls when a set
is removed, was Swift-only. The parity ratchet names that as a new one-sided
declaration, and `PARITY_GOVERNANCE.md` asks for the twin rather than debt: a
disposition cannot settle `add-unpaired-function` anyway (ryanbr#2163).

`DeviceRegistryDao` already holds the lift tables' delete queries, so the
by-id delete sits with them. It is ported ahead of its consumer, as ryanbr#2232 did
for `LiftMetrics.kt`: Android has no Lift Log screen yet, so nothing calls it.
Both sides now declare the pair in their doc comments.

Measured, not assumed: with this the ratchet reports no error and the ledger no
new finding, so the branch needs no authority refresh and touches neither
`parity_twin_map.json` nor `parity_ledger_baseline.json`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant