oracle-samples/clara-rules

Provide information on facts accumulated over in session inspection

Closed this issue · 5 comments

Clara's session inspection functionality currently doesn't provide visibility into the facts that were accumulated over. So, for example, if we have a condition like

[?coldest-temp <- (acc/min :temperature :returns-fact true) :from [Cold]]

and we had a session with the facts

[(->Cold -10) (->Cold -20)]

session inspection would provide the user with the value of ?coldest-temp when this condition matched, but would not provide visibility into the individual Cold facts that were accumulated over. In this case the logic is trivial, but in cases where there are nontrivial checks on facts before accumulation it would be helpful to know which facts passed the checks and were thus accumulated on. Furthermore, while simple cases like the example above are easy to reason about for a direct human consumer, tooling built on top of session inspection needs the information to be explicitly provided by Clara.

Note that after #190 the engine keeps references to the facts that match accumulators (rather than discarding them after calculating the accumulator result as previously; see that issue for discussion of why the changes were made.

I'm looking at adding the accumulated facts to the Explanation record.

However, there is a question of passitivity. If I were doing this from a blank slate, I'd go with a schema like

(s/defrecord Explanation [matches :- [{:fact s/Any 
                                       :condition schema/Condition 
                                       (s/optional-key :accumulated-facts) [s/Any]]
                          bindings :- {s/Keyword s/Any}])

However, if anyone is consuming Explanation records programmatically, this would obviously be a non-passive change, although all of the data currently present on the explanation would still be present. This could be made passive by adding the accumulated-facts onto the end of each match tuple if present, but this would be problematic if we ever decided to add something else to the :matches.

On balance, I'm currently learning toward making the non-passive uplift for the :matches to be maps. If anyone has built tooling on top of session inspection, there would need to be a relatively small uplift, but turning the matches into maps will facilitate future improvements, since any more additions that need to be made will just be additional keys on the map (and thus their addition would be passive).

Thoughts?

I think it's okay to make this breaking change in a non-fix release. I haven't yet heard of use of these explanation records outside of Cerner, and it's small enough that I think it's fair to ask any users to change when upgrading to a new version.

I have created a PR for this at #279. I added another protocol in the engine to retrieve the information, which is some additional complexity there, but it is minimal enough that I think it is worth it. Since the way to retrieve the facts is dependent on the way the AccumulateNode and AccumulateWithJoinFilterNode store data in the memory I think it is appropriate for that retrieval code to be part of the node implementations rather than the inspect namespace.

I have merged the PR and don't see anything left to do for this issue. Closing.