[validation] Close generated Pydantic vs canonical JSON Schema semantic gaps #1102

Activity

1. bokelley

   added

   bugSomething isn't workingSomething isn't working

   claude-triagingTriage routine is actively working on this issue (1-3 min)Triage routine is actively working on this issue (1-3 min)

   on Aug 29, 2026
2. bokelley commented on Aug 29, 2026

   

   bokelley

   on Aug 29, 2026

   ContributorAuthor

   More actions

   Triage

   Classification: Bug — validation gap
   Bucket(s): validation (label not in repo — flagging gap in run summary)
   Status: ready-for-human

   What the experts said:

   • ad-tech-protocol-expert: Architectural — gap is a real billing-compliance risk; ReportingRevision with missing finality_basis/finality_policy_id breaks the adcp#6953 audit chain; Option 2 (wire-boundary jsonschema validator, already partially implemented in src/adcp/validation/schema_validator.py) is the correct primary path; Option 1 (full Pydantic parity via codegen) is unsound with datamodel-code-generator and breaking.
   • code-reviewer: Analysis was still completing at post time; protocol expert covered implementation-layer findings from direct codebase inspection.

   My take: The core decision — full Pydantic conditional parity vs structural types + wire-boundary validator — is architectural with breaking-change implications that needs a maintainer call before implementation begins. A non-breaking third option exists: narrow @model_validator(mode='after') stubs in _ergonomic.py for billing-critical conditionals only (precedented by src/adcp/validation/legacy.py), but all paths hinge on whether bundled schemas already include the if/then/else blocks for ReportingRevision.

   Open questions for @bokelley:

   1. Do bundled schemas (via scripts/sync_schemas.py) already carry the if/then/else conditionals for ReportingRevision? If yes, scope narrows to _ergonomic.py stubs and CI fixtures only — wire-boundary may already be protected.
   2. Does draft PR feat(reporting): add reliable ledger reconciliation #1095 introduce these schema fields, or consume them from an upstream schema that already carries them? (Determines whether a schema re-sync must precede SDK work.)
   3. Does the idempotency replay cache short-circuit validate_response? If so, replay paths are unprotected even with Option 2 in place.

   Additional concern: the RequestContext.metadata context-echo contract (CLAUDE.md) means a structurally-incomplete ReportingRevision can replay null-filled fields into governance agents, and discriminated union shadowing may silently misroute an official revision as provisional when co-required fields are absent.

   ---

   Triaged by Claude Code. Session: https://claude.ai/code/session_01FUxGWD4ut2q3aq6UGxt9mm

   ---

   Generated by Claude Code
3. bokelley

   added

   claude-triaged

   and removed

   claude-triagingTriage routine is actively working on this issue (1-3 min)Triage routine is actively working on this issue (1-3 min)

   on Aug 29, 2026
4. bokelley

   closed this as completedin #1117on Sep 3, 2026
5. bokelley

   added a commit that references this issue on Sep 3, 2026

   fix(validation): enforce canonical schema boundaries (#1117)

   ...

   0e003a5
6. KonstantinMirin commented on Oct 3, 2026

   

   KonstantinMirin

   on Oct 3, 2026

   Collaborator

   More actions

   The gap this issue describes is still measurable on 8.0.0, and the differential
   fixtures that closed it cover one tool.

   tests/fixtures/validation-parity-cases.json holds 8 cases, all on
   get_signals, request direction only, with 3 allowlisted divergences — all
   three conditionals (else/anyOf/required, if/then/not, dependencies). The
   expected work in the description reads wider than that:

   Cover conditional required/forbidden fields, discriminated unions, dependent
   constraints, numeric/string/array bounds, and selected behavioral extensions.

   Measuring the rest: for each tool that ships a request schema, instantiate that
   schema into a payload the bundled validator declares valid, replace one leaf at
   a time with a value of a different JSON type, and compare the bundled
   validator's verdict with the generated model's. Of the 84 tools, 78 yield a
   valid payload and 72 of those also expose a request model, which is the set
   graded — get_signals is one of the six the instantiator cannot satisfy, so
   these fixtures and that measurement cover disjoint tools.

   documents compared : 39546
   divergences        : 5995
     the bundled schema accepts a document the model refuses : 4604
     the model accepts a document the bundled schema refuses : 1391
   

   17 of the 72 request models that exist refuse a payload the bundled schema
   declares valid. Two causes account for most of it, and neither is a
   conditional, so neither is covered by the three allowlisted entries:

   1. The bundled validator does not enforce format: uri. schema_loader builds
      each validator with a bare FormatChecker() plus a hand-registered
      date-time, and a bare FormatChecker() carries only the checks whose
      optional dependencies import — uri needs rfc3987, which this package does
      not depend on. format: uri is the most-used format in the 3.2 bundle and
      not one occurrence is checked. This is a defect in the Option 2 path this
      issue's triage chose as primary: the wire-boundary validator is weaker than
      the schema it loads, and weaker again or stronger depending on what else the
      adopter has installed.

   2. Pydantic's lax mode coerces where JSON Schema does not.
      ListAccountsRequest.model_validate({"sandbox": "yes"}).sandbox is True
      while validate_request("list_accounts", {"sandbox": "yes"}).valid is
      False. Measured against pydantic 2.13, bool accepts 1, 0, "1",
      "0", "yes", "true"; int accepts "1", True, 1.0; float accepts
      "1" and True. str coerces nothing, so only those three types diverge.

   Both are fixable without the full-codegen-parity path the triage ruled out: one
   format checker in schema_loader, and --strict-types bool int float on the
   generator (plus an integer annotation that admits an integral float, which JSON
   Schema counts as an integer and StrictInt does not).

   The remaining divergences — explicit null on an optional field, a date-time
   field parsing a number as a Unix timestamp, a format: uri field dropping its
   sibling pattern, the canonical model refusing a legacy creative identity the
   schema retains — each need a decision rather than a patch, and are enumerated
   with counts in the issue linked below.

   What stops this reopening on the next version bump is driving the suite off the
   bundle instead of a fixture list: enumerate the schemas, synthesize the payloads,
   and name the differences by cause rather than by tool and pointer. The three
   entries get_signals allowlists today are all one cause — a conditional
   subschema that code generation cannot express — and stated that way the entry
   covers every tool at once.

   Details, measurements and a suite built that way: #1359