Files
XZBT-NGN/docs/implementation_plan.md
2026-09-14 13:01:45 -07:00

14 KiB

Step 4 MVP — Triage of Independent Review Findings

A. Executive Triage Verdict

The Step 4 MVP is sound. The independent review confirmed 20/20 tests, generic host implementation, and no domain vocabulary in production source. None of the five findings invalidate the MVP.

After triage against the actual Contract 5.2 text, the current implementation, and the authoring guide:

  • One finding (F-1) is NOT ACTUALLY A DEFECT — the fixture behavior matches the normative Contract text.
  • One finding (F-3) SHOULD BE FIXED NOW — a small README header note to prevent future re-copy confusion.
  • One finding (F-2) is SAFE TO DEFER — permissive but conformant; future restore work will need it.
  • One finding (F-4) is INFORMATIONAL ONLY — the current evidence model is sufficient for an MVP.
  • One finding (F-5) is INFORMATIONAL ONLY — already documented in the Authoring Guide.

No contract blockers exist. No owner decisions are required.

The milestone can be closed after one small documentation patch (F-3 README header).


B. F-1 — INVALID_ARGUMENTS vs INVALID_VALUE

Contract Basis

The review claims that Contract §24 and Authoring Guide Appendix A line 522 require INVALID_ARGUMENTS as a distinct error code for argument failures.

Actual Contract text (§24):

Implementations SHOULD use stable machine-readable error codes. Recommended base codes include: [...] INVALID_ARGUMENTS [...]

This is SHOULD + "recommended", not MUST. The codes are a recommended palette, not mandatory assignments.

Actual Contract text (§9.5) — the normative argument validation section:

Invoke args MUST be an object. Every argument with required: true MUST be present. Undeclared keys MUST be rejected with INVALID_VALUE. Values MUST match the declared type, and all declared enum, bounds, step, and length constraints MUST be enforced.

This is the only normative text that specifies which error code to use for argument failures, and it explicitly mandates INVALID_VALUE — including for undeclared keys, which are the most "shape-like" argument failure.

Authoring Guide Appendix A line 522:

Unknown target, invalid value, invalid session, and invalid arguments each return the correct distinct error code.

This checklist item says "the correct distinct error code" — it does not say which code that is. Reading it together with §9.5, the "correct" code for argument failures is INVALID_VALUE, because that is what §9.5 normatively requires.

INVALID_ARGUMENTS is listed in §24 as an available code but has no normative triggering condition anywhere in the Contract. It exists as a palette option for exhibits that choose to distinguish argument-shape failures from value-constraint failures, but nothing requires it.

Classification: NOT ACTUALLY A DEFECT

The fixture core returns INVALID_VALUE for all argument failures. This is exactly what Contract §9.5 mandates. The fixture is conformant.

The host already handles any error code an exhibit returns — if a future exhibit emits INVALID_ARGUMENTS, the host will surface it correctly. No host change is needed.

No code change required. The fixture is conformant with the normative Contract text.

Optionally, add one clarifying sentence to PROVENANCE.md noting that INVALID_VALUE is used for argument failures per §9.5 and that INVALID_ARGUMENTS remains available but has no normative trigger in Contract 5.2. This is strictly optional.

Blocks MVP Closure: No


C. F-2 — Descriptor Validation Strictness

Field-by-Field Requirement Analysis

Field Contract §9 Base Descriptor Normative Strength Currently Validated Classification
id Required, canonical grammar in §8 MUST (§8.1) Yes
kind Required, four kinds in §10 MUST (§10) Yes
readable Required boolean MUST (§9, §13) Yes
writable Required boolean MUST (§9, §12) Yes
requires Required array of capability IDs MUST (§9, §17) Yes
restorable Present in base descriptor example Illustrative example, not MUST No SHOULD validate — future restore logic needs it
category Present in base descriptor example Illustrative example, not MUST No Optional for MVP — no host feature uses it
label Present in base descriptor example Illustrative example, not MUST No Optional — host can default to id
unit Present in base descriptor example as null Illustrative, OPTIONAL No Optional

Key analysis: Contract §9 says "Each target returned by describe MUST include enough metadata for a generic host to inspect and operate it" and then shows a "Base descriptor" example. The section does not use RFC 2119 MUST/REQUIRED language for restorable, category, label, or unit individually. The normative MUST statements in §9 are about the descriptor being sufficient for generic operation — which id, kind, readable, writable, and requires satisfy.

The Authoring Guide line 94 lists these fields together, but as an authoring recommendation, not a host validation mandate.

Classification: SAFE TO DEFER

Current validateCatalog is permissive but not nonconformant. It checks everything needed for the host to operate: id, kind, readable, writable, requires, and kind-specific metadata (min/max/step for ranges, options for selections). The host does not currently use restorable or category for any behavior.

Defer restorable and category validation to the scenario/restore milestone (future Step 4 or Step 5 work), where restorable becomes load-bearing. Adding it now is harmless but not required.

label validation is genuinely optional — the host can and does fall back to id.

Blocks MVP Closure: No


D. F-3 — Fixture README / Genericity Framing

Classification: SHOULD FIX NOW — documentation/evidence-integrity issue with concrete future risk

This is not a product defect but a real risk to future development:

  1. The copied README.md is an upstream document that describes upstream verification and lists bugs as "found and fixed during verification" — but they are not fixed upstream. The upstream repository at G:\.vibe\SciFi-XZBT still has all three defects.

  2. A future developer or agent re-copying from the stated upstream source will silently reintroduce all defects.

  3. PROVENANCE.md is accurate, but the README is longer, more prominent, and contradicts it.

Add the following header note to the top of test-fixtures/reference-exhibits/README.md, before the existing content:

> **Local copy notice.** This directory contains locally corrected copies of
> the upstream reference exhibits. The corrections are documented in
> `../PROVENANCE.md`. The text below is the original upstream README; its
> verification claims describe the upstream state, not this corrected copy.
> Re-copying from the upstream source without reapplying corrections will
> reintroduce known defects.

Do not rewrite the upstream README body. One blockquote header is sufficient.

Blocks MVP Closure: Should be done before closure, but is not a contract blocker


E. F-4 — Browser Evidence

Sufficiency Assessment

The current evidence model is:

  • Reproducible baseline: 20/20 automated Node.js tests (npm test), fully reproducible from the repository
  • Supporting evidence: 3/3 real-browser verification via /test-fixtures/host-verification.html, manually triggered, with results described in the report

This is appropriate for an engineering MVP. The report correctly hedges: "This is local Chromium evidence, not a cross-browser certification" and "No success claim relies on mocks alone."

Must the browser suite produce a machine-captured artifact?

No. The automated suite is the reproducible baseline. The browser suite supplements it with end-to-end evidence that cannot be captured without a real browser (rendering, audio context lifecycle, iframe postMessage). Its manual nature is inherent to what it tests.

Should automated browser execution be deferred?

Yes. Automated browser execution (Playwright, Puppeteer, etc.) is CI/infrastructure work, not Step 4 MVP scope.

Classification: INFORMATIONAL ONLY

Blocks MVP Closure: No


F. F-5 — stateRevision Across Session Reconnect

Semantic Assessment

The current implementation preserves stateRevision across sessions while resetting sequence. The review notes this is an "unwritten" cross-component invariant.

Actual document coverage:

  1. Contract §14.4: "stateRevision tracks committed persistent-state history." — History is not session-scoped.
  2. Contract §16.1: "Event sequence [...] is session-scoped and resets when a new sessionId is issued." — Only sequence is declared session-scoped. stateRevision is not.
  3. Authoring Guide §H (line 200): "stateRevision tracks committed persistent-state history and does not reset with a new session"This is already explicitly stated.
  4. Authoring Guide Appendix A (line 519): "Events carry monotonic sequence, reset on new session; stateRevision does not reset with session."Also already explicit.

The invariant is written down — in the Authoring Guide, which is an authoritative project document. The Contract itself implies it through §14.4 (state history is not session-scoped) and by the asymmetry with §16.1 (only sequence is called session-scoped).

Is the current host behavior reasonable?

Yes. A host that detects stateRevision regression on reconnect is correctly enforcing the monotonicity requirement of §14.

Would an exhibit that resets stateRevision on every session violate the contract?

Yes — it would violate §14's monotonicity requirement and contradict the Authoring Guide's explicit statement.

Classification: INFORMATIONAL ONLY

The Authoring Guide already states this explicitly. A future Contract §14 revision could add one sentence for completeness, but this is not a gap that needs closing before the MVP.

Blocks MVP Closure: No


G. Required Owner Decisions

NO OWNER DECISIONS REQUIRED


H. Minimal Completion Patch List

Patch 1 (the only patch): README copy-notice header

Attribute Detail
File test-fixtures/reference-exhibits/README.md
Change Add a blockquote header note (5 lines) at the top of the file, before existing content
Why Prevents future re-copy confusion; distinguishes local corrected copy from upstream claims
Verification Visual review of the added header; confirm existing README body is unchanged; git diff --check

Exact content:

> **Local copy notice.** This directory contains locally corrected copies of
> the upstream reference exhibits. The corrections are documented in
> `../PROVENANCE.md`. The text below is the original upstream README; its
> verification claims describe the upstream state, not this corrected copy.
> Re-copying from the upstream source without reapplying corrections will
> reintroduce known defects.

No other patches are required for MVP closure.


I. Safe Deferrals

Item Defer To Reason
validateCatalog adding restorable/category checks (F-2) Scenario/restore milestone Not needed until restore logic depends on restorable; current behavior is permissive but conformant
Automated browser test execution (F-4) CI/testing infrastructure work Infrastructure concern, not an MVP requirement
Contract §14 explicit stateRevision session-persistence sentence (F-5) Future Contract revision Already stated in the Authoring Guide; not blocking
Optional PROVENANCE.md note about INVALID_VALUE per §9.5 (F-1) Any convenient time Purely informational; the fixture is already conformant
Report framing refinements (review §5 recommendations) Owner's discretion Report accuracy improvements; do not block the MVP

The following paragraph should be used in future reports to describe the genericity result:

A single unmodified XZBT-NGN host drove three reference exhibits — Aquarium (12 targets), Planetarium (14 targets), and Haunted House (13 targets) — copied from the upstream AbacusAI reference set and locally corrected to conform to Contract 5.2. The corrections addressed non-standard descriptor kinds, a misnamed metadata field, missing argument validation, and a stale internal ID mapping; they are documented in test-fixtures/PROVENANCE.md. The host contains no exhibit-domain vocabulary and required no fixture-specific logic. The upstream reference source was not modified.

This wording makes clear that:

  • The NGN host remained generic and unmodified
  • Fixture corrections were necessary and load-bearing
  • Corrections were Contract-oriented (conformance repairs), not host-specific exceptions
  • Upstream reference sources were not modified
  • The corrections are documented and traceable

K. Final Triage Verdict

STEP 4 MVP REQUIRES MINOR COMPLETION PATCHES

One documentation patch (F-3 README header) should be applied before changing the milestone verdict. All other findings are either not defects, informational, or safely deferred.

After that patch, the milestone verdict should become:

STEP 4 MVP COMPLETE — READY FOR NEXT STEP 4 PHASE