Skip to content

[Issue #736] ADR: Nullable Optional Fields#855

Merged
jcrichlake merged 3 commits into
mainfrom
736-optional-fields-nullability
May 26, 2026
Merged

[Issue #736] ADR: Nullable Optional Fields#855
jcrichlake merged 3 commits into
mainfrom
736-optional-fields-nullability

Conversation

@jcrichlake
Copy link
Copy Markdown
Collaborator

Summary

Changes proposed

Adds ADR-0024 documenting the decision on optional field nullability in the CommonGrants protocol spec and SDKs.

Context for reviewers

This ADR resolves the question of whether optional fields in the base protocol (TypeSpec-generated) should be nullable. The current situation: both the Python SDK
(Optional[X]) and TypeScript SDK (.nullish()) render optional fields as nullable, but the base protocol declares them as optional-but-non-nullable. Once the
resolveAnyOf() bug in cg check spec is fixed (#735), both SDKs will fail validation.

Decision: Make optional fields nullable in the base protocol (Option 1).

The key insight driving this decision is that publishers need to express three distinct field states — "not provided" (field absent), "doesn't apply" (explicit
null), and "has a value" — and this three-state model is only achievable with a nullable spec, especially for scalar types like dates where no natural sentinel
value exists.

No code changes are included in this PR; the ADR documents the rationale so the corresponding TypeSpec and implementation changes can follow in a subsequent PR.

Additional information

The three options evaluated in the ADR:

Option Description Chosen?
Option 1 Make optional fields nullable in the base protocol
Option 2 Keep non-nullable; update SDKs to remove .nullish() / Optional[X]
Option 3 Allow both representations; treat null and absent as equivalent

@github-actions github-actions Bot added website Issues related to the website typescript Issue or PR related to TypeScript tooling labels May 26, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 26, 2026

🚀 Website Preview Deployed!

Preview your changes at: https://cg-pr-855.billy-daly.workers.dev

This preview will be automatically deleted when the PR is closed.

@jcrichlake jcrichlake changed the title Initial ADR [Issue #736] ADR: Nullable Optional Fields May 26, 2026
@SnowboardTechie SnowboardTechie self-requested a review May 26, 2026 15:03
Copy link
Copy Markdown
Collaborator

@SnowboardTechie SnowboardTechie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@SnowboardTechie
Copy link
Copy Markdown
Collaborator

👍 — approved.

Heads up that this ADR has handler-level implications the SDK PoCs didn't anticipate. The TypeScript transforms PoC (#825) had three coercing handlers (numberToString, stringToNumber, switchOnValue) that collapsed an explicit null source into undefined, which would have made the publisher's "doesn't apply" assertion unobservable through toCommon / fromCommon once ADR-0024 lands.

I've pushed alignment commits to #825 that:

  • preserve null through the coercing handlers (return type widens accordingly)
  • add an opt-in case: { "null": "..." } to match / switch for target-side translation, while keeping pass-through as the default
  • document the three-state contract in the SDK README for custom-handler authors
  • exercise the round-trip end-to-end via the existing example (source_url: null survives both directions)

ADR-0024's "SDKs already reflect this model and require no changes" framing is accurate for the validation surface (.nullish() already accepts both), but the transforms layer is downstream of that and needs its own pin. Worth a follow-up against the Python PoC (#810) for the same alignment — TS leads, Python parity tracked there.

No changes needed in ADR-0024 itself.

@jcrichlake jcrichlake merged commit 8533527 into main May 26, 2026
5 checks passed
@jcrichlake jcrichlake deleted the 736-optional-fields-nullability branch May 26, 2026 17:03
@github-actions
Copy link
Copy Markdown
Contributor

🗑️ Preview Cleaned Up

The preview for this PR has been automatically deleted.

SnowboardTechie added a commit that referenced this pull request May 26, 2026
The "Null handling" cross-SDK note claimed the Python ADR-0024 parity
follow-up was "tracked there" and linked #810. Reading #810 directly (body +
diff) confirms it is the base Python Transform PoC (closes #799, merged
2026-05-13), scoped "to the transform layer only" with no null/three-state
content — its coercing handlers collapse absent and null into `None`
(`str(val) if val is not None else None`; `if val is None: return None`).
ADR-0024 (PR #855) was created and merged 2026-05-26, 13 days after #810, so
#810 neither did the parity work nor tracks it. No dedicated Python-parity
issue exists.

Reword: drop the false "tracked there" claim, link ADR-0024 directly as the
contract Python must meet, state plainly that Python predates the ADR and that
handler parity is a pending follow-up. No tracker is asserted.

Docs-only; prettier clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

typescript Issue or PR related to TypeScript tooling website Issues related to the website

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ADR: Should optional fields in the CommonGrants protocol be nullable?

2 participants