Skip to content

Commit 536d3a6

Browse files
MartinCastroAlvarezmartin-castro-laminr-aiclaude
authored
chore: code-quality audit fixes (#651#656) (#660)
* chore: consolidate Python lint stack on Ruff + mypy + bandit (#651, #652) Remove Black, standalone isort, and flake8 entirely — their config blocks, dev dependencies, and scripts/lint.sh steps. Ruff now owns lint + format + import order (the `I` rules); mypy and bandit run alongside. This resolves the three-formatter conflict and the Black 24-vs-26 pin skew (#452). Enable a strict mypy subset on the package (disallow_untyped_defs, warn_return_any) and wire the now-green Python lint gate (ruff check + ruff format --check + mypy + bandit) into backend CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: fix stale comments and tighten typing (#654, #655) - Remove the misleading "Real implementation lands in PR #2" note on the shipped _PackageSettings dataclass and a dead `# noqa: ARG002` line in views.py that suppressed nothing (#654). (audit.py dead-code removal landed in the preceding lint-stack commit.) - Type the admin_site view helpers as AdminSite (type-only import, so the package still works with django.contrib.admin removed) and add the missing `from typing import Any` to tests/test_spa_index.py (F821) (#655). - Normalize residual lint debt (import order + slice/assert formatting) across tests and templatetags now that the gate is green (#651). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: repoint dangling doc references and add a doc-ref guard (#653) Repoint or remove docstring / comment / pre-commit citations to docs that no longer exist (docs/ux/pwa.md, pwa.md, theming.md, ACCEPTANCE.md, REVIEW_CHECKLIST.md, docs/threat-model.md) so they target the surviving ARCHITECTURE.md / SECURITY.md sections. (conf.py's docs/ux/pwa.md cites were fixed alongside the comment cleanup in the preceding commit.) Add tests/test_doc_refs.py plus a pre-commit hook that fails when a *.md file or §N section cited in the package source, tests, or .pre-commit-config.yaml no longer exists, so this defect class can't recur. Also drop the black + isort pre-commit hooks per #651/#652. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(frontend): enforce no-explicit-any and tidy JSDoc (#656) - Flip @typescript-eslint/no-explicit-any from 'off' to 'error' to lock in the existing zero-`any` state; no violations surfaced. - Remove the stale .eslintrc.cjs entry from the flat-config ignores. - Add /** JSDoc to the Checkbox, Input, Spinner, EmptyState, and DateHierarchyBar primitives for doc consistency. Verified: pnpm lint:js, pnpm lint:css, pnpm typecheck all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: add CHANGELOG with [Unreleased] entries for the audit fixes (#651-#656) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Martin Castro Laminrs <mcastro@laminr.ai> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b7ec202 commit 536d3a6

21 files changed

Lines changed: 290 additions & 381 deletions

.github/workflows/ci.yml

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,12 @@
1111
# (run the test suites in CI); the no-CI revisit is recorded in
1212
# `SECURITY.md` §8 / `docs/agents/decisions.md` / OQ-A-001.
1313
#
14-
# SCOPE: the backend job runs `pytest` (the regression that motivated
15-
# #452 was a broken test). The frontend job runs the full `pnpm` gate —
16-
# typecheck + lint + test + build — which is already self-consistent and
17-
# green. Enforcing the *Python lint* gate in CI is a deliberate near-term
18-
# follow-up: `scripts/lint.sh` currently runs two formatters (`ruff
19-
# format` + `black`) whose output conflicts, so it is not yet satisfiable
20-
# on a clean tree. That gets de-conflicted and the small existing lint
21-
# debt cleared first (follow-up off #452), then the lint step is added
22-
# here.
14+
# SCOPE: the backend job runs the Python lint gate (ruff check + ruff
15+
# format --check + mypy + bandit) and then `pytest`. The frontend job
16+
# runs the full `pnpm` gate — typecheck + lint + test + build. The Python
17+
# lint stack was collapsed onto a single Ruff-based chain in #651/#652
18+
# (black/isort/flake8 removed), which made the gate satisfiable on a
19+
# clean tree and let it be wired in here.
2320
#
2421
# SECURITY POSTURE:
2522
# - Least-privilege: top-level `contents: read`; no job needs write.
@@ -87,6 +84,18 @@ jobs:
8784
- name: Pin Django to the matrix version
8885
run: poetry run pip install "django~=${{ matrix.django }}.0"
8986

87+
# Python lint gate (#651/#652): a single Ruff-based stack —
88+
# ruff check (lint, incl. `I` import order) + ruff format --check +
89+
# mypy (strict subset on the package) + bandit (security). Black,
90+
# standalone isort, and flake8 were removed. This is the same gate
91+
# scripts/lint.sh runs locally.
92+
- name: Lint (ruff + mypy + bandit)
93+
run: |
94+
poetry run ruff check django_admin_react tests
95+
poetry run ruff format --check django_admin_react tests
96+
poetry run mypy django_admin_react
97+
poetry run bandit -r django_admin_react -c pyproject.toml -q
98+
9099
# pytest with coverage (per pyproject `addopts`), including
91100
# tests/test_security.py. `filterwarnings = ["error"]` means a new
92101
# warning fails the run.

.pre-commit-config.yaml

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
# pre-commit install
66
#
77
# Then every `git commit` runs these hooks against the staged diff.
8-
# Anything below the [BLOCK] threshold in
9-
# `docs/agents/security-expert/REVIEW_CHECKLIST.md` §1 aborts the commit.
8+
# The security rules these enforce are documented in `SECURITY.md` §3.
109
#
1110
# This file is the commit-time half of the quality gate, run together
1211
# with `scripts/lint.sh` before a PR. CI (`.github/workflows/ci.yml`)
13-
# currently runs the test suites, not these hooks; wiring the lint/security
14-
# hooks into CI is a follow-up (#452).
12+
# runs the Python lint gate (ruff check + ruff format --check + mypy +
13+
# bandit) and the test suites; these hooks add the secret-scan + pygrep
14+
# house rules at commit time.
1515

1616
repos:
1717
# -------------------------------------------------------------------
@@ -24,7 +24,9 @@ repos:
2424
args: ["protect", "--staged", "--no-banner", "--redact"]
2525

2626
# -------------------------------------------------------------------
27-
# Python formatting + linting (same tools scripts/lint.sh uses).
27+
# Python lint + format + import order — Ruff is the single source of
28+
# truth (#651/#652). Black, standalone isort, and flake8 were removed;
29+
# ruff-format owns formatting and the `I` rules own import sorting.
2830
# -------------------------------------------------------------------
2931
- repo: https://github.com/astral-sh/ruff-pre-commit
3032
rev: v0.6.9
@@ -35,18 +37,6 @@ repos:
3537
- id: ruff-format
3638
files: ^(django_admin_react|tests|examples)/
3739

38-
- repo: https://github.com/psf/black-pre-commit-mirror
39-
rev: 24.8.0
40-
hooks:
41-
- id: black
42-
files: ^(django_admin_react|tests|examples)/
43-
44-
- repo: https://github.com/pycqa/isort
45-
rev: 5.13.2
46-
hooks:
47-
- id: isort
48-
files: ^(django_admin_react|tests|examples)/
49-
5040
# -------------------------------------------------------------------
5141
# Python security lint (bandit). Runs only over the package, not tests
5242
# (asserts in tests are fine and would otherwise be noisy).
@@ -63,18 +53,17 @@ repos:
6353

6454
# -------------------------------------------------------------------
6555
# House rules — local hooks that enforce package-specific invariants
66-
# from docs/agents/security-expert/REVIEW_CHECKLIST.md.
56+
# from SECURITY.md §3.
6757
# -------------------------------------------------------------------
6858
- repo: local
6959
hooks:
7060
# No partial / redacted token references anywhere in the diff.
7161
#
7262
# The `exclude` list covers the small set of files that
7363
# legitimately *document* the forbidden patterns (e.g., the
74-
# rule itself in SECURITY.md / ACCEPTANCE.md, the review
75-
# checklist, the security test that scans for them, and forum
76-
# review files that quote the rule when reviewing it). All
77-
# other files in the repo MUST be free of these substrings.
64+
# rule itself in SECURITY.md and the security test that scans for
65+
# them). All other files in the repo MUST be free of these
66+
# substrings.
7867
- id: no-partial-tokens
7968
name: No partial token redactions (e.g., ghp_…XYZ)
8069
language: pygrep
@@ -83,10 +72,7 @@ repos:
8372
exclude: |
8473
(?x)^(
8574
SECURITY\.md
86-
|ACCEPTANCE\.md
8775
|\.pre-commit-config\.yaml
88-
|docs/threat-model\.md
89-
|docs/agents/security-expert/.*
9076
|tests/test_security\.py
9177
|scripts/README\.md
9278
)$
@@ -118,3 +104,12 @@ repos:
118104
language: pygrep
119105
entry: "from ['\"]@dar/api['\"]"
120106
files: '^frontend/packages/(list|details|models|shell)/'
107+
108+
# Doc-reference integrity (#653): fail if a docstring/comment cites
109+
# a *.md file or §N section that no longer exists.
110+
- id: doc-ref-guard
111+
name: No dangling *.md / §N doc references
112+
language: system
113+
entry: poetry run pytest tests/test_doc_refs.py -q
114+
pass_filenames: false
115+
files: '\.(py|yaml)$'

CHANGELOG.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Changelog
2+
3+
All notable changes to this project are documented here.
4+
5+
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
6+
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
7+
8+
## [Unreleased]
9+
10+
### Changed
11+
12+
- **Python lint stack consolidated onto Ruff (#651, #652).** Removed Black,
13+
standalone isort, and flake8 entirely (their `[tool.*]` config, dev
14+
dependencies, pre-commit hooks, and `scripts/lint.sh` steps). Ruff now owns
15+
lint + format + import order (the `I` rules), with mypy + bandit alongside —
16+
resolving the three-formatter conflict (#452/#452-skew) and the Black 24-vs-26
17+
pin skew. The now-green Python lint gate (ruff check + ruff format --check +
18+
mypy + bandit) is wired into backend CI.
19+
- **mypy tightened on the package (#655).** Enabled the `disallow_untyped_defs`
20+
and `warn_return_any` strict subset for `django_admin_react`; typed the
21+
`admin_site` view helpers as `AdminSite` (type-only import) instead of `Any`.
22+
- **Frontend `@typescript-eslint/no-explicit-any` promoted from `off` to
23+
`error` (#656)** to lock in the existing zero-`any` state, and added `/**`
24+
JSDoc to the `Checkbox`, `Input`, `Spinner`, `EmptyState`, and
25+
`DateHierarchyBar` primitives.
26+
27+
### Removed
28+
29+
- **Dead `django_admin_react/audit.py` module (#654).** It was imported nowhere
30+
and had 0% coverage; the `LogEntry` access it duplicated belongs in the
31+
sibling `django-admin-rest-api`.
32+
33+
### Fixed
34+
35+
- **Dangling documentation references (#653).** Repointed or removed docstring /
36+
comment / pre-commit citations to docs that no longer exist (`docs/ux/pwa.md`,
37+
`pwa.md`, `theming.md`, `ACCEPTANCE.md`, `REVIEW_CHECKLIST.md`,
38+
`docs/threat-model.md`) so they target the surviving `ARCHITECTURE.md` /
39+
`SECURITY.md` sections. Added a fast doc-reference guard
40+
(`tests/test_doc_refs.py` + a pre-commit hook) that fails when a `*.md` file
41+
or `§N` section cited in source no longer exists.
42+
- **Stale comments (#654).** Removed the misleading "Real implementation lands
43+
in PR #2" note on the shipped `_PackageSettings` dataclass and a dead
44+
`# noqa: ARG002` line in `views.py` that suppressed nothing.

django_admin_react/audit.py

Lines changed: 0 additions & 58 deletions
This file was deleted.

django_admin_react/conf.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@
9191
"REACT_LOGIN": True,
9292
# PWA (Issue #86) — all optional; sane defaults make the manifest
9393
# work with zero config. See ``django_admin_react/pwa.py`` +
94-
# ``docs/ux/pwa.md``.
94+
# ``ARCHITECTURE.md`` §5.4.
9595
#
9696
# ``PWA_NAME`` — installed-app name. ``None`` (default) falls
9797
# back to the AdminSite ``site_header``, then
@@ -152,8 +152,11 @@
152152
class _PackageSettings:
153153
"""Resolved package settings.
154154
155-
Real implementation lands in PR #2. For now this is a stub so other
156-
modules can import the typed attribute names.
155+
An immutable, frozen record of the merged
156+
``settings.DJANGO_ADMIN_REACT`` overrides on top of :data:`DEFAULTS`,
157+
built once by :func:`_load` and cached. Each field carries its
158+
default; modules read the typed attribute names off the cached
159+
instance via this module's :func:`__getattr__`.
157160
"""
158161

159162
ADMIN_SITE: str = DEFAULTS["ADMIN_SITE"]

django_admin_react/pwa.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""PWA surface: web app manifest + service worker (Issue #86).
22
3-
Wire/UX contract: ``docs/ux/pwa.md``. The Security lane owns this
4-
surface because its load-bearing properties are security ones:
3+
Frontend build/ship context: ``ARCHITECTURE.md`` §5.4. The Security lane
4+
owns this surface because its load-bearing properties are security ones:
55
66
- The **manifest** (``<mount>/web.manifest``) is served unauthenticated
77
(the install prompt fires before login) and is computed at request
@@ -15,8 +15,7 @@
1515
no-store`` (so the package's no-store API reads are never cached),
1616
never caches non-GET requests (mutation safety), and exposes a
1717
cache-purge message used on logout so read-cached payloads can't
18-
outlive the session (``pwa.md`` §5 — defense-in-depth atop session
19-
expiry).
18+
outlive the session (defense-in-depth atop session expiry).
2019
2120
Both views live **outside** ``api/`` because they're served at the
2221
mount root, not under ``api/v1/``, and the manifest is intentionally
@@ -33,13 +32,13 @@
3332
from django.shortcuts import render
3433
from django.views.generic import View
3534

36-
from django_admin_react import conf as dar_conf
37-
3835
# Re-use the API package's admin-site lookup (this repo implements no
3936
# API; the registry helper lives there). The PWA only needs the active
4037
# `AdminSite.name` for the manifest's start URL.
4138
from django_admin_rest_api.api.registry import get_admin_site
4239

40+
from django_admin_react import conf as dar_conf
41+
4342
# Theme colours keyed by the resolved colour scheme. Kept here (not in
4443
# the SPA's CSS-var system) because the manifest is rendered server-side
4544
# before any CSS loads; these are the install-banner / splash colours
@@ -79,7 +78,7 @@ def _mount(request: HttpRequest, suffix: str) -> str:
7978
def _resolved_scheme(request: HttpRequest) -> str:
8079
"""Resolve light/dark from the ``Sec-CH-Prefers-Color-Scheme`` hint.
8180
82-
Pairs with the theming client-hint path (``theming.md`` §2). Any
81+
Pairs with the theming client-hint path (``ARCHITECTURE.md`` §5.3). Any
8382
value other than a case-insensitive ``"dark"`` resolves to light —
8483
the safe, neutral default when the hint is absent or unexpected.
8584
"""
@@ -93,7 +92,7 @@ class ManifestView(View):
9392
Unauthenticated by design (the install prompt needs it pre-login).
9493
Carries no per-user data; every field is static or mount-/header-
9594
derived. ``Cache-Control: no-store`` is **not** set — the manifest
96-
is deliberately cacheable/network-first (``pwa.md`` §2.1).
95+
is deliberately cacheable/network-first.
9796
"""
9897

9998
http_method_names = ["get"]

django_admin_react/templatetags/experience_toggle.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def experience_toggle_strip(context: dict[str, Any]) -> dict[str, Any]:
7676
if not path.startswith(legacy_root):
7777
return {"visible": False}
7878

79-
tail = path[len(legacy_root):]
79+
tail = path[len(legacy_root) :]
8080
query = request.META.get("QUERY_STRING", "") if hasattr(request, "META") else ""
8181
target = react_root + tail + (("?" + query) if query else "")
8282
return {"visible": True, "target": target, "react_root": react_root}

0 commit comments

Comments
 (0)