Skip to content

test(wasm-sdk): remove invalid position from document-type root in fixtures#3524

Merged
QuantumExplorer merged 1 commit intov3.1-devfrom
fix/wasm-sdk-tests-position-meta-schema
Apr 23, 2026
Merged

test(wasm-sdk): remove invalid position from document-type root in fixtures#3524
QuantumExplorer merged 1 commit intov3.1-devfrom
fix/wasm-sdk-tests-position-meta-schema

Conversation

@QuantumExplorer
Copy link
Copy Markdown
Member

@QuantumExplorer QuantumExplorer commented Apr 23, 2026

Issue being fixed or feature implemented

The scheduled v3.1-dev Tests job has been failing the Run WASM SDK functional tests step: run 24641468414, job 72046508655 — 6 failures / 82 passing.

What was done?

Removed four stray position: N entries placed at the document-type root of the test fixture schemas in contracts.spec.ts and documents.spec.ts. position is only meaningful inside a property definition (to order fields in the serialized document row); it is not a valid document-type-level config key.

Also replaced the misleading comment ("Position property is required for document types and their properties") with an accurate note about where position belongs.

Root cause

The v0 document meta-schema silently accepted unknown root-level keys because additionalProperties was unconstrained. #3475 (merged 2026-04-12, gated on protocol v12, closing #2703) added additionalProperties: false to the v1 meta-schema. With v12 live, these stray keys are correctly rejected and fail the test fixtures:

state transition broadcast error: JsonSchemaError:
  Additional properties are not allowed ('position' was unexpected), path:

Four of the six failures were cascading expected null to exist assertions in tests (contractUpdate, documentReplace, documentDelete, documentTransfer) that all depend on the contract publish succeeding first.

How Has This Been Tested?

  • Verified all remaining position: lines in the changed files are at the property level (14-space indent), not the document-type root (10-space indent).
  • Unit fixtures at packages/wasm-sdk/tests/unit/fixtures/*.ts were audited and already place position correctly inside properties.*.

Breaking Changes

None — test-only change.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

Summary by CodeRabbit

  • Tests
    • Updated contract and document test schemas to comply with protocol v12's strict meta-schema validation
    • Repositioned position field from document-type root to property level for field ordering
    • Enhanced documentation clarifying validation rules and schema structure requirements

…fixtures

Four test fixtures in the wasm-sdk functional test suite placed
`position: N` at the document-type root (outside `properties`). This key
is only valid inside property definitions — it's how DPP orders fields
in the serialized document row, not a document-type-level config.

The v0 document meta-schema silently accepted stray root-level keys
because `additionalProperties` was unconstrained. PR #3475 (merged
2026-04-12, gated on protocol v12) added `additionalProperties: false`
to the v1 meta-schema, which correctly rejects the stray `position`.

Since protocol v12 went live this caused the `Run WASM SDK functional
tests` step to fail with:

    state transition broadcast error: JsonSchemaError:
      Additional properties are not allowed ('position' was unexpected), path:

on contract publish, with cascading `expected null to exist` failures on
the 4 downstream tests that depend on the contract being published.

Also replace the misleading comment `"Position property is required for
document types and their properties"` with an accurate note about where
`position` belongs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v3.1.0 milestone Apr 23, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d11376eb-787a-4641-b041-52d880026829

📥 Commits

Reviewing files that changed from the base of the PR and between af35dbd and 79c10f5.

📒 Files selected for processing (2)
  • packages/wasm-sdk/tests/functional/transitions/contracts.spec.ts
  • packages/wasm-sdk/tests/functional/transitions/documents.spec.ts

📝 Walkthrough

Walkthrough

Test schemas for data contracts are updated to remove position from document-type root objects, relocating it exclusively to property definitions. Inline comments are enhanced to clarify that protocol v12 enforces strict schema validation and rejects unexpected root-level keys.

Changes

Cohort / File(s) Summary
Test Schema Updates
packages/wasm-sdk/tests/functional/transitions/contracts.spec.ts, packages/wasm-sdk/tests/functional/transitions/documents.spec.ts
Removed position from root-level document-type objects (note, task, mutableNote, transferableItem) and updated comments to clarify that position belongs only in property definitions and that protocol v12 enforces strict root-level schema validation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 The schemas dance with proper grace,
Position finds its rightful place—
No root-level keys to cause a fuss,
Protocol v12 trusts in us!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: removing invalid position entries from document-type root in test fixtures, with appropriate scope and context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wasm-sdk-tests-position-meta-schema

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented Apr 23, 2026

Review Gate

Commit: 79c10f56

  • Debounce: 1m ago (need 30m)

  • CI checks: checks still running (1 pending)

  • CodeRabbit review: comment found

  • Off-peak hours: off-peak (11:54 PM PT Wednesday)

  • Run review now (check to override)

@QuantumExplorer
Copy link
Copy Markdown
Member Author

Reviewed AI based PR.

@QuantumExplorer QuantumExplorer merged commit d2c33d6 into v3.1-dev Apr 23, 2026
14 checks passed
@QuantumExplorer QuantumExplorer deleted the fix/wasm-sdk-tests-position-meta-schema branch April 23, 2026 06:59
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.

2 participants