Skip to content

[codex] Add explicit interface implements#44

Merged
zth merged 2 commits into
mainfrom
codex/explicit-interface-implements
May 25, 2026
Merged

[codex] Add explicit interface implements#44
zth merged 2 commits into
mainfrom
codex/explicit-interface-implements

Conversation

@zth

@zth zth commented May 25, 2026

Copy link
Copy Markdown
Owner

Summary

Adds repeatable @gql.implements("InterfaceName") for object types and interfaces, while preserving the existing record-spread interface mechanism.

The new explicit path records implementation declarations separately from spreads, attaches them during schema processing without copying interface fields into the ReScript record, expands transitive parent interfaces required by GraphQL.js, and validates implementation parity before generated schema code is emitted.

README and interface docs now cover the explicit API alongside the existing spread style.

Validation covered

  • Missing implemented interface
  • Invalid @gql.implements payload
  • Unsupported @gql.implements target rejection
  • Missing interface field
  • Field output type mismatch
  • GraphQL output subtype compatibility for nullable/non-null fields
  • GraphQL output subtype compatibility for object-to-interface fields
  • Transitive interface parent expansion for GraphQL.js schema validity
  • Concrete-only interface resolver assets for interface-to-interface implementations
  • Runtime explicit interface return resolution
  • Runtime explicit interface field execution with extra nullable args
  • Missing field argument
  • Field argument type mismatch
  • Extra required field argument rejection
  • Extra nullable field argument acceptance
  • Interface implementing another interface
  • Interface implementation cycle rejection
  • Existing spread-based implementations still generate

Checks

  • make format
  • make test
  • make -C tests test

@zth
zth force-pushed the codex/explicit-interface-implements branch 4 times, most recently from 764b3ba to 8c19204 Compare May 25, 2026 12:13
@zth
zth marked this pull request as ready for review May 25, 2026 12:22

zth commented May 25, 2026

Copy link
Copy Markdown
Owner Author

@codex review

zth commented May 25, 2026

Copy link
Copy Markdown
Owner Author

Codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c19204b20

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/ml/GenerateSchemaValidation.ml Outdated
Comment on lines +202 to +205
interfaceField.args
|> List.iter (fun (interfaceArg : gqlArg) ->
match findArgByName implementationField.args interfaceArg.name with
| None ->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Ignore injected resolver args in interface arg parity checks

Validate only GraphQL-exposed arguments when comparing interface field signatures. This loop compares every raw resolver arg, so injected args like InjectContext, InjectInfo, or InjectInterfaceTypename are treated as schema arguments and can incorrectly produce "missing argument" errors when an override omits an injected parameter. That rejects valid implementations even though these args are intentionally non-printable and not part of the GraphQL contract.

Useful? React with 👍 / 👎.

Comment thread src/ml/GenerateSchemaValidation.ml Outdated
Comment on lines +221 to +226
implementationField.args
|> List.iter (fun (implementationArg : gqlArg) ->
match findArgByName interfaceField.args implementationArg.name with
| Some _ -> ()
| None ->
if isRequiredInputType implementationArg.typ then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Exclude injected args from extra-required-argument validation

This check also runs on raw resolver arguments, so internal injected args are considered user-facing field arguments. If an implementing resolver includes ~ctx/~info while the interface field does not, it is flagged as adding a required argument, even though injected args are not GraphQL inputs. This creates false-positive validation failures for otherwise valid interface implementations.

Useful? React with 👍 / 👎.

@zth
zth force-pushed the codex/explicit-interface-implements branch from 8c19204 to 456d284 Compare May 25, 2026 12:33
@zth

zth commented May 25, 2026

Copy link
Copy Markdown
Owner Author

Codex review

@zth
zth force-pushed the codex/explicit-interface-implements branch from 456d284 to 746dc6f Compare May 25, 2026 12:46
@zth

zth commented May 25, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@zth
zth merged commit a7e15ae into main May 25, 2026
5 checks passed
@zth
zth deleted the codex/explicit-interface-implements branch May 25, 2026 13:09
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.

1 participant