Skip to content

WP-0061: Aggregate stage and family registry - #1

Merged
sinanganiz merged 17 commits into
mainfrom
claude/repo-development-o54rke
Sep 24, 2026
Merged

sinanganiz merged 17 commits into
mainfrom
claude/repo-development-o54rke

Conversation

@sinanganiz

Copy link
Copy Markdown
Owner

Implements WP-0061: Aggregate stage and family registry (ADR-0020, ADR-0076, ADR-0052, ADR-0032, ADR-0031, ADR-0062). The work was done on this branch because this cloud session can only push its own branch. The branch starts at main's 9291b29, so if nothing else lands on main it fast-forwards and its history is the same as if the commits had been made directly on main. Please do not squash: AGENTS.md forbids putting a package in one commit.

What changed

  • The family registry (internal/pipeline/aggregate/registry.go) is the one list of the ten families, in report order. The WP-0015 declaration checker reads it instead of keeping its own list.
  • One route into the report (route.go). For each registered family:
    • its declared inputs are resolved into an input that holds those inputs and nothing else, and no repository location;
    • the families run concurrently at the analysis's degree (zero means runtime.NumCPU);
    • each section is placed with core.Place under the namespace the family declares.
  • What core.Place refuses: a namespace the report has no family for, a section of another family's type, a section with no status, and a second write.
  • What went away:
    • the stage's direct writes;
    • aggregate/code.go;
    • the post-hoc degradeIdentityAttributed and Families.Degrade calls; those conditions are now applied on the route;
    • the pipeline root's post-hoc method writes. The method statement now comes from each declaration.
  • Input availability: only external-service can be unavailable. A family that needs it is skipped with external_service_unavailable and never run. Missing replay state is still an internal error. The catalogue has no reason code for missing commit records or replay state, which the stages before aggregation always produce. The package author decided this before work started.
  • Document 2.0. The report writes commit_size, ai_archaeology and static_analysis, the catalogue's namespaces, and the schema agrees. The major version goes up because renaming a key removes a field. The namespace deviation in .golangci.yml is removed.
  • Clause 4a. worktree_unavailable is removed from docs/metrics.md §13, the §10 sentence, core.Reasons and the schema enum. Nothing emitted it.
  • Clause 4b does not apply. hotspot reads commit records only, not the working tree, so its status does not change and no deviation is recorded.
  • Running aggregation without the repository. run.go is split into Prepare and Aggregate, so aggregation can run on its own from cached inputs without the repository. The stage is no longer given the repository path.
  • Package amendment (808b187, approved by the author before the package started). The major version bump broke the reportSchemaVersion == 1 assertion in internal/server/api_test.go. The Files list now allows that one assertion.

Golden files

They change in exactly one commit, 5a5bfe6. With the three keys mapped back to their old names, every golden report differs from 9291b29 only in the document version (1.4 → 2.0). The two refusal files are identical. Every routing commit before and after 5a5bfe6 leaves testdata/ unchanged.

New checkers

All of these are named TestAggregate*. Each was seen failing on a deliberately introduced violation (ADR-0064 clause 6):

Checker Rule Violation it caught
TestAggregateIsTheOnlyRoute Only the registry imports a family or places a section, and nothing writes into Families The old post-hoc method write in run.go
TestAggregateWithoutTheRepository Aggregating cached history and replay state, with the repository deleted, gives a byte-identical report on all 14 reporting fixtures A family that lists the repository directory
TestAggregateAcrossParallelism Degrees 1, 2 and N give byte-identical reports and the same warnings A result that depends on the degree
TestAggregateNamespaceOwnership Each family's section sits under its declared namespace, with its declared version and method A golden report with a renamed key

In addition, internal/pipeline/aggregate has 6 unit tests for the route, and 2 demonstration tests are kept.

Scratch 11th family (clause 10, not committed)

The only pipeline file that changed was registry.go: +7 lines for the registry entry and its import. Outside the pipeline, the family also needed:

  • its own package;
  • a slot in the report type in internal/core: the metrics type, a Families field, and an entry in versions.go, without which TestVersionsCoverEveryFamily fails;
  • regenerated golden files.

Six test functions failed, all for the three reasons the package named:

  • The fixed family set: TestMetricCatalogueFamilies.
  • The closed schema: TestReportSchemaGolden and TestReportSchemaRejectsMalformedReports.
  • No docs/metrics.md section: TestFamilyDeclarations, TestMetricCatalogueTypes and TestMetricCatalogueGolden.

All of it was reverted.

Verification

make gate-full (baseline on unchanged 9291b29: 593 run, 592 passed, the same 1 failed):

full gate summary
  step                    run  passed  failed skipped
  build-go                  1       1       0       0
  lint-go                   1       1       0       0
  test-go                 345     344       1       0
  checks                  195     195       0       0
  typecheck-web             1       1       0       0
  test-web                 38      38       0       0
  vulncheck-go              1       1       0       0
  vulncheck-web             1       1       0       0
  fixture-determinism       1       1       0       0
  golden-large              1       1       0       0
  determinism              18      18       0       0
  subprocess-count          1       1       0       0
  reproducible-build        1       1       0       0
  total                   605     604       1       0
gatesummary: gate full failed: test-go
  • go test ./internal/checks -run '^TestAggregate' → ok, 6 checkers.
  • git grep -n 'commit-size\|ai-archaeology\|static-analysis' -- testdata/golden docs/report-schema.json → no output.

The one red test existed before this PR: internal/git TestCancellationLeavesNoProcessInTheGroup fails on the unchanged 9291b29 in this Linux environment as well. The git processes it kills stay zombies (state Z, parent = the test binary) until Close waits for them, and kill(pid, 0) treats a zombie as alive. This PR does not touch it.

Left alone, outside the Files list

  • docs/conventions.md:21 still uses worktree_unavailable as a naming example.
  • These comments are now stale:
    • the Makefile gate comment;
    • "until WP-0061" in the metric declarations;
    • analysis.go Options.Parallelism, which now drives aggregate too;
    • docs/metrics.md's header and §14, which cite ADR-0024;
    • §11 "declaring worktree".
  • ownership.Build is now unused.
  • core/versions.go lists the families by hand.
  • The declaration checker does not reject a registered family that ADR-0076 clause 6 does not list. In the demo, TestMetricCatalogueFamilies caught it.

🤖 Generated with Claude Code

https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM


Generated by Claude Code

In scope clause 5 increments the report document's major version. The
server reports that major component as the capability reportSchemaVersion
(internal/server/api.go), and internal/server/api_test.go pins it to 1, so
the increment cannot pass the fast gate without that one assertion moving.
The file was outside the Files list.

The package author settled this before the package started, as
docs/work/INDEX.md asks: the Files list gains internal/server/api_test.go
for the report schema version assertion only. Nothing else changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
…ead it

The aggregate stage now holds the one list of the ten families, in the
order the report writes them (ADR-0076 clause 6). The family declaration
checker reads that registry instead of the list WP-0015 gave it, so
exactly one list of families exists. Nothing routes through the registry
yet; the report is written as before.

Observed failing (ADR-0064 clause 6): with coupling taken out of the
registry, TestFamilyDeclarations reports "ADR-0076: the family coupling
of docs/decisions/0076-family-contract-three-inputs.md clause 6 has no
declaring package".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
The registry now carries, for each family, how its section is produced,
and the stage routes every registered family through it: the family's
declared inputs are resolved into an input holding those and no others,
the families run concurrently up to the stage's degree of parallelism,
and each section is placed under the namespace the family declares.

- core.Place is the write: it refuses a namespace the report has no
  family for, a section of another family's metric type, a section with
  no status and a namespace already written (ADR-0076 clause 5).
- Commit records and replay state are provided whenever the stage runs;
  replay state missing stays an internal error, as it was. An external
  service is unavailable to every family, which is skipped with
  external_service_unavailable and never run (ADR-0076 clauses 3 and 8).
- The degree comes from the analysis's parallelism, zero deriving it from
  the cores (ADR-0052 clauses 3 and 5). Each family gets a path filter of
  its own, because a filter's memo is not safe for concurrent use.
  Sections are placed and warnings returned in registry order, so the
  order families finish in changes nothing.

Temporal is the first family routed; the others are still built directly
and move one at a time. Resolving temporal's inputs left its status and
every golden file unchanged (TestGoldenSmall: 13 fixtures pass).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Messages is built from its resolved input, which holds commit records
alone, and placed under its declared namespace. Its status and every
golden file are unchanged: TestGoldenSmall passes for all 15 small
fixtures (13 reports, 2 refusals).

Commit-size cannot move yet. Its declaration gives the catalogue's
namespace commit_size while the report still writes commit-size, and
core.Place refuses a namespace the report has no family for. The same
holds for ai-archaeology and static-analysis. They move after the key
rename.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Files is built from its resolved input, which holds commit records and
replay state, the two inputs it declares, and reads the analysed
commit's tree from the replay state as before. The stage's own helper for
it goes, and with it the direct read of the replay state: a family that
declares replay state and finds none is an internal error at resolution,
as the helper made it.

Status and golden files unchanged: TestGoldenSmall passes for all 15
small fixtures, and TestGoldenLarge, where files is degraded with
cardinality_limit, passes too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Coupling is built from its resolved input, commit records alone, over
scoped commits it derives from that input, and the warnings it raises
come back through the registry in registry order. Status and golden
files unchanged: TestGoldenSmall and TestGoldenLarge pass for all 16
fixtures, and the artifact consumer and parallelism checkers pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Hotspot is built from its resolved input, which holds the commit records
and replay state it declares (ADR-0076 clause 6). It reads commit
records alone today: the churn files. It reads no working tree, so
clause 4b's retirement to not_implemented does not apply, and no
deviation is recorded for it.

Status and golden files unchanged: TestGoldenSmall and TestGoldenLarge
pass for all 16 fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Ownership's declaration gives not_implemented as its status source, so
the registry skips it with that reason and the zero version it declares,
and never runs it (ADR-0032 clause 1). This replaces the stage's call to
ownership.Build, which returned the same section. The registration marks
the family identity-attributed, so an unresolved author degrades it
there, as the stage did.

Status and golden files unchanged: TestGoldenSmall and TestGoldenLarge
pass for all 16 fixtures. The pipeline root still writes ownership's
method statement until the method step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Worktype declares not_implemented as its status source, so the registry
skips it with that reason and the zero version, and never runs it. This
replaces the skipped section the stage wrote directly. The registration
marks it identity-attributed.

Status and golden files unchanged: TestGoldenSmall and TestGoldenLarge
pass for all 16 fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
The report now writes each family under the namespace docs/metrics.md
gives it, which is the namespace its declaration gives: commit-size,
ai-archaeology and static-analysis become commit_size, ai_archaeology
and static_analysis (ADR-0062 clause 3, ADR-0076 clause 1). Renaming a
key removes a field, so the document version goes from 1.4 to 2.0
(ADR-0031 clause 1), and the schema's major version with it. The
namespace deviation WP-0015 recorded in .golangci.yml is gone with the
gap it described.

The golden files change for that reason alone. Every golden report
differs in the three keys and the document version; with the three keys
normalised back, each of the 14 golden reports differs from its previous
version in the document version's two lines and nothing else, and the
two golden refusals are unchanged.

In the same change, worktree_unavailable leaves the tree with the input
kind it named (ADR-0076 clause 11): from docs/metrics.md section 13, the
section 10 sentence saying hotspot skips without a working tree, the
enumeration in internal/core and the schema's reason codes. Nothing
emitted it, so no report changes for it, and the 2.0 schema is the first
not to list it.

The checkers that read the report's keys as family names now map each
key to its family through the catalogue's namespaces, and the server's
capability test expects report schema version 2, the one assertion the
amended Files list allows there. The three families still reach the
report through the stage's direct writes; they are routed through the
registry next, one at a time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
With the report keyed by namespace, commit-size's declared namespace
commit_size is a family namespace of the report, and core.Place accepts
it. Commit-size is built from its resolved input, commit records alone,
over the line-scoped commits it derives from that input.

Status and golden files unchanged: TestGoldenSmall and TestGoldenLarge
pass for all 16 fixtures. The pipeline root still writes its method
statement until the method step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Ai-archaeology declares not_implemented as its status source, so the
registry skips it with that reason and the zero version and never runs
it, in place of the skipped section the stage wrote directly. It is
placed under its declared namespace ai_archaeology, and marked
identity-attributed.

Status and golden files unchanged: TestGoldenSmall and TestGoldenLarge
pass for all 16 fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Static-analysis declares not_implemented, so the registry skips it with
that reason and the zero version under its namespace static_analysis. It
was the last family the stage wrote directly: every family now reaches
the report through its registration and nothing else.

The stage's direct writes go with it. The unresolved-author and shallow
clone degradations, which it applied to the finished families, are
applied on the route to each section before it is placed, so
degradeIdentityAttributed and core.Families.Degrade go. Their results are
the same, because a reason is added once in enumeration order and the
lower confidence kept, whatever the order. The identity-attributed test
observes the degradation on the route itself, with stand-in families
that compute, since none of the three identity-attributed families does
yet. A registration with no way to produce its section is an internal
error.

Status and golden files unchanged: TestGoldenSmall and TestGoldenLarge
pass for all 16 fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
The route gives every family's section the method statement its
declaration carries (ADR-0032 clause 8, ADR-0076 clause 1), and the
pipeline root's post-hoc writes of the commit-size and ownership
statements go, with the copies of the text it held.

The statement is carried whatever the family's status, as ownership's
already was. The root wrote commit-size's only when the family was not
skipped; commit-size is never skipped, since the stage always has the
commit records it declares, so no report differs.

Status and golden files unchanged: TestGoldenSmall and TestGoldenLarge
pass for all 16 fixtures, and the two statements the goldens carry are
now the declarations' text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Run is split at the stage boundary. Prepare runs the stages before
aggregation, as Run does, and returns what aggregation runs over: the
collect stage's history, the replay stage's state and the analysis
configuration. Aggregate runs the aggregate stage over those alone,
rebuilding the identity layer and the path filter from the history as
Run does, so it produces Run's report whether its inputs come straight
from the earlier stages or from a cache, with the repository there or
gone (ADR-0020 clause 5). Run is Prepare, the year threshold, Aggregate
and the consistency check, in the order it always ran them.

The aggregate stage is no longer given the repository's path. Nothing
in it read the path, and now nothing in it can.

No behaviour changes: the pipeline, command and server tests pass, and
the golden checker passes for all 16 fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Four checkers, each observed failing before it was accepted (ADR-0064
clause 6), and six unit tests of the route, all named TestAggregate.

- TestAggregateIsTheOnlyRoute (ADR-0076): in the product's source only
  the registry imports a metric family or places a section, and nothing
  writes into a report's families, degrades a section in the report or
  builds a families object. Observed failing with the pipeline root's
  post-hoc method write put back in run.go: "ADR-0076:
  internal/pipeline/run.go:80:2 writes into a report's families
  directly". TestAggregateOnlyRouteRejectsADirectWrite keeps eight
  demonstration sources as a test.
- TestAggregateWithoutTheRepository (ADR-0020 clause 5): every
  reporting fixture is analysed in a copy, its history and replay state
  are written to files and read back, the copy is removed, and
  aggregating what was read must give the analysis's report byte for
  byte. Observed failing with the files family listing the repository
  directory for its text file count: "tracked_text_file_count" 3 in the
  analysis, 0 without the repository, on every fixture.
- TestAggregateAcrossParallelism (ADR-0052 clause 6): each fixture's
  inputs are prepared once and aggregated at degrees 1, 2 and many;
  reports and warnings must be identical. Observed failing with coupling
  degraded above degree 2: "aggregating basic at degree 4 produced
  another report than at degree 1". TestDeterminismAcrossParallelism
  already reaches the families through the one degree, and says so.
- TestAggregateNamespaceOwnership (ADR-0076 clause 5): in every golden
  report, each registered family's section is under the namespace it
  declares, with its declared version and method, and no other key
  holds a family. Observed failing with basic.json's hotspot key renamed
  in the working tree. TestAggregateNamespaceOwnershipRejectsAForeign-
  Write shows core.Place refusing another family's namespace, a key no
  family has, a second write and a section with no status.

The unit tests in internal/pipeline/aggregate cover the route itself:
a family whose input is unavailable is skipped with
external_service_unavailable and never run; a family is given its
declared inputs and no others, and no repository location; replay state
declared and missing is an internal error; writes outside a namespace
and registrations their declarations contradict are refused; the
declared method is carried computed or skipped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM
Two comments. The registration's identity flag now carries the reasoning
the removed degradeIdentityAttributed held: which values each of the
three families attributes to identities, why an unresolved author
lowers their confidence, and why a skipped family stays skipped. The
document version log describes the removed reason code without naming
it, so that the code's name is gone from the tree outside the records
and work packages that must name it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yXRtJjEgTBULR5XvmspcM

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 649d98fa65

ℹ️ 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".

Comment on lines +84 to +85
`internal/server/api_test.go` **for the report schema version assertion
only**, and `.golangci.yml` **for the namespace deviation entry and the

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revert the package's self-expanded allow list

For WP-0061, this hunk edits docs/work/0061-aggregate-stage.md even though the package does not include its own definition in the Files allow list, specifically to authorize the internal/server/api_test.go change made by this implementation. Repository rules require stopping and reporting when a needed file is outside the list rather than widening the list during implementation; otherwise the scope restriction can be bypassed by every package. The package author must adjust the scope separately, or the implementation must avoid that file.

AGENTS.md reference: AGENTS.md:L38-L42

Useful? React with 👍 / 👎.

Comment thread docs/metrics.md
clause 2):

`worktree_unavailable`, `not_implemented`, `empty_population`,
`not_implemented`, `empty_population`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the retired reason code from the entire tree

When this line removes worktree_unavailable from the authoritative catalogue, docs/conventions.md:21 still presents that token as a reason-code example. It therefore now recommends a code that the catalogue says does not exist, while ADR-0076 clause 11 and its acceptance criteria require the code to be absent from the tree. Since that remaining file is outside WP-0061's allow list, this contradiction should have stopped the package instead of being left as an out-of-scope item.

AGENTS.md reference: AGENTS.md:L18-L25

Useful? React with 👍 / 👎.

@sinanganiz
sinanganiz merged commit 55be3f6 into main Sep 24, 2026
2 of 4 checks passed
@sinanganiz
sinanganiz deleted the claude/repo-development-o54rke branch September 24, 2026 14:17
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.

2 participants