From 977708891ed79393b301531d13eb8af4cd009f1d Mon Sep 17 00:00:00 2001 From: calebyhan Date: Mon, 13 Apr 2026 13:19:31 -0400 Subject: [PATCH 01/10] fix: atomic dataset unlink via RPC, resolve dedup callbacks, guard metadata fetch errors, harden null count check --- backend/database/schema.sql | 41 +++++++- frontend/hooks/useDatasetManager.ts | 120 +++++++++++++++++++++--- frontend/lib/services/datasetService.ts | 102 ++++++++++++++------ 3 files changed, 220 insertions(+), 43 deletions(-) diff --git a/backend/database/schema.sql b/backend/database/schema.sql index b4cc5d0..721ec84 100644 --- a/backend/database/schema.sql +++ b/backend/database/schema.sql @@ -526,4 +526,43 @@ CREATE POLICY "canvas_elements_service_role" ON canvas_elements -- ============================================================ -- Enable Realtime on canvas_datasets so clients receive live dataset-linked events -ALTER PUBLICATION supabase_realtime ADD TABLE canvas_datasets; \ No newline at end of file +ALTER PUBLICATION supabase_realtime ADD TABLE canvas_datasets; + +-- ============================================================ +-- unlink_dataset_from_canvas: atomic unlink + conditional delete +-- ============================================================ +-- Removes a dataset from a specific canvas, then hard-deletes the datasets row +-- if no other canvases still reference it. Both operations happen inside a +-- single PL/pgSQL transaction, eliminating the TOCTOU race that exists when +-- the two steps are performed as separate round-trips from the client. +-- +-- SECURITY DEFINER is required so that: +-- 1. The canvas_datasets DELETE can check canvas edit-access via the RLS helper. +-- 2. The datasets DELETE is not blocked by the RLS policy "Users can delete +-- their own datasets" when the remover is a collaborator rather than the +-- dataset owner — the cleanup is safe because no canvas references remain. +DROP FUNCTION IF EXISTS unlink_dataset_from_canvas(UUID, UUID); +CREATE OR REPLACE FUNCTION unlink_dataset_from_canvas( + p_dataset_id UUID, + p_canvas_id UUID +) RETURNS void +LANGUAGE plpgsql +SECURITY DEFINER +AS $$ +BEGIN + -- Step 1: remove the specific canvas link. + DELETE FROM canvas_datasets + WHERE dataset_id = p_dataset_id + AND canvas_id = p_canvas_id; + + -- Step 2: delete the dataset row only when no other canvas still references it. + -- The NOT EXISTS subquery and the DELETE are evaluated atomically within this + -- transaction, preventing a concurrent linkDatasetToCanvas from racing past the + -- check before the delete fires. + DELETE FROM datasets + WHERE id = p_dataset_id + AND NOT EXISTS ( + SELECT 1 FROM canvas_datasets WHERE dataset_id = p_dataset_id + ); +END; +$$; \ No newline at end of file diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index bfea3c0..5f1cd2d 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -126,28 +126,29 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { } try { - // Check if a dataset with this filename already exists to prevent duplicates - // Use canvas-scoped datasets when on a canvas so collaborators see each other's uploads + // Check if a dataset with this filename already exists to prevent duplicates. + // Use canvas-scoped datasets when on a canvas so collaborators see each other's uploads. const existingDatasets = canvasId ? await DatasetService.getCanvasDatasets(canvasId) : await DatasetService.getUserDatasets(userId); - const duplicateExists = existingDatasets.some(d => d.filename === file.name); - if (duplicateExists) { + const duplicateInCanvas = existingDatasets.find(d => d.filename === file.name); + if (duplicateInCanvas) { // Clear in-progress flag before early return so the same file can be re-uploaded later if (typeof window !== 'undefined') { delete (window as any)[inProgressKey]; } - // Don't throw error, just return existing dataset to prevent UI issues - const existing = existingDatasets.find(d => d.filename === file.name)!; - const metadata = (existing.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; + // Resolve progress/status callbacks so callers don't hang in a loading state + onStatusChange?.('completed'); + onProgress?.(100); + const metadata = (duplicateInCanvas.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; return { - id: existing.id, + id: duplicateInCanvas.id, name: file.name.replace(/\.[^/.]+$/, ''), type: 'csv', columns: metadata.columns ?? 0, rows: metadata.rows ?? 0, - size: formatFileSize(existing.file_size), - lastModified: formatDate(existing.updated_at), + size: formatFileSize(duplicateInCanvas.file_size), + lastModified: formatDate(duplicateInCanvas.updated_at), dataTypes: metadata.dataTypes ?? { numerical: 0, categorical: 0, @@ -156,10 +157,64 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { }, preview: metadata.preview ?? [], data: Array.isArray(metadata.sample_data) ? metadata.sample_data : [], - processingStatus: existing.processing_status as ProcessingStatus + processingStatus: duplicateInCanvas.processing_status as ProcessingStatus }; } + // When on a canvas with an authenticated user, check if this file already exists globally + // (uploaded to a different canvas). Match on both filename AND file size to avoid silently + // returning the wrong dataset when the user re-uses a filename for different data. + // Only reuse a dataset that has completed processing — failed/pending ones should be + // re-created so the user isn't stuck with a broken import. + // Skip this check in dev mode (userId null): getUserDatasets(null) returns all rows + // where user_id IS NULL, which could include datasets created by other anonymous + // sessions on the same instance. Without a user identity there is no safe way to + // determine ownership, so always create a fresh row instead. + if (canvasId && userId) { + // Wrap in try/catch so a transient fetch failure here degrades gracefully + // (skip the global dedup, proceed to upload) rather than aborting the entire + // upload with a confusing error message. + let userDatasets: Tables<'datasets'>[] = []; + try { + userDatasets = await DatasetService.getUserDatasets(userId); + } catch (dedupeError) { + console.error('useDatasetManager: global dedup check failed, proceeding with upload:', dedupeError); + } + const globalDuplicate = userDatasets.find( + d => d.filename === file.name && + d.file_size === file.size && + d.processing_status === 'completed' + ); + if (globalDuplicate) { + if (typeof window !== 'undefined') { + delete (window as any)[inProgressKey]; + } + await DatasetService.linkDatasetToCanvas(globalDuplicate.id, canvasId); + // Resolve progress/status callbacks so callers don't hang in a loading state + onStatusChange?.('completed'); + onProgress?.(100); + const metadata = (globalDuplicate.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; + return { + id: globalDuplicate.id, + name: file.name.replace(/\.[^/.]+$/, ''), + type: 'csv', + columns: metadata.columns ?? 0, + rows: metadata.rows ?? 0, + size: formatFileSize(globalDuplicate.file_size), + lastModified: formatDate(globalDuplicate.updated_at), + dataTypes: metadata.dataTypes ?? { + numerical: 0, + categorical: 0, + temporal: 0, + geographic: 0 + }, + preview: metadata.preview ?? [], + data: Array.isArray(metadata.sample_data) ? metadata.sample_data : [], + processingStatus: globalDuplicate.processing_status as ProcessingStatus + }; + } + } + // Step 1: Create dataset with pending status onStatusChange?.('pending'); onProgress?.(10); @@ -310,6 +365,33 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { const userId = user?.id || null; + // Canvas-scoped dedup: if a completed dataset with this filename is already linked + // to the current canvas, return it rather than re-running analysis and creating a + // duplicate datasets row. Requiring 'completed' prevents returning a broken import + // (failed/pending) when the user re-uploads to fix a previous failure. + if (canvasId && filename) { + const canvasDatasets = await DatasetService.getCanvasDatasets(canvasId); + const existing = canvasDatasets.find( + d => d.filename === filename && d.processing_status === 'completed' + ); + if (existing) { + const metadata = (existing.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; + return { + id: existing.id, + name: filename.replace(/\.[^/.]+$/, ''), + type: 'csv', + columns: metadata.columns ?? 0, + rows: metadata.rows ?? 0, + size: formatFileSize(existing.file_size), + lastModified: formatDate(existing.updated_at), + dataTypes: metadata.dataTypes ?? { numerical: 0, categorical: 0, temporal: 0, geographic: 0 }, + preview: metadata.preview ?? [], + data: Array.isArray(metadata.sample_data) ? metadata.sample_data : [], + processingStatus: existing.processing_status as ProcessingStatus, + }; + } + } + // Analyze the data const dataAnalysis = analyzeData(rawData); const dataSize = new Blob([JSON.stringify(rawData)]).size; @@ -398,7 +480,13 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { const removeDatasetMutation = useMutation({ mutationFn: async (datasetId: string) => { const userId = user?.id || null; - await DatasetService.deleteDataset(datasetId, userId); + // When on a canvas, unlink from this canvas only. Hard-delete only if no other + // canvases reference the dataset — avoids wiping a shared dataset from sibling canvases. + if (canvasId) { + await DatasetService.removeDatasetFromCanvas(datasetId, canvasId, userId); + } else { + await DatasetService.deleteDataset(datasetId, userId); + } return datasetId; }, onSuccess: (deletedId) => { @@ -459,7 +547,13 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { queryClient.invalidateQueries({ queryKey: ['datasets', 'canvas', canvasId] }); } ) - .subscribe(); + .subscribe((status, err) => { + if (status === 'CHANNEL_ERROR' || status === 'TIMED_OUT') { + // Don't throw — the 30 s polling fallback (refetchInterval) handles live updates. + // Log so the failure is observable without requiring a Supabase dashboard check. + console.error(`canvas_datasets realtime subscription failed (${status}):`, err); + } + }); return () => { supabase.removeChannel(channel); }; }, [canvasId, queryClient]); diff --git a/frontend/lib/services/datasetService.ts b/frontend/lib/services/datasetService.ts index b468958..60360b8 100644 --- a/frontend/lib/services/datasetService.ts +++ b/frontend/lib/services/datasetService.ts @@ -39,8 +39,8 @@ export interface DatasetMetadata { export class DatasetService { /** - * Create a new dataset with pending status (supports null userId for dev mode) - * Includes duplicate prevention by checking for existing datasets with the same filename + * Create a new dataset with pending status (supports null userId for dev mode). + * Callers are responsible for deduplication before calling this method. */ static async createDataset(params: { userId: string | null; @@ -51,30 +51,6 @@ export class DatasetService { }): Promise> { const { userId, filename, fileSize, fileType, initialMetadata = {} } = params; - // First, check if a dataset with this filename already exists for this user - let existingQuery = supabase - .from('datasets') - .select('id, filename') - .eq('filename', filename); - - if (userId === null) { - existingQuery = existingQuery.is('user_id', null); - } else { - existingQuery = existingQuery.eq('user_id', userId); - } - - const { data: existing, error: checkError } = await existingQuery.maybeSingle(); - - // Only throw if we actually found a duplicate (ignore query errors for now) - if (existing && !checkError) { - console.log('Dataset with filename already exists:', filename, 'ID:', existing.id); - throw new Error(`Dataset with filename "${filename}" already exists`); - } - - if (checkError) { - console.warn('Warning: Could not check for duplicates:', checkError.message); - } - const datasetInsert: TablesInsert<'datasets'> = { user_id: userId, filename, @@ -121,11 +97,15 @@ export class DatasetService { // Update metadata with processing info — always read-and-merge to avoid clobbering file_info if (status === 'processing') { - const { data: currentDataset } = await supabase + const { data: currentDataset, error: fetchError } = await supabase .from('datasets') .select('metadata') .eq('id', datasetId) .single(); + if (fetchError) { + console.error('updateProcessingStatus: failed to read metadata before marking processing:', fetchError); + throw fetchError; + } const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; updateData.metadata = { ...currentMetadata, @@ -136,11 +116,15 @@ export class DatasetService { }; } else if (status === 'completed' || status === 'failed') { // Get current metadata to preserve existing data - const { data: currentDataset } = await supabase + const { data: currentDataset, error: fetchError } = await supabase .from('datasets') .select('metadata') .eq('id', datasetId) .single(); + if (fetchError) { + console.error(`updateProcessingStatus: failed to read metadata before marking ${status}:`, fetchError); + throw fetchError; + } const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; const processingInfo = currentMetadata.processing_info || {}; @@ -342,7 +326,67 @@ export class DatasetService { } /** - * Delete dataset and all related data (supports null userId for dev mode) + * Unlink a dataset from a specific canvas without deleting the dataset itself. + */ + static async unlinkDatasetFromCanvas(datasetId: string, canvasId: string): Promise { + const { error } = await (supabase + .from('canvas_datasets' as any) + .delete() + .eq('dataset_id', datasetId) + .eq('canvas_id', canvasId)) as unknown as { error: { message: string; code: string } | null }; + + if (error) { + console.error('Failed to unlink dataset from canvas:', error); + throw error; + } + } + + /** + * Count how many canvases currently reference a dataset. + */ + static async getCanvasLinksCount(datasetId: string): Promise { + const { count, error } = await (supabase + .from('canvas_datasets' as any) + .select('*', { count: 'exact', head: true }) + .eq('dataset_id', datasetId)) as unknown as { count: number | null; error: { message: string } | null }; + + if (error) { + console.error('Failed to count canvas links for dataset:', error); + throw error; + } + // Treat a null count as an indeterminate result — do not fall back to 0, which + // would cause removeDatasetFromCanvas to hard-delete a potentially referenced dataset. + if (count === null) { + throw new Error(`getCanvasLinksCount: Supabase returned null count for dataset ${datasetId} — aborting to prevent data loss`); + } + return count; + } + + /** + * Remove a dataset from a canvas. If no other canvases reference the datasets row, + * hard-deletes it too. The unlink and conditional delete are performed atomically + * inside a single Postgres transaction via the unlink_dataset_from_canvas RPC, + * preventing the TOCTOU race that would exist across two separate round-trips. + * + * Use this instead of deleteDataset when operating from a canvas context. + */ + static async removeDatasetFromCanvas(datasetId: string, canvasId: string, _userId: string | null): Promise { + const { error } = await supabase.rpc('unlink_dataset_from_canvas' as any, { + p_dataset_id: datasetId, + p_canvas_id: canvasId, + }); + + if (error) { + console.error('Failed to remove dataset from canvas:', error); + throw error; + } + } + + /** + * Delete a datasets row and all related data (supports null userId for dev mode). + * Prefer removeDatasetFromCanvas when operating from a canvas context — it unlinks + * from the specific canvas and only hard-deletes the datasets row when no other + * canvases still reference it (canvas_datasets rows cascade on datasets DELETE). */ static async deleteDataset(datasetId: string, userId: string | null): Promise { let query = supabase From 083826e6074d87128f357772f5437bcc492be2dc Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 15:08:06 -0400 Subject: [PATCH 02/10] fix: harden canvas dataset unlink RPC --- backend/database/schema.sql | 18 ++++++++++++++++-- frontend/hooks/useDatasetManager.ts | 14 +++++++------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/backend/database/schema.sql b/backend/database/schema.sql index 721ec84..1482c42 100644 --- a/backend/database/schema.sql +++ b/backend/database/schema.sql @@ -537,7 +537,7 @@ ALTER PUBLICATION supabase_realtime ADD TABLE canvas_datasets; -- the two steps are performed as separate round-trips from the client. -- -- SECURITY DEFINER is required so that: --- 1. The canvas_datasets DELETE can check canvas edit-access via the RLS helper. +-- 1. The function can enforce canvas edit-access before unlinking. -- 2. The datasets DELETE is not blocked by the RLS policy "Users can delete -- their own datasets" when the remover is a collaborator rather than the -- dataset owner — the cleanup is safe because no canvas references remain. @@ -548,13 +548,24 @@ CREATE OR REPLACE FUNCTION unlink_dataset_from_canvas( ) RETURNS void LANGUAGE plpgsql SECURITY DEFINER +SET search_path = public, pg_catalog AS $$ BEGIN + IF NOT user_has_canvas_edit(p_canvas_id) THEN + RAISE EXCEPTION 'Not authorized to modify this canvas' USING ERRCODE = '42501'; + END IF; + -- Step 1: remove the specific canvas link. DELETE FROM canvas_datasets WHERE dataset_id = p_dataset_id AND canvas_id = p_canvas_id; + -- If the requested link did not exist, stop here. This prevents callers from + -- passing an arbitrary unlinked dataset_id and hard-deleting an orphaned row. + IF NOT FOUND THEN + RETURN; + END IF; + -- Step 2: delete the dataset row only when no other canvas still references it. -- The NOT EXISTS subquery and the DELETE are evaluated atomically within this -- transaction, preventing a concurrent linkDatasetToCanvas from racing past the @@ -565,4 +576,7 @@ BEGIN SELECT 1 FROM canvas_datasets WHERE dataset_id = p_dataset_id ); END; -$$; \ No newline at end of file +$$; + +REVOKE EXECUTE ON FUNCTION unlink_dataset_from_canvas(UUID, UUID) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION unlink_dataset_from_canvas(UUID, UUID) TO authenticated; diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index 5f1cd2d..a4f17fa 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -131,8 +131,8 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { const existingDatasets = canvasId ? await DatasetService.getCanvasDatasets(canvasId) : await DatasetService.getUserDatasets(userId); - const duplicateInCanvas = existingDatasets.find(d => d.filename === file.name); - if (duplicateInCanvas) { + const existingDuplicate = existingDatasets.find(d => d.filename === file.name); + if (existingDuplicate) { // Clear in-progress flag before early return so the same file can be re-uploaded later if (typeof window !== 'undefined') { delete (window as any)[inProgressKey]; @@ -140,15 +140,15 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { // Resolve progress/status callbacks so callers don't hang in a loading state onStatusChange?.('completed'); onProgress?.(100); - const metadata = (duplicateInCanvas.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; + const metadata = (existingDuplicate.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; return { - id: duplicateInCanvas.id, + id: existingDuplicate.id, name: file.name.replace(/\.[^/.]+$/, ''), type: 'csv', columns: metadata.columns ?? 0, rows: metadata.rows ?? 0, - size: formatFileSize(duplicateInCanvas.file_size), - lastModified: formatDate(duplicateInCanvas.updated_at), + size: formatFileSize(existingDuplicate.file_size), + lastModified: formatDate(existingDuplicate.updated_at), dataTypes: metadata.dataTypes ?? { numerical: 0, categorical: 0, @@ -157,7 +157,7 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { }, preview: metadata.preview ?? [], data: Array.isArray(metadata.sample_data) ? metadata.sample_data : [], - processingStatus: duplicateInCanvas.processing_status as ProcessingStatus + processingStatus: existingDuplicate.processing_status as ProcessingStatus }; } From 7237312f4e20e7b795113dac7a5a1e6c2e9cbc66 Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 15:20:07 -0400 Subject: [PATCH 03/10] fix: harden dataset dedupe and unlink cleanup --- backend/database/schema.sql | 16 +++++++++------- frontend/hooks/useDatasetManager.ts | 8 ++++++-- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/backend/database/schema.sql b/backend/database/schema.sql index 1482c42..51d71cc 100644 --- a/backend/database/schema.sql +++ b/backend/database/schema.sql @@ -532,15 +532,15 @@ ALTER PUBLICATION supabase_realtime ADD TABLE canvas_datasets; -- unlink_dataset_from_canvas: atomic unlink + conditional delete -- ============================================================ -- Removes a dataset from a specific canvas, then hard-deletes the datasets row --- if no other canvases still reference it. Both operations happen inside a --- single PL/pgSQL transaction, eliminating the TOCTOU race that exists when --- the two steps are performed as separate round-trips from the client. +-- only when no other canvases still reference it and the caller owns that row +-- (or the row is anonymous/dev-mode data). Both operations happen inside a +-- single PL/pgSQL transaction, eliminating the TOCTOU race that exists when the +-- two steps are performed as separate round-trips from the client. -- -- SECURITY DEFINER is required so that: -- 1. The function can enforce canvas edit-access before unlinking. --- 2. The datasets DELETE is not blocked by the RLS policy "Users can delete --- their own datasets" when the remover is a collaborator rather than the --- dataset owner — the cleanup is safe because no canvas references remain. +-- 2. The function can perform the final cleanup DELETE after explicitly +-- checking ownership and remaining canvas references. DROP FUNCTION IF EXISTS unlink_dataset_from_canvas(UUID, UUID); CREATE OR REPLACE FUNCTION unlink_dataset_from_canvas( p_dataset_id UUID, @@ -566,12 +566,14 @@ BEGIN RETURN; END IF; - -- Step 2: delete the dataset row only when no other canvas still references it. + -- Step 2: delete the dataset row only when no other canvas still references it + -- and the caller is allowed to delete the dataset itself. -- The NOT EXISTS subquery and the DELETE are evaluated atomically within this -- transaction, preventing a concurrent linkDatasetToCanvas from racing past the -- check before the delete fires. DELETE FROM datasets WHERE id = p_dataset_id + AND (user_id = auth.uid() OR user_id IS NULL) AND NOT EXISTS ( SELECT 1 FROM canvas_datasets WHERE dataset_id = p_dataset_id ); diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index a4f17fa..22147e9 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -126,12 +126,16 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { } try { - // Check if a dataset with this filename already exists to prevent duplicates. + // Check if this exact completed dataset already exists to prevent duplicates. // Use canvas-scoped datasets when on a canvas so collaborators see each other's uploads. const existingDatasets = canvasId ? await DatasetService.getCanvasDatasets(canvasId) : await DatasetService.getUserDatasets(userId); - const existingDuplicate = existingDatasets.find(d => d.filename === file.name); + const existingDuplicate = existingDatasets.find( + d => d.filename === file.name && + d.file_size === file.size && + d.processing_status === 'completed' + ); if (existingDuplicate) { // Clear in-progress flag before early return so the same file can be re-uploaded later if (typeof window !== 'undefined') { From d0a407690f21f5cd231712a0e2934b0dc05e86ad Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 15:33:04 -0400 Subject: [PATCH 04/10] fix: handle dataset reuse errors consistently --- frontend/hooks/useDatasetManager.ts | 8 +++++++- frontend/lib/services/datasetService.ts | 16 +++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index 22147e9..81a118c 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -193,7 +193,13 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { if (typeof window !== 'undefined') { delete (window as any)[inProgressKey]; } - await DatasetService.linkDatasetToCanvas(globalDuplicate.id, canvasId); + try { + await DatasetService.linkDatasetToCanvas(globalDuplicate.id, canvasId); + } catch (linkError) { + onStatusChange?.('failed'); + onProgress?.(0); + throw linkError; + } // Resolve progress/status callbacks so callers don't hang in a loading state onStatusChange?.('completed'); onProgress?.(100); diff --git a/frontend/lib/services/datasetService.ts b/frontend/lib/services/datasetService.ts index 60360b8..480c7a0 100644 --- a/frontend/lib/services/datasetService.ts +++ b/frontend/lib/services/datasetService.ts @@ -1,5 +1,6 @@ import { supabase, isSupabaseConfigured } from '@/lib/supabase/client'; import { Tables, TablesInsert, TablesUpdate } from '@/lib/supabase/types'; +import type { PostgrestError } from '@supabase/supabase-js'; export type ProcessingStatus = 'pending' | 'processing' | 'completed' | 'failed'; @@ -37,6 +38,15 @@ export interface DatasetMetadata { }; } +function toPostgrestError(error: PostgrestError): Error & PostgrestError { + return Object.assign(new Error(error.message), { + name: 'PostgrestError', + code: error.code, + details: error.details, + hint: error.hint, + }); +} + export class DatasetService { /** * Create a new dataset with pending status (supports null userId for dev mode). @@ -104,7 +114,7 @@ export class DatasetService { .single(); if (fetchError) { console.error('updateProcessingStatus: failed to read metadata before marking processing:', fetchError); - throw fetchError; + throw toPostgrestError(fetchError); } const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; updateData.metadata = { @@ -123,7 +133,7 @@ export class DatasetService { .single(); if (fetchError) { console.error(`updateProcessingStatus: failed to read metadata before marking ${status}:`, fetchError); - throw fetchError; + throw toPostgrestError(fetchError); } const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; @@ -495,4 +505,4 @@ export class DatasetService { console.log('Dataset metadata updated:', datasetId); } -} \ No newline at end of file +} From f4e852c4b23973073a5c48a59ba9dab88c4ad613 Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 15:41:50 -0400 Subject: [PATCH 05/10] fix: normalize canvas dataset errors --- backend/database/schema.sql | 14 +++++++++++++- frontend/lib/services/datasetService.ts | 25 ++++++++++++++++--------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/backend/database/schema.sql b/backend/database/schema.sql index 51d71cc..1adeccd 100644 --- a/backend/database/schema.sql +++ b/backend/database/schema.sql @@ -526,7 +526,19 @@ CREATE POLICY "canvas_elements_service_role" ON canvas_elements -- ============================================================ -- Enable Realtime on canvas_datasets so clients receive live dataset-linked events -ALTER PUBLICATION supabase_realtime ADD TABLE canvas_datasets; +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM pg_publication_tables + WHERE pubname = 'supabase_realtime' + AND schemaname = 'public' + AND tablename = 'canvas_datasets' + ) THEN + ALTER PUBLICATION supabase_realtime ADD TABLE canvas_datasets; + END IF; +END; +$$; -- ============================================================ -- unlink_dataset_from_canvas: atomic unlink + conditional delete diff --git a/frontend/lib/services/datasetService.ts b/frontend/lib/services/datasetService.ts index 480c7a0..cedd005 100644 --- a/frontend/lib/services/datasetService.ts +++ b/frontend/lib/services/datasetService.ts @@ -38,12 +38,19 @@ export interface DatasetMetadata { }; } -function toPostgrestError(error: PostgrestError): Error & PostgrestError { +type PostgrestLikeError = { + message: string; + code?: string; + details?: string | null; + hint?: string | null; +}; + +function toPostgrestError(error: PostgrestLikeError): Error & PostgrestError { return Object.assign(new Error(error.message), { name: 'PostgrestError', - code: error.code, - details: error.details, - hint: error.hint, + code: error.code ?? 'UNKNOWN_ERROR', + details: error.details ?? '', + hint: error.hint ?? '', }); } @@ -293,7 +300,7 @@ export class DatasetService { if (linkError) { console.error('Failed to fetch canvas_datasets links:', linkError); - throw linkError; + throw toPostgrestError(linkError); } const datasetIds = (links || []).map((r) => r.dataset_id).filter(Boolean); @@ -331,7 +338,7 @@ export class DatasetService { if (error && error.code !== '23505') { // 23505 = unique_violation (already linked) console.error('Failed to link dataset to canvas:', error); - throw error; + throw toPostgrestError(error); } } @@ -347,7 +354,7 @@ export class DatasetService { if (error) { console.error('Failed to unlink dataset from canvas:', error); - throw error; + throw toPostgrestError(error); } } @@ -362,7 +369,7 @@ export class DatasetService { if (error) { console.error('Failed to count canvas links for dataset:', error); - throw error; + throw toPostgrestError(error); } // Treat a null count as an indeterminate result — do not fall back to 0, which // would cause removeDatasetFromCanvas to hard-delete a potentially referenced dataset. @@ -388,7 +395,7 @@ export class DatasetService { if (error) { console.error('Failed to remove dataset from canvas:', error); - throw error; + throw toPostgrestError(error); } } From 1e7e05ca4c35b1d330c7ddaccb6ca342da7c9303 Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 15:57:33 -0400 Subject: [PATCH 06/10] fix: match legacy canvas dedupe by file size --- frontend/hooks/useDatasetManager.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index 81a118c..eeb01cf 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -374,15 +374,18 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { } const userId = user?.id || null; + const dataSize = new Blob([JSON.stringify(rawData)]).size; - // Canvas-scoped dedup: if a completed dataset with this filename is already linked + // Canvas-scoped dedup: if this exact completed dataset is already linked // to the current canvas, return it rather than re-running analysis and creating a // duplicate datasets row. Requiring 'completed' prevents returning a broken import // (failed/pending) when the user re-uploads to fix a previous failure. if (canvasId && filename) { const canvasDatasets = await DatasetService.getCanvasDatasets(canvasId); const existing = canvasDatasets.find( - d => d.filename === filename && d.processing_status === 'completed' + d => d.filename === filename && + d.file_size === dataSize && + d.processing_status === 'completed' ); if (existing) { const metadata = (existing.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; @@ -404,7 +407,6 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { // Analyze the data const dataAnalysis = analyzeData(rawData); - const dataSize = new Blob([JSON.stringify(rawData)]).size; // Create dataset with completed status since we have all the data const dbDataset = await DatasetService.createDataset({ From 97fa15a9bede12d7a98e035acd90a659abe868ef Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 16:04:54 -0400 Subject: [PATCH 07/10] fix: handle anonymous canvas dataset removal --- frontend/hooks/useDatasetManager.ts | 8 ++++---- frontend/lib/services/datasetService.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index eeb01cf..a061306 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -492,10 +492,10 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { const removeDatasetMutation = useMutation({ mutationFn: async (datasetId: string) => { const userId = user?.id || null; - // When on a canvas, unlink from this canvas only. Hard-delete only if no other - // canvases reference the dataset — avoids wiping a shared dataset from sibling canvases. - if (canvasId) { - await DatasetService.removeDatasetFromCanvas(datasetId, canvasId, userId); + // Authenticated canvas users unlink from this canvas only. Dev/anonymous mode + // falls back to user-scoped delete because the unlink RPC requires auth.uid(). + if (canvasId && userId) { + await DatasetService.removeDatasetFromCanvas(datasetId, canvasId); } else { await DatasetService.deleteDataset(datasetId, userId); } diff --git a/frontend/lib/services/datasetService.ts b/frontend/lib/services/datasetService.ts index cedd005..116bb41 100644 --- a/frontend/lib/services/datasetService.ts +++ b/frontend/lib/services/datasetService.ts @@ -387,7 +387,7 @@ export class DatasetService { * * Use this instead of deleteDataset when operating from a canvas context. */ - static async removeDatasetFromCanvas(datasetId: string, canvasId: string, _userId: string | null): Promise { + static async removeDatasetFromCanvas(datasetId: string, canvasId: string): Promise { const { error } = await supabase.rpc('unlink_dataset_from_canvas' as any, { p_dataset_id: datasetId, p_canvas_id: canvasId, From 7f188ea9e52cac90b334b3438cb9d1f7edde3689 Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 16:13:12 -0400 Subject: [PATCH 08/10] fix: make legacy canvas dedupe best effort --- frontend/hooks/useDatasetManager.ts | 46 ++++++++++++++----------- frontend/lib/services/datasetService.ts | 21 ----------- 2 files changed, 25 insertions(+), 42 deletions(-) diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index a061306..74e884c 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -381,27 +381,31 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { // duplicate datasets row. Requiring 'completed' prevents returning a broken import // (failed/pending) when the user re-uploads to fix a previous failure. if (canvasId && filename) { - const canvasDatasets = await DatasetService.getCanvasDatasets(canvasId); - const existing = canvasDatasets.find( - d => d.filename === filename && - d.file_size === dataSize && - d.processing_status === 'completed' - ); - if (existing) { - const metadata = (existing.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; - return { - id: existing.id, - name: filename.replace(/\.[^/.]+$/, ''), - type: 'csv', - columns: metadata.columns ?? 0, - rows: metadata.rows ?? 0, - size: formatFileSize(existing.file_size), - lastModified: formatDate(existing.updated_at), - dataTypes: metadata.dataTypes ?? { numerical: 0, categorical: 0, temporal: 0, geographic: 0 }, - preview: metadata.preview ?? [], - data: Array.isArray(metadata.sample_data) ? metadata.sample_data : [], - processingStatus: existing.processing_status as ProcessingStatus, - }; + try { + const canvasDatasets = await DatasetService.getCanvasDatasets(canvasId); + const existing = canvasDatasets.find( + d => d.filename === filename && + d.file_size === dataSize && + d.processing_status === 'completed' + ); + if (existing) { + const metadata = (existing.metadata as unknown as DatasetMetadata) || {} as DatasetMetadata; + return { + id: existing.id, + name: filename.replace(/\.[^/.]+$/, ''), + type: 'csv', + columns: metadata.columns ?? 0, + rows: metadata.rows ?? 0, + size: formatFileSize(existing.file_size), + lastModified: formatDate(existing.updated_at), + dataTypes: metadata.dataTypes ?? { numerical: 0, categorical: 0, temporal: 0, geographic: 0 }, + preview: metadata.preview ?? [], + data: Array.isArray(metadata.sample_data) ? metadata.sample_data : [], + processingStatus: existing.processing_status as ProcessingStatus, + }; + } + } catch (dedupeError) { + console.error('useDatasetManager: legacy canvas dedup check failed, proceeding with create:', dedupeError); } } diff --git a/frontend/lib/services/datasetService.ts b/frontend/lib/services/datasetService.ts index 116bb41..ede0061 100644 --- a/frontend/lib/services/datasetService.ts +++ b/frontend/lib/services/datasetService.ts @@ -358,27 +358,6 @@ export class DatasetService { } } - /** - * Count how many canvases currently reference a dataset. - */ - static async getCanvasLinksCount(datasetId: string): Promise { - const { count, error } = await (supabase - .from('canvas_datasets' as any) - .select('*', { count: 'exact', head: true }) - .eq('dataset_id', datasetId)) as unknown as { count: number | null; error: { message: string } | null }; - - if (error) { - console.error('Failed to count canvas links for dataset:', error); - throw toPostgrestError(error); - } - // Treat a null count as an indeterminate result — do not fall back to 0, which - // would cause removeDatasetFromCanvas to hard-delete a potentially referenced dataset. - if (count === null) { - throw new Error(`getCanvasLinksCount: Supabase returned null count for dataset ${datasetId} — aborting to prevent data loss`); - } - return count; - } - /** * Remove a dataset from a canvas. If no other canvases reference the datasets row, * hard-deletes it too. The unlink and conditional delete are performed atomically From 51dd481daf29fb300cafb52a69fa98c71e56d7cb Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 16:22:36 -0400 Subject: [PATCH 09/10] fix: optimize dataset dedupe error handling --- frontend/hooks/useDatasetManager.ts | 15 +++++------ frontend/lib/services/datasetService.ts | 34 +++++++++++++++++++++++++ frontend/lib/services/errorHandler.ts | 13 ++++++++-- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index 74e884c..7730152 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -2,7 +2,7 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useCallback, useEffect } from 'react'; import Papa from 'papaparse'; import { isSupabaseConfigured, supabase } from '@/lib/supabase/client'; -import { Tables, TablesInsert } from '@/lib/supabase/types'; +import { Tables } from '@/lib/supabase/types'; import { Dataset } from '@/components/AutoVizAgent'; import { useAuthContext } from '@/components/providers/AuthProvider'; import { DatasetService, DatasetMetadata, ProcessingStatus } from '@/lib/services/datasetService'; @@ -178,17 +178,16 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { // Wrap in try/catch so a transient fetch failure here degrades gracefully // (skip the global dedup, proceed to upload) rather than aborting the entire // upload with a confusing error message. - let userDatasets: Tables<'datasets'>[] = []; + let globalDuplicate = null; try { - userDatasets = await DatasetService.getUserDatasets(userId); + globalDuplicate = await DatasetService.getCompletedUserDatasetByFile( + userId, + file.name, + file.size + ); } catch (dedupeError) { console.error('useDatasetManager: global dedup check failed, proceeding with upload:', dedupeError); } - const globalDuplicate = userDatasets.find( - d => d.filename === file.name && - d.file_size === file.size && - d.processing_status === 'completed' - ); if (globalDuplicate) { if (typeof window !== 'undefined') { delete (window as any)[inProgressKey]; diff --git a/frontend/lib/services/datasetService.ts b/frontend/lib/services/datasetService.ts index ede0061..62c4dc8 100644 --- a/frontend/lib/services/datasetService.ts +++ b/frontend/lib/services/datasetService.ts @@ -12,6 +12,11 @@ interface CanvasDatasetRow { added_at: string; } +type CompletedDatasetMatch = Pick< + Tables<'datasets'>, + 'id' | 'filename' | 'file_size' | 'updated_at' | 'metadata' | 'processing_status' +>; + export interface DatasetMetadata { columns: number; rows: number; @@ -279,6 +284,35 @@ export class DatasetService { return data || []; } + /** + * Find a completed user-owned dataset by exact file identity. + */ + static async getCompletedUserDatasetByFile( + userId: string, + filename: string, + fileSize: number + ): Promise { + const { data, error } = await supabase + .from('datasets') + .select('id, filename, file_size, updated_at, metadata, processing_status') + .eq('user_id', userId) + .eq('filename', filename) + .eq('file_size', fileSize) + .eq('processing_status', 'completed') + .order('updated_at', { ascending: false }) + .limit(1) as unknown as { + data: CompletedDatasetMatch[] | null; + error: PostgrestLikeError | null; + }; + + if (error) { + console.error('Failed to fetch completed user dataset by file:', error); + throw toPostgrestError(error); + } + + return data?.[0] ?? null; + } + /** * Get all datasets linked to a canvas (visible to both owner and collaborators) */ diff --git a/frontend/lib/services/errorHandler.ts b/frontend/lib/services/errorHandler.ts index a8bf0cc..8ab599e 100644 --- a/frontend/lib/services/errorHandler.ts +++ b/frontend/lib/services/errorHandler.ts @@ -10,6 +10,15 @@ export interface DatabaseError { retryAfter?: number; // seconds } +function isPostgrestLikeError(error: unknown): error is PostgrestError { + if (typeof error !== 'object' || error === null) { + return false; + } + + const maybeError = error as { code?: unknown; message?: unknown }; + return typeof maybeError.code === 'string' && typeof maybeError.message === 'string'; +} + export class DatabaseErrorHandler { /** * Convert Supabase/PostgreSQL errors to user-friendly error objects @@ -329,7 +338,7 @@ export class DatabaseErrorHandler { */ export function useErrorHandler() { const handleError = (error: unknown): DatabaseError => { - if (error instanceof Error) { + if (error instanceof Error || isPostgrestLikeError(error)) { return DatabaseErrorHandler.handleDatabaseError(error); } @@ -355,4 +364,4 @@ export function useErrorHandler() { formatError, getRetryStrategy, }; -} \ No newline at end of file +} From f13cc7a5772697c756d5e7863e187fb96a6f50e4 Mon Sep 17 00:00:00 2001 From: calebyhan Date: Wed, 6 May 2026 16:38:30 -0400 Subject: [PATCH 10/10] fix: make dataset status updates resilient --- frontend/hooks/useDatasetManager.ts | 2 +- frontend/lib/services/datasetService.ts | 62 ++++++++++++------------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/frontend/hooks/useDatasetManager.ts b/frontend/hooks/useDatasetManager.ts index 7730152..8aa1ac5 100644 --- a/frontend/hooks/useDatasetManager.ts +++ b/frontend/hooks/useDatasetManager.ts @@ -178,7 +178,7 @@ export function useDatasetManager(options: DatasetManagerOptions = {}) { // Wrap in try/catch so a transient fetch failure here degrades gracefully // (skip the global dedup, proceed to upload) rather than aborting the entire // upload with a confusing error message. - let globalDuplicate = null; + let globalDuplicate: Awaited> = null; try { globalDuplicate = await DatasetService.getCompletedUserDatasetByFile( userId, diff --git a/frontend/lib/services/datasetService.ts b/frontend/lib/services/datasetService.ts index 62c4dc8..0e03563 100644 --- a/frontend/lib/services/datasetService.ts +++ b/frontend/lib/services/datasetService.ts @@ -117,7 +117,8 @@ export class DatasetService { processing_status: status, }; - // Update metadata with processing info — always read-and-merge to avoid clobbering file_info + // Update metadata with processing info when available. If the metadata read fails, + // still update processing_status so rows do not get stuck in an old state. if (status === 'processing') { const { data: currentDataset, error: fetchError } = await supabase .from('datasets') @@ -125,17 +126,17 @@ export class DatasetService { .eq('id', datasetId) .single(); if (fetchError) { - console.error('updateProcessingStatus: failed to read metadata before marking processing:', fetchError); - throw toPostgrestError(fetchError); + console.error('updateProcessingStatus: failed to read metadata before marking processing; updating status only:', fetchError); + } else { + const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; + updateData.metadata = { + ...currentMetadata, + processing_info: { + ...currentMetadata.processing_info, + started_at: new Date().toISOString(), + }, + }; } - const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; - updateData.metadata = { - ...currentMetadata, - processing_info: { - ...currentMetadata.processing_info, - started_at: new Date().toISOString(), - }, - }; } else if (status === 'completed' || status === 'failed') { // Get current metadata to preserve existing data const { data: currentDataset, error: fetchError } = await supabase @@ -144,24 +145,23 @@ export class DatasetService { .eq('id', datasetId) .single(); if (fetchError) { - console.error(`updateProcessingStatus: failed to read metadata before marking ${status}:`, fetchError); - throw toPostgrestError(fetchError); + console.error(`updateProcessingStatus: failed to read metadata before marking ${status}; updating status only:`, fetchError); + } else { + const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; + const processingInfo = currentMetadata.processing_info || {}; + + updateData.metadata = { + ...currentMetadata, + processing_info: { + ...processingInfo, + completed_at: new Date().toISOString(), + ...(processingInfo.started_at && { + processing_duration_ms: Date.now() - new Date(processingInfo.started_at).getTime(), + }), + ...(errorMessage && { error_message: errorMessage }), + }, + }; } - - const currentMetadata = (currentDataset?.metadata as unknown as DatasetMetadata) || {}; - const processingInfo = currentMetadata.processing_info || {}; - - updateData.metadata = { - ...currentMetadata, - processing_info: { - ...processingInfo, - completed_at: new Date().toISOString(), - ...(processingInfo.started_at && { - processing_duration_ms: Date.now() - new Date(processingInfo.started_at).getTime(), - }), - ...(errorMessage && { error_message: errorMessage }), - }, - }; } const { error } = await supabase @@ -300,10 +300,8 @@ export class DatasetService { .eq('file_size', fileSize) .eq('processing_status', 'completed') .order('updated_at', { ascending: false }) - .limit(1) as unknown as { - data: CompletedDatasetMatch[] | null; - error: PostgrestLikeError | null; - }; + .limit(1) + .returns(); if (error) { console.error('Failed to fetch completed user dataset by file:', error);