Skip to content

Run All with unapproved chatbook cells executes out of cell order with no indication #469

Description

@pjdoland

Summary

Running "Run All Cells" on a chatbook that contains unapproved natural-language cells produces a notebook whose execution order does not match its cell order, and nothing in the UI says so. This is a consequence of the confirm-bar design working as intended, not a bug in the send-ordering logic, so I am filing it as a design question rather than a defect.

What happens

With "Always confirm" enabled and a three-cell chatbook where cell 2 is an NL cell that has not been approved yet:

  1. Cell 1 runs and gets [2].
  2. Cell 2 shows its confirm bar and does not execute.
  3. Cell 3 runs immediately and gets [3].
  4. Approving cell 2 afterwards runs it, and it gets [4].

Cell 2 therefore executes against state that cell 3 has already mutated. On a second Run All, with the generated code cached and approved, all three run in order, which is why this only shows up on first approval.

Why this is the designed behavior

An unapproved NL cell answers its execute_request immediately with a reply that carries no execution count, rather than holding the reply until the user decides. Run All awaits each CodeCell.execute in turn, sees that reply resolve, and moves to the next cell. sendInChatbookOrder pins the order in which requests are sent, which it does correctly; it is not involved in whether a cell's work actually ran.

Holding the reply until approval would keep the notebook in cell order, but it would block the kernel's message queue behind a dialog waiting on a human, stalling everything else in the notebook. The current tradeoff looks like the right one.

The gap

The cost of that tradeoff is invisible to the user. After a Run All with pending confirmations, the notebook's execution counts are the only evidence that cells ran out of order, and a cell approved later silently runs against a state its author would not expect (later cells may have redefined names it reads, or it may overwrite values later cells computed).

Some options, roughly in order of cost:

  1. Say something when Run All finishes with cells still awaiting confirmation, so the reordering is at least acknowledged.
  2. Mark cells that were skipped pending approval distinctly from cells that simply have no output yet.
  3. Offer to run the approved cells as an ordered batch after approval, rather than one at a time in approval order.

Happy to prototype whichever direction seems right, or to close this if the current behavior is considered acceptable and understood.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions