Skip to content

[Performance] Optimize outputDebug to skip expensive work when disabled #7563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gonzaloriestra wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −0
Open

[Performance] Optimize outputDebug to skip expensive work when disabled #7563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@craigmichaelmartin craigmichaelmartin May 19, 2026

Choose a reason for hiding this comment

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

💬 Suggestion: .jules/bolt.md is a Jules AI agent reasoning log — not project documentation. Similar AI tool directories (.cursor/, .claude/) are typically gitignored. Without a .gitignore entry, these files will accumulate as Jules is used on more PRs, adding noise to git history.

Suggestion: Add .jules/ to .gitignore to prevent Jules metadata from being committed. If the file is already merged, remove it in a follow-up.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 2025-05-14 - Optimize outputDebug to skip expensive string construction
**Learning:** outputDebug always constructs a timestamped message even when debug logging is disabled. This involves calling `new Date().toISOString()` and string concatenation, which is wasteful in the hot path of logging.
**Action:** Add an early return in `outputDebug` using `shouldOutput('debug')` to skip work when not in verbose mode.
2 changes: 2 additions & 0 deletions packages/cli-kit/src/public/node/output.ts
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@craigmichaelmartin craigmichaelmartin May 19, 2026

Choose a reason for hiding this comment

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

💬 Suggestion: There are no dedicated tests for outputDebug in output.test.ts. Given that this PR changes the control flow of a function called ~295 times across the codebase, even basic tests would increase confidence: (1) that collectLog still captures debug content after the early return, and (2) that the expensive string construction is actually skipped when debug is disabled.

Suggestion: Add a test in packages/cli-kit/src/public/node/output.test.ts that verifies outputDebug still populates collectedLogs.debug when isUnitTest() returns true, and that the logger is NOT called when shouldOutput('debug') returns false.

Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ export function outputCompleted(content: OutputMessage, logger: Logger = console
*/
export function outputDebug(content: OutputMessage, logger: Logger = consoleWarn): void {
if (isUnitTest()) collectLog('debug', content)
if (!shouldOutput('debug')) return

const message = colors.gray(stringifyMessage(content))
outputWhereAppropriate('debug', logger, `${new Date().toISOString()}: ${message}`)
}
Expand Down
Loading