docs(env): slim .env.example to essentials and fill env-var doc gaps - #624
Conversation
Reduce infra/compose/.env.example to only the variables required to boot the default compose stack (LLM, VLM, embedder, required secrets, auth, Ray), with a header pointing to the env-vars reference for everything else. Sync the quickstart-displayed snippet (docs/assets/env_example.env) so it no longer omits the required MinIO/Postgres secrets. Document variables that existed in .env.example but nowhere in the docs: MinIO credentials, rate limiting (RATE_LIMIT_*), the MCP server (OPENRAG_MCP_*), MAX_PARTITIONS_PER_USER, MAX_UPLOAD_SIZE_MB, UVICORN_FORWARDED_ALLOW_IPS and APP_UID. Add a same-origin reverse-proxy diagram to the Admin UI section explaining why VITE_API_BASE_URL is empty by default.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughDocs, environment templates, and API reference content were updated to match the ChangesDeployment docs and env template migration
Estimated code review effort: 3 (Moderate) | ~25 minutes API documentation for extracts, partitions, presets, and model endpoints
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
The single-file quick_start compose is superseded by the full stack under infra/compose/. Drop the folder, the CLAUDE.md layout reference, and the two unit tests that loaded the now-deleted compose assets (infra/quick_start/vdb/milvus.yaml and docs/assets/compose_ollama_cpu.yaml).
The bundled vLLM CPU profile is the supported CPU path; drop the parallel ollama compose asset, its env sample, and infra/compose/.env.ollama.
Collapse the quickstart onto the single supported path (the full stack under infra/compose/), make PyMuPDFLoader the documented default PDF parser, and drop the now-redundant mac-specific quickstart and standalone usage pages (their content is consolidated into the main quickstart).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/content/docs/documentation/API.mdx (1)
466-511: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the error behavior for deleting/renaming a default preset.
Line 468 states the default preset of each type cannot be deleted or renamed-style constraint (i.e., "the
defaultpreset of each type cannot be deleted or renamed"), but thePUT/DELETEendpoint docs at 503-509 don't mention what response (e.g.,400/403) callers should expect when they attempt this. Adding the error case would make the contract complete for API consumers.📝 Suggested addition
`PUT` accepts a partial body (`name` to rename and/or `config`; at least one required) and returns the updated preset. `DELETE` returns `204 No Content`. + +Attempting to rename or delete a `default` preset returns `400 Bad Request`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/documentation/API.mdx` around lines 466 - 511, The preset endpoint docs for the Get / Update / Delete section omit the failure behavior for protected defaults. Update the `PUT /presets/{preset_type}/{name}` and `DELETE /presets/{preset_type}/{name}` documentation to explicitly state what happens when callers try to rename or delete the `default` preset for either type, including the expected error response and a brief error description. Use the existing `Preset`/`preset_type` route section as the anchor for where to add this note so the contract is clear for API consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/content/docs/documentation/API.mdx`:
- Around line 466-511: The preset endpoint docs for the Get / Update / Delete
section omit the failure behavior for protected defaults. Update the `PUT
/presets/{preset_type}/{name}` and `DELETE /presets/{preset_type}/{name}`
documentation to explicitly state what happens when callers try to rename or
delete the `default` preset for either type, including the expected error
response and a brief error description. Use the existing `Preset`/`preset_type`
route section as the anchor for where to add this note so the contract is clear
for API consumers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 958bd9e9-77c9-4ae9-b1e1-47ad4ce9da70
📒 Files selected for processing (17)
CLAUDE.mdREADME.mddocs/assets/compose_ollama_cpu.yamldocs/assets/env_example.envdocs/assets/env_ollama_cpu.envdocs/content/docs/documentation/API.mdxdocs/content/docs/documentation/env_vars.mddocs/content/docs/getting_started/quickstart.mdxdocs/content/docs/getting_started/quickstart_mac.mdxdocs/content/docs/getting_started/usage.mdxinfra/compose/.env.exampleinfra/compose/.env.ollamainfra/quick_start/docker-compose.yamlinfra/quick_start/extern/infinity.yamlinfra/quick_start/extern/vllm/Dockerfile.cpuinfra/quick_start/vdb/milvus.yamltests/unit/infra/test_compose_storage.py
💤 Files with no reviewable changes (11)
- docs/content/docs/getting_started/usage.mdx
- infra/quick_start/vdb/milvus.yaml
- infra/quick_start/extern/vllm/Dockerfile.cpu
- infra/compose/.env.ollama
- docs/content/docs/getting_started/quickstart_mac.mdx
- docs/assets/compose_ollama_cpu.yaml
- infra/quick_start/extern/infinity.yaml
- docs/assets/env_ollama_cpu.env
- CLAUDE.md
- infra/quick_start/docker-compose.yaml
- tests/unit/infra/test_compose_storage.py
Keep only the must-set variables in .env.example with inline guidance (external LLM/VLM/embedder/reranker, PDFLOADER, image captioning), ship working dev defaults for the MinIO/Postgres secrets with a production warning, and document the PDFLOADER default (PyMuPDFLoader) plus whisper worker count in the env-var reference. Keep docs/assets/env_example.env in sync as the canonical mirror.
…routes Add reference sections for the admin pipeline-preset registry (/presets) and model-endpoint registry (/model-endpoints), enrich the document-extract endpoint with its real response shape and error cases, and complete the Partitions & files section (CRUD, chunks, pipeline config, members) with the correct /partition-prefixed paths.
The ollama quickstart compose asset was removed in this PR, but test_ollama_compose_admin_ui_honors_port_override still read it and failed CI with FileNotFoundError.
852241c to
7392252
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
…mplify-env-example # Conflicts: # infra/compose/.env.ollama
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/content/docs/documentation/env_vars.md (1)
25-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the PDF loader default mismatch.
This section now says
PDFLOADERdefaults toPyMuPDFLoader, but the Marker subsection still callsMarkerLoaderthe default parser. That contradiction will mislead users about the actual startup path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/documentation/env_vars.md` around lines 25 - 33, The PDF loader documentation is inconsistent about the default parser: the `PDFLOADER` table says `PyMuPDFLoader` is the default, while the `Marker Loader Configuration` section still describes `MarkerLoader` as the default. Update the `MarkerLoader` subsection in the env vars doc to match the actual default set by `PDFLOADER`, and adjust the wording so only one default is described consistently across both the `PDFLOADER` entry and the `MarkerLoader` section.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/docs/documentation/API.mdx`:
- Line 358: The in-page backlink for the Pipeline Presets reference points to
the wrong anchor and may not resolve. Update the link target in the API.mdx
content to match the rendered slug for the `### 🧩 Pipeline Presets` heading,
and verify the markdown link text still references the preset section correctly.
- Around line 286-294: The chunk payload schema is inconsistent because the
extract response uses page_content while the chunk-listing response documents
content for the same text field. Update the API docs to use the same field name
as the actual serializer across both responses, and align the chunk-listing
example with page_content so clients see one consistent schema.
---
Outside diff comments:
In `@docs/content/docs/documentation/env_vars.md`:
- Around line 25-33: The PDF loader documentation is inconsistent about the
default parser: the `PDFLOADER` table says `PyMuPDFLoader` is the default, while
the `Marker Loader Configuration` section still describes `MarkerLoader` as the
default. Update the `MarkerLoader` subsection in the env vars doc to match the
actual default set by `PDFLOADER`, and adjust the wording so only one default is
described consistently across both the `PDFLOADER` entry and the `MarkerLoader`
section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37a0db98-d76c-4b31-976b-d2a6b7fdac7c
📒 Files selected for processing (5)
docs/assets/env_example.envdocs/content/docs/documentation/API.mdxdocs/content/docs/documentation/env_vars.mdinfra/compose/.env.exampletests/unit/infra/test_admin_ui_compose.py
💤 Files with no reviewable changes (1)
- tests/unit/infra/test_admin_ui_compose.py
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/assets/env_example.env
…tract The chunk-listing endpoint returns chunk text under `content` while the single-extract endpoint returns it under `page_content` (they use different serializers). Document the difference so clients aren't surprised.
hedhoud
left a comment
There was a problem hiding this comment.
Thanks for the cleanup here. I found two setup mismatches that I think should be fixed before approval. The other notes are small docs cleanup.
The mount was /$SHARED_ENV:/ray_mount/.env with no default, so an unset
SHARED_ENV (the case for the slim .env.example) mounted host / into the
container. Use ${SHARED_ENV:-./.env}, matching the env_file directive below it.
…& whitespace - quickstart: the bundled embedder (vllm) and reranker DO start by default; document how to switch to external endpoints instead of implying external-only. - env_vars: the Marker section no longer calls MarkerLoader the default. - drop trailing whitespace in .env.example and its docs mirror.
hedhoud
left a comment
There was a problem hiding this comment.
Rechecked the latest update. The setup comments are fixed now: the env mount has a safe fallback, the quickstart matches the compose behavior, the PDF loader wording is consistent, and the whitespace cleanup is done. I also verified the compose config, docs build, and infra tests. LGTM.
Summary
Draft — more commits (API endpoint docs) to follow.
Simplifies
infra/compose/.env.exampledown to only the variables needed to boot the default compose stack, and closes the gaps where variables shipped in.env.examplebut weren't documented anywhere.Changes
infra/compose/.env.example(228 → ~65 lines)RAY_*lines are kept deliberately: they're supplied only via.env(not baked into images/compose/entrypoint), andRAY_ENABLE_UV_RUN_RUNTIME_ENV=0is flagged critical for recentuv.docs/assets/env_example.env(quickstart-displayed snippet)docs/.../env_vars.md.env.examplebut nowhere in the docs: MinIO credentials, rate limiting (RATE_LIMIT_*), the MCP server (OPENRAG_MCP_*),MAX_PARTITIONS_PER_USER,MAX_UPLOAD_SIZE_MB,UVICORN_FORWARDED_ALLOW_IPS,APP_UID.VITE_API_BASE_URLis empty by default.mcp.py,rate_limit.py,partition_limits.py,files.py,main.py, compose).Still TODO (later commits)
Notes
GRAFANA_ADMIN_PASSWORDis${...:?}-required by the monitoring profile but is neither in.env.examplenor documented — flagged, out of scope for this pass.Summary by CodeRabbit
infra/composeworkflow and streamlined compose commands.PDFLOADER(PyMuPDF default with scanned/OCR guidance via MarkerLoader), rate limiting, storage, access control, and MCP settings.