Update instructions - Part 2, phase 3

.github/instructions/providers.instructions.md

@@ -0,0 +1,120 @@
+---
+description: Provider adapter rules for evidence capture, secret safety, and client lifecycle.
+applyTo: 'src/transcription/providers/**/*.py'
+---
+
+# Provider Adapters
+
+Primary references:
+
+- `docs/invariant/ai_evidence_and_provenance.md` (canonical; provider adapters own provider-boundary evidence capture)
+- `docs/architecture.md`
+- `docs/schema.md`
+
+The provider layer is where an external API becomes application data. It is also the only place
+that can capture what actually crossed the wire — once a response reaches a service, the evidence
+it did not preserve is gone permanently. Treat capture correctness as the primary job of this
+layer and text extraction as secondary.
+
+## Layer Boundary
+
+- Adapters may depend on `transcription.config`, `transcription.providers.*`, the HTTP client, and
+  the provider SDK. They must not import `services`, `db`, `ui`, or `api`.
+- Provider specifics — headers, model slugs, payload shapes, SDK types, error classes — stop here.
+  Callers receive `TranscriptionResult` and `ProviderError` subclasses only.
+- Adapters raise provider/domain exceptions. They must not emit user-facing text, notifications,
+  or remediation wording; that translation belongs to services and UI. See
+  [error-handling instructions](./error-handling.instructions.md).
+- Adapters do not persist. They return evidence; services decide what is written and when.
+
+Enforced by `tests/test_provider_boundaries.py`.
+
+## Contract Surface
+
+- Every adapter satisfies the `TranscriptionProvider` protocol in `base.py`, including
+  `current_request_manifest` and `current_transport_evidence`, which exist so a *failed* call still
+  yields evidence.
+- `TranscriptionResult`, `RequestManifest`, and `TransportEvidence` are `extra="forbid"` and frozen.
+  Add a field to the contract rather than smuggling data through an untyped dict.
+- Evidence contracts in `evidence.py` are versioned (`schema_name` + `schema_version`). A change to
+  the meaning or shape of a captured field requires a version bump, not a silent redefinition —
+  stored evidence must keep its original meaning.
+
+## Transport Evidence
+
+The rules below implement `docs/invariant/ai_evidence_and_provenance.md` §3.4-3.5. That document
+wins if this file drifts from it.
+
+- Capture the response body **at the HTTP boundary, before SDK parsing**, so fields the SDK does
+  not model are not lost. `_CapturingAsyncClient` exists for this; do not replace it with a
+  post-parse `model_dump()` and call the result transport evidence.
+- Reset per-call capture state at the start of every call. Without it, a connection failure can
+  attach the *previous* call's response as evidence for this one. Guarded by
+  `tests/test_v42_evidence.py::test_openrouter_does_not_reuse_prior_response_on_connection_failure`.
+- Handle the streamed-body case (`httpx.ResponseNotRead`) rather than assuming `response.content`
+  is always available.
+- When no response arrives — timeout, DNS, connection reset — emit
+  `TransportEvidence(response_received=False)`. Absence of a response is itself evidence and must
+  be explicit, never an empty body or a missing record.
+- Preserve safe response evidence for **unsuccessful** calls too, whenever a response was received.
+- Never relabel an SDK snapshot or normalized metadata as transport evidence, and never backfill
+  it into an execution that predates capture.
+
+## Secret Safety
+
+- Persist response headers only through `filter_safe_response_headers` and the
+  `SAFE_RESPONSE_HEADERS` allowlist. Allowlist, never denylist: capture-then-redact is prohibited,
+  because an unknown header is unsafe by default.
+- Adding a header to the allowlist is a deliberate evidence decision. Confirm it carries no
+  credential, cookie, or session material, and state why it is needed for correlation, content
+  interpretation, rate-limit diagnosis, or audit.
+- API keys, `Authorization`, and cookies must never appear in a manifest, evidence record, log
+  line, or exception message.
+- The request manifest references source content by identity (digest, size, media type, page).
+  Do not duplicate base64 source bytes into it — `_replace_embedded_media` exists for this.
+
+## Execution Specification
+
+The manifest must let a reader reconstruct what was asked, per invariant §3.3:
+
+- Provider, requested model, full effective prompt text, and prompt digest.
+- Every explicitly supplied parameter, and — separately — which optional parameters were
+  **omitted**. Omission is not the same as a null value or an assumed provider default; the
+  `optional_parameter_states` distinction between `omitted`, `null`, and `value` is deliberate.
+- Timeout budget, retry policy, source reference, and `SoftwareContext` versions.
+- Manifest digests use `canonical_json_bytes`. Do not hash a plain `json.dumps()`; key order and
+  separators must stay deterministic or digests become uncomparable.
+
+## Client Lifecycle and Async Safety
+
+- Reuse one pooled `AsyncClient` per adapter instance; do not construct a client per request.
+- Accept an injected client so tests can drive the adapter without network access.
+- Derive timeouts from `Settings` (`worker_provider_timeout_seconds`) rather than hard-coding, and
+  keep the client timeout aligned with the configured budget so the SDK cannot expire first and
+  hide the real failure.
+- Implement `aclose()` and release pooled resources. An adapter that creates a client owns closing
+  it; one given a client must not close a caller-owned resource it did not create.
+- Never block the event loop. Offload CPU-bound work (hashing large payloads, image encoding) with
+  `asyncio.to_thread`.
+- Propagate `asyncio.CancelledError` untouched — do not convert cancellation into a provider error.
+
+## Failure Handling
+
+- Raise `ProviderAuthError` for authentication, `ProviderResponseError` for malformed or unusable
+  responses, and `ProviderError` otherwise.
+- Always attach `request_manifest`, `transport_evidence`, and an accurate `failure_phase` to raised
+  errors. `failure_phase` must distinguish a received-but-failed response from a call that never
+  reached the provider.
+- Validate responses with Pydantic rather than indexing into raw dicts.
+- Invalid *optional* metadata (for example unparsable token counts) must not discard an otherwise
+  valid transcript. Degrade the metadata, not the result.
+
+## Contract Sync Rule
+
+If capture behavior, evidence schema, or the header allowlist changes:
+
+1. Update `docs/invariant/ai_evidence_and_provenance.md` only if the durable preservation contract
+   itself is changing — that revision is deliberate and reviewed, not incidental.
+2. Update `docs/schema.md` when persisted evidence fields change.
+3. Update or add tests in the same change (`tests/providers/`, `tests/test_v42_evidence.py`).
+4. Bump the affected evidence `schema_version` when a field's meaning changes.

tests/test_meta_contract_guards.py

@@ -18,6 +18,7 @@ ACTIVE_CONTRACT_FILES = (
     ".github/instructions/services.instructions.md",
     ".github/instructions/ui.instructions.md",
     ".github/instructions/error-handling.instructions.md",
+    ".github/instructions/providers.instructions.md",
     ".github/skills/python-code-reviewer/skill.md",
     ".github/skills/evidence-provenance-auditor/skill.md",
     ".github/skills/test-effectiveness-auditor/skill.md",
@@ -237,6 +238,12 @@ def test_canonical_authority_references_are_present():
             "docs/error_handling.md",
             "docs/requirements.md",
         ),
+        ".github/instructions/providers.instructions.md": (
+            "docs/invariant/ai_evidence_and_provenance.md",
+            "docs/schema.md",
+            "tests/test_provider_boundaries.py",
+            "SAFE_RESPONSE_HEADERS",
+        ),
         ".github/skills/python-code-reviewer/skill.md": (
             "docs/*",
             "docs/schema.md",

tests/test_provider_boundaries.py

@@ -0,0 +1,74 @@
+"""Structural rules for the providers package.
+
+`.github/instructions/providers.instructions.md` requires provider specifics to stop at the
+adapter boundary: adapters translate an external API into `TranscriptionResult` and
+`ProviderError`, and know nothing about persistence, services, or the UI. Without this guard the
+rule is only advice, and a single convenience import of a service or model would invert the
+dependency direction the architecture depends on.
+"""
+
+from __future__ import annotations
+
+import ast
+from pathlib import Path
+
+PROVIDERS_DIR = Path(__file__).resolve().parents[1] / "src" / "transcription" / "providers"
+
+# Application packages an adapter must never reach into. `config` is intentionally absent:
+# adapters read Settings for timeouts and credentials.
+FORBIDDEN_PACKAGES = frozenset({"services", "db", "ui", "api", "worker", "worker_service"})
+
+
+def _module_paths() -> list[Path]:
+    return sorted(PROVIDERS_DIR.glob("*.py"))
+
+
+def _imported_application_packages(tree: ast.Module) -> set[str]:
+    """Return first-level `transcription.<package>` names imported by this module."""
+    imported: set[str] = set()
+    for node in ast.walk(tree):
+        if isinstance(node, ast.ImportFrom) and node.module:
+            parts = node.module.split(".")
+            if node.level == 0 and parts[0] == "transcription" and len(parts) > 1:
+                imported.add(parts[1])
+        elif isinstance(node, ast.Import):
+            for alias in node.names:
+                parts = alias.name.split(".")
+                if parts[0] == "transcription" and len(parts) > 1:
+                    imported.add(parts[1])
+    return imported
+
+
+def test_provider_modules_are_discovered():
+    """Guard the guard: the rules below are meaningless if nothing is scanned."""
+    assert {path.stem for path in _module_paths()} >= {"base", "evidence", "openrouter"}
+
+
+def test_provider_modules_do_not_import_application_layers():
+    """A provider adapter must not depend on services, persistence, UI, API, or the worker."""
+    violations: dict[str, list[str]] = {}
+    for path in _module_paths():
+        tree = ast.parse(path.read_text(encoding="utf-8"))
+        found = sorted(_imported_application_packages(tree) & FORBIDDEN_PACKAGES)
+        if found:
+            violations[path.name] = found
+    assert violations == {}
+
+
+def test_response_headers_are_filtered_through_the_allowlist():
+    """Header persistence must be allowlist-based, not capture-then-redact."""
+    from transcription.providers.evidence import SAFE_RESPONSE_HEADERS
+    from transcription.providers.evidence import filter_safe_response_headers
+
+    filtered = filter_safe_response_headers(
+        {
+            "Content-Type": "application/json",
+            "Authorization": "Bearer super-secret",
+            "Set-Cookie": "session=super-secret",
+            "X-Unknown-Future-Header": "unreviewed",
+        }
+    )
+
+    assert filtered == {"content-type": "application/json"}
+    assert "authorization" not in SAFE_RESPONSE_HEADERS
+    assert "set-cookie" not in SAFE_RESPONSE_HEADERS