Skip to content

fix(wiki): reserve manual/jobs only at the wiki root, not at any depth - #57

Open
dev-noaman wants to merge 1 commit into
AlmanacCode:mainfrom
dev-noaman:fix/reserved-wiki-dirs-top-level-only
Open

fix(wiki): reserve manual/jobs only at the wiki root, not at any depth#57
dev-noaman wants to merge 1 commit into
AlmanacCode:mainfrom
dev-noaman:fix/reserved-wiki-dirs-top-level-only

Conversation

@dev-noaman

Copy link
Copy Markdown

Fixes the over-broad reserved-directory check reported in #56.

The bug

is_reserved_page_path tests every directory component:

return any(part in RESERVED_WIKI_SOURCE_DIRS for part in relative.parts[:-1])

But the reserved directories are only ever created at the wiki root:

  • services/wiki/service.pyself.manual.install_missing(almanac_path / "manual")
  • workflows/build/service.pymanual_root=repository.almanac_path / "manual"

So the predicate is broader than its purpose, and any user page under a directory named manual or jobs — at any depth — is dropped from indexing, health, and frontmatter rewriting.

The failure is silent, and that is what makes it expensive: health and validate walk with the same iterator, so the tool cannot report pages it refuses to see. The only visible symptoms are a broken link pointing at a file that plainly exists on disk, and a page count that quietly disagrees with the filesystem (reindex reported 46 pages (46 files seen) against 50 .md files).

It also makes sync fail permanently — the post-run validation gate trips on the resulting broken links, so the run exits failed and no ingest can complete while such a directory exists.

In the wiki where this surfaced it hid four committed pages (~29 KB) under architecture/jobs/, and nearly caused a duplicate to be written: the coverage map listed the target as unwritten and health called the link broken — both consistent with "missing" — when the page existed and was hidden.

The change

Restrict the check to the first directory component:

directories = relative.parts[:-1]
return bool(directories) and directories[0] in RESERVED_WIKI_SOURCE_DIRS

page_id_for_path, immediately below in the same module, already reasons positionally (if len(relative.parts) == 1), so top-level-only matches the established convention here.

Behaviour:

path before after
manual/intro.md excluded excluded
manual/nested/deep.md excluded excluded
jobs/run.md excluded excluded
architecture/jobs/queue.md excluded indexed
guides/manual/setup.md excluded indexed
architecture/risk/overview.md indexed indexed
jobs.md (a file, not a directory) indexed indexed

Nested content inside the bundled manual stays excluded, because only the first component is consulted.

Tests

Adds test_page_iteration_keeps_nested_dirs_named_after_reserved_roots, which asserts a nested jobs/manual directory is indexed while a root-level manual/ is not. It fails on main and passes with this change, so it pins the regression rather than merely describing it. The existing test_page_iteration_excludes_repository_manuals continues to cover the intended root-level exclusion and is unmodified.

Checks

Per CONTRIBUTING.md:

  • uv run pytest — the failing-test set is identical before and after this change (14 both ways on this machine; all pre-existing and unrelated — 5 are macOS launchd tests running on Windows). tests/test_wiki_parsing.py goes 9 passed → 10 passed.
  • uv run ruff check . — All checks passed
  • uv build — sdist and wheel built

Note on a related rough edge

Even with this fix, a page skipped by reservation is dropped without a trace. A health notice along the lines of "N page(s) skipped: reserved directory" would make this class of problem self-diagnosing — as it stands, the only clue is a broken link to a file that exists. Happy to add that in a follow-up if you'd want it.

is_reserved_page_path tested every directory component:

    any(part in RESERVED_WIKI_SOURCE_DIRS for part in relative.parts[:-1])

but the reserved directories are only ever created at the wiki root
(WikiService: `almanac_path / "manual"`, BuildWorkflow: `almanac_path /
"manual"`). Any user page under a directory named `manual` or `jobs`, at any
depth, was therefore excluded from indexing, health and frontmatter rewriting.

Silently, which is the costly part: health and validate walk with the same
iterator, so the tool cannot report pages it refuses to see. The visible
symptom is a broken link to a file that plainly exists on disk, plus a page
count that disagrees with the filesystem. It also makes `sync` fail
permanently, since the post-run validation gate trips on those broken links.

In one real wiki this hid four committed pages (~29KB) under
architecture/jobs/, and nearly caused a duplicate to be written: the coverage
map listed the target as unwritten and health called the link broken, both
consistent with "missing", when it existed and was hidden.

Restricting the check to the first directory component keeps the intended
exclusion (the bundled manual, including its subdirectories) while leaving user
pages alone. page_id_for_path in the same module already reasons positionally,
so top-level-only matches the established convention here.

The added test fails without this change and passes with it. Root-level manual/
exclusion stays covered by the existing test.

Refs AlmanacCode#56
@dev-noaman

Copy link
Copy Markdown
Author

Withdrawing the follow-up I floated at the end of the description — a health notice for reservation-skipped pages. It is already covered, and I should have checked before offering it.

services/health/runtime.py declares RUNTIME_DIRS = ("jobs", "runs"), so a user page at almanac/jobs/notes.md already raises:

[runtime_state] jobs - runtime state belongs under ~/.codealmanac/

Verified against this branch:

indexed pages       : ['architecture/wiki.md']
jobs page indexed?  : False
EXISTING CHECK FIRES: [runtime_state] jobs - runtime state belongs under ~/.codealmanac/

With this PR merged the reservation cases are: almanac/jobs/** (already reported, and with a better remedy than the message I had in mind), almanac/manual/** (tool-installed, present in every wiki, and correctly silent), and nested architecture/jobs/** (no longer skipped — this PR). Nothing is left to notice.

Adding one would also have been actively harmful: ValidationResult.ok is len(issues) == 0 with no severity tier, so an informational issue on the bundled manual/ would flip validate non-zero and make ensure_valid raise — failing sync on every wiki, which is the failure mode this PR exists to remove.

No change to the diff; this PR remains the one-line fix plus its regression test.

@dev-noaman

Copy link
Copy Markdown
Author

Opened #58 with the severity tier this thread ran into — ValidationResult.ok is len(issues) == 0, so there is currently no way to report a condition without failing validate and everything gated on it.

It is cut from main and independent of this PR; the two can merge in either order. No existing check is reclassified, so behaviour is unchanged until someone decides a category is a warning.

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