|
| 1 | +# httpware docs & comments audit — 2026-07-13 |
| 2 | + |
| 3 | +**Status:** complete |
| 4 | +**Scope:** source-level docstrings/comments (`src/httpware/**/*.py`, 23 files) and |
| 5 | +the living documentation surface (`README.md`, `docs/**`, `architecture/**`). |
| 6 | +`planning/` was excluded — it is a kept historical record, not living truth, |
| 7 | +per the planning convention. This is the first audit to cover source comments |
| 8 | +and `architecture/` directly; the 2026-06-13 docs audit covered only the |
| 9 | +user-facing `docs/`/README surface and used `architecture/` purely as a |
| 10 | +reference. |
| 11 | +**Method:** two parallel first-pass agents (one over `src/`, on a cheap model; |
| 12 | +one over docs/architecture, on the standard model), each returning a compact |
| 13 | +candidate list. Every candidate was then independently re-verified against the |
| 14 | +real source before being written up here — several early candidates turned out |
| 15 | +correct on inspection and are recorded under Verified correct rather than as |
| 16 | +findings. |
| 17 | + |
| 18 | +## Summary |
| 19 | + |
| 20 | +- Stale/inconsistent comments or docstrings: **2** (both in `src/`) |
| 21 | +- Cross-doc contradictions: **4** (all in `docs/`/`architecture/`) |
| 22 | +- Duplication / compaction candidates: **3** |
| 23 | + |
| 24 | +**Verdict on your compaction/dedup question:** worth doing, but narrowly — the |
| 25 | +`src/` comment surface is already lean (the first-pass sweep found zero |
| 26 | +comments that just restate obvious code; every comment/docstring earns its |
| 27 | +keep). The dedup opportunity is entirely on the docs side, and it's |
| 28 | +concentrated: `ResponseTooLargeError`'s behavior is spelled out near-verbatim |
| 29 | +in **three** places (D1) and the "why not respx" paragraph in two (D2, tangled |
| 30 | +up with the `httpx`/`httpx2` slip in I4) — both good candidates for "full |
| 31 | +account in one place, cross-reference from the rest," the same pattern |
| 32 | +`architecture/` already uses elsewhere. The `CircuitBreaker` overlap (D3) is a |
| 33 | +lower-priority, mostly-intentional tutorial-vs-reference depth split. |
| 34 | + |
| 35 | +## Findings |
| 36 | + |
| 37 | +### Stale/inconsistent (source) |
| 38 | + |
| 39 | +**C1 — `errors.py:184` — `BulkheadFullError` docstring says "AsyncBulkhead" but the error is shared by both bulkheads.** |
| 40 | +Docstring: `"Raised when acquire_timeout elapses before an AsyncBulkhead slot becomes available."` |
| 41 | +Both `AsyncBulkhead.acquire` (`bulkhead.py:133`) and sync `Bulkhead.acquire` |
| 42 | +(`bulkhead.py:175`) raise it via the shared `_emit_bulkhead_rejected` helper |
| 43 | +(`bulkhead.py:50-68`) — it is not async-specific. |
| 44 | +*Fix:* drop "Async" — `"Raised when acquire_timeout elapses before a bulkhead slot becomes available."` |
| 45 | +**Resolved** (`2026-07-13.07`). |
| 46 | + |
| 47 | +**C2 — `client.py:1037` vs `client.py:2009` — `stream()` docstrings disagree on whether the middleware-bypass is version-scoped.** |
| 48 | +`AsyncClient.stream()`: `"Bypasses the middleware chain (...) for v1 — see architecture/client.md for the contract."` |
| 49 | +`Client.stream()`: `"Bypasses the middleware chain (...) — matches AsyncClient.stream() behavior."` (no "for v1"). |
| 50 | +`architecture/client.md:23` states the bypass as a permanent design choice ("Both bypass the middleware chain by design"), not a v1-only caveat — so the async docstring's "for v1" doesn't match the truth home either, and the two client docstrings say different things about the same shared behavior. |
| 51 | +*Fix:* drop "for v1" from the async docstring (or, if the bypass genuinely is meant to be revisited post-v1, say so in `architecture/client.md` too and mirror the caveat into the sync docstring). |
| 52 | +**Resolved** (`2026-07-13.07`) — "for v1" dropped; both client docstrings now agree with `architecture/client.md`. |
| 53 | + |
| 54 | +### Cross-doc contradictions |
| 55 | + |
| 56 | +**I1 — `architecture/errors.md:17` misstates where `DecodeError` wrapping happens.** |
| 57 | +`errors.md`: `"The wrap happens at the seam in Client.send / AsyncClient.send — except Exception translates any decoder-side failure into DecodeError(...)."` |
| 58 | +Verified against source: the `try/except Exception: raise DecodeError(...)` lives in `_BoundDecoder.decode` (`decoders/_resolver.py:38-43`), called from `client.py` as `bound.decode(response)` — not inline in `send`. `architecture/decoders.md:12` already states this correctly ("Any exception is wrapped by `_BoundDecoder.decode`"). The two truth-home files contradict each other on the same fact. |
| 59 | +*Fix:* correct `errors.md:17` to point at `_BoundDecoder.decode` (`decoders/_resolver.py`), matching `decoders.md`. |
| 60 | +**Resolved** (`2026-07-13.07`). |
| 61 | + |
| 62 | +**I2 — `docs/dev/contributing.md:28` vs `architecture/conventions.md:24-25` — contradictory docstring requirement.** |
| 63 | +`contributing.md`: `"Module docstrings are required; per-method docstrings only when types alone are insufficient."` (conditional) |
| 64 | +`conventions.md`: `"Module / class / public-method docstrings are required ..."` (unconditional) |
| 65 | +*Fix:* pick one policy and make both files say it — recommend keeping `conventions.md`'s unconditional wording since it's the capability truth home, and updating `contributing.md` to match. |
| 66 | + |
| 67 | +**I3 — `docs/dev/contributing.md:32-34` understates what CI machine-checks, vs `architecture/overview.md:9`.** |
| 68 | +`contributing.md`: `"The CI lint pass (...) catches what the linters can see (e.g. print() via ruff T201); the rest are enforced in code review."` — reads as "only `print()` is machine-checked." |
| 69 | +`overview.md`: documents two more checks contributing.md omits — `PGH003` (blanket `# type: ignore`) is machine-checked, and `SLF001` partially checks the `httpx2._` ban (attribute access, not import). |
| 70 | +This is a fresh drift, not a re-flag of the 2026-06-13 audit's I1 (that finding was about a since-removed "CI grep gates" phrase, already fixed) — `overview.md`'s finer breakdown was apparently added after `contributing.md`'s wording was last touched. |
| 71 | +*Fix:* replace `contributing.md`'s "the rest are enforced in code review" with the same three-tier breakdown `overview.md` uses (machine-checked / partially-checked / review-only), or have it link to `overview.md` instead of restating. |
| 72 | + |
| 73 | +**I4 — `docs/testing.md:110` says `httpx` where it means `httpx2`.** |
| 74 | +`"MockTransport is the public test seam in httpx — supported by the maintainers, stable across versions ... respx patches private internals and has historically broken across httpx major versions."` |
| 75 | +Every other reference in the same file (line 3) and in `architecture/testing.md:4` correctly says `httpx2` — a real, separate PyPI package (`httpx2>=2.0.0,<3.0`, maintained by Pydantic Services Inc.), not an alias for the original `httpx`. This line reads as a leftover phrase from before the `httpx2` rename, and as written it inaccurately implies `respx`'s breakage history is about `httpx2` specifically. |
| 76 | +*Fix:* change both `httpx` occurrences on that line to `httpx2` (verify against `respx`'s actual `httpx2` support status if this section is touched — it may be that `respx` doesn't support `httpx2` at all, which is a stronger reason to use `MockTransport` than "it breaks across versions"). |
| 77 | +**Resolved** (`2026-07-13.07`) — both occurrences corrected to `httpx2`; the underlying "does respx support httpx2 at all" question is left as-is, not part of this mechanical fix. |
| 78 | + |
| 79 | +### Duplication / compaction candidates |
| 80 | + |
| 81 | +**D1 — `ResponseTooLargeError` behavior is spelled out near-verbatim in three files.** |
| 82 | +`docs/errors.md:193-204`, `architecture/errors.md:23`, and `architecture/client.md:36` all restate the same handful of facts (status-agnostic, counts decoded bytes, fires from the non-streaming terminal and `stream()`'s error pre-read but not user-driven iteration, the `"declared"`/`"streamed"` reason split, "neither StatusError, NetworkError, nor TimeoutError — not retried, doesn't count toward the circuit breaker") in matching or near-matching phrasing. `docs/errors.md` has the fullest account. |
| 83 | +*Suggest:* keep the full account in `docs/errors.md` (or `architecture/errors.md`, whichever is meant to be canonical for this fact), compress the other two to a one-line cross-reference. |
| 84 | + |
| 85 | +**D2 — "why not respx" is duplicated between `docs/testing.md:108-110` and `architecture/testing.md:4`.** |
| 86 | +Same argument, ~3 near-identical sentences in each. Bundle this cleanup with the I4 fix (same lines) — reconcile the `httpx`/`httpx2` wording and de-duplicate the reasoning in the same edit, keeping the fuller version in one file. |
| 87 | + |
| 88 | +**D3 — `CircuitBreaker` states/failure-classification/rate-mode overlap between `docs/resilience.md:158-212` and `architecture/resilience.md:17,21`.** Lower priority: this is mostly a legitimate tutorial-vs-compressed-reference depth split, not verbatim duplication, but several exact clauses ("4xx including 429 count as successes," the `window_seconds=30.0`/`minimum_calls=20` defaults) are copied rather than merely covering the same ground. Worth a light pass if `resilience.md` is next revised for another reason — not worth a dedicated change on its own. |
| 89 | + |
| 90 | +## Verified correct (negative results) |
| 91 | + |
| 92 | +- **`src/` comment/docstring redundancy:** zero comments found that merely restate what well-named code already makes obvious — every inline comment explains a non-obvious constraint or invariant (e.g. wire-body header stripping, coverage pragmas, semaphore behavior). |
| 93 | +- **README ↔ `docs/index.md` duplication (regression check):** the ~70% prose duplication fixed in change `2026-06-14.01` has **not** crept back. The only verbatim overlap remaining is short, non-prose boilerplate (the Pre-1.0 status line, the "Part of `modern-python`" footer) — not the capability-description duplication the prior fix targeted. The install-extras blocks differ in *coverage* (README documents the `[all]` extra, `docs/index.md` doesn't) rather than in duplicated content. |
| 94 | +- Terminology elsewhere (`AsyncMiddleware`/`Middleware`, `AsyncNext`/`Next`, "terminal", "Seam A/B/C", phase-decorator names) is used consistently across every file checked — I4 is an isolated slip, not a pattern. |
| 95 | + |
| 96 | +## Spawned changes |
| 97 | + |
| 98 | +- **`2026-07-13.07-docs-comments-audit-fixes`** (lightweight) — fixes C1, C2, |
| 99 | + I1, I4. Verified against source; `just lint-ci`, `mkdocs build --strict`, |
| 100 | + and `just test` (780 passed, 100% coverage) all clean. |
| 101 | + |
| 102 | +## Deferred / next steps |
| 103 | + |
| 104 | +- **Needs your call:** I3's fix direction (restate the three-tier breakdown in |
| 105 | + `contributing.md`, or replace it with a link to `overview.md`) and I2's |
| 106 | + policy choice (unconditional vs conditional method-docstring requirement) — |
| 107 | + both are wording contradictions where either side could be "the fix," |
| 108 | + not a clear code-vs-doc mismatch. Not yet scheduled. |
| 109 | +- **Compaction:** D1 (`ResponseTooLargeError` triplication) and D2 (bundled |
| 110 | + with I4's `httpx`/`httpx2` fix — the "why not respx" duplication itself |
| 111 | + wasn't touched by `2026-07-13.07`, only the terminology slip within it) are |
| 112 | + still open. D3 is a defer-until-touched item, not worth its own change. |
0 commit comments