Fix AttributeError in port_column_knowls

[roed-math]

db.<table>.port_column_knowls(other_table) calls knowldb.get_column_description(other_table), but the knowl backend only ever defined get_column_descriptions (plural, lmfdb/knowledge/knowl.py:495). Every call raised

AttributeError: 'KnowlBackend' object has no attribute 'get_column_description'.
Did you mean: 'get_column_descriptions'?

before doing any work, so the function has never run. It is reachable from the command line and is advertised to editors on the defunct-column-knowls page (knowl-columns.html).

This is a typo, not a stale caller left behind by a rename: get_column_descriptions was already plural in 017ab53, one day before b354cb5 added port_column_knowls calling the singular. Nothing else in the repo references the singular name, and psycodict defines neither name nor any __getattr__ fallback on PostgresBase.

The plural version is clearly the intended target. It returns a dict mapping column name to Knowl, which is exactly how the caller consumes it (for col, knowl in knowls.items()).

Tests

PortColumnKnowlsTest in lmfdb/tests/test_dynamic_knowls.py runs port_column_knowls with the knowl writes mocked out, following the pattern used elsewhere in that file: knowldb.get_column_descriptions, knowldb.save, knowldb.actually_rename, knowldb.delete and db.login are patched, so nothing touches the read-only devmirror connection.

Three tests cover the backend call itself, the keep_old=True copying branch (including that a column the target table lacks is skipped, and that the old knowl is passed as most_recent so its authors survive), and the keep_old=False branch (rename the shared column, delete the one this table does not have).

All three fail with the AttributeError above on the unfixed code and pass with the fix. sage -python -m pytest -q lmfdb/tests/test_dynamic_knowls.py gives 12 passed; pyflakes and pylint are clean on both changed files.

[roed-math]

Both required changes are in, pushed as 1c14c68.

1. The current version of a description is now selected

get_column_descriptions sorted its DISTINCT ON (id) subselect by ascending timestamp, so it kept the version each description was created with. Now ORDER BY id, timestamp DESC, with the outer ordering by id unchanged.

get_table_description has the identical query shape and the identical defect, and it backs db.<table>.description(), so I fixed it in the same commit. That is one line beyond what the review asked for; say the word and I will split it out.

The keep_old=True branch needed nothing else: Knowl.copy recomputes the title from the new id and knowldb.save derives cat, type, source, source_name, defines and the keywords from it, so once the content is the current one the inserted row is coherent. That is asserted rather than assumed (see below).

2. keep_old=False is now metadata-safe

Took the first of your suggested options: a dedicated backend operation, KnowlBackend.rename_description_knowl(knowl, new_name), called by port_column_knowls. It does one UPDATE setting id, cat, type, source, source_name, title and defines on every version of the knowl, from a new module level helper description_metadata(new_name), then recomputes _keywords per version (each version has its own content), then does the referrer content and links rewrite and cache update that actually_rename does. All of it inside DelayCommit, no rows added or removed, so the edit history survives intact.

description_metadata is the single source of truth for what a type 2 knowl's id implies. I did not rewire Knowl.__init__ or save through it, to keep the diff off the save path, but there is a test pinning that the title it stores is the title Knowl computes for display.

Two related judgment calls, both easy to back out:

• rename_description_knowl refuses to move onto an id that already has rows, including deleted ones, rather than interleaving two sets of versions under one id (which is what the old code did silently). This raises inside the port's DelayCommit, so the whole port rolls back.
• actually_rename now raises for a knowl that is not normal, pointing type 2 knowls at the new method. Nothing else in the repo passed it one: the rename box in knowl-edit.html is gated on k.type == 0, and start_rename already refused.

3. Tests

The suite's database connection is read only, so no writable fixture. Rather than assert on the SQL text, the tests now stand in for KnowlBackend._execute with an in-memory kwl_knowls (FakeKnowlTable) that answers selects using the ordering of the query it is handed, and applies the inserts and updates it is given. So the port runs through the real get_column_descriptions, save, rename_description_knowl and delete, and the assertions are on the rows that end up stored.

The two mocked branch tests are replaced by fake-table versions that keep every assertion they made and add the persisted values; test_descriptions_of_the_other_table_are_requested is unchanged. Coverage, against 17 tests in the file:

• the current version is read, for a column description and for a table description, from two non-deleted versions;
• keep_old=True copies only shared columns, with the current content, the new id and title, authors carried over and the porting user only as last_author (minor=True), and leaves the other table's knowls untouched;
• keep_old=False on a description with a reviewed older version and a newer beta version: both versions keep their content, status and timestamp, both get source = nf_fields, source_name = degree, title = Column degree of table nf_fields, defines = ['degree'], cat, type, and keywords that contain nf_fields and not old_nf_fields;
• the edit page identity, via Knowl built from the stored row: Edit column information for 'degree' in 'nf_fields';
• a normal knowl referring to the old id follows it in both content and links;
• a column the target table does not have is deleted;
• on failure partway through, every write so far was made inside the transaction (checked against _nocommit_stack at each write), so it rolls back.

Both regression tests fail at the previous head. With the ascending ordering restored:

E       AssertionError: assert 'The first draft' == 'The current text'

and with port_column_knowls calling actually_rename again:

E       AssertionError: assert 'Column degre...old_nf_fields' == 'Column degre...ble nf_fields'

Validation

sage -python -m pytest -q lmfdb/tests/test_dynamic_knowls.py     17 passed
sage -python -m pytest -q lmfdb/tests/test_connection_reset.py lmfdb/tests/test_root.py lmfdb/api/test_api.py     13 passed, 3 skipped

pyflakes, pylint --score=no -d C,R,E,W -e W0129,W0108 and ruff check --preview --select=E722 are clean on all three changed files.

🤖 Generated with Claude Code

[roed-math]

Remaining change is in, pushed as 3a3240c.

Keywords are now recomputed after all content rewriting

Took the follow-up SELECT rather than RETURNING, because once the keyword pass moves to the end it can cover the renamed knowl and its referrers in one go, so the two passes collapse into one:

referrers = self.ids_referencing(old_name, old=True)
self._execute(updator, values)                       # rewrite content and links
selecter = SQL("SELECT id, timestamp, content, title FROM kwl_knowls WHERE id = ANY(%s)")
updator = SQL("UPDATE kwl_knowls SET _keywords = %s WHERE id = %s AND timestamp = %s")
for kid, timestamp, content, title in self._safe_execute(selecter, [[new_name] + referrers]):
    self._execute(updator, [make_keywords(content, kid, title), kid, timestamp])

Every version of every affected knowl is reindexed from what is stored after the rewrite, using that row's own id, title and content. The self-reference case falls out of this: ids_referencing runs after the id update, so a description referring to its own old id comes back in referrers under the new id, its content is rewritten with everyone else's, and the single keyword pass then indexes it from the rewritten content. The duplicate id in [new_name] + referrers is harmless to = ANY.

Tests

Three new tests, all porting from oldtable so the old name is a keyword token, each asserting the content and links rewrite, row["_keywords"] == make_keywords(row["content"], row["id"], row["title"]), and that oldtable is gone:

• a normal knowl referring to columns.oldtable.degree;
• a description referring to its own old id;
• columns.oldtable.class_number referring to columns.oldtable.degree, which is the multi-column ordering case: class_number is ported and indexed first, and its content is only rewritten later, while degree is being moved.

All three fail at 1c14c68, for example:

E       AssertionError: assert ['degree', 'n... 'knowl', ...] == ['degree', 'n...'nf', 'knowl']
E         Left contains one more item: 'oldtable'

I also added the same keyword-consistency assertion to the existing referrer test in test_renames_shared_columns_when_not_keeping_old, but it is not a regression guard: that one ports from old_nf_fields, and make_keywords extracts no token at all from old_nf_fields (the underscores block the \b in text_keywords), so its keywords happen to be unchanged by the rewrite. That is exactly the point of your oldtable note, and the reason the three new tests use it.

One related gap, left alone

actually_rename has the same shape of problem for normal knowls: it rewrites referrers' content without touching their _keywords, and reindexes only two versions of the knowl being renamed. That is the pre-existing normal-rename path, outside what this PR touches, so I left it. Happy to fix it here or in a separate PR if you want it.

Validation

sage -python -m pytest -q lmfdb/tests/test_dynamic_knowls.py     20 passed

pyflakes, pylint --score=no -d C,R,E,W -e W0129,W0108 and ruff check --preview --select=E722 clean on all changed files.

🤖 Generated with Claude Code

[roed314]

GPT signed off.

[roed-math]

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