refactor(change): persist typed change details from the change provider

## Summary

### Why?

The change provider already produces rich per-URI facts (author, changed
files, line counts), but its value types lived in the extension layer and
the data was thrown away — validate fetched ChangeInfo only to log a file
count, and ChangeRecord stored an opaque Metadata JSON string that was never
written. Nothing downstream could read typed change facts.

### What?

- Move the change value types into entities: entity.User, entity.ChangedFile
  (now with LinesModified), entity.ChangeDetails (the facts), and
  entity.ChangeInfo (URI -> Details), with aggregation helpers. The
  changeprovider extension and GitHub impl now produce these.
- Replace ChangeRecord.Metadata (opaque string) with typed Details
  (ChangeDetails); the change table's metadata JSON column becomes details.
- Add ChangeStore.UpdateDetails — a version-guarded conditional write,
  following the optimistic-locking contract (arithmetic in the controller).
- validate now persists each fetched ChangeInfo onto the request's change
  records (per-URI, idempotent; ErrVersionMismatch is a benign no-op).

This is the producer half: typed details now exist and are persisted. The
score controller consumes them in a follow-up.

## Test Plan

- ✅ `make build`, `make test`, `make lint`, `make check-mocks/gazelle/tidy`
- ✅ `make integration-test` (storage contract suite round-trips Details and
  covers UpdateDetails create/update/version-mismatch)

Author: behinddwalls

submitqueue/entity/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
         "batch_dependent.go",
         "build.go",
         "cancel_request.go",
+        "change_provider.go",
         "change_record.go",
         "land_request.go",
         "queue_config.go",

submitqueue/entity/change_provider.go

@@ -0,0 +1,78 @@
+// 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 entity
+
+// User represents the author of a change.
+type User struct {
+	// Name is the display name of the user.
+	Name string `json:"name"`
+	// Email is the email address of the user.
+	Email string `json:"email"`
+}
+
+// ChangedFile represents a single file modification in a change.
+type ChangedFile struct {
+	// Path is the file path relative to the repository root.
+	Path string `json:"path"`
+	// Patch is the diff patch content for this file.
+	Patch string `json:"patch,omitempty"`
+	// LinesAdded is the number of lines added in this file.
+	LinesAdded int `json:"lines_added"`
+	// LinesDeleted is the number of lines deleted in this file.
+	LinesDeleted int `json:"lines_deleted"`
+	// LinesModified is the number of lines modified in this file. Some providers
+	// (e.g. GitHub) report only additions and deletions and leave this zero.
+	LinesModified int `json:"lines_modified"`
+}
+
+// TotalLines returns the total number of lines touched in this file.
+func (f ChangedFile) TotalLines() int {
+	return f.LinesAdded + f.LinesDeleted + f.LinesModified
+}
+
+// ChangeDetails holds the provider-supplied facts about a single change (author,
+// modified files, line counts). It carries no identity — the owning URI lives on
+// ChangeInfo (provider correlation) and ChangeRecord (persisted claim).
+type ChangeDetails struct {
+	// User is the author of the change.
+	User User `json:"user"`
+	// ChangedFiles is the list of files modified in this change. Order is unspecified.
+	ChangedFiles []ChangedFile `json:"changed_files,omitempty"`
+}
+
+// TotalLinesChanged returns the total number of lines touched across all files in the change.
+func (d ChangeDetails) TotalLinesChanged() int {
+	total := 0
+	for _, f := range d.ChangedFiles {
+		total += f.TotalLines()
+	}
+	return total
+}
+
+// FileCount returns the number of files touched in the change.
+func (d ChangeDetails) FileCount() int {
+	return len(d.ChangedFiles)
+}
+
+// ChangeInfo maps a change URI to its details. It is the change provider's return
+// type: for a Change with multiple URIs (e.g. a stacked PR set), the provider
+// returns one ChangeInfo per URI so callers can correlate results to inputs by URI.
+type ChangeInfo struct {
+	// URI is the full change URI for correlation with the input request
+	// (e.g., "github://uber/repo/pull/98/c3a4d5e6f7890123456789abcdef0123456789ab").
+	URI string `json:"uri"`
+	// Details is the provider-supplied facts for this URI.
+	Details ChangeDetails `json:"details"`
+}

submitqueue/entity/change_record.go

@@ -15,9 +15,9 @@
 package entity
 
 // ChangeRecord represents a single URI's claim by a request, persisted in the change store.
-// The (Queue, URI, RequestID) triple is the identity and is immutable; Metadata may be
-// updated over time as additional information about the change (e.g., PR title, author,
-// mergeability) becomes available.
+// The (Queue, URI, RequestID) triple is the identity and is immutable; Details may be
+// updated over time as the change provider supplies information about the change (author,
+// changed files, line counts).
 type ChangeRecord struct {
 	// URI identifies the change (RFC 3986). Same scheme/format as entity.Change.URIs.
 	// Example: "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab".
@@ -29,27 +29,27 @@ type ChangeRecord struct {
 	// RequestID participates in the change-store primary key so that concurrent claims
 	// by different requests on the same URI coexist as distinct rows. Same-request
 	// retries collide on the PK and are absorbed idempotently; different-request
-	// collisions surface as additional rows that callers detect via FindOverlapping.
+	// collisions surface as additional rows that callers detect via GetByURI.
 	RequestID string `json:"request_id"`
 
 	// Queue is the queue the owning request belongs to. It is the leading column of
 	// the change-store primary key, so queue-scoped duplicate checks become PK-prefix
 	// scans and the table is shardable by queue.
 	Queue string `json:"queue"`
 
-	// Metadata is a JSON-encoded blob of provider-specific information about the change
-	// (e.g., PR title, author, mergeable state). Stored as `'{}'` when no metadata has
-	// been populated yet; updated by downstream enrichment.
-	Metadata string `json:"metadata,omitempty"`
+	// Details holds the provider-supplied facts about the change (author, changed files,
+	// line counts). Zero-valued when no provider data has been populated yet; updated by
+	// downstream enrichment (the validate controller) via the change store's UpdateDetails.
+	Details ChangeDetails `json:"details"`
 
 	// CreatedAt is the Unix milliseconds timestamp when this record was first created.
 	CreatedAt int64 `json:"created_at"`
 
-	// UpdatedAt is the Unix milliseconds timestamp when this record's Metadata was last updated.
+	// UpdatedAt is the Unix milliseconds timestamp when this record's Details was last updated.
 	// Equal to CreatedAt when the record has never been updated.
 	UpdatedAt int64 `json:"updated_at"`
 
-	// Version is the optimistic-locking counter for mutable fields (Metadata).
+	// Version is the optimistic-locking counter for mutable fields (Details).
 	// Starts at 1 on Create and is incremented by callers on every update.
 	// Mirrors the request-store convention: callers compute newVersion = oldVersion + 1
 	// and pass both to the update method; the store performs a pure conditional write.

submitqueue/extension/changeprovider/change_provider.go

@@ -22,44 +22,17 @@ import (
 	"github.com/uber/submitqueue/submitqueue/entity"
 )
 
-// User represents the author of a change.
-type User struct {
-	// Name is the display name of the user.
-	Name string
-	// Email is the email address of the user.
-	Email string
-}
-
-// ChangedFile represents a single file modification in a change.
-type ChangedFile struct {
-	// Path is the file path relative to the repository root.
-	Path string
-	// Patch is the diff patch content for this file.
-	Patch string
-	// LinesAdded is the number of lines added in this file.
-	LinesAdded int
-	// LinesDeleted is the number of lines deleted in this file.
-	LinesDeleted int
-}
-
-// ChangeInfo contains metadata and file changes for a code change.
-type ChangeInfo struct {
-	// URI is the full change URI for correlation with the input request
-	// (e.g., "github://uber/repo/pull/98/c3a4d5e6f7890123456789abcdef0123456789ab" or "phab://D123/xyz789").
-	URI string
-	// User is the author of the change.
-	User User
-	// ChangedFiles is the list of files modified in this change. Order is unspecified.
-	ChangedFiles []ChangedFile
-}
-
 // ChangeProvider fetches change metadata from external systems
 // Each implementation is configured for a specific provider (GitHub, GitLab, Phabricator).
+//
+// The change value types it produces — entity.ChangeInfo, entity.ChangeDetails,
+// entity.User, entity.ChangedFile — live in the entity package so the same typed
+// facts can be persisted (entity.ChangeRecord) and scored without re-declaration.
 type ChangeProvider interface {
 	// Get retrieves change information for the provided Change.
 	// For a Change with multiple URIs (e.g., stacked PRs), returns one ChangeInfo per URI.
 	// Returns a slice of ChangeInfo, one for each change in the stack.
-	Get(ctx context.Context, change entity.Change) ([]ChangeInfo, error)
+	Get(ctx context.Context, change entity.Change) ([]entity.ChangeInfo, error)
 }
 
 // Config carries the per-queue identity handed to a Factory. The system knows

submitqueue/extension/changeprovider/github/convert.go

@@ -1,30 +1,32 @@
 package github
 
 import (
+	"github.com/uber/submitqueue/submitqueue/entity"
 	entitygithub "github.com/uber/submitqueue/submitqueue/entity/github"
-	"github.com/uber/submitqueue/submitqueue/extension/changeprovider"
 )
 
-// convertToChangeInfo converts GitHub PR data to ChangeInfo.
-func convertToChangeInfo(parsed entitygithub.ChangeID, prData *pullRequestData) changeprovider.ChangeInfo {
-	changedFiles := convertFiles(prData.Files.Nodes)
-
-	return changeprovider.ChangeInfo{
+// convertToChangeInfo converts GitHub PR data to an entity.ChangeInfo.
+func convertToChangeInfo(parsed entitygithub.ChangeID, prData *pullRequestData) entity.ChangeInfo {
+	return entity.ChangeInfo{
 		URI: parsed.String(),
-		User: changeprovider.User{
-			Name:  prData.Author.Name,
-			Email: prData.Author.Email,
+		Details: entity.ChangeDetails{
+			User: entity.User{
+				Name:  prData.Author.Name,
+				Email: prData.Author.Email,
+			},
+			ChangedFiles: convertFiles(prData.Files.Nodes),
 		},
-		ChangedFiles: changedFiles,
 	}
 }
 
-// convertFiles converts GitHub file nodes to ChangedFile structs.
-func convertFiles(nodes []fileNode) []changeprovider.ChangedFile {
-	changedFiles := make([]changeprovider.ChangedFile, 0, len(nodes))
+// convertFiles converts GitHub file nodes to entity.ChangedFile structs.
+// GitHub's API reports only additions and deletions per file, so LinesModified
+// is left zero here.
+func convertFiles(nodes []fileNode) []entity.ChangedFile {
+	changedFiles := make([]entity.ChangedFile, 0, len(nodes))
 
 	for _, file := range nodes {
-		changedFiles = append(changedFiles, changeprovider.ChangedFile{
+		changedFiles = append(changedFiles, entity.ChangedFile{
 			Path:         file.Path,
 			Patch:        file.Patch,
 			LinesAdded:   file.Additions,

submitqueue/extension/changeprovider/github/provider.go

@@ -44,7 +44,7 @@ func NewProvider(params Params) changeprovider.ChangeProvider {
 
 // Get retrieves change information from GitHub for the provided Change.
 // Returns one ChangeInfo per URI (one per PR in stacked changes).
-func (p *provider) Get(ctx context.Context, change entity.Change) (_ []changeprovider.ChangeInfo, retErr error) {
+func (p *provider) Get(ctx context.Context, change entity.Change) (_ []entity.ChangeInfo, retErr error) {
 	op := coremetrics.Begin(p.metricsScope, "get")
 	defer func() { op.Complete(retErr) }()
 
@@ -85,8 +85,8 @@ func (p *provider) Get(ctx context.Context, change entity.Change) (_ []changepro
 func (p *provider) fetchAllPRs(
 	ctx context.Context,
 	changeIDs []entitygithub.ChangeID,
-) ([]changeprovider.ChangeInfo, error) {
-	changeInfos := make([]changeprovider.ChangeInfo, 0, len(changeIDs))
+) ([]entity.ChangeInfo, error) {
+	changeInfos := make([]entity.ChangeInfo, 0, len(changeIDs))
 
 	for _, cid := range changeIDs {
 		prData, err := p.fetchPullRequest(ctx, cid)
@@ -109,7 +109,7 @@ func (p *provider) fetchAllPRs(
 			"org", cid.Org,
 			"repo", cid.Repo,
 			"pr", cid.PRNumber,
-			"files_count", len(changeInfo.ChangedFiles),
+			"files_count", len(changeInfo.Details.ChangedFiles),
 			"head_sha", prData.HeadRefOid,
 		)
 	}

submitqueue/extension/changeprovider/github/provider_test.go

@@ -115,7 +115,7 @@ func TestProvider_Get(t *testing.T) {
 			require.NoError(t, err)
 			require.Len(t, infos, 1)
 			assert.Equal(t, tt.uris[0], infos[0].URI)
-			assert.Len(t, infos[0].ChangedFiles, 2)
+			assert.Len(t, infos[0].Details.ChangedFiles, 2)
 		})
 	}
 }
@@ -154,7 +154,7 @@ func TestProvider_Get_Pagination(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, 2, callCount)
 	require.Len(t, infos, 1)
-	assert.Len(t, infos[0].ChangedFiles, 2)
+	assert.Len(t, infos[0].Details.ChangedFiles, 2)
 }
 
 func TestProvider_Get_MultiplePRs(t *testing.T) {

submitqueue/extension/changeprovider/mock/change_provider_mock.go

@@ -24,8 +24,8 @@ import (
 
 // ChangeStore manages per-URI claim records for in-flight land requests.
 // Each row records that a specific URI was claimed by a specific request, scoped to a queue.
-// The (Queue, URI, RequestID) triple is the immutable identity of a record. Metadata may
-// evolve over time.
+// The (Queue, URI, RequestID) triple is the immutable identity of a record. Details may
+// evolve over time as the change provider supplies information about the change.
 //
 // The interface is intentionally per-record / per-URI so that any backend (SQL, DynamoDB,
 // Bigtable, …) can implement it without needing batch-atomicity or multi-key query support.
@@ -48,4 +48,17 @@ type ChangeStore interface {
 	// state — callers that want to skip self filter by RequestID, and callers
 	// that want only live owners consult RequestStore for liveness.
 	GetByURI(ctx context.Context, queue string, uri string) ([]entity.ChangeRecord, error)
+
+	// Update conditionally replaces a record's mutable fields (Details, UpdatedAt)
+	// and sets its version to newVersion, guarded by oldVersion. The record carries
+	// the row to write: its (Queue, URI, RequestID) identify the row and its Details
+	// and UpdatedAt are the new values. The store performs a pure conditional write —
+	// it applies the record only when the persisted version equals oldVersion.
+	// Version arithmetic is owned by the caller, which passes both oldVersion (the
+	// where-clause guard) and newVersion (the value written to the version column);
+	// record.Version is not read by the store.
+	//
+	// Returns ErrVersionMismatch when no row matches the identity at oldVersion —
+	// i.e. a stale read, a concurrent update that won, or a missing record.
+	Update(ctx context.Context, record entity.ChangeRecord, oldVersion int32, newVersion int32) error
 }