Je rebondis sur ce sujet du coup, j’ai terminé la vérification pour la PR sur l’amélioration du recalcul du suivi de l’énergie. Je pense que tu peux review :
S’ensuivra la PR suivante qui ajoute du logging plus claire pour l’utilisateur, notamment pour voir ou en étaient les dernière tache lancées si jamais un redémarrage Gladys ou un problème était survenu pendant un recalcul (évitant de devoir refaire tout le recalcul …) :
Est-ce que tu pourrais décrire plus en détail ce que fait cette PR ?
Comme cela touche au core, ce sont des changements assez sensibles.
J’aimerais bien comprendre précisément le périmètre pour pouvoir évaluer correctement l’impact.
Lors du développement j’ai passé plusieurs mois sur la fiabilisation des calculs, et c’est vital pour moi que le calcul ne soit pas impacté 
Tu as raison de demander le détail, je comprends tout à fait. Et si possible, je préférerais que tu le test également, en le laissant tourner à côté de ta prod por bien vérifier. C’est ce que j’ai fais pour bien évaluer le bon fonctionnement. Je n’ai pas trouvé de cas de figure où ça plante. Mais par exemple, je n’ai pas de zigbee énergie par exemple. Même si il n’y a aucune raison que ce soit différent, ça couvrirait l’ensemble.
Le diff global est gros (+3203 / -428), mais le périmètre fonctionnel que je vise dans cette PR est surtout celui-ci :
calculate-cost-range)calculate-consumption-from-index-range)feature_selectors).YYYY-MM-DD côté controller (avec BadParameters si format invalide).destroyStatesBetween) avant recomputation.destroyStatesFrom).ENERGY_INDEX_LAST_PROCESSED en fin de traitement sur les recalculs partiels/sélectionnés pour éviter d’abîmer le curseur global.wrapperDetached pour les recalculs longs (from beginning / range), pour renvoyer immédiatement un job_id.job_id est bien retourné.range si une date est fournie, sinon des endpoints from-beginning.contracts.calculateCost reste le même).Je n’ai :
J’ai « seulement » modifié le périmètre d’entrée (features sélectionnées, plage de dates) et l’encadrement technique des recalculs (purge ciblée, jobs, API/front).
Et pour tout te dire, en début de semaine, j’ai faillis fermer les PR de ce sujet et t’écrire pour te dire que je te laissais la main sur ce sujet.
J’avais peur que ça te prenne plus de temps à review qu’à le faire toi même. Qu’en plus visuellement ça ne te convienne pas. Et que du coup tu aurais sûrement été mieux à le faire tel que tu le penses.
Mais je balance entre cette question et le fait qu’on soit quand meme quelques uns à être en attente. Et du fait que je vois bien que tu es occupé dans bien d’autres choses.
Alors je suis parti du posta de proposer, et que toi tu disposes. Et tu me donneras ton retour / ressenti. 
Merci pour les précisions, je te tiens au courant quand j’ai pu regarder
J’ai regardé rapidement la PR et j’ai l’impression qu’elle touche à plus de choses que le simple re-calcul avec les dates.
Par exemple, les quatre premiers fichiers semblent être un reliquat d’une autre PR :
Pour le code métier, la PR est assez massive et touche à tellement d’éléments qu’il est difficile d’être certain qu’elle ne modifie pas l’algorithme de calcul.
Il y a même une nouvelle logique introduite avec des notions nouvelles (comme shouldRestoreLastProcessed), donc ce n’est clairement pas une PR triviale.
Pourtant intuitivement, si je réfléchis 2 minutes à l’idée, j’ai du mal à comprendre pourquoi c’est aussi complexe.
Je me demande si ton intuition n’est pas bonne malheureusement :
C’est vraiment dommage, car je suis plutôt d’accord avec le résultat et l’UX me paraît bonne. Le problème, c’est que le code est énorme : je ne peux pas merger sans le lire entièrement, comprendre les nouveaux concepts et effectuer tous les tests nécessaires… 
Qu’en penses-tu ?
Bonjour @pierre-gilles
Merci d’avoir commencé à regarder.
La branche de la PR partait de la branche Tasmota, qui as eu pas mal de changement suite à nos discussions.
Après l’update de la PR une fois Tasmota mergée, j’ai refais une passe, mais je suis passé à côté du fichier ‹ front/src/components/device/index.js › qui intègre toujours la création des device conso/coût => Je ne comprend pas comment je suis passé à côté, c’est le 1er fichier et c’est un pavé. C’est retiré
front/src/components/device/UpdateDevice.jsx et front/src/components/device/UpdateDeviceFeature.jsx c’était l’ajout d’un raccourci depuis les features qui avaient des consos / coût associés pour aller directement sur la page de suivi de l’énergie. Ca pourra etre proposé plus tard, j’ai retiré.
front/src/components/drag-and-drop/DeviceListWithDragAndDrop.jsx : le composant est utilisé dans la sélection des devices à recalculer. Il y avait besoin d’ajouter une impossibilité de changer le nom pour cet usage.
Je suis d’accord, ce n’est pas une PR triviale sur ce point.
De base, j’ai envisagé de dupliquer la logique dans un moteur de recalcul séparé pour ne pas dépendre de ENERGY_INDEX_LAST_PROCESSED.
Mais j’ai volontairement évité ça: on aurait eu 2 implémentations parallèles (incrémental vs recalcul), donc plus de complexité avec un risque de divergence dans le temps.
Aujourd’hui j’ai ajouté calculateConsumptionFromIndexRange en tant qu’orchestrateur (bornes de dates, nettoyage, boucle des fenêtres).
J’ai voulu conserver le calcul réel de chaque fenêtre en passant toujours par calculateConsumptionFromIndex, qui lui utilise ENERGY_INDEX_LAST_PROCESSED.
Du coup, shouldRestoreLastProcessed sert à protéger le curseur incrémental live lors des recalculs partiels/périodes passées. Parce qu’avant, le calcul allait toujours « jusqu’au bout ». Mais là le but c’est de pouvoir ajouter un prix de contrat ancien et de lancer le recalcul sur cette plage seulement. Si c’était il y a 1 an, sans ça le prochain calcul 30 minutes aurait repris sur une année complete au lieu de redémarrer au dernier vrai run de suivi.
Sans restauration:
Avec restauration:
Parce que ce curseur est utilisé par le calcul de consommation index.
En bref, c’est un garde-fou pour garder un moteur unique maintenable, sans effet de bord sur les runs incrémentaux suivants.
Bien entendu je me trompe peut-etre sur la logique, mais notamment sur ce point, j’ai travaillé avec l’IA et tenter de tourner ca dans tous les sens pour être certains que ce soit une bonne manière de faire (pas forcément la meilleur, la review sert à ça !)
Je suis bien conscient de cela, et je t’en ai parlé dès le début de cette PR. Ca touche un élément sensible, et j’esperais que justement tu puisses le faire tourner sur une instance à côté de ta prod pour être certains que les calculs (qui n’ont pas été touchés) sont bons meme après recalcul sur plages, etc.
Pour ce qui est du code, pour info ce sont les tests les plus gros :
Bon week-end à toi et bonne course
Salut @pierre-gilles,
Ne sachant pas si tu es en cours ou quand tu passera en review sur cette PR, j’ai pensé bon de relancer une review complète de @coderabbitai car je m’aperçois qu’il n’en refait plus après quelques passes.
Et au vue du nombre de commit fait depuis sa 1ère review je me suis dit qu’il serait bien d’en refaire une pleine sur la PR complète : https://github.com/GladysAssistant/Gladys/pull/2413#pullrequestreview-3943430667
Je pense qu’il a ressortis de bonne chose. Si tu n’as pas commencé ta review, puis-je push les changements ? Sinon que préfères-tu ?
Je n’ai pas encore regardé, tu peux modifier la PR
Ok, c’est fait.
