Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/skills/ojhunt-crawlers/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ Site accessible?
Every crawler test file must have all three (see the test template in `docs/dev/crawlers.md`):

```python notest
async def test_user_not_exist(session): ... # raises ValueError "The user does not exist"
async def test_username_with_space(session): ... # raises ValueError
async def test_user_not_exist(session): ... # ValueError "The user does not exist"
async def test_username_with_space(session): ... # ValueError
async def test_valid_user(session): ... # asserts solved/submissions/solved_list
```

Expand Down
10 changes: 10 additions & 0 deletions docs/BACKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,13 @@ texts still need editing by hand.

Option: put `explanation` and `credential_args` next to `LoginType.label` and generate the
paragraphs from there.

## Eolymp interpolates the username into its GraphQL query text

`src/ojhunt/crawlers/eolymp.py` builds its query with `%`-formatting and escapes the username by
hand (`username.replace('"', '\\"')`). GraphQL variables are the right mechanism: pass the query
with a `$search` parameter and send the value in the request's `variables` object. That removes
the manual escaping and the `# noqa: UP031`, because no brace has to survive a format call.

Left alone because it changes the request payload, so it needs its own network verification
against api.eolymp.com rather than a drive-by in a lint sweep.
69 changes: 69 additions & 0 deletions docs/adr/0016-adopt-ruff-default-rule-set.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# ADR 0016 — Adopt Ruff's Default Rule Set, Which Reverses the Typing Convention

**Status:** Accepted

## Context

`pyproject.toml` never pinned `[tool.ruff.lint] select`, so the project always linted with
whatever ruff shipped as its default. That default was stable for years at 59 rules
(`E4`, `E7`, `E9`, `F`). Ruff 0.16.0 expanded it to 413 rules. A dependabot bump from
0.15.22 to 0.16.0 therefore turned 0 findings into 534 across 98 files, and CI failed.

Two of the newly enabled rule groups collide with the codebase on purpose rather than by
accident:

- `UP006`, `UP007`, `UP035` and `UP045` — 341 of the 534 findings — demand PEP 585 and PEP 604
syntax (`dict[str, X]`, `X | None`). `docs/dev/python.md` said the opposite: "Use `Dict`,
`List`, `Union` from the `typing` module." Both rules cannot hold.
- `BLE001` flags 23 `except Exception` blocks. Every crawler ends in one by design.

## Options Considered

### Option A: Pin the old default set, `select = ["E4", "E7", "E9", "F"]`

**Rejected because:** it freezes the linter at the 2023 default forever and hides real defects
this bump surfaced. Four of them were genuine: two functions timed their own work by
subtracting `datetime.now()` readings, which an NTP step corrupts, and two rendered timestamps
named no timezone. A rule set chosen to produce zero findings cannot find those.

### Option B: Adopt the new default set, but keep the typing convention

**Rejected because:** it needs `ignore = ["UP006", "UP007", "UP035", "UP045"]` — a permanent
exemption whose only argument is that the code already looks that way. The
`format-lint-python.sh` hook runs `ruff check --fix` on every edited file, so without the
exemption the convention is unenforceable anyway, and with it the codebase keeps a style that
Python has deprecated since 3.9.

### Option C: Adopt the new default set and modernise the typing (chosen)

The autofix does the mechanical work. `requires-python` is already `>=3.12`, so no annotation
in this repo needs the `typing` spelling for compatibility.

## Decision

**Option C.** The default rule set stands as ruff ships it. Three narrow exceptions are
recorded in config, and three at the site:

| Exception | Where | Why |
|-----------|-------|-----|
| `ignore = ["BLE001"]` | `pyproject.toml` | The catch-all is the crawler contract: any parse or transport failure becomes a `RuntimeError` the runner reports per crawler, so one judge's surprise never aborts a run. |
| `extend-immutable-calls = ["fastapi.File"]` | `pyproject.toml` | `File(...)` in a parameter default is how FastAPI declares an upload. |
| `# noqa: FLY002` | `web/app.py` | The fix collapses the CSP into one 300-character line and deletes the comments inside it. That block is load-bearing — see [ADR 0010](0010-relaxed-csp-for-inline-alpine.md). |
| `# noqa: UP031` | `crawlers/eolymp.py` | `.format()` needs every brace in the GraphQL body doubled. |
| `# noqa: DTZ007` | `web/pdf.py` | The chart axis parses day keys that are already local-day strings. It needs their order, not an instant. |

`docs/dev/python.md` now states the PEP 585/604 rule, and the `query()` templates in
`docs/dev/crawlers.md` were updated so a new crawler does not reintroduce the old style.

## Consequences

- A future ruff release that expands the defaults again will surface findings the same way.
That is accepted: CI runs `./doit.sh lint`, so the bump fails loudly on the dependabot PR
rather than landing silently.
- `Dict`, `List`, `Optional` and `Union` no longer appear in annotations. A patch that adds one
back gets rewritten by the `format-lint-python.sh` hook on the next edit.
- The 413-rule set covers `SIM`, `C4`, `B`, `DTZ`, `RUF`, `PL` and more, so new code meets
checks that were never applied to the code already in the tree.
- `./doit.sh lint` runs `ruff format --check` beside `ruff check`, because ruff 0.16 also
formats Python blocks inside Markdown and that drift was invisible to a check-only gate.
Both passes always run, so one report lists every problem.
30 changes: 19 additions & 11 deletions docs/dev/crawlers.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ All crawlers return:

```python notest
{
"solved": int, # Number of accepted problems
"submissions": int, # Total submissions, never below "solved"
"solved_list": list|None # Problem IDs (None if unavailable)
"solved": int, # Number of accepted problems
"submissions": int, # Total submissions, never below "solved"
"solved_list": list | None, # Problem IDs (None if unavailable)
}
```

Expand Down Expand Up @@ -73,7 +73,6 @@ without it. The module docstring is not available for this — it holds the lice
# (copy the full header from an existing crawler)

import aiohttp
from typing import Dict, List, Optional, Union

__crawler_meta__ = {
"title": "OJ Name",
Expand All @@ -82,7 +81,10 @@ __crawler_meta__ = {
"test_username": "known_active_user",
}

async def query(session: aiohttp.ClientSession, username: str, password: Optional[str] = None) -> Dict[str, Union[int, List[str], None]]:

async def query(
session: aiohttp.ClientSession, username: str, password: str | None = None
) -> dict[str, int | list[str] | None]:
"""Query OJ Name for user statistics.

Args:
Expand Down Expand Up @@ -126,7 +128,6 @@ async def query(session: aiohttp.ClientSession, username: str, password: Optiona

import aiohttp
from selectolax.lexbor import LexborHTMLParser
from typing import Dict, List, Optional, Union

__crawler_meta__ = {
"title": "Your OJ",
Expand All @@ -135,7 +136,10 @@ __crawler_meta__ = {
"test_username": "known_active_user",
}

async def query(session: aiohttp.ClientSession, username: str, password: Optional[str] = None) -> Dict[str, Union[int, List[str], None]]:

async def query(
session: aiohttp.ClientSession, username: str, password: str | None = None
) -> dict[str, int | list[str] | None]:
"""Query Your OJ for user statistics.

Args:
Expand Down Expand Up @@ -183,13 +187,14 @@ __crawler_meta__ = {
"test_username": "known_active_user",
}


async def query(
session: aiohttp.ClientSession,
username: str,
password: Optional[str] = None,
login_user: Optional[str] = None,
login_password: Optional[str] = None,
) -> Dict[str, Union[int, List[str], None]]:
password: str | None = None,
login_user: str | None = None,
login_password: str | None = None,
) -> dict[str, int | list[str] | None]:
"""Query Your OJ for user statistics.

Your OJ hides profiles from guests, so a login is always required. Any account
Expand Down Expand Up @@ -229,16 +234,19 @@ from ojhunt.crawlers.example import query, __crawler_meta__
TEST_USERNAME = __crawler_meta__["test_username"]
NOT_EXIST_USERNAME = "fmv84zcq3hwu_notexist"


@pytest.mark.asyncio
async def test_user_not_exist(session):
with pytest.raises(ValueError, match="The user does not exist"):
await query(session, NOT_EXIST_USERNAME)


@pytest.mark.asyncio
async def test_username_with_space(session):
with pytest.raises(ValueError):
await query(session, " ")


@pytest.mark.asyncio
async def test_valid_user(session):
result = await query(session, TEST_USERNAME)
Expand Down
4 changes: 3 additions & 1 deletion docs/dev/e2e.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ See [`docs/dev/testing.md`](testing.md) for shared pytest fixture and assertion
covered by `test_query.py`. The success response shape is:
```python notest
{
"crawler": "<name>", "username": "<user>", "error": False,
"crawler": "<name>",
"username": "<user>",
"error": False,
"data": {"solved": 100, "submissions": 200, "solvedList": ["1A"], "duration": 0.1},
"message": None,
}
Expand Down
17 changes: 14 additions & 3 deletions docs/dev/python.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@

## Import order

Standard library → third-party → typing:
Standard library → third-party → first-party, with a blank line between the groups.
`typing` and `collections.abc` are standard library, so they belong in the first group:

```python notest
import re
from collections.abc import Callable

import aiohttp
from selectolax.lexbor import LexborHTMLParser
from typing import Dict, List, Union

from ojhunt.core.models import CrawlerResult
```

Ruff enforces this order (`I001`) and fixes it, so you do not have to sort by hand.

## Naming

- Files: `snake_case.py` (crawlers), `*_test.py` (tests)
Expand All @@ -19,7 +25,12 @@ from typing import Dict, List, Union

## Typing

Use `Dict`, `List`, `Union` from the `typing` module — not the `dict[str, ...]` syntax.
Use built-in generics and the `|` union syntax: `dict[str, int]`, `list[str]`, `str | None`.
Do not use `Dict`, `List`, `Optional` or `Union` from `typing` — ruff rewrites them
(`UP006`, `UP007`, `UP035`, `UP045`), and the `format-lint-python.sh` hook applies that
rewrite on every edit. Take `Callable` and `Awaitable` from `collections.abc`, not `typing`.

See [ADR 0016](../adr/0016-adopt-ruff-default-rule-set.md) for why this reversed.

## Prefer asserts and names over comments

Expand Down
3 changes: 2 additions & 1 deletion docs/dev/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ New page routes must have a corresponding unit test. Use `TestClient` with monke

```python notest
from starlette.testclient import TestClient

client = TestClient(app, follow_redirects=False)

# File upload syntax:
files={"field": ("name.pdf", bytes_content, "application/pdf")}
files = {"field": ("name.pdf", bytes_content, "application/pdf")}
```

## Markdown doc tests
Expand Down
1 change: 1 addition & 0 deletions docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,4 @@ Significant architectural decisions and their rationale are recorded in [`docs/a
- [ADR 0013](./adr/0013-lazy-crawler-registry.md) — the registry is the module attribute `ojhunt.crawlers.crawlers`, discovered on first access; the `TYPE_CHECKING` declaration is load-bearing for ruff F822
- [ADR 0014](./adr/0014-generated-crawler-help.md) — crawler `help()` text is generated from `__crawler_meta__` and attached to `CrawlerInfo.__doc__`; crawler module docstrings stay reserved for the license header
- [ADR 0015](./adr/0015-submissions-floor-is-solved.md) — every crawler reports at least its `solved` count as `submissions`, so the figure is a lower bound; the rule lives in the crawler files, not in `CrawlerResult`
- [ADR 0016](./adr/0016-adopt-ruff-default-rule-set.md) — ruff's default rule set stands unpinned, which reversed the typing convention to PEP 585/604; the five exceptions (`BLE001`, `fastapi.File`, and three `noqa`s) are listed there
4 changes: 2 additions & 2 deletions docs/library.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ should.
### `query_sync()`

```text
query_sync(crawler: Union[CrawlerInfo, Callable[..., Awaitable[Any]]], username: str, **kwargs: Any) -> CrawlerResult
query_sync(crawler: CrawlerInfo | Callable[..., Awaitable[Any]], username: str, **kwargs: Any) -> CrawlerResult

Query a crawler synchronously, opening and closing a session for you.

Expand Down Expand Up @@ -200,7 +200,7 @@ Attributes:
Fields
solved: int
submissions: int
solved_list: Optional[List[str]] = None
solved_list: list[str] | None = None
```

### `CrawlerInfo`
Expand Down
13 changes: 11 additions & 2 deletions doit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,16 @@ status() {
logs() { tail -F "$SERVER_LOG"; }

lint() {
run_logged lint uv run ruff check .
run_logged lint _ruff_check_and_format
}

# Both ruff passes always run, so one report lists every problem. Ruff 0.16 also
# formats python blocks inside markdown, which `ruff check` alone does not see.
_ruff_check_and_format() {
local status=0
uv run ruff check . || status=1
uv run ruff format --check . || status=1
return "$status"
}

gen-docs() {
Expand Down Expand Up @@ -300,7 +309,7 @@ Server:
reap kill orphaned servers whose git worktree has been removed

Tests:
lint run ruff linter
lint run ruff linter and formatter check
test-unit run unit tests (no network, no playwright) [pytest-args...]
test-e2e run e2e tests excluding visual (starts server if needed) [pytest-args...]
test-visual run visual regression tests (starts server if needed) [pytest-args...]
Expand Down
10 changes: 10 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ packages = ["src/ojhunt"]
[tool.ruff]
exclude = ["archived_crawlers"]

[tool.ruff.lint]
# BLE001: every crawler ends in `except Exception` by contract — any parse or
# transport failure becomes a RuntimeError that the runner reports per crawler,
# so one crawler's surprise never aborts the run. See ADR 0016.
ignore = ["BLE001"]

[tool.ruff.lint.flake8-bugbear]
# B008: `File(...)` in a parameter default is how FastAPI declares an upload.
extend-immutable-calls = ["fastapi.File"]

[tool.pytest.ini_options]
python_files = ["*_test.py", "test_*.py"]
testpaths = ["tests", "README.md", "docs/"]
Expand Down
27 changes: 18 additions & 9 deletions scripts/generate_library_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
"""

import inspect
from collections.abc import Callable
from dataclasses import MISSING, Field, fields
from enum import Enum
from pathlib import Path
from typing import Any, Callable, List
from typing import Any

import ojhunt.crawlers
from ojhunt.core.models import (
Expand Down Expand Up @@ -58,7 +59,11 @@ def _doc(obj: Any) -> str:

def _readable(annotation: str) -> str:
"""Drop the module prefixes that only add noise."""
return annotation.replace("typing.", "").replace("ojhunt.core.models.", "")
return (
annotation.replace("typing.", "")
.replace("collections.abc.", "")
.replace("ojhunt.core.models.", "")
)


def _signature(fn: Callable[..., Any]) -> inspect.Signature:
Expand Down Expand Up @@ -138,7 +143,7 @@ def _crawler_row(crawler: CrawlerInfo) -> str:


def render_library_docs() -> str:
crawlers: List[CrawlerInfo] = [c for _, c in sorted(crawler_registry.items())]
crawlers: list[CrawlerInfo] = [c for _, c in sorted(crawler_registry.items())]
assert crawlers, "no crawlers discovered — the table and example would be empty"

sections = [
Expand All @@ -151,9 +156,11 @@ def render_library_docs() -> str:
"",
"## API",
"",
"Everything below is importable from `ojhunt.crawlers`. The registry itself "
"is the module attribute `crawlers`, a `CrawlerRegistry` built on first "
"access.",
(
"Everything below is importable from `ojhunt.crawlers`. The registry itself "
"is the module attribute `crawlers`, a `CrawlerRegistry` built on first "
"access."
),
"",
_class_entry(CrawlerRegistry),
"",
Expand All @@ -171,9 +178,11 @@ def render_library_docs() -> str:
"",
"## Supported crawlers",
"",
f"{len(crawlers)} crawlers. Every one takes `(session, username)`; the "
'arguments below are additional. Run `help(crawlers["<name>"])` for one '
"crawler's full entry.",
(
f"{len(crawlers)} crawlers. Every one takes `(session, username)`; the "
'arguments below are additional. Run `help(crawlers["<name>"])` for one '
"crawler's full entry."
),
"",
"| Crawler | Platform | Login | Username / notes | Extra arguments |",
"| --- | --- | --- | --- | --- |",
Expand Down
Loading