diff --git a/backend/tests/test_source_size.py b/backend/tests/test_source_size.py index 1a09835d..373a1f84 100644 --- a/backend/tests/test_source_size.py +++ b/backend/tests/test_source_size.py @@ -4,8 +4,10 @@ checklist, measured rather than argued). The frontend's twin is src/lib/sourceSi Two limits, each with today's offenders as a CEILING: * a module over MAX_LINES lines — a listed one may not grow past its line count; any other module may not cross the limit; - * a function over ruff's C901 limit (cyclomatic complexity, MAX_COMPLEXITY) — per module, the - offenders' values highest-first may not rise, and no new offender may appear. + * a function over ruff's C901 limit (cyclomatic complexity, MAX_COMPLEXITY) — a listed one, by its + NAME (`module::function`), may not rise; no other function may cross the limit. (Ranked per + module until the E32 review: one function could grow by what another in the same module lost.) + A rename reads as a new offender plus a stale entry: move its row. Shrinking is free; a module or function that drops UNDER the limit must be taken off its list (red until it is), so the lists only get shorter. Raising a ceiling is a decision for the owner, with the reason in docs/gates.md. @@ -37,32 +39,39 @@ BIG_MODULES: dict[str, int] = { "app/routes/downloads.py": 1237, } -COMPLEX: dict[str, list[int]] = { - "app/bodylimit.py": [11], - "app/discover/hero.py": [14], - "app/discover/pins.py": [12], - "app/downloads/edit.py": [16, 15], - "app/downloads/formats.py": [11], - "app/downloads/gc.py": [15, 11], - "app/downloads/og.py": [12, 11], - "app/downloads/pipeline.py": [14, 12], - "app/feedquery.py": [28, 14], - "app/plex/query.py": [13], - "app/plex/stream.py": [14], - "app/plex/subtitles.py": [12], - "app/plex/sync.py": [13], - "app/plex/tmdb.py": [11], - "app/plex/watch_sync.py": [12], - "app/routes/auth.py": [19], - "app/routes/debug_sessions.py": [12], - "app/routes/downloads.py": [11], - "app/routes/plex/browse.py": [25, 18, 13], - "app/routes/plex/detail.py": [13], - "app/routes/search.py": [15], - "app/sync/playlists.py": [19], - "app/sync/runner.py": [13], - "app/sync/tagsuggest.py": [14], - "app/worker.py": [18], +COMPLEX: dict[str, int] = { + "app/bodylimit.py::__call__": 11, + "app/discover/hero.py::pick": 14, + "app/discover/pins.py::_shelf_card": 12, + "app/downloads/edit.py::normalize_edit_spec": 16, + "app/downloads/edit.py::run_ffmpeg": 15, + "app/downloads/formats.py::build_ydl_opts": 11, + "app/downloads/gc.py::_sweep_orphan_files": 15, + "app/downloads/gc.py::run_download_gc": 11, + "app/downloads/og.py::_jpeg_size": 12, + "app/downloads/og.py::_watch_tags": 11, + "app/downloads/pipeline.py::_download": 14, + "app/downloads/pipeline.py::_process_edit": 12, + "app/feedquery.py::feed_summary": 14, + "app/feedquery.py::filtered_query": 28, + "app/plex/query.py::meta_filter_conds": 13, + "app/plex/stream.py::start_session": 14, + "app/plex/subtitles.py::_ffmpeg_extract": 12, + "app/plex/sync.py::sync": 13, + "app/plex/tmdb.py::parse_tmdb": 11, + "app/plex/watch_sync.py::run_plex_watch_sync": 12, + "app/routes/auth.py::callback": 19, + "app/routes/debug_sessions.py::send_issue": 12, + "app/routes/downloads.py::resolve_source": 11, + "app/routes/plex/browse.py::facets": 25, + "app/routes/plex/browse.py::per_table": 13, + "app/routes/plex/browse.py::unified_library": 18, + "app/routes/plex/detail.py::item_detail": 13, + "app/routes/search.py::search_youtube": 15, + "app/sync/playlists.py::merge_playlist": 19, + "app/sync/runner.py::run_rss_poll": 13, + "app/sync/tagsuggest.py::compute_tag_suggestions": 14, + "app/worker.py::_run_supervised": 18, } @@ -73,7 +82,9 @@ def _line_counts() -> dict[str, int]: } -def _complexity() -> dict[str, list[int]]: +def _complexity() -> dict[str, int]: + """`module::function` -> its complexity; a name taken twice in a module gets ` 2`, ` 3` in + source order.""" out = subprocess.run( [ sys.executable, @@ -93,13 +104,17 @@ def _complexity() -> dict[str, list[int]]: text=True, check=True, ).stdout - found: dict[str, list[int]] = {} - for m in json.loads(out): + found: dict[str, int] = {} + for m in sorted(json.loads(out), key=lambda m: (m["filename"], m["location"]["row"])): value = re.search(r"\((\d+) > \d+\)", m["message"]) - assert value, m["message"] + name = re.search(r"`([^`]+)`", m["message"]) + assert value and name, m["message"] rel = Path(m["filename"]).resolve().relative_to(BACKEND).as_posix() - found.setdefault(rel, []).append(int(value.group(1))) - return {f: sorted(v, reverse=True) for f, v in found.items()} + key, n = f"{rel}::{name.group(1)}", 2 + while key in found: + key, n = f"{rel}::{name.group(1)} {n}", n + 1 + found[key] = int(value.group(1)) + return found def test_no_module_crosses_the_line_limit_and_a_listed_one_does_not_grow(): @@ -117,21 +132,14 @@ def test_lists_no_module_that_is_back_under_the_limit(): def test_no_function_crosses_the_complexity_limit_and_a_listed_one_does_not_rise(): - worse = [] - for f, values in _complexity().items(): - ceiling = COMPLEX.get(f, []) - for i, v in enumerate(values): - cap = ceiling[i] if i < len(ceiling) else MAX_COMPLEXITY - if v > cap: - worse.append(f"{f}: #{i + 1} is {v} (ceiling {cap})") + worse = [ + f"{fn}: {v} (ceiling {COMPLEX.get(fn, MAX_COMPLEXITY)})" + for fn, v in _complexity().items() + if v > COMPLEX.get(fn, MAX_COMPLEXITY) + ] assert worse == [] -def test_lists_no_offender_that_is_back_under_the_limit(): +def test_lists_no_function_that_is_back_under_the_limit_or_gone(): actual = _complexity() - stale = [ - f"{f}: listed {len(v)}, now {len(actual.get(f, []))}" - for f, v in COMPLEX.items() - if len(actual.get(f, [])) < len(v) - ] - assert stale == [] + assert [fn for fn in COMPLEX if fn not in actual] == [] diff --git a/docs/gates.md b/docs/gates.md index 72d8be23..8fb4b7cb 100644 --- a/docs/gates.md +++ b/docs/gates.md @@ -75,7 +75,7 @@ project checks needs a row. | --- | --- | --- | --- | | backend + scripts | `E402` — ignored | A few modules (`main.py`, `scripts/e2e_seed.py`) run setup — logging, `sys.path` — before their imports on purpose. | — | | backend + scripts | `B008` — narrowed: `fastapi.Depends`, `fastapi.Query` declared immutable | FastAPI's dependency injection IS a call in a default argument, on every route. Every other call in a default still fails. | 474 sites, all of them FastAPI | -| backend + scripts | `C901` — not in ruff; measured by `tests/test_source_size.py` | Ruff has no per-function baseline, so turning it on would need a `noqa` per offender, and a `noqa` lets a function grow without limit. The test holds each offender at its current value. | 32 functions over 10 | +| backend/app | `C901` — not in ruff; measured by `tests/test_source_size.py` | Ruff has no per-function baseline, so turning it on would need a `noqa` per offender, and a `noqa` lets a function grow without limit. The test holds each offender, by name, at its current value. Tests and scripts are outside it: see the size / complexity rows under Lanes. | 32 functions over 10 | ## basedpyright (backend) @@ -109,7 +109,8 @@ project checks needs a row. | CVE gate | HIGH/CRITICAL with a fix available only | `--ignore-unfixed` + `--severity HIGH,CRITICAL`: a finding with no fix cannot be acted on by an upgrade; it shows in the scan output. A fixable one fails the publish, and the only way past it is `.trivyignore.yaml`. | 0 open (E32 S1) | | npm audit (CI) | `--omit=dev --audit-level=high` | What ships to the browser, at high or above. The dev toolchain's advisories were burned down by hand in E32 S1 (18 → 0). | 0 | | React Compiler | the functions it skips, pinned | `components/compilerCoverage.contract.test.ts` — the list may only shrink. End state: only VirtualGrid (TanStack Virtual, an IncompatibleLibrary) remains. | 93 (E32 S1) → 29 (E32 S3) → 1, VirtualGrid (E32 S4) | -| size / complexity | today's offenders as ceilings | `src/lib/sourceSize.contract.test.ts`, `backend/tests/test_source_size.py`: listed files and functions may not grow, nothing new may cross the limit, an entry back under it must be deleted. | files >800 lines: 13 FE / 11 BE; functions over the limit: 64 FE / 32 BE | +| size / complexity | today's offenders as ceilings | `src/lib/sourceSize.contract.test.ts`, `backend/tests/test_source_size.py`: listed files and functions may not grow, nothing new may cross the limit, an entry back under it must be deleted. A function is held by its name, not its rank in the file. | files >800 lines: 13 FE / 9 BE; functions over the limit: 64 FE / 32 BE (E32 review) | +| size / complexity | shipped code only — not tests, test kits, e2e or `scripts/` | The ratchet guards what runs in production. A test file is long because it is a list of cases, and a test kit is test code; splitting either moves lines without making anything easier to change. `scripts/` is release and seed tooling, not in the image. | outside it: tests >800 lines 2 FE / 5 BE; functions over the limit 2 FE test code (`gates.contract.test.ts` 30, `compilerSkips.testkit.ts` 25), 1 BE test (11), 1 script (`e2e_seed.py` `main` 13) | ## Accepted findings diff --git a/frontend/src/lib/sourceSize.contract.test.ts b/frontend/src/lib/sourceSize.contract.test.ts index b6cd8146..9375ebcd 100644 --- a/frontend/src/lib/sourceSize.contract.test.ts +++ b/frontend/src/lib/sourceSize.contract.test.ts @@ -12,8 +12,10 @@ import { // CEILING: // • a file over 800 lines — the listed ones may not grow past their line count; any other // file may not cross the limit; -// • a function over complexity 20 (eslint `complexity`, cyclomatic) — per file, the -// offenders' values highest-first may not rise, and no new offender may appear. +// • a function over complexity 20 (eslint `complexity`, cyclomatic) — a listed one, by its +// NAME, may not rise; no other function may cross the limit. (Ranked per file until the E32 +// review: one function could grow by what another in the same file lost.) A rename reads as a +// new offender plus a stale entry: move its row. // Shrinking is free; a file or function that drops UNDER the limit must be taken off its list (red // until it is), so the lists only get shorter. Raising a ceiling is a decision for the owner, with // the reason in docs/gates.md. JSX branches count toward complexity, so a component scores high by @@ -35,61 +37,71 @@ const BIG_FILES: Record = { "lib/plexPlayerHooks.ts": 922, }; -const COMPLEX: Record = { - "App.tsx": [28], - "components/AccountPanel.tsx": [29], - "components/AdminLogs.tsx": [35], - "components/auditColumns.tsx": [24], - "components/ChannelPage.tsx": [91], - "components/Channels.tsx": [53], - "components/ConfigPanel.tsx": [28, 24], - "components/DownloadCenter.tsx": [36], - "components/downloadRow.tsx": [25], - "components/Feed.tsx": [64], - "components/Home.tsx": [34, 32, 21], - "components/IssueComposer.tsx": [34], - "components/KeyGate.tsx": [22], - "components/NotificationsPanel.tsx": [28], - "components/OnboardingWizard.tsx": [23], - "components/PersonalViewsModal.tsx": [24], - "components/PlayerModal.tsx": [99], - "components/Playlists.tsx": [66], - "components/PlexBrowse.tsx": [45], - "components/plexCards.tsx": [58], - "components/PlexCollectionView.tsx": [41], - "components/PlexDiscover.tsx": [21], - "components/PlexInfo.tsx": [65], - "components/PlexPlayer.tsx": [103, 22], - "components/PlexPlaylistView.tsx": [22], - "components/PlexSidebar.tsx": [32], - "components/PlexStatsOverlay.tsx": [29], - "components/plexViews.tsx": [50, 30, 21], - "components/Scheduler.tsx": [21, 21], - "components/SetupWizard.tsx": [21], - "components/SyncStatus.tsx": [22], - "components/ui/AnchoredPopover.tsx": [22], - "components/ui/anchoredPopoverHooks.ts": [30], - "components/ui/Combobox.tsx": [22], - "components/ui/DataTable.tsx": [42], - "components/ui/PanelGroup.tsx": [23], - "components/ui/SidePanel.tsx": [45], - "components/ui/SortControl.tsx": [23], - "components/usePlaylistActions.tsx": [25], - "components/usePlexActiveFilters.ts": [29], - "components/useVideoActions.tsx": [25], - "components/VideoCard.tsx": [39], - "components/VideoEditor.tsx": [50], - "components/WatchPage.tsx": [30], - "components/Welcome.tsx": [39, 24], - "lib/api/core.ts": [48, 22], - "lib/api/plex.ts": [26], - "lib/diagnostics.ts": [22], - "lib/keymap.ts": [28], - "lib/logHooks.ts": [21], - "lib/plexDetail.ts": [24], - "lib/plexPlayerHooks.ts": [30], - "lib/urlState.ts": [26, 24], - "lib/useActiveFilters.ts": [28], +const COMPLEX: Record = { + "App.tsx::App": 28, + "components/AccountPanel.tsx::AccountPanel": 29, + "components/AdminLogs.tsx::AdminLogs": 35, + "components/ChannelPage.tsx::ChannelPage": 91, + "components/Channels.tsx::Channels": 53, + "components/ConfigPanel.tsx::ConfigPanel": 24, + "components/ConfigPanel.tsx::Field": 28, + "components/DownloadCenter.tsx::DownloadCenter": 36, + "components/Feed.tsx::Feed": 64, + "components/Home.tsx::Hero": 32, + "components/Home.tsx::Home": 34, + "components/Home.tsx::useHomeCardMenu": 21, + "components/IssueComposer.tsx::ComposerBody": 34, + "components/KeyGate.tsx::KeyGate": 22, + "components/NotificationsPanel.tsx::NotificationRow": 28, + "components/OnboardingWizard.tsx::OnboardingWizard": 21, + "components/PersonalViewsModal.tsx::PersonalViewsModal": 22, + "components/PlayerModal.tsx::PlayerModal": 98, + "components/Playlists.tsx::Playlists": 66, + "components/PlexBrowse.tsx::PlexBrowse": 45, + "components/PlexCollectionView.tsx::PlexCollectionView": 41, + "components/PlexDiscover.tsx::PlexDiscover": 21, + "components/PlexInfo.tsx::PlexInfo": 65, + "components/PlexPlayer.tsx::(anonymous)": 22, + "components/PlexPlayer.tsx::PlexPlayer": 103, + "components/PlexPlaylistView.tsx::PlexPlaylistView": 22, + "components/PlexSidebar.tsx::PlexSidebar": 32, + "components/PlexStatsOverlay.tsx::(anonymous)": 25, + "components/Scheduler.tsx::JobRow": 21, + "components/Scheduler.tsx::Scheduler": 21, + "components/SetupWizard.tsx::SetupWizard": 21, + "components/SyncStatus.tsx::SyncStatus": 22, + "components/VideoCard.tsx::VideoCard": 39, + "components/VideoEditor.tsx::VideoEditor": 49, + "components/WatchPage.tsx::WatchPage": 30, + "components/Welcome.tsx::AuthCard": 39, + "components/Welcome.tsx::onSubmit": 24, + "components/auditColumns.tsx::localDetails": 24, + "components/downloadRow.tsx::DownloadRow": 25, + "components/plexCards.tsx::PlexPosterCard": 58, + "components/plexViews.tsx::PlexPersonView": 30, + "components/plexViews.tsx::PlexSeasonView": 21, + "components/plexViews.tsx::PlexShowView": 50, + "components/ui/AnchoredPopover.tsx::PopoverSurfaceInner": 22, + "components/ui/Combobox.tsx::Combobox": 22, + "components/ui/DataTable.tsx::DataTable": 42, + "components/ui/PanelGroup.tsx::PanelGroup": 23, + "components/ui/SidePanel.tsx::SidePanel": 45, + "components/ui/SortControl.tsx::SortControl": 23, + "components/ui/anchoredPopoverHooks.ts::useAnchoredPopover": 30, + "components/usePlaylistActions.tsx::pushSteps": 23, + "components/usePlexActiveFilters.ts::buildPlexActiveFilters": 25, + "components/useVideoActions.tsx::(anonymous)": 25, + "lib/api/core.ts::localizeDetail": 48, + "lib/api/core.ts::req": 22, + "lib/api/plex.ts::appendPlexFilters": 26, + "lib/diagnostics.ts::applyDiagnostics": 22, + "lib/keymap.ts::dispatch": 28, + "lib/logHooks.ts::controlAttrs": 21, + "lib/plexDetail.ts::useDetailAside": 24, + "lib/plexPlayerHooks.ts::usePlexTracks": 30, + "lib/urlState.ts::filtersToParams": 24, + "lib/urlState.ts::paramsToFilters": 26, + "lib/useActiveFilters.ts::useActiveFilters": 25, }; describe("file size", () => { @@ -112,23 +124,13 @@ describe("function complexity", async () => { const actual = await complexityOffenders(); it(`no function crosses ${MAX_COMPLEXITY}, and a listed one does not rise`, () => { - const worse: string[] = []; - for (const [file, values] of Object.entries(actual)) { - const ceiling = COMPLEX[file] ?? []; - values.forEach((v, i) => { - if (v > (ceiling[i] ?? MAX_COMPLEXITY)) - worse.push(`${file}: #${i + 1} is ${v} (ceiling ${ceiling[i] ?? MAX_COMPLEXITY})`); - }); - } + const worse = Object.entries(actual) + .filter(([fn, v]) => v > (COMPLEX[fn] ?? MAX_COMPLEXITY)) + .map(([fn, v]) => `${fn}: ${v} (ceiling ${COMPLEX[fn] ?? MAX_COMPLEXITY})`); expect(worse).toEqual([]); }); - it("lists no offender that is back under the limit (shorten the entry)", () => { - const stale = Object.entries(COMPLEX) - .filter(([file, values]) => (actual[file]?.length ?? 0) < values.length) - .map( - ([file, values]) => `${file}: listed ${values.length}, now ${actual[file]?.length ?? 0}`, - ); - expect(stale).toEqual([]); + it("lists no function that is back under the limit, or gone (delete its row)", () => { + expect(Object.keys(COMPLEX).filter((fn) => !(fn in actual))).toEqual([]); }); }); diff --git a/frontend/src/lib/sourceSize.testkit.ts b/frontend/src/lib/sourceSize.testkit.ts index a9f70bde..9777c71a 100644 --- a/frontend/src/lib/sourceSize.testkit.ts +++ b/frontend/src/lib/sourceSize.testkit.ts @@ -27,9 +27,12 @@ export function lineCounts(files: string[]): Record { return out; } -/** file → the complexities over MAX_COMPLEXITY, highest first. A bare rule run (no type info, no - * other rules), so it costs a couple of seconds, not the full lint. */ -export async function complexityOffenders(): Promise> { +/** `file::function` → its complexity, for every function over MAX_COMPLEXITY. Keyed by NAME, not by + * rank in the file: a ranked list let one function grow while another in the same file shrank by as + * much (E32 review). A function without a name is `(anonymous)`; a name taken twice in a file gets + * ` 2`, ` 3` in source order. A bare rule run (no type info, no other rules), so it costs a couple of + * seconds, not the full lint. */ +export async function complexityOffenders(): Promise> { const eslint = new ESLint({ cwd: fileURLToPath(new URL("../../", import.meta.url)), overrideConfigFile: true, @@ -45,19 +48,19 @@ export async function complexityOffenders(): Promise> { }, ], }); - const out: Record = {}; + const out: Record = {}; for (const r of await eslint.lintFiles(["src/**/*.{ts,tsx}"])) { - const values = r.messages - .filter((m) => m.ruleId === "complexity") - .map((m) => Number(/complexity of (\d+)/.exec(m.message)?.[1])) - .sort((a, b) => b - a); - if (values.length) - out[ - r.filePath - .split("\\") - .join("/") - .replace(/^.*\/frontend\/src\//, "") - ] = values; + const file = r.filePath + .split("\\") + .join("/") + .replace(/^.*\/frontend\/src\//, ""); + for (const m of r.messages) { + if (m.ruleId !== "complexity") continue; + const name = /'([^']+)'/.exec(m.message)?.[1] ?? "(anonymous)"; + let key = `${file}::${name}`; + for (let n = 2; key in out; n++) key = `${file}::${name} ${n}`; + out[key] = Number(/complexity of (\d+)/.exec(m.message)?.[1]); + } } return out; }