docs(plans): plan the monolith repository sidebar package and image counts

Narendranrequested to merge
nkannan/plan-repository-sidebar-counts into main

Summary

Adds the implementation plan for showing live package and image counts in the monolith's hosted repository sidebar, plus its row in the plans index and an amendment to S05.

Plan only, no production code ships here. Per the AGENTS.md plan-MR guardrail, no step MR opens against the monolith until this lands.

  • docs/plans/monolith/2026-09-28-repository-sidebar-counts.md (new)
  • docs/plans/README.md (index row)
  • docs/specs/monolith/S05-repository-detail.md (amendment)

The hosted sidebar's Packages/Images figure reads artifactsCount, which AR counts as versions (Maven, npm) or manifests (Docker, OCI). A Maven repository with one package at two versions shows "2 Packages" beside a one-row table. S05 specified that source, so the implementation followed the spec.

Design (bonnie-tsang, on the issue) wants package and image counts, not a relabel to Versions/Manifests. rchanila found that AR already serves the live counts: GET /api/v1/{slug}/repositories/{repository_name}/statistics (getRepositoryStatistics) returns packages_count (Maven, npm) and images_count (Docker, OCI). These are live counts over the same visible rows the artifact lists return (the RepositoryStatistics contract in AR's api/openapi/v1.yaml, required by AR's S17 and asserted by its store integration tests). Hosted only: remote and virtual answer 404.

The decomposition

Step What Type
1 ArtifactRegistry::Client#repository_statistics(slug:, repository_name:, format:) and an ArtifactRegistry::RepositoryStatistics value object (packages_count, images_count only). format is not sent; it checks the body carries the family's count type::feature
2 statistics field on ArtifactRegistryRepositoryDetails (new ArtifactRegistryRepositoryStatistics type with packagesCount/imagesCount). Hosted only; null without an AR call on remote and virtual. Follows the version-statistics resolver precedent (S03 error rendering, once per operation). type::feature
3 Sidebar reads the count from its own get_repository_statistics document, unbatched and skipped until the detail read shows a hosted repository. A null or failed read omits the figure only. type::bug

The steps form a linear chain (1, then 2, then 3), because the sidebar is the field's only consumer. All EE, dark behind artifact_registry_ui.

Decisions worth a look

  • Own document, unbatched, skipped until hosted. Not inside the detail query, whose error hook swaps the whole page for an alert. Not batched either: BatchHttpLink returns one response for the whole batch, so the first paint and the artifact table (which starts after the detail read) would wait on AR's live count, which past a threshold on the repository's share of its partition scans the whole partition. rchanila raised the same page-blocking risk on gitlab!256618. Cost: the figure lands one round trip after the detail read; remote and virtual pages make no statistics request at all.
  • The extra AR repository read is kept. Selecting userPermissions in the statistics document would share the detail document's read, but only inside one batch (the resolver's read cache lives for one HTTP request), so it needs the batching rejected above. The page's packages, images, and upstream documents already pay the same parent read.
  • The delete refetch stays outside the awaited set. artifacts_table.vue refetches with awaitRefetchQueries: true, so adding the statistics document there would make the row wait on the live count and turn a statistics outage into the delete's "Something went wrong" alert after a delete AR accepted. Instead, a hosted delete refetches the statistics document separately, un-awaited, and the query's own error() clears the figure. A remote Delete cache entry issues no statistics refetch.
  • No fallback to artifactsCount when statistics is null. A wrong number beside a correct table is worse than no figure. S05 uses the same reasoning for Downloads.
  • No blanket rescue in the resolver. The separate document already isolates its errors, so the resolver keeps the S03 default.
  • A body missing the family's count is an error, not a null count. For example, a Maven read answered with only images_count raises an unexpected success and surfaces as null plus a top-level service-unavailable error.
  • Permissions. AR gates the statistics route on read_repository and the package/image lists on read_artifact. Every built-in ADR-021 role grants both, so the figure and the table count the same rows. The monolith's read_artifact_registry is granted to every organization member (organization_user role), and the type-level check is skipped under artifactRegistryRepository, so AR is the only ADR-021 gate on the figure. A future custom role with read_repository only would not see the figure: AR's list denial resolves the connection null, and the page renders not-found with no sidebar.

Spec amendment

S05 stays Approved. Changes:

  • New Dependencies row for the statistics route.
  • Type extensions: artifactsCount is now documented as versions/manifests and remote-only; new statistics field.
  • Ruby client table: new row, plus a paragraph saying the method takes format only to check the body.
  • Sidebar stats table: hosted row source changes to statistics, omitted only when null (a zero renders). The remote and virtual rows are unchanged.
  • Apollo: the statistics document is unbatched and sent once the detail read shows a hosted repository; until it answers the figure is absent, as for null; a hosted delete refetches it without holding the delete on it.
  • Acceptance criteria: three new criteria appended at the end of their lists, so existing criterion numbers do not shift, plus one amended criterion.
    • The field criterion also states the error outcomes: 401, 403, and 404 give a silent null; 429, 5xx, other 4xx, and a body missing the family's count give null plus a top-level error.
    • The client criterion covers the missing-count case.
  • One new bullet each in Error cases, Follow-ups, and Resolutions.

Out of scope

  • The remote figure. It keeps reading artifactsCount as "Packages cached"/"Images cached" and stays mislabelled, because AR serves no remote repository statistics. It stays tracked on gitlab#628241, which is why this MR says "Related to" and not "Closes".
  • Size, Downloads, and virtual repositories.

Changes since the last review

Round 7 (15ac54be):

  • TLS case in Step 1: a TLS failure does not retry and logs one attempt plus the terminal failure, beside the timeout and connection cases that retry once, matching Step 2.
  • Browser-side gap stated plainly: a failed browser-to-Rails request leaves no signal beyond the omitted figure. That is accepted because Sentry's beforeSend drops a failed fetch even from createAlert.

Round 6 (d308fbc6):

  • Inherited flag gate named: the resolver subclasses BaseResolver, whose resolve returns nil unless artifact_registry_ui is on, so the resolver spec's flag-off case is the inherited gate, as in version_statistics_resolver_spec.rb. The Feature flag bullet names both layers.
  • Error-tracking per failure path:
    • Step 2 states where each failure is logged: 429, 5xx, transport, unexpected success, and unreadable body in the client's raise_terminal; 401 and 403 in RendersErrors; 404 and other 4xx nowhere.
    • Step 1's client spec pins the deliveries.
    • Approach says the dropped post-delete refetch rejection forgoes the browser-side Sentry capture.
  • Final-head sweep fixes:
    • the sidebar pluralizes its label on the statistics count, so the defect case renders "1 Package", not "1 Packages";
    • the parent field's authorize: :read_artifact_registry is named as what stops a non-member;
    • S05's statistics error case says it overrides the general 401/403 and transport cases for that read.
  • Wording: the title-marker note follows the convention's rule rather than attributing the action to the doc.

Round 5 (6e813ea3):

  • Pinned verification refs: Research Findings now opens with the refs every claim was checked against (gitlab master at e856e4d1, AR main at a6965b6e, 2026-09-30) and a table of the recently merged monolith MRs whose code the plan cites. Two round-5 findings came from a checkout that predates gitlab!256618 (merged 09-25) and gitlab!258077 (merged 09-29).
  • Revisit claim qualified: a revisit renders the cached figure at once only with its cache entry intact. That needs notifyOnNetworkStatusChange: true on the statistics query, now specified in Step 3 and pinned by a detail-spec case.
  • Assistive technology: the figure's arrival, change, or removal is not announced, following S05's live-region rules; S05's Apollo note records it.
  • Final-head sweep fixes:
    • the table region's own skeleton is acknowledged;
    • the pending-window case matches isAwaitingMainContent;
    • the inline createMockApollo sites are chosen by rule, since a fourth, virtual one landed 09-30;
    • spec ownership per layer is stated;
    • the post-delete figure lag is accepted explicitly;
    • the sibling suites' dropped cases are added (transport failure, timeout, credential, exact field set, the statistics field block, the unknown-key reader);
    • S05 names the detail type, says "pages through", and scopes "omitted only when null" to a rendered block.

Round 4 (a91f9ea4):

  • Delete refetch redesigned (found in a self-review): the statistics refetch moved out of the table's awaited refetchQueries, for the reason under Decisions worth a look. Step 3's table spec covers the toast, the pending state, and the remote case.
  • Permissions: the plan now states what grants the monolith's read_artifact_registry and corrects the custom-role outcome (not-found, no sidebar).
  • "Same visible rows" cited: the AR contract, S17, and the per-format store integration tests.
  • Figure transitions: the detail spec now owns the figure appearing once the read resolves and re-rendering after a refetch.
  • hosted? cited: ArtifactRegistry::Repository#hosted? in ee/lib/artifact_registry/repository.rb, delegated by the presenter.
  • Precision passes: Step 1's acceptance mirrors the sibling #version_statistics and #namespace_statistics client suites, and the resolver spec mirrors version_statistics_resolver_spec.rb, each naming the contexts it drops. The research notes are corrected (live-count floor, empty-repository stored counters, the namespace-statistics rescue, the eviction paths, the loading skeleton). Quoted spec titles are verbatim. The Dependencies section gains the milestone, related-issue, ADR-freshness, and title-marker lines the sibling plans carry.
  • S05: the 404 list names a viewer who may not see the repository; the zero case renders; the Apollo note covers pop-in and the delete refetch; the null criterion no longer pins Downloads to Maven, which the shipped sidebar does not.

Earlier rounds:

  • Batching trade-off: Step 3 is now unbatched and skipped until the detail read shows a hosted repository. The rejected alternatives are recorded.
  • Shared repository read: recorded as rejected, with the reason (it only works inside one batch).
  • Fixture precedent: corrected to repository_upstream_repositories.rb.
  • Step 3 query shape: now names get_repository_upstream_repositories.query.graphql as the sibling to copy, with the organization(id:) wrapper and typed variables.
  • Missing-count guard: Step 1's guard is now per family via format:, and Step 2's request spec covers the missing-count body.
  • Cache interactions (new finding): statistics is stored inline on the name-keyed ArtifactRegistryRepositoryDetails entry, so evictRepositoryDetailsEntry after an edit drops it and the revisit waits on the read; an upstream's Clear cache evicts a remote entry, which never holds statistics. A silent-null parent writes artifactRegistryRepository: null onto Organization, as sibling documents already do.
  • Step 3 test work: spelled out. The statistics handler goes into the shared createComponent helper and the file's three inline createMockApollo calls. artifacts_table_spec watches the statistics document in the hosted cases, because a refetch by document reaches only active queries.
  • Permissions (new finding): recorded under Decisions worth a look.
  • S05 field criterion: now states the error behavior.
  • Two overlong lines rewrapped.

Governing ADRs

  • ADR-014 (Frontend to Artifact Registry Interaction): the new field follows the Rails GraphQL resolver pattern with the client acquired through the cached helper; no cross-service join is added. Conforms.
  • ADR-021 (Authorization): statistics is a read_repository operation, as ADR-021's table lists it; see the permissions note above. Conforms.
  • ADR-009 (API Design): the stored counters stay buffered and may lag; this plan moves one figure off them onto AR's live counts rather than changing them. Conforms.

The local ADR mirror is current (./scripts/adr-freshness.sh exit 0, 2026-09-30).

Testing

Docs-only MR. markdownlint-cli2, vale (error level), and lychee (offline, fragments) pass on the three changed files. These are the checks the docs:lint CI job runs. Every code and contract claim in the plan was checked against AR main and gitlab-org/gitlab master; the script below re-checks the load-bearing ones.

How to setup and validate locally

From an artifact-registry checkout, with GITLAB_MONOLITH_PATH pointing at a gitlab-org/gitlab checkout and the docs lint tools installed (markdownlint-cli2, vale, lychee). It prints OK on success and names the failed check otherwise.

#!/usr/bin/env bash
# Run from an artifact-registry checkout. Set GITLAB_MONOLITH_PATH to a gitlab-org/gitlab checkout.
set -euo pipefail

git fetch -q origin nkannan/plan-repository-sidebar-counts
orig=$(git symbolic-ref -q --short HEAD || git rev-parse HEAD)
trap 'git checkout -q "$orig"' EXIT
git checkout -q --detach origin/nkannan/plan-repository-sidebar-counts

files=(
  docs/plans/monolith/2026-09-28-repository-sidebar-counts.md
  docs/plans/README.md
  docs/specs/monolith/S05-repository-detail.md
)

# 1. The docs:lint job, scoped to the changed files.
markdownlint-cli2 "${files[@]}"
vale --minAlertLevel error "${files[@]}"
lychee --offline --include-fragments "${files[@]}"

# 2. AR-side facts the plan relies on.
check() { grep -q -- "$2" "$1" || { echo "FAIL: $1 lacks: $2"; exit 1; }; }
check api/openapi/v1.yaml 'operationId: getRepositoryStatistics'
check internal/managementapi/handler.go 'repositoryStatisticsPattern, requirePermission(authz.ActionReadRepository'
check internal/managementapi/handler.go 'packagesPattern, requirePermission(authz.ActionReadArtifact'
check internal/managementapi/statistics_repository.go 'writeRepositoryNotFound'
check docs/dev/database-query-patterns.md 'live counts cost the partition, not the repository'

# 3. Monolith-side facts, read from origin/master only.
mono() { git -C "$GITLAB_MONOLITH_PATH" show "origin/master:$1"; }
monocheck() { mono "$1" | grep -- "$2" >/dev/null || { echo "FAIL: monolith $1 lacks: $2"; exit 1; }; }
git -C "$GITLAB_MONOLITH_PATH" fetch -q origin master
monocheck app/assets/javascripts/lib/graphql.js 'new BatchHttpLink'
monocheck ee/app/graphql/resolvers/artifact_registry/repository_resolver.rb 'include_permissions$'
monocheck ee/app/graphql/resolvers/artifact_registry/version_statistics_resolver.rb 'FieldCallCount, limit: 1'
monocheck ee/lib/artifact_registry/client.rb 'def missing_expected_keys?'
monocheck ee/lib/artifact_registry/repository.rb 'def hosted?'
monocheck ee/app/assets/javascripts/packages_and_registries/artifact_registry/repositories/detail/repository_detail.vue 'isAwaitingMainContent || hasNoMainContent'
monocheck ee/app/assets/javascripts/packages_and_registries/artifact_registry/repositories/detail/repository_detail.vue 'notifyOnNetworkStatusChange: true'
monocheck ee/app/assets/javascripts/packages_and_registries/artifact_registry/repositories/detail/artifacts_table.vue 'awaitRefetchQueries: true'
monocheck config/authz/roles/organization_user.yml 'read_artifact_registry'
monocheck ee/app/assets/javascripts/packages_and_registries/artifact_registry/constants.js "REPOSITORY_KIND_HOSTED = 'HOSTED'"
monocheck ee/spec/frontend/fixtures/artifact_registry/repository_upstream_repositories.rb "let(:format) { 'docker' }"
if mono ee/lib/artifact_registry/client.rb | grep "def repository_statistics" >/dev/null; then
  echo "FAIL: repository_statistics already exists on master"; exit 1
fi

echo OK

Review notes

  • !2998 (merged) (draft, another author) also edits S05. This MR avoids the lines it touches (the upstream Dependencies row and the end of Resolutions), so expect at most a trivial rebase.
  • The plan records two existing spec/code divergences and leaves them alone: Downloads renders on every format though S05 says Maven only, and the remote label appends "cached" though S05 says it uses the hosted label.

Related to gitlab-org/gitlab#628241

Edited by Narendran

Merge request reports