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

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,76 @@
+// 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
+
+// Author represents the author of a change.
+type Author struct {
+	// Name is the display name of the author.
+	Name string `json:"name"`
+	// Email is the email address of the author.
+	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"`
+	// 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 {
+	// Author is the author of the change.
+	Author Author `json:"author"`
+	// 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 whole record is immutable: the (Queue, URI, RequestID) triple is its identity and the
+// Details (author, changed files, line counts) are captured once at claim time from the
+// change provider. There is no update path.
 type ChangeRecord struct {
 	// URI identifies the change (RFC 3986). Same scheme/format as entity.Change.URIs.
 	// Example: "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab".
@@ -29,29 +29,28 @@ 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). It is captured at claim time (the validate controller, after
+	// fetching from the change provider) and written once with the record — records are
+	// immutable, so Details is never updated after Create.
+	Details ChangeDetails `json:"details"`
 
-	// CreatedAt is the Unix milliseconds timestamp when this record was first created.
+	// CreatedAt is the Unix milliseconds timestamp when this record was created.
 	CreatedAt int64 `json:"created_at"`
 
-	// UpdatedAt is the Unix milliseconds timestamp when this record's Metadata was last updated.
-	// Equal to CreatedAt when the record has never been updated.
+	// UpdatedAt is the Unix milliseconds timestamp when this record was created. Records
+	// are immutable, so it always equals CreatedAt; retained for schema symmetry.
 	UpdatedAt int64 `json:"updated_at"`
 
-	// Version is the optimistic-locking counter for mutable fields (Metadata).
-	// 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.
+	// Version is the record version. Records are immutable, so it is always 1; retained
+	// for schema symmetry with the other stores.
 	Version int32 `json:"version"`
 }

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.Author, 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,32 +1,33 @@
 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{
+			Author: entity.Author{
+				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,
 			LinesDeleted: file.Deletions,
 		})

submitqueue/extension/changeprovider/github/graphql.go

@@ -34,7 +34,6 @@ query($owner: String!, $repo: String!, $prNumber: Int!, $filesCursor: String) {
           additions
           deletions
           changeType
-          patch
         }
       }
     }
@@ -98,7 +97,6 @@ type fileNode struct {
 	Additions  int    `json:"additions"`
 	Deletions  int    `json:"deletions"`
 	ChangeType string `json:"changeType"`
-	Patch      string `json:"patch"`
 }
 
 // buildGraphQLRequest builds a GraphQL request for fetching pull request data.

submitqueue/extension/changeprovider/github/graphql_test.go

@@ -77,7 +77,7 @@ func TestParseGraphQLResponse(t *testing.T) {
 							"files": {
 								"totalCount": 1,
 								"pageInfo": {"endCursor": "cur1", "hasNextPage": false},
-								"nodes": [{"path": "main.go", "additions": 10, "deletions": 2, "changeType": "MODIFIED", "patch": "diff content"}]
+								"nodes": [{"path": "main.go", "additions": 10, "deletions": 2, "changeType": "MODIFIED"}]
 							}
 						}
 					}
@@ -90,7 +90,7 @@ func TestParseGraphQLResponse(t *testing.T) {
 				Files: filesData{
 					TotalCount: 1,
 					PageInfo:   pageInfo{EndCursor: "cur1", HasNextPage: false},
-					Nodes:      []fileNode{{Path: "main.go", Additions: 10, Deletions: 2, ChangeType: "MODIFIED", Patch: "diff content"}},
+					Nodes:      []fileNode{{Path: "main.go", Additions: 10, Deletions: 2, ChangeType: "MODIFIED"}},
 				},
 			},
 		},

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/storage/change_store.go

@@ -24,19 +24,19 @@ 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, and a record's
+// Details are captured at claim time and never updated — records are immutable.
 //
 // 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.
 // Callers loop when they have multiple URIs to claim or check; the typical request has a
 // small number of URIs (a single PR or a short stack), so the loop overhead is negligible.
 type ChangeStore interface {
-	// Create persists a single ChangeRecord. A primary-key conflict on
-	// (Queue, URI, RequestID) is silently ignored, which makes the call
-	// idempotent under queue redeliveries of the same request. Records belonging
-	// to different requests do not conflict on the PK — cross-request overlap
-	// is detected by GetByURI, not by Create.
+	// Create persists a single ChangeRecord (identity + Details) in one write. A
+	// primary-key conflict on (Queue, URI, RequestID) is silently ignored, which makes
+	// the call idempotent under queue redeliveries of the same request (first write
+	// wins). Records belonging to different requests do not conflict on the PK —
+	// cross-request overlap is detected by GetByURI, not by Create.
 	Create(ctx context.Context, record entity.ChangeRecord) error
 
 	// GetByURI returns every ChangeRecord for the given (queue, uri). Multiple