Hide ui.-prefixed knowls from the public knowl index - #7160
Open
roed-math wants to merge 4 commits into
Open
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 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the code side of #3721. Deliberately no
Closes: the convention still needs the knowls themselves renamed in the database, so the issue should stay open until that lands.Implements the
ui.naming convention proposed in #3721 for user-interface helper knowls (search-box help 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-out hiding (including the category and?category=uipaths) and the logged-in visibility.No existing knowls start with
ui.yet. A proposed list of ~36 high-confidence knowls to rename into the namespace (plus largerrcs.*/dq.*/doc.*families to consider) is recorded on the old PR for admins to apply in the database, since renames also require updating the corresponding code references.Ported from roed-math#24, where the full write-up and comment history live.
🤖 Generated with Claude Code