Skip to content

Commit b9f400c

Browse files
Marfuenclaude
andcommitted
refactor(timelines): make findAllForOrganization pure-read
Previously GET /timelines mutated state (completed phases, reverted regressed phases, updated timeline status). That violates HTTP semantics and races under concurrent reads. - extract the sync loop into TimelinesService.reconcileAutoPhasesForOrganization (handles both advancement and regression, replaces the inline logic) - findAllForOrganization is now a pure read: ensureTimelinesExist (idempotent backfill) + fetch + in-memory enrichment with live completion percentages - call reconcileAutoPhasesForOrganization from checkAutoCompletePhases so every existing event hook (task/policy/people/findings mutations) drives both advancement and regression — no stale state left for reads to repair - recreateAllForOrganization explicitly reconciles before returning so freshly-recreated timelines are synced against current metrics Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d8753ca commit b9f400c

File tree

2 files changed

+136
-104
lines changed

2 files changed

+136
-104
lines changed

apps/api/src/frameworks/frameworks-timeline.helper.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,4 +330,17 @@ export async function checkAutoCompletePhases({
330330
);
331331
}
332332
}
333+
334+
// After per-phase completion attempts, run the full org-level
335+
// reconciliation so metric drops (regressions) also get picked up.
336+
// Without this, only event-driven advances would apply — regressions
337+
// previously happened in the GET /timelines read path, which is now pure.
338+
await timelinesService
339+
.reconcileAutoPhasesForOrganization(organizationId)
340+
.catch((err) =>
341+
logger.warn(
342+
'reconcileAutoPhasesForOrganization failed',
343+
err instanceof Error ? err.message : err,
344+
),
345+
);
333346
}

apps/api/src/timelines/timelines.service.ts

Lines changed: 123 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -34,107 +34,69 @@ export class TimelinesService {
3434
// Customer-facing queries
3535
// ---------------------------------------------------------------------------
3636

37-
async findAllForOrganization(
38-
organizationId: string,
39-
options: TimelinesQueryOptions = {},
40-
) {
37+
/**
38+
* Customer-facing read of timelines.
39+
*
40+
* Pure read (except for the one-time ensureTimelinesExist backfill). The
41+
* AUTO_* phase advancement + regression sync lives in
42+
* reconcileAutoPhasesForOrganization and fires from mutation event hooks
43+
* (task/policy/people/findings updates) so GET /timelines is idempotent.
44+
*/
45+
async findAllForOrganization(organizationId: string) {
4146
await this.ensureTimelinesExist(organizationId);
4247

43-
const fetchTimelines = async () =>
44-
db.timelineInstance.findMany({
45-
where: { organizationId },
46-
include: {
47-
phases: { orderBy: { orderIndex: 'asc' } },
48-
frameworkInstance: { include: { framework: true } },
49-
template: true,
50-
},
51-
});
52-
53-
const enrichWithCompletionPercent = (
54-
timelines: Awaited<ReturnType<typeof fetchTimelines>>,
55-
pctMap: Record<string, number>,
56-
) => {
57-
for (const timeline of timelines) {
58-
if (timeline.status !== 'ACTIVE') continue;
59-
for (const phase of timeline.phases) {
60-
const livePct = pctMap[phase.completionType];
61-
if (livePct !== undefined) {
62-
(phase as any).completionPercent = livePct;
63-
}
64-
}
65-
}
66-
};
67-
68-
let timelines = await fetchTimelines();
48+
const timelines = await db.timelineInstance.findMany({
49+
where: { organizationId },
50+
include: {
51+
phases: { orderBy: { orderIndex: 'asc' } },
52+
frameworkInstance: { include: { framework: true } },
53+
template: true,
54+
},
55+
});
6956

70-
// Compute live completion percentages for AUTO_* phases
57+
// Enrich AUTO_* phases with live completion percentages in-memory.
7158
const scores = await getOverviewScores(organizationId).catch(() => null);
7259
if (!scores) return timelines;
7360

74-
const policyPct = scores.policies.total > 0
75-
? Math.round((scores.policies.published / scores.policies.total) * 100)
76-
: 0;
77-
const taskPct = scores.tasks.total > 0
78-
? Math.round((scores.tasks.done / scores.tasks.total) * 100)
79-
: 0;
80-
const peoplePct = scores.people.total > 0
81-
? Math.round((scores.people.completed / scores.people.total) * 100)
82-
: 0;
83-
84-
const pctMap: Record<string, number> = {
85-
AUTO_POLICIES: policyPct,
86-
AUTO_TASKS: taskPct,
87-
AUTO_PEOPLE: peoplePct,
88-
};
61+
const pctMap = this.buildAutoPctMap(scores);
8962

90-
const isTimelineLocked = (timeline: { status: string; lockedAt: Date | null }) =>
91-
timeline.status === 'COMPLETED' || !!timeline.lockedAt;
63+
for (const timeline of timelines) {
64+
if (timeline.status !== 'ACTIVE') continue;
65+
for (const phase of timeline.phases) {
66+
const livePct = pctMap[phase.completionType];
67+
if (livePct !== undefined) {
68+
(phase as any).completionPercent = livePct;
69+
}
70+
}
71+
}
9272

93-
const reopenFromRegressedPhase = async (
94-
timelineId: string,
95-
regressedOrderIndex: number,
96-
) => {
97-
await db.$transaction(async (tx) => {
98-
const freshPhases = await tx.timelinePhase.findMany({
99-
where: { instanceId: timelineId },
100-
orderBy: { orderIndex: 'asc' },
101-
});
73+
return timelines;
74+
}
10275

103-
const affected = freshPhases.filter(
104-
(p) => p.orderIndex >= regressedOrderIndex,
105-
);
106-
for (let i = 0; i < affected.length; i++) {
107-
const phase = affected[i];
108-
await tx.timelinePhase.update({
109-
where: { id: phase.id },
110-
data: {
111-
status:
112-
i === 0
113-
? TimelinePhaseStatus.IN_PROGRESS
114-
: TimelinePhaseStatus.PENDING,
115-
completedAt: null,
116-
completedById: null,
117-
readyForReview: false,
118-
readyForReviewAt: null,
119-
regressedAt: null,
120-
},
121-
});
122-
}
76+
/**
77+
* Reconcile AUTO_* phase status with live metrics. Handles both directions:
78+
* advances phases when metrics hit 100% and reverts completed phases whose
79+
* metric dropped below 100% (respecting the regression grace period).
80+
*
81+
* Called from mutation hooks in tasks/policies/people/findings services
82+
* after events that could shift the underlying scores.
83+
*/
84+
async reconcileAutoPhasesForOrganization(
85+
organizationId: string,
86+
options: TimelinesQueryOptions = {},
87+
): Promise<void> {
88+
const scores = await getOverviewScores(organizationId).catch(() => null);
89+
if (!scores) return;
12390

124-
// If the timeline was COMPLETED, regression means it's no longer
125-
// complete — flip it back to ACTIVE and clear completion metadata
126-
// so status stays consistent with phase state.
127-
await tx.timelineInstance.updateMany({
128-
where: { id: timelineId, status: 'COMPLETED' },
129-
data: { status: 'ACTIVE', completedAt: null },
130-
});
91+
const pctMap = this.buildAutoPctMap(scores);
92+
93+
const fetchTimelines = () =>
94+
db.timelineInstance.findMany({
95+
where: { organizationId },
96+
include: { phases: { orderBy: { orderIndex: 'asc' } } },
13197
});
132-
};
13398

134-
// Sync AUTO_* phase statuses with live metrics until no further transition occurs.
135-
// This ensures consecutive AUTO phases can complete in one request when metrics
136-
// are already at 100% (common when starting a renewal cycle).
137-
enrichWithCompletionPercent(timelines, pctMap);
99+
let timelines = await fetchTimelines();
138100

139101
const maxSyncPasses = 20;
140102
for (let pass = 0; pass < maxSyncPasses; pass++) {
@@ -145,17 +107,17 @@ export class TimelinesService {
145107
continue;
146108
}
147109

148-
const locked = isTimelineLocked(timeline);
110+
const locked =
111+
timeline.status === 'COMPLETED' || !!timeline.lockedAt;
149112
const canAutoTransition = timeline.status === 'ACTIVE' && !locked;
150113

151114
for (const phase of timeline.phases) {
152-
if (!AUTO_PHASE_TYPES.has(phase.completionType)) {
153-
continue;
154-
}
115+
if (!AUTO_PHASE_TYPES.has(phase.completionType)) continue;
155116

156117
const livePct = pctMap[phase.completionType];
157118
if (livePct === undefined) continue;
158119

120+
// Non-completed phase: clear regression flag if it somehow lingers.
159121
if (
160122
phase.status !== TimelinePhaseStatus.COMPLETED &&
161123
phase.regressedAt
@@ -175,7 +137,10 @@ export class TimelinesService {
175137
(AUTO_PHASE_TYPES_NO_GRACE.has(phase.completionType) ||
176138
options.bypassRegressionGrace === true)
177139
) {
178-
await reopenFromRegressedPhase(timeline.id, phase.orderIndex);
140+
await this.reopenFromRegressedPhase(
141+
timeline.id,
142+
phase.orderIndex,
143+
);
179144
changed = true;
180145
break;
181146
}
@@ -193,7 +158,10 @@ export class TimelinesService {
193158
const elapsedMs =
194159
Date.now() - new Date(phase.regressedAt).getTime();
195160
if (elapsedMs >= REGRESSION_GRACE_MS) {
196-
await reopenFromRegressedPhase(timeline.id, phase.orderIndex);
161+
await this.reopenFromRegressedPhase(
162+
timeline.id,
163+
phase.orderIndex,
164+
);
197165
changed = true;
198166
break;
199167
}
@@ -208,7 +176,6 @@ export class TimelinesService {
208176
}
209177
}
210178

211-
// Complete if in-progress and metric hit 100%
212179
if (
213180
canAutoTransition &&
214181
phase.status === TimelinePhaseStatus.IN_PROGRESS &&
@@ -223,23 +190,71 @@ export class TimelinesService {
223190
changed = true;
224191
break;
225192
} catch {
226-
// Phase might not be completable (e.g., prior phases not done)
193+
// Phase may not be completable (e.g., prior phases not done).
227194
}
228195
}
229196
}
230197

231198
if (changed) break;
232199
}
233200

234-
if (!changed) {
235-
break;
236-
}
237-
201+
if (!changed) break;
238202
timelines = await fetchTimelines();
239-
enrichWithCompletionPercent(timelines, pctMap);
240203
}
204+
}
241205

242-
return timelines;
206+
private buildAutoPctMap(scores: {
207+
policies: { total: number; published: number };
208+
tasks: { total: number; done: number };
209+
people: { total: number; completed: number };
210+
}): Record<string, number> {
211+
const pct = (num: number, den: number) =>
212+
den > 0 ? Math.round((num / den) * 100) : 0;
213+
return {
214+
AUTO_POLICIES: pct(scores.policies.published, scores.policies.total),
215+
AUTO_TASKS: pct(scores.tasks.done, scores.tasks.total),
216+
AUTO_PEOPLE: pct(scores.people.completed, scores.people.total),
217+
};
218+
}
219+
220+
private async reopenFromRegressedPhase(
221+
timelineId: string,
222+
regressedOrderIndex: number,
223+
): Promise<void> {
224+
await db.$transaction(async (tx) => {
225+
const freshPhases = await tx.timelinePhase.findMany({
226+
where: { instanceId: timelineId },
227+
orderBy: { orderIndex: 'asc' },
228+
});
229+
230+
const affected = freshPhases.filter(
231+
(p) => p.orderIndex >= regressedOrderIndex,
232+
);
233+
for (let i = 0; i < affected.length; i++) {
234+
const phase = affected[i];
235+
await tx.timelinePhase.update({
236+
where: { id: phase.id },
237+
data: {
238+
status:
239+
i === 0
240+
? TimelinePhaseStatus.IN_PROGRESS
241+
: TimelinePhaseStatus.PENDING,
242+
completedAt: null,
243+
completedById: null,
244+
readyForReview: false,
245+
readyForReviewAt: null,
246+
regressedAt: null,
247+
},
248+
});
249+
}
250+
251+
// If the timeline was COMPLETED, regression means it's no longer
252+
// complete — flip it back to ACTIVE to stay consistent with phases.
253+
await tx.timelineInstance.updateMany({
254+
where: { id: timelineId, status: 'COMPLETED' },
255+
data: { status: 'ACTIVE', completedAt: null },
256+
});
257+
});
243258
}
244259

245260
/**
@@ -550,8 +565,12 @@ export class TimelinesService {
550565
}
551566
}
552567

553-
return this.findAllForOrganization(organizationId, {
568+
// After a full recreate, immediately reconcile AUTO phases against live
569+
// metrics (bypassing regression grace) so the returned state is fresh.
570+
await this.reconcileAutoPhasesForOrganization(organizationId, {
554571
bypassRegressionGrace: true,
555572
});
573+
574+
return this.findAllForOrganization(organizationId);
556575
}
557576
}

0 commit comments

Comments
 (0)