Skip to content

docs: R7 tells contributors to normalise stage strings, but the tree deliberately uses a predicate #2363

Closed

Description

@ryanbr

docs/VALIDATION_PROTOCOL.md R7 ends with three prescriptions for stage strings:

Normalise at the boundary, assert the closed set in a test, and never let a stage string be compared by literal in more than one place.

The first one points away from what the tree deliberately does. SleepStageVocabulary is a predicate, not a canonicaliser, and says why:

A PREDICATE, deliberately, not a canonicaliser: it fixes the comparisons without rewriting any stored string, so no persisted hypnogram changes meaning and neither vocabulary above moves.

The two vocabularies are both legitimate and both persisted. Segment stage strings (StageSegment.stage, hypnogram rows) canonicalise to wake; minutes-dictionary keys (SleepStageTotals, SleepWindowReclip) canonicalise to awake. Normalising at the boundary would mean rewriting one of them, which changes the meaning of stored data to make a comparison tidier.

So a contributor who follows R7 literally would undo a considered decision, and the protocol gives them no way to know that.

What to change

A clause in R7 acknowledging the case: where two vocabularies are legitimate and persisted, the rule is satisfied by ONE shared predicate rather than by rewriting strings at the boundary. SleepStageVocabulary.isWake is the worked example, and it is a better illustration of R7's actual intent (one definition of the alias rule, not eleven literal comparisons) than the generic advice is, because it shows the intent surviving a constraint that rules out the obvious implementation.

The other two prescriptions are unaffected and correct as written.

Why this is separate

The protocol landed in ed7fbcb. Its author re-cut that branch once already and it sat seven weeks; this is a one-paragraph docs edit that should not have held the document. Recording it here so the clause is tracked rather than remembered.

Android side for reference: canonicalStage (SleepStageTimelineLogic.kt:163) folds through SleepStageVocabulary.kt, so the alias rule has one definition there too, reached a different way. Worth a sentence in the same clause if anyone writes it, since the two platforms satisfy R7 with different shapes and both are correct.

Refs #988

Activity

  1. ryanbr commented on Sep 27, 2026

    @ryanbr
    OwnerAuthor

    Addressed by #2409, from @kavemang.

    R7 previously told contributors to normalise stage strings at the boundary full stop, which is the opposite of what the tree does where two vocabularies are both legitimate and persisted. It now distinguishes the two cases: normalise at the boundary when one stored vocabulary is authoritative, and where both are real, put the alias rule in ONE shared predicate rather than rewriting either.

    It also names the worked example that already existed, so the guidance points at code instead of describing an ideal. I checked all three claims against the tree before merging:

    • SleepStageVocabulary.isWake exists on both platforms (SleepStageVocabulary.swift:26, SleepStageVocabulary.kt:28).
    • Swift consumers call it directly: SleepStager, SleepAwareStepCounter, WakeMotionRefinement, and both the Wearable and Xiaomi importers.
    • Android's canonicalStage folds through it (SleepStageTimelineLogic.kt:167), already carrying a comment that says the alias rule has ONE definition.

    Stored strings stay untouched either way, which was the substance of the report: the docs were asking for a migration the tree had deliberately declined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions