perf: compile-check reads the names of the plan only when a run compiles or needs them

[hatayama]

Summary

• compile-check no longer reads the names of the assemblies in the plan on runs that never use them. A run that compiles nothing and leaves every dependent out because its references kept their surface now finishes without that read.
• Which assemblies compile or are skipped, and the diagnostics, stay the same.

User Impact

The name index from #3348 read the sources of every assembly with references in the plan at the start of every run, and every run waited for that read before returning. On a large project where one assembly is referenced directly by 67 others, a no-change run of 0.83 s spent 0.33 to 0.49 s waiting for the read. In alternating pairs against the build before #3348, a no-change run went from 0.25 s to 0.58 s, and the whole difference was the read. Runs that only replay recorded results now skip the read, so they should be back near their earlier time. A run that compiles, or has the worker check changed files, still reads the names, but it does so while that work runs.

Changes

• The saved generator output of those assemblies is still read before anything compiles, because a compile removes and rewrites it.
• The sources are read from whichever comes first:
  • the first compiler work the run does: a hook on the compiler, called right before csc compiles an assembly or the worker checks its changed files. It is never called for a result replayed from its record, or for a check replayed from the last check of the same edit;
  • the first verdict that needs names, for example a named change replayed from a record.
• If neither happens, the sources are never read. The read starts at most once per run.
• At every exit, the run still waits for a read it started, so the read never outlives the run.
• The reported name-index duration is the time each read took, measured from its own start, so the time a run spends before it asks for the sources is not counted.
• The VibeLog name_index entry gains sources_read. docs/vibe-logs.md and docs/compile-check.md describe when the names are read.

Verification

• New tests:
  • TestCompileUnitsReadsNoSourcesWhenNothingCompilesAndEveryVerdictIsKept: a replayed run with kept verdicts never touches a dependent's source. The source points at a missing file, so any read would count as unreadable.
  • TestCompileUnitsReadsTheSourcesWhenAReplayedVerdictNamesAChange
  • TestCompileUnitsReadsTheSourcesOnceAUnitCompilesInFull
  • TestReuseOrCompileCallsTheCompilerWorkHookOnlyBeforeACompileOrACheck: covers full compile, replay, changed-files check, and replayed check.
  • TestNameIndexReportCountsOnlyTheTimeSpentReading: the one timing assertion. It waits 300 ms before asking for the sources and expects under 150 ms.
  • TestNameIndexWaitReturnsAtOnceWhenTheSourcesWereNeverAsked: the source is a FIFO that no writer ever opens.
  • A VibeLog case with sources_read: false.
• Updated tests: TestNameIndexReadsGeneratedCodeBeforeStartReturns now starts the source read only after the generated directory has been removed.
• Mutations, each tried on the committed code and then reverted:

Mutation	Result
start also starts reading the sources (the old behaviour)	...ReadsNoSourcesWhenNothingCompiles... fails (Unreadable: 1, SourcesRead: true); ...WaitReturnsAtOnce... fails (held 5 s)
lookup does not start the read	...ReadsTheSourcesWhenAReplayedVerdictNamesAChange hangs on the wait, and the run with -timeout 30s fails with a timeout
hook called at the top of reuseOrCompile	the full-compile, replayed and changed-files-check cases fail (hook called twice, or for a replay)
hook not called before the worker check	the changed-files-check case fails ([check])
hook called before the replayed-check lookup	the replayed-check case fails ([hook])
source read timed from start	the duration test fails (301 ms)
hook called after csc instead of before	the full-compile case fails ([compile hook])
hook never wired in the scheduler	...ReadsTheSourcesOnceAUnitCompilesInFull fails

• Commands, per module (common, dispatcher, project-runner, release-automation):
  • gofmt -l, go vet, golangci-lint run and go test ./...;
  • go test ./internal/architecture/... in release-automation;
  • go test -race -count=1 ./internal/compilecheck/... ./internal/dispatcher/... in dispatcher;
  • scripts/check-code-complexity.sh and scripts/check-file-length.sh.
• Results: all four modules have no gofmt output and pass go vet, and golangci-lint reports 0 issues in each. go test passes in dispatcher and release-automation, including architecture. The race run, complexity and file length all pass. In common and project-runner one test each fails; both tests are unchanged and fail only inside the local sandbox, which blocks file ownership reads and connect. name_index.go is 427 lines.

[coderabbitai]

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: 2c50339b-4789-440f-9d50-327db7632aa8

📥 Commits

Reviewing files that changed from the base of the PR and between 9452a07 and 90c096e.

📒 Files selected for processing: 11

• cli/dispatcher/internal/compilecheck/compiler.go
• cli/dispatcher/internal/compilecheck/name_index.go
• cli/dispatcher/internal/compilecheck/name_index_fifo_test.go
• cli/dispatcher/internal/compilecheck/name_index_test.go
• cli/dispatcher/internal/compilecheck/result_cache.go
• cli/dispatcher/internal/compilecheck/result_cache_full_compile_test.go
• cli/dispatcher/internal/compilecheck/scheduler_names_test.go
• cli/dispatcher/internal/dispatcher/compile_check_vibe_log.go
• cli/dispatcher/internal/dispatcher/compile_check_vibe_log_test.go
• docs/compile-check.md
• docs/vibe-logs.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

Compile-check now reads generator output at index startup and defers assembly source indexing until a full compile or a name-based lookup requires it. The name-index report and logs record whether source reading began.

Changes: Compile-check source indexing

Layer / File(s)	Summary
Defer source indexing cli/dispatcher/internal/compilecheck/name_index.go, cli/dispatcher/internal/compilecheck/name_index_fifo_test.go, cli/dispatcher/internal/compilecheck/name_index_test.go	The name index retains generator results and starts source indexing on demand. wait and report wait for source reading only after it starts. Tests cover deferred reads and return from wait and report when no source read was requested.
Trigger indexing on full compile cli/dispatcher/internal/compilecheck/compiler.go, cli/dispatcher/internal/compilecheck/result_cache.go, cli/dispatcher/internal/compilecheck/name_index.go, cli/dispatcher/internal/compilecheck/result_cache_full_compile_test.go, cli/dispatcher/internal/compilecheck/scheduler_names_test.go	The full-compile path invokes a callback before CompileUnit, and the scheduler connects it to source indexing. Tests cover callback ordering, replay and fast-check paths, and scheduler source-read cases.
Expose source-read status cli/dispatcher/internal/dispatcher/compile_check_vibe_log.go, cli/dispatcher/internal/dispatcher/compile_check_vibe_log_test.go, docs/compile-check.md, docs/vibe-logs.md	The name-index log includes whether source reading began. The documentation describes when source reading occurs and when the status is present.

Priority: ⬇️ Low

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

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant ResultCache
  participant NameIndex
  participant Compiler
  ResultCache->>NameIndex: Invoke onFullCompile callback
  NameIndex->>NameIndex: Start readSources in background
  ResultCache->>Compiler: Call CompileUnit

Loading

Merge Risk: ⚪ Minimal · up to 90c09

This change defers assembly source reads in compile-check until a full compile or a name-based verdict needs them, and adds a sources_read field to the logs. No concrete merge-blocking risk was identified.

Security architecture risk: ⚪ Minimal · up to 90c09

The reviewed change preserves the existing file-access and compiler-execution authority. Indexing starts at most once, generated output is captured before compilation can replace it, and started work finishes before the run returns. No material security regression was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed indexing operation remains scoped to the current compile-check plan’s units with references and their saved generator output. Relative to the base, the new trigger changes timing rather than granting additional filesystem or compiler-execution authority.

Trust Boundaries and Controls

• observed — Project source paths still flow through the existing sourcePath resolution and file reader; generated files still use the existing directory reader. The callback supplies no new path or command input. The existing compiler invocation retains its configured executable, arguments, working directory, environment, and timeout.

Resilience and Maintainability Implications

• observed — Individual indexing file reads remain non-context-cancellable, as in the base. Cancellation therefore does not necessarily make a started read return promptly, but scheduler cleanup still joins started indexing rather than allowing it to outlive the run. This is an inherited limitation, not an introduced architecture concern.

Pre-merge checks: 5

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 16 functions across 9 files. (2 skipped: 2…
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.
Description check		The description clearly explains the deferred source-name reads, compiler hook, VibeLog update, tests, and validation results.
Title check		The title clearly summarizes the main change: compile-check reads plan names only when compilation or name evaluation requires them.

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 16 functions across 9 files. (2 skipped: 2 unsupported.)

✨ 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.}