feat: compile-check replays a changed-files check whose inputs did not change

Packages/src/Editor/CliOnlyTools~/CompileCheck/Skill/SKILL.md

@@ -82,7 +82,7 @@ A JSON payload:
 - `ReusedAssemblies`: the assemblies reported from the previous run's recorded result, without compiling
 - `BlockedAssemblies`: the assemblies not compiled because an assembly they reference has errors, as Unity's own build would stop there; fix those errors and run again
 - `FastCheckedAssemblies`: the assemblies checked on their changed files only
-- `FastChecks`: per fast-checked assembly, `ChangedFiles`, `UncheckedCodes` (rules only `--full` runs), and `GeneratorsNotRerun`
+- `FastChecks`: per fast-checked assembly, `ChangedFiles`, `UncheckedCodes` (rules only `--full` runs), `GeneratorsNotRerun`, and `Reused` (the previous run's check of the same edit, reported again)
 - `FastPathFallbacks`: `Assembly` and `Reason` for each assembly that was compiled in full instead
 - `FastPathBaselineNotes`: `Assembly` and `Reason` for each successful full compile that left no baseline for the next changed-files check
 - `SkippedAssemblies`: how many assemblies were left out: nothing they compile from changed, or every assembly they reference kept the same public surface

cli/dispatcher/internal/compilecheck/compiler.go

@@ -62,6 +62,10 @@ type UnitResult struct {
 	// FastChecked marks a result that came from checking only the changed files against the last
 	// successful full compile, instead of compiling the assembly.
 	FastChecked bool
+	// FastCheckReused marks a fast check this run replayed from the previous run's check of the same
+	// changed files, because the inputs, the baseline and the worker build were all as that check saw
+	// them.
+	FastCheckReused bool
 	// FastCheckChangedFiles lists the files a fast check checked, spelled as the response file does.
 	FastCheckChangedFiles []string
 	// UncheckedCodes lists the diagnostic IDs a fast check could not run; only a full compile does.

cli/dispatcher/internal/compilecheck/fast_path.go

@@ -108,14 +108,16 @@ func (runner *fastPathRunner) Try(
 		return fallBack(fmt.Sprintf("too many changed files (%d, limit %d)", len(changed), fastPathMaximumChangedFiles))
 	}
 
-	return runner.check(ctx, c, plan, unit, baseline, changed)
+	return runner.check(ctx, c, plan, unit, baseline, changed, keyResult.Key)
 }
 
 // check runs the worker over the changed files and merges what it found with the diagnostics the
-// last full compile reported for the files that did not change.
+// last full compile reported for the files that did not change. When the previous run checked the
+// same inputs against the same baseline with the same worker build, it reports that check instead.
+// key is the unit's input key, read before the check.
 func (runner *fastPathRunner) check(
 	ctx context.Context, c Compiler, plan BuildPlan, unit CompileUnit,
-	baseline resultstore.Baseline[Diagnostic], changed []string,
+	baseline resultstore.Baseline[Diagnostic], changed []string, key string,
 ) fastPathAttempt {
 	startedAt := time.Now()
 	worker, err := runner.Worker.Ensure(ctx)
@@ -130,6 +132,13 @@ func (runner *fastPathRunner) check(
 		return fallBack("baseline was fingerprinted by another worker build")
 	}
 
+	// Why the record is looked up only here: everything above is what a check needs, so a check is
+	// replayed only where it would have run, for the same inputs it ran on.
+	records := c.records(plan)
+	if recorded, found := records.ReusableFastCheck(unit.Assembly.AssemblyName, key, baseline, worker.Key); found {
+		return fastPathAttempt{Used: true, Result: fastCheckedUnitResult(unit, recorded, time.Since(startedAt), true)}
+	}
+
 	responseFilePath, err := writeRewrittenResponseFile(
 		c.ProjectRoot, plan, unit, unit.Assembly.AssemblyName+fastCheckResponseFileSuffix, nil)
 	if err != nil {
@@ -149,17 +158,39 @@ func (runner *fastPathRunner) check(
 	}
 
 	diagnostics := mergeDiagnostics(c.ProjectRoot, unit, baseline.Diagnostics, response.Diagnostics, changed)
+	checked := resultstore.FastCheckResult[Diagnostic]{
+		Succeeded:          !containsError(diagnostics),
+		Diagnostics:        diagnostics,
+		ChangedFiles:       changed,
+		UncheckedCodes:     response.UncheckedCodes,
+		GeneratorsNotRerun: response.GeneratorsNotRerun,
+	}
+	// Why the key is read again before recording: the worker reads the sources while it checks, so an
+	// edit made during the check would otherwise be recorded under the key of the bytes before it.
+	if c.keyStillHolds(plan, unit, key) {
+		if err := records.RecordFastCheck(unit.Assembly.AssemblyName, key, baseline, worker.Key, checked); err != nil {
+			return fastPathAttempt{Err: err}
+		}
+	}
+
+	return fastPathAttempt{Used: true, Result: fastCheckedUnitResult(unit, checked, time.Since(startedAt), false)}
+}
 
-	return fastPathAttempt{Used: true, Result: UnitResult{
+// fastCheckedUnitResult turns a check of the changed files into the unit's result.
+func fastCheckedUnitResult(
+	unit CompileUnit, checked resultstore.FastCheckResult[Diagnostic], duration time.Duration, reused bool,
+) UnitResult {
+	return UnitResult{
 		Assembly:              unit.Assembly.AssemblyName,
-		Diagnostics:           diagnostics,
-		Succeeded:             !containsError(diagnostics),
-		Duration:              time.Since(startedAt),
+		Diagnostics:           checked.Diagnostics,
+		Succeeded:             checked.Succeeded,
+		Duration:              duration,
 		FastChecked:           true,
-		FastCheckChangedFiles: changed,
-		UncheckedCodes:        response.UncheckedCodes,
-		GeneratorsNotRerun:    response.GeneratorsNotRerun,
-	}}
+		FastCheckReused:       reused,
+		FastCheckChangedFiles: checked.ChangedFiles,
+		UncheckedCodes:        checked.UncheckedCodes,
+		GeneratorsNotRerun:    checked.GeneratorsNotRerun,
+	}
 }
 
 // projectRelativeReason rewrites the absolute paths the worker put in a reason the way the output

cli/dispatcher/internal/compilecheck/fast_path_fixture_test.go

@@ -31,11 +31,13 @@ type fakeFastCheckWorker struct {
 	reason         string
 	diagnostics    []fastcheck.WorkerDiagnostic
 	uncheckedCodes []string
-	onCheck        func()
-	ensures        int
-	fingerprints   int
-	checks         int
-	lastCheck      fastcheck.CheckRequest
+	// generatorsNotRerun is what Check reports about the source generators.
+	generatorsNotRerun bool
+	onCheck            func()
+	ensures            int
+	fingerprints       int
+	checks             int
+	lastCheck          fastcheck.CheckRequest
 }
 
 func newFakeFastCheckWorker() *fakeFastCheckWorker {
@@ -97,10 +99,11 @@ func (worker *fakeFastCheckWorker) Check(
 	}
 
 	return fastcheck.CheckResponse{
-		Outcome:        worker.outcome,
-		Reason:         worker.reason,
-		Diagnostics:    worker.diagnostics,
-		UncheckedCodes: worker.uncheckedCodes,
+		Outcome:            worker.outcome,
+		Reason:             worker.reason,
+		Diagnostics:        worker.diagnostics,
+		UncheckedCodes:     worker.uncheckedCodes,
+		GeneratorsNotRerun: worker.generatorsNotRerun,
 	}, nil
 }
 
@@ -145,7 +148,12 @@ func (project *fastPathProject) sourceName(index int) string {
 // editBody changes the body part of Alpha's source at index, keeping its declaration.
 func (project *fastPathProject) editBody(t *testing.T, index int, body string) {
 	t.Helper()
-	project.writeFile(t, project.sourceName(index), fmt.Sprintf("class File%03d %s %s", index, fakeDeclarationSeparator, body))
+	project.writeFile(t, project.sourceName(index), bodyContent(index, body))
+}
+
+// bodyContent is what Alpha's source at index holds once its body is body.
+func bodyContent(index int, body string) string {
+	return fmt.Sprintf("class File%03d %s %s", index, fakeDeclarationSeparator, body)
 }
 
 // editDeclaration changes the declaration part of Alpha's source at index.

cli/dispatcher/internal/compilecheck/fast_path_replay_test.go

@@ -0,0 +1,196 @@
+package compilecheck
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"reflect"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/fastcheck"
+)
+
+// newEditedFastPathProject records a baseline and edits the body of Alpha's first source, with the
+// worker reporting an error in it, an unchecked code, and generators it did not rerun - every field
+// a check reports holds something other than its zero value.
+func newEditedFastPathProject(t *testing.T) *fastPathProject {
+	t.Helper()
+	project := newFastPathProject(t, fastPathMinimumSources)
+	project.recordBaseline(t)
+	project.editBody(t, 0, "int x = \"s\";")
+	project.worker.diagnostics = []fastcheck.WorkerDiagnostic{{
+		File: project.absoluteSource(0), Line: 1, Column: 20, Severity: severityError, Code: "CS0029", Message: "cannot convert",
+	}}
+	project.worker.uncheckedCodes = []string{"CS0169"}
+	project.worker.generatorsNotRerun = true
+
+	return project
+}
+
+// runFastChecked runs the plan and fails the test unless Alpha was fast-checked without csc.
+func (project *fastPathProject) runFastChecked(t *testing.T) UnitResult {
+	t.Helper()
+	recorder := newCachedSchedulerRecorder("Alpha", "Beta")
+	result := resultOf(t, project.run(t, recorder, false).Units, "Alpha")
+	if !result.FastChecked || result.Reused || recorder.runs["Alpha"] != 0 {
+		t.Fatalf("expected Alpha to be fast-checked without csc, got %+v after %d runs", result, recorder.runs["Alpha"])
+	}
+
+	return result
+}
+
+// Verifies a second run over the same edit reports the first run's check without calling the worker,
+// and reports exactly what that check reported.
+func TestFastCheckReplaysTheLastCheckOfTheSameEdit(t *testing.T) {
+	project := newEditedFastPathProject(t)
+	first := project.runFastChecked(t)
+	if first.FastCheckReused || project.worker.checks != 1 {
+		t.Fatalf("expected the first run to check, got %+v after %d checks", first, project.worker.checks)
+	}
+
+	second := project.runFastChecked(t)
+
+	if !second.FastCheckReused || project.worker.checks != 1 {
+		t.Fatalf("expected the second run to replay the check, got %+v after %d checks", second, project.worker.checks)
+	}
+	if first.Succeeded || second.Succeeded {
+		t.Errorf("expected both runs to report the error, got %t then %t", first.Succeeded, second.Succeeded)
+	}
+	if !hasDiagnosticCode(second.Diagnostics, "CS0029") || !reflect.DeepEqual(second.Diagnostics, first.Diagnostics) {
+		t.Errorf("diagnostics = %+v, want the first run's %+v", second.Diagnostics, first.Diagnostics)
+	}
+	assertStrings(t, "changed files", second.FastCheckChangedFiles, first.FastCheckChangedFiles)
+	assertStrings(t, "unchecked codes", second.UncheckedCodes, []string{"CS0169"})
+	if !second.GeneratorsNotRerun {
+		t.Error("expected the replay to keep the generators the check did not rerun")
+	}
+}
+
+// Verifies a different edit is checked again, and that check then replaces the recorded one.
+func TestFastCheckChecksAgainAfterAnotherEdit(t *testing.T) {
+	project := newEditedFastPathProject(t)
+	project.runFastChecked(t)
+	project.editBody(t, 0, "int y = \"s\";")
+
+	edited := project.runFastChecked(t)
+	if edited.FastCheckReused || project.worker.checks != 2 {
+		t.Fatalf("expected another edit to be checked, got %+v after %d checks", edited, project.worker.checks)
+	}
+
+	again := project.runFastChecked(t)
+	if !again.FastCheckReused || project.worker.checks != 2 {
+		t.Errorf("expected the new check to be replayed, got %+v after %d checks", again, project.worker.checks)
+	}
+}
+
+// Verifies a check is not replayed once the worker build changed.
+func TestFastCheckDoesNotReplayAfterTheWorkerBuildChanged(t *testing.T) {
+	project := newEditedFastPathProject(t)
+	project.runFastChecked(t)
+	project.worker.key = "worker-b"
+	recorder := newCachedSchedulerRecorder("Alpha", "Beta")
+
+	outcome := project.run(t, recorder, false)
+
+	assertFallback(t, outcome, recorder, "baseline was fingerprinted by another worker build")
+}
+
+// Verifies a check is not replayed once the generator output it read is gone, so the replay is
+// looked up only after everything the check needs has been confirmed.
+func TestFastCheckDoesNotReplayWhenTheGeneratorOutputIsGone(t *testing.T) {
+	project := newFastPathProject(t, fastPathMinimumSources)
+	generatedPath := project.recordBaselineWithGeneratorOutput(t)
+	project.editBody(t, 0, "edited")
+	project.runFastChecked(t)
+	removeGeneratorOutput(t, generatedPath)
+	recorder := newCachedSchedulerRecorder("Alpha", "Beta")
+
+	outcome := project.run(t, recorder, false)
+
+	assertFallback(t, outcome, recorder, "generator output of the baseline is gone")
+}
+
+// Verifies a full compile leaves no record of the check that came before it: the files the check
+// keeps for Alpha are the ones the first full compile left.
+func TestFullCompileLeavesNoRecordOfAnEarlierFastCheck(t *testing.T) {
+	project := newFastPathProject(t, fastPathMinimumSources)
+	project.recordBaseline(t)
+	recorded := snapshotPaths(project.outputSnapshot(t))
+	project.editBody(t, 0, "edited")
+	project.runFastChecked(t)
+
+	project.run(t, newCachedSchedulerRecorder("Alpha", "Beta"), true)
+
+	assertStrings(t, "files kept for Alpha", snapshotPaths(project.outputSnapshot(t)), recorded)
+}
+
+// Verifies a check whose result cannot be recorded fails the run, as a compile whose result cannot
+// be recorded does.
+func TestFastCheckFailsTheRunWhenItsResultCannotBeRecorded(t *testing.T) {
+	project := newFastPathProject(t, fastPathMinimumSources)
+	project.recordBaseline(t)
+	project.editBody(t, 0, "edited")
+	var removeErr error
+	project.worker.onCheck = func() { removeErr = os.RemoveAll(project.outputDirectory()) }
+
+	_, err := project.runWithContext(context.Background(), newCachedSchedulerRecorder("Alpha", "Beta"), false)
+
+	if removeErr != nil {
+		t.Fatalf("failed to remove the output directory: %v", removeErr)
+	}
+	want := "failed to write " + project.outputDirectory() + string(filepath.Separator)
+	if err == nil || !strings.HasPrefix(err.Error(), want) {
+		t.Errorf("err = %v, want it to start with %q", err, want)
+	}
+}
+
+// Verifies a check of sources edited while it ran is not recorded under the key of the bytes before
+// the edit, so putting those bytes back checks them again rather than replaying the edited file.
+func TestFastCheckDoesNotRecordACheckOfSourcesEditedDuringIt(t *testing.T) {
+	project := newFastPathProject(t, fastPathMinimumSources)
+	project.recordBaseline(t)
+	project.editBody(t, 0, "X")
+	var editErr error
+	project.worker.onCheck = func() {
+		editErr = os.WriteFile(project.absoluteSource(0), []byte(bodyContent(0, "Y")), 0o600)
+	}
+	project.runFastChecked(t)
+	if editErr != nil {
+		t.Fatalf("failed to edit the source during the check: %v", editErr)
+	}
+	if project.worker.checks != 1 {
+		t.Fatalf("expected one check, got %d", project.worker.checks)
+	}
+	project.worker.onCheck = nil
+	project.editBody(t, 0, "X")
+
+	result := project.runFastChecked(t)
+
+	if result.FastCheckReused || project.worker.checks != 2 {
+		t.Errorf("expected the reverted source to be checked again, got %+v after %d checks", result, project.worker.checks)
+	}
+}
+
+// hasDiagnosticCode reports whether any of the diagnostics has the code.
+func hasDiagnosticCode(diagnostics []Diagnostic, code string) bool {
+	for _, diagnostic := range diagnostics {
+		if diagnostic.Code == code {
+			return true
+		}
+	}
+
+	return false
+}
+
+// snapshotPaths lists the paths of a snapshot in order.
+func snapshotPaths(snapshot map[string]string) []string {
+	paths := make([]string, 0, len(snapshot))
+	for path := range snapshot {
+		paths = append(paths, path)
+	}
+	slices.Sort(paths)
+
+	return paths
+}

cli/dispatcher/internal/compilecheck/fast_path_test.go

@@ -331,8 +331,8 @@ func TestWorkerKeyMismatchFallsBack(t *testing.T) {
 }
 
 // Verifies the clean changed-files check: Alpha is not compiled, its outputs, manifest, and baseline
-// stay as they were, the diagnostics of unchanged files are carried over, and a dependent whose
-// reference kept its surface is still skipped.
+// stay as they were with only the record of the check itself added, the diagnostics of unchanged
+// files are carried over, and a dependent whose reference kept its surface is still skipped.
 func TestFastPathCleanResultKeepsBaselineFiles(t *testing.T) {
 	project := newFastPathProject(t, fastPathMinimumSources)
 	markSelectedAsDependent(t, project.plan, "Beta")
@@ -356,8 +356,14 @@ func TestFastPathCleanResultKeepsBaselineFiles(t *testing.T) {
 	if len(result.Diagnostics) != 1 || result.Diagnostics[0].File != "Assets/Alpha/File001.cs" {
 		t.Errorf("expected the warning of the unchanged file to be carried over, got %+v", result.Diagnostics)
 	}
-	if after := project.outputSnapshot(t); !reflect.DeepEqual(after, before) {
-		t.Errorf("expected the outputs, manifest, and baseline to stay, got %v then %v", before, after)
+	after := project.outputSnapshot(t)
+	for path, hash := range before {
+		if after[path] != hash {
+			t.Errorf("expected %s to stay as it was, got %q then %q", path, hash, after[path])
+		}
+	}
+	if len(after) != len(before)+1 {
+		t.Errorf("expected only the record of the check to be added, got %v then %v", before, after)
 	}
 	if recorder.runs["Beta"] != 0 || outcome.Skipped != 1 {
 		t.Errorf("expected Beta to be skipped, got %d runs and %d skipped", recorder.runs["Beta"], outcome.Skipped)

cli/dispatcher/internal/compilecheck/input_key.go

@@ -56,6 +56,13 @@ func computeInputKey(
 	return inputKeyResult{Key: key, SourceHashes: sourceHashes}, nil
 }
 
+// keyStillHolds reports whether the unit's inputs still give the key the run started from.
+func (c Compiler) keyStillHolds(plan BuildPlan, unit CompileUnit, key string) bool {
+	rechecked, err := computeInputKey(c.ProjectRoot, c.Paths, plan, unit)
+
+	return err == nil && rechecked.Key == key
+}
+
 // nonSourceIdentity is the non-source key of a unit together with a digest of each kind of input it
 // covers, so a fallback can say which kind changed.
 type nonSourceIdentity struct {