Reconstruct the risk analysis treatment plans as of a date - #1782
Reconstruct the risk analysis treatment plans as of a date#1782SachaProbo wants to merge 1 commit into
Conversation
f99eb1f to
dfef3e7
Compare
dfef3e7 to
0b35996
Compare
|
@cubic-dev-ai please review |
@SachaProbo I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 42 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
4803510 to
6e0415d
Compare
6e0415d to
8e143fe
Compare
There was a problem hiding this comment.
All reported issues were addressed across 48 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
e047f4c to
f0539a7
Compare
f0539a7 to
9fa10b0
Compare
There was a problem hiding this comment.
All reported issues were addressed across 48 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
9fa10b0 to
ce303d1
Compare
There was a problem hiding this comment.
All reported issues were addressed across 49 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
ce303d1 to
0dc2e51
Compare
There was a problem hiding this comment.
8 issues found across 49 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/coredata/measure_event.go">
<violation number="1" location="pkg/coredata/measure_event.go:66">
P2: Historical measure nodes always return a nil description. `MeasureEvent` does not snapshot `Description`, so `Measure()` drops any description that existed at the requested instant. Persist and restore the description in measure events, including migration backfill and event writers.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanProgressBar.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanProgressBar.tsx:50">
P2: Screen-reader users cannot determine the in-progress or not-implemented counts because this bar exposes status only through color and the visible text reports `done/total`. Add an accessible progress label or visually hidden legend containing all three status counts, with localized status names.</violation>
</file>
<file name="pkg/riskmanagement/as_of.go">
<violation number="1" location="pkg/riskmanagement/as_of.go:607">
P2: `ListMeasuresAsOf` bypasses SQL pagination and materializes every historical measure linked to a plan before applying the cursor. Add a historical coredata loader that applies the filter, cursor, and limit in SQL to avoid unbounded results and memory use.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx:183">
P1: When switching from a historical date back to today, this marks unplanned data ready before the refetch completes. The component then mounts `RiskAnalysisUnplannedPlans` against a fragment fetched with `includeUnplanned: false`, causing the unplanned pagination fragment to be missing and the section to fail instead of loading current unplanned risks. Keep unplanned rendering gated until the refetch has completed.</violation>
</file>
<file name="pkg/server/api/console/v1/treatment_plan_resolvers.go">
<violation number="1" location="pkg/server/api/console/v1/treatment_plan_resolvers.go:206">
P2: When a principal can list treatment plans but cannot read membership profiles, this historical branch still returns the plan owner through `loaders.Profile`. Preserve the `iam.ActionMembershipProfileGet` check (in addition to the historical plan check) before loading the profile.</violation>
</file>
<file name="pkg/riskmanagement/treatment_plan.go">
<violation number="1" location="pkg/riskmanagement/treatment_plan.go:474">
P2: When a delete request is retried after the plan was removed, `LoadByIDForUpdate` returns not-found and breaks the existing idempotent delete behavior. Treat not-found as success while recording a deletion event only when the plan was loaded.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/RiskAnalysisTreatmentPlansPageLoader.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/RiskAnalysisTreatmentPlansPageLoader.tsx:53">
P1: After opening a historical URL, this omits the unplanned fragment. Returning to today sets `unplannedReady` during render before `refetch` supplies that fragment, so the unplanned list can read missing data; gate readiness on the completed refetch.</violation>
</file>
<file name="pkg/probo/measure_service.go">
<violation number="1" location="pkg/probo/measure_service.go:476">
P1: If a delete commits after this update loads the measure, the zero-row update still records an UPDATE event, making the deleted measure reappear in as-of reads. Serialize the operations or verify the update affected a row before recording the event.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| riskAnalysisId, | ||
| filter: treatmentPlanFilterFromCell(parseMatrixCell(paramsRef.current)), | ||
| asOf, | ||
| includeUnplanned: asOf == null, |
There was a problem hiding this comment.
P1: After opening a historical URL, this omits the unplanned fragment. Returning to today sets unplannedReady during render before refetch supplies that fragment, so the unplanned list can read missing data; gate readiness on the completed refetch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/RiskAnalysisTreatmentPlansPageLoader.tsx, line 53:
<comment>After opening a historical URL, this omits the unplanned fragment. Returning to today sets `unplannedReady` during render before `refetch` supplies that fragment, so the unplanned list can read missing data; gate readiness on the completed refetch.</comment>
<file context>
@@ -44,9 +45,12 @@ export default function RiskAnalysisTreatmentPlansPageLoader() {
riskAnalysisId,
filter: treatmentPlanFilterFromCell(parseMatrixCell(paramsRef.current)),
+ asOf,
+ includeUnplanned: asOf == null,
});
}
</file context>
| eventType = coredata.MeasureEventTypeUpdated | ||
| } | ||
|
|
||
| if err := insertMeasureEvent( |
There was a problem hiding this comment.
P1: If a delete commits after this update loads the measure, the zero-row update still records an UPDATE event, making the deleted measure reappear in as-of reads. Serialize the operations or verify the update affected a row before recording the event.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/probo/measure_service.go, line 476:
<comment>If a delete commits after this update loads the measure, the zero-row update still records an UPDATE event, making the deleted measure reappear in as-of reads. Serialize the operations or verify the update affected a row before recording the event.</comment>
<file context>
@@ -463,10 +463,27 @@ func (s MeasureService) Import(
+ eventType = coredata.MeasureEventTypeUpdated
+ }
+
+ if err := insertMeasureEvent(
+ ctx,
+ tx,
</file context>
| return fmt.Errorf("cannot load measure events: %w", err) | ||
| } | ||
|
|
||
| measures := measuresFromEvents(linked, measureEvents) |
There was a problem hiding this comment.
P2: ListMeasuresAsOf bypasses SQL pagination and materializes every historical measure linked to a plan before applying the cursor. Add a historical coredata loader that applies the filter, cursor, and limit in SQL to avoid unbounded results and memory use.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/riskmanagement/as_of.go, line 607:
<comment>`ListMeasuresAsOf` bypasses SQL pagination and materializes every historical measure linked to a plan before applying the cursor. Add a historical coredata loader that applies the filter, cursor, and limit in SQL to avoid unbounded results and memory use.</comment>
<file context>
@@ -0,0 +1,752 @@
+ return fmt.Errorf("cannot load measure events: %w", err)
+ }
+
+ measures := measuresFromEvents(linked, measureEvents)
+
+ total = len(measures)
</file context>
0dc2e51 to
818b61f
Compare
There was a problem hiding this comment.
9 issues found across 50 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/coredata/migrations/20260826T135100Z.sql">
<violation number="1" location="pkg/coredata/migrations/20260826T135100Z.sql:29">
P2: As-of reads will scan and sort the entire event history for each request because these tables have no supporting indexes. Add composite lookup indexes covering tenant, entity scope, and `created_at DESC` before event history grows.</violation>
</file>
<file name="pkg/coredata/measure_event_type.go">
<violation number="1" location="pkg/coredata/measure_event_type.go:44">
P3: This enum omits the standard `MeasureEventTypes()` helper. Add it so callers can enumerate all valid event values without duplicating the validation switch.</violation>
</file>
<file name="pkg/riskmanagement/as_of.go">
<violation number="1" location="pkg/riskmanagement/as_of.go:451">
P2: When one measure is linked to plans from multiple analyses, this loop performs separate event loads for each analysis. Batch event and state loading across all page plans to avoid O(number of analyses) database round trips.</violation>
<violation number="2" location="pkg/riskmanagement/as_of.go:638">
P2: When an as-of measure is returned, `event.Measure()` leaves `Description` nil and `ReferenceID` empty because `MeasureEvent` does not snapshot either field. Persist and reconstruct those fields before exposing historical measures.</violation>
</file>
<file name="pkg/coredata/treatment_plan_event.go">
<violation number="1" location="pkg/coredata/treatment_plan_event.go:252">
P2: When two events for one treatment plan have the same stored `created_at`, `DISTINCT ON` has no deterministic winner, so as-of reads can return stale plan fields or resurrect a deleted plan. Add a monotonic event sequence/unique event identifier and order by it after `created_at` in both loaders.</violation>
</file>
<file name="pkg/probo/measure_service.go">
<violation number="1" location="pkg/probo/measure_service.go:617">
P2: Description-only edits never enter the event stream because this guard ignores `Description`; as-of measures therefore cannot reflect those edits. Include `Description` in the event snapshot and emit an update event whenever it changes.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanMeasureList.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanMeasureList.tsx:344">
P2: When a historical measure has been deleted, this branch cannot render the new deleted label because the historical API filters deleted events before returning rows; the measure disappears instead. Expose a deleted placeholder in the API or remove this unreachable UI branch.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/_lib/matrixAsOf.ts">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/_lib/matrixAsOf.ts:58">
P2: In non-UTC time zones, the local minimum date can precede the UTC date used by the as-of cutoff, allowing a selectable date before the analysis existed and showing an empty or incomplete snapshot. Derive the minimum from the UTC calendar date used by `matrixAsOf`.</violation>
</file>
<file name="pkg/coredata/measure_filter.go">
<violation number="1" location="pkg/coredata/measure_filter.go:93">
P2: When an as-of measure search contains tsquery punctuation, this builds invalid `to_tsquery` input and the measure connection returns a database error. Normalize or escape query tokens before appending `:*`, as the live search path also intends.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| 'MEASURE_UNLINKED' | ||
| ); | ||
|
|
||
| CREATE TABLE treatment_plan_events ( |
There was a problem hiding this comment.
P2: As-of reads will scan and sort the entire event history for each request because these tables have no supporting indexes. Add composite lookup indexes covering tenant, entity scope, and created_at DESC before event history grows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/migrations/20260826T135100Z.sql, line 29:
<comment>As-of reads will scan and sort the entire event history for each request because these tables have no supporting indexes. Add composite lookup indexes covering tenant, entity scope, and `created_at DESC` before event history grows.</comment>
<file context>
@@ -0,0 +1,141 @@
+ 'MEASURE_UNLINKED'
+);
+
+CREATE TABLE treatment_plan_events (
+ tenant_id TEXT NOT NULL,
+ organization_id TEXT NOT NULL REFERENCES organizations(id)
</file context>
| byAnalysis[plan.RiskAnalysisID] = append(byAnalysis[plan.RiskAnalysisID], plan) | ||
| } | ||
|
|
||
| for analysisID, group := range byAnalysis { |
There was a problem hiding this comment.
P2: When one measure is linked to plans from multiple analyses, this loop performs separate event loads for each analysis. Batch event and state loading across all page plans to avoid O(number of analyses) database round trips.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/riskmanagement/as_of.go, line 451:
<comment>When one measure is linked to plans from multiple analyses, this loop performs separate event loads for each analysis. Batch event and state loading across all page plans to avoid O(number of analyses) database round trips.</comment>
<file context>
@@ -0,0 +1,752 @@
+ byAnalysis[plan.RiskAnalysisID] = append(byAnalysis[plan.RiskAnalysisID], plan)
+ }
+
+ for analysisID, group := range byAnalysis {
+ progress, _, err := loadAsOfPlanExtras(
+ ctx,
</file context>
| continue | ||
| } | ||
|
|
||
| measures = append(measures, event.Measure()) |
There was a problem hiding this comment.
P2: When an as-of measure is returned, event.Measure() leaves Description nil and ReferenceID empty because MeasureEvent does not snapshot either field. Persist and reconstruct those fields before exposing historical measures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/riskmanagement/as_of.go, line 638:
<comment>When an as-of measure is returned, `event.Measure()` leaves `Description` nil and `ReferenceID` empty because `MeasureEvent` does not snapshot either field. Persist and reconstruct those fields before exposing historical measures.</comment>
<file context>
@@ -0,0 +1,752 @@
+ continue
+ }
+
+ measures = append(measures, event.Measure())
+ }
+
</file context>
| AND created_at < @as_of | ||
| ORDER BY | ||
| treatment_plan_id, | ||
| created_at DESC |
There was a problem hiding this comment.
P2: When two events for one treatment plan have the same stored created_at, DISTINCT ON has no deterministic winner, so as-of reads can return stale plan fields or resurrect a deleted plan. Add a monotonic event sequence/unique event identifier and order by it after created_at in both loaders.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan_event.go, line 252:
<comment>When two events for one treatment plan have the same stored `created_at`, `DISTINCT ON` has no deterministic winner, so as-of reads can return stale plan fields or resurrect a deleted plan. Add a monotonic event sequence/unique event identifier and order by it after `created_at` in both loaders.</comment>
<file context>
@@ -0,0 +1,370 @@
+ AND created_at < @as_of
+ ORDER BY
+ treatment_plan_id,
+ created_at DESC
+) latest
+WHERE
</file context>
| return fmt.Errorf("cannot update measure: %w", err) | ||
| } | ||
|
|
||
| if measure.Name != previousName || measure.State != previousState || measure.Category != previousCategory { |
There was a problem hiding this comment.
P2: Description-only edits never enter the event stream because this guard ignores Description; as-of measures therefore cannot reflect those edits. Include Description in the event snapshot and emit an update event whenever it changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/probo/measure_service.go, line 617:
<comment>Description-only edits never enter the event stream because this guard ignores `Description`; as-of measures therefore cannot reflect those edits. Include `Description` in the event snapshot and emit an update event whenever it changes.</comment>
<file context>
@@ -593,6 +614,19 @@ func (s MeasureService) Update(
return fmt.Errorf("cannot update measure: %w", err)
}
+ if measure.Name != previousName || measure.State != previousState || measure.Category != previousCategory {
+ if err := insertMeasureEvent(
+ ctx,
</file context>
| const { t } = useTranslation(); | ||
| const organizationId = useOrganizationId(); | ||
| const measure = useFragment(measureFragment, measureKey); | ||
| const deleted = measure.name === ""; |
There was a problem hiding this comment.
P2: When a historical measure has been deleted, this branch cannot render the new deleted label because the historical API filters deleted events before returning rows; the measure disappears instead. Expose a deleted placeholder in the API or remove this unreachable UI branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanMeasureList.tsx, line 344:
<comment>When a historical measure has been deleted, this branch cannot render the new deleted label because the historical API filters deleted events before returning rows; the measure disappears instead. Expose a deleted placeholder in the API or remove this unreachable UI branch.</comment>
<file context>
@@ -309,26 +328,43 @@ export function TreatmentPlanMeasureList({
const { t } = useTranslation();
const organizationId = useOrganizationId();
const measure = useFragment(measureFragment, measureKey);
+ const deleted = measure.name === "";
return (
</file context>
| return todayAsDateInput(); | ||
| } | ||
|
|
||
| return localDateInput(parsed); |
There was a problem hiding this comment.
P2: In non-UTC time zones, the local minimum date can precede the UTC date used by the as-of cutoff, allowing a selectable date before the analysis existed and showing an empty or incomplete snapshot. Derive the minimum from the UTC calendar date used by matrixAsOf.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/_lib/matrixAsOf.ts, line 58:
<comment>In non-UTC time zones, the local minimum date can precede the UTC date used by the as-of cutoff, allowing a selectable date before the analysis existed and showing an empty or incomplete snapshot. Derive the minimum from the UTC calendar date used by `matrixAsOf`.</comment>
<file context>
@@ -0,0 +1,100 @@
+ return todayAsDateInput();
+ }
+
+ return localDateInput(parsed);
+}
+
</file context>
| TRUE | ||
| ELSE | ||
| to_tsvector('simple', name) @@ ( | ||
| SELECT to_tsquery('simple', string_agg(lexeme || ':*', ' & ')) |
There was a problem hiding this comment.
P2: When an as-of measure search contains tsquery punctuation, this builds invalid to_tsquery input and the measure connection returns a database error. Normalize or escape query tokens before appending :*, as the live search path also intends.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure_filter.go, line 93:
<comment>When an as-of measure search contains tsquery punctuation, this builds invalid `to_tsquery` input and the measure connection returns a database error. Normalize or escape query tokens before appending `:*`, as the live search path also intends.</comment>
<file context>
@@ -81,3 +81,37 @@ AND
+ TRUE
+ ELSE
+ to_tsvector('simple', name) @@ (
+ SELECT to_tsquery('simple', string_agg(lexeme || ':*', ' & '))
+ FROM unnest(regexp_split_to_array(trim(@query), '\s+')) AS lexeme
+ )
</file context>
| _ encoding.TextUnmarshaler = (*MeasureEventType)(nil) | ||
| ) | ||
|
|
||
| func (v MeasureEventType) IsValid() bool { |
There was a problem hiding this comment.
P3: This enum omits the standard MeasureEventTypes() helper. Add it so callers can enumerate all valid event values without duplicating the validation switch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure_event_type.go, line 44:
<comment>This enum omits the standard `MeasureEventTypes()` helper. Add it so callers can enumerate all valid event values without duplicating the validation switch.</comment>
<file context>
@@ -0,0 +1,73 @@
+ _ encoding.TextUnmarshaler = (*MeasureEventType)(nil)
+)
+
+func (v MeasureEventType) IsValid() bool {
+ switch v {
+ case
</file context>
818b61f to
84b170e
Compare
There was a problem hiding this comment.
4 existing issues remain and 10 new issues found across 50 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/riskmanagement/treatment_plan.go">
<violation number="1" location="pkg/riskmanagement/treatment_plan.go:293">
P2: When the mapping is already absent, this added event is still recorded because `DeleteMeasureMapping` does not verify that `mapping.Delete` removed a row. As-of reads then report a later update for an unlink that never changed the plan; only insert the unlink event when a mapping was actually deleted.</violation>
</file>
<file name="pkg/coredata/treatment_plan_event.go">
<violation number="1" location="pkg/coredata/treatment_plan_event.go:91">
P2: After a referenced risk’s category changes, as-of treatment-plan reads keep the old category because no treatment-plan snapshot is recorded. Record a snapshot when the risk category changes or source category from versioned risk history.</violation>
<violation number="2" location="pkg/coredata/treatment_plan_event.go:127">
P2: When the latest event only links or unlinks a measure, this reconstruction reports the event time as the plan’s `UpdatedAt`, although the plan row was not updated. Preserve the plan’s actual updated timestamp in events or update it consistently for measure changes.</violation>
</file>
<file name="pkg/cmd/treatment-plan/list/list.go">
<violation number="1" location="pkg/cmd/treatment-plan/list/list.go:117">
P2: Existing `--output json` consumers lose the `risk.category` field on every live treatment-plan listing, even when `--as-of` is not used. Preserve the established JSON shape while adding a separate snapshot-aware field or serialization path for the table/as-of output.</violation>
</file>
<file name="pkg/coredata/migrations/20260826T135100Z.sql">
<violation number="1" location="pkg/coredata/migrations/20260826T135100Z.sql:115">
P1: When an existing plan or measure was edited before this migration, its seed event is backdated to the entity’s creation time while containing current values. Use the migration timestamp for the seed event `created_at` and retain the entity timestamp only in `*_created_at`, so unrecoverable pre-migration dates do not return fabricated history.</violation>
</file>
<file name="pkg/coredata/measure_filter.go">
<violation number="1" location="pkg/coredata/measure_filter.go:104">
P1: As-of measure queries fail because the repeated `@state` parameter is cast to both `mitigation_state` and `text`. Keep the parameter type consistent, for example by casting the enum to text after the enum cast.</violation>
</file>
<file name="pkg/coredata/treatment_plan.go">
<violation number="1" location="pkg/coredata/treatment_plan.go:241">
P2: Keep `LoadByIDForUpdate` limited to the treatment-plan table, then load the risk separately to obtain `category`; coredata entity methods must not add cross-entity joins.</violation>
<violation number="2" location="pkg/coredata/treatment_plan.go:695">
P2: When a plan snapshot still contains a measure ID after that measure is deleted, these net-score filters use the deleted event’s stored state. Exclude deleted measure events when selecting `latest_state` and pass the deleted event type as a query argument.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx:304">
P2: When the as-of date or cell filter changes, this effect fetches treatment plans again after the parent fragment has already fetched them with the same `filter` and `asOf`. Coordinate the parent and pagination refetches so date/filter changes issue one plans request, while sort changes still refetch the pagination fragment.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/RiskAnalysisTreatmentPlansPageLoader.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/RiskAnalysisTreatmentPlansPageLoader.tsx:48">
P2: When the `asOf` search parameter changes while the initial query is still loading, this effect does not rerun and the request keeps the previous `asOf`; the child then skips its first refetch and can display the wrong snapshot. Reload or otherwise invalidate the preloaded query when the relevant search parameters change.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 4 unresolved issues already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| ARRAY[]::TEXT[] | ||
| ), | ||
| r.category, | ||
| tp.created_at, |
There was a problem hiding this comment.
P1: When an existing plan or measure was edited before this migration, its seed event is backdated to the entity’s creation time while containing current values. Use the migration timestamp for the seed event created_at and retain the entity timestamp only in *_created_at, so unrecoverable pre-migration dates do not return fabricated history.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/migrations/20260826T135100Z.sql, line 115:
<comment>When an existing plan or measure was edited before this migration, its seed event is backdated to the entity’s creation time while containing current values. Use the migration timestamp for the seed event `created_at` and retain the entity timestamp only in `*_created_at`, so unrecoverable pre-migration dates do not return fabricated history.</comment>
<file context>
@@ -0,0 +1,141 @@
+ ARRAY[]::TEXT[]
+ ),
+ r.category,
+ tp.created_at,
+ tp.created_at
+FROM treatment_plans tp
</file context>
| WHEN @state::mitigation_state IS NULL THEN | ||
| TRUE | ||
| ELSE | ||
| state = @state::text |
There was a problem hiding this comment.
P1: As-of measure queries fail because the repeated @state parameter is cast to both mitigation_state and text. Keep the parameter type consistent, for example by casting the enum to text after the enum cast.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure_filter.go, line 104:
<comment>As-of measure queries fail because the repeated `@state` parameter is cast to both `mitigation_state` and `text`. Keep the parameter type consistent, for example by casting the enum to text after the enum cast.</comment>
<file context>
@@ -81,3 +81,37 @@ AND
+ WHEN @state::mitigation_state IS NULL THEN
+ TRUE
+ ELSE
+ state = @state::text
+ END
+)
</file context>
| state = @state::text | |
| state = @state::mitigation_state::text |
| return fmt.Errorf("cannot insert treatment plan: %w", err) | ||
| } | ||
|
|
||
| if err := insertTreatmentPlanEvent( |
There was a problem hiding this comment.
P2: When the mapping is already absent, this added event is still recorded because DeleteMeasureMapping does not verify that mapping.Delete removed a row. As-of reads then report a later update for an unlink that never changed the plan; only insert the unlink event when a mapping was actually deleted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/riskmanagement/treatment_plan.go, line 293:
<comment>When the mapping is already absent, this added event is still recorded because `DeleteMeasureMapping` does not verify that `mapping.Delete` removed a row. As-of reads then report a later update for an unlink that never changed the plan; only insert the unlink event when a mapping was actually deleted.</comment>
<file context>
@@ -288,6 +290,18 @@ func (s *Service) CreateTreatmentPlan(
return fmt.Errorf("cannot insert treatment plan: %w", err)
}
+ if err := insertTreatmentPlanEvent(
+ ctx,
+ tx,
</file context>
| ResidualRiskScore: e.ResidualLikelihood * e.ResidualImpact, | ||
| Category: e.Category, | ||
| CreatedAt: e.TreatmentPlanCreatedAt, | ||
| UpdatedAt: e.CreatedAt, |
There was a problem hiding this comment.
P2: When the latest event only links or unlinks a measure, this reconstruction reports the event time as the plan’s UpdatedAt, although the plan row was not updated. Preserve the plan’s actual updated timestamp in events or update it consistently for measure changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan_event.go, line 127:
<comment>When the latest event only links or unlinks a measure, this reconstruction reports the event time as the plan’s `UpdatedAt`, although the plan row was not updated. Preserve the plan’s actual updated timestamp in events or update it consistently for measure changes.</comment>
<file context>
@@ -0,0 +1,370 @@
+ ResidualRiskScore: e.ResidualLikelihood * e.ResidualImpact,
+ Category: e.Category,
+ CreatedAt: e.TreatmentPlanCreatedAt,
+ UpdatedAt: e.CreatedAt,
+ }
+}
</file context>
| } | ||
| startTransition(() => { | ||
| refetchUnplanned({ orderBy }); | ||
| refetchPlans({ filter, orderBy, asOf }); |
There was a problem hiding this comment.
P2: When the as-of date or cell filter changes, this effect fetches treatment plans again after the parent fragment has already fetched them with the same filter and asOf. Coordinate the parent and pagination refetches so date/filter changes issue one plans request, while sort changes still refetch the pagination fragment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx, line 304:
<comment>When the as-of date or cell filter changes, this effect fetches treatment plans again after the parent fragment has already fetched them with the same `filter` and `asOf`. Coordinate the parent and pagination refetches so date/filter changes issue one plans request, while sort changes still refetch the pagination fragment.</comment>
<file context>
@@ -210,27 +294,16 @@ function RiskAnalysisPlansTable({
- }
- startTransition(() => {
- refetchUnplanned({ orderBy });
+ refetchPlans({ filter, orderBy, asOf });
});
- }, [orderBy, refetchUnplanned, startTransition]);
</file context>
| type treatmentPlan struct { | ||
| ID string `json:"id"` | ||
| Treatment string `json:"treatment"` | ||
| Category string `json:"category"` |
There was a problem hiding this comment.
P2: Existing --output json consumers lose the risk.category field on every live treatment-plan listing, even when --as-of is not used. Preserve the established JSON shape while adding a separate snapshot-aware field or serialization path for the table/as-of output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/treatment-plan/list/list.go, line 117:
<comment>Existing `--output json` consumers lose the `risk.category` field on every live treatment-plan listing, even when `--as-of` is not used. Preserve the established JSON shape while adding a separate snapshot-aware field or serialization path for the table/as-of output.</comment>
<file context>
@@ -119,12 +114,10 @@ query($id: ID!, $first: Int, $after: CursorKey, $orderBy: TreatmentPlanOrder, $f
type treatmentPlan struct {
ID string `json:"id"`
Treatment string `json:"treatment"`
+ Category string `json:"category"`
InherentRiskScore int `json:"inherentRiskScore"`
ResidualRiskScore int `json:"residualRiskScore"`
</file context>
|
|
||
| useEffect(() => { | ||
| if (riskAnalysisId) { | ||
| const asOf = matrixAsOf(parseAsOfDate(paramsRef.current)); |
There was a problem hiding this comment.
P2: When the asOf search parameter changes while the initial query is still loading, this effect does not rerun and the request keeps the previous asOf; the child then skips its first refetch and can display the wrong snapshot. Reload or otherwise invalidate the preloaded query when the relevant search parameters change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/RiskAnalysisTreatmentPlansPageLoader.tsx, line 48:
<comment>When the `asOf` search parameter changes while the initial query is still loading, this effect does not rerun and the request keeps the previous `asOf`; the child then skips its first refetch and can display the wrong snapshot. Reload or otherwise invalidate the preloaded query when the relevant search parameters change.</comment>
<file context>
@@ -44,9 +45,12 @@ export default function RiskAnalysisTreatmentPlansPageLoader() {
useEffect(() => {
if (riskAnalysisId) {
+ const asOf = matrixAsOf(parseAsOfDate(paramsRef.current));
loadQuery({
riskAnalysisId,
</file context>
| r.category | ||
| FROM | ||
| locked | ||
| INNER JOIN |
There was a problem hiding this comment.
P2: Keep LoadByIDForUpdate limited to the treatment-plan table, then load the risk separately to obtain category; coredata entity methods must not add cross-entity joins.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan.go, line 241:
<comment>Keep `LoadByIDForUpdate` limited to the treatment-plan table, then load the risk separately to obtain `category`; coredata entity methods must not add cross-entity joins.</comment>
<file context>
@@ -188,6 +188,84 @@ LIMIT 1;
+ r.category
+FROM
+ locked
+INNER JOIN
+ risks r ON r.id = locked.risk_id
+`
</file context>
| SELECT 1 | ||
| FROM unnest(measure_ids) AS mid | ||
| LEFT JOIN LATERAL ( | ||
| SELECT me.state |
There was a problem hiding this comment.
P2: When a plan snapshot still contains a measure ID after that measure is deleted, these net-score filters use the deleted event’s stored state. Exclude deleted measure events when selecting latest_state and pass the deleted event type as a query argument.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan.go, line 695:
<comment>When a plan snapshot still contains a measure ID after that measure is deleted, these net-score filters use the deleted event’s stored state. Exclude deleted measure events when selecting `latest_state` and pass the deleted event type as a query argument.</comment>
<file context>
@@ -517,6 +595,309 @@ WHERE %s
+ SELECT 1
+ FROM unnest(measure_ids) AS mid
+ LEFT JOIN LATERAL (
+ SELECT me.state
+ FROM measure_events me
+ WHERE me.measure_id = mid
</file context>
Pick a past date and see the heatmap, table, and measures as they were. Each event stores the full plan so as-of reads the latest row. Owners and risks stay required; deleting either while an event still references them is restricted. Today stays on live data. Signed-off-by: Sacha Al Himdani <sacha@probo.com>
84b170e to
9fbf4d4
Compare
Pick a past date and see the heatmap, table,
and measures as they were. Each event stores
the full plan so as-of reads the latest row.
Owners and risks stay required; deleting
either while an event still references them
is restricted. Today stays on live data.
Summary by cubic
Risk analyses can now reconstruct their heatmap, treatment plans, and linked measure states at a chosen instant instead of always reading current tables. Live views remain unchanged; historical results are read-only, and owners and risks cannot be deleted while snapshots still reference them.
Tests
+1162-3Covers matrix reconstruction, plan and measure history, forked plans, and access control for historical requests.
Coredata
+1816-32Adds event snapshots, a migration, and historical filters and loaders for treatment plans and measures.
GraphQL API
+305-13Exposes
asOfon matrix cells, treatment plans, and measures, including historical plan category and progress.MCP
+106-2Adds
as_ofto risk-analysis and treatment-plan tools, returns historical categories and linked measures, and rejects invalid scopes.prb (CLI)
+60-19Adds
--as-ofto risk-analysis and treatment-plan commands and includes plan category in output.Service
+910-4Records plan and measure changes, reconstructs historical matrix data, and preserves plan categories and snapshots when forking.
App: console
+671-236Adds localized As of controls, historical progress and measure views, and hides editing actions for read-only plans.
Package:
n8n-node+50-5Adds As Of inputs for risk-analysis and scoped treatment-plan retrieval and returns plan category.
Written for commit 9fbf4d4. Summary will update on new commits.