Skip to content

Commit 1c55b09

Browse files
Add lint CI and apply style cleanups
Add a GitHub Actions workflow to run ruff and mypy on changed Python files and add ruff/mypy to the dev dependency group with minimal configs in pyproject.toml. Apply widespread non-functional style and formatting changes across the codebase (whitespace, line-wrapping, minor import reorderings, single-line logging calls, and test formatting tweaks) to satisfy linting/formatting. No behavioral logic changes intended.
1 parent 184a669 commit 1c55b09

19 files changed

Lines changed: 457 additions & 212 deletions

.github/workflows/lint-backend.yml

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
name: Lint Backend
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- 'src/**.py'
7+
- 'tests/**.py'
8+
- 'pyproject.toml'
9+
- 'uv.lock'
10+
- '.github/workflows/lint-backend.yml'
11+
12+
# Cancel in-flight runs on the same PR when a new push lands.
13+
concurrency:
14+
group: lint-backend-${{ github.ref }}
15+
cancel-in-progress: true
16+
17+
jobs:
18+
lint:
19+
name: Ruff and mypy on changed files
20+
runs-on: ubuntu-latest
21+
22+
steps:
23+
- name: Checkout
24+
uses: actions/checkout@v4
25+
with:
26+
# Need the full base ref locally so we can compute the diff
27+
# against the PR's merge base.
28+
fetch-depth: 0
29+
30+
- name: Install uv
31+
uses: astral-sh/setup-uv@v3
32+
with:
33+
enable-cache: true
34+
35+
- name: Set up Python
36+
run: uv python install 3.13
37+
38+
- name: Install dev dependencies
39+
run: uv sync --group dev
40+
41+
- name: Compute changed Python files
42+
id: changed
43+
run: |
44+
base="${{ github.event.pull_request.base.sha }}"
45+
head="${{ github.event.pull_request.head.sha }}"
46+
# Diff against the merge base so we only see files touched in the PR.
47+
merge_base=$(git merge-base "$base" "$head")
48+
mapfile -t files < <(git diff --name-only --diff-filter=ACMR "$merge_base" "$head" -- '*.py')
49+
if [ "${#files[@]}" -eq 0 ]; then
50+
echo "No Python files changed."
51+
echo "files=" >> "$GITHUB_OUTPUT"
52+
exit 0
53+
fi
54+
printf '%s\n' "${files[@]}"
55+
# Pass as a single space-separated string to subsequent steps.
56+
echo "files=${files[*]}" >> "$GITHUB_OUTPUT"
57+
58+
- name: Ruff (auto-fix dry run + format check)
59+
if: steps.changed.outputs.files != ''
60+
run: |
61+
# `--no-fix` reports the same set of issues that `--fix` would
62+
# apply, so the developer sees what to run locally to clean up.
63+
uv run ruff check --no-fix --output-format=github ${{ steps.changed.outputs.files }}
64+
uv run ruff format --check ${{ steps.changed.outputs.files }}
65+
66+
- name: Mypy
67+
if: steps.changed.outputs.files != ''
68+
run: |
69+
# Run mypy from src/ so module paths resolve, dropping the
70+
# leading "src/" from each path. Tests files are checked from
71+
# the repo root.
72+
src_files=()
73+
test_files=()
74+
for f in ${{ steps.changed.outputs.files }}; do
75+
case "$f" in
76+
src/*) src_files+=("${f#src/}") ;;
77+
tests/*) test_files+=("$f") ;;
78+
esac
79+
done
80+
if [ "${#src_files[@]}" -gt 0 ]; then
81+
(cd src && uv run mypy "${src_files[@]}")
82+
fi
83+
if [ "${#test_files[@]}" -gt 0 ]; then
84+
uv run mypy "${test_files[@]}"
85+
fi

pyproject.toml

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,14 @@ dependencies = [
6363
]
6464

6565
[dependency-groups]
66-
dev = ["pytest>=9.0.3", "pytest-asyncio>=0.21.0", "pytest-mock>=3.12.0", "pytest-cov>=4.0.0"]
66+
dev = [
67+
"pytest>=9.0.3",
68+
"pytest-asyncio>=0.21.0",
69+
"pytest-mock>=3.12.0",
70+
"pytest-cov>=4.0.0",
71+
"ruff>=0.15.12",
72+
"mypy>=2.0.0",
73+
]
6774

6875
[project.scripts]
6976
openrag = "tui.main:run_tui"
@@ -77,4 +84,22 @@ filterwarnings = [
7784
package = true
7885
override-dependencies = ["python-dotenv>=1.1.0"]
7986

87+
# Ruff and mypy run only against files changed in a PR (see
88+
# .github/workflows/lint-backend.yml). The configs below stay
89+
# intentionally minimal — broad enforcement is out of scope for the
90+
# refactor that introduced them.
91+
[tool.ruff]
92+
line-length = 100
93+
target-version = "py313"
94+
95+
[tool.ruff.lint]
96+
select = ["E", "F", "I", "B", "UP"]
97+
ignore = ["E501"] # line-length handled by formatter
98+
99+
[tool.mypy]
100+
python_version = "3.13"
101+
ignore_missing_imports = true
102+
no_strict_optional = true
103+
warn_unused_ignores = true
104+
80105

src/api/health.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
"""Liveness and readiness probes."""
2+
23
import asyncio
34

45
import httpx
@@ -21,9 +22,7 @@ async def opensearch_health_ready(request):
2122
from config.settings import IBM_AUTH_ENABLED, OPENSEARCH_URL
2223

2324
if IBM_AUTH_ENABLED:
24-
logger.debug(
25-
"[OPENSEARCH] OpenSearch auth mode enabled, health check per-request"
26-
)
25+
logger.debug("[OPENSEARCH] OpenSearch auth mode enabled, health check per-request")
2726
# In IBM auth mode we cannot rely on the global OpenSearch client
2827
# (auth is established per-request), so perform a lightweight,
2928
# unauthenticated connectivity check against the OpenSearch endpoint.
@@ -53,9 +52,7 @@ async def opensearch_health_ready(request):
5352
status_code=503,
5453
)
5554
except Exception as e:
56-
logger.error(
57-
"[OPENSEARCH] OpenSearch health check failed", error=str(e)
58-
)
55+
logger.error("[OPENSEARCH] OpenSearch health check failed", error=str(e))
5956
return JSONResponse(
6057
{
6158
"status": "not_ready",
@@ -72,9 +69,7 @@ async def opensearch_health_ready(request):
7269
status_code=200,
7370
)
7471
except Exception as e:
75-
logger.error(
76-
"[OPENSEARCH] OpenSearch health check failed", error=str(e)
77-
)
72+
logger.error("[OPENSEARCH] OpenSearch health check failed", error=str(e))
7873
return JSONResponse(
7974
{
8075
"status": "not_ready",

src/app/container.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
Returns a dict consumed by routes (via FastAPI Depends) and the lifespan hook.
44
"""
5+
56
import os
67

78
from api.connector_router import ConnectorRouter
@@ -31,16 +32,14 @@
3132
from session_manager import SessionManager
3233
from utils.jwt_keygen import generate_jwt_keys
3334
from utils.logging_config import get_logger
34-
from utils.telemetry import TelemetryClient, Category, MessageId
35+
from utils.telemetry import Category, MessageId, TelemetryClient
3536

3637
logger = get_logger(__name__)
3738

3839

3940
async def initialize_services():
4041
"""Initialize all services and their dependencies"""
41-
await TelemetryClient.send_event(
42-
Category.SERVICE_INITIALIZATION, MessageId.ORB_SVC_INIT_START
43-
)
42+
await TelemetryClient.send_event(Category.SERVICE_INITIALIZATION, MessageId.ORB_SVC_INIT_START)
4443
from config.settings import IBM_AUTH_ENABLED
4544

4645
if IBM_AUTH_ENABLED:
@@ -128,9 +127,7 @@ async def initialize_services():
128127
loaded_count=loaded_count,
129128
)
130129
except Exception as e:
131-
logger.error(
132-
"Failed to load persisted connections on startup", error=str(e)
133-
)
130+
logger.error("Failed to load persisted connections on startup", error=str(e))
134131
await TelemetryClient.send_event(
135132
Category.CONNECTOR_OPERATIONS, MessageId.ORB_CONN_LOAD_FAILED
136133
)
@@ -179,8 +176,9 @@ def _lazy_session_factory():
179176
# (session_ownership + conversation_persistence). They lazy-resolve
180177
# `db.engine.SessionLocal` as a fallback, but setting it here makes
181178
# the wiring explicit and avoids the import path on the hot loop.
182-
from services.session_ownership_service import session_ownership_service
183179
from services.conversation_persistence_service import conversation_persistence
180+
from services.session_ownership_service import session_ownership_service
181+
184182
session_ownership_service._session_factory = _lazy_session_factory
185183
conversation_persistence._session_factory = _lazy_session_factory
186184

src/app/factory.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
Wires the service container, middleware, routes, and lifespan together
44
into a ready-to-serve FastAPI app.
55
"""
6+
67
from fastapi import FastAPI
78
from prometheus_fastapi_instrumentator import Instrumentator
89

src/app/lifespan.py

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
Order matches the original behavior exactly — see plan
99
main-py-is-too-long-wiggly-knuth.md.
1010
"""
11+
1112
import asyncio
1213
import os
1314

@@ -16,7 +17,7 @@
1617
from config.settings import clients, get_openrag_config
1718
from services.startup_orchestrator import startup_tasks
1819
from utils.logging_config import get_logger
19-
from utils.telemetry import TelemetryClient, Category, MessageId
20+
from utils.telemetry import Category, MessageId, TelemetryClient
2021

2122
logger = get_logger(__name__)
2223

@@ -31,9 +32,7 @@ async def cleanup_subscriptions_proper(services):
3132

3233
all_connections = await connector_service.connection_manager.list_connections()
3334
active_connections = [
34-
c
35-
for c in all_connections
36-
if c.is_active and c.config.get("webhook_channel_id")
35+
c for c in all_connections if c.is_active and c.config.get("webhook_channel_id")
3736
]
3837

3938
for connection in active_connections:
@@ -42,15 +41,11 @@ async def cleanup_subscriptions_proper(services):
4241
"Cancelling subscription for connection",
4342
connection_id=connection.connection_id,
4443
)
45-
connector = await connector_service.get_connector(
46-
connection.connection_id
47-
)
44+
connector = await connector_service.get_connector(connection.connection_id)
4845
if connector:
4946
subscription_id = connection.config.get("webhook_channel_id")
5047
await connector.cleanup_subscription(subscription_id)
51-
logger.info(
52-
"Cancelled subscription", subscription_id=subscription_id
53-
)
48+
logger.info("Cancelled subscription", subscription_id=subscription_id)
5449
except Exception as e:
5550
logger.error(
5651
"Failed to cancel subscription",
@@ -75,17 +70,13 @@ async def _periodic_backup(services):
7570

7671
config = get_openrag_config()
7772
if not config.edited:
78-
logger.debug(
79-
"Onboarding not completed yet, skipping periodic backup"
80-
)
73+
logger.debug("Onboarding not completed yet, skipping periodic backup")
8174
continue
8275

8376
flows_service = services.get("flows_service")
8477
if flows_service:
8578
logger.info("Running periodic flow backup")
86-
backup_results = await flows_service.backup_all_flows(
87-
only_if_changed=True
88-
)
79+
backup_results = await flows_service.backup_all_flows(only_if_changed=True)
8980
if backup_results["backed_up"]:
9081
logger.info(
9182
"Periodic backup completed",
@@ -147,6 +138,7 @@ async def run_startup(app: FastAPI):
147138
# the operator owns the trade-off.
148139
from services.rbac_service import is_rbac_enforced
149140
from utils.run_mode_utils import get_run_mode
141+
150142
if is_rbac_enforced():
151143
logger.info("RBAC enforcement is ON", run_mode=get_run_mode())
152144
else:
@@ -163,6 +155,7 @@ async def run_startup(app: FastAPI):
163155
# at call time, so it will pick up the binding we set here.
164156
try:
165157
from db.engine import init_engine
158+
166159
init_engine()
167160
except Exception as e:
168161
logger.error("DB engine init failed at startup", error=str(e))
@@ -173,6 +166,7 @@ async def run_startup(app: FastAPI):
173166
try:
174167
from db.engine import SessionLocal as _SL
175168
from db.migrations_runtime import run as run_runtime_migration
169+
176170
if _SL is not None:
177171
async with _SL() as _session:
178172
await run_runtime_migration(_session)
@@ -181,9 +175,7 @@ async def run_startup(app: FastAPI):
181175
logger.error("Runtime DB migration failed", error=str(e))
182176
raise
183177

184-
await TelemetryClient.send_event(
185-
Category.APPLICATION_STARTUP, MessageId.ORB_APP_STARTED
186-
)
178+
await TelemetryClient.send_event(Category.APPLICATION_STARTUP, MessageId.ORB_APP_STARTED)
187179

188180
# FastMCP requires its own lifespan to be entered before requests
189181
# arrive so the StreamableHTTPSessionManager task group exists.
@@ -212,9 +204,7 @@ async def run_shutdown(app: FastAPI):
212204

213205
logger.info("Application shutdown initiated")
214206

215-
await TelemetryClient.send_event(
216-
Category.APPLICATION_SHUTDOWN, MessageId.ORB_APP_SHUTDOWN
217-
)
207+
await TelemetryClient.send_event(Category.APPLICATION_SHUTDOWN, MessageId.ORB_APP_SHUTDOWN)
218208

219209
# Drain any pending workspace_config DB-mirror tasks before we
220210
# tear down the engine. Without this, a save_config triggered
@@ -238,6 +228,7 @@ async def run_shutdown(app: FastAPI):
238228
# Gracefully shutdown OpenSearch connection
239229
try:
240230
from utils.opensearch_utils import graceful_opensearch_shutdown
231+
241232
await graceful_opensearch_shutdown(clients.opensearch)
242233
except Exception as e:
243234
logger.error("Error during graceful OpenSearch shutdown", error=str(e))
@@ -252,11 +243,13 @@ async def run_shutdown(app: FastAPI):
252243
# path did this; the live shutdown_event leaked the engine.
253244
try:
254245
from db.engine import dispose_engine
246+
255247
await dispose_engine()
256248
except Exception as e:
257249
logger.error("Error disposing DB engine", error=str(e))
258250

259251
# Cleanup telemetry client
260252
from utils.telemetry.client import cleanup_telemetry_client
253+
261254
await cleanup_telemetry_client()
262255
logger.info("Application shutdown completed")

src/app/middleware.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
"""ASGI middleware for request correlation IDs and structured access logs."""
2+
23
import time
34
import uuid
45

@@ -31,9 +32,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
3132
_structlog.contextvars.clear_contextvars()
3233

3334
headers_dict = dict(scope.get("headers", []))
34-
request_id = (
35-
headers_dict.get(b"x-request-id", b"").decode() or str(uuid.uuid4())
36-
)
35+
request_id = headers_dict.get(b"x-request-id", b"").decode() or str(uuid.uuid4())
3736
path = scope.get("path", "")
3837
method = scope.get("method", "")
3938

src/app/routes/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
The factory calls `register_all_routes(app, services)`. Each sub-module
44
owns one cohesive group of endpoints.
55
"""
6+
67
from fastapi import FastAPI
78

89
from app.routes.internal import register_internal_routes

0 commit comments

Comments
 (0)