-
Notifications
You must be signed in to change notification settings - Fork 54
LCORE-304: devel documentation #262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LCORE-304: devel documentation #262
Conversation
WalkthroughDocumentation files were updated to improve clarity and usability. Typographical errors were corrected in CONTRIBUTING.md, with adjustments to the table of contents. README.md received expanded instructions for running the Llama Stack as a separate server. docs/README.md and docs/testing.md were enhanced with better navigation and detailed testing instructions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Docs
participant LlamaStackServer
User->>Docs: Read setup instructions (README.md)
User->>Docs: Read test instructions (docs/testing.md)
User->>LlamaStackServer: Start server using run.yaml and pyproject.toml
User->>LlamaStackServer: Verify server with curl (OpenAPI JSON)
User->>Docs: Reference testing commands
User->>LlamaStackServer: Run tests via Makefile or pytest
Estimated code review effort2 (30–90 minutes) Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
CONTRIBUTING.md (1)
6-10: Fix unordered-list indentation to satisfy markdownlint (MD007)
Bullets nested under “Prerequisites” start with four leading spaces, but the linter expects two. Align the indentation to keep automated docs checks green.-* [Prerequisites](#prerequisites) - * [Tooling installation](#tooling-installation) +* [Prerequisites](#prerequisites) + * [Tooling installation](#tooling-installation)docs/testing.md (2)
31-37: Add language hint to fenced code block (MD040)
Specify the language so GitHub & docs renderers get syntax highlighting and the linter is satisfied.-``` +```bash make test--- `53-61`: **Add language hints to pytest invocation examples** ```diff -``` +```bash uv run python -m pytest -vv tests/unit/utils/...
-+bash
uv run python -m pytest -vv tests/unit/utils/ --cov=src/utils/ --cov-report term-missingREADME.md (2)
116-142: Over-restrictive Python version pin in examplepyproject.toml
The sample requiresrequires-python = "==3.12.*", but the project-wide docs state that 3.13 is also supported. Consider a looser constraint to avoid confusing readers:-requires-python = "==3.12.*" +requires-python = ">=3.12,<3.14"
152-162: Missing language specifiers in command-line examples (MD040)Add
bashso the blocks are highlighted and markdownlint passes.-``` +```bash export OPENAI_API_KEY="sk-{YOUR-KEY}" uv run llama stack run run.yaml...
-+bash
curl -X 'GET' localhost:8321/openapi.json | jq .docs/README.md (1)
3-4: Relative link may break on GitHub Pages
../README.mdresolves correctly in the repo view, but on Pages the root HTML is usuallyindex.html, not the markdown README. Consider linking to the deployed root or to/lightspeed-stack/to avoid 404s.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CONTRIBUTING.md(2 hunks)README.md(3 hunks)docs/README.md(1 hunks)docs/testing.md(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/testing.md
6-6: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
8-8: Link fragments should be valid
(MD051, link-fragments)
10-10: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
17-17: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
152-152: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
160-160: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
e00b0af to
daadb5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/testing.md (3)
5-17: Fix list indentation to satisfy MD007 & keep TOC readableCurrent bullets use 4-/8-space indents; markdownlint (and GitHub renderer) expect +2 spaces per level.
-* [Running tests](#running-tests) - * [Run all tests](#run-all-tests) - * [Select one group of tests](#select-one-group-of-tests) - * [Select individual test or group of tests to be run](#select-individual-test-or-group-of-tests-to-be-run) -* [Unit tests](#unit-tests) - * [Unit tests structure](#unit-tests-structure) -* [Integration tests](#integration-tests) -* [End to end tests](#end-to-end-tests) -* [Tips and hints](#tips-and-hints) - * [Developing unit tests](#developing-unit-tests) - * [Patching](#patching) - * [Verifying that some exception is thrown](#verifying-that-some-exception-is-thrown) - * [Checking what was printed and logged to stdout or stderr by the tested code](#checking-what-was-printed-and-logged-to-stdout-or-stderr-by-the-tested-code) +* [Running tests](#running-tests) + * [Run all tests](#run-all-tests) + * [Select one group of tests](#select-one-group-of-tests) + * [Select individual test or group of tests to be run](#select-individual-test-or-group-of-tests-to-be-run) +* [Unit tests](#unit-tests) + * [Unit tests structure](#unit-tests-structure) +* [Integration tests](#integration-tests) +* [End to end tests](#end-to-end-tests) +* [Tips and hints](#tips-and-hints) + * [Developing unit tests](#developing-unit-tests) + * [Patching](#patching) + * [Verifying that some exception is thrown](#verifying-that-some-exception-is-thrown) + * [Checking what was printed and logged to stdout or stderr by the tested code](#checking-what-was-printed-and-logged-to-stdout-or-stderr-by-the-tested-code)
35-37: Add code-block language for lint & syntax highlighting-``` +```bash make test -``` +```Applies MD040 and improves readability.
53-61: Tag the remaining command snippets withbashSame MD040 issue for both examples:
-``` +```bash uv run python -m pytest -vv tests/unit/utils/ -``` +``` -``` +```bash uv run python -m pytest -vv tests/unit/utils/ --cov=src/utils/ --cov-report term-missing -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/testing.md(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/testing.md
6-6: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
10-10: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
17-17: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pr
daadb5c to
b9674b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/testing.md (1)
14-17: Third-level bullets use 8-space indent; expected 4Lines 14-17 are indented twice as much as the markdownlint default (MD007). Reduce to four spaces per bullet (two spaces per nesting level) or disable the rule if you have strong styling reasons.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/testing.md(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/testing.md
6-6: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
10-10: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
17-17: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pr
🔇 Additional comments (1)
docs/testing.md (1)
51-61: LGTM – new subsection and examples are clearThe heading, anchor, and pytest/coverage examples are accurate and improve developer guidance.
| * [Running tests](#running-tests) | ||
| * [Run all tests](#run-all-tests) | ||
| * [Select one group of tests](#select-one-group-of-tests) | ||
| * [Select individual test or group of tests to be run](#select-individual-test-or-group-of-tests-to-be-run) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix MD007 list-indent violations in the TOC
The second-level bullets are indented with four spaces instead of two, triggering markdownlint warnings and producing slightly off alignment in some renderers.
-* [Running tests](#running-tests)
- * [Run all tests](#run-all-tests)
- * [Select one group of tests](#select-one-group-of-tests)
- * [Select individual test or group of tests to be run](#select-individual-test-or-group-of-tests-to-be-run)
+* [Running tests](#running-tests)
+ * [Run all tests](#run-all-tests)
+ * [Select one group of tests](#select-one-group-of-tests)
+ * [Select individual test or group of tests to be run](#select-individual-test-or-group-of-tests-to-be-run)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [Running tests](#running-tests) | |
| * [Run all tests](#run-all-tests) | |
| * [Select one group of tests](#select-one-group-of-tests) | |
| * [Select individual test or group of tests to be run](#select-individual-test-or-group-of-tests-to-be-run) | |
| * [Running tests](#running-tests) | |
| * [Run all tests](#run-all-tests) | |
| * [Select one group of tests](#select-one-group-of-tests) | |
| * [Select individual test or group of tests to be run](#select-individual-test-or-group-of-tests-to-be-run) |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In docs/testing.md around lines 5 to 8, the second-level bullet points in the
table of contents are indented with four spaces instead of the recommended two
spaces, causing markdownlint MD007 list-indent violations. Fix this by reducing
the indentation of these second-level bullets from four spaces to two spaces to
ensure proper alignment and compliance with markdownlint rules.
| ```bash | ||
| make test-unit Run the unit tests | ||
| make test-integration Run integration tests tests | ||
| make test-e2e Run end to end tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate word “tests” in integration command description
make test-integration Run integration tests tests repeats the word “tests”.
-make test-integration Run integration tests tests
+make test-integration Run integration tests📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```bash | |
| make test-unit Run the unit tests | |
| make test-integration Run integration tests tests | |
| make test-e2e Run end to end tests |
🤖 Prompt for AI Agents
In docs/testing.md around lines 43 to 46, the description for the integration
test command contains a duplicate word "tests". Remove the extra "tests" so the
line reads: "make test-integration Run integration tests".
Description
Type of change
Related Tickets & Documents
Summary by CodeRabbit