fix: Preserve compile diagnostics across Unity reloads#1282
Conversation
Keep CLI VibeLog enablement controlled by the ULOOP_DEBUG environment while recording compile request dispatch, status polling, and Unity-side compile/reload checkpoints for post-reload diagnosis.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughThis PR adds comprehensive observability to compile workflows across the C# editor and Go CLI by instrumenting request lifecycle, timing, status polling, and domain reload events. No functional behavior changes; all alterations are logging/observability enhancements. ChangesCompile Command Observability
Sequence Diagram(s)sequenceDiagram
participant Client as CLI Client
participant Poller as waitForCompileCompletion
participant StatusAPI as CompileStatus API
participant Logger as CLI Vibe Logger
Client->>Poller: start compile wait
Poller->>Logger: log poll_start
loop status polling
Poller->>StatusAPI: query compile status
StatusAPI->>Poller: return compile status
Poller->>Logger: log changed status observation
end
Poller->>Logger: log completion, timeout, or cancellation
Poller->>Client: return result or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Align the Unity package minimum CLI requirement with the next CLI contract version so PR checks acknowledge the Go CLI diagnostic changes.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs (1)
236-270:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
pending_request_replacedappears to be hardcoded incorrectly.Line 263 always logs
pending_request_replaced = false, but the actual behavior depends on whetherpreviousPendingRequestshad any entries. IfpreviousPendingRequests.Length > 0, thenMarkPendingCompileRequest(line 252-255) would replace the existing request, makingpending_request_replacedeffectivelytrue.🔧 Proposed fix
new { request_id = request.RequestId, force_recompile = request.ForceRecompile, - pending_request_replaced = false, + pending_request_replaced = previousPendingRequests.Length > 0, pending_request_count_before = previousPendingRequests.Length, previous_request_id = previousPendingRequests.Length > 0 ? previousPendingRequests[0].RequestId : "" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs` around lines 236 - 270, The compile request status-polling log in MarkPendingCompileRequestIfNeeded is hardcoding pending_request_replaced to false even when a previous pending request exists. Update the anonymous payload passed to VibeLogger.LogInfo so pending_request_replaced is derived from previousPendingRequests.Length > 0, alongside the existing pending_request_count_before and previous_request_id fields, to reflect whether MarkPendingCompileRequest actually replaced an earlier request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs`:
- Around line 236-270: The compile request status-polling log in
MarkPendingCompileRequestIfNeeded is hardcoding pending_request_replaced to
false even when a previous pending request exists. Update the anonymous payload
passed to VibeLogger.LogInfo so pending_request_replaced is derived from
previousPendingRequests.Length > 0, alongside the existing
pending_request_count_before and previous_request_id fields, to reflect whether
MarkPendingCompileRequest actually replaced an earlier request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6468585-ebb7-4895-abea-a1b36d54ce6b
📒 Files selected for processing (10)
Packages/src/Editor/FirstPartyTools/Compile/CompileController.csPackages/src/Editor/FirstPartyTools/Compile/CompileSessionResultService.csPackages/src/Editor/FirstPartyTools/Compile/CompileUseCase.csPackages/src/Editor/Infrastructure/Api/CompileStatusBridgeCommand.csPackages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.csPackages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cscli/internal/cli/cli_vibe.gocli/internal/cli/compile_wait.gocli/internal/cli/compile_wait_test.gocli/internal/cli/run.go
Summary
uloop compilenow writes persisted CLI VibeLog checkpoints whenULOOP_DEBUG=1is enabled for the CLI process.User Impact
ULOOP_DEBUGenvironment variable. The CLI does not inspect UnityProjectSettings.assetfor this behavior.Changes
Verification
scripts/check-go-cli.shcli/dist/darwin-arm64/uloop compile --project-path <PROJECT_ROOT>cli/dist/darwin-arm64/uloop run-tests --project-path <PROJECT_ROOT> --test-mode EditMode --filter-type regex --filter-value "io\.github\.hatayama\.UnityCliLoop\.Tests\.Editor\.(CompileStatusBridgeCommandTests|CompileSessionResultServiceTests|CompileLifecycleWatchdogTests|DomainReloadDetectionServiceTests|UnityCliLoopServerStartupProtectionTests)\."$codex-review v3-betaRelated