fix: review reported "approve, risk 0" on documents that score "block"

AGENTS.md

@@ -5,7 +5,23 @@ Drive `nda-review-cli` from an LLM agent or non-interactive client. Same three-f
 ## Output contract
 
 - **Success**: structured JSON to **stdout**, exit `0`. Reviews emit `{review: {findings: [...], decision, scores}}`; negotiation commands emit the new state file (or a diff of it).
-- **Failure**: `{ok: false, error: {code, message, details?}}` to **stderr**, non-zero exit. Stable error `code` values include `POLICY_INVALID`, `STATE_HASH_MISMATCH`, `MISSING_PLAYBOOK`, `LLM_DECLINED`, `STALEMATE_DETECTED`.
+- **Failure**: `{ok: false, error: {code, message, details?}}` to **stderr**, non-zero exit. Stable error `code` values include `POLICY_INVALID`, `STATE_HASH_MISMATCH`, `MISSING_PLAYBOOK`, `LLM_DECLINED`, `STALEMATE_DETECTED`, `EMPTY_DOCUMENT`, `PLAYBOOK_NO_RULES`.
+
+### Never trust `approve` without reading `coverage`
+
+Every review carries a `coverage` block. `decision: "approve"` with `coverage.rules_matched: 0` would mean *nothing was checked*, not *nothing was wrong* — so the CLI never emits that combination. It returns `needs_review` instead.
+
+```json
+"decision": "approve",
+"coverage": {"rules_evaluated": 11, "rules_matched": 6, "extraction_status": "ok:docx-xml", "text_chars": 4213}
+```
+
+| `decision` | Means |
+|---|---|
+| `approve` / `escalate` / `block` | Rules ran, matched, and scored. |
+| `needs_review` | Rules ran, **nothing matched**. The document is probably not an NDA, or its text failed to extract. Escalate to a human; don't treat it as clean. |
+
+An empty document (`EMPTY_DOCUMENT`) and a rule-less playbook (`PLAYBOOK_NO_RULES`) are hard failures with exit `2`, not decisions. Don't paper over them by retrying with a different playbook until one returns `approve`.
 - Human-readable summaries also go to **stderr** by default. Suppress them with `NDA_CLI_QUIET=1` for clean pipelines.
 
 ## Exit codes
@@ -50,6 +66,9 @@ The state file format is hash-chained: any tampering breaks `negotiate validate`
 | `MISSING_SIGNER_POLICY` | No `config/signer-policy.json` and no `--signer-policy` | Expected when nobody has granted signing authority. Fall back to human sign-off; don't author a permissive policy to unblock yourself. |
 | `STALEMATE_DETECTED` | `negotiate analyze --state <path>` for the stuck clauses | Surface the `block_diagnosis` to a human. Don't auto-resolve; the stalemate exists for a reason. |
 | Counterparty profile unknown | `nda-review-cli profile-learn --counterparty <name> --review-json <path>` | One-shot learning from a saved review. |
+| `decision: needs_review` | Read `coverage.rules_matched` (0) and `coverage.extraction_status` | Rules ran but matched nothing. Either the file isn't an NDA, or extraction failed. Hand it to a human — never re-run until something returns `approve`. |
+| `EMPTY_DOCUMENT` | `details.extraction_status` + `details.extractors_tried` | Extraction produced no text. For `.pdf` install `pdftotext`; for `.docx` confirm the file isn't corrupt. Don't fall back to reading it as plain text. |
+| `PLAYBOOK_NO_RULES` | `nda-review-cli policy-validate --file <path>` | The playbook has no keyword-bearing rules. Rebuild with `build-playbook`, or point `--playbook` at a real policy file. |
 
 ## Recommended defaults
 
@@ -111,11 +130,16 @@ Note the default `allowed_amendment_sources` excludes `agent`, so a round *you*
 ```python
 from nda_review_cli import review_nda, load_policy
 
-policy = load_policy("config/org-policy.json")
-result = review_nda(text=nda_text, policy=policy, learn_profile=False)
-# → { "findings": [...], "decision": "escalate", "scores": {...} }
+policy = load_policy("config/org-policy.json")   # or config/default-policy.json, or a built playbook
+result = review_nda(text=nda_text, policy=policy)
+# → { "decision": "escalate", "risk_score": 7.2, "findings": [...],
+#     "coverage": {"rules_evaluated": 11, "rules_matched": 6, ...} }
 ```
 
+`load_policy` accepts either playbook shape (a `clause_rules` object or a built `policy` list). `review_nda` raises `EmptyDocumentError` on blank text and `PlaybookError` when the policy yields no keyword-bearing rules — both are cases where a silent `approve` would be a lie. Check `result["coverage"]` before trusting `result["decision"]`.
+
+To read a `.docx` / `.pdf`, use `read_nda_input(path)` → `(text, extraction_status)`; reading those with `Path.read_text` yields zip/binary noise that no keyword can match.
+
 Stdlib-only at runtime. LLM augmentation requires only `urllib`; no `anthropic` / `openai` SDK dependency.
 
 ## See also

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [Unreleased]
 
 ### Fixed
+
 - **Red-flag detection was inert on every negotiation path.** `rule_engine.red_flag_hits(clause, text)`
   takes the clause *key* first, but three call sites passed the clause *text* first, so the
   `RED_FLAG_PATTERNS` lookup always missed and the function always returned `[]`. Consequences,
@@ -21,7 +22,41 @@
   "Could not auto-detect which party you are" because it never set `org_name` on the two
   workspaces — the CI job does, so the drift went unnoticed. The Makefile now mirrors CI.
 
+**`review` reported `approve, risk_score 0, findings []` on documents that score `block`.**
+Two independent defects each zeroed out the review, and the fail-open design reported
+"nothing was evaluated" as "nothing was wrong."
+
+- **The playbook schema mismatch was silent.** `review_text` iterated `playbook["policy"]`,
+  a list. But `config/default-policy.json`, `config/org-policy.json`, and everything
+  `quickstart` writes key their rules under `clause_rules`, a dict. Such a file has no
+  `policy` key, so the rule loop never executed — no error, no warning. Both shapes are now
+  normalized by `normalize_playbook_rules()`.
+
+  This hit the documented happy path: **`demo`** (the zero-config first-run command) and the
+  README's own `review --playbook config/default-policy.json` both returned `approve / 0 / 0`
+  on the bundled sample NDA, which actually scores `block` at risk 17.4 with 6 findings.
+
+- **`review` read `.docx` as raw zip bytes.** `cmd_review` used `_read_text_file`, a plain
+  `Path.read_text`, so a `.docx` arrived as `PK\x03\x04...` and no keyword could ever match.
+  The correct reader — `_read_any_text`, which routes `.docx`/`.pdf` to real extractors and
+  returns an `extraction_status` — already existed and simply wasn't called. `review` and
+  `calibrate-scoring` now use it.
+
+- **`negotiate review` was vacuous too.** With no built playbook it hand-rolled a fallback
+  `{"policy": [...]}` whose entries carried `clause` and `preferred` but **no `keywords`**, and
+  `clause_hit(text, [])` is always False. It now passes the org policy through the normalizer,
+  and reviews the current round for real (9 findings on the bundled mutual template).
+
+**Why 113 green tests sat on top of this:** `tests/test_review_golden.py` and
+`test_review_explainability.py` converted `clause_rules` into a `policy` list *themselves*
+before calling `review_text` — performing exactly the step the CLI was missing. The engine was
+well tested; the wiring that feeds it never was. The calibration fixture went further and
+encoded the bug as expected behaviour: its `approve` case was `{"text": "", "expected_decision":
+"approve"}` — an empty document. Tests now drive the real CLI with an unconverted policy file,
+and the fixture's `approve` case is a genuinely clean NDA.
+
 ### Added
+
 - **`signer policy` — declarative signing authority.** A deterministic, read-only gate answering
   whether a converged negotiation may be signed off without a human. Previously documented in
   `AGENTS.md` as future scope.
@@ -52,6 +87,28 @@
   - New: `config/signer-policy.json.example`, `docs/reference/signer-policy.md`, and
     `tests/test_signer_policy.py` (46 tests).
 
+- **Fail-closed review contract.** A review can no longer report `approve` for a document it
+  never inspected.
+  - Every result carries a **`coverage`** block: `rules_evaluated`, `rules_matched`,
+    `extraction_status`, `text_chars`. `risk_score: 0` with `rules_matched: 0` is now
+    self-evidently "nothing was checked" rather than "nothing was wrong."
+  - New decision **`needs_review`**, emitted when rules ran but matched nothing on a non-empty
+    document. Any real NDA trips at least one clause keyword, so zero matches means the text is
+    not an NDA or extraction mangled it. `normalize_decision_bucket` and the calibration
+    confusion matrix learned the new bucket (both now derive from `DECISION_BUCKETS`).
+  - New hard failures, exit `2`: **`EMPTY_DOCUMENT`** (blank text or failed extraction, with
+    `extraction_status` + `extractors_tried` in `details`) and **`PLAYBOOK_NO_RULES`** (no
+    keyword-bearing rules, so no clause could ever match). Missing input files keep their
+    existing plain-text `error: ...` wording and exit `4`.
+
+- **The documented library API now exists.** `AGENTS.md` told importers to
+  `from nda_review_cli import review_nda, load_policy` — neither function was defined. Both are
+  now real, alongside `read_nda_input(path) -> (text, extraction_status)` for `.docx`/`.pdf`.
+
+- `tests/test_review_coverage.py` (24 tests) and `tests/test_review_golden.py::ClauseRulesPlaybookTests`
+  (3 tests). 22 of the 24 fail against the previous source.
+
+
 ## [0.5.4] - 2026-06-03
 
 Security/robustness fixes from a follow-up source audit.

README.md

@@ -106,6 +106,7 @@ Examples below use the cloned form (`./nda_review_cli.py`) for portability; once
 | **Profile** | Per-counterparty memory: their typical positions, what they've conceded, what triggered escalation. Updated automatically with `--learn-profile`. | `profiles/<name>.json` |
 | **Playbook** | A snapshot built from your corpus + policy: clause-by-clause guidance the review engine consults. Rebuilt on demand. | `output/nda_playbook.json` (+ `.md`) |
 | **Stance** | How aggressively your agent negotiates: `conservative` / `middleground` / `compromising`. | `config/org-policy.json` `defaults.negotiation_stance`. See [docs/reference/stance.md](docs/reference/stance.md). |
+| **Coverage** | How much of the policy actually ran against this document. Guards against reading "nothing was checked" as "nothing was wrong". | The `coverage` block on every review. See [docs/reference/scoring.md](docs/reference/scoring.md). |
 | **Signer policy** | Declarative signing authority: which negotiation outcomes an agent may sign off on, and which escalate to a human. Deny-by-default. | `config/signer-policy.json` (see `.example`). See [docs/reference/signer-policy.md](docs/reference/signer-policy.md). |
 
 Rule of thumb: **edit the policy, let the profile learn, regenerate the playbook.**
@@ -121,6 +122,10 @@ Score any NDA against your policy. `--why` adds explainability evidence (matched
   --out-md output/reviews/review.md
 ```
 
+`--playbook` accepts either a built playbook (`output/nda_playbook.json`) or a policy file (`config/default-policy.json`, `config/org-policy.json`) — both review identically.
+
+Every result carries a `coverage` block (`rules_evaluated`, `rules_matched`, `extraction_status`, `text_chars`). **Read it before trusting the decision:** `risk_score: 0` with `rules_matched: 0` means nothing was checked, not that nothing was wrong. When rules run but match nothing, the decision is `needs_review`, never `approve`. An empty document or a playbook with no keyword-bearing rules is a hard failure (exit `2`), not an `approve`.
+
 For opt-in LLM-augmented review, see [docs/setup/](docs/setup/) — pick the provider (Anthropic, OpenAI, Ollama, OpenAI-compatible) and follow that file.
 
 ## Drafting

docs/reference/exit-codes.md

@@ -41,6 +41,8 @@ Structured outputs (review JSON, negotiation state, etc.) print to **stdout**, e
 | `POLICY_INVALID` | input | `2` | Policy file fails `policy-validate`. `details.path` + `details.reason`. |
 | `MISSING_PLAYBOOK` | not-found | `4` | No playbook on disk; run `setup --quick --yes` or `build-playbook`. |
 | `MISSING_INPUT_FILE` | not-found | `4` | The `--file <path>` doesn't exist. |
+| `EMPTY_DOCUMENT` | input | `2` | The document is empty, or text extraction produced nothing (`details.extraction_status`, `details.extractors_tried`). Reviewing it would report `approve` for the wrong reason. |
+| `PLAYBOOK_NO_RULES` | input | `2` | The playbook contributes no keyword-bearing rules, so no clause could ever match. Rebuild with `build-playbook`, or check the file's shape. |
 | `STATE_HASH_MISMATCH` | policy | `3` | Negotiation state file is tampered or corrupted. `details.brokenRound` identifies the first break. |
 | `STALEMATE_DETECTED` | policy | `3` | `negotiate counter` after `stalemate_threshold` rounds with no progress. `details.stuckClauses` lists them. |
 | `NON_NEGOTIABLE_CONFLICT` | policy | `3` | Both parties' non-negotiables overlap with conflicting text. Surfaces in `block_diagnosis`. |

docs/reference/policy.md

@@ -50,6 +50,19 @@ Required top-level keys: `org_name`, `clause_rules`, `negotiation_signal_pattern
 
 Required per clause rule: `keywords`, `preferred`, `red_flags`.
 
+## Two shapes, one meaning
+
+`review --playbook <path>` accepts either shape, and reviews them identically:
+
+| Shape | Written by | Looks like |
+|---|---|---|
+| `clause_rules` object | `quickstart`, `init`, this file's schema | `{"clause_rules": {"term_length": {"keywords": [...], ...}}}` |
+| `policy` list | `build-playbook` → `output/nda_playbook.json` | `{"policy": [{"clause": "term_length", "keywords": [...], ...}]}` |
+
+Historically only the `policy` list was read, so pointing `--playbook` at a policy file evaluated zero rules and reported `approve` with `risk_score: 0`. Both shapes now normalize to the same rule list, and a playbook that yields no keyword-bearing rules is rejected outright (`PLAYBOOK_NO_RULES`, exit `2`) rather than reviewed vacuously.
+
+**`keywords` is what makes a rule real.** A rule without them can never match any document, so it contributes nothing to `coverage.rules_evaluated`.
+
 ## Per-clause fields
 
 | Field | Type | What it does |

docs/reference/scoring.md

@@ -48,6 +48,33 @@ For each review:
    - `total_score <= approve_max` → `approve`
    - `total_score <= escalate_max` → `escalate`
    - else → `block`
+   - **no rule matched at all** → `needs_review` (overrides the above)
+
+## `needs_review` and the coverage block
+
+Scores only mean something if rules actually ran. Every review therefore carries a `coverage` block:
+
+```json
+"coverage": {
+  "rules_evaluated": 11,
+  "rules_matched": 6,
+  "extraction_status": "ok:docx-xml",
+  "text_chars": 4213
+}
+```
+
+`rules_evaluated` is how many keyword-bearing rules the playbook contributed; `rules_matched` is how many found their clause in the document. A score of `0` with `rules_matched: 0` is not a clean NDA — it is a review that checked nothing.
+
+When `rules_matched == 0` on a non-empty document, the decision is **`needs_review`**, never `approve`. Every real NDA trips at least one clause keyword (`confidential information`, `term`, …), so zero matches means the text is not an NDA or extraction mangled it.
+
+Two conditions are hard failures rather than decisions, because no honest score exists for them:
+
+| Condition | Error code | Exit |
+|---|---|---|
+| Document empty, or extraction produced nothing | `EMPTY_DOCUMENT` | `2` |
+| Playbook contributes no keyword-bearing rules | `PLAYBOOK_NO_RULES` | `2` |
+
+A rule without `keywords` can never match — `clause_hit(text, [])` is always `False` — so it is not counted as coverage.
 
 ## Customizing