Review feedback (part 1): Extend type/format guesses to return both "safe" and "maybe" options

alaric-rd · alaric-rd · commit 5f448a13dc09 · 2025-10-23T17:31:25.000+01:00
"Safe" options are valid for EVERY value in the column.
"Maybe" options are valid for SOME values in the column (but not every).

The frontend needs update to reflect this distinction, only showing
safe/maybe options (rather than ALL options as currently).
diff --git a/lib/importer/src/dudk/backend.js b/lib/importer/src/dudk/backend.js
@@ -147,26 +147,34 @@ exports.SessionGetColumnsNeedingFormats = (sid) => {
 // Given guesses as returned by SessionGuessTypes and a list of domain-model
 // fields (with name and type fields), returns the type guesses with each column
 // augmented with a `fields` field - a Map mapping likely field names for this
-// column to an array of possible formats for that field.
+// column to an array of safe formats for that field - and a `maybeFields` field
+// - the same but for "maybe" fields that might be a partial match for the data.
 
 exports.SessionSuggestFields = (sid, typeGuesses, domainModelFields) => {
   let result = [];
 
   typeGuesses.forEach((colTypeGuess) => {
-    let fieldFormats = new Map(); // Map from field names to likely formats
+    let safeFieldFormats = new Map(); // Map from field names to safe formats
+    let maybeFieldFormats = new Map(); // Map from field names to possible formats
     // Examine every field, and check if they have a type that this column might
     // contain
     domainModelFields.forEach((field) => {
-      if(colTypeGuess.types.has(field.type)) {
-        // Add this field to the possibilities for the column, using the guessed
+      if(colTypeGuess.safeTypes.has(field.type)) {
+        // Add this field to the safe possibilities for the column, using the guessed
         // formats
-        fieldFormats.set(field.name, colTypeGuess.types.get(field.type));
+        safeFieldFormats.set(field.name, colTypeGuess.safeTypes.get(field.type));
+      }
+      if(colTypeGuess.maybeTypes.has(field.type)) {
+        // Add this field to the maybe possibilities for the column, using the guessed
+        // formats
+        maybeFieldFormats.set(field.name, colTypeGuess.maybeTypes.get(field.type));
       }
     });
 
     // Extend the column type guess with the "fields" field
     let typeGuessesAndFields = Object.assign({}, colTypeGuess);
-    typeGuessesAndFields.fields = fieldFormats;
+    typeGuessesAndFields.safeFields = safeFieldFormats;
+    typeGuessesAndFields.maybeFields = maybeFieldFormats;
     result.push(typeGuessesAndFields);
   });
 
@@ -532,10 +540,12 @@ exports.SessionGetInputSampleRows = (sid, range, startCount, middleCount, endCou
 // interpreted as extra text values.
 
 // The return value is an array, indexed on the relative column number within
-// the range; each element is an object with elements "types" listing a range of
-// possible types in a map whose values are either 'false' for types without
-// formats, or an Array of possible formats for that type; and a field "full"
-// indicating if the column has no missing values.
+// the range; each element is an object with elements "safeTypes" listing a
+// range of types that all cells in the column match, in a map whose values are
+// either 'false' for types without formats, or an Array of possible formats for
+// that type; "maybeTypes" in the same format, but listing type/format pairs
+// which *some* but not all cells match; and a field "full" indicating if the
+// column has no missing values.
 exports.SessionGuessTypes = (sid, range) => {
   assert(sessionStore.has(sid), `No such session ${sid} when guessing types`);
   assert(sessionStore.get(sid).wb.Sheets[range.sheet], `No such sheet ${range.sheet} in session ${sid} when guessing types`);
@@ -556,7 +566,8 @@ exports.SessionGuessTypes = (sid, range) => {
 
   // Initialise state, that we will refine as we go
   for (let col=0;col<columns;col++) {
-    guesses[col] = {types: undefined,
+    guesses[col] = {safeTpes: undefined,
+                    maybeTypes: undefined,
                     full:true}; // This is unset if we find a blank/missing
                                 // value
   }
@@ -577,32 +588,87 @@ exports.SessionGuessTypes = (sid, range) => {
         // Blank cell, so mark the column as not full but don't reduce the list of guessed types
         guesses[col].full = false;
       } else if(possibleCellTypes) {
-        if(guesses[col].types) {
-          // Remove any types in the current guess that aren't possibly valid for
+        // Check if this is the first time we've seen this column. safeTypes and
+        // maybeTypes are both set at the same time, so we can use safeTypes
+        // being set as a proxy for both.
+        if(guesses[col].safeTypes) {
+          // Remove any safeTypes in the current guess that aren't possibly valid for
           // this cell
-          let newGuesses = new Map();
-          guesses[col].types.forEach((formats, typeName) => {
+          let newSafeGuesses = new Map();
+          guesses[col].safeTypes.forEach((formats, typeName) => {
             if(possibleCellTypes.has(typeName)) {
               if(formats) {
                 // Type with formats, so find the intersection of the current possible formats and this cell's possible formats
                 let possibleCellFormats = possibleCellTypes.get(typeName);
                 let newPossibleFormats = formats.filter(value => possibleCellFormats.includes(value));
-                newGuesses.set(typeName, newPossibleFormats);
+                newSafeGuesses.set(typeName, newPossibleFormats);
               } else {
                 // Type without formats
-                newGuesses.set(typeName, formats);
+                newSafeGuesses.set(typeName, formats);
               }
             }
+            // Add any types in the current guess to maybeTypes
+            possibleCellTypes.forEach((formats, typeName) => {
+              const maybe = guesses[col].maybeTypes;
+              if(formats) {
+                // Type with formats, so make sure we've added all the formats to the list
+                if(!maybe.has(typeName)) {
+                  maybe.set(typeName, []);
+                }
+                formats.forEach((format) => {
+                  if(!maybe.get(typeName).includes(format)) {
+                    maybe.get(typeName).push(format);
+                  }
+                });
+              } else {
+                // Type without formats, just set it to false
+                maybe.set(typeName, false);
+              }
+              guesses[col].maybeTypes = maybe;
+            });
           });
-          guesses[col].types = newGuesses;
+          guesses[col].safeTypes = newSafeGuesses;
         } else {
           // First cell examined, so start off with its possible types
-          guesses[col].types = possibleCellTypes;
+          guesses[col].safeTypes = possibleCellTypes;
+          guesses[col].maybeTypes = possibleCellTypes;
         }
       }
     }
   }
 
+  // Now we must go back through the guesses and remove any safe types/formats
+  // from the maybe types/formats, so "maybe" ONLY contains options that are
+  // valid for some *but not all* values; every safe type/format will be listed
+  // as a maybe type/format due to the process used above.
+
+  guesses.forEach((colGuesses) => {
+    let maybeTypes = colGuesses.maybeTypes;
+    colGuesses.safeTypes.forEach((safeFormats, typeName) => {
+      if(safeFormats) {
+        // It's a type with formats
+
+        // typeName will always be present in maybeTypes at this point
+        let formats = maybeTypes.get(typeName);
+        safeFormats.forEach((format) => {
+          const index = formats.indexOf(format);
+          // index will never be -1
+          formats.splice(index, 1);
+        });
+        // If this is the last format for this type in maybeTypes, remove it. We
+        // won't get any other safeTypes for this maybeType so there's no chance
+        // of the above code being invoked again, and failing because typeName is
+        // no longe rin maybeTypes.
+        if(formats.length == 0) {
+          maybeTypes.delete(typeName);
+        }
+      } else {
+        // It's a type without formats
+        maybeTypes.delete(typeName);
+      }
+    });
+  });
+
   return guesses;
 };
 
diff --git a/lib/importer/src/dudk/backend.test.js b/lib/importer/src/dudk/backend.test.js
@@ -609,15 +609,33 @@ test('guessing types', () => {
   const guesses = backend.SessionGuessTypes(sid, dataRange);
   expect(guesses).toMatchObject([
     // Order is actually irrelevant for all formats lists, so this test over-specifies a bit
-    {full:true, types:new Map([["number", false],["text", false]])},
-    {full:true, types:new Map([["date", ["ymd"]],["text", false]])},
-    {full:true, types:new Map([["date", ["ydm"]],["text", false]])},
-    {full:true, types:new Map([["date", ["dmy"]],["text", false]])},
-    {full:true, types:new Map([["date", ["mdy"]],["text", false]])},
-    {full:true, types:new Map([["date", ["ymd","ydm"]],["text", false]])},
-    {full:false, types:new Map([["number", false],["text", false]])},
-    {full:true, types:new Map([["text", false]])},
-    {full:false, types:new Map([["text", false]])}
+    {full:true,
+     safeTypes:new Map([["number", false],["text", false]]),
+     maybeTypes:new Map([])},
+    {full:true,
+     safeTypes:new Map([["date", ["ymd"]],["text", false]]),
+     maybeTypes: new Map([["date", ["ydm","dmy","mdy"]]])},
+    {full:true,
+     safeTypes:new Map([["date", ["ydm"]],["text", false]]),
+     maybeTypes: new Map([["date", ["ymd","dmy","mdy"]]])},
+    {full:true,
+     safeTypes:new Map([["date", ["dmy"]],["text", false]]),
+     maybeTypes: new Map([["date", ["mdy", "ymd"]]])},
+    {full:true,
+     safeTypes:new Map([["date", ["mdy"]],["text", false]]),
+     maybeTypes: new Map([["date", ["dmy"]]])},
+    {full:true,
+     safeTypes:new Map([["date", ["ymd","ydm"]],["text", false]]),
+     maybeTypes: new Map([])},
+    {full:false,
+     safeTypes:new Map([["number", false],["text", false]]),
+     maybeTypes: new Map([])},
+    {full:true,
+     safeTypes:new Map([["text", false]]),
+     maybeTypes: new Map([])},
+    {full:false,
+     safeTypes:new Map([["text", false]]),
+     maybeTypes: new Map([])}
   ]);
 
   const fieldSuggestions = backend.SessionSuggestFields(sid, guesses, [
@@ -645,48 +663,65 @@ test('guessing types', () => {
 
   expect(fieldSuggestions).toMatchObject([
     // Order is actually irrelevant for all formats lists, so this test over-specifies a bit
-    {full:true, types:new Map([["number", false],["text", false]]),
-     fields: new Map([
-       ["numberField",false],
-       ["textField",false]
+    // ABS FIXME add maybeFields
+    {safeFields: new Map([
+      ["numberField",false],
+      ["textField",false]
+    ]),
+     maybeFields: new Map([])},
+
+    {safeFields: new Map([
+      ["dateField",["ymd"]],
+      ["textField",false]
+    ]),
+     maybeFields: new Map([
+       ["dateField",["ydm","dmy","mdy"]]
      ])},
-    {full:true, types:new Map([["date", ["ymd"]],["text", false]]),
-     fields: new Map([
-       ["dateField",["ymd"]],
-       ["textField",false]
-     ])},
-    {full:true, types:new Map([["date", ["ydm"]],["text", false]]),
-     fields: new Map([
-       ["dateField",["ydm"]],
-       ["textField",false]
-     ])},
-    {full:true, types:new Map([["date", ["dmy"]],["text", false]]),
-     fields: new Map([
-       ["dateField",["dmy"]],
-       ["textField",false]
-     ])},
-    {full:true, types:new Map([["date", ["mdy"]],["text", false]]),
-     fields: new Map([
-       ["dateField",["mdy"]],
-       ["textField",false]
-     ])},
-    {full:true, types:new Map([["date", ["ymd","ydm"]],["text", false]]),
-     fields: new Map([
-       ["dateField",["ymd","ydm"]],
-       ["textField",false]
+
+    {safeFields: new Map([
+      ["dateField",["ydm"]],
+      ["textField",false]
+    ]),
+     maybeFields: new Map([
+       ["dateField",["ymd","dmy","mdy"]]
      ])},
-    {full:false, types:new Map([["number", false],["text", false]]),
-     fields: new Map([
-       ["numberField",false],
-       ["textField",false]
+
+    {safeFields: new Map([
+      ["dateField",["dmy"]],
+      ["textField",false]
+    ]),
+     maybeFields: new Map([
+       ["dateField",["mdy","ymd"]]
      ])},
-    {full:true, types:new Map([["text", false]]),
-     fields: new Map([
-       ["textField",false]
+
+    {safeFields: new Map([
+      ["dateField",["mdy"]],
+      ["textField",false]
+    ]),
+     maybeFields: new Map([
+       ["dateField",["dmy"]]
      ])},
-    {full:false, types:new Map([["text", false]]),
-     fields: new Map([
-       ["textField",false]
-     ])}
+
+    {safeFields: new Map([
+      ["dateField",["ymd","ydm"]],
+      ["textField",false]
+    ]),
+     maybeFields: new Map([])},
+
+    {safeFields: new Map([
+      ["numberField",false],
+      ["textField",false]
+    ]),
+     maybeFields: new Map([])},
+
+    {safeFields: new Map([
+      ["textField",false]
+    ]),
+     maybeFields: new Map([])},
+
+    {safeFields: new Map([
+      ["textField",false]
+    ]),
+     maybeFields: new Map([])}
   ]);
 });
