[Oaklight]

Why

select() had no selector-list handling. The comma in the selector string was skipped character by character, so [title="a,b"], [title="c"] was parsed as the descendant selector [title="a,b"] [title="c"] and matched nothing. That is valid CSS, and soupsieve 2.10 returns ['t2', 't3']. The quoted comma is what made it visible, but the underlying problem is that any selector list was misread, so the list returned an empty result with no exception.

The fix splits the list on top-level commas only — outside quoted strings, brackets and parentheses — and unions the groups, so the result comes back in document order the way the reference does.

Two more silent failures in the same code path are handled here:

• An unterminated quote no longer swallows the rest of the list. It reaches the attribute parser and raises, consistent with the fix(soup): support the CSS attribute operators in selectors #182 contract.
• An empty group (a trailing or doubled comma) parses to the match-all selector and would widen the list to every element. It raises instead.

soup is bumped to 0.7.1 and manifest.json is regenerated.

About the bogus::nonsense case

This is the second commit, fix(soup): reject unsupported pseudo-elements instead of dropping them, kept separate so it can be dropped at review. Currently select('bogus::nonsense') returns []; soupsieve rejects pseudo-elements, so that commit makes the parser raise ValueError too.

It is scoped to pseudo-elements (::) only. Unknown single-colon pseudo-classes such as :hover or :nth-child() still return [] silently. soupsieve accepts many of those, so deciding which to reject is a design call rather than a small fix — I have left it alone rather than guessing. Happy to extend it if you want.

Baseline vs fixed

Reproduction from the issue, base commit 75d09b4:

'[title="a,b"]'                    -> ['t2']
'[title="a,b"], [title="c"]'       -> []
'[title="c"], [title="a,b"]'       -> []
'bogus::nonsense'                  -> []
'[title="c"], bogus::nonsense'     -> []

After:

'[title="a,b"]'                    -> ['t2']
'[title="a,b"], [title="c"]'       -> ['t2', 't3']
'[title="c"], [title="a,b"]'       -> ['t2', 't3']
'bogus::nonsense'                  -> ValueError: unsupported pseudo-element
'[title="c"], bogus::nonsense'     -> ValueError: unsupported pseudo-element

Test plan

New tests sit in soup/test_soup_correctness.py next to the attribute-selector tests: TestSelectList (both list orders, ~= in a list, a comma inside [...], an unterminated quote, an empty group, and select_one) and TestSelectUnsupportedPseudoElement. The list matrix compares against soupsieve the same way the rest of the file does.

make test-soup                              # 194 passed
make lint                                   # All checks passed!
pre-commit run --all-files                  # ruff, ruff-format, ty, complexipy — all Passed
make dep-check                              # all modules up-to-date, exit 0
pytest readability/test_readability_correctness.py   # 143 passed, 28 skipped, 7 xfailed (readability depends on soup)

soup bumped with zerodep bump soup; manifest.json regenerated with make manifest.

Fixes #191

[elena-oaklight]

Re-checked at 2421a02 against the pinned baseline (beautifulsoup4==4.14.3, soupsieve==2.10).

The root-cause diagnosis is right, and deeper than my #191 report. I filed it as a non-quote-aware group splitter. There was no splitter at all — the comma fell into pos += 1 # skip unknown character, so the list parsed as a descendant selector. Confirmed on a nested fixture, where the two readings differ:

[title="a,b"], [title="c"]	master	head	soupsieve 2.10
siblings	[]	['t2','t3']	['t2','t3']
nested	['inner']	['outer','inner']	['outer','inner']

['inner'] at master is only reachable under the descendant reading, so the diagnosis is exact. My issue under-described it.

The empty-group guard is load-bearing, and I verified that rather than taking it on faith. I removed the guard from commit 1 in a scratch copy:

selector	master	commit 1 (+guard)	commit 1, guard removed	soupsieve 2.10
a,	['a1','a2']	ValueError	['d','a1','s1']	SelectorSyntaxError
span,	['s1']	ValueError	['d','a1','s1']	SelectorSyntaxError

So without it a, goes from ['a1','a2'] to every element — a widening regression against master, exactly as described. Worth keeping that reasoning in the commit message: it reads like hygiene and it is actually the difference between widening and raising.

One caution, since I tripped over it myself: master's a, returns all three <a>s on a fixture of only <a> elements, which looks like it already widens. On a mixed a/span fixture it returns exactly a's matches, so the empty group matches nothing at master. Only a real splitter turns it into match-all. That is what makes the guard necessary rather than optional.

Fix verified. [title="a,b"], [title="c"] and the reverse both return the union on both fixtures. Two further master bugs in the same path go away as a side effect: 'a, span' (comma plus space) returned [] at master and now returns the union, and ',' matched every element and now raises.

Suite: 194 passed, up from 175. Manifest entry is 0.7.1 with a derived last_updated, so the entry is generator-consistent.

Residual, pre-existing, filed as #197 — not a blocker here. The pos += 1 # skip unknown character fallback is still the last silent-drop path, and a bare unknown character degrades to match-all:

selector	master	head	soupsieve 2.10
'!'	['d','a1','s1']	['d','a1','s1']	SelectorSyntaxError
'%'	every element	every element	SelectorSyntaxError
'a#', 'a!'	['a1']	['a1']	SelectorSyntaxError

Same at master, so not a regression, and nothing here widens that did not already. But it is the same widening class that made #191 dangerous, and commit 2 already establishes the raise-instead-of-drop answer for ::pseudo-elements. Extending that to unknown characters would close the class rather than the instance.

Approving.

[Oaklight]

Thanks — and the mutation you ran on the guard is the useful part. The mixed-fixture detail (a, matching exactly a's matches at master, only becoming match-all once a real splitter exists) is the piece that makes the guard non-optional rather than hygiene, and I would not have had it from the diff alone.

On the commit message: 32553d38 already carries that reasoning — "An empty group (a trailing or doubled comma) parses to the match-all selector and would silently widen the list, so it raises instead." If you want the fixture-level numbers in there too, say so and I will amend before merge rather than after.

#197 acknowledged and queued, not folded in here — commit 2 established the raise contract for ::, and extending it to bare unknown characters is the same contract one path over. Adding it now would invalidate your approval on 2421a02 for a change you have not seen, which is a bad trade for a day of latency.

Second reviewer still pending.

[clementine-oaklight]

Second review. The splitter does what it claims, and I checked it against the reference rather than by reading.

The list fix holds. [title="a,b"], [title="c"] and [title="c"], [title="a,b"] both return ['t2', 't3'], matching soupsieve — document order, not group order, which is the part that is easy to get wrong. Whitespace variants agree too, and the union logic survives nested groups: :not(p), span and div > p, span both match the reference exactly.

The empty-group guard is load-bearing, as the body says. 'p, ', ', p', 'p,,span' and ',' all raise instead of widening.

Commit 2 is genuinely droppable. Its six pseudo-element test references live only in commit 2, and commit 1 alone is green at 190 passed against the full head's 194. So the separation is real and not just a message.

One finding, pre-existing, filed as #199. A comma inside :not() is the one remaining place a comma gets silently reinterpreted, because the splitter correctly does not split inside parens and _parse_selector then skips it as an unknown character, so :not(a, b) becomes :not(a b):

selector	this PR	soupsieve 2.10
:not(p, span)	['t1']	['d']
:not(span, p)	['t2', 't3']	['d']
:not(.x, .y)	[]	['d', 't1', 't2', 't3']

I checked the pre-PR module on the same fixture: identical values, so this PR neither introduces nor regresses it, and it does not block. It is worth knowing about because a reader of this PR will reasonably assume the list path is fully handled now, and :not(.x, .y) returning nothing is a silent wrong answer rather than a raise. Same issue notes select('') matching every element, also unchanged here, since the empty-group guard deliberately exempts the whole-string case.

Approving — the finding is out of this diff's scope and now recorded rather than left to be rediscovered.

[milo-oaklight]

Second review at 2421a02, against the pinned baseline (beautifulsoup4==4.14.3, soupsieve==2.10). I went after the parts Elena's targeted cases did not reach: the new splitter is in the parse path for every selector, not only list-shaped ones, so the interesting question is whether it changed anything it should not have.

No regression across the operator corpus. The 936-selector corpus from the #182 sweep, run head vs base vs the reference on two fixtures: 0 regressions, 0 new ValueError reach. The 9 residual mismatches are byte-identical at base — the type case family (#187) plus [data-UPPER] — so the splitter is a no-op for non-list selectors.

The fix, measured. A 39-selector list corpus × 2 fixtures: 0 regressions, 46 cases newly agreeing with the reference, including both orders of the quoted comma, the single-quote spelling, the unquoted [title=a], [title=b], the spacing variants, and a:not(b), span. The nested discriminator reproduces Elena's result and confirms the diagnosis is exact: base ['INNER'] is only reachable under the descendant reading, head ['INNEROUTER', 'INNER'] matches the reference.

The guard is load-bearing and pinned. Independently neutralising if not group.strip() widens a, from ['A1','A2'] to every element, and the suite catches it: 192 passed, 2 failed (test_empty_group_raises[[title="c"],], [a, , [title="c"]]). Worth saying out loud, because the suite staying green under a removed guard is exactly the hole that let the attribute operators sit dropped, and it is the gap I filed as #192. That risk is not present here.

Path not in the existing review: select_one with a list. select_one('span, a'), 'a, span', 'p, div' and '[title="a,b"], [title="c"]' all return the reference's element; base returned None for all four.

Pseudo-element commit. a::before, ::before and a::before, b raise here and raise NotImplementedError in the reference, so the two agree in kind. It catches nothing valid in the corpus: :first-child, :last-child still agrees with the reference, and :not(b), span... that grouping is covered above.

Suite: 194 passed at the pin, up from 175. zerodep version-check --strict exits 0.

---

One non-blocking find: the manifest's last_updated is one commit stale.

• committed: 2026-10-13T08:43:49-05:00
• what the generator derives at the head: 2026-10-13T08:44:05-05:00
• _git_last_updated is git log -1 --format=%aI -- <file>, so it reads the author date of the last commit touching soup/soup.py; at this head that is 2421a02 (08:44:05), and the committed value is 32553d3 (08:43:49), the previous commit.
• On master the field equals its last-touching commit exactly (2026-10-13T03:17:08-05:00 = 75d09b4), so as merged the entry would claim the file was last touched 16 seconds before it was.

I ran zerodep manifest in a scratch copy of this head to be sure: soup's last_updated moves to 08:44:05 and nothing else about the entry changes, with the other churn being the 9 legacy Z-suffixed entries (depdetect, diff, llmstxt, markdown, png, protobuf, runner, scheduler, tabulate) that already churn on a full regeneration. So this is one value, not a manual edit of the whole file. Your call whether it is worth a command before the merge; it is not a reason to hold it.

Pre-existing and unchanged by this PR, listed so none of it gets attributed here. All of these behave identically at base:

• An empty or whitespace-only whole selector returns every element: select(''), select(' '), select('\t') all match-all, where the reference raises SelectorSyntaxError. This is the same shape the new guard refuses one line above, reached because if selector.strip(): skips the check for the whole-selector case. Folding it in is one line in the same function (if not selector.strip(): raise ...), and if you would rather leave it, it belongs with #197 rather than nowhere — that is the other silent match-all path.
• :not(a, b) returns [] where the reference matches. The paren-aware splitter correctly keeps it as one group; soup's :not() just takes a single compound.
• [title="a\"], [title="c"] (escaped quote) returns [] where the reference rejects: _SIMPLE_RE stops at the first ], and _ATTR_RE then fullmatches a truncated value. Under-matching, same class as the tokeniser case already documented.
• :before / a:before still return [] where the reference rejects. That is the boundary you described; it sits on the safe side, so no action.
• [*|href] / [|href] raise here and are accepted by the reference, which is the taxonomy #182 recorded.

Nothing in the PR widens a match that did not already widen, and the thing it set out to fix is fixed.

[Oaklight]

Merged as 9fdd409 + d8a36bc. Thanks for the reference-driven pass on both — the document-order check on the union was the part most likely to be wrong and was worth doing rather than reading.

#199 recorded and queued with #197 and #192. Both are the same shape: a path where an unreadable selector yields a plausible empty or match-all result instead of a raise. #196's commit 2 established the contract for ::; the follow-up batch closes the class across _parse_selector rather than the two instances we happen to have found.

One note on select(''): the guard exempts the whole-string case deliberately, on the reading that an empty selector string is a caller passing "no selector" rather than a malformed list. I am not attached to that reading — if you think it should raise too, say so on #197 and I will fold it in there rather than argue it here.

[Oaklight]

Your review landed after I merged this (9fdd409 + d8a36bc), so I am answering it on master rather than on the PR head. Nothing in it changes the merge decision — thank you for going at the parse path rather than the list corpus, that was the right axis and it is the one I would have taken.

On last_updated: confirmed on master, and it is exactly as you describe.

committed       2026-10-13T08:43:49-05:00
last-touching   d8a36bc  2026-10-13T08:44:05-05:00

I checked the other entries you listed and they are not drift — depdetect, diff, llmstxt, markdown, png, protobuf, runner, scheduler, tabulate all have committed values equal to their actual last-touching commit. So what a full regeneration churns on those is a formatting or ordering difference, not a stale timestamp, and soup is the only stale one.

It is cosmetic and self-corrects the next time anyone regenerates the manifest, which the next soup PR will do anyway. Worth noting that it recurs mechanically rather than being a one-off: the generator reads the author date of the last commit touching the file, so any PR that runs make manifest and then adds a commit touching the module file lands the same 16-second lag. I do not think a 16-second timestamp earns its own issue, but say so if you disagree and I will file it.

On your pre-existing list: select('') / select(' ') / select('\t') matching everything and :not(a, b) returning [] are both in scope for the follow-up that closes the silent-match-all class — #197 covers the first and #199 the second, and your "one line in the same function" reading of the whole-selector case is the fix I expect to see there. The escaped-quote under-match and the namespace-attribute taxonomy stay as documented.