From 25dc70d86c386308024ab7a6732fa4717300cc89 Mon Sep 17 00:00:00 2001 From: FZ2000 Date: Thu, 30 Jul 2026 18:17:58 -0700 Subject: [PATCH] docs: fix the last drifted claims, and make usage-line drift impossible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit's remaining findings, plus a structural fix so this class stops recurring. CONSOLE TRANSCRIPTS THE CODE CANNOT PRODUCE docs/commands/README.md promises "Output shown in `console` blocks is real output from the command, not an illustration." Three blocks were not: - getting-started.md showed `feed sync stopped: complete`. sync.py rewrites a surviving "complete" to TIME_BUDGET_EXHAUSTED before the summary prints, precisely so a truncated run cannot look finished — so that string never reaches a terminal. Now `feed exhausted`. - troubleshooting.md showed a 503 as `[error] HTTP 503 from : Service Unavailable` followed by `re-run with -v for the full traceback`. Both lines are wrong: `_advice_for_http` prints "DeviantArt is having trouble (HTTP 503 Service Unavailable)", and for a 5xx `offer_traceback` is False so the second line never appears at all — and where it does appear it says "re-run with -v to see the request that failed". Replaced with the real two lines, captured by stubbing a 503. - auth.md showed `[error] refresh failed: HTTP 401 ...`. The string "refresh failed" does not exist anywhere in dacli/. The real message names the refresh token and tells you to run `da auth`. USAGE LINES — and the reason they drifted unnoticed Three were stale: `da daily` had gained `--json`, `da search user` had gained `--json`, and `da sync watched` had gained a mutually exclusive group, so argparse prints `[--user USER | --via-feed]` where the doc still showed `[--user USER] [--via-feed]`. tools/check_doc_flags.py already walks the parser, but it only ever compared the flag *tables*. The `usage:` line above each table — the part a reader copies verbatim — was checked by nothing. It is now: every `usage: da ...` block in docs/ is resolved to its subparser and compared against `format_usage()`, whitespace-normalised and first line only, since argparse wraps to terminal width. 31 usage lines now verified alongside the 67 flag rows. The check earned its place immediately: I had fixed two usage lines by hand from the audit's list, and it found a third I had missed (`da search user`). Negative control confirmed — reverting one drifted line takes the count from 0 problems to 1. Two bugs in my own addition, both caught before pushing: I inserted the call after the loop that prints `problems`, so usage findings were counted but never displayed; and the summary line still said "flag-table problem(s)" for what are now documentation problems generally. ADR 0007 had drifted past its own refactor: "thirteen command handlers" (that is the top-level subcommand count; there are 29 handlers) and "the largest module is 848 lines" (sync.py is ~1,200, and it is the walk plus its checkpointing — not usefully divisible, which is worth saying rather than quoting a number that will drift again). Verified: ruff, mypy, doc references, doc flags + usage lines, codespell all clean; lychee 194 OK / 0 errors with fragments; 868 tests pass. --- docs/commands/auth.md | 2 +- docs/commands/search.md | 4 +- docs/commands/sync.md | 2 +- docs/explanation/adr/0007-package-layout.md | 7 +- docs/getting-started.md | 2 +- docs/guides/troubleshooting.md | 4 +- pyproject.toml | 4 + tools/check_doc_flags.py | 85 ++++++++++++++++++++- tools/gen_cli_docs.py | 4 +- 9 files changed, 99 insertions(+), 15 deletions(-) diff --git a/docs/commands/auth.md b/docs/commands/auth.md index 56a2917..0ac9b43 100644 --- a/docs/commands/auth.md +++ b/docs/commands/auth.md @@ -512,7 +512,7 @@ DeviantArt's own error body, truncated: ```console $ da refresh [warn] refreshing access token via refresh_token -[error] refresh failed: HTTP 401 {"error":"invalid_client","error_description":"Client authentication failed.","status":"error"} +[error] DeviantArt rejected the refresh token (HTTP 401): {"error":"invalid_client","error_description":"Client authentication failed.","status":"error"}. Run `da auth` to sign in again. ``` `invalid_client` there means the `client_id` or `client_secret` is wrong; diff --git a/docs/commands/search.md b/docs/commands/search.md index 20b68c9..1abf287 100644 --- a/docs/commands/search.md +++ b/docs/commands/search.md @@ -389,7 +389,7 @@ $ da search tag-suggest zzz --json | head -12 ## `da search user` ```text -usage: da search user [-h] query [query ...] +usage: da search user [-h] [--json] query [query ...] ``` Resolves one or more usernames to DeviantArt user records. Use it to @@ -429,7 +429,7 @@ da search user spyed devart ## `da daily` ```text -usage: da daily [-h] [--mature] [date] +usage: da daily [-h] [--mature] [--json] [date] ``` Prints DeviantArt's Daily Deviation picks — the staff-selected diff --git a/docs/commands/sync.md b/docs/commands/sync.md index cfd261e..30df759 100644 --- a/docs/commands/sync.md +++ b/docs/commands/sync.md @@ -387,7 +387,7 @@ for the stop reason rather than trusting the exit code. ## da sync watched ```text -usage: da sync watched [-h] [--user USER] [--via-feed] [--feed-max FEED_MAX] +usage: da sync watched [-h] [--user USER | --via-feed] [--feed-max FEED_MAX] [--mature | --no-mature] [--time-budget SECONDS] [--delay-api DELAY_API] [--delay-image DELAY_IMAGE] [--full] [--jitter JITTER] [--concurrency CONCURRENCY] diff --git a/docs/explanation/adr/0007-package-layout.md b/docs/explanation/adr/0007-package-layout.md index c6f45e5..815e2ba 100644 --- a/docs/explanation/adr/0007-package-layout.md +++ b/docs/explanation/adr/0007-package-layout.md @@ -8,7 +8,8 @@ Accepted (2026-07-28). Binding. da-cli spans several distinct concerns — paths, logging, config, secrets, a SQLite index, an HTTP client, OAuth 2.1, a concurrent sync -engine, thirteen command handlers, and the argument parser. Held in one +engine, twenty-nine command handlers across thirteen top-level +subcommands, and the argument parser. Held in one namespace they were mutually reachable, so nothing recorded which parts were meant to depend on which, and any documentation of the internal structure had to be maintained by hand against line numbers. @@ -70,7 +71,9 @@ touches it, so it stays a plain module global. ### Positive -- Each concern is readable on its own; the largest module is 848 lines. +- Each concern is readable on its own. The largest is `sync.py` at ~1,200 + lines, which is the walk plus its checkpointing and is not usefully + divisible; the rest sit well under that. - Dependencies between concerns are explicit imports rather than shared-namespace assumptions. - `ARCHITECTURE.md` describes modules, so it cannot drift the way a diff --git a/docs/getting-started.md b/docs/getting-started.md index 62b1520..1485cb5 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -308,7 +308,7 @@ Output looks like: [0s] feed offset=0 + ArtistName/Sample Title 245 KB + AnotherArtist/Cool Art 1.2 MB -feed sync stopped: complete; ok=2 dup=0 noimg=0 fail=0 +feed sync stopped: feed exhausted; ok=2 dup=0 noimg=0 fail=0 ``` Check what was downloaded: diff --git a/docs/guides/troubleshooting.md b/docs/guides/troubleshooting.md index d8f60c1..32d2a81 100644 --- a/docs/guides/troubleshooting.md +++ b/docs/guides/troubleshooting.md @@ -38,8 +38,8 @@ Failures print what went wrong and exit `2`: ```console $ da whoami -[error] HTTP 503 from https://www.deviantart.com/api/v1/oauth2/user/whoami: Service Unavailable -[error] re-run with -v for the full traceback +[error] DeviantArt is having trouble (HTTP 503 Service Unavailable). +This is usually temporary and on their end. The next run resumes where this one stopped; nothing has been lost. ``` The short form keeps the common case readable and keeps the exit code diff --git a/pyproject.toml b/pyproject.toml index 7255219..6716fda 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -189,6 +189,10 @@ ignore = [ # `X as X` alias — the form ruff recognises as a re-export. A blanket # suppression hid 28 imports that had stopped being anything at all. "dacli/__init__.py" = ["SLF001"] +# argparse exposes no public accessor for its subparser tree, so both +# doc tools walk `_actions` / `_SubParsersAction` to reach it. +"tools/check_doc_flags.py" = ["SLF001"] +"tools/gen_cli_docs.py" = ["SLF001"] "tests/**" = [ "D", # docstrings optional in tests "ANN", # annotations optional in tests diff --git a/tools/check_doc_flags.py b/tools/check_doc_flags.py index f71b682..9a6922a 100644 --- a/tools/check_doc_flags.py +++ b/tools/check_doc_flags.py @@ -49,8 +49,8 @@ def parser_surface() -> tuple[dict[str, dict[str, object]], set[str]]: def walk(parser: argparse.ArgumentParser, prefix: str) -> None: opts: dict[str, object] = {} - for action in parser._actions: # noqa: SLF001 — argparse exposes no public API - if isinstance(action, argparse._SubParsersAction): # noqa: SLF001 + for action in parser._actions: + if isinstance(action, argparse._SubParsersAction): for name, sub in action.choices.items(): walk(sub, f"{prefix} {name}") for opt in action.option_strings: @@ -83,6 +83,77 @@ def _defaults_agree(claimed: str, actual: object) -> bool: return str(actual).lower() in c +def check_usage_blocks(problems: list[str]) -> int: + """Every `usage: da ...` block in the docs must match the parser. + + The flag-table check above catches a flag that is documented wrongly or + not at all. It does not look at the `usage:` line above the table, and + that line drifts independently — two were wrong when this was added: + `da daily` had gained `--json`, and `da sync watched` had gained a + mutually-exclusive group, so the docs showed + `[--user USER] [--via-feed]` where argparse prints + `[--user USER | --via-feed]`. Both are exactly the kind of detail a + reader copies verbatim. + + Compared on the first line only, and whitespace-normalised: argparse + wraps to terminal width, so the continuation lines are cosmetic. + """ + checked = 0 + for doc in sorted((REPO / "docs").rglob("*.md")): + # cli.md is generated by tools/gen_cli_docs.py from this same parser + # and is verified by a git-diff check in CI; re-checking it here + # would just duplicate that. + if doc.name == "cli.md": + continue + rel = doc.relative_to(REPO) + for lineno, line in enumerate(doc.read_text().splitlines(), 1): + if not line.startswith("usage: da "): + continue + checked += 1 + parts = line[len("usage: ") :].split() + path: list[str] = [] + for tok in parts[1:]: + if tok.startswith(("[", "-", "{")): + break + path.append(tok) + sub = _resolve(path) + if sub is None: + problems.append( + f"{rel}:{lineno}: `usage: da {' '.join(path)}` names no such command" + ) + continue + real = sub.format_usage().strip().splitlines()[0] + if _norm(real) != _norm(line): + problems.append( + f"{rel}:{lineno}: usage line drifted\n doc: {line}\n real: {real}" + ) + return checked + + +def _norm(s: str) -> str: + return " ".join(s.split()) + + +def _resolve(path: list[str]) -> argparse.ArgumentParser | None: + """Walk the subparser tree to the parser for `da `. + + Reaching into `_actions` / `_SubParsersAction` is the only way argparse + exposes its subparser tree; there is no public accessor. The rest of + this file and tools/gen_cli_docs.py do the same for the same reason. + """ + cur = dacli.build_parser() + for name in path: + nxt = None + for a in cur._actions: + if isinstance(a, argparse._SubParsersAction) and name in a.choices: + nxt = a.choices[name] + break + if nxt is None: + return None + cur = nxt + return cur + + def main() -> int: surface, known = parser_surface() problems: list[str] = [] @@ -162,12 +233,18 @@ def main() -> int: if missing: problems.append(f"{rel}: `{command}` table omits {sorted(missing)}") + usage_checked = check_usage_blocks(problems) + for p in problems: print(p) + if problems: - print(f"\n{len(problems)} flag-table problem(s).", file=sys.stderr) + print(f"\n{len(problems)} documentation problem(s).", file=sys.stderr) return 1 - print(f"{rows_checked} flag rows across {tables_checked} tables match the parser") + print( + f"{rows_checked} flag rows across {tables_checked} tables and " + f"{usage_checked} usage lines match the parser" + ) return 0 diff --git a/tools/gen_cli_docs.py b/tools/gen_cli_docs.py index 0f70809..9c5e293 100644 --- a/tools/gen_cli_docs.py +++ b/tools/gen_cli_docs.py @@ -106,8 +106,8 @@ def _subparsers(parser: argparse.ArgumentParser) -> dict[str, argparse.ArgumentP since Python 3.2 and the generator is dev-only — a break shows up as a failed `make docs`, never as a runtime error for a user. """ - for action in parser._actions: # noqa: SLF001 - if isinstance(action, argparse._SubParsersAction): # noqa: SLF001 + for action in parser._actions: + if isinstance(action, argparse._SubParsersAction): return dict(action.choices) return {}