Skip to content

Add Custom Metadata overlay for SDK groups and PropertyDefinition narrowing#101

Open
chinmaya-singal-glean wants to merge 1 commit into
mainfrom
custom-metadata-overlay
Open

Add Custom Metadata overlay for SDK groups and PropertyDefinition narrowing#101
chinmaya-singal-glean wants to merge 1 commit into
mainfrom
custom-metadata-overlay

Conversation

@chinmaya-singal-glean
Copy link
Copy Markdown
Contributor

@chinmaya-singal-glean chinmaya-singal-glean commented May 7, 2026

Summary

Stacked on #100 — depends on the transformer fix landing first so the path keys end up at /rest/api/index/... (rather than the doubled-prefix variant the transformer currently produces for path-level-server endpoints).

  • Groups all 5 Custom Metadata endpoints under indexing.customMetadata in generated SDKs and applies clean method names (upsert, delete, getSchema, upsertSchema, deleteSchema).
  • Introduces a CustomMetadataPropertyDefinition schema that exposes only name, propertyType, and skipIndexing, and re-points CustomMetadataSchema.metadataKeys.items to it. The shared PropertyDefinition schema (used by the Datasource / Custom Properties API) carries fields like displayLabel, displayLabelPlural, uiOptions, hideUiFacet, uiFacetOrder, group that are not relevant to Custom Metadata. We can't strip them from PropertyDefinition itself without affecting the Indexing API surface, so we narrow via a separate schema.
  • Registers the overlay in both glean-api-specs and glean-indexing-api-specs.

Stacking

Test plan

  • pnpm test — all tests pass

🤖 Generated with Claude Code

…rowing

Stacked on #100 — relies on the transformer fix to produce path keys under
/rest/api/index, which is what this overlay's targets are written against.

- Groups all 5 Custom Metadata endpoints under indexing.customMetadata in
  generated SDKs and applies clean method names (upsert, delete, getSchema,
  upsertSchema, deleteSchema).
- Introduces a CustomMetadataPropertyDefinition schema that exposes only
  name, propertyType, and skipIndexing, and re-points
  CustomMetadataSchema.metadataKeys.items to it. The shared PropertyDefinition
  schema (used by the Datasource / Custom Properties API) carries fields
  that are not relevant to Custom Metadata; we cannot strip them from
  PropertyDefinition itself without affecting the Indexing API surface.
- Registers the overlay in both glean-api-specs and glean-indexing-api-specs.

The Custom Metadata endpoints themselves are still x-internal: true upstream
in scio. This overlay is a no-op until that flag is removed in Phase 2;
landing it here ahead of time so the SDK shape is in place when GA flips.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@chinmaya-singal-glean chinmaya-singal-glean marked this pull request as ready for review May 7, 2026 16:32
@chinmaya-singal-glean chinmaya-singal-glean requested a review from a team as a code owner May 7, 2026 16:32
Comment on lines +34 to +40
# Introduce a Custom Metadata-specific PropertyDefinition schema that exposes only
# the fields applicable to Custom Metadata. The shared PropertyDefinition schema
# (used by the Datasource / Custom Properties API) carries fields like displayLabel,
# displayLabelPlural, uiOptions, hideUiFacet, uiFacetOrder, and group that are not
# relevant to Custom Metadata. We cannot strip those fields from PropertyDefinition
# itself without affecting the Indexing API surface, so we create a
# narrower schema scoped to Custom Metadata and re-point CustomMetadataSchema to it.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This makes sense, but it's unfortunate to need an overlay here.

I presume it's not feasible to change the upstream so that the type that is actually shared is one that has the common properties because the type is currently in use?

Base automatically changed from honor-path-level-servers-in-transformer to main May 13, 2026 23:31
@steve-calvert-glean
Copy link
Copy Markdown
Contributor

A few things worth flagging before this lands.

1. PR description is stale

The body says:

The Custom Metadata endpoints themselves are still x-internal: true upstream in scio. This overlay is a no-op until that flag is removed in Phase 2.

That's no longer accurate as of today — source_specs/indexing.yaml was updated earlier today (commit 1d56839, source SHA 503e49e8…) and the Custom Metadata path keys (/document/{docId}/custom-metadata/{groupName} and /custom-metadata/schema/{groupName}) are now in the source spec without x-internal: true. So this overlay will take effect immediately when merged on top of #100, not in a follow-up phase. Worth updating the body so the merge audit trail isn't misleading.

2. The CustomMetadataSchema target may not match anything

Grepping the current source spec for any schema named CustomMetadataSchema — it only appears as x-exportParamName: CustomMetadataSchema on a requestBody whose content: {} is empty. There's no actual schema defined under components.schemas.CustomMetadataSchema. So this action:

- target: $["components"]["schemas"]["CustomMetadataSchema"]["properties"]["metadataKeys"]
  update:
    items:
      $ref: "#/components/schemas/CustomMetadataPropertyDefinition"

…has nothing to update. The new CustomMetadataPropertyDefinition schema does get added (the $["components"]["schemas"] action successfully adds a new key), but nothing in the spec ends up $ref-ing it.

Could you confirm whether (a) upstream is going to add a real CustomMetadataSchema definition before the GA target, at which point this narrowing starts working as intended, or (b) the schema is already defined upstream and the local source spec just hasn't picked it up yet? If neither, the narrowing half of this PR is currently dead code.

3. Minor: no overlay test

Not blocking, but tests/post_transform_smoke.test.js would be the natural place to assert that after Speakeasy applies overlays, the five Custom Metadata operations land under x-speakeasy-group: indexing.customMetadata. Otherwise there's no signal if a future overlay reorder or upstream schema rename quietly breaks the SDK grouping.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants