Skip to content

Commit fe6100a

Browse files
authored
Merge pull request #77 from Programcode/fix/debate-pin-state
fix(debate): keep saved pins through early changes and free deleted pins
2 parents 778a882 + bf24d44 commit fe6100a

6 files changed

Lines changed: 316 additions & 28 deletions

File tree

‎src/components/features/debate/DebateContextControl.tsx‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { useDebateContextPlan } from '@/lib/debate/context-state';
99
import type { ContextOutputItem, ContextSourceItem, DebateContextPlan } from '@/lib/debate/context';
1010
import { useCurrentDebate, useDebateStore } from '@/lib/stores/debate-store';
1111
import { outputKey, useDebateContextStore, type DebateContextUsage } from '@/lib/stores/debate-context-store';
12+
import { useSessionStore } from '@/lib/stores/session-store';
13+
import { useSparringStore } from '@/lib/stores/sparring-store';
1214
import type { DebateContextFeature, DebateContextSide } from '../../../../supabase/functions/_shared/debate-context';
1315

1416
interface DebateContextControlProps {
@@ -42,6 +44,12 @@ export function DebateContextControl({ feature, side, includeSources = true, deb
4244
const debateId = debateIdOverride !== undefined ? debateIdOverride : (oneOff ? null : debate?.id ?? null);
4345
const plan = useDebateContextPlan(feature, side, debateId, { includeSources });
4446
const pinsLoaded = useDebateContextStore((state) => (debateId ? state.pinsLoaded[debateId] : true));
47+
// The pin limit discounts pins on results that are not listed. Until saved
48+
// results have loaded, a missing result may just not be here yet; if a list
49+
// failed to load, every pin counts.
50+
const serverHydration = useSessionStore((state) => state.serverHydration);
51+
const sparringHydrated = useSparringStore((state) => state.hydrated);
52+
const sparringComplete = useSparringStore((state) => state.historyComplete);
4553
const loadPins = useDebateContextStore((state) => state.loadPins);
4654
const usage = useDebateContextStore((state) => state.lastUsage[feature]);
4755
const [open, setOpen] = useState(false);
@@ -99,14 +107,26 @@ export function DebateContextControl({ feature, side, includeSources = true, deb
99107
</Button>
100108
<Sheet open={open} onOpenChange={setOpen}>
101109
<SheetContent className="flex w-full flex-col gap-0 overflow-y-auto p-0 sm:max-w-md">
102-
<DebateContextReview plan={plan} motion={debate?.id === debateId ? debate.motion : undefined} usage={usage} />
110+
<DebateContextReview
111+
plan={plan}
112+
motion={debate?.id === debateId ? debate.motion : undefined}
113+
usage={usage}
114+
pinsReady={Boolean(pinsLoaded) && serverHydration !== 'pending' && sparringHydrated}
115+
resultsComplete={serverHydration === 'complete' && sparringComplete}
116+
/>
103117
</SheetContent>
104118
</Sheet>
105119
</div>
106120
);
107121
}
108122

109-
function DebateContextReview({ plan, motion, usage }: { plan: DebateContextPlan; motion?: string; usage?: DebateContextUsage }) {
123+
function DebateContextReview({ plan, motion, usage, pinsReady, resultsComplete }: {
124+
plan: DebateContextPlan;
125+
motion?: string;
126+
usage?: DebateContextUsage;
127+
pinsReady: boolean;
128+
resultsComplete: boolean;
129+
}) {
110130
const { t } = useTranslation();
111131
const setSourceChoice = useDebateContextStore((state) => state.setSourceChoice);
112132
const setOutputChoice = useDebateContextStore((state) => state.setOutputChoice);
@@ -116,7 +136,7 @@ function DebateContextReview({ plan, motion, usage }: { plan: DebateContextPlan;
116136

117137
const onPin = async (output: ContextOutputItem) => {
118138
try {
119-
await togglePin(plan.debateId, output.ref);
139+
await togglePin(plan.debateId, output.ref, resultsComplete ? plan.resultKeys : undefined);
120140
} catch (error) {
121141
toast({
122142
title: error instanceof Error && error.message === 'pin_limit'
@@ -180,6 +200,7 @@ function DebateContextReview({ plan, motion, usage }: { plan: DebateContextPlan;
180200
output={output}
181201
onChange={(include) => setOutputChoice(plan.feature, plan.debateId, output.ref, include)}
182202
onPin={() => void onPin(output)}
203+
pinDisabled={!pinsReady}
183204
/>
184205
))}
185206
</ul>
@@ -221,7 +242,7 @@ function SourceRow({ source, onChange }: { source: ContextSourceItem; onChange:
221242
);
222243
}
223244

224-
function OutputRow({ output, onChange, onPin }: { output: ContextOutputItem; onChange: (include: boolean) => void; onPin: () => void }) {
245+
function OutputRow({ output, onChange, onPin, pinDisabled }: { output: ContextOutputItem; onChange: (include: boolean) => void; onPin: () => void; pinDisabled: boolean }) {
225246
const { t } = useTranslation();
226247
const id = `debate-context-output-${output.ref.kind}-${output.ref.id}`;
227248
const note = output.included ? statusNote(output, t) : null;
@@ -247,6 +268,7 @@ function OutputRow({ output, onChange, onPin }: { output: ContextOutputItem; onC
247268
? t('debateContext.unpinAria', { title: output.title || kind })
248269
: t('debateContext.pinAria', { title: output.title || kind })}
249270
onClick={onPin}
271+
disabled={pinDisabled}
250272
>
251273
{output.pinned ? <PinOff className="h-4 w-4" /> : <Pin className="h-4 w-4" />}
252274
</Button>

‎src/lib/debate/context.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ export interface DebateContextPlan {
6464
policy: DebateContextPolicy;
6565
sources: ContextSourceItem[];
6666
outputs: ContextOutputItem[];
67+
/** Every result in this debate, the open Ask chat included; pins on anything else no longer count. */
68+
resultKeys: ReadonlySet<string>;
6769
request: DebateContextRequest;
6870
}
6971

@@ -153,6 +155,10 @@ export function planDebateContext(
153155

154156
const pinnedKeys = new Set(inputs.pins.map(outputKey));
155157
const candidates = policy.outputs ? outputCandidates(debateId, inputs) : [];
158+
// A pin on a result that isn't loaded here, because it was deleted or is
159+
// older than local history keeps, is never shown or sent. It must not use up
160+
// the limit, or the student could never free that slot.
161+
const resultKeys = new Set(outputCandidates(debateId, { ...inputs, excludeChatId: null }).map((item) => outputKey(item.ref)));
156162
const defaults = selectDefaultOutputs(
157163
candidates.map((item) => ({ ...item.ref, side: item.side, createdAt: item.createdAt, pinned: pinnedKeys.has(outputKey(item.ref)) })),
158164
feature,
@@ -194,6 +200,7 @@ export function planDebateContext(
194200
policy,
195201
sources,
196202
outputs,
203+
resultKeys,
197204
request: {
198205
debateId,
199206
side,

‎src/lib/stores/debate-context-store.ts‎

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ interface DebateContextState {
4040
lastUsage: Partial<Record<DebateContextFeature, DebateContextUsage>>;
4141

4242
loadPins: (debateId: string) => Promise<void>;
43-
togglePin: (debateId: string, ref: DebateOutputRef) => Promise<void>;
43+
/** Only pins on `resultKeys`, the debate's existing results, count toward the limit. */
44+
togglePin: (debateId: string, ref: DebateOutputRef, resultKeys?: ReadonlySet<string>) => Promise<void>;
4445
setSourceChoice: (feature: DebateContextFeature, debateId: string, sourceId: string, include: boolean) => void;
4546
setOutputChoice: (feature: DebateContextFeature, debateId: string, ref: DebateOutputRef, include: boolean) => void;
4647
resetOverride: (feature: DebateContextFeature) => void;
@@ -52,9 +53,20 @@ function ensureOverride(current: DebateContextOverride | undefined, debateId: st
5253
return current && current.debateId === debateId ? current : { debateId, sources: {}, outputs: {} };
5354
}
5455

55-
// Counts pin changes per debate, so a load that started before a change
56-
// cannot bring back what the student just pinned or unpinned.
57-
const pinChanges: Record<string, number> = {};
56+
// Every local pin change, per debate and in order. A load replays the changes
57+
// it may have missed: ones made after it started, and ones that were still
58+
// saving at any point during the load, because its read may predate them.
59+
// `before` is the result's state before this change; if the change fails, the
60+
// result goes back to it.
61+
interface PinChange { seq: number; ref: DebateOutputRef; pinned: boolean; before: boolean; saving: boolean }
62+
const pinChanges: Record<string, PinChange[]> = {};
63+
let pinSeq = 0;
64+
// Each debate's pin writes run one at a time, so the database ends in the
65+
// same state as the screen even after a quick pin and unpin.
66+
const pinWrites: Record<string, Promise<unknown>> = {};
67+
// Bumped when local data is reset on sign-out. Pin work started before that
68+
// belongs to the previous account, so it must not write or change state.
69+
let pinGeneration = 0;
5870

5971
function withPin(pins: DebateOutputRef[], ref: DebateOutputRef, pinned: boolean): DebateOutputRef[] {
6072
const without = pins.filter((item) => outputKey(item) !== outputKey(ref));
@@ -68,44 +80,73 @@ export const useDebateContextStore = create<DebateContextState>()((set, get) =>
6880
lastUsage: {},
6981

7082
loadPins: async (debateId) => {
71-
const changesBefore = pinChanges[debateId] ?? 0;
83+
const startedAt = pinSeq;
84+
const generation = pinGeneration;
85+
const savingAtStart = new Set((pinChanges[debateId] ?? []).filter((change) => change.saving));
7286
const { data, error } = await supabase
7387
.from('debate_context_pins')
7488
.select('item_kind,item_id')
7589
.eq('debate_id', debateId);
90+
if (generation !== pinGeneration) return;
7691
// Pins are an optional preference. Until the table exists everywhere, or
7792
// if it cannot be read, requests fall back to the automatic selection.
78-
if (error || (pinChanges[debateId] ?? 0) !== changesBefore) {
93+
if (error) {
7994
set((state) => ({ pinsLoaded: { ...state.pinsLoaded, [debateId]: true } }));
8095
return;
8196
}
82-
const pins = ((data ?? []) as Array<{ item_kind: unknown; item_id: unknown }>).flatMap((row) =>
97+
let pins = ((data ?? []) as Array<{ item_kind: unknown; item_id: unknown }>).flatMap((row) =>
8398
isDebateContextOutputKind(row.item_kind) && typeof row.item_id === 'string'
8499
? [{ kind: row.item_kind as DebateContextOutputKind, id: row.item_id }]
85100
: []);
101+
for (const change of pinChanges[debateId] ?? []) {
102+
if (change.seq > startedAt || change.saving || savingAtStart.has(change)) pins = withPin(pins, change.ref, change.pinned);
103+
}
86104
set((state) => ({
87105
pins: { ...state.pins, [debateId]: pins },
88106
pinsLoaded: { ...state.pinsLoaded, [debateId]: true },
89107
}));
90108
},
91109

92-
togglePin: async (debateId, ref) => {
110+
togglePin: async (debateId, ref, resultKeys) => {
93111
const current = get().pins[debateId] ?? [];
94112
const pinned = current.some((item) => outputKey(item) === outputKey(ref));
95-
if (!pinned && current.length >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit');
96-
pinChanges[debateId] = (pinChanges[debateId] ?? 0) + 1;
97-
set((state) => ({ pins: { ...state.pins, [debateId]: withPin(current, ref, !pinned) } }));
98-
const { data } = await supabase.auth.getUser();
99-
const userId = data.user?.id;
100-
const result = !userId
101-
? { error: { message: 'Sign in to pin results' } }
102-
: pinned
113+
const counted = resultKeys ? current.filter((item) => resultKeys.has(outputKey(item))) : current;
114+
if (!pinned && counted.length >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit');
115+
const generation = pinGeneration;
116+
const change: PinChange = { seq: ++pinSeq, ref, pinned: !pinned, before: pinned, saving: true };
117+
(pinChanges[debateId] ??= []).push(change);
118+
set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, !pinned) } }));
119+
const save = async (): Promise<{ error: { message?: string; code?: string } | null }> => {
120+
if (generation !== pinGeneration) return { error: null };
121+
const { data } = await supabase.auth.getUser();
122+
const userId = data.user?.id;
123+
if (generation !== pinGeneration) return { error: null };
124+
if (!userId) return { error: { message: 'Sign in to pin results' } };
125+
return pinned
103126
? await supabase.from('debate_context_pins').delete()
104127
.eq('user_id', userId).eq('debate_id', debateId).eq('item_kind', ref.kind).eq('item_id', ref.id)
105128
: await supabase.from('debate_context_pins').insert({ user_id: userId, debate_id: debateId, item_kind: ref.kind, item_id: ref.id });
129+
};
130+
const write = (pinWrites[debateId] ?? Promise.resolve()).then(save);
131+
pinWrites[debateId] = write.catch((): void => undefined);
132+
const result: Awaited<ReturnType<typeof save>> = await write
133+
.catch((error: unknown) => ({ error: { message: error instanceof Error ? error.message : undefined } }));
134+
change.saving = false;
135+
if (generation !== pinGeneration) return;
136+
// The row already exists, so the result is pinned as asked.
137+
if (change.pinned && result.error?.code === '23505') return;
106138
if (result.error) {
107-
// Undo only this change; other pins may have changed in the meantime.
108-
set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, pinned) } }));
139+
// Undo only this change; other pins may have changed in the meantime. A
140+
// later change to the same result is queued behind this one and decides
141+
// its final state. It assumed this change was saved, so if it fails too
142+
// it must go back to the state before this one.
143+
const next = pinChanges[debateId].find((item) => item.seq > change.seq && outputKey(item.ref) === outputKey(ref));
144+
pinChanges[debateId] = pinChanges[debateId].filter((item) => item !== change);
145+
if (next) {
146+
next.before = change.before;
147+
return;
148+
}
149+
set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, change.before) } }));
109150
throw new Error(result.error.message ?? 'Could not save the pin');
110151
}
111152
},
@@ -134,7 +175,12 @@ export const useDebateContextStore = create<DebateContextState>()((set, get) =>
134175
return { lastUsage };
135176
}),
136177

137-
resetLocalData: () => set({ pins: {}, pinsLoaded: {}, overrides: {}, lastUsage: {} }),
178+
resetLocalData: () => {
179+
pinGeneration += 1;
180+
for (const debateId of Object.keys(pinChanges)) delete pinChanges[debateId];
181+
for (const debateId of Object.keys(pinWrites)) delete pinWrites[debateId];
182+
set({ pins: {}, pinsLoaded: {}, overrides: {}, lastUsage: {} });
183+
},
138184
}));
139185

140186
/** Reads the server's `meta.debateContext`, ignoring anything malformed. */

‎src/lib/stores/session-store.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,11 @@ export interface SessionState {
217217
clearLibrary: () => Promise<void>;
218218

219219
hydrateFromServer: () => Promise<void>;
220+
/**
221+
* This page load's server hydration: `partial` when a saved-result list
222+
* failed to load and may be missing results. Not persisted.
223+
*/
224+
serverHydration: 'pending' | 'partial' | 'complete';
220225
clearAllData: () => Promise<void>;
221226
resetLocalData: () => void;
222227
clearSession: () => void;
@@ -484,6 +489,7 @@ export const useSessionStore = create<SessionState>()(
484489
setFactoidsForm: (data) => set((state) => ({ factoidsForm: mergeFormState(state.factoidsForm, data) })),
485490

486491
pendingFlowchartData: null as FlowchartData | null,
492+
serverHydration: 'pending',
487493
setPendingFlowchartData: (data) => set({ pendingFlowchartData: data }),
488494

489495
addSource: async (source) => {
@@ -1173,6 +1179,7 @@ export const useSessionStore = create<SessionState>()(
11731179
savedFlowcharts: flowchartsData ? serverFlowcharts : state.savedFlowcharts,
11741180
sourceFolders: mergeSourceFolders([...(sourceFoldersData ? [] : state.sourceFolders), ...serverSourceFolders], sourcesData ? serverSources : state.sourceLibrary),
11751181
sourceLibrary: sourcesData ? serverSources : state.sourceLibrary,
1182+
serverHydration: casesData && attacksData && brainstormsData && factoidsData && chatsData && crossfiresData ? 'complete' : 'partial',
11761183
}));
11771184
},
11781185

@@ -1207,6 +1214,7 @@ export const useSessionStore = create<SessionState>()(
12071214
});
12081215
},
12091216
resetLocalData: () => set({
1217+
serverHydration: 'pending',
12101218
savedCases: [],
12111219
savedAttacks: [],
12121220
savedBrainstorms: [],
@@ -1273,6 +1281,7 @@ export const useSessionStore = create<SessionState>()(
12731281
askLoading,
12741282
askStreamingAssistant,
12751283
pendingFlowchartData,
1284+
serverHydration,
12761285
...persisted
12771286
}) => persisted,
12781287
}

‎src/lib/stores/sparring-store.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ interface SparringStoreState {
5555
privateSessionBackup: SparringSession | null;
5656
savedSessions: SparringSession[];
5757
hydrated: boolean;
58+
/** False until saved history has loaded successfully; hydration can finish without it. */
59+
historyComplete: boolean;
5860
beginWorkspaceSession: () => void;
5961
endWorkspaceSession: () => void;
6062
startSession: (input: StartSessionInput) => SparringSession;
@@ -300,6 +302,7 @@ export const useSparringStore = create<SparringStoreState>()(
300302
privateSessionBackup: null as SparringSession | null,
301303
savedSessions: [] as SparringSession[],
302304
hydrated: false,
305+
historyComplete: false,
303306

304307
beginWorkspaceSession: () => set((state) => ({
305308
currentSession: null,
@@ -462,7 +465,7 @@ export const useSparringStore = create<SparringStoreState>()(
462465
prepareForUser: (userId) => {
463466
if (get().ownerUserId === userId) return;
464467
hydrationEpoch += 1;
465-
set({ ownerUserId: userId, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false });
468+
set({ ownerUserId: userId, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false, historyComplete: false });
466469
},
467470

468471
hydrateSavedSessions: async () => {
@@ -484,7 +487,7 @@ export const useSparringStore = create<SparringStoreState>()(
484487
set({ hydrated: true });
485488
return;
486489
}
487-
set((state) => ({ savedSessions: mergeSessions(state.savedSessions, parseSavedRows(data)), hydrated: true }));
490+
set((state) => ({ savedSessions: mergeSessions(state.savedSessions, parseSavedRows(data)), hydrated: true, historyComplete: true }));
488491
},
489492

490493
loadSavedSession: (id) => {
@@ -511,7 +514,7 @@ export const useSparringStore = create<SparringStoreState>()(
511514
},
512515
resetLocalData: () => {
513516
hydrationEpoch += 1;
514-
set({ ownerUserId: null, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false });
517+
set({ ownerUserId: null, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false, historyComplete: false });
515518
},
516519
}),
517520
{

0 commit comments

Comments
 (0)