Skip to content

THOTH-GQL-DATALOADER-01: add request-scoped GraphQL DataLoader foundation - #802

Draft
ja573 wants to merge 21 commits into
developfrom
feature/shared-architecture/graphql-dataloader-foundation
Draft

THOTH-GQL-DATALOADER-01: add request-scoped GraphQL DataLoader foundation#802
ja573 wants to merge 21 commits into
developfrom
feature/shared-architecture/graphql-dataloader-foundation

Conversation

@ja573

@ja573 ja573 commented Aug 11, 2026

Copy link
Copy Markdown
Member

STATUS: BLOCKED / IMPLEMENTATION INCOMPLETE — DO NOT REVIEW AS A COMPLETED IMPLEMENTATION

Task identity

  • Programme: Shared Thoth GraphQL / Backend Architecture
  • Task: THOTH-GQL-DATALOADER-01
  • Risk: HIGH
  • Base: develop at 060052b47490d3d977db3b9d9f188c4c70760a9a
  • Branch: feature/shared-architecture/graphql-dataloader-foundation
  • Current implementation head: 56f5027431b7456a0fedbfa8d71314e809eb0a7b
  • Specification: repository-authoritative docs/engineering/ai-delivery/tasks/THOTH-GQL-DATALOADER-01.md
  • Architecture: repository-authoritative ADR-0007
  • CTO implementation authorization: recorded on merged PR THOTH-GQL-DATALOADER-01: specify GraphQL DataLoader foundation #801 on 2026-08-11

This PR is the one bounded implementation authorized by the CTO. It remains DRAFT, adopts no production GraphQL field, and carries no merge, deployment, release, production-access or activation authorization.

Completed on this branch

  • Added dataloader 0.18.x dependency configuration with defaults disabled and Tokio runtime support; CI resolves exactly dataloader 0.18.0.
  • Added normal Tokio rt support for production spawn_blocking; test runtime features remain dev-only.
  • Added the conventional non-cached DataLoader foundation with explicit max-batch 200 / yield-count 10 construction and try_load-oriented test consumers.
  • Added request-local loader-bundle type and test-only representative consumers.
  • Added real synchronous Diesel batching behind tokio::task::spawn_blocking: Arc<PgPool> and immutable keys enter the closure, connection acquisition and all Diesel work occur inside it, and the batch is set-based with eq_any.
  • Added total batch output semantics and missing-key fail-closed coverage.
  • Added non-caching, request-isolation, read-write-read freshness, immediate/yield/delayed/loader-behind-loader scheduling, and N=1/100/200/201/500 batching evidence on both Tokio runtime flavours.
  • Added real-Diesel N=250 statement-count evidence using Diesel connection instrumentation, targeting [200, 50] / two set-based imprint statements.
  • Added a cloneable non-serde batch-error projection preserving the field family's actual current GraphQL error convention, plus direct-vs-loader backend-failure equivalence tests with no retry/fallback dispatch.
  • Rehosted the independent mutation-guard/query-path/baseline/directive/duplicate-mutation regression coverage onto A2-independent fixtures.
  • Removed the A2 look-ahead prefetch module, response-scope shim, A2 fixture and A2 test modules.
  • Removed MutationGuardMode::store_available() and its guard-as-store-switch API/documentation; preserved the independent guard and its default/activation state.
  • Production HTTP handler now constructs Context independently of mutation guard mode after the request-boundary guard decision.
  • No production field adoption and no intentional production SDL/API change.

Temporary compatibility bridge — MUST NOT SURVIVE REVIEW

thoth-api/src/graphql/batching.rs currently contains only a temporary source-level type alias from the legacy GraphqlBatchStore name to the new RequestLoaders bundle. The old A2 stored-result/store/scope/failure machinery is gone, and the temporary constructor ignores guard mode, but Context still exposes the legacy batch_store field and with_guard_mode constructor because this execution environment cannot safely patch the large existing model.rs file.

This is not specification-compliant final state. The PR must not be presented for independent review until the alias, legacy field and legacy constructor are removed and Context directly owns RequestLoaders.

Missing required work

The following is missing work, not waived acceptance evidence:

  1. Final Context migration in thoth-api/src/graphql/model.rs: remove A2 imports, rename/remove batch_store, add direct RequestLoaders ownership, delete Context::with_guard_mode, and then delete the temporary batching.rs alias/module.
  2. Delete/reconcile the obsolete test helper test_context_with_guard_mode and ADR-0006 invariant-30 comment in thoth-api/src/model/tests.rs after the Context migration.
  3. General GraphQL unit-test async migration in thoth-api/src/graphql/tests.rs: replace the central/general execute_sync path with the approved async bridge and add explicit nested-runtime misuse evidence. The new dedicated DataLoader and rehosted guard suites are async, but this repository-wide test seam is still outstanding.
  4. Commit Cargo.lock with the resolved dataloader 0.18.0 package/dependency entry. CI has independently demonstrated the resolution, but the lockfile is not yet committed.
  5. Formatting: cargo fmt --all -- --check is currently red on newly added test files; exact rustfmt diffs are available in CI and must be applied rather than suppressed.
  6. Clippy: current exact-head lint reaches the implementation and reports only the obsolete test_context_with_guard_mode helper as dead code; that helper must be removed, not allow-listed.
  7. CHANGELOG: add the required PR THOTH-GQL-DATALOADER-01: add request-scoped GraphQL DataLoader foundation #802 implementation entry. check-changelog is currently red for this missing control record; no no-changelog bypass is acceptable.
  8. Production SDL evidence: generate and record base/head byte length and SHA-256/equivalent proof of byte identity.
  9. Full validation gate: record successful fmt/check/clippy/backend/workspace tests plus focused loader/guard tests on the final exact head.
  10. Implementation report: create the repository-template report with exact base/head, resolved versions, evidence matrix, migrations/ops/authorization posture, SDL proof and rollback/rollout state.

Current CI evidence at head 56f5027431b7456a0fedbfa8d71314e809eb0a7b

  • Dependency resolution/type checking progresses through dataloader v0.18.0 and the new loader code.
  • lint: FAIL only on dead-code test_context_with_guard_mode in thoth-api/src/model/tests.rs; this is expected missing A2-retirement cleanup and must be removed.
  • format_check: FAIL with rustfmt-only diffs in newly added DataLoader/guard test files.
  • check-changelog: FAIL because the implementation entry is not yet present.
  • test / build / migration/publish-classification runs must be reported from their final completed state; no result is inferred while a job is still running.

Execution-environment blocker

The connected GitHub app exposes whole-file replacement for existing files, but no line/patch write primitive. This runtime also has no network-capable local checkout, no installed Rust/rustfmt toolchain and no installed agent-browser CLI. Whole-file replacement is safe for the new/small files already changed, but using it to manually reconstruct the multi-thousand-line authoritative model.rs, graphql/tests.rs, Cargo.lock or CHANGELOG.md would create an unacceptable accidental-diff risk on a HIGH-risk shared backend task.

No architectural relaxation is being made to work around that tooling limitation. Missing evidence remains missing work.

Authorization boundaries — unchanged

NOT authorized by this PR: merge, deployment, release, production access, workflow dispatch, BE-02, Thoth Metrics adoption, OBSERVE, ENFORCE, PR #799 remediation/merge/closure, or architecture changes outside ADR-0007.

PR #799 remains outside this task and untouched.

Next gate

No independent implementation review yet. A capable implementation environment with a normal patchable checkout must continue this same bounded branch/PR, complete the missing items above, obtain green final exact-head CI/evidence, and only then present PR #802 for fresh independent exact-head review. The implementing agent must not approve its own work. Separate explicit CTO merge authorization remains required after independent approval.

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