Skip to content

fix(docs): detect generated skill documentation drift#67328

Open
0xCAFEBABEx0 wants to merge 1 commit into
NousResearch:mainfrom
0xCAFEBABEx0:agent/check-generated-skill-docs
Open

fix(docs): detect generated skill documentation drift#67328
0xCAFEBABEx0 wants to merge 1 commit into
NousResearch:mainfrom
0xCAFEBABEx0:agent/check-generated-skill-docs

Conversation

@0xCAFEBABEx0

Copy link
Copy Markdown

What does this PR do?

Adds a read-only drift check for generated skill documentation and makes generator ownership explicit. CI now verifies committed pages, catalogs, and sidebar content instead of silently regenerating them.

Normal generation fails closed on a hand-written page collision, removes only stale pages carrying the exact ownership marker, and bounds unified diff output. The branch also commits the one-time synchronization for the 177 skills currently discovered on main.

Related Issue

Fixes #67325

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Add website/scripts/generate-skill-docs.py --check as a read-only verification mode.
  • Build every expected output in memory before checking or writing.
  • Detect missing, outdated, stale, and non-generator-owned output conflicts.
  • Add a stable ownership marker and marker-gated stale cleanup.
  • Bound generated diff output for CI readability.
  • Change docs-site-checks.yml to run the read-only check.
  • Add tests for no-write checking, bounded output, safe cleanup, collision refusal, and CLI routing.
  • Regenerate catalogs, sidebar, moved skill pages, newly discovered pages, and other stale generated content from the latest main source tree.

How to Test

  1. Run scripts/run_tests.sh tests/website/test_generate_skill_docs.py.
  2. Run python3 website/scripts/generate-skill-docs.py.
  3. Run python3 website/scripts/generate-skill-docs.py --check and confirm it reports Skill documentation is up to date. without changing the worktree.
  4. Run npm run build from website/.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A; CI workflow and generator are updated directly
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

  • Generator tests: 12 passed.
  • Latest-main generation: 177 skills written.
  • Read-only check: Skill documentation is up to date.
  • Full Docusaurus production build passed with pre-existing unrelated broken-link warnings.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing a verified documentation-integrity gap. On current main, website/scripts/generate-skill-docs.py:452-459 discovers all skill definitions, but the checkout has 177 SKILL.md inputs and only 174 marker-owned generated bundled/optional pages; five expected generated paths are absent and two tracked pages correspond to removed source skills. The existing CI job silently rewrites these outputs at .github/workflows/docs-site-checks.yml:40-41.

The PR changes that job to --check at .github/workflows/docs-site-checks.yml:40-41, while website/scripts/generate-skill-docs.py:805-841 compares in-memory expected outputs without writes and :909-941 preserves collision refusal plus marker-gated stale cleanup. The deployment workflow intentionally retains normal generation at .github/workflows/deploy-site.yml:150-154.

No verified blocking problems found in the reviewed change.

Automated hermes-sweeper review.

Copy link
Copy Markdown
Author

Merge-order note: #67327 is intentionally stacked on this exact head commit to avoid conflicting updates to the generated authoring page and skills catalog. Merge this generator/baseline PR first, then re-evaluate #67327 against updated main.

@teknium1 teknium1 added sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 19, 2026
@alt-glitch alt-glitch added type/feature New feature or request comp/dashboard Web dashboard / control panel UI (dashboard/, landing) tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have labels Jul 19, 2026
@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Automated triage review — verified against upstream/main.

The implementation faithfully covers the #67325 spec, but there are two concerns a maintainer should weigh before merge.

What's solid

The --check design in website/scripts/generate-skill-docs.py hits every acceptance criterion:

  • Read-only check mode (check_generated_outputs, routed via generate_skill_docs(check=...) / main --check) builds all expected outputs in memory and never writes.
  • Explicit generator ownership via GENERATED_PAGE_MARKER + _is_owned_generated_page, so stale-page removal (_remove_stale_generated_pages) only touches marked pages.
  • Hand-written collisions fail closed (_unowned_page_conflicts) — it refuses to overwrite a non-owned path.
  • Bounded diff output (_print_content_diff) keeps CI logs usable.
  • .github/workflows/docs-site-checks.yml runs the read-only check instead of silently regenerating.
  • tests/website/test_generate_skill_docs.py covers no-write checking, bounded output, safe cleanup, collision refusal, and CLI routing.

Concern 1 — decision gate

#67325 is labeled needs-decision ("Awaiting maintainer decision before any implementation") and P3. This is a ~2.1k-line implementation landing ahead of that gate. The mechanism looks good, but the contract (generator-ownership marker semantics, CI switching from repair to fail-closed) is exactly the kind of thing the label is holding for a maintainer call. Worth an explicit ack before merging.

Concern 2 — mixed infra + bulk regeneration

738 deletions / large additions come from a one-time re-sync of 177 skills (moved pages, new pages, catalogs, sidebar) bundled with the tooling change. That's defensible (it's what makes --check green), but it makes the diff hard to review and will churn/conflict against any other doc-touching PR. Consider splitting the tooling+CI change from the bulk regeneration commit, or at least calling out that the regeneration is mechanical output of the new generator.

Cross-PR note

#67327 (for #67324) currently re-includes this PR's entire changeset — 22 identical files, including a byte-identical 289-line diff of generate-skill-docs.py, the same CI change, sidebar, and all regenerated skill pages. If both merge as-is they will conflict / double-apply. These two PRs should be de-duplicated: land the generator change once (here), and rebase #67327 to drop the shared files.

Net: strong implementation; recommend a maintainer decision-ack + de-duplication with #67327 before merge.

@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Jul 19, 2026
@0xCAFEBABEx0
0xCAFEBABEx0 marked this pull request as ready for review July 19, 2026 05:52
@alt-glitch alt-glitch removed the comp/dashboard Web dashboard / control panel UI (dashboard/, landing) label Jul 19, 2026
@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Automated triage review

Reviewed against upstream/main; the generator refactor is clean and satisfies the acceptance criteria in #67325.

What's solid:

  • --check is genuinely read-only. build_expected_outputs() renders every page, both catalogs, and the sidebar in memory, and check_generated_outputs() only reads and prints — no write_text/unlink on the check path. This meets "check mode never writes files."
  • Ownership is marker-based rather than name-based. GENERATED_PAGE_MARKER gates stale cleanup (_stale_generated_pages / _remove_stale_generated_pages), so hand-written Markdown under bundled/optional is never removed. This is a real improvement over the old dead HAND_WRITTEN = {"google-workspace.md"} constant, which was defined on main but referenced nowhere — removing it is safe.
  • Fail-closed on collisions: _unowned_page_conflicts() makes normal generation refuse to overwrite an expected output path that lacks the marker (returns exit 1), satisfying "collisions fail closed."
  • Diff output is bounded by MAX_REPORTED_DIFF_LINES = 80, keeping CI logs usable.
  • The workflow swap in docs-site-checks.yml (generate-skill-docs.py--check) correctly runs after extract-skills.py, so CI now reports drift instead of silently repairing it.

Catalogs/sidebar handling is correct: they're in expected_outputs (content-checked) but excluded from expected_page_paths, so they're never subject to marker/stale logic — right, since they're fixed known files, not per-skill pages.

One thing to flag — overlap with #67327 (fix(agent): harden skill authoring workflow, for #67324): that PR contains this entire change — the same generate-skill-docs.py --check/marker refactor, the same docs-site-checks.yml switch, the same sidebars.ts rewrite, the same tests/website/test_generate_skill_docs.py, and the same ~15 regenerated pages — plus its authoring changes. Whichever of the two merges first will force a non-trivial conflict/rebase on the other, and the 177-skill regeneration commit will double-apply. Since this PR is the focused, single-purpose home for the #67325 drift-check contract, the cleaner path is to land this one first and strip the duplicated generator/CI/pages hunks out of #67327 so it carries only the authoring-specific diff. Worth coordinating before either merges.

Verdict: the drift-check implementation itself is correct, well-tested (12 passing generator tests covering no-write, bounded output, safe cleanup, collision refusal, CLI routing), and matches the issue. The only real risk is the entanglement with #67327, which is a sequencing/scope problem rather than a defect in this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Detect generated skill documentation drift without mutating CI

4 participants