THOTH-GQL-DATALOADER-01: specify GraphQL DataLoader foundation - #801
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 310bc4fef8
ℹ️ 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".
| @@ -0,0 +1,1084 @@ | |||
| # THOTH-GQL-DATALOADER-01 - Request-scoped GraphQL DataLoader foundation and A2 retirement | |||
|
|
|||
| Status: APPROVED | |||
There was a problem hiding this comment.
Add the required implementation report
This specification is marked approved after review, but no corresponding report exists under docs/engineering/ai-delivery/implementation-reports/ (I also searched the review records for THOTH-GQL-DATALOADER and PR 801). Consequently, the exact base/head, changed-file inventory, documentation checks, CI status, and operational/authorization assessment required before review have no durable evidence; add a report completed from the repository template.
AGENTS.md reference: AGENTS.md:L321-L338
Useful? React with 👍 / 👎.
| @@ -0,0 +1,1084 @@ | |||
| # THOTH-GQL-DATALOADER-01 - Request-scoped GraphQL DataLoader foundation and A2 retirement | |||
There was a problem hiding this comment.
Replace the superseded batching task in the delivery index
Adding this approved ADR-0007 task without updating docs/engineering/ai-delivery/README.md leaves the active delivery index pointing only to THOTH-GQL-BATCH-01 as a current DRAFT dependent on the now-superseded ADR-0006, while omitting this replacement. Agents using the documented index can therefore select the obsolete architecture; add this task and mark or remove the old entry from the active source set.
AGENTS.md reference: docs/engineering/AGENTS.md:L70-L80
Useful? React with 👍 / 👎.
CTO implementation authorization — THOTH-GQL-DATALOADER-01Date: 2026-08-11 The CTO explicitly authorizes implementation of repository-authoritative Authorized scope is exactly the approved task specification and ADR-0007: create the bounded implementation branch This authorization does not authorize merge of the implementation PR, deployment, release, production activation, production access, workflow dispatch, Implementation must preflight a fresh exact |
Task identity
THOTH-GQL-DATALOADER-SPEC-01THOTH-GQL-DATALOADER-01687ee0a40360fb28ef9aab1aa41fc69e35ed93ea(develop, the merge of PR ADR-0007: record approved GraphQL DataLoader architecture #800; liveorigin/developverified identical at authoring time)docs/shared-architecture/dataloader-foundation-specAuthority
ADR-0007(docs/engineering/decisions/ADR-0007-conventional-request-scoped-graphql-dataloader.md) isAPPROVED(CTO, 2026-08-11) and repository-authoritative through the merge of PR #800.ADR-0006remainsSUPERSEDEDfor batching architecture and preserved as the historical record; its pinned-Juniper duplicate-top-level-mutation-execution finding remains a live, separately controlled concern.This PR carries the implementation specification only. The specification received CTO specification approval on 2026-08-11, granted against the exact independently reviewed head
bb4f57e0d446270b4c404879ef61b5e229e90bbfand limited to the exact specification content at that head; commit310bc4femechanically records that approval in the repository. The approval is not implementation authorization, not implementation-branch authorization, and not PR #801 merge authorization.THOTH-GQL-DATALOADER-01implementation remains NOT AUTHORIZED.Scope
Documentation/control record only:
docs/engineering/ai-delivery/tasks/THOTH-GQL-DATALOADER-01.md(new,Status: APPROVED— CTO specification approval recorded 2026-08-11;Implementation: NOT AUTHORIZED; the section 18 implementation-authorization record remains blank)CHANGELOG.md(oneUnreleased/Addedentry describing the specification/control record, updated to reflect the recorded approval)No runtime, Rust, Cargo, GraphQL schema, migration, workflow, configuration or production change. No implementation branch created.
Architecture carried forward
The proposed specification carries
ADR-0007into a bounded implementable task:dataloader0.18.x,non_cached::Loader, default features off, Tokio runtime support), owned by the real GraphQLContext, one request per loader, no static/global/cross-request state;execute_syncunit-test path;200max batch size /10yield count configuration — no crate defaults;try_loadonly for Thoth DB loaders; total, fail-closed batch functions;tokio::task::spawn_blockingboundary — pool handle crosses the async boundary, the connection never does, no connection across.await, no hidden per-parent SQL loop;ADR-0006A2 infrastructure with a concrete source inventory (batching.rs/GraphqlBatchStore,prefetch.rs,scope.rsshim,Context.batch_store, A2 fixtures/tests). Thebatching_tests.rsquery_pathmodule is split, not wholly retired: its A2-independent mutation-guard/query-path regressions (valid queries never restricted and emit no guard event in any mode; valid-query responses equivalent to the no-guard baseline across guard modes; baseline-invalid queries keep Juniper's canonical error behaviour with no guard event) are preserved and rehosted onto A2-independent fixtures; only its store/scope-specific coverage is retired;mutation_guard.rsis not preserved in full: the A2-onlyMutationGuardMode::store_available()API and its guard-mode-as-store-switch coupling documentation are removed with the store; DataLoader availability is independent of guard mode; guard/duplicate-mutation regression evidence rehosted onto A2-independent fixtures;Plus a full acceptance-evidence matrix: batch boundaries (N = 1/100/200/201/500 with expected chunk shapes, both Tokio runtime flavours), scheduling fixtures (including deliberate fragmentation and loader-behind-loader), request isolation, completed-result non-caching, read-write-read freshness, missing-key fail-closed behaviour, real-Diesel connection-instrumented query-count evidence across a chunk boundary (e.g. N=250 → two set-based statements), backend-failure equivalence with no fallback/retry SQL, error-representation tests, preserved duplicate-mutation regression, SDL byte-identity, and the full repository fmt/check/clippy/backend/workspace gate.
Non-goals
No BE-02 implementation and no
Publisher.distributionPlatforms; no Thoth Metrics adoption; no production GraphQL field adoption; no Juniper upgrade; no async-graphql migration; no database/data/schema migration; no deployment, workflow dispatch, production secrets, protected-source inspection, production database access or fleet/orchestrator queries; noOBSERVE/ENFORCEactivation; no mutation-guard redesign; no PR #799 remediation/merge/closure; no OPS-03 continuation or OPS-04 implementation; no unrelated error-contract normalization, authorization changes or refactors.Evidence
687ee0a40360fb28ef9aab1aa41fc69e35ed93ea310bc4fef853e3d778da70e4ceb3cf53ba5952b5310bc4ferecords the 2026-08-11 CTO specification approval granted at the previously reviewed exact headbb4f57e0d446270b4c404879ef61b5e229e90bbf(spec header status/approved-by/dependency lines plus the narrowly correctedCHANGELOG.mdstatus clause; no other content change). The approval-only deltabb4f57e0..310bc4ferequires fresh independent exact-head verification before separate CTO merge authorization is requestedbb4f57e0corrects the threeCHANGES REQUIREDfindings from the independent review of the previous head26b722d0a16493febeb7a4006159af91b1234f73(query_pathpreservation boundary;mutation_guard.rspreservation boundary /store_available()removal; explicit observability and operations disposition)ADR-0007(full),ADR-0006(supersession/A2 surfaces),decisions/README.md,decision-register.md, rootAGENTS.md,thoth-api/AGENTS.md,task-specification-template.md,THOTH-GQL-BATCH-01.md(conventions),CHANGELOG.mdthoth-api/src/graphql/{mod,model,batching,prefetch,scope,mutation_guard,batching_fixture,batching_tests,tests}.rs,thoth-api/src/model/tests.rs,thoth-api/tests/support/mod.rs,thoth-api-server/src/lib.rs(guard + async execute path),thoth-errors/src/lib.rs(IntoFieldError),thoth-api/src/db.rs(PgPool),thoth-client/build.rs(SDL generation),src/bin/thoth.rs(guard-mode wiring),Cargo.toml/thoth-api/Cargo.toml/Cargo.lock(juniper 0.16.2, diesel 2.3.10, tokio 1.52.3 dev-only in thoth-api, no dataloader)git diff --check: cleancheck_changelog.yml)Review gate
CTO specification approval was granted 2026-08-11 at exact head
bb4f57e0d446270b4c404879ef61b5e229e90bbfand is recorded by commit310bc4fe.Fresh independent exact-head verification of the approval-only delta (
bb4f57e0..310bc4fe) is required before separate CTO merge authorization is requested.Implementation authorization remains a distinct later CTO decision, available only after the approved specification is repository-authoritative on
develop. This PR does not claim, and must not be read as granting, implementation authorization.