tools/flow: if comprehension bug

[jlongtine]

What version of CUE are you using (cue version)?

$ cue version
cue version v0.4.3-beta.1 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What did you do?

With the following main_tool.cue

package main

import (
	"tool/cli"
	"tool/exec"
)

// Say hello!
command: prompter: {
	ask: cli.Ask & {
		prompt:   "What is your name?"
		response: string
	}

	// starts after ask
	echo: exec.Run & {
		cmd: ["echo", "Hello", ask.response + "!"]
		stdout: string // capture stdout
	}

	// also starts after echo
	if len(ask.response) > 10 {
		printLong: cli.Print & {
			text: echo.stdout + " - You have a long name"
		}
	}
	if len(ask.response) <= 10 {
		printShort: cli.Print & {
			text: echo.stdout + " - You have a short name"
		}
	}
}

Run

$ cue cmd prompter

What did you expect to see?

For the ask, echo and either printLong or printShort tasks to run (depending on the user input).

What did you see instead?

command.prompter: incomplete argument string (type string):
    ./main_tool.cue:32:5
    ./main_tool.cue:17:13
command.prompter: incomplete argument string (type string):
    ./main_tool.cue:38:5
    ./main_tool.cue:17:13

Additional Info

We're seeing this same error in Dagger, with the tools/flow API and similar task definitions.

Also, this works, which is... interesting:

package main

import (
	"tool/cli"
	"tool/exec"
)

// Say hello!
command: prompter: {
	// save transcript to this file
	var: {
		file: *"out.txt" | string @tag(file)
	}

	ask: cli.Ask & {
		prompt:   "What is your name?"
		response: string
	}

	// starts after ask
	echo: exec.Run & {
		cmd: ["echo", "Hello", ask.response + "!"]
		stdout: string // capture stdout
	}

	stuff: {
		echo2: exec.Run & {
			cmd: ["echo", "Hello again", ask.response + "!"]
		}
		// also starts after echo
		if len(ask.response) > 10 {
			printLong: cli.Print & {
				text: echo.stdout + " - You have a long name"
			}
		}
		if len(ask.response) <= 10 {
			printShort: cli.Print & {
				text: echo.stdout + " - You have a short name"
			}
		}
	}
}

[verdverm]

Note sure this is unique to the beta, seeing the same result with v0.4.2

A couple of things I found

This does not work, seems the definition for cli.Ask is missing the id field, possibly related to the reduced backwards compatibility lookup? (edit: already seems fixed though https://github.com/cue-lang/cue/commit/977d3532536e1a10053c63cddfe357c33d897f02)

package main

import (
	"tool/cli"
)

// Say hello!
command: prompter: {
	ask: cli.Ask & {
    // $id: "tool/cli.Ask"
		prompt:   "What is your name?"
		response: string
	}
}

This does work as expected, note the conditional inside vs outside of tasks. I believe this bug already has an issue.

package main

import (
	"tool/cli"
	"tool/exec"
)

// Say hello!
command: prompter: {
	ask: cli.Ask & {
    $id: "tool/cli.Ask"
		prompt:   "What is your name?"
		response: string
	}

	// starts after ask
	echo: exec.Run & {
		cmd: ["echo", "Hello", ask.response + "!"]
		stdout: string // capture stdout
	}

	// also starts after echo
  print: cli.Print & {
    if len(ask.response) > 10 {
        text: echo.stdout + " - You have a long name"
    }
    if len(ask.response) <= 10 {
      text: echo.stdout + " - You have a short name"
    }
	}
}

[verdverm]

This also seems to work, note the foo which wraps the guarded tasks

package main

import (
	"tool/cli"
	"tool/exec"
)

// Say hello!
command: prompter: {
	ask: cli.Ask & {
    $id: "tool/cli.Ask"
		prompt:   "What is your name?"
		response: string
	}

	// starts after ask
	echo: exec.Run & {
		cmd: ["echo", "Hello", ask.response + "!"]
		stdout: string // capture stdout
	}

	// also starts after echo
  foo: {
    if len(ask.response) > 10 {
      print: cli.Print & {
        text: echo.stdout + " - You have a long name"
      }
    }
    if len(ask.response) <= 10 {
      printShort: cli.Print & {
        text: echo.stdout + " - You have a short name"
      }
    }
  }
}

[mpvl]

I was able to reproduce the problem. It only seems to occur with some value types and not others. That is why existing tests that do similar things did not catch this case.

[mpvl]

Ah, of course. The cases where it worked there was a(n implicit) default value, like [...int].

Also, this works, which is... interesting:

This also makes sense.

Basically, because the if comprehensions are in the same struct as they depend on, CUE evaluation will fail. So in the first example, the comprehensions need to be evaluated to complete the value of prompter, which happens to be the parent of all tasks. This means that no task can be evaluated to completion, including ask.

However, by putting it in stuff, task ask can evaluate to completion independent of stuff, which will be associated an "incomplete" error.

We could consider ignoring comprehensions that are needed to create a task. In the comprehension rework, this may work out quite nicely, as the cycle semantics changes slightly, which would make it much clearer when this is meaningful or not. So we may actually allow this in certain cases, depending on to which node we assign errors associated with comprehensions.

So actually, the cases were it currently works (where there is a default value), are actually the buggy cases. Basically, if a comprehension depends on default value of another tasks, it should still run after this task, and not assume the default value.

This is related to #1568 and this case is a strong argument that we should always run a tasks B after a task A if B depends on any value of A, even if that value is concrete. That would solve these buggy cases.

[mpvl]

So depending on the exact definition of how we attribute errors after the comprehension rework, this Issue will be either a feature request or a bug. :).

[mpvl]

I would suggest for now the proper way to do this is:

// Say hello!
command: prompter: {
	ask: cli.Ask & {
		prompt:   "What is your name?"
		response: string
	}

	// starts after ask
	echo: exec.Run & {
		cmd: ["echo", "Hello", ask.response + "!"]
		stdout: string // capture stdout
	}

	// also starts after echo
	if ask.response+"" != _|_ {
		if len(ask.response) > 10 {
			printLong: cli.Print & {
				text: echo.stdout + " - You have a long name"
			}
		}
		if len(ask.response) <= 10 {
			printShort: cli.Print & {
				text: echo.stdout + " - You have a short name"
			}
		}
	}
}

We can look at something more idiomatic.

[myitcv]

@mpvl just to check, the following would also be sufficient?

exec cue cmd prompter

-- x_tool.cue --
package x

import (
	"tool/cli"
	"tool/exec"
)

// Say hello!
command: prompter: {
	ask: cli.Ask & {
		prompt:   "What is your name?"
		response: string
	}

	// starts after ask
	echo: exec.Run & {
		cmd: ["echo", "Hello", ask.response + "!"]
		stdout: string // capture stdout
	}

	respond: {
		if len(ask.response) > 10 {
			printLong: cli.Print & {
				text: echo.stdout + " - You have a long name"
			}
		}
		if len(ask.response) <= 10 {
			printShort: cli.Print & {
				text: echo.stdout + " - You have a short name"
			}
		}
	}
}

[myitcv]

The v0.5.x milestone is now closed because we have moved onto focus on v0.6.0. Hence moving this to v0.6.x. This particular issue is not critical to the release of v0.6.0, but completing it as a stretch goal would be good.