Fix/handling wrong data in csv import by Pranav-0440 · Pull Request #194 · eclipse-editdor/editdor

Pranav-0440 · 2026-02-10T19:05:41Z

Fixes #134

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>

Copilot

Pull request overview

This PR introduces a warning-collection mechanism during CSV parsing to flag “wrong but parseable” data (e.g., unexpected type / modbus:entity values) and surfaces those warnings to users during CSV-based TD/TM creation. It also adds CSV example files/documentation, plus some unrelated UI refactors.

Changes:

Update parseCsv to return { data, warnings } and add basic validations that emit warnings (invalid type, invalid modbus:entity).
Surface CSV warnings in the CSV import flow in CreateTd.
Add example CSV files + README describing expected warning behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`src/utils/parser.ts`	Adds `CsvWarning` type + warning collection in `parseCsv`; changes return type.
`src/utils/parser.test.ts`	Updates tests for new `parseCsv` return shape; adds warning-related test cases.
`src/components/App/CreateTd.tsx`	Displays warnings returned by `parseCsv` during CSV import.
`src/components/Editor/JsonEditor.tsx`	Refactors editor setup/handlers and options (includes a potential race/crash).
`src/components/Dialogs/ConvertTmDialog.tsx`	Major behavioral change to TM→TD conversion dialog (currently appears breaking/unrelated to CSV warnings).
`doc/examples/csv/valid.csv`	Adds a “good” CSV example.
`doc/examples/csv/invalid.csv`	Adds a CSV example intended to trigger warnings.
`doc/examples/csv/README.md`	Documents example CSVs and expected warnings (currently inconsistent with code).
`test/csv-examples.test.ts`	Adds a console-driven “test” script (not picked up by current Vitest config).

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

Copilot · 2026-02-10T19:10:18Z

src/utils/parser.ts

-      .join("; ");
-    throw new Error(`CSV parse failed: ${msg}`);
+    throw new Error(
+      res.errors.map((e) => `Row ${e.row}: ${e.message}`).join("; ")


The parse error message uses Row ${e.row} directly; PapaParse can emit errors where row is undefined, which would produce messages like Row undefined: .... Consider defaulting to ? and/or including the code field for better diagnostics.

Suggested change

res.errors.map((e) => `Row ${e.row}: ${e.message}`).join("; ")

res.errors

.map((e) => {

const rowLabel = e.row != null ? String(e.row) : "?";

const codeLabel = e.code ? ` [code: ${e.code}]` : "";

return `Row ${rowLabel}${codeLabel}: ${e.message}`;

})

.join("; ")

Copilot · 2026-02-10T19:10:18Z

src/utils/parser.ts

+    if (row["modbus:entity"]) {
+      const entityLower = row["modbus:entity"].toLowerCase();
+      const validEntityLower = VALID_MODBUS_ENTITIES.map((e) => e.toLowerCase());
+      if (!validEntityLower.includes(entityLower)) {
+        warnings.push({


VALID_MODBUS_ENTITIES.map(e => e.toLowerCase()) is recomputed for every row. Precompute the lowercased set once outside the loop (e.g., a Set) to avoid unnecessary allocations during large imports.

Copilot · 2026-02-10T19:10:18Z

src/components/Dialogs/ConvertTmDialog.tsx

+  const [display, setDisplay] = useState(false);
+  const [warnings, setWarnings] = useState<CsvWarning[]>([]);
+  const [validVersion, setValidVersion] = useState(false);
+  const [versionInput, setVersionInput] = useState("");


setWarnings is declared but never used, and warnings is never populated from anywhere, so the warning UI will never show and this will likely trigger no-unused-vars lint failures. Either wire warnings into this dialog via props/context or remove this state and related UI.

Copilot · 2026-02-10T19:10:19Z

src/components/App/CreateTd.tsx

+            .join("\n");
+          setError({ open: true, message: `Warnings:\n${warningMessage}` });


Warnings are surfaced via ErrorDialog with a newline-delimited string, but DialogTemplate renders description in an <h2> without preserving newlines, so the warning list will collapse into a hard-to-read paragraph and is labeled as an “Error”. Consider rendering warnings as a list (or using white-space: pre-line) and distinguishing warnings from blocking errors.

Suggested change

.join("\n");

setError({ open: true, message: `Warnings:\n${warningMessage}` });

.join("; ");

setError({ open: true, message: `Warnings: ${warningMessage}` });

Copilot · 2026-02-10T19:10:19Z

doc/examples/csv/README.md

+### `invalid.csv`
+A CSV file with intentional validation errors to demonstrate the warning system. When imported, this file will trigger the following warnings:
+
+1. **Row 2, type**: `number123` is invalid (should be `number`, `string`, or `boolean`)
+2. **Row 2, modbus:entity**: `holdingregister` is invalid (case-sensitive, should be `HoldingRegister`)
+3. **Row 3, modbus:entity**: `InvalidRegister` is not a recognized Modbus entity
+4. **Row 4, type**: `invalid_type` is not a valid type
+5. **Row 5, modbus:entity**: `coil` is invalid (case-sensitive, should be `Coil`)
+
+## Usage
+
+1. Open EditDor
+2. Select "Thing Description" or "Thing Model"
+3. Click "Load a CSV File"
+4. Select either `valid.csv` or `invalid.csv`
+5. Observe the results:
+   - `valid.csv`: Should load successfully without warnings
+   - `invalid.csv`: Should display validation warnings but still load the data
+
+## CSV Format
+
+Required columns:
+- `name`: Property name (required)
+- `type`: Data type (number, string, or boolean)
+- `modbus:entity`: Modbus entity type
+- `modbus:address`: Modbus address (required)
+- `modbus:unitID`: Modbus unit ID
+- `modbus:quantity`: Number of registers
+- `modbus:zeroBasedAddressing`: Boolean (true/false)
+- `modbus:function`: Modbus function code
+- `modbus:mostSignificantByte`: Boolean (true/false)
+- `modbus:mostSignificantWord`: Boolean (true/false)
+- `href`: Property endpoint path
+
+Note: The validation is case-insensitive for Modbus entities, so `coil`, `Coil`, and `COIL` are all treated as equivalent.


The README contradicts itself about Modbus entity casing: it lists warnings for holdingregister/coil as “case-sensitive” issues, but later states validation is case-insensitive. Also, the current parser implementation won’t warn on casing differences, so the described warnings won’t occur. Align the documentation with the actual behavior (or update the parser to match the documented behavior).

Copilot · 2026-02-10T19:10:19Z

src/utils/parser.ts

 export const parseCsv = (
  csvContent: string,
  hasHeaders: boolean = true
-): CsvData[] => {
-  if (csvContent === "") throw new Error("CSV content is empty");
+): { data: CsvData[]; warnings: CsvWarning[] } => {
+  if (!csvContent) throw new Error("CSV content is empty");
+
+  const warnings: CsvWarning[] = [];

  const res = Papa.parse<CsvData>(csvContent, {
    header: true,
-    quoteChar: '"',
    skipEmptyLines: true,
-    dynamicTyping: false,
    transformHeader: (h) => h.trim(),
-    transform: (value) => (typeof value === "string" ? value.trim() : value),
-    complete: (results) => {
-      console.log(results.data, results.errors, results.meta);
-    },
+    transform: (v) => (typeof v === "string" ? v.trim() : v),
  });


parseCsv takes a hasHeaders parameter but the PapaParse config hard-codes header: true, so callers passing false will be ignored. Either wire header: hasHeaders (and adjust return type/row numbering accordingly) or remove the parameter to avoid a misleading API.

Copilot · 2026-02-10T19:10:20Z

src/utils/parser.ts

+      const entityLower = row["modbus:entity"].toLowerCase();
+      const validEntityLower = VALID_MODBUS_ENTITIES.map((e) => e.toLowerCase());
+      if (!validEntityLower.includes(entityLower)) {
+        warnings.push({
+          row: rowNum,
+          column: "modbus:entity",
+          message: `Invalid modbus entity "${row["modbus:entity"]}"`,
+        });


Current Modbus entity validation is case-insensitive, so inputs like holdingregister/coil will not generate warnings even though Issue #134 calls out casing mistakes as “wrong data”. If the goal is to accept but warn on non-canonical casing, add a separate check that compares the raw value to the canonical spelling and emits a warning when they differ.

Suggested change

const entityLower = row["modbus:entity"].toLowerCase();

const validEntityLower = VALID_MODBUS_ENTITIES.map((e) => e.toLowerCase());

if (!validEntityLower.includes(entityLower)) {

warnings.push({

row: rowNum,

column: "modbus:entity",

message: `Invalid modbus entity "${row["modbus:entity"]}"`,

});

const rawEntity = row["modbus:entity"];

const entityLower = rawEntity.toLowerCase();

const canonicalEntity = VALID_MODBUS_ENTITIES.find(

(e) => e.toLowerCase() === entityLower

);

if (!canonicalEntity) {

warnings.push({

row: rowNum,

column: "modbus:entity",

message: `Invalid modbus entity "${rawEntity}"`,

});

} else if (rawEntity !== canonicalEntity) {

warnings.push({

row: rowNum,

column: "modbus:entity",

message: `Non-canonical casing for modbus entity "${rawEntity}", expected "${canonicalEntity}"`,

});

Copilot · 2026-02-10T19:10:20Z

src/components/Dialogs/ConvertTmDialog.tsx

+      [],
+      [],
+      [],


processConversionTMtoTD filters affordances by the provided properties/actions/events arrays. Passing [] for all of them will strip all properties/actions/events from the generated TD, which breaks the TM→TD conversion flow. Restore the previous affordance-selection behavior (or pass all keys by default) so conversion doesn’t silently drop interactions.

Suggested change

[],

[],

[],

undefined,

undefined,

undefined,

Copilot · 2026-02-10T19:10:20Z

src/components/Editor/JsonEditor.tsx

  useEffect(() => {
    const updateMonacoSchemas = (schemaMap: SchemaMapMessage) => {
      proxy.splice(0, proxy.length);

-      schemaMap.forEach(function (schema, schemaUri) {
-        console.debug(`using schema: ${schemaUri}`);
-        proxy.push({ schemaUri: schemaUri, schema: schema });
+      schemaMap.forEach((schema, schemaUri) => {
+        proxy.push({ schemaUri, schema });
      });
    };

    schemaWorker.onmessage = (ev: MessageEvent<SchemaMapMessage>) => {
-      console.debug("received message from schema worker");
      updateMonacoSchemas(ev.data);
    };

    validationWorker.onmessage = (ev: MessageEvent<IValidationMessage>) => {
-      console.debug("received message from validation worker");
-
-      const validationResults: IValidationMessage = ev.data;
-      context.updateValidationMessage(validationResults);
+      context.updateValidationMessage(ev.data);
    };
  }, [schemaWorker, validationWorker, proxy, context]);


schemaWorker.onmessage mutates proxy via proxy.splice(...), but proxy starts as undefined and workers can respond before editorDidMount runs and setProxy is called (there’s an effect that posts messages on mount). Add a guard (e.g., ignore messages until proxy exists) or refactor proxy into a ref to avoid a startup race that can crash the editor.

Copilot · 2026-02-10T19:10:21Z

test/csv-examples.test.ts

+import { parseCsv, mapCsvToProperties } from "../src/utils/parser";
+import * as fs from "fs";
+import * as path from "path";
+import { fileURLToPath } from "url";
+
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = path.dirname(__filename);
+
+console.log("=== Testing CSV Examples ===\n");
+
+// Test valid.csv
+console.log("1. Testing valid.csv:");
+const validCsv = fs.readFileSync(
+  path.join(__dirname, "../doc/examples/csv/valid.csv"),
+  "utf-8"
+);
+const validResult = parseCsv(validCsv, true);
+console.log(`   - Rows parsed: ${validResult.data.length}`);
+console.log(`   - Warnings: ${validResult.warnings.length}`);
+if (validResult.warnings.length > 0) {
+  console.log("   FAIL: Expected no warnings but got:");
+  validResult.warnings.forEach((w) =>
+    console.log(`      Row ${w.row}, ${w.column}: ${w.message}`)
+  );
+} else {
+  console.log("   PASS: No warnings as expected");
+}
+
+// Try to map properties
+try {
+  const properties = mapCsvToProperties(validResult.data);
+  console.log(`   - Properties created: ${Object.keys(properties).length}`);
+  console.log("   PASS: Properties mapped successfully");
+} catch (err) {
+  console.log(`   FAIL: ${(err as Error).message}`);
+}
+
+console.log("\n2. Testing invalid.csv:");
+const invalidCsv = fs.readFileSync(
+  path.join(__dirname, "../doc/examples/csv/invalid.csv"),
+  "utf-8"
+);
+const invalidResult = parseCsv(invalidCsv, true);
+console.log(`   - Rows parsed: ${invalidResult.data.length}`);
+console.log(`   - Warnings: ${invalidResult.warnings.length}`);
+
+if (invalidResult.warnings.length === 0) {
+  console.log("   FAIL: Expected warnings but got none");
+} else {
+  console.log("   PASS: Warnings detected as expected:");
+  invalidResult.warnings.forEach((w) =>
+    console.log(`      Row ${w.row}, ${w.column}: ${w.message}`)
+  );
+}
+
+// Try to map properties - should still work despite warnings
+try {
+  const properties = mapCsvToProperties(invalidResult.data);
+  console.log(`   - Properties created: ${Object.keys(properties).length}`);
+  console.log("   PASS: Properties mapped successfully despite warnings");
+} catch (err) {
+  console.log(`   FAIL: ${(err as Error).message}`);
+}
+
+console.log("\n=== Summary ===");
+console.log("All CSV example files tested successfully!");
+console.log(
+  "The validation system correctly identifies issues while still allowing data import."
+);


This “test” file won’t run under the current Vitest config (vite.config.js only includes src/**/*.{test,spec}.{ts,tsx}), and it contains no test()/assertions (just console.log). Move it under src/ and convert it into real Vitest cases, or move it to a non-test script location (e.g., scripts/) so it’s not misleading.

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>

netlify · 2026-02-18T18:38:25Z

✅ Deploy Preview for editdor ready!

Name	Link
🔨 Latest commit	`135e6f0`
🔍 Latest deploy log	https://app.netlify.com/projects/editdor/deploys/6996071df886ef0008fd4436
😎 Deploy Preview	https://deploy-preview-194--editdor.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

TejInaco · 2026-02-19T16:46:18Z

Close this PR because the commit changes don't make sense for this issue. @egekorkan Same changes are commit in different pull requests. Second time there is a PR dedicated to the same issue.

Pranav-0440 added 4 commits February 8, 2026 23:16

Configure Monaco Format Document to use 2-space indentation

9f488b1

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>

Configure Monaco JSON formatter to use 2-space indentation

a269362

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>

fix(editor): configure Monaco JSON indentation to 2 spaces

005b337

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>

Add CSV validation warnings and example CSV files

f18973e

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>

Copilot AI review requested due to automatic review settings February 10, 2026 19:05

Copilot started reviewing on behalf of Pranav-0440 February 10, 2026 19:06 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

chore: apply prettier formatting

135e6f0

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>

TejInaco marked this pull request as draft February 19, 2026 16:42

TejInaco closed this Feb 19, 2026

-      res.errors.map((e) => `Row ${e.row}: ${e.message}`).join("; ")
+      res.errors
+        .map((e) => {
+          const rowLabel = e.row != null ? String(e.row) : "?";
+          const codeLabel = e.code ? ` [code: ${e.code}]` : "";
+          return `Row ${rowLabel}${codeLabel}: ${e.message}`;
+        })
+        .join("; ")

		.join("\n");
		setError({ open: true, message: `Warnings:\n${warningMessage}` });

-      [],
-      [],
-      [],
+      undefined,
+      undefined,
+      undefined,

Conversation

Pranav-0440 commented Feb 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

netlify bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for editdor ready!

Uh oh!

TejInaco commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Feb 18, 2026 •

edited

Loading

TejInaco commented Feb 19, 2026 •

edited

Loading