Skip to content

fix(wiki): a draft section carries a heading and a body, both text (#587) - #588

Merged
cdeust merged 2 commits into
mainfrom
fix/refine-section-heading
Sep 16, 2026
Merged

cdeust merged 2 commits into
mainfrom
fix/refine-section-heading

Conversation

@cdeust

@cdeust cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Summary

_validate_against_contract (mcp_server/handlers/wiki_refine.py) checked that every heading the kind requires is present and that no body is blank, never a section's own heading. For a kind that declares no required section, {"heading": " ", "body": "real prose"} and {} itself passed and were written to the draft, and wiki_compile then rendered a section with no title. The nested required: [heading, body] of the tool schema is documentation: the client-visible schema is derived from the registered wrapper's signature, list[dict[str, Any]] | None, so a section missing its heading is a reachable client input.

Both fields are now validated as non-blank text through _is_text. A section whose heading is not text is named by its index, a blank heading no longer counts toward a required section, and a number in either field returns a validation error instead of raising AttributeError inside the handler (ADR-1070). The handler already refused a call whose sections fail validation before any write, so a refused call still touches neither the draft nor wiki.memos.

Found by the review of PR #586, which drove both shapes end to end on a note draft.

Closes #587

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test plan

  • New tests_py/handlers/test_wiki_refine_sections.py drives the validator directly: empty section, missing heading, blank heading, non-text heading; four body shapes including a number; a well-formed section with and without a matching required section; and a blank heading against a required one.
  • New test_a_headless_section_writes_nothing_even_without_a_contract in tests_py/handlers/test_wiki_refine_confidence.py drives the handler on SQLite with a kind contract carrying no required section, and asserts the sections column, synth_model and the draft's memo count are unchanged.
  • Six of those cases fail on the previous validator, one of them with the AttributeError this fixes.
  • Full suite: 8691 passed, 8 skipped, no errors.
  • ruff check, ruff format --check: clean. pyright mcp_server/: 0 errors. scripts/check_craftsmanship.py --base origin/main: OK. scripts/check_doc_claims.py: OK.

Audit notes

  • The required-section check now reads a heading set built only from headings that are text, so a blank heading cannot satisfy a required one. That is a behavior change in the strict direction and is covered by its own test.

Coding-standards compliance

  • §2.2 Layer dependency direction preserved.
  • §4.1 / §4.2 No new size violation; the gate passes against origin/main.
  • §8 No numeric constant added.
  • §9 No dead code, no TODO.

Breaking changes

A caller that used a section as an untitled block of prose now gets a validation error. Nothing did: the tool has been registered only since #581.

Reviewer checklist

  • CHANGELOG.md updated under Unreleased / Fixed.
  • Documentation updated (ADR-1070).
  • No secrets or PII in the diff.
  • CI passes on the latest commit.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Cz7A6goxVzfKL9NT7k4zDH

)

_validate_against_contract checked the kind's required headings and each
section's body, never a section's own heading. For a kind with no
required section, {"heading": "   ", "body": "prose"} and {} itself
passed and were written to the draft, and wiki_compile rendered the
section untitled. The nested required: [heading, body] of the tool
schema is documentation: the client-visible schema comes from the
registered wrapper's signature, list[dict[str, Any]] | None.

Both fields are now validated as non-blank text through _is_text, a
section whose heading is not text is named by its index, and a blank
heading no longer counts toward a required section. A number for either
field returns a validation error instead of raising AttributeError
(ADR-1070). A refused call still writes neither the draft nor a memo.

Six of the new cases fail on the previous validator. Full suite: 8691
passed, 8 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cz7A6goxVzfKL9NT7k4zDH
@cdeust

cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

ZETETIC-REVIEW: APPROVE

Summary

_validate_against_contract in mcp_server/handlers/wiki_refine.py now checks a section's own heading (non-blank text) in addition to the kind's required headings and each body, and reports a non-text field with the section's index instead of crashing. Verified against the registered wiki_refine_draft MCP tool end-to-end (not just the internal function), with the mutation checks the task requested, the full gate suite, and the full test suite. One real gap remains outside this PR's stated scope (see Issue 1 below) and one factual overstatement in the issue/ADR/PR-body text (Issue 2) — neither blocks this PR, which does exactly what it claims for the entry point it targets.

1. Is _validate_against_contract correct for every shape a client can send through wiki_refine_draft?

Ran adversarial payloads through the actual registered tool (mcp.call_tool("wiki_refine_draft", ...), not just the bare function), on a real MCPServer with tool_registry_wiki_drafts.register():

  • sections not a mapping/list ("not-a-list", 42, ["just a string"], [1, 2, 3]): all refused before reaching the handler, by Pydantic validating the wrapper's sections: list[dict[str, Any]] | None signature (mcp_server/tool_registry_wiki_drafts.py:66) — e.g. ToolError: Input should be a valid list [type=list_type], sections.0 Input should be a valid dictionary [type=dict_type]. So the handler-level fix in wiki_refine.py:296-315 never needs to defend against non-mapping items — the framework already does. Confirmed, no gap.
  • Duplicate headings: [{"heading":"Context","body":"a"},{"heading":"Context","body":"b"}] on a kind with required_sections: ["Context", ...] passes with no duplicate-heading error (the headings set at wiki_refine.py:303 dedupes silently). Not something the issue or ADR-1070 claims to fix, and not blocking, but worth naming as a real, unaddressed gap: two same-titled sections are written to the draft with no refusal.
  • Heading differing only by surrounding whitespace (" Context " vs required "Context"): satisfies the required section, because of .strip() equality at wiki_refine.py:303. Correct — matches ADR-1070's own claim ("a blank heading cannot satisfy a required one," not "whitespace differences are rejected").
  • Heading differing only by case ("context" vs required "Context"): does not satisfy the required section (case-sensitive comparison) — correct, unchanged pre-existing behavior, not a regression.
  • Very large sections list (20,000 items): validated correctly and quickly, no perf cliff observed.
  • Non-text heading/body (heading: 3): refused with "section 0: heading must be non-blank text", per _is_text at wiki_refine.py:288-290. No AttributeError. Matches the ADR-1070 claim.

Verdict on item 1: correct for everything a client can actually send through the registered tool. ADR-1070's core claim ("a section carries a heading and a body, both non-blank text, or the call is refused before any write") holds under adversarial testing of the real tool surface.

2. Other consumers of a section's heading — gap found

Checked call sites (grep -rn "heading" across mcp_server/core, mcp_server/handlers, mcp_server/infrastructure):

  • mcp_server/core/draft_compiler.py:168-173 — the .md renderer already guards if not heading: continue (added in commit dc6876c9, 2026-08-01, well before this PR/issue). So a headless section is silently dropped from the rendered wiki page, not "rendered with no title" as ADR-1070/issue [bug] a draft section with a blank or non-text heading is accepted when its kind has no required sections #587/PR body claim (see Issue 2).
  • mcp_server/handlers/wiki_compile.py:155-160 — builds the separate wiki.pages.sections DB-mirror dict with s.get("heading") used directly as a dict key, no guard at all. Demonstrated live: inserted a draft directly via insert_draft() (simulating a pre-existing/pre-fix row, or one written by a path other than wiki_refine_draft, e.g. wiki_synthesize) with sections=[{"heading": "", "body": "orphan prose with no heading"}], set it approved, ran wiki_compile.handler_refine-equivalent (_publish_one). Result: the .md file correctly drops the section (per the pre-existing guard above), but wiki.pages.sections in the DB ends up as {'': 'orphan prose with no heading'} — a headless section survives into the persisted mirror row that other tools (search, wiki_get_draft-style consumers of wiki.pages) read. This is a real, still-live instance of "a section's heading assumed present," in a call site this PR did not touch.
  • mcp_server/core/draft_curator.py:59-72 — _missing_required_sections does heading = getattr(s, "heading", None) or s.get("heading", "") then heading.strip(). For a dict section with a non-string heading (e.g. an int, which the new validator now blocks for wiki_refine_draft but which wiki_synthesize-authored or directly-inserted rows could still carry), this line raises AttributeError — the exact defect class this PR fixed in wiki_refine.py, still present here.

Does this PR's change break reading/compiling already-stored headless sections? No — it doesn't touch draft_compiler.py, wiki_compile.py, or draft_curator.py, so pre-existing drafts compile exactly as they did before (silently dropping the untitled section on .md render; carrying it forward as an empty-string key in wiki.pages.sections). The fix is scoped correctly to its stated target (the wiki_refine_draft entry point) and doesn't regress anything downstream — but it also doesn't close the gap ADR-1070's own reasoning ("a section whose heading is empty is the same defect one level down") implies should be closed. Non-blocking: this PR's scope (issue #587) is explicitly the wiki_refine_draft validator, and the ledger reconciliation for this diff maps cleanly to that scope — but wiki_compile.py:155-160 and draft_curator.py:64-66 are the same defect class, unfixed, and worth a follow-up issue.

3. Test strength (mutation check)

Baseline: tests_py/handlers/test_wiki_refine_sections.py + tests_py/handlers/test_wiki_refine_confidence.py — 20 passed.

  • Mutant A — _is_text returns True for any non-None value: 6 tests failed (blank-heading, heading-not-text, empty-body, blank-body, body-not-text, test_a_headless_section_writes_nothing_even_without_a_contract). Killed.
  • Mutant B — removed the heading check from the per-section loop: 5 tests failed (empty-section, no-heading, blank-heading, heading-not-text, test_a_headless_section_writes_nothing_even_without_a_contract). Killed.
  • Mutant C — restored old headings = {s.get("heading", "").strip() for s in sections} (no text filter): 1 test failed, with the exact AttributeError: 'int' object has no attribute 'strip' this PR fixes (heading-not-text case). Killed.

All three mutants restored; git status --short clean after each and at the end. The suite is mutation-strong for this fix.

4. Gates

All run from the worktree with UV_NO_SYNC=1:

  • ruff check: All checks passed.
  • ruff format --check: 1459 files already formatted.
  • scripts/check_craftsmanship.py --base origin/main: OK.
  • scripts/check_doc_claims.py: OK (1 declared not-a-claim exemption, pre-existing, unrelated to this diff).
  • .venv/bin/python -m pyright mcp_server/: 0 errors, 0 warnings, 0 informations.
  • pytest -q (full suite): 8691 passed, 8 skipped, 2 warnings, 459 subtests passed in 263s — matches the PR body's claim exactly.

5. Factual accuracy of claims (issue #587, ADR-1070, CHANGELOG)

  • The core validator claims (required [heading, body] in the schema is documentation not enforcement; the client-visible schema derives from the registered wrapper's list[dict[str, Any]] | None signature; a number in either field previously raised AttributeError; six of the new tests fail on the old validator with one AttributeError; registered only since feat(wiki): expose the Path B draft tools over MCP (#579) #581) — all verified true (see §1, §4, and the insert_draft/PR-581-merge checks below).
  • One overstated claim, repeated in the issue, ADR-1070, and the PR body: "wiki_compile renders a page whose section has no title" / "wiki_compile then rendered a section with no title." Demonstrated (§2) that the rendered .md file does not render an untitled section — draft_compiler.py's pre-existing if not heading: continue guard (since 2026-08-01, predating this issue) silently drops it instead. The accurate defect is in the separate wiki.pages.sections DB-mirror dict built by wiki_compile.py:155-160, which does carry the headless section under an empty-string key — a real defect, just not the one described. Non-blocking (doesn't change the merits of this PR, which targets wiki_refine.py correctly), but the ADR and issue text should be corrected in a follow-up so the record matches what was actually demonstrated.
  • git log --format='%an' --since='90 days ago' -- mcp_server/handlers/wiki_refine.py: cdeust, Clement, github-actions[bot] — multi-author file, and this PR changes behavior of a registered public MCP tool, so I classified this High-stakes (full Moves 1-6) despite the diff's small size.
  • No new imports in wiki_refine.py (git diff origin/main...HEAD shows none) — no layer-boundary change to check.

Issues

Non-blocking

  • mcp_server/handlers/wiki_compile.py:155-160 — same defect class as the one this PR fixes (headless section reaches a persisted structure unguarded), still live for any draft not authored through wiki_refine_draft. Suggest filing a follow-up issue; draft_compiler.py:168-173's if not heading: continue guard is the pattern to mirror.
  • mcp_server/core/draft_curator.py:64-66 — heading.strip() on a value that may not be a string (non-wiki_refine_draft origin) can still raise AttributeError; same defect class, unfixed, out of this PR's scope.
  • mcp_server/handlers/wiki_refine.py:303 — duplicate headings within one sections submission are silently deduped, not refused; not claimed as fixed by ADR-1070 but worth a decision either way.
  • docs/adr/ADR-1070-...md, wiki/adr/cortex/1070-...md, and issue [bug] a draft section with a blank or non-text heading is accepted when its kind has no required sections #587's own text overstate wiki_compile's current behavior (see §5) — the rendered .md silently drops the section rather than rendering it untitled. Worth a doc correction in a follow-up so the ADR record matches demonstrated behavior.

Commands run

gh issue view 587 --repo cdeust/Cortex
gh pr view 588 --repo cdeust/Cortex --json title,body,additions,deletions,files
gh pr diff 588 --repo cdeust/Cortex
UV_NO_SYNC=1 uv run python <adversarial MCP call_tool script over registered wiki_refine_draft>
UV_NO_SYNC=1 uv run python <wiki_compile / draft_compiler headless-section repro, with CORTEX_CLAUDE_DIR pointed at a tempdir>
UV_NO_SYNC=1 uv run pytest -q tests_py/handlers/test_wiki_refine_sections.py tests_py/handlers/test_wiki_refine_confidence.py   # baseline: 20 passed
# 3x: mutate wiki_refine.py, rerun the two files above, restore, git status --short
UV_NO_SYNC=1 uv run ruff check
UV_NO_SYNC=1 uv run ruff format --check
UV_NO_SYNC=1 uv run python scripts/check_craftsmanship.py --base origin/main
UV_NO_SYNC=1 uv run python scripts/check_doc_claims.py
.venv/bin/python -m pyright mcp_server/
UV_NO_SYNC=1 uv run pytest -q   # 8691 passed, 8 skipped
git log --format='%an' --since='90 days ago' -- mcp_server/handlers/wiki_refine.py
git show origin/main:mcp_server/handlers/wiki_refine.py > /tmp/.../wiki_refine_main.py  # ran the new tests against it: 6 failed, one with AttributeError
gh pr view 581 --repo cdeust/Cortex --json title,mergedAt

Note: one adversarial-repro script for item 2 initially wrote a stray file to the real ~/.claude/methodology/wiki/notes/_general/ tree before I set CORTEX_CLAUDE_DIR to a tempdir; caught it and deleted the stray .md immediately (that directory is not a git repo, so no tracked pollution occurred). No file in this git worktree was edited, committed, or pushed — git status --short is clean throughout and at the end of the review.

…ge (#587)

ADR-1070 and the CHANGELOG said `wiki_compile` rendered such a section
with no title. The review of PR #588 demonstrated otherwise:
core/draft_compiler.py skips a section with no heading, so the published
Markdown drops it, while handlers/wiki_compile.py mirrors the draft's
sections into wiki.pages.sections with no guard, leaving the prose under
an empty key. The validation this PR adds is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cz7A6goxVzfKL9NT7k4zDH
@cdeust

cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Correcting one claim in the PR body, per the review above.

The body said wiki_compile "renders a page whose section has no title". The published Markdown does not: core/draft_compiler.py:168-173 skips a section with no heading. The headless section survives in the DB mirror instead, handlers/wiki_compile.py:155-160, which builds wiki.pages.sections with no guard, leaving the prose under an empty key.

Commit 9f1efac corrects ADR-1070, its docs/adr mirror and the CHANGELOG entry. No code or test changed.

The two gaps the review names one layer down, the unguarded DB mirror and core/draft_curator.py:64-66 calling .strip() on a heading that may not be a string, plus duplicate headings being deduped rather than refused, are tracked in a follow-up issue.

@cdeust

cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

ZETETIC-REVIEW: APPROVE

Summary

Re-verification of PR #588 at new head 9f1efacf (previous verdict was posted against 4d6320fa, which the merge gate correctly refused as a verdict on a stale diff). 9f1efacf is documentation-only, made in direct response to the one factual finding from my prior review (Issue 2 there): it corrects ADR-1070, its docs/adr mirror, and the CHANGELOG to say the published Markdown drops a headless section (core/draft_compiler.py skips it) while handlers/wiki_compile.py mirrors it into wiki.pages.sections under an empty key — instead of the previous, inaccurate "wiki_compile rendered it untitled."

Diff scope confirmed

git diff 4d6320fa..9f1efacf --stat: CHANGELOG.md, docs/adr/ADR-1070-...md, wiki/adr/cortex/1070-...md — 3 files, 4 insertions, 3 deletions. No code, no test file touched. Content diff (read in full) is the same one-paragraph correction applied identically to the CHANGELOG entry, the canonical wiki ADR page, and its generated docs/adr mirror.

Claim re-verified against the two files it names

Read mcp_server/core/draft_compiler.py:168-173 and mcp_server/handlers/wiki_compile.py:155-160 again directly on this head (unchanged since my prior review, since this commit touches no code):

  • draft_compiler.py:171-172: if not heading: continue — a section with no heading is skipped when building the rendered Markdown body. Confirmed: the published .md file drops such a section rather than rendering it with an empty title.
  • wiki_compile.py:156-160: builds wiki.pages.sections as {(s.get("heading") ...): (s.get("body") ...) for s in (draft.get("sections") or [])} — no guard on a falsy/empty heading, so a headless section's key in that dict is "". Confirmed: this is where the headless prose actually survives, under an empty key, exactly as the corrected text now states.

Both claims are exactly what I empirically demonstrated in the previous review round (live repro: inserting a draft with sections=[{"heading": "", "body": "orphan prose"}], approving it, and compiling it produced wiki.pages.sections == {'': 'orphan prose with no heading'} while the rendered .md omitted the section entirely). The new doc text matches that evidence precisely; nothing further to correct.

Gates run (doc-only delta), from the worktree with UV_NO_SYNC=1

  • scripts/check_doc_claims.py: OK (1 declared not-a-claim exemption, pre-existing, unrelated — CONTRIBUTING.md:36).
  • scripts/check_project_wiki.py: "Project wiki mirrors match their canonical sources." — confirms the docs/adr mirror and wiki/adr/cortex canonical page were edited in lockstep (both diff hunks are identical).
  • ruff check: All checks passed.
  • ruff format --check: 1459 files already formatted.
  • scripts/check_craftsmanship.py --base origin/main: OK.
  • git status --short: clean before and after every command; nothing in the worktree was edited, committed, or pushed.

No code or test changed since the full-suite run in my prior review (8691 passed, 8 skipped, all other gates green), so I did not re-run the full pytest suite for this doc-only delta — the two gates the coordinator asked for plus lint/craftsmanship are sufficient evidence for a documentation correction that touches no source.

Issues

None blocking. The two gaps I flagged one layer down in the prior round (the unguarded wiki.pages.sections mirror in wiki_compile.py, and draft_curator.py's .strip() on a possibly-non-string heading), plus the duplicate-heading dedup gap, are tracked in issue #589 per the coordinator — no further action needed here.

Commands run

git fetch origin
git log --oneline -3 HEAD   # confirms worktree already at 9f1efacf
git diff --stat 4d6320fa..9f1efacf
git diff 4d6320fa..9f1efacf   # full text, read in full
sed -n '160,175p' mcp_server/core/draft_compiler.py
sed -n '150,161p' mcp_server/handlers/wiki_compile.py
UV_NO_SYNC=1 uv run python scripts/check_doc_claims.py
UV_NO_SYNC=1 uv run python scripts/check_project_wiki.py
UV_NO_SYNC=1 uv run ruff check
UV_NO_SYNC=1 uv run ruff format --check
UV_NO_SYNC=1 uv run python scripts/check_craftsmanship.py --base origin/main
git status --short

@cdeust
cdeust merged commit 3138ec6 into main Sep 16, 2026
28 checks passed
@cdeust
cdeust deleted the fix/refine-section-heading branch September 16, 2026 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] a draft section with a blank or non-text heading is accepted when its kind has no required sections

1 participant