Skip to content

fix: skip example checks in instance data - #64

Open
FanouZeng-TT wants to merge 3 commits into
Universal-Commerce-Protocol:mainfrom
FanouZeng-TT:fix/skip-example-keyword-instance-recursion
Open

fix: skip example checks in instance data#64
FanouZeng-TT wants to merge 3 commits into
Universal-Commerce-Protocol:mainfrom
FanouZeng-TT:fix/skip-example-keyword-instance-recursion

Conversation

@FanouZeng-TT

Copy link
Copy Markdown
Contributor

Description

check_examples walked every object and array value below a schema node. default, const, examples, and enum hold JSON instance data, so a business object such as { "type": "string", "examples": [123] } inside one of those keywords was reinterpreted as a schema and reported as E008.

Fix: skip those four instance-data keyword values during example traversal while preserving validation at real schema locations. The regression test covers all four keywords, and the existing invalid-example test continues to protect real examples arrays.

Category (Required)

  • Core Protocol: Changes to core protocol specifications. (Requires Technical Council approval)
  • Governance/Contributing: Changes to governance or contributing processes. (Requires Governance Council approval)
  • Capability: New or updated capabilities. (Requires Maintainer approval)
  • Documentation: Documentation-only changes. (Requires Maintainer approval)
  • Infrastructure: Build, CI, or deployment changes. (Requires DevOps Maintainer approval)
  • Maintenance: Dependencies and repository maintenance. (Requires DevOps Maintainer approval)
  • SDK: Language-specific SDK updates and releases. (Requires DevOps Maintainer approval)
  • Samples / Conformance: Maintaining samples and the conformance suite. (Requires Maintainer approval)
  • UCP Schema: Changes to the ucp-schema tool. (Requires Maintainer approval)
  • Community Health (.github): Organization-wide community files. (Requires DevOps Maintainer approval)

Related Issues

N/A

Checklist

  • I have followed the Contributing Guide and Code of Conduct.
  • I have updated the documentation (if applicable).
  • My changes pass all local linting and formatting checks.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • (For Core/Capability) I have included/updated the relevant JSON schemas.
  • I have regenerated Python Pydantic models by running generate_models.sh under python_sdk (not applicable).

Screenshots / Logs (if applicable)

N/A — verified with cargo test --all-targets, cargo clippy --all-targets -- -D warnings, cargo fmt --check, all pre-commit hooks, and git diff --check.

@damaz91 damaz91 added status:needs-triage Signal that the PR is ready for human triage status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Aug 16, 2026
@damaz91

damaz91 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Thanks for submitting this fix @FanouZeng-TT!

A couple of quick observations and suggestions:

  1. Property Name Collisions (under properties / $defs):
    Because check_examples does a generic JSON tree traversal, checking key.as_str() directly inside for (key, child) in map will also cause subschemas of properties named "default", "const", "examples", or "enum" to be skipped (e.g., properties: { "default": { "type": "string", "examples": [123] } }).

    To avoid missing example checks on properties with these names, we could check if we are inside a properties / $defs container (or ensure children of properties are always treated as subschemas).

  2. Singular "example" keyword (OpenAPI):
    OpenAPI 3.0 / JSON Schema Draft-04 schemas often use the singular "example" keyword for instance data. It might be worth adding "example" to the list as well:

    if matches!(key.as_str(), "default" | "const" | "examples" | "example" | "enum") {
        continue;
    }

@FanouZeng-TT

Copy link
Copy Markdown
Contributor Author

Addressed both points in f3f631c: schema maps (properties, patternProperties, $defs, and definitions) now recurse directly into their values, so property names that collide with instance-data keywords still receive E008 validation. Singular OpenAPI example values are now skipped as instance data as well. Added regression coverage for both cases; all 317 tests, clippy, fmt, pre-commit, and diff check pass.

@FanouZeng-TT

Copy link
Copy Markdown
Contributor Author

Small follow-up in aab664d: included dependentSchemas, the other object-valued schema map in Draft 2020-12, and extended the collision regression to cover it. Targeted linter tests, clippy, fmt, pre-commit, and diff check pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants