Simplifications on the hooks to do AGS

common/src/main/scala/explore/model/reusability.scala

@@ -35,6 +35,9 @@ import lucuma.schemas.ObservationDB.Enums.Existence
 import lucuma.schemas.model.*
 import lucuma.ui.reusability.given
 
+import java.time.Duration
+import java.time.Instant
+
 /**
  * Reusability instances for model classes
  */
@@ -129,3 +132,12 @@ object reusability:
   given Reusability[CategoryAllocationList]             = Reusability.byEq
   given Reusability[InstrumentOverrides]                = Reusability.byEq
   given [A: Reusability]: Reusability[NonEmptyChain[A]] = Reusability.by(_.toNonEmptyList)
+
+  // We want to re render only when the vizTime changes at least a month
+  // We keep the candidates data pm corrected for the viz time
+  // If it changes over a month we'll request the data again and recalculate
+  // This way we avoid recalculating pm for example if only pos angle or
+  // conditions change
+  val siderealTargetReusability: Reusability[Instant] = Reusability[Instant] {
+    Duration.between(_, _).toDays().abs < 30L
+  }

explore/src/main/scala/explore/targeteditor/AladinCell.scala

@@ -30,6 +30,7 @@ import explore.model.boopickle.CatalogPicklers.given
 import explore.model.enums.AgsState
 import explore.model.enums.Visible
 import explore.model.reusability.given
+import explore.model.reusability.siderealTargetReusability
 import explore.optics.ModelOptics
 import japgolly.scalajs.react.*
 import japgolly.scalajs.react.vdom.html_<^.*
@@ -53,7 +54,6 @@ import monocle.Lens
 import org.typelevel.log4cats.Logger
 import queries.schemas.UserPreferencesDB
 
-import java.time.Duration
 import java.time.Instant
 import scala.concurrent.duration.*
 
@@ -142,14 +142,10 @@ object AladinCell extends ModelOptics with AladinCommon:
 
   private type Props = AladinCell
 
-  // We want to re render only when the vizTime changes at least a month
-  // We keep the candidates data pm corrected for the viz time
-  // If it changes over a month we'll request the data again and recalculate
-  // This way we avoid recalculating pm for example if only pos angle or
-  // conditions change
-  private given Reusability[Instant] = Reusability[Instant] {
-    Duration.between(_, _).toDays().abs < 30L
-  }
+  private given Reusability[Instant] = siderealTargetReusability
+
+  // only compare candidates by id
+  private given Reusability[GuideStarCandidate] = Reusability.by(_.id)
 
   private val fovLens: Lens[AsterismVisualOptions, Fov] =
     Lens[AsterismVisualOptions, Fov](t => Fov(t.fovRA, t.fovDec))(f =>
@@ -230,29 +226,28 @@ object AladinCell extends ModelOptics with AladinCommon:
       // target options, will be read from the user preferences
       .useStateView(Pot.pending[AsterismVisualOptions])
       // to get faster reusability use a serial state, rather than check every candidate
-      .useSerialState(none[List[GuideStarCandidate]])
-      // Analysis results
-      .useSerialState(List.empty[AgsAnalysis])
       // Request data again if vizTime changes more than a month
-      .useEffectWithDepsBy((p, _, _, _, _) => (p.vizTime, p.asterism.baseTracking)) {
-        (props, ctx, _, gs, _) => (vizTime, baseTracking) =>
+      .useEffectKeepResultWithDepsBy((p, _, _) => (p.vizTime, p.asterism.baseTracking)) {
+        (props, ctx, _) => (vizTime, baseTracking) =>
           import ctx.given
 
-          (for {
-            _          <- props.obsConf
-                            .flatMap(_.agsState)
-                            .foldMap(_.async.set(AgsState.LoadingCandidates))
-            candidates <- CatalogClient[IO]
-                            .requestSingle(
-                              CatalogMessage.GSRequest(baseTracking, vizTime)
-                            )
-            _          <- gs.setStateAsync(candidates)
-          } yield ())
-            .guarantee(
-              props.obsConf.flatMap(_.agsState).foldMap(_.async.set(AgsState.Idle))
-            )
-            .whenA(props.needsAGS)
+          if (props.needsAGS)
+            (for {
+              _          <- props.obsConf
+                              .flatMap(_.agsState)
+                              .foldMap(_.async.set(AgsState.LoadingCandidates))
+              candidates <- CatalogClient[IO]
+                              .requestSingle(
+                                CatalogMessage.GSRequest(baseTracking, vizTime)
+                              )
+            } yield candidates)
+              .guarantee(
+                props.obsConf.flatMap(_.agsState).foldMap(_.async.set(AgsState.Idle))
+              )
+          else none.pure
       }
+      // Analysis results
+      .useSerialState(List.empty[AgsAnalysis])
       // Reference to root
       .useMemo(())(_ => domRoot)
       // Load target preferences
@@ -301,7 +296,7 @@ object AladinCell extends ModelOptics with AladinCommon:
          p.obsConf.flatMap(_.wavelength),
          p.vizTime,
          p.obsConf.flatMap(_.configuration),
-         candidates.value
+         candidates.toOption.flatten
         )
       ) { (props, ctx, _, _, ags, _, selectedIndex, _, agsOverride) =>
         {
@@ -314,13 +309,13 @@ object AladinCell extends ModelOptics with AladinCommon:
                 vizTime,
                 observingMode,
                 candidates
-              ) if props.needsAGS =>
+              ) if props.needsAGS && candidates.nonEmpty =>
             import ctx.given
 
             val runAgs = (positions,
                           tracking.at(vizTime),
                           props.obsConf.flatMap(_.agsState),
-                          candidates.value
+                          candidates
             ).mapN { (positions, base, agsState, candidates) =>
 
               val fpu    = observingMode.flatMap(_.fpuAlternative)
@@ -496,7 +491,7 @@ object AladinCell extends ModelOptics with AladinCommon:
                         agsState.get,
                         props.modeSelected,
                         props.durationAvailable,
-                        candidates.value.isDefined
+                        candidates.nonEmpty
                       )
                     )
                   )

explore/src/main/scala/explore/targeteditor/AladinContainer.scala

@@ -19,6 +19,7 @@ import explore.model.GlobalPreferences
 import explore.model.ObsConfiguration
 import explore.model.enums.Visible
 import explore.model.reusability.given
+import explore.model.reusability.siderealTargetReusability
 import explore.visualization.*
 import japgolly.scalajs.react.*
 import japgolly.scalajs.react.Reusability.*
@@ -73,7 +74,10 @@ object AladinContainer extends AladinCommon {
       (t.target.id, NonEmptyList.of((Angle.Angle0, Area.MaxArea))).some
     case _                                            => None
   })
-  private given Reusability[List[AgsAnalysis]]   = Reusability.by(_.length)
+
+  private given Reusability[List[AgsAnalysis]] = Reusability.by(_.length)
+
+  private given Reusability[Instant] = siderealTargetReusability
 
   private val AladinComp = Aladin.component
 
@@ -311,7 +315,7 @@ object AladinContainer extends AladinCommon {
             )
           )
 
-        val sciencePositions        =
+        val sciencePositions =
           if (scienceTargets.length > 1)
             scienceTargets.flatMap { (selected, name, pm, base) =>
               pm.foldMap { pm =>
@@ -345,6 +349,7 @@ object AladinContainer extends AladinCommon {
               if visible.isVisible
             } yield SVGTarget.OffsetIndicator(c, idx, o, oType, css, 4)
           }
+
         val scienceOffsetIndicators =
           offsetIndicators(
             _.scienceOffsets,

explore/src/main/scala/explore/targeteditor/SiderealTargetEditor.scala

@@ -27,6 +27,7 @@ import explore.model.ObservationsAndTargets
 import explore.model.OnCloneParameters
 import explore.model.TargetEditObsInfo
 import explore.model.reusability.given
+import explore.model.reusability.siderealTargetReusability
 import explore.syntax.ui.*
 import explore.undo.UndoSetter
 import explore.utils.*
@@ -79,6 +80,8 @@ case class SiderealTargetEditor(
 object SiderealTargetEditor:
   private type Props = SiderealTargetEditor
 
+  private given Reusability[Instant] = siderealTargetReusability
+
   private def cloneTarget(
     programId:     Program.Id,
     targetId:      Target.Id,
@@ -167,7 +170,7 @@ object SiderealTargetEditor:
       .useStateView(false)          // cloning
       .useStateView(none[ObsIdSet]) // obs ids to clone to.
       // If vizTime is not set, change it to now
-      .useEffectResultWithDepsBy((p, _, _, _) => p.vizTime) { (_, _, _, _) => vizTime =>
+      .useEffectKeepResultWithDepsBy((p, _, _, _) => p.vizTime) { (_, _, _, _) => vizTime =>
         IO(vizTime.getOrElse(Instant.now()))
       }
       // select the aligner to use based on whether a clone will be created or not.

model/shared/src/main/scala/explore/model/enums/AgsState.scala

@@ -16,7 +16,7 @@ enum AgsState:
       case Saving            => saving
       case Error             => error
 
-  def isCalculating: Boolean = fold(false, true, true, false, false)
+  def isCalculating: Boolean = fold(false, false, true, false, false)
 
   def isIdle: Boolean = fold(true, false, false, false, false)
 

project/Versions.scala

@@ -8,7 +8,7 @@ object Versions {
   val circeGolden            = "0.3.0"
   val coulomb                = "0.8.0"
   val clue                   = "0.40.0"
-  val crystal                = "0.45.1"
+  val crystal                = "0.45.2"
   val discipline             = "1.7.0"
   val disciplineMUnit        = "2.0.0"
   val fs2                    = "3.11.0"