Skip to content

[ui] Fix tag filter parsing for values containing = signs#33699

Open
guillaume wants to merge 2 commits into
dagster-io:masterfrom
guillaume:fix-tag-filter-equals-parsing
Open

[ui] Fix tag filter parsing for values containing = signs#33699
guillaume wants to merge 2 commits into
dagster-io:masterfrom
guillaume:fix-tag-filter-equals-parsing

Conversation

@guillaume

Copy link
Copy Markdown

Summary & Motivation

Tag values containing = characters (e.g. hive-style partition keys like program=foo/project=bar) are truncated at the first = when parsed into filter objects. This causes "Add tag to filter" on the Runs page to produce incorrect filters that match no runs.

The root cause is split('=') without limiting to the first occurrence in both tagValueToFilterObject and runsFilterForSearchTokens. Replaced with indexOf/slice to split only on the first =, preserving the full tag value.

How I Tested These Changes

  • Added unit tests for tagValueToFilterObject and runsFilterForSearchTokens covering tag values with multiple = signs
  • Verified all existing tests continue to pass (yarn jest — 150 suites, 1244 tests passed)
  • yarn tsgo and yarn lint pass with no errors
  • Manually tested in the Dagster UI with a run tagged dagster/partition=foo=bar/baz=qux

Tag values with `=` characters (e.g. hive-style partition keys like
`program=foo/project=bar`) were being truncated at the first `=` when
parsed into filter objects. This caused "Add tag to filter" to produce
incorrect filters that matched no runs.

Use `indexOf`/`slice` to split only on the first `=`, preserving
the full tag value.
@greptile-apps

greptile-apps Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR correctly fixes the root-cause bug where split('=') without a limit truncates tag values containing = signs (e.g. hive-style partition keys) in tagValueToFilterObject and runsFilterForSearchTokens. The fix is accomplished by replacing array destructuring on split('=') with indexOf('=') + slice, which honours only the first =. New unit tests for both functions are added and all existing tests continue to pass.

Key findings:

  • The fix in tagValueToFilterObject (line 782–789) and runsFilterForSearchTokens (line 160–162) is correct and complete.
  • A third occurrence of the same pattern at line 647 inside the tagFilter state useMemo (const [key, value] = token.value.split('=')) was not fixed. For custom (non-excluded) tags whose values contain =, the displayed active filter value will be truncated, mirroring the original bug.
  • The remaining split('=')[0] calls (lines 399, 417, 457, 473, 542, 576, 644, 659) only extract the key and are safe because tag keys do not contain =.

Confidence Score: 3/5

  • Partially safe — the core fix is correct but one instance of the same bug in the tagFilter state memo was missed.
  • The PR fixes two of the three places where split('=') causes incorrect tag value truncation, and ships solid test coverage for both. However, line 647 in RunsFilterInput.tsx retains the identical anti-pattern, meaning custom tags with = in their value will still appear truncated in the tag filter's active state chip. This is the same class of user-visible bug the PR set out to eliminate.
  • js_modules/ui-core/src/runs/RunsFilterInput.tsx — specifically the tagFilter state useMemo around line 647.

Important Files Changed

Filename Overview
js_modules/ui-core/src/runs/RunsFilterInput.tsx Fixes split('=') truncation in runsFilterForSearchTokens and tagValueToFilterObject, but misses the same pattern on line 647 inside the tagFilter state memo — custom tags with = in their values will still be mishandled there.
js_modules/ui-core/src/runs/tests/RunFilterInput.test.tsx Adds well-targeted tests for tagValueToFilterObject and runsFilterForSearchTokens covering multi-= tag values; no issues found in the test file itself.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["token.value string containing multiple equals signs"]

    A --> B["runsFilterForSearchTokens\nline 160-162 - FIXED"]
    A --> C["tagValueToFilterObject\nline 783-787 - FIXED"]
    A --> D["tagFilter state useMemo\nline 647 - NOT FIXED"]

    B --> B1["indexOf plus slice approach\npreserves full tag value"]
    C --> C1["indexOf plus slice approach\npreserves full tag value"]
    D --> D1["split destructuring approach\ntruncates tag value at first delimiter"]

    B1 --> E["RunsFilter object sent to API"]
    C1 --> F["Filter object used for set comparison"]
    D1 --> G["tagSuggestionValueObject receives wrong value\nActive filter chip shows truncated value"]
Loading

Comments Outside Diff (1)

  1. js_modules/ui-core/src/runs/RunsFilterInput.tsx, line 647-649 (link)

    P1 Missed fix: same split('=') bug in tagFilter state

    This line uses the same destructuring-split pattern that was fixed in runsFilterForSearchTokens and tagValueToFilterObject, but it was not updated here. For any custom tag whose value contains = (e.g. my-tag=foo=bar), value will be silently truncated to foo, causing the tag filter's active state to display and save the wrong value.

Reviews (1): Last reviewed commit: "[ui] fix tag filter parsing for values c..." | Re-trigger Greptile

Extract splitTagString() helper to DRY up the indexOf/slice logic
across runsFilterForSearchTokens, tagValueToFilterObject, and the
tag filter state mapping. Also fixes a third instance of the same
split('=') bug on the tag filter setState path.

Add dedicated unit tests for splitTagString edge cases.
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