Skip to content

Improve /api/<table> schema display and surface table-description knowls (#4811, #4812) - #22

Closed
roed-math wants to merge 5 commits into
mainfrom
ai/t18-api-schema-knowls
Closed

Improve /api/<table> schema display and surface table-description knowls (#4811, #4812)#22
roed-math wants to merge 5 commits into
mainfrom
ai/t18-api-schema-knowls

Conversation

@roed-math

Copy link
Copy Markdown
Owner

The per-table API pages (/api/<table>) get two related improvements. For LMFDB#4812, the schema table gains an Example column populated from a random row (db.<table>.random(projection=2)), with values quoted/truncated at 100 chars and [binary data] shown for bytea columns; the schema holder regains a shared schema-holder class and is now capped with max-height: 500px; overflow-y: auto in style.css instead of the dead height: 300px block left behind by an earlier div-class rename — so small tables no longer render an oversized fixed-height scroll box while large tables still scroll. For LMFDB#4811, the tables.<name> description knowl is surfaced via KNOWL_INC above the schema (read-only for everyone, with an edit link for logged-in knowl editors) instead of being appended to the page title in parentheses. Datapages share the same schema template but pass no example data, so they are unchanged apart from the holder styling; two new tests in test_api.py cover both the API-page Example column and the datapage's absence of it. Note this branch will conflict lightly at merge time with the other open API PRs (ranges/null and raw-format) since they also edit api.py/collection.html/test_api.py in nearby-but-distinct regions. Addresses LMFDB#4811 and LMFDB#4812.

🤖 Generated with Claude Code

…wls (LMFDB#4811, LMFDB#4812)

LMFDB#4812: give the schema table an Example column populated from a random
row (db.<table>.random(projection=2)), with values truncated at 100
chars, string reprs quoted, and "[binary data]" for bytea columns. The
holder div regains a shared `schema-holder` class and style.css caps it
with `max-height: 500px; overflow-y: auto` (via max-height so small
tables no longer get an oversized fixed-height scroll box) instead of the
dead `height: 300px` block, which no longer matched after the div class
was renamed. The duplicate `id="schema-table"` becomes a class.

LMFDB#4811: stop appending the table description to the page title and instead
surface the `tables.<name>` knowl via KNOWL_INC in collection.html
(read-only content for everyone, edit link for logged-in knowl editors).

Verified: sage -python -m pytest lmfdb/api/test_api.py (8 passed, incl.
two new tests); pyflakes clean on api.py and test_api.py; test-client
render of nf_fields, ec_curvedata, mf_hecke_traces, lat_lattices,
gps_groups and char_dirichlet shows a populated Example column, the
description knowl above the schema, and a title without the parenthetical;
datapages still render with no Example column; /style.css serves 200.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…DB#4812)

The /api/<table> schema Example column was sampled with
coll.random(projection=2).  Switch to projection=3, the same full
mask (id + all search + all extra columns) the datapage query already
uses, so the Example column is sampled consistently and every row shown
in the schema table can be populated.

Strengthen test_api_schema_display: instead of only asserting that
schema-example cells exist, assert that the Example cell for the
never-null search column nf_fields.degree carries an actual value, so a
future change that blanks search-column examples is caught.

Verified: sage -python -m pytest lmfdb/api/test_api.py -> 8 passed;
pyflakes clean; test-client renders of /api/nf_fields,
/api/ec_curvedata and /api/maass_newforms show nonblank Example cells
for search columns (degree, conductor, conrey_index) and the extra
column (maass_newforms.coefficients).

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

Copy link
Copy Markdown
Owner Author

Addressed the external review's P2 finding (Example column projection): commit eae9585 changes the sample to random(projection=3) and strengthens the test to assert a known search-column Example (nf_fields.degree) is nonblank.

Honest note on the diagnosis: against the pinned psycodict (roed314/psycodict 0.1.13), projection=2 already returns search+extra columns, so search-column Examples were not actually blank — the reviewer's mechanism did not reproduce here. The 2→3 change is still the right mask (it matches the datapage query and is robust to psycodict variants where 2 means extra-only), and the new nonblank assertion guards the behavior either way.

roed314 and others added 2 commits August 4, 2026 17:28
…a-knowls

# Conflicts:
#	lmfdb/api/api.py
#	lmfdb/api/templates/apischema.html
…MFDB#4811)

The tables.<table> knowl was included on /api/<table> without a heading,
so the page lost the description text that used to sit in its title
without gaining the header-backed, editable knowl the issue asks for.
Three things stood in the way, all fixed here.

Knowl.__init__ overwrote every type 2 title with a generated string, so
a custom title saved on a tables.<table> knowl was discarded on load.
Generated titles are what we want for columns.<table>.<column>, so keep
those; for a table description, use the stored title and only fall back
to "Table <table>" when the record has none.  The defunct marker is
still appended, but not a second time when a title saved while the table
was defunct already carries it.

knowl-edit.html showed every type 2 title as read-only text plus a
hidden input, and offered a "Postgres column type" row that is None for
a table.  Split the two cases apart by source (a column description
records its table, a table description has none): a table description
now gets a labelled, editable title input and no column type row, while
a column description is unchanged.

KNOWL_INC emitted no heading when called without an explicit title, with
the knowl's own title left commented out.  Add an opt-in show_title
argument that renders it, which leaves the other 42 call sites alone and
takes the title from the same Knowl object as the content, so the two
cannot come from different versions.  collection.html now calls it with
htag='h2', show_title=True.  A missing knowl is untouched: anonymous
users still see nothing rather than an empty heading, and authenticated
users still get the create link.

Tests: test_api_table_description asserts the heading is present, is
derived from the knowl rather than hard coded, and precedes both the
description and the schema controls; DescriptionKnowlTest covers the
model (custom title kept, generated fallback, defunct marker not
doubled, column titles still generated) and renders knowl-edit.html for
both kinds to check the editable/read-only split and the column type
row.  All four fail on the previous head.  Also fixed the stale
assertion in test_api_schema_display, which looked for a <title> prefix
that never matches.

Verified: sage -python -m pytest lmfdb/api/test_api.py
lmfdb/tests/test_dynamic_knowls.py -> 22 passed; pyflakes clean; a
test-client render of /api/artin_old2new_labels (a table with no
description knowl) shows an empty description div and no bogus heading.

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

Copy link
Copy Markdown
Owner Author

Thanks for the review. Pushed ee561c5 addressing the blocking gap: the table description now has a title that is both displayed and genuinely editable.

1. Knowl.__init__ no longer discards a stored table title (lmfdb/knowledge/knowl.py)

Column descriptions keep their generated Column <col> of table <table> title and their coltype, exactly as before. For a two-part tables.<table> id we now use the title loaded from the record and only fall back to Table <table> when the record has none, so a custom title survives a load. The defunct marker is still appended, with one addition: it is not appended a second time when the stored title already ends in (DEFUNCT). That case is real rather than hypothetical, since the current code writes the generated title back on save, and devmirror already has rows like tables.belyi_galmaps_new -> Table belyi_galmaps_new (DEFUNCT).

2. The editor tells the two kinds of type 2 knowl apart (lmfdb/knowledge/templates/knowl-edit.html)

Using the k.source distinction you suggested, via two named flags at the top of the template so the intent is readable at each use site. A table description gets a normal editable title input labelled Title; a column description keeps its read-only text plus hidden input under the existing Description knowl label. The Postgres column type row is now rendered only for column descriptions, so the table editor no longer shows None.

3. KNOWL_INC grew an opt-in show_title (lmfdb/templates/knowl-defs.html)

As proposed, defaulting to False, so the other 42 call sites are untouched (all of them pass everything but the id by keyword, so the new trailing parameter is safe). The title comes from the same Knowl object as the content, which avoids the cached-title-from-a-different-version problem. The commented-out {{ knowl.title }} line is gone, since it is now implemented rather than disabled. collection.html calls it as KNOWL_INC('tables.'+table, htag='h2', show_title=True).

Missing-knowl behavior is unchanged, as that branch was not touched: rendering /api/artin_old2new_labels (one of 45 tables with no description knowl) anonymously gives an empty description div, no bogus heading, and authenticated users still get the create link.

4. Tests

test_api_table_description in lmfdb/api/test_api.py asserts the heading is present, that it precedes both the description body and the schema controls, and that the content still renders. The expected title and content are read from the knowl rather than hard-coded, as you suggested. New DescriptionKnowlTest in lmfdb/tests/test_dynamic_knowls.py covers the model (custom title kept, generated fallback, defunct marker not doubled, column titles still generated with the right coltype) and renders knowl-edit.html for both kinds to check the editable/read-only split and the column-type row. I put it in an existing test file on purpose: CI has a hard-coded test-file count and an explicit per-group file list, so a new module would have meant touching .github/workflows/ too.

All four new tests fail on f9a5ea7 and pass on ee561c5. I also fixed a stale assertion in test_api_schema_display, which looked for <title>Database - <table> ( when the real markup is <title>LMFDB - Database - ..., so it could never have caught the regression it was guarding.

Verification

sage -python -m pytest -q lmfdb/api/test_api.py lmfdb/tests/test_dynamic_knowls.py
-> 22 passed

Also ran the rest of the CI group that contains test_dynamic_knowls.py (test_root.py, test_code_coverage.py, test_homepage.py, test_template_namespace.py, test_browse_page.py) -> 28 passed, 2 skipped, and pyflakes is clean. Visually confirmed against devmirror: /api/nf_fields shows an h2 reading Table nf_fields above the Number fields description and the schema controls, and the Example column still populates.

One note on what this does not do, matching your non-goals: no data migration. All 184 existing tables.* records still hold the generated Table <name> title, so that is what the headings read today, and editors can improve them one at a time. nice_name, status, contact and code are untouched.

)

Making the title editable exposed a way to lose it.  Whenever a table
description title becomes something an editor chose rather than a string
we regenerate, every writer has to carry it, and set_table_description
did not: it built its Knowl from content and defines alone, so the new
constructor supplied the "Table <table>" fallback and KnowlBackend.save
persisted that as the title.  most_recent merges authors, not the title.

The effect was that db.<table>.description("...") -- a content-only
update, typically from a data-loading script -- would silently reset a
title an editor had set.  Fetch the old record before building the data
dict, which set_table_description already did for the authors, and pass
its title through.  A table description with no record yet still gets
the generated fallback.

set_column_description needs no such change: column titles are always
regenerated from the id, so there is nothing there to lose.

Tests: test_content_update_keeps_table_title mocks out the database
write (get_knowl, save and db.login are patched, so nothing touches the
read-only connection) and asserts the knowl handed to save keeps the old
title while taking the new content; test_content_update_generates_
missing_table_title covers the no-previous-record case.  The first fails
on ee561c5 with 'Table nf_fields' == 'Number field data'.

Verified: sage -python -m pytest -q lmfdb/tests/test_dynamic_knowls.py
lmfdb/api/test_api.py -> 25 passed; pyflakes clean.

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

Copy link
Copy Markdown
Owner Author

Confirmed and fixed in 16b27c2. You were right about both the mechanism and the consequence.

set_table_description built its knowl from content and defines only, so with the new constructor it picked up the Table <table> fallback, and save wrote that as the title. most_recent merges authors and nothing else. So db.nf_fields.description("...") from a data-loading script would have quietly reset an editor's title. That is a regression my previous commit introduced: before it, the title was regenerated on every load anyway, so there was nothing to lose. Making the title editable is what put the burden on every writer to carry it.

The fix moves the get_knowl call above the data dict, which set_table_description was already making for the authors, and passes the old title through. No extra query. A table description with no record yet still gets the generated fallback.

I left set_column_description alone: column titles are always regenerated from the id, so there is nothing there to lose. yaml_import already carries title explicitly on both its save paths (it refuses to create a knowl without one), and port_column_knowls goes through Knowl.copy, which keeps the whole record, so those are fine.

Tests, as you asked, with the write mocked: test_content_update_keeps_table_title patches get_knowl, save and db.login, so nothing touches the read-only connection, and asserts the knowl handed to save keeps the old title while taking the new content. It fails on ee561c5 with assert 'Table nf_fields' == 'Number field data'. test_content_update_generates_missing_table_title covers the no-previous-record case, so the fallback stays exercised through this path too.

sage -python -m pytest -q lmfdb/tests/test_dynamic_knowls.py lmfdb/api/test_api.py
-> 25 passed

pyflakes clean.

Unrelated, found while checking the other write paths and left alone here: LMFDBSearchTable.port_column_knowls calls knowldb.get_column_description(other_table), but the method is get_column_descriptions, plural. That looks like it would raise AttributeError on any call. It predates this PR and is in a separate admin helper, so I have not touched it, but it may be worth its own issue.

@roed314

roed314 commented Aug 5, 2026

Copy link
Copy Markdown

GPT signed off.

@roed-math

Copy link
Copy Markdown
Owner Author

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