From 282fd1f6788deb447fd9e80700f2e6cc321237f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 21:07:32 +0000 Subject: [PATCH 1/5] Delete dead frontend assets and unused global typings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the pre-React window declaration (src/global.d.ts, unreferenced) and the empty/unused SVGs (react, uparrow, downarrow, trash, plus) — all verified unreferenced across src. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg --- packages/frontend/src/assets/downarrow.svg | 14 -------------- packages/frontend/src/assets/plus.svg | 14 -------------- packages/frontend/src/assets/react.svg | 1 - packages/frontend/src/assets/trash.svg | 12 ------------ packages/frontend/src/assets/uparrow.svg | 14 -------------- packages/frontend/src/global.d.ts | 12 ------------ 6 files changed, 67 deletions(-) delete mode 100644 packages/frontend/src/assets/downarrow.svg delete mode 100644 packages/frontend/src/assets/plus.svg delete mode 100644 packages/frontend/src/assets/react.svg delete mode 100644 packages/frontend/src/assets/trash.svg delete mode 100644 packages/frontend/src/assets/uparrow.svg delete mode 100644 packages/frontend/src/global.d.ts diff --git a/packages/frontend/src/assets/downarrow.svg b/packages/frontend/src/assets/downarrow.svg deleted file mode 100644 index b2e3bb2..0000000 --- a/packages/frontend/src/assets/downarrow.svg +++ /dev/null @@ -1,14 +0,0 @@ - - - \ No newline at end of file diff --git a/packages/frontend/src/assets/plus.svg b/packages/frontend/src/assets/plus.svg deleted file mode 100644 index d594686..0000000 --- a/packages/frontend/src/assets/plus.svg +++ /dev/null @@ -1,14 +0,0 @@ - - - - plus-circle - Created with Sketch Beta. - - - - - - - - - \ No newline at end of file diff --git a/packages/frontend/src/assets/react.svg b/packages/frontend/src/assets/react.svg deleted file mode 100644 index 6c87de9..0000000 --- a/packages/frontend/src/assets/react.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/packages/frontend/src/assets/trash.svg b/packages/frontend/src/assets/trash.svg deleted file mode 100644 index ef89567..0000000 --- a/packages/frontend/src/assets/trash.svg +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - - - \ No newline at end of file diff --git a/packages/frontend/src/assets/uparrow.svg b/packages/frontend/src/assets/uparrow.svg deleted file mode 100644 index 7f7517c..0000000 --- a/packages/frontend/src/assets/uparrow.svg +++ /dev/null @@ -1,14 +0,0 @@ - - - \ No newline at end of file diff --git a/packages/frontend/src/global.d.ts b/packages/frontend/src/global.d.ts deleted file mode 100644 index 0a958ad..0000000 --- a/packages/frontend/src/global.d.ts +++ /dev/null @@ -1,12 +0,0 @@ -declare global { - interface Window { - JsPsychMetadata: new () => JsPsychMetadataInstance; - } - } - - interface JsPsychMetadataInstance { - generate: (data: any, metadataOptions?: object, csv?: boolean) => void; - getMetadata: () => object; - } - - export {}; \ No newline at end of file From ce50cf07951d471d15b744a7b5215f93fd7e3851 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 21:08:18 +0000 Subject: [PATCH 2/5] Zip download: sequential single-worker streaming (issue #95) Compress dataset entries strictly one at a time: for each file create a single AsyncZipDeflate, feed it in bounded ~1 MiB Blob slices, and await that entry's final chunk before starting the next. fflate spawns one worker per live AsyncZipDeflate and buffers later entries until earlier ones finish, so the old "create every deflater up front" loop held N workers plus most of the compressed dataset in the heap; this keeps ~one worker and one file's working set resident. Public API is unchanged; the header comment now matches reality. Tests: fake-fflate ordering test asserts entry N+1 isn't created/pushed before entry N emits its final chunk; downloadDatasetZip (previously uncovered) gains picker-abort -> false, streaming-error -> sink.abort()+propagate, blob fallback, and streaming-success cases. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg --- packages/frontend/src/staging/datasetZip.ts | 70 +++++++++++++++---- packages/frontend/tests/datasetZip.test.ts | 64 ++++++++++++++++- .../tests/datasetZipStreaming.test.ts | 68 ++++++++++++++++++ 3 files changed, 186 insertions(+), 16 deletions(-) create mode 100644 packages/frontend/tests/datasetZipStreaming.test.ts diff --git a/packages/frontend/src/staging/datasetZip.ts b/packages/frontend/src/staging/datasetZip.ts index 5ea0a35..bab6ea7 100644 --- a/packages/frontend/src/staging/datasetZip.ts +++ b/packages/frontend/src/staging/datasetZip.ts @@ -1,17 +1,24 @@ // Builds and streams the downloadable Psych-DS dataset zip from the staged file store. // -// Uses fflate's AsyncZipDeflate (DEFLATE compressed, worker-backed when available) to emit -// output chunks as each file finishes compressing — so neither the full input set nor the -// complete output zip ever lives in the JS heap at once. On Chromium (showSaveFilePicker) each -// chunk is written to the user-chosen file as it arrives, bounding peak heap to roughly one -// file's working set. On other browsers (or if the picker fails) chunks are collected into a -// Blob and downloaded via an object URL. +// Files are compressed strictly one at a time. For each entry we create a single fflate +// AsyncZipDeflate, feed the file's bytes in bounded ~1 MiB slices (read lazily from the +// disk-backed Blob), and wait for that entry's final compressed chunk before starting the next +// one. fflate spawns one worker per live AsyncZipDeflate and buffers later entries until earlier +// ones finish, so processing sequentially keeps exactly one worker (and roughly one file's +// working set) resident at a time rather than N workers plus most of the compressed dataset in +// the heap. Output chunks are emitted as they are produced: on Chromium (showSaveFilePicker) each +// is written to the user-chosen file as it arrives, bounding peak heap to about one file's +// working set; on other browsers (or if the picker fails) chunks are collected into a Blob and +// downloaded via an object URL. import { Zip, AsyncZipDeflate } from 'fflate'; import { DATASET_DESCRIPTION_FILENAME } from '../datasetLayout'; import { blobDownload } from '../download'; import type { DatasetFileSource } from './stagedFileStore'; +/** Slice size for feeding a file into the deflater — bounds peak heap to ~one slice per file. */ +const PUSH_CHUNK_BYTES = 1 << 20; // 1 MiB + const readmeContents = (projectName: string): string => `# ${projectName}\nHuman-readable description of the project and dataset.`; @@ -36,35 +43,68 @@ interface ZipSink { /** * Runs the zip build and routes each output chunk to `onChunk`. Resolves once the zip is - * complete (all chunks delivered). Each data file is read from the store one at a time. + * complete (all chunks delivered). Entries are compressed strictly one at a time: each file is + * read from the store lazily, pushed into its own deflater in ~1 MiB slices, and awaited to its + * final compressed chunk before the next entry's deflater is created. */ async function buildZip(opts: BuildDatasetZipOptions, onChunk: (dat: Uint8Array) => void): Promise { return new Promise((resolve, reject) => { + let failed = false; + const fail = (err: unknown) => { if (!failed) { failed = true; reject(err); } }; + const zip = new Zip((err, dat, final) => { - if (err) { reject(err); return; } + if (err) { fail(err); return; } onChunk(dat); if (final) resolve(); }); - const addEntry = (filename: string, content: Uint8Array): void => { + // Add one entry, feed it in bounded slices, and resolve only once its final compressed chunk + // has been emitted — so no second AsyncZipDeflate (hence no second worker) is created until + // this one is done. + const addEntrySequential = (filename: string, content: Blob): Promise => { const entry = new AsyncZipDeflate(filename, { level: 6 }); zip.add(entry); - entry.push(content, true); + // zip.add installs the routing handler that streams this entry's bytes into the archive; + // wrap it so we also learn when this entry has emitted its final chunk. + const routeToZip = entry.ondata!; + return new Promise((resolveEntry, rejectEntry) => { + entry.ondata = (err, dat, final) => { + routeToZip(err, dat, final); + if (err) rejectEntry(err); + else if (final) resolveEntry(); + }; + void (async () => { + try { + const size = content.size; + if (size === 0) { entry.push(new Uint8Array(0), true); return; } + for (let off = 0; off < size; off += PUSH_CHUNK_BYTES) { + const end = Math.min(off + PUSH_CHUNK_BYTES, size); + const slice = new Uint8Array(await content.slice(off, end).arrayBuffer()); + entry.push(slice, end >= size); + } + } catch (e) { + rejectEntry(e); + } + })(); + }); }; (async () => { try { - addEntry(DATASET_DESCRIPTION_FILENAME, new TextEncoder().encode(opts.metadataJson)); + await addEntrySequential( + DATASET_DESCRIPTION_FILENAME, + new Blob([new TextEncoder().encode(opts.metadataJson)]), + ); if (opts.dataFiles) { for await (const [path, blob] of opts.dataFiles.entries()) { - addEntry(path, new Uint8Array(await blob.arrayBuffer())); + await addEntrySequential(path, blob); } } - addEntry('README.md', new TextEncoder().encode(readmeContents(opts.projectName))); - addEntry('CHANGES.md', new TextEncoder().encode(CHANGES_CONTENTS)); + await addEntrySequential('README.md', new Blob([readmeContents(opts.projectName)])); + await addEntrySequential('CHANGES.md', new Blob([CHANGES_CONTENTS])); zip.end(); } catch (e) { - reject(e); + fail(e); } })(); }); diff --git a/packages/frontend/tests/datasetZip.test.ts b/packages/frontend/tests/datasetZip.test.ts index 4384780..73f005a 100644 --- a/packages/frontend/tests/datasetZip.test.ts +++ b/packages/frontend/tests/datasetZip.test.ts @@ -1,5 +1,5 @@ import { unzipSync } from 'fflate'; -import { buildDatasetZipBlob } from '../src/staging/datasetZip'; +import { buildDatasetZipBlob, downloadDatasetZip } from '../src/staging/datasetZip'; import { createStagedFileStore } from '../src/staging/stagedFileStore'; async function readZip(blob: Blob): Promise> { @@ -58,3 +58,65 @@ describe('buildDatasetZipBlob', () => { expect(Object.keys(files).filter((k) => k.startsWith('data/'))).toHaveLength(0); }); }); + +type PickerWindow = { showSaveFilePicker?: (...args: unknown[]) => Promise }; +const pickerWindow = window as unknown as PickerWindow; + +describe('downloadDatasetZip', () => { + const originalPicker = pickerWindow.showSaveFilePicker; + afterEach(() => { + if (originalPicker === undefined) delete pickerWindow.showSaveFilePicker; + else pickerWindow.showSaveFilePicker = originalPicker; + }); + + test('returns false when the user aborts the save dialog', async () => { + pickerWindow.showSaveFilePicker = jest + .fn() + .mockRejectedValue(Object.assign(new Error('cancelled'), { name: 'AbortError' })); + + const result = await downloadDatasetZip({ metadataJson: '{"name":"x"}', projectName: 'x' }, 'x.zip'); + expect(result).toBe(false); + }); + + test('streams to the picked file and returns true on success', async () => { + const written: Uint8Array[] = []; + const writable = { + write: jest.fn(async (d: Uint8Array) => { written.push(d); }), + close: jest.fn(async () => {}), + abort: jest.fn(async () => {}), + }; + pickerWindow.showSaveFilePicker = jest + .fn() + .mockResolvedValue({ createWritable: jest.fn().mockResolvedValue(writable) }); + + const result = await downloadDatasetZip({ metadataJson: '{"name":"x"}', projectName: 'x' }, 'x.zip'); + expect(result).toBe(true); + expect(writable.write).toHaveBeenCalled(); + expect(writable.close).toHaveBeenCalled(); + expect(writable.abort).not.toHaveBeenCalled(); + }); + + test('propagates streaming errors and aborts the sink', async () => { + const writable = { + write: jest.fn().mockRejectedValue(new Error('disk full')), + close: jest.fn(async () => {}), + abort: jest.fn(async () => {}), + }; + pickerWindow.showSaveFilePicker = jest + .fn() + .mockResolvedValue({ createWritable: jest.fn().mockResolvedValue(writable) }); + + await expect( + downloadDatasetZip({ metadataJson: '{"name":"x"}', projectName: 'x' }, 'x.zip'), + ).rejects.toThrow('disk full'); + expect(writable.abort).toHaveBeenCalled(); + expect(writable.close).not.toHaveBeenCalled(); + }); + + test('falls back to a blob download when the file-system picker is unavailable', async () => { + delete pickerWindow.showSaveFilePicker; + const result = await downloadDatasetZip({ metadataJson: '{"name":"x"}', projectName: 'x' }, 'x.zip'); + expect(result).toBe(true); + expect(URL.createObjectURL).toHaveBeenCalled(); + }); +}); diff --git a/packages/frontend/tests/datasetZipStreaming.test.ts b/packages/frontend/tests/datasetZipStreaming.test.ts new file mode 100644 index 0000000..c688598 --- /dev/null +++ b/packages/frontend/tests/datasetZipStreaming.test.ts @@ -0,0 +1,68 @@ +// Verifies the memory-bounded contract of the zip builder: entries are compressed strictly one at +// a time, so entry N+1's deflater isn't created (and its bytes aren't pushed) until entry N has +// emitted its final chunk. fflate is faked so we can observe the exact ordering of create/push/final +// events — with the real (worker-backed) build this ordering isn't directly observable. + +const mockEvents: string[] = []; + +jest.mock('fflate', () => { + class FakeAsyncZipDeflate { + ondata: ((err: unknown, dat: Uint8Array, final: boolean) => void) | null = null; + constructor(public filename: string) { + mockEvents.push(`create:${filename}`); + } + push(data: Uint8Array, final: boolean) { + mockEvents.push(`push:${this.filename}:${final}`); + // Emit asynchronously so that a missing `await` between entries would let their pushes + // interleave — the ordering assertions below would then fail. + Promise.resolve().then(() => { + this.ondata?.(null, data, final); + if (final) mockEvents.push(`final:${this.filename}`); + }); + } + } + class FakeZip { + constructor(private cb: (err: unknown, dat: Uint8Array, final: boolean) => void) {} + add(entry: FakeAsyncZipDeflate) { + // Route an entry's chunks into the archive stream (never the whole-zip `final`). + entry.ondata = (err, dat) => this.cb(err, dat, false); + } + end() { + this.cb(null, new Uint8Array(0), true); + } + } + return { Zip: FakeZip, AsyncZipDeflate: FakeAsyncZipDeflate }; +}); + +import { buildDatasetZipBlob } from '../src/staging/datasetZip'; +import { createStagedFileStore } from '../src/staging/stagedFileStore'; + +beforeEach(() => { + mockEvents.length = 0; +}); + +test('compresses entries strictly sequentially (N+1 not started before N finishes)', async () => { + const store = createStagedFileStore({ forceMemory: true }); + await store.write('data/a.csv', 'aaa'); + await store.write('data/b.csv', 'bbb'); + + await buildDatasetZipBlob({ metadataJson: '{}', projectName: 'p', dataFiles: store }); + + const order = ['dataset_description.json', 'data/a.csv', 'data/b.csv', 'README.md', 'CHANGES.md']; + + // Every entry was created and finished. + for (const name of order) { + expect(mockEvents).toContain(`create:${name}`); + expect(mockEvents).toContain(`final:${name}`); + } + + // Each entry finishes before the next is created or pushed. + for (let i = 0; i < order.length - 1; i++) { + const finalI = mockEvents.indexOf(`final:${order[i]}`); + const createNext = mockEvents.indexOf(`create:${order[i + 1]}`); + const pushNext = mockEvents.indexOf(`push:${order[i + 1]}:true`); + expect(finalI).toBeGreaterThanOrEqual(0); + expect(finalI).toBeLessThan(createNext); + expect(finalI).toBeLessThan(pushNext); + } +}); From 0256e1b1c4fd913f55b97e524d482623e6901162 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 21:08:36 +0000 Subject: [PATCH 3/5] Fix wizard state-loss bugs (revisit/upload/navigation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Load existing metadata exactly once per uploaded file: gate ProjectInfo's load effect on a file-identity token stored in the AppShell-level session, so revisiting Project Info no longer re-runs loadMetadata and clobbers session edits (resurrecting deleted authors / reverting variables). Propagate load success/failure into the session and gate Data pre-completion and the "variables loaded from existing metadata" banner on it, so a failed parse no longer pre-completes Data. Make metadata and staged data stay consistent across batches: "Upload additional files" is now truly additive (no store.clear(), used-filename sets persisted in the session and seeded for the new batch, files appended); "Replace all data" / "Change folder" confirm first, then fully reset — clear the store AND reset the metadata variables via a resetMetadata callback owned by AppShell (drops generated variables, reloads the existing metadata file if present, re-applies project-info fields). Re-configure join keys reprocesses the whole dataset as a replace so it can't double-stage a batch. Block sidebar navigation and Start over while a run is in flight (DataUpload reports busy by phase; Sidebar gains a `locked` prop), with a polite processing summary. Restore the 'ready' phase from session.files when files were picked but not processed, and move sourceName into the session. Set webkitdirectory declaratively on every folder input so it survives phase-branch remounts. Reset the folder input value after selection; move the .psychds-ignore write inside the per-file try so an OPFS failure can't hang the run. Warn before unload while there is unsaved work, cleared after a download. aria-live on the pick error and processing summary. Tests: real-page AppShell integration (load-once + failed-parse gating), batch consistency (additive keeps both batches + persists name sets; replace confirms, clears the store, fires reset), navigation-lock busy reporting, session round-trip, zip basename handling, and Sidebar locked. Existing AppShell/ DataUpload tests updated for the load-status gating and zip basename change. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg --- packages/frontend/src/components/AppShell.tsx | 75 ++++- packages/frontend/src/components/Sidebar.tsx | 7 +- .../frontend/src/pages/DataUpload.module.css | 69 +++++ packages/frontend/src/pages/DataUpload.tsx | 275 ++++++++++++++---- packages/frontend/src/pages/ProjectInfo.tsx | 75 ++++- packages/frontend/tests/AppShell.test.tsx | 40 ++- .../tests/AppShellIntegration.test.tsx | 71 +++++ packages/frontend/tests/DataUpload.test.tsx | 167 ++++++++++- packages/frontend/tests/Sidebar.test.tsx | 17 ++ packages/frontend/tests/setup.ts | 16 + 10 files changed, 705 insertions(+), 107 deletions(-) create mode 100644 packages/frontend/tests/AppShellIntegration.test.tsx diff --git a/packages/frontend/src/components/AppShell.tsx b/packages/frontend/src/components/AppShell.tsx index 53859ee..60664f6 100644 --- a/packages/frontend/src/components/AppShell.tsx +++ b/packages/frontend/src/components/AppShell.tsx @@ -1,8 +1,8 @@ -import { useState } from 'react'; +import { useState, useEffect, useRef, useCallback } from 'react'; import JsPsychMetadata from '@jspsych/metadata'; import Sidebar from './Sidebar'; import PreviewDrawer from './PreviewDrawer'; -import ProjectInfo, { ProjectInfoSession, emptyProjectInfoSession } from '../pages/ProjectInfo'; +import ProjectInfo, { ProjectInfoSession, emptyProjectInfoSession, applyProjectInfoFields } from '../pages/ProjectInfo'; import DataUpload, { DataSession, emptyDataSession } from '../pages/DataUpload'; import Variables from '../pages/Variables'; import Authors from '../pages/Authors'; @@ -26,20 +26,64 @@ interface AppShellProps { } const AppShell: React.FC = ({ jsPsychMetadata, existingMetadataFile, onStartOver }) => { - const isExistingProject = !!existingMetadataFile; - const [currentStep, setCurrentStep] = useState('projectInfo'); - // Pre-complete the Data step for existing projects — variables are already loaded from the JSON - const [completedSteps, setCompletedSteps] = useState>( - () => isExistingProject ? new Set(['data']) : new Set() - ); + const [completedSteps, setCompletedSteps] = useState>(() => new Set()); const [dataProcessed, setDataProcessed] = useState(false); + const [dataBusy, setDataBusy] = useState(false); const [dataSession, setDataSession] = useState(emptyDataSession); const [projectInfoSession, setProjectInfoSession] = useState( () => emptyProjectInfoSession() ); const [previewOpen, setPreviewOpen] = useState(false); + // An existing project's Data step is only "done for free" once its metadata actually loaded — + // a failed parse must not pre-complete Data or claim variables were loaded from it. + const existingLoaded = projectInfoSession.loadStatus === 'loaded'; + + // Pre-complete the Data step when an existing project's metadata loads successfully — its + // variables come from the JSON, so no data upload is required to advance. + useEffect(() => { + if (existingLoaded) { + setCompletedSteps(prev => (prev.has('data') ? prev : new Set([...prev, 'data']))); + } + }, [existingLoaded]); + + // Latest project-info fields, read when rebuilding metadata after a data replace. + const projectInfoSessionRef = useRef(projectInfoSession); + projectInfoSessionRef.current = projectInfoSession; + + // Full data reset for the "replace all data" flow: drop every generated variable so the + // metadata no longer describes the discarded dataset, then (for an existing project) restore + // the uploaded metadata file's variables and re-apply the user's edited project-info fields. + const resetMetadata = useCallback(async () => { + for (const name of jsPsychMetadata.getVariableNames()) jsPsychMetadata.deleteVariable(name); + if (existingMetadataFile) { + try { + jsPsychMetadata.loadMetadata(await existingMetadataFile.text()); + } catch { + /* leave the cleared state if the file no longer parses */ + } + applyProjectInfoFields(jsPsychMetadata, projectInfoSessionRef.current); + } + }, [jsPsychMetadata, existingMetadataFile]); + + // Warn before an accidental tab close/reload while there's unsaved work (files staged or metadata + // edited) that hasn't been downloaded yet — nothing is persisted server-side. Lifted once the + // user downloads their dataset. + const [downloaded, setDownloaded] = useState(false); + const hasUnsavedWork = + !downloaded && + (dataSession.files.length > 0 || + (dataSession.convertedStore?.paths().length ?? 0) > 0 || + projectInfoSession.name.trim() !== '' || + projectInfoSession.description.trim() !== ''); + useEffect(() => { + if (!hasUnsavedWork) return; + const onBeforeUnload = (e: BeforeUnloadEvent) => { e.preventDefault(); e.returnValue = ''; }; + window.addEventListener('beforeunload', onBeforeUnload); + return () => window.removeEventListener('beforeunload', onBeforeUnload); + }, [hasUnsavedWork]); + // Discard this session's on-disk staging before tearing the shell down — Start Over throws the // whole project away, so the converted CSVs/raw originals shouldn't linger in OPFS (otherwise // they sit there until the next startup sweep). Fire-and-forget: clear() swallows its own errors @@ -90,8 +134,10 @@ const AppShell: React.FC = ({ jsPsychMetadata, existingMetadataFi { setDataProcessed(true); completeStep('data'); }} + onResetMetadata={resetMetadata} + onBusyChange={setDataBusy} session={dataSession} onSessionChange={setDataSession} /> @@ -101,7 +147,13 @@ const AppShell: React.FC = ({ jsPsychMetadata, existingMetadataFi case 'authors': return completeStep('authors')} />; case 'review': - return ; + return ( + setDownloaded(true)} + /> + ); } }; @@ -112,8 +164,9 @@ const AppShell: React.FC = ({ jsPsychMetadata, existingMetadataFi currentStep={currentStep} completedSteps={completedSteps} canNavigateTo={canNavigateTo} - onNavigate={(stepId) => { if (canNavigateTo(stepId)) setCurrentStep(stepId); }} + onNavigate={(stepId) => { if (!dataBusy && canNavigateTo(stepId)) setCurrentStep(stepId); }} onStartOver={handleStartOver} + locked={dataBusy} />
{renderStep()} diff --git a/packages/frontend/src/components/Sidebar.tsx b/packages/frontend/src/components/Sidebar.tsx index 9abee38..d267228 100644 --- a/packages/frontend/src/components/Sidebar.tsx +++ b/packages/frontend/src/components/Sidebar.tsx @@ -15,6 +15,8 @@ interface SidebarProps { canNavigateTo: (stepId: StepId) => boolean; onNavigate: (stepId: StepId) => void; onStartOver: () => void; + /** When true (e.g. data is processing) all navigation is disabled so a run can't be orphaned. */ + locked?: boolean; } const Sidebar: React.FC = ({ @@ -24,6 +26,7 @@ const Sidebar: React.FC = ({ canNavigateTo, onNavigate, onStartOver, + locked = false, }) => { const [confirming, setConfirming] = useState(false); const dialogRef = useRef(null); @@ -42,7 +45,7 @@ const Sidebar: React.FC = ({ {steps.map((step) => { const isActive = step.id === currentStep; const isCompleted = completedSteps.has(step.id); - const isLocked = !canNavigateTo(step.id) && !isActive; + const isLocked = locked || (!canNavigateTo(step.id) && !isActive); const cls = [ styles.step, @@ -79,7 +82,7 @@ const Sidebar: React.FC = ({ > What is Psych-DS? ↗ - diff --git a/packages/frontend/src/pages/DataUpload.module.css b/packages/frontend/src/pages/DataUpload.module.css index c008078..85bde87 100644 --- a/packages/frontend/src/pages/DataUpload.module.css +++ b/packages/frontend/src/pages/DataUpload.module.css @@ -416,3 +416,72 @@ .continueBtn:hover { background-color: var(--c-amber-hover); } + +/* Additive-upload hint + replace link */ + +.additionalHint { + margin: 0.75rem 0 0; + font-size: 0.85rem; + color: var(--c-ink-3); +} + +.replaceLink { + background: none; + border: none; + padding: 0; + font-size: 0.85rem; + font-family: inherit; + color: var(--c-ink-2); + text-decoration: underline; + cursor: pointer; +} + +.replaceLink:hover { + color: var(--c-ink); +} + +/* Processing summary (announced politely to assistive tech) */ + +.processSummary { + margin: 0 0 0.75rem; + font-size: 0.85rem; + color: var(--c-ink-3); +} + +/* Replace-all-data confirmation */ + +.replaceConfirm { + margin: 0 0 1.25rem; + padding: 1rem 1.1rem; + border: 1px solid var(--c-border); + border-radius: 8px; + background: var(--c-bg-raised); +} + +.replaceConfirmMsg { + margin: 0 0 0.85rem; + font-size: 0.9rem; + color: var(--c-ink); +} + +.replaceConfirmBtns { + display: flex; + align-items: center; + gap: 1rem; +} + +.replaceConfirmYes { + background-color: var(--c-danger); + color: #fff; + border: none; + border-radius: 6px; + padding: 0.5em 1.2em; + font-size: 0.9rem; + font-weight: 500; + font-family: inherit; + cursor: pointer; +} + +.replaceConfirmYes:hover { + filter: brightness(0.94); +} diff --git a/packages/frontend/src/pages/DataUpload.tsx b/packages/frontend/src/pages/DataUpload.tsx index 604e093..6b62bab 100644 --- a/packages/frontend/src/pages/DataUpload.tsx +++ b/packages/frontend/src/pages/DataUpload.tsx @@ -14,7 +14,10 @@ export type FileStatus = { }; export type DataSession = { + /** Every uploaded file processed into the current staged payload, accumulated across batches. */ files: File[]; + /** Display name of the last picked folder/zip (component-local state moved here so it survives remounts). */ + sourceName: string; /** * Staged Psych-DS `data/` payload built from the uploaded files (dataset-relative paths such as * `data/subject-sub01_data.csv`, `data/raw/sub01.json`). JSON is converted to Psych-DS-named CSV @@ -23,6 +26,13 @@ export type DataSession = { * held in the heap; null until the first processing run. See {@link StagedFileStore}. */ convertedStore: StagedFileStore | null; + /** + * Filenames already used for converted CSVs / preserved raw originals in the staged store. + * Persisted so a later additive batch disambiguates against the whole output directory (not just + * its own files), keeping the staged data and the metadata's variableMeasured in agreement. + */ + usedArrayFilenames: string[]; + usedRawFilenames: string[]; joinKeyCandidates: JoinKeyCandidate[]; joinKeyProblemFile: string; selectedKeys: string[]; @@ -31,7 +41,10 @@ export type DataSession = { export const emptyDataSession: DataSession = { files: [], + sourceName: '', convertedStore: null, + usedArrayFilenames: [], + usedRawFilenames: [], joinKeyCandidates: [], joinKeyProblemFile: '', selectedKeys: ['trial_index'], @@ -43,6 +56,13 @@ interface DataUploadProps { dataProcessed: boolean; existingMetadataLoaded?: boolean; onComplete: () => void; + /** + * Full data reset for the "replace all data" flow: clears generated variables so metadata stops + * describing the discarded dataset (see AppShell). Called before staging a replacement batch. + */ + onResetMetadata?: () => void | Promise; + /** Reports whether a run is in flight so the shell can lock navigation while processing. */ + onBusyChange?: (busy: boolean) => void; session: DataSession; onSessionChange: (s: DataSession) => void; } @@ -97,23 +117,53 @@ const disambiguateFlatFilename = (name: string, used: Set): string => { return `${stem}${n}${ext}`; }; +/** Confirmation shown before a destructive "replace all data" — clears staged data and metadata. */ +const ReplaceConfirm: React.FC<{ onConfirm: () => void; onCancel: () => void }> = ({ onConfirm, onCancel }) => ( +
+

+ Replace all uploaded data? This discards the data you already staged and the variables generated + from it, then lets you pick a new folder or zip. +

+
+ + +
+
+); + const DataUpload: React.FC = ({ jsPsychMetadata, dataProcessed, existingMetadataLoaded, onComplete, + onResetMetadata, + onBusyChange, session, onSessionChange, }) => { const inputRef = useRef(null); const zipInputRef = useRef(null); - const initialPhase: Phase = dataProcessed ? 'hasData' : existingMetadataLoaded ? 'fromExisting' : 'idle'; + // Restore 'ready' when files were picked but never processed, so a navigation round-trip doesn't + // silently drop the selection (files live in the session, which survives the page remount). + const initialPhase: Phase = dataProcessed + ? 'hasData' + : session.files.length > 0 + ? 'ready' + : existingMetadataLoaded + ? 'fromExisting' + : 'idle'; const [phase, setPhase] = useState(initialPhase); + // `files` is the accumulated set backing the session; `batch` is the not-yet-processed pick that + // the ready list shows and runGenerate processes. On mount both start from the session so an + // unprocessed selection reappears. const [files, setFiles] = useState(session.files); - const [sourceName, setSourceName] = useState(''); + const [batch, setBatch] = useState(session.files); + const [sourceName, setSourceName] = useState(session.sourceName); const [pickError, setPickError] = useState(''); const [convertedStore, setConvertedStore] = useState(session.convertedStore); + const [usedArrayFilenames, setUsedArrayFilenames] = useState(session.usedArrayFilenames); + const [usedRawFilenames, setUsedRawFilenames] = useState(session.usedRawFilenames); const [fileStatuses, setFileStatuses] = useState(session.fileStatuses); const [joinKeyCandidates, setJoinKeyCandidates] = useState(session.joinKeyCandidates); const [joinKeyProblemFile, setJoinKeyProblemFile] = useState(session.joinKeyProblemFile); @@ -122,59 +172,114 @@ const DataUpload: React.FC = ({ const [proceedAnyway, setProceedAnyway] = useState(false); const [joinKeyReturnPhase, setJoinKeyReturnPhase] = useState('ready'); const [showJoinKeyHelp, setShowJoinKeyHelp] = useState(false); + // 'additive' appends the next batch to the existing staged data + metadata; 'replace' starts the + // data over (after a confirm + full reset). Set when the user picks a folder/zip. + const [pendingMode, setPendingMode] = useState<'replace' | 'additive'>('replace'); + // Which picker to open once the user confirms a destructive replace, or null when no confirm is pending. + const [replaceConfirm, setReplaceConfirm] = useState(null); // Keep parent session in sync const onSessionChangeRef = useRef(onSessionChange); onSessionChangeRef.current = onSessionChange; useEffect(() => { onSessionChangeRef.current({ - files, convertedStore, joinKeyCandidates, joinKeyProblemFile, selectedKeys: committedKeys, fileStatuses, + files, sourceName, convertedStore, usedArrayFilenames, usedRawFilenames, + joinKeyCandidates, joinKeyProblemFile, selectedKeys: committedKeys, fileStatuses, }); - }, [files, convertedStore, joinKeyCandidates, joinKeyProblemFile, committedKeys, fileStatuses]); + }, [files, sourceName, convertedStore, usedArrayFilenames, usedRawFilenames, joinKeyCandidates, joinKeyProblemFile, committedKeys, fileStatuses]); + // Report processing state so the shell can lock navigation while a run is in flight — otherwise + // navigating away mid-run orphans the staged data and loses the partially-generated variables. + const onBusyChangeRef = useRef(onBusyChange); + onBusyChangeRef.current = onBusyChange; useEffect(() => { - if (inputRef.current) (inputRef.current as any).webkitdirectory = true; // not in TS lib - }, []); + onBusyChangeRef.current?.(phase === 'preflight' || phase === 'processing'); + }, [phase]); + + /** Whether a prior run already staged data (so replacing it needs a confirm + reset). */ + const hasStagedData = (): boolean => (convertedStore?.paths().length ?? 0) > 0; + + // Open a picker. A destructive replace over already-staged data asks for confirmation first; + // everything else (first pick, additional files) opens immediately. + const requestPick = (kind: 'folder' | 'zip', mode: 'replace' | 'additive') => { + setPickError(''); + if (mode === 'replace' && hasStagedData()) { + setReplaceConfirm(kind); + return; + } + setPendingMode(mode); + (kind === 'folder' ? inputRef : zipInputRef).current?.click(); + }; + + // Confirmed replace: wipe the staged data and reset the metadata variables so both start empty, + // then open the chosen picker to stage the replacement (as a fresh, non-additive run). + const confirmReplace = async () => { + const kind = replaceConfirm; + setReplaceConfirm(null); + await onResetMetadata?.(); + await convertedStore?.clear(); + setConvertedStore(null); + setFiles([]); + setBatch([]); + setFileStatuses([]); + setUsedArrayFilenames([]); + setUsedRawFilenames([]); + setJoinKeyCandidates([]); + setJoinKeyProblemFile(''); + setSourceName(''); + setPendingMode('replace'); + setPhase('idle'); + if (kind === 'folder') inputRef.current?.click(); + else if (kind === 'zip') zipInputRef.current?.click(); + }; const handleFolderChange = (e: React.ChangeEvent) => { - if (!e.target.files) return; + if (!e.target.files) { return; } const picked = [...e.target.files].filter(f => !f.name.startsWith('.')); const folderName = picked[0]?.webkitRelativePath.split('/')[0] ?? ''; - setFiles(picked); + // Reset the input value so re-picking the same folder fires change again. + e.target.value = ''; + if (picked.length === 0) return; + setBatch(picked); + if (pendingMode === 'replace') setFiles(picked); setSourceName(folderName); setPickError(''); setPhase('ready'); - setFileStatuses([]); }; const handleZipChange = async (e: React.ChangeEvent) => { const zipFile = e.target.files?.[0]; + // Reset input so the same file can be re-selected. + e.target.value = ''; if (!zipFile) return; setPickError(''); try { const zip = await JSZip.loadAsync(zipFile); + // Extract entries sequentially as Blobs (not concurrently as strings) so the whole archive + // isn't inflated into the heap at once. Use the entry basename for the File name — the full + // in-archive path defeats the Psych-DS compliant-name check and produces nested data/raw/ + // paths — while keeping the path on webkitRelativePath for display and disambiguation. const extracted: File[] = []; - await Promise.all( - Object.values(zip.files).map(async entry => { - if (entry.dir) return; - if (entry.name.startsWith('__MACOSX') || entry.name.split('/').pop()?.startsWith('.')) return; - const text = await entry.async('text'); - extracted.push(new File([text], entry.name)); - }) - ); + for (const entry of Object.values(zip.files)) { + if (entry.dir) continue; + const base = entry.name.split('/').pop() ?? entry.name; + if (entry.name.startsWith('__MACOSX') || base.startsWith('.')) continue; + const blob = await entry.async('blob'); + const file = new File([blob], base); + Object.defineProperty(file, 'webkitRelativePath', { value: entry.name, configurable: true }); + extracted.push(file); + } if (extracted.length === 0) { setPickError('No readable files found in the zip archive.'); return; } - setFiles(extracted); + setBatch(extracted); + if (pendingMode === 'replace') setFiles(extracted); setSourceName(zipFile.name.replace(/\.zip$/i, '')); setPhase('ready'); - setFileStatuses([]); } catch { setPickError('Could not read the zip file — make sure it is a valid .zip archive.'); } - // Reset input so the same file can be re-selected - e.target.value = ''; }; const handleProcess = async () => { @@ -189,7 +294,7 @@ const DataUpload: React.FC = ({ // upload is never resident in the heap at once (File objects are already disk-backed // by the browser). The trade-off is that JSON files are read again in runGenerate — // intentional: this step bounds peak memory, not total I/O. - for (const file of files) { + for (const file of batch) { const name = file.webkitRelativePath || file.name; if (dataFileType(file.name) !== 'json') continue; if (name === 'dataset_description.json' || name.endsWith('/dataset_description.json')) continue; @@ -245,24 +350,30 @@ const DataUpload: React.FC = ({ suppressWarning: boolean ) => { setPhase('processing'); - const initial: FileStatus[] = files.map(f => ({ name: f.name, status: 'pending' })); - setFileStatuses(initial); + // Additive runs append this batch's outcomes below the previous batches'; replace runs start + // the status list (and the staged data) over. + const additive = pendingMode === 'additive'; + const priorStatuses = additive ? fileStatuses : []; + const offset = priorStatuses.length; + const initial: FileStatus[] = batch.map(f => ({ name: f.name, status: 'pending' })); + setFileStatuses([...priorStatuses, ...initial]); const update = (i: number, patch: Partial) => - setFileStatuses(prev => prev.map((s, idx) => idx === i ? { ...s, ...patch } : s)); + setFileStatuses(prev => prev.map((s, idx) => idx === offset + i ? { ...s, ...patch } : s)); // Psych-DS `data/` payload built alongside metadata generation, staged to disk (OPFS) as each // file is produced so the whole study is never resident in the heap at once. The name sets are // shared across all files so converted CSVs are disambiguated against the whole output // directory (mirrors the CLI's processDirectory), and `data/raw/` is flat so originals too. - // Reuse the existing store across re-processing runs, clearing stale output first. + // An additive run keeps the existing staged files and seeds the name sets from the session so + // the new batch is disambiguated against them; a replace run starts from an empty store. const store = convertedStore ?? createStagedFileStore(); - await store.clear(); - const usedArrayFilenames = new Set(); - const usedRawFilenames = new Set(); + if (!additive) await store.clear(); + const arrayNames = new Set(additive ? usedArrayFilenames : []); + const rawNames = new Set(additive ? usedRawFilenames : []); - for (let i = 0; i < files.length; i++) { - const file = files[i]; + for (let i = 0; i < batch.length; i++) { + const file = batch[i]; update(i, { status: 'loading' }); @@ -332,15 +443,21 @@ const DataUpload: React.FC = ({ extractedArrays: jsPsychMetadata.getExtractedArrays(), extractedObjects: jsPsychMetadata.getExtractedObjects(), joinKeys: jsPsychMetadata.getArrayJoinKeys(), - usedArrayFilenames, + usedArrayFilenames: arrayNames, }); for (const f of built) await store.write(`data/${f.filename}`, f.content); // Preserve the original JSON under data/raw/ (CSV inputs are already tabular). if (type === 'json') { - const rawName = disambiguateFlatFilename(file.name, usedRawFilenames); - usedRawFilenames.add(rawName); + const rawName = disambiguateFlatFilename(file.name, rawNames); + rawNames.add(rawName); await store.write(`data/raw/${rawName}`, content); + // Tell the validator to skip data/raw/ so preserved originals don't surface as + // FILE_NOT_CHECKED. Written inside the per-file try (guarded against re-writing) so an + // OPFS failure is reported as this file's error instead of hanging the run in 'processing'. + if (!store.has(PSYCHDS_IGNORE_FILENAME)) { + await store.write(PSYCHDS_IGNORE_FILENAME, PSYCHDS_IGNORE_CONTENT); + } } update(i, { status: 'success' }); @@ -349,13 +466,11 @@ const DataUpload: React.FC = ({ } } - // When we preserved raw originals, tell the validator to skip data/raw/ so they don't - // surface as FILE_NOT_CHECKED (shared definition with the CLI in @jspsych/metadata). - if (usedRawFilenames.size > 0) { - await store.write(PSYCHDS_IGNORE_FILENAME, PSYCHDS_IGNORE_CONTENT); - } - + // Persist the accumulated name sets and staged store so a later additive batch stays consistent. + setUsedArrayFilenames([...arrayNames]); + setUsedRawFilenames([...rawNames]); setConvertedStore(store); + setFiles(prev => (additive ? [...prev, ...batch] : batch)); setPhase('done'); }; @@ -367,9 +482,14 @@ const DataUpload: React.FC = ({ return ·; }; - // Drop into join key chooser, preserving the previously chosen keys + // Drop into join key chooser, preserving the previously chosen keys. Re-configuring reprocesses + // the whole accumulated dataset with the new keys as a fresh replace run (clears the store and + // rebuilds it), so batch is reset to every uploaded file and the mode to 'replace' — otherwise an + // additive re-run would double-stage the last batch. const enterReConfigureJoinKeys = (returnTo: Phase) => { setProceedAnyway(false); + setBatch(files); + setPendingMode('replace'); setJoinKeyReturnPhase(returnTo); setPhase('joinKeys'); }; @@ -394,16 +514,17 @@ const DataUpload: React.FC = ({ Optionally, upload your data folder to add new variables or refresh levels and ranges.

- or -
- + + {pickError &&

{pickError}

}
or -
- +

+ New files are added to your dataset.{' '} + +

+ {pickError &&

{pickError}

} + + {replaceConfirm && ( + setReplaceConfirm(null)} /> + )} ); @@ -481,16 +612,16 @@ const DataUpload: React.FC = ({ {/* Pickers */}
- or - - {files.length > 0 && sourceName && ( + {batch.length > 0 && sourceName && ( - {sourceName} ({files.length} file{files.length !== 1 ? 's' : ''}) + {sourceName} ({batch.length} file{batch.length !== 1 ? 's' : ''}) )} = ({ multiple style={{ display: 'none' }} onChange={handleFolderChange} + {...{ webkitdirectory: '' }} /> = ({ onChange={handleZipChange} />
- {pickError &&

{pickError}

} + {pickError &&

{pickError}

} + + {replaceConfirm && ( + setReplaceConfirm(null)} /> + )} {/* File list (before processing) */} {phase === 'ready' && ( <>
    - {files.map(f => ( + {batch.map(f => (
  • · {f.webkitRelativePath || f.name} @@ -529,7 +665,7 @@ const DataUpload: React.FC = ({ {/* Pre-flight spinner */} {phase === 'preflight' && ( -

    Reading files…

    +

    Reading files…

    )} {/* Join key chooser */} @@ -617,15 +753,26 @@ const DataUpload: React.FC = ({ {/* Per-file status (processing + done) */} {(phase === 'processing' || phase === 'done') && ( -
      - {fileStatuses.map((s, i) => ( -
    • - {statusIcon(s.status)} - {s.name} - {s.detail && {s.detail}} -
      • - ))} -
    + <> +

    + {(() => { + const total = fileStatuses.length; + const done = fileStatuses.filter(s => s.status !== 'pending' && s.status !== 'loading').length; + return phase === 'processing' + ? `Processing… ${done} of ${total} file${total !== 1 ? 's' : ''} processed.` + : `Done. ${done} of ${total} file${total !== 1 ? 's' : ''} processed.`; + })()} +

    +
      + {fileStatuses.map((s, i) => ( +
    • + {statusIcon(s.status)} + {s.name} + {s.detail && {s.detail}} +
      • + ))} +
    + )} {phase === 'done' && ( diff --git a/packages/frontend/src/pages/ProjectInfo.tsx b/packages/frontend/src/pages/ProjectInfo.tsx index c378861..01a5378 100644 --- a/packages/frontend/src/pages/ProjectInfo.tsx +++ b/packages/frontend/src/pages/ProjectInfo.tsx @@ -14,11 +14,26 @@ export const OPTIONAL_FIELDS: { key: string; label: string; hint: string; help?: help: 'Choose the option that matches your IRB approval or data-sharing agreement:\n• open — data can be shared publicly without restriction\n• open_deidentified — data can be shared after removing directly identifying information (names, dates of birth, etc.)\n• open_redacted — data can be shared after removing specific sensitive fields\n• private — data is not to be shared outside your team' }, ]; +export type MetadataLoadStatus = 'idle' | 'loading' | 'loaded' | 'error'; + export type ProjectInfoSession = { name: string; description: string; optional: Record; optionalOpen: boolean; + /** + * Outcome of loading an existing `dataset_description.json` into this session. Lives at the + * AppShell level (via the session) so it survives page remounts and gates whether the Data + * step is pre-completed and shown as "variables loaded from existing metadata" — a failed + * parse must not look like a successful load. + */ + loadStatus: MetadataLoadStatus; + /** + * Identity of the existing-metadata file this session was loaded from (name:size:lastModified), + * or null if none. The load effect runs exactly once per file identity: on remount, a matching + * token means the load already happened, so it is not re-run (which would clobber session edits). + */ + loadToken: string | null; }; export const emptyProjectInfoSession = (): ProjectInfoSession => ({ @@ -26,8 +41,31 @@ export const emptyProjectInfoSession = (): ProjectInfoSession => ({ description: '', optional: Object.fromEntries(OPTIONAL_FIELDS.map(f => [f.key, ''])), optionalOpen: false, + loadStatus: 'idle', + loadToken: null, }); +/** Stable identity for an uploaded file, used to load its metadata exactly once. */ +const fileIdentity = (file: File): string => `${file.name}:${file.size}:${file.lastModified}`; + +/** + * Writes the project-info fields (name, description, optional) into the metadata instance — + * shared by Continue and by the data-replace reset, which reloads existing metadata and then + * re-applies the user's edited fields on top. + */ +export function applyProjectInfoFields(meta: JsPsychMetadata, session: ProjectInfoSession): void { + meta.setMetadataField('name', session.name.trim()); + meta.setMetadataField('description', session.description.trim() || 'No description provided.'); + for (const { key } of OPTIONAL_FIELDS) { + const val = (session.optional[key] ?? '').trim(); + if (val) { + meta.setMetadataField(key, val); + } else { + meta.deleteMetadataField(key); + } + } +} + interface ProjectInfoProps { jsPsychMetadata: JsPsychMetadata; existingMetadataFile?: File; @@ -43,7 +81,14 @@ const ProjectInfo: React.FC = ({ onSessionChange, onComplete, }) => { - const [loadStatus, setLoadStatus] = useState<'idle' | 'loading' | 'loaded' | 'error'>('idle'); + const fileToken = existingMetadataFile ? fileIdentity(existingMetadataFile) : null; + // Has this exact file already been loaded (or attempted) into the session? If so, don't reload + // on remount — that would clobber edits the user made on other steps. + const alreadyAttempted = fileToken !== null && session.loadToken === fileToken; + + const [loadStatus, setLoadStatus] = useState( + alreadyAttempted ? session.loadStatus : existingMetadataFile ? 'loading' : 'idle', + ); const [error, setError] = useState(''); const [helpOpen, setHelpOpen] = useState(null); const [pendingUpload, setPendingUpload] = useState | null>(null); @@ -55,7 +100,11 @@ const ProjectInfo: React.FC = ({ const toggleHelp = (key: string) => setHelpOpen(prev => prev === key ? null : key); useEffect(() => { - if (!existingMetadataFile) return; + if (!existingMetadataFile || fileToken === null) return; + // Load the existing metadata exactly once per uploaded file. A matching token means this file + // was already loaded into the session on an earlier mount; re-running loadMetadata would + // resurrect deleted authors / revert edited variables and clobber the form session. + if (session.loadToken === fileToken) return; setLoadStatus('loading'); const reader = new FileReader(); reader.onload = () => { @@ -69,16 +118,22 @@ const ProjectInfo: React.FC = ({ description: jsPsychMetadata.getMetadataField('description') as string || '', optional: optionalVals, optionalOpen: OPTIONAL_FIELDS.some(f => !!jsPsychMetadata.getMetadataField(f.key)), + loadStatus: 'loaded', + loadToken: fileToken, }); setLoadStatus('loaded'); } catch { setLoadStatus('error'); setError('Failed to parse the metadata file — check that it is valid JSON.'); + // Record the attempt (so it isn't retried) and propagate the failure so the Data step + // isn't pre-completed or shown as "variables loaded from existing metadata". + onSessionChange({ ...session, loadStatus: 'error', loadToken: fileToken }); } }; reader.onerror = () => { setLoadStatus('error'); setError('Failed to read the file.'); + onSessionChange({ ...session, loadStatus: 'error', loadToken: fileToken }); }; reader.readAsText(existingMetadataFile); }, [existingMetadataFile]); @@ -144,19 +199,7 @@ const ProjectInfo: React.FC = ({ const handleContinue = () => { if (!session.name.trim()) { setError('Project name is required.'); return; } setError(''); - - jsPsychMetadata.setMetadataField('name', session.name.trim()); - jsPsychMetadata.setMetadataField('description', session.description.trim() || 'No description provided.'); - - for (const { key } of OPTIONAL_FIELDS) { - const val = (session.optional[key] ?? '').trim(); - if (val) { - jsPsychMetadata.setMetadataField(key, val); - } else { - jsPsychMetadata.deleteMetadataField(key); - } - } - + applyProjectInfoFields(jsPsychMetadata, session); onComplete(); }; @@ -382,7 +425,7 @@ const ProjectInfo: React.FC = ({ )} - {error &&

    {error}

    } + {error &&

    {error}

    } - - ), - emptyProjectInfoSession: () => ({ name: "", description: "", optional: {}, optionalOpen: false }), - OPTIONAL_FIELDS: [], -})); +jest.mock("../src/pages/ProjectInfo", () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const React = require("react"); + return { + __esModule: true, + // Real ProjectInfo reports a successful existing-metadata load into the session; the shell + // gates Data pre-completion on that, so the stub mirrors it when a file is present. + default: ({ onComplete, existingMetadataFile, session, onSessionChange }: any) => { + React.useEffect(() => { + if (existingMetadataFile) { + onSessionChange({ ...session, loadStatus: "loaded", loadToken: "tok" }); + } + }, []); + return ( +
    + ProjectInfo page + +
    + ); + }, + emptyProjectInfoSession: () => ({ + name: "", description: "", optional: {}, optionalOpen: false, loadStatus: "idle", loadToken: null, + }), + OPTIONAL_FIELDS: [], + applyProjectInfoFields: jest.fn(), + }; +}); // A fake staged store the DataUpload stub can push into the session, so Start Over cleanup can be // asserted. The `mock` prefix lets the hoisted jest.mock factory reference it (jest rule). const mockStagedClear = jest.fn(); -const mockStagedStore = { clear: mockStagedClear } as any; +const mockStagedStore = { clear: mockStagedClear, paths: () => [] } as any; jest.mock("../src/pages/DataUpload", () => ({ __esModule: true, diff --git a/packages/frontend/tests/AppShellIntegration.test.tsx b/packages/frontend/tests/AppShellIntegration.test.tsx new file mode 100644 index 0000000..19415ff --- /dev/null +++ b/packages/frontend/tests/AppShellIntegration.test.tsx @@ -0,0 +1,71 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import JsPsychMetadata from "@jspsych/metadata"; +import AppShell from "../src/components/AppShell"; + +// Integration test with the REAL ProjectInfo page inside the REAL AppShell (only the validator, +// which Review lazy-loads, would touch the network — and we never open Review here). Guards the +// state-loss bug where revisiting Project Info re-ran loadMetadata and clobbered session edits. + +const METADATA_JSON = JSON.stringify({ + name: "Loaded Study", + description: "A loaded description.", + schemaVersion: "Psych-DS 0.4.0", + variableMeasured: [ + { name: "rt", type: "PropertyValue", description: "reaction time" }, + { name: "stimulus", type: "PropertyValue", description: "the stimulus" }, + ], +}); + +function existingFile() { + return new File([METADATA_JSON], "dataset_description.json", { type: "application/json" }); +} + +describe("AppShell + real ProjectInfo (existing project)", () => { + test("loads existing metadata exactly once, even after revisiting Project Info", async () => { + const meta = new JsPsychMetadata(); + const loadSpy = jest.spyOn(meta, "loadMetadata"); + + render(); + + // First mount loads the file once and populates the form. + await screen.findByText(/Loaded from/); + expect(loadSpy).toHaveBeenCalledTimes(1); + expect(screen.getByRole("textbox", { name: /Project name/ })).toHaveValue("Loaded Study"); + + // Continue — existing metadata is loaded, so Data is pre-completed and we skip to Variables. + await userEvent.click(screen.getByRole("button", { name: "Continue →" })); + await screen.findByText(/Review and edit the variables/); + + // Simulate an edit made on another step: delete a variable directly on the instance. + meta.deleteVariable("rt"); + expect(meta.getVariableNames()).not.toContain("rt"); + + // Navigate back to Project Info via the sidebar — this remounts the page. + await userEvent.click(screen.getByRole("button", { name: /Project Info/ })); + await screen.findByRole("textbox", { name: /Project name/ }); + + // loadMetadata must NOT have run again (it would resurrect the deleted variable and clobber edits). + expect(loadSpy).toHaveBeenCalledTimes(1); + expect(meta.getVariableNames()).not.toContain("rt"); + // Session preserved — the name field still shows the loaded value. + expect(screen.getByRole("textbox", { name: /Project name/ })).toHaveValue("Loaded Study"); + }); + + test("a failed metadata parse does not pre-complete Data or claim variables were loaded", async () => { + const meta = new JsPsychMetadata(); + const badFile = new File(["not valid json {"], "dataset_description.json"); + + render(); + + await screen.findByText(/Failed to parse the metadata file/); + + // Continuing must land on the Data step (not skip it), and Data must NOT show the + // "variables loaded from existing metadata" banner. + await userEvent.type(screen.getByRole("textbox", { name: /Project name/ }), "Manual"); + await userEvent.click(screen.getByRole("button", { name: "Continue →" })); + + expect(await screen.findByText(/Select your data folder/)).toBeInTheDocument(); + expect(screen.queryByText(/Variables loaded from existing metadata/)).not.toBeInTheDocument(); + }); +}); diff --git a/packages/frontend/tests/DataUpload.test.tsx b/packages/frontend/tests/DataUpload.test.tsx index 09e7b31..c2ac934 100644 --- a/packages/frontend/tests/DataUpload.test.tsx +++ b/packages/frontend/tests/DataUpload.test.tsx @@ -1,9 +1,10 @@ import { render, screen, fireEvent, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import JSZip from "jszip"; -import { analyzeJoinKeys } from "@jspsych/metadata"; +import { analyzeJoinKeys, buildPsychDSDataFiles } from "@jspsych/metadata"; import DataUpload, { emptyDataSession } from "../src/pages/DataUpload"; import type { DataSession } from "../src/pages/DataUpload"; +import { createStagedFileStore } from "../src/staging/stagedFileStore"; jest.mock("@jspsych/metadata", () => ({ __esModule: true, @@ -450,7 +451,10 @@ describe("DataUpload", () => { await waitFor(() => { const lastCall = onSessionChange.mock.calls.at(-1)?.[0] as DataSession; expect(lastCall?.files).toHaveLength(1); - expect(lastCall.files[0].name).toBe("data/sub01.csv"); + // Zip entries now use their basename for the File name (the in-archive path is kept on + // webkitRelativePath), so a compliant/flat name reaches the Psych-DS builder. + expect(lastCall.files[0].name).toBe("sub01.csv"); + expect(lastCall.files[0].webkitRelativePath).toBe("data/sub01.csv"); }); }); @@ -468,4 +472,163 @@ describe("DataUpload", () => { }); }); }); + + // ── session round-trip (fix 4) ───────────────────────────────────────────── + + describe("session round-trip", () => { + test("restores 'ready' with the file list when files were picked but not processed", () => { + const session: DataSession = { + ...emptyDataSession, + files: [makeFile("sub01.csv"), makeFile("sub02.csv")], + }; + render(); + + // Not dataProcessed and not existing-metadata, but session.files exist → 'ready', list shown. + expect(screen.getByRole("button", { name: "Process files" })).toBeInTheDocument(); + expect(screen.getByText("sub01.csv")).toBeInTheDocument(); + expect(screen.getByText("sub02.csv")).toBeInTheDocument(); + }); + }); + + // ── navigation lock while processing (fix 3) ─────────────────────────────── + + describe("processing busy state", () => { + test("reports busy=true while a run is in flight and busy=false when it finishes", async () => { + const onBusyChange = jest.fn(); + let releaseGenerate: () => void = () => {}; + meta.generate.mockImplementation( + () => new Promise((resolve) => { releaseGenerate = () => resolve(); }), + ); + + const { container } = render(); + const input = container.querySelector("input[multiple]") as HTMLInputElement; + fireEvent.change(input, { target: { files: [makeFile("sub01.csv", "rt\n1")] } }); + await userEvent.click(screen.getByRole("button", { name: "Process files" })); + + // generate() is pending, so the run is still in flight — the shell must lock navigation. + await waitFor(() => expect(onBusyChange).toHaveBeenLastCalledWith(true)); + + releaseGenerate(); + await waitFor(() => expect(onBusyChange).toHaveBeenLastCalledWith(false)); + }); + }); + + // ── batch consistency: additive vs replace (fix 2) ───────────────────────── + + describe("batch consistency", () => { + async function storeWith(paths: string[]) { + const store = createStagedFileStore({ forceMemory: true }); + for (const p of paths) await store.write(p, "x"); + return store; + } + + test("'Upload additional files' stages the new batch alongside the first (no clear)", async () => { + // Make the builder emit one datafile per input and record its name, mirroring real behaviour. + // Once-scoped so it doesn't leak into other tests (which expect the default empty result). + (buildPsychDSDataFiles as jest.Mock).mockImplementationOnce( + ({ base, usedArrayFilenames }: { base: string; usedArrayFilenames: Set }) => { + const filename = `${base}_data.csv`; + usedArrayFilenames.add(filename); + return [{ filename, content: "x" }]; + }, + ); + + const store = await storeWith(["data/subject-a_data.csv"]); + const session: DataSession = { + ...emptyDataSession, + files: [makeFile("a.json", "[]")], + convertedStore: store, + usedArrayFilenames: ["subject-a_data.csv"], + usedRawFilenames: ["a.json"], + fileStatuses: [{ name: "a.json", status: "success" }], + }; + + render(); + + // "Upload additional files" (additive) — does NOT confirm/reset. + await userEvent.click(screen.getByRole("button", { name: "Choose folder" })); + const input = document.querySelector("input[multiple]") as HTMLInputElement; + fireEvent.change(input, { target: { files: [makeFile("b.json", "[]")] } }); + await userEvent.click(screen.getByRole("button", { name: "Process files" })); + await screen.findByRole("button", { name: "Continue →" }); + + // The store keeps batch A's output and gains batch B's — both are present. + expect(store.paths()).toEqual(expect.arrayContaining([ + "data/subject-a_data.csv", + "data/subject-b_data.csv", + ])); + // Used-name sets persisted across batches (seeded from the session, extended by the new batch). + const last = onSessionChange.mock.calls.at(-1)?.[0] as DataSession; + expect(last.usedArrayFilenames).toEqual(expect.arrayContaining([ + "subject-a_data.csv", + "subject-b_data.csv", + ])); + expect(last.usedRawFilenames).toEqual(expect.arrayContaining(["a.json", "b.json"])); + }); + + test("'Replace all data' confirms, clears the store, and fires the metadata reset", async () => { + const store = await storeWith(["data/subject-a_data.csv"]); + const clearSpy = jest.spyOn(store, "clear"); + const onResetMetadata = jest.fn().mockResolvedValue(undefined); + const session: DataSession = { + ...emptyDataSession, + files: [makeFile("a.json", "[]")], + convertedStore: store, + usedArrayFilenames: ["subject-a_data.csv"], + }; + + render(); + + await userEvent.click(screen.getByRole("button", { name: "Replace all data instead" })); + // A destructive replace over staged data asks first. + expect(screen.getByRole("alertdialog")).toBeInTheDocument(); + await userEvent.click(screen.getByRole("button", { name: "Yes, replace" })); + + await waitFor(() => expect(onResetMetadata).toHaveBeenCalledTimes(1)); + expect(clearSpy).toHaveBeenCalled(); + }); + + test("Cancel on the replace confirm leaves the staged data intact", async () => { + const store = await storeWith(["data/subject-a_data.csv"]); + const clearSpy = jest.spyOn(store, "clear"); + const onResetMetadata = jest.fn(); + const session: DataSession = { + ...emptyDataSession, + files: [makeFile("a.json", "[]")], + convertedStore: store, + }; + render(); + + await userEvent.click(screen.getByRole("button", { name: "Replace all data instead" })); + await userEvent.click(screen.getByRole("button", { name: "Cancel" })); + + expect(onResetMetadata).not.toHaveBeenCalled(); + expect(clearSpy).not.toHaveBeenCalled(); + }); + }); + + // ── zip basename handling (fix 8) ────────────────────────────────────────── + + describe("zip basename handling", () => { + test("uses the entry basename for nested paths and reads entries as blobs", async () => { + const asyncMock = jest.fn().mockResolvedValue("rt\n1"); + mockLoadAsync.mockResolvedValue({ + files: { "nested/dir/sub01.csv": { dir: false, name: "nested/dir/sub01.csv", async: asyncMock } }, + }); + + const { container } = render(); + fireEvent.change(container.querySelector("input[accept='.zip']")!, { + target: { files: [makeFile("exp.zip")] }, + }); + + await screen.findByRole("button", { name: "Process files" }); + // Extracted as a blob (not text). + expect(asyncMock).toHaveBeenCalledWith("blob"); + await waitFor(() => { + const last = onSessionChange.mock.calls.at(-1)?.[0] as DataSession; + expect(last.files[0].name).toBe("sub01.csv"); + expect(last.files[0].webkitRelativePath).toBe("nested/dir/sub01.csv"); + }); + }); + }); }); diff --git a/packages/frontend/tests/Sidebar.test.tsx b/packages/frontend/tests/Sidebar.test.tsx index 00a8511..4637e48 100644 --- a/packages/frontend/tests/Sidebar.test.tsx +++ b/packages/frontend/tests/Sidebar.test.tsx @@ -100,6 +100,23 @@ describe("Sidebar", () => { }); }); + // ── navigation lock (while data is processing) ───────────────────────────── + + describe("locked", () => { + test("locks every step and Start over so a run can't be orphaned", async () => { + render( + true, locked: true })} />, + ); + // Even otherwise-navigable steps are disabled while locked. + expect(screen.getByRole("button", { name: /Project Info/ })).toBeDisabled(); + expect(screen.getByRole("button", { name: /^Data/ })).toBeDisabled(); + expect(screen.getByRole("button", { name: "← Start over" })).toBeDisabled(); + + await userEvent.click(screen.getByRole("button", { name: /^Data/ })); + expect(onNavigate).not.toHaveBeenCalled(); + }); + }); + // ── start over dialog ───────────────────────────────────────────────────── describe("start over dialog", () => { diff --git a/packages/frontend/tests/setup.ts b/packages/frontend/tests/setup.ts index 8c0c3e3..70f9497 100644 --- a/packages/frontend/tests/setup.ts +++ b/packages/frontend/tests/setup.ts @@ -36,6 +36,22 @@ if (typeof (Blob.prototype as any).arrayBuffer !== 'function') { URL.createObjectURL = jest.fn(() => "blob:mock-url"); URL.revokeObjectURL = jest.fn(); +// jsdom doesn't implement .showModal/close (used by Sidebar and PreviewDrawer). Stub them +// so modal dialogs render as open and fire their close event on programmatic close. +if (typeof HTMLDialogElement !== "undefined") { + HTMLDialogElement.prototype.showModal = jest + .fn() + .mockImplementation(function (this: HTMLDialogElement) { + this.setAttribute("open", ""); + }); + HTMLDialogElement.prototype.close = jest + .fn() + .mockImplementation(function (this: HTMLDialogElement) { + this.removeAttribute("open"); + this.dispatchEvent(new Event("close")); + }); +} + beforeEach(() => { localStorage.clear(); }); From 665afadb40a8b0104a7d8094518b7b3d4e5018c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 21:08:45 +0000 Subject: [PATCH 4/5] A11y & UX pass: modal preview, labels, safe zip name Convert PreviewDrawer to a native opened with showModal() so it gets a real focus trap, Escape-to-close, and an inert backdrop (matching Sidebar), replacing the div+role="dialog" with no focus management. Associate every Variables control with a label (Type select, description textarea, and levels/range groups via aria-labelledby). Review: sanitize the ${projectName}.zip filename (strip characters illegal in filenames), disable the zip button while a build is in flight, and wrap validation results in an aria-live region. Tests: PreviewDrawer modal/Escape/backdrop behaviour under the jsdom dialog stub. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg --- .../src/components/PreviewDrawer.module.css | 17 ++++---- .../frontend/src/components/PreviewDrawer.tsx | 40 +++++++++++++------ packages/frontend/src/pages/Review.tsx | 32 ++++++++++++--- packages/frontend/src/pages/Variables.tsx | 20 +++++++--- .../frontend/tests/PreviewDrawer.test.tsx | 31 ++++++++++++-- 5 files changed, 105 insertions(+), 35 deletions(-) diff --git a/packages/frontend/src/components/PreviewDrawer.module.css b/packages/frontend/src/components/PreviewDrawer.module.css index 4e97c92..980638e 100644 --- a/packages/frontend/src/components/PreviewDrawer.module.css +++ b/packages/frontend/src/components/PreviewDrawer.module.css @@ -1,18 +1,17 @@ -.backdrop { - position: fixed; - inset: 0; - background: rgba(0, 0, 0, 0.35); - z-index: 30; -} - .drawer { position: fixed; top: 0; right: 0; bottom: 0; + left: auto; width: 420px; max-width: calc(100vw - var(--sidebar-w)); /* never wider than the content area */ + max-height: 100vh; + margin: 0; /* override the UA-centred dialog placement */ + padding: 0; background: var(--c-bg-raised); + color: inherit; + border: none; border-left: 1px solid var(--c-border); z-index: 31; display: flex; @@ -20,6 +19,10 @@ animation: slideIn 0.2s cubic-bezier(0, 0, 0.2, 1); } +.drawer::backdrop { + background: rgba(0, 0, 0, 0.35); +} + @keyframes slideIn { from { transform: translateX(100%); } to { transform: translateX(0); } diff --git a/packages/frontend/src/components/PreviewDrawer.tsx b/packages/frontend/src/components/PreviewDrawer.tsx index 5db8813..ebeba00 100644 --- a/packages/frontend/src/components/PreviewDrawer.tsx +++ b/packages/frontend/src/components/PreviewDrawer.tsx @@ -1,4 +1,4 @@ -import { useMemo, useEffect } from 'react'; +import { useMemo, useLayoutEffect, useRef } from 'react'; import JsPsychMetadata from '@jspsych/metadata'; import JsonViewer from './JsonViewer'; import styles from './PreviewDrawer.module.css'; @@ -11,25 +11,39 @@ interface PreviewDrawerProps { const PreviewDrawer: React.FC = ({ jsPsychMetadata, onClose }) => { // Fresh snapshot on each open (component mounts when drawer opens) const data = useMemo(() => jsPsychMetadata.getMetadata(), []); + const dialogRef = useRef(null); - useEffect(() => { + // A native opened with showModal() gives a real focus trap, Escape-to-close, and an + // inert backdrop for free (mirroring Sidebar's confirm dialog). Escape fires 'cancel' → onClose. + useLayoutEffect(() => { + const dialog = dialogRef.current; + if (dialog && !dialog.open) dialog.showModal(); document.body.style.overflow = 'hidden'; return () => { document.body.style.overflow = ''; }; }, []); + // A click landing on the dialog element itself (not its content) is a backdrop click → close. + const handleClick = (e: React.MouseEvent) => { + if (e.target === dialogRef.current) onClose(); + }; + return ( - <> -