feat(bisect): add guessing game example

In doing this, I improved some of the comments/assertions on the consumer interface that became apparent when implementing the example.

I found some bugs by hand while testing and proptest also found the same bugs.

Author: arxanas

scm-bisect/examples/guessing_game.proptest-regressions

@@ -0,0 +1,9 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 67760b0a17386062ba77f3c425a851fa115bcdcb9fbe2697c66e2a1867e600a7 # shrinks to inputs = [Less, Less, Less, Greater, Less, Less, Less]
+cc 30da0c32ee62618d38b87f1ef1fe8b135a6703ca4b5965e29da5ecf47f5860e9 # shrinks to input = 0
+cc f7618c6f857bd07b2c8f1b086c2ca175e73d1a6189b1d14d7ffb0f7ddd35ae68 # shrinks to input = 100

scm-bisect/examples/guessing_game.rs

@@ -0,0 +1,135 @@
+use std::cmp::Ordering;
+use std::collections::HashSet;
+use std::convert::Infallible;
+use std::io;
+use std::ops::RangeInclusive;
+
+use indexmap::IndexMap;
+use scm_bisect::search;
+
+type Node = isize;
+
+#[derive(Debug)]
+struct Graph;
+
+impl search::Graph for Graph {
+    type Node = Node;
+
+    type Error = Infallible;
+
+    fn is_ancestor(
+        &self,
+        ancestor: Self::Node,
+        descendant: Self::Node,
+    ) -> Result<bool, Self::Error> {
+        // Note that a node is always considered an ancestor of itself.
+        Ok(ancestor <= descendant)
+    }
+}
+
+#[derive(Debug)]
+struct Strategy {
+    range: RangeInclusive<Node>,
+}
+
+impl search::Strategy<Graph> for Strategy {
+    type Error = Infallible;
+
+    fn midpoint(
+        &self,
+        _graph: &Graph,
+        success_bounds: &HashSet<Node>,
+        failure_bounds: &HashSet<Node>,
+        _statuses: &IndexMap<Node, search::Status>,
+    ) -> Result<Option<Node>, Self::Error> {
+        let lower_bound = success_bounds
+            .iter()
+            .max()
+            .copied()
+            .unwrap_or_else(|| self.range.start() - 1);
+        let upper_bound = failure_bounds
+            .iter()
+            .min()
+            .copied()
+            .unwrap_or_else(|| self.range.end() + 1);
+        let midpoint = if lower_bound < upper_bound - 1 {
+            (lower_bound + upper_bound) / 2
+        } else {
+            return Ok(None);
+        };
+        assert!(self.range.contains(&midpoint));
+        Ok(Some(midpoint))
+    }
+}
+
+fn play<E>(mut read_input: impl FnMut(isize) -> Result<Ordering, E>) -> Result<Option<isize>, E> {
+    let search_range = 0..=100;
+    let mut search = search::Search::new(Graph, search_range.clone());
+    let strategy = Strategy {
+        range: search_range,
+    };
+
+    let result = loop {
+        let guess = {
+            let mut guess = search.search(&strategy).unwrap();
+            match guess.next_to_search.next() {
+                Some(guess) => guess.unwrap(),
+                None => {
+                    break None;
+                }
+            }
+        };
+        let input = read_input(guess)?;
+        match input {
+            Ordering::Less => search.notify(guess, search::Status::Failure).unwrap(),
+            Ordering::Greater => search.notify(guess, search::Status::Success).unwrap(),
+            Ordering::Equal => {
+                break Some(guess);
+            }
+        }
+    };
+    Ok(result)
+}
+
+fn main() -> io::Result<()> {
+    println!("Think of a number between 0 and 100.");
+    let result = play(|guess| -> io::Result<_> {
+        println!("Is your number {guess}? [<=>]");
+        let result = loop {
+            let mut input = String::new();
+            std::io::stdin().read_line(&mut input)?;
+            match input.trim() {
+                "<" => break Ordering::Less,
+                "=" => break Ordering::Equal,
+                ">" => break Ordering::Greater,
+                _ => println!("Please enter '<', '=', or '>'."),
+            }
+        };
+        Ok(result)
+    })?;
+    match result {
+        Some(result) => println!("I win! Your number was: {result}"),
+        None => println!("I give up!"),
+    }
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    proptest::proptest! {
+        #[test]
+        fn test_no_crashes_on_valid_input(inputs: Vec<Ordering>) {
+            struct Exit;
+            let mut iter = inputs.into_iter();
+            let _: Result<Option<isize>, Exit> = play(move |_| iter.next().ok_or(Exit));
+        }
+
+        #[test]
+        fn test_finds_number(input in 0..=100_isize) {
+            let result = play(|guess| -> Result<Ordering, Infallible> { Ok(input.cmp(&guess)) });
+            assert_eq!(result, Ok(Some(input)));
+        }
+    }
+}

scm-bisect/src/search.rs

@@ -16,16 +16,25 @@ pub trait Graph: Debug {
     /// An error type.
     type Error: std::error::Error;
 
-    /// Return whether or not `node` is an ancestor of `descendant`.
+    /// Return whether or not `node` is an ancestor of `descendant`. A node `X``
+    /// is said to be an "ancestor" of node `Y` if one of the following is true:
+    ///
+    /// - `X == Y`
+    /// - `X` is an immediate parent of `Y`.
+    /// - `X` is an ancestor of an immediate parent of `Y` (defined
+    /// recursively).
     fn is_ancestor(
         &self,
         ancestor: Self::Node,
         descendant: Self::Node,
     ) -> Result<bool, Self::Error>;
 
     /// Filter `nodes` to only include nodes that are not ancestors of any other
-    /// node in `nodes`. This is not strictly necessary, but it makes the
-    /// intermediate results more sensible.
+    /// node in `nodes`. This is not strictly necessary, but it improves
+    /// performance as some operations are linear in the size of the success
+    /// bounds, and it can make the intermediate results more sensible.
+    ///
+    /// This operation is called `heads` in e.g. Mercurial.
     #[instrument]
     fn simplify_success_bounds(
         &self,
@@ -35,8 +44,11 @@ pub trait Graph: Debug {
     }
 
     /// Filter `nodes` to only include nodes that are not descendants of any
-    /// other node in `nodes`. This is not strictly necessary, but it makes the
-    /// intermediate results more sensible.
+    /// other node in `nodes`. This is not strictly necessary, but it improves
+    /// performance as some operations are linear in the size of the failure
+    /// bounds, and it can make the intermediate results more sensible.
+    ///
+    /// This operation is called `roots` in e.g. Mercurial.
     #[instrument]
     fn simplify_failure_bounds(
         &self,
@@ -102,6 +114,10 @@ pub trait Strategy<G: Graph>: Debug {
     /// For example, linear search would return a node immediately "after"
     /// the node(s) in `success_bounds`, while binary search would return the
     /// node in the middle of `success_bounds` and `failure_bounds`.`
+    ///
+    /// NOTE: This must not return a value that has already been included in the
+    /// success or failure bounds, since then you would search it again in a
+    /// loop indefinitely. In that case, you must return `None` instead.
     fn midpoint(
         &self,
         graph: &G,
@@ -150,7 +166,10 @@ pub struct EagerSolution<Node: Debug + Hash + Eq> {
 
 #[allow(missing_docs)]
 #[derive(Debug, thiserror::Error)]
-pub enum SearchError<TGraphError, TStrategyError> {
+pub enum SearchError<TNode, TGraphError, TStrategyError> {
+    #[error("node {node:?} has already been classified as a {status:?} node, but was returned as a new midpoint to search; this would loop indefinitely")]
+    AlreadySearchedMidpoint { node: TNode, status: Status },
+
     #[error(transparent)]
     Graph(TGraphError),
 
@@ -247,8 +266,8 @@ impl<G: Graph> Search<G> {
         &'a self,
         strategy: &'a S,
     ) -> Result<
-        LazySolution<G::Node, SearchError<G::Error, S::Error>>,
-        SearchError<G::Error, S::Error>,
+        LazySolution<G::Node, SearchError<G::Node, G::Error, S::Error>>,
+        SearchError<G::Node, G::Error, S::Error>,
     > {
         let success_bounds = self.success_bounds().map_err(SearchError::Graph)?;
         let failure_bounds = self.failure_bounds().map_err(SearchError::Graph)?;
@@ -268,7 +287,7 @@ impl<G: Graph> Search<G> {
         }
 
         impl<'a, G: Graph, S: Strategy<G>> Iterator for Iter<'a, G, S> {
-            type Item = Result<G::Node, SearchError<G::Error, S::Error>>;
+            type Item = Result<G::Node, SearchError<G::Node, G::Error, S::Error>>;
 
             fn next(&mut self) -> Option<Self::Item> {
                 while let Some(state) = self.states.pop_front() {
@@ -290,6 +309,31 @@ impl<G: Graph> Search<G> {
                         Err(err) => return Some(Err(SearchError::Strategy(err))),
                     };
 
+                    for success_node in success_bounds.iter() {
+                        match self.graph.is_ancestor(node.clone(), success_node.clone()) {
+                            Ok(true) => {
+                                return Some(Err(SearchError::AlreadySearchedMidpoint {
+                                    node,
+                                    status: Status::Success,
+                                }));
+                            }
+                            Ok(false) => (),
+                            Err(err) => return Some(Err(SearchError::Graph(err))),
+                        }
+                    }
+                    for failure_node in failure_bounds.iter() {
+                        match self.graph.is_ancestor(failure_node.clone(), node.clone()) {
+                            Ok(true) => {
+                                return Some(Err(SearchError::AlreadySearchedMidpoint {
+                                    node,
+                                    status: Status::Failure,
+                                }));
+                            }
+                            Ok(false) => (),
+                            Err(err) => return Some(Err(SearchError::Graph(err))),
+                        }
+                    }
+
                     // Speculate failure:
                     self.states.push_back(State {
                         success_bounds: success_bounds.clone(),