Repository navigation
refactor(docs): replace vendored docsfy renderer with pi-docsite package - #1239
Conversation
scripts/generate_docs.py and scripts/docs_render/ were a vendored copy of myk-org/docsfy. That generator is now published as `pi-docsite` on PyPI, so the vendored copy is deleted and the site is rebuilt with `uvx pi-docsite`. The vendored copy was already a strict subset of the package; the package adds: - docs/nav.json as the sidebar layout, replacing the NAVIGATION list that used to be hardcoded in the generator. A page missing from nav.json is still built but lands under a fallback group and the build prints a note naming it. - Per-page `nav_group` / `nav_order` frontmatter overrides. - Stale HTML is only deleted when it carries the generator's marker, so a hand-written 404.html survives a rebuild. - `?v=<hash>` cache-busting on asset URLs, so a rebuilt asset is not served stale from a browser cache. - Wide tables are wrapped in a scrollable div instead of widening the page. - The header badge reads `pi-docsite` and links to this repo. It previously hardcoded the label "pi-config" while pointing the href at this repo. - The footer says "Built from docs/*.md by pi-docsite" Sidebar group order, page titles and llms.txt are unchanged: llms.txt is byte-identical before and after. Rebuilding twice produces identical bytes. Drops the docs build dependencies (markdown, jinja2, pygments) from the dev group, since the renderer is no longer imported from this repo, and removes the [tool.mypy] namespace_packages workaround that existed only for scripts/docs_render/. Docs site regenerated with pi-docsite 4.7.2. Assisted-by: PI (gpt-6-luna) <noreply@pi.dev> Signed-off-by: Meni Yakove <myakove@gmail.com>
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
PR Summary by QodoReplace vendored docs renderer with pi-docsite
AI Description
Diagram
High-Level Assessment
Files changed (24)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
pi-docsite renders docs/*.html, docs/assets/, llms.txt, llms-full.txt and search-index.json on every build. Those files are owned by the generator, not this repo, so a review comment on rendered output is a complaint about pi-docsite and belongs upstream. docs/*.md and docs/nav.json stay in scope; they are authored here. Assisted-by: PI (gpt-6-luna) <noreply@pi.dev> Signed-off-by: Meni Yakove <myakove@gmail.com>
|
Code review by qodo was updated up to the latest commit 83590c8 |
AGENTS.md line 13 listed the rebuild as a bare `uvx pi-docsite`, but pi-docsite defaults --tagline to an empty string. A contributor copying that command regenerates the checked-in pages with an empty tagline, wiping the landing-page description and the page meta description. Verified: the bare command produces `<meta name="description" content="">`, while the command now on line 13 reproduces the committed site with no diff. Reported by Qodo on PR #1239. Assisted-by: PI (gpt-6-luna) <noreply@pi.dev> Signed-off-by: Meni Yakove <myakove@gmail.com>
|
/qodo Diagnostic question about how you resolved the review config for this PR. Please answer specifically, and say 'unknown' rather than guessing if you cannot determine something.
|
|
Code review by qodo was updated up to the latest commit 006be4e |
|
|
/qodo The following review comments were reviewed and a decision was made:
|
|
Correction to the previous comment: two upstream issue links were truncated by an editing error on my side. The correct numbers are:
Both were filed; only the URLs above are wrong. The disposition for both is unchanged: not fixable in this repository, filed upstream, and tracked here in #1240. Also worth stating explicitly for this finding set: the status is @qodo-code-review no further action is needed on the three findings above. |
The dispositions are consistent with the review findings:
All three findings are closed; no repository-side changes are needed for findings 1 or 2. |
|
/qodo Please re-evaluate all remaining sticky findings against the current code. |
Understood. No further action is needed:
|
I re-evaluated the findings against the current repository state. There are no remaining active sticky findings:
No dismissal action was needed because all findings are already closed. |
|
Correction required — the previous cleanup response closed a finding on a false premise. The cleanup response states:
That is not true, and this repository did not fix it. Three checks:
The word "addressed" in the previous reply caused this. It was intended to mean "dispositioned — the finding is valid, owned upstream, and tracked", and the reply said so. Read as a code-status term, it conveyed the opposite. That is a wording failure on my side, not a disagreement about the finding. The finding remains open and unfixed. It is not fixable here: Please re-open finding 2. Finding 1 ( Finding 3 ( @qodo-code-review finding 2 must not be treated as resolved on the strength of this comment. |
|
/qodo The following review comments were reviewed and a decision was made:
|
Understood. Findings 1 and 2 are documented as valid but not fixed in this repository; they remain owned by |
You’re right about the distinction: the evidence supports that finding 2 remains technically valid and unfixed. Its issue is specifically that Finding 1 also remains a valid, unfixed test-coverage gap for the generated search and copy assets. Finding 3 is implemented in I can’t reopen findings or change their disposition from this interface; the findings index currently records finding 1 and finding 2 as dismissed, and finding 3 as implemented. I also won’t describe findings 1 or 2 as code-addressed based on this comment. |
|
/qodo Please re-evaluate all remaining sticky findings against the current code. |
There are no remaining active sticky findings to re-evaluate. The findings index shows:
No findings need to be removed or updated. |
|
/qodo Please re-evaluate all remaining sticky findings against the current code. |
There are no remaining active findings to remove or retain.
The findings index remains at 0 active findings. |
|
/qodo Please re-evaluate all remaining sticky findings against the current code. |
There are no remaining active sticky findings to re-evaluate. All findings are already closed:
No findings were removed or reopened. |
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
Summary
scripts/generate_docs.pyandscripts/docs_render/were a vendored copy of the docsfy renderer. That generator is now published on PyPI aspi-docsite, so the vendored copy is deleted and the site is rebuilt withuvx pi-docsite.Net: -4,047 / +535 across 42 files.
Why the vendored copy can go
The vendored renderer was already a strict subset of the package — I diffed the 4.7.0 wheel against
scripts/docs_render/and the Jinja templates (index,page,_sidebar,_theme) were byte-identical. The vendored copy existed only so docs could be rebuilt without the other repo present. That reason is now obsolete.What changes for a docs author
Before: add a line to the
NAVIGATIONlist inscripts/generate_docs.py, then rebuild.Now: add the slug to
docs/nav.json, then rebuild.A page missing from
nav.jsonis still built — it lands under a fallback group namedMoreand the build prints a note naming it, so a page can never be silently invisible. A slug innav.jsonwith no.mdbehind it is a build error, so a dead sidebar link is a build failure rather than a silently broken page.Per-page
nav_group/nav_orderfrontmatter is supported as an override when one page must live somewhere else, instead of restructuring the array.What the package adds over the vendored copy
?v=<hash>cache-busting on asset URLs — previously a rebuilt asset could be served stale from a browser cache, since the URLs never changeddivinstead of widening the page past the viewport<slug>.htmlis only deleted when it carries the generator's marker, so a hand-written404.htmlsurvives a rebuildfile://pi-docsiteand links to this repoThat last one is a fix for a bug this repo was about to inherit: the badge hardcoded the label
pi-configwhile thehrefpointed at the consuming repo. Reported and fixed upstream in myk-org/pi-config#871, released in 4.7.2.Verification
uvx prek run --all-files→ exit 0uvx tox→unittestsOK, 1816 passed, coverage 90.35%Getting Started,User Guides,Explore,Reference) in the original orderdocs/llms.txtis byte-identical before and after — group structure and page titles fully preservedmarkdown,jinja2andpygmentsfrom thedevgroup after confirming nothing outside the deleted renderer imported them;uv.lockresyncedKnown issue, not addressed here
tox -e unused-codefails locally onsidecar-helper/node_modules/pi-orchestrator-config/scripts/httpd.py:find_free_port. That directory is gitignored, and the same command passes on a cleanorigin/mainworktree, so CI is unaffected. It will trip for anyone who has runnpm installinsidecar-helper/. The fix is--exclude-filesintox.toml, kept out of this PR as an unrelated pre-existing bug.🤖 Generated with Claude Code