Repository navigation
docs: remove obsolete docs and fix stale references - #1988
Merged
Merged
Conversation
Remove four documents under docs/ that no longer describe anything that
exists in the repository:
- execute-dynamic-code-windows-handoff.md was a one-off handoff note for
resuming work on branch `codex/rebuild-execute-dynamic-code`, which no
longer exists. Eight of the nine source paths it tells the reader to
open are gone, because the onion refactor moved the whole pipeline
under Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/. It also
ends with a "paste this into Windows Codex" prompt, so it was never
meant to outlive that session.
- native-cli-simplification-plan.md proposed consolidating the Go modules
into a single `cli` module with internal/{cli,unityipc,project,...}
packages. The repository instead runs cli/{common,dispatcher,
project-runner,release-automation}, and ADR 0002 records the deliberate
decision to keep the dispatcher/runner split. Its verification snippet
also still used the pre-V3 `uloop compile --wait-for-domain-reload`
spelling, which V3 replaced with `--no-wait-for-domain-reload`.
- unity-cli-loop-onion-refactor-plan.md was the working record of a
finished refactor, still written as a plan ("Create branch...",
"Test Plan"). The resulting structure is now enforced by the asmdef
dependency tests it describes, so the document adds nothing a reader
cannot get from the code.
- execute-dynamic-code-run-log.md listed the snippets used in a single
benchmark session on 2026-04-16.
Nothing else in the repository linked to any of them except the handoff
note, which linked to the rebuild doc and to itself.
The Server and CLI entries described the Unity-side endpoint as a TCP IPC endpoint. There is no TCP listener in the package: BridgeTransportEndpoint resolves to BridgeTransportKind.UnixDomainSocket on macOS/Linux and BridgeTransportKind.WindowsNamedPipe on Windows, which is also what the repository guidelines state. Since the glossary is the reference other docs and reviews are supposed to follow, a wrong transport here propagates into anything written against it.
Two documented commands could not work as written. github-actions-security.md told the reader to run the workflow-pinning tests "from `cli`". `cli` is only a go.work root with no test packages; the tests live in cli/release-automation/internal/architecture. Ran the command from the corrected directory to confirm it passes. execute-dynamic-code-examples.md passed `--parameters '[6,7]'` and read the values back as `parameters[0]`. ExecuteDynamicCodeSchema.Parameters is a Dictionary<string, object>, so the flag takes a JSON object and the values arrive under `param0`/`param1` — the array form never bound. The values also arrive boxed, so a direct `(int)` cast is not enough. The replacement example was run against a live Editor and returns 42. Added the "advanced, usually unnecessary" note the tool's skill file already carries, so the example does not read as the default way to pass data into a snippet.
The document still described a prewarm use case that no longer exists. `IPrewarmDynamicCodeUseCase` / `PrewarmDynamicCodeUseCase` are gone, and with them the `UnityCliLoopServerController -> PrewarmUseCase` entry edge: server readiness is now `UnityCliLoopFirstPartyServerLifecycleBinding` in the composition root, which warms the project IPC transport with the internal `get-version` command and never enters this pipeline. Dynamic-code warm-up moved into the pipeline itself, split across `DynamicCodeForegroundWarmupRunner`, `DynamicCodeForegroundWarmupState`, `ExecuteDynamicCodeReadinessProbe`, and `DynamicCodeForegroundWarmupSnippets`, with `ExecuteDynamicCodeUseCase` running the foreground fallback. Added a Warm-up module so the reason those four types share one snippet list — no path may report warm while a shape the user hits first is still cold — is visible in the diagram rather than only in the code comments. Also corrected two edges that no longer exist. `DynamicCodeExecutionFacade` does not touch `ICompiledAssemblyBuilder`; it depends on the executor pool and `DynamicCodeExecutionScheduler`, which is what arbitrates foreground versus idle-only execution. And the composition graph attributed the whole compiler collaborator set to the registry, when `DynamicCodeServicesRegistry` only wires runtime access — the planning, backend build, and safety/load collaborators are built by `DynamicCodeCompiler`'s default constructor, several hops away. Named the assembly and folder the pipeline lives in at the top, so the Entry/UseCase/Infrastructure headings are not misread as onion assemblies.
Of the 17 examples in this file, only the --parameters one carried information specific to uloop. The rest were plain Unity API usage — GameObject.Find, SceneManager.GetActiveScene, FindObjectsByType<Camera> — which the tool's skill file explicitly tells the agent to write from its own Unity knowledge instead of copying from a catalogue. The repository already removed generic cookbooks from skill files for the same reason. The "Long Examples" section made the file actively misleading. Its whole premise was wrapping long snippets in CODE=$(cat <<'EOF' ... EOF), which --code-file replaced; the skill file now routes shell-quoting trouble to --code-file directly. This document never mentioned that flag, so its one genuinely uloop-specific technique was the superseded one.
The Operating Policy still described the original rollout at a maximum cyclomatic complexity of 25. The threshold has since been lowered to 15 in both places that declare it — MAX_COMPLEXITY in scripts/check-code-complexity.sh and cyclop.max-complexity in cli/.golangci-complexity.yml — and the workflow's artifact step passes 15 as well. Running the script locally prints "max 15" for both stacks. The closing advice was inverted by the same drift: it told the reader not to lower the repository-wide threshold yet, when it had already been lowered. Replaced it with the guidance that actually matters for a check that never fails the build — the report only helps if someone reads it. Also named the two declaration sites and the workflow's trigger paths, so the next threshold change has an obvious checklist and this document is less likely to drift again.
Of the previous 314 lines, 144 were mermaid structure diagrams and 70 were a one-line-per-class list. Both duplicate what the source already declares, and both are what actually rotted: every error found in the preceding commit lived in the diagrams — a use case that no longer exists, a facade edge to ICompiledAssemblyBuilder that was never there, five dependencies attributed to a registry that does not hold them. The duplication is not worth carrying. The 101 files in this folder hold 263 summary and Why comments between them, and the repository's comment policy requires the rationale to live there. So the diagrams competed with the code for the same job and lost. What is left is the part reading the code does not give you: the order to read it in, the rule each module boundary was chosen to satisfy, and the cross-cutting design intent. That content does not change when a class moves, so this file should now stay true across refactors instead of needing a sweep after each one. Renamed the heading away from "rebuild", which described a migration that finished rather than the document's subject.
This file was written as the design writeup for PR #901, "Rebuild execute-dynamic-code with shared Roslyn compilation and layered architecture" — the "rebuild" in its name was that PR's, not a description of its subject. Its only inbound link was the Windows handoff note created alongside it, and both were artifacts of that one migration. In the three months since, it was never updated on purpose: each of its three edits came from a rename sweep that happened to touch it. Reducing it to the parts code cannot state left almost nothing standing. The warm-up rationale it carried is already in the source three times over (DynamicCodeForegroundWarmupRunner, ExecuteDynamicCodeReadinessProbe). The server-readiness explanation is in UnityCliLoopFirstPartyServerLifecycleBinding's own summary. The reading order it prescribed is what following the delegation from ExecuteDynamicCodeTool gives you anyway, and the dependency-direction rules are enforced by the asmdef dependency tests rather than by prose. A finished migration's design memo left in the tree gets read as a description of current behavior. This one had already required two rounds of correction for exactly that reason, which is the same failure the other plan documents removed in this branch showed. docs/architecture/ is now empty and goes away with it.
Seven files under docs/ had no path from AGENTS.md, so an agent reading only the repository guidelines never learned they existed. Rather than append a flat index, state each doc's rule where it fires: the glossary next to the naming policy, action SHA pinning under CI automation language, and new sections for the complexity threshold and broken-release recovery. Add a docs/ entry to the repository map that also covers docs/adr/, which had none. docs/dispatcher-pin-release-order.md stays reachable through docs/project-runner-pin.md and needs no separate pointer.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (6)
📝 WalkthroughWalkthroughRepository guidance now covers glossary compliance, SHA-pinned GitHub Actions, broken CLI release handling, and code complexity. IPC terminology and complexity documentation were updated, a test command path was corrected, and several architecture and execution documents were removed. ChangesRepository governance and documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
RyanXie123
pushed a commit
to RyanXie123/unity-cli-loop
that referenced
this pull request
Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audited every file under
docs/against the code, scripts, and workflows they describe. Six documents no longer described anything that exists, and three carried factual errors that would mislead a reader who trusted them. Removed the former, corrected the latter, and gave the survivors a path fromAGENTS.md.Removed (6 files, all completed-work artifacts)
docs/architecture/execute-dynamic-code-windows-handoff.mdcodex/rebuild-execute-dynamic-code, which no longer exists. 8 of its 9 referenced paths are gone. Ends with a "paste this into Windows Codex" prompt.docs/architecture/native-cli-simplification-plan.mdclimodule withinternal/{cli,unityipc,...}. The actual layout iscli/{common,dispatcher,project-runner,release-automation}, and ADR 0002 settled the dispatcher/runner split deliberately. Also held the stale--wait-for-domain-reloadexample.docs/architecture/unity-cli-loop-onion-refactor-plan.mddocs/architecture/execute-dynamic-code-rebuild.mdf48cdaae(PR #901) as the design memo for that rebuild. The rebuild shipped; the memo described aPrewarmDynamicCodeUseCasethat no longer exists.docs/execute-dynamic-code-examples.mduloop. Its central heredoc technique was superseded by--code-file, which the doc never mentioned.docs/execute-dynamic-code-run-log.mdVerified after deletion that no file in the repository references any of them.
Corrected
docs/glossary.md— described the server and CLI as talking over TCP. There is no TCP path:BridgeTransportEndpointoffersUnixDomainSocketandWindowsNamedPipeonly.docs/github-actions-security.md— told the reader to run the architecture tests fromcli, which is not a Go module. Corrected tocli/release-automation; the documented command now passes as written.docs/code-complexity.md— stated a threshold of 25. Both declarations (MAX_COMPLEXITYinscripts/check-code-complexity.sh,cyclop.max-complexityincli/.golangci-complexity.yml) say 15. Also replaced the "first rollout is advisory" framing, which read as provisional years after the fact, with what the advisory mode actually demands of a reader.Discoverability
Seven files under
docs/had no path fromAGENTS.md, so an agent reading only the repository guidelines never learned they existed. Rather than appending a flat index — which does not fire at the moment it is needed — each doc's rule now sits where it applies: the glossary beside the naming policy, action SHA pinning under CI automation language, and new sections for the complexity threshold and broken-release recovery. The repository map gained adocs/entry, which also coversdocs/adr/.Verification
go test ./internal/architecture -run 'TestWorkflowActions|TestPullRequestWorkflow' -count=1passes from the directory the corrected doc names.BridgeTransportEndpoint.cs; complexity threshold checked against both declaration sites.docs/*.mdis now reachable fromAGENTS.md, directly or through one hop.No code, script, or workflow changed, and
docs/belongs to no release-please package root — no release-trigger follow-up applies.Follow-up
Two dead symbols surfaced during the audit are tracked separately in #1987, together with the exemption attribute and CI gating discussed there.