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

  1. lidge-jun commented on Aug 27, 2026

    @lidge-jun
    Owner

    리뷰 · 우선순위 76 / 80

    이슈는 지금 dev HEAD 8b1b65b8d의 Moonshot/Kimi JSON Schema 정규화에서, 형제 $ref와 타깃이 같은 중첩 properties 멤버를 가질 때 제약이 약해질 수 있다는 보고입니다. src/adapters/openai-chat.ts의 composeProperties(약 1118행 근처)는 한쪽에만 있는 프로퍼티는 잘 남기지만, 양쪽이 같은 이름을 object schema로 정의하면 멤버 스키마를 얕게 합치고 required를 제외한 중복 키는 sibling 값으로 덮어씁니다. 그래서 타깃이 shared.minLength = 5이고 sibling이 shared.minLength = 1이면 결과가 1이 되어, JSON Schema 2020-12에서 둘 다 적용되어야 하는 교집합(더 센 하한 5)보다 느슨해집니다. 최상위에는 이미 MOONSHOT_BOUND_KEYWORDS + intersectBound로 min은 max·max는 min을 취하는 규칙이 있는데, 중첩 properties 멤버 합성에는 그 규칙이 들어가지 않습니다. 이슈가 말한 "밖에 보이는 리뷰에서 #2350 옆에 붙은 발견"과도 맞습니다. #2350은 이 composition을 건드리지 않으니 그 PR을 막을 이유는 없습니다.

    라인 openai-chat.ts composeProperties 멤버 루프 - k === "required" ? unionRequired(...) : v만 특별 취급합니다. minLength/maximum 같은 bound와, 더 안쪽 properties/items 재귀 교집합은 없습니다. 이슈의 재현(타깃 minLength 5 vs sibling 1 → 1)은 현재 코드로 그대로 납니다.

    라인 openai-chat.ts normalizeMoonshotSchemaNode properties 분기 - 최상위에서는 composeProperties를 쓰고, 그 바로 옆 키에는 MOONSHOT_BOUND_KEYWORDS 교집합이 있습니다. 즉 "같은 파일 안에 이미 정답 규칙이 있는데, 한 lev el 아래 properties 멤버에만 빠져 있는" 형태입니다. 수정 범위는 기존 first-party Kimi/Moonshot 게이트 안으로 한정하는 것이 맞습니다. 일반 OpenAI chat schema 전체 rewrite로 키우면 안 됩니다.

    이슈 본문의 기대 수정 - 겹치는 멤버 스키마를 약화하지 않게 합성하고, 최소한 lower bound는 max·upper bound는 min을 취하며, object assertion 재귀 합성 또는 명시적 교집합도 허용합니다. 양쪽 방향 회귀(느슨한 sibling이 타깃을 풀지 못함, 더 센 sibling은 좁힘)와 기존 depth/node/ref 예산·순환/미해결 ref fallback 유지도 필요합니다. 지금 공유 프로퍼티 회귀가 "sibling이 minLength를 아예 안 가진 경우"만 보면, 얕은 overwrite와 올바른 교집합을 구분하지 못합니다.

    메인테이너의 판단이 필요한 지점

    • composeProperties 안에서 bound만 intersect할지, properties/items까지 재귀로 같은 규칙을 펼칠지
    • 충돌하는 type/enum처럼 bound가 아닌 키는 sibling 우선(현재)을 유지할지, fail-closed로 둘지
    • 이슈를 help wanted로 둘지, 작은 전용 PR로 바로 받을지(#2350과는 분리)

    너의 추천
    이슈는 유효하고 우선순위도 높습니다. composeProperties에 최상위와 같은 intersectBound/MOONSHOT_BOUND_KEYWORDS를 멤버 스키마에 적용하고, 가능하면 한 단계 properties 재귀까지 넣은 작은 PR을 dev 대상으로 받되 #2350과는 분리하세요. 회귀 테스트에 이슈의 minLength 5 vs 1 양방향 케이스를 그대로 넣으면 됩니다.

    이 댓글은 grok-bot이 작성했습니다

  2. Ingwannu commented on Aug 27, 2026

    @Ingwannu
    OwnerAuthor

    Implemented the bounded fix in draft PR #2764: #2764

    The pre-fix regression reproduced the weakening at both the shared-property level and one nested properties level (minLength: 5 / maxLength: 10 became 1 / 99). The patch reuses the existing lower-bound max, upper-bound min, and required union rules recursively inside overlapping property maps, while keeping the first-party Kimi/Moonshot destination gate and all existing normalization budgets.

    Focused isolated validation is 18/18 green and git diff --check passes. The PR remains Draft and this issue remains open until exact-head CI is green and a non-author maintainer reviews/merges it.

  3. lidge-jun commented on Aug 27, 2026

    @lidge-jun
    Owner

    Landed via #2764 at 3b53024

  4. lidge-jun commented on Aug 27, 2026

    @lidge-jun
    Owner

    Already on dev via #2764.

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