fix(querystring): preserve empty string values

[sylvesterkaczmarek]

Summary

• preserve explicit empty-string query values instead of dropping them
• keep None query values omitted
• add scalar, nested, and repeated-array regression coverage

Fixes #3837

Tests

Regression coverage added in tests/test_qs_empty_values.py. The connected development host was unavailable for a local suite run.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-10T02:21:40.087103Z	6b4e277	PR opened
🔒 Security Review	✅ Completed 2026-09-10T02:22:49.355195Z	6b4e277	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[sylvesterkaczmarek]

Following up after the Group 6 maintenance pass. This branch is current with main, mergeable, and has no failing checks or unresolved review threads. Could a maintainer review it when convenient?

[hossiendehghan989]

The scalar fix is correct, but I found one remaining branch-specific inconsistency that the current tests do not cover.

_stringify_item() handles array_format="comma" separately at src/openai/_qs.py:87-93, and that branch is unchanged by this PR. As a result, an array containing only None is still serialized as an explicit empty query parameter:

from openai._qs import Querystring

qs = Querystring()

assert qs.stringify({"filter": None}, array_format="comma") == ""
assert qs.stringify({"filter": [None]}, array_format="comma") == "filter="  # currently fails the invariant

This matters because the PR establishes the semantic distinction None -> omitted versus "" -> filter=. The same distinction should hold when the value is nested in an array. The current implementation already filters None while joining comma arrays, but it does not remove the key when every element was None; that makes [None] behave differently from None and can send a server-side empty filter instead of omitting the parameter.

A regression test should cover both sides of the distinction:

assert stringify({"filter": [None]}, array_format="comma") == ""
assert stringify({"filter": [""]}, array_format="comma") == "filter="
assert stringify({"filter": [None, "active"]}, array_format="comma") == "filter=active"

The implementation can preserve explicit empty strings while omitting an all-None array by checking whether the input contains any non-None item before returning the comma-joined pair. I reproduced the current behavior locally against src/openai/_qs.py; this is independent of the issue’s scalar case and should be a small, targeted addition to the otherwise solid fix.

[sylvesterkaczmarek]

Thanks, reproduced. Fixed in GitHub-Verified commit 2eff92f: comma-format arrays now drop None values and omit the parameter entirely when nothing remains, while explicit empty strings still serialize as filter=. Added regressions for [], [None], [""], and [None, "active"]. Focused test_qs suite passes 11/11; Ruff check/format and git diff --check are clean. A maintainer re-review of the refreshed head would be appreciated.

[hossiendehghan989]

Checked the refreshed head against both sides of the distinction: value is None now returns early, the comma branch filters None items and returns nothing when the filtered list is empty, and an empty string still serialises as an explicit empty value. The issue's scalar case and the nested-array case I raised are both covered.

One residual test gap I would close before merge, because the invariant now lives in the shared helper rather than in the comma branch alone: all four array formats should agree on the nothing-left-after-filtering outcome.

@pytest.mark.parametrize("array_format", ["comma", "repeat", "indices", "brackets"])
def test_empty_array_omits_the_parameter(array_format):
assert serialise({"filter": []}, array_format=array_format) == ""
assert serialise({"filter": [None]}, array_format=array_format) == ""

repeat, indices and brackets already produce the empty string for both inputs today, since each one recurses per element back into _stringify_item, but only the comma cases are asserted. A parameterised test would tie that to the shared implementation, so a later change to one branch cannot leave the four formats disagreeing.

The non-empty cases stay deliberately format-specific — an empty string becomes filter= under comma but filter[]= or filter[0]= under the others — so a one-line comment in the test saying that only the empty outcome is shared would stop the next contributor from trying to unify them. Since the PR notes that no local suite run was possible, CI is the only verification of the refreshed head, and the focused test_qs additions look sufficient on their own.

[sylvesterkaczmarek]

Added the cross-format regression in d42e99f. comma, repeat, indices and brackets now all explicitly assert that [] and [None] omit the parameter, with a note that non-empty serialization remains intentionally format-specific. Focused test_qs passes 15/15; Ruff check/format and git diff --check are clean. Commit is GitHub Verified.