exec plugin: Introduce a State Machine

[winder]

What

Introduce a state machine to manage execution across multiple rounds of OCR.

Why

To help manage complexity when a 3rd round of OCR is required.

[makramkd]

Can we have a diagram with the states and the state transitions?

[winder]

Can we have a diagram with the states and the state transitions?

In this PR

flowchart TD
    UNKNOWN[Initial State]
    A[Get Commit Reports] 
    B[Get Report Messages]
    C[Filter]
    UNKNOWN --> A
    A -->|no report| B
    B -->|report + restart| A

Loading

In the next PR

flowchart TD
    UNKNOWN[Initial State]
    A[Get Commit Reports] 
    B[Get Report Messages]
    C[Filter]
    UNKNOWN --> A
    A -->|no report| B
    B -->|no report| C
    C -->|report + restart| A

Loading

[asoliman92]

// Unknown is the default value indicating there was no previous state.

If there was no previous state, should it start getting new messages? Are we going to start the plugin state with Unknown? If we expect to start with it, why fallthrough?

[winder]

Basically, I chose to do it this way so that an uninitialized previous outcome (i.e. the first round after startup) can transition to the "Next" state with no special handling. So you can get to the first state from Unknown or Filter (or GetMessages in this PR, since I haven't implemented Filter yet).

[makramkd]

@winder you can include those mermaid diagrams in a README.md file, it should render natively on GitHub IIRC.

[makramkd]

First state is self explanatory but these might need some comments, especially Filter

[makramkd]

Or maybe call it FilterValidMessages?

[winder]

Added comments 👍

[makramkd]

I wonder if it would be useful to have a MarshalJSON on this so that the outcomes that are JSON encoded are still easily understandable (due to a human-readable string rather than a number). I think similar logic applies to the commit states

[winder]

Maybe. I would personally prefer if these had a more efficient encoding rather than attempting to make them human readable.

In this case the states would be 1->2->3, so still pretty human readable.

[makramkd]

I may be unlike others when it comes to this but even just 1, 2 and 3 is already a cognitive jump because I have to continuously remind myself what each number means.

Having debugged the v2 plugins quite a bit I'd rather we optimize for debuggability to start and then come back and make encoding tighter if needed.

[winder]

OK. Changed this type to a string.

[makramkd]

Also worth noting that we have states for commit as well, with their own integers, and states in many other systems which have their own integers, etc. Having all of this in mind I feel like the actual string encoding is just better and it doesn't really have that much overhead to outcome encoding size

[winder]

The phases could also be named "ReadDest", "ReadSource" and "ReadDestAgain"

[makramkd]

Less descriptive I feel, I kinda like the current names better, but not sure what other folks think. @asoliman92?

[winder]

@winder you can include those mermaid diagrams in a README.md file, it should render natively on GitHub IIRC.

@makramkd Good idea. I'll create a plugin-level README with this diagram and other information in a separate PR.

[github-actions]

Test Coverage

Branch	Coverage
will/6-exec-state-machine	76.2%
ccip-develop	77.3%

[makramkd]

🤠