Skip to content

Moonshot schema composition can weaken overlapping nested property constraints #2763

Description

@Ingwannu

Summary

Current dev (8b1b65b8d858d3b92e4fd4852e9e368d43e73f39) can weaken a JSON Schema assertion while normalizing sibling $ref nodes for first-party Kimi/Moonshot chat destinations.

composeProperties in src/adapters/openai-chat.ts correctly keeps properties that exist on only one side, but when the referenced target and the sibling node both define the same property as object schemas, it shallow-merges their member schema and lets the sibling overwrite every duplicate keyword except required.

Deterministic failure path

A referenced target containing:

{"properties":{"shared":{"type":"string","minLength":5}}}

combined with a sibling node containing:

{"properties":{"shared":{"type":"string","minLength":1}}}

currently emits shared.minLength = 1. Under JSON Schema 2020-12 both schemas apply, so the intersection must retain the stricter lower bound (5). The normalized schema silently permits values the original schema rejected.

This is the nested-property equivalent of the top-level bound bug already covered by the MOONSHOT_BOUND_KEYWORDS logic. The existing shared-property regression only covers a sibling that omits minLength, so it cannot distinguish correct composition from the current shallow overwrite.

Scope and expected fix

  • Keep the behavior limited to the existing first-party Kimi/Moonshot normalization gate.
  • Compose overlapping member schemas without weakening either side. At minimum, lower bounds should take the maximum and upper bounds the minimum, consistent with the existing top-level rule; recursively composing supported object assertions or retaining an explicit intersection are also valid approaches.
  • Do not broaden this into a general OpenAI-chat schema rewrite.
  • Add focused regressions for both directions: a looser sibling must not relax the target, and a tighter sibling must still narrow it.
  • Preserve the existing depth/node/ref budgets and cyclic/unresolvable-ref fallback.

This was surfaced as an outside-diff review finding on #2350, but #2350 does not introduce or modify this composition logic and should not be blocked by it: #2350 (review)

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    landed-via-maintainerOriginal PR closed after landing via a maintainer merge train

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions