Fix constraint violations display (#1084)

* Remove needless mutation * Attempt to simplify constraints reactive variables * Use the word `violations` consistently * Be robust to nullish violations * Another nullish violations * Add | null * Propagate | null type
NASA-AMMOS · Oct 21, 2024 · f6e019c · f6e019c
1 parent 07ba1de
commit f6e019c
Show file tree

Hide file tree

Showing 12 changed files with 77 additions and 85 deletions.
diff --git a/src/components/constraints/ConstraintsPanel.svelte b/src/components/constraints/ConstraintsPanel.svelte
@@ -8,7 +8,6 @@
   import PlanRightArrow from '@nasa-jpl/stellar/icons/plan_with_right_arrow.svg?component';
   import VisibleHideIcon from '@nasa-jpl/stellar/icons/visible_hide.svg?component';
   import VisibleShowIcon from '@nasa-jpl/stellar/icons/visible_show.svg?component';
-  import { isEmpty } from 'lodash-es';
   import { PlanStatusMessages } from '../../enums/planStatusMessages';
   import {
     checkConstraintsStatus,
@@ -65,18 +64,19 @@
     $constraints.forEach(constraint => {
       const constraintResponse = $constraintResponseMap[constraint.id];
       if (constraintResponse) {
-        // Filter violations/windows by time bounds
-        if (constraintResponse.results && !isEmpty(constraintResponse.results)) {
-          constraintResponse.results.violations = constraintResponse.results.violations.map(violation => ({
-            ...violation,
-            windows: violation.windows.filter(window => window.end >= startTimeMs && window.start <= endTimeMs),
-          }));
-        }
         filteredConstraintResponseMap[constraint.id] = {
           constraintId: constraintResponse.constraintId,
           constraintName: constraintResponse.constraintName,
           errors: constraintResponse.errors,
-          results: constraintResponse.results,
+          results: constraintResponse.results && {
+            ...constraintResponse.results,
+            violations:
+              constraintResponse.results.violations?.map(violation => ({
+                ...violation,
+                // Filter violations/windows by time bounds
+                windows: violation.windows.filter(window => window.end >= startTimeMs && window.start <= endTimeMs),
+              })) ?? null,
+          },
           success: constraintResponse.success,
           type: constraintResponse.type,
         };

diff --git a/src/components/timeline/ConstraintViolations.svelte b/src/components/timeline/ConstraintViolations.svelte
@@ -4,10 +4,10 @@
   import type { ScaleTime } from 'd3-scale';
   import { select } from 'd3-selection';
   import { createEventDispatcher, onMount } from 'svelte';
-  import type { ConstraintResult } from '../../types/constraint';
+  import type { ConstraintResultWithName } from '../../types/constraint';
   import type { TimeRange } from '../../types/timeline';
 
-  export let constraintResults: ConstraintResult[] = [];
+  export let constraintResults: ConstraintResultWithName[] = [];
   export let drawHeight: number = 0;
   export let drawWidth: number = 0;
   export let mousemove: MouseEvent | undefined;
@@ -82,7 +82,7 @@
   function onMousemove(e: MouseEvent | undefined): void {
     if (e) {
       const { offsetX } = e;
-      const constraintResultsWithViolations: ConstraintResult[] = [];
+      const constraintResultsWithViolations: ConstraintResultWithName[] = [];
 
       for (const constraintResult of constraintResults || []) {
         for (const constraintViolation of constraintResult.violations || []) {

diff --git a/src/components/timeline/Row.svelte b/src/components/timeline/Row.svelte
@@ -15,7 +15,7 @@
     ActivityDirectivesMap,
   } from '../../types/activity';
   import type { User } from '../../types/app';
-  import type { ConstraintResult } from '../../types/constraint';
+  import type { ConstraintResultWithName } from '../../types/constraint';
   import type { Plan } from '../../types/plan';
   import type { Resource, SimulationDataset, Span, SpanId, SpansMap, SpanUtilityMaps } from '../../types/simulation';
   import type {
@@ -51,7 +51,7 @@
   export let activityDirectivesByView: ActivityDirectivesByView = { byLayerId: {}, byTimelineId: {} };
   export let activityDirectivesMap: ActivityDirectivesMap = {};
   export let autoAdjustHeight: boolean = false;
-  export let constraintResults: ConstraintResult[] = [];
+  export let constraintResults: ConstraintResultWithName[] = [];
   export let dpr: number = 0;
   export let drawHeight: number = 0;
   export let drawWidth: number = 0;
@@ -102,7 +102,7 @@
   let mouseDownActivityDirectivesByLayer: Record<number, ActivityDirective[]> = {};
   let mouseDownSpansByLayer: Record<number, Span[]> = {};
   let mouseOverActivityDirectivesByLayer: Record<number, ActivityDirective[]> = {};
-  let mouseOverConstraintResults: ConstraintResult[] = []; // For this row.
+  let mouseOverConstraintResults: ConstraintResultWithName[] = []; // For this row.
   let mouseOverPointsByLayer: Record<number, Point[]> = {};
   let mouseOverSpansByLayer: Record<number, Span[]> = {};
   let mouseOverGapsByLayer: Record<number, Point[]> = {};

diff --git a/src/components/timeline/Timeline.svelte b/src/components/timeline/Timeline.svelte
@@ -9,7 +9,7 @@
   import { viewUpdateTimeline } from '../../stores/views';
   import type { ActivityDirectiveId, ActivityDirectivesByView, ActivityDirectivesMap } from '../../types/activity';
   import type { User } from '../../types/app';
-  import type { ConstraintResult } from '../../types/constraint';
+  import type { ConstraintResultWithName } from '../../types/constraint';
   import type { Plan } from '../../types/plan';
   import type {
     Resource,
@@ -54,7 +54,7 @@
 
   export let activityDirectivesByView: ActivityDirectivesByView = { byLayerId: {}, byTimelineId: {} };
   export let activityDirectivesMap: ActivityDirectivesMap = {};
-  export let constraintResults: ConstraintResult[] = [];
+  export let constraintResults: ConstraintResultWithName[] = [];
   export let hasUpdateDirectivePermission: boolean = false;
   export let hasUpdateSimulationPermission: boolean = false;
   export let maxTimeRange: TimeRange = { end: 0, start: 0 };

diff --git a/src/components/timeline/TimelineHistogram.svelte b/src/components/timeline/TimelineHistogram.svelte
@@ -190,7 +190,7 @@
     constraintResults
       .filter(result => !isEmpty(result))
       .forEach(constraintResult => {
-        constraintResult.violations.forEach(violation => {
+        constraintResult.violations?.forEach(violation => {
           violation.windows.forEach(window => {
             if (xScaleMax !== null) {
               const xStart = xScaleMax(window.start);

diff --git a/src/components/timeline/Tooltip.svelte b/src/components/timeline/Tooltip.svelte
@@ -2,18 +2,16 @@
 
 <script lang="ts">
   import { select } from 'd3-selection';
-  import { find } from 'lodash-es';
-  import { constraintResponseMap } from '../../stores/constraints';
   import type { ActivityDirective } from '../../types/activity';
-  import type { ConstraintResponse, ConstraintResult } from '../../types/constraint';
+  import type { ConstraintResultWithName } from '../../types/constraint';
   import type { Span } from '../../types/simulation';
   import type { LinePoint, MouseOver, Point, XRangePoint } from '../../types/timeline';
   import { getDoyTime } from '../../utilities/time';
 
   export let mouseOver: MouseOver | null;
 
   let activityDirectives: ActivityDirective[] = [];
-  let constraintResults: ConstraintResult[] = [];
+  let constraintResults: ConstraintResultWithName[] = [];
   let points: Point[] = [];
   let gaps: Point[] = [];
   let spans: Span[] = [];
@@ -123,7 +121,7 @@
       tooltipText = `${tooltipText}<hr>`;
     }
 
-    constraintResults.forEach((constraintResult: ConstraintResult, i: number) => {
+    constraintResults.forEach((constraintResult: ConstraintResultWithName, i: number) => {
       const text = textForConstraintViolation(constraintResult);
       tooltipText = `${tooltipText} ${text}`;
 
@@ -187,21 +185,14 @@
     `;
   }
 
-  function textForConstraintViolation(constraintViolation: ConstraintResult): string {
-    const matchResponse: ConstraintResponse = find(
-      Object.values(constraintResponseMap),
-      (response: ConstraintResponse) => response.results === constraintViolation,
-    );
-
-    return matchResponse
-      ? `
+  function textForConstraintViolation(constraintViolation: ConstraintResultWithName): string {
+    return `
       <div>
         Constraint Violation
         <br>
-        Name: ${matchResponse.constraintName}
+        Name: ${constraintViolation.constraintName}
       </div>
-    `
-      : '';
+    `;
   }
 
   function textForLinePoint(point: LinePoint): string {

diff --git a/src/components/timeline/XAxis.svelte b/src/components/timeline/XAxis.svelte
@@ -5,14 +5,14 @@
   import { select, type Selection } from 'd3-selection';
   import { zoom as d3Zoom, zoomIdentity, type D3ZoomEvent, type ZoomBehavior, type ZoomTransform } from 'd3-zoom';
   import { createEventDispatcher } from 'svelte';
-  import type { ConstraintResult } from '../../types/constraint';
+  import type { ConstraintResultWithName } from '../../types/constraint';
   import type { TimeRange, XAxisTick } from '../../types/timeline';
   import { getTimeZoneName } from '../../utilities/time';
   import { TimelineInteractionMode } from '../../utilities/timeline';
   import ConstraintViolations from './ConstraintViolations.svelte';
   import RowXAxisTicks from './RowXAxisTicks.svelte';
 
-  export let constraintResults: ConstraintResult[] = [];
+  export let constraintResults: ConstraintResultWithName[] = [];
   export let drawHeight: number = 70;
   export let drawWidth: number = 0;
   export let marginLeft: number = 50;

diff --git a/src/routes/plans/[id]/+page.svelte b/src/routes/plans/[id]/+page.svelte
@@ -37,7 +37,7 @@
     selectActivity,
     selectedActivityDirectiveId,
   } from '../../../stores/activities';
-  import { checkConstraintsStatus, constraintResults, resetConstraintStores } from '../../../stores/constraints';
+  import { checkConstraintsStatus, constraintResponseMap, resetConstraintStores } from '../../../stores/constraints';
   import {
     activityErrorRollups,
     allErrors,
@@ -616,7 +616,11 @@
       >
         <VerticalCollapseIcon />
         <svelte:fragment slot="metadata">
-          <div>Constraint violations: {$constraintResults.filter(result => result.violations.length).length}</div>
+          <div>
+            Constraints violated: {Object.values($constraintResponseMap).filter(
+              response => response.results.violations?.length,
+            ).length}
+          </div>
         </svelte:fragment>
       </PlanNavButton>
       <PlanNavButton

diff --git a/src/stores/constraints.ts b/src/stores/constraints.ts
@@ -1,6 +1,6 @@
-import { isEmpty, keyBy } from 'lodash-es';
+import { keyBy } from 'lodash-es';
 import { derived, get, writable, type Readable, type Writable } from 'svelte/store';
-import type { Constraint, ConstraintResponse, ConstraintResult } from '../types/constraint';
+import type { Constraint, ConstraintResponse, ConstraintResultWithName } from '../types/constraint';
 import gql from '../utilities/gql';
 import type { Status } from '../utilities/status';
 import { modelId, planId, planStartTimeMs } from './plan';
@@ -34,48 +34,44 @@ export const constraintVisibilityMap: Readable<Record<Constraint['id'], boolean>
 
 export const checkConstraintsStatus: Writable<Status | null> = writable(null);
 
-export const constraintResponse: Writable<ConstraintResponse[]> = writable([]);
+export const rawConstraintResponses: Writable<ConstraintResponse[]> = writable([]);
 
 export const constraintsColumns: Writable<string> = writable('2fr 3px 1fr');
 export const constraintsFormColumns: Writable<string> = writable('1fr 3px 2fr');
 
 /* Derived. */
 
-export const constraintResults: Readable<ConstraintResult[]> = derived(
-  [constraintResponse, planStartTimeMs],
-  ([$constraintResponse, $planStartTimeMs]) =>
-    $constraintResponse
-      .filter(response => response.success)
-      .map(successfulResponse => successfulResponse.results)
-      .reduce((list: ConstraintResult[], constraintResult) => {
-        list.push({
-          ...constraintResult,
-          violations: !isEmpty(constraintResult)
-            ? constraintResult.violations.map(violation => ({
-                ...violation,
-                windows: violation.windows.map(({ end, start }) => ({
-                  end: $planStartTimeMs + end / 1000,
-                  start: $planStartTimeMs + start / 1000,
-                })),
-              }))
-            : [],
-        });
-
-        return list;
-      }, []),
+export const constraintResponseMap: Readable<Record<Constraint['id'], ConstraintResponse>> = derived(
+  [rawConstraintResponses, planStartTimeMs],
+  ([$constraintResponses, $planStartTimeMs]) =>
+    keyBy(
+      $constraintResponses.map(response => ({
+        ...response,
+        results: {
+          ...response.results,
+          violations:
+            response.results.violations?.map(violation => ({
+              ...violation,
+              windows: violation.windows.map(({ end, start }) => ({
+                end: $planStartTimeMs + end / 1000,
+                start: $planStartTimeMs + start / 1000,
+              })),
+            })) ?? null,
+        },
+      })),
+      'constraintId',
+    ),
 );
 
-export const visibleConstraintResults: Readable<ConstraintResult[]> = derived(
-  [constraintResponse, constraintVisibilityMap],
-  ([$constraintResponse, $constraintVisibilityMap]) =>
-    $constraintResponse
+export const visibleConstraintResults: Readable<ConstraintResultWithName[]> = derived(
+  [constraintResponseMap, constraintVisibilityMap],
+  ([$constraintResponseMap, $constraintVisibilityMap]) =>
+    Object.values($constraintResponseMap)
       .filter(response => $constraintVisibilityMap[response.constraintId])
-      .map(response => response.results),
-);
-
-export const constraintResponseMap: Readable<Record<Constraint['id'], ConstraintResponse>> = derived(
-  [constraintResponse],
-  ([$constraintResponse]) => keyBy($constraintResponse, 'constraintId'),
+      .map(response => ({
+        ...response.results,
+        constraintName: response.constraintName,
+      })),
 );
 
 /* Helper Functions. */
@@ -95,5 +91,5 @@ export function setAllConstraintsVisible(visible: boolean) {
 
 export function resetConstraintStores(): void {
   checkConstraintsStatus.set(null);
-  constraintResponse.set([]);
+  rawConstraintResponses.set([]);
 }
diff --git a/src/types/constraint.ts b/src/types/constraint.ts
@@ -31,9 +31,11 @@ export type ConstraintViolation = {
 export type ConstraintResult = {
   gaps: TimeRange[];
   resourceIds: string[];
-  violations: ConstraintViolation[];
+  violations: ConstraintViolation[] | null;
 };
 
+export type ConstraintResultWithName = ConstraintResult & { constraintName: string };
+
 export type ConstraintResponse = {
   constraintId: Constraint['id'];
   constraintName: Constraint['name'];

diff --git a/src/types/timeline.ts b/src/types/timeline.ts
@@ -1,6 +1,6 @@
 import type { Selection } from 'd3-selection';
 import type { ActivityDirective } from './activity';
-import type { ConstraintResult } from './constraint';
+import type { ConstraintResultWithName } from './constraint';
 import type { Span } from './simulation';
 
 export interface ActivityLayer extends Layer {
@@ -96,7 +96,7 @@ export type MouseDown = {
 
 export type MouseOver = {
   activityDirectives?: ActivityDirective[];
-  constraintResults?: ConstraintResult[];
+  constraintResults?: ConstraintResultWithName[];
   e: MouseEvent;
   gaps?: Point[];
   layerId: number;

diff --git a/src/utilities/effects.ts b/src/utilities/effects.ts
@@ -5,7 +5,7 @@ import type { CommandDictionary as AmpcsCommandDictionary } from '@nasa-jpl/aeri
 import { get } from 'svelte/store';
 import { SearchParameters } from '../enums/searchParameters';
 import { activityDirectives, activityDirectivesMap, selectedActivityDirectiveId } from '../stores/activities';
-import { checkConstraintsStatus, constraintResponse } from '../stores/constraints';
+import { checkConstraintsStatus, rawConstraintResponses } from '../stores/constraints';
 import { catchError, catchSchedulingError } from '../stores/errors';
 import {
   createExpansionRuleError,
@@ -311,23 +311,22 @@ const effects = {
           },
           user,
         );
-        const { constraintResponses } = data;
-        if (constraintResponses) {
-          constraintResponse.set(constraintResponses);
+        if (data.constraintResponses) {
+          rawConstraintResponses.set(data.constraintResponses);
 
           // find only the constraints compiled.
-          const successfulConstraintResults: ConstraintResult[] = constraintResponses
+          const successfulConstraintResults: ConstraintResult[] = data.constraintResponses
             .filter(constraintResponse => constraintResponse.success)
             .map(constraintResponse => constraintResponse.results);
 
-          const failedConstraintResponses = constraintResponses.filter(
+          const failedConstraintResponses = data.constraintResponses.filter(
             constraintResponse => !constraintResponse.success,
           );
 
-          if (successfulConstraintResults.length === 0 && constraintResponses.length > 0) {
+          if (successfulConstraintResults.length === 0 && data.constraintResponses.length > 0) {
             showFailureToast('All Constraints Failed');
             checkConstraintsStatus.set(Status.Failed);
-          } else if (successfulConstraintResults.length !== constraintResponses.length) {
+          } else if (successfulConstraintResults.length !== data.constraintResponses.length) {
             showFailureToast('Partial Constraints Checked');
             checkConstraintsStatus.set(successfulConstraintResults.length !== 0 ? Status.Incomplete : Status.Failed);
           } else {