Skip to content

Add bounded debugger performance tracing - #161

Draft
bjdodson-openai wants to merge 1 commit into
bjd/debugger-settingsfrom
bjd/debugger-tracing
Draft

Add bounded debugger performance tracing#161
bjdodson-openai wants to merge 1 commit into
bjd/debugger-settingsfrom
bjd/debugger-tracing

Conversation

@bjdodson-openai

Copy link
Copy Markdown
Collaborator

Description

Adds lifecycle-safe, bounded performance tracing across the CLI, renderer, runtime, and native tracer layers.

  • Preserves the existing native tracer ABI while adding bounded debugger results.
  • Handles uncertain Start/Stop outcomes, reload recovery, target disappearance, and result expiry.
  • Caps full response envelopes, trace events, dropped-event counts, and retained results.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Documentation improvement
  • Performance optimization
  • Test improvement
  • Other (new debugger capability)

Testing

  • Tests pass locally (bazel test //...)
  • Added/updated tests for changes (if applicable)
  • Tested on multiple platforms (iOS/Android/Web/macOS as applicable)
  • Manual testing performed (describe below)

Testing Details

  • Incremental branch: Restacked CLI 261/261 and production build passed; focused server/daemon coverage and independent native ABI review passed.
  • Assembled debugger stack: npm test passed 436/436; the CLI production build passed.
  • Focused //src/valdi_modules/src/valdi/web_renderer:test passed.
  • bazel query //... passed.
  • The broad Valdi suite reproduced the established 12 unrelated failures; all new debugger specs passed.

Checklist

  • Code follows project style guidelines
  • Documentation updated (if needed)
  • No breaking changes (or documented in description)
  • Commit messages follow conventional format
  • No secrets, API keys, or internal URLs included

Related Issues

Relates to #154

Additional Context

Stack 8/22. Stacked on #160 (bjd/debugger-settings). Review this PR as the single incremental commit 06be7132 against that base; do not merge it before its parent.

@github-actions

Copy link
Copy Markdown

📊 PR Size: size/XL

Total changes: 4449 lines (28 files)

Top files changed:

  • npm_modules/cli/src/debugger/server.ts: +599 -2
  • npm_modules/cli/src/core/packageFiles.spec.ts: +532 -1
  • src/valdi_modules/src/valdi/valdi_core/test/PerformanceTraceMessageHandler.spec.ts: +494 -0
  • npm_modules/cli/debugger/debugger-performance.js: +439 -4
  • npm_modules/cli/src/debugger/server.spec.ts: +348 -1
  • src/valdi_modules/src/valdi/valdi_core/src/debugging/PerformanceTraceMessageHandler.ts: +336 -0
  • npm_modules/cli/src/utils/daemonClient.ts: +279 -22
  • npm_modules/cli/src/utils/daemonClient.spec.ts: +283 -2
  • src/valdi_modules/src/valdi/valdi_core/src/debugging/Messages.ts: +255 -4
  • src/valdi_modules/src/valdi/valdi_core/src/utils/Trace.ts: +134 -12

...and 18 more files

Size calculated as additions + deletions. Labels: XS (<10), S (<50), M (<250), L (<1000), XL (1000+)

@github-actions github-actions Bot added area/runtime Valdi runtime (C++/native) area/docs Documentation labels Aug 26, 2026
@github-actions

Copy link
Copy Markdown

⚠️ Bazel & CI Test Results

Test Suite Result
macOS: C++ & Platform Tests ❌ failure
valdi_web Integration Test ✅ success
Linux: Build Compiler ✅ success
API Surface Check ✅ success
Valdi Smoke Tests ✅ success
Snapshot Tests ✅ success
Linux: Module Tests ✅ success
Linux: Registry Validation ✅ success
Test Coverage Delta ✅ success
Linux: C++ Tests ✅ success
Linux: Hotreload Smoke ✅ success
Linux: Build & Export ✅ success

Some tests failed. Please check the workflow logs for details.

🚀 Bazel remote cache is now enabled - future builds will be faster!

Workflow: Valdi CI

@clholgat clholgat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please carry this feedback into the squashed PR(s).

🟢 Low — runTraceTransition (npm_modules/cli/src/debugger/server.ts) serializes trace start/stop/capture across ALL targets and ports through a single process-wide traceTransitionInProgress flag. During a one-shot capture (up to 15s) the flag is held for the whole window, so a manual stop of any other target/context is rejected with "Another renderer trace transition is already in progress," and an in-progress capture cannot be aborted. (Status polling is intentionally not gated, so it stays responsive.) Mirrors the existing profileTransitionInProgress pattern, so it's a conservative choice rather than a data bug — but consider keying the in-progress guard per target (port/clientId/contextId) so unrelated targets aren't blocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Documentation area/runtime Valdi runtime (C++/native) size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants