Add interactive shrink view to conformance-test-viewer by nbloomf · Pull Request #18 · tweag/cardano-node

nbloomf · 2025-12-13T11:51:54Z

Description

Addresses tweag/cardano-conformance-testing-of-consensus#61.
Depends on #17.

This PR adds a new option --interactive to the conformance-test-viewer tool. When present, the tool enters a primitive REPL that does the following:

Print the selected test case.
Navigate to the parent, left and right sibling, and first child of the current node in the shrink tree, using the arrow keys (Left, Up, Down, and Right respectively), and print a diff of the old and new nodes.
Quit by pressing q.

The input must be supplied by flag in this mode, since we need to use stdin to read online input.

Here is an example. Suppose the file test.json contains "hello world". An interactive session where I press Right, Down, Down, Right, Down looks like this:

cabal run cardano-node:conformance-test-viewer -- --interactive --input=tmp/test.json
"hello world"
[]
-"hello world" +""
[0]
-"" +" world"
[1]
-" world" +"hellod"
[2]
-"hellod" +""
[2,0]
-"" +"lod"
[2,1]

Diffing is supplied by the ToExpr class from quickcheck-state-machine, which can be derived generically.

Checklist

Commit sequence broadly makes sense and commits have useful messages
New tests are added if needed and existing tests are updated. These may include:
- golden tests
- property tests
- roundtrip tests
- integration tests
  See Runnings tests for more details
Any changes are noted in the CHANGELOG.md for affected package
The version bounds in .cabal files are updated
CI passes. See note on CI. The following CI checks are required:
- Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
- Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
- Code builds on Linux, MacOS and Windows for ghc-9.6 and ghc-9.12
Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

isovector

I really love the idea, but am not sold on the implementation. In particular, it seems like we should reuse the ShrinkIndex combinators rather than having a parallel implementation in Data.RoseTree.

isovector · 2025-12-14T16:21:39Z

+    '\ESC' -> do
+      c2 <- getChar
+      c3 <- getChar
+      case (c2, c3) of
+        ('[', 'A') -> pure ToLeftSibling
+        ('[', 'B') -> pure ToRightSibling
+        ('[', 'C') -> pure ToFirstChild
+        ('[', 'D') -> pure ToParent


I guess you're parsing ansi codes for arrows here? But a comment would go a long way!

isovector · 2025-12-14T16:22:57Z

+  | Quit
+  deriving (Eq, Show)
+
+readCommand :: IO Command


Can this instead have type

Suggested change

readCommand :: IO Command

readCommand :: String -> [Command]

?

Laziness will allow us to stream it off stdin, which then means we can drop the unnecessary IO.

isovector · 2025-12-14T16:25:08Z

+  = NoOp
+  | ToParent


How about removing this constructor

Suggested change

= NoOp

| ToParent

= ToParent

and then use Maybe Command instead? Then we get the whole bevy of Maybe combinators like catMaybes or a foldable instance that automatically ignores the no-ops.

isovector · 2025-12-14T16:29:47Z

+      let allShrinks = makeRoseTreeZipper $ applyRoseTreeWithIndex (,) $ makeRoseTree (\x -> (node x, branches x)) $ tree
+      interactWithShrinks allShrinks opts Nothing
+
+interactWithShrinks


This function is really, really cool!

isovector · 2025-12-14T16:32:17Z

+    ToParent -> case toParent zipper of
+      Just zipper' -> interactWithShrinks zipper' opts Nothing
+      Nothing -> interactWithShrinks zipper opts $ Just "already at the root!"


This pattern is crying out for a helper.

withZipper :: (Zipper -> Maybe Zipper) -> String -> Zipper -> ...

and then:

Suggested change

ToParent -> case toParent zipper of

Just zipper' -> interactWithShrinks zipper' opts Nothing

Nothing -> interactWithShrinks zipper opts $ Just "already at the root!"

ToParent -> withZipper toParent "already at the root" zipper

isovector · 2025-12-14T16:39:38Z

+      interactWithShrinks allShrinks opts Nothing
+
+interactWithShrinks
+  :: (Show a, ToExpr a) => RoseTreeZipper ([Int], a) -> Options -> Maybe String -> IO ()


I think this function can/should be decomposed into three pieces:

folding Commands into state updates on the zipper

generating output

some IO plumbing to make it all work

As a general rule, the more code you can move outside of IO the better. Which makes this a prime candidate.

isovector · 2025-12-14T16:40:18Z

+
+
+
+data RoseTree a = RoseTree


This is identical to our ShrinkTree!

isovector · 2025-12-14T16:42:15Z

+makeRoseTree decompose x = case decompose x of
+  (a, bs) -> RoseTree a $ fmap (makeRoseTree decompose) bs


My interpretation of this can't-fail unipattern match is to force the decompose x thunk? If that's on purpose, please document what's going on. If not, I think a let binding would be more idiomatic (and thus avoid me wondering).

isovector · 2025-12-14T16:44:14Z

+  } deriving (Eq, Show, Functor, Foldable, Traversable)
+
+data RoseTreeZipper a
+  = RoseTreeZipper (RoseTree a) [RoseTreeCtx a] (Maybe a)


Please add some per-field haddock (and maybe record names) to document what each of these is.

isovector · 2025-12-14T16:49:13Z

+{-# LANGUAGE RankNTypes #-}
+{-# LANGUAGE ScopedTypeVariables #-}
+
+module Data.RoseTree where


I'm wondering what this module buys us. It seems like we can already do all of these operations via algebra on the ShrinkIndex, and manifest the results via lookup. IMO such would be much more preferable, since it would reuse the logic.

If your concerns are repeatedly paying the lookup costs, we could use MemoTrie to automatically memoize it.

ninioArtillero

I like this new interactive mode. A neat way of exploring a shrinking strategy :) I approve this, as the few concerns I had were already pointed out in @isovector's review: primary wondering that would be better to use the existing shrink index machinery, but I suspect it could be argued someway (which I'm open to).

dpulls · 2025-12-17T16:34:04Z

🎉 All dependencies have been resolved !

nbloomf requested review from isovector and ninioArtillero December 13, 2025 11:51

nbloomf self-assigned this Dec 13, 2025

isovector requested changes Dec 14, 2025

View reviewed changes

ninioArtillero approved these changes Dec 16, 2025

View reviewed changes

Base automatically changed from add/shrink-view-tool to conformance-testing December 17, 2025 16:34

Incorporate feedback

0e638d3

nbloomf force-pushed the add/interactive-shrink-view branch from 6d23363 to 0e638d3 Compare January 5, 2026 23:28

		makeRoseTree decompose x = case decompose x of
		(a, bs) -> RoseTree a $ fmap (makeRoseTree decompose) bs

Uh oh!

Conversation

nbloomf commented Dec 13, 2025

Description

Checklist

Note on CI

Uh oh!

isovector left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ninioArtillero left a comment

Choose a reason for hiding this comment

Uh oh!

dpulls Bot commented Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants