Skip to content

chore: Fail CI on high-confidence C# dead code and remove leftover readiness probes - #1995

Merged
hatayama merged 5 commits into
v3-betafrom
fix/dead-code-ci-gate
Jul 25, 2026
Merged

chore: Fail CI on high-confidence C# dead code and remove leftover readiness probes#1995
hatayama merged 5 commits into
v3-betafrom
fix/dead-code-ci-gate

Conversation

@hatayama

@hatayama hatayama commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Pull requests that touch package C# (or the scanner itself) now fail CI when high-confidence dead code is introduced.
  • Three leftover execute-dynamic-code readiness probe helpers with zero callers are removed.

User Impact

  • Before: the dead-code scanner existed but never ran in CI, so unused C# symbols could land unnoticed.
  • After: Unused / UnusedPrivateMember / UnusedLocal findings fail the new Dead Code Gate. PublicCandidate and TestOnly still require manual review and do not fail CI.

Changes

  • Add .github/workflows/dead-code.yml (scanner unit tests + --fail-on high-confidence scan; no advisory artifacts).
  • Remove:
    • DynamicCodeForegroundWarmupRunner.TryRunBackgroundSequenceAsync — never had a caller since introduction.
    • FirstPartyToolsEditorStartup.CreateExecuteDynamicCodeReadinessProbeCode — leftover after commit 7e45f1e7 intentionally switched the Editor readiness probe from execute-dynamic-code to get-version so user-disabled tools cannot block startup. Rewiring the old path would reverse that design; deletion is correct.
    • ExecuteDynamicCodeReadinessProbe.CreatePrimaryReturnStringProbeCode — only called by the deleted helper above; removed in the same change.
  • Document the CI gate in docs/dead-code-scanner.md.

Out of scope

Verification

  • scripts/check-dead-code.sh ... --fail-on high-confidence → exit 0
  • Before/after scan counts on origin/v3-beta: 244 → 242 (PublicCandidate 187 → 185)
  • dotnet test tests/UnityCliLoop.DeadCodeScanner.Tests/... → Passed 4 / Failed 0
  • cd cli/release-automation && go test ./internal/architecture -run 'TestWorkflowActions|TestPullRequestWorkflow' -count=1 → ok
  • dist/darwin-arm64/uloop compile → ErrorCount 0, WarningCount 0

Refs #1987

Review in cubic

hatayama and others added 3 commits July 26, 2026 01:01
These three symbols had zero callers after readiness probing moved to
get-version in 7e45f1e. Delete them rather than rewire the old path,
which would let user-disabled tools block Editor startup again.

Co-authored-by: Cursor <cursoragent@cursor.com>
The scanner already existed but was never wired into GitHub Actions, so
unused C# symbols could land unnoticed. Run it on package and scanner
path changes with --fail-on high-confidence only.

Co-authored-by: Cursor <cursoragent@cursor.com>
Contributors need to know which categories fail CI and which still need
manual review of non-C# references.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@hatayama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 91ae9ac1-7021-4509-a606-f810e7360518

📥 Commits

Reviewing files that changed from the base of the PR and between fa77edc and f29589f.

📒 Files selected for processing (4)
  • .github/workflows/dead-code.yml
  • docs/dead-code-scanner.md
  • tests/UnityCliLoop.DeadCodeScanner.Tests/CommandLineOptionsTests.cs
  • tests/UnityCliLoop.DeadCodeScanner.Tests/DeadCodeScannerTests.cs
📝 Walkthrough

Walkthrough

The pull request removes unused dynamic-code warmup and readiness helpers, adds a GitHub Actions dead-code scanning workflow for pull requests and manual runs, and documents the CI failure criteria.

Changes

Dead code cleanup and CI gate

Layer / File(s) Summary
Remove obsolete dynamic-code paths
Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs, Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/ExecuteDynamicCodeReadinessProbe.cs, Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs
Removes the background warmup path and two public readiness-probe code-generation helpers.
Add dead-code CI validation
.github/workflows/dead-code.yml, docs/dead-code-scanner.md
Adds automated scanner tests and high-confidence dead-code checks for selected pull-request changes, with documentation of failing and non-failing finding categories.

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

Possibly related issues

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the CI dead-code gate and the removal of leftover readiness probe helpers.
Description check ✅ Passed The description matches the changeset and explains the CI gate, code removals, documentation update, and verification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dead-code-ci-gate

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.

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/dead-code.yml (1)

3-11: 🩺 Stability & Availability | 🔵 Trivial

Confirm this path-filtered workflow is not a globally required check.

PRs targeting main or v3-beta that touch unrelated files will not receive a workflow result. If branch protection requires Dead Code Gate for every PR, those changes can be blocked; use an always-running workflow with conditional expensive steps instead.

Based on learnings, always-required checks should use an always-running workflow rather than a path-filtered workflow.

🤖 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 @.github/workflows/dead-code.yml around lines 3 - 11, Update the workflow
trigger so it runs for every pull request targeting main or v3-beta, removing
the path filter from the on.pull_request configuration. Keep the dead-code scope
restrictions within conditional job or step execution so unrelated changes still
produce the required Dead Code Gate result without running expensive scanning
unnecessarily.

Source: Learnings

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

Inline comments:
In @.github/workflows/dead-code.yml:
- Around line 23-24: Add persist-credentials: false to the actions/checkout step
in the dead-code workflow, keeping the existing pinned checkout action
unchanged.

In `@docs/dead-code-scanner.md`:
- Around line 22-24: Update the workflow trigger description in the dead-code
documentation to state that it runs only for pull requests targeting main or
v3-beta, and include .github/workflows/dead-code.yml itself among the triggering
path changes. Preserve the existing listed source, scanner, test, and script
paths.

---

Nitpick comments:
In @.github/workflows/dead-code.yml:
- Around line 3-11: Update the workflow trigger so it runs for every pull
request targeting main or v3-beta, removing the path filter from the
on.pull_request configuration. Keep the dead-code scope restrictions within
conditional job or step execution so unrelated changes still produce the
required Dead Code Gate result without running expensive scanning unnecessarily.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9faa9984-dbf8-4af5-8314-150b82ae7ea6

📥 Commits

Reviewing files that changed from the base of the PR and between ffc2737 and fa77edc.

📒 Files selected for processing (5)
  • .github/workflows/dead-code.yml
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/ExecuteDynamicCodeReadinessProbe.cs
  • Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs
  • docs/dead-code-scanner.md
💤 Files with no reviewable changes (3)
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/ExecuteDynamicCodeReadinessProbe.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs
  • Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs

Comment thread .github/workflows/dead-code.yml
Comment thread docs/dead-code-scanner.md Outdated
CodeRabbit flagged credential persistence on a PR-triggered workflow that
runs repository scripts, and the docs omitted the base-branch filter and
self-path trigger that the YAML already enforces.

Co-authored-by: Cursor <cursoragent@cursor.com>
The CI gate only fails when --fail-on high-confidence is parsed and
findings are classified as high-confidence candidates. Without tests on
either side, a broken parse or inverted category check would stay green
forever while the gate looked effective.

Co-authored-by: Cursor <cursoragent@cursor.com>
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