Skip to content

Add bounded component-property inspection - #172

Closed
bjdodson-openai wants to merge 1 commit into
bjd/debugger-target-pickerfrom
bjd/debugger-component-properties
Closed

Add bounded component-property inspection#172
bjdodson-openai wants to merge 1 commit into
bjd/debugger-target-pickerfrom
bjd/debugger-component-properties

Conversation

@bjdodson-openai

Copy link
Copy Markdown
Collaborator

Description

Adds bounded read-only component-property inspection for exact web targets.

  • Reads only own enumerable data descriptors and never invokes getters.
  • Shares a 64 KiB emitted-value budget with bounded depth, keys, strings, and entries.
  • Fails closed on hostile or revoked Proxies while preserving the hierarchy fallback.

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: Focused registry/panel/server 149/149 and full CLI 398/398 passed; web-renderer Bazel and query passed, with the broad suite matching its exact baseline.
  • 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 19/22. Stacked on #171 (bjd/debugger-target-picker). Review this PR as the single incremental commit 012775d6 against that base; do not merge it before its parent.

@github-actions

Copy link
Copy Markdown

📊 PR Size: size/L

Total changes: 904 lines (16 files)

Top files changed:

  • src/valdi_modules/src/valdi/web_renderer/src/debug/DebuggerValueSnapshot.ts: +403 -0
  • src/valdi_modules/src/valdi/web_renderer/test/LegacyWebDebuggerAdapter.spec.ts: +149 -1
  • src/valdi_modules/src/valdi/web_renderer/test/DebuggerValueSnapshot.spec.ts: +131 -0
  • npm_modules/cli/src/debugger/devtoolsPanel.spec.ts: +75 -5
  • src/valdi_modules/src/valdi/web_renderer/src/debug/ComponentHierarchySnapshot.ts: +46 -0
  • npm_modules/cli/debugger/devtools-panel.js: +26 -5
  • src/valdi_modules/src/valdi/web_renderer/src/ValdiWebRendererDelegate.ts: +17 -0
  • npm_modules/cli/debugger/README.md: +13 -0
  • npm_modules/cli/src/debugger/server.spec.ts: +9 -2
  • src/valdi_modules/src/valdi/valdi_core/src/Renderer.ts: +6 -1

...and 6 more files

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

@github-actions

Copy link
Copy Markdown

⚠️ Bazel & CI Test Results

Test Suite Result
valdi_web Integration Test ✅ success
macOS: C++ & Platform Tests ❌ failure
Test Coverage Delta ✅ success
Linux: Hotreload Smoke ✅ success
Snapshot Tests ✅ success
Valdi Smoke Tests ✅ success
API Surface Check ✅ success
Linux: C++ Tests ✅ success
Linux: Build & Export ✅ success
Linux: Registry Validation ✅ success
Linux: Module Tests ✅ success
Linux: Build Compiler ✅ 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.

Review of bounded component-property inspection. The bounds and fail-closed handling of hostile/revoked Proxies look solid. One low-severity performance note:

  • 🟢 Low — captureObject in DebuggerValueSnapshot.ts only counts enumerable descriptors against the entry/byte caps; non-enumerable own keys are skipped (continue) without consuming budget or incrementing the entry count. A hostile object/Proxy with millions of non-enumerable own keys therefore forces one getOwnPropertyDescriptor (trap) call per key, with work bounded only by total own-key count rather than the 50-entry / 64 KiB caps → CPU stall on the snapshot path. Suggested fix: count every own key inspected against a hard iteration cap (or decrement the entry budget for skipped non-enumerable keys too) so traversal cost is bounded by the caps, not by the object's key count.

Note for the squash: please carry this feedback into the squashed PR that contains component-property inspection.

@bjdodson-openai

Copy link
Copy Markdown
Collaborator Author

Superseded by #180, which consolidates this patch into the reviewed debugger capabilities landing unit. The replacement carries forward the feedback and fixes discussed here. Closing this draft to reduce the active stack; this PR and its discussion remain the historical review record.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants