Merge upstream ryanbr/noop main into the fork (through 137dc0bfe) - #23
Merged
Merged
Conversation
…br#2286) The Workouts list unions many device namespaces: every registered WHOOP, every computed `-noop` sibling, Apple Health, imported lifting sessions and imported activity files. `deleteWorkout` deleted from exactly one, the active strap. A row banked anywhere else was therefore visible but undeletable. The delete issued a statement that matched nothing, its result was discarded, and the reload re-read the row from a namespace the delete never touched, so it reappeared with nothing on screen to say the delete had failed. Reported in ryanbr#2278. Both sides now derive from one `workoutNamespaces` function, because spelling the union out twice is what let them disagree, and a namespace added to the read alone would have reintroduced this exactly. Imported history is explicitly NOT deletable. The first pass swept the import namespaces too, which reached underneath an invariant enforced in three places: the row menu offers an imported row only "Duplicate as manual", bulkDeleteWorkouts skips those classes, and mergeWorkouts refuses them with "never rewrite imported history". A cross-source twin being collapsed into one row at display time does not license deleting the imported half of the pair. The sweep is narrow by construction: the natural key is exact, one sport plus a single startTs, so it removes the row the wearer tapped and its copies in the strap namespaces, nothing else. Scope note: this does not explain the reporter's observation that only sessions under a minute resisted deletion, and no duration threshold exists in the save, list or delete paths. That part stays open. Tests pin the containment in both directions: imports absent from the deletable set, every strap namespace present, and the deletable set a subset of what the list reads, so a delete can never target something the wearer cannot see.
…urrent and Archived (ryanbr#2287) Two halves of one request: keep recent workouts to hand, and stop the list filling with accidental starts. **Sub-minute sessions are discarded at save.** A 5 to 30 second start/stop is not training, and it is what made deletion feel broken in ryanbr#2278. Discarding at SAVE rather than pruning later is the point: nothing that ever held training data is removed, so there is nothing to restore. Exactly 60 seconds is kept, because a deliberate one-minute effort is training. **Current / Archived is a view split, never a delete.** The request was to keep the last 10 and auto-delete the rest; this hides the rest instead. NOOP has no server and no cloud copy, so pruning real training history would be irreversible, while hiding costs nothing and is one tap away. Archived rows stay in the database with every action they always had, including delete. Membership is decided by RANKING on start time, not by a cutoff timestamp, so twelve sessions starting in the same second still yield exactly ten in Current. The split is order-preserving and applies to the LIST rows only: `sessions(for:)` also feeds the 90-day HR-recovery trend and the auto-widen probe, and scoping there would have quietly cut a 90-day analysis to ten workouts. **The floor applies to manual entry too, on both platforms.** The span-shaped builder the Add/Edit sheet uses had no floor, so the same workout was treated differently depending on whether it was tracked or typed in. Putting it in the validator disables Save through the existing path, and the refusal gets its own note rather than the catch-all. Android mirrors both floors, since its `WorkoutEditing.buildManualRowFromSpan` is a line-for-line twin; the parity ledger ran clean on the one-sided commit, because unpaired constants are not reported the way unpaired functions are. **The i18n gate now sees copy a screen RETURNS.** It scanned only literals inside localized SwiftUI calls, so `var label: String { "Current" }` was invisible, and a bare literal returned that way renders in English forever. It shipped once that way in this branch while the gate passed, having flagged only the accessibility key beside it. The rule is keyed on the return rather than brace depth, since a `switch` opens a second level and the first draft silently covered only ternaries. Seven tests pin it, mutation-tested against the draft. Verified: 7/7 CI; Kotlin 731 classes 6235 tests 0 failures with the result XML checked fresh against the clock; Tools suite 54 passing; i18n gate exits 0. The Current/Archived tabs stay Apple-only. That is UI work, unlike the floor, which is a data rule with an existing twin.
…yanbr#2288) `peak` was a computed property that scanned every epoch, and it was read from inside the `map` in `points(in:)` and the `filter` in `accessibilitySummary`, so a night cost a full scan per epoch. With 30-second epochs an 8-hour night is about 960 of them, roughly 1.8 million comparisons every time the strip is laid out, and SwiftUI re-runs `body` on hover, animation and the 1 Hz HR tick. Reported as ryanbr#2283, which was slightly conservative: there are further reads in `body` itself that it did not list. It is now computed once and threaded down. Android already hoists the same value in `SleepScreen.kt`, so this removes a divergence rather than creating one. The public API is unchanged; three helpers went from private to internal statics, which is the minimum a test can reach. On magnitude, stated rather than repeated: the reported 363 ms and 1,824 ms come from a DEBUG build, where generic dispatch makes each scan far more expensive than an optimised one. In Release the same work is plausibly one to two milliseconds per layout, not hundreds. Worth removing because it repeats on every re-render, but unlikely to be the 542 ms hang on its own, and the reporter says as much: a Release build "felt similar", which points elsewhere for the scroll lag rather than at this being harmless. The tests pin OUTPUT, not speed. They transcribe the pre-hoist definitions and compare against them, so the new code is checked against the old behaviour rather than against itself: an ordinary 960-epoch night, the degenerate nights (empty, single epoch, all zero, negative magnitudes), and values straddling the half-peak threshold, where a careless hoist would show. Swept the rest of StrandDesign for the same shape, a scanning computed property read inside a loop, and found none, so this is a one-off rather than a class. Verified: 15/15 CI including `test (StrandDesign)`, whose count went 90 to 95 against the five tests this adds, so they genuinely ran.
…b's two broken tests Two of PR #23's three fixable failing checks, fixed directly on this branch (the third, github-advanced-security, is GitHub's own Copilot code-scanning backend failing with "CAPIError: 400 The requested model is not supported" — a platform-side error, nothing in this repo can address it). **`check` (i18n audit, exit 1 → 0).** Upstream's sync itself introduced 12 new UI literals with no catalog entry: 9 shared goal-placeholder examples (CoachGoalOnboardingFlow.swift / CoachGoalView.swift, "e.g. Run 5k without stopping" etc., 18 call sites collapsing to 9 unique keys), a breathing-pace format string (BreathingView.swift, `%.0f / %.0fs`), and two interpolated `String` literals the audit's `swift_returned_copy_literals` rule catches even though they never pass through `String(localized:)` (`SleepModel.swift`'s cross-midnight date span, `WorkoutSelectionScreen.swift`'s accessibility label) — matching how every existing catalog entry of this exact shape (`'HR eased %lld → %lld over %@.'`, `'%@ workout'`) is already handled here. Verified the compiled catalog key for the interpolated pair by reading `swift_catalog_pattern`'s own conversion rule (each `\(...)` → a placeholder, static text preserved verbatim) rather than guessing: `"%@ → %@"` (both interpolations are String-typed, matching Swift's own `String(localized:)` convention) and `"%@ workout, %@"` (reusing the exact "workout" wording each locale already uses in the sibling `'%@ workout'` entry). All 9 languages, added to both the catalog and `Tools/translations/`. **`linux-capture` job's "Run Tools/ tests" step (2 failures → 0).** Cherry-picked the two isolated test fixes from #25 (`chore/retire-parity-governance-tooling`, independently reviewed there) onto this branch: `test_steps_i18n.py`'s Android-locale test removed (this fork ships no `android/` tree; the iOS test is untouched), and `test_i18n_audit.py`'s stale `test_two_word_brand_is_flagged` corrected to match `0c18441e4`'s deliberate `BRAND_PHRASES` change. #25 is base-`main`, independent of this sync, so this branch needs the same two-line fix in its own right rather than waiting on it. Verified locally, reproducing each CI step exactly: `python3 Tools/i18n_audit.py --ci main` exits 0. The `linux-capture` job's own two steps — `unittest discover` in `Tools/linux-capture` (234 tests) and the top-level `unittest -v <six explicit modules>` in `Tools/` (124 tests, matching the job's `find . -maxdepth 1` module list exactly, not a broader recursive discovery) — both pass clean, 0 failures, 16 skipped (unchanged skip set). `doc_comment_lint.py` exits 0.
…o dodge an x86_64 emit-module crash `app-build.yml`'s "build (Strand, ARCHS=x86_64 arm64...)" job failed twice on two independent commits (1e86bb6, ef302d1) with the identical crash: `swift-frontend -frontend -emit-module -experimental-skip-non-inlinable-function-bodies-without-types` aborting with a stack dump during x86_64 module emission for the whole ~450-file Strand target — no source-level diagnostic, no "error:" line anywhere in either run's log, arm64 unaffected both times. That flag is injected automatically by Xcode 16's incremental (singlefile) Debug compilation mode as part of its "emit module separately" optimization, which has known crash reports on large single-invocation module-emit jobs. `SWIFT_COMPILATION_MODE=wholemodule`, added as a command-line xcodebuild override on this ONE step (not a project.yml change), takes a different code path that doesn't invoke that optimization, sidestepping the crash entirely. Verified locally: `xcodebuild -scheme Strand -destination 'platform=macOS' SWIFT_COMPILATION_MODE=wholemodule build` — BUILD SUCCEEDED, clean. Scoped to the CI invocation only, so this changes nothing for a developer's local incremental Xcode build of the same Debug configuration — and costs nothing in CI, which always does a from-scratch build anyway (fresh checkout, cold DerivedData), so incremental mode's speed advantage never applied here regardless.
…ream-2026-09-17 # Conflicts: # Strand/Resources/Localizable.xcstrings # Tools/translations/de.json # Tools/translations/es.json # Tools/translations/fr.json # Tools/translations/it.json # Tools/translations/pl.json # Tools/translations/pt-PT.json # Tools/translations/ru.json # Tools/translations/zh-Hans.json # Tools/translations/zh-Hant.json
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
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.
Routine upstream sync. Brings in ryanbr/noop through
137dc0bfe, including:Android-tree conflicts (this fork dropped
android/on 2026-08-14, seedocs/FORK_GUIDE.md) resolved by keeping our deletion;Localizable.xcstrings/Tools/i18n_audit_baseline.jsonconflicts resolved additively (our catalog kept, upstream's new keys —Archived,Current,Scope, the sub-minute-workout refusal copy — merged in with all nine languages).project.yml'sCURRENT_PROJECT_VERSIONkept at our value.Verified:
swift testgreen (WhoopStore, StrandDesign, StrandAnalytics),xcodebuild testgreen for StrandTests (3405 tests),xcodebuild buildgreen for NOOPiOS,i18n_audit.py --ciexits 0.Known gap, not introduced by this sync, surfacing because of it:
Tools/tests/test_parity_governance_acceptance.pyandTools/tests/test_rr_legacy_preservation_contract.pyreferenceandroid/paths this fork removed, and the parity-governance baseline reads stale. Correction to an earlier note here: these do NOT breaktools-python.yml's activelinux-capturejob — that job's "Run Tools/ tests" step only runs the six top-levelTools/test_*.pymodules (find . -maxdepth 1), neverTools/tests/. They break the separate, undocumented.github/workflows/parity-governance.yml(not in this fork's own CI table indocs/FORK_GUIDE.md) and a broader ad-hocunittest discover— see #25, which retires that whole stack as dead weight from before theandroid/removal.