Skip to content

fix: Preserve compile diagnostics across Unity reloads#1282

Merged
hatayama merged 2 commits into
v3-betafrom
feature/hatayama/fix-issue-1280-fresh
Jun 3, 2026
Merged

fix: Preserve compile diagnostics across Unity reloads#1282
hatayama merged 2 commits into
v3-betafrom
feature/hatayama/fix-issue-1280-fresh

Conversation

@hatayama

@hatayama hatayama commented Jun 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • uloop compile now writes persisted CLI VibeLog checkpoints when ULOOP_DEBUG=1 is enabled for the CLI process.
  • Unity debug logs now include compile request, polling, finish, result storage, reload, and bridge binding context under the same request id.

User Impact

  • When a compile command appears stuck or Unity reloads while diagnostics are active, saved VibeLogs can show whether the request was sent, accepted, polled, completed, timed out, or cancelled.
  • CLI debug activation remains controlled by the existing ULOOP_DEBUG environment variable. The CLI does not inspect Unity ProjectSettings.asset for this behavior.

Changes

  • Added CLI compile wait VibeLog events for request dispatch, status polling, completion, timeout, and cancellation.
  • Added Unity-side compile and reload checkpoints that preserve request ids, pending request state, status query state, and bridge endpoint context.
  • Added focused Go tests for the CLI VibeLog lifecycle, timeout, cancellation, and send-result logging.

Verification

  • scripts/check-go-cli.sh
  • cli/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-beta

Related

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.
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 837b022f-321e-4ccc-be28-0bf1f51db818

📥 Commits

Reviewing files that changed from the base of the PR and between cf92ea8 and 01892bb.

📒 Files selected for processing (3)
  • Packages/src/Editor/Domain/CliConstants.cs
  • cli/contract.json
  • cli/internal/tools/default-tools.json
✅ Files skipped from review due to trivial changes (3)
  • cli/internal/tools/default-tools.json
  • Packages/src/Editor/Domain/CliConstants.cs
  • cli/contract.json

📝 Walkthrough

Walkthrough

This 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.

Changes

Compile Command Observability

Layer / File(s) Summary
Compile Start-Time Tracking and Elapsed Logging
Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs
CompileController now tracks _compileStartedAtUtc when compilation begins and resets it on abort, completion, and cleanup. The HandleCompileFinished handler was refactored to log compile_finish_callback_received with computed elapsed_ms derived from the UTC start timestamp, plus success/error/warning state and request id.
Compile Request Reception and Pending Request Logging
Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs, Packages/src/Editor/FirstPartyTools/Compile/CompileSessionResultService.cs
CompileUseCase now logs incoming compile requests via LogCompileRequestReceived helper and passes correlationId to pending request registration. MarkPendingCompileRequestIfNeeded was extended to log compile_request_registered_for_status_polling with prior pending count and previous request id (when replaced). CompileSessionResultService queries previous result/pending request state and includes store_sequence, pending_request_before, and duplicate_result_for_request fields in storage completion logs.
Compile Status Query Logging
Packages/src/Editor/Infrastructure/Api/CompileStatusBridgeCommand.cs
Execute now logs status queries via LogCompileStatusQueryReceived helper, emitting compile_status_query_received with request id and key response fields (Ready, HasResult, IsCompiling, IsUpdating, IsDomainReloadInProgress, Message).
Domain Reload Pending Request Tracking
Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs
StartDomainReload and CompleteDomainReload now capture pending compile requests and include request count and ID list in their VibeLogger payloads. Added ToPendingCompileRequestIds helper to convert request arrays to ID strings.
Server Binding Logging Structure
Packages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cs
Updated binding-success log to emit server.Endpoint via structured anonymous-object payload rather than string interpolation.
Project Identity Hashing Infrastructure
cli/internal/cli/cli_vibe.go
Added cryptographic hashing (SHA-256) for deterministic project identity. New projectIdentity helper resolves symlinks, hashes the path, and returns project_<16-hex-char prefix> identifier; returns empty string for empty input.
Compile Wait Polling Instrumentation
cli/internal/cli/compile_wait.go, cli/internal/cli/compile_wait_test.go
waitForCompileCompletion now tracks attempts, lastStatus, lastErr, and lastObservationKey for deduplication and logging. Emits logs for poll-start, status observations (gated by change detection), completion with result summary, timeout, and cancellation—all including context like attempt count and last status. Added compileReloadExternalSceneChangesEnabled to interpret reload-external-scene-changes parameter (defaulting true). Introduced logging helpers for debug-mode resolution, request preparation (with computed flags like stop_on_external_scene_changes), dispatch outcome, polling lifecycle, result summary extraction, and status-context mapping. New tests cover polling lifecycle, timeout with status details, cancellation with request id and last status, and domain-reload flow with IPC handshake simulation (skipped on Windows).
Compile Request Dispatch Logging
cli/internal/cli/run.go
runCompileWithDomainReloadWait now emits debug logs after compile parameters are prepared (including request ID and debug-mode resolution) and after compile request send (capturing send result, error, and timing reference).
CLI Version / Contract Bumps
Packages/src/Editor/Domain/CliConstants.cs, cli/contract.json, cli/internal/tools/default-tools.json
Bumped MINIMUM_REQUIRED_CLI_VERSION, cliVersion, and default-tools version from 3.0.0-beta.243.0.0-beta.25.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: preserving compile diagnostics across Unity reloads, which aligns with the core objective of adding VibeLog checkpoints and request correlation.
Description check ✅ Passed The description is well-related to the changeset, providing clear context about VibeLog persistence, request id correlation across compile lifecycle, and the impact on diagnosability during Unity reloads.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/hatayama/fix-issue-1280-fresh

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 and usage tips.

Align the Unity package minimum CLI requirement with the next CLI contract version so PR checks acknowledge the Go CLI diagnostic changes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_replaced appears to be hardcoded incorrectly.

Line 263 always logs pending_request_replaced = false, but the actual behavior depends on whether previousPendingRequests had any entries. If previousPendingRequests.Length > 0, then MarkPendingCompileRequest (line 252-255) would replace the existing request, making pending_request_replaced effectively true.

🔧 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

📥 Commits

Reviewing files that changed from the base of the PR and between b0608f3 and cf92ea8.

📒 Files selected for processing (10)
  • Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs
  • Packages/src/Editor/FirstPartyTools/Compile/CompileSessionResultService.cs
  • Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs
  • Packages/src/Editor/Infrastructure/Api/CompileStatusBridgeCommand.cs
  • Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs
  • Packages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cs
  • cli/internal/cli/cli_vibe.go
  • cli/internal/cli/compile_wait.go
  • cli/internal/cli/compile_wait_test.go
  • cli/internal/cli/run.go

@hatayama
hatayama merged commit 447a697 into v3-beta Jun 3, 2026
8 checks passed
@hatayama
hatayama deleted the feature/hatayama/fix-issue-1280-fresh branch June 3, 2026 14:39
@github-actions github-actions Bot mentioned this pull request Jun 3, 2026
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