Skip to content

fix(moss-cli): keep newlines in quoted csv fields on doc import - #488

Open
RyanRana wants to merge 1 commit into
usemoss:mainfrom
RyanRana:fix-csv-multiline-import
Open

fix(moss-cli): keep newlines in quoted csv fields on doc import#488
RyanRana wants to merge 1 commit into
usemoss:mainfrom
RyanRana:fix-csv-multiline-import

Conversation

@RyanRana

@RyanRana RyanRana commented Jul 24, 2026

Copy link
Copy Markdown

moss doc add -f file.csv fed content.splitlines() to csv.DictReader, so a newline inside a quoted text field was dropped and the words around it got glued together ("line oneline two"). now parses from a stream, validates the header, strips a utf-8 bom, and reports real line numbers. tests added.

@CLAassistant

CLAassistant commented Jul 24, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codex review

No issues found.

@HarshaNalluru

Copy link
Copy Markdown
Contributor

@RyanRana sign the CLA #488 (comment)

@RyanRana

Copy link
Copy Markdown
Author

done!

@HarshaNalluru
HarshaNalluru requested a review from Copilot July 27, 2026 16:40
HarshaNalluru
HarshaNalluru previously approved these changes Jul 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes moss doc add -f file.csv CSV importing so that newlines inside quoted CSV fields are preserved (instead of being lost via splitlines()), while improving CSV validation and error reporting in the moss-cli import path.

Changes:

  • Parse CSV from a stream (io.StringIO) instead of splitlines() so quoted multi-line fields remain intact.
  • Validate/normalize the CSV header (trim whitespace, require id and text) and improve error messages with real CSV line numbers.
  • Add pytest coverage for multi-line fields, header trimming/validation, and line-numbered error reporting.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/moss-cli/src/moss_cli/documents.py Switch CSV parsing to a stream, validate headers, and report CSV line numbers; refactor JSON parsing for metadata/embedding columns.
packages/moss-cli/tests/test_documents.py Add tests covering multi-line quoted fields, header trimming, missing columns, short rows, and line-numbered errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/moss-cli/src/moss_cli/documents.py Outdated
Comment thread packages/moss-cli/src/moss_cli/documents.py
Comment thread packages/moss-cli/src/moss_cli/documents.py
csv.DictReader was fed content.splitlines(), so newlines inside quoted fields were dropped and words on either side were joined. parse from a stream instead, validate the header, strip a utf-8 bom, and report real file line numbers in errors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@RyanRana
RyanRana force-pushed the fix-csv-multiline-import branch from 59c012a to 27cb157 Compare July 27, 2026 17:00
@RyanRana

Copy link
Copy Markdown
Author

Pushed a fix and squashed to a single commit.

The blocking codex-review finding was correct — the Apply suggestions from code review commit landed reader = csv.DictReader(...) dedented outside _parse_csv_docs, which made the module a syntax error and failed python-lint. That is fixed, and history is now one clean commit on top of 4027809.

All required checks are green. Locally, ruff==0.15.22 check . passes and all 7 tests in packages/moss-cli/tests/test_documents.py pass. (The two failures in tests/test_completions.py are pre-existing and untouched by this PR; moss-cli tests do not run in CI.)

@r4ghu @ashvathsureshkumar review requests are still pending — ready for another look. @HarshaNalluru your review was dismissed by the force-push; I do not have permission to re-request it from a fork, so flagging here.

@RyanRana
RyanRana requested a review from HarshaNalluru July 27, 2026 17:38
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.

4 participants