From 8e0c2738ce1c1e2acd8bfe4cbfcd0cb80b52786b Mon Sep 17 00:00:00 2001 From: Sylvain Bettinelli Date: Thu, 24 Sep 2026 08:00:30 +0000 Subject: [PATCH] =?UTF-8?q?Les=20cinq=20constats=20natifs=20de=20l'audit,?= =?UTF-8?q?=20v=C3=A9rifi=C3=A9s=20ligne=20=C3=A0=20ligne=20=E2=80=94=20sa?= =?UTF-8?q?ns=20rien=20corriger?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L'audit du 22/09 marquait cinq constats **[rapporté]** : issus de revues automatisées, jamais reconfrontés au code, et il le disait lui-même — « les confirmer avant d'agir ». Sylvain a autorisé la lecture, pas la modification : aucune ligne de Swift ne bouge ici. **Les cinq sont confirmés, aucun faux positif.** Trois lectures s'écartent toutefois de ce que l'audit décrivait : § 1.2 est **plus grave**. L'audit ne voyait que le `try?` ; `pushToWatch` a quatre sorties silencieuses sur sept lignes — WCSession non supportée, session non activée, encodage échoué, envoi refusé. Et l'aggravant se vérifie : `HeartRateZones` ne porte aucune date, alors que le backend sert une version figée et datée (règle 9 du CLAUDE.md web). La date existe côté serveur et se perd au transport. `asOf()` périme l'eau, les kcal, la routine et le score de forme, mais pas les zones — délibérément, et c'est défendable pour une péremption journalière ; ça laisse ouvert le cas d'une révision des bornes. § 2.3 est **plus nuancé**. Le code sait déjà que `finishWorkout()` rend nil sans erreur quand la montre est verrouillée, et que la séance est alors bien écrite — d'où `recentlySavedWorkout()`. Le défaut n'est donc pas « la séance est perdue » mais que le double `try?` rend indiscernables deux situations opposées : montre verrouillée (séance sauvée) et `endCollection` qui a levé (rien de sauvé). Les deux tombent dans le même `else`. § 2.4 est **plus grave** aussi : `self.builder = nil` détruit la seule référence au builder. La perte devient définitive dans l'instruction même qui l'ignore, et le GPS continue de tourner après la fin apparente de la séance. § 1.3 et § 2.6 sont exacts tels que décrits. Pour § 2.6, un fait qui manquait : le même fichier utilise `NSLock` pour ce motif à quatre autres endroits — c'est une omission ponctuelle, pas une convention absente. ⚠️ Les numéros de ligne de l'audit ne valent plus : les correctifs du 22/09 au soir ont décalé `WorkoutManager.swift` et `CoachHealthRoute.swift`. Le document porte les références à jour. Co-Authored-By: Claude Opus 5 (1M context) --- docs/DIAGNOSTIC-NATIF-2026-09-24.md | 239 ++++++++++++++++++++++++++++ 1 file changed, 239 insertions(+) create mode 100644 docs/DIAGNOSTIC-NATIF-2026-09-24.md diff --git a/docs/DIAGNOSTIC-NATIF-2026-09-24.md b/docs/DIAGNOSTIC-NATIF-2026-09-24.md new file mode 100644 index 0000000..6907a72 --- /dev/null +++ b/docs/DIAGNOSTIC-NATIF-2026-09-24.md @@ -0,0 +1,239 @@ +# Diagnostic natif — les cinq constats ouverts de l'audit, vérifiés + +**24/09/2026. Lecture seule : aucune ligne de Swift n'a été modifiée.** +Demandé par Sylvain après l'audit du 22/09, dont cinq constats étaient marqués +**[rapporté]** — issus de revues automatisées, jamais reconfrontés au code. +L'audit lui-même le disait : « les confirmer avant d'agir ». + +Ce document dit ce que le code fait **aujourd'hui**, ce que l'audit décrivait +mal, et ce qu'il faudra corriger le jour où le gel sur le natif sera levé. Il ne +propose aucun geste : la correction demande un build Mac, et rien n'est réel +tant que ce build n'a pas tourné. + +⚠️ **Les numéros de ligne de l'audit ne valent plus.** Les correctifs du 22/09 +au soir ont décalé `WorkoutManager.swift` et `CoachHealthRoute.swift`. Les +références ci-dessous sont celles du dépôt à `6031be8`. + +--- + +## Verdict d'ensemble + +**Les cinq sont confirmés.** Aucun faux positif. Deux sont plus graves que +l'audit ne le disait, un est plus nuancé. + +| # | Constat | Vérifié | Écart avec l'audit | +|---|---|---|---| +| 1.2 | Zones cardiaques muettes vers la montre | ✅ | **Aggravé** : 4 sorties muettes, pas 1 | +| 1.3 | « Refusé » ≡ « aucune donnée » (sommeil) | ✅ | Exact | +| 2.3 | Séance perdue sans log | ✅ | **Nuancé** : `nil` n'est pas toujours une perte | +| 2.4 | `didFailWithError` laisse le GPS allumé | ✅ | **Aggravé** : `builder = nil` rend la récupération impossible | +| 2.6 | Tampon GPS sans verrou | ✅ | Exact | + +--- + +## 1.2 — Les zones cardiaques peuvent ne jamais atteindre la montre + +**`ios/App/App/CoachWidgetBridge.swift:175-183`** (`pushToWatch`) + +L'audit ne signalait que le `try?`. Il y a **quatre** sorties silencieuses sur +sept lignes : + +```swift +guard WCSession.isSupported() else { return } // 1. muet +let session = WCSession.default +guard session.activationState == .activated, // 2. muet + let data = try? JSONEncoder().encode(snapshot) else { return } // 3. muet +try? session.updateApplicationContext(["widgetSnapshot": data]) // 4. avalé +``` + +Aucun log, aucune remontée au JS. Le snapshot porte `zones: HeartRateZones?` +(`CoachWidgetSnapshot.swift:96`) — les bornes Z1…Z5 et `clinicalTargetBpm`, la +cible sous bêtabloquant. + +**Ce que fait le chemin voisin, dans le même dépôt** +(`CoachRoutineBridge.swift:87-101`) pour la routine de gainage : il vérifie +`isPaired` **et** `isWatchAppInstalled` avec une raison explicite, entoure +l'appel d'un `do/catch`, journalise l'échec et appelle `call.reject`. + +Le chemin instrumenté porte des exercices de mobilité. Le chemin muet porte les +bornes cardiaques d'un patient post-infarctus. + +### L'aggravant est réel, et il se compose avec le premier + +`HeartRateZones` (`ios/App/App/HeartRateZones.swift`) ne porte **aucune date** : +ni `version`, ni `validFrom`, ni `computedAt`. Or le backend sert une version +**figée et datée** (`data/hr_zones_history.json`, via `app.current_zones()`) — +c'est même une règle non négociable du projet web (règle 9 du CLAUDE.md). + +**La date existe côté serveur et se perd au transport.** + +Et `CoachWidgetSnapshot.asOf(_:)` périme l'eau, le café, les kcal, la routine, +l'activité du jour et le score de forme — mais **pas `zones`**, délibérément +(« version figée, valable des semaines »). Le raisonnement est bon pour une +péremption journalière ; il laisse ouvert le cas d'une **révision** des bornes. + +Conséquence composée : après un `POST /api/cardiac-zones/confirm`, si le push +échoue silencieusement, la montre affiche les anciennes bornes **sans limite de +temps et sans que rien ne le signale**, ni au poignet ni à l'iPhone. + +**À corriger, le jour venu** : aligner `pushToWatch` sur `CoachRoutineBridge` +(do/catch, log, `isPaired`/`isWatchAppInstalled`), et faire voyager la date de +version des zones pour que la montre puisse dire « bornes du 12/08 » plutôt que +d'afficher un chiffre sans âge. + +--- + +## 1.3 — « Accès refusé » et « aucune donnée » sont rendus identiques + +**`ios/App/App/CoachSleep.swift`** — en-tête ligne 30 : + +> `// r.available = false si HealthKit est indisponible ou non autorisé` + +Le code teste `HKHealthStore.isHealthDataAvailable()` (l. 59, 63) et la +disponibilité du type (l. 68). **`authorizationStatus(for:)` n'apparaît nulle +part dans le fichier.** La requête aboutit et rend : + +```swift +call.resolve(["available": true, "samples": out, ...]) // l. 92-96 +``` + +La documentation du fichier promet donc quelque chose que le code ne fait pas. + +**La nuance de l'audit est juste** : HealthKit ne distingue pas volontairement +« refusé » de « vide » **en lecture** — c'est une protection de vie privée +documentée par Apple. Mais `authorizationStatus(for:)` renvoie `.notDetermined` +de façon fiable, et c'est le cas fréquent : réinstallation de l'app, +autorisation jamais accordée. C'est celui-là qu'on peut distinguer. + +**Conséquence côté serveur** : `sleep_score.py` calcule une régularité de +coucher sur des nuits absentes, sans savoir qu'elles sont absentes parce qu'on +ne nous laisse plus regarder. Une valeur fausse, pas une valeur manquante. + +Même famille dans `CoachHealthRoute.swift` (l. 173 et 277 selon l'audit — à +revérifier, les lignes ont bougé). + +--- + +## 2.3 — Une séance peut disparaître sans un seul log + +**`ios/App/CoachWatch/WorkoutManager.swift:383-398`** + +```swift +LocationTracker.shared.stopTracking() +try? await builder.endCollection(at: date) +let workout = try? await builder.finishWorkout() +if let workout { … } else if let recovered = await self.recentlySavedWorkout() { … } +else { LocationTracker.shared.discardRoute() } +``` + +**Ce que l'audit ne disait pas, et qui change la lecture** : le commentaire du +code explique que `finishWorkout()` rend `nil` **sans erreur** quand la montre +est verrouillée, et que dans ce cas la séance **est** écrite dans HealthKit — +seul l'objet manque. C'est pour ça que `recentlySavedWorkout()` existe. Un `nil` +n'est donc pas une perte en soi, et le code le sait. + +**Le défaut est plus précis que « la séance est perdue » :** le double `try?` +efface la différence entre deux situations opposées — + +| Ce qui s'est passé | `workout` | La séance est-elle sauvée ? | +|---|---|---| +| Montre verrouillée à l'arrêt | `nil` | **Oui**, `recentlySavedWorkout()` la retrouve | +| `endCollection` a levé | `nil` | **Non**, il n'y a rien à retrouver | + +Les deux chemins convergent vers le même `else`, et `discardRoute()` ne +journalise que la perte de la **trace**. `statusMessage` n'est pas touché : +après deux heures de sortie, l'utilisateur voit l'écran de fin habituel. + +**À corriger** : remplacer les deux `try?` par un `do/catch` qui journalise +l'erreur — sans changer la logique de récupération, qui est bonne. + +--- + +## 2.4 — `didFailWithError` ne clôt rien et laisse le GPS allumé + +**`ios/App/CoachWatch/WorkoutManager.swift:451-464`** + +```swift +workoutLog.error("session didFailWithError domain=… code=…: \(message)") +Task { @MainActor in + self.statusMessage = "Erreur séance : \(message)" + self.isRunning = false + self.session = nil + self.builder = nil + ConnectivityManager.shared.sendEnded(reason: "session-failed") +} +``` + +Comparé au chemin `.ended` (l. 383-398), il manque **tout** : `stopTracking()`, +`endCollection`, `finishWorkout`, `finishRoute`/`discardRoute`. + +**Plus grave que l'audit ne le dit** : `self.builder = nil` détruit la seule +référence au builder. Après cette ligne, plus aucun code ne peut tenter de +sauver ce qu'il avait collecté — la perte devient définitive dans la même +instruction qui l'ignore. + +Deux conséquences distinctes : +1. **Le GPS continue de tourner.** `LocationTracker` n'est jamais arrêté, alors + que la séance est finie du point de vue de l'interface. Sur une montre, ça se + paie en batterie, et rien ne l'indique. +2. **Les données collectées sont jetées sans tentative de sauvegarde.** + +⚠️ À la différence de § 2.3, ici `statusMessage` **est** renseigné : l'utilisateur +voit « Erreur séance ». Il sait qu'il s'est passé quelque chose ; il ne sait pas +qu'il a perdu la séance, et le GPS tourne toujours. + +--- + +## 2.6 — Accès concurrent non protégé sur le tampon GPS + +**`ios/App/App/CoachHealthRoute.swift:183-205`** (`collectLocations`) + +```swift +var all: [CLLocation] = [] +let group = DispatchGroup() +var pluginErr: Error? +for route in routes { + group.enter() + let q = HKWorkoutRouteQuery(route: route) { _, locations, done, err in + if let err = err { pluginErr = err; group.leave(); return } + if let locs = locations { all.append(contentsOf: locs) } // ⚠️ sans verrou + … +``` + +`all` et `pluginErr` sont mutés depuis les callbacks de N requêtes, livrées sur +des files HealthKit indépendantes. **Aucun verrou.** + +**Le même fichier utilise `NSLock` pour ce motif exact à quatre endroits** : +lignes 283, 380, 442 et 565. Ce n'est donc pas une convention absente du +projet — c'est une omission à un seul endroit. + +**Déclencheur** : un workout portant plusieurs `HKWorkoutRoute` (multisport, ou +reprise après interruption). Avec une seule route, la boucle n'a qu'une +itération et le défaut est inobservable. + +⚠️ **On ne sait pas s'il s'est déjà produit** : les routes vivent dans +HealthKit, pas sur le serveur, et rien ne journalise `routes.count`. Une +corruption de tableau en Swift ne lève pas forcément — elle peut donner une +trace tronquée ou un crash, sans rien qui désigne la cause. + +--- + +## Ce que ce diagnostic ne dit pas + +- **Aucune de ces corrections n'a été écrite.** Le gel du 22/09 sur le natif + tient ; Sylvain a autorisé la lecture, pas la modification. +- **Rien n'est vérifiable sans build Mac.** Ces cinq constats sont établis par + lecture du code, pas par exécution. Une lecture peut manquer un chemin. +- **La dette assumée du 22/09 reste ouverte** : le `defer { completion() }` de + `CoachWorkoutObserver` déclare la livraison traitée alors que les requêtes + sont encore en vol, sans `beginBackgroundTask`. Elle n'est pas revue ici, elle + était déjà documentée et assumée. +- **Le § 4 de l'audit (gravité 3, justesse et cohérence) n'a pas été revérifié.** + +## Si un seul devait être corrigé + +**§ 1.2**, et pas parce qu'il est le plus spectaculaire : parce que c'est le seul +dont la conséquence est un **chiffre faux affiché pendant l'effort**, chez +quelqu'un qui règle son intensité dessus. Les quatre autres perdent de la donnée +ou de la batterie — c'est réparable après coup. Une borne de zone périmée au +poignet ne se rattrape pas : elle a déjà servi à décider.