Skip to content

fix(trace_events): align descriptors and empty filters - #9884

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9202-trace-event-descriptors
Closed

fix(trace_events): align descriptors and empty filters#9884
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9202-trace-event-descriptors

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Two trace_events rows in the #9202 Node-suite audit remain reproducibly red. Perry marks the module namespace exports configurable, while Node exposes writable/enumerable/non-configurable bindings. Perry also treats the literal CLI argument "" as an enabled category and emits an application event, while Node writes metadata only for that empty selection.

This makes both named and default trace-events namespace bindings non-configurable, normalizes a quoted empty category list, and emits the synthetic application marker only for the implemented node, node.bootstrap, and node.console categories. Non-empty and legacy category behavior is preserved.

Validation:

  • node-suite/trace_events/surface/module-descriptors: 1/1 parity pass against Node 26.5.1 on the final rebased head
  • node-suite/trace_events/native-output/category-filtering: 1/1 parity pass on the final rebased head
  • adjacent disable-filter, file-shape-exit, and pre-enabled-categories fixtures: exact Node output
  • focused quoted-empty-category unit test: pass
  • cargo test -p perry-runtime -- --test-threads=1: 3,215 passed, 4 ignored; doc tests green
  • post-rebase scripts/run_lint_gates.sh: all 64 gates passed; 2 CI-expression gates skipped locally

No version bump.

Addresses the two trace_events rows in #9202.

Summary by CodeRabbit

  • Bug Fixes

    • Improved node:trace_events compatibility by aligning export property behavior with Node.js.
    • Empty trace-event categories are now filtered correctly, including quoted empty category lists.
    • Trace output no longer includes an included-marker event when no relevant Node categories are enabled.
    • Added support for consistent category parsing across supported command-line formats.
  • Documentation

    • Added changelog coverage for the updated node:trace_events behavior.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 8f420ed0-a54b-4224-9038-490b3339cf3e

📥 Commits

Reviewing files that changed from the base of the PR and between 890514a and 3034937.

📒 Files selected for processing (3)
  • changelog.d/9884-trace-events-descriptors.md
  • crates/perry-runtime/src/node_submodules/mod.rs
  • crates/perry-runtime/src/node_submodules/trace_events.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The node:trace_events namespace now exposes non-configurable properties. CLI category parsing removes quotes and empty entries. Trace output omits the included marker when no relevant category is enabled. Tests and the changelog cover these changes.

Changes

Trace events alignment

Layer / File(s) Summary
Non-configurable trace events descriptors
crates/perry-runtime/src/node_submodules/mod.rs
Trace event export and default properties are now writable, enumerable, and non-configurable.
Category parsing and output markers
crates/perry-runtime/src/node_submodules/trace_events.rs, changelog.d/9884-trace-events-descriptors.md
Both CLI category forms use shared parsing that removes surrounding quotes and empty entries. Trace output emits the marker only for relevant categories. A test covers quoted empty categories, and the changelog records the alignment.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 30349

This aligns trace-event export descriptors, category parsing, and marker emission with Node behavior. The changes preserve enabled tracing metadata and avoid emitting markers for unrelated categories, with no current merge-blocking risk identified.

Suggested reviewers: thehypnoo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the two trace_events fixes: descriptor alignment and empty-category filtering.
Description check ✅ Passed The description provides a clear summary, concrete changes, related issue reference, validation results, and confirmation that no version bump was included. It does not use the template headings or in…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via merge train #9898. Validated as a tree: 64/64 lint gates, and perry-runtime/codegen/hir/stdlib all green (5,939 tests, 0 failures). Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant