Retraction broken when using custom :fact-type-fn

[jpthomson]

Not sure if this is a known limitation? This test:

(clojure.test/is
 (= (do

      ;; Records

      (defrecord A [])
      (defrecord B [])

      (defrule a->b
        [A] => (insert! (->B)))

      (defquery b? []
        [?b <- B])

      (-> (mk-session [a->b b?])
          (insert (->A))
          (fire-rules)
          (retract (->A))
          (query b?)))

    (do

      ;; Maps

      (defn ->a [] {:type :a})
      (defn ->b [] {:type :b})

      (defrule a->b
        [:a] => (insert! (->b)))

      (defquery b? []
        [?b <- :b])

      (-> (mk-session [a->b b?] :fact-type-fn :type)
          (insert (->a))
          (fire-rules)
          (retract (->a))
          (query b?)))))

fails with

(not (= () ({:?b {:type :b}})))

Haven't had a chance to dig into why yet, just posting incase I'm missing something obvious or well-known!

[rbrush]

This looks like a bug. Thanks for reporting it! I poked around the code a bit and have a pretty good idea of what's going on and will get a fix posted soon.

[rbrush]

I believe this is fixed in the above commit. The problem was that the truth maintenance flow didn't have the custom type function provided by the user, and was always using Clojure's built-in type. This is an edge case we hadn't run into since we don't make heavy use of custom type functions, but it's definitely an important one and I'm glad we found and fixed it.

The key change is around line 281 in engine.clj. I had to add the needed function to the working memory data structure to make it visible at that point in the flow. I also added some unit tests to be sure we don't break this in the future.

If this works for your use case I can get it merged into master and push out a fix release without too much trouble.

Thanks for reporting this and for posting a simple unit test! It made finding it much easier.

-Ryan

[jpthomson]

Thanks so much Ryan!

I noticed there wasn't a unit test for custom type fns and retract, so can understand how this might have slipped through the net! All makes sense and the fix definitely works for me.

I'm hoping to use Clara as the 'view model' in a reagent / datomic app - so will likely touch on a few other less well trodden aspects. Happy to help solve problems as they arise once I'm a little more familiar with the code base!

So far it's been smooth sailing, so thank you for all the work you've put in.

-Jonathan

[rbrush]

Hey Jonathan,

Sounds great! I went ahead and merged this to master, so I think we can consider this closed. Feel free to reopen or open another issue if you disagree.

Cool use case, by the way. If you do run into other edge cases in your app please don't hesitate to report them here. My use of Clara is heavily oriented to Clojure rather than ClojureScript, but all of this only makes the system stronger.

Thanks!

-Ryan