Fix: Handle CSV separator correctly in sort_for_self_referencing function

- Added separator parameter to sort_for_self_referencing function to properly handle semicolon-separated CSV files
- Fixed temp file writing to preserve original file separator
- Improved error handling to distinguish between fatal file errors and schema validation issues
- Schema validation errors no longer abort imports unnecessarily
- Updated tests to reflect new parameter requirements
- All 370 tests continue to pass

This resolves the false error where malformed CSV detection was incorrectly aborting valid imports.

Author: bosd

src/odoo_data_flow/importer.py

@@ -189,6 +189,7 @@ def run_import(  # noqa: C901
             id_column=import_plan["id_column"],
             parent_column=import_plan["parent_column"],
             encoding=encoding,
+            separator=separator,
         )
         if isinstance(sorted_temp_file, str):
             file_to_process = sorted_temp_file

src/odoo_data_flow/lib/preflight.py

@@ -113,7 +113,10 @@ def self_referencing_check(
     # We assume 'id' and 'parent_id' as conventional names.
     # This could be made configurable later if needed.
     result = sort.sort_for_self_referencing(
-        filename, id_column="id", parent_column="parent_id"
+        filename,
+        id_column="id",
+        parent_column="parent_id",
+        separator=kwargs.get("separator", ";"),
     )
     if result is False:
         # This means there was an error in sort_for_self_referencing

src/odoo_data_flow/lib/sort.py

@@ -5,11 +5,16 @@
 
 import polars as pl
 
+from ..logging_config import log
 from .internal.ui import _show_error_panel
 
 
 def sort_for_self_referencing(
-    file_path: str, id_column: str, parent_column: str, encoding: str = "utf-8"
+    file_path: str,
+    id_column: str,
+    parent_column: str,
+    encoding: str = "utf-8",
+    separator: str = ",",
 ) -> Optional[Union[str, bool]]:
     """Sorts a CSV file for self-referencing hierarchies.
 
@@ -28,19 +33,39 @@ def sort_for_self_referencing(
         id_column (str): The name of the unique identifier column.
         parent_column (str): The name of the column containing the parent reference.
         encoding (str): The encoding of the CSV file.
+        separator (str): The field separator used in the CSV file.
 
     Returns:
         Optional[Union[str, bool]]: The path to the temporary sorted CSV file if sorting
         was performed, None if no sorting is needed or possible, or False if
         there was an error reading the file.
     """
     try:
-        df = pl.read_csv(file_path, encoding=encoding)
-    except (FileNotFoundError, pl.exceptions.PolarsError) as e:
+        # For the sort function, we only care about being able to read the file
+        # well enough to detect self-referencing hierarchies. We don't need
+        # to parse all columns perfectly, so we use a very tolerant approach.
+        df = pl.read_csv(
+            file_path,
+            separator=separator,
+            encoding=encoding,
+            truncate_ragged_lines=True,
+            infer_schema_length=0,  # Don't infer schema, treat everything as string
+        )
+    except FileNotFoundError as e:
+        _show_error_panel(
+            "File Read Error", f"Could not read the file {file_path}: {e}"
+        )
+        return False  # Return False to indicate an error occurred
+    except pl.exceptions.NoDataError as e:
         _show_error_panel(
             "File Read Error", f"Could not read the file {file_path}: {e}"
         )
         return False  # Return False to indicate an error occurred
+    except Exception as e:
+        # For other errors (like schema validation), we don't want to abort the import
+        # These should be handled by the field validation preflight check
+        log.warning(f"Could not fully parse file {file_path} for sorting: {e}")
+        return None  # Return None to indicate no sorting needed/possible
 
     if id_column not in df.columns or parent_column not in df.columns:
         return None
@@ -58,9 +83,9 @@ def sort_for_self_referencing(
         pl.col(parent_column).is_null(), parent_column, descending=[True, False]
     )
 
-    # Write to a temporary file
+    # Write to a temporary file with the same separator as the original
     temp_file = tempfile.NamedTemporaryFile(
         mode="w+", delete=False, suffix=".csv", newline=""
     )
-    sorted_df.write_csv(temp_file.name)
+    sorted_df.write_csv(temp_file.name, separator=separator)
     return temp_file.name

tests/test_preflight.py

@@ -63,7 +63,7 @@ def test_check_plans_strategy_when_hierarchy_detected(
         assert import_plan["id_column"] == "id"
         assert import_plan["parent_column"] == "parent_id"
         mock_sort.assert_called_once_with(
-            "file.csv", id_column="id", parent_column="parent_id"
+            "file.csv", id_column="id", parent_column="parent_id", separator=";"
         )
 
     @patch("odoo_data_flow.lib.preflight.sort.sort_for_self_referencing")

tests/test_sort.py

@@ -39,7 +39,7 @@ def non_hierarchical_csv(tmp_path: Path) -> str:
 def test_sorts_correctly_when_self_referencing(hierarchical_csv: str) -> None:
     """Verify that a self-referencing CSV is sorted correctly."""
     sorted_file = sort_for_self_referencing(
-        hierarchical_csv, id_column="id", parent_column="parent_id"
+        hierarchical_csv, id_column="id", parent_column="parent_id", separator=","
     )
     assert sorted_file is not None
     # Make sure it's not False (error case)
@@ -60,7 +60,7 @@ def test_sorts_correctly_when_self_referencing(hierarchical_csv: str) -> None:
 def test_returns_none_when_not_self_referencing(non_hierarchical_csv: str) -> None:
     """Verify that None is returned if the hierarchy is not self-referencing."""
     sorted_file = sort_for_self_referencing(
-        non_hierarchical_csv, id_column="id", parent_column="category_id"
+        non_hierarchical_csv, id_column="id", parent_column="category_id", separator=","
     )
     assert sorted_file is None
 
@@ -74,7 +74,7 @@ def test_returns_none_if_columns_missing() -> None:
 
     assert (
         sort_for_self_referencing(
-            str(file_path), id_column="id", parent_column="parent_id"
+            str(file_path), id_column="id", parent_column="parent_id", separator=","
         )
         is None
     )
@@ -84,6 +84,6 @@ def test_returns_none_if_columns_missing() -> None:
 def test_returns_false_for_non_existent_file() -> None:
     """Verify that False is returned if the input file does not exist."""
     result = sort_for_self_referencing(
-        "non_existent.csv", id_column="id", parent_column="parent_id"
+        "non_existent.csv", id_column="id", parent_column="parent_id", separator=","
     )
     assert result is False