feat(predictions): record what is expected, resolve it, score the confidence (#597) - #599

ZETETIC-REVIEW: APPROVE

Summary

Adds a predictions table (both backends), a pure core/calibration.py, and three handlers (predict, resolve_prediction, calibration), closing #597. Verified: the Brier-score math, the reliability breakdown, the repo-agnostic resolution contract, CHECK-constraint enforcement and atomic double-resolve prevention on both SQLite and PostgreSQL, tool registration/counts across the tree, and the full test suite (8763 passed, 8 skipped, matches the PR's claim exactly, reproduced independently). One pre-existing (not PR-introduced) doc-arithmetic gap noted below; everything else checks out.

Move 0, ledger / seen-defect check

No unmapped branch: every code path (predict refusals for blank claim/prediction/test/out-of-range confidence, resolve refusals for unknown id/blank source_ref/already-resolved, calibration scored/abandoned/empty) is both implemented and covered by tests_py/handlers/test_prediction_records.py and tests_py/core/test_calibration.py. No "pre-existing"/"unrelated"/"out of scope" rationalization anywhere in the PR body; the one unchecked reviewer-checklist box ("CI passes on the latest commit") is an honest pending item, not a dismissal. Proceeds to full review.

1. Scoring (derived independently, not trusted from the docstring)

• brier_score = mean((confidence − outcome)²), outcome ∈ {0,1}, mcp_server/core/calibration.py:66-72. This is the modern single-probability convention (matches scikit-learn's brier_score_loss, the Wikipedia "Brier score" article for binary events), not Brier's original 1950 two-category summation Σ_r(f_r−e_r)² which for R=2 categories equals 2·(f−e)². The code is self-consistent (its own UNINFORMATIVE_BRIER is derived from the same formula) and this convention is what "Brier score" means in essentially all contemporary usage for a single binary forecast probability, defensible, not a bug. Worth a one-line docstring note distinguishing "the modern binary-forecast convention" from "Brier's original multi-category formula," but non-blocking.
• UNINFORMATIVE_BRIER = 0.25: for a constant p=0.5 forecast, (0.5−0)² = (0.5−1)² = 0.25 → mean 0.25. Correct given the code's own formula. Confirmed by test_always_saying_half_scores_the_uninformative_reference (tests_py/core/test_calibration.py:34-37) and independently by hand.
• _band_of edges, checked by hand at every boundary (mcp_server/core/calibration.py:59-64, BAND_EDGES = (0.0, 0.2, 0.4, 0.6, 0.8, 1.0)):
  • 0.0 → (0.0, 0.2) ✓. 0.2 → (0.2, 0.4) ✓ (half-open, low inclusive). 0.6 → (0.6, 0.8) ✓. 0.8 → (0.8, 1.0) ✓. 1.0 → (0.8, 1.0) ✓ (top band closed at 1.0, per the high == BAND_EDGES[-1] escape).
  • Values above 1.0 or below 0.0 (unreachable through the public contract, predict's handler and both DDLs' CHECK (confidence BETWEEN 0 AND 1) block them before they can reach a resolved row, see §3) still get silently bucketed: on the last loop iteration the condition high == BAND_EDGES[-1] is True unconditionally, so any confidence, including 1.5 or −0.5, lands in (0.8, 1.0) or, for a value under 0.0, in (0.0, 0.2) on the first iteration. The return (BAND_EDGES[-2], BAND_EDGES[-1]) fallback after the loop (mcp_server/core/calibration.py:64) is consequently dead code, the loop always returns. Non-blocking (unreachable via the actual write path, confirmed empirically, see below), but worth a follow-up: either an explicit if not 0.0 <= confidence <= 1.0: raise at the top of _band_of, or delete the unreachable fallback line and note why it's safe.
• reliability() (mcp_server/core/calibration.py:76-92) reports exactly what a reliability diagram reports: per-bucket mean predicted probability vs. observed frequency, bucket size, low-band-first, matches test_reliability_reports_the_observed_frequency_per_band (tests_py/core/test_calibration.py:44-53).

2. Repo-agnostic claim

grep -rn "ZETETIC-REVIEW\|github\|GitHub\|forge" mcp_server/handlers/predict.py mcp_server/handlers/resolve_prediction.py mcp_server/handlers/calibration.py mcp_server/infrastructure/prediction_store.py mcp_server/core/calibration.py mcp_server/infrastructure/sqlite_schema_predictions.py mcp_server/tool_registry_predictions.py finds exactly one hit: resolve_prediction.py:80, a JSON-schema examples field showing a GitHub PR-comment URL as one illustrative source_ref shape, not logic, not a hardcoded convention, not a marker check. No code branches on host, forge, or the ZETETIC-REVIEW: string. The store, the handlers and the DDL are genuinely host-agnostic: resolve_prediction takes verdict/observed/source_kind/source_ref as opaque caller-supplied strings and never fetches anything. Verified end to end with a caller that has no GitHub involvement at all, direct DB-level tests against both SQLite and PostgreSQL (below), and the handler test suite, resolve/predict/score with plain strings, no forge dependency anywhere. source_kind (review, ci, test, manual) is a defensible, minimal exhaustive-enough taxonomy for "what kind of process produced this verdict" without naming any specific tool inside those categories.

3. Both backends, commands run and results

• SQLite: UV_NO_SYNC=1 uv run pytest -q tests_py/handlers/test_prediction_records.py tests_py/core/test_calibration.py → 47 passed (includes unrelated collected tests bundled in the same run, the prediction-specific 22 tests are within that 47).
• PostgreSQL (cortex_test DB, reachable locally): CORTEX_MEMORY_STORE_BACKEND=postgresql UV_NO_SYNC=1 uv run pytest -q tests_py/handlers/test_prediction_records.py tests_py/core/test_calibration.py -v → 22/22 passed, same assertions, same backend as production PG installs.
• CHECK constraints, direct SQL against both backends (bypassing the handler entirely): a bare UPDATE ... SET status='resolved' with no verdict/source_kind/source_ref is rejected by IntegrityError/CheckViolation on SQLite and on PostgreSQL; a resolved row missing only source_ref is rejected on both; an out-of-range confidence=1.5 insert is rejected on both (predictions_confidence_check / SQLite CHECK).
• Concurrency: 20 threads calling resolve_prediction() concurrently against the same open row on PostgreSQL, exactly 1 of 20 succeeded (sum(results) == 1), the other 19 got rowcount == 0 and returned False; final row state is the first writer's verdict. This is correct because the atomicity comes from the UPDATE ... WHERE id = %(id)s AND status = 'open' (mcp_server/infrastructure/prediction_store.py:83) being a single atomic statement, not from the earlier get_prediction() existence check (which is advisory/UX only, and does have a TOCTOU window, but the real safety property doesn't depend on it). test_a_prediction_resolves_once (tests_py/handlers/test_prediction_records.py:123-140) pins the same property at the handler level, on SQLite.
• One defense-in-depth observation, not a bug: the DB CHECK constraint requires source_ref IS NOT NULL but does not reject an empty string, confirmed directly (UPDATE ... SET source_ref='' succeeds against a bare SQLite connection with no handler in front of it). Only the handler's .strip() refusal (resolve_prediction.py:100-101) blocks a blank reference. This matches the PR's own stated design ("the handler refuses a blank reference before the database has to"), it is intentional layering, not an oversight, but it does mean a caller that writes to the table directly (bypassing the handler) could still leave a blank source_ref on a resolved row. Non-blocking.

4. Registration and counts

• mcp_server/__main__.py:45,104,121 imports, merges (merged_schemas()), and registers tool_registry_predictions alongside every other registry, confirmed by reading the file.
• mcp_server/tool_profiles.py docstring/count references and tests_py/test_tool_profiles.py:111 (assert len(...) == 57) agree.
• tests_py/test_main.py:126,138 assert 57 standalone / 60 with both upstreams, ran green in the full suite.
• Every 54/57/60 reference across docs/mcp-tools.md, README.md, docs/module-inventory.md, manifest.json, scripts/docker_smoke.sh, CHANGELOG.md, docs/ROADMAP.md, docs/agent-guidance.md, CONTRIBUTING.md, docs/api-reference.md was grepped and is internally consistent: 57 standalone, 60 with both upstreams, moved from 54 by this PR (grep transcript available on request).
• mcp_server/handlers/latency_class.py:56-59, predict/resolve_prediction/calibration classified interactive, consistent with the rest of that map.
• mcp_server/infrastructure/sqlite_schema.py correctly wires get_prediction_ddl() into get_all_ddl().
• docs/mcp-tools.md tier arithmetic: Tier 1 (22) + Tier 2 (7) + Tier 3 (13) + Tier 4 (12) = 54, but the doc's own header claims 57 standalone tools. This gap is pre-existing, not introduced by this PR, on origin/main before this change, the same tiers summed to 22+7+10+12=51 against a header claim of 54 (same 3-tool shortfall). The PR only added 3 tools to Tier 3 (10→13) and correctly bumped every count reference in lockstep, carrying the pre-existing gap forward unchanged rather than creating it. Reviewing the delta only: not a defect of this PR. Flagging for the maintainer's own backlog, not blocking: the doc likely has 3 tools that exist and register but aren't listed in any of the four tier tables (candidates worth checking: tools mentioned only in the "Project decision retrieval" prose subsection, or omitted entirely).

5. Mutation testing (mutants applied to copies outside the worktree, not committed; worktree left untouched, git status --short clean throughout)

• brier_score mutated to mean absolute error: hand-computed against the existing tests. test_always_saying_half_scores_the_uninformative_reference (pairs all at confidence 0.5) would compute MAE = 0.5 ≠ the asserted UNINFORMATIVE_BRIER (0.25), fails. test_the_report_counts_the_abandoned_apart asserts the exact MSE value ((0.8-1)**2+(0.8-0)**2)/2 = 0.34; MAE gives (0.2+0.8)/2 = 0.5, fails. Two independent assertions catch this mutation.
• The blank-source_ref handler refusal dropped: test_a_resolution_names_its_evidence sends source_ref=" " and asserts resolved is False and "source_ref" in error. Without the refusal, the handler proceeds to the DB; per §3's empty-string finding, the DB CHECK does not reject source_ref="" either, so the resolve would silently succeed and both assertions would fail. Caught.
• resolve_prediction allowed to settle an already-resolved row (the AND status = 'open' guard dropped): test_a_prediction_resolves_once resolves once, then resolves again with a different verdict, and asserts the second call returns resolved is False and "already resolved" in error. Without the guard, UPDATE ... WHERE id = %(id)s would match unconditionally and rowcount would be 1 on the second call too, both assertions fail. Caught. (Independently reproduced live against PostgreSQL: with the real guard in place, exactly 1 of 20 concurrent resolvers succeeds, see §3.)
• git status --short confirmed clean after every experiment; no worktree file was edited, committed, or left modified.

6. Gates and full suite

• ruff check + ruff format --check on every new/changed prediction file: clean.
• python scripts/check_craftsmanship.py --base origin/main: OK.
• .venv/bin/python -m pyright on the new prediction modules: 0 errors, 0 warnings.
• python scripts/check_project_wiki.py: mirrors match (wiki/adr/.../1076-*.md ↔ docs/adr/ADR-1076-*.md).
• python scripts/check_doc_claims.py: OK (one pre-existing declared exemption, unrelated to this PR).
• python scripts/check_venv_lock_parity.py: OK, no PostgreSQL-extra drift.
• Full suite: UV_NO_SYNC=1 uv run pytest -q → 8763 passed, 8 skipped, 459 subtests passed in 275.94s, matches the PR's claimed figures exactly, independently reproduced end to end (not merely trusted from the PR body).

7. Accuracy of PR body / ADR-1076 / CHANGELOG claims

• "796 procedural skills... every one at proficiency 0.5... 0 of 413 session-log entries carry a score", this is issue [feature] prediction records: store what was expected, resolve it from an external verdict, score the calibration #597's own stated measurement (2026-09-16), carried verbatim into the ADR and CHANGELOG; not independently re-measured in this review (it's a claim about the maintainer's own local transcript corpus, not something the diff computes), but it is consistently repeated, not contradicted, and not falsified by anything in the diff.
• ADR-1076 and docs/adr/ADR-1076-*.md are byte-identical past frontmatter (check_project_wiki.py confirms mirror match) and match the schema/handler behavior as implemented, read and cross-checked line by line against prediction_store.py, both DDLs, and the three handlers; no drift found.
• CHANGELOG's "Standalone tool count 54 to 57, 60 with both upstream integrations", confirmed against every count site in §4.
• "Largest new file is 121 lines" (Coding-standards compliance section), actual largest new prediction file is resolve_prediction.py at 136 lines; calibration.py (core) is 115. The 121 figure in the PR body doesn't match any single new file exactly, but every new file is comfortably under the 300/500-line caps regardless, so this is a citation-accuracy nit, not a size violation. Non-blocking.

Stakes classification

Medium, new business-logic surface (predictions table + scoring), not touching auth/billing/crypto/migrations of existing data, but adds a new table and three tools with real persistence. Full Moves 1-4 applied; Moves 5-6 applied at every changed call site given the surface's novelty.

Layer check

core/calibration.py, pure, no I/O, imports only stdlib (typing). infrastructure/prediction_store.py, imports only other infrastructure modules (pg_store_wiki_common, row_factory). handlers/{predict,resolve_prediction,calibration}.py, the composition roots, importing from core.calibration and infrastructure.prediction_store/memory_store/memory_config, exactly per the documented server → handlers → core ← shared, infrastructure → shared direction. No violation.

SOLID

No violations found. No new conditionals added to an existing type-switch; each handler has one responsibility (validate + delegate); resolve_prediction.py's _refusal() helper is a clean SRP extraction. DIP: handlers depend on concrete store functions rather than an injected interface, but this matches the pre-existing pattern used by every other handler in the codebase (e.g. the wiki-draft handlers), not a new violation introduced here.

Test adequacy

Every new branch (predict: 3 blank-field refusals + 3 confidence-range refusals; resolve: unknown-id, blank-ref, already-resolved, success; calibration: scored/abandoned/empty/domain-scoped) has at least one test asserting the actual postcondition (exact Brier value, exact error substring, resolved boolean), not merely that the handler ran. Concurrency and cross-backend behavior verified independently in this review (§3), beyond what's in the committed suite.

Complexity

All seven new/changed files are 53-136 lines; well under both the 40-line/method and 300/500-line caps. No smells.

Security & hygiene

No user input reaches a query unparameterized (list_predictions's dynamic WHERE is built from literal clause strings, values are bound %(param)s, # noqa: S608 justified per docs/ASSURANCE-CASE.md §5). No secrets in the diff. Commit message is conventional (feat(predictions): ...), single logical change.

Issues

Blocking

None.

Non-blocking

• mcp_server/core/calibration.py:64, dead fallback branch in _band_of (loop always returns via high == BAND_EDGES[-1]); either add an explicit out-of-range guard or delete the unreachable line with a comment explaining why the CHECK constraints make it unreachable.
• mcp_server/core/calibration.py docstring, clarify that the implemented formula is the modern single-probability convention, distinct from Brier 1950's original two-category summation (both are "the Brier score" in common usage, but citing the 1950 paper directly for the exact formula as written slightly overclaims fidelity to the original).
• docs/mcp-tools.md tier tables sum to 54 against the doc's own 57 header claim, pre-existing (already present pre-PR at 51 vs 54), not introduced here, carried forward unchanged.
• PR body's "Largest new file is 121 lines" doesn't match any actual new file (resolve_prediction.py is 136); all files are well under the size caps regardless.
• No single automated test diffs the SQLite vs. PostgreSQL DDL to guarantee schema parity mechanically; functional parity was verified in this review by running the handler suite against both backends and by direct constraint checks, but a future backend drift wouldn't be caught by CI alone.

Hand-offs

None required, no root-cause fix pending, no structural decomposition question, no concurrency proof beyond what was empirically verified, no new attack surface, no unverified performance claim.

Verdict

APPROVE.

ZETETIC-REVIEW: APPROVE

Summary

Re-reviewed PR #599 at head 53ec2bb (fix commit closing the five non-blocking findings from the prior review of 581b29b). All five are verified fixed with independent re-derivation, not merely trusted from the PR description. No new findings. Verdict unchanged: APPROVE.

Re-verification method

Re-fetched origin, diffed 581b29b..53ec2bb to see exactly what changed, then built a throwaway copy of the repo checked out at 53ec2bb outside the review worktree (/tmp/cortex_review_v2, its own resynced venv) to run tests and gates without touching the worktree. The reviewed worktree itself was never edited, committed to, or pushed from; git status --short there is clean throughout and after this review.

Finding 1: Brier convention, re-verified

mcp_server/core/calibration.py's docstring, ADR-1076 and the CHANGELOG now all state the implemented scale is mean((p-o)^2), the single-probability convention scikit-learn's brier_score_loss and most contemporary writing use; that Brier's 1950 paper sums over both categories and reports exactly twice this value; and that the two scales rank forecasters identically. Confirmed by reading all three files at 53ec2bb. UNINFORMATIVE_BRIER's source comment now cites the arithmetic (mean((0.5-o)^2)=0.25 for any {0,1} sequence) and test_always_saying_half_scores_the_uninformative_reference, not the paper's section number. Re-derived by hand: correct.

Finding 2: _band_of boundary handling, re-verified

Read the new implementation: it raises ValueError for any confidence outside [0,1], the zip is now strict=True over BAND_EDGES[:-1]/BAND_EDGES[1:], and the trailing return (BAND_EDGES[-2], BAND_EDGES[-1]) is reachable only for confidence exactly 1.0 (documented in a comment), since the loop condition is now the plain confidence < high with no high == BAND_EDGES[-1] escape. Ran it directly against every claimed edge case:

0.0 -> (0.0, 0.2); 0.2 -> (0.2, 0.4); 0.6 -> (0.6, 0.8); 0.8 -> (0.8, 1.0); 1.0 -> (0.8, 1.0)
-0.01 -> ValueError; 1.01 -> ValueError; 2.0 -> ValueError; -5.0 -> ValueError

All match the claim exactly. reliability() inherits the guard through _band_of (confirmed: reliability([(1.5, 1.0)]) raises ValueError). The three new parametrized test cases (-0.01, 1.01, 2.0) exist in tests_py/core/test_calibration.py and pass.

Finding 3: non-empty source_ref/observed CHECK, re-verified on both backends

Read both DDLs: the resolved-row CHECK now additionally requires source_ref <> '' and observed <> '' (previously only IS NOT NULL). Confirmed against a fresh SQLite in-memory table (the new test_the_table_itself_refuses_an_empty_reference test in tests_py/handlers/test_prediction_records.py passes: sqlite3.IntegrityError on source_ref=''). Confirmed independently and directly against the reachable local PostgreSQL instance (cortex_test), bypassing the ORM/handler entirely:

• source_ref='', observed='x' -> CheckViolation (rejected, as expected)
• source_ref='ref1', observed='' -> CheckViolation (rejected, as expected)
• source_ref='ref1', observed='x' -> succeeds, row resolves correctly
  Both backends now reject a blank reference or a blank observation at the schema level, closing the defense-in-depth gap from the prior review.

Finding 4: docs/mcp-tools.md tier arithmetic, re-verified programmatically

ingest_findings, lesson_promotion and get_grooming_health are now three new rows under Tier 3, which the header correctly reads "(16 tools)". Verified two ways, not just by eyeballing the diff:

• Parsed every tier table's declared count against its actual row count: Tier 1 = 22/22, Tier 2 = 7/7, Tier 3 = 16/16, Tier 4 = 12/12, sum = 57, matching the doc's own top-of-file header claim exactly.
• Cross-checked against the live registered tool set: built an MCPServer, called mcp_server.__main__.register_all(mcp, codebase=False, prd=False), and diffed the 57 registered tool names against the 57 tool names parsed out of every | \name` |` row across all four tier tables. Zero missing, zero extra, both directions.
  This is now a mechanically-checked match, not merely arithmetic that happens to add up.

Finding 5: PR body "largest new file" line

The PR body now reads "Largest new file is resolve_prediction.py at 136 lines" (confirmed via gh pr view), matching the actual file size (136 lines, unchanged since the base commit). Corrected as stated.

Gates, re-run against 53ec2bb

• ruff check . (whole repo): all checks passed.
• ruff format --check . (whole repo): 1477 files already formatted.
• python scripts/check_craftsmanship.py --base origin/main: OK.
• python scripts/check_doc_claims.py: OK (same one pre-existing, unrelated exemption as before).
• python scripts/check_project_wiki.py: mirrors match.
• python scripts/check_venv_lock_parity.py: OK, no drift.
• pyright on the three changed prediction files: 0 errors, 0 warnings.
• pytest tests_py/core/test_calibration.py tests_py/handlers/test_prediction_records.py: 26/26 on SQLite, 26/26 again forcing CORTEX_MEMORY_STORE_BACKEND=postgresql against the reachable local instance (up from 22/22 at the prior head, the +4 being the three new confidence-range cases plus the new empty-reference CHECK test).
• Full suite (pytest -q, throwaway copy, fresh venv): 8610 passed, 165 skipped, 0 failed, exit code 0, 459 subtests passed, in 321s. The skip count is higher than the coordinator's reported 8767/8 and than this reviewer's own prior 8763/8 run against the base commit in the actual (properly configured) worktree; investigated and attributed to this throwaway copy's cold/rebuilt venv rather than to the diff itself, since: (a) _pg_schema_provision.ensure_pg_ready() reports PostgreSQL ready in that same copy, (b) zero tests failed, and (c) every prediction-specific and doc-count check above was independently re-run and passed cleanly on both backends in this same copy. Recommend the coordinator's own 8767/8 figure (measured in the properly-configured environment) as authoritative; this reviewer's anomalous skip count does not indicate a regression.

Verdict

APPROVE. All five findings from the prior round are closed and independently re-verified; no new issues found.

ZETETIC-REVIEW: APPROVE

Summary

Re-reviewed PR #599 at head 4a86760 (fix commit for the CI failure caused by scripts/verify_mcp_hosts.py's stale literal MIN_FULL_TOOL_COUNT = 52). The fix derives the host-contract bounds from the registries instead of restating them, closes the class of defect that caused the CI failure, and I independently found and audited every other place a tool-count literal lives outside prose, as asked. Two pre-existing stale copies exist elsewhere in the tree (neither touched by this PR, neither the cause of the CI failure), plus one currently-correct but still ungated literal in the same risk class as the bug this PR fixes. None of these block this PR; they are reported below as the requested audit. Verdict: APPROVE.

What changed, re-verified against the diff (53ec2bb..4a86760)

• mcp_server/tool_surface.py (new, 55 lines): standalone_tool_names() unions the SCHEMAS maps of the nine standalone registry modules plus the non-upstream slice of tool_registry_ingest.SCHEMAS; UPSTREAM_TOOL_NAMES is the frozen three-name set. It imports registry modules only, confirmed by reading the import list, so no MCP server startup is paid for a count, as the docstring claims.
• scripts/verify_mcp_hosts.py: MIN_FULL_TOOL_COUNT = len(standalone_tool_names()) and MAX_FULL_TOOL_COUNT = MIN_FULL_TOOL_COUNT + len(UPSTREAM_TOOL_NAMES), replacing the literal 52 / 52 + 3. Confirmed the craftsmanship-gate-driven two-line source comment split the coordinator described (# source: ADR-0788 / # The correction is recorded as ADR number 1077.) is present and the gate passes.
• tests_py/test_tool_surface.py (new): three tests confirmed present and passing, the derived set equals what register_all() actually registers; the gated names are exactly the difference between the codebase/prd-enabled and disabled registrations; the verifier's bounds equal the derived count and that count plus the gated names.
• ADR-1077 and the CHANGELOG entry read and cross-checked against the code: accurate, including the explicit statement "The remaining copies are prose for humans, and the doc-claim gate already cross-checks them against the pinned test name", this scoping statement is itself accurate (see audit below) with two exceptions the ADR does not claim to cover.

Re-run evidence (throwaway copy at 4a86760, worktree itself untouched, git status --short clean throughout and after)

• pytest tests_py/test_tool_surface.py tests_py/test_main.py tests_py/test_tool_profiles.py: 28/28 passed.
• python scripts/verify_mcp_hosts.py -- hypermnesia-mcp: all six cases pass (claude-code/full, claude-code/lean, gemini-cli/full, gemini-cli/lean, codex-cli/full, codex-cli/lean), full profile discovering 59 tools (within the now-derived 57..60 bound; two of the three upstream integrations happen to be installed in this environment), lean discovering 10.
• ruff check . / ruff format --check . (whole repo): clean.
• python scripts/check_craftsmanship.py --base origin/main: OK.
• python scripts/check_doc_claims.py: OK, same one pre-existing declared exemption as before.
• python scripts/check_project_wiki.py: mirrors match.
• pyright on the three changed/new files: 0 errors, 0 warnings.
• Full suite: 8613 passed, 165 skipped, 0 failed, exit 0, 459 subtests, in 351.6s. As in the prior round, the skip count in this throwaway copy is inflated relative to the coordinator's reported 8770/8 (the same artifact, same magnitude, reproduced identically a second time in an independent fresh copy, this consistency strengthens the "environment artifact, not a regression" diagnosis from the prior round rather than weakening it). Zero failures both times. Recommend the coordinator's own figure, measured in the properly-configured environment, as authoritative for the exact numbers; what this reviewer can independently guarantee is: no failure, and the specific fix's tests pass on both the targeted files and the six-case host verification.

The requested audit: every other copy of the tool count outside prose

Grepped scripts, workflows, manifests, benchmarks and .bestpractices.json for a literal count or range. Full inventory, each classified as derived, gate-checked, or stale:

• mcp_server/tool_surface.py, derived (the new source of truth).
• scripts/verify_mcp_hosts.py MIN_FULL_TOOL_COUNT/MAX_FULL_TOOL_COUNT, derived (fixed this commit).
• tests_py/test_tool_surface.py, gate-checks the derivation three ways (live registration, gated-name diff, verifier bounds).
• tests_py/test_main.py::test_standalone_baseline_is_57_tools, the pinned baseline; itself asserts against a live register_all() build, so it is gate-checked against reality (it is the literal that everything else in the gate chain reads).
• docs/mcp-tools.md header sentence ("57 standalone tools... 60 total"), gate-checked: scripts/doc_claim_sources.py::canonical_tool_counts() cross-references this sentence against the pinned test name in test_main.py and fails the build on disagreement (confirmed by reading the function).
• docs/mcp-tools.md tier-table row counts (22+7+16+12=57), not mechanically gate-checked by any script; nothing sums the tier tables and compares them to the header. Currently correct (verified by hand in the prior review round and again here), but this specific sub-claim is arithmetic a human could get wrong without CI catching it, unlike the header sentence itself.
• README.md, CONTRIBUTING.md, manifest.json, docs/module-inventory.md, docs/api-reference.md, docs/ROADMAP.md, docs/agent-guidance.md, docs/ASSURANCE-CASE.md, GOVERNANCE.md, SECURITY.md, CLAUDE.md, server.json, all in check_doc_claims.py's SCANNED_FILES and matched by its TOOL_CLAIM/TOOL_TOTAL_CLAIM regexes, gate-checked.
• scripts/docker_smoke.sh MIN_TOOL_COUNT="${CORTEX_SMOKE_MIN_TOOLS:-57}", a bare shell literal, not scanned by check_doc_claims.py (not in SCANNED_FILES, and it is a .sh file, not prose) and not asserted against the true count by any test I could find (greped tests_py/ and scripts/ for anything referencing docker_smoke.sh's tool-count floor; nothing does). Currently correct (57, bumped alongside everything else in the original PR), but this is the same defect class as the bug this PR fixes: a literal copy with no automated check tying it to the registries. Not touched by this fix commit and not what CI caught this round, so non-blocking, but worth a follow-up given ADR-1077 exists specifically to close this class of drift.
• .bestpractices.json:234, stale, and the staleness evades the gate despite the file being in SCANNED_FILES: the sentence embeds the literal test name test_standalone_baseline_is_52_tools (the real one is now _57_tools), but check_doc_claims.py's TOOL_CLAIM regex (\d+)\s+(?:memory|standalone|MCP)\s+tools\b requires a digit-space-word pattern and does not match is_52_tools (no space, and the digits are glued to the test-name identifier). Confirmed pre-existing: identical text is present on origin/main at 0fc6bd1a (the commit before this entire PR chain), so this predates PR feat(predictions): record what is expected, resolve it, score the confidence (#597) #599 and is not introduced or touched by it. Reported per the coordinator's request, not blocking this PR.
• .claude-plugin/marketplace.json, stale and entirely outside the gate: the description field states "50 MCP tools (53 with the optional ai-architect-mcp-codebase + ai-architect-mcp-spec integrations)", a much older figure than even the pre-feat(predictions): record what is expected, resolve it, score the confidence (#597) #599 baseline (52/55). This file is not in check_doc_claims.py's SCANNED_FILES list at all, confirmed by reading the list. Confirmed pre-existing and untouched by this PR (git diff 581b29b3..4a86760f -- .claude-plugin/marketplace.json is empty). Reported per the coordinator's request, not blocking this PR.
• tests_py/scripts/test_check_doc_claims.py's PINNED_TEST = "...is_52_tools...", not a live claim: a synthetic fixture string (CATALOGUE, PINNED_TEST, BIBLIOGRAPHY constants feeding a _FakeRepo double) used to unit-test the gate's own regex-extraction logic in isolation. Its value is arbitrary and unrelated to the real repository count; correctly excluded from consideration.
• wiki/adr/cortex/*.md and docs/adr/*.md historical ADR pages (0093, 0713, 0732, 1066) that mention older counts (49/50/52/54), frozen history, not live claims: each describes the tool count at the time that ADR was accepted, the same way CHANGELOG.md's dated entries do. Not stale in the sense that matters; they are not supposed to track the current count.
• benchmarks/mcp_profile_tokens.py, computes tool_count at runtime from a live _list_tools_payload(server) call, derived, no literal.
• .github/workflows/{ci,release,upstream-identity}.yml, grepped for a tool-count literal or range; none found (the one numeric hit, tool_timeout_sec=600, is an MCP per-call timeout setting, unrelated to a count).

Stakes classification

Low-Medium, this fix commit touches CI tooling and a verification script, not production business logic or data; no auth/billing/crypto/migration surface. Moves 1, 3, 6 applied in full; Moves 2/4/5 applied informally given the low blast radius, consistent with the prior two rounds' classification of the underlying feature as Medium.

Issues

Blocking

None.

Non-blocking

• .bestpractices.json:234 still cites test_standalone_baseline_is_52_tools; pre-existing, not introduced by this PR, evades check_doc_claims.py's regex because of the embedded-identifier phrasing. A minimal fix would tighten TOOL_CLAIM to also match _baseline_is_(\d+)_tools or simply update the prose to state the number in prose form.
• .claude-plugin/marketplace.json states "50 MCP tools (53 with...)", well behind the current 57/60; pre-existing, not introduced by this PR, and the file is entirely outside check_doc_claims.py's scanned set.
• scripts/docker_smoke.sh's MIN_TOOL_COUNT default remains a bare literal (currently correct at 57) with no test tying it to the registries, the same defect class ADR-1077 exists to close, left open for this one script.

Hand-offs

None required.

Verdict

APPROVE. The CI-breaking defect (a stale literal in verify_mcp_hosts.py) is fixed at its root by deriving the bounds from the registries, pinned by three new tests, and independently re-verified: all six verify_mcp_hosts.py cases pass, the targeted test files pass, and the whole-repo gates are clean. The broader audit requested finds two pre-existing stale copies and one pre-existing ungated-but-correct copy elsewhere in the tree, none touched by or blocking this PR, reported as the class-of-defect evidence asked for.

ZETETIC-REVIEW: APPROVE

Summary

Re-reviewed PR #599 at head c710439 (fix commit closing the three stale tool-count copies the prior review round found: .bestpractices.json, .claude-plugin/marketplace.json, scripts/docker_smoke.sh). All three are now corrected and gate-checked or pinned by a test. I independently answered the coordinator's three specific questions, all favorably: the marketplace regex cannot match a historical sentence and fails closed if the current one is removed; the fixture edit added assertions without removing or weakening any existing one; and a fresh audit found no remaining escaping copy. Verdict: APPROVE.

Question 1: can the marketplace regex match a historical sentence instead of the current one?

Read the full .claude-plugin/marketplace.json description field (1429 characters, containing roughly 20 version-history entries back to v3.17.0). It does contain multiple historical tool-count mentions in different phrasing: "standalone tool count 49->50 (53 with upstream integrations)", "49 standalone / 52 full-upstream MCP tools (1 new: get_grooming_health)", "registers all 43 standalone MCP tools without a PostgreSQL connection (46 with the upstream integrations)", "46 MCP tools remain.", "PostgreSQL remains the mandatory backend; 46 MCP tools." None of these match MARKETPLACE_TOOL_COUNTS = re.compile(r"(\d+) MCP tools \((\d+) with the optional"): each either has an extra word between the digit and "MCP tools" (e.g. "43 standalone MCP tools"), a different parenthetical phrase ("with upstream integrations" / "with the upstream integrations", not "with the optional"), or no parenthetical follows "MCP tools" at all ("MCP tools remain.", "MCP tools."). Verified empirically, not just by reading: ran the regex against the real description text and got exactly one match, the current 57/60 sentence, at its actual offset; confirmed "with the optional" occurs exactly once in the whole field. Then simulated the coordinator's exact scenario, removing only the current sentence's matching substring and rerunning the regex: zero matches, which drives check_marketplace_tool_counts into its match is None branch and returns the "the gate has lost its subject" failure rather than silently matching a historical count. Confirmed by running the actual function against both the real file and a mutated-to-55/58 copy (each produces the correct two named failures) and against a copy with the current sentence deleted (produces the fail-closed failure), all on a throwaway copy outside the worktree.

Question 2: did the fixture edit weaken any existing assertion in the gate's own tests?

git diff 4a86760f..c7104395 -- tests_py/scripts/test_check_doc_claims.py contains zero removed lines (git diff ... | grep -c '^-[^-]' = 0), the change is a pure addition: a new comment, a new MARKETPLACE class constant, and one new key in the _install() fixture dict. Nothing pre-existing was deleted or loosened. Ran the two most exposed pre-existing tests directly against the new fixtures: test_a_consistent_repository_reports_nothing and test_each_family_reports_its_own_stale_claim both pass, the four-family mutation loop in the latter only ever touches the CONSISTENT string's "52 memory tools" / "(55 total" / "2-reference" / "36 mechanisms" substrings, none of which overlaps the newly-added pinned-test-name line or the separately-supplied MARKETPLACE fixture, so each subtest still produces exactly one failure as asserted. Ran the full tests_py/scripts/test_check_doc_claims.py file (105 tests) plus tests_py/test_tool_surface.py (4, including the new docker-smoke test): 134/134 passed.

Question 3: does any count copy still escape both the generic scan and the two targeted checks?

Re-ran the audit from the prior round plus a fresh grep pass for "MCP tools\|standalone tools\|memory tools" across every .json/.yml/.sh/.py/.toml file in the tree (excluding benchmark result dumps and campaign artifacts, which carry no tool-count claims). Findings:

• manifest.json ("57 memory tools standalone (60 with upstream integrations available)"), gate-checked via SCANNED_FILES, correct.
• scripts/doc_claim_structural.py / doc_claim_sources.py, the check implementations themselves, not claims.
• .claude-plugin/plugin.json, .agents/plugins/marketplace.json, plugins/hypermnesia-mcp-codex/.codex-plugin/plugin.json, plugins/cortex-deprecated/.claude-plugin/plugin.json, plugins/cortex-viz-deprecated/.claude-plugin/plugin.json, checked each individually for a numeric "N tools" claim; none exists in any of them (the codex-plugin's longDescription says "the complete tool profile" with no number, the deprecated shims say they have no MCP server or tools at all).
• server.json, no tool-count claim beyond what's already gate-checked in SCANNED_FILES.
• No stray literal or range found outside what was already fixed. The audit turns up nothing new to report.

Independent mutation-driven re-verification (throwaway copy of c710439, worktree untouched throughout, git status --short clean before/during/after)

• .bestpractices.json mutated test_standalone_baseline_is_57_tools -> _52_tools: python scripts/check_doc_claims.py reports exactly .bestpractices.json:234: advertises 52 tools in the pinned test name, canonical is 57. Restored: gate passes clean again.
• .claude-plugin/marketplace.json mutated 57/60 -> 55/58: check_marketplace_tool_counts reports both counts by name. Restored: clean.
• scripts/docker_smoke.sh mutated CORTEX_SMOKE_MIN_TOOLS:-57 -> :-52: test_the_docker_smoke_floor_follows_the_surface fails with assert 52 == 57, naming both sides. Restored: passes.
• Every mutation was applied and reverted on the throwaway copy at /tmp, never on the reviewed worktree.

Gates and full suite, re-run on c710439

• ruff check . / ruff format --check .: clean.
• python scripts/check_craftsmanship.py --base origin/main: OK.
• python scripts/check_doc_claims.py: OK, same one pre-existing declared exemption.
• python scripts/check_project_wiki.py: mirrors match.
• pyright on the three changed files (doc_claim_structural.py, check_doc_claims.py, tool_surface.py): 0 errors.
• Full suite: 8614 passed, 165 skipped, 0 failed, exit 0, 459 subtests, 355.3s. The skip count in this throwaway copy is again inflated relative to the coordinator's reported 8771/8, the third consecutive round in which this exact artifact reproduces at the same magnitude in an independently rebuilt copy, which by now is strong evidence it is a property of the cp -r + fresh-venv throwaway-copy technique itself (most likely a PostgreSQL-reachability difference this copy's environment resolves differently than the properly-configured worktree), not a property of any of the three commits reviewed. Zero failures in all three rounds. The coordinator's own figure, measured in the correctly configured environment, is the one to trust for the exact number.

Stakes classification

Low, this fix commit only tightens doc-claim tooling (two new checks, one new test, corrected prose in two JSON files); no runtime/business-logic surface touched. Moves 1, 3, 6 applied; 2/4/5 informal, consistent with this being a documentation/CI-tooling change.

Issues

Blocking

None.

Non-blocking

None new. The three findings from the prior round are closed.

Hand-offs

None required.

Verdict

APPROVE. All three stale copies found in the prior audit are corrected and now either gate-checked (.bestpractices.json, .claude-plugin/marketplace.json) or test-pinned (scripts/docker_smoke.sh). The three specific concerns raised (regex ambiguity against history, fixture-assertion weakening, remaining escapees) were each independently checked and none held up: the marketplace regex is specific enough to match only the current sentence and fails closed on its removal, the fixture change added coverage without touching any existing assertion, and a fresh audit found no further escaping copy.

ZETETIC-REVIEW: REQUEST_CHANGES

Summary

Re-reviewed PR #599 at head ebe8a6b (fix commit for the mcp-host-config job breakage caused by the previous fix's module-scope import). The production fix itself is correct and independently verified: moving the import of mcp_server.tool_surface inside a function genuinely resolves the ModuleNotFoundError that broke CI, and the full-profile bounds check still runs unconditionally when it should, never silently skipped. However, the regression test added specifically to pin this fix and prevent recurrence, test_the_verifier_imports_without_the_mcp_sdk, is demonstrably non-functional: its Blocker class implements an import-system protocol (find_module/load_module) that CPython 3.12 never invokes, so the test passes unconditionally whether or not the fix is present. I proved this by running the exact test against the pre-fix code and watching it pass when it should have failed. This is a real, reproduced defect in the new test, not a hypothesis, and it means the safety net this PR describes as pinning the fix currently pins nothing. Requesting a fix to the test before merge.

The central finding, reproduced independently

tests_py/test_tool_surface.py::test_the_verifier_imports_without_the_mcp_sdk builds a Blocker class:

class Blocker:
    def find_module(self, name, path=None):
        blocked = name == "mcp" or name.startswith("mcp.")
        return self if blocked else None
    def load_module(self, name):
        raise ImportError(name)
sys.meta_path.insert(0, Blocker())

find_module/load_module is the legacy meta-path-finder protocol PEP 451 replaced with find_spec in Python 3.4. Modern CPython's import machinery (importlib._bootstrap._find_spec) does not call find_module on a finder that lacks find_spec; it silently skips such finders and falls through to the next one on sys.meta_path, eventually finding the real, installed mcp package via the normal PathFinder. I confirmed this two ways, both on Python 3.12.11 (the interpreter this repo's own venv and CI use):

1. Added a print("find_module called for", name) line inside Blocker.find_module and ran import mcp with the blocker installed: the print statement never fires, and mcp imports successfully anyway.
2. Ran the actual committed test (copied verbatim, unmodified) against a checkout of the PRE-FIX commit (c710439, module-scope from mcp_server.tool_surface import (...) at scripts/verify_mcp_hosts.py:44): the test passes (1 passed in 23.22s). It should fail on that commit, since that is precisely the commit mcp-host-config broke on. A test that passes identically on both the broken commit and the fixed commit is not testing what it claims to test.

To confirm the underlying production fix is nonetheless real and correct, I wrote a working blocker using the modern find_spec protocol and reran the same "import scripts.verify_mcp_hosts with mcp blocked" scenario against both commits:

• Against c710439 (module-scope import): fails with ImportError: blocked: mcp, raised from mcp_server/tool_registry_advanced.py:10's from mcp.server.mcpserver import MCPServer, reached via the module-scope tool_surface import chain, this is the exact failure class that broke mcp-host-config in CI.
• Against ebe8a6b (late import): imports cleanly, mcp is never touched at module scope.

This proves two things at once: the production fix in mcp_server/tool_surface.py's full_tool_bounds() and scripts/verify_mcp_hosts.py's late-import wrapper is genuinely necessary and correct; and the committed regression test does not exercise that fix at all, because its blocking mechanism is inert on the Python version this project runs.

Required change

Rewrite Blocker in test_the_verifier_imports_without_the_mcp_sdk to implement the modern MetaPathFinder protocol, e.g.:

class Blocker:
    def find_spec(self, name, path, target=None):
        if name == "mcp" or name.startswith("mcp."):
            raise ImportError(f"blocked: {name}")
        return None

(or equivalently, poison sys.modules["mcp"] = None before the probe, which also reliably raises ImportError on any subsequent import mcp). Whichever mechanism is chosen, verify it the same way this review did: run the corrected test against a checkout of c710439 (module-scope import) and confirm it fails with the expected ModuleNotFoundError/ImportError, then confirm it passes against ebe8a6b. That before/after pair is the actual proof the test pins anything; right now neither half of that pair has been demonstrated for the committed test, and I've shown the "before" half fails to distinguish the two commits.

The three specific questions asked, answered

1. Does the late import weaken the bounds check when it runs (must a full profile still be checked, not silently skipped)? No weakening. Read _verify_tool_surface in scripts/verify_mcp_hosts.py:93-119: the else branch (full profile) unconditionally calls full_tool_bounds() and raises ContractError if the discovered tool count falls outside the derived bounds, exactly as before the refactor, the only change is that the two numbers now come from a function call instead of two module-level names. Ran all six verify_mcp_hosts.py cases directly: claude-code/full, gemini-cli/full, codex-cli/full all report 59 tools, correctly inside the derived [57, 60] bound, and the check demonstrably executes (not skipped) because the earlier PASS ... (59 tools, ...) lines only print after the bounds check has already run without raising.
2. Can the wrapper and the surface function disagree? No. scripts/verify_mcp_hosts.py's full_tool_bounds() (lines 55-62) is a pure one-line delegation: from mcp_server.tool_surface import full_tool_bounds as surface_bounds; return surface_bounds(). There is no separate computation on the verifier side to drift from the surface module's own full_tool_bounds() (mcp_server/tool_surface.py:56-68); it is the same function, imported under an alias, not reimplemented. tests_py/test_tool_surface.py::test_the_host_contract_bounds_follow_the_surface additionally pins the two call sites' results against standalone_tool_names()/UPSTREAM_TOOL_NAMES directly, and passes.
3. Does anything else import tool_surface at module scope in a context without the SDK? Grepped the whole tree for tool_surface: exactly two importers exist, tests_py/test_tool_surface.py (module-scope, but this is a pytest test file that always runs with the full dev dependency set, including the SDK, never in the SDK-less mcp-host-config job) and scripts/verify_mcp_hosts.py (function-scope, the fix under review). No other file references mcp_server.tool_surface.

Gates and full suite, re-run on ebe8a6b

• ruff check . / ruff format --check .: clean.
• python scripts/check_craftsmanship.py --base origin/main: OK. scripts/verify_mcp_hosts.py is 292 lines, under the 300-line cap it would otherwise have crossed at 303 (confirmed by wc -l).
• python scripts/check_doc_claims.py, python scripts/check_project_wiki.py: clean.
• pyright on the three changed files: 0 errors.
• All six verify_mcp_hosts.py cases pass (claude-code/gemini-cli/codex-cli x full/lean).
• tests_py/test_tool_surface.py: 5/5 pass (including the vacuous one, which is exactly the problem, it cannot fail).
• Full suite: 8615 passed, 165 skipped, 0 failed, 317.6s. As in every prior round, the skip count in this throwaway copy is inflated relative to the coordinator's reported 8772/8 (the same well-established cp -r-copy artifact from the last four rounds); zero failures.

Stakes classification

Low, for the change's runtime blast radius (CI tooling only). Elevated attention on test adequacy specifically, because the whole point of this fix commit is "prevent this exact regression from recurring silently," and the mechanism meant to guarantee that does not.

Issues

Blocking

• tests_py/test_tool_surface.py::test_the_verifier_imports_without_the_mcp_sdk, the Blocker class uses the legacy find_module/load_module finder protocol, which Python 3.12 never invokes, so the test passes unconditionally and does not detect the regression it is named for. Demonstrated: the unmodified test passes against a checkout of the pre-fix commit (c710439). Required change: reimplement Blocker using find_spec (or sys.modules poisoning), and verify the corrected test fails against c710439's module-scope import before merging.

Non-blocking

None new.

Hand-offs

None required, the fix is a targeted regression test correction.

Verdict

REQUEST_CHANGES. The production code fix (late import) is correct, necessary, and independently verified against the actual pre-fix/post-fix behavior. The regression test meant to guarantee it stays fixed does not work on the Python version this project runs, and I have demonstrated that concretely rather than asserting it: the exact committed test passes against the broken commit. Fix the test's blocking mechanism and re-verify the before/after pair, then this is ready to merge.

ZETETIC-REVIEW: APPROVE

Summary

Re-reviewed PR #599 at head ea927622, verifying the single blocking finding of the previous round: tests_py/test_tool_surface.py::test_the_verifier_imports_without_the_mcp_sdk used a pre-PEP 451 find_module/load_module meta-path finder that CPython never invokes, so the test passed against c7104395, the commit whose module-scope import broke the mcp-host-config job. The fix is correct and, unlike the previous round's test, it is now demonstrably able to fail: I ran the committed test verbatim against c7104395 and it fails with exactly the CI symptom, and I ran the anti-vacuity guard itself against an inert blocker and it fires. The rest of the PR (predictions table, calibration, three tools, tool-surface derivation), approved across three prior rounds, got a sanity pass only. Verdict: APPROVE.

1. The corrected probe uses find_spec and its self-check cannot pass vacuously

tests_py/test_tool_surface.py:27-47, module constant _SDK_FREE_IMPORT_PROBE:

• tests_py/test_tool_surface.py:32, def find_spec(self, name, path=None, target=None):. This is the modern MetaPathFinder protocol, raising ImportError(name) for mcp and mcp.* (line 33-34) and returning None otherwise (line 35), so non-SDK imports fall through untouched. mcp_server is correctly unaffected: it is neither == "mcp" nor startswith("mcp.").
• tests_py/test_tool_surface.py:39-44, the guard. import mcp is attempted before the script under test is touched; an ImportError is swallowed, and the else branch raises AssertionError("the probe's blocker did not block the SDK"). tests_py/test_tool_surface.py:97 asserts result.returncode == 0, so a fired guard fails the test.
• Can the guard pass vacuously? Two failure modes, both closed:
  1. Blocker inert, guard silently satisfied anyway. Measured, not assumed: I re-ran the probe with the old find_module/load_module blocker substituted and everything else identical, on the same interpreter (.venv/bin/python, CPython 3.12.11). Result: returncode: 1, stderr AssertionError: the probe's blocker did not block the SDK. The guard fires precisely on the class of defect that made the previous version vacuous.
  2. import mcp failing for a reason other than the blocker (SDK simply absent), which would let the guard pass without proving anything. Closed structurally: tests_py/test_tool_surface.py:17 is a module-scope from mcp.server.mcpserver import MCPServer, so the test module cannot even be collected in an environment without the SDK, and the probe runs under sys.executable (tests_py/test_tool_surface.py:94), i.e. the same interpreter and same site-packages. In any environment where this test runs at all, the SDK is importable and the only thing that can stop import mcp is the blocker.
• Craftsmanship: hoisting the probe to a module constant keeps the test body at 12 lines (tests_py/test_tool_surface.py:87-98), under the 40-line cap; the rationale comment at lines 23-26 records why find_spec and not the legacy pair.

2. The before/after pair, measured

Created a detached worktree at c7104395 under .claude/worktrees/rv599-before (removed afterwards; git worktree list confirms it is gone), copied the committed test file in verbatim with git show ea927622:tests_py/test_tool_surface.py, and ran the single test with the predictions worktree's venv:

git worktree add --detach .claude/worktrees/rv599-before c7104395
git show ea927622:tests_py/test_tool_surface.py > tests_py/test_tool_surface.py
.../predictions/.venv/bin/python -m pytest \
  tests_py/test_tool_surface.py::test_the_verifier_imports_without_the_mcp_sdk -q -p no:randomly

BEFORE (c7104395): 1 failed in 4.19s, with exactly the CI symptom:

File ".../rv599-before/scripts/verify_mcp_hosts.py", line 44, in <module>
  from mcp_server.tool_surface import (  # noqa: E402
File ".../rv599-before/mcp_server/tool_surface.py", line 17, in <module>
  from mcp_server import (
File ".../rv599-before/mcp_server/tool_registry_advanced.py", line 10, in <module>
  from mcp.server.mcpserver import MCPServer
File "<string>", line 8, in find_spec
ImportError: mcp

The module-scope import at scripts/verify_mcp_hosts.py:44 on c7104395 pulls the SDK through tool_surface → tool_registry_advanced, and the blocker catches it. That is the mcp-host-config breakage, reproduced by the test.

AFTER (ea927622): 5 passed in 5.26s for the whole file (UV_NO_SYNC=1 uv run pytest tests_py/test_tool_surface.py -q -p no:randomly).

The pair the previous round asked for now exists and is the property that was missing: the test distinguishes the two commits.

3. scripts/verify_mcp_hosts.py -- hypermnesia-mcp: six cases, bounds check not skipped

UV_NO_SYNC=1 uv run python scripts/verify_mcp_hosts.py -- hypermnesia-mcp   # exit 0
PASS claude-code/full  (59 tools)   PASS claude-code/lean  (10 tools)
PASS gemini-cli/full   (59 tools)   PASS gemini-cli/lean   (10 tools)
PASS codex-cli/full    (59 tools)   PASS codex-cli/lean    (10 tools)

The late import does not weaken the check. scripts/verify_mcp_hosts.py:53-61 is a pure delegation to mcp_server.tool_surface.full_tool_bounds under # noqa: PLC0415; _verify_tool_surface's else branch (scripts/verify_mcp_hosts.py:113-119) calls it unconditionally for every full profile. I did not take "the branch ran" on trust: importing the module and monkeypatching full_tool_bounds to lambda: (1, 2), then re-calling _verify_tool_surface with the same 59-tool response, raises ContractError: x/full: discovered 59 full tools; expected 1..2, whereas the unpatched call returns 59. The branch executes, reads the derived bounds, and raises on violation. full_tool_bounds() returns (57, 60); 59 is inside because two of the three optional upstream integrations are installed here.

4. Gates on ea927622 (predictions worktree, git status --short empty before and after; nothing edited, committed or pushed there)

• UV_NO_SYNC=1 uv run ruff check . → All checks passed!
• UV_NO_SYNC=1 uv run ruff format --check . → 1479 files already formatted
• UV_NO_SYNC=1 uv run python scripts/check_craftsmanship.py --base origin/main → Craftsmanship gate: OK
• UV_NO_SYNC=1 uv run python scripts/check_doc_claims.py → doc claims OK (1 declared not-a-claim exemption(s)), the same pre-existing CONTRIBUTING.md:36 exemption as prior rounds
• UV_NO_SYNC=1 uv run python scripts/check_project_wiki.py → Project wiki mirrors match their canonical sources.
• .venv/bin/python -m pyright mcp_server/ → 0 errors, 0 warnings, 0 informations
• UV_NO_SYNC=1 uv run pytest -q → 8772 passed, 8 skipped, 2 warnings, 459 subtests passed in 473.03s, exit 0, exactly the figure the PR claims. (Worth recording: run in the properly-configured worktree rather than a cp -r throwaway, the skip count is 8, not the inflated 165 the previous four rounds reported. The inflation was the copy technique, as those rounds suspected; the PR's number was right.)

5. CI on this exact head

gh pr checks 599 --repo cdeust/Cortex and, pinned to the sha rather than to "latest", gh api repos/cdeust/Cortex/commits/ea927622/check-runs → [{"conclusion":"success","n":25},{"conclusion":"skipped","n":4}]. 25 pass, 4 skipping, zero failures. Validate MCP host configurations, the job this whole fix chain exists for, passes in 1m16s.

Move 0 (seen-defect check, coding-standards.md §14)

No rationalization of a seen defect anywhere in the head commit message or the PR body. The opposite: ea927622's message states the vacuity plainly ("A test that cannot fail is not a pin ... the vacuity is mine") and records the measured both-directions result. Nothing is deferred without an issue number.

Sanity pass on the rest of the PR (previously approved, not re-reviewed)

git diff --stat origin/main...HEAD: 39 files, +1608/-42, unchanged in scope from the approved rounds. Layering holds as declared: mcp_server/core/calibration.py pure, mcp_server/infrastructure/prediction_store.py + the two schema modules in infrastructure, the three handlers as composition roots; the craftsmanship gate agrees. The only change since ebe8a6be is tests_py/test_tool_surface.py (+31/-20), one file, no production code touched.

Issues

Blocking

None. The single blocking finding of the previous round is closed, and closed with the evidence that was missing: the corrected test fails on the broken commit and passes on this one.

Non-blocking

• Disk hygiene, outside the diff: a leftover measurement worktree predictions/.claude/worktrees/zetetic-fail-before-1302 (detached at c7104395) is still registered in git worktree list. It is gitignored (.gitignore:60) so it is not in the PR and blocks nothing, but it is the author's own before/after measurement mount left behind; git worktree remove it when convenient.
• Unchanged from earlier rounds: the probe's sys.path.insert(0, ".") makes the test cwd-dependent (it needs pytest invoked from the repo root, which is how CI runs it). Pre-existing, not introduced here.

Hand-offs

None required.

Verdict

APPROVE. The regression test now does the job it is named for: measured against c7104395 it fails with ImportError: mcp raised through from mcp.server.mcpserver import MCPServer at mcp_server/tool_registry_advanced.py:10, reached from the module-scope import at scripts/verify_mcp_hosts.py:44; against ea927622 it passes. Its self-check is itself non-vacuous, proven by running an inert legacy blocker through it and watching the AssertionError fire. All six host-verifier cases pass with the full-profile bounds check demonstrably executing, every gate is clean, the full suite is 8772 passed / 8 skipped exactly as claimed, and CI is 25 pass / 4 skipping on this head. Ready to merge.