Skip to content

fix(docs): run docs:validate under the engine's Python - #2384

Merged
asclearuc merged 1 commit into
developfrom
fix/RR-2383-docs-validate-engine-python
Oct 1, 2026
Merged

asclearuc merged 1 commit into
developfrom
fix/RR-2383-docs-validate-engine-python

Conversation

@asclearuc

@asclearuc asclearuc commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • docs:validate now uses one interpreter for all three phases: the engine in dist/server when it is built, else the first of python3, python that actually runs and is Python 3.10+. Before, phases 1–2 used a bare python3, which is the Microsoft Store stub on most Windows machines, so ./builder test failed there with exit 9009.
  • Without the engine, the validator's unit tests (tests/test_validate_node_readme.py) run when the chosen Python has pytest, and are skipped otherwise. docs-schemas.yml already installs pytest, so that gate now really runs them. Before, it always skipped them because it has no engine.
  • docs/development/ci-gates.md now says which Python the task uses.

Type

fix

Testing

  • Tests added or updated (builder task actions have no unit-test harness; checked with the scenarios below)
  • Tested locally
  • ./builder test passes (only docs:validate and docs:check were run)

Windows 11, node scripts/build.js docs:validate:

Setup Result
Engine built Pass: all phases under dist\server\engine.exe, 24 tests passed
No engine (ROCKETRIDE_DIST_ROOT set to an empty dir) Pass: python3 stub rejected, real python.exe used, 24 tests passed
No engine, Python without pytest Pass: 140 node READMEs checked, unit tests skipped with a message
No engine, no Python on PATH Fails with no Python 3.10+ found (tried …, python3, python)
A node README broken on purpose Fails with the right validator errors
Unchanged develop Fails with exit 9009 (the bug)

Not tested locally on Linux or macOS. This PR touches docs/**, so docs-schemas.yml runs on it and covers the Linux path with no engine.

Checklist

  • Commit messages follow conventional commits
  • No secrets or credentials included
  • Wiki updated (if applicable). Not applicable; the contributor doc is updated in docs/development/ci-gates.md
  • Breaking changes documented (if applicable). Not applicable; no breaking changes

Linked Issue

Fixes #2383

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Clarified how documentation validation selects a Python interpreter.
    • Documented requirements for running unit tests and when tests are skipped.
  • Bug Fixes

    • Documentation validation now supports the built engine or the first compatible Python 3.10+ executable available.
    • Validation reports an error when no compatible interpreter is available.
    • Regression tests use the selected interpreter when pytest is installed.

- `docs:validate` ran its two schema validators with a bare `python3`. On
  Windows this is usually the Microsoft Store stub, so the task failed with
  exit 9009 even when the engine was built.
- Pick one interpreter for all three phases: the engine in `dist/server`
  when it is built, else the first of `python3`, `python` that runs and is
  Python 3.10+. Each candidate is run through `execCommand`, not just
  looked up, so the Store stubs are rejected.
- Without the engine, run the validator's unit tests when that Python has
  `pytest`, and skip them otherwise. `docs-schemas.yml` installs `pytest`,
  so that doc-path gate now runs them instead of always skipping them.
- Document the interpreter choice in `docs/development/ci-gates.md`.

Fixes #2383
@github-actions github-actions Bot added docs Documentation builder Node builder tooling and ./builder workflows labels Sep 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor
🤖 Internal: Discord sync marker

Auto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: rocketride-org/rocketride-server/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 19277b7b-bc4c-4213-b8ca-f64696fe62eb

📥 Commits

Reviewing files that changed from the base of the PR and between 51345ba and 90bff79.

📒 Files selected for processing (2)
  • docs/development/ci-gates.md
  • docs/docusaurus/scripts/tasks.js

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

docs:validate now selects the built engine or the first working Python 3.10+ executable on PATH. Validation uses that interpreter, and pytest is skipped when unavailable outside the engine.

Changes

Docs validation interpreter

Layer / File(s) Summary
Interpreter resolution
docs/docusaurus/scripts/tasks.js
The task prefers the built engine and falls back to working python3 or python executables that report Python 3.10 or newer.
Validation command integration
docs/docusaurus/scripts/tasks.js, docs/development/ci-gates.md
Schema validation uses the resolved interpreter. Pytest is skipped only when a non-engine interpreter lacks pytest. The CI documentation describes the selection and skip behavior.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the main change: running docs:validate with the engine's Python. It does not mention fallback interpreter selection, but it remains concise and clearly related to the p…
Linked Issues check ✅ Passed Issue #2383 requires docs:validate to use a working Python on Windows, including when the built engine exists. resolvePython() first selects the built engine, then probes python3 and python wi…
Out of Scope Changes check ✅ Passed The pull request changes only docs:validate interpreter selection, its validator test behavior, and the related CI-gate documentation. These changes directly support issue #2383. No unrelated implem…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (1 skipped: 1 …
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: dependency version conflict. Check your lock file or package.json.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thegoodengineer

Copy link
Copy Markdown

Verdict: approve with nits. The fix does what #2383 asks, and I could not break it on Windows. Two low items below, neither blocking.

I tested on Windows 11 with Node 22.17.0, on a machine that has no python3 at all and no engine build, which is a close cousin of the Store stub case. I ran the docs:validate action from the PR head and from its merge base under controlled PATH setups. I called the action through a small script instead of node scripts/build.js, because the builder's dependency preflight starts a full workspace install that needs node-gyp on this box. The action code that runs is the same.

Baseline, develop at 51345baa, normal PATH: fails at the first spawn with spawn python3 ENOENT, and dumps the whole environment to stderr on the way out.

This PR at 90bff79e:

Setup Result
Normal PATH: python is 3.12 with pytest, no python3, no engine Pass. 140 node READMEs, 24 tests ran under python
python3 is a fake Store stub (.cmd that prints the Store hint, exit 9009), python is real Pass. Stub rejected, tests ran under python
python3 is some other non-Python .exe, python is real Pass
python3 is a pyenv-win style .cmd shim to a real 3.12, nothing else on PATH Pass. The < in the version probe survives cmd /c
python is a venv without pytest Pass. 140 READMEs checked, tests skipped with the install hint
No Python, only Store stubs for python3 and python Fails with no Python 3.10+ found (tried ..., python3, python), and no env dump
Only the py launcher plus the Store stubs Fails the same way, although py -3 works on that PATH (finding 1)
Stand-in engine at dist/server/engine.exe, no Python on PATH Pass. All three phases ran under the engine
A node README broken on purpose, python fallback Fails with store_chroma: FAIL (2 failed, 0 warnings, 21 passed)

The stand-in engine is a copied venv launcher, not a real engine build. I don't have one on this machine, but tools/sync_models already runs plain scripts through ENGINE, so running the validators through it is not new.

The CI side checks out too. In the Docs site build job for this head, the pytest phase collected 24 items and all passed on Ubuntu with no engine. The same job on a develop-based branch (feat/RR-2390-media-speech) still prints pytest: engine not built ... skipping tests/test_validate_node_readme.py. So the claim that the gate now really runs them holds.

Findings

1. [low] The fallback does not try the py launcher
Location: docs/docusaurus/scripts/tasks.js:166-169
Claim: without an engine, a Windows machine whose only working Python on PATH is py.exe still fails. The classic python.org installer leaves "Add python.exe to PATH" unchecked by default and installs py.exe, so this is a common setup. It is the machine from #2383 minus the engine build.
Evidence: with PATH set to the two Store stubs, C:\Windows (for py.exe), node and System32, the task fails with no Python 3.10+ found. On the same PATH, py -3 -c "import sys; print(sys.version)" prints 3.13.7.
Refutation attempted: the issue's repro builds the engine first, and with an engine present the PR takes the engine path, so the reported case is fixed. The error message is also clear, and develop fails here too. That makes this a gap on the no-engine path, not a regression.
Suggested direction: on Windows, try py -3 as a third candidate (the candidate would need to carry the -3 argument), or accept an explicit interpreter override from an env var. Fine as a follow-up.

2. [low] The interpreter selection has no automated test
Location: docs/docusaurus/scripts/tasks.js:147-170, :198-201
Claim: resolvePython, pythonRuns and the pytest-missing skip are covered only by manual runs. CI exercises one branch: Linux, python3 with pytest.
Evidence: no test touches tasks.js. The repo does have a node --test harness that docs:unit already runs (docs/docusaurus/src/lib/*.test.mjs), plus scripts/lib/sync.test.js.
Refutation attempted: the PR body says task actions have no harness, which is true for the actions themselves. The selection logic, though, is small enough to live in a pure helper that takes the probe function as a parameter and gets tested under docs:unit. Given the size of the change and the manual matrix above, I would not block on it.
Suggested direction: optional. Pull the selection into a helper and test the ordering, the stub rejection and the no-Python error.

Side note, not from this PR: the paragraph just above the new text in docs/development/ci-gates.md says the task "validates the nodes changed relative to the merge base with develop". No such phase exists in tasks.js, here or on develop. The task runs client-doc parity, the --all corpus pass, and the unit tests. That wording dates back to #2139. A one-line fix could ride along since you are already in that section, but it is out of scope.

Metrics

  • Change size (git diff --numstat): build script +34/-12, docs +5/-0, tests 0, config 0, generated 0.
  • New surface: two module-private functions in tasks.js. Nothing is exported and there are no new dependencies.
  • Tests: the validator suite passed 24 of 24 locally (Python 3.12.10, pytest 9.1.1) and 24 of 24 in CI. The only warnings are Unknown config option: asyncio_mode and timeout from pyproject.toml, which CI shows too and which are harmless.
  • Diff coverage: not measured, because there is no JS coverage tooling for builder scripts. Every new branch was exercised by hand in the table above.
  • CI (gh pr checks): 16 passed, 4 skipped, 0 failing, 0 pending. The three Build jobs finished in 3-4s, so the engine-present path is not exercised by this PR's CI. I covered it locally with the stand-in engine only.
  • Mergeable: yes. It is only waiting on review.

Invariants checked

  • Conventional commit, fix/RR-2383-... branch, base develop: passed.
  • Contributor doc updated in the same change: passed.
  • JS style (tabs, single quotes, semicolons): passed.
  • Pytest tasks prefer the engine when it is built: passed. The task is still engine-first.
  • No check weakened: passed, and it is actually stronger. CI now runs the 24 tests it used to skip, and the skip condition (a PATH Python without pytest) is narrower than before (no engine).
  • Secrets: passed. Nothing added, and gitleaks is green.
  • Security boundary: passed. The command names are a fixed list and spawn uses shell: false. .cmd shims go through execCommand's existing quoting, which I checked with the < in the probe.
  • Public contract: passed. The task name and flags are unchanged. The only new failure mode replaces an ENOENT with a clearer message.
  • Data safety: not applicable, since the validators are read-only.

Things I tried that did not break it

  • A Python older than 3.10, or Python 2, as python: sys.exit(sys.version_info < (3, 10)) exits 1 there, so it is rejected. I checked this by reading only, since there is no old Python on this machine.
  • python !== ENGINE: resolvePython returns the ENGINE constant itself, so the identity check is sound.
  • The plugin-only ini keys in pyproject.toml (timeout, asyncio_mode) under a plain PATH pytest produce warnings, not errors.
  • The validators and the test file import only the standard library, so any 3.10+ interpreter is enough.
  • A broken engine now fails phases 1-2 instead of only phase 3. The task fails either way, so this is not a regression.
  • Failed probes use silent: true, so a missing Python no longer dumps the environment to stderr the way develop does.

Reviewed in an isolated worktree at 90bff79ec4342d921888c7544427c6e71bf55ac5.

@jmaionchi @Rod-Christensen @stepmikhaylov

@asclearuc
asclearuc enabled auto-merge (squash) September 30, 2026 17:06
@asclearuc
asclearuc merged commit 5ae09ed into develop Oct 1, 2026
24 checks passed
@asclearuc
asclearuc deleted the fix/RR-2383-docs-validate-engine-python branch October 1, 2026 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

builder Node builder tooling and ./builder workflows docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(docs): docs:validate fails on Windows when python3 is the Microsoft Store stub

3 participants