Skip to content

fix(entity_file): emit clear error when schema header field has no type#66

Open
gkorland wants to merge 2 commits into
masterfrom
fix/header-missing-type
Open

fix(entity_file): emit clear error when schema header field has no type#66
gkorland wants to merge 2 commits into
masterfrom
fix/header-missing-type

Conversation

@gkorland

Copy link
Copy Markdown

Summary

In schema mode, convert_header_with_schema split each header field on : and immediately indexed pair[1]. If a field contained no colon (e.g. name instead of name:STRING), this raised an opaque IndexError: list index out of range instead of a helpful schema validation error.

Changes

  • falkordb_bulk_loader/entity_file.py: detect len(pair) < 2 after the existing len(pair) > 2 check and raise a descriptive CSVError ("Schema header field '' is missing a type (expected 'name:TYPE')").

Testing

uv run flake8 falkordb_bulk_loader clean.

Memory / Performance Impact

N/A.

Related Issues

From the comprehensive code-review report (BUG-4).

@coderabbitai

coderabbitai Bot commented Apr 26, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@gkorland has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 22 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 89e0ec9b-b134-40e9-b4de-2d64918f7228

📥 Commits

Reviewing files that changed from the base of the PR and between 3954856 and 213b890.

📒 Files selected for processing (2)
  • falkordb_bulk_loader/entity_file.py
  • test/test_label.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/header-missing-type

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 47 minutes and 22 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.54%. Comparing base (3954856) to head (213b890).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
+ Coverage   89.50%   89.54%   +0.03%     
==========================================
  Files           9        9              
  Lines         610      612       +2     
==========================================
+ Hits          546      548       +2     
  Misses         64       64              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gkorland
gkorland marked this pull request as ready for review April 26, 2026 17:57
@gkorland

Copy link
Copy Markdown
Author

Copilot Code Review

No significant issues found.

gkorland and others added 2 commits April 29, 2026 09:32
If a schema-mode CSV header contained a field without a colon (e.g. just
`name` instead of `name:STRING`), `convert_header_with_schema` raised
an unhelpful `IndexError: list index out of range` from `pair[1]`.
Detect the missing-colon case explicitly and raise a descriptive
`CSVError` instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add test_schema_header_field_missing_type_raises to TestBulkLoader to
exercise the new CSVError raised when a schema header field has no colon
(e.g. 'name' instead of 'name:STRING'), closing the Codecov patch gap.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gkorland
gkorland force-pushed the fix/header-missing-type branch from 97c4bd5 to 213b890 Compare April 29, 2026 06:34
@gkorland

Copy link
Copy Markdown
Author

Agent Review Summary

PR: fix(entity_file): emit clear error when schema header field has no type

Changes reviewed: falkordb_bulk_loader/entity_file.py — 7 additions, 0 deletions.


✅ Code Review

The fix is correct and minimal:

  • Adds a len(pair) < 2 guard immediately after the existing len(pair) > 2 guard in convert_header_with_schema
  • Raises a descriptive CSVError("... is missing a type (expected 'name:TYPE')") instead of an opaque IndexError
  • No logic regressions; the happy path is unchanged

🔧 Actions Taken

  1. Rebased onto master (branch was 5 commits behind)
  2. Added missing test test_schema_header_field_missing_type_raises in test/test_label.py to cover the new error branch — this closed the Codecov patch coverage gap (was 50 %, now 100 %)
  3. Force-pushed with --force-with-lease

🟢 CI Status

All 9 checks passed after the push:

  • CI/Test with Python 3.10
  • CI/Test with Python 3.11
  • CI/Code linting
  • CodeQL (python + actions)
  • codecov/patch ✅ (was ❌ before)
  • codecov/project ✅ (was ❌ before)
  • CodeRabbit

📝 No blocking issues found. Ready to merge.

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.

1 participant