Skip to content

docs(publisher-services): specify the BE-02 distribution platform model - #788

Draft
ja573 wants to merge 3 commits into
developfrom
feature/publisher-services/be-02-spec
Draft

docs(publisher-services): specify the BE-02 distribution platform model#788
ja573 wants to merge 3 commits into
developfrom
feature/publisher-services/be-02-spec

Conversation

@ja573

@ja573 ja573 commented Aug 7, 2026

Copy link
Copy Markdown
Member

BE-02 specification (documentation only)

Authors the bounded BE-02 - Distribution platform model implementation
specification required by operating-model.md Gate 1 before any BE-02 runtime
work may be authorized.

This PR implements nothing. It adds no BE-02 runtime code, migration,
PostgreSQL type, Rust enum, model, GraphQL field or query, descriptor,
assignment table or test, and creates no feature/publisher-services/be-02
implementation branch.

Files

File Purpose
docs/engineering/ai-delivery/tasks/BE-02.md the bounded BE-02 task specification
docs/publisher-services/task-status.md tracker row and next-action reconciliation (the BE-02 row pointed at a non-existent specification and carried TBD)
docs/engineering/ai-delivery/implementation-reports/BE-02-SPEC-implementation-report.md required implementation report
CHANGELOG.md required ## [Unreleased] entry

Runtime files changed: none.

Specification mechanism

Gate 1 and root AGENTS.md section 1 permit either a committed specification or
an authoritative GitHub issue. The issue route was attempted first as the less
redundant option and is not available: GitHub caps an issue body at 65,536
characters and the complete specification is approximately 97,000. Root
AGENTS.md makes an issue sufficient only when it contains the
template-required information, so an issue cannot satisfy Gate 1 here without
deleting required content or splitting the specification across mutable
comments, which would leave an exact-head review with an ambiguous target. The
committed route is the smallest compliant mechanism and matches the existing
convention for every task specification to date.

What the specification settles from live repository evidence

Inspected at develop 5a8c27b1b7c11a4f6bd26d459556468099f8c1f4, using BE-01's
actual merged implementation (PR #779) as the closest precedent rather than its
narrative report:

  • public.distribution_platform with exactly the 17 ADR-0004 values in binding
    declaration order; no OTHER, no fallback, no Default;
  • the closed Rust/GraphQL DistributionPlatform enum following the merged
    ThothPackage pattern;
  • the publisher_distribution_platform schema - composite primary key,
    ON DELETE CASCADE foreign key, NOT NULL activation_id/enabled_at with
    the single-row invariant enabled = (disabled_at IS NULL), partial enabled
    reverse-lookup index, set_updated_at trigger, application-generated
    activation_id and transaction-timestamp lifecycle timestamps;
  • all six activation-lifecycle transitions with write/no-write, activation_id,
    timestamp and idempotency semantics;
  • group-level linked OAPEN/DOAB normalization - one transaction, one shared
    activation identity, identical timestamps, atomic rollback, no supported
    one-sided enabled state, enforced transactionally rather than by a new
    database trigger subsystem;
  • OCLC_KB / EX_LIBRIS_KB as independent assignments sharing a feed profile
    without duplicate feed state;
  • code-owned exhaustive descriptors proved by a wildcard-free match, the
    BackCatalogueBehaviour vocabulary, and an internal adapter profile that
    represents shared mechanism identity without collapsing destinations;
  • JISC_NBK visible but non-assignable, enforced fail-closed in the domain
    layer;
  • the four public GraphQL surfaces on the repository's existing offset/limit
    pagination, with a mandatory publisher_id tie-breaker that the existing
    Publisher::all lacks, and count/lookup agreement by construction;
  • ADR-0003 Architecture A atomicity, empty and representative populated
    disposable-database migration evidence, a DDL lock assessment recording the
    SHARE ROW EXCLUSIVE lock the foreign key takes on the populated publisher
    table (reads unaffected, writes blocked) with no unevidenced
    production-duration claim, and the retained-foundation operational rollback.

Independent-review remediation (head d411d493)

An independent review returned CHANGES REQUIRED with six findings. Five are
resolved on this branch; the sixth is escalated to the CTO rather than decided
here.

# Finding Status
1 contradictory lifecycle/approval metadata (Status: DRAFT with Approved by: CTO) RESOLVED - transient fields removed; durable approval-authority triple, gate-based dependency rows, ten-event lifecycle table
2 claimed the migration takes no lock on any populated table RESOLVED - corrected against PostgreSQL 17 docs: ADD FOREIGN KEY takes SHARE ROW EXCLUSIVE on publisher; reads continue, writes block; pg_locks verification now mandatory
3 row constraint permitted states the lifecycle forbids RESOLVED - activation_id/enabled_at now NOT NULL, no default on enabled, invariant reduced to enabled = (disabled_at IS NULL), new section 6.1.1
4 N+1 underspecified for Publisher.distributionPlatforms BLOCKED - escalated. No batching mechanism exists in the repository; every remedy changes shared architecture, adds a dependency, waives a control or drops approved API. Sections 9.2.1 / 19.1 state the four options and the exact CTO decision required
5 rollout conflated repository merge with environment state RESOLVED - section 14 split into merge guarantees (14.1) and conditional environment behaviour (14.2)
6 GraphQL inventory counts wrong (four enums named three; two root fields, three specified) RESOLVED - binding section 12.1 inventory: 3 root fields, 1 Publisher field, 2 object types, 3 enums, plus the 3 internal Rust enums explicitly excluded

One open gate blocks implementation authorization:
BLOCKED - N+1 CONTROL REQUIRES ARCHITECTURE DECISION. thoth-api/AGENTS.md
section 6 is preserved unweakened; the mechanism choice is the CTO's.

Review focus

  1. 17-value inventory fidelity against ADR-0004 sections 4.1, 4.2, 4.3 and 5.
  2. Completeness and internal consistency of the six lifecycle transitions.
  3. The group-level linked-transition rule, including its
    partially-enabled-group repair behaviour.
  4. Whether enforcing the cross-row OAPEN/DOAB invariant transactionally rather
    than in the database is the right architectural boundary.
  5. The composite-primary-key departure from existing repository practice and its
    BLOCKED fallback.
  6. The timestamp with time zone choice against the mixed historical
    convention.
  7. Whether the public surface exposes the right fields, and whether withholding
    activationId, disabledAt and the adapter/feed profile is correct.
  8. Reverse-lookup pagination determinism and count/lookup agreement.
  9. Completeness of the fail-closed table.
  10. That the specification authorizes nothing.
  11. The remediated content: the tightened row invariant, the foreign-key lock
    statement, whether finding 4 is correctly escalated rather than silently
    resolved, the merge/deployment separation, the exact contract inventory, and
    the durability of the approval wording under ADR-0005.

Authorization state

Merging this specification does not authorize BE-02 implementation. BE-02
remains BLOCKED. Implementation additionally requires fresh verification of
the then-current exact develop head and separate explicit CTO authorization
bound to that SHA, after which feature/publisher-services/be-02 may be
created.

Live review, authorization and merge evidence is this pull-request record and is
not copied into the repository (ADR-0005).

ja573 added 3 commits August 7, 2026 18:05
Author the bounded BE-02 implementation specification required by Gate 1
before any BE-02 runtime work may be authorized. BE-02 is the HIGH-risk,
schema-bearing task that adds the inactive additive backend foundation for
publisher distribution-platform configuration.

The specification settles every low-level representation choice from live
repository conventions inspected at develop 5a8c27b, rather than from the
provisional values in the approved design, which ADR-0004 supersedes:

- the PostgreSQL public.distribution_platform enum with exactly the 17
  ADR-0004 values in binding declaration order, no OTHER and no fallback;
- the closed Rust and GraphQL DistributionPlatform enum following the merged
  BE-01 ThothPackage pattern, with Default deliberately not implemented
  because a default would act as a fallback;
- the publisher_distribution_platform schema: composite primary key, ON
  DELETE CASCADE foreign key, a single-row lifecycle check constraint, the
  partial enabled reverse-lookup index and the set_updated_at trigger;
- the six activation-lifecycle transitions with their write, activation_id,
  timestamp and idempotency semantics, with disabled rows retained and a
  genuine re-enable generating a new activation;
- group-level linked OAPEN/DOAB normalization: one transaction, one shared
  activation identity, identical transaction timestamps, atomic rollback,
  and no supported path to a one-sided enabled state, enforced
  transactionally rather than through a new database trigger subsystem;
- OCLC_KB and EX_LIBRIS_KB as independent assignments sharing a feed profile
  without duplicate feed state or shared activations;
- code-owned exhaustive descriptors proved by a wildcard-free match, with
  the BackCatalogueBehaviour vocabulary and an internal adapter profile that
  represents shared mechanism identity without collapsing destinations;
- JISC_NBK visible but non-assignable, enforced fail-closed in the domain
  layer because assignability is code-owned descriptor metadata;
- the four public GraphQL surfaces using the repository's existing
  offset/limit pagination, with a mandatory publisher_id tie-breaker that the
  existing Publisher::all lacks, and count/lookup agreement by construction.

The specification also fixes the ADR-0003 Architecture A atomicity
requirement, the empty and representative populated disposable-database
migration evidence, the DDL lock assessment with no unevidenced production
duration claim, the retained-foundation operational rollback, the concrete
required tests, the acceptance criteria and the stop conditions.

Reconcile the Publisher Services tracker, whose BE-02 row pointed at a
non-existent specification and carried TBD.

Documentation only. No BE-02 runtime code, migration, schema, model, GraphQL
surface, descriptor, assignment table or test is added; no implementation
branch is created; issue #765 is not modified; no ADR, inventory, evidence
ledger or evidence count changes; and nothing is deployed, released,
migrated or activated. BE-02 remains BLOCKED and unauthorized: implementation
requires this approved specification plus separate explicit CTO authorization
bound to a freshly verified exact develop head.
Add the required CHANGELOG entry under Unreleased for the BE-02 specification,
now that the specification pull request number exists, and record that number
in the specification implementation report.

Root AGENTS.md section 13 requires every pull request to update CHANGELOG.md
and to reference the pull request number where available, which is only
knowable after the pull request is opened.

Documentation only. No specification content changes, no runtime change, and
no authorization is granted.
Independent review of the BE-02 specification returned CHANGES REQUIRED with
six findings. Five are resolved; the sixth is an architecture decision that
belongs to the CTO and is escalated rather than papered over.

1. Lifecycle and approval wording was internally contradictory: Status: DRAFT
   sat alongside "Approved by: CTO" and "Approved for implementation by: CTO",
   a dependency row asserted the specification was approved, and other prose
   said CTO approval was still to come. Under ADR-0005 any value those fields
   could hold is false before the event and stale after it, and the commit
   correcting them would invalidate the exact-head review that justified it.
   Remove the transient fields entirely and record only durable authority:
   approval authority CTO, approval evidence the GitHub pull-request record,
   implementation authorization separate and absent. Section 2.1's last rows
   become named gates rather than statuses; section 23 becomes a ten-event
   table naming where each event's evidence lives and separating repository
   authority from CTO approval and from implementation authorization; section
   24 records approval authority and effect, never whether approval occurred.

2. The lock assessment claimed the migration takes no lock on any populated
   table. That is wrong: the foreign key references publisher, and per the
   PostgreSQL 17 documentation ADD FOREIGN KEY acquires SHARE ROW EXCLUSIVE on
   the referenced table as well as on the constrained one. Because SHARE ROW
   EXCLUSIVE conflicts with ROW EXCLUSIVE but not with ACCESS SHARE or ROW
   SHARE, publisher reads continue and publisher writes are blocked while it is
   held. Rewrite section 13.3 with per-operation locks, a blocked/not-blocked
   table, a conservative assessment naming lock acquisition and queueing rather
   than duration as the dominant risk, an explicit rejection of NOT VALID as
   useless against an empty child table, mandatory pg_locks verification, and
   prohibitions on the old claim and on any production-duration claim.

3. The row check constraint permitted states the lifecycle forbids: disabled
   rows with a null activation_id or enabled_at, enabled rows still carrying a
   disabled_at, and an enabled DEFAULT false that implied a never-activated row
   was valid. Since a row is created only by ABSENT -> ENABLED, every persisted
   row has an activation. Make activation_id and enabled_at NOT NULL, drop the
   default on enabled, leave disabled_at as the only nullable lifecycle column,
   and reduce the constraint to enabled = (disabled_at IS NULL). Add section
   6.1.1 with the row-existence rule, the two legal states, the rejected
   states, per-operation satisfaction and the Diesel mapping; correct the model
   fields from Option<Uuid>/Option<Timestamp> to Uuid/Timestamp.

4. N+1 access for Publisher.distributionPlatforms on publisher lists is not
   settled and cannot be settled by this task. thoth-api/AGENTS.md section 6
   requires new lists to avoid N+1 and use set-based SQL or batched loaders,
   and the repository provides nothing to reuse: no DataLoader, no look_ahead,
   no request-scoped state on the GraphQL Context, and all 56 existing child
   resolvers query once per parent. The exposure also arises through the
   pre-existing publishers root query, so no change confined to BE-02's own
   root fields removes it. Every remedy either adds cross-cutting GraphQL
   architecture, adds a dependency, waives a standing control or removes
   approved public API. Record the binding prohibition, keep section 6 intact,
   and escalate: sections 9.2.1 and 19.1 state the four options and the exact
   CTO decision required, section 18.7b requires measured query-count evidence,
   and section 22 gains the decision as a step before authorization.

5. Rollout conflated repository merge with environment state, claiming the
   table would be empty "in every environment" after merge. A merge deploys
   nothing and runs no migration. Split section 14 into what a merge guarantees
   in the repository and, conditionally, how an environment behaves once
   deployment and migration execution have been separately authorized and
   performed. Correct the same conflation in rollback and performance.

6. Compatibility claimed four new enums while naming three and two new root
   query fields while specifying three. Add section 12.1 as the binding
   inventory - 3 root fields, 1 new Publisher field, 2 object types, 3 GraphQL
   enums, 0 inputs, mutations or scalars - with an explicit table of the three
   internal Rust enums that must not reach the generated SDL, and assert it in
   section 18.7a.

Acceptance criteria, required tests, the implementation-report expectations,
the tracker and the CHANGELOG entry for PR #788 are updated to match. A
classified search was used for affected claims; no global replacement was made
and historical records outside this pull request are untouched.

Documentation only. No runtime file changes, no migration, schema, model,
GraphQL or error implementation, no implementation branch, no new issue or
pull request, no ADR change and no modification of issue #765. BE-02 runtime
implementation remains unauthorized.
ja573 pushed a commit that referenced this pull request Aug 7, 2026
Independent architecture review returned CHANGES REQUIRED with three P1 findings
and one P2. Option A / A2 - look-ahead-driven set-based prefetch into
request-scoped state - is unchanged; B, C and D were not reconsidered.

P1 cache identity: the store was keyed by (loader, parent key), which collides
for the argument-bearing child fields Thoth already has. Identity is now
(loader identity, normalized load shape, parent key), with typed loader-specific
shapes, one constructor shared by prefetch and lookup, and explicit default
normalization - Juniper look-ahead reads only literal AST arguments and never
applies schema defaults, while the child resolver receives the default-applied
value. BE-02's argument-free field takes a Unit shape; no production field gains
an argument.

P1 failure state: the draft required a failed prefetch both to leave keys absent
and to suppress the fallback that absence triggers. Replaced with a three-state
store - NotLoaded falls back, Loaded (including empty) never queries, LoadFailed
returns the error with no retry - the failure recorded once per dispatch, the
parent list field still resolving, and a GraphQL-visible equivalence contract
over errors[].path, null propagation and extensions.type rather than error text.

P1 path coverage: correctness and N+1 compliance are now distinct. Publisher
fans out through Imprint.publisher and Contact.publisher as well as the
publishers root query, so a loader-backed field with one prefetch site can still
issue a query per parent. Adopting tasks owe an exact-base path inventory,
coverage or escalation, and per-path measurement; the BE-02 inventory belongs to
BE-02.

P2 measurement: statement counts must use a pool constructed after the
instrumentation hook, not the process-wide OnceLock test pool.

Documentation only. ADR-0006 remains PROPOSED, THOTH-GQL-BATCH-01 remains DRAFT
and unauthorized, PR #788 and issue #765 are unmodified, and the changed head
requires a fresh independent exact-head review.
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