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.
Hola @pierre-gilles,
Vuelvo a ti sobre la PR #2413 (recalculo de energía).
Sigo esperando este recalculo para poder utilizar plenamente la integración de seguimiento de energía. He retomado la PR correctamente (merge de master, correcciones de CodeRabbit, re-test en mi producción durante casi 2 meses sin ninguna divergencia de cálculo con la versión oficial). La he reabierto.
Para recordarte el hilo de discusión: a principios de abril me indicaste que preferías hacer el tema tú mismo porque la PR te parecía demasiado grande y demasiado sensible. Acepté y puse la PR en pausa.
Para avanzar concretamente, te propongo dividir la PR en varias pequeñas PR independientes, mergeables una por una. Podrías revisar por partes, probar cada etapa en tu producción y retroceder fácilmente si es necesario:
destroyStatesBetween + sus pruebas (utilidad pura, aislada, ~100-200 líneas).Podrías mergear PR1 y PR2 sin comprometer los conceptos más sensibles (shouldRestoreLastProcessed solo aparece en PR3).
Si validas esta división, me pongo con ello esta semana. Si prefieres retomarlo tú mismo con un horizonte claro, dime y cerraré la PR. Lo que me molesta es la falta de decisión y no poder utilizar plenamente esta integración, me parece que otros usuarios también estaban afectados.
Gracias por tu respuesta.
Hola,
El tema es sobre todo una cuestión de confianza 
Lo que me preocupa de la PR en su estado actual es que modifica en profundidad la lógica de cálculo. El año pasado, esta parte me llevó varios meses de trabajo para los tres tipos de contratos gestionados (base, horas punta/horas valle y Tempo), con muchas pruebas en datos reales de decenas de usuarios.
Normalmente, las pruebas unitarias sirven precisamente para detectar regresiones. Pero hoy en día, con la IA, las pruebas en sí pueden ser modificadas masivamente de forma automática, lo cual es el caso aquí. Por lo tanto, ya no puedo considerar las pruebas como una garantía fiable. La IA puede haber introducido perfectamente regresiones y luego adaptado las pruebas para validarlas.
Para fusionar esta PR tal cual, sería necesario que yo revisara y comprendiera todo el código modificado, y luego que repitiera toda la fase de validación realizada el año pasado. Y desafortunadamente, ya no tengo los conjuntos de datos de prueba que algunos usuarios me enviaron en ese momento. Por lo tanto, es un proyecto casi equivalente al desarrollo inicial, no una simple iteración.
En cambio, si propones una PR “solo front” que simplemente añada la posibilidad de recalcular en el último mes / 3 últimos meses / 6 últimos meses / 12 últimos meses, probablemente podría fusionarse en menos de una hora, sin una fase de validación pesada, ya que el código de negocio no se modificaría.
El desglose en pequeñas PR puede ayudar a hacer los cambios más legibles, pero tan pronto como se toca la lógica de negocio aquí, habrá de todos modos una gran fase de validación inevitable.
¿Qué opinas?
Y disculpa si esto puede parecer duro, no es realmente en contra de tu trabajo. Es sobre todo que, con mi agenda actual, proyectos de esta envergadura son muy complicados de absorber 
Me gustaría tener más tiempo para este tipo de revisión exhaustiva, pero el proyecto aún no está en ese punto.
Primero, gracias por esta respuesta, que tiene el mérito de ser completa y honesta sobre tus limitaciones.
Para poder responderte plenamente, y no solo con un « estoy seguro de mi código », tomé el tiempo de realizar una auditoría rigurosa de la PR esta tarde. Establecí el requisito explícito de verificar que cada cambio es necesario, sólido, bien pensado, sin regresiones y, sobre todo, que las pruebas no están distorsionadas — incluso rompiendo intencionalmente el código para verificar que las pruebas detectan las regresiones (pruebas de mutación reales). Es precisamente la duda legítima que planteas sobre la IA y las pruebas, y quería responder con algo verificable, no con una afirmación.
A continuación, te presento el resultado de esta auditoría, factual y reproducible en tu entorno.
Mantengo mi respuesta anterior, pero esta vez con la verificación línea por línea. Los dos archivos donde reside la lógica de negocio son:
server/services/energy-monitoring/lib/energy-monitoring.calculateConsumptionFromIndex.jsserver/services/energy-monitoring/lib/energy-monitoring.calculateCostFrom.jsHe revisado cada línea modificada. Lo que está estrictamente sin cambios:
convertEnergyUnit(...) — conversiones de unidadescontracts[contract](...) — todas las fórmulas de costo (base, HP/HC, Tempo)subtract(30, 'minutes')saveMultipleHistoricalStates / saveHistoricalStateLos únicos añadidos en calculateConsumptionFromIndex.js están encapsulados en un if (selectorSet.size > 0) (filtro por lista blanca): desactivados en ausencia de lista blanca, por lo que el comportamiento legado se conserva estrictamente para el cálculo incremental en vivo que se ejecuta cada 30 minutos.
En calculateCostFrom.js, los añadidos son:
if (selectorSet.size > 0))destroyStatesBetween si se proporciona endAt, de lo contrario destroyStatesFrom (comportamiento legado)try/catch por característica de costo (robustez: una característica que falla no bloquea las otras)La fórmula de costo en sí — aquella que te tomó meses estabilizar en base/HP/HC/Tempo — no se ha movido ni un carácter. Es verificable en unos minutos con git diff master HEAD -- server/services/energy-monitoring/lib/energy-monitoring.calculateCostFrom.js y un Ctrl+F en contracts[contract], convertEnergyUnit, energyPricesForDate.
Es un temor perfectamente legítimo, y es precisamente por eso que hice pruebas de mutación reales: rompí temporalmente 3 puntos críticos del código y volví a ejecutar toda la suite. El objetivo es verificar que las pruebas detectan la regresión, no que pasen con la regresión.
Aquí están las 3 mutaciones probadas y el resultado:
| Mutación aplicada | Pruebas que fallan |
|---|---|
| Validación de fechas retirada del controlador | reject invalid start date format + reject invalid end date format (2 pruebas) |
Filtro de lista blanca del motor en vivo calculateConsumptionFromIndex retirado |
skip consumption features not in whitelist selectors (1 prueba) |
Bloque finally de restauración del cursor ENERGY_INDEX_LAST_PROCESSED retirado |
restore last processed value on selector-based recalculation + restore last processed value on window error (2 pruebas) |
Total: 5 pruebas fallan precisamente en las afirmaciones correctas. Las pruebas no son un lavado de cara de la IA, realmente detectan las regresiones en las protecciones críticas. Reproducible en tu entorno: solo necesitas comentar estos 3 bloques y volver a ejecutar npm run test-service --service=energy-monitoring. Archivos restaurados después de la verificación, confirmé con git status que no hay modificaciones residuales.
También verifiqué la no regresión de las pruebas existentes:
calculateConsumptionFromIndex.test.js) tienen únicamente un añadido de undefined como segundo parámetro (para alinearse con la nueva firma). Ninguna afirmación ha sido debilitada o eliminada.calculateCostFromYesterday.test.js está en realidad refuerza: antes solo verificaba calledOnce, ahora verifica la firma exacta de los argumentos pasados.try/catch + expect.fail es reemplazado por next(error) — es el patrón correcto de Express.Entiendo la lógica: evitar la fase de validación pesada en el negocio sin tocar el back-end. Pero esta propuesta no cubre la necesidad real de los usuarios, y es importante que lo explique:
Caso de uso n.º 1 — El recálculo histórico en dispositivos existentes añadidos al seguimiento a posteriori
Esta es la promesa del seguimiento de energía en Gladys. Muchos usuarios (incluido yo) tienen tomas Tasmota o Zigbee2mqtt en sus instancias Gladys durante varios años, a veces con 4 años de datos de índice almacenados. Cuando se añade la integración de seguimiento de energía a estos dispositivos, se debe poder recalcular el consumo y el costo en todo el historial disponible, y no solo en los últimos 12 meses.
Hoy, el único medio es el from-beginning global, que:
Un preset « últimos 12 meses » no resuelve ninguno de estos tres problemas. Una selección por característica + rango de fechas los resuelve a los tres.
Caso de uso n.º 2 — La tarifa retroactiva
Un usuario que se da cuenta de que cambió de tarifa hace 18 meses y que quiere recalcular solo ese rango está bloqueado con un preset de 12 meses.
Caso de uso n.º 3 — La granularidad
Cuando tienes 30 dispositivos seguidos y quieres corregir solo un dato en un dispositivo durante una semana, lanzar un recálculo global en 1/3/6/12 meses es desproporcionado y arriesgado para los otros dispositivos.
También quiero mencionar dos puntos en los que la PR hace el código más seguro que master:
wrapperDetached (nueva primitiva de trabajo) desacopla la solicitud HTTP del cálculo largo. Antes, el from-beginning bloqueaba la solicitud HTTP hasta el final del cálculo, lo que provocaba timeouts en las instalaciones grandes. Probablemente es la causa silenciosa de los « no funciona » que reportamos regularmente.try/finally alrededor de la restauración del cursor ENERGY_INDEX_LAST_PROCESSED protege la instancia de un estado corrupto si una ventana falla durante el recálculo. En master, si una ventana falla, el cursor queda destruido y el cálculo incremental en vivo comienza desde cero en la próxima iteración.Para no decirte que todo es perfecto, la auditoría también identificó 3 limpiezas menores (no bloqueantes, sin riesgo de regresión):
calculateCostFrom que ya no tiene utilidad (todos los llamadores internos ya pasan la nueva firma).FromBeginning y Range que puedo factorizar.calculateCostFrom.test.js que no prueban realmente lo que pretenden (para endurecer o retirar — las pruebas de negocio reales permanecen en las pruebas originales no modificadas).Me comprometo a corregir estos 3 puntos antes del corte.
Dado lo anterior, te propongo seguir con el corte en sub-PRs, pero adaptando tu preocupación por « fase de validación incompresible »:
destroyStatesBetween + wrapperDetached + sus pruebas. ~150-200 líneas. Sin lógica de negocio, solo utilidades. Fusionable rápidamente, bajo riesgo.if (selectorSet.size > 0), por lo que el comportamiento heredado se preserva estrictamente.shouldRestoreLastProcessed. Aquí es donde se centra la « verdadera » revisión de negocio.En PR3 — la que más validación te requiere — estoy listo para:
shouldRestoreLastProcessedSi validas este desglose, empiezo por PR1 esta semana. Si prefieres que espere, dime con un plazo — aunque sea amplio — y cerraré la PR de nuevo con tranquilidad.
Gracias por tu tiempo, y disculpa si la respuesta es densa — he preferido darte hechos en lugar de afirmaciones.
@pierre-gilles, complemento a mi mensaje anterior.
Para respaldar concretamente lo que escribía sobre el audit, he realizado las limpiezas que había anunciado.
calculateCostFrom.test.jsAl auditar las pruebas añadidas por este PR en este archivo (las pruebas originales en master no se han tocado), encontré 5 pruebas problemáticas de 6:
should handle case where no energy price is found for the device at the given date)device.create() + db.duckDbBatchInsertState() + energyPrice.create()), como hacen todas las pruebas originales del archivo en master. Las versiones anteriores utilizaban stubs que hacían las pruebas menos fiables e inconsistentes con el resto del archivo.Durante el audit, también descubrí que la prueba should handle case where no energy price is found for the device at the given date que está en master no cubre realmente la rama que afirma probar. Razón técnica: su energy_parent_id en la característica de costo apunta a la característica de índice en lugar de la característica de consumo, por lo que el matching falla antes en el código y la rama if (energyPricesForDate.length === 0) nunca se alcanza. Esta rama estaba cubierta en realidad únicamente por una de mis pruebas stubbadas que quería eliminar.
Por lo tanto, he añadido una prueba limpia en base de datos real para preservar esta cobertura. No he modificado la prueba master que presenta el error — me he prohibido tocar el legado.
¡He hecho una revisión de la PR 1!
Gracias a ti,
Te he respondido y he corregido.
También he abierto la PR2 (añadir recálculos solo en la función, sin los rangos de fechas que se implementarán en la PR3) para referencia.
Aún no lo he vuelto a probar todo. Lo haré mañana por la noche.
En todo caso, a mi nivel, estoy bastante impresionado por la rigurosidad de Terdious en la descripción de lo que ha sido preparado. Espero que juntos encuentren la manera de llevar esta evolución hasta una puesta en producción, porque realmente será útil poder hacer estos recálculos dirigidos.
Hola @StephaneB,
Gracias por tu mensaje, me ha conmovido.
Sin embargo, por honestidad y transparencia hacia Pierre-Gilles, me permito matizar un punto:
No soy desarrollador profesional, soy autodidacta. Tengo ideas claras y generalmente logro codificar lo que necesito, pero en cuanto a la rigurosidad de la arquitectura, me apoyo mucho en los comentarios de revisión para progresar — y es completamente normal que esto le lleve más ciclos a Pierre-Gilles conmigo que con otros contribuyentes.
De hecho, desde la PR1, había optado por duplicar dos funciones en lugar de extender las existentes — por exceso de precaución con el núcleo de Gladys, que sé que es sensible. Finalmente, fue la mala elección en este contexto, y Pierre-Gilles tuvo razón en hacérmelo notar. Lo he refactorizado en la dirección correcta.
Por lo tanto, sí, quiero llevar a cabo este desarrollo porque creo que sirve una verdadera necesidad compartida — pero sin atribuirme cualidades que no tengo. La seriedad que puedo poner en la descripción y las pruebas proviene principalmente del tiempo que le dedico y de las herramientas que utilizo, no de un nivel de desarrollo profesional.
Gracias de todos modos por tu apoyo.
Hola @pierre-gilles,
Para información, he publicado un informe de prueba en la PR1 (#2528). He dejado ejecutar una imagen Docker terdious/gladys:energy-monitoring-pr1 en una copia aislada de mi base de datos de producción desde anoche, he lanzado un recálculo completo de costos (1h30, logs limpios), y he comparado los valores recalculados con mi producción mes a mes y año a año — coincide.
Puedo dejarlo ejecutar uno o dos días más para validar el incremento a lo largo del tiempo, o pasar directamente a una imagen de prueba PR2 si lo prefieres. Dime.
¡Muchas gracias, te aviso en cuanto pueda echar un vistazo!