Hide ui.-prefixed knowls from the public knowl index (#3721) - #24
Hide ui.-prefixed knowls from the public knowl index (#3721)#24roed-math wants to merge 4 commits into
Conversation
Knowls whose id starts with "ui." are user-interface helper texts (search-box help bubbles, sort-order and statistics-extent explanations, section landing-page intros) rather than context-free definitions, and should not clutter the public knowl browse/search index. In lmfdb/knowledge/main.py index(), when the visitor is not logged in, drop knowls whose id starts with "ui." from the search results before categories are counted, so the "ui" category never appears publicly; also reset an explicit ?category=ui request to the empty category so no empty "ui" bucket is seeded. Logged-in editors still see everything. Verified with a logged-out flask test client: new lmfdb/knowledge/ test_knowledge.py patches knowldb.search to inject a synthetic ui.* knowl and asserts it (and its category) are hidden when logged out and shown when authenticated; the real /knowledge/ index still returns 200 with no "ui" category. pyflakes clean; existing test_dynamic_knowls tests still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The PR added lmfdb/knowledge/test_knowledge.py but left the Tests workflow asserting exactly 43 test files, so every matrix job failed at the census step before tests ran. Bump the count to 44 and assign the new module to the knowls shard (alongside test_dynamic_knowls.py) in both the proddb and devmirror matrix entries so it is actually run. Verified: replicated census command (find ... | wc -l == 44) passes; matrix_includes.json parses and lists the file in both shards; sage -python -m pytest lmfdb/knowledge/test_knowledge.py -> 4 passed; pyflakes clean. Note: open PR #4 (ai/t07-negative-ranges) applies the same census bump for its own new test file; the second to merge re-bumps to 45 (trivial same-line conflict, intentional). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The T07 PR added lmfdb/utils/test_search_parsing.py but left the CI test-file census asserting 43 files, so every matrix job failed at the "checking that we didn't miss any test files" step before running any tests or linting. - python-package.yml: bump census count 43 -> 44. - matrix_includes.json: assign the new module to the shard that already runs lmfdb/tests/test_utils.py (both proddb and devmirror copies) so its tests actually execute; add "utils" to that shard's display folders. Coordination: open PR #24 (ai/t28-ui-knowls) adds another new test file needing the same fix. Each PR bumps the census by one for its own file, so whichever of #4/#24 merges second must bump again (44 -> 45): a trivial, intentional same-line conflict on the census line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the external review's P1 finding (CI test-file census): commit 0b04e3e bumps the census 43→44 and registers Coordination note: PR #4 makes the identical 43→44 bump for its own new test file — whichever merges second must re-bump to 45 (deliberate same-line conflict). |
The PR added lmfdb/knowledge/test_knowledge.py to matrix_includes.json but not to the two places that keep CI bookkeeping consistent: - .github/workflows/python-package.yml still asserted 44 test files, so every test-matrix job would fail at the census step before running its assigned tests. The branch now has 45 matching files. - .github/workflows/generate-matrix.py did not list the new test, so the checked-in matrix and the script that produces it disagreed; regenerating later would have silently dropped the test from its shard. Added the file to the test_dynamic_knowls group in generate-matrix.py in the same position it already occupies in matrix_includes.json, and bumped the census to 45. matrix_includes.json is unchanged: it carries manual entries the Cartesian-product generator does not model (test_code_coverage.py, test_random_redirects.py, test_cmdline_search.py, the devmirror-only groups/abstract shard, and the modular_curves shard), so it was left alone rather than regenerated. Verified: census predicate returns 45; test_knowledge.py appears once in generate-matrix.py and twice in matrix_includes.json; json.tool validates the matrix; a dry-run of generate-matrix.py differs from the checked-in JSON only in those pre-existing exceptions; the 4 knowl visibility tests pass; pyflakes, pylint and ruff E722 are clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Addressed both blocking review items in 25dcef5. No changes to the Fix 1: test-file census. Bumped the expected count in Fix 2: matrix generator. Added Per the review note, I did not regenerate the matrix. I ran Verification Lint: Acceptance criteria are met; over to CI for the matrix jobs. |
|
GPT signed off. |
|
Superseded by LMFDB#7160, opened upstream from this same branch. Closing here; review continues upstream. (The proposed rename list in this thread stays the reference for the database side.) |
2 participants
Implements the
ui.naming convention proposed in LMFDB#3721 for user-interface helper knowls (search-boxhelp bubbles, sort-order and statistics-extent explanations, section landing-page intros) that are not
context-free definitions and should not clutter the public knowledge browse/search index. In
lmfdb/knowledge/main.pyindex(), logged-out visitors no longer see knowls whose id starts withui., and theuicategory never appears (an explicit?category=uirequest is also neutralized);logged-in editors are unaffected. Adds
lmfdb/knowledge/test_knowledge.pycovering the logged-outhiding (including the category and
?category=uipaths) and the logged-in visibility. No existingknowls start with
ui.yet; a proposed list of ~36 high-confidence knowls to rename into thenamespace (plus larger
rcs.*/dq.*/doc.*families to consider) is recorded on the tracking issuefor admins to apply in the database, since renames also require updating the corresponding code
references. Addresses LMFDB#3721.
🤖 Generated with Claude Code