mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-08-17 18:25:42 +02:00
[eric] linter: add ruff + pyright checks from Haik (narrowed F-codes, mixin-safe pyright; grandfather pre-existing)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
@@ -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<path>.+?):(?P<line>\d+):(?P<col>\d+): (?P<code>[A-Z]+\d+) (?P<msg>.+)$")
|
||||
|
||||
|
||||
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
|
||||
@@ -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"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
+25
-4
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user