Remove leftover [embed:trace] debug logging from the window-open/first-frame path#696
Draft
azchohfi wants to merge 2 commits into
Draft
Remove leftover [embed:trace] debug logging from the window-open/first-frame path#696azchohfi wants to merge 2 commits into
azchohfi wants to merge 2 commits into
Conversation
…t-frame path PR #543 (Spec 056 embedded preview Phase 1) left unguarded Console.Error.WriteLine("[embed:trace] ...") printf-debugging in the window-open path: four success-path writes fire on EVERY window open (even non-embed) plus catch-block traces, and six embed-only writes in ReactorWindow setup. Removed all of them while preserving behavior exactly: the MountAndActivate catch keeps its real cleanup (UnregisterWindow + Dispose) before rethrow; the ctor/configure and embed-setup try/catch blocks only logged-then-rethrew, so after removing the logs they are no-ops and were simplified away (exceptions propagate identically). No control-flow, ordering, or Register/Mount sequencing changes. Consumer check: the [embed:trace] lines go to stderr only; the VS preview handshake is CAPTURE_PORT/CAPTURE_TOKEN parsed from stdout (ReactorChildLauncher.cs, extension.ts). stderr is purely diagnostic (16KB tail buffer + Output pane), so nothing programmatic depends on these lines. Also updated a stale troubleshooting landmark in tests/stress_perf/ci/README.md that referenced the removed "MountAndActivate ok" trace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Removes leftover unguarded [embed:trace] Console.Error.WriteLine(...) printf-style debug logging from the window-open / first-frame activation path (including embed setup), keeping behavior and ordering unchanged while eliminating unconditional stderr noise.
Changes:
- Removed unconditional stderr trace lines from
ReactorApp.OpenWindowCoreand simplified no-op try/catch wrappers that only logged-then-rethrew. - Removed embed-only trace lines from
ReactorWindowembed setup and simplified the same log-only exception wrapper. - Updated the stress-perf CI troubleshooting note to no longer rely on the removed trace landmark.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Reactor/Hosting/ReactorApp.cs | Drops unconditional stderr traces in OpenWindowCore; preserves cleanup-on-activation-failure behavior. |
| src/Reactor/Hosting/ReactorWindow.cs | Removes embed setup stderr traces; keeps embed init ordering and watchdog start behavior intact. |
| tests/stress_perf/ci/README.md | Rewords troubleshooting guidance to avoid referencing removed trace output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…race The pr-review skill flagged that .github/skills/perf-compare/SKILL.md still referenced the removed "MountAndActivate ok" trace as a headless-crash landmark. Reworded to "during first-frame window activation" to match the README change so the skill no longer points readers at a stderr line that no longer prints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
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
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.
What
PR #543 (commit
f810c021, "Spec 056: Visual Studio embedded preview — Phase 1") left unguardedConsole.Error.WriteLine("[embed:trace] ...")printf-debugging in the window-open / first-frame code path. This PR removes it.There is no
#if DEBUG, no log level, no abstraction — these are bare stderr writes. Two files:src/Reactor/Hosting/ReactorApp.cs(OpenWindowCore) — 4 success-path traces that fired unconditionally on EVERY window open, including the non-embed path ("OpenWindowCore enter", "ReactorWindow ctor ok", "configure ok", "MountAndActivate ok"), plus 3...THREWtraces inside catch blocks that immediately rethrow.src/Reactor/Hosting/ReactorWindow.cs(embed setup) — 6 embed-only traces ("entering embed setup", "VerifyEmbedDpiAwareness ok", "ApplyEmbedInitialStyles ok", "EmbedHostWatchdog started", "embed setup THREW",ex.ToString()).Behavior preserved exactly
MountAndActivatecatch keeps its real cleanup (UnregisterWindow(window)+window.Dispose()) before rethrow — only the trace line was dropped (catch (Exception ex)→catchsinceexis now unused).catch { throw; }wrappers, so they were simplified away — exceptions now propagate naturally with identical observable behavior.RegisterWindow/MountAndActivatesequencing changes.VerifyEmbedDpiAwareness→ApplyEmbedInitialStyles→ watchdog start order is unchanged. No replacement diagnostics / logging framework introduced (out of scope).Consumer check (done before deleting)
Grepped
src/,tests/,src/vscode-reactor/,src/vs-reactor/, andsrc/Reactor.Cli/for any consumer of these stderr lines. Result: pure leftover — nothing programmatic depends on them.CAPTURE_PORT=/CAPTURE_TOKEN=, parsed from the child's stdout (ReactorChildLauncher.OnStdoutLine, andextension.ts).ReactorChildLauncher.OnStderrLineappends to a 16 KBLastStderrtail buffer and forwards to the VS Output pane;extension.tsappends stderr to its output channel. Neither pattern-matches stderr for any progress/handshake signal.[embed:trace]output. The only non-source reference was a troubleshooting note intests/stress_perf/ci/README.mdthat quoted"MountAndActivate ok"as a diagnostic landmark; reworded so it no longer depends on the removed line.Verification
dotnet build src/Reactor/Reactor.csproj -c Release(core lib promotes trim/AOT warnings to errors): 0 warnings / 0 errors.dotnet test tests/Reactor.Tests: 9702 passed, 0 failed, 64 skipped (headless WinUI skips).dotnet test tests/Reactor.SelfTests(live WinUI, realOpenWindowCorewindow-open path): 1229 passed, 0 failed.Status
Draft / HOLD —
src/Reactorruntime change held for the project owner's personal review per standing instruction. Gate-clean but not to be merged yet.