Don't allow empty clauses in feature vector indices

src/SCFirstOrderLogic.Tests/ClauseIndexing/FeatureVectorIndexTests.cs

@@ -2,16 +2,34 @@
 using FlUnit;
 using SCFirstOrderLogic.ClauseIndexing.Features;
 using SCFirstOrderLogic.TestData;
+using System;
 using System.Collections.Generic;
 using System.Linq;
+using static SCFirstOrderLogic.SentenceCreation.OperableSentenceFactory;
 
 namespace SCFirstOrderLogic.ClauseIndexing;
 
 public static class FeatureVectorIndexTests
 {
     private record GetTestCase(CNFClause Query, CNFClause[] Expected, CNFClause[] NotExpected);
 
-    // TODO: shouldn't be allowed to add empty clause
+    private record AddTestCase(CNFClause[] PriorContent, CNFClause Add);
+
+    public static Test NegativeAddBehaviour => TestThat
+        .GivenEachOf<AddTestCase>(() =>
+        [
+            new([], CNFClause.Empty),
+            ////new([new(P(U))], new(P(V)))
+        ])
+        .When(tc =>
+        {
+            var index = new FeatureVectorIndex<OccurenceCountFeature>(
+                OccurenceCountFeature.MakeFeatureVector,
+                OccurenceCountFeature.MakeFeatureComparer(Comparer<object>.Default),
+                tc.PriorContent);
+            index.Add(tc.Add);
+        })
+        .ThenThrows((_, ex) => ex.Should().BeOfType<ArgumentException>());
 
     public static Test GetSubsumedBehaviour => TestThat
         .GivenEachOf(() =>
@@ -114,4 +132,6 @@ private static CNFClause[] GetNonSubsumingClausesFromSubsumptionFacts(CNFClause
 
         return xClauses.Concat(yClauses).Except([CNFClause.Empty]).Distinct().ToArray();
     }
+
+    private static OperablePredicate P(params OperableTerm[] arguments) => new(nameof(P), arguments);
 }

src/SCFirstOrderLogic/ClauseIndexing/AsyncFeatureVectorIndexDictionaryNode{TFeature,TValue}.cs

@@ -23,7 +23,8 @@ namespace SCFirstOrderLogic.ClauseIndexing;
 /// </summary>
 /// <typeparam name="TFeature">The type of the keys of the feature vectors.</typeparam>
 /// <typeparam name="TValue">The type of the value associated with each stored clause.</typeparam>
-// todo: ordered lists make way more sense than dictionaries here..
+// todo-breaking-v7: ordered lists for children make way more sense than dictionaries here, but this does of course
+// raise questions about what should be responsible for the feature comparison logic
 public class AsyncFeatureVectorIndexDictionaryNode<TFeature, TValue> : IAsyncFeatureVectorIndexNode<TFeature, TValue>
     where TFeature : notnull
 {

src/SCFirstOrderLogic/ClauseIndexing/AsyncFeatureVectorIndex{TFeature,TValue}.cs

@@ -120,6 +120,11 @@ public async Task AddAsync(CNFClause key, TValue value)
     {
         ArgumentNullException.ThrowIfNull(key);
 
+        if (key == CNFClause.Empty)
+        {
+            throw new ArgumentException("The empty clause is not a valid key", nameof(key));
+        }
+
         var currentNode = root;
         foreach (var element in MakeOrderedFeatureVector(key))
         {

src/SCFirstOrderLogic/ClauseIndexing/FeatureVectorIndexDictionaryNode{TFeature,TValue}.cs

@@ -13,7 +13,8 @@ namespace SCFirstOrderLogic.ClauseIndexing;
 /// </summary>
 /// <typeparam name="TFeature">The type of the keys of the feature vectors.</typeparam>
 /// <typeparam name="TValue">The type of the value associated with each stored clause.</typeparam>
-// todo: ordered lists make way more sense than dictionaries here..
+// todo-breaking-v7: ordered lists for children make way more sense than dictionaries here, but this does of course
+// raise questions about what should be responsible for the feature comparison logic
 public class FeatureVectorIndexDictionaryNode<TFeature, TValue> : IFeatureVectorIndexNode<TFeature, TValue>
     where TFeature : notnull
 {

src/SCFirstOrderLogic/ClauseIndexing/FeatureVectorIndex{TFeature,TValue}.cs

@@ -187,6 +187,11 @@ public void Add(CNFClause key, TValue value)
     {
         ArgumentNullException.ThrowIfNull(key);
 
+        if (key == CNFClause.Empty)
+        {
+            throw new ArgumentException("The empty clause is not a valid key", nameof(key));
+        }
+
         var currentNode = root;
         foreach (var component in MakeOrderedFeatureVector(key))
         {

src/SCFirstOrderLogic/SentenceManipulation/Normalisation/CNFConversion.cs

@@ -46,7 +46,7 @@ public static Sentence ApplyTo(Sentence sentence)
 
         // It might be possible to do some of these conversions at the same time, but for now
         // at least we do them sequentially - and in so doing favour maintainability over performance.
-        // Perhaps revisit this later (but given that the main purpose of this library is learning, probably not).
+        // Perhaps revisit this later.
         sentence = implicationElimination.ApplyTo(sentence);
         sentence = nnfConversion.ApplyTo(sentence);
         sentence = new Skolemisation(standardisedSentence).ApplyTo(sentence);