Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesBOM handling
Suggested reviewers: Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The BOM normalization changes have no identified merge-blocking issue. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e980c4462
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _parse_csv(self, content: str, source_ref: str) -> LoaderResult: | ||
| try: | ||
| csv_reader = csv.DictReader(StringIO(content)) | ||
| csv_reader = csv.DictReader(StringIO(content.removeprefix("\ufeff"))) |
There was a problem hiding this comment.
When a URL serves BOM-prefixed UTF-8 bytes as Content-Type: text/csv without a charset, Requests assigns ISO-8859-1 before load_from_url() reads response.text, so this method receives name,... rather than a leading U+FEFF. The new removeprefix() is therefore a no-op and the first header remains corrupted; the URL test masks this by mocking .text as already-correct Unicode. Detect the raw BOM and select utf-8-sig before reading the response text, and exercise that behavior with a byte-backed response.
AGENTS.md reference: AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced with a real requests.Response containing UTF-8 BOM bytes and Content-Type: text/csv without a charset. Requests selected Latin-1, and four byte-backed CSV regression cases failed on the previous implementation.
Fixed in 0e5075d. The shared load_from_url() helper now detects BOM_UTF8 in response.content and selects utf-8-sig before reading response.text.
The URL tests now exercise real response decoding and cover quoted/unquoted headers, non-ASCII values, BOM-free UTF-8 and Latin-1 responses, and embedded U+FEFF. Additional JSON/XML tests preserve compatibility with automatic and explicit BOM decoding in other users of the shared helper.
Validation: 68 tests passed, including 30 CSV tests; Ruff lint/format and mypy passed. The only warning is the existing crewai.utilities.lock_store deprecation warning.
1 participant
Related issue
Fixes #7466
Summary
A leading UTF-8 BOM becomes part of CSVLoader's first column name. Before a quoted header containing a comma, it also prevents quote recognition and assigns row values to the wrong columns. URL responses with
Content-Type: text/csvand no charset have an additional decoding problem: Requests defaults to Latin-1, corrupting both the BOM and non-ASCII values before CSV parsing.The shared URL loader now recognizes a UTF-8 byte signature before reading
response.textand selectsutf-8-sigdecoding. CSVLoader also removes one leading U+FEFF from decoded input beforecsv.DictReaderparses the content. Responses without a UTF-8 BOM keep their existing encoding, and U+FEFF inside field values is preserved.Regression tests cover file, inline-text and URL inputs, quoted/unquoted headers, BOM/no-BOM inputs, non-ASCII values, Latin-1 responses and embedded U+FEFF, plus JSON/XML compatibility with automatic and explicit BOM decoding. URL regressions use real
requests.Responseobjects backed by bytes, with Requests' HTTP-header encoding inference.Verification
Python 3.13.13, macOS 26.6.2:
uv run --locked pytest lib/crewai-tools/tests/rag/test_csv_loader.py -n 0 -q: 30 passed. The expanded byte-backed suite produced 4 failed, 26 passed before the URL decoding fix, reproducing the review finding.uv run --locked pytest lib/crewai-tools/tests/rag/test_csv_loader.py lib/crewai-tools/tests/rag/test_loader_utils.py lib/crewai-tools/tests/rag/test_json_loader.py lib/crewai-tools/tests/rag/test_mdx_loader.py lib/crewai-tools/tests/rag/test_xml_loader.py -n 0 -q: 68 passed.uv run --locked mypy lib/crewai-tools/src/crewai_tools/rag/loaders/utils.py lib/crewai-tools/src/crewai_tools/rag/loaders/csv_loader.py: no issues found.The test runs emit one existing
crewai.utilities.lock_storedeprecation warning.Additional context
This contribution was prepared with Codex assistance. Per CONTRIBUTING.md, please apply the
llm-generatedlabel; this external contributor account does not have permission to add labels.