PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The replacement PR now tests and uses the better existing substrate identified in the cycle-1 Drop+Supersede: an explicit OpenAPI union, compiled by the existing OpenAPI→Zod bridge, with a focused executable regression. The head is merge-safe without another author cycle; the remaining Kimi projection question is environment-bound post-merge validation, not deferred server correctness.

Peer-Review Opening: Iris — this is the right superseding shape. The raw-wire evidence from the dropped docs PR is preserved, while the untested “no Neo-side repair exists” inference has been replaced by a concrete schema repair and executable falsifier.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #15678 including its amended Contract Ledger and revalidation comments; the two-file changed-surface list; current dev CallMethodRequest, openApiValidator.mjs, InstanceService.callMethod(), and the existing Neural Link string/object oneOf precedent; the prior terminal review on #15683; ADR-0019; the canonical unit-test placement rules; Memory Core prior art; and the Knowledge Base’s Neural Link/OpenAPI ownership references.
• Expected Solution Shape: Replace only the ambiguous args.items: {} sentinel with an explicit JSON-value class union that the existing compiler can validate and expose, leaving method dispatch and writer authority untouched. The schema must not hardcode Kimi-specific coercion or point generic MCP consumers at an in-process fixture path; the regression belongs in the existing validator-compliance unit surface and must independently check emission plus representative JSON values.
• Patch Verdict: Matches and improves the expected shape. CallMethodRequest.args.items.oneOf now declares string/null, number, boolean, open object, and array branches; exact-head emission produces those five top-level anyOf branches; InstanceService remains a transparent args pass-through; and the focused spec checks both emitted shape and runtime parsing.
• Premise Coherence: Coheres with verify-before-assert and friction→gold: the earlier premature docs workaround was dropped, its evidence was salvaged, and the replacement turns the untested explicit-union hypothesis into executable contract evidence without importing client-specific behavior into Neo’s server boundary.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15678
• Related Graph Nodes: Related: #15683, #15681; Neural Link OpenAPI→Zod tool-shape compiler; Kimi schema projection boundary

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: The exact-head emitted array branch deliberately still contains items: {}. The top-level ambiguity that blocked native object arguments is gone, and deeper nested arrays parse correctly, so this is not a merge blocker. The remaining unverified assumption is client-specific: Kimi must preserve the top-level union and may still narrow nested array items. The declared post-merge receipt should compare raw tools/list with the Kimi-facing schema; a remaining collapse belongs in the upstream/follow-up path rather than another server-schema cycle.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: the explicit-union framing matches the diff and exact-head emission; the Kimi result is explicitly labeled post-merge validation.
• Anchor & Echo summaries: the test comment describes the durable tool-shape contract and does not encode ticket/session chronology.
• [RETROSPECTIVE] tag: none added.
• Linked anchors: the amended ticket and dropped predecessor establish the stated salvage path.

Findings: Pass — no framing overshoot remains in the shipped schema or test.

🧠 Graph Ingestion Notes

• [KB_GAP]: The Knowledge Base identifies OpenAPI and the validator as the owning surfaces but does not yet explain the specific oneOf → z.union → emitted anyOf compatibility path; exact source and executable coverage supplied the authority here.
• [TOOLING_GAP]: None affecting the PR. The reviewer’s first standalone probe used the wrong js-yaml ESM import form; retrying with the repository’s namespace-import pattern produced the expected exact-head schema.
• [RETROSPECTIVE]: This successor demonstrates the useful Drop+Supersede shape: preserve the cross-harness receipt, discard unproven client-facing prose, and convert the live hypothesis into a small executable wire-contract repair.

🎯 Close-Target Audit

• Close-targets identified: #15678 from the newline-isolated PR-body Resolves line; no magic close keyword appears in the commit message.
• #15678 is labeled bug, documentation, and ai, not epic.

Findings: Pass.

📑 Contract Completeness Audit

• The amended #15678 body contains a Contract Ledger covering server emission, runtime validation, Kimi projection, generic MCP access, and the separate write-lock sequel.
• The diff matches that ledger: explicit top-level JSON-value union, open-object and array branches, focused validator coverage, caller-reachable description text, and no write-lock changes.

Findings: Pass — no contract drift.

🪜 Evidence Audit

• The PR body declares L1 validator evidence and names the Kimi projection as Post-Merge Validation.
• Exact-head CI plus the focused emission/parser proof cover server correctness; the Kimi-facing projection cannot consume the unmerged head through a verified branch-artifact route.
• The residual is explicitly owned in the PR’s Post-Merge Validation section and the ticket’s Kimi-projection Contract Ledger row.
• No L1 evidence is promoted to a Kimi runtime receipt; failure of the post-merge projection routes to the upstream/follow-up path.

Findings: Pass — the two evidence ceilings are kept distinct.

📡 MCP-Tool-Description Budget Audit

• The modified args description is a single 182-character line.
• It contains no internal tickets, sessions, phases, or architectural history.
• It is usage-focused: accepted native JSON classes plus verbatim string behavior.
• It remains far below the 1024-character cap.

Findings: Pass.

🔌 Wire-Format Compatibility Audit

• The change makes the existing heterogeneous contract explicit; it does not narrow values accepted by runtime dispatch.
• buildZodSchemaFromNode() already maps oneOf to z.union, and exact-head emission maps the disjoint branches to anyOf without losing nullability or open-object semantics.
• InstanceService.callMethod() still forwards the args array unchanged; no downstream service or App Worker protocol changed.
• Neural Link already carries a string/object oneOf precedent, and current-head CI exercises strict-client schema compliance.
• The environment-specific Kimi consumer remains covered by the explicit post-merge comparison rather than being assumed from server evidence.

Findings: Pass — backward-compatible schema clarification with a bounded consumer receipt.

🔗 Cross-Skill Integration Audit

• No new MCP tool, operation, workflow primitive, or calling convention is introduced.
• The existing Neural Link Capability Matrix continues to classify call_method as admin; request-schema typing does not change its authority tier.
• The tool’s own consumed description was updated at the owning OpenAPI surface.
• Repository-wide searches found no skill or guide that encodes the old items: {}/Array<unknown> shape and therefore needs a synchronized edit.

Findings: All checks pass — no integration gaps.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI is fully green at 214ec45d30a6901808d323d2e538edf6b36f53c7; author receipts report 45 focused validator tests and 413 MCP tests passing.
• Reviewer falsifier: exact-head standalone compiler probe confirmed five top-level emitted branches, nullable: true, additionalProperties: true, preserved nested-array items, and successful parsing of null, strings, integers/floats, booleans, open/nested objects, empty arrays/objects, and deeper mixed arrays.
• Test location: pass — the regression extends the existing right-hemisphere OpenAPI validator-compliance spec under test/playwright/unit/ai/mcp/validation/.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.

• [ARCH_ALIGNMENT]: 100 - Checked and cleared the relevant failure modes: no Kimi-specific hardcoding, no coercion layer, no dispatch mutation, no duplicated compiler logic, and no misplaced test; the contract stays in OpenAPI and the proof stays in validator compliance.
• [CONTENT_COMPLETENESS]: 100 - No methods were added or modified; the schema description is caller-focused, the test carries a durable rationale block, and the PR body supplies the Contract Ledger, evidence boundary, supersession delta, and post-merge owner.
• [EXECUTION_QUALITY]: 96 - Exact-head CI, diff check, structure map, schema emission, and representative parse probes are green; four points remain because the Kimi-facing projection is necessarily unverified until the merged schema reaches that seat.
• [PRODUCTIVITY]: 96 - The PR delivers the ticket’s Neo-side repair and executable guard in 35 added lines; the last four points reflect the explicitly retained Kimi projection receipt.
• [IMPACT]: 72 - This repairs native object-bearing arguments for a client that collapsed the prior unknown-item schema and strengthens the shared MCP wire contract, but it is scoped to one admin-tier Neural Link operation.
• [COMPLEXITY]: 28 - Two existing files, one inline five-branch union, and one focused compliance test create low implementation and review load; the main complexity is cross-client schema projection rather than code volume.
• [EFFORT_PROFILE]: Quick Win - A narrow schema-and-test delta yields high interoperability value while reusing the existing compiler and test substrate.

Clean successor. The exact head is ready for the human merge gate; the post-merge Kimi projection remains the honest final consumer receipt.