refactor: move compile-check's records into a resultstore package

[hatayama]

Summary

• compile-check's records now live in their own package, resultstore. Everything else in compile-check reaches them only through one store type, and the compiler enforces that.
• No behaviour changes: the output JSON, the fallback reasons and their order, and the names and bytes of every record are the same.

User Impact

• None directly. The next change, reusing the changed-files check's result, adds a fourth record. With this package it is one more file in the store plus a load and a record method, and Forget stays the single place that removes every record.

Changes

• The store's API:

  func Open[D any](projectRoot, outputDir string) Store[D]
  func (s Store[D]) Reusable(assembly, key string) (CompileResult[D], bool)
  func (s Store[D]) RecordCompile(assembly, key string, result CompileResult[D], outputs []string) error
  func (s Store[D]) LoadBaseline(assembly string) (Baseline[D], string)
  func (s Store[D]) GeneratorOutputUnchanged(baseline Baseline[D]) bool
  func (s Store[D]) RecordBaseline(assembly string, inputs BaselineInputs) error
  func (s Store[D]) GeneratedDirectory(assembly string) string
  func (s Store[D]) Forget(assembly string) error

  The record formats, file names, version checks, atomic writes and hashing are unexported. The package comment states the contract: records are written through a temporary file, a baseline is handed out only while its compile result is still on disk, Forget removes every kind of record, and the reasons LoadBaseline gives.
• Why the diagnostic type is a type parameter D:
  • The store never reads a diagnostic, and the type parameter lets the compiler guarantee that.
  • Diagnostics are not copied field by field at the boundary.
  • No extra package is needed for a single type.
  • compile-check uses one instantiation, resultstore.Open[Diagnostic].
• The non-source key crosses the boundary as resultstore.KeyDigest. It has the same fields as compile-check's own nonSourceIdentity, so the two convert into each other with a plain type conversion and nothing is copied. compile-check keeps owning how the key is computed.
• The store keeps private copies of hashFile (12 lines) and the project-relative path helper rather than sharing them through another package. resultstore imports only the standard library.
• The store's own tests use a stand-in diagnostic type with the same field order, so the golden record bytes are unchanged. A golden test in compile-check pins the JSON form of the real diagnostic type, because the store records diagnostics exactly as it is given them. Swapping two of its fields, or adding a JSON tag, fails that test.
• New tests at the store's boundary (separate commit). Coverage is measured per package, so the baseline paths that compile-check's tests used to exercise would otherwise count for nothing:
  • a recorded baseline comes back as recorded;
  • a changed, removed or added generated file stops the generator output matching;
  • a compile result that is gone, unreadable, of another key, failed, or whose output changed stops the baseline being handed out;
  • each reason a baseline is missing;
  • a failed baseline write.

Verification

• go list -deps ./internal/compilecheck/resultstore | grep -c 'internal/compilecheck$': 0. Its direct imports are all standard library.
• git grep -P over cli/dispatcher/internal/compilecheck/*.go (not descending into subpackages) for record internals (struct names, constants, paths, internal functions, record file names): no matches.
• No expected reason, byte or count changed in the moved tests. The tests outside the store only switched to the public names.
• Mutation checks, each restored afterwards:
  • swapping two fields of either record struct fails its golden test;
  • swapping the fields of KeyDigest stops the type conversion compiling and fails the baseline golden test;
  • checking the manifest before the baseline's version fails the order test;
  • moving the generator output check before the non-source checks fails the order test;
  • dropping the failed-compile check from LoadBaseline fails a parent test and a new store test;
  • dropping the file-count check from the generator output match fails the new store test.
• cli/dispatcher:
  • golangci-lint fmt --diff is empty; go vet ./... passes; golangci-lint run and the complexity config both report 0 issues.
  • go test ./... and go test -race ./internal/compilecheck/... (including resultstore) pass.
  • Coverage with the CI exclusions: 94.25% (baseline 94.2). It was 93.74% before the boundary tests.
• cli/release-automation: go test ./... passes. The file-length check passes.
• Out of scope: keeping the previous baseline after a failed compile is still awaiting a decision. If it is done later, it only touches Forget and RecordCompile.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: c08958cd-49db-4042-bb5d-427b7f72862f

📥 Commits

Reviewing files that changed from the base of the PR and between 0b87dda and cd104de.

📒 Files selected for processing: 20

• cli/dispatcher/internal/compilecheck/baseline_record.go
• cli/dispatcher/internal/compilecheck/baseline_record_test.go
• cli/dispatcher/internal/compilecheck/compiler.go
• cli/dispatcher/internal/compilecheck/diagnostic_format_test.go
• cli/dispatcher/internal/compilecheck/fast_path.go
• cli/dispatcher/internal/compilecheck/fast_path_fixture_test.go
• cli/dispatcher/internal/compilecheck/fast_path_order_test.go
• cli/dispatcher/internal/compilecheck/fast_path_test.go
• cli/dispatcher/internal/compilecheck/record_store.go
• cli/dispatcher/internal/compilecheck/result_cache.go
• cli/dispatcher/internal/compilecheck/result_cache_write_test.go
• cli/dispatcher/internal/compilecheck/resultstore/baseline.go
• cli/dispatcher/internal/compilecheck/resultstore/baseline_contract_test.go
• cli/dispatcher/internal/compilecheck/resultstore/baseline_test.go
• cli/dispatcher/internal/compilecheck/resultstore/compile_result.go
• cli/dispatcher/internal/compilecheck/resultstore/compile_result_test.go
• cli/dispatcher/internal/compilecheck/resultstore/fixture_test.go
• cli/dispatcher/internal/compilecheck/resultstore/record_format_test.go
• cli/dispatcher/internal/compilecheck/resultstore/store.go
• cli/dispatcher/internal/compilecheck/scheduler_cache_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7c9e5282-bfe7-4654-b381-e74677055dd4

📥 Commits

Reviewing files that changed from the base of the PR and between 462f57d and 0b87dda.

📒 Files selected for processing: 19

• cli/dispatcher/internal/compilecheck/baseline_record.go
• cli/dispatcher/internal/compilecheck/baseline_record_test.go
• cli/dispatcher/internal/compilecheck/compiler.go
• cli/dispatcher/internal/compilecheck/fast_path.go
• cli/dispatcher/internal/compilecheck/fast_path_fixture_test.go
• cli/dispatcher/internal/compilecheck/fast_path_order_test.go
• cli/dispatcher/internal/compilecheck/fast_path_test.go
• cli/dispatcher/internal/compilecheck/record_store.go
• cli/dispatcher/internal/compilecheck/result_cache.go
• cli/dispatcher/internal/compilecheck/result_cache_write_test.go
• cli/dispatcher/internal/compilecheck/resultstore/baseline.go
• cli/dispatcher/internal/compilecheck/resultstore/baseline_contract_test.go
• cli/dispatcher/internal/compilecheck/resultstore/baseline_test.go
• cli/dispatcher/internal/compilecheck/resultstore/compile_result.go
• cli/dispatcher/internal/compilecheck/resultstore/compile_result_test.go
• cli/dispatcher/internal/compilecheck/resultstore/fixture_test.go
• cli/dispatcher/internal/compilecheck/resultstore/record_format_test.go
• cli/dispatcher/internal/compilecheck/resultstore/store.go
• cli/dispatcher/internal/compilecheck/scheduler_cache_test.go

💤 Files with no reviewable changes: 1

• cli/dispatcher/internal/compilecheck/record_store.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Compile-check persistence moves into the generic resultstore package. It exposes compile-result and baseline APIs, retains manifest validation and output hashing, and provides atomic writes and assembly cleanup. Compile-check cache, baseline, compiler, and test code now use the exported store API.

Changes: Compile-check result storage

Layer / File(s)	Summary
Store and compile-result manifests cli/dispatcher/internal/compilecheck/record_store.go, cli/dispatcher/internal/compilecheck/resultstore/store.go, cli/dispatcher/internal/compilecheck/resultstore/compile_result.go, cli/dispatcher/internal/compilecheck/resultstore/compile_result_test.go, cli/dispatcher/internal/compilecheck/resultstore/fixture_test.go	Compile-result persistence, reuse, output hashing, atomic writes, and assembly cleanup now reside in resultstore. Tests cover result reuse and recording, including validation and error cases.
Baseline records and validation cli/dispatcher/internal/compilecheck/resultstore/baseline.go, cli/dispatcher/internal/compilecheck/resultstore/baseline_contract_test.go, cli/dispatcher/internal/compilecheck/resultstore/baseline_test.go, cli/dispatcher/internal/compilecheck/resultstore/record_format_test.go	Baseline data types and operations are exposed on Store[D]. Tests cover baseline loading, recording, generator-output comparison, and failure explanations.
Compile-check integration cli/dispatcher/internal/compilecheck/baseline_record.go, cli/dispatcher/internal/compilecheck/compiler.go, cli/dispatcher/internal/compilecheck/fast_path.go, cli/dispatcher/internal/compilecheck/result_cache.go, cli/dispatcher/internal/compilecheck/*_test.go	Compile-check cache, compiler, baseline, and fast-path code now use resultstore types and methods. Related tests use the exported APIs.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 0b87d

Existing compile results and baselines remain compatible with the new store. No actionable merge risk was identified beyond normal checks.

Security architecture risk: ⚪ Minimal · up to 0b87d

The refactor preserves existing access, stored formats, validation and cleanup behavior. No new security-sensitive capability or material security regression was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed operations retain the existing local compiler process's filesystem scope: project inputs, assembly outputs, cached records and generated files. The new package uses the same caller-supplied identities and paths; the inspected migration does not expand that authority into another service or privilege domain.

Trust Boundaries and Controls

• observed — The persistence boundary retains its existing consistency controls: version and key matching, output hashing, successful-compile correlation for baselines, and generated-output validation before fast-path use. These controls were not weakened by exporting the storage methods.

Resilience and Maintainability Implications

• observed — New compiles still remove prior outputs and records before execution, and stopped invocations return without recording a result. Record replacement remains atomic per file through temporary-file writes and rename. Cleanup and write errors remain observable. There is no new multi-record transaction or output lock; partial cleanup and concurrent-writer limitations predate this PR.

Pre-merge checks: 5

Passed checks: 5 passed

Check name	Status	Explanation
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 18 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: moving compile-check records into a dedicated resultstore package.
Description check		The description directly explains the resultstore refactor, API changes, preserved behavior, tests, verification, and out-of-scope work.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}