Skip to content

fix(nodes): preserve dot in event type filter prefix matching#516

Open
staging-devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1779719732-fix-event-type-filter
Open

fix(nodes): preserve dot in event type filter prefix matching#516
staging-devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1779719732-fix-event-type-filter

Conversation

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor

@staging-devin-ai-integration staging-devin-ai-integration Bot commented May 25, 2026

Summary

  • matches_event_type_filter in both TelemetryOutNode and TelemetryTapNode stripped both .* from glob patterns like vad.*, producing prefix vad which incorrectly matched underscore-separated names like vad_something.
  • Now only the trailing * is stripped, so vad.* produces prefix vad. and matches vad.speech_start but not vad_something. A bare vad* still matches both.
  • The separate .* branch is removed since the generic * branch handles both cases correctly after this change.
  • Added regression assertions to both nodes' filter tests verifying vad.* rejects vad_something.

Closes #510

Review & Validation

  • vad.* matches vad.speech_start / vad.speech_end but not vad_something
  • vad* still matches both vad.speech_start and vad_something
  • Exact match (stt.result) still works and rejects stt.result.extra
  • Both telemetry_out and telemetry_tap nodes have consistent behavior

Link to Devin session: https://staging.itsdev.in/sessions/8e91aee7c1794fc3ac29ded15fcc0230
Requested by: @streamer45


Devin Review

Status Commit
🕐 Outdated 61af307 (HEAD is 95f69b9)

Run Devin Review

Open in Devin Review (Staging)

Strip only the trailing '*' from glob patterns like 'vad.*' so the
prefix retains the dot ('vad.') and no longer matches underscore-
separated names like 'vad_something'.

The separate '.*' branch is removed since the generic '*' branch
now handles both cases correctly.

Closes #510

Signed-off-by: streamkit-devin <devin@streamkit.dev>
@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Copy link
Copy Markdown
Contributor Author

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review (Staging)
Debug

Playground

let prefix = &pattern[..pattern.len() - 2];
event_type.starts_with(prefix)
} else if pattern.ends_with('*') {
if pattern.ends_with('*') {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 Parallel telemetry tap filter still strips the dot from .* patterns

This change fixes TelemetryOutNode so event_type_filter: ["vad.*"] requires the literal vad. prefix, but the same public filter on core::telemetry_tap still has the old special case that strips .* to vad before calling starts_with (crates/nodes/src/core/telemetry_tap.rs:120-126, registered as core::telemetry_tap at crates/nodes/src/core/mod.rs:137-148). Pipelines using the tap node with the documented vad.* pattern will still emit unrelated event types such as vad_something, leaving the prefix-matching fix inconsistent across the telemetry nodes.

Prompt for agents
Apply the same dot-preserving event_type_filter semantics to core::telemetry_tap. Its TelemetryTapNode::matches_event_type_filter in crates/nodes/src/core/telemetry_tap.rs still treats patterns ending in `.*` specially by removing both the star and the dot, so `vad.*` matches `vad_something`. Update that matcher (or share the implementation with TelemetryOutNode) so trailing `*` removes only the star, and add a regression assertion comparable to TelemetryOutNode's `vad_something` case.
Open in Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — applied the same fix to TelemetryTapNode::matches_event_type_filter and added a vad_something regression assertion. See 95f69b9.

The telemetry_tap node had the same bug as telemetry_out — stripping
both '.' and '*' from '.*' patterns. Apply the identical fix and add
a regression assertion.

Signed-off-by: streamkit-devin <devin@streamkit.dev>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.38%. Comparing base (b2d01c2) to head (95f69b9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #516      +/-   ##
==========================================
- Coverage   79.39%   79.38%   -0.01%     
==========================================
  Files         232      232              
  Lines       66904    66900       -4     
  Branches     1909     1909              
==========================================
- Hits        53117    53111       -6     
- Misses      13781    13783       +2     
  Partials        6        6              
Flag Coverage Δ
backend 79.11% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 85.27% <ø> (ø)
engine 83.68% <ø> (ø)
api 89.98% <ø> (ø)
nodes 75.32% <100.00%> (-0.02%) ⬇️
server 80.26% <ø> (ø)
plugin-native 83.47% <ø> (ø)
plugin-wasm 91.90% <ø> (ø)
ui-services 84.67% <ø> (ø)
ui-components 60.49% <ø> (ø)
Files with missing lines Coverage Δ
crates/nodes/src/core/telemetry_out.rs 98.58% <100.00%> (-0.01%) ⬇️
crates/nodes/src/core/telemetry_tap.rs 76.31% <100.00%> (+0.99%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

telemetry_out: vad.* glob matches broader prefix than expected

2 participants