Skip to content

Commit c99df73

Browse files
spotta85spotta85
authored
Provider: single-writer data flow, drop composite hot path, expose remove (#185)
Rework the audit provider's data loading (cleanup plan Phase 6): - Drop the compositeAuditData memo. The single-audit hot path reads auditData.requirements directly instead of round-tripping through getCompositeAuditRequirements, which recomputed per-audit names and duplicate-course flags no single-audit UI reads. Those calculations stay in audit-calculations.ts for the future multi-audit feature. - Split the two racing effects that both wrote auditData (and flashed null on every switch) into single-responsibility effects: - Effect A (ids only): keep currentAuditId valid against history, falling back to the first valid audit; never touches auditData. - Effect B (sole auditData writer): new observeAuditData() in audit-storage.ts mirrors observeAuditHistory (initial read + watch, watch wins over a late initial read). setLoaded(false) on switch so the loaded gate shows LoadingPage instead of a blank flash. - Expose removePlannedCourse(courseId) on the context, implemented like moveCourseToNewSemester over the existing audit-mutations helper (completed courses are not removable). wipePlannedCourses stays unexposed. - Document observeAuditData and the single-writer contract in docs/architecture.md.
1 parent ed2999c commit c99df73

3 files changed

Lines changed: 87 additions & 65 deletions

File tree

‎docs/architecture.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,18 @@ the existing keys and stored shapes for:
106106

107107
- audit history (`getAuditHistory`, `observeAuditHistory`, `saveAuditHistory`,
108108
`renameAudit`);
109-
- individual audit data (`getAuditData`, `watchAuditData`, `saveAuditData`,
110-
`getUncachedAuditIds`); and
109+
- individual audit data (`getAuditData`, `watchAuditData`, `observeAuditData`,
110+
`saveAuditData`, `getUncachedAuditIds`); and
111111
- saved audit combinations (`getCachedComposites`, `createComposite`,
112112
`updateCachedComposite`, `deleteCachedComposite`, `loadCompositeAudit`).
113113

114-
`observeAuditHistory` hides the initial read plus subsequent storage watch behind
115-
one cleanup function. It prevents a delayed initial read from overwriting a newer
116-
watched update.
114+
`observeAuditHistory` and `observeAuditData` each hide the initial read plus
115+
subsequent storage watch behind one cleanup function. Both prevent a delayed
116+
initial read from overwriting a newer watched update. `observeAuditData` is the
117+
provider's single writer of the selected audit: one effect selects the id, a
118+
second observes its data, and no code path reads audit data alongside the
119+
observer. This keeps a single source of truth and avoids the blank-flash that two
120+
competing writers caused.
117121

118122
`audit-mutations.ts` contains immutable, browser-free changes to cached audit
119123
data: add, remove, wipe, and move planned courses. `audit-calculations.ts`

‎features/audit/audit-provider.tsx‎

Lines changed: 46 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
type AuditHistoryEntry,
44
type AuditRequirement,
55
type CachedAuditData,
6-
type CompositeAuditData,
76
getAuditDisplayName,
87
} from "@/domain/audit";
98
import type {
@@ -14,22 +13,19 @@ import type {
1413
} from "@/domain/course";
1514
import type { CurrentAuditProgress } from "@/domain/progress";
1615
import LoadingPage from "@/components/loading-page";
16+
import { calculateWeightedDegreeCompletion } from "./audit-calculations";
1717
import {
18-
calculateWeightedDegreeCompletion,
19-
getCompositeAuditRequirements,
20-
} from "./audit-calculations";
21-
import {
22-
getAuditData,
18+
observeAuditData,
2319
observeAuditHistory,
2420
renameAudit,
2521
saveAuditData,
26-
watchAuditData,
2722
} from "./audit-storage";
2823
import { createContext, useContext, useEffect, useMemo, useState } from "react";
2924
import { usePreferences } from "@/features/preferences/preferences-provider";
3025
import {
3126
addPlannedCourse as addCourse,
3227
moveCourseToSemester,
28+
removePlannedCourse as removeCourse,
3329
} from "./audit-mutations";
3430

3531
type SemesterInfo = Record<StringSemester, Course[]>;
@@ -55,6 +51,7 @@ interface AuditContextValue {
5551
requirementTitle: string,
5652
ruleTitle: string,
5753
) => Promise<CourseId | null>;
54+
removePlannedCourse: (courseId: CourseId) => Promise<boolean>;
5855
}
5956

6057
const AuditContext = createContext<AuditContextValue | null>(null);
@@ -79,24 +76,11 @@ export function AuditContextProvider({
7976
);
8077
const currentAuditName =
8178
getAuditDisplayName(currentAudit) ?? "Degree Requirements";
82-
const compositeAuditData = useMemo<CompositeAuditData>(
83-
() =>
84-
auditData && currentAuditId
85-
? {
86-
audits: [
87-
{
88-
...auditData,
89-
name: getAuditDisplayName(currentAudit) ?? currentAuditId,
90-
},
91-
],
92-
}
93-
: { audits: [] },
94-
[auditData, currentAudit, currentAuditId],
95-
);
96-
const sections = useMemo(
97-
() => getCompositeAuditRequirements(compositeAuditData),
98-
[compositeAuditData],
99-
);
79+
// Single-audit hot path reads requirements directly. The composite decorations
80+
// (per-audit names, duplicate-course flags) live in audit-calculations.ts
81+
// (getCompositeAuditRequirements) for the future multi-audit feature; no
82+
// single-audit UI reads them, so we don't compute them here.
83+
const sections = auditData?.requirements ?? [];
10084
const courseMap = useMemo(() => auditData?.courses ?? {}, [auditData]);
10185
const progresses = useMemo(
10286
() => calculateWeightedDegreeCompletion(sections, courseMap),
@@ -123,48 +107,43 @@ export function AuditContextProvider({
123107
);
124108
}, []);
125109

110+
// Effect A (ids only): keep currentAuditId valid against history. If the
111+
// current id isn't a known audit, fall back to the first valid one. Never
112+
// touches auditData.
126113
useEffect(() => {
127-
if (!currentAuditId) return;
114+
if (!history) return;
128115

129-
setAuditData(null);
130-
return watchAuditData(currentAuditId, setAuditData);
131-
}, [currentAuditId]);
116+
const isKnown = history.audits.some(
117+
({ auditId }) => auditId === currentAuditId,
118+
);
119+
if (isKnown) return;
120+
121+
const fallbackId = history.audits.find(({ auditId }) => auditId)?.auditId;
122+
if (fallbackId && fallbackId !== currentAuditId) {
123+
setCurrentAuditIdState(fallbackId);
124+
updateLastAuditId(fallbackId);
125+
}
126+
}, [currentAuditId, history, updateLastAuditId]);
132127

128+
// Effect B (sole auditData writer): observe the selected audit. The watch wins
129+
// over a late initial read, so switching audits never flashes null — the
130+
// `loaded` gate shows LoadingPage until the observed data arrives.
133131
useEffect(() => {
134-
if (!history) return;
132+
if (!currentAuditId) return;
135133

136-
const storedHistory = history;
137-
let cancelled = false;
138134
setLoaded(false);
139-
140-
async function loadAudit() {
141-
try {
142-
const selectedId = storedHistory.audits.some(
143-
({ auditId }) => auditId === currentAuditId,
144-
)
145-
? currentAuditId
146-
: storedHistory.audits.find(({ auditId }) => auditId)?.auditId;
147-
if (!selectedId) return;
148-
149-
if (selectedId !== currentAuditId) {
150-
setCurrentAuditIdState(selectedId);
151-
updateLastAuditId(selectedId);
152-
return;
153-
}
154-
const storedAudit = await getAuditData(selectedId);
155-
if (!cancelled) setAuditData(storedAudit);
156-
} catch (error) {
135+
return observeAuditData(
136+
currentAuditId,
137+
(audit) => {
138+
setAuditData(audit);
139+
setLoaded(true);
140+
},
141+
(error) => {
157142
console.error("Failed to load audit:", error);
158-
} finally {
159-
if (!cancelled) setLoaded(true);
160-
}
161-
}
162-
163-
void loadAudit();
164-
return () => {
165-
cancelled = true;
166-
};
167-
}, [currentAuditId, history, updateLastAuditId]);
143+
setLoaded(true);
144+
},
145+
);
146+
}, [currentAuditId]);
168147

169148
const value = useMemo<AuditContextValue>(() => {
170149
// currentAuditId and history are guaranteed non-null past the loading
@@ -215,6 +194,13 @@ export function AuditContextProvider({
215194
await saveAuditData(currentAuditId, result.audit);
216195
return result.courseId;
217196
},
197+
removePlannedCourse: async (courseId) => {
198+
if (!auditData || !currentAuditId) return false;
199+
const updated = removeCourse(auditData, courseId);
200+
if (!updated) return false;
201+
await saveAuditData(currentAuditId, updated);
202+
return true;
203+
},
218204
};
219205
}, [
220206
sections,

‎features/audit/audit-storage.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,38 @@ export function watchAuditData(
109109
);
110110
}
111111

112+
/**
113+
* Observe a single audit's data: an initial read plus a subsequent storage
114+
* watch behind one cleanup function. A delayed initial read never overwrites a
115+
* newer watched update (mirrors {@link observeAuditHistory}). This is the single
116+
* writer of audit data in the provider — callers should not also read directly.
117+
*/
118+
export function observeAuditData(
119+
auditId: string,
120+
listener: (audit: CachedAuditData | null) => void,
121+
onError?: (error: unknown) => void,
122+
): () => void {
123+
let active = true;
124+
let receivedUpdate = false;
125+
const unwatch = watchAuditData(auditId, (audit) => {
126+
receivedUpdate = true;
127+
if (active) listener(audit);
128+
});
129+
130+
void getAuditData(auditId)
131+
.then((audit) => {
132+
if (active && !receivedUpdate) listener(audit);
133+
})
134+
.catch((error: unknown) => {
135+
if (active && !receivedUpdate) onError?.(error);
136+
});
137+
138+
return () => {
139+
active = false;
140+
unwatch();
141+
};
142+
}
143+
112144
export async function getUncachedAuditIds(
113145
auditIds: string[],
114146
): Promise<string[]> {

0 commit comments

Comments
 (0)