Skip to content

fix(import): support CSV files without a header row - #712

Open
2160039878-cyber wants to merge 2 commits into
libredb:mainfrom
2160039878-cyber:fix/csv-headerless-import-689
Open

fix(import): support CSV files without a header row#712
2160039878-cyber wants to merge 2 commits into
libredb:mainfrom
2160039878-cyber:fix/csv-headerless-import-689

Conversation

@2160039878-cyber

@2160039878-cyber 2160039878-cyber commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Description

Importing a CSV without headers currently consumes the first data row as column names. Add a First row is header checkbox to the CSV preview, checked by default. Unchecking it generates column_1, column_2, etc. and preserves the first row in the preview and generated SQL.

Returning from Configure Import and toggling the header option also preserves edited target column names, including an off/on round trip.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Test addition or update

Related Issue

Closes #689

Changes Made

  • Add an optional, default-enabled header argument to parseCSV and reparse the original CSV on each toggle.
  • Update mappings only when the parsed headers change. Default newly introduced columns while retaining existing entries for both interpretations, so returning to a previous header mode restores the user's edits.
  • Reset the header option and mappings for the next upload.
  • Cover headerless and single-row input, quoted/empty fields, repeated toggles, mapping preservation, reset, and generated SQL.

Testing

Latest review revision: d691d46e95dcb237630f91db3b0af4db186088d0.

  • The three new mapping regression cases failed against the previous implementation before the fix: ordinary headers, identical generated/header names, and partially shared names.
  • bun run test:components --pass-with-no-tests -t 'DataImportModal': 47 passed, 0 failed, using the repository's isolation groups. Other component tests were filtered out.
  • The mapping cases verify surviving entries, defaults for newly introduced columns, the complete off/on round trip, and the SQL sent to the import callback. The reset test verifies that mappings do not carry over to a new file.
  • All of these local checks passed:
bun run format
bun run lint
bun run typecheck
bun run knip
bun run readme:check
bun run chart:check
bun run channels:showcase:check
bun run security:check
bun run build
bun run build:lib
bun run attw

git diff --check also passed. Lint reports existing warnings with zero errors.

Complete CI and local limitations

Official CI for this revision completed successfully. All nine executable jobs passed: full unit/integration/component tests and the 100% line-coverage gate, lint/typecheck/build, Helm, Node 24/26 smoke, browser and subpath E2E, PostgreSQL smoke, and tarball/npx installation tests. The PR reports 20 successful checks and two expected skips (Image Scan and SonarCloud). CodeQL, Secret Scan, dependency scanning, platform rules, GitGuardian, Socket, Semgrep, and Codecov patch coverage also passed.

The complete bun run test, bun run test:coverage, bun run coverage:check, Helm dependency/lint commands, and browser E2E were not rerun locally. This Windows host lacks Helm and the chart's PostgreSQL dependency; the earlier full component run also encountered Windows SQLite cleanup EBUSY in unchanged agent tests. Official Linux CI verifies these stages. No repository-wide coverage claim is based on the focused local run.

Test Environment

  • LibreDB Studio Version: 0.15.0
  • Browser: happy-dom component tests locally; Playwright in official CI
  • OS: Windows locally; Ubuntu in official CI
  • Node.js/Bun Version: Node.js 24.18.1 / Bun 1.4.2
  • Database Type: Local import tests assert SQL with a mocked callback; official CI includes PostgreSQL smoke

Checklist

  • Followed the project's code style and reviewed the complete diff
  • Added regression tests that fail without the fix
  • Relevant new and existing component tests pass locally
  • The latest revision passes the required CI 100% line-coverage gate
  • Provider triad: not applicable; no provider files changed

Additional Notes

This is AI-assisted work. No dependencies were added.

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cevheri cevheri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there. Everything #689 asked for is here, and the tests are real: reverting only DataImportModal.tsx turns five of your six new cases red, so they measure the fix instead of agreeing with it. The checkbox markup matches the existing ConnectionModal SSH pattern exactly, which is the right call over the shadcn primitive.

One change before I merge. The toggle handler always rebuilds columnMapping to identity, so it silently discards target column names the user has already typed. The path is real: Configure Import, rename a target column, Back at line 597, then check and uncheck the box. The headers are byte-identical after that round trip and the mapping is gone anyway, with nothing on screen saying so. I reproduced it: the target for name came back as name instead of full_name.

That is the same shape as the defect #689 is about, a silent loss with no indication, so it is worth closing rather than inheriting. Cheapest fix: rebuild the mapping only when the header set actually changed, and keep the existing entry for any header that survives. A test for the round trip belongs with it.

Two things I found are not yours and I am not asking for them here: a ragged CSV emits an INSERT whose value tuple is wider than the column list, and header text reaches the generated SQL as a bare identifier. Both predate this PR on the header path, and I am tracking them separately.

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.

CSV import always treats the first row as a header

2 participants