Fixed issue with Metadata.delete, transcript now passes!

Issue was that it was unilaterally deleting the D2 dimension for whatever type was being unlinked (this is the dimension that supports efficient lookup by metadata type - "find me all the docs"). Correct behavior is to only delete from this dimension when there are no more metadata values for a metadata type - for instance, if you delete the last `Doc`, you can delete the index that lets you find all the docs.

parser-typechecker/src/Unison/Codebase/Editor/HandleInput.hs

@@ -435,18 +435,14 @@ loop = do
                         ->  Branch.Star r NameSegment)
                     -> MaybeT (StateT (LoopState m v) (F m (Either Event Input) v)) ()
         manageLinks srcs mdValue2 op = do
-          traceM "In manage links, before srcle"
           let !srcle = toList . getHQ'Terms =<< srcs
               !srclt = toList . getHQ'Types =<< srcs
-          traceM "In manage links, AFTER srcle"
           mdValuel <- toList <$> getHQTerms mdValue2
-          traceM "Got mdValuel"
           names0 <- basicPrettyPrintNames0
           ppe <- PPE.suffixifiedPPE <$> prettyPrintEnvDecl (Names names0 mempty)
           case (srcle, srclt, mdValuel) of
             (srcle, srclt, [r@(Referent.Ref mdValue)]) -> do
               mdType <- eval $ LoadTypeOfTerm mdValue
-              traceM $ "Loaded type for " <> show mdValue
               case mdType of
                 Nothing -> respond $ MetadataMissingType ppe r
                 Just ty -> do

parser-typechecker/src/Unison/Codebase/Metadata.hs

@@ -11,6 +11,7 @@ import Unison.Util.Relation (Relation)
 import qualified Unison.Util.Relation as R
 import Unison.Util.Relation4 (Relation4)
 import qualified Unison.Util.Relation4 as R4
+import qualified Unison.Util.List as List
 
 type Type = Reference
 type Value = Reference
@@ -43,7 +44,15 @@ insert :: (Ord a, Ord n) => (a, Type, Value) -> Star a n -> Star a n
 insert (a, ty, v) = Star3.insertD23 (a, ty, (ty,v))
 
 delete :: (Ord a, Ord n) => (a, Type, Value) -> Star a n -> Star a n
-delete (a, ty, v) = Star3.deleteD23 (a, ty, (ty,v))
+delete (a, ty, v) s = let
+  s' = Star3.deleteD3 (a, (ty,v)) s
+  -- if (ty,v) is the last metadata of type ty
+  -- we also delete (a, ty) from the d2 index
+  metadataByType = List.multimap (toList (R.lookupDom a (Star3.d3 s)))
+  in 
+  case Map.lookup ty metadataByType of
+    Just vs | all (== v) vs -> Star3.deleteD2 (a, ty) s' 
+    _ -> s'
 
 -- parallel composition - commutative and associative
 merge :: Metadata -> Metadata -> Metadata

parser-typechecker/src/Unison/CommandLine/InputPatterns.hs

@@ -1077,7 +1077,7 @@ unlink = InputPattern
       md <- case HQ.fromString md of
         Nothing -> Left "Invalid hash qualified identifier for metadata." 
         Just hq -> pure hq
-      Right $ traceShowId $ Input.UnlinkI defs md
+      Right $ Input.UnlinkI defs md
     _ -> Left (I.help unlink)
   )
 

parser-typechecker/src/Unison/CommandLine/OutputMessages.hs

@@ -1301,7 +1301,7 @@ listOfLinks ::
 listOfLinks _ [] = pure . P.callout "😶" . P.wrap $
   "No results. Try using the " <>
   IP.makeExample IP.link [] <>
-  "command to add outgoing links to a definition."
+  "command to add metadata to a definition."
 listOfLinks ppe results = pure $ P.lines [
     P.numberedColumn2 num [
     (P.syntaxToColor $ prettyHashQualified hq, ": " <> prettyType typ) | (hq,typ) <- results

parser-typechecker/src/Unison/Util/Star3.hs

@@ -160,13 +160,19 @@ insertD23 (f, x, y) s = Star3 fact' (d1 s) d2' d3' where
   d2'   = R.insert f x (d2 s)
   d3'   = R.insert f y (d3 s)
 
-deleteD23 :: (Ord fact, Ord d1, Ord d2, Ord d3)
+deleteD3 :: (Ord fact, Ord d1, Ord d2, Ord d3)
-          => (fact, d2, d3)
+          => (fact, d3)
           -> Star3 fact d1 d2 d3
           -> Star3 fact d1 d2 d3
-deleteD23 (f, x, y) s = Star3 (fact s) (d1 s) d2' d3' where
+deleteD3 (f, x) s = Star3 (fact s) (d1 s) (d2 s) d3' where
+  d3' = R.delete f x (d3 s)
+
+deleteD2 :: (Ord fact, Ord d1, Ord d2, Ord d3)
+          => (fact, d2)
+          -> Star3 fact d1 d2 d3
+          -> Star3 fact d1 d2 d3
+deleteD2 (f, x) s = Star3 (fact s) (d1 s) d2' (d3 s) where
   d2' = R.delete f x (d2 s)
-  d3' = R.delete f y (d3 s)
 
 deleteFact :: (Ord fact, Ord d1, Ord d2, Ord d3)
            => Set fact -> Star3 fact d1 d2 d3 -> Star3 fact d1 d2 d3

unison-src/transcripts/fix1356.md

@@ -37,13 +37,16 @@ Step 6: Great that seems to have worked but let me check just in case
 .> display 1
 ```
 
-Step 7: I have been guided to check `display 1`, so now let's try and unlink
+Step 7: I have been guided to check `display 1`, so now let's try and unlink to get rid of the second link. We can remove by number, or by hash:
-to get rid of the second link.
 
 ```ucm
-.> unlink x.doc x
+.> unlink 2 x
+.> undo
+.> unlink #v8f1hhvs57 x
 ```
 
-Issuing the `unlink x.doc x` command here breaks ucm, and no transcript output is generated.
+We expect that after the `unlink`, the `docs` command now works as there's a single `Doc` again:
-The captured output is `ucm: Maybe.fromJust: Nothing`
 
+```ucm
+.> docs x
+```

unison-src/transcripts/fix1356.output.md

@@ -0,0 +1,140 @@
+##### This transcript reproduces the failure to unlink documentation
+
+Step 1: code a term and documentation for it
+```unison
+x = 1
+x.doc = [: I am the documentation for x:]
+```
+
+```ucm
+
+  I found and typechecked these definitions in scratch.u. If you
+  do an `add` or `update`, here's how your codebase would
+  change:
+  
+    ⍟ These new definitions are ok to `add`:
+    
+      x     : Nat
+      x.doc : Doc
+   
+  Now evaluating any watch expressions (lines starting with
+  `>`)... Ctrl+C cancels.
+
+```
+Step 2: add term and documentation, link, and check the documentation
+```ucm
+.> add
+
+  ⍟ I've added these definitions:
+  
+    x     : Nat
+    x.doc : Doc
+
+.> link x.doc x
+
+  Updates:
+  
+    1. x : Nat
+       + 2. x.doc : Doc
+
+.> docs x
+
+  I am the documentation for x
+
+```
+Step 3: Oops I don't like the doc, so I will re-code it!
+```unison
+x.doc = [: I am the documentation for x, and I now look better:]
+```
+
+```ucm
+
+  I found and typechecked these definitions in scratch.u. If you
+  do an `add` or `update`, here's how your codebase would
+  change:
+  
+    ⍟ These names already exist. You can `update` them to your
+      new definition:
+    
+      x.doc : Doc
+   
+  Now evaluating any watch expressions (lines starting with
+  `>`)... Ctrl+C cancels.
+
+```
+Step 4: I add it and expect to see it
+```ucm
+.> update
+
+  ⍟ I've updated these names to your new definition:
+  
+    x.doc : base.Doc
+
+.> docs x
+
+  I am the documentation for x
+
+```
+But I don't see an update, so from here on I am in panic mode and have no clue what I am doing!
+
+Step 5: maybe re-link it?
+```ucm
+.> link x.doc x
+
+  Updates:
+  
+    1. x : Nat
+       + 2. x.doc : Doc
+
+```
+Step 6: Great that seems to have worked but let me check just in case
+
+```ucm
+.> docs x
+
+  1. x.doc            : Doc
+  2. x.doc#v8f1hhvs57 : Doc
+  
+  Tip: Try using `display 1` to display the first result or
+       `view 1` to view its source.
+
+.> display 1
+
+  I am the documentation for x, and I now look better
+
+```
+Step 7: I have been guided to check `display 1`, so now let's try and unlink to get rid of the second link. We can remove by number, or by hash:
+
+```ucm
+.> unlink 2 x
+
+  Updates:
+  
+    1. x : Nat
+       - 2. (unnamed metadata) : Doc
+
+.> undo
+
+  Here's the changes I undid
+  
+  Updates:
+  
+    1. x : Nat
+       - 2. (unnamed metadata) : Doc
+
+.> unlink #v8f1hhvs57 x
+
+  Updates:
+  
+    1. x : Nat
+       - 2. (unnamed metadata) : Doc
+
+```
+We expect that after the `unlink`, the `docs` command now works as there's a single `Doc` again:
+
+```ucm
+.> docs x
+
+  I am the documentation for x, and I now look better
+
+```