feat: lint pattern to extract and process logs (capture invalid sql) (#6407)

## 📝 Summary

Captures log information and formats it as a lint error if possible:

<img width="858" height="308" alt="image"
src="https://github.com/user-attachments/assets/38a9a9bf-7856-4641-8841-c19cf4d8d49c"
/>

Added a `SQLErrorMetadata(TypedDict)` we can potentially surface
elsewhere:
https://github.com/marimo-team/marimo/pull/6407/files#diff-ddd9a90856776e58792b931fec7c9a68366432991b6e9b0957f5731d0ae0aed6R22

A bit more churn here because manually generating the graph opposed to
going through `cell_manager` gives cell level granularity for log and
error collection.

Commit history is a bit messy because it has just been in the backlog,
but let me know if you'd appreciate a rebase

Author: dmadisetti

development_docs/adding_lint_rules.md

@@ -309,7 +309,11 @@ Bad: `"Multiple definition error"`
 
 ### Fixability
 
-TODO: Right now, the only fixable errors are via re-serialization.
+Safely fixable errors are applied by default via re-serialization. However,
+rules may implement unsafe fixes (mutating the notebook structure) that require
+the `--unsafe-fixes` flag. To do this, implement an `async def
+apply_unsafe_fixes(self, notebook, diagnostics) -> Notebook` method in your
+rule class, and inherit from `UnsafeFixRule` instead of `LintRule`.
 
 ## Common Patterns
 
@@ -388,6 +392,22 @@ Before submitting your rule:
 - [ ] Error messages are clear and actionable
 - [ ] Performance is reasonable for large notebooks
 
-## Example: Simple Rule Implementation
+## Examples
 
-Here's a complete example of a simple rule that checks for syntax errors: https://github.com/marimo-team/marimo/pull/6384
+### Simple Rule Implementation
+
+Example of a simple rule that checks for syntax errors: https://github.com/marimo-team/marimo/pull/6384
+
+### Rule Implementation with `--unsafe-fixes`
+
+Some rules may have "fixes" that mutate the notebook structure.
+An example of a rule that mutates the notebook structure (i.e. an unsafe fix) by removing empty cells: https://github.com/marimo-team/marimo/pull/6398
+
+### Rule Implementation with Log Context
+
+Some parsing issues result in log warnings or errors.
+An example of a rule that hooks into issued logs can be found here:
+
+**Note**: Resist intentionally adding log statements such that they trigger
+lint rules. Log statements should be added only when they provide useful
+context, on notebook startup.

docs/guides/lint_rules/index.md

@@ -43,6 +43,8 @@ These are style and formatting issues.
 | [MF002](rules/parse_stdout.md) | parse-stdout | Parse captured stdout during notebook loading | ❌ |
 | [MF003](rules/parse_stderr.md) | parse-stderr | Parse captured stderr during notebook loading | ❌ |
 | [MF004](rules/empty_cells.md) | empty-cells | Empty cells that can be safely removed. | ⚠️ |
+| [MF005](rules/sql_parse_error.md) | sql-parse-error | SQL parsing errors during dependency analysis | ❌ |
+| [MF006](rules/misc_log_capture.md) | misc-log-capture | Miscellaneous log messages during processing | ❌ |
 
 ## Legend
 

docs/guides/lint_rules/rules/misc_log_capture.md

@@ -0,0 +1,30 @@
+# MF006: misc-log-capture
+
+✨ **Formatting** ❌ Not Fixable
+
+MF006: Miscellaneous log messages during processing.
+
+## What it does
+
+Captures warning and error level log messages that aren't handled by
+other specific log rules and creates diagnostics to surface them.
+
+## Why is this bad?
+
+Unhandled log messages may indicate:
+- Unexpected issues during notebook processing
+- Configuration problems
+- Library warnings that affect execution
+- Performance or resource issues
+
+## Examples
+
+**Triggered by:**
+- General warnings from imported libraries
+- Configuration issues
+- Unexpected errors during processing
+
+## References
+
+- [Understanding Errors](https://docs.marimo.io/guides/understanding_errors/)
+

docs/guides/lint_rules/rules/sql_parse_error.md

@@ -0,0 +1,31 @@
+# MF005: sql-parse-error
+
+✨ **Formatting** ❌ Not Fixable
+
+MF005: SQL parsing errors during dependency analysis.
+
+## What it does
+
+Captures SQL parsing error logs and creates diagnostics pointing to
+problematic SQL statements in cells.
+
+## Why is this bad?
+
+SQL parsing failures can lead to:
+- Incorrect dependency analysis for SQL-using cells
+- Missing dataframe references in dependency graph
+- Reduced effectiveness of reactive execution
+- Potential runtime errors when SQL is executed
+
+## Examples
+
+**Triggered by:**
+- Invalid SQL syntax in cell code
+- Unsupported SQL dialects or extensions
+- Complex SQL that exceeds parser capabilities
+
+## References
+
+- [Understanding Errors](https://docs.marimo.io/guides/understanding_errors/)
+- [SQL Support](https://docs.marimo.io/guides/sql/)
+

examples/sql/connect_to_motherduck.py

@@ -212,7 +212,7 @@ def _(MONTHS, duckdb, mo):
         """
         SELECT DISTINCT type as 'HN Type'
         FROM sample_data.hn.hacker_news
-        WHERE score NOT NULL AND descendants NOT NULL
+        WHERE score IS NOT NULL AND descendants IS NOT NULL
         LIMIT 10;
         """
     ).df()
@@ -259,8 +259,7 @@ def _(hn_type_select, mo, month_list):
                 AND
                 MONTH(timestamp) in ({month_list})
                 AND
-                descendants NOT NULl
-
+                descendants IS NOT NULL
         )
 
         SELECT

examples/third_party/motherduck/embeddings/embeddings_explorer.py

@@ -16,13 +16,17 @@
 
 import marimo
 
-__generated_with = "0.15.5"
+__generated_with = "0.16.0"
 app = marimo.App(width="medium")
 
 with app.setup:
     # Use this notebook to follow along with the tutorial at
     # https://motherduck.com/blog/MotherDuck-Visualize-Embeddings-Marimo/
     import marimo as mo
 
+
+
+
+
 if __name__ == "__main__":
     app.run()

marimo/_ast/sql_utils.py

@@ -1,6 +1,8 @@
 # Copyright 2025 Marimo. All rights reserved.
 
-from typing import Literal, Optional, Union
+import ast
+import re
+from typing import Callable, Literal, Optional, TypedDict, Union
 
 from marimo import _loggers
 from marimo._dependencies.dependencies import DependencyManager
@@ -17,6 +19,20 @@
 ]
 
 
+class SQLErrorMetadata(TypedDict):
+    """Structured metadata for SQL parsing errors."""
+
+    lint_rule: str
+    error_type: str
+    clean_message: str  # Just the meaningful error without SQL trace
+    node_lineno: int
+    node_col_offset: int
+    sql_statement: str  # Truncated if needed
+    sql_line: Optional[int]  # 0-based line within SQL
+    sql_col: Optional[int]  # 0-based column within SQL
+    context: str
+
+
 def classify_sql_statement(
     sql_statement: str, dialect: Optional[SQLGLOT_DIALECTS] = None
 ) -> Union[SQL_TYPE, Literal["unknown"]]:
@@ -33,7 +49,14 @@ def classify_sql_statement(
         with _loggers.suppress_warnings_logs("sqlglot"):
             expression_list = parse(sql_statement, dialect=dialect)
     except ParseError as e:
-        LOGGER.debug(f"Unable to parse SQL. Error: {e}")
+        log_sql_error(
+            LOGGER.debug,
+            message="Failed to parse SQL statement for classification.",
+            exception=e,
+            rule_code="MF005",
+            node=None,
+            sql_content=sql_statement,
+        )
         return "unknown"
 
     for expression in expression_list:
@@ -52,3 +75,45 @@ def classify_sql_statement(
             return "DQL"
 
     return "unknown"
+
+
+def log_sql_error(
+    logger: Callable[..., None],
+    *,
+    message: str,
+    exception: BaseException,
+    rule_code: str,
+    node: Optional[ast.expr] = None,
+    sql_content: str = "",
+    context: str = "",
+) -> None:
+    """Utility to log SQL-related errors with consistent metadata."""
+    # Parse SQL position from exception message if available
+    sql_line = None
+    sql_col = None
+
+    exception_msg = str(exception)
+    line_col_match = re.search(r"Line (\d+), Col: (\d+)", exception_msg)
+    if line_col_match:
+        sql_line = int(line_col_match.group(1)) - 1  # Convert to 0-based
+        sql_col = int(line_col_match.group(2)) - 1  # Convert to 0-based
+
+    # Truncate long SQL content
+    truncated_sql = sql_content
+    if sql_content and len(sql_content) > 200:
+        truncated_sql = sql_content[:200] + "..."
+
+    # Create metadata using TypedDict
+    metadata: SQLErrorMetadata = {
+        "lint_rule": rule_code,
+        "error_type": type(exception).__name__,
+        "clean_message": exception_msg.split("\n", 1)[0],
+        "node_lineno": node.lineno if node else 0,
+        "node_col_offset": node.col_offset if node else 0,
+        "sql_statement": truncated_sql,
+        "sql_line": sql_line,
+        "sql_col": sql_col,
+        "context": context,
+    }
+
+    logger(message, exception, extra=metadata)

marimo/_ast/sql_visitor.py

@@ -250,6 +250,7 @@ def find_sql_defs(sql_statement: str) -> SQLDefs:
                         LOGGER.warning(
                             "Unexpected number of parts in CREATE TABLE: %s",
                             parts,
+                            extra={"parts": parts},
                         )
 
                     if is_table:
@@ -497,7 +498,6 @@ def find_sql_refs(sql_statement: str) -> set[SQLRef]:
     DependencyManager.sqlglot.require(why="SQL parsing")
 
     from sqlglot import exp, parse
-    from sqlglot.errors import ParseError
     from sqlglot.optimizer.scope import build_scope
 
     def get_ref_from_table(table: exp.Table) -> Optional[SQLRef]:
@@ -507,7 +507,6 @@ def get_ref_from_table(table: exp.Table) -> Optional[SQLRef]:
         catalog_name = table.catalog or None
 
         if table_name is None:
-            LOGGER.warning("Table name cannot be found in the SQL statement")
             return None
 
         # Check if the table name looks like a URL or has a file extension.
@@ -520,12 +519,9 @@ def get_ref_from_table(table: exp.Table) -> Optional[SQLRef]:
             table=table_name, schema=schema_name, catalog=catalog_name
         )
 
-    try:
-        with _loggers.suppress_warnings_logs("sqlglot"):
-            expression_list = parse(sql_statement, dialect="duckdb")
-    except ParseError as e:
-        LOGGER.error(f"Unable to parse SQL. Error: {e}")
-        return set()
+    # May raise a ParseError
+    with _loggers.suppress_warnings_logs("sqlglot"):
+        expression_list = parse(sql_statement, dialect="duckdb")
 
     refs: set[SQLRef] = set()
 

marimo/_ast/visitor.py

@@ -12,6 +12,7 @@
 
 from marimo import _loggers
 from marimo._ast.errors import ImportStarError
+from marimo._ast.sql_utils import log_sql_error
 from marimo._ast.sql_visitor import (
     SQLDefs,
     SQLKind,
@@ -26,8 +27,10 @@
 
 LOGGER = _loggers.marimo_logger()
 
+
 Name = str
 
+
 Language = Literal["python", "sql"]
 
 
@@ -632,6 +635,18 @@ def visit_Call(self, node: ast.Call) -> ast.Call:
                 and sql
             ):
                 import duckdb  # type: ignore[import-not-found,import-untyped,unused-ignore] # noqa: E501
+
+                # Import ParseError outside try block so we can except it
+                # Use a Union approach to handle both cases
+                ParseErrorType: type[Exception]
+                if DependencyManager.sqlglot.has():
+                    from sqlglot.errors import ParseError as SQLGLOTParseError
+
+                    ParseErrorType = SQLGLOTParseError
+                else:
+                    # Fallback when sqlglot is not available
+                    ParseErrorType = Exception
+
                 # TODO: Handle other SQL languages
                 # TODO: Get the engine so we can differentiate tables in diff engines
 
@@ -655,7 +670,14 @@ def visit_Call(self, node: ast.Call) -> ast.Call:
                     # We catch base exceptions because we don't want to
                     # fail due to bugs in duckdb -- users code should
                     # be saveable no matter what
-                    LOGGER.warning("Unexpected duckdb error %s", e)
+                    log_sql_error(
+                        LOGGER.warning,
+                        message=f"Unexpected duckdb error {e}",
+                        exception=e,
+                        node=node,
+                        rule_code="MF005",
+                        sql_content=sql,
+                    )
                     self.generic_visit(node)
                     return node
 
@@ -667,7 +689,15 @@ def visit_Call(self, node: ast.Call) -> ast.Call:
                     except duckdb.ProgrammingError:
                         sql_defs = SQLDefs()
                     except BaseException as e:
-                        LOGGER.warning("Unexpected duckdb error %s", e)
+                        log_sql_error(
+                            LOGGER.warning,
+                            message=f"Unexpected duckdb error {e}",
+                            exception=e,
+                            node=node,
+                            rule_code="MF005",
+                            sql_content=sql,
+                            context="sql_defs_extraction",
+                        )
                         sql_defs = SQLDefs()
 
                     defined_names = set()
@@ -699,13 +729,23 @@ def visit_Call(self, node: ast.Call) -> ast.Call:
 
                     sql_refs: set[SQLRef] = set()
                     try:
+                        # Take results
                         sql_refs = find_sql_refs_cached(statement.query)
-                    except (duckdb.ProgrammingError, duckdb.IOException):
-                        LOGGER.debug(
-                            "Error parsing SQL statement: %s", statement.query
+                    except (
+                        duckdb.ProgrammingError,
+                        duckdb.IOException,
+                        ParseErrorType,
+                        BaseException,
+                    ) as e:
+                        # Use first_arg (SQL string node) for accurate positioning
+                        log_sql_error(
+                            LOGGER.error,
+                            message=f"Error parsing SQL statement: {e}",
+                            exception=e,
+                            node=first_arg,
+                            rule_code="MF005",
+                            sql_content=statement.query,
                         )
-                    except BaseException as e:
-                        LOGGER.warning("Unexpected duckdb error %s", e)
 
                     for ref in sql_refs:
                         name = ref.qualified_name