fix(persistence): address PR review feedback (4 low-cost improvements)
From PR #114 external review agent — adopted the real, low-cost findings; remaining items (false positives / design trade-offs) explained in PR replies: - coerceEpoch: !Number.isNaN → Number.isFinite — reject ±Infinity, which previously slipped through and produced Invalid Date via new Date(Infinity) - enforceRetentionCap pass2/pass3: decrement overflow only when idbDelete actually succeeds — a failed best-effort delete no longer under-evicts - cloudListStories: explicit column list instead of select() — avoid pulling the bulky session_jsonb when only metadata is needed - Supabase stories: composite primary key (user_id, id) + onConflict user_id,id — avoid a cross-user Session.id collision rejecting the second user's save (skeleton not yet deployed, so the migration is edited in place) typecheck + build:cf green.
This commit is contained in:
@@ -122,7 +122,7 @@ export async function cloudSaveStory(
|
|||||||
deleted_at: null,
|
deleted_at: null,
|
||||||
session_jsonb: blob.session,
|
session_jsonb: blob.session,
|
||||||
},
|
},
|
||||||
{ onConflict: "id" },
|
{ onConflict: "user_id,id" },
|
||||||
)
|
)
|
||||||
.select()
|
.select()
|
||||||
.single();
|
.single();
|
||||||
@@ -163,9 +163,13 @@ export async function cloudListStories(): Promise<StoryMeta[]> {
|
|||||||
if (!userId) return [];
|
if (!userId) return [];
|
||||||
try {
|
try {
|
||||||
const supabase = await createClient();
|
const supabase = await createClient();
|
||||||
|
// Explicit column list (not select()) so the list query doesn't pull the
|
||||||
|
// bulky session_jsonb — rowToMeta only needs the denormalized metadata.
|
||||||
const { data, error } = await supabase
|
const { data, error } = await supabase
|
||||||
.from("stories")
|
.from("stories")
|
||||||
.select()
|
.select(
|
||||||
|
"id, world_setting, style_guide, orientation, scene_count, created_at, updated_at",
|
||||||
|
)
|
||||||
.eq("user_id", userId)
|
.eq("user_id", userId)
|
||||||
.is("deleted_at", null)
|
.is("deleted_at", null)
|
||||||
.order("updated_at", { ascending: false });
|
.order("updated_at", { ascending: false });
|
||||||
|
|||||||
@@ -82,8 +82,9 @@ async function enforceRetentionCap(): Promise<void> {
|
|||||||
// Keep records that still owe the cloud a push (pending edits/soft-deletes
|
// Keep records that still owe the cloud a push (pending edits/soft-deletes
|
||||||
// or a synced baseline) — hard-deleting them would lose that work silently.
|
// or a synced baseline) — hard-deleting them would lose that work silently.
|
||||||
if (r.syncState !== "local-only") continue;
|
if (r.syncState !== "local-only") continue;
|
||||||
await idbDelete(STORIES_STORE, r.id);
|
// Only count a slot freed when the delete actually succeeded — a failed
|
||||||
overflow--;
|
// best-effort delete must not decrement overflow (would under-evict).
|
||||||
|
if (await idbDelete(STORIES_STORE, r.id)) overflow--;
|
||||||
}
|
}
|
||||||
|
|
||||||
// 3. Last-resort: if step 2 couldn't reach the cap, every remaining over-cap
|
// 3. Last-resort: if step 2 couldn't reach the cap, every remaining over-cap
|
||||||
@@ -101,8 +102,7 @@ async function enforceRetentionCap(): Promise<void> {
|
|||||||
for (const r of live) {
|
for (const r of live) {
|
||||||
if (overflow <= 0) break;
|
if (overflow <= 0) break;
|
||||||
if (r.syncState === "local-only") continue; // already evicted in step 2
|
if (r.syncState === "local-only") continue; // already evicted in step 2
|
||||||
await idbDelete(STORIES_STORE, r.id);
|
if (await idbDelete(STORIES_STORE, r.id)) overflow--;
|
||||||
overflow--;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
@@ -19,7 +19,9 @@ export const STORY_SCHEMA_VERSION = 1;
|
|||||||
* crosses a storage/serialization boundary and could arrive as a non-number,
|
* crosses a storage/serialization boundary and could arrive as a non-number,
|
||||||
* guarding against the historical `t.getTime is not a function` white-screen. */
|
* guarding against the historical `t.getTime is not a function` white-screen. */
|
||||||
export function coerceEpoch(value: unknown, fallback: number): number {
|
export function coerceEpoch(value: unknown, fallback: number): number {
|
||||||
if (typeof value === "number" && !Number.isNaN(value)) return value;
|
// Number.isFinite (not just !isNaN) so ±Infinity also falls through to the
|
||||||
|
// fallback — new Date(Infinity).getTime() is NaN, not a usable epoch.
|
||||||
|
if (typeof value === "number" && Number.isFinite(value)) return value;
|
||||||
const d = value instanceof Date ? value : new Date(value as string | number);
|
const d = value instanceof Date ? value : new Date(value as string | number);
|
||||||
const t = d.getTime();
|
const t = d.getTime();
|
||||||
return Number.isNaN(t) ? fallback : t;
|
return Number.isNaN(t) ? fallback : t;
|
||||||
|
|||||||
@@ -16,7 +16,7 @@
|
|||||||
-- dropped-then-created (Postgres has no `create policy if not exists`).
|
-- dropped-then-created (Postgres has no `create policy if not exists`).
|
||||||
|
|
||||||
create table if not exists public.stories (
|
create table if not exists public.stories (
|
||||||
id text primary key, -- = Session.id ("s_xxx"), shared with the local record
|
id text not null, -- = Session.id ("s_xxx"), unique only per user
|
||||||
user_id uuid not null references auth.users (id) on delete cascade,
|
user_id uuid not null references auth.users (id) on delete cascade,
|
||||||
world_setting text not null default '',
|
world_setting text not null default '',
|
||||||
style_guide text not null default '',
|
style_guide text not null default '',
|
||||||
@@ -26,7 +26,11 @@ create table if not exists public.stories (
|
|||||||
created_at timestamptz not null default now(),
|
created_at timestamptz not null default now(),
|
||||||
updated_at timestamptz not null default now(),
|
updated_at timestamptz not null default now(),
|
||||||
deleted_at timestamptz, -- soft-delete tombstone; null = live
|
deleted_at timestamptz, -- soft-delete tombstone; null = live
|
||||||
session_jsonb jsonb not null -- slim Session blob (voice + styleReferenceImage stripped)
|
session_jsonb jsonb not null, -- slim Session blob (voice + styleReferenceImage stripped)
|
||||||
|
-- Composite PK: a random Session.id ("s_xxx") is unique only within a user, so
|
||||||
|
-- scope the key by user_id — otherwise a cross-user id collision would reject
|
||||||
|
-- the second user's save with a PK violation.
|
||||||
|
primary key (user_id, id)
|
||||||
);
|
);
|
||||||
|
|
||||||
-- List query path: a user's stories newest-first.
|
-- List query path: a user's stories newest-first.
|
||||||
|
|||||||
Reference in New Issue
Block a user