Skip to content

Feat: Add inline diagnostics for Metaflow flow structure errors#16

Open
deveshidwivedi wants to merge 5 commits intoouterbounds:mainfrom
deveshidwivedi:feat/flow-diagnostics
Open

Feat: Add inline diagnostics for Metaflow flow structure errors#16
deveshidwivedi wants to merge 5 commits intoouterbounds:mainfrom
deveshidwivedi:feat/flow-diagnostics

Conversation

@deveshidwivedi
Copy link
Copy Markdown

Summary

The extension had no way to catch flow structure errors before runtime. This adds inline diagnostics so mistakes like a missing start step or a bad self.next() reference show up as squiggly underlines while we are writing the flow.

Context

Right now the extension only helps run and spin flows. If we have a typo in a self.next() reference or forget a start step, we find out when the flow crashes. This adds inline warnings so the feedback loop is tighter.

Closes #7

How the parser works

Went with a regex scan in JS rather than spawning Python for an AST. The main reason being that an AST approach breaks on incomplete files mid-edit, which is exactly when we want diagnostics most. Metaflow flows are structured enough that regex handles real-world cases fine.

Steps:

  1. Find all FlowSpec subclasses, handles class X(FlowSpec), class X(Base, FlowSpec), class X(metaflow.FlowSpec)
  2. Assign @step methods to their owner class, bounded by class body so steps from adjacent non-flow classes don't bleed in
  3. Strip inline comments before scanning: replacing with spaces not removing, so character offsets stay valid for squiggle placement
  4. Scan for all self.next() calls per step : handles multiple calls in if/else branches as pointed out in the issue discussion
  5. BFS from start to find unreachable steps

Testing

Tested via Extension Development Host. Created test_flow.py with four flow classes covering all error cases plus one clean flow and one non-flow helper class.

test_flow.py
from metaflow import FlowSpec, step

# case 1: valid flow -> should show zero diagnostics
class GoodFlow(FlowSpec):

    @step
    def start(self):
        self.next(self.end)

    @step
    def end(self):
        pass


# case 2: all four error types
class BadFlow(FlowSpec):

    @step
    def start(self):
        self.next(self.missing_step)    # ERROR: missing_step doesnt exist

    @step
    def orphan(self):                   # WARNING: not reachable from start
        self.next(self.end)

    @step
    def no_next(self):                  # WARNING: missing self.next()
        pass

    @step
    def end(self):
        pass


# case 3: multiple self.next() in if/else branches
class BranchFlow(FlowSpec):

    @step
    def start(self):
        if True:
            self.next(self.ghost_a)     # ERROR: ghost_a doesnt exist
        else:
            self.next(self.ghost_b)     # ERROR: ghost_b doesnt exist

    @step
    def end(self):
        pass


# case 4: commented-out self.next() should NOT trigger anything
class CommentFlow(FlowSpec):

    @step
    def start(self):
        # self.next(self.fake_step)
        self.next(self.end)

    @step
    def end(self):
        pass


# case 5: non-flow class -> should be completely ignored
class Helper:

    @step
    def start(self):
        pass

Problems panel output:

image

Zero diagnostics from GoodFlow (valid flow), CommentFlow (commented self.next() correctly ignored), and Helper (not a FlowSpec, ignored entirely).

Trade-offs / Design Decisions

  • Debounce at 300ms on onDidChangeTextDocument :fast enough to feel live without firing on every character
  • Both onDidChangeActiveTextEditor and onDidChangeTextDocument used as discussed in the issue thread

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flow validation diagnostics

1 participant