Skip to content

Add a maximal subgroups mode to the subgroup diagram and profile (LMFDB#6891) - #37

Closed
roed-math wants to merge 3 commits into
mainfrom
ai/t35-maximal-subgroups-mode
Closed

Add a maximal subgroups mode to the subgroup diagram and profile (LMFDB#6891)#37
roed-math wants to merge 3 commits into
mainfrom
ai/t35-maximal-subgroups-mode

Conversation

@roed-math

Copy link
Copy Markdown
Owner

Adds a "maximal subgroups" button next to "all subgroups"/"normal subgroups" on abstract group pages, as
suggested in the issue. The new mode works for both the diagram and the profile views, crossed with the
existing up-to-conjugacy/up-to-automorphism toggle, and gets its own fullpage diagram routes. The diagram
shows the whole group on top with the classes of maximal subgroups below it, built directly from the stored
maximal column, so it displays even for groups whose full subgroup diagram is disabled for size (e.g.
32.45, 1024.dke); for groups where inclusions were never computed (e.g. 1024.dip) the profile still lists
all maximal subgroup classes. Verified on S4, PGL(2,7), p-groups with huge/partial/missing lattices, and the
trivial group; abstract-groups test suite passes and a new test covers the mode.
Addresses LMFDB#6891.

🤖 Generated with Claude Code

roed314 and others added 3 commits July 19, 2026 13:41
Adds a "maximal subgroups" button next to "all subgroups" and "normal
subgroups" on abstract group pages, working for both the diagram and the
profile views and crossed with the up-to-conjugacy/up-to-automorphism
toggle. The diagram shows the whole group above the classes of maximal
subgroups, built from the stored maximal column, so it displays even for
groups whose full subgroup diagram is disabled for size (32.45,
1024.dke), and the profile lists maximal subgroups even when inclusions
were never computed (1024.dip). Adds /maximal_diagram/<label> and
/maximal_autdiagram/<label> fullpage routes and generalizes the
mode-toggle javascript from pairs to option groups.

Verified with the abstract groups test suites (test_abstract_groups.py 8
passed including a new test, test_browse_page.py 71 passed), pyflakes, and
by exercising all mode combinations in a browser on 24.12, 32.45,
1024.dke, 1024.dip, 128.2328, 336.208 and 1.1 with no JS console errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The added tests could pass with no maximal graph generated: they matched
the header divs that abstract-show-group.html emits for every group, empty
slot or not, and the fullpage route titles, which render even when
diagram_js_string() returns "".

Test the graphs instead.  A helper parses the make_sdiagram payload out of
a page, so a test can assert which of the six graph slots is filled and
with which nodes and edges.  We now check the node and edge sets of
subgroup_lattice("maximal", "") and ("maximal", "aut") for 32.45, where 15
classes of maximal subgroups fuse into 2 up to automorphism while the
118-class subgroup diagram is suppressed; the slots embedded in the group
page; the payload of both fullpage routes for 24.12 and 32.45; and the
maximal profile of 1024.dip, whose subgroup inclusions were never computed.

Verified by mutation: ten deliberate defects (maximal slots never filled,
whole group dropped, a maximal class omitted, a nonmaximal class included,
a missing star edge, reversed star edges, no deduplication by aut_label, a
fullpage title with no payload, the profile lost when inclusions are
unknown, altered 24.12 profile contents) each make the tests fail.  The
button matrix was also exercised in a browser on all three groups.

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

Copy link
Copy Markdown
Owner Author

Addressed the review's required change: the maximal-diagram tests could pass with no maximal graph generated. They now test the graphs themselves.

What the old assertions proved. "maximal_diagram" in page only matched the header <div> that abstract-show-group.html emits for every group, empty slot or not, and the fullpage assertions only matched the route title, which renders even when diagram_js_string() returns "". Both survive an entirely empty diagram.

What the tests do now (lmfdb/groups/abstract/test_abstract_groups.py, split into four test_maximal_subgroups* methods):

  • A helper parses the make_sdiagram("subdiagram", ...) payload out of a page, so a test can assert which of the six graph slots is filled and with exactly which nodes and edges.
  • A. Conjugacy structure (32.45): diagram_count("subgroup", "", limit=100) == 0 while diagram_count("maximal", "", limit=100) == 16; subgroup_lattice("maximal", "") has exactly one node with quotient_order == 1, exactly 15 maximal nodes, and its edge set equals {(H.short_label, top.short_label) for H in maxima}, so a missing, extra, reversed or spurious edge fails.
  • B. Automorphism semantics (32.45 is itself the fusion example): len(aut_nodes) == diagram_count("maximal", "aut") == 3, one node per aut_label, the aut node set covers every aut_label of the conjugacy graph, and 3 < 16 shows the aut mode is not aliasing the conjugacy graph.
  • C. Fullpage payloads (24.12 and 32.45, both routes): status 200, the expected show_info("maximal_[aut]diagram"), exactly one filled slot (4 for conjugacy, 5 for automorphism), the star shape, and the node set equal to {H.short_label for H in G.subgroups.values() if H.maximal or H.quotient_order == 1} (conjugacy) or one representative per aut_label (automorphism). A title-only response fails.
  • D. Profile without inclusions (1024.dip): maximal_subgroups_known and not subgroup_inclusions_known, no diagram payload on the page, and the profile lists the 255 classes of order 512 (C_2^6:D_4 x 248, D_4 x C_2^6 x 3, C_2^8.C_2 x 3, C_2^9) without saying "not computed".
  • E. The 24.12 profile assertions (A_4, D_4, S_3) are kept, and now also checked on the automorphism profile.

Mutation check. Each acceptance criterion was turned into a real defect in web_groups.py/main.py; all ten fail the tests: maximal slots never filled, whole group dropped, a maximal class omitted, a nonmaximal class included, a missing star edge, reversed star edges, no deduplication by aut_label, fullpage title with no payload, profile lost when inclusions are unknown, and altered 24.12 profile contents.

F. Browser check (local server, DOM state rather than screenshots, since the preview pane could not paint): on /Groups/Abstract/24.12, /32.45 and /1024.dip all twelve button combinations (all/normal/maximal x conjugacy/automorphism x diagram/profile) show the intended div and load the intended graph slot (0-5), with exactly one button active per group. The order-for-height checkbox moves the slot by exactly glist.length / 2 = 6 in every mode and keeps the same node set, and unchecking returns to the original slot; the same holds on both fullpage maximal diagrams, where the canvas actually paints. 32.45 behaves as expected with its subgroup and normal-conjugacy diagrams suppressed and both maximal diagrams drawn; 1024.dip shows the maximal profile with no diagram. No new console errors.

One pre-existing observation, not touched here: on a group with no diagram at all (1024.dip), clicking any diagram button throws ReferenceError: sdiagram is not defined from show_info, because that line is unguarded. It is identical on main (the mode still switches, since the visibility change happens first), so it is out of scope for this PR.

Verification:

sage -python -m pytest lmfdb/groups/abstract/test_abstract_groups.py -k maximal_subgroups -q   # 4 passed
sage -python -m pytest lmfdb/groups/abstract/test_browse_page.py lmfdb/groups/abstract/test_abstract_groups.py -q   # 82 passed (the CI job's target)
pyflakes / pylint / ruff on the changed file: clean

Labels used: 15-class maximal graph and automorphism fusion, 32.45; maximal profile without inclusions, 1024.dip; profile contents and star shape, 24.12.

@roed314

roed314 commented Aug 5, 2026

Copy link
Copy Markdown

GPT signed off.

@roed-math

Copy link
Copy Markdown
Owner Author

Superseded by LMFDB#7171, 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