[codex] Fix reactive go-to-definition lookup

[nkgotcode]

This pull request was authored by a coding agent.

Summary

• prevent current-cell lookups from falling back to the first matching local variable when resolving reactive variables
• keep local scope resolution for same-cell definitions and then fall through to notebook reactive variable lookup
• add coverage for jumping from a reactive variable use in one cell to its defining cell

Root cause

goToDefinition checked same-cell definitions first, but goToVariableDefinition always fell back to the first same-cell name match. For a reactive variable used in another cell, that fallback treated the usage token as a local match and stopped before the notebook variable lookup could jump to the defining cell.

Validation

• pnpm --filter @marimo-team/frontend test src/core/codemirror/go-to-definition/__tests__/utils.test.ts --run
• Result: 1 test file passed, 3 tests passed.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
marimo-docs	Ready	Preview, Comment	Jun 3, 2026 12:27am

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[nkgotcode]

I have read the CLA Document and I hereby sign the CLA

[nkgotcode]

recheck

[akshayka]

@kirangadhave this might be right, can you confirm?

[Copilot]

Pull request overview

This PR fixes go-to-definition behavior in the frontend CodeMirror integration so that reactive variable references in one cell can correctly jump to the variable’s defining cell, instead of incorrectly “resolving” to a same-cell name match.

Changes:

• Adds a fallbackToFirstMatch toggle to goToVariableDefinition to prevent same-cell lookups from incorrectly short-circuiting reactive resolution.
• Updates goToDefinition to disable first-match fallback when a usage position is available, allowing cross-cell reactive lookup to proceed.
• Adds a test covering go-to-definition from a reactive variable usage in one cell to its defining cell.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
frontend/src/core/codemirror/go-to-definition/utils.ts	Adjusts local vs reactive resolution flow to avoid false-positive local matches.
frontend/src/core/codemirror/go-to-definition/commands.ts	Extends variable-definition navigation API with an opt-out for first-match fallback.
frontend/src/core/codemirror/go-to-definition/tests/utils.test.ts	Adds regression coverage for cross-cell reactive go-to-definition.

Comments suppressed due to low confidence (1)

frontend/src/core/codemirror/go-to-definition/utils.ts:90

• When goToDefinition is invoked on a private variable and no same-cell definition is found, the current logic proceeds to getEditorForVariable() which returns the current editor for private vars, then calls goToVariableDefinition(editorWithVariable, variableName) without a usagePosition. Because goToVariableDefinition can fall back to the first VariableName match, this can end up "navigating" to the usage token itself even though no definition exists. Since private variables are intended to be same-cell only, it’s better to return false once the scoped lookup fails.

  if (usagePosition !== undefined) {
    const foundLocally = goToVariableDefinition(
      view,
      variableName,
      usagePosition,
      false,
    );
    if (foundLocally) {
      return true;
    }
  }

[kirangadhave]

@cubic-dev-ai

[cubic-dev-ai]

@cubic-dev-ai

@kirangadhave I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

No issues found across 3 files

Architecture diagram

sequenceDiagram
    participant User as "User (Cursor)"
    participant View as "EditorView (CodeMirror)"
    participant Store as "Jotai Store"
    participant Defs as "goToDefinition()"
    participant VarDef as "goToVariableDefinition()"
    participant Scoped as "findScopedDefinitionPosition()"
    participant First as "findFirstMatchingVariable()"
    participant Cells as "cellHandles (Notebook)"

    Note over User,Cells: Reactive Variable Go-To-Definition Flow

    User->>View: Cursor at variable "a" in usage cell
    View->>Defs: goToDefinition(usageView, variableName, usagePosition)
    
    alt Same-cell variable (existing)
        Note over Defs: fallbackToFirstMatch = false
        Defs->>VarDef: goToVariableDefinition(view, variableName, usagePosition, false)
        VarDef->>Scoped: findScopedDefinitionPosition(state, variableName, usagePosition)
        alt Scoped definition found
            Scoped-->>VarDef: Position {from, to} (same cell)
            VarDef->>View: Set cursor to definition
            VarDef-->>Defs: true (foundLocally)
            Defs-->>User: Jumped to same-cell definition
        else No scoped definition
            Scoped-->>VarDef: null
            VarDef->>First: fallbackToFirstMatch = false (skip call)
            Note over VarDef: NEW: fallbackToFirstMatch flag prevents fallback
            VarDef-->>Defs: false (not found locally)
            Defs->>Cells: Lookup reactive variable in notebook.variablesAtom
            Cells-->>Defs: Variable data (declaredBy / usedBy)
            Defs->>Cells: Navigate to defining cell for "a"
            Cells-->>User: Jumped to defining cell
        end
    else Same-cell variable (old behavior before PR)
        Note over Defs: Old: always fallbackToFirstMatch = true
        Defs->>VarDef: goToVariableDefinition(view, variableName, usagePosition, true)
        VarDef->>Scoped: findScopedDefinitionPosition(state, variableName, usagePosition)
        alt Scoped definition found
            Scoped-->>VarDef: Position (same cell)
            VarDef-->>Defs: true
        else No scoped definition
            Scoped-->>VarDef: null
            VarDef->>First: findFirstMatchingVariable(state, variableName)
            Note over First: NEW: This fallback is now skipped for cross-cell lookups
            First-->>VarDef: First same-cell match (incorrectly treats reactive var as local)
            VarDef-->>Defs: true (incorrect local match)
            Defs-->>User: Jumps to wrong first variable in same cell
        end
    end

    Note over User,Cells: Key architectural boundary: local scope vs notebook reactive variables
    Note over Defs,VarDef: CHANGED: fallbackToFirstMatch parameter controls boundary

Loading

_{Re-trigger cubic}

[kirangadhave]

two small followups

[kirangadhave]

jsdoc doesn't reflect the new param, default value and what it does

[kirangadhave]

ternaries with null coalescing operator is difficult to parse visually. let's simplify:

let from: number | null = null;
if (usagePosition !== undefined) {
  from = findScopedDefinitionPosition(state, variableName, usagePosition);
}
if (from === null && fallbackToFirstMatch) {
  from = findFirstMatchingVariable(state, variableName);
}

[nkgotcode]

Addressed the remaining JSDoc feedback in 39b195b74: the comment now describes scoped-definition lookup, the fallbackToFirstMatch behavior, and its default.