From 5578b951deb7bdb2102b71f64c5362db2635b7c1 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Wed, 24 Jun 2026 23:26:37 -0700 Subject: [PATCH] [eric] linter: add ruff + pyright checks from Haik (narrowed F-codes, mixin-safe pyright; grandfather pre-existing) --- backend/requirements-dev.txt | 5 ++ linter/checks/__init__.py | 13 +++++ linter/checks/pyright.py | 95 ++++++++++++++++++++++++++++++++ linter/checks/ruff.py | 92 +++++++++++++++++++++++++++++++ linter/config/config.json | 61 +++++++++++++++++++- linter/config/pyright_check.json | 20 +++++++ linter/lint.py | 29 ++++++++-- 7 files changed, 309 insertions(+), 6 deletions(-) create mode 100644 linter/checks/pyright.py create mode 100644 linter/checks/ruff.py create mode 100644 linter/config/pyright_check.json diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt index 383d07e4..fe153506 100644 --- a/backend/requirements-dev.txt +++ b/backend/requirements-dev.txt @@ -13,3 +13,8 @@ pytest-asyncio==0.25.2 # Used by linter/lint.py (the vulture dead-code check). watchfiles, also # needed by lint.py, already comes in transitively via uvicorn[standard]. vulture==2.16 + +# Used by linter/lint.py: ruff (scoped dead-code lint) + pyright (existence +# checks). Both must be on PATH; pyright's config expects the venv at backend/.venv. +ruff==0.15.19 +pyright==1.1.411 diff --git a/linter/checks/__init__.py b/linter/checks/__init__.py index 8baad9ce..de36d13e 100644 --- a/linter/checks/__init__.py +++ b/linter/checks/__init__.py @@ -9,6 +9,19 @@ from pathlib import Path LINTIGNORE_PREFIX = ".lintignore" +class CheckError(Exception): + """Raised when a check could not complete (missing tool, timeout, crash). + + Distinct from "ran and found zero problems": the orchestrator surfaces this + loudly so a partial run is never silently reported as a clean one (returning + [] on failure made a timed-out/crashed tool look identical to a passing one). + """ + + def __init__(self, reason: str) -> None: + super().__init__(reason) + self.reason = reason + + def _matches_any(text: str, patterns: list[str]) -> bool: return any(fnmatch.fnmatch(text, p) for p in patterns) diff --git a/linter/checks/pyright.py b/linter/checks/pyright.py new file mode 100644 index 00000000..daf19d05 --- /dev/null +++ b/linter/checks/pyright.py @@ -0,0 +1,95 @@ +"""Pyright runner: catches references to methods/attributes/names that don't exist. + +The other checks can't do this. ruff is per-file and never builds a cross-symbol +type graph; vulture finds dead (unused) definitions, not invalid references; the +naming checks only inspect where names are *defined*, not where they're *read*. +So a missed rename like ``self._per_session`` (when the attribute is actually +``p_per_session``) sails through everything and only blows up at runtime. + +Pyright resolves types/inheritance/imports, so its ``reportAttributeAccessIssue`` +flags exactly that. We run it in the lowest-noise mode possible +(config/pyright_check.json sets typeCheckingMode "off" and re-enables only the +existence-checking rules as errors), so this section stays high-signal without a +full strict-mode cleanup. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +from . import CheckError, is_excepted, is_lintignored + +# Cold first run downloads the pinned node binary (pip wrapper) and warms the +# import graph; keep this generous so a slow first pass doesn't time out and +# silently report zero. +_TIMEOUT = 240 + +_CONFIG_REL = Path("linter") / "config" / "pyright_check.json" + + +def run_pyright( + root: Path, + exceptions: dict[str, list[str]], + ignores: dict[Path, set[str]] | None = None, +) -> list[str]: + """Run pyright on the Python backend and return existence errors.""" + pyright_bin = root / "backend" / ".venv" / "bin" / "pyright" + if not pyright_bin.exists(): + found = shutil.which("pyright") + if not found: + raise CheckError("pyright executable not found in backend/.venv/bin or PATH") + pyright_bin = Path(found) + + config = root / _CONFIG_REL + if not config.exists(): + raise CheckError(f"pyright config not found at {_CONFIG_REL}") + + cmd = [str(pyright_bin), "--project", str(config), "--outputjson"] + + try: + result = subprocess.run( + cmd, capture_output=True, text=True, cwd=str(root), timeout=_TIMEOUT, + ) + except subprocess.TimeoutExpired as e: + raise CheckError(f"timed out after {_TIMEOUT}s (machine under load or cold cache)") from e + except OSError as e: + raise CheckError(f"failed to launch pyright ({e})") from e + + # pyright exits 0 (no errors) or 1 (errors found) on a successful run. Any + # other code with no parseable JSON means pyright itself failed (e.g. node + # missing, bad config) — surface it rather than treating empty as clean. + out = result.stdout.strip() + if not out: + detail = (result.stderr or "no output").strip()[:300] + raise CheckError(f"pyright produced no JSON (exit {result.returncode}): {detail}") + try: + data = json.loads(out) + except json.JSONDecodeError as e: + raise CheckError(f"pyright JSON parse failed (exit {result.returncode}): {e}") from e + + errors: list[str] = [] + for diag in data.get("generalDiagnostics", []): + if diag.get("severity") != "error": + continue + file_abs = diag.get("file", "") + try: + relpath = str(Path(file_abs).resolve().relative_to(root)) + except ValueError: + # Diagnostic outside the repo root (stub/site-packages); ignore. + continue + if is_excepted(relpath, "pyright", exceptions): + continue + if ignores and is_lintignored(root / relpath, root, "pyright", ignores): + continue + # pyright ranges are 0-based; the linter/IDE want 1-based line+col. + start = diag.get("range", {}).get("start", {}) + line = int(start.get("line", 0)) + 1 + col = int(start.get("character", 0)) + 1 + rule = diag.get("rule", "") + msg = " ".join((diag.get("message", "") or "").splitlines()).strip() + rule_part = f"{rule} " if rule else "" + errors.append(f"{relpath}:{line}:{col}: error: [pyright] {rule_part}{msg}") + return errors diff --git a/linter/checks/ruff.py b/linter/checks/ruff.py new file mode 100644 index 00000000..896f2b93 --- /dev/null +++ b/linter/checks/ruff.py @@ -0,0 +1,92 @@ +"""Ruff runner: per-file, scope-aware lint that vulture's global name-set can't do. + +Owns the checks ruff is strictly better at than vulture: + - F401 unused imports (real per-file scope; honors __all__ / redundant-alias re-exports) + - F811 redefinition of an unused name + - F841 unused local variable assignment + - ARG001/ARG002 unused function/method arguments + +Vulture is narrowed (see checks/vulture.py) to only emit dead functions, methods, +classes, and attributes, the whole-program reachability that ruff structurally +does not attempt. The two are complementary, not redundant. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +from pathlib import Path + +from . import CheckError, is_excepted, is_lintignored + +# Generous because the first run after a window reload races the editor's +# startup load with a cold ruff cache; a tight limit there is exactly what made +# the check time out and silently report zero. +_TIMEOUT = 120 + +# Ruff emits ANSI color into its concise output even on a non-tty / NO_COLOR; +# strip the escapes before parsing so the regex sees plain text. +_ANSI_RE = re.compile(r"\x1b\[[0-9;]*m") + +# `path:line:col: CODE message`. The optional `[*]` fixable marker is dropped. +_LINE_RE = re.compile(r"^(?P.+?):(?P\d+):(?P\d+): (?P[A-Z]+\d+) (?P.+)$") + + +def run_ruff( + root: Path, + select: str, + exceptions: dict[str, list[str]], + ignores: dict[Path, set[str]] | None = None, +) -> list[str]: + """Run ruff on the Python backend and return errors.""" + ruff_bin = root / "backend" / ".venv" / "bin" / "ruff" + if not ruff_bin.exists(): + found = shutil.which("ruff") + if not found: + raise CheckError("ruff executable not found in backend/.venv/bin or PATH") + ruff_bin = Path(found) + + targets = ["backend"] + if (root / "debug.py").exists(): + targets.append("debug.py") + cmd = [ + str(ruff_bin), "check", *targets, + "--isolated", # ignore any stray pyproject/ruff.toml so the linter is hermetic + "--select", select, + "--output-format", "concise", + "--no-fix", + "--exclude", ".venv,__pycache__,data,uv-bin,webapp_template,.runner-venv", + ] + + try: + result = subprocess.run( + cmd, capture_output=True, text=True, cwd=str(root), timeout=_TIMEOUT, + ) + except subprocess.TimeoutExpired as e: + raise CheckError(f"timed out after {_TIMEOUT}s (machine under load or cold cache)") from e + except OSError as e: + raise CheckError(f"failed to launch ruff ({e})") from e + + # ruff exits 0 (no findings) or 1 (findings) on success; anything else with + # no parseable output means ruff itself errored (e.g. could not write its + # cache) — surface it instead of treating the empty stdout as "clean". + if result.returncode not in (0, 1) and not result.stdout.strip(): + detail = _ANSI_RE.sub("", result.stderr).strip()[:300] or "no output" + raise CheckError(f"ruff exited with code {result.returncode}: {detail}") + + errors: list[str] = [] + for line in result.stdout.strip().splitlines(): + m = _LINE_RE.match(_ANSI_RE.sub("", line).strip()) + if not m: + continue + filepath = m.group("path") + if is_excepted(filepath, "ruff", exceptions): + continue + if ignores and is_lintignored(root / filepath, root, "ruff", ignores): + continue + errors.append( + f"{filepath}:{m.group('line')}:{m.group('col')}: " + f"error: [ruff] {m.group('code')} {m.group('msg')}" + ) + return errors diff --git a/linter/config/config.json b/linter/config/config.json index da356ce7..b66463a4 100644 --- a/linter/config/config.json +++ b/linter/config/config.json @@ -10,7 +10,9 @@ "endpoints": false, "classes": false, "no-underscore-names": true, - "p-private": true + "p-private": true, + "ruff": true, + "pyright": true }, "_notes": { "no-nested-imports": "Off on purpose: this codebase uses function-level/lazy imports to break import cycles (400+ sites). Flagging them all is wrong for us.", @@ -19,6 +21,7 @@ "max-file-lines-exceptions": "Grandfather list of pre-existing >300-line files (existing debt, not new). Paths updated after the folder-tree restructure moved several of them. The two manager/prompt/* entries are from the agent_manager decomposition: prompt_context.py aggregates the system-prompt context builders and attachments.py is one cohesive 230-line attachment resolver; both are single-responsibility and a few lines over, not splittable without an artificial seam.", "max-folder-items-exceptions": "Exact-path allow for folders intentionally over the cap. The rule trips at >7 (7 items is fine, the 8th tips it), so only genuinely 8+ folders are listed. backend/ and backend/apps are FastAPI feature-package registries (each child is an app mounted in main.py); agents/ aggregates agent subsystems; agents/manager/ is the agent_manager god-object decomposition (cohesive AgentManager mixins + standalone run helpers + the streaming/permissions/prompt/session subtrees), conventionally flat like agents/ and core/ since its standalone helpers are heterogeneous and don't group cleanly; agents/manager/streaming and agents/manager/session are flat peer collections of one-module-per-concern handlers; core/, tools_lib/, tests/ are conventionally flat. Frontend: app/pages is the page registry, AgentChat/ChatInput/Settings-sections/Onboarding are organizational parents, and shared/state (Redux slices) plus hooks/steps/mcp-cards/Views are flat peer collections. scripts/, electron/, linter/checks/ are flat tool dirs. These replaced blanket .lintignore-max-folder-items sentinels (backend, frontend, scripts, electron, linter/checks) so the rule still catches NEW unplanned bloat everywhere else. Kept as whole-subtree sentinels on purpose: debugger/ (self-contained injected sub-tool with its own Vite GUI), webapp_template (Vite scaffold payload), and vendored mcp-bundles.", "import-cycles": "Flags RUNTIME circular imports only (SCC>1). Skips type-only imports (import type / export type) and dynamic import() since neither runs at module init, which is why the idiomatic Redux store<->hooks type cycle is not flagged. Frontend alias resolution comes from import-cycle-aliases. Zero cycles today; the check keeps it that way.", + "ruff + pyright": "Ported from Haik's linter (haik/feat/ingest). ruff is narrowed to F401/F811/F841 (unused imports/redefs/locals) and intentionally DROPS Haik's ARG001/ARG002 (unused args): our SDK-callback signatures require unused params (can_use_tool/pre_tool_hook take a `context` they don't use) and we ban the `_unused` prefix, so ARG is noise here. pyright runs Haik's existence-only config (typeCheckingMode off) but DISABLES reportAttributeAccessIssue: our AgentManager is decomposed into mixins that read attributes defined on the composed class (self.sessions etc.), which that rule can't see without a typed mixin base — 82 false positives. Kept reportUndefinedVariable + reportMissingImports, which caught a real dangling `_conns` ref in configure_provider_env. Re-enabling attribute-access cleanly needs a typed mixin contract (future). Both grandfather pre-existing debt by file; the refactor surface is clean. Requires `ruff` + `pyright` on PATH (added to requirements-dev.txt); pyright's config expects the venv at backend/.venv.", "no-underscore-names + p-private": "Convention checks ported verbatim from Haik's linter (haik/feat/ingest): no-underscore-names bans leading-underscore names (a dead-code-tooling blind spot; use p_ for private), p-private enforces that p_-prefixed names are accessed only inside their owning file/class (cross-file/class use means the name should be public). Backend Python only. The exception lists grandfather pre-existing debt that landed with the workflows/analytics forward-ports (eric's 'don't mass-migrate untouched files' rule); the agent_manager refactor surface is clean. NOTE: Haik's full linter (his branch also adds pyright + ruff and runs a different enabled set) should eventually supersede this; these two were lifted to enforce the p_ conventions on eric/dev now." }, "rules": { @@ -28,7 +31,8 @@ "vulture-min-confidence": 1, "vulture-error-threshold": 1, "no-nested-imports": true, - "endpoint-ignore-routes": ["*/callback", "*/callback/*"] + "endpoint-ignore-routes": ["*/callback", "*/callback/*"], + "ruff-select": "F401,F811,F841" }, "include_extensions": [".py", ".ts", ".tsx", ".js", ".jsx"], "exclude": [ @@ -196,6 +200,59 @@ "backend/tests/test_ssrf_guard.py", "backend/tests/test_workflows_semantics.py", "backend/tests/test_workflows_storage.py" + ], + "ruff": [ + "backend/apps/agents/agents.py", + "backend/apps/agents/browser/browser_agent.py", + "backend/apps/agents/providers/registry.py", + "backend/apps/agents/proxy/anthropic_proxy.py", + "backend/apps/agents/proxy/anthropic_to_openai.py", + "backend/apps/agents/tools/web.py", + "backend/apps/dashboards/dashboards.py", + "backend/apps/health/health.py", + "backend/apps/modes/modes.py", + "backend/apps/nine_router/process.py", + "backend/apps/outputs/outputs.py", + "backend/apps/service/buffer.py", + "backend/apps/service/service.py", + "backend/apps/settings/settings.py", + "backend/apps/subscription/router.py", + "backend/apps/swarm/redact.py", + "backend/apps/tools_lib/tools_lib.py", + "backend/apps/web/web.py", + "backend/apps/workflows/models.py", + "backend/apps/workflows/workflows.py", + "backend/auth.py", + "backend/main.py", + "backend/tests/conftest.py", + "backend/tests/test_browser_agent_loop.py", + "backend/tests/test_browser_meta_playbook.py", + "backend/tests/test_browser_orchestrator_routing.py", + "backend/tests/test_bundled_extracted_modules.py", + "backend/tests/test_disconnect_resilience.py", + "backend/tests/test_disk_caches.py", + "backend/tests/test_effective_tools.py", + "backend/tests/test_outputs_runtime_cleanup.py", + "backend/tests/test_phase1_stress.py", + "backend/tests/test_schedule_e2e.py", + "backend/tests/test_schedule_recurrence.py", + "backend/tests/test_service_legacy.py", + "backend/tests/test_settings_meta_guard.py", + "backend/tests/test_skills_folders.py", + "backend/tests/test_swarm_bundle.py", + "backend/tests/test_system_prompt.py", + "backend/tests/test_v2_invariants.py", + "backend/tests/test_v2_label_logic.py", + "backend/tests/test_web_mcp_decision.py", + "backend/tests/test_workflows_semantics.py", + "backend/tests/test_ws_integration.py" + ], + "pyright": [ + "backend/apps/google_workspace_mcp_shim/run.py", + "backend/apps/health/health.py", + "backend/apps/settings/settings.py", + "backend/apps/web/web.py", + "backend/config/Apps.py" ] } } diff --git a/linter/config/pyright_check.json b/linter/config/pyright_check.json new file mode 100644 index 00000000..df29b177 --- /dev/null +++ b/linter/config/pyright_check.json @@ -0,0 +1,20 @@ +{ + "include": ["../../backend"], + "exclude": [ + "../../backend/.venv", + "../../backend/uv-bin", + "../../backend/data", + "../../backend/tests", + "../../backend/apps/outputs/webapp_template", + "**/__pycache__" + ], + "venvPath": "../../backend", + "venv": ".venv", + "pythonVersion": "3.11", + "extraPaths": ["../.."], + "reportMissingTypeStubs": false, + "typeCheckingMode": "off", + "reportAttributeAccessIssue": "none", + "reportUndefinedVariable": "error", + "reportMissingImports": "error" +} diff --git a/linter/lint.py b/linter/lint.py index 4ff84f41..ccd605f2 100644 --- a/linter/lint.py +++ b/linter/lint.py @@ -10,7 +10,7 @@ import sys from pathlib import Path from typing import Any -from checks import is_excluded, is_excepted, is_lintignored, collect_lintignores +from checks import CheckError, is_excluded, is_excepted, is_lintignored, collect_lintignores from checks.structural import check_file_lines, check_folder_items, check_nested_imports from checks.vulture import run_vulture from checks.eslint import run_eslint @@ -20,6 +20,8 @@ from checks.classes import run_class_check from checks.cycles import run_cycle_check from checks.no_underscore_names import run_underscore_check from checks.p_private import run_p_private_check +from checks.ruff import run_ruff +from checks.pyright import run_pyright from watchfiles import watch, DefaultFilter SCRIPT_DIR = Path(__file__).resolve().parent @@ -31,7 +33,7 @@ def load_config() -> dict[str, Any]: return json.load(f) -def run_checks(root: Path) -> tuple[list[str], list[str], list[str], list[str], list[str], list[str], list[str], list[str], list[str]]: +def run_checks(root: Path) -> tuple[list[str], list[str], list[str], list[str], list[str], list[str], list[str], list[str], list[str], list[str], list[str]]: config = load_config() enabled: dict[str, bool] = config.get("enabled", {}) rules: dict[str, int] = config["rules"] @@ -112,7 +114,23 @@ def run_checks(root: Path) -> tuple[list[str], list[str], list[str], list[str], underscore_errors = run_underscore_check(root, exceptions, excludes, ignores) if enabled.get("no-underscore-names", False) else [] p_private_errors = run_p_private_check(root, exceptions, excludes, ignores) if enabled.get("p-private", False) else [] - return sorted(structural_errors), sorted(vulture_errors), sorted(eslint_errors), sorted(knip_errors), sorted(endpoint_errors), sorted(class_errors), sorted(cycle_errors), sorted(underscore_errors), sorted(p_private_errors) + # ruff (scoped dead-code codes) + pyright (existence errors), also from Haik's + # linter. Both shell out to a tool, so a missing tool / timeout raises CheckError + # and is surfaced as a loud error rather than a silently-clean empty result. + ruff_errors: list[str] = [] + if enabled.get("ruff", False): + try: + ruff_errors = run_ruff(root, rules.get("ruff-select", "F401,F811,F841,ARG001,ARG002"), exceptions, ignores) + except CheckError as e: + ruff_errors = [f"ruff: check could not run: {e.reason}"] + pyright_errors: list[str] = [] + if enabled.get("pyright", False): + try: + pyright_errors = run_pyright(root, exceptions, ignores) + except CheckError as e: + pyright_errors = [f"pyright: check could not run: {e.reason}"] + + return sorted(structural_errors), sorted(vulture_errors), sorted(eslint_errors), sorted(knip_errors), sorted(endpoint_errors), sorted(class_errors), sorted(cycle_errors), sorted(underscore_errors), sorted(p_private_errors), sorted(ruff_errors), sorted(pyright_errors) def _print_section(name: str, errors: list[str]) -> None: @@ -127,7 +145,8 @@ def print_results( eslint_errors: list[str], knip_errors: list[str], endpoint_errors: list[str], class_errors: list[str], cycle_errors: list[str], underscore_errors: list[str], - p_private_errors: list[str], + p_private_errors: list[str], ruff_errors: list[str], + pyright_errors: list[str], ) -> None: _print_section("structural", structural_errors) _print_section("vulture", vulture_errors) @@ -138,6 +157,8 @@ def print_results( _print_section("import-cycles", cycle_errors) _print_section("no-underscore-names", underscore_errors) _print_section("p-private", p_private_errors) + _print_section("ruff", ruff_errors) + _print_section("pyright", pyright_errors) def watch_loop(root: Path) -> None: