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

[DrBaher]

Fixes the defect report for v0.5.4: review_nda returned decision: approve, risk_score: 0, findings: [] for a document that scores block once its inputs are actually parsed.

All three reported bugs reproduce and are fixed. Investigation turned up three more of the same kind.

The two bugs that zero out the review

1. Playbook schema mismatch, silently ignored. 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.

2. 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 match. _read_any_text, which routes .docx/.pdf to real extractors and returns an extraction_status, already existed and simply wasn't called.

Same document, same rules, before → after:

Input	Playbook	Before	After
.txt	clause_rules	approve, 0, 0 findings	block, 17.4, 6 findings
.docx	clause_rules	approve, 0, 0 findings	block, 17.4, 6 findings
.docx	policy list	approve, 0, 0 findings	block, 17.4, 6 findings

Three more the report didn't cover

• demo was broken. The flagship zero-config first-run command returned approve / 0 / 0 on the bundled sample NDA. So did the exact review --playbook config/default-policy.json command in the README. A new user's first impression of this tool was a vacuous approve.
• negotiate review was vacuous on the same axis. 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 returns 9 findings on the bundled mutual template.
• The documented library API didn't exist. AGENTS.md tells importers to from nda_review_cli import review_nda, load_policy. Neither function was defined. Both are now real.

Fail-closed contract (report's Bug 3)

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, not nothing was wrong.
• New decision needs_review when rules ran but matched nothing on a non-empty document. Every real NDA trips at least one clause keyword (confidential information, term, …), so zero matches means the text isn't an NDA or extraction mangled it.
• New hard failures at exit 2: EMPTY_DOCUMENT (blank text or failed extraction, with extraction_status and 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 — introducing a JSON envelope for one command would have made the CLI inconsistent with itself.

needs_review is a fourth decision value. normalize_decision_bucket and the calibration confusion matrix both learned it, and now derive from a single DECISION_BUCKETS constant so they can't desync again.

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:

{ "id": "approve-empty", "text": "", "expected_decision": "approve" }

An empty document, expected to approve. That case is now a genuinely clean NDA (approve at risk 2.4, two clauses matched), plus a needs_review case. The tests now drive the real CLI with an unconverted policy file, and test_llm_review's keyword-less fixture — which was also reviewing nothing — was fixed.

Verification

• 140 tests pass, up from 113. The 27 new ones are regression guards: 22 of the 24 in test_review_coverage.py fail against the previous source, as do all 3 in ClauseRulesPlaybookTests. The rest are negative controls (a genuinely clean NDA must still approve; needs_review must not swallow it).
• Calibration now scores accuracy 1.0 across all four buckets.
• demo, tutorial, --catalog json, policy-validate, stdin (--file -) all verified working.

Note: make smoke's negotiate flow is broken on main for an unrelated reason (it never sets org_name). That's fixed in #14, not here.

Reported-by: baher@medicus.ai

🤖 Generated with Claude Code