Repository navigation
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This branch has not been deployed
1 participant
Summary
The logger reads its caller location from raw V8 call sites, so a log line no longer runs source-map-support's stack hook. Refs #6556.
Before this change, every printed or persisted log line ran
new Error().stack.src/desktop/main.tsandsrc/cli/server.tsinstallsource-map-support/register, which maps each frame of each formatted stack, so the first log line from a file parsed that file's whole.js.mapon the main thread. This hits both Electron main andxum server.Implementation
getCallerLocation()swapsError.prepareStackTracefor one capture (Error.captureStackTraceinto a holder, read inside the override, previous hook restored infinally). It then scans the call sites with the same logger-frame skipping and the sameformatCallerLocationoutput./node/services/log.{ts,js}. Raw call sites name the compileddist/node/services/log.js, which the old check only saw after source mapping.Trade-off: dist builds, including
make dev, show the compiled.jsfile and line (dist/node/services/workspaceService.js:3484) instead of the.tsline. The.js.mapstill ships next to each file, so the line can be mapped later. Bun (tests,bunruns of src) is unchanged, because there the sources are the.tsfiles.Validation
log.test.ts› "a log line names its caller without running the global stack formatter": a counting globalError.prepareStackTracestands in for source-map-support.log.infomust call it 0 times, and the location must besrc/node/services/log.test.ts:<line>. With origin/main'slog.tsrestored, it fails withExpected: 0, Received: 1. It passes under Bun 1.4.3, so Bun supports the CallSite API this needs.node dist/cli/index.js serverwith a tempXUM_ROOTwrites locations such asdist/node/services/serviceContainer.js:269anddist/node/services/workspaceService.js:3484, and no line nameslog.js.node --cpu-prof dist/cli/index.js server --port 0, 12 s, tempXUM_ROOT, 2 runs per side. The host is shared: 28 CPUs, load average 70-88 during the runs.getCallerLocationSourceMapConsumer_parseMappingsfdfeb84d7bDesktop Cold Start (D2), base = main
fdfeb84d7b, head =fe8a506b4b, 20 pairs x 3 repeats: run 39635019962.xum:app-shell-readymark (60 launches)Paired statistic (20% trimmed mean of per-pair deltas): -24.9%, one-sided 95% upper bound -22.6%, half-width 2.33% (within the 2.5% gate).
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$14.73