diff --git a/.golangci.yml b/.golangci.yml index 72e3704..8687a50 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,12 +25,6 @@ # section, and internal/pipeline/replay, which reads # no year (ADR-0074), counts their work-type change # events all the same -# the report writes the families commit-size, ADR-0062 WP-0061 -# ai-archaeology and static-analysis under those -# keys, while their declarations give the -# catalogue's namespaces commit_size, ai_archaeology -# and static_analysis (clause 3); nothing routes -# output through a declaration until WP-0061 # # Two rules from table 1 are enforced by a checker rather than here, because # this linter's configuration cannot express them (ADR-0055 clause 2): diff --git a/docs/metrics.md b/docs/metrics.md index 00c2d37..115866c 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -324,9 +324,6 @@ family's method field says so. | `hotspots` | Top 100 files by `hotspot_score`, with all three values. | | `churn_files` | Files receiving at least 5 analysed commits within any rolling 30-day window, limit 100. | -Without working tree access, the family is `skipped` with reason -`worktree_unavailable`. - --- ## 11. `static-analysis` @@ -373,7 +370,7 @@ so the set can be read mechanically. Family status codes, carried by a skipped or degraded family (ADR-0032 clause 2): -`worktree_unavailable`, `not_implemented`, `empty_population`, +`not_implemented`, `empty_population`, `cardinality_limit`, `low_classification_confidence`, `shallow_clone`, `limit_reached_commits`, `limit_reached_size`, `limit_reached_duration`, `limit_reached_memory`, `symlink_escaped_root`, `capability_unavailable_in_mode`, diff --git a/docs/report-schema.json b/docs/report-schema.json index 4c3b2e1..a32a9da 100644 --- a/docs/report-schema.json +++ b/docs/report-schema.json @@ -27,7 +27,7 @@ "major": { "type": "integer", "enum": [ - 1 + 2 ] }, "minor": { @@ -193,18 +193,18 @@ "families": { "type": "object", "additionalProperties": false, - "description": "Every family of ADR-0024 clause 5, each with a version and a status (ADR-0031 clause 2, ADR-0032).", + "description": "Every family of ADR-0076 clause 6, each under the namespace docs/metrics.md gives it (ADR-0076 clause 1, ADR-0062 clause 3), with a version and a status (ADR-0031 clause 2, ADR-0032). The namespaces became the keys in document version 2.0.", "required": [ "temporal", - "commit-size", + "commit_size", "messages", "files", "coupling", "ownership", "worktype", - "ai-archaeology", + "ai_archaeology", "hotspot", - "static-analysis" + "static_analysis" ], "properties": { "temporal": { @@ -278,7 +278,7 @@ } ] }, - "commit-size": { + "commit_size": { "oneOf": [ { "type": "object", @@ -570,7 +570,7 @@ "$ref": "#/$defs/skippedFamily", "description": "Not implemented yet; present as skipped (ADR-0032 clause 1)." }, - "ai-archaeology": { + "ai_archaeology": { "$ref": "#/$defs/skippedFamily", "description": "Not implemented yet; present as skipped (ADR-0032 clause 1)." }, @@ -645,7 +645,7 @@ } ] }, - "static-analysis": { + "static_analysis": { "$ref": "#/$defs/skippedFamily", "description": "Not implemented yet; present as skipped (ADR-0032 clause 1)." } @@ -749,7 +749,6 @@ "items": { "type": "string", "enum": [ - "worktree_unavailable", "not_implemented", "empty_population", "cardinality_limit", diff --git a/docs/work/0061-aggregate-stage.md b/docs/work/0061-aggregate-stage.md index 7f1abe9..aba7f5c 100644 --- a/docs/work/0061-aggregate-stage.md +++ b/docs/work/0061-aggregate-stage.md @@ -80,9 +80,10 @@ repository unreadable. `internal/pipeline/run.go`, `internal/core/**`, `internal/checks/**`, `docs/report-schema.json`, `testdata/**` golden files, `docs/metrics.md` **for the section 10 sentence and the section 13 code in clause 4a only**, `internal/metrics/hotspot/**` -and `internal/metrics/staticanalysis/**` **for their declarations only**, and -`.golangci.yml` **for the namespace deviation entry and the hotspot deviation -entry only**. +and `internal/metrics/staticanalysis/**` **for their declarations only**, +`internal/server/api_test.go` **for the report schema version assertion +only**, and `.golangci.yml` **for the namespace deviation entry and the +hotspot deviation entry only**. **Must not touch:** `internal/metrics/**` other than the two declarations above, `internal/pipeline/collect/**`, `internal/pipeline/replay/**`, `cmd/**`, `docs/decisions/**`, `docs/metrics.md`. diff --git a/internal/checks/aggregate_test.go b/internal/checks/aggregate_test.go new file mode 100644 index 0000000..a79abb9 --- /dev/null +++ b/internal/checks/aggregate_test.go @@ -0,0 +1,508 @@ +package checks + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "os" + "path/filepath" + "reflect" + "strconv" + "strings" + "testing" + + "github.com/sinanganiz/commitography/internal/core" + "github.com/sinanganiz/commitography/internal/pipeline" + "github.com/sinanganiz/commitography/internal/pipeline/aggregate" + "github.com/sinanganiz/commitography/internal/pipeline/collect" + "github.com/sinanganiz/commitography/internal/pipeline/render" +) + +// The aggregate stage's checkers (WP-0061). The registry is the only route by +// which a family's output reaches the report, and each family's section lands +// under the namespace the family declares (ADR-0076 clauses 1 and 5). The +// stage runs from cached collect output and replay state with the repository +// gone (ADR-0020 clause 5), and the degree it runs families at changes +// nothing (ADR-0052 clause 6). + +// registryFile is the one source file that runs metric families and places +// their sections into a report. +const registryFile = "internal/pipeline/aggregate/registry.go" + +// renderReport writes a report as the command writes report.json, and +// returns what it wrote. +func renderReport(t *testing.T, record int, r *core.Report) string { + t.Helper() + out := filepath.Join(t.TempDir(), render.ReportFile) + if err := render.WriteReportJSON(r, out); err != nil { + fatal(t, record, "writing the report: %v", err) + } + data, err := os.ReadFile(out) + if err != nil { + fatal(t, record, "reading the report back: %v", err) + } + return string(data) +} + +// cacheInputs writes what aggregation runs over to files, the collect +// stage's history in its own artifact format and the replay state as JSON, +// and returns a function reading them back. +func cacheInputs(t *testing.T, in *pipeline.Inputs) func() pipeline.Inputs { + t.Helper() + dir := t.TempDir() + historyFile, replayFile := filepath.Join(dir, "history.json"), filepath.Join(dir, "replay.json") + if err := collect.WriteHistory(in.History, historyFile); err != nil { + fatal(t, 20, "writing the history artifact: %v", err) + } + state, err := json.Marshal(in.Replay) + if err != nil { + fatal(t, 20, "encoding the replay state: %v", err) + } + if err := os.WriteFile(replayFile, state, 0o644); err != nil { + fatal(t, 20, "writing the replay state: %v", err) + } + analysis := in.Analysis + return func() pipeline.Inputs { + history, err := collect.ReadHistory(historyFile) + if err != nil { + fatal(t, 20, "reading the history artifact: %v", err) + } + data, err := os.ReadFile(replayFile) + if err != nil { + fatal(t, 20, "reading the replay state: %v", err) + } + var replayed core.ReplayState + if err := json.Unmarshal(data, &replayed); err != nil { + fatal(t, 20, "decoding the replay state: %v", err) + } + return pipeline.Inputs{History: history, Replay: &replayed, Analysis: analysis} + } +} + +// TestAggregateWithoutTheRepository enforces ADR-0020 clause 5: the aggregate +// stage is stateless and re-runnable without touching git. Every fixture the +// analysis reports on is analysed in a copy. The collect stage's history and +// the replay stage's state are written to files, the copy is removed, and +// aggregating what is read back must produce the report the analysis +// produced, byte for byte. +func TestAggregateWithoutTheRepository(t *testing.T) { + t.Parallel() + repo := openRepository(t) + ctx := context.Background() + for _, fixture := range collectedFixtures(t, repo) { + dir := filepath.Join(t.TempDir(), fixture) + copyTree(t, filepath.Join(repo.root, "testdata", "fixtures", fixture), dir) + analyzer, opts := newAnalyzer(), pipeline.Options{RepoPath: dir} + + result, err := analyzer.Run(ctx, opts, nil) + if err != nil { + fatal(t, 20, "analysing %s: %v", fixture, err) + } + want := renderReport(t, 20, result.Report) + prepared, err := analyzer.Prepare(ctx, opts) + if err != nil { + fatal(t, 20, "preparing %s: %v", fixture, err) + } + cached := cacheInputs(t, prepared) + + if err := os.RemoveAll(dir); err != nil { + fatal(t, 64, "removing the %s repository: %v", fixture, err) + } + if _, err := os.Stat(dir); !errors.Is(err, fs.ErrNotExist) { + fatal(t, 64, "the %s repository is still there, so aggregating without it proves nothing", fixture) + } + aggregated, _, err := analyzer.Aggregate(ctx, cached(), opts) + if err != nil { + report(t, 20, "aggregating %s from its cached inputs, with the repository removed: %v", fixture, err) + continue + } + if got := renderReport(t, 20, aggregated); got != want { + report(t, 20, "aggregating %s from its cached inputs, with the repository removed, produced another "+ + "report than the analysis (- analysis, + aggregation):\n%s", fixture, unifiedDiff(want, got)) + } + } +} + +// TestAggregateAcrossParallelism is ADR-0052 clause 6 for the families: each +// fixture's inputs are prepared once, and aggregating them at degrees 1, 2 and +// many produces byte-identical reports and the same warnings in the same +// order. Collect is not run again, so a difference is the family degree's +// alone; TestDeterminismAcrossParallelism holds the whole analysis, collect +// included, to the same degrees. +func TestAggregateAcrossParallelism(t *testing.T) { + t.Parallel() + repo := openRepository(t) + ctx := context.Background() + analyzer := newAnalyzer() + for _, fixture := range collectedFixtures(t, repo) { + prepared, err := analyzer.Prepare(ctx, pipeline.Options{RepoPath: filepath.Join(repo.root, "testdata", + "fixtures", fixture)}) + if err != nil { + fatal(t, 52, "preparing %s: %v", fixture, err) + } + var first string + var firstWarnings []string + for i, degree := range parallelDegrees() { + aggregated, warnings, err := analyzer.Aggregate(ctx, *prepared, pipeline.Options{Parallelism: degree}) + if err != nil { + fatal(t, 52, "aggregating %s at degree %d: %v", fixture, degree, err) + } + got := renderReport(t, 52, aggregated) + if i == 0 { + first, firstWarnings = got, warnings + continue + } + if got != first { + report(t, 52, "aggregating %s at degree %d produced another report than at degree 1 "+ + "(- degree 1, + degree %d):\n%s", fixture, degree, degree, unifiedDiff(first, got)) + } + if !reflect.DeepEqual(warnings, firstWarnings) { + report(t, 52, "aggregating %s at degree %d raised the warnings %q, and at degree 1 %q", + fixture, degree, warnings, firstWarnings) + } + } + } +} + +// TestAggregateNamespaceOwnership is ADR-0076 clause 5 as every golden report +// carries it: each registered family's section is the one under the namespace +// the family declares, carrying the version and the method statement the +// family declares, and no family is written under any other key. The golden +// checker holds the golden reports to what the analysis produces. +func TestAggregateNamespaceOwnership(t *testing.T) { + t.Parallel() + repo := openRepository(t) + declared := map[string]core.FamilyDeclaration{} + for _, f := range aggregate.Registry() { + d := f.Declaration() + declared[d.Namespace] = d + } + checked := 0 + for _, file := range repo.tracked { + if !strings.HasPrefix(file, goldenDir+"/") || !strings.HasSuffix(file, ".json") { + continue + } + var document struct { + Families map[string]struct { + Version core.Version `json:"version"` + Status core.Status `json:"status"` + Reasons []string `json:"reasons"` + Method string `json:"method"` + } `json:"families"` + } + if err := json.Unmarshal([]byte(repo.read(t, 76, file)), &document); err != nil { + report(t, 76, "%s is not a report: %v", file, err) + continue + } + checked++ + for _, namespace := range sortedKeys(document.Families) { + if _, ok := declared[namespace]; !ok { + report(t, 76, "%s carries a family under %s, which no registered family declares as its namespace", + file, namespace) + } + } + for _, namespace := range sortedKeys(declared) { + d := declared[namespace] + section, ok := document.Families[namespace] + switch { + case !ok: + report(t, 32, "%s carries no section under %s, the namespace the family %s declares", file, + namespace, d.Name) + case section.Version != d.Version: + report(t, 31, "%s carries version %s under %s, and the family %s declares version %s", file, + section.Version, namespace, d.Name, d.Version) + case section.Method != d.Method: + report(t, 76, "%s carries another method statement under %s than the family %s declares", file, + namespace, d.Name) + case d.Status == core.StatusNotImplemented && + (section.Status != core.StatusSkipped || !containsString(section.Reasons, string(core.ReasonNotImplemented))): + report(t, 32, "%s gives %s the status %s %v, and the family %s declares it not implemented", file, + namespace, section.Status, section.Reasons, d.Name) + } + } + } + if checked == 0 { + fatal(t, 64, "no golden report is tracked under %s, so there is nothing to check", goldenDir) + } +} + +func containsString(list []string, want string) bool { + for _, s := range list { + if s == want { + return true + } + } + return false +} + +// TestAggregateNamespaceOwnershipRejectsAForeignWrite is the failure +// demonstration for the write itself (ADR-0064 clause 6): core.Place, the one +// write into a report's families, refuses a section under another family's +// namespace, under a key no family has, under a namespace already written, +// and a section with no status. +func TestAggregateNamespaceOwnershipRejectsAForeignWrite(t *testing.T) { + t.Parallel() + var families core.Families + churn := core.Computed(core.Version{Major: 1}, core.HotspotMetrics{}) + + if err := core.Place(&families, "files", churn); err == nil || !strings.Contains(err.Error(), "ADR-0076 clause 5") { + report(t, 64, "a hotspot section was placed under files, the files family's namespace: %v", err) + } + if err := core.Place(&families, "commit-size", core.Computed(core.Version{Major: 2}, core.CommitSizeMetrics{})); err == nil { + report(t, 64, "a section was placed under commit-size, which is no family's namespace") + } + if err := core.Place(&families, "temporal", core.Family[core.TemporalMetrics]{}); err == nil { + report(t, 64, "a section with no status was placed under temporal") + } + if err := core.Place(&families, "hotspot", churn); err != nil { + report(t, 76, "the hotspot section was refused under hotspot, its own namespace: %v", err) + } + if err := core.Place(&families, "hotspot", churn); err == nil || !strings.Contains(err.Error(), "one owner") { + report(t, 64, "a second section was placed under hotspot: %v", err) + } + if missing := families.Unplaced(); len(missing) != reflect.TypeOf(families).NumField()-1 || containsString(missing, "hotspot") { + report(t, 64, "after one placement the unplaced namespaces are %v", missing) + } +} + +// routeViolations returns what a parsed production file does that could put +// a family's output into a report other than through the registry: import a +// metric family, place a section, or write into a report's families, by +// assignment or by degrading a section in place. +// +// A write is recognised by what it is written through: a field named +// Families, or a variable declared as core.Families, a pointer to one, or the +// address of a report's Families field. Within internal/core the type is +// named Families alone. +func routeViolations(fset *token.FileSet, file string, parsed *ast.File) []string { + var out []string + corePath := modulePath + "/internal/core" + inCore := strings.HasPrefix(file, "internal/core/") + local := map[string]string{} + for _, spec := range parsed.Imports { + path, err := strconv.Unquote(spec.Path.Value) + if err != nil { + continue + } + name := filepath.Base(path) + if spec.Name != nil { + name = spec.Name.Name + } + local[name] = path + if strings.HasPrefix(path, modulePath+"/internal/metrics/") && file != registryFile { + out = append(out, fmt.Sprintf("%s imports the metric family %s; only the registry runs a family", + fset.Position(spec.Pos()), path)) + } + } + isCore := func(x ast.Expr, name string) bool { + if ident, ok := x.(*ast.Ident); ok { + return inCore && ident.Name == name + } + selector, ok := x.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != name { + return false + } + pkg, ok := selector.X.(*ast.Ident) + return ok && local[pkg.Name] == corePath + } + isFamiliesType := func(x ast.Expr) bool { + if star, ok := x.(*ast.StarExpr); ok { + x = star.X + } + return isCore(x, "Families") + } + // families holds the variables that hold a report's families. + families := map[string]bool{} + var throughFamilies func(ast.Expr) bool + throughFamilies = func(x ast.Expr) bool { + switch e := x.(type) { + case *ast.Ident: + return families[e.Name] + case *ast.SelectorExpr: + return e.Sel.Name == "Families" || throughFamilies(e.X) + case *ast.IndexExpr: + return throughFamilies(e.X) + case *ast.StarExpr: + return throughFamilies(e.X) + case *ast.ParenExpr: + return throughFamilies(e.X) + case *ast.UnaryExpr: + return e.Op == token.AND && throughFamilies(e.X) + } + return false + } + ast.Inspect(parsed, func(n ast.Node) bool { + switch node := n.(type) { + case *ast.Field: + if isFamiliesType(node.Type) { + for _, name := range node.Names { + families[name.Name] = true + } + } + case *ast.ValueSpec: + if node.Type != nil && isFamiliesType(node.Type) { + for _, name := range node.Names { + families[name.Name] = true + } + } + } + return true + }) + ast.Inspect(parsed, func(n ast.Node) bool { + switch node := n.(type) { + case *ast.AssignStmt: + if node.Tok == token.DEFINE { + for i, lhs := range node.Lhs { + if ident, ok := lhs.(*ast.Ident); ok && len(node.Rhs) == len(node.Lhs) && + throughFamilies(node.Rhs[i]) { + families[ident.Name] = true + } + } + return true + } + for _, lhs := range node.Lhs { + if throughFamilies(lhs) { + out = append(out, fmt.Sprintf("%s writes into a report's families directly; a family's "+ + "output reaches the report through its registration alone", fset.Position(lhs.Pos()))) + } + } + case *ast.IncDecStmt: + if throughFamilies(node.X) { + out = append(out, fmt.Sprintf("%s writes into a report's families directly", fset.Position(node.Pos()))) + } + case *ast.CompositeLit: + if isCore(node.Type, "Families") { + out = append(out, fmt.Sprintf("%s builds a report's families directly", fset.Position(node.Pos()))) + } + case *ast.CallExpr: + fun := node.Fun + if index, ok := fun.(*ast.IndexExpr); ok { + fun = index.X + } + if isCore(fun, "Place") && file != registryFile { + out = append(out, fmt.Sprintf("%s places a family section; only the registry does", + fset.Position(node.Pos()))) + } + if selector, ok := fun.(*ast.SelectorExpr); ok && selector.Sel.Name == "Degrade" && + throughFamilies(selector.X) { + out = append(out, fmt.Sprintf("%s degrades a section already in the report; the registry "+ + "applies every condition before it places the section", fset.Position(node.Pos()))) + } + } + return true + }) + return out +} + +// TestAggregateIsTheOnlyRoute enforces that registration is the only route by +// which a family's output reaches the report (ADR-0076 clauses 1 and 5): in +// the product's source, only the registry imports a metric family and places +// a section, and nothing writes into a report's families directly. The metric +// families' own packages are left out, since a family never holds a report, +// and so are the tests, which may build any report they examine. +func TestAggregateIsTheOnlyRoute(t *testing.T) { + t.Parallel() + repo := openRepository(t) + if !repo.isTracked(registryFile) { + fatal(t, 76, "%s is not tracked, so no registry exists to route through", registryFile) + } + fset := token.NewFileSet() + scanned := 0 + for _, file := range repo.tracked { + if !strings.HasSuffix(file, ".go") || strings.HasSuffix(file, "_test.go") || + strings.HasPrefix(file, "internal/checks/") || strings.HasPrefix(file, "internal/metrics/") { + continue + } + parsed, err := parser.ParseFile(fset, file, repo.read(t, 76, file), 0) + if err != nil { + fatal(t, 76, "cannot parse %s: %v", file, err) + } + scanned++ + for _, finding := range routeViolations(fset, file, parsed) { + report(t, 76, "%s", finding) + } + } + if scanned == 0 { + fatal(t, 64, "no production source was found, so nothing was checked") + } +} + +// TestAggregateOnlyRouteRejectsADirectWrite is the failure demonstration +// (ADR-0064 clause 6): the writes the stage and the pipeline root made before +// the registry, and a section placed outside it, are each found, and reading +// a report's families is not. +func TestAggregateOnlyRouteRejectsADirectWrite(t *testing.T) { + t.Parallel() + violations := func(file, source string) []string { + fset := token.NewFileSet() + parsed, err := parser.ParseFile(fset, file, source, 0) + if err != nil { + fatal(t, 64, "parsing a demonstration source: %v", err) + } + return routeViolations(fset, file, parsed) + } + const header = `import ( + "github.com/sinanganiz/commitography/internal/core" + "github.com/sinanganiz/commitography/internal/metrics/temporal" +) +` + for _, c := range []struct { + name, file, source string + want int + }{ + {"the pipeline root's post-hoc method write", "internal/pipeline/run.go", `package pipeline +import "github.com/sinanganiz/commitography/internal/core" +func f(r *core.Report) { r.Families.CommitSize.Method = "text" } +`, 1}, + {"a family run and written by the stage", "internal/pipeline/aggregate/aggregate.go", "package aggregate\n" + + header + `func f(r *core.Report, in core.Input) { r.Families.Temporal = temporal.Build(in, nil) } +`, 2}, + {"a family run by the registry and written directly", registryFile, "package aggregate\n" + header + + `func f(f *core.Families, in core.Input) { f.Temporal = temporal.Build(in, nil) } +`, 1}, + {"a families object built outside the registry", "internal/server/api.go", `package server +import "github.com/sinanganiz/commitography/internal/core" +func f() core.Report { return core.Report{Families: core.Families{}} } +`, 1}, + {"a section placed outside the registry", "internal/pipeline/aggregate/aggregate.go", `package aggregate +import "github.com/sinanganiz/commitography/internal/core" +func f(f *core.Families) error { return core.Place(f, "temporal", core.Family[core.TemporalMetrics]{}) } +`, 1}, + {"a write through the address of a report's families", "internal/pipeline/aggregate/aggregate.go", + `package aggregate +import "github.com/sinanganiz/commitography/internal/core" +func f(r *core.Report) { f := &r.Families; f.Ownership = core.Family[core.OwnershipMetrics]{} } +`, 1}, + {"a section degraded where it lies in the report", "internal/pipeline/aggregate/aggregate.go", + `package aggregate +import "github.com/sinanganiz/commitography/internal/core" +func f(r *core.Report) { r.Families.Temporal.Degrade(core.ReasonShallowClone, core.ConfidenceLow) } +`, 1}, + {"a write in core outside the placement", "internal/core/report.go", `package core +func (f *Families) reset() { f.Hotspot = Family[HotspotMetrics]{} } +`, 1}, + } { + if got := violations(c.file, c.source); len(got) != c.want { + report(t, 64, "%s gave %d findings, want %d: %v", c.name, len(got), c.want, got) + } + } + if clean := violations("internal/pipeline/render/render.go", `package render +import ( + "github.com/sinanganiz/commitography/internal/core" + "github.com/sinanganiz/commitography/internal/core/model" +) +func f(r *core.Report, c *model.Commit) core.Status { + c.Files = nil + s := r.Families.Temporal.Status + return s +} +`); len(clean) != 0 { + report(t, 76, "reading a report's families, or writing a field of the same name elsewhere, was "+ + "reported as a write: %v", clean) + } +} diff --git a/internal/checks/artifact_test.go b/internal/checks/artifact_test.go index cdcd3a4..0947373 100644 --- a/internal/checks/artifact_test.go +++ b/internal/checks/artifact_test.go @@ -203,7 +203,7 @@ func recordConsumerFamilies(t *testing.T, history *model.History, repoPath strin couplingFamily, _ := coupling.Build(core.ScopedCommits(in, in.LineScoped())) families := map[string]any{ "temporal": temporal.Build(in, in.Analyzed()).Metrics, - "commit-size": commitsize.Build(in, in.LineScoped()).Metrics, + "commit_size": commitsize.Build(in, in.LineScoped()).Metrics, "messages": messages.Build(in.Analyzed()).Metrics, "coupling": couplingFamily.Metrics, } diff --git a/internal/checks/catalogue_test.go b/internal/checks/catalogue_test.go index 1ff2594..b9d48f1 100644 --- a/internal/checks/catalogue_test.go +++ b/internal/checks/catalogue_test.go @@ -152,17 +152,29 @@ func familyStatusCodes(section string) map[string]bool { return out } +// namespaceOwners inverts the namespaces docs/metrics.md gives the families: +// the family whose namespace each report key is. +func namespaceOwners(namespaces map[string]string) map[string]string { + out := map[string]string{} + for family, namespace := range namespaces { + out[namespace] = family + } + return out +} + // typeViolations compares a Families-shaped type with the catalogue: every -// field is a family the catalogue defines, and every JSON name of that -// family's metric type is a metric its section defines. -func typeViolations(families reflect.Type, catalogue map[string]map[string]bool) []string { +// field's key is the namespace of a family the catalogue defines, and every +// JSON name of that family's metric type is a metric its section defines. +// owners gives the family each namespace belongs to. +func typeViolations(families reflect.Type, catalogue map[string]map[string]bool, owners map[string]string) []string { var out []string for i := 0; i < families.NumField(); i++ { field := families.Field(i) - family := jsonName(field) - metrics, known := catalogue[family] - if !known { - out = append(out, "the report type has a family "+family+", which "+metricsCatalogue+" does not define") + family, known := owners[jsonName(field)] + metrics := catalogue[family] + if !known || metrics == nil { + out = append(out, "the report type has a family under "+jsonName(field)+", which is the namespace of "+ + "no family "+metricsCatalogue+" defines") continue } mf, ok := field.Type.FieldByName("Metrics") @@ -198,13 +210,17 @@ func sortedKeys[V any](m map[string]V) []string { } // TestMetricCatalogueFamilies requires the report's families, the catalogue's -// family sections and ADR-0076 clause 6 to be one set. +// family sections and ADR-0076 clause 6 to be one set, with the report +// writing each family under the namespace the catalogue gives it (ADR-0076 +// clause 1, ADR-0062 clause 3). func TestMetricCatalogueFamilies(t *testing.T) { t.Parallel() repo := openRepository(t) - catalogue := catalogueFamilies(repo.read(t, 62, metricsCatalogue)) + content := repo.read(t, 62, metricsCatalogue) + catalogue := catalogueFamilies(content) + namespaces := catalogueNamespaces(content) record := recordFamilies(repo.read(t, 76, familyRecord)) - if len(catalogue) == 0 || len(record) == 0 { + if len(catalogue) == 0 || len(namespaces) == 0 || len(record) == 0 { fatal(t, 64, "no family was read from %s or %s; the checker would pass vacuously", metricsCatalogue, familyRecord) } @@ -217,8 +233,9 @@ func TestMetricCatalogueFamilies(t *testing.T) { if catalogue[f] == nil { report(t, 62, "%s clause 6 names the family %s, which %s has no section for", familyRecord, f, metricsCatalogue) } - if !inReport[f] { - report(t, 32, "the family %s is absent from the report type, so it is absent from every report", f) + if namespace, ok := namespaces[f]; !ok || !inReport[namespace] { + report(t, 32, "the family %s is absent from the report type under the namespace %s gives it, so it "+ + "is absent from every report", f, metricsCatalogue) } } for _, f := range sortedKeys(catalogue) { @@ -226,9 +243,11 @@ func TestMetricCatalogueFamilies(t *testing.T) { report(t, 76, "%s has a section for %s, which %s clause 6 does not list", metricsCatalogue, f, familyRecord) } } - for _, f := range sortedKeys(inReport) { - if !record[f] { - report(t, 76, "the report type carries the family %s, which %s clause 6 does not list", f, familyRecord) + owners := namespaceOwners(namespaces) + for _, key := range sortedKeys(inReport) { + if family, ok := owners[key]; !ok || !record[family] { + report(t, 76, "the report type carries a family under %s, which is the namespace of no family %s "+ + "clause 6 lists", key, familyRecord) } } } @@ -238,8 +257,9 @@ func TestMetricCatalogueFamilies(t *testing.T) { func TestMetricCatalogueTypes(t *testing.T) { t.Parallel() repo := openRepository(t) - catalogue := catalogueFamilies(repo.read(t, 62, metricsCatalogue)) - for _, v := range typeViolations(reflect.TypeOf(core.Families{}), catalogue) { + content := repo.read(t, 62, metricsCatalogue) + owners := namespaceOwners(catalogueNamespaces(content)) + for _, v := range typeViolations(reflect.TypeOf(core.Families{}), catalogueFamilies(content), owners) { report(t, 62, "%s", v) } } @@ -259,7 +279,8 @@ func TestMetricCatalogueRejectsAnInventedMetric(t *testing.T) { Mood core.Family[struct{}] `json:"mood"` } catalogue := map[string]map[string]bool{"temporal": {"hour_histogram": true}} - got := strings.Join(typeViolations(reflect.TypeOf(invented{}), catalogue), "\n") + owners := map[string]string{"temporal": "temporal"} + got := strings.Join(typeViolations(reflect.TypeOf(invented{}), catalogue, owners), "\n") for _, want := range []string{"hour_weekday_grid", "mood"} { if !strings.Contains(got, want) { report(t, 64, "the catalogue checker did not report the invented %s; it reported:\n%s", want, got) @@ -326,7 +347,9 @@ func TestMetricCatalogueRejectsAnInventedIdentityField(t *testing.T) { func TestMetricCatalogueGolden(t *testing.T) { t.Parallel() repo := openRepository(t) - catalogue := catalogueFamilies(repo.read(t, 62, metricsCatalogue)) + content := repo.read(t, 62, metricsCatalogue) + catalogue := catalogueFamilies(content) + owners := namespaceOwners(catalogueNamespaces(content)) codes := familyStatusCodes(catalogueSection(t, repo, reasonSection)) if len(codes) == 0 { fatal(t, 64, "no family status code was read from %s section 13", metricsCatalogue) @@ -364,7 +387,7 @@ func TestMetricCatalogueGolden(t *testing.T) { for _, name := range sortedKeys(document.Families) { family := document.Families[name] for _, metric := range sortedKeys(family.Metrics) { - if !catalogue[name][metric] { + if !catalogue[owners[name]][metric] { report(t, 62, "%s carries the metric %s.%s, which %s does not define", file, name, metric, metricsCatalogue) } } diff --git a/internal/checks/determinism_test.go b/internal/checks/determinism_test.go index 042e641..a3752d7 100644 --- a/internal/checks/determinism_test.go +++ b/internal/checks/determinism_test.go @@ -28,9 +28,10 @@ import ( // hide. // // The parallelism half: the collect stage splits a large history across -// concurrent readers, and the degree changes neither a commit record nor the -// report (ADR-0052 clause 6). The aggregate stage's degree joins it with -// WP-0061. +// concurrent readers, the aggregate stage runs the families concurrently, one +// degree governs both, and the degree changes neither a commit record nor the +// report (ADR-0052 clause 6). TestAggregateAcrossParallelism holds the family +// degree alone to the same degrees on every fixture. // metadataSection is the path of the generation metadata in the report. const metadataSection = "metadata" @@ -134,7 +135,8 @@ func parallelDegrees() []int { } // TestDeterminismAcrossParallelism requires the commit records and the report -// to be identical at every degree. +// to be identical at every degree, which reaches the collect stage's readers +// and the aggregate stage's families alike. func TestDeterminismAcrossParallelism(t *testing.T) { t.Parallel() repo := openRepository(t) diff --git a/internal/checks/dockersmoke/smoke_test.go b/internal/checks/dockersmoke/smoke_test.go index 4e61c10..3026233 100644 --- a/internal/checks/dockersmoke/smoke_test.go +++ b/internal/checks/dockersmoke/smoke_test.go @@ -195,7 +195,7 @@ func (s smoke) cliDefaultCommandWritesTheDashboard(t *testing.T) { } // The CLI names the repository after the path it analyzed, here /repo. data, err := os.ReadFile(filepath.Join(repo, "out", "report.json")) - if err != nil || json.Unmarshal(data, &report) != nil || report.DocumentVersion.Major != 1 || report.Repository.Name != "repo" { + if err != nil || json.Unmarshal(data, &report) != nil || report.DocumentVersion.Major != 2 || report.Repository.Name != "repo" { t.Fatalf("out/report.json = %+v (%v)", report, err) } } diff --git a/internal/checks/familydeclaration_test.go b/internal/checks/familydeclaration_test.go index 7ca02d4..d7d1e90 100644 --- a/internal/checks/familydeclaration_test.go +++ b/internal/checks/familydeclaration_test.go @@ -7,42 +7,16 @@ import ( "testing" "github.com/sinanganiz/commitography/internal/core" - "github.com/sinanganiz/commitography/internal/metrics/aiarchaeology" - "github.com/sinanganiz/commitography/internal/metrics/commitsize" - "github.com/sinanganiz/commitography/internal/metrics/coupling" - "github.com/sinanganiz/commitography/internal/metrics/files" - "github.com/sinanganiz/commitography/internal/metrics/hotspot" - "github.com/sinanganiz/commitography/internal/metrics/messages" - "github.com/sinanganiz/commitography/internal/metrics/ownership" - "github.com/sinanganiz/commitography/internal/metrics/staticanalysis" - "github.com/sinanganiz/commitography/internal/metrics/temporal" - "github.com/sinanganiz/commitography/internal/metrics/worktype" + "github.com/sinanganiz/commitography/internal/pipeline/aggregate" ) // The family declaration checker (ADR-0076 clause 1, ADR-0031 clause 2, // ADR-0062 clause 3): every family of ADR-0076 clause 6 has a package that // declares its contract, and every declaration agrees with the catalogue. // -// It checks the declarations, not the report. Nothing routes a family's output -// through its declaration yet, so the report's keys are not compared here. - -// familyDeclarations is the checker's own list of the ten family packages. -// WP-0061 replaces it with the aggregate stage's registry, so that one list -// exists in the end. -func familyDeclarations() []core.MetricFamily { - return []core.MetricFamily{ - temporal.Family{}, - commitsize.Family{}, - messages.Family{}, - files.Family{}, - coupling.Family{}, - ownership.Family{}, - worktype.Family{}, - aiarchaeology.Family{}, - hotspot.Family{}, - staticanalysis.Family{}, - } -} +// It checks the declarations, not the report. The families it checks are the +// aggregate stage's registry, the one list of families in the tree, so a +// family the stage runs cannot escape it and it holds no list of its own. // catalogueNamespaces returns the namespace docs/metrics.md gives each family: // the backticked name after "**Namespace:**" in the family's section. A @@ -201,7 +175,7 @@ func TestFamilyDeclarations(t *testing.T) { familyRecord, metricsCatalogue) } - declared := declarationsOf(familyDeclarations()) + declared := declarationsOf(aggregate.Registry()) for _, d := range declared { t.Logf("%-15s inputs %v namespace %s version %s status %s method %t", d.Name, d.Inputs, d.Namespace, d.Version, d.Status, d.Method != "") @@ -218,7 +192,7 @@ func TestFamilyDeclarations(t *testing.T) { func TestFamilyDeclarationRejectsEachViolation(t *testing.T) { t.Parallel() valid := func() ([]core.FamilyDeclaration, map[string][]core.InputKind, map[string]string) { - declared := declarationsOf(familyDeclarations()) + declared := declarationsOf(aggregate.Registry()) rows := map[string][]core.InputKind{} namespaces := map[string]string{} for _, d := range declared { diff --git a/internal/checks/schema_test.go b/internal/checks/schema_test.go index abfa27b..d46a6b7 100644 --- a/internal/checks/schema_test.go +++ b/internal/checks/schema_test.go @@ -106,7 +106,7 @@ func TestReportSchemaRejectsMalformedReports(t *testing.T) { "a section version has no major component": func(d map[string]any) { d["sections"].(map[string]any)["configuration"] = map[string]any{"minor": 0.0} }, - "a family is absent": func(d map[string]any) { delete(families(d), "static-analysis") }, + "a family is absent": func(d map[string]any) { delete(families(d), "static_analysis") }, "a top-level key is invented": func(d map[string]any) { d["warnings"] = []any{} }, "a skipped family carries a metric": func(d map[string]any) { family(d, "ownership")["metrics"] = map[string]any{"bus_factor": 0.0} diff --git a/internal/core/declaration.go b/internal/core/declaration.go index f10950b..2c71c9f 100644 --- a/internal/core/declaration.go +++ b/internal/core/declaration.go @@ -3,10 +3,11 @@ // // A declaration is a shape, not a composition. Assembling the ten families // needs imports only the pipeline may make (ADR-0040), so the registry that -// holds them, and the routing of each family's output through its declaration, -// belong to the aggregate stage (WP-0061). Until then nothing reads a -// declaration but the family declaration checker, and the report is written as -// it was. +// holds them belongs to the aggregate stage. The stage routes each family's +// output into the report through its declaration: the family is given the +// inputs it declares, its section must carry the version it declares, and it +// is placed under the namespace the family declares, carrying the method +// statement the family declares. package core diff --git a/internal/core/family.go b/internal/core/family.go index e47d9c6..6292fc0 100644 --- a/internal/core/family.go +++ b/internal/core/family.go @@ -70,10 +70,15 @@ func Computed[M any](version Version, metrics M) Family[M] { return Family[M]{Version: version, Status: StatusOK, Metrics: metrics} } -// Skipped returns a skipped family. Its metrics are the zero value, which -// serialises as an empty object. -func Skipped[M any](version Version, reason Reason) Family[M] { - return Family[M]{Version: version, Status: StatusSkipped, Reasons: []Reason{reason}} +// Skipped returns a skipped family, for the reason given and any others that +// also apply, listed in enumeration order. Its metrics are the zero value, +// which serialises as an empty object. +func Skipped[M any](version Version, reason Reason, more ...Reason) Family[M] { + reasons := []Reason{reason} + for _, r := range more { + reasons = withReason(reasons, r) + } + return Family[M]{Version: version, Status: StatusSkipped, Reasons: reasons} } // Degrade marks a computed family degraded for reason. The confidence it ends diff --git a/internal/core/filter/paths.go b/internal/core/filter/paths.go index a01c786..044d885 100644 --- a/internal/core/filter/paths.go +++ b/internal/core/filter/paths.go @@ -80,6 +80,16 @@ func NewPathFilterFromAttributes(cfg config.Analysis, attributes []byte) (*PathF return f, nil } +// Clone returns a filter that decides every path as f does, with a memo of its +// own. The memo is written as paths are decided, so a filter is not safe for +// concurrent use; each concurrent reader takes a clone instead. +func (f *PathFilter) Clone() *PathFilter { + if f == nil { + return nil + } + return &PathFilter{exclude: f.exclude, negated: f.negated, cache: make(map[string]bool)} +} + // Excluded reports whether a path should be omitted from line-based metrics. // Results are memoized because the same paths recur across thousands of commits. func (f *PathFilter) Excluded(path string) bool { diff --git a/internal/core/input.go b/internal/core/input.go index 75c2aab..1ad1001 100644 --- a/internal/core/input.go +++ b/internal/core/input.go @@ -30,6 +30,11 @@ type Input struct { // Progress, when set, is called as each stage begins. Progress func(stage, detail string, current, total int) + // Parallelism is how many metric families the aggregate stage runs at + // once (ADR-0052 clauses 3 and 5). Zero derives it from the available + // cores. It changes no value in the report (clause 6). + Parallelism int + // ToolVersion is the build's version, recorded in the report's generation // metadata (ADR-0061 clause 6). ToolVersion string diff --git a/internal/core/reason.go b/internal/core/reason.go index d7deb6c..700336a 100644 --- a/internal/core/reason.go +++ b/internal/core/reason.go @@ -21,7 +21,6 @@ type Reason string // declares them, so the enumeration is complete from here on and each later // package fills a slot rather than extending the set. const ( - ReasonWorktreeUnavailable Reason = "worktree_unavailable" ReasonNotImplemented Reason = "not_implemented" ReasonEmptyPopulation Reason = "empty_population" ReasonCardinalityLimit Reason = "cardinality_limit" @@ -63,7 +62,6 @@ const ( // omitted here is caught rather than silently unlisted. func Reasons() []Reason { return []Reason{ - ReasonWorktreeUnavailable, ReasonNotImplemented, ReasonEmptyPopulation, ReasonCardinalityLimit, diff --git a/internal/core/report.go b/internal/core/report.go index d1a3264..4cff3d5 100644 --- a/internal/core/report.go +++ b/internal/core/report.go @@ -7,13 +7,17 @@ // // The report document follows docs/metrics.md, which is authoritative // (ADR-0062): no field exists in it that the catalogue does not define, every -// family of ADR-0024 clause 5 is present with a status (ADR-0032 clause 1), -// and everything that varies between two runs over one commit is confined to -// the metadata section (ADR-0021 clause 6). docs/report-schema.json describes -// it. +// family of ADR-0076 clause 6 is present with a status (ADR-0032 clause 1) +// under the namespace the catalogue gives it, and everything that varies +// between two runs over one commit is confined to the metadata section +// (ADR-0021 clause 6). docs/report-schema.json describes it. package core -import "time" +import ( + "reflect" + "strings" + "time" +) // DocumentVersion is the version of the report's structure: its top-level // shape, identity representation, status fields and metadata section @@ -30,8 +34,14 @@ import "time" // configuration (ADR-0026 clause 2) // 1.4 the sections object added, carrying a version for every top-level // section that is not a family (ADR-0070 clause 1) +// 2.0 every family written under the namespace docs/metrics.md gives it, +// so commit-size, ai-archaeology and static-analysis became +// commit_size, ai_archaeology and static_analysis (ADR-0062 clause 3, +// ADR-0076 clause 1); and the family status code naming a missing +// working tree removed, which nothing produced once no family read the +// working tree (ADR-0076 clause 11) func DocumentVersion() Version { - return Version{Major: 1, Minor: 4} + return Version{Major: 2, Minor: 0} } // Report is the complete analysis artifact: one repository, at one commit, @@ -102,34 +112,80 @@ type MergeCandidate struct { Signal string `json:"signal"` } -// Families holds every family of ADR-0024 clause 5, in that order, keyed by -// family name. A struct rather than a map, so that no report can be built -// without one of them. +// Families holds every family of ADR-0076 clause 6, in that order, each keyed +// by the namespace docs/metrics.md gives it, which is the namespace the family +// declares (ADR-0076 clause 1, ADR-0062 clause 3). A struct rather than a map, +// so that no report can be built without one of them. type Families struct { Temporal Family[TemporalMetrics] `json:"temporal"` - CommitSize Family[CommitSizeMetrics] `json:"commit-size"` + CommitSize Family[CommitSizeMetrics] `json:"commit_size"` Messages Family[MessagesMetrics] `json:"messages"` Files Family[FilesMetrics] `json:"files"` Coupling Family[CouplingMetrics] `json:"coupling"` Ownership Family[OwnershipMetrics] `json:"ownership"` Worktype Family[WorktypeMetrics] `json:"worktype"` - AIArchaeology Family[AIArchaeologyMetrics] `json:"ai-archaeology"` + AIArchaeology Family[AIArchaeologyMetrics] `json:"ai_archaeology"` Hotspot Family[HotspotMetrics] `json:"hotspot"` - StaticAnalysis Family[StaticAnalysisMetrics] `json:"static-analysis"` + StaticAnalysis Family[StaticAnalysisMetrics] `json:"static_analysis"` } -// Degrade marks every computed family degraded for a condition that affects -// all of them, such as an incomplete history. Skipped families are left as -// they are. -func (f *Families) Degrade(reason Reason, confidence Confidence) { - f.Temporal.Degrade(reason, confidence) - f.CommitSize.Degrade(reason, confidence) - f.Messages.Degrade(reason, confidence) - f.Files.Degrade(reason, confidence) - f.Coupling.Degrade(reason, confidence) - f.Ownership.Degrade(reason, confidence) - f.Worktype.Degrade(reason, confidence) - f.AIArchaeology.Degrade(reason, confidence) - f.Hotspot.Degrade(reason, confidence) - f.StaticAnalysis.Degrade(reason, confidence) +// Place writes one family's section into the report under namespace, the key +// the report writes that family under (ADR-0076 clauses 1 and 5). The +// aggregate stage's registry places every family's section through it, each +// under the namespace the family declares. +// +// It refuses a namespace the report has no family for, a section whose +// metric type is not the one that namespace holds, a section with no status, +// and a namespace already written. So a family's output cannot land outside +// its namespace, or in a namespace another family has written. +func Place[M any](f *Families, namespace string, section Family[M]) error { + slot, ok := familySlot(f, namespace) + if !ok { + return Internalf(nil, "placing a family section under %q, which is no family namespace of the report", namespace) + } + target, ok := slot.Addr().Interface().(*Family[M]) + if !ok { + return Internalf(nil, "placing a %T under %q, which holds a %s: a family writes only into its own "+ + "namespace (ADR-0076 clause 5)", section, namespace, slot.Type()) + } + if section.Status == "" { + return Internalf(nil, "placing a section with no status under %q (ADR-0032 clause 2)", namespace) + } + if target.Status != "" { + return Internalf(nil, "placing a second section under %q: a namespace has one owner (ADR-0076 clause 5)", + namespace) + } + *target = section + return nil +} + +// Unplaced returns the namespaces no section has been placed under, in the +// order the report writes them. A report with any is missing a family, which +// no report may be (ADR-0032 clause 1). +func (f *Families) Unplaced() []string { + var out []string + v := reflect.ValueOf(f).Elem() + for i := 0; i < v.NumField(); i++ { + if v.Field(i).FieldByName("Status").String() == "" { + out = append(out, familyKey(v.Type().Field(i))) + } + } + return out +} + +// familySlot returns the field of f the report writes under namespace. +func familySlot(f *Families, namespace string) (reflect.Value, bool) { + v := reflect.ValueOf(f).Elem() + for i := 0; i < v.NumField(); i++ { + if familyKey(v.Type().Field(i)) == namespace { + return v.Field(i), true + } + } + return reflect.Value{}, false +} + +// familyKey returns the key the report writes a family field under. +func familyKey(field reflect.StructField) string { + name, _, _ := strings.Cut(field.Tag.Get("json"), ",") + return name } diff --git a/internal/core/versions.go b/internal/core/versions.go index 6e32062..fdcfe26 100644 --- a/internal/core/versions.go +++ b/internal/core/versions.go @@ -76,15 +76,15 @@ func (r *Report) Versions() VersionSet { }, Families: map[string]Version{ "temporal": f.Temporal.Version, - "commit-size": f.CommitSize.Version, + "commit_size": f.CommitSize.Version, "messages": f.Messages.Version, "files": f.Files.Version, "coupling": f.Coupling.Version, "ownership": f.Ownership.Version, "worktype": f.Worktype.Version, - "ai-archaeology": f.AIArchaeology.Version, + "ai_archaeology": f.AIArchaeology.Version, "hotspot": f.Hotspot.Version, - "static-analysis": f.StaticAnalysis.Version, + "static_analysis": f.StaticAnalysis.Version, }, } } diff --git a/internal/pipeline/aggregate/aggregate.go b/internal/pipeline/aggregate/aggregate.go index 60ceaec..9cc4e98 100644 --- a/internal/pipeline/aggregate/aggregate.go +++ b/internal/pipeline/aggregate/aggregate.go @@ -1,21 +1,20 @@ // Package aggregate is the aggregate stage (ADR-0020): it builds the report -// document by running the metric families over the filtered commits -// (ADR-0024, ADR-0040). Every family of ADR-0024 clause 5 is placed in the -// document with a status (ADR-0032 clause 1); a family with no implementation -// yet is skipped with reason not_implemented. Generation values go to the -// metadata section alone (ADR-0021 clause 6). Beside the families it writes -// the identities section, the resolved contributors a reader selects from -// (ADR-0010 clause 2, docs/metrics.md section 14). +// document from the collect stage's records and the replay stage's state, +// reading no file and starting no process (clause 5). Every metric family +// reaches the report through the stage's registry and by no other route: +// each family's declared inputs are resolved, the families run concurrently +// (ADR-0052 clause 3), and each section is placed under the namespace its +// family declares (ADR-0076 clauses 3 and 5, ADR-0040). Every family of +// ADR-0076 clause 6 is present with a status (ADR-0032 clause 1); a family +// with no implementation yet is skipped with reason not_implemented. +// Generation values go to the metadata section alone (ADR-0021 clause 6). +// Beside the families it writes the identities section, the resolved +// contributors a reader selects from (ADR-0010 clause 2, docs/metrics.md +// section 14). package aggregate import ( "github.com/sinanganiz/commitography/internal/core" - "github.com/sinanganiz/commitography/internal/metrics/commitsize" - "github.com/sinanganiz/commitography/internal/metrics/coupling" - "github.com/sinanganiz/commitography/internal/metrics/hotspot" - "github.com/sinanganiz/commitography/internal/metrics/messages" - "github.com/sinanganiz/commitography/internal/metrics/ownership" - "github.com/sinanganiz/commitography/internal/metrics/temporal" ) // Builder is the aggregate stage. It holds the clock that stamps the report's @@ -36,9 +35,6 @@ func New(clock core.Clock, _ core.Filesystem) *Builder { // them where it shows its other warnings. func (b *Builder) Build(in core.Input) (*core.Report, []string, error) { analyzed := in.Analyzed() - lineScoped := in.LineScoped() - var warnings []string - r := &core.Report{ DocumentVersion: core.DocumentVersion(), Sections: core.CurrentSectionVersions(), @@ -57,50 +53,15 @@ func (b *Builder) Build(in core.Input) (*core.Report, []string, error) { // configured name, and a pseudonym only means something where the reader // can see the entry it names (ADR-0068 clause 4). r.Configuration = core.EmbedConfiguration(in.Config, in.Resolver, r.Identities) - f := &r.Families - - progress(in, "metrics", "temporal", 0, 0) - f.Temporal = temporal.Build(in, analyzed) - progress(in, "metrics", "code", 0, 0) - f.CommitSize = commitsize.Build(in, lineScoped) - files, err := b.buildFiles(in, lineScoped) + warnings, err := route(in, &r.Families, entries()) if err != nil { return nil, nil, err } - f.Files = files - - progress(in, "metrics", "messages", 0, 0) - f.Messages = messages.Build(analyzed) - - progress(in, "metrics", "social", 0, 0) - scoped := core.ScopedCommits(in, lineScoped) - var couplingWarnings []string - f.Coupling, couplingWarnings = coupling.Build(scoped) - warnings = append(warnings, couplingWarnings...) - f.Hotspot = hotspot.Build(scoped) - f.Ownership = ownership.Build() - - // The families below have no implementation yet. Each is present, as - // skipped, so the document's shape is final and the package that - // implements a family replaces one line here (ADR-0032 clause 1). - f.Worktype = core.Skipped[core.WorktypeMetrics](core.Version{}, core.ReasonNotImplemented) - f.AIArchaeology = core.Skipped[core.AIArchaeologyMetrics](core.Version{}, core.ReasonNotImplemented) - f.StaticAnalysis = core.Skipped[core.StaticAnalysisMetrics](core.Version{}, core.ReasonNotImplemented) - - // An author that cannot be resolved into an identity is neither dropped - // nor split: every family attributing values to identities is marked - // degraded instead (ADR-0032, docs/metrics.md section 13). - if unresolvedAuthor(in, analyzed) { - degradeIdentityAttributed(f) - } - - // An analysis the operator let proceed on a shallow clone has computed - // every family over an incomplete history (docs/metrics.md section 13). - if in.Repository.IsShallow { - f.Degrade(core.ReasonShallowClone, core.ConfidenceLow) + // Every family is present in every report (ADR-0032 clause 1). + if missing := r.Families.Unplaced(); len(missing) > 0 { + return nil, nil, core.Internalf(nil, "the report has no section for the families %v", missing) } - return r, warnings, nil } diff --git a/internal/pipeline/aggregate/code.go b/internal/pipeline/aggregate/code.go deleted file mode 100644 index d8ba79b..0000000 --- a/internal/pipeline/aggregate/code.go +++ /dev/null @@ -1,20 +0,0 @@ -package aggregate - -import ( - "github.com/sinanganiz/commitography/internal/core" - "github.com/sinanganiz/commitography/internal/core/model" - "github.com/sinanganiz/commitography/internal/metrics/files" -) - -// buildFiles runs the files family over the analysed commit's tree, as the -// replay stage listed it. This stage lists no tree and reads no file: working -// tree and repository access are the replay stage's alone (ADR-0020 -// clause 3). -func (b *Builder) buildFiles(in core.Input, lineScoped []model.Commit) (core.Family[core.FilesMetrics], error) { - if in.Replay == nil { - return core.Family[core.FilesMetrics]{}, core.Internalf(nil, - "aggregating without the replay stage's listing of the analysed commit's tree") - } - tree := files.Tree{Tracked: in.Replay.Tracked, TextFileCount: in.Replay.TextFileCount} - return files.Build(in, lineScoped, tree), nil -} diff --git a/internal/pipeline/aggregate/identities.go b/internal/pipeline/aggregate/identities.go index 42d6834..6ec2167 100644 --- a/internal/pipeline/aggregate/identities.go +++ b/internal/pipeline/aggregate/identities.go @@ -178,16 +178,3 @@ func unresolvedAuthor(in core.Input, analyzed []model.Commit) bool { } return false } - -// degradeIdentityAttributed marks degraded, with reason unresolved_identity, -// every family whose values are attributed to identities: ownership's lines by -// identity and bus factor, worktype's editor-by-owner breakdown, and -// ai-archaeology's identity ratio (docs/metrics.md sections 7 to 9). The -// commits of an unresolved author are counted, so the values present may be -// attributed wrongly, which is low confidence (ADR-0032 clause 2). A family -// that is skipped stays skipped: it computed nothing to distrust. -func degradeIdentityAttributed(f *core.Families) { - f.Ownership.Degrade(core.ReasonUnresolvedIdentity, core.ConfidenceLow) - f.Worktype.Degrade(core.ReasonUnresolvedIdentity, core.ConfidenceLow) - f.AIArchaeology.Degrade(core.ReasonUnresolvedIdentity, core.ConfidenceLow) -} diff --git a/internal/pipeline/aggregate/identities_test.go b/internal/pipeline/aggregate/identities_test.go index 6fb480f..45fc2de 100644 --- a/internal/pipeline/aggregate/identities_test.go +++ b/internal/pipeline/aggregate/identities_test.go @@ -9,6 +9,7 @@ import ( "github.com/sinanganiz/commitography/internal/core" "github.com/sinanganiz/commitography/internal/core/config" + "github.com/sinanganiz/commitography/internal/core/filter" "github.com/sinanganiz/commitography/internal/core/identity" "github.com/sinanganiz/commitography/internal/core/model" ) @@ -172,14 +173,19 @@ func TestAnUnresolvableAuthorDegradesTheIdentityAttributedFamilies(t *testing.T) t.Fatal("a commit carrying an address was treated as unresolved") } - // The identity-attributed families are computed here, as they will be - // once their packages exist, so the degradation can be observed. + // The identity-attributed families compute nothing yet, so stand-ins that + // compute take their places, and the degradation is observed on the route + // every family takes into the report. + in.Filtered = filter.Summarize(commits) var f core.Families - f.Temporal = core.Computed(core.Version{Major: 1}, core.TemporalMetrics{}) - f.Ownership = core.Computed(core.Version{Major: 1}, core.OwnershipMetrics{}) - f.Worktype = core.Computed(core.Version{Major: 1}, core.WorktypeMetrics{}) - f.AIArchaeology = core.Skipped[core.AIArchaeologyMetrics](core.Version{}, core.ReasonNotImplemented) - degradeIdentityAttributed(&f) + if _, err := route(in, &f, []entry{ + computedStandIn[core.TemporalMetrics]("temporal", false), + computedStandIn[core.OwnershipMetrics]("ownership", true), + computedStandIn[core.WorktypeMetrics]("worktype", true), + notImplementedStandIn[core.AIArchaeologyMetrics]("ai_archaeology", true), + }); err != nil { + t.Fatalf("routing the stand-ins: %v", err) + } for name, got := range map[string]struct { status core.Status diff --git a/internal/pipeline/aggregate/registry.go b/internal/pipeline/aggregate/registry.go new file mode 100644 index 0000000..93b368a --- /dev/null +++ b/internal/pipeline/aggregate/registry.go @@ -0,0 +1,180 @@ +// The family registry (ADR-0076 clauses 1, 3, 5 and 6): the one list of the +// metric families the report carries, in the order the report writes them, +// and the route by which each family's output reaches the report. The family +// declaration checker in internal/checks reads it rather than holding a list +// of its own, so no second list exists to disagree with this one. + +package aggregate + +import ( + "github.com/sinanganiz/commitography/internal/core" + "github.com/sinanganiz/commitography/internal/metrics/aiarchaeology" + "github.com/sinanganiz/commitography/internal/metrics/commitsize" + "github.com/sinanganiz/commitography/internal/metrics/coupling" + "github.com/sinanganiz/commitography/internal/metrics/files" + "github.com/sinanganiz/commitography/internal/metrics/hotspot" + "github.com/sinanganiz/commitography/internal/metrics/messages" + "github.com/sinanganiz/commitography/internal/metrics/ownership" + "github.com/sinanganiz/commitography/internal/metrics/staticanalysis" + "github.com/sinanganiz/commitography/internal/metrics/temporal" + "github.com/sinanganiz/commitography/internal/metrics/worktype" +) + +// entry is one family's registration. +type entry struct { + family core.MetricFamily + // section produces the family's section. + section sectionFunc + // identities marks a family whose values are attributed to identities: + // ownership's lines by identity and bus factor, worktype's editor-by-owner + // breakdown, and ai-archaeology's identity ratio (docs/metrics.md + // sections 7 to 9). An author that cannot be resolved into an identity + // degrades such a family with unresolved_identity: the author's commits + // are counted, so values may be attributed wrongly, which is low + // confidence (ADR-0032 clause 2, docs/metrics.md section 13). A skipped + // family stays skipped, having computed nothing to distrust. + identities bool + // progress is the progress detail reported as the family is started, for + // the families that begin a progress stage. + progress string +} + +// entries returns every registration, in the order the report writes the +// families (ADR-0076 clause 6). +func entries() []entry { + return []entry{ + { + family: temporal.Family{}, + section: computed(func(in core.Input) (core.Family[core.TemporalMetrics], []string) { + return temporal.Build(in, in.Analyzed()), nil + }), + progress: "temporal", + }, + { + family: commitsize.Family{}, + section: computed(func(in core.Input) (core.Family[core.CommitSizeMetrics], []string) { + return commitsize.Build(in, in.LineScoped()), nil + }), + progress: "code", + }, + { + family: messages.Family{}, + section: computed(func(in core.Input) (core.Family[core.MessagesMetrics], []string) { + return messages.Build(in.Analyzed()), nil + }), + progress: "messages", + }, + { + family: files.Family{}, + // The files family reads the analysed commit's tree as replay + // listed it; this stage lists no tree and reads no file + // (ADR-0020 clause 3). + section: computed(func(in core.Input) (core.Family[core.FilesMetrics], []string) { + tree := files.Tree{Tracked: in.Replay.Tracked, TextFileCount: in.Replay.TextFileCount} + return files.Build(in, in.LineScoped(), tree), nil + }), + progress: "code", + }, + { + family: coupling.Family{}, + section: computed(func(in core.Input) (core.Family[core.CouplingMetrics], []string) { + return coupling.Build(core.ScopedCommits(in, in.LineScoped())) + }), + progress: "social", + }, + {family: ownership.Family{}, section: notImplemented[core.OwnershipMetrics](), identities: true}, + {family: worktype.Family{}, section: notImplemented[core.WorktypeMetrics](), identities: true}, + {family: aiarchaeology.Family{}, section: notImplemented[core.AIArchaeologyMetrics](), identities: true}, + { + family: hotspot.Family{}, + section: computed(func(in core.Input) (core.Family[core.HotspotMetrics], []string) { + return hotspot.Build(core.ScopedCommits(in, in.LineScoped())), nil + }), + }, + {family: staticanalysis.Family{}, section: notImplemented[core.StaticAnalysisMetrics]()}, + } +} + +// Registry returns every metric family the report carries, in the order the +// report writes them. It is the only list of families in the tree. +func Registry() []core.MetricFamily { + registered := entries() + out := make([]core.MetricFamily, 0, len(registered)) + for _, e := range registered { + out = append(out, e.family) + } + return out +} + +// sectionFunc produces one family's section from the input resolved for it: +// skipped for the reasons given, where there are any, and computed otherwise. +// It returns the warnings the family raised beside it. +type sectionFunc func(in core.Input, declared core.FamilyDeclaration, skip []core.Reason) (section, []string, error) + +// computed is the section of a family whose status comes from its +// computation: run over the family's resolved input unless the family is +// skipped, in which case it is never run (ADR-0076 clause 3). It carries the +// method statement the family declares. +func computed[M any](run func(core.Input) (core.Family[M], []string)) sectionFunc { + return func(in core.Input, declared core.FamilyDeclaration, skip []core.Reason) (section, []string, error) { + if declared.Status != core.StatusFromComputation { + return nil, nil, core.Internalf(nil, "the family %s declares the status source %s, and its "+ + "registration computes it", declared.Name, declared.Status) + } + if len(skip) > 0 { + return declaredSection(core.Skipped[M](declared.Version, skip[0], skip[1:]...), declared), nil, nil + } + family, warnings := run(in) + if family.Version != declared.Version { + return nil, nil, core.Internalf(nil, "the family %s computed version %s and declares version %s", + declared.Name, family.Version, declared.Version) + } + return declaredSection(family, declared), warnings, nil + } +} + +// notImplemented is the section of a family that computes nothing yet. It is +// skipped with reason not_implemented, beside any reason its inputs give, and +// carries the zero version its declaration gives (ADR-0032 clause 1). +func notImplemented[M any]() sectionFunc { + return func(_ core.Input, declared core.FamilyDeclaration, skip []core.Reason) (section, []string, error) { + if declared.Status != core.StatusNotImplemented { + return nil, nil, core.Internalf(nil, "the family %s declares the status source %s, and its "+ + "registration computes nothing", declared.Name, declared.Status) + } + skipped := core.Skipped[M](declared.Version, core.ReasonNotImplemented, skip...) + return declaredSection(skipped, declared), nil, nil + } +} + +// declaredSection returns a family's section carrying the method statement +// its declaration gives (ADR-0032 clause 8, ADR-0076 clause 1). The +// statement is carried whatever the family's status: it says how the family's +// values are derived, which a reader of a skipped family learns as well. +func declaredSection[M any](family core.Family[M], declared core.FamilyDeclaration) section { + family.Method = declared.Method + return §ionOf[M]{family} +} + +// section is one family's section on its way into the report, whatever the +// family's metric type. +type section interface { + // degrade marks the section degraded for a condition of the analysis as + // a whole. A skipped section stays skipped. + degrade(core.Reason, core.Confidence) + // place writes the section into the report under namespace. + place(families *core.Families, namespace string) error +} + +// sectionOf is a section of metric type M. +type sectionOf[M any] struct { + family core.Family[M] +} + +func (s *sectionOf[M]) degrade(reason core.Reason, confidence core.Confidence) { + s.family.Degrade(reason, confidence) +} + +func (s *sectionOf[M]) place(families *core.Families, namespace string) error { + return core.Place(families, namespace, s.family) +} diff --git a/internal/pipeline/aggregate/route.go b/internal/pipeline/aggregate/route.go new file mode 100644 index 0000000..3d791bd --- /dev/null +++ b/internal/pipeline/aggregate/route.go @@ -0,0 +1,167 @@ +// The route from the registry into the report: every registered family's +// declared inputs are resolved, the families run concurrently, and each +// section is placed under the namespace its family declares (ADR-0076 clauses +// 3 and 5, ADR-0052 clauses 3, 5 and 6). + +package aggregate + +import ( + "context" + "runtime" + "sync" + + "github.com/sinanganiz/commitography/internal/core" +) + +// job is one registered family with its input resolved, ready to run. +type job struct { + entry entry + declared core.FamilyDeclaration + input core.Input + skip []core.Reason +} + +// outcome is what running one job produced. +type outcome struct { + section section + warnings []string + err error +} + +// route runs every registered family and places its section into families: +// the only route by which a family's output reaches the report. The families +// run at once, up to the stage's degree of parallelism, and nothing they +// produce depends on the order they finish in: sections are placed and +// warnings returned in registry order (ADR-0052 clause 6). +func route(in core.Input, families *core.Families, registered []entry) ([]string, error) { + ctx := in.Context + if ctx == nil { + ctx = context.Background() + } + + jobs := make([]job, 0, len(registered)) + for _, e := range registered { + declared := e.family.Declaration() + if e.section == nil { + return nil, core.Internalf(nil, "the family %s is registered with no way to produce its section", + declared.Name) + } + input, skip, err := resolve(in, declared) + if err != nil { + return nil, err + } + jobs = append(jobs, job{entry: e, declared: declared, input: input, skip: skip}) + } + + outcomes := make([]outcome, len(jobs)) + work := make(chan int) + var wg sync.WaitGroup + for range min(degree(in), len(jobs)) { + wg.Go(func() { + for i := range work { + j := jobs[i] + s, warnings, err := j.entry.section(j.input, j.declared, j.skip) + outcomes[i] = outcome{section: s, warnings: warnings, err: err} + } + }) + } + // Progress is reported from here alone, as each family is handed out, so + // the caller's callback is never called concurrently. A stage is reported + // once, where its first family starts. + started := map[string]bool{} +dispatch: + for i, j := range jobs { + if detail := j.entry.progress; detail != "" && !started[detail] { + started[detail] = true + progress(in, "metrics", detail, 0, 0) + } + select { + case work <- i: + case <-ctx.Done(): + break dispatch + } + } + close(work) + wg.Wait() + if err := ctx.Err(); err != nil { + return nil, err + } + + // Two conditions of the analysis as a whole reach families from here. + // An author that cannot be resolved into an identity is neither dropped + // nor split: every family attributing values to identities is degraded + // instead (ADR-0032, docs/metrics.md section 13). An analysis the operator + // let proceed on a shallow clone has computed every family over an + // incomplete history. + unresolved := unresolvedAuthor(in, in.Analyzed()) + var warnings []string + for i, j := range jobs { + o := outcomes[i] + if o.err != nil { + return nil, o.err + } + if unresolved && j.entry.identities { + o.section.degrade(core.ReasonUnresolvedIdentity, core.ConfidenceLow) + } + if in.Repository.IsShallow { + o.section.degrade(core.ReasonShallowClone, core.ConfidenceLow) + } + if err := o.section.place(families, j.declared.Namespace); err != nil { + return nil, err + } + warnings = append(warnings, o.warnings...) + } + return warnings, nil +} + +// resolve returns the input a family runs over, holding its declared inputs +// and no others, and the reasons it is skipped for instead of run: one for +// each declared input this analysis cannot provide (ADR-0076 clause 3). +// +// The collect stage's records and the replay stage's state are provided +// whenever the stage runs, so a family declaring them is never skipped for +// them, and replay state missing is a defect of composition rather than a +// condition. No external service is available to a family: the capability +// defaults to unavailable (ADR-0076 clause 8) and nothing configures one. +// +// The input names no repository location, because no input kind is the +// repository (ADR-0076 clause 2). It carries no progress callback, which is +// the stage's own, and a path filter of its own, because a filter's memo is +// not safe for concurrent use. +func resolve(in core.Input, declared core.FamilyDeclaration) (core.Input, []core.Reason, error) { + out := core.Input{ + Context: in.Context, + Repository: in.Repository, + Config: in.Config, + PathFilter: in.PathFilter.Clone(), + } + out.Repository.Path = "" + var skip []core.Reason + for _, kind := range declared.Inputs { + switch kind { + case core.InputCommitRecords: + out.Filtered, out.Resolver = in.Filtered, in.Resolver + case core.InputReplayState: + if in.Replay == nil { + return core.Input{}, nil, core.Internalf(nil, "aggregating the family %s, which declares replay "+ + "state, without the replay stage's state", declared.Name) + } + out.Replay = in.Replay + case core.InputExternalService: + skip = append(skip, core.ReasonExternalServiceUnavailable) + default: + return core.Input{}, nil, core.Internalf(nil, "the family %s declares the input %q, which is none of "+ + "the three kinds", declared.Name, kind) + } + } + return out, skip, nil +} + +// degree returns how many families run at once: the configured degree, or +// the number of available cores where none is configured (ADR-0052 clause 5). +func degree(in core.Input) int { + if in.Parallelism > 0 { + return in.Parallelism + } + return max(runtime.NumCPU(), 1) +} diff --git a/internal/pipeline/aggregate/route_test.go b/internal/pipeline/aggregate/route_test.go new file mode 100644 index 0000000..b90e6e8 --- /dev/null +++ b/internal/pipeline/aggregate/route_test.go @@ -0,0 +1,216 @@ +package aggregate + +import ( + "context" + "testing" + + "github.com/sinanganiz/commitography/internal/core" + "github.com/sinanganiz/commitography/internal/core/filter" + "github.com/sinanganiz/commitography/internal/core/model" +) + +// standIn is a family that exists only in a test, declaring what the test +// gives it. A stand-in takes the place of a registered family, so that what +// the route does with a family can be observed where no registered family +// does it yet. +type standIn core.FamilyDeclaration + +func (s standIn) Declaration() core.FamilyDeclaration { return core.FamilyDeclaration(s) } + +// computedStandIn registers a stand-in that computes an ok section of metric +// type M, and writes it under namespace as the family owning it would. +func computedStandIn[M any](namespace string, identities bool, inputs ...core.InputKind) entry { + version := core.Version{Major: 1} + if len(inputs) == 0 { + inputs = []core.InputKind{core.InputCommitRecords} + } + return entry{ + family: standIn{Name: "stand-in " + namespace, Inputs: inputs, Namespace: namespace, Version: version, + Status: core.StatusFromComputation}, + section: computed(func(core.Input) (core.Family[M], []string) { + var metrics M + return core.Computed(version, metrics), nil + }), + identities: identities, + } +} + +// notImplementedStandIn registers a stand-in that computes nothing, under +// namespace. +func notImplementedStandIn[M any](namespace string, identities bool) entry { + return entry{ + family: standIn{Name: "stand-in " + namespace, Inputs: []core.InputKind{core.InputCommitRecords}, + Namespace: namespace, Status: core.StatusNotImplemented}, + section: notImplemented[M](), + identities: identities, + } +} + +// routeOne routes a single registration into an empty report, over an input +// holding a commit record and a replay state. +func routeOne(e entry, in core.Input) (core.Families, []string, error) { + var families core.Families + warnings, err := route(in, &families, []entry{e}) + return families, warnings, err +} + +// fullInput is an input carrying everything aggregation is ever given. +func fullInput() core.Input { + commits := []model.Commit{authored("Ada", "ada@example.com", 1)} + return core.Input{ + Context: context.Background(), + RepoPath: "/the/repository", + Repository: model.RepositoryInfo{Path: "/the/repository", Name: "repository"}, + Filtered: filter.Summarize(commits), + Replay: &core.ReplayState{Tracked: []string{"a.go"}, TextFileCount: 1}, + Progress: func(string, string, int, int) {}, + } +} + +// TestAggregateSkipsAFamilyWhoseInputIsUnavailable is ADR-0076 clause 3: a +// family declaring an input the analysis cannot provide is skipped with the +// matching reason and never run. No external service is ever available. +func TestAggregateSkipsAFamilyWhoseInputIsUnavailable(t *testing.T) { + t.Parallel() + ran := false + e := computedStandIn[core.HotspotMetrics]("hotspot", false, core.InputCommitRecords, core.InputExternalService) + e.section = computed(func(core.Input) (core.Family[core.HotspotMetrics], []string) { + ran = true + return core.Computed(core.Version{Major: 1}, core.HotspotMetrics{}), nil + }) + families, _, err := routeOne(e, fullInput()) + if err != nil { + t.Fatalf("routing a family whose input is unavailable: %v", err) + } + if ran { + t.Error("a family whose input is unavailable was run") + } + got := families.Hotspot + if got.Status != core.StatusSkipped || len(got.Reasons) != 1 || got.Reasons[0] != core.ReasonExternalServiceUnavailable { + t.Errorf("the family is %s %v, want skipped with external_service_unavailable", got.Status, got.Reasons) + } + if got.Version != (core.Version{Major: 1}) { + t.Errorf("the skipped family carries version %s, not the one it declares", got.Version) + } +} + +// TestAggregateGivesAFamilyOnlyItsDeclaredInputs is ADR-0076 clauses 2 and 3: +// a family is run over the inputs it declares and no others, and over no +// repository location, since no input kind is the repository. +func TestAggregateGivesAFamilyOnlyItsDeclaredInputs(t *testing.T) { + t.Parallel() + for _, c := range []struct { + inputs []core.InputKind + records, replayable bool + }{ + {[]core.InputKind{core.InputCommitRecords}, true, false}, + {[]core.InputKind{core.InputReplayState}, false, true}, + {[]core.InputKind{core.InputCommitRecords, core.InputReplayState}, true, true}, + } { + var given core.Input + e := computedStandIn[core.FilesMetrics]("files", false, c.inputs...) + e.section = computed(func(in core.Input) (core.Family[core.FilesMetrics], []string) { + given = in + return core.Computed(core.Version{Major: 1}, core.FilesMetrics{}), nil + }) + if _, _, err := routeOne(e, fullInput()); err != nil { + t.Fatalf("routing a family declaring %v: %v", c.inputs, err) + } + if got := len(given.Analyzed()) > 0; got != c.records { + t.Errorf("a family declaring %v was given commit records: %t", c.inputs, got) + } + if got := given.Replay != nil; got != c.replayable { + t.Errorf("a family declaring %v was given replay state: %t", c.inputs, got) + } + if given.RepoPath != "" || given.Repository.Path != "" || given.Progress != nil { + t.Errorf("a family declaring %v was given the repository's location or the stage's progress", c.inputs) + } + } +} + +// TestAggregateRequiresTheReplayStateAFamilyDeclares keeps replay state an +// input the stage is always given: a family declaring it, with none to give, +// is an internal error rather than a skip, since no reason code names it. +func TestAggregateRequiresTheReplayStateAFamilyDeclares(t *testing.T) { + t.Parallel() + in := fullInput() + in.Replay = nil + _, _, err := routeOne(computedStandIn[core.FilesMetrics]("files", false, core.InputReplayState), in) + if err == nil || core.ClassOf(err) != core.ClassInternal { + t.Errorf("a family declaring replay state was routed without one: %v", err) + } +} + +// TestAggregateRefusesAWriteOutsideTheNamespace is ADR-0076 clause 5 on the +// route: a family whose section is not of the type its declared namespace +// holds, two families declaring one namespace, and a namespace the report +// has no family for are each refused, and nothing is written for them. +func TestAggregateRefusesAWriteOutsideTheNamespace(t *testing.T) { + t.Parallel() + foreign := computedStandIn[core.HotspotMetrics]("files", false) + twice := []entry{computedStandIn[core.FilesMetrics]("files", false), computedStandIn[core.FilesMetrics]("files", false)} + retired := computedStandIn[core.CommitSizeMetrics]("commit-size", false) + for name, registered := range map[string][]entry{ + "a hotspot section under files": {foreign}, + "two families under files": twice, + "a section under commit-size": {retired}, + } { + var families core.Families + if _, err := route(fullInput(), &families, registered); err == nil { + t.Errorf("%s was accepted", name) + } + } +} + +// TestAggregateRefusesARegistrationItsDeclarationContradicts keeps the +// declaration the source of truth: a registration computing a family that +// declares itself not implemented, one computing nothing for a family that +// declares a computed status, a section carrying another version than the +// declared one, and a registration with no section are each refused. +func TestAggregateRefusesARegistrationItsDeclarationContradicts(t *testing.T) { + t.Parallel() + computesTheUnimplemented := notImplementedStandIn[core.WorktypeMetrics]("worktype", true) + computesTheUnimplemented.section = computedStandIn[core.WorktypeMetrics]("worktype", true).section + skipsTheComputed := computedStandIn[core.TemporalMetrics]("temporal", false) + skipsTheComputed.section = notImplemented[core.TemporalMetrics]() + otherVersion := computedStandIn[core.MessagesMetrics]("messages", false) + otherVersion.section = computed(func(core.Input) (core.Family[core.MessagesMetrics], []string) { + return core.Computed(core.Version{Major: 2}, core.MessagesMetrics{}), nil + }) + noSection := computedStandIn[core.CouplingMetrics]("coupling", false) + noSection.section = nil + for name, e := range map[string]entry{ + "a computed not-implemented family": computesTheUnimplemented, + "a skipped computed family": skipsTheComputed, + "a version other than the declared": otherVersion, + "a registration producing nothing": noSection, + } { + if _, _, err := routeOne(e, fullInput()); err == nil { + t.Errorf("%s was accepted", name) + } + } +} + +// TestAggregateCarriesTheDeclaredMethod is ADR-0032 clause 8 through the +// declaration: a section carries the method statement its family declares, +// computed or skipped. +func TestAggregateCarriesTheDeclaredMethod(t *testing.T) { + t.Parallel() + computedFamily := computedStandIn[core.CommitSizeMetrics]("commit_size", false) + d := computedFamily.family.Declaration() + d.Method = "how commit sizes are counted" + computedFamily.family = standIn(d) + skippedFamily := notImplementedStandIn[core.OwnershipMetrics]("ownership", true) + s := skippedFamily.family.Declaration() + s.Method = "how ownership is derived" + skippedFamily.family = standIn(s) + + var families core.Families + if _, err := route(fullInput(), &families, []entry{computedFamily, skippedFamily}); err != nil { + t.Fatalf("routing: %v", err) + } + if families.CommitSize.Method != d.Method || families.Ownership.Method != s.Method { + t.Errorf("methods = %q and %q, want the declared %q and %q", families.CommitSize.Method, + families.Ownership.Method, d.Method, s.Method) + } +} diff --git a/internal/pipeline/run.go b/internal/pipeline/run.go index d7f2661..bbfa884 100644 --- a/internal/pipeline/run.go +++ b/internal/pipeline/run.go @@ -18,24 +18,6 @@ import ( const minWrappedCommits = 10 -// commitSizeMethod states how effective lines are counted (docs/metrics.md -// section 1), where that differs from counting a file's lines. -const commitSizeMethod = "Effective lines are the lines git's diff adds and removes in each file that is not " + - "excluded. A file git detects as binary, by a NUL byte within its first 8 000 bytes unless a repository " + - "attribute says otherwise, contributes none. Rename detection is on, so a renamed file contributes the " + - "lines its content changed, and a file moved without change contributes none." - -// ownershipMethod states how line ownership is derived (ADR-0032 clause 8, -// docs/metrics.md section 7), where that differs from git blame, the -// reference implementation. -const ownershipMethod = "Line ownership is derived by forward replay of the history's diffs over the commit graph, " + - "not by git blame. Each version of a file is aligned with the version it was changed from by one fixed " + - "alignment computed in-process, and a line keeps its owner for as long as it is unchanged; at a merge, a line " + - "keeps the owner it has in whichever parent holds it unchanged, and a line no parent holds is the merge's. " + - "Blame's copy and move detection is not reproduced, so a line copied or moved from another file is owned by " + - "the commit that copied or moved it. A text file larger than the single-file-size limit is not read, and is " + - "marked as such." - // configurationRemedy is the remedy for every configuration refusal: they all // come from the same file and are all fixed the same way. const configurationRemedy = "Correct the setting in " + config.FileName + @@ -62,19 +44,146 @@ func (a *Analyzer) Run(ctx context.Context, opts Options, sink ProgressSink) (*R if ctx == nil { ctx = context.Background() } + emit := eventEmitter{sink: sink} + p, err := a.prepare(ctx, opts, &emit) + if err != nil { + return nil, err + } + cfg, filtered := p.inputs.Analysis, p.derived.filtered + + // The year is an analysis value, from --wrapped or from the configuration, + // and filters every metric. Deviation, removed by WP-0017: ADR-0008 + // clause 2 requires Wrapped to be generated from the same report as the + // dashboard, so the year must stop reaching the pipeline at all. Removing + // it changes what callers see, which is that package's to do. + if cfg.Year != 0 { + inYear := countInYear(filtered.Commits, cfg.Year, cfg) + if inYear < minWrappedCommits { + return nil, core.NewUserError(core.ReasonYearBelowThreshold, strconv.Itoa(cfg.Year), + fmt.Sprintf("Choose a year with at least %d analysed commits, or drop --wrapped and the year setting.", + minWrappedCommits), + "the requested year has %d analysed commits and the year in review needs %d", + inYear, minWrappedCommits) + } + } + + if err := contextError(ctx); err != nil { + return nil, err + } + report, buildWarnings, err := a.aggregate(ctx, p.inputs, p.derived, opts, &emit) + if err != nil { + return nil, err + } + for _, message := range buildWarnings { + p.warnings.add(message) + } + + var previousYearCommits *int + if cfg.Year != 0 { + previous := countInYear(filtered.Commits, cfg.Year-1, cfg) + if previous > 0 { + previousYearCommits = &previous + } + } + emit.emit(StageFinalizing, "analysis complete") + + history := p.inputs.History + result := &Result{ + Report: report, + Repository: history.Repository, + Analysis: cfg, + Operational: p.operational, + Warnings: append([]string(nil), p.warnings.messages...), + PreviousYearCommits: previousYearCommits, + } + if opts.CheckConsistency { + if err := contextError(ctx); err != nil { + return nil, err + } + end, err := a.collector.Preflight(ctx, p.repoPath, opts.SuppliedPath()) + if err != nil { + result.Stale = true + result.StaleReason = fmt.Sprintf("%s: %s", StaleRevalidationFailed, core.Artifact(err)) + } else { + result.EndRepository = &end + result.Stale, result.StaleReason = repositoryChanged(history.Repository, end) + } + } + return result, nil +} + +// Inputs are what the aggregate stage runs over: the collect stage's history, +// the replay stage's state, and the analysis configuration both were produced +// under. Each is independently cacheable (ADR-0020 clauses 2 and 5), and +// aggregation reads nothing else. +type Inputs struct { + History *model.History + Replay *core.ReplayState + Analysis config.Analysis +} + +// Prepare runs the stages before aggregation as Run does, and returns what +// aggregation runs over. Prepare followed by Aggregate is Run in two halves, +// without the progress events and without the checks Run makes around +// aggregation: the year threshold and the end-of-run consistency check. +func (a *Analyzer) Prepare(ctx context.Context, opts Options) (*Inputs, error) { + if ctx == nil { + ctx = context.Background() + } + p, err := a.prepare(ctx, opts, &eventEmitter{}) + if err != nil { + return nil, err + } + return &p.inputs, nil +} + +// Aggregate runs the aggregate stage alone, over inputs. It rebuilds the +// identity layer and the path filter from the history, as Run does, and +// reads no file and starts no process. So it produces the report Run +// produces, whether the inputs come straight from the earlier stages or from +// a cache, and whether the repository is still there or not (ADR-0020 +// clause 5). Of opts it reads the build's version and the degree of +// parallelism alone. +func (a *Analyzer) Aggregate(ctx context.Context, in Inputs, opts Options) (*core.Report, []string, error) { + if ctx == nil { + ctx = context.Background() + } + if in.History == nil || in.Replay == nil { + return nil, nil, core.Internalf(nil, "aggregating without the collect stage's history or the replay "+ + "stage's state") + } + d, err := derive(in.Analysis, in.History) + if err != nil { + return nil, nil, err + } + return a.aggregate(ctx, in, d, opts, &eventEmitter{}) +} + +// prepared is what the stages before aggregation leave for the rest of a run. +type prepared struct { + inputs Inputs + derived derived + operational config.Operational + warnings *warningLog + // repoPath is the resolved form of the repository path, used for every + // git invocation and every containment check. opts.RepoPath is the form + // the operator supplied, and is the only one a message may name + // (ADR-0067 clause 5). + repoPath string +} + +// prepare runs every stage before aggregation: preflight, configuration, +// collect and replay. +func (a *Analyzer) prepare(ctx context.Context, opts Options, emit *eventEmitter) (*prepared, error) { if err := contextError(ctx); err != nil { return nil, err } - // repoPath is the resolved form, used for every git invocation and every - // containment check. opts.RepoPath is the form the operator supplied, and - // is the only one a message may name (ADR-0067 clause 5). repoPath, err := filepath.Abs(opts.RepoPath) if err != nil { return nil, core.Internalf(err, "resolving the repository path") } - emit := eventEmitter{sink: sink} emit.emit(StagePreflight, "validating repository") info, err := a.collector.Preflight(ctx, repoPath, opts.SuppliedPath()) if err != nil { @@ -129,13 +238,7 @@ func (a *Analyzer) Run(ctx context.Context, opts Options, sink ProgressSink) (*R configurationRemedy, "an exclude_paths pattern could not be compiled") } - warnings := make([]string, 0) - collectWarn := func(message string) { - warnings = append(warnings, message) - if opts.OnWarning != nil { - opts.OnWarning(message) - } - } + warnings := &warningLog{onWarning: opts.OnWarning} emit.emit(StageCollecting, "reading history") history, err := a.collector.Collect(collect.Options{ RepoPath: repoPath, @@ -144,7 +247,7 @@ func (a *Analyzer) Run(ctx context.Context, opts Options, sink ProgressSink) (*R Parallelism: opts.Parallelism, ToolVersion: opts.ToolVersion, Context: ctx, - OnWarning: collectWarn, + OnWarning: warnings.add, OnProgress: func(current, total int) { if total > 0 { emit.emitCount(StageCollecting, fmt.Sprintf("%d of %d commits", current, total), current, total) @@ -166,15 +269,13 @@ func (a *Analyzer) Run(ctx context.Context, opts Options, sink ProgressSink) (*R // filter, which replay and the files family apply. Neither reads the // repository. emit.emit(StageIdentity, "resolving identities") - resolver := identity.NewResolver(cfg, history.Commits) - identities := resolver.Identities() - emit.emitCount(StageIdentity, fmt.Sprintf("%d contributors", len(identities)), len(identities), len(identities)) - - pathFilter, err := filter.NewPathFilterFromAttributes(cfg, []byte(history.Attributes)) + d, err := derive(cfg, history) if err != nil { - return nil, core.Internalf(err, "building the path filter from the collected attributes") + return nil, err } - filtered := filter.Summarize(history.Commits) + identities := d.resolver.Identities() + emit.emitCount(StageIdentity, fmt.Sprintf("%d contributors", len(identities)), len(identities), len(identities)) + filtered := d.filtered emit.emitCount(StageFiltering, fmt.Sprintf("%d excluded", filtered.TotalCommits-filtered.AnalyzedCommits), filtered.TotalCommits-filtered.AnalyzedCommits, filtered.TotalCommits) // Replay is the only stage that reads the repository's contents @@ -185,7 +286,7 @@ func (a *Analyzer) Run(ctx context.Context, opts Options, sink ProgressSink) (*R Context: ctx, RepoPath: repoPath, History: history, - PathFilter: pathFilter, + PathFilter: d.pathFilter, MaxFileBytes: operational.MaxFileBytes, }) if err != nil { @@ -194,33 +295,50 @@ func (a *Analyzer) Run(ctx context.Context, opts Options, sink ProgressSink) (*R if err := contextError(ctx); err != nil { return nil, err } + return &prepared{ + inputs: Inputs{History: history, Replay: replayed, Analysis: cfg}, + derived: d, + operational: operational, + warnings: warnings, + repoPath: repoPath, + }, nil +} - // The year is an analysis value, from --wrapped or from the configuration, - // and filters every metric. Deviation, removed by WP-0017: ADR-0008 - // clause 2 requires Wrapped to be generated from the same report as the - // dashboard, so the year must stop reaching the pipeline at all. Removing - // it changes what callers see, which is that package's to do. - if cfg.Year != 0 { - inYear := countInYear(filtered.Commits, cfg.Year, cfg) - if inYear < minWrappedCommits { - return nil, core.NewUserError(core.ReasonYearBelowThreshold, strconv.Itoa(cfg.Year), - fmt.Sprintf("Choose a year with at least %d analysed commits, or drop --wrapped and the year setting.", - minWrappedCommits), - "the requested year has %d analysed commits and the year in review needs %d", - inYear, minWrappedCommits) - } +// derived is what the stages after collection rebuild from its history +// alone: the identity layer, the path filter and the filtered records. +// Nothing in it reads the repository. +type derived struct { + resolver *identity.Resolver + pathFilter *filter.PathFilter + filtered filter.Result +} + +func derive(cfg config.Analysis, history *model.History) (derived, error) { + resolver := identity.NewResolver(cfg, history.Commits) + pathFilter, err := filter.NewPathFilterFromAttributes(cfg, []byte(history.Attributes)) + if err != nil { + return derived{}, core.Internalf(err, "building the path filter from the collected attributes") } + return derived{resolver: resolver, pathFilter: pathFilter, filtered: filter.Summarize(history.Commits)}, nil +} - input := core.Input{ +// aggregate runs the aggregate stage over the inputs and what derive rebuilt +// from their history. The stage is given no repository location: nothing it +// does needs one (ADR-0020 clause 5). +func (a *Analyzer) aggregate(ctx context.Context, in Inputs, d derived, opts Options, + emit *eventEmitter) (*core.Report, []string, error) { + return a.builder.Build(core.Input{ Context: ctx, - RepoPath: repoPath, - Repository: history.Repository, - Config: cfg, - Filtered: filtered, - Resolver: resolver, - PathFilter: pathFilter, - Replay: replayed, + Repository: in.History.Repository, + Config: in.Analysis, + Filtered: d.filtered, + Resolver: d.resolver, + PathFilter: d.pathFilter, + Replay: in.Replay, ToolVersion: opts.ToolVersion, + // One degree governs both parallel stages: collect's readers and + // aggregate's families (ADR-0052 clauses 1, 3 and 5). + Parallelism: opts.Parallelism, Progress: func(stage, detail string, current, total int) { mapped := StageCode if stage == "metrics" { @@ -239,63 +357,21 @@ func (a *Analyzer) Run(ctx context.Context, opts Options, sink ProgressSink) (*R } emit.emitProgress(mapped, detail, current, total) }, - } + }) +} - if err := contextError(ctx); err != nil { - return nil, err - } - report, buildWarnings, err := a.builder.Build(input) - if err != nil { - return nil, err - } - for _, message := range buildWarnings { - collectWarn(message) - } - // Effective lines rest on how the collect stage reads a diff, which - // differs from a plain line count in two ways a reader must be told of - // (ADR-0032 clause 8, WP-0012 clause 10c). The statement is set here, on - // the family whose values are effective lines, until the family registry - // of WP-0015 gives a family's own declaration a place to carry it. - if report.Families.CommitSize.Status != core.StatusSkipped { - report.Families.CommitSize.Method = commitSizeMethod - } - // Replay-derived ownership differs from blame, and the report says so - // whatever the family's status (ADR-0020, ADR-0032 clause 8). The family - // stays skipped until WP-0023 computes it from the replay state, and its - // own declaration carries the statement once WP-0015 gives it a place. - report.Families.Ownership.Method = ownershipMethod +// warningLog keeps a run's warnings for its result, and hands each to the +// caller as it arrives. +type warningLog struct { + messages []string + onWarning func(string) +} - var previousYearCommits *int - if cfg.Year != 0 { - previous := countInYear(filtered.Commits, cfg.Year-1, cfg) - if previous > 0 { - previousYearCommits = &previous - } +func (w *warningLog) add(message string) { + w.messages = append(w.messages, message) + if w.onWarning != nil { + w.onWarning(message) } - emit.emit(StageFinalizing, "analysis complete") - - result := &Result{ - Report: report, - Repository: history.Repository, - Analysis: cfg, - Operational: operational, - Warnings: append([]string(nil), warnings...), - PreviousYearCommits: previousYearCommits, - } - if opts.CheckConsistency { - if err := contextError(ctx); err != nil { - return nil, err - } - end, err := a.collector.Preflight(ctx, repoPath, opts.SuppliedPath()) - if err != nil { - result.Stale = true - result.StaleReason = fmt.Sprintf("%s: %s", StaleRevalidationFailed, core.Artifact(err)) - } else { - result.EndRepository = &end - result.Stale, result.StaleReason = repositoryChanged(history.Repository, end) - } - } - return result, nil } func repositoryChanged(start, end model.RepositoryInfo) (bool, string) { diff --git a/internal/server/api_test.go b/internal/server/api_test.go index 1704253..7a1b66c 100644 --- a/internal/server/api_test.go +++ b/internal/server/api_test.go @@ -29,7 +29,7 @@ func TestAPICapabilitiesAndJobList(t *testing.T) { if err := json.Unmarshal(res.Body.Bytes(), &capabilities); err != nil { t.Fatal(err) } - if capabilities.APIVersion != "v1" || capabilities.ReportSchemaVersion != 1 || capabilities.ActiveJobLimit != 1 { + if capabilities.APIVersion != "v1" || capabilities.ReportSchemaVersion != 2 || capabilities.ActiveJobLimit != 1 { t.Fatalf("capabilities = %+v", capabilities) } diff --git a/testdata/golden/agent-coauthor-trailers.json b/testdata/golden/agent-coauthor-trailers.json index 8537ab5..aa51c70 100644 --- a/testdata/golden/agent-coauthor-trailers.json +++ b/testdata/golden/agent-coauthor-trailers.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -175,7 +175,7 @@ "last_commit_date": "2025-10-09" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -261,7 +261,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -287,7 +287,7 @@ ] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/basic.json b/testdata/golden/basic.json index 6f2d9e8..5bab086 100644 --- a/testdata/golden/basic.json +++ b/testdata/golden/basic.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -202,7 +202,7 @@ "last_commit_date": "2026-02-25" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -313,7 +313,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -339,7 +339,7 @@ ] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/binary.json b/testdata/golden/binary.json index e098f2d..bad5cbb 100644 --- a/testdata/golden/binary.json +++ b/testdata/golden/binary.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -175,7 +175,7 @@ "last_commit_date": "2025-10-04" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -276,7 +276,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -297,7 +297,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/bots.json b/testdata/golden/bots.json index 72a6a89..3b38d15 100644 --- a/testdata/golden/bots.json +++ b/testdata/golden/bots.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -184,7 +184,7 @@ "last_commit_date": "2025-10-04" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -270,7 +270,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -291,7 +291,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/coupling.json b/testdata/golden/coupling.json index c1a1fcc..47bebc0 100644 --- a/testdata/golden/coupling.json +++ b/testdata/golden/coupling.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -175,7 +175,7 @@ "last_commit_date": "2025-10-12" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -279,7 +279,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -309,7 +309,7 @@ ] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/large-history.json b/testdata/golden/large-history.json index ec47e40..377b2d7 100644 --- a/testdata/golden/large-history.json +++ b/testdata/golden/large-history.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -274,7 +274,7 @@ "last_commit_date": "2025-09-30" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -513,7 +513,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -551,7 +551,7 @@ ] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/mailmap.json b/testdata/golden/mailmap.json index 5106b54..4ee627f 100644 --- a/testdata/golden/mailmap.json +++ b/testdata/golden/mailmap.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -193,7 +193,7 @@ "last_commit_date": "2026-02-25" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -309,7 +309,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -335,7 +335,7 @@ ] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/merged-side-branch.json b/testdata/golden/merged-side-branch.json index d0c6db2..0674635 100644 --- a/testdata/golden/merged-side-branch.json +++ b/testdata/golden/merged-side-branch.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -193,7 +193,7 @@ "last_commit_date": "2025-10-07" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -289,7 +289,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -310,7 +310,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/merges.json b/testdata/golden/merges.json index add54e8..a873952 100644 --- a/testdata/golden/merges.json +++ b/testdata/golden/merges.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -193,7 +193,7 @@ "last_commit_date": "2025-10-31" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -329,7 +329,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -350,7 +350,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/multi-year-gap.json b/testdata/golden/multi-year-gap.json index 0c6b8f0..2523240 100644 --- a/testdata/golden/multi-year-gap.json +++ b/testdata/golden/multi-year-gap.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -175,7 +175,7 @@ "last_commit_date": "2023-06-01" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -261,7 +261,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -282,7 +282,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/noise.json b/testdata/golden/noise.json index 07a489f..a3e326b 100644 --- a/testdata/golden/noise.json +++ b/testdata/golden/noise.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -184,7 +184,7 @@ "last_commit_date": "2025-10-05" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -275,7 +275,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -296,7 +296,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/renames-and-copied-block.json b/testdata/golden/renames-and-copied-block.json index 3d4f834..f9d4a21 100644 --- a/testdata/golden/renames-and-copied-block.json +++ b/testdata/golden/renames-and-copied-block.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -193,7 +193,7 @@ "last_commit_date": "2025-10-05" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -299,7 +299,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -320,7 +320,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/single.json b/testdata/golden/single.json index 18f12ef..ea1bfe9 100644 --- a/testdata/golden/single.json +++ b/testdata/golden/single.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -175,7 +175,7 @@ "last_commit_date": "2025-10-01" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -261,7 +261,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -282,7 +282,7 @@ "churn_files": [] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0 diff --git a/testdata/golden/worktype-events.json b/testdata/golden/worktype-events.json index 912f977..8e96407 100644 --- a/testdata/golden/worktype-events.json +++ b/testdata/golden/worktype-events.json @@ -1,7 +1,7 @@ { "document_version": { - "major": 1, - "minor": 4 + "major": 2, + "minor": 0 }, "sections": { "metadata": { @@ -202,7 +202,7 @@ "last_commit_date": "2025-12-03" } }, - "commit-size": { + "commit_size": { "version": { "major": 2, "minor": 0 @@ -293,7 +293,7 @@ ], "metrics": {} }, - "ai-archaeology": { + "ai_archaeology": { "version": { "major": 0, "minor": 0 @@ -319,7 +319,7 @@ ] } }, - "static-analysis": { + "static_analysis": { "version": { "major": 0, "minor": 0