Skip to content

fix(metadata,cli,frontend): ingest jsPsych CSVs with unescaped quotes#132

Merged
jodeleeuw merged 4 commits into
mainfrom
fix/csv-relax-quotes
Jul 21, 2026
Merged

fix(metadata,cli,frontend): ingest jsPsych CSVs with unescaped quotes#132
jodeleeuw merged 4 commits into
mainfrom
fix/csv-relax-quotes

Conversation

@Mandyx22

Copy link
Copy Markdown
Contributor

Problem

Some jsPsych experiments export the stimulus column as unquoted HTML containing literal " (e.g. <div class = "EncodingBox">), which violates strict RFC-4180 quoting. Surfaced on a real 1258-file OSF working-memory dataset (osf.io/phxq4) where the tool read 0 filescsv-parse threw Invalid Opening Quote and dropped every file. Both the CLI and the browser uploader were affected.

There were two layers to fix:

  1. Read: the parser rejected the file outright.
  2. Write: even once read, the data file is copied verbatim, so the malformed quotes would land in the Psych-DS data/ payload and the validator (which also strict-parses CSV) rejected it with CSV_FORMATTING_ERROR.

Changes

  • parseCSV now sets relax_quotes: true, so a quote inside an unquoted field no longer throws and drops the file; the HTML is kept intact.
  • New parseCSVForWrite helper returns the parsed rows plus a verbatimSafe flag (strict-parse probe). The CLI and frontend use it so:
    • a clean CSV still keeps its exact bytes (written verbatim — unchanged behavior), and
    • a file that only parsed thanks to quote relaxation is re-serialised to well-formed CSV, so the written file is valid.

Result (verified end to end)

Before After
Library/CLI read 0 files all files read
Written data file verbatim (malformed) re-serialised, quotes escaped
Psych-DS validation CSV_FORMATTING_ERROR ✅ 0 errors
Frontend (real DataUpload) every file error success, output validates (0 errors)

Clean CSVs are unaffected (still verbatim) — only files that were previously 100% unreadable change.

Tests

  • Metadata regression: an unquoted field containing " now parses instead of dropping the file.
  • Frontend wiring test: clean → verbatim, malformed → re-serialised and strictly re-parseable.
  • Updated the DataUpload mock for the new helper.
  • Full suites green: metadata 251, CLI 160, frontend 247.

🤖 Generated with Claude Code

jsPsych can export the `stimulus` column as unquoted HTML containing
literal `"` (e.g. `<div class = "EncodingBox">`), which violates strict
RFC-4180 quoting. Previously csv-parse threw "Invalid Opening Quote" and
the entire file was dropped, making such datasets unreadable end to end
(observed on a 1258-file OSF working-memory dataset: 0 files read).

- parseCSV sets `relax_quotes: true` so the row parses instead of being
  rejected.
- New `parseCSVForWrite` reports whether the content was already strictly
  valid CSV. The CLI and frontend use it so a clean file keeps its exact
  bytes (verbatim), while a file that only parsed thanks to relaxation is
  re-serialised to well-formed CSV — otherwise the malformed bytes land
  in the Psych-DS data/ payload and the validator rejects them with
  CSV_FORMATTING_ERROR.

Net: these datasets now ingest and pass Psych-DS validation through both
the library/CLI and the browser uploader. Adds regression tests for the
parse and the re-serialise-on-write behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: fd55433

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@jspsych/metadata Patch
@jspsych/metadata-cli Patch
frontend Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jsPsych stimulus HTML often contains both a literal `"` and a `,`. relax_quotes
keeps the quote literal but the comma still splits the field, so csv-parse throws
"Invalid Record Length" and the file is still dropped. Documents the gap as a
test.failing so CI stays green and the spec flips to a hard failure once
comma-bearing stimuli ingest correctly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jodeleeuw

Copy link
Copy Markdown
Member

The fix handles quote-only fields but not the common quote+comma case.

relax_quotes: true stops INVALID_OPENING_QUOTE, but it does not stop an inner comma from splitting an unquoted field. jsPsych stimulus HTML frequently contains both a literal " and a , (instruction text, lists, key prompts), e.g.:

rt,stimulus,trial_type,trial_index
300,<p>Press "F", "J" to respond</p>,html-keyboard-response,0

The stimulus's inner comma makes the row 5 fields against 4 headers, so csv-parse throws Invalid Record Length: columns length is 4, got 5 even with relaxation. In parseCSVForWrite the strict probe throws, the catch retries leniently, and the lenient parse throws the same record-length error — which propagates uncaught. Since the loop in processDirectory (packages/cli/src/data.ts) isn't wrapped in try/catch, that aborts the whole run, reproducing the "0 files read" symptom the PR targets. The existing regression test only covers a comma-free stimulus, so this gap isn't caught.

I pushed a target spec documenting it (commit 0dc55d0, packages/metadata/tests/csv-input.stress.test.ts) as test.failing so CI stays green; it will flip to a hard failure once comma-bearing stimuli ingest correctly, prompting removal of the marker.

Note "make it parse" isn't free: the only csv-parse knob that lets these through is relax_column_count, which would then silently mis-split the stimulus into the wrong columns and re-serialise valid-but-wrong CSV that passes Psych-DS validation — trading a loud drop for silent corruption. A robust fix probably needs the field quoted at the jsPsych source or a smarter pre-pass. Worth deciding deliberately before claiming the end-to-end fix.

jodeleeuw added a commit that referenced this pull request Jul 20, 2026
* docs: add repository review and prioritized improvement plan

Full review of all three packages, the open PRs (#132, #133, #47), and
repo infrastructure. Documents verified data-loss bugs, performance
issues affecting #95, CI gaps, test coverage gaps, and feature
opportunities, organized into six phased workstreams.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg

* docs: note open Dependabot alerts and behind-major dependency stack in plan

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg

---------

Co-authored-by: Claude <noreply@anthropic.com>
jodeleeuw and others added 2 commits July 20, 2026 21:25
…commas; strict-first parsing

Closes the gap documented by the former test.failing: a stimulus like
'<p>Press "F", "J" to respond</p>' over-splits under relax_quotes and
threw CSV_RECORD_INCONSISTENT_COLUMNS, dropping the whole file.

- parseCSV now escalates leniency per file: strict RFC-4180 -> relax_quotes
  -> row-repair fallback. Strict-first means well-formed files are never
  exposed to lenient reinterpretation (review concern about relax_quotes
  silently tolerating malformed quoted fields).
- The repair pass re-reads over-split rows with two rules: a closing quote
  only counts before a delimiter/EOL (recovers half-quoted fields), and on
  retry a comma followed by a space is literal text (machine-written jsPsych
  delimiters are never followed by a space; prose/HTML commas are). A row
  that still doesn't fit the header is skipped with a warning - one bad row
  costs that row, not the file.
- CLI and frontend now warn when a file parsed leniently and is being
  re-serialised, so rewritten bytes are never silent.
- test.failing flipped to a passing regression test; new coverage: quoted
  field with unescaped quotes+commas (repaired, outer quotes stripped),
  quote-only quoted field (pinned relax_quotes behavior), irreparable row
  skipped while the file survives, and a CLI end-to-end test asserting the
  rewritten output strict-parses with the stimulus intact.
- Changeset updated (adds frontend bump, documents the leniency ladder and
  the malformed-file multi-parse tradeoff); trailing newline fixed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jodeleeuw

Copy link
Copy Markdown
Member

Maintainer update (discussed with @jodeleeuw): we merged current main into this branch and pushed a follow-up commit that closes the quote+comma gap this PR's test.failing documented — thanks for adding that marker, it became the acceptance test.

What changed:

  • parseCSV now escalates leniency only as far as a file needs: strict RFC-4180 first, then relax_quotes, then a new row-repair fallback. Strict-first also addresses the earlier review concern about relax_quotes being applied unconditionally to well-formed files.
  • The repair pass re-reads over-split rows with two rules: a closing quote only counts when followed by a delimiter/end-of-line, and (on retry) a comma followed by a space is treated as literal text — machine-written jsPsych delimiters are never followed by a space, while prose/HTML commas (Press "F", "J") almost always are. A row that still can't be matched to the header is skipped with a warning, so one bad row costs that row, not the file.
  • The CLI and web wizard now print a warning whenever a file parsed leniently and gets re-serialised, so rewritten bytes are never silent.
  • test.failing is flipped to a passing regression test, with new coverage for the half-quoted variant (quoted field, unescaped inner quotes + commas), the quote-only quoted-field edge case, an irreparable-row-is-skipped case, and a CLI end-to-end test asserting the rewritten output strict-parses with the stimulus intact.
  • Changeset updated (adds the frontend bump and documents the leniency ladder + the malformed-file multi-parse tradeoff).

Note for #133: since it's based on this branch, it will need this merged in — happy to help with that rebase.

🤖 Generated with Claude Code

@jodeleeuw
jodeleeuw merged commit 326af48 into main Jul 21, 2026
3 checks passed
@jodeleeuw
jodeleeuw deleted the fix/csv-relax-quotes branch July 21, 2026 01:41
jodeleeuw added a commit that referenced this pull request Jul 21, 2026
… trigger; cover the rename-plan path

Post-merge fixes after rebasing onto main (which now contains the final #132):

- Docs: 12 pages (docs/, website/docs/, package READMEs) said CSV inputs are
  never duplicated under data/raw/ — now describe the actual rule: the
  original is preserved whenever the output is not a byte-for-byte,
  same-named copy of the input (converted JSON, re-serialised CSVs,
  renamed CSVs), with the verbatim same-named clean CSV as the only no-raw
  case (maintainer-approved behavior).
- Test: the interactive CLI reaches processFile through a renamePlan
  whenever any file needs renaming — the flagship real-world path, which
  had no coverage. New test drives processDirectory with a planRenames()
  plan and asserts the renamed CSV's original lands in data/raw/.
- Test hygiene: mockIsValid implementation overrides now restored after
  each test (clearAllMocks does not reset implementations).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jodeleeuw
jodeleeuw restored the fix/csv-relax-quotes branch July 21, 2026 01:47
@jodeleeuw
jodeleeuw deleted the fix/csv-relax-quotes branch July 21, 2026 01:47
jodeleeuw added a commit that referenced this pull request Jul 21, 2026
… is transformed (#133)

* fix(metadata,cli,frontend): ingest jsPsych CSVs with unescaped quotes

jsPsych can export the `stimulus` column as unquoted HTML containing
literal `"` (e.g. `<div class = "EncodingBox">`), which violates strict
RFC-4180 quoting. Previously csv-parse threw "Invalid Opening Quote" and
the entire file was dropped, making such datasets unreadable end to end
(observed on a 1258-file OSF working-memory dataset: 0 files read).

- parseCSV sets `relax_quotes: true` so the row parses instead of being
  rejected.
- New `parseCSVForWrite` reports whether the content was already strictly
  valid CSV. The CLI and frontend use it so a clean file keeps its exact
  bytes (verbatim), while a file that only parsed thanks to relaxation is
  re-serialised to well-formed CSV — otherwise the malformed bytes land
  in the Psych-DS data/ payload and the validator rejects them with
  CSV_FORMATTING_ERROR.

Net: these datasets now ingest and pass Psych-DS validation through both
the library/CLI and the browser uploader. Adds regression tests for the
parse and the re-serialise-on-write behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli,frontend): preserve original under data/raw/ whenever output is transformed

Broaden raw-original preservation beyond JSON. Previously both the CLI
(processFile) and the web wizard (runGenerate) kept the untouched input
under data/raw/ only for JSON/JSONL. Now they preserve it whenever the
Psych-DS output is not a verbatim, same-named copy of the input:

  transformed = isJsonDataExt(ext) || !csvVerbatimEligible || mainName !== inputName

This newly covers CSV inputs that were renamed to a compliant name
(e.g. mydata.csv -> subject-x_data.csv) and CSV inputs that had to be
re-serialised (malformed quotes repaired in #132). A clean CSV written
byte-for-byte under its own compliant name is the only no-raw case, so
nothing is duplicated. The existing flat-dir disambiguation and root
.psychds-ignore (so the validator skips data/raw/) apply to these too.

Tested end-to-end on the real OSF phxq4 dataset (unquoted stimulus HTML
with literal quotes): files read, re-serialised output in data/, malformed
originals preserved in data/raw/. Adds 3 CLI + 3 frontend tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs,test(preserve-raw): update all data/raw docs for the transformed trigger; cover the rename-plan path

Post-merge fixes after rebasing onto main (which now contains the final #132):

- Docs: 12 pages (docs/, website/docs/, package READMEs) said CSV inputs are
  never duplicated under data/raw/ — now describe the actual rule: the
  original is preserved whenever the output is not a byte-for-byte,
  same-named copy of the input (converted JSON, re-serialised CSVs,
  renamed CSVs), with the verbatim same-named clean CSV as the only no-raw
  case (maintainer-approved behavior).
- Test: the interactive CLI reaches processFile through a renamePlan
  whenever any file needs renaming — the flagship real-world path, which
  had no coverage. New test drives processDirectory with a planRenames()
  plan and asserts the renamed CSV's original lands in data/raw/.
- Test hygiene: mockIsValid implementation overrides now restored after
  each test (clearAllMocks does not reset implementations).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Josh de Leeuw <josh.deleeuw@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants