Skip to content

chore: compile-check docs say an assembly a run leaves out keeps its changed-files baseline - #3351

Merged
hatayama merged 3 commits into
feature/compile-check-changed-files-checkfrom
docs/compile-check-unselected-assembly-keeps-its-record
Oct 16, 2026
Merged

hatayama merged 3 commits into
feature/compile-check-changed-files-checkfrom
docs/compile-check-unselected-assembly-keeps-its-record

Conversation

@hatayama

@hatayama hatayama commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • The compile-check docs now say correctly which runs drop an assembly's changed-files baseline, so a reader no longer expects a full compile where a changed-files check actually happens.
  • A test pins that an assembly a run leaves out of its plan keeps its outputs, result, and baseline.

User Impact

  • Before: the docs said a run that skips an assembly because nothing it compiles from changed removes its result and baseline, so its next body edit is compiled in full. On a large project, a body edit made after such a run was checked against the earlier baseline (a fast check, or DeclarationChanged), not compiled in full with no baseline, which contradicted the docs.
  • After: the docs say that an assembly the plan never selects keeps its result and baseline, and its next body edit is checked against them. A dependent the run picks up only because a reference changed loses them either way: compiled, it records no baseline; left out, because every such reference kept its public surface or changed only names it never writes, its result and baseline are removed. The next body edit of that dependent is compiled in full.

Changes

  • docs/compile-check.md: corrected the no baseline: no full compile of this assembly is on record entry and the matching bullet under "So the first body edit is compiled in full more often than it looks".
  • compile-check SKILL.md: the list of runs after which the first body edit is compiled in full no longer names every run that skipped the assembly, only one that picked it up because a reference changed. No generated copy of this skill is tracked, and the skill stays under the 8,000-byte cap.
  • ADR 0016, ADR 0017 and the limits section were checked; they describe the dependent case only and stay as they are.
  • fast_path_test.go: TestRunLeavingAnAssemblyOutKeepsItsBaseline records a baseline, runs a plan that leaves the assembly out (one subtest compiles nothing, the other compiles only another assembly), checks that every file the check keeps for the assembly hashes the same, and then checks that a body edit is fast-checked without csc.

No Go production code changes.

Verification

  • Mutation: a run that forgets the left-out assembly's record at its start makes both subtests fail, because the result and baseline files disappear. The test passes again once the mutation is reverted.
  • go test -race -count=1 ./internal/compilecheck/... ./internal/dispatcher/... in cli/dispatcher: ok
  • For each of the four Go modules: gofmt -l is empty, go vet exits 0, and golangci-lint run reports 0 issues.
  • go test ./... passes in cli/dispatcher and in cli/release-automation, including ./internal/architecture/.... In cli/common and cli/project-runner, one test each fails only inside the local sandbox: TestOSUnixMetadataReaderReportsRealOwnerAndMode and TestSendWithTransientConnectionRetryAbortsOnRefusedConnect. This PR does not touch either module.
  • check-skill-size and scripts/sync-tool-docs.sh --check: pass
  • scripts/check-code-complexity.sh and scripts/check-file-length.sh: exit 0

…baseline

An assembly whose own inputs did not change is not planned, and nothing in the
run touches what the check recorded for it. Pin that its outputs, result, and
baseline survive such a run, whether the run compiles nothing or only another
assembly, and that its next body edit is checked rather than compiled in full.
The changed-files check docs said a run that skips an assembly because nothing
it compiles from changed removes its result and baseline. Only a dependent the
run picked up because a reference changed, and then left out because every such
reference kept its surface or changed only names it never writes, loses them.
An assembly the plan never selects keeps both, so its next body edit is checked.
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: f8b9d1c4-bbd2-459f-a980-e4cc97936068
📥 Commits

Reviewing files that changed from the base of the PR and between 8de6940 and 5e9857d.

📒 Files selected for processing: 2
  • cli/dispatcher/internal/compilecheck/fast_path_test.go
  • docs/compile-check.md

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

📝 Walkthrough

The changes add a test for preserving an assembly’s outputs, result, and baseline when it is omitted from a run. The compile-check documentation clarifies how results and baselines are handled for omitted, blocked, and reference-triggered skipped assemblies.

Changes: Compile-check baseline handling

Layer / File(s) Summary
Baseline retention and fallback behavior
docs/compile-check.md, cli/dispatcher/internal/compilecheck/fast_path_test.go
The documentation distinguishes omitted assemblies from assemblies selected only because a reference changed and then skipped. The test checks that omitting Alpha preserves its outputs, result, and baseline, then invokes the fast-path check after an Alpha body edit.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~7 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 5e985

The documentation reflects the current compile-check behavior, and the new test covers retaining an omitted assembly’s state. No merge-blocking issue was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed 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 3 functions across 1 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly describes the main documentation change: an assembly left out of a run retains its changed-files baseline.
Description check Passed The description directly explains the documentation updates, added regression test, expected behavior, and verification results.
Full details: Docstring Coverage

Explanation

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 3 functions across 1 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 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

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

A dependent the run picks up only because a reference changed records no
baseline when it is compiled, and has its result and baseline removed when it
is left out, so the docs no longer say the latter is the only case. The
compile-check skill said a run that skipped the assembly makes its next body
edit compile in full; narrow that to such a dependent.
@hatayama
hatayama merged commit 22518a4 into feature/compile-check-changed-files-check Oct 16, 2026
3 of 4 checks passed
@hatayama
hatayama deleted the docs/compile-check-unselected-assembly-keeps-its-record branch October 16, 2026 12:26
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