Left-retract on ProductionNode causes excessive retractions for tokens with both pending activations and logical insertions
Closed this issue · 4 comments
In the implementation of left-retract on ProductionNode [1] we currently take every token that is being retracted, attempt to remove a corresponding pending activation, and then attempt to retract a logical insertion that was based on that token. As a result a single retracted token can cause both a pending activation and a previous logical insertion to be removed. From a user-facing perspective, in scenarios where one of a fact should be retracted two could be retracted instead. To give a concrete example, I'd expect the following test to pass, but it doesn't:
(deftest test-remove-pending-activation-with-equal-previous-insertion
(let [lousy-weather-rule (dsl/parse-rule [[?cw <- ColdAndWindy]]
(insert! (->LousyWeather)))
lousy-weather-query (dsl/parse-query [] [[LousyWeather]])
empty-session (mk-session [lousy-weather-rule lousy-weather-query] :cache false)
end-session (-> empty-session
(insert (->ColdAndWindy 10 10))
fire-rules
(insert (->ColdAndWindy 10 10))
(retract (->ColdAndWindy 10 10))
fire-rules)]
(is (= (query end-session lousy-weather-query)
[{}]))))
FAIL in (test-remove-pending-activation-with-equal-previous-insertion) (test_rules.clj:5013)
expected: (= (query end-session lousy-weather-query) [{}])
actual: (not (= () [{}]))
nil
I have a draft of a fix for this at WilliamParker@7c78128
@WilliamParker I had a few comments there. However, overall this was a sneaky issue and a good find.
I have created a PR that addresses this issue at #252
The above PR has been merged, which resolves this issue. Closing.