Merge pull request #2 from modern-python/review/stories-1-1-and-1-2-patches

Review/stories 1 1 and 1 2 patches

Author: lesnik512

.github/workflows/ci.yml

@@ -42,7 +42,7 @@ jobs:
           cache-dependency-glob: "**/pyproject.toml"
       - run: uv python install ${{ matrix.python-version }}
       - run: just install
-      - run: just test --cov=src/httpware --cov-report xml
+      - run: just test --cov-report xml
       - uses: codecov/codecov-action@v4.0.1
         env:
           CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

CHANGELOG.md

@@ -10,11 +10,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 - Initial project scaffold: `src/httpware/` package, `py.typed` marker, `pyproject.toml` with `uv_build` backend.
 - Org conventions ported from `modern-python/modern-di`: `Justfile`, `.github/workflows/ci.yml`, `[tool.ruff]` config, `[tool.pytest.ini_options]`, dev and lint dep groups.
-- Declared dependencies: `httpx2>=2.0.0b1,<3.0`, `pydantic>=2.0,<3.0`.
+- Declared dependencies: `httpx2>=2.0.0,<3.0`, `pydantic>=2.0,<3.0`.
 - Declared install extras: `[msgspec]`, `[otel]`, `[niquests]`, `[all]`.
 - `SECURITY.md` with 90-day private-disclosure window.
 - `CONTRIBUTING.md` with development workflow.
 - `CLAUDE.md` with AI-agent guidance.
 - Core data types: `Request`, `Response`, `Limits`, `Timeout`, `ClientConfig` — frozen+slotted dataclasses with `with_*` immutability helpers on `Request` and computed `text`/`json()` accessors on `Response` (Story 1.2).
 
-[Unreleased]: https://github.com/modern-python/httpware/compare/HEAD...HEAD
+[Unreleased]: https://github.com/modern-python/httpware/commits/main

README.md

@@ -22,6 +22,7 @@ Optional extras:
 ```bash
 pip install httpware[msgspec]    # msgspec ResponseDecoder
 pip install httpware[otel]       # OpenTelemetry instrumentation
+pip install httpware[niquests]   # niquests transport
 pip install httpware[all]        # all of the above
 ```
 

docs/deferred-work.md

@@ -0,0 +1,25 @@
+# Deferred Work
+
+Items raised in reviews that are real but not actionable now.
+
+## Deferred from: code review of story-1-2 (2026-05-13)
+
+- **Charset parser robustness** — quoted whitespace, mismatched quotes, multi-`charset=` directives, substring false-positives (e.g. `boundary` containing `charset=`). (`src/httpware/response.py:21-26`)
+- **Header name/value validation** — `with_header` accepts CR/LF (injection), `None`, empty string. Lands with header-handling story (2.3 or later). (`src/httpware/request.py:21-23`)
+- **URL validation** — `with_url("")` accepts empty; `base_url` has no trailing-slash normalization. (`src/httpware/request.py:25-27`, `src/httpware/config.py:27-33`)
+- **`with_query(None)` handling** — currently accepted and breaks downstream iteration. (`src/httpware/request.py:33-35`)
+- **`Timeout` / `Limits` negative-value validation** — no `__post_init__` guard; nonsensical values silently accepted. (`src/httpware/config.py:10-22`)
+- **Multi-valued query params** — `Mapping[str, str]` cannot express `?tag=a&tag=b`. Type widening needed. (`src/httpware/request.py:8`)
+- **Streaming / async-iterable request bodies** — `body: bytes | None` only. Revisit in transport stories. (`src/httpware/request.py:11`)
+- **`with_headers` / `with_cookie` / `with_extension` merge helpers** — only `with_header` (single) and `with_query` (replace) exist. Story 2.3 will fill this in. (`src/httpware/request.py:20-35`)
+- **`Response.json()` honor declared charset** — `json.loads(bytes)` auto-detects only UTF-8/16/32. Real APIs vary. (`src/httpware/response.py:44-45`)
+- **`@final` to prevent subclassing** — frozen+slots subclassing is fragile. No current subclasser; defer until needed. (`src/httpware/request.py`, `response.py`, `config.py`)
+
+## Deferred from: code review of story-1-1 (2026-05-13)
+
+- **Codecov upload fails on fork PRs** — fork PRs cannot access `CODECOV_TOKEN`; matches modern-di pattern, accepted tradeoff. (`.github/workflows/ci.yml:46-52`)
+- **`just publish` lacks env-var validation** — recipe assumes `GITHUB_REF_NAME` and `PYPI_TOKEN` are set; running locally could corrupt the version. Add `test -n "$GITHUB_REF_NAME"` guard before release work. (`Justfile:25-29`)
+- **`uv_build>=0.11,<0.12` narrow window** — single-minor band will expire as soon as uv_build 0.12 ships; bump when that happens. (`pyproject.toml:54`)
+- **Python 3.14 wheel availability risk** — `httpx2` / `pydantic` / `uv_build` may not have 3.14 wheels yet, breaking the matrix entry. Watch CI red on 3.14. (`.github/workflows/ci.yml:30-33`)
+- **Unpinned `ruff`/`ty` with `select=["ALL"]`** — any new ruff release adds rules and can break CI overnight. Pin major versions or pin specific rules when a regression occurs. (`pyproject.toml:70-72, 84-85`)
+- **No `[test]` extra; CI uses `--all-extras`** — future heavy extras will be installed in every CI run. Declare a `test` extra and switch CI to `--extra test`. (`pyproject.toml:35-47`)

docs/stories/1-1-project-scaffold-and-tooling.md

@@ -3,7 +3,7 @@ story_key: 1-1-project-scaffold-and-tooling
 epic: 1
 story: 1
 title: Project scaffold and tooling
-status: review
+status: done
 created: 2026-05-12
 completed: 2026-05-13
 input_documents:
@@ -168,7 +168,7 @@ input_documents:
 
 ## File List
 
-Files added (15 total):
+Files added (14 total):
 
 - `.github/workflows/ci.yml` — CI workflow (ruff/ty/pytest, Python 3.11-3.14 matrix)
 - `.gitignore` — modern-di convention + project-specific ignores
@@ -201,4 +201,25 @@ Generated/transient (not committed):
 
 ## Status
 
-`review`
+`done`
+
+### Review Findings
+
+_Code review run: 2026-05-13. Reviewers: Blind Hunter, Edge Case Hunter, Acceptance Auditor. 6 patches applied, 3 dismissed by maintainer (not errors), 6 deferred, 20 dismissed as noise (2 decision-needed items were resolved → dismissed: `.gitignore plan.md` blacklist is intentional convention; AC6 lint-on-single-version matches modern-di canon and is accepted)._
+
+- [x] [Review][Patch] CHANGELOG declared `httpx2>=2.0.0b1,<3.0` while pyproject shipped `>=2.0.0,<3.0`. [`CHANGELOG.md:13`] — applied
+- [x] [Review][Patch] CHANGELOG `[Unreleased]` link was `compare/HEAD...HEAD` — replaced with `commits/main` until first tag. [`CHANGELOG.md:20`] — applied
+- [x] [Review][Dismissed] `version = "0"` placeholder. [`pyproject.toml:29`] — dismissed by maintainer, not an error
+- [x] [Review][Patch] `[all]` extra refactored to self-reference siblings: `all = ["httpware[msgspec,otel,niquests]"]`. [`pyproject.toml:42-47`] — applied
+- [x] [Review][Patch] README "Optional extras" snippet — added `pip install httpware[niquests]`. [`README.md:22-26`] — applied
+- [x] [Review][Dismissed] CI `timeout-minutes`. [`.github/workflows/ci.yml:14, 26`] — dismissed by maintainer, not an error
+- [x] [Review][Dismissed] CI explicit `permissions:` block. [`.github/workflows/ci.yml:1-12`] — dismissed by maintainer, not an error
+- [x] [Review][Patch] Duplicate `--cov` flag — dropped `--cov=src/httpware` from CI invocation; `addopts` is now the single source of `--cov` source. [`.github/workflows/ci.yml:45`] — applied
+- [x] [Review][Patch] File List header `15 total` → `14 total`. [`docs/stories/1-1-project-scaffold-and-tooling.md:171`] — applied
+
+- [x] [Review][Defer] Codecov upload fails on fork PRs without `CODECOV_TOKEN`; matches modern-di pattern, accepted tradeoff. [`.github/workflows/ci.yml:46-52`] — deferred, pre-existing
+- [x] [Review][Defer] `just publish` does not validate `GITHUB_REF_NAME` / `PYPI_TOKEN`; local invocation could corrupt the version. [`Justfile:25-29`] — deferred, release-flow hygiene
+- [x] [Review][Defer] `uv_build>=0.11,<0.12` is a one-minor-version window that will expire fast. [`pyproject.toml:54`] — deferred, will bump on release
+- [x] [Review][Defer] Python 3.14 in CI matrix; httpx2 / pydantic / uv_build wheels may not yet exist on 3.14, causing the matrix entry to fail. [`.github/workflows/ci.yml:30-33`] — deferred, wait-and-see
+- [x] [Review][Defer] `[tool.ruff.lint] select = ["ALL"]` paired with unpinned `ruff`/`ty` — any new ruff release adds rules and breaks CI overnight. [`pyproject.toml:70-72, 84-85`] — deferred, matches modern-di
+- [x] [Review][Defer] No `[test]` extra declared; CI relies on `--all-extras`, so any future heavy extra is pulled into every CI run. [`pyproject.toml:35-47`] — deferred, scope creep concern

docs/stories/1-2-core-data-types.md

@@ -3,7 +3,7 @@ story_key: 1-2-core-data-types
 epic: 1
 story: 2
 title: Core data types
-status: ready-for-dev
+status: done
 created: 2026-05-13
 input_documents:
   - docs/prd.md
@@ -197,7 +197,7 @@ Followed TDD per module: write failing tests → confirm collection failure (RED
 - **AC5** ✓ `Timeout`, `Limits`, `ClientConfig` in `src/httpware/config.py` are all `@dataclass(frozen=True, slots=True)`. Defaults exactly match the spec. `ClientConfig.timeout` and `.limits` use `field(default_factory=Timeout)` / `field(default_factory=Limits)` — never bare instances. `Redactor` is intentionally not present (Story 5.3).
 - **AC6** ✓ `src/httpware/__init__.py` re-exports all five via explicit absolute imports; `__all__` is the sorted list `["ClientConfig", "Limits", "Request", "Response", "Timeout"]`. Smoke import succeeds.
 - **AC7** ✓ `just lint-ci` reports zero diagnostics across `eof-fixer`, `ruff format --check`, `ruff check --no-fix`, `ty check`.
-- **AC8** ✓ 24 tests cover every documented case: frozen on all 4 dataclasses, `with_*` immutability + new-instance identity + replace-semantics on `with_header`, equality, independent default mappings, charset case-insensitivity (parametrized over three casings), UTF-8 / latin-1 / missing-charset paths, JSON round-trip, default constructor returns documented defaults. `pytest` exits 0; coverage 100%.
+- **AC8** ✓ 24 tests cover every documented case: frozen on all 5 dataclasses, `with_*` immutability + new-instance identity + replace-semantics on `with_header`, equality, independent default mappings, charset case-insensitivity (parametrized over three casings), UTF-8 / latin-1 / missing-charset paths, JSON round-trip, default constructor returns documented defaults. `pytest` exits 0; coverage 100%.
 
 **Deviations from the story spec (worth flagging for reviewer):**
 
@@ -229,4 +229,26 @@ Followed TDD per module: write failing tests → confirm collection failure (RED
 
 ## Status
 
-`review`
+`done`
+
+### Review Findings
+
+_Code review run: 2026-05-13. Reviewers: Blind Hunter, Edge Case Hunter, Acceptance Auditor. Acceptance Auditor reported **no AC violations**. 5 patches applied, 11 deferred, 36 dismissed as noise. Post-patch verification: `just lint-ci` clean, 27 tests pass (+3 new), 100% coverage retained._
+
+- [x] [Review][Patch] `Response.text` now wraps `bytes.decode` in `try/except LookupError` and falls back to UTF-8 when the declared charset is unknown. [`src/httpware/response.py:42-46`] — applied
+- [x] [Review][Patch] `Response` class docstring now documents `elapsed` as wall-clock seconds from request send to response receipt. [`src/httpware/response.py:29-32`] — applied
+- [x] [Review][Patch] Added `test_response_text_strips_quotes_around_charset` (parametrized over `"latin-1"` and `'latin-1'`) covering the quote-stripping branch. [`tests/test_response.py`] — applied
+- [x] [Review][Patch] Equality tests now also assert `!=` against per-field variants (Request and Response). [`tests/test_request.py`, `tests/test_response.py`] — applied
+- [x] [Review][Patch] Completion Notes AC8 — "frozen on all 4 dataclasses" → "all 5". [`docs/stories/1-2-core-data-types.md:200`] — applied
+
+- [x] [Review][Defer] Charset parser robustness — whitespace inside quotes, mismatched quotes, multiple `charset=` directives, `charset=` substring inside a quoted boundary param. [`src/httpware/response.py:21-26`] — deferred, pragmatic v0; revisit when transport tests reveal real-world breakage
+- [x] [Review][Defer] Header name/value validation (CR/LF injection, `None`, empty string) on `Request.with_header`. [`src/httpware/request.py:21-23`] — deferred to header-handling story (2.3 or later)
+- [x] [Review][Defer] URL validation — `with_url("")` accepts empty, `base_url` has no trailing-slash normalization. [`src/httpware/request.py:25-27`, `src/httpware/config.py:27-33`] — deferred, lands when transport composes URLs
+- [x] [Review][Defer] `with_query(None)` is currently accepted and breaks downstream iteration. [`src/httpware/request.py:33-35`] — deferred, type system already says `Mapping[str, str]`
+- [x] [Review][Defer] `Timeout` / `Limits` accept negative or zero values silently (no `__post_init__`). [`src/httpware/config.py:10-22`] — deferred, validation lands with transport integration
+- [x] [Review][Defer] `params: Mapping[str, str]` cannot express multi-valued query strings (`?tag=a&tag=b`). [`src/httpware/request.py:8`] — deferred, type widening tracked separately
+- [x] [Review][Defer] `body: bytes | None` precludes streaming / async-iterable bodies. [`src/httpware/request.py:11`] — deferred, intentional minimal scope; revisit in transport stories
+- [x] [Review][Defer] No `with_headers` / `with_cookie` / `with_extension` merge helpers; only `with_header` (single) and `with_query` (replace). [`src/httpware/request.py:20-35`] — deferred to Story 2.3 (merge/case-insensitive helpers)
+- [x] [Review][Defer] `Response.json()` ignores declared charset; `json.loads(bytes)` auto-detects only UTF-8/16/32 by BOM. [`src/httpware/response.py:44-45`] — deferred, most APIs are UTF-8; revisit if a real backend breaks
+- [x] [Review][Defer] `Request.body` / `Response.content` (bytes) render verbatim in the default dataclass `__repr__`, leaking large payloads / secrets into logs. [`src/httpware/request.py`, `src/httpware/response.py`] — deferred to Story 5.3 (`Redactor`)
+- [x] [Review][Defer] No `@final` decorator — frozen+slots subclassing is fragile and the spec doesn't bless it; an explicit `@final` would prevent the footgun. [`src/httpware/request.py`, `src/httpware/response.py`, `src/httpware/config.py`] — deferred, no current subclasser

pyproject.toml

@@ -39,12 +39,7 @@ otel = [
     "opentelemetry-sdk>=1.20",
 ]
 niquests = ["niquests>=3.18"]
-all = [
-    "msgspec>=0.18",
-    "opentelemetry-api>=1.20",
-    "opentelemetry-sdk>=1.20",
-    "niquests>=3.18",
-]
+all = ["httpware[msgspec,otel,niquests]"]
 
 [project.urls]
 repository = "https://github.com/modern-python/httpware"

src/httpware/response.py

@@ -26,7 +26,10 @@ def _parse_charset(content_type: str) -> str | None:
 
 @dataclass(frozen=True, slots=True)
 class Response:
-    """Immutable HTTP response value type."""
+    """Immutable HTTP response value type.
+
+    `elapsed` is wall-clock seconds from request send to response receipt.
+    """
 
     status: int
     headers: Mapping[str, str]
@@ -38,7 +41,10 @@ class Response:
     def text(self) -> str:
         """Decode `content` using the response's declared charset (default UTF-8)."""
         charset = _parse_charset(_get_content_type(self.headers)) or "utf-8"
-        return self.content.decode(charset)
+        try:
+            return self.content.decode(charset)
+        except LookupError:
+            return self.content.decode("utf-8")
 
     def json(self) -> Any:  # noqa: ANN401
         """Parse `content` as JSON."""

tests/test_request.py

@@ -28,6 +28,9 @@ def test_request_equality_on_identical_fields() -> None:
     r1 = Request(method="GET", url="/x", headers={"a": "1"})
     r2 = Request(method="GET", url="/x", headers={"a": "1"})
     assert r1 == r2
+    assert r1 != Request(method="POST", url="/x", headers={"a": "1"})
+    assert r1 != Request(method="GET", url="/y", headers={"a": "1"})
+    assert r1 != Request(method="GET", url="/x", headers={"a": "2"})
 
 
 def test_with_header_adds_when_absent() -> None:

tests/test_response.py

@@ -48,6 +48,36 @@ def test_response_text_falls_back_to_utf8_on_missing_charset() -> None:
     assert resp.text == '{"x": 1}'
 
 
+@pytest.mark.parametrize(
+    "content_type",
+    [
+        'text/plain; charset="latin-1"',
+        "text/plain; charset='latin-1'",
+    ],
+)
+def test_response_text_strips_quotes_around_charset(content_type: str) -> None:
+    body = "café".encode("latin-1")
+    resp = Response(
+        status=200,
+        headers={"content-type": content_type},
+        content=body,
+        url="/",
+        elapsed=0.0,
+    )
+    assert resp.text == "café"
+
+
+def test_response_text_falls_back_to_utf8_on_unknown_charset() -> None:
+    resp = Response(
+        status=200,
+        headers={"content-type": "text/plain; charset=not-a-real-codec"},
+        content=b"hello",
+        url="/",
+        elapsed=0.0,
+    )
+    assert resp.text == "hello"
+
+
 def test_response_json_parses_body() -> None:
     resp = Response(status=200, headers={}, content=b'{"a": 1, "b": [2, 3]}', url="/", elapsed=0.0)
     assert resp.json() == {"a": 1, "b": [2, 3]}
@@ -57,3 +87,5 @@ def test_response_equality_on_identical_fields() -> None:
     r1 = Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5)
     r2 = Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5)
     assert r1 == r2
+    assert r1 != Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.6)
+    assert r1 != Response(status=201, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5)