Skip to content

Make documentId deterministic SHA-256 string and keep schemaId as in-memory int cache key#575

Open
Copilot wants to merge 40 commits into
devfrom
copilot/fix-documentid-issue
Open

Make documentId deterministic SHA-256 string and keep schemaId as in-memory int cache key#575
Copilot wants to merge 40 commits into
devfrom
copilot/fix-documentid-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

  • Review new actionable comment and related ExecutionTests code path
  • Investigate recent CI workflow failures via GitHub MCP logs
  • Run baseline build/tests before changes
  • Update ExecutionTests to assert/avoid validation-error-only path for escaped-string documentId
  • Format changed files with Fantomas
  • Run targeted build/tests for touched area
  • Run parallel validation
  • Reply to the new comment with commit hash

Copilot AI linked an issue May 17, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix documentId consistency for the same query content Make documentId deterministic for identical GraphQL document content May 17, 2026
Copilot AI requested a review from xperiandri May 17, 2026 18:50
Comment thread tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json
Comment thread tests/FSharp.Data.GraphQL.Tests/Literals.fs Outdated
Comment thread src/FSharp.Data.GraphQL.Server/Executor.fs Outdated
Comment thread src/FSharp.Data.GraphQL.Server/Executor.fs Outdated
Copilot AI changed the title Make documentId deterministic for identical GraphQL document content Make documentId deterministic as a SHA-256 string for identical GraphQL document content May 17, 2026
Copilot AI requested a review from xperiandri May 17, 2026 20:10
Copy link
Copy Markdown
Collaborator

@xperiandri xperiandri left a comment

Choose a reason for hiding this comment

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

Comment thread docs/execution-pipeline.md Outdated
Comment thread src/FSharp.Data.GraphQL.Server/Executor.fs Outdated
Comment thread src/FSharp.Data.GraphQL.Shared/Helpers/DocumentId.fs Outdated
Comment thread src/FSharp.Data.GraphQL.Shared/FSharp.Data.GraphQL.Shared.fsproj Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR changes GraphQL execution metadata so documentId is generated as a deterministic SHA-256 lowercase hex string from the printed query document instead of using unstable AST hash codes.

Changes:

  • Adds shared DocumentId.fromCanonicalQuery hashing helper and uses it in server execution and design-time validation cache key creation.
  • Updates DocumentId-related contract types, tests, docs, and JSON fixtures from int to string.
  • Replaces an inline introspection literal with a file-backed literal provider dependency.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/FSharp.Data.GraphQL.Shared/Helpers/DocumentId.fs Adds SHA-256 document ID helper.
src/FSharp.Data.GraphQL.Shared/FSharp.Data.GraphQL.Shared.fsproj Includes the new helper in compilation.
src/FSharp.Data.GraphQL.Shared/ValidationResultCache.fs Changes validation cache key document ID type to string.
src/FSharp.Data.GraphQL.Shared/TypeSystem.fs Changes planning/execution plan document ID types to string.
src/FSharp.Data.GraphQL.Server/Executor.fs Computes document IDs via SHA-256 over ToQueryString().
src/FSharp.Data.GraphQL.Server/IO.fs Updates response/result document ID contracts to string.
src/FSharp.Data.GraphQL.Client.DesignTime/ProvidedTypesHelper.fs Uses deterministic document ID in design-time validation keys.
tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs Updates document ID assertions for deterministic string hashes.
tests/FSharp.Data.GraphQL.Tests/Literals.fs Replaces inline JSON literal with file-backed literal provider.
tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj Adds literal provider package reference.
tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json Updates fixture document ID to string hash.
tests/FSharp.Data.GraphQL.IntegrationTests/integration-introspection.json Updates fixture document ID to string hash.
Packages.props Adds central package version for literal provider.
README.md Updates sample output document ID to string hash.
docs/execution-pipeline.md Documents deterministic SHA-256 string document IDs.

Comment thread src/FSharp.Data.GraphQL.Shared/ValidationResultCache.fs
Comment thread src/FSharp.Data.GraphQL.Shared/Helpers/DocumentId.fs
Comment thread src/FSharp.Data.GraphQL.Server/Executor.fs Outdated
Comment thread README.md Outdated
Comment thread tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.

Comment thread tests/FSharp.Data.GraphQL.Tests/ValidationCacheTests.fs
Comment thread tests/FSharp.Data.GraphQL.Tests/ValidationCacheTests.fs
Comment thread src/FSharp.Data.GraphQL.Server/Executor.fs Outdated
Comment thread src/FSharp.Data.GraphQL.Client.DesignTime/ProvidedTypesHelper.fs
Comment thread src/FSharp.Data.GraphQL.Shared/AstExtensions.fs Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.

Comment thread src/FSharp.Data.GraphQL.Server/Executor.fs
Comment thread src/FSharp.Data.GraphQL.Client.DesignTime/ProvidedTypesHelper.fs Outdated
Comment thread tests/FSharp.Data.GraphQL.Tests/ValidationCacheTests.fs
Comment thread tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs
xperiandri and others added 2 commits May 18, 2026 19:25
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

documentId has different values for the same query content

3 participants