fix(wiki): skip uppercase markdown suffixes so a .MD file cannot break every query - #66
fix(wiki): skip uppercase markdown suffixes so a .MD file cannot break every query#66Hotragn wants to merge 2 commits into
Conversation
…k every query
`iter_page_paths` globs `rglob("*.md")`, which is case-insensitive on Windows and
on default macOS volumes, so it also matches "NOTES.MD" and "Mixed.Md".
`page_id_for_path` then compares the suffix exactly and raises
`ValidationFailed("wiki page must be markdown")`.
`load_page_document` calls `page_id_for_path` unguarded, so the error propagates
through `load_documents` -> `load_index_sources` -> the implicit reindex that
every query command performs. One uppercase-suffixed file anywhere under
`almanac/` therefore takes down `search`, `show`, `health` and `reindex`:
$ codealmanac search notes
codealmanac: wiki page must be markdown: ...\almanac\NOTES.MD
This is not Windows-only. macOS is case-insensitive by default, so it reproduces
on the one platform the project currently supports. Linux never matched these
files, which is why CI is green.
The producer now yields only exact ".md" suffixes, so all three platforms derive
the same page set from the same wiki tree. Fixing `iter_page_paths` rather than
each caller covers the index, health and frontmatter-rewrite paths at once.
Whether ".MD" should instead be *accepted* as a page is a product call, not a bug
fix, so this change keeps the Linux behaviour of ignoring it. Happy to follow up
if you would rather canonicalize the suffix the way slugs already are.
The two tests added in the previous commit only reach the new filter on a
case-insensitive filesystem. On Linux `rglob("*.md")` never returns "NOTES.MD",
so both assertions held without the guard ever executing — meaning the fix was
untested on the only platform CI runs.
Replaces the weaker of the two with one that feeds the case-insensitive match in
directly, so it exercises the filter regardless of the host filesystem. Verified
by removing the guard: the new test fails, and it fails for the right reason
rather than because of the platform it happens to run on.
|
Pushed 4474b05 to fix a real weakness in my own tests, before anyone spends review time on them. Both tests I originally added only reach the new filter on a case-insensitive filesystem. On Linux, The replacement feeds the case-insensitive match in directly, so it exercises the filter regardless of the host filesystem: monkeypatch.setattr(Path, "rglob", lambda self, pattern: iter((page, upper)))
iterated = tuple(iter_page_paths(almanac_path))
assert iterated == (page,)
for path in iterated:
assert page_id_for_path(almanac_path, path)Verified it pins rather than describes — with the guard removed: The second of those is the one that will now also fail on Linux. I kept the filesystem-based test alongside it, since it covers the real Windows/macOS glob path end to end. The asserted invariant is deliberately "everything Also re-confirmed the end-to-end symptom is gone, on the same wiki that reproduced it ( Before the fix that first command exited non-zero with No change to |
Summary
iter_page_pathsglobsrglob("*.md"), which is case-insensitive on Windows and on default macOS volumes, so it also matchesNOTES.MDandMixed.Md.page_id_for_paththen compares the suffix exactly and raises, andload_page_documentcalls it unguarded — so one uppercase-suffixed file takes down every command that reindexes..mdsuffixes, so all three platforms derive the same page set from the same wiki tree.Why
This reproduces on macOS, the one platform the project currently supports. APFS is case-insensitive by default, so the glob matches a file the validator will reject.
End-to-end repro — a wiki containing a single
NOTES.MD:The failure path is
load_page_document→page_id_for_path(raisesValidationFailed) →load_documents→load_index_sources→ the implicit reindex that every query command performs. Sosearch,show,healthandreindexall fail, not just one command.load_documentshas adocument is None→files_skippedpath, but an exception bypasses it.Linux never matched these files, which is why CI is green — this is the same Ubuntu-only CI blind spot that hides the
launchdbehaviour.Verification
The two added tests are net-new coverage; this fix does not flip an existing failing test, which is precisely why the bug survived — nothing exercised a non-
.mdsuffix.Docs and wiki
Notes for reviewers
iter_page_pathshas four callers (index/sources.py,health/sources.py,wiki/frontmatter_rewrite.py), and all three subsystems inherit the fix. Filtering in each caller would be the same rule written three times.rglob("*.md")never matched.MDthere, so the page set is unchanged; this only brings Windows and macOS in line with it.test_page_ids_resolve_for_every_iterated_pageis the durable guard. It asserts the producer/validator agreement directly rather than hard-coding one bad suffix, so it also catches any future divergence between the two..MDshould be accepted as a page rather than skipped. That changes indexing semantics and is a product call — the existing non-negotiable that "slugs are kebab-case of the filename" suggests you may well want canonicalization instead. I kept the Linux behaviour (ignore) so this stays a pure bug fix; happy to send the canonicalizing version if you prefer it.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.