fix(ui): check |* suffix before rendering wildcard in formatWithTemplate#512
Conversation
The regex in formatWithTemplate captured the |* suffix but never
checked whether the placeholder actually included it, so any field
with a wildcard_value rendered as * regardless of the template syntax.
Now only placeholders written as {field|*} trigger wildcard rendering;
plain {field} placeholders always show the concrete value.
Signed-off-by: streamkit-devin <devin@streamkit.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #512 +/- ##
==========================================
- Coverage 79.39% 79.38% -0.01%
==========================================
Files 232 232
Lines 66904 66904
Branches 1909 1909
==========================================
- Hits 53117 53115 -2
- Misses 13781 13783 +2
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| }); | ||
|
|
||
| it('does not render wildcard when template placeholder omits |*', () => { | ||
| // Register a type whose template uses plain {width} (no |*) for a field with wildcard_value |
There was a problem hiding this comment.
🟡 New tests add how-comments that repository rules explicitly forbid
AGENTS.md makes the comment guideline mandatory and lists “Line narration” plus “Diff-oriented comments” as comments not to write. The newly added test comments narrate exactly what the immediately following registry/test data does and explain this edit’s {field|*} behavior rather than a non-obvious constraint, so they violate that repository rule. The affected added comments are on ui/src/utils/packetTypes.test.ts:224, ui/src/utils/packetTypes.test.ts:241-242, and ui/src/utils/packetTypes.test.ts:250-251.
| // Register a type whose template uses plain {width} (no |*) for a field with wildcard_value |
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| // width=null matches wildcard_value but placeholder lacks |* — render literally | ||
| // height=null matches wildcard_value and placeholder has |* — render as * |
There was a problem hiding this comment.
🟡 Additional wildcard test comments violate the no narration rule
AGENTS.md explicitly forbids comments that restate what the next lines do or explain the edit instead of a hidden constraint. These added comments describe that width=null/height=null match the wildcard and how {sample_format} differs from {sample_rate|*}, which is already expressed by the test setup and assertion, so they violate the repository’s mandatory comment rule.
| // width=null matches wildcard_value but placeholder lacks |* — render literally | |
| // height=null matches wildcard_value and placeholder has |* — render as * |
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| // sample_rate=0 matches wildcard via {sample_rate|*}, but sample_format uses | ||
| // {sample_format} (no |*) so even a wildcard_value match renders literally |
There was a problem hiding this comment.
🟡 Sample-rate wildcard test comments restate the assertion
The added comments above this assertion explain how the concrete values in the test are expected to render, which is directly visible from the input object and expected string. Because AGENTS.md explicitly forbids line narration and diff-oriented comments, these comments should not be added unless they explain a non-obvious constraint.
| // sample_rate=0 matches wildcard via {sample_rate|*}, but sample_format uses | |
| // {sample_format} (no |*) so even a wildcard_value match renders literally |
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| if (compat && compat.kind === 'structfieldwildcard') { | ||
| const rule = compat.fields.find((f) => f.name === field); | ||
| if (rule && 'wildcard_value' in rule) { | ||
| if (rule && 'wildcard_value' in rule && _m.includes('|*')) { |
There was a problem hiding this comment.
📝 Info: Wildcard display marker now has presentation-only semantics
The added _m.includes('|*') check means wildcard_value no longer automatically affects every placeholder in a template; it only affects placeholders explicitly marked with |*. That matches the metadata contract in crates/core/src/packet_meta.rs:38 and ui/src/types/generated/api-types.ts:147-150, while connection compatibility still uses wildcard_value without consulting the display template in ui/src/utils/packetTypes.ts:117-126 and crates/core/src/packet_meta.rs:225-249. I therefore did not flag the behavior change as a bug, but reviewers should be aware that new packet-type metadata must include |* anywhere the UI should render matching wildcard values as *.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Signed-off-by: streamkit-devin <devin@streamkit.dev>
- Thread the regex capture group as a callback arg instead of string .includes() on the full match. - Drop two redundant tests already covered by existing assertions. - Add a real-world regression test using the default RawVideo meta with pixel_format: null to document the user-facing fix. Signed-off-by: streamkit-devin <devin@streamkit.dev>
2 participants
Summary
formatWithTemplatecaptured the|*suffix via regex but never gated on it — any field with awildcard_valuerendered as*regardless of template syntax.{field|*}placeholders trigger wildcard rendering; plain{field}always shows the concrete value. Uses the regex capture group directly instead of a.includes()check.{field}/{field|*}) and one using the realRawVideometa withpixel_format: nullto document the user-facing regression.Closes #463
Review & Validation
formatWithTemplate(line 37:&& star)just test-ui)Link to Devin session: https://staging.itsdev.in/sessions/f937c399d5244c269cfbe81e52072886
Requested by: @streamer45
Devin Review
f5a5fc3(HEAD ise5e9b74)