Skip to content

Commit 1733d95

Browse files
dwinter3claude
andcommitted
fix(personas): lint cleanup — D102/D205/SIM118/N802 + ruff format
CI uncovered after rebasing onto #228: - D102 add docstring to is_active property - D205 reformat two store method docstrings (summary line + blank + detail) - SIM118 drop .keys() in set comprehension - N802 lowercase test function names (test_..._LAST_ADMIN → test_..._last_admin, same for USER_DISABLED) - ruff format reformatted 3 files All ruff checks pass locally. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent f25a660 commit 1733d95

4 files changed

Lines changed: 30 additions & 62 deletions

File tree

server/backend/src/cq_server/app.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,11 @@
4040
from .network import router as network_router
4141
from .passkey_routes import router as passkey_router
4242
from .provisioning import recover_stuck_jobs
43-
from .provisioning import router as provisioning_router
4443
from .quality import check_propose_quality
4544
from .reflect import router as reflect_router
4645
from .reputation_routes import router as reputation_router
4746
from .review import router as review_router
4847
from .scoring import apply_confirmation, apply_flag
49-
from .persona_routes import router as persona_router
5048
from .store import normalize_domains
5149
from .store._sqlite import SqliteStore
5250
from .theme_routes import router as theme_router

server/backend/src/cq_server/persona_routes.py

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class PersonaAssignment(BaseModel):
5555

5656
@property
5757
def is_active(self) -> bool:
58+
"""True iff this persona assignment is not soft-disabled."""
5859
return self.disabled_at is None
5960

6061

@@ -169,9 +170,6 @@ async def create_persona(
169170
409 when a persona assignment already exists for this username.
170171
422 when the email is malformed.
171172
"""
172-
from .email_sender import EmailSender
173-
from .invite_routes import get_email_sender
174-
175173
# Check for an existing assignment.
176174
existing = await store.get_persona_assignment(req.username)
177175
if existing is not None:
@@ -183,9 +181,7 @@ async def create_persona(
183181
# M-5: per-admin invite rate-limit. Count persona assignments this
184182
# admin has issued in the trailing hour; cap at 20.
185183
since = (datetime.now(UTC) - timedelta(hours=1)).isoformat()
186-
recent_count = await store.count_invites_by_admin(
187-
admin_username=admin, since=since
188-
)
184+
recent_count = await store.count_invites_by_admin(admin_username=admin, since=since)
189185
if recent_count >= 20:
190186
raise HTTPException(
191187
status_code=429,
@@ -279,10 +275,7 @@ async def patch_persona(
279275
raise HTTPException(
280276
status_code=409,
281277
detail={
282-
"error": (
283-
"user is disabled — re-enable first via "
284-
"POST /admin/personas/{username}/enable"
285-
),
278+
"error": ("user is disabled — re-enable first via POST /admin/personas/{username}/enable"),
286279
"code": "USER_DISABLED",
287280
},
288281
)

server/backend/src/cq_server/store/_sqlite.py

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4283,26 +4283,17 @@ def _get_user_by_email_sync(self, email: str) -> dict[str, Any] | None:
42834283
"email": row[7],
42844284
}
42854285

4286-
42874286
# ------------------------------------------------------------------
42884287
# AS-1 (#200) — persona_assignments CRUD
42894288
# ------------------------------------------------------------------
42904289

4291-
async def list_persona_assignments(
4292-
self, limit: int = 50, offset: int = 0
4293-
) -> tuple[list[dict], int]:
4290+
async def list_persona_assignments(self, limit: int = 50, offset: int = 0) -> tuple[list[dict], int]:
42944291
"""Return paginated persona assignments joined with user email."""
4295-
return await asyncio.get_event_loop().run_in_executor(
4296-
None, self._list_persona_assignments_sync, limit, offset
4297-
)
4292+
return await asyncio.get_event_loop().run_in_executor(None, self._list_persona_assignments_sync, limit, offset)
42984293

4299-
def _list_persona_assignments_sync(
4300-
self, limit: int = 50, offset: int = 0
4301-
) -> tuple[list[dict], int]:
4294+
def _list_persona_assignments_sync(self, limit: int = 50, offset: int = 0) -> tuple[list[dict], int]:
43024295
with self._engine.connect() as conn:
4303-
total_row = conn.execute(
4304-
text("SELECT COUNT(*) FROM persona_assignments")
4305-
).fetchone()
4296+
total_row = conn.execute(text("SELECT COUNT(*) FROM persona_assignments")).fetchone()
43064297
total = total_row[0] if total_row else 0
43074298
rows = conn.execute(
43084299
text(
@@ -4332,9 +4323,7 @@ def _list_persona_assignments_sync(
43324323

43334324
async def get_persona_assignment(self, username: str) -> dict | None:
43344325
"""Return the persona assignment for a user, or None."""
4335-
return await asyncio.get_event_loop().run_in_executor(
4336-
None, self._get_persona_assignment_sync, username
4337-
)
4326+
return await asyncio.get_event_loop().run_in_executor(None, self._get_persona_assignment_sync, username)
43384327

43394328
def _get_persona_assignment_sync(self, username: str) -> dict | None:
43404329
with self._engine.connect() as conn:
@@ -4506,11 +4495,12 @@ def _disable_persona_assignment_sync(
45064495
# ------------------------------------------------------------------
45074496

45084497
async def count_active_admins(self) -> int:
4509-
"""Return count of persona_assignments rows with persona='admin'
4510-
and disabled_at IS NULL. Used by the last-admin guard."""
4511-
return await asyncio.get_event_loop().run_in_executor(
4512-
None, self._count_active_admins_sync
4513-
)
4498+
"""Return count of active admin persona assignments.
4499+
4500+
Counts rows where persona='admin' and disabled_at IS NULL. Used
4501+
by the last-admin guard on the disable endpoint.
4502+
"""
4503+
return await asyncio.get_event_loop().run_in_executor(None, self._count_active_admins_sync)
45144504

45154505
def _count_active_admins_sync(self) -> int:
45164506
with self._engine.connect() as conn:
@@ -4524,18 +4514,17 @@ def _count_active_admins_sync(self) -> int:
45244514
).fetchone()
45254515
return int(row[0]) if row else 0
45264516

4527-
async def count_invites_by_admin(
4528-
self, admin_username: str, since: str
4529-
) -> int:
4530-
"""Count persona_assignments rows assigned by this admin since
4531-
the given ISO-8601 timestamp. Used as a proxy for invite-rate."""
4517+
async def count_invites_by_admin(self, admin_username: str, since: str) -> int:
4518+
"""Count persona assignments minted by this admin since a timestamp.
4519+
4520+
Used as a proxy for invite-rate-limit accounting on the persona
4521+
create endpoint (M-5).
4522+
"""
45324523
return await asyncio.get_event_loop().run_in_executor(
45334524
None, self._count_invites_by_admin_sync, admin_username, since
45344525
)
45354526

4536-
def _count_invites_by_admin_sync(
4537-
self, admin_username: str, since: str
4538-
) -> int:
4527+
def _count_invites_by_admin_sync(self, admin_username: str, since: str) -> int:
45394528
with self._engine.connect() as conn:
45404529
row = conn.execute(
45414530
text(
@@ -4550,9 +4539,7 @@ def _count_invites_by_admin_sync(
45504539

45514540
async def list_persona_audit(self, username: str) -> list[dict]:
45524541
"""Return audit rows for a username, oldest-first (test helper)."""
4553-
return await asyncio.get_event_loop().run_in_executor(
4554-
None, self._list_persona_audit_sync, username
4555-
)
4542+
return await asyncio.get_event_loop().run_in_executor(None, self._list_persona_audit_sync, username)
45564543

45574544
def _list_persona_audit_sync(self, username: str) -> list[dict]:
45584545
with self._engine.connect() as conn:
@@ -4582,9 +4569,7 @@ def _list_persona_audit_sync(self, username: str) -> list[dict]:
45824569

45834570
async def set_user_email(self, username: str, email: str) -> None:
45844571
"""Update the email on a users row (used when creating persona assignments)."""
4585-
await asyncio.get_event_loop().run_in_executor(
4586-
None, self._set_user_email_sync, username, email
4587-
)
4572+
await asyncio.get_event_loop().run_in_executor(None, self._set_user_email_sync, username, email)
45884573

45894574
def _set_user_email_sync(self, username: str, email: str) -> None:
45904575
with self._engine.begin() as conn:

server/backend/tests/test_persona_routes.py

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,7 @@ def test_list_personas_populated(client: TestClient) -> None:
150150
# ---------------------------------------------------------------------------
151151

152152

153-
def test_create_persona_happy_path(
154-
client: TestClient, mock_sender: MockEmailSender
155-
) -> None:
153+
def test_create_persona_happy_path(client: TestClient, mock_sender: MockEmailSender) -> None:
156154
headers = _login(client, ADMIN)
157155
resp = client.post(
158156
"/api/v1/admin/personas",
@@ -300,9 +298,7 @@ def test_disabled_user_cannot_obtain_session_cookie(client: TestClient) -> None:
300298
# we use it, otherwise fall through; the disable check below is
301299
# the load-bearing assertion.
302300
# Disable the user.
303-
disable_resp = client.post(
304-
"/api/v1/admin/personas/fred/disable", headers=headers
305-
)
301+
disable_resp = client.post("/api/v1/admin/personas/fred/disable", headers=headers)
306302
assert disable_resp.status_code == 200, disable_resp.text
307303
# Login attempt with username + password — must be refused with 403.
308304
resp = client.post(
@@ -315,7 +311,7 @@ def test_disabled_user_cannot_obtain_session_cookie(client: TestClient) -> None:
315311
assert resp.status_code in (401, 403)
316312
assert resp.status_code != 200
317313
# And there must be no Set-Cookie header.
318-
assert "set-cookie" not in {k.lower() for k in resp.headers.keys()}
314+
assert "set-cookie" not in {k.lower() for k in resp.headers}
319315

320316

321317
def test_persona_changes_create_audit_rows(client: TestClient) -> None:
@@ -352,9 +348,7 @@ def test_persona_changes_create_audit_rows(client: TestClient) -> None:
352348
store = _get_store()
353349
import asyncio
354350

355-
rows = asyncio.new_event_loop().run_until_complete(
356-
store.list_persona_audit("gina")
357-
)
351+
rows = asyncio.new_event_loop().run_until_complete(store.list_persona_audit("gina"))
358352
assert len(rows) == 4, rows
359353
assert rows[0]["action"] == "CREATED"
360354
assert rows[0]["old_persona"] is None
@@ -370,7 +364,7 @@ def test_persona_changes_create_audit_rows(client: TestClient) -> None:
370364
assert rows[3]["new_persona"] is None
371365

372366

373-
def test_disabling_last_admin_returns_409_LAST_ADMIN(client: TestClient) -> None:
367+
def test_disabling_last_admin_returns_409_last_admin(client: TestClient) -> None:
374368
"""H-3: must refuse to disable the only remaining active admin."""
375369
headers = _login(client, ADMIN)
376370
# Promote the test admin's persona assignment to 'admin' explicitly.
@@ -384,17 +378,15 @@ def test_disabling_last_admin_returns_409_LAST_ADMIN(client: TestClient) -> None
384378
},
385379
)
386380
# Now try to disable the sole admin — must 409 with LAST_ADMIN code.
387-
resp = client.post(
388-
"/api/v1/admin/personas/sole_admin/disable", headers=headers
389-
)
381+
resp = client.post("/api/v1/admin/personas/sole_admin/disable", headers=headers)
390382
assert resp.status_code == 409, resp.text
391383
body = resp.json()
392384
# FastAPI wraps the dict under "detail".
393385
detail = body.get("detail", body)
394386
assert detail.get("code") == "LAST_ADMIN"
395387

396388

397-
def test_patch_disabled_user_returns_409_USER_DISABLED(client: TestClient) -> None:
389+
def test_patch_disabled_user_returns_409_user_disabled(client: TestClient) -> None:
398390
"""M-2: PATCH must not silently re-enable a disabled assignment."""
399391
headers = _login(client, ADMIN)
400392
client.post(

0 commit comments

Comments
 (0)