Skip to content

Allow multiple intermediate fields in number field search (LMFDB#6827) - #35

Closed
roed-math wants to merge 3 commits into
mainfrom
ai/t38-nf-multi-subfield
Closed

Allow multiple intermediate fields in number field search (LMFDB#6827)#35
roed-math wants to merge 3 commits into
mainfrom
ai/t38-nf-multi-subfield

Conversation

@roed-math

@roed-math roed-math commented Jul 19, 2026

Copy link
Copy Markdown
Owner

The number field search's "Intermediate field" box previously accepted a single subfield. It now
accepts a comma-separated list of subfields (polynomials, field labels, or nicknames, freely mixed),
returning number fields that contain every listed subfield — i.e. the AND of the containment
conditions, equivalently fields containing their compositum. This makes it easy to search for fields
such as Q(√2,√3,√5) or Q(∜2,∛5) without first computing a defining polynomial for the compositum.
Implementation-wise, parse_subfield splits on commas and parses each piece with the existing
input_to_subfield helper, emitting a single {"$contains": [...]} on the text[] subfields
column; this is the same @> containment operator the single-field search already used, so
single-subfield behavior is unchanged. An empty entry (a leading, trailing or repeated comma) is
rejected with a flashed error rather than dropped, since dropping it would silently broaden the
search. Help text was added, along with tests that check the parsed query directly (single entry,
list by polynomial, list by label, mixed formats, ordering, and each malformed or empty input) plus
one bounded end-to-end search confirming the AND semantics.
Addresses LMFDB#6827.

🤖 Generated with Claude Code

roed314 and others added 3 commits July 19, 2026 13:11
The "Intermediate field" search box now accepts a comma-separated list of
subfields (polynomials, field labels, or nicknames, freely mixed). The search
returns number fields containing every listed subfield, i.e. the AND of the
containment conditions (equivalently, fields containing their compositum).
This makes it easy to search for e.g. Q(sqrt2, sqrt3, sqrt5) without first
computing a defining polynomial for the compositum.

parse_subfield splits the input on commas and parses each piece with the
existing input_to_subfield helper, emitting {"$contains": [...]} on the
text[] subfields column. This compiles to a single "subfields @> ARRAY[...]"
Postgres containment test (no per-row recomputation) -- the same operator the
single-subfield search already used, so single-field behavior is unchanged.
Whitespace is stripped by the search parser, so "x^2-2, x^2-3" works too. A
malformed entry raises the existing clean SearchParsingError. Help text and a
test were added.

Verified with the flask test client: single label/polynomial (unchanged),
multiple polynomials and multiple labels both returning the compositum, three
quadratics correctly excluding the degree-4 field, and malformed input giving
a flashed error rather than a 500.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…directly

Review follow-up to the comma-separated "Intermediate field" search.

parse_subfield dropped list entries that parsed to None, so "x^2-2," and
",x^2-2" were silently treated as "x^2-2", "x^2-2,,x^2-3" as two entries, and
"," dropped the subfields constraint altogether, turning the request into an
unfiltered number field search. Every entry is now required to be nonempty,
both before parsing and after input_to_subfield has removed unsupported
characters; an empty one raises the usual SearchParsingError and is flashed.

The new tests call the parser directly and compare the query dictionary, so
they pin the semantics (one $contains holding every entry, in input order,
rather than the first entry alone or an OR) without a database search, and
check that each bad input raises and leaves the query untouched. The
integration test now bounds degree and discriminant, which makes it decisive
instead of timeout-tolerant and cuts it from about 626s on CI to about 5s:
among quartic fields of discriminant at most 3000 the search for x^2-2 returns
both 4.4.2304.1 and 4.0.256.1, while the search for x^2-2,x^2-3 returns only
4.4.2304.1.

Verified: the three tests pass, and each fails if parse_subfield is mutated to
keep only the first entry, to emit an $or, or to drop empty entries as before.
pyflakes and the repository pylint check are clean on all changed files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@roed-math

roed-math commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

Both requested changes are in 17c6320.

1. Empty list entries are now rejected. parse_subfield used to drop entries that parsed to None, so x^2-2, and ,x^2-2 were silently treated as x^2-2, x^2-2,,x^2-3 as a two-entry list, and , dropped the subfields constraint entirely, turning the request into an unfiltered number field search. Every entry is now required to be nonempty, both before parsing and after input_to_subfield has removed unsupported characters, and an empty one raises the usual SearchParsingError ("Entries in the comma-separated list must be nonempty."), flashed like any other search box error. The single-field path, the single $contains list representation, mixing labels/polynomials/nicknames, the existing error for a malformed entry such as notafield, and the help text are all unchanged.

2. The tests are now decisive and fast. test_parse_subfield and test_parse_subfield_errors call the decorated parser directly and compare the resulting query dictionary, with no database search:

  • x^2-2 gives the pre-existing scalar {"subfields": {"$contains": "-2.0.1"}};
  • x^2-2,x^2-3 gives one {"$contains": ["-2.0.1", "-3.0.1"]}, and x^2-3,x^2-2 the reverse order, so first-entry-only and OR behavior are both ruled out;
  • labels, nicknames and polynomials mix (2.2.8.1,Qsqrt3) and whitespace is stripped;
  • ,, x^2-2,, ,x^2-2, x^2-2,,x^2-3, x^2-2,é (empty only after non-ASCII stripping) and x^2-2,notafield each raise SearchParsingError inside a request context and leave the query dictionary empty.

test_search_subfield keeps one end-to-end semantic check, but bounded by degree and discriminant so it no longer needs check_args_with_timeout: among quartic fields of discriminant at most 3000, subfield=x^2-2 returns both 4.4.2304.1 and 4.0.256.1, while subfield=x^2-2,x^2-3 returns 4.4.2304.1 and not 4.0.256.1. The single-field search is the control that makes the negative assertion meaningful, since it shows 4.0.256.1 is in range and would appear if the second entry were ignored. It also checks that both a malformed and an empty entry give a clean error page.

The bounds matter for runtime, since the sort plus LIMIT scans until it fills a page and a two-entry containment matches very few rows. Measured against devmirror: unbounded subfield=x^2-2,x^2-3 510s, degree=4 alone 191s, degree=4&discriminant=1-3000 2.5s. Single-entry searches were never the problem (1.4s with degree=4) because matches are dense.

Verification

  • sage -python -m pytest lmfdb/number_fields/test_numberfield.py -k subfield : 3 passed, with test_search_subfield at 5s versus roughly 626s on the reviewed run.
  • Mutation checks, confirming the tests catch a weaker parser: keeping only the first entry fails all three tests; emitting an $or fails test_search_subfield; restoring the old drop-empty-entries behavior fails test_parse_subfield_errors and test_search_subfield.
  • The whole test file passes: 37 tests, no failures. It had to be run in two batches locally (30 tests including all three subfield tests, then the remaining 7) because this box is saturated with other test runs. CI runs the file against both devmirror and proddb.
  • pyflakes, the repository's pylint --score=no -d C,R,E,W -e W0129,W0108 invocation, and ruff check --preview --select=E722 are clean on the changed files. No new test file, so the workflow's test-file count is unchanged.

The remaining slowness is a database-side matter and is now filed separately as LMFDB#7136: nf_fields.subfields has no index at all, unlike ramps and local_algs which have GIN indexes, so a selective containment makes the ordered scan walk most of the 22.2M rows. That affects single rare subfields too, not just lists.

@roed314

roed314 commented Aug 5, 2026

Copy link
Copy Markdown

GPT signed off.

@roed-math

Copy link
Copy Markdown
Owner Author

Superseded by LMFDB#7169, opened upstream from this same branch. Closing here; review continues upstream.

@roed-math roed-math closed this Aug 6, 2026
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.

2 participants