Skip to content

refactor(conflict): take a batch of changes as analyzer input#202

Open
behinddwalls wants to merge 1 commit into
preetam/mock-extfrom
preetam/conflict-analyzer
Open

refactor(conflict): take a batch of changes as analyzer input#202
behinddwalls wants to merge 1 commit into
preetam/mock-extfrom
preetam/conflict-analyzer

Conversation

@behinddwalls
Copy link
Copy Markdown
Collaborator

@behinddwalls behinddwalls commented Jun 5, 2026

Summary

Why?

The conflict analyzer received entity.Batch values (request IDs only), so a real
analyzer — eventually backed by uber/tango ChangedTargetsAndEdges — could not see
the actual changes without storage lookups, which extensions must not do. The
scorer already solved this by moving to entity.BatchChanges; mirror it here.

What?

  • conflict.Analyzer.Analyze input changes from entity.Batch to entity.BatchChanges
    for both candidate and in-flight; the output ([]Conflict) is unchanged. No new
    entity types — a Tango-backed analyzer derives targets from the change URIs/files
    itself, with the target branch injected per queue at construction.
  • New submitqueue/core/batchchanges.Collect: shared assembler that resolves a
    batch's request IDs -> change records -> entity.BatchChanges. The score and batch
    controllers both use it (extracted from the score controller).
  • batch controller assembles candidate + in-flight BatchChanges before Analyze.
  • all/none updated to the new input; conflict/fake switched to the
    sq-fake=conflict-error URI marker (drops FailOn/FailAlways); mock regenerated.
  • example: conflictfake.New(all/none) without a predicate; removed the dedicated
    e2e-conflict-error-queue (the marker now reaches the analyzer on any queue).

Test Plan

  • ✅ make test (45/45 unit), make e2e-test (1/1), make integration-test (7/7)
  • ✅ bazel build //...; check-gazelle / check-tidy / check-mocks clean once committed

Issues

Stack

  1. feat(extensions): fake implementations with error injection #197
  2. feat(example): per-queue extension wiring; retire buildrunner/noop #193
  3. @ refactor(conflict): take a batch of changes as analyzer input #202

@behinddwalls behinddwalls marked this pull request as ready for review June 5, 2026 03:25
@behinddwalls behinddwalls requested review from a team and sbalabanov as code owners June 5, 2026 03:25
@behinddwalls behinddwalls force-pushed the preetam/conflict-analyzer branch from 4ff3f88 to 3e09ea0 Compare June 5, 2026 03:35
@behinddwalls behinddwalls force-pushed the preetam/conflict-analyzer branch from 3e09ea0 to 75b838e Compare June 5, 2026 03:46
@behinddwalls behinddwalls force-pushed the preetam/conflict-analyzer branch from 75b838e to eb71dbe Compare June 5, 2026 04:44
@behinddwalls behinddwalls force-pushed the preetam/conflict-analyzer branch 2 times, most recently from 3deb3ff to f377508 Compare June 5, 2026 05:41
behinddwalls added a commit that referenced this pull request Jun 5, 2026
## Summary
### Why?

The scorer took entity.Change (just URIs), so it could not score on real
change size — the example heuristic counted URIs as a placeholder. With
typed change details now persisted on change records, the scorer can
score a
batch on its actual lines/files changed.

### What?

- Add entity.BatchChanges — the normalized, batch-level view of all
changes
  in a batch (BatchID, Queue, []ChangeInfo) with aggregation helpers.
- Scorer.Score now takes entity.BatchChanges; the heuristic ValueFunc
and the
  composite scorer operate over it.
- The score controller resolves each request's change records, flattens
their
  details into BatchChanges, and scores the batch once — replacing the
  per-request multiplicative product over len(URIs).
- Example wiring buckets by total lines changed.

Consumes the typed details persisted by the change-details change.

## Test Plan
- ✅ `make build`, `make test`, `make lint`, `make
check-mocks/gazelle/tidy`
- ✅ `make integration-test`, `make e2e-test` (start -> validate enrich
->
  score normalizes the batch and scores on real change size)

## Issues


## Stack
1. #195
1. @ #196
1. #197
1. #193
1. #202
@behinddwalls behinddwalls force-pushed the preetam/conflict-analyzer branch from f377508 to fb39a16 Compare June 5, 2026 05:56
## Summary

### Why?

The conflict analyzer received entity.Batch values (request IDs only), so a real
analyzer — eventually backed by uber/tango ChangedTargetsAndEdges — could not see
the actual changes without storage lookups, which extensions must not do. The
scorer already solved this by moving to entity.BatchChanges; mirror it here.

### What?

- conflict.Analyzer.Analyze input changes from entity.Batch to entity.BatchChanges
  for both candidate and in-flight; the output ([]Conflict) is unchanged. No new
  entity types — a Tango-backed analyzer derives targets from the change URIs/files
  itself, with the target branch injected per queue at construction.
- New submitqueue/core/batchchanges.Collect: shared assembler that resolves a
  batch's request IDs -> change records -> entity.BatchChanges. The score and batch
  controllers both use it (extracted from the score controller).
- batch controller assembles candidate + in-flight BatchChanges before Analyze.
- all/none updated to the new input; conflict/fake switched to the
  sq-fake=conflict-error URI marker (drops FailOn/FailAlways); mock regenerated.
- example: conflictfake.New(all/none) without a predicate; removed the dedicated
  e2e-conflict-error-queue (the marker now reaches the analyzer on any queue).

## Test Plan

- ✅ make test (45/45 unit), make e2e-test (1/1), make integration-test (7/7)
- ✅ bazel build //...; check-gazelle / check-tidy / check-mocks clean once committed
@behinddwalls behinddwalls force-pushed the preetam/conflict-analyzer branch from fb39a16 to 1336a44 Compare June 5, 2026 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant