diff --git a/.claude/hooks/format-lint-python.sh b/.claude/hooks/format-lint-python.sh index ed92dfc..ed392a4 100644 --- a/.claude/hooks/format-lint-python.sh +++ b/.claude/hooks/format-lint-python.sh @@ -10,4 +10,33 @@ elif command -v ruff &>/dev/null; then ruff format "$FILE" 2>/dev/null || true ruff check --fix "$FILE" 2>/dev/null || true fi + +# The project's own rules (lint/rules/), on the one file that just changed. Same +# rules ./doit.sh lint runs over everything, so the hook cannot drift from CI. +# Each rule decides whether the file is in its own scope. +command -v uv &>/dev/null || exit 0 + +# An environment that cannot run anything (fresh clone, offline uv cache) would +# otherwise fail every rule and block every edit on a uv traceback. The exit code +# cannot tell that apart afterwards: a rule that raises also exits 1. The ruff +# steps above skip for the same reason. +uv run python -c "" &>/dev/null || exit 0 + +FAILURES="" +collect() { + OUTPUT=$("$@" 2>&1) || FAILURES+="$OUTPUT"$'\n' + return 0 +} + +for RULE in lint/rules/*.py; do + [ -e "$RULE" ] || continue + collect uv run python "$RULE" "$FILE" +done +if compgen -G "lint/rules/*.yml" >/dev/null; then + collect uv run ast-grep scan "$FILE" +fi +if [[ -n "$FAILURES" ]]; then + printf '%s\n' "$FAILURES" >&2 + exit 2 +fi exit 0 diff --git a/.claude/rules/docs.md b/.claude/rules/docs.md new file mode 100644 index 0000000..f58f4f1 --- /dev/null +++ b/.claude/rules/docs.md @@ -0,0 +1,45 @@ +--- +paths: + - "docs/**/*.md" + - "README.md" + - ".claude/**/*.md" +--- + +# Documentation conventions + +For *where* a fact belongs (docs vs rule vs skill vs hook vs ADR), load the +**ojhunt-update-env** skill. This rule covers how to write once you know the target. + +## Write only what cannot be derived + +Two questions kill most draft lines: + +- Can an agent derive this by opening one file? Delete the line. +- Does a test, hook, or linter already assert it? Delete the line. + +Docs capture intent — why a construct is load-bearing, why the obvious alternative is +wrong. Before writing a rule, try to make a test assert it instead: a test fails when +somebody breaks the rule, a doc line does not. + +**Prefer deleting to adding.** Appending a clarification is the reflex; usually the file reads +better with the wrong line gone. A false observation — a flaky test that was really a bad +connection, a gotcha that was really local misconfiguration — is deleted, not corrected. A +paragraph saying an earlier paragraph was wrong leaves the reader two claims to adjudicate. + +## README.md is a quick start + +Keep it short and direct: what the tool is, how to run it, one example per interface. +Everything else is a link. + +When a section grows past a screen, move the detail into `docs/` and leave a pointer. +Reference material belongs in [`docs/cli.md`](../../docs/cli.md), +[`docs/web.md`](../../docs/web.md) and [`docs/library.md`](../../docs/library.md) — not in +the README. + +`docs/library.md` is generated by `./doit.sh gen-docs`: write library-facing prose in the +docstrings it compiles, never in the file itself. A unit test fails while the committed copy +is stale. + +To check whether it is stale, run the task and read `.doit/gen-docs.log`: never suppress the +generator's stderr, because a crash leaves the file untouched and so looks exactly like +"already fresh". diff --git a/docs/dev/e2e.md b/.claude/rules/e2e.md similarity index 95% rename from docs/dev/e2e.md rename to .claude/rules/e2e.md index 9d977dc..4043f38 100644 --- a/docs/dev/e2e.md +++ b/.claude/rules/e2e.md @@ -1,6 +1,11 @@ +--- +paths: + - "tests/e2e/**/*.py" +--- + # E2E tests (Playwright) -See [`docs/dev/testing.md`](testing.md) for shared pytest fixture and assertion conventions. +See [`tests.md`](tests.md) for shared pytest fixture and assertion conventions. ## Setup diff --git a/.claude/rules/hooks.md b/.claude/rules/hooks.md new file mode 100644 index 0000000..a0f0981 --- /dev/null +++ b/.claude/rules/hooks.md @@ -0,0 +1,30 @@ +--- +paths: + - ".claude/hooks/**" + - ".claude/settings.json" +--- + +# Project hooks + +Hooks decide *when* a check runs. Check `.claude/settings.json` to see what is currently +active. + +## Location + +Scripts live in `.claude/hooks/`, wired in `.claude/settings.json` (checked +into git — not `settings.local.json`, not `~/.claude/`). + +## Keep the check out of the hook + +`format-lint-python.sh` runs ruff and then every rule in `lint/rules/` over the file that +changed. What each rule checks is defined there, not here, so the hook and `./doit.sh lint` +can never disagree. Adding a check to the hook alone gives you a rule that CI does not run and +nobody can test — write it in `lint/rules/` instead (see `.claude/rules/lint-rules.md`). + +## Regex gotcha for command bans + +Do NOT use `(^|[;&|[:space:]])word[[:space:]]` — this matches the word inside +quoted strings and heredocs (e.g. commit messages containing the banned word). + +Use `^[[:space:]]*(sudo[[:space:]]+)?word[[:space:]]` to only match when the +word is the actual command at the start of the line. diff --git a/.claude/rules/lint-rules.md b/.claude/rules/lint-rules.md new file mode 100644 index 0000000..efca82f --- /dev/null +++ b/.claude/rules/lint-rules.md @@ -0,0 +1,51 @@ +--- +paths: + - "lint/**" + - "tests/lint/**" +--- + +# Project lint rules + +Rules the project enforces on its own code, beyond what ruff covers. They live in +`lint/rules/`, run from `./doit.sh lint`, and run again from the edit-time hook on the single +file that changed — one definition, so the hook cannot drift from CI. + +```bash +./doit.sh lint # every rule over the whole repo +./doit.sh lint-rules # the rules' own tests +uv run python lint/rules/.py ... # one rule, one file +``` + +## When a convention earns a rule + +A rule pays for itself when the convention is objective and somebody will break it without +noticing. If judging a case needs context the parser does not have, leave the convention in +`.claude/rules/*.md` as prose — a rule that misfires teaches everyone to ignore it. + +Before writing prose, check whether a rule can assert it instead: a rule fails when somebody +breaks the convention, a paragraph does not. + +## Pick the form + +| Form | Use when | Tests | +|------|----------|-------| +| `lint/rules/.yml` | The match is purely structural — a node shape, nesting, a pattern | `lint/rule-tests/-test.yml` | +| `lint/rules/.py` | The match needs source position, file paths, or cross-file state | `tests/lint/_test.py` | + +Position is the usual reason to reach for Python. A YAML rule sees the parse tree, where a +trailing comment and an own-line comment are the same shape; only line and column separate +them. `comment_above_assert.py` exists for exactly that reason — its docstring says so. + +A Python rule is a script: `find_violations(source, path)` for tests to call, and a `main()` +that takes file arguments, prints to stderr and returns 1 on a finding. Copy the shape from +`lint/rules/comment_above_assert.py`. + +For ast-grep's rule syntax, node kinds and relational operators, query the ast-grep +documentation through Context7 rather than looking for it here. + +## Every rule ships with its tests + +`tests/lint/rules_are_tested_test.py` fails when a rule has no test file, so this is not a +convention you can forget. Cover the edge cases, not just the happy path — for anything +position-sensitive, pin **which line** each case is blamed on, and include the cases that must +stay silent. diff --git a/docs/dev/python.md b/.claude/rules/python.md similarity index 85% rename from docs/dev/python.md rename to .claude/rules/python.md index 9489cc1..238eaa3 100644 --- a/docs/dev/python.md +++ b/.claude/rules/python.md @@ -1,3 +1,9 @@ +--- +paths: + - "**/*.py" + - "pyproject.toml" +--- + # Python conventions ## Import order @@ -30,7 +36,7 @@ Do not use `Dict`, `List`, `Optional` or `Union` from `typing` — ruff rewrites (`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. +See [ADR 0016](../../docs/adr/0016-adopt-ruff-default-rule-set.md) for why this reversed. ## Prefer asserts and names over comments @@ -45,7 +51,9 @@ Asserts are for what the code guarantees itself. Data arriving from outside — argument, a judge's response — needs a real `raise`, because `python -O` strips asserts. In tests, put the explanation in the assert message, not in a comment above it: -`assert "items" in listed, "dict's own attributes must survive"`. +`assert "items" in listed, "dict's own attributes must survive"`. This is enforced — +`lint/rules/comment_above_assert.py` flags a comment beside an `assert`, whether it sits +above the line or trails it. Where a comment would explain *what* an expression produces, name the value instead: `single_line_description = " ".join(text.split())`. @@ -67,9 +75,9 @@ resolve. This applies to all packages including FastAPI, uvicorn, etc. ## Ruff removes imports before their usage -Ruff (pre-commit hook) runs between edits and strips imports that appear unused at -that moment. If you add an import in one `Edit` call and the code that uses it in a -second call, ruff will delete the import between the two calls. +Ruff runs between edits and strips imports that appear unused at that moment. If you add an +import in one `Edit` call and the code that uses it in a second call, ruff will delete the +import between the two calls. **Fix:** always add new imports and the code that uses them in the same `Edit` call. diff --git a/docs/dev/testing.md b/.claude/rules/tests.md similarity index 76% rename from docs/dev/testing.md rename to .claude/rules/tests.md index 4d7d1b6..73e79f8 100644 --- a/docs/dev/testing.md +++ b/.claude/rules/tests.md @@ -1,7 +1,14 @@ +--- +paths: + - "**/*_test.py" + - "tests/**/*.py" +--- + # Pytest conventions -Crawler-specific test conventions live in [`docs/dev/crawlers.md`](crawlers.md) and the -**ojhunt-crawlers** skill. Playwright e2e tests live in [`docs/dev/e2e.md`](e2e.md). +Crawler-specific test conventions live in [`docs/dev/crawlers.md`](../../docs/dev/crawlers.md) +and the **ojhunt-crawlers** skill. Playwright e2e tests have their own rule in +[`e2e.md`](e2e.md). ## Running tests @@ -47,6 +54,13 @@ client = TestClient(app, follow_redirects=False) files = {"field": ("name.pdf", bytes_content, "application/pdf")} ``` +## Prefer discovery over a hardcoded list + +A test that enumerates today's routes, crawlers, or files goes stale in silence: the thing it +was meant to catch is exactly the new entry nobody added to the list. Walk the router or glob +the directory instead. When a shape genuinely cannot be discovered, assert that no such shape +exists yet, so the day one appears the suite says so. + ## Markdown doc tests Python fenced blocks in `README.md` and `docs/` are collected by `pytest-markdown-docs` diff --git a/.claude/rules/web.md b/.claude/rules/web.md new file mode 100644 index 0000000..a85cf7f --- /dev/null +++ b/.claude/rules/web.md @@ -0,0 +1,97 @@ +--- +paths: + - "src/ojhunt/web/**" +--- + +# Web layer conventions + +Dev server lifecycle and environment variables are operational, not code conventions — +they live in [`docs/dev/web.md`](../../docs/dev/web.md). For user-facing usage see +[`docs/web.md`](../../docs/web.md). + +## PDF internals + +- `extract_data(pdf_bytes)` returns `PdfEmbeddedData` — has `settings` and `history` only. + It does **not** have a `snapshot`; the snapshot is never embedded in the PDF. Build one + manually from history if needed. +- For page routes returning `application/pdf` on success and HTML on error: return explicit + `Response(content=..., media_type="application/pdf")` or `HTMLResponse(...)` — do not use + `response_class=HTMLResponse` on the decorator. +- When adding form-based page endpoints accessible to agents, document them in `llms.txt`. + +## Minimal JS principle + +Keep JavaScript minimal. Business logic belongs in Python; JS handles only browser-specific +concerns. + +- Day boundary / timezone computation → backend (frontend sends IANA timezone string) +- History merge/dedup → backend +- Pydantic schemas on all new API endpoints (not loose dicts) +- Prefer a POST to the server over inline JS computation + +JS is appropriate for: reading local files, computing timezone name via +`Intl.DateTimeFormat().resolvedOptions().timeZone`, triggering downloads, reactive UI state. + +## FastAPI trailing slashes + StaticFiles + +When `StaticFiles` is mounted at `"/"`, FastAPI's `redirect_slashes` is suppressed. +`Mount("/")` matches every path first and returns 404 for paths that aren't real files — +the redirect never fires. + +- `fetch()` URLs in `app.js` must exactly match the route path in `api.py` (no trailing + slashes unless the route has one) +- HTML `href` attributes are harmless (browsers follow 307 redirects), but `fetch()` calls + can silently fail because StaticFiles returns a non-JSON 404 body that breaks `response.json()` + +## Reverse-proxy rate limiting (429) + +The app itself never emits HTTP 429 — rate limiting is enforced by the production reverse +proxy, and its 429 body is **not** JSON. Any client `fetch` that reaches the server can +therefore receive a 429 with an HTML/plain-text body. + +- Client `fetch` handlers must special-case `response.status === 429` **before** calling + `response.json()` — otherwise the non-JSON body makes `response.json()` throw and the user + sees a confusing parse error instead of a rate-limit message. +- Established guards live in `app.js`: `executeQuery`, `calculateReport`, `downloadReport`. + +## CSS conventions + +CSS files live at `src/ojhunt/web/static/assets/`: + +| File | What belongs here | +|------|-------------------| +| `base.css` | Design-system tokens (`:root`, `[data-accent=...]`), page resets, `.topbar`, `.page`, `.header`, `.footer`, `dialog`/`.dlg-*`, `.card` base layout (including `::before` stripe and `:hover`), `[x-cloak]`, `:focus-visible`, responsive media queries for shared components | +| `index.css` | Everything used *only* by the home page: `.step`, `.report-slot`, `.grid`, card internals (`.c-hd`, `.c-body-row`, `.c-ft`, `.c-err-msg`, `.solved-link`, `.subs-val`, `.iconbtn`, `.loading-dots`, `.card-empty`), card status variants (`.card.r-ok::before` etc.), `.download-card`/`.summary`/`.dc-*`/`.stat`, `.composer`, `.field`, `.btn` (all variants), `.step-actions` | + +**Short templates** (crawlers, about, pdf pages) keep their inline `