fix(wiki): refuse a draft refinement whose value is empty (#585) - #586

ZETETIC-REVIEW: APPROVE

Summary

Fixes exactly the gap issue #585 describes. _content_refusal (mcp_server/handlers/wiki_refine.py:268-285) now refuses any wiki_refine_draft call where a present content field carries an empty value (blank/whitespace string, empty list, empty mapping), before anything is written. Verified against a real SQLite draft, not just by reading the diff.

Stakes: Medium (bug fix in a handler with 1 non-test file changed plus docs; not touching auth/billing/schema; changed function is called from one place).

1. Is _content_refusal correct for every shape a client can send?

Ran all five shapes from the task plus two more via the actual registered tool composition root (mcp_server/tool_registry_wiki_drafts.py:62-90, the tool's real argument type is sections: list[dict[str, Any]] | None, so the inputSchema's per-item required: [heading, body] is documentation only, per _tool_meta.apply_param_docs:104-126; it is not enforced, so a client really can send sections=[{}]):

• sections=[{}] (empty dict in the list): refused, but not by _content_refusal. The list is non-empty so _is_empty passes it through, and it is caught downstream by _validate_against_contract (wiki_refine.py:288-300), which flags "section None has empty body". Still refused before any write. Confirmed on SQLite (kind=adr): response was {'error': 'validation failed', 'validation_errors': [..., "section None has empty body"], ...}, draft's sections column unchanged.
• sections entries with blank headings but a real, non-blank body (for example [{"heading": " ", "body": "real prose"}]) against a kind with no required sections (kind=note, required_sections=[]): not refused. _content_refusal sees a non-empty list and passes; _validate_against_contract only checks that required headings are present and that each section's body is non-blank. It never checks that a section's own heading is non-blank. I drove this end to end on a real note draft: handler_refine({"draft_id": ..., "sections": [{"heading": " ", "body": "real prose, not blank"}]}) returned updated: True and overwrote the draft's sections with the blank heading. This is a real gap in the spirit of "blanking a field is not a refinement," but it is out of scope for this PR: ADR-1069's decision text is explicitly about the four top-level content fields being empty ("A content field present in the call must carry a value, where empty means a blank string, empty list, or empty mapping"), not about per-item shape inside sections. _validate_against_contract is unchanged in this diff, present before fix(wiki): refuse a draft refinement whose value is empty (#585) #586. Issue [bug] wiki_refine_draft writes an empty lead, title, sections or frontmatter over the draft #585 and the PR body only ever describe the four top-level shapes. I am not blocking on this; it is a non-blocking follow-up, not a defect in what this PR claims to fix.
• Frontmatter with a key whose value is empty ({"tags": ""}): accepted (updated: True), correctly. The field itself is non-empty, and ADR-1069 scopes "empty" to the field as a whole, not its nested values. Confirmed on SQLite.
• Title of a single character ("x"): accepted, correctly (not empty).
• Lead made only of a newline ("\n"): refused. .strip() reduces it to "". Confirmed: {'error': 'empty value for: lead...', 'updated': False}.

So for every shape ADR-1069 actually claims to cover, the four top-level fields, the code does what it claims. The one shape that slips through (blank heading, non-blank body, no-required-section kind) is a real but pre-existing and out-of-scope gap, flagged below as a follow-up, not blocking.

2. Does the refusal run before any write, on every path?

Yes, proved with spies (unittest.mock.patch.object on wr.update_draft and wr.insert_memo) against a real note draft, for all four _content_refusal shapes plus the sections=[{}] kind-contract-validation-failure path:

{'lead': ''}         -> updated: False, update_draft called: False, insert_memo called: False
{'title': '   '}     -> updated: False, update_draft called: False, insert_memo called: False
{'sections': []}     -> updated: False, update_draft called: False, insert_memo called: False
{'frontmatter': {}}  -> updated: False, update_draft called: False, insert_memo called: False
{'sections': [{}]}   -> updated: None,  update_draft called: False, insert_memo called: False

update_draft and insert_memo are only reached after both _content_refusal (wiki_refine.py:316-318) and, when sections is present, _validate_against_contract (wiki_refine.py:320-332) return clean. Confirmed by reading the control flow and by the spy run above.

3. Test strength (mutation)

• Mutated _is_empty to return False unconditionally: all 5 parametrized cases of test_a_refine_carrying_an_empty_value_writes_nothing failed (assert True is False on out["updated"]).
• Mutated _is_empty to drop .strip() (return not value for strings): the blank-title case (" ") failed; the other 4 still passed as expected (empty string, empty list, empty dict do not need .strip()).
• Restored both times with git checkout -- mcp_server/handlers/wiki_refine.py; git status --short is clean.

The suite kills both plausible mutants of the fix.

4. Gates (run from the worktree, UV_NO_SYNC=1)

• ruff check: All checks passed.
• ruff format --check: 1458 files already formatted.
• scripts/check_craftsmanship.py --base origin/main: OK.
• scripts/check_doc_claims.py: OK (1 declared exemption, unrelated to this PR).
• .venv/bin/python -m pyright mcp_server/: 0 errors, 0 warnings, 0 informations.
• pytest -q (full suite): 8680 passed, 8 skipped, 459 subtests passed in 265s. Matches the PR body's claim exactly.

5. Factual-claim accuracy

• draft_curator.py:116-118: elif not title.strip(): reasons.append("title is empty"); score -= 0.3. Matches the PR/ADR-1069 claim of minus 0.3 for an empty title, verbatim.
• draft_curator.py:79-82 _lead_is_placeholder: if not lead: return True, an empty lead reads as placeholder. Combined with draft_curator.py:121-123 (score -= 0.25), matches the claim of minus 0.25 for a blanked lead.
• draft_synthesizer.py:297: frontmatter={"updated": _now_iso()}. Confirms frontmatter: {} on refine really does drop the synthesizer's updated stamp, as claimed.
• CHANGELOG "Fixed" entry for [bug] wiki_refine_draft writes an empty lead, title, sections or frontmatter over the draft #585 (CHANGELOG.md:29-36) accurately restates the above; the ADR-1069 wiki page and its docs/adr/ mirror are consistent with each other and with the code.
• "The tool has been registered only since feat(wiki): expose the Path B draft tools over MCP (#579) #581": verified. PR feat(wiki): expose the Path B draft tools over MCP (#579) #581 merged 2026-09-16 (today), consistent with the diff being on top of that history.
• Breaking-changes note "the store holds one refined_llm memo, from a test runner in April 2026" is a claim about the author's own production database that I cannot verify from this worktree (no access to that store). It is not a claim about this diff's correctness, so it does not block.

Move 0 (ledger and seen-defect check)

No unmapped branch in the diff. The change is a single new refusal-detection function plus a call site in handler_refine, and the added test enumerates all five parametrized shapes matching the diff's _is_empty/_content_refusal logic. No "pre-existing," "unrelated," or "out of scope" rationalization anywhere in the PR body dismissing an observed defect. The Reviewer checklist box for CI is honestly left unchecked rather than asserted. No refusal trigger fires.

Layer, SOLID, wiring

• Layer: mcp_server/handlers/wiki_refine.py stays a handler calling infrastructure.pg_store_wiki. No new import crosses a boundary.
• SOLID: _content_refusal and _is_empty are small, single-purpose, pure functions. No violation.
• Wiring: _content_refusal has exactly one caller (handler_refine, wiki_refine.py:316). No dead code, no TODOs, no debug prints. The old all(args.get(field) is None ...) inline check was replaced cleanly, not duplicated.

Security and hygiene

No new I/O surface, no secrets, one conventional commit (fix(wiki): ...), scoped to the one bug.

Non-blocking follow-up (not filed as an issue by the author, noting for the record)

mcp_server/handlers/wiki_refine.py:288-300 (_validate_against_contract, unchanged by this diff): a section whose heading is blank or whitespace-only but whose body is real prose is written verbatim when its kind has no required sections (for example kind=note). Consider extending validation to reject a blank heading unconditionally, independent of the kind contract. Worth its own issue, not a defect in what #585/#586 claim to fix.

Commands run: gh issue view 585, gh pr view 586 --json body,title,files, gh pr diff 586, scratch scripts driving handler_refine directly against a real SQLite-backed store (top-level empty-value shapes, blank-heading/no-required-section shape, spy-based writer-call proof), git checkout -- ... and git status --short after two mutation trials, ruff check, ruff format --check, python scripts/check_craftsmanship.py --base origin/main, python scripts/check_doc_claims.py, .venv/bin/python -m pyright mcp_server/, full pytest -q.