Fix/utils urls preserve query params

[akshatsinha0]

Use parse_qsl(keep_blank_values=True) and urlencode(doseq=True), override keys without dropping duplicates and keep original key order where it can be seen that possible.

Given url: http://x?a=1&a=2&b=&c=3
modify_url_query(url, a=[4,5]) => http://x?a=4&a=5&b=&c=3
modify_url_query(url, d=7) => http://x?a=1&a=2&b=&c=3&d=7
modify_url_query(url, b="") preserves empty value.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[bito-code-review]

Code Review Agent Run #b56472

Actionable Suggestions - 0

Additional Suggestions - 2

• superset/utils/urls.py - 2

  • Use modern union syntax in isinstance · Line 56-56
    Multiple `isinstance` calls use tuple syntax instead of modern union syntax. On lines 56 and 59, replace `(list,tuple)` with `list | tuple` for better readability and modern Python style.
    Code suggestion

     @@ -55,57 +55,57 @@
    -            new=[(k,str(x)) for x in v] if isinstance(v, (list,tuple)) else [(k,str(v))]
    +            new=[(k,str(x)) for x in v] if isinstance(v, list | tuple) else [(k,str(v))]

  • Use elif instead of else if · Line 58-58
    Use `elif` instead of `else` followed by `if` to reduce indentation and improve readability.
    Code suggestion

     @@ -57,63 +57,62 @@
    -        else:
    -            if isinstance(v,(list,tuple)):
    -                pairs.extend((k,str(x)) for x in v)
    -            else:
    -                pairs.append((k,str(v)))
    +        elif isinstance(v, list | tuple):
    +            pairs.extend((k,str(x)) for x in v)
    +        else:
    +            pairs.append((k,str(v)))

Review Details

• Files reviewed - 7 · Commit Range: d7a1654..8cecb74

  • superset-frontend/src/components/GridTable/Header.test.tsx
  • superset-frontend/src/explore/reducers/exploreReducer.js
  • superset/commands/database/test_connection.py
  • superset/reports/notifications/email.py
  • superset/utils/log.py
  • superset/utils/urls.py
  • tests/unit_tests/utils/screenshot_test.py
• Files skipped - 1

  • UPDATING.md - Reason: Filter setting
• Tools

  • Eslint (Linter) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[akshatsinha0]

@msyavuz

[korbit-ai]

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Inefficient double iteration over query parameters ▹ view
	Expensive list slice insertion operation ▹ view

Files scanned

​

File Path	Reviewed
superset/utils/urls.py	✅
superset/reports/notifications/email.py	✅
superset-frontend/src/explore/reducers/exploreReducer.js	✅
superset/utils/log.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

[korbit-ai]

Inefficient double iteration over query parameters

Tell me more

What is the issue?

The code performs two separate iterations over the pairs list - first to find indices, then to filter out matching keys, resulting in O(n²) time complexity when updating multiple parameters.

Why this matters

This approach scales poorly with large query strings or when updating many parameters, as each parameter update requires scanning the entire pairs list twice.

Suggested change ∙ Feature Preview

Use a single pass approach to build the new pairs list while tracking positions, or use a dictionary-based approach for better performance:

result_pairs = []
for i, (kk, vv) in enumerate(pairs):
    if kk not in kwargs:
        result_pairs.append((kk, vv))
    elif kk in kwargs and kk not in processed:
        # Insert new values at first occurrence position
        v = kwargs[kk]
        new = [(kk, str(x)) for x in v] if isinstance(v, (list, tuple)) else [(kk, str(v))]
        result_pairs.extend(new)
        processed.add(kk)

Provide feedback to improve future suggestions

💬 Looking for more details? Reply to this comment to chat with Korbit.

[korbit-ai]

Expensive list slice insertion operation

Tell me more

What is the issue?

Using slice assignment to insert elements at a specific position in a list has O(n) time complexity due to the need to shift all subsequent elements.

Why this matters

This operation becomes expensive for large query strings as it requires moving all elements after the insertion point, contributing to the overall poor performance of the function.

Suggested change ∙ Feature Preview

Build the result list incrementally without slice insertions, or use a more efficient data structure like collections.deque for insertions, then convert back to list at the end.

Provide feedback to improve future suggestions

💬 Looking for more details? Reply to this comment to chat with Korbit.