docs: surface Docker deployment and the dev container, and gate docs on --strict - #583
Open
chinmayajha wants to merge 4 commits into
Open
chinmayajha wants to merge 4 commits into
chinmayajha wants to merge 4 commits into
Conversation
Docker/deployment.md was never reachable from the docs site or the README, so the only people who found it were the ones already browsing Docker/. docs/usage/mcp_usage.md linked to it as ../../Docker/deployment.md, a path that resolves above mkdocs' docs_dir and therefore never worked — it is the one warning that stops `mkdocs build --strict` from being usable as a gate. The prose moves to docs/usage/docker_deployment.md and Docker/ keeps a short pointer, so the two cannot drift. Links to the compose file and Dockerfiles are absolute GitHub URLs, since mkdocs cannot resolve a relative path out of docs/. Corrections made while moving: - The old build instructions (`cd prod/ && docker build .`) cannot work. Every image copies poetry.lock and scripts/lock_pins.py, so the build context has to be the repository root; all commands now use `-f Docker/... .` from the root. - For the same reason the dev images read the wheel from the repository's own dist/, which is where `poetry build` writes it — no copying into Docker/dev/dist/, and WHL_FILE is only needed to disambiguate. - Documents the app/dev compose services, OPTICS_FRAMEWORK_VERSION, UVICORN_WORKERS and MCP_PORT, none of which the old guide mentioned, and notes that the compose dev service starts without the Xvfb display.
.devcontainer/ has been a working, fully provisioned environment for a while — Python 3.12, Poetry, the dependency groups, pre-commit, the editor extensions the project lints with — and nothing in the docs site, the README or CONTRIBUTING.md mentioned it, so a newcomer's only visible route in was the manual toolchain install. Adds a short page under Contribution covering how to open it in a Codespace or locally, what the setup command installs, and the limits that matter in practice: no Android SDK, emulator or Appium server, so a device suite needs a reachable Appium — host.docker.internal locally, a remote grid from a Codespace. Links to it from the Developer Guide's setup section, CONTRIBUTING and the README.
With the last broken link gone, --strict is finally usable as a gate: it turns an unresolvable link into a failed build instead of a warning nobody reads in the log. Applied to both mkdocs invocations in Build-mkdocs-docs — the pull-request build and the deploy — so a bad link cannot reach Pages. The Developer Guide now names the same command, and spells out why a link from docs/ to a file above it (Dockerfile, docker-compose.yml, devcontainer.json) has to be an absolute GitHub URL, which is what produced the link this gate would have caught.
Contributor
Documentation preview
Updated automatically when the docs build succeeds; removed when the PR is closed. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+184
to
+185
| On Linux, `host.docker.internal` resolves only when the container is started | ||
| with `--add-host=host.docker.internal:host-gateway`. |
There was a problem hiding this comment.
On Linux, the documented Compose services cannot use the recommended host.docker.internal Appium URL. None of the four services in Docker/docker-compose.yml defines the required host-gateway mapping, and docker compose up cannot use the shown docker run --add-host option. A contributor following the Compose workflow will therefore fail to connect to an Appium server on the host. Add extra_hosts: ["host.docker.internal:host-gateway"] to the applicable services or document a Compose override that supplies it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/usage/docker_deployment.md
Line: 184-185
Comment:
**Compose Lacks Host Mapping**
On Linux, the documented Compose services cannot use the recommended `host.docker.internal` Appium URL. None of the four services in `Docker/docker-compose.yml` defines the required `host-gateway` mapping, and `docker compose up` cannot use the shown `docker run --add-host` option. A contributor following the Compose workflow will therefore fail to connect to an Appium server on the host. Add `extra_hosts: ["host.docker.internal:host-gateway"]` to the applicable services or document a Compose override that supplies it.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Collaborator
Author
There was a problem hiding this comment.
Valid — verified against the compose file: none of the four services declares extra_hosts, so the docker run --add-host recipe alone left the Compose path broken on Linux. Fixed in 86b8367 by documenting the override-file form alongside it.
The Appium-from-a-container section only showed `docker run --add-host`, which a Compose user cannot apply: none of the four services in docker-compose.yml declares extra_hosts, so on Linux the documented host.docker.internal URL would simply not resolve. Adds the override-file form alongside it.
|
malto101
requested changes
Sep 23, 2026
malto101
left a comment
malto101
left a comment
Member
There was a problem hiding this comment.
rest looks good
thanks for the update
Comment on lines
+129
to
+131
| `--strict` turns MkDocs warnings into failures, so a link to a page that does not exist fails the build instead of shipping. The `Build-mkdocs-docs` workflow uses it for both the pull-request build and the deploy to GitHub Pages — run it locally before pushing a docs change. | ||
|
|
||
| A link to a file outside `docs/` (a Dockerfile, `docker-compose.yml`, `.devcontainer/devcontainer.json`) cannot be relative: `docs/` is the site root, so anything above it is unreachable. Link to those with their full `https://github.com/mozarkai/optics-framework/blob/main/...` URL. |
Member
There was a problem hiding this comment.
no need for user to know this
can be removed
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #581
Closes #393
Problem
Two things the repository already has were invisible to anyone who wasn't browsing the source tree:
Docker/deployment.md— the build-and-run guide for the REST API and MCP images — was not in the mkdocs nav, not in the README, and not linked from anywhere in the site..devcontainer/— a fully provisioned dev environment — was mentioned nowhere in the docs, the README, or CONTRIBUTING.And the one link that did point at the Docker guide,
docs/usage/mcp_usage.md:102, was../../Docker/deployment.md. That resolves abovedocs/, which is mkdocs'docs_dir, so it has never worked:That single warning is why
mkdocs build --strictcould not be turned on in CI, so every subsequent broken link would also have gone unnoticed.What changed
docs: move the Docker deployment guide into the docs siteThe prose moves to
docs/usage/docker_deployment.md(nav: Usage → Docker Deployment) andDocker/deployment.mdbecomes a short pointer to it plus a table of what the directory contains — so someone landing there from GitHub still gets to the real page, and there is only one copy of the prose to keep current. The Dockerfiles anddocker-compose.ymldo not move. Links from the page to those files are absolutegithub.com/...URLs, because a relative link out ofdocs/cannot resolve.Three corrections were needed while moving, all against the current Dockerfiles:
cd prod/ && docker build .,cd dev/ && docker build ...) cannot work. Every image doesCOPY poetry.lock scripts/lock_pins.py /app/, so the build context has to be the repository root. All commands now build with-f Docker/... .from the root.dist/— exactly wherepoetry buildwrites it — not fromDocker/dev/dist/.WHL_FILEis only needed to disambiguate whendist/holds more than one wheel.app/devCompose services,OPTICS_FRAMEWORK_VERSION,UVICORN_WORKERSorMCP_PORT; it now documents all four, notes that the Composedevservice overrides the image command and so runs withoutXvfb, and carries the "unauthenticated, binds0.0.0.0" warning next to the connection instructions.docs/usage/mcp_usage.mdnow links to the new page, anddocs/usage/usage.mdgains a card for it.docs: document the dev container and CodespacesNew
docs/contribution/dev_container.md(nav: Contribution → Dev Container and Codespaces): how to open it in a Codespace or in VS Code, what the setup command installs, and the limits that actually bite — no Android SDK, no emulator, no Appium server, so a device suite needs a reachable Appium (host.docker.internallocally, a remote grid from a Codespace). Linked from the Developer Guide's setup section, CONTRIBUTING and the README.ci: build the docs with --strictBoth
mkdocs buildinvocations inBuild-mkdocs-docs— the pull-request build and the Pages deploy — now pass--strict, so a link to a page that does not exist fails the build instead of shipping. The Developer Guide names the same command and spells out why a link fromdocs/to a file above it must be an absolute GitHub URL, which is the mistake this gate now catches.Verification
mkdocs build --stricton this branch, with the same plugin set CI installs:No warnings;
--strictwas the only pre-existing failure in the tree, so nothing else needed fixing to make the gate usable. Each of the three commits builds green on its own.pre-commit run --files <changed>passes.The PR should not merge until the documented Linux Compose workflow provides a working host-gateway mapping for Appium.
Fix with agent prompt
Summary
This PR moves the Docker deployment guide into the MkDocs site, documents the existing dev container and Codespaces workflow, and makes documentation warnings fail CI.
mkdocs build --strictfor pull-request validation and Pages deployment.Reviews (1) · Last reviewed commit: "ci: build the docs with --strict"