From fa8736ab99336babbaf7711b5953a5fc5f3a0e00 Mon Sep 17 00:00:00 2001 From: Adam Date: Mon, 28 Sep 2026 19:13:11 +0000 Subject: [PATCH 1/7] fix(debate): keep saved pins through early changes and free deleted pins A pin made before the first pin load returned threw that load away, so saved pins looked unpinned and could not be unpinned. Loads now replay local changes on top of the saved rows. Pin writes for a debate run in order, the limit counts only pins whose result still exists, and pin buttons wait for saved pins to load. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/debate/DebateContextControl.tsx | 10 ++-- src/lib/debate/context.ts | 6 +++ src/lib/stores/debate-context-store.ts | 53 +++++++++++++------ src/test/debate/debate-context.test.tsx | 51 +++++++++++++++++- 4 files changed, 98 insertions(+), 22 deletions(-) diff --git a/src/components/features/debate/DebateContextControl.tsx b/src/components/features/debate/DebateContextControl.tsx index e9e2b0c08..9c7adc08e 100644 --- a/src/components/features/debate/DebateContextControl.tsx +++ b/src/components/features/debate/DebateContextControl.tsx @@ -99,14 +99,14 @@ export function DebateContextControl({ feature, side, includeSources = true, deb - + ); } -function DebateContextReview({ plan, motion, usage }: { plan: DebateContextPlan; motion?: string; usage?: DebateContextUsage }) { +function DebateContextReview({ plan, motion, usage, pinsReady }: { plan: DebateContextPlan; motion?: string; usage?: DebateContextUsage; pinsReady: boolean }) { const { t } = useTranslation(); const setSourceChoice = useDebateContextStore((state) => state.setSourceChoice); const setOutputChoice = useDebateContextStore((state) => state.setOutputChoice); @@ -116,7 +116,7 @@ function DebateContextReview({ plan, motion, usage }: { plan: DebateContextPlan; const onPin = async (output: ContextOutputItem) => { try { - await togglePin(plan.debateId, output.ref); + await togglePin(plan.debateId, output.ref, plan.pinnedCount); } catch (error) { toast({ title: error instanceof Error && error.message === 'pin_limit' @@ -180,6 +180,7 @@ function DebateContextReview({ plan, motion, usage }: { plan: DebateContextPlan; output={output} onChange={(include) => setOutputChoice(plan.feature, plan.debateId, output.ref, include)} onPin={() => void onPin(output)} + pinDisabled={!pinsReady} /> ))} @@ -221,7 +222,7 @@ function SourceRow({ source, onChange }: { source: ContextSourceItem; onChange: ); } -function OutputRow({ output, onChange, onPin }: { output: ContextOutputItem; onChange: (include: boolean) => void; onPin: () => void }) { +function OutputRow({ output, onChange, onPin, pinDisabled }: { output: ContextOutputItem; onChange: (include: boolean) => void; onPin: () => void; pinDisabled: boolean }) { const { t } = useTranslation(); const id = `debate-context-output-${output.ref.kind}-${output.ref.id}`; const note = output.included ? statusNote(output, t) : null; @@ -247,6 +248,7 @@ function OutputRow({ output, onChange, onPin }: { output: ContextOutputItem; onC ? t('debateContext.unpinAria', { title: output.title || kind }) : t('debateContext.pinAria', { title: output.title || kind })} onClick={onPin} + disabled={pinDisabled} > {output.pinned ? : } diff --git a/src/lib/debate/context.ts b/src/lib/debate/context.ts index 4cdfd36da..68bbecbf8 100644 --- a/src/lib/debate/context.ts +++ b/src/lib/debate/context.ts @@ -64,6 +64,8 @@ export interface DebateContextPlan { policy: DebateContextPolicy; sources: ContextSourceItem[]; outputs: ContextOutputItem[]; + /** Pins that still point at a result in this debate, the open Ask chat included. */ + pinnedCount: number; request: DebateContextRequest; } @@ -153,6 +155,9 @@ export function planDebateContext( const pinnedKeys = new Set(inputs.pins.map(outputKey)); const candidates = policy.outputs ? outputCandidates(debateId, inputs) : []; + // A pin on a deleted result is never shown, so it must not use up the limit. + const pinnedCount = outputCandidates(debateId, { ...inputs, excludeChatId: null }) + .filter((item) => pinnedKeys.has(outputKey(item.ref))).length; const defaults = selectDefaultOutputs( candidates.map((item) => ({ ...item.ref, side: item.side, createdAt: item.createdAt, pinned: pinnedKeys.has(outputKey(item.ref)) })), feature, @@ -194,6 +199,7 @@ export function planDebateContext( policy, sources, outputs, + pinnedCount, request: { debateId, side, diff --git a/src/lib/stores/debate-context-store.ts b/src/lib/stores/debate-context-store.ts index 33846be2e..f8f6a2dbd 100644 --- a/src/lib/stores/debate-context-store.ts +++ b/src/lib/stores/debate-context-store.ts @@ -40,7 +40,8 @@ interface DebateContextState { lastUsage: Partial>; loadPins: (debateId: string) => Promise; - togglePin: (debateId: string, ref: DebateOutputRef) => Promise; + /** `pinnedCount` is how many pins still point at an existing result. */ + togglePin: (debateId: string, ref: DebateOutputRef, pinnedCount?: number) => Promise; setSourceChoice: (feature: DebateContextFeature, debateId: string, sourceId: string, include: boolean) => void; setOutputChoice: (feature: DebateContextFeature, debateId: string, ref: DebateOutputRef, include: boolean) => void; resetOverride: (feature: DebateContextFeature) => void; @@ -52,9 +53,14 @@ function ensureOverride(current: DebateContextOverride | undefined, debateId: st return current && current.debateId === debateId ? current : { debateId, sources: {}, outputs: {} }; } -// Counts pin changes per debate, so a load that started before a change -// cannot bring back what the student just pinned or unpinned. -const pinChanges: Record = {}; +// Every local pin change, per debate and in order. A load replays the changes +// it may have missed: ones made after it started, and ones still saving. +interface PinChange { seq: number; ref: DebateOutputRef; pinned: boolean; saving: boolean } +const pinChanges: Record = {}; +let pinSeq = 0; +// Each debate's pin writes run one at a time, so the database ends in the +// same state as the screen even after a quick pin and unpin. +const pinWrites: Record> = {}; function withPin(pins: DebateOutputRef[], ref: DebateOutputRef, pinned: boolean): DebateOutputRef[] { const without = pins.filter((item) => outputKey(item) !== outputKey(ref)); @@ -68,43 +74,53 @@ export const useDebateContextStore = create()((set, get) => lastUsage: {}, loadPins: async (debateId) => { - const changesBefore = pinChanges[debateId] ?? 0; + const startedAt = pinSeq; const { data, error } = await supabase .from('debate_context_pins') .select('item_kind,item_id') .eq('debate_id', debateId); // Pins are an optional preference. Until the table exists everywhere, or // if it cannot be read, requests fall back to the automatic selection. - if (error || (pinChanges[debateId] ?? 0) !== changesBefore) { + if (error) { set((state) => ({ pinsLoaded: { ...state.pinsLoaded, [debateId]: true } })); return; } - const pins = ((data ?? []) as Array<{ item_kind: unknown; item_id: unknown }>).flatMap((row) => + let pins = ((data ?? []) as Array<{ item_kind: unknown; item_id: unknown }>).flatMap((row) => isDebateContextOutputKind(row.item_kind) && typeof row.item_id === 'string' ? [{ kind: row.item_kind as DebateContextOutputKind, id: row.item_id }] : []); + for (const change of pinChanges[debateId] ?? []) { + if (change.seq > startedAt || change.saving) pins = withPin(pins, change.ref, change.pinned); + } set((state) => ({ pins: { ...state.pins, [debateId]: pins }, pinsLoaded: { ...state.pinsLoaded, [debateId]: true }, })); }, - togglePin: async (debateId, ref) => { + togglePin: async (debateId, ref, pinnedCount) => { const current = get().pins[debateId] ?? []; const pinned = current.some((item) => outputKey(item) === outputKey(ref)); - if (!pinned && current.length >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit'); - pinChanges[debateId] = (pinChanges[debateId] ?? 0) + 1; - set((state) => ({ pins: { ...state.pins, [debateId]: withPin(current, ref, !pinned) } })); - const { data } = await supabase.auth.getUser(); - const userId = data.user?.id; - const result = !userId - ? { error: { message: 'Sign in to pin results' } } - : pinned + if (!pinned && (pinnedCount ?? current.length) >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit'); + const change: PinChange = { seq: ++pinSeq, ref, pinned: !pinned, saving: true }; + (pinChanges[debateId] ??= []).push(change); + set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, !pinned) } })); + const save = async (): Promise<{ error: { message?: string } | null }> => { + const { data } = await supabase.auth.getUser(); + const userId = data.user?.id; + if (!userId) return { error: { message: 'Sign in to pin results' } }; + return pinned ? await supabase.from('debate_context_pins').delete() .eq('user_id', userId).eq('debate_id', debateId).eq('item_kind', ref.kind).eq('item_id', ref.id) : await supabase.from('debate_context_pins').insert({ user_id: userId, debate_id: debateId, item_kind: ref.kind, item_id: ref.id }); + }; + const write = (pinWrites[debateId] ?? Promise.resolve()).then(save); + pinWrites[debateId] = write.catch((): void => undefined); + const result = await write.catch((error: unknown) => ({ error: { message: error instanceof Error ? error.message : undefined } })); + change.saving = false; if (result.error) { // Undo only this change; other pins may have changed in the meantime. + pinChanges[debateId] = pinChanges[debateId].filter((item) => item !== change); set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, pinned) } })); throw new Error(result.error.message ?? 'Could not save the pin'); } @@ -134,7 +150,10 @@ export const useDebateContextStore = create()((set, get) => return { lastUsage }; }), - resetLocalData: () => set({ pins: {}, pinsLoaded: {}, overrides: {}, lastUsage: {} }), + resetLocalData: () => { + for (const debateId of Object.keys(pinChanges)) delete pinChanges[debateId]; + set({ pins: {}, pinsLoaded: {}, overrides: {}, lastUsage: {} }); + }, })); /** Reads the server's `meta.debateContext`, ignoring anything malformed. */ diff --git a/src/test/debate/debate-context.test.tsx b/src/test/debate/debate-context.test.tsx index 90917d992..134ed984b 100644 --- a/src/test/debate/debate-context.test.tsx +++ b/src/test/debate/debate-context.test.tsx @@ -95,6 +95,15 @@ describe('Current Debate context plan', () => { const plan = planDebateContext('ask', 'both', TEST_DEBATE_ID, inputs({ pins: [{ kind: 'chat', id: uuid(20) }], excludeChatId: uuid(20) })); expect(plan.outputs.map((item) => item.ref.id)).not.toContain(uuid(20)); }); + + it('counts only pins whose result still exists, including the chat open in Ask', () => { + const deleted = { kind: 'case' as const, id: uuid(40) }; + const plan = planDebateContext('ask', 'both', TEST_DEBATE_ID, inputs({ + pins: [{ kind: 'case', id: uuid(10) }, { kind: 'chat', id: uuid(20) }, deleted], + excludeChatId: uuid(20), + })); + expect(plan.pinnedCount).toBe(2); + }); }); describe('Current Debate context in requests and the Review control', () => { @@ -215,10 +224,50 @@ describe('Pinned results', () => { useDebateContextStore.setState({ pins: { [TEST_DEBATE_ID]: [caseRef] } }); const pinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, attackRef); - await useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + const unpinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); insert.resolve({ error: { message: 'offline' } }); await expect(pinning).rejects.toThrow('offline'); + await unpinning; + + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([]); + }); + + it('keeps saved pins when the student pins before the first load returns', async () => { + const load = deferred<{ data: unknown; error: null }>(); + supabaseMock.from.mockImplementationOnce((() => ({ select: () => ({ eq: () => load.promise }) })) as never); + + const loading = useDebateContextStore.getState().loadPins(TEST_DEBATE_ID); + await useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, attackRef); + load.resolve({ data: [{ item_kind: 'case', item_id: caseRef.id }], error: null }); + await loading; + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([caseRef, attackRef]); + }); + + it('saves a quick pin and unpin of the same result in the order they were made', async () => { + const insert = deferred<{ error: null }>(); + const calls: string[] = []; + const deleted = { eq: () => deleted, then: (done: (value: { error: null }) => void) => { calls.push('delete'); done({ error: null }); } }; + supabaseMock.from + .mockImplementationOnce((() => ({ insert: () => { calls.push('insert'); return insert.promise; } })) as never) + .mockImplementationOnce((() => ({ delete: () => deleted })) as never); + + const pinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + const unpinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + await new Promise((done) => setTimeout(done, 0)); + expect(calls).toEqual(['insert']); + insert.resolve({ error: null }); + await Promise.all([pinning, unpinning]); + + expect(calls).toEqual(['insert', 'delete']); expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([]); }); + + it('does not count pins on deleted results toward the limit', async () => { + const gone = [40, 41, 42].map((n) => ({ kind: 'case' as const, id: uuid(n) })); + useDebateContextStore.setState({ pins: { [TEST_DEBATE_ID]: gone } }); + + await expect(useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, attackRef, 2)).resolves.toBeUndefined(); + await expect(useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef, 3)).rejects.toThrow('pin_limit'); + }); }); From c8d5d39a4431ab4a6239bd633915948bf6374f75 Mon Sep 17 00:00:00 2001 From: Adam Date: Mon, 28 Sep 2026 19:55:52 +0000 Subject: [PATCH 2/7] fix(debate): check the pin limit against current pins, not the rendered count Two quick pin clicks shared one rendered count, so both could pass the three-pin limit. The plan now passes the debate's existing result keys, and the store counts its current pins against them. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/debate/DebateContextControl.tsx | 2 +- src/lib/debate/context.ts | 9 ++++---- src/lib/stores/debate-context-store.ts | 9 ++++---- src/test/debate/debate-context.test.tsx | 23 +++++++++++-------- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/components/features/debate/DebateContextControl.tsx b/src/components/features/debate/DebateContextControl.tsx index 9c7adc08e..e7aeea5f4 100644 --- a/src/components/features/debate/DebateContextControl.tsx +++ b/src/components/features/debate/DebateContextControl.tsx @@ -116,7 +116,7 @@ function DebateContextReview({ plan, motion, usage, pinsReady }: { plan: DebateC const onPin = async (output: ContextOutputItem) => { try { - await togglePin(plan.debateId, output.ref, plan.pinnedCount); + await togglePin(plan.debateId, output.ref, plan.resultKeys); } catch (error) { toast({ title: error instanceof Error && error.message === 'pin_limit' diff --git a/src/lib/debate/context.ts b/src/lib/debate/context.ts index 68bbecbf8..ab3fa01f0 100644 --- a/src/lib/debate/context.ts +++ b/src/lib/debate/context.ts @@ -64,8 +64,8 @@ export interface DebateContextPlan { policy: DebateContextPolicy; sources: ContextSourceItem[]; outputs: ContextOutputItem[]; - /** Pins that still point at a result in this debate, the open Ask chat included. */ - pinnedCount: number; + /** Every result in this debate, the open Ask chat included; pins on anything else no longer count. */ + resultKeys: ReadonlySet; request: DebateContextRequest; } @@ -156,8 +156,7 @@ export function planDebateContext( const pinnedKeys = new Set(inputs.pins.map(outputKey)); const candidates = policy.outputs ? outputCandidates(debateId, inputs) : []; // A pin on a deleted result is never shown, so it must not use up the limit. - const pinnedCount = outputCandidates(debateId, { ...inputs, excludeChatId: null }) - .filter((item) => pinnedKeys.has(outputKey(item.ref))).length; + const resultKeys = new Set(outputCandidates(debateId, { ...inputs, excludeChatId: null }).map((item) => outputKey(item.ref))); const defaults = selectDefaultOutputs( candidates.map((item) => ({ ...item.ref, side: item.side, createdAt: item.createdAt, pinned: pinnedKeys.has(outputKey(item.ref)) })), feature, @@ -199,7 +198,7 @@ export function planDebateContext( policy, sources, outputs, - pinnedCount, + resultKeys, request: { debateId, side, diff --git a/src/lib/stores/debate-context-store.ts b/src/lib/stores/debate-context-store.ts index f8f6a2dbd..bb7db9f1d 100644 --- a/src/lib/stores/debate-context-store.ts +++ b/src/lib/stores/debate-context-store.ts @@ -40,8 +40,8 @@ interface DebateContextState { lastUsage: Partial>; loadPins: (debateId: string) => Promise; - /** `pinnedCount` is how many pins still point at an existing result. */ - togglePin: (debateId: string, ref: DebateOutputRef, pinnedCount?: number) => Promise; + /** Only pins on `resultKeys`, the debate's existing results, count toward the limit. */ + togglePin: (debateId: string, ref: DebateOutputRef, resultKeys?: ReadonlySet) => Promise; setSourceChoice: (feature: DebateContextFeature, debateId: string, sourceId: string, include: boolean) => void; setOutputChoice: (feature: DebateContextFeature, debateId: string, ref: DebateOutputRef, include: boolean) => void; resetOverride: (feature: DebateContextFeature) => void; @@ -98,10 +98,11 @@ export const useDebateContextStore = create()((set, get) => })); }, - togglePin: async (debateId, ref, pinnedCount) => { + togglePin: async (debateId, ref, resultKeys) => { const current = get().pins[debateId] ?? []; const pinned = current.some((item) => outputKey(item) === outputKey(ref)); - if (!pinned && (pinnedCount ?? current.length) >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit'); + const counted = resultKeys ? current.filter((item) => resultKeys.has(outputKey(item))) : current; + if (!pinned && counted.length >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit'); const change: PinChange = { seq: ++pinSeq, ref, pinned: !pinned, saving: true }; (pinChanges[debateId] ??= []).push(change); set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, !pinned) } })); diff --git a/src/test/debate/debate-context.test.tsx b/src/test/debate/debate-context.test.tsx index 134ed984b..27df74108 100644 --- a/src/test/debate/debate-context.test.tsx +++ b/src/test/debate/debate-context.test.tsx @@ -96,13 +96,14 @@ describe('Current Debate context plan', () => { expect(plan.outputs.map((item) => item.ref.id)).not.toContain(uuid(20)); }); - it('counts only pins whose result still exists, including the chat open in Ask', () => { - const deleted = { kind: 'case' as const, id: uuid(40) }; + it('lists every result in the debate for the pin limit, including the chat open in Ask', () => { const plan = planDebateContext('ask', 'both', TEST_DEBATE_ID, inputs({ - pins: [{ kind: 'case', id: uuid(10) }, { kind: 'chat', id: uuid(20) }, deleted], + pins: [{ kind: 'case', id: uuid(10) }, { kind: 'chat', id: uuid(20) }, { kind: 'case', id: uuid(40) }], excludeChatId: uuid(20), })); - expect(plan.pinnedCount).toBe(2); + expect(plan.resultKeys.has(`case:${uuid(10)}`)).toBe(true); + expect(plan.resultKeys.has(`chat:${uuid(20)}`)).toBe(true); + expect(plan.resultKeys.has(`case:${uuid(40)}`)).toBe(false); }); }); @@ -264,10 +265,14 @@ describe('Pinned results', () => { }); it('does not count pins on deleted results toward the limit', async () => { - const gone = [40, 41, 42].map((n) => ({ kind: 'case' as const, id: uuid(n) })); - useDebateContextStore.setState({ pins: { [TEST_DEBATE_ID]: gone } }); - - await expect(useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, attackRef, 2)).resolves.toBeUndefined(); - await expect(useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef, 3)).rejects.toThrow('pin_limit'); + const live = [12, 14].map((n) => ({ kind: 'case' as const, id: uuid(n) })); + const existing = new Set([...live, caseRef, attackRef].map((ref) => `${ref.kind}:${ref.id}`)); + useDebateContextStore.setState({ pins: { [TEST_DEBATE_ID]: [{ kind: 'case', id: uuid(40) }, ...live] } }); + + // Two quick clicks share one rendered plan; the second must see the first. + const first = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, attackRef, existing); + const second = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef, existing); + await expect(first).resolves.toBeUndefined(); + await expect(second).rejects.toThrow('pin_limit'); }); }); From afb7288a5242076f9354c8c5fc8ab5a6fe9d534c Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 30 Sep 2026 13:21:30 +0000 Subject: [PATCH 3/7] fix(debate): drop pin work left over from before a sign-out A pin save queued before sign-out ran after it, reading the next account's user ID, and a failed old save could undo the next account's pin. The next account's first pin change also waited behind the old request, and a pin load that returned after sign-out filled in the old account's pins. Resetting local data now starts a new generation. Old writes that have not started are skipped, old results leave state alone, the write queue starts fresh, and late loads are ignored. Also explains why pins on results that are not loaded locally do not count toward the limit: they cannot be shown, sent or unpinned. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/lib/debate/context.ts | 4 ++- src/lib/stores/debate-context-store.ts | 11 +++++++ src/test/debate/debate-context.test.tsx | 38 +++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/lib/debate/context.ts b/src/lib/debate/context.ts index ab3fa01f0..fecb09031 100644 --- a/src/lib/debate/context.ts +++ b/src/lib/debate/context.ts @@ -155,7 +155,9 @@ export function planDebateContext( const pinnedKeys = new Set(inputs.pins.map(outputKey)); const candidates = policy.outputs ? outputCandidates(debateId, inputs) : []; - // A pin on a deleted result is never shown, so it must not use up the limit. + // A pin on a result that isn't loaded here, because it was deleted or is + // older than local history keeps, is never shown or sent. It must not use up + // the limit, or the student could never free that slot. const resultKeys = new Set(outputCandidates(debateId, { ...inputs, excludeChatId: null }).map((item) => outputKey(item.ref))); const defaults = selectDefaultOutputs( candidates.map((item) => ({ ...item.ref, side: item.side, createdAt: item.createdAt, pinned: pinnedKeys.has(outputKey(item.ref)) })), diff --git a/src/lib/stores/debate-context-store.ts b/src/lib/stores/debate-context-store.ts index bb7db9f1d..ebb17a864 100644 --- a/src/lib/stores/debate-context-store.ts +++ b/src/lib/stores/debate-context-store.ts @@ -61,6 +61,9 @@ let pinSeq = 0; // Each debate's pin writes run one at a time, so the database ends in the // same state as the screen even after a quick pin and unpin. const pinWrites: Record> = {}; +// Bumped when local data is reset on sign-out. Pin work started before that +// belongs to the previous account, so it must not write or change state. +let pinGeneration = 0; function withPin(pins: DebateOutputRef[], ref: DebateOutputRef, pinned: boolean): DebateOutputRef[] { const without = pins.filter((item) => outputKey(item) !== outputKey(ref)); @@ -75,10 +78,12 @@ export const useDebateContextStore = create()((set, get) => loadPins: async (debateId) => { const startedAt = pinSeq; + const generation = pinGeneration; const { data, error } = await supabase .from('debate_context_pins') .select('item_kind,item_id') .eq('debate_id', debateId); + if (generation !== pinGeneration) return; // Pins are an optional preference. Until the table exists everywhere, or // if it cannot be read, requests fall back to the automatic selection. if (error) { @@ -103,12 +108,15 @@ export const useDebateContextStore = create()((set, get) => const pinned = current.some((item) => outputKey(item) === outputKey(ref)); const counted = resultKeys ? current.filter((item) => resultKeys.has(outputKey(item))) : current; if (!pinned && counted.length >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit'); + const generation = pinGeneration; const change: PinChange = { seq: ++pinSeq, ref, pinned: !pinned, saving: true }; (pinChanges[debateId] ??= []).push(change); set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, !pinned) } })); const save = async (): Promise<{ error: { message?: string } | null }> => { + if (generation !== pinGeneration) return { error: null }; const { data } = await supabase.auth.getUser(); const userId = data.user?.id; + if (generation !== pinGeneration) return { error: null }; if (!userId) return { error: { message: 'Sign in to pin results' } }; return pinned ? await supabase.from('debate_context_pins').delete() @@ -119,6 +127,7 @@ export const useDebateContextStore = create()((set, get) => pinWrites[debateId] = write.catch((): void => undefined); const result = await write.catch((error: unknown) => ({ error: { message: error instanceof Error ? error.message : undefined } })); change.saving = false; + if (generation !== pinGeneration) return; if (result.error) { // Undo only this change; other pins may have changed in the meantime. pinChanges[debateId] = pinChanges[debateId].filter((item) => item !== change); @@ -152,7 +161,9 @@ export const useDebateContextStore = create()((set, get) => }), resetLocalData: () => { + pinGeneration += 1; for (const debateId of Object.keys(pinChanges)) delete pinChanges[debateId]; + for (const debateId of Object.keys(pinWrites)) delete pinWrites[debateId]; set({ pins: {}, pinsLoaded: {}, overrides: {}, lastUsage: {} }); }, })); diff --git a/src/test/debate/debate-context.test.tsx b/src/test/debate/debate-context.test.tsx index 27df74108..60d094366 100644 --- a/src/test/debate/debate-context.test.tsx +++ b/src/test/debate/debate-context.test.tsx @@ -275,4 +275,42 @@ describe('Pinned results', () => { await expect(first).resolves.toBeUndefined(); await expect(second).rejects.toThrow('pin_limit'); }); + + it('drops pin writes left over from before a sign-out', async () => { + const oldInsert = deferred<{ error: { message: string } }>(); + const calls: string[] = []; + supabaseMock.from + .mockImplementationOnce((() => ({ insert: () => { calls.push('old'); return oldInsert.promise; } })) as never) + .mockImplementationOnce((() => ({ insert: () => { calls.push('new'); return Promise.resolve({ error: null }); } })) as never); + + const oldPin = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + const oldQueued = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, attackRef); + await new Promise((done) => setTimeout(done, 0)); + useDebateContextStore.getState().resetLocalData(); + + // The next account's change must not wait behind the old one. + await useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + expect(calls).toEqual(['old', 'new']); + + // The old failure must not undo the new account's pin, and the queued old + // write must never run under the new account. + oldInsert.resolve({ error: { message: 'offline' } }); + await expect(oldPin).resolves.toBeUndefined(); + await expect(oldQueued).resolves.toBeUndefined(); + expect(calls).toEqual(['old', 'new']); + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([caseRef]); + }); + + it('ignores a pin load that returns after a sign-out', async () => { + const load = deferred<{ data: unknown; error: null }>(); + supabaseMock.from.mockImplementationOnce((() => ({ select: () => ({ eq: () => load.promise }) })) as never); + + const loading = useDebateContextStore.getState().loadPins(TEST_DEBATE_ID); + useDebateContextStore.getState().resetLocalData(); + load.resolve({ data: [{ item_kind: 'case', item_id: caseRef.id }], error: null }); + await loading; + + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toBeUndefined(); + expect(useDebateContextStore.getState().pinsLoaded[TEST_DEBATE_ID]).toBeUndefined(); + }); }); From dc74e3a4489ce6d112e412be344b163586ed1abd Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 30 Sep 2026 13:29:18 +0000 Subject: [PATCH 4/7] fix(debate): keep a later pin when an earlier save fails Pin, unpin and pin again while the first save was pending: if that save failed, its rollback removed the latest pin from the screen, while the queued delete and insert still succeeded. The database ended pinned and the app did not. A failed save now undoes its change only when no later change to the same result is queued behind it. An insert that hits the primary key means the result is already pinned, so it counts as saved. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/lib/stores/debate-context-store.ts | 13 +++++++--- src/test/debate/debate-context.test.tsx | 33 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/lib/stores/debate-context-store.ts b/src/lib/stores/debate-context-store.ts index ebb17a864..cea87755d 100644 --- a/src/lib/stores/debate-context-store.ts +++ b/src/lib/stores/debate-context-store.ts @@ -112,7 +112,7 @@ export const useDebateContextStore = create()((set, get) => const change: PinChange = { seq: ++pinSeq, ref, pinned: !pinned, saving: true }; (pinChanges[debateId] ??= []).push(change); set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, !pinned) } })); - const save = async (): Promise<{ error: { message?: string } | null }> => { + const save = async (): Promise<{ error: { message?: string; code?: string } | null }> => { if (generation !== pinGeneration) return { error: null }; const { data } = await supabase.auth.getUser(); const userId = data.user?.id; @@ -125,12 +125,19 @@ export const useDebateContextStore = create()((set, get) => }; const write = (pinWrites[debateId] ?? Promise.resolve()).then(save); pinWrites[debateId] = write.catch((): void => undefined); - const result = await write.catch((error: unknown) => ({ error: { message: error instanceof Error ? error.message : undefined } })); + const result: Awaited> = await write + .catch((error: unknown) => ({ error: { message: error instanceof Error ? error.message : undefined } })); change.saving = false; if (generation !== pinGeneration) return; + // The row already exists, so the result is pinned as asked. + if (!pinned && result.error?.code === '23505') return; if (result.error) { - // Undo only this change; other pins may have changed in the meantime. + // Undo only this change; other pins may have changed in the meantime. A + // later change to the same result is queued behind this one and decides + // its final state, so this failure no longer matters. + const superseded = pinChanges[debateId].some((item) => item.seq > change.seq && outputKey(item.ref) === outputKey(ref)); pinChanges[debateId] = pinChanges[debateId].filter((item) => item !== change); + if (superseded) return; set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, pinned) } })); throw new Error(result.error.message ?? 'Could not save the pin'); } diff --git a/src/test/debate/debate-context.test.tsx b/src/test/debate/debate-context.test.tsx index 60d094366..e58f16eac 100644 --- a/src/test/debate/debate-context.test.tsx +++ b/src/test/debate/debate-context.test.tsx @@ -276,6 +276,39 @@ describe('Pinned results', () => { await expect(second).rejects.toThrow('pin_limit'); }); + it('keeps a later pin when an earlier save of the same result fails', async () => { + const firstInsert = deferred<{ error: { message: string } }>(); + const calls: string[] = []; + const deleted = { eq: () => deleted, then: (done: (value: { error: null }) => void) => { calls.push('delete'); done({ error: null }); } }; + supabaseMock.from + .mockImplementationOnce((() => ({ insert: () => { calls.push('insert'); return firstInsert.promise; } })) as never) + .mockImplementationOnce((() => ({ delete: () => deleted })) as never) + .mockImplementationOnce((() => ({ insert: () => { calls.push('insert'); return Promise.resolve({ error: null }); } })) as never); + + // Pin, unpin and pin again while the first save is still pending. + const toggles = [0, 1, 2].map(() => useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef)); + firstInsert.resolve({ error: { message: 'offline' } }); + await Promise.all(toggles); + + expect(calls).toEqual(['insert', 'delete', 'insert']); + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([caseRef]); + }); + + it('treats pinning a result that is already saved as pinned', async () => { + const deleted = { eq: () => deleted, then: (done: (value: { error: { message: string } }) => void) => done({ error: { message: 'offline' } }) }; + supabaseMock.from + .mockImplementationOnce((() => ({ delete: () => deleted })) as never) + .mockImplementationOnce((() => ({ insert: () => Promise.resolve({ error: { code: '23505', message: 'duplicate key' } }) })) as never); + useDebateContextStore.setState({ pins: { [TEST_DEBATE_ID]: [caseRef] } }); + + // The unpin fails, so the row stays; pinning again must not unpin locally. + const unpinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + const pinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + await Promise.all([unpinning, pinning]); + + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([caseRef]); + }); + it('drops pin writes left over from before a sign-out', async () => { const oldInsert = deferred<{ error: { message: string } }>(); const calls: string[] = []; From 253c43601c397e48b1102b7afa307a84c3ed1182 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 30 Sep 2026 13:41:13 +0000 Subject: [PATCH 5/7] fix(debate): wait for saved results before allowing pins The pin limit leaves out pins on results that are not listed, so a deleted result cannot hold a slot forever. On a fresh sign-in, results load in parallel with pins. Until they arrive, pins on results still loading were left out too, and a fourth pin could get through. Pin buttons now also wait for saved results and sparring history to load. The session store gets a serverHydrated flag for this. It is not persisted and is cleared on sign-out. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/debate/DebateContextControl.tsx | 8 ++++++- src/lib/stores/session-store.ts | 6 +++++ src/test/debate/debate-context.test.tsx | 23 ++++++++++++++++++- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/components/features/debate/DebateContextControl.tsx b/src/components/features/debate/DebateContextControl.tsx index e7aeea5f4..c5973cbcb 100644 --- a/src/components/features/debate/DebateContextControl.tsx +++ b/src/components/features/debate/DebateContextControl.tsx @@ -9,6 +9,8 @@ import { useDebateContextPlan } from '@/lib/debate/context-state'; import type { ContextOutputItem, ContextSourceItem, DebateContextPlan } from '@/lib/debate/context'; import { useCurrentDebate, useDebateStore } from '@/lib/stores/debate-store'; import { outputKey, useDebateContextStore, type DebateContextUsage } from '@/lib/stores/debate-context-store'; +import { useSessionStore } from '@/lib/stores/session-store'; +import { useSparringStore } from '@/lib/stores/sparring-store'; import type { DebateContextFeature, DebateContextSide } from '../../../../supabase/functions/_shared/debate-context'; interface DebateContextControlProps { @@ -42,6 +44,10 @@ export function DebateContextControl({ feature, side, includeSources = true, deb const debateId = debateIdOverride !== undefined ? debateIdOverride : (oneOff ? null : debate?.id ?? null); const plan = useDebateContextPlan(feature, side, debateId, { includeSources }); const pinsLoaded = useDebateContextStore((state) => (debateId ? state.pinsLoaded[debateId] : true)); + // The pin limit discounts pins on results that are not listed. Until saved + // results have loaded, a missing result may just not be here yet. + const sessionHydrated = useSessionStore((state) => state.serverHydrated); + const sparringHydrated = useSparringStore((state) => state.hydrated); const loadPins = useDebateContextStore((state) => state.loadPins); const usage = useDebateContextStore((state) => state.lastUsage[feature]); const [open, setOpen] = useState(false); @@ -99,7 +105,7 @@ export function DebateContextControl({ feature, side, includeSources = true, deb - + diff --git a/src/lib/stores/session-store.ts b/src/lib/stores/session-store.ts index abea0a433..30c245c5b 100644 --- a/src/lib/stores/session-store.ts +++ b/src/lib/stores/session-store.ts @@ -217,6 +217,8 @@ export interface SessionState { clearLibrary: () => Promise; hydrateFromServer: () => Promise; + /** True once this page load's server hydration has finished. Not persisted. */ + serverHydrated: boolean; clearAllData: () => Promise; resetLocalData: () => void; clearSession: () => void; @@ -484,6 +486,7 @@ export const useSessionStore = create()( setFactoidsForm: (data) => set((state) => ({ factoidsForm: mergeFormState(state.factoidsForm, data) })), pendingFlowchartData: null as FlowchartData | null, + serverHydrated: false, setPendingFlowchartData: (data) => set({ pendingFlowchartData: data }), addSource: async (source) => { @@ -1173,6 +1176,7 @@ export const useSessionStore = create()( savedFlowcharts: flowchartsData ? serverFlowcharts : state.savedFlowcharts, sourceFolders: mergeSourceFolders([...(sourceFoldersData ? [] : state.sourceFolders), ...serverSourceFolders], sourcesData ? serverSources : state.sourceLibrary), sourceLibrary: sourcesData ? serverSources : state.sourceLibrary, + serverHydrated: true, })); }, @@ -1207,6 +1211,7 @@ export const useSessionStore = create()( }); }, resetLocalData: () => set({ + serverHydrated: false, savedCases: [], savedAttacks: [], savedBrainstorms: [], @@ -1273,6 +1278,7 @@ export const useSessionStore = create()( askLoading, askStreamingAssistant, pendingFlowchartData, + serverHydrated, ...persisted }) => persisted, } diff --git a/src/test/debate/debate-context.test.tsx b/src/test/debate/debate-context.test.tsx index e58f16eac..0afa4361e 100644 --- a/src/test/debate/debate-context.test.tsx +++ b/src/test/debate/debate-context.test.tsx @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import { resetMockState, setMockInvokeResponse, setMockSessionToken, supabaseMock } from '@/test/mocks/supabase'; vi.mock('@/lib/supabase/client', async () => { @@ -19,6 +19,7 @@ import { brainstorm } from '@/lib/ai/client'; import { useDebateContextStore } from '@/lib/stores/debate-context-store'; import { useDebateStore } from '@/lib/stores/debate-store'; import { useSessionStore, type SavedItem, type Source } from '@/lib/stores/session-store'; +import { useSparringStore } from '@/lib/stores/sparring-store'; import { enableOneOffMode, resetDebates, seedCurrentDebate, TEST_DEBATE_ID } from '@/test/helpers/debate'; const OTHER_DEBATE = '99999999-2222-4333-8444-555555555555'; @@ -117,7 +118,9 @@ describe('Current Debate context in requests and the Review control', () => { sourceLibrary: [source(1), source(2, { isActive: false })], savedCases: [result(10, caseData) as SavedItem], savedAttacks: [], savedBrainstorms: [], savedFactoids: [], savedChats: [], savedCrossfires: [], + serverHydrated: true, }); + useSparringStore.setState({ hydrated: true }); }); afterEach(() => { resetDebates(); @@ -188,6 +191,24 @@ describe('Current Debate context in requests and the Review control', () => { await waitFor(() => expect(control).toHaveTextContent('Using 2 sources and 1 saved result')); expect(within(sheet).getByRole('button', { name: /Pin Result 10/ })).toHaveAttribute('aria-pressed', 'false'); }); + + it('keeps pin buttons disabled until saved results have loaded', async () => { + // Pins on results that have not loaded yet cannot be counted toward the limit. + seedCurrentDebate(); + useSessionStore.setState({ serverHydrated: false }); + useSparringStore.setState({ hydrated: false }); + render(); + fireEvent.click(within(screen.getByTestId('debate-context-control')).getByRole('button', { name: 'Review' })); + const sheet = await screen.findByRole('dialog'); + await waitFor(() => expect(useDebateContextStore.getState().pinsLoaded[TEST_DEBATE_ID]).toBe(true)); + const pin = within(sheet).getByRole('button', { name: /Pin Result 10/ }); + + expect(pin).toBeDisabled(); + act(() => useSessionStore.setState({ serverHydrated: true })); + expect(pin).toBeDisabled(); + act(() => useSparringStore.setState({ hydrated: true })); + expect(pin).toBeEnabled(); + }); }); describe('Pinned results', () => { From ad4329417fc9483b5d41c6c40f6c351ad4038319 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 30 Sep 2026 14:09:01 +0000 Subject: [PATCH 6/7] fix(debate): count every pin when saved results failed to load A saved-result or sparring history query that fails still finishes hydration, leaving that list empty. The pin limit then left out real pins on the missing results, so a fourth pin could get through. The session store now reports serverHydration as pending, partial or complete, and the sparring store reports historyComplete. Pins on unlisted results are left out of the limit only when every list loaded. A quick pin and unpin that both failed also left the result pinned, although neither save reached the database. A failed change that a later change replaced now hands its prior state to that change, so a failure rolls back to the last saved state. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/debate/DebateContextControl.tsx | 24 ++++++++-- src/lib/stores/debate-context-store.ts | 20 +++++--- src/lib/stores/session-store.ts | 15 +++--- src/lib/stores/sparring-store.ts | 9 ++-- src/test/debate/debate-context.test.tsx | 48 ++++++++++++++++--- 5 files changed, 89 insertions(+), 27 deletions(-) diff --git a/src/components/features/debate/DebateContextControl.tsx b/src/components/features/debate/DebateContextControl.tsx index c5973cbcb..19af42a1b 100644 --- a/src/components/features/debate/DebateContextControl.tsx +++ b/src/components/features/debate/DebateContextControl.tsx @@ -45,9 +45,11 @@ export function DebateContextControl({ feature, side, includeSources = true, deb const plan = useDebateContextPlan(feature, side, debateId, { includeSources }); const pinsLoaded = useDebateContextStore((state) => (debateId ? state.pinsLoaded[debateId] : true)); // The pin limit discounts pins on results that are not listed. Until saved - // results have loaded, a missing result may just not be here yet. - const sessionHydrated = useSessionStore((state) => state.serverHydrated); + // results have loaded, a missing result may just not be here yet; if a list + // failed to load, every pin counts. + const serverHydration = useSessionStore((state) => state.serverHydration); const sparringHydrated = useSparringStore((state) => state.hydrated); + const sparringComplete = useSparringStore((state) => state.historyComplete); const loadPins = useDebateContextStore((state) => state.loadPins); const usage = useDebateContextStore((state) => state.lastUsage[feature]); const [open, setOpen] = useState(false); @@ -105,14 +107,26 @@ export function DebateContextControl({ feature, side, includeSources = true, deb - + ); } -function DebateContextReview({ plan, motion, usage, pinsReady }: { plan: DebateContextPlan; motion?: string; usage?: DebateContextUsage; pinsReady: boolean }) { +function DebateContextReview({ plan, motion, usage, pinsReady, resultsComplete }: { + plan: DebateContextPlan; + motion?: string; + usage?: DebateContextUsage; + pinsReady: boolean; + resultsComplete: boolean; +}) { const { t } = useTranslation(); const setSourceChoice = useDebateContextStore((state) => state.setSourceChoice); const setOutputChoice = useDebateContextStore((state) => state.setOutputChoice); @@ -122,7 +136,7 @@ function DebateContextReview({ plan, motion, usage, pinsReady }: { plan: DebateC const onPin = async (output: ContextOutputItem) => { try { - await togglePin(plan.debateId, output.ref, plan.resultKeys); + await togglePin(plan.debateId, output.ref, resultsComplete ? plan.resultKeys : undefined); } catch (error) { toast({ title: error instanceof Error && error.message === 'pin_limit' diff --git a/src/lib/stores/debate-context-store.ts b/src/lib/stores/debate-context-store.ts index cea87755d..803c1ef21 100644 --- a/src/lib/stores/debate-context-store.ts +++ b/src/lib/stores/debate-context-store.ts @@ -55,7 +55,9 @@ function ensureOverride(current: DebateContextOverride | undefined, debateId: st // Every local pin change, per debate and in order. A load replays the changes // it may have missed: ones made after it started, and ones still saving. -interface PinChange { seq: number; ref: DebateOutputRef; pinned: boolean; saving: boolean } +// `before` is the result's state before this change; if the change fails, the +// result goes back to it. +interface PinChange { seq: number; ref: DebateOutputRef; pinned: boolean; before: boolean; saving: boolean } const pinChanges: Record = {}; let pinSeq = 0; // Each debate's pin writes run one at a time, so the database ends in the @@ -109,7 +111,7 @@ export const useDebateContextStore = create()((set, get) => const counted = resultKeys ? current.filter((item) => resultKeys.has(outputKey(item))) : current; if (!pinned && counted.length >= MAX_PINNED_OUTPUTS) throw new Error('pin_limit'); const generation = pinGeneration; - const change: PinChange = { seq: ++pinSeq, ref, pinned: !pinned, saving: true }; + const change: PinChange = { seq: ++pinSeq, ref, pinned: !pinned, before: pinned, saving: true }; (pinChanges[debateId] ??= []).push(change); set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, !pinned) } })); const save = async (): Promise<{ error: { message?: string; code?: string } | null }> => { @@ -130,15 +132,19 @@ export const useDebateContextStore = create()((set, get) => change.saving = false; if (generation !== pinGeneration) return; // The row already exists, so the result is pinned as asked. - if (!pinned && result.error?.code === '23505') return; + if (change.pinned && result.error?.code === '23505') return; if (result.error) { // Undo only this change; other pins may have changed in the meantime. A // later change to the same result is queued behind this one and decides - // its final state, so this failure no longer matters. - const superseded = pinChanges[debateId].some((item) => item.seq > change.seq && outputKey(item.ref) === outputKey(ref)); + // its final state. It assumed this change was saved, so if it fails too + // it must go back to the state before this one. + const next = pinChanges[debateId].find((item) => item.seq > change.seq && outputKey(item.ref) === outputKey(ref)); pinChanges[debateId] = pinChanges[debateId].filter((item) => item !== change); - if (superseded) return; - set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, pinned) } })); + if (next) { + next.before = change.before; + return; + } + set((state) => ({ pins: { ...state.pins, [debateId]: withPin(state.pins[debateId] ?? [], ref, change.before) } })); throw new Error(result.error.message ?? 'Could not save the pin'); } }, diff --git a/src/lib/stores/session-store.ts b/src/lib/stores/session-store.ts index 30c245c5b..436398935 100644 --- a/src/lib/stores/session-store.ts +++ b/src/lib/stores/session-store.ts @@ -217,8 +217,11 @@ export interface SessionState { clearLibrary: () => Promise; hydrateFromServer: () => Promise; - /** True once this page load's server hydration has finished. Not persisted. */ - serverHydrated: boolean; + /** + * This page load's server hydration: `partial` when a saved-result list + * failed to load and may be missing results. Not persisted. + */ + serverHydration: 'pending' | 'partial' | 'complete'; clearAllData: () => Promise; resetLocalData: () => void; clearSession: () => void; @@ -486,7 +489,7 @@ export const useSessionStore = create()( setFactoidsForm: (data) => set((state) => ({ factoidsForm: mergeFormState(state.factoidsForm, data) })), pendingFlowchartData: null as FlowchartData | null, - serverHydrated: false, + serverHydration: 'pending', setPendingFlowchartData: (data) => set({ pendingFlowchartData: data }), addSource: async (source) => { @@ -1176,7 +1179,7 @@ export const useSessionStore = create()( savedFlowcharts: flowchartsData ? serverFlowcharts : state.savedFlowcharts, sourceFolders: mergeSourceFolders([...(sourceFoldersData ? [] : state.sourceFolders), ...serverSourceFolders], sourcesData ? serverSources : state.sourceLibrary), sourceLibrary: sourcesData ? serverSources : state.sourceLibrary, - serverHydrated: true, + serverHydration: casesData && attacksData && brainstormsData && factoidsData && chatsData && crossfiresData ? 'complete' : 'partial', })); }, @@ -1211,7 +1214,7 @@ export const useSessionStore = create()( }); }, resetLocalData: () => set({ - serverHydrated: false, + serverHydration: 'pending', savedCases: [], savedAttacks: [], savedBrainstorms: [], @@ -1278,7 +1281,7 @@ export const useSessionStore = create()( askLoading, askStreamingAssistant, pendingFlowchartData, - serverHydrated, + serverHydration, ...persisted }) => persisted, } diff --git a/src/lib/stores/sparring-store.ts b/src/lib/stores/sparring-store.ts index 045581514..1ecf0dcf9 100644 --- a/src/lib/stores/sparring-store.ts +++ b/src/lib/stores/sparring-store.ts @@ -55,6 +55,8 @@ interface SparringStoreState { privateSessionBackup: SparringSession | null; savedSessions: SparringSession[]; hydrated: boolean; + /** False until saved history has loaded successfully; hydration can finish without it. */ + historyComplete: boolean; beginWorkspaceSession: () => void; endWorkspaceSession: () => void; startSession: (input: StartSessionInput) => SparringSession; @@ -300,6 +302,7 @@ export const useSparringStore = create()( privateSessionBackup: null as SparringSession | null, savedSessions: [] as SparringSession[], hydrated: false, + historyComplete: false, beginWorkspaceSession: () => set((state) => ({ currentSession: null, @@ -462,7 +465,7 @@ export const useSparringStore = create()( prepareForUser: (userId) => { if (get().ownerUserId === userId) return; hydrationEpoch += 1; - set({ ownerUserId: userId, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false }); + set({ ownerUserId: userId, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false, historyComplete: false }); }, hydrateSavedSessions: async () => { @@ -484,7 +487,7 @@ export const useSparringStore = create()( set({ hydrated: true }); return; } - set((state) => ({ savedSessions: mergeSessions(state.savedSessions, parseSavedRows(data)), hydrated: true })); + set((state) => ({ savedSessions: mergeSessions(state.savedSessions, parseSavedRows(data)), hydrated: true, historyComplete: true })); }, loadSavedSession: (id) => { @@ -511,7 +514,7 @@ export const useSparringStore = create()( }, resetLocalData: () => { hydrationEpoch += 1; - set({ ownerUserId: null, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false }); + set({ ownerUserId: null, currentSession: null, privateSessionBackup: null, savedSessions: [], hydrated: false, historyComplete: false }); }, }), { diff --git a/src/test/debate/debate-context.test.tsx b/src/test/debate/debate-context.test.tsx index 0afa4361e..1e5eb0d33 100644 --- a/src/test/debate/debate-context.test.tsx +++ b/src/test/debate/debate-context.test.tsx @@ -118,9 +118,9 @@ describe('Current Debate context in requests and the Review control', () => { sourceLibrary: [source(1), source(2, { isActive: false })], savedCases: [result(10, caseData) as SavedItem], savedAttacks: [], savedBrainstorms: [], savedFactoids: [], savedChats: [], savedCrossfires: [], - serverHydrated: true, + serverHydration: 'complete', }); - useSparringStore.setState({ hydrated: true }); + useSparringStore.setState({ hydrated: true, historyComplete: true }); }); afterEach(() => { resetDebates(); @@ -195,8 +195,8 @@ describe('Current Debate context in requests and the Review control', () => { it('keeps pin buttons disabled until saved results have loaded', async () => { // Pins on results that have not loaded yet cannot be counted toward the limit. seedCurrentDebate(); - useSessionStore.setState({ serverHydrated: false }); - useSparringStore.setState({ hydrated: false }); + useSessionStore.setState({ serverHydration: 'pending' }); + useSparringStore.setState({ hydrated: false, historyComplete: false }); render(); fireEvent.click(within(screen.getByTestId('debate-context-control')).getByRole('button', { name: 'Review' })); const sheet = await screen.findByRole('dialog'); @@ -204,11 +204,33 @@ describe('Current Debate context in requests and the Review control', () => { const pin = within(sheet).getByRole('button', { name: /Pin Result 10/ }); expect(pin).toBeDisabled(); - act(() => useSessionStore.setState({ serverHydrated: true })); + act(() => useSessionStore.setState({ serverHydration: 'complete' })); expect(pin).toBeDisabled(); - act(() => useSparringStore.setState({ hydrated: true })); + act(() => useSparringStore.setState({ hydrated: true, historyComplete: true })); expect(pin).toBeEnabled(); }); + + it.each([ + ['a saved-result query', () => useSessionStore.setState({ serverHydration: 'partial' })], + ['the sparring history query', () => useSparringStore.setState({ historyComplete: false })], + ])('counts every pin toward the limit when %s failed', async (_name, failHydration) => { + // A result missing because its list failed to load may still exist. + seedCurrentDebate(); + failHydration(); + render(); + fireEvent.click(within(screen.getByTestId('debate-context-control')).getByRole('button', { name: 'Review' })); + const sheet = await screen.findByRole('dialog'); + await waitFor(() => expect(useDebateContextStore.getState().pinsLoaded[TEST_DEBATE_ID]).toBe(true)); + const unlisted = [40, 41, 42].map((n) => ({ kind: 'case' as const, id: uuid(n) })); + act(() => useDebateContextStore.setState((state) => ({ pins: { ...state.pins, [TEST_DEBATE_ID]: unlisted } }))); + + const pin = within(sheet).getByRole('button', { name: /Pin Result 10/ }); + expect(pin).toBeEnabled(); + fireEvent.click(pin); + await new Promise((done) => setTimeout(done, 0)); + + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual(unlisted); + }); }); describe('Pinned results', () => { @@ -315,6 +337,20 @@ describe('Pinned results', () => { expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([caseRef]); }); + it('ends unpinned when a quick pin and unpin both fail to save', async () => { + const deleted = { eq: () => deleted, then: (done: (value: { error: { message: string } }) => void) => done({ error: { message: 'offline' } }) }; + supabaseMock.from + .mockImplementationOnce((() => ({ insert: () => Promise.resolve({ error: { message: 'offline' } }) })) as never) + .mockImplementationOnce((() => ({ delete: () => deleted })) as never); + + const pinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + const unpinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + await Promise.allSettled([pinning, unpinning]); + + // Neither save reached the database, so the result was never pinned. + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([]); + }); + it('treats pinning a result that is already saved as pinned', async () => { const deleted = { eq: () => deleted, then: (done: (value: { error: { message: string } }) => void) => done({ error: { message: 'offline' } }) }; supabaseMock.from From bf24d44687d84ceaaecf62df3a72041736fb50bc Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 30 Sep 2026 14:22:16 +0000 Subject: [PATCH 7/7] fix(debate): keep a pin saved while an older load was in flight A pin load reads the table and answers later. If a pin was still saving when the load read the table, but finished before the load answered, the load's older rows replaced it and the pin vanished from the screen. A load now also replays every change that was still saving when it started. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/lib/stores/debate-context-store.ts | 6 ++++-- src/test/debate/debate-context.test.tsx | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/lib/stores/debate-context-store.ts b/src/lib/stores/debate-context-store.ts index 803c1ef21..ea743827d 100644 --- a/src/lib/stores/debate-context-store.ts +++ b/src/lib/stores/debate-context-store.ts @@ -54,7 +54,8 @@ function ensureOverride(current: DebateContextOverride | undefined, debateId: st } // Every local pin change, per debate and in order. A load replays the changes -// it may have missed: ones made after it started, and ones still saving. +// it may have missed: ones made after it started, and ones that were still +// saving at any point during the load, because its read may predate them. // `before` is the result's state before this change; if the change fails, the // result goes back to it. interface PinChange { seq: number; ref: DebateOutputRef; pinned: boolean; before: boolean; saving: boolean } @@ -81,6 +82,7 @@ export const useDebateContextStore = create()((set, get) => loadPins: async (debateId) => { const startedAt = pinSeq; const generation = pinGeneration; + const savingAtStart = new Set((pinChanges[debateId] ?? []).filter((change) => change.saving)); const { data, error } = await supabase .from('debate_context_pins') .select('item_kind,item_id') @@ -97,7 +99,7 @@ export const useDebateContextStore = create()((set, get) => ? [{ kind: row.item_kind as DebateContextOutputKind, id: row.item_id }] : []); for (const change of pinChanges[debateId] ?? []) { - if (change.seq > startedAt || change.saving) pins = withPin(pins, change.ref, change.pinned); + if (change.seq > startedAt || change.saving || savingAtStart.has(change)) pins = withPin(pins, change.ref, change.pinned); } set((state) => ({ pins: { ...state.pins, [debateId]: pins }, diff --git a/src/test/debate/debate-context.test.tsx b/src/test/debate/debate-context.test.tsx index 1e5eb0d33..b10e29126 100644 --- a/src/test/debate/debate-context.test.tsx +++ b/src/test/debate/debate-context.test.tsx @@ -288,6 +288,25 @@ describe('Pinned results', () => { expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([caseRef, attackRef]); }); + it('keeps a pin that finished saving while an older load was in flight', async () => { + const insert = deferred<{ error: null }>(); + const load = deferred<{ data: unknown; error: null }>(); + supabaseMock.from + .mockImplementationOnce((() => ({ insert: () => insert.promise })) as never) + .mockImplementationOnce((() => ({ select: () => ({ eq: () => load.promise }) })) as never); + + const pinning = useDebateContextStore.getState().togglePin(TEST_DEBATE_ID, caseRef); + await new Promise((done) => setTimeout(done, 0)); + // The load reads the table before the insert commits, then answers after it. + const loading = useDebateContextStore.getState().loadPins(TEST_DEBATE_ID); + insert.resolve({ error: null }); + await pinning; + load.resolve({ data: [], error: null }); + await loading; + + expect(useDebateContextStore.getState().pins[TEST_DEBATE_ID]).toEqual([caseRef]); + }); + it('saves a quick pin and unpin of the same result in the order they were made', async () => { const insert = deferred<{ error: null }>(); const calls: string[] = [];