feat(managementapi): npm repo statistics (S17 Phase 8 plan: 9/13)

Hayley Swimelarrequested to merge
316/step-9-npm-repository-statistics into main

Why

The repository-statistics route resolves its repository and refuses every non-hosted kind, then answers 501 on all three format arms. This fills the npm one: the three stored counters the repository resource already serves, plus the four live counts npm's schema declares.

Builds on the resolution gate in chore(managementapi): repository stats gate (S1... (!1994 - merged) • Hayley Swimelar • 19.4, now merged, so this branch is a plain delta on main.

What (the non-obvious parts)

The dist-tag arm's version join carries no soft-delete filter, deliberately. A dist-tag counts when its package is visible, since npm_tags has no soft_deleted_at of its own. The arm reuses npmDistTagVersionJoin(), the same expression the dist-tag list uses unfiltered, and the spec pins live counts to match the lists, so a version-visibility gate here would diverge from the list in exactly the state it filters. Three tests pin it, two seeding states no write path produces.

One statement, four scalar subqueries, no FROM (the recomputeNamespaceComponentsCountStmt precedent). That is also what lets npm_versions appear in three arms without a table alias, and what avoids the fan-out a flat LEFT JOIN with COUNT(DISTINCT) would scan.

Scoped by npm_repositories.id, not repositories.id. That is the column every npm management list is scoped by, so count-equals-list is checkable by reading two predicates. The child row resolves through the existing NpmVersionRepositories seam, so this adds exactly one Deps field. A missing binding under a hosted repository is a logged 500: a broken invariant, read the way resolveNpmRepositoryID reads it.

Both seams take the request-resolved namespace id, not the row's copy, which would agree with a leak by construction. chore(managementapi): repository stats gate (S1... (!1994 - merged) • Hayley Swimelar • 19.4 refuses a mismatched row above the format dispatch, so the npm pin that drove a foreign row through the handler is deleted here rather than kept: the gate answers first and the arm never runs, which makes that pin unreachable rather than redundant. TestStatisticsNpmRepository_CountsScopeToTheChildRow's fixture resolves the request to the same namespace id the row already carries, so its assertions pass whichever of the two a seam reads: the shared gate is what holds the row-vs-request property now, not this test.

Stored figures serve verbatim. A counter the buffered pipeline has driven below zero reaches the body unclamped. chore(api): drop statistics floors the counter ... (!1995 - merged) • Hayley Swimelar • 19.4 dropped the minimum: 0 floors from StoredArtifactsCount and StoredSizeBytes, so that exchange now validates inside the sweep instead of sitting beside it, which is what Statistics contract declares minimum: 0 that no... (#921 - closed) • Unassigned asked for. The figures themselves stay pinned in the unit suite, because the schema no longer constrains them.

Read cost is unbounded and known to be. ADR-004 caps versions per package, not packages per repository. The spec prices this scope as rare and names denormalized counters as the fallback, and the trigger for that fallback is database_query_duration_seconds{name="npm_packages_select_aggregate_entity_counts"} rather than the route's status distribution, because a client-abandoned read records as 200 with zero bytes and no error line.

Two files here are step 4's. statistics_repository_test.go and its integration twin relax the shared family-arm helper from "answers 501" to "reached an arm", now that the npm arm answers 200. That helper's text is byte-identical to steps 8 and 10's, measured at their tips 9d767f5d8 and 2ef3a1d67, so the branches merge as one edit. The same file's TestRepositoryStatistics_HostedReachesEveryFamilyArm doc comment rested on identical arm bodies, which this arm ends, and the correction is one-sided so far. The filled step anchors stay in place: deleting a line adjacent to the sibling arms' anchors conflicts with steps 8 and 10, so draining all three is a wave-close task.

The Deps seam literal collides on a same-delta edit. Three branches raising handler_test.go's count by one merge as a single edit with no conflict, so the collision surfaces as a red walk rather than as a conflict. This branch reads 83 against main's 82, and one more field lands with the Maven arm in feat(managementapi): maven repo statistics (S17... (!2028 - merged) • Hayley Swimelar • 19.4 and with the container arm in feat(managementapi): container repo stats (S17 ... (!2030) • Hayley Swimelar • 19.4. Each of the three sets its literal to what TestNewHandler_PanicsOnNilDependency reports after its own rebase, never by arithmetic. The merged-results pipeline catches a stale one before main does, so the cost is a re-push.

Accepted duplication: the visible-npm-version predicate now has a third home beside recomputeNpmVersionsStmt and the version list. Keyed on the child id rather than the parent, and the reconcile file's own doc argues against folding such walks together.

Diff size

1670 reviewable LOC, over the 500 guardrail: 212 production (npm_packages.go 123, statistics_repository.go 57, query_names.go 13 of which 12 are gofmt realignment, statistics_resources.go 11, handler.go 4, wire_management.go 4) and 1458 test. The production half is inside the band, and splitting the aggregate from its handler arm would land a 501 route over an unreachable store.

Test plan

  • go test ./... green repo-wide, go-lint-ci 0 issues, comment caps clean against origin/main.
  • Both integration packages green against Postgres tuned as CI tunes it (max_locks_per_transaction=1024, dynamic_shared_memory_type=mmap). A stock fallback at 64 yields SQLSTATE 53200 on unrelated pre-existing tests.
  • Datastore integration compares every figure to the length of its own Phase 3 list rather than a literal, with a soft-delete case at each level, tenant scoping, and the argument guards.
  • TestAggregateNpmRepositoryEntityCountsStmt pins the statement without a database, the shape the file's other two builders already carry. It counts fragments rather than containing them, because every predicate holds in more than one arm and a Contains passes on the first while the arm that lost it over-counts silently.
  • Six assertions are mutation-verified, not argued. Each mutation was applied, the named test observed red, and the mutation reverted:
    • Swapping two subqueries' aliases in the projection reds the shape fixture and the statement test. Its four figures are pairwise distinct at {2,3,5,4} for that reason: at the earlier {2,3,3,2} the versions-for-files and packages-for-dist-tags swaps were both invisible, and both now red.
    • Tautologizing the four join-side namespace equalities reds the twin pin, {2,3,3,3} against {2,2,2,2}. That pin seeds one npm_packages.id under two namespaces beneath one shared npm_repositories.id.
    • Dropping npmDistTagVersionJoin()'s package arm reds the cross-package tag pin on the count and on the list, while their agreement stays green. That is why those two figures are literals rather than sums.
    • A 500 writer echoing its error reds both binding-failure rows. The absent-binding row previously asserted against text its own arrangement never produced.
    • Removing the aggregate's repository log attribute reds its log-field test.
    • Restoring minimum: 0 on StoredArtifactsCount reds the sweep's drifted-counter row, which is what proves that row exercises the dropped floor rather than passing vacuously.
  • Handler table over a fake aggregate, handler integration through the mux, and the contract_test.go sweep, the drifted-counter exchange included.

No e2e scenario added: docs/testing/ carries no coverage for any Phase 8 statistics scope, so the first one belongs with the scope that completes the surface rather than with one format arm.

Context for LLM agents

Rationale

  • Filter the dist-tag arm's version join on soft_deleted_at. Rejected because the list leaves the same join unfiltered, so the filter diverges from the list in exactly the state it filters.
  • One flat FROM with LEFT JOIN and COUNT(DISTINCT). Rejected for the four-way fan-out, and because npm_versions appears in three arms so it would need aliases the shared join expression cannot carry.
  • Scope by repositories.id, joining npm_repositories inside the statement. Rejected because matching the lists' own scoping column is what makes count-equals-list checkable.
  • A per-statement context deadline. Rejected because a plain MVCC read cannot be lock-stuck and a re-run repeats the identical scan, the reasoning statementBoundStore records.
  • Keep the npm foreign-row pin beside the new shared gate. Rejected because the gate refuses the row before the arm executes, so the pin's own preconditions no longer hold and it fails rather than duplicating.

Consequences: the dist-tag agreement rests on one function owning the join for both count and list, so hand-inlining it splits them silently, and a count-against-list comparison cannot detect that split on its own.

Non-goals

Related to #316

Edited by Hayley Swimelar

Merge request reports