refactor(conflict)!: key path overlap by file or by directory (#575)

## Summary

### Why?

`fileoverlap` serialized two batches only when they changed the exact
same file. That is the narrowest useful reading of target overlap, and
for a queue whose directories are tightly coupled it is too narrow: two
changes to sibling files in one package can break each other without
either one touching the path the other did, and the analyzer will
happily speculate them in parallel.

The right granularity is not a property of the analyzer. It follows from
how tightly coupled a directory's contents are in the repository behind
the queue, which is something only the integrator wiring that queue up
can know. So it belongs at construction, next to the resolver, rather
than baked into the package.

### What?

Overlap is measured on a key projected from each changed path. `PathKey`
is that projection, chosen at construction and applied to every path
before the two batches' sets are intersected; `Analyze` is otherwise
unchanged.

Two projections ship with the package. `ByFile` keys on the whole path
and reproduces the previous behaviour. `ByDirectory` keys on the
immediate parent, so batches touching sibling files conflict too —
strictly coarser, since every file overlap is also a directory overlap.
It buys protection against semantic conflicts between neighbouring files
and pays for it in parallelism, which is the trade the integrator is
choosing between.

Paths at the repository root key on `.` under `ByDirectory`, so a batch
touching `README.md` conflicts with one touching `go.mod`. Root files
are usually build configuration and usually do interact, so this is
deliberate rather than incidental.

The package is renamed `fileoverlap` → `pathoverlap`, because the unit
of overlap is now a path-derived key rather than a file. `New` takes the
key as a third argument and panics on nil, mirroring `heuristic.New`.
`conflict.Analyzer`, `conflict.Config` and `ConflictTypeTargetOverlap`
are untouched — a folder is a coarser target, not a different kind of
one. The only caller, `file-overlap-queue` in the orchestrator profiles,
passes `ByFile` and keeps its behaviour and its name.

One incidental behaviour change: `ByFile` runs `path.Clean`, where paths
were previously compared verbatim. A provider emitting an unclean path
used to produce a missed conflict.

## Test Plan

✅ `make test` — 98 pass
✅ `make lint`, `make check-gazelle`, `make check-tidy`

New coverage in `pathoverlap_test.go`:

- `TestPathKey` — both projections over a nested path, a repository-root
file, and an unclean path.
- Sibling files in one directory: no conflict under `ByFile`, conflict
under `ByDirectory`; files in sibling directories conflict under
neither; the same file still conflicts under both.
- Two root-level files conflict under `ByDirectory` while a nested file
in the same batch set does not.
- `New` panics when the key is nil.

Author: behinddwalls

doc/rfc/submitqueue/modular-queue-wiring.md

@@ -466,7 +466,7 @@ func run(ctx context.Context) error {
                 ChangeProvider(github.New(cfg.GitHub)).
                 BuildRunner(local.New()).
                 Scorer(heuristic.New()).
-                ConflictAnalyzer(fileoverlap.New()),
+                ConflictAnalyzer(pathoverlap.New()),
         ).
         Option(pipeline.TopicNames(cfg.TopicNames)).
         Option(pipeline.Classifiers(backendClassifiers())).
@@ -497,7 +497,7 @@ app, err := submitqueue.New().
     ).
     Queue(base.Named("monorepo/exp").
         BuildRunner(local.New()).
-        ConflictAnalyzer(fileoverlap.New()),
+        ConflictAnalyzer(pathoverlap.New()),
     ).
     Queue(base.Named("monorepo/test").
         BuildRunner(noop.New()).

service/submitqueue/orchestrator/server/BUILD.bazel

@@ -37,8 +37,8 @@ go_library(
         "//submitqueue/extension/conflict:go_default_library",
         "//submitqueue/extension/conflict/all:go_default_library",
         "//submitqueue/extension/conflict/fake:go_default_library",
-        "//submitqueue/extension/conflict/fileoverlap:go_default_library",
         "//submitqueue/extension/conflict/none:go_default_library",
+        "//submitqueue/extension/conflict/pathoverlap:go_default_library",
         "//submitqueue/extension/scorer:go_default_library",
         "//submitqueue/extension/scorer/composite:go_default_library",
         "//submitqueue/extension/scorer/fake:go_default_library",

service/submitqueue/orchestrator/server/profiles.go

@@ -27,8 +27,8 @@ import (
 	"github.com/uber/submitqueue/submitqueue/extension/conflict"
 	"github.com/uber/submitqueue/submitqueue/extension/conflict/all"
 	conflictfake "github.com/uber/submitqueue/submitqueue/extension/conflict/fake"
-	"github.com/uber/submitqueue/submitqueue/extension/conflict/fileoverlap"
 	"github.com/uber/submitqueue/submitqueue/extension/conflict/none"
+	"github.com/uber/submitqueue/submitqueue/extension/conflict/pathoverlap"
 	"github.com/uber/submitqueue/submitqueue/extension/scorer"
 	"github.com/uber/submitqueue/submitqueue/extension/scorer/composite"
 	scorerfake "github.com/uber/submitqueue/submitqueue/extension/scorer/fake"
@@ -256,9 +256,10 @@ func newProfiles(logger *zap.Logger, scope tally.Scope, resolver changeset.Resol
 
 	// file-overlap-queue: a real analyzer that serializes only batches sharing
 	// a changed file, resolving each batch's files itself via the resolver.
+	// pathoverlap.ByDirectory would coarsen this to whole directories.
 	fileOverlapQueue := base
 	fileOverlapQueue.Analyzer = analyzerFunc(func(c conflict.Config) (conflict.Analyzer, error) {
-		return fileoverlap.New(c, resolver), nil
+		return pathoverlap.New(c, resolver, pathoverlap.ByFile), nil
 	})
 
 	// e2e-test-queue: composite scorer; no conflicts (maximum parallelism).

submitqueue/extension/conflict/fileoverlap/README.md

@@ -2,8 +2,8 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["fileoverlap.go"],
-    importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/fileoverlap",
+    srcs = ["pathoverlap.go"],
+    importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/pathoverlap",
     visibility = ["//visibility:public"],
     deps = [
         "//submitqueue/core/changeset:go_default_library",
@@ -14,7 +14,7 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["fileoverlap_test.go"],
+    srcs = ["pathoverlap_test.go"],
     embed = [":go_default_library"],
     deps = [
         "//submitqueue/core/changeset/fake:go_default_library",

submitqueue/extension/conflict/fileoverlap/fileoverlap.go

@@ -0,0 +1,18 @@
+# pathoverlap
+
+`pathoverlap` is a `conflict.Analyzer` that reports a conflict between two batches when the paths they change share a key.
+
+## Behavior
+
+The files a batch changes are drawn from each change's provider-supplied details, and each path is projected onto a key by the `PathKey` chosen at construction. The candidate batch conflicts with an in-flight batch when their key sets intersect; each such in-flight batch is reported once, preserving the in-flight order. A shared path key is the concrete notion of *target overlap*, so conflicts are classified as `ConflictTypeTargetOverlap`. A batch that changes no files conflicts with nothing, and an empty in-flight list yields no conflicts. A failure to resolve a batch's changes is returned as a (retryable) error.
+
+## Granularity
+
+Two projections ship with the package, selected per queue in the wiring layer:
+
+- **`ByFile`** keys on the whole path, so only batches touching the same file conflict.
+- **`ByDirectory`** keys on the path's immediate parent directory, so batches touching sibling files conflict too. Paths at the repository root share the key `.`, which serializes batches that touch any two root-level files.
+
+`ByDirectory` is strictly coarser: every file overlap is also a directory overlap. It trades parallelism for protection against semantic conflicts between neighbouring files — edits that break each other without touching the same file. Which trade is right is a per-queue judgement about how tightly coupled a directory's contents are, so the choice is a construction parameter rather than a property of the analyzer.
+
+Path-key intersection is a deliberately simple notion of overlap. A richer one that needs inputs beyond the changed paths — build targets, ownership boundaries — would be a separate analyzer rather than another `PathKey`.

…tension/conflict/fileoverlap/BUILD.bazel …tension/conflict/pathoverlap/BUILD.bazelsubmitqueue/extension/conflict/fileoverlap/BUILD.bazel renamed to submitqueue/extension/conflict/pathoverlap/BUILD.bazel submitqueue/extension/conflict/fileoverlap/BUILD.bazel renamed to submitqueue/extension/conflict/pathoverlap/BUILD.bazel

@@ -0,0 +1,138 @@
+// Copyright (c) 2025 Uber Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package pathoverlap provides a conflict.Analyzer that reports a conflict
+// between two batches when the paths they change share a key. The key is a
+// projection of the path chosen at construction: ByFile compares whole paths,
+// so only batches touching the same file conflict; ByDirectory compares parent
+// directories, so batches touching sibling files conflict too. It is the first
+// analyzer to use the capability the extension contract unblocks: it takes only
+// batch identity and resolves each batch's changed files itself through an
+// injected changeset resolver, rather than depending on the controller to
+// pre-compute them. A shared path key is the concrete notion of target overlap,
+// so it reports entity.ConflictTypeTargetOverlap.
+package pathoverlap
+
+import (
+	"context"
+	"fmt"
+	"path"
+
+	"github.com/uber/submitqueue/submitqueue/core/changeset"
+	"github.com/uber/submitqueue/submitqueue/entity"
+	"github.com/uber/submitqueue/submitqueue/extension/conflict"
+)
+
+// PathKey projects a changed file path onto the key overlap is measured on. Two
+// batches conflict when any of their changed paths yield the same key, so a
+// coarser projection serializes more batches.
+type PathKey func(path string) string
+
+// ByFile keys on the whole path: batches conflict only when they change the
+// same file.
+func ByFile(p string) string {
+	return path.Clean(p)
+}
+
+// ByDirectory keys on the path's immediate parent directory: batches conflict
+// when they change any files in the same directory, whether or not the files
+// themselves are the same. Overlap by directory is strictly coarser than
+// overlap by file — every file overlap is also a directory overlap. Paths at
+// the repository root share the key ".".
+func ByDirectory(p string) string {
+	return path.Dir(p)
+}
+
+// analyzer reports a conflict between batches whose changed paths share a key.
+// The paths a batch changes are resolved from each batch's change details.
+type analyzer struct {
+	// cfg is the per-queue identity this analyzer was built for.
+	cfg      conflict.Config
+	resolver changeset.Resolver
+	// key projects each changed path onto the key overlap is measured on.
+	key PathKey
+}
+
+// New returns a conflict.Analyzer that flags an in-flight batch as conflicting
+// when it changes a path whose key matches one the candidate batch changes,
+// bound to the queue named in cfg. The resolver resolves each batch's changed
+// files, and key selects the granularity of overlap.
+// Panics if key is nil.
+func New(cfg conflict.Config, resolver changeset.Resolver, key PathKey) conflict.Analyzer {
+	if key == nil {
+		panic("pathoverlap.New: key must not be nil")
+	}
+	return analyzer{cfg: cfg, resolver: resolver, key: key}
+}
+
+// Analyze returns one ConflictTypeTargetOverlap Conflict per in-flight batch
+// that shares a path key with batch, preserving the in-flight order. A batch
+// that changes no files conflicts with nothing.
+func (a analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []entity.Batch) ([]entity.Conflict, error) {
+	if len(inFlight) == 0 {
+		return nil, nil
+	}
+
+	candidate, err := a.keys(ctx, batch)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve files for batch %s: %w", batch.ID, err)
+	}
+	if len(candidate) == 0 {
+		return nil, nil
+	}
+
+	var conflicts []entity.Conflict
+	for _, other := range inFlight {
+		keys, err := a.keys(ctx, other)
+		if err != nil {
+			return nil, fmt.Errorf("failed to resolve files for batch %s: %w", other.ID, err)
+		}
+		if intersects(candidate, keys) {
+			conflicts = append(conflicts, entity.Conflict{
+				BatchID: other.ID,
+				Type:    entity.ConflictTypeTargetOverlap,
+			})
+		}
+	}
+	return conflicts, nil
+}
+
+// keys resolves the set of path keys the batch changes.
+func (a analyzer) keys(ctx context.Context, batch entity.Batch) (map[string]struct{}, error) {
+	changes, err := a.resolver.DetailedForBatch(ctx, batch)
+	if err != nil {
+		return nil, err
+	}
+	keys := make(map[string]struct{})
+	for _, change := range changes.Changes {
+		for _, file := range change.Details.ChangedFiles {
+			keys[a.key(file.Path)] = struct{}{}
+		}
+	}
+	return keys, nil
+}
+
+// intersects reports whether the two sets share any element.
+func intersects(a, b map[string]struct{}) bool {
+	// Iterate the smaller set for fewer lookups.
+	if len(b) < len(a) {
+		a, b = b, a
+	}
+	for k := range a {
+		if _, ok := b[k]; ok {
+			return true
+		}
+	}
+	return false
+}