docs(config): fix configuration file formatting and accuracy - #2605
docs(config): fix configuration file formatting and accuracy#2605Olawoyin365 wants to merge 15 commits into
Conversation
Reviewer's GuideRewrites and restructures the RamaLama configuration file documentation into a Docusaurus-friendly page with frontmatter, clearer sections, tables, TOML code samples, and more precise option descriptions and examples. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Several sections that were originally documented as array-of-tables (e.g.
[[ramalama.http_client]],[[ramalama.provider]],[[ramalama.provider.openai]],[[ramalama.benchmarks]],[[ramalama.user]]) are now presented as single tables (e.g.[ramalama.http_client],[ramalama.provider.openai]); please double-check the actual config schema and keep the TOML notation consistent so users don’t adopt an invalid structure. - In a few option descriptions the declared type or structure is confusing or inconsistent (e.g.
ramalama.provider.openaiis described asType: stringbut then used as a table,portis a string but is conceptually numeric); consider tightening these to match the real types expected by the parser. - There are a couple of small phrasing/format issues that could be cleaned up for clarity and polish (for example, “As mentioned above that the configuration file uses…” under Configuration Format, and the missing space in
**Type:**stringforport).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several sections that were originally documented as array-of-tables (e.g. `[[ramalama.http_client]]`, `[[ramalama.provider]]`, `[[ramalama.provider.openai]]`, `[[ramalama.benchmarks]]`, `[[ramalama.user]]`) are now presented as single tables (e.g. `[ramalama.http_client]`, `[ramalama.provider.openai]`); please double-check the actual config schema and keep the TOML notation consistent so users don’t adopt an invalid structure.
- In a few option descriptions the declared type or structure is confusing or inconsistent (e.g. `ramalama.provider.openai` is described as `Type: string` but then used as a table, `port` is a string but is conceptually numeric); consider tightening these to match the real types expected by the parser.
- There are a couple of small phrasing/format issues that could be cleaned up for clarity and polish (for example, “As mentioned above that the configuration file uses…” under Configuration Format, and the missing space in `**Type:**string` for `port`).
## Individual Comments
### Comment 1
<location path="docs/ramalama.conf.5.md" line_range="18" />
<code_context>
+## File Locations
-For user specific configuration it reads
+RamaLama searches for configuration files in multiple locations, Files processed later override settings from earlier ones.
-| Paths | Exception |
</code_context>
<issue_to_address>
**suggestion (typo):** Fix comma splice and capitalization in this sentence about configuration file search order.
For example: "RamaLama searches for configuration files in multiple locations. Files processed later override settings from earlier ones."
```suggestion
RamaLama searches for configuration files in multiple locations. Files processed later override settings from earlier ones.
```
</issue_to_address>
### Comment 2
<location path="docs/ramalama.conf.5.md" line_range="61" />
<code_context>
+
+## Configuration Format
+
+As mentioned above that the configuration file uses the [TOML format](https://toml.io). Every option is nested under its table, with no bare options allowed.
+
+**Basic TOML structure:**
</code_context>
<issue_to_address>
**suggestion (typo):** Rephrase the awkward "As mentioned above that" construction.
Consider: "As mentioned above, the configuration file uses the [TOML format](https://toml.io)." and keep the rest of the sentence unchanged.
```suggestion
As mentioned above, the configuration file uses the [TOML format](https://toml.io). Every option is nested under its table, with no bare options allowed.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly updates the ramalama.conf documentation, transitioning it to a more structured Markdown format with detailed configuration references, examples, and environment variable overrides. The review feedback focuses on ensuring the documentation aligns with the technical limitations of the TOMLParser (such as lack of support for inline comments and array-of-tables syntax), correcting field names to match the underlying BaseConfig (e.g., rag_image vs rag_images), and improving grammatical clarity and consistency throughout the document.
| engine = "podman" | ||
| store = "$HOME/.local/share/ramalama" | ||
|
|
||
| [[ramalama.images]] |
There was a problem hiding this comment.
The TOMLParser in ramalama/toml_parser.py does not support the [[ ]] (array of tables) syntax. It will incorrectly parse this as a section named [ramalama.images]. Since images is defined as a dict[str, str] in BaseConfig, a standard table [ramalama.images] should be used instead.
| [[ramalama.images]] | |
| [ramalama.images] |
| ```toml | ||
| [ramalama] | ||
| backend = "vulkan" # Force Vulkan for all GPUs | ||
| backend = "vulkan" # Force Vulkan for all GPUs |
There was a problem hiding this comment.
The TOMLParser implementation is very strict and does not support inline comments. Including a # after the value will cause _parse_value to fail because the string won't end with a quote or match the expected numeric regex, leading to a ValueError.
| backend = "vulkan" # Force Vulkan for all GPUs | |
| backend = "vulkan" |
|
|
||
| --- | ||
|
|
||
| #### rag_images (string) |
There was a problem hiding this comment.
The field name in BaseConfig for the single image string is rag_image (singular). rag_images (plural) is used for the dictionary of overrides. Updating this header improves accuracy.
| #### rag_images (string) | |
| #### rag_image |
| #### rag_images (table array overrides) | ||
|
|
||
| **Type:** table array |
There was a problem hiding this comment.
This section refers to the rag_images dictionary. The type should be table and the header should match the field name in the code.
| #### rag_images (table array overrides) | |
| **Type:** table array | |
| #### rag_images | |
| **Type:** table |
| port = "8080" | ||
| pull = "newer" | ||
|
|
||
| [[ramalama.images]] |
There was a problem hiding this comment.
Correcting the syntax in the complete example to use a standard table for the images dictionary.
| [[ramalama.images]] | |
| [ramalama.images] |
| ## File Locations | ||
|
|
||
| For user specific configuration it reads | ||
| RamaLama searches for configuration files in multiple locations, Files processed later override settings from earlier ones. |
There was a problem hiding this comment.
There is a comma splice here. It should be a period to separate the two independent clauses.
| RamaLama searches for configuration files in multiple locations, Files processed later override settings from earlier ones. | |
| RamaLama searches for configuration files in multiple locations. Files processed later override settings from earlier ones. |
|
|
||
| ## Configuration Format | ||
|
|
||
| As mentioned above that the configuration file uses the [TOML format](https://toml.io). Every option is nested under its table, with no bare options allowed. |
There was a problem hiding this comment.
The sentence structure is slightly awkward. Removing 'that' and adding a comma makes it more natural.
| As mentioned above that the configuration file uses the [TOML format](https://toml.io). Every option is nested under its table, with no bare options allowed. | |
| As mentioned above, the configuration file uses the [TOML format](https://toml.io). Every option is nested under its table, with no bare options allowed. |
|
|
||
| #### images | ||
|
|
||
| **Type:** table array |
There was a problem hiding this comment.
The images field is a dict[str, str] in the Python configuration (ramalama/config.py). It should be described as a table rather than a table array to be consistent with TOML terminology and the underlying data structure.
| **Type:** table array | |
| **Type:** table |
|
|
||
| #### port | ||
|
|
||
| **Type:**string |
There was a problem hiding this comment.
Missing space after the colon for consistency with other field definitions.
| **Type:**string | |
| **Type:** string |
Thanks for the feedback. I’ve reviewed the points raised: I’ll verify the actual configuration schema in the codebase to ensure correct use of TOML table vs array-of-table notation before making changes. Appreciate the review, I’ll push updates shortly after validating against the actual implementation. |
|
Thanks for the work on this PR. I reviewed it and found a few major issues that may cause CI/test failures:
Could you please take a look? Thanks! Update: I checked the Packit/COPR logs. CI is currently failing in
|
Thanks for the detailed review, this is really helpful. I’m currently going through each of the points you raised. I will revert and push an update once everything is verified. Looking forward to further reviews together |
| ## Configuration Reference | ||
|
|
||
| **api**="none" | ||
| ### ramalama Table |
There was a problem hiding this comment.
@Olawoyin365 kindly check if this is applicable;
according to the formatting guideline the heading format should be; capitalize only the first word and proper nouns in this case Table doesn't follow that as it is not a proper noun.
| **Type:** string | ||
| **Default:** `"registry.access.redhat.com/ubi10-micro:latest"` | ||
|
|
||
| OCI model car image used when building and pushing models with `--type=car`. |
There was a problem hiding this comment.
@Olawoyin365 kindly check if this is applicable;
car image is a Config.Key and according to our formatting guidelines config.keys should be in inline code.
|
|
||
| **port**="8080" | ||
| **Type:** string | ||
| **Default:** Based on container engine |
There was a problem hiding this comment.
@Olawoyin365 kindly check if this is applicable;
all the 'defaults' have back ticks except for this line.
| --- | ||
|
|
||
| The maximum number of times to retry a failed download | ||
| ### ramalama.http_client Table |
There was a problem hiding this comment.
@Olawoyin365 kindly check if this is applicable;
according to the formatting guideline the heading format should be; capitalize only the first word and proper nouns in this case Table doesn't follow that as it is not a proper noun.
| --- | ||
|
|
||
| **storage_folder**="\<default store>/benchmarks" | ||
| ### ramalama.benchmarks Table |
There was a problem hiding this comment.
@Olawoyin365 kindly check if this is applicable;
according to the formatting guideline the heading format should be; capitalize only the first word and proper nouns in this case Table doesn't follow that as it is not a proper noun.
| --- | ||
|
|
||
| The maximum delay between retry attempts in seconds | ||
| ### ramalama.provider Table |
There was a problem hiding this comment.
@Olawoyin365 kindly check if this is applicable;
according to the formatting guideline the heading format should be; capitalize only the first word and proper nouns in this case Table doesn't follow that as it is not a proper noun.
| --- | ||
|
|
||
| `[[ramalama.user]]` | ||
| ### ramalama.user Table |
There was a problem hiding this comment.
@Olawoyin365 kindly check if this is applicable;
according to the formatting guideline the heading format should be; capitalize only the first word and proper nouns in this case Table doesn't follow that as it is not a proper noun.
- Fix heading hierarchy for better readability - Convert plain text tables to proper Markdown tables - Add TOML code blocks with syntax highlighting - Fix grammar errors and typos - Add Docusaurus-compatible note blocks - Standardize capitalization - Verify configuration paths and options - Test backend auto-detection behavior This improves documentation readability and ensures accuracy against actual RamaLama behavior. Fixes: containers#111 Signed-off-by: woyin365 <woyin365@gmail.com>
- Fix heading hierarchy for better readability - Convert plain text tables to proper Markdown tables - Add TOML code blocks with syntax highlighting - Fix grammar errors and typos - Add Docusaurus-compatible note blocks - Standardize capitalization - Verify configuration paths and options - Test backend auto-detection behavior This improves documentation readability and ensures accuracy against actual RamaLama behavior. Fixes: containers#111 Signed-off-by: woyin365 <woyin365@gmail.com>
- Fix heading hierarchy for better readability - Convert plain text tables to proper Markdown tables - Add TOML code blocks with syntax highlighting - Fix grammar errors and typos - Add Docusaurus-compatible note blocks - Standardize capitalization - Verify configuration paths and options - Test backend auto-detection behavior This improves documentation readability and ensures accuracy against actual RamaLama behavior. Fixes: containers#111 Signed-off-by: woyin365 <woyin365@gmail.com>
Signed-off-by: woyin365 <woyin365@gmail.com>
Signed-off-by: woyin365 <woyin365@gmail.com>
…structure Signed-off-by: woyin365 <woyin365@gmail.com>
Signed-off-by: woyin365 <woyin365@gmail.com>
Signed-off-by: woyin365 <woyin365@gmail.com>
|
Thanks for taking the time to highlight all of those casing differences When formatting the documentation pages, I've split the headers into two distinct categories based on the formatting guidelines (I have updated it in our HackMD too): Conceptual English Headings: For standard topics like ## Environment Variables or ## Configuration Format, I use Title Case (capitalizing the first letter) because they act like standard book chapters or article sections. Code Literal Headings: For headers that directly reference code or configuration keys (like ### ramalama table or ### RAMALAMA_CONFIG), I preserve the exact programmatic casing (lowercase for TOML tables, ALL-CAPS for bash environment variables). This ensures users don't accidentally copy-paste a capitalized literal like [Ramalama], which would fail because TOML is case-sensitive! (And for the word "table", since it's just a generic noun, it stays lowercase to follow the sentence-case rule you mentioned perfectly). Thanks again for the sharp eyes on this, it's really helping to improve the documentation |
You are most welcome @Olawoyin365 and thank you too. |
|
Thanks for the updates. I re-checked the latest changes. I still see one major concern:
Could you please revert the test-file changes and keep the fix limited to @mikebonnet, @rhatdan, @bmahabirbu could you please confirm whether changing |
@mikebonnet, @rhatdan, @bmahabirbu could you please confirm whether changing |
| #### openai | ||
|
|
||
| **openai**="" | ||
| **openai** |
There was a problem hiding this comment.
Hi @Olawoyin365 kindly check if this is applicable, should it be "openAI"
There was a problem hiding this comment.
Hi @praxyfarhana, thanks for the observation. I've used openai (lowercase) specifically where it refers to the TOML configuration key (e.g., [ramalama.provider.openai]), as these keys are consistently lowercase in our configuration schema. Using the exact technical casing ensures users don't face parsing issues in their config files.
However, I've used 'OpenAI' in the descriptive prose where I'm referring to the organization. I believe this distinction helps maintain both technical accuracy and professional grammar.
|
|
||
| **Type:** string | ||
| **Type:** integer | ||
| **Default:** `"8080"` |
There was a problem hiding this comment.
Hi @Olawoyin365 have noticed some inconsistencies with ports' data type (Default: "8080").
|
|
||
| **Type:** string | ||
| **Type:** float | ||
| **Default:** `"0.8"` |
There was a problem hiding this comment.
Hi @Olawoyin365 have noticed some inconsistencies with data type (Default: "0.8").
|
|
||
| **Type:** string | ||
| **Type:** table | ||
| **Default:** `""` |
There was a problem hiding this comment.
Hi @Olawoyin365 have noticed some inconsistencies with data type (Default: "").
Thanks for the feedback, @amkr6207. The changes to As you have also suggested, I'm happy to wait for the maintainers (@mikebonnet, @rhatdan, @bmahabirbu) to confirm if changes to the |
|
Hi @praxyfarhana, You're right that The documentation in this PR is intentionally aligned with the codebase to ensure users understand how the configuration is initially parsed and stored from TOML/Environment sources. I've corrected the temp type designation in the manpage to string to be consistent with the other fields and the implementation. Thank you for the detailed review |
There was a problem hiding this comment.
Hi @Olawoyin365, great progress on the formatting! The documentation will look much cleaner with these changes.
While looking at how these changes interact with the CI pipeline, I noticed an edge case in the test suite that we should probably future-proof against regarding the provider and benchmarks sections.
The issue:
The parser in test/unit/test_config_documentation.py appears to rely on regex-based extraction of documented fields (e.g., matching field_name). It effectively assumes that any bolded field corresponds to a top-level [ramalama] config unless it is explicitly excluded via the subsections_with_fields handling.
Currently, provider and benchmarks seem to be missing from that exclusion logic in get_documented_fields_in_manpage(). Because of this, if a nested field is documented using the fieldname syntax, the test may incorrectly treat it as a top-level global field.
Why this does not fail CI currently:
The api_key formatting passes CI because api_key exists at the top level of BaseConfig. Since the test compares sets of documented vs. actual config fields, this doesn’t trigger a failure. However, if a provider-specific field like model or base_url were added, this could cause the test to fail.
Additionally, it seems the parser only matches one level of sections (e.g., [[ramalama.
Inconsistency:
It looks like benchmarks is already handled in get_documented_fields_in_conf(), but not in get_documented_fields_in_manpage(), which introduces an inconsistency between the two parsers.
Suggestion:
To avoid future CI issues as documentation expands, we could:
Option A: Use backticks (e.g., api_key) for nested fields so they are ignored by the parser
Option B: Update the test suite to include provider and benchmarks in the subsection exclusion logic (and potentially improve handling of nested sections)
It is not a blocker for this PR, but maybe worth addressing to make the documentation pipeline more robust.
Great work on the documentation improvements!
|
Please verify tat the man pages actually generate look good. man ./*.1 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughManpage generation now pipes Markdown through a new preprocessor ( Changes
Sequence Diagram(s)sequenceDiagram
participant Makefile
participant Cleaner as "docs/clean_for_man.py"
participant Sed as "sed transforms"
participant GOMD as "GOMD2MAN"
participant Man as "Manpage"
Makefile->>Cleaner: run python3 docs/clean_for_man.py $<
Cleaner-->>Makefile: processed markdown (stdout)
Makefile->>Sed: pipe stdout into sed substitutions
Sed-->>Makefile: transformed markdown (stdout)
Makefile->>GOMD: GOMD2MAN -in /dev/stdin -out $@
GOMD-->>Man: generated manpage file
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/ramalama.conf.5.md (1)
639-642:⚠️ Potential issue | 🟡 MinorType/Default format inconsistency for
temp.The type is documented as
floatbut the default value uses string formatting with quotes ("0.8"). This inconsistency was also flagged in past review comments.If
tempis truly a float in the code, the default should be documented as0.8(without quotes). If it's stored as a string, thenType: stringwould be more accurate.💡 Suggested fix (if temp is a float)
**Type:** float -**Default:** `"0.8"` +**Default:** `0.8`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ramalama.conf.5.md` around lines 639 - 642, The documentation shows the setting temp with Type: float but Default: "0.8" (string); fix this by making the representation consistent: if temp is a numeric float in code, change the documented Default value for temp from "0.8" to 0.8 (remove quotes); if temp is actually stored as a string in code, update the documented Type for temp from float to string; adjust only the temp entry so Type and Default match the code.
🧹 Nitpick comments (3)
docs/clean_for_man.py (3)
4-6: Consider adding type hints for better maintainability.As per the coding guidelines, type hints are encouraged. Adding them would improve code clarity.
✨ Proposed enhancement
-def clean_markdown(content, filename): +def clean_markdown(content: str, filename: str) -> str: # Strip frontmatter🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/clean_for_man.py` around lines 4 - 6, Add type hints to the clean_markdown function signature and its return type to follow the project guidelines: change def clean_markdown(content, filename): to include types (e.g., content: str, filename: str) and annotate the return type (-> str); update any internal variable annotations if helpful and ensure imports for typing are added only if you use types like Optional or Union.
20-21: Fix ruff E701: multiple statements on one line.The static analysis tool flagged these lines for having multiple statements on a single line, which violates ruff formatting rules.
♻️ Proposed fix
- if len(lines) < 3: return match.group(0) - if '|' not in lines[0] or '-' not in lines[1]: return match.group(0) + if len(lines) < 3: + return match.group(0) + if '|' not in lines[0] or '-' not in lines[1]: + return match.group(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/clean_for_man.py` around lines 20 - 21, Split the two single-line if statements into separate multi-line statements to satisfy ruff E701: replace "if len(lines) < 3: return match.group(0)" with a standard if block (if len(lines) < 3:\n return match.group(0)) and similarly replace "if '|' not in lines[0] or '-' not in lines[1]: return match.group(0)" with an if block (if '|' not in lines[0] or '-' not in lines[1]:\n return match.group(0)); keep the exact conditions and the return of match.group(0) using the same variables (lines, match) to preserve behavior.
26-26: Fix ruff E701: multiple statements on one line.♻️ Proposed fix
- if not cols: continue + if not cols: + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/clean_for_man.py` at line 26, The single-line conditional "if not cols: continue" combines two statements and triggers ruff E701; fix it by converting it into a proper if-block that keeps the condition on its own line ending with a colon and moves the follow-up statement to the next indented line (i.e., check the variable cols and place the continue on its own indented line), updating the code path that references cols to use this two-line form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/clean_for_man.py`:
- Around line 43-48: The script reads sys.argv[1] unguarded which raises
IndexError when no CLI argument is provided; update the __main__ block to
validate arguments before using sys.argv by checking len(sys.argv) >= 2 (or
try/except IndexError) and print a concise usage/help message then exit with
non-zero status; ensure the filename variable is only assigned after validation
and that file open still uses encoding='utf-8' so behavior of filename,
open(...) and content remains unchanged.
---
Duplicate comments:
In `@docs/ramalama.conf.5.md`:
- Around line 639-642: The documentation shows the setting temp with Type: float
but Default: "0.8" (string); fix this by making the representation consistent:
if temp is a numeric float in code, change the documented Default value for temp
from "0.8" to 0.8 (remove quotes); if temp is actually stored as a string in
code, update the documented Type for temp from float to string; adjust only the
temp entry so Type and Default match the code.
---
Nitpick comments:
In `@docs/clean_for_man.py`:
- Around line 4-6: Add type hints to the clean_markdown function signature and
its return type to follow the project guidelines: change def
clean_markdown(content, filename): to include types (e.g., content: str,
filename: str) and annotate the return type (-> str); update any internal
variable annotations if helpful and ensure imports for typing are added only if
you use types like Optional or Union.
- Around line 20-21: Split the two single-line if statements into separate
multi-line statements to satisfy ruff E701: replace "if len(lines) < 3: return
match.group(0)" with a standard if block (if len(lines) < 3:\n return
match.group(0)) and similarly replace "if '|' not in lines[0] or '-' not in
lines[1]: return match.group(0)" with an if block (if '|' not in lines[0] or '-'
not in lines[1]:\n return match.group(0)); keep the exact conditions and the
return of match.group(0) using the same variables (lines, match) to preserve
behavior.
- Line 26: The single-line conditional "if not cols: continue" combines two
statements and triggers ruff E701; fix it by converting it into a proper
if-block that keeps the condition on its own line ending with a colon and moves
the follow-up statement to the next indented line (i.e., check the variable cols
and place the continue on its own indented line), updating the code path that
references cols to use this two-line form.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 68f61dd8-21dc-45c7-b53e-2c363c85eb0b
📒 Files selected for processing (5)
docs/Makefiledocs/clean_for_man.pydocs/ramalama.conf.5.mddocsite/convert_manpages.pytest/unit/test_config_documentation.py
7bcd48c to
78745bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/ramalama.conf.5.md`:
- Line 796: The documentation uses the array TOML table notation
`[[ramalama.user]]` but `user` is a single nested instance (see UserConfig and
the ramalama.config.py declaration user: UserConfig =
field(default_factory=UserConfig); change the doc to use the single table
notation for ramalama.user (i.e., replace the double-bracket table with a
single-bracket table) so the TOML matches the UserConfig structure.
- Around line 743-745: The TOML example is invalid: replace the array table
[[ramalama.provider]] and string assignment openai = "" with a nested table for
the OpenAI provider and its fields; update the example to use
[ramalama.provider.openai] and include the OpenaiProviderConfig fields (e.g.,
api_key = "your-api-key-here") so the config matches the OpenaiProviderConfig
dataclass used in ramalama/config.py.
- Line 770: Replace the incorrect array-table TOML notation
``[[ramalama.benchmarks]]`` with the single-table form ``[ramalama.benchmarks]``
because the configuration field `benchmarks` is a single `Benchmarks` instance
(see `Benchmarks` and `benchmarks: Benchmarks =
field(default_factory=Benchmarks)`), ensuring the document reflects the proper
nested config structure.
- Around line 354-369: The TOML examples use array-of-tables syntax
`[[ramalama.images]]` but the config expects a mapping (`images: dict[str,
str]`); update both examples to use a single table header ` [ramalama.images]`
instead of double-bracket array notation and keep the same key/value pairs
(including the VLLM and VLLM_<GPU_ENV_VAR> entries like `VLLM` and
`VLLM_CUDA_VISIBLE_DEVICES`) so the examples match the `images` dict in
ramalama/config.py.
- Line 730: The TOML example uses the array-table syntax `[[ramalama.provider]]`
but `provider` is a single nested object (see ProviderConfig and
ramalama.provider), so update the example to use the single-table notation by
replacing the double-bracket table marker with a single-bracket one (i.e.,
`[ramalama.provider]`) so the docs match the configuration type.
- Line 690: The docs use TOML array-table notation ``[[ramalama.http_client]]``
but the runtime config defines a single nested object (http_client:
HTTPClientConfig), so change the documentation to use a single table notation
`[ramalama.http_client]` (replace the double-bracket instance with a
single-bracket one) to accurately reflect the http_client / HTTPClientConfig
shape.
- Around line 760-762: The TOML example incorrectly uses an array-of-tables
marker for the OpenAI provider; replace the double-bracket table header
[[ramalama.provider.openai]] with a single-table header
[ramalama.provider.openai] so the example matches the single-instance
OpenaiProviderConfig used by ramalama.config.py and the ramalama.provider.openai
configuration key.
- Line 86: The TOML table is incorrectly documented as the array-of-tables form
`[[ramalama]]`; change the example to the single-table notation `[ramalama]` so
it matches the implementation and the test examples (replace the symbol
`[[ramalama]]` with `[ramalama]` in the docs/ramalama.conf.5.md entry).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0f393abf-a5f2-43e2-8d3f-b75edc8a1386
📒 Files selected for processing (4)
docs/Makefiledocs/clean_for_man.pydocs/ramalama.conf.5.mddocsite/convert_manpages.py
✅ Files skipped from review due to trivial changes (1)
- docs/Makefile
🚧 Files skipped from review as they are similar to previous changes (2)
- docsite/convert_manpages.py
- docs/clean_for_man.py
78745bd to
4afb0cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/ramalama.conf.5.md`:
- Line 69: Remove the stray lone character "B" that precedes the example block
in the documentation; delete that accidental character so the example starts
cleanly, and verify surrounding spacing/blank line is preserved and no other
stray characters remain.
- Line 86: The doc uses single-bracket backticked section markers like
`[ramalama]` which no longer match the docs parser (it expects double-bracket
markers like `[[ramalama...]]` as asserted in
test_unit/test_config_documentation.py); fix by replacing the single-bracket
backticked markers (e.g., `[ramalama]`) in this file with the double-bracket
form (e.g., `[[ramalama]]`) for all similar entries so the parser/test can
recognize the sections again.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8e2ca3ff-d3e0-4e63-90de-7bb59b556d58
📒 Files selected for processing (4)
docs/Makefiledocs/clean_for_man.pydocs/ramalama.conf.5.mddocsite/convert_manpages.py
✅ Files skipped from review due to trivial changes (1)
- docs/Makefile
🚧 Files skipped from review as they are similar to previous changes (2)
- docsite/convert_manpages.py
- docs/clean_for_man.py
|
|
||
| **Example configuration:** | ||
|
|
||
| B |
There was a problem hiding this comment.
Remove accidental stray character before example block.
Line 69 contains a lone B, which appears to be an editing artifact and should be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ramalama.conf.5.md` at line 69, Remove the stray lone character "B" that
precedes the example block in the documentation; delete that accidental
character so the example starts cleanly, and verify surrounding spacing/blank
line is preserved and no other stray characters remain.
4afb0cf to
8b37ad8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/unit/test_config_documentation.py (1)
50-55:⚠️ Potential issue | 🟠 MajorNested subsection matching is broken for
provider.openaiin both parsing functions.Lines 50 and 131 add
provider.openaitosubsections_with_fields, but the regex patterns at lines 54 and 136 only match a single token afterramalama.using([a-z_]+). This means section headers like[ramalama.provider.openai]won't be recognized, making the exclusion ineffective.The regex patterns need to support dotted subsection names:
- Line 54: Change
([a-z_]+)to([a-z_]+(?:\.[a-z_]+)*)- Line 136: Change
([a-z_]+)to([a-z_]+(?:\.[a-z_]+)*)and update bracket matching to\[{1,2}and\]{1,2}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/test_config_documentation.py` around lines 50 - 55, The section-matching regexes are too restrictive and don't recognize dotted subsection names like "provider.openai"; update the patterns used in the parsing functions by changing section_pattern (currently r'^\s*(#?)\s*\[ramalama\.([a-z_]+)\]') to accept dotted names (use ([a-z_]+(?:\.[a-z_]+)*)) and likewise update the other regex used later (the one around main_section_pattern/second section matcher) to use ([a-z_]+(?:\.[a-z_]+)*) and broaden bracket matching to \[{1,2} and \]{1,2} so headers like [ramalama.provider.openai] (and single/double bracket variants) are correctly recognized; update references to section_pattern and the second section regex in both parsing functions where subsections_with_fields is checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/ramalama.conf.5.md`:
- Around line 18-25: The docs table in docs/ramalama.conf.5.md omits the
$XDG_DATA_HOME-based path that the runtime loader actually checks; update the
table rows to include an entry for $XDG_DATA_HOME/ramalama/ramalama.conf (marked
as Linux / non-Windows defaults) and also mirror this addition where the same
table appears (lines ~30-35) so both $XDG_DATA_HOME/ramalama/ramalama.conf and
$XDG_CONFIG_HOME/ramalama/ramalama.conf are documented alongside the existing
/etc and /usr paths.
---
Outside diff comments:
In `@test/unit/test_config_documentation.py`:
- Around line 50-55: The section-matching regexes are too restrictive and don't
recognize dotted subsection names like "provider.openai"; update the patterns
used in the parsing functions by changing section_pattern (currently
r'^\s*(#?)\s*\[ramalama\.([a-z_]+)\]') to accept dotted names (use
([a-z_]+(?:\.[a-z_]+)*)) and likewise update the other regex used later (the one
around main_section_pattern/second section matcher) to use
([a-z_]+(?:\.[a-z_]+)*) and broaden bracket matching to \[{1,2} and \]{1,2} so
headers like [ramalama.provider.openai] (and single/double bracket variants) are
correctly recognized; update references to section_pattern and the second
section regex in both parsing functions where subsections_with_fields is
checked.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d45d6dae-85c3-4bb4-b5e6-306736051aa8
📒 Files selected for processing (5)
docs/Makefiledocs/clean_for_man.pydocs/ramalama.conf.5.mddocsite/convert_manpages.pytest/unit/test_config_documentation.py
🚧 Files skipped from review as they are similar to previous changes (3)
- docsite/convert_manpages.py
- docs/clean_for_man.py
- docs/Makefile
| | Path | Exception | | ||
| |------|----------| | ||
| | `/usr/share/ramalama/ramalama.conf` | Linux | | ||
| | `/usr/local/share/ramalama/ramalama.conf` | Linux | | ||
| | `/etc/ramalama/ramalama.conf` | Linux | | ||
| | `/etc/ramalama/ramalama.conf.d/*.conf` | Linux | | ||
| | `$HOME/.local/.pipx/venvs/usr/share/ramalama/ramalama.conf` | macOS (pipx installation) | | ||
|
|
There was a problem hiding this comment.
Documented config search paths are missing the $XDG_DATA_HOME location.
The runtime loader includes both $XDG_DATA_HOME/ramalama/ramalama.conf and $XDG_CONFIG_HOME/ramalama/ramalama.conf in non-Windows defaults, but only the latter is documented here. This can mislead users debugging load order.
📝 Suggested table update
| Path | Notes |
|------|-------|
+| `$XDG_DATA_HOME/ramalama/ramalama.conf` | User data-dir config |
+| `$XDG_DATA_HOME/ramalama/ramalama.conf.d/*.conf` | User data-dir drop-in files |
| `$XDG_CONFIG_HOME/ramalama/ramalama.conf` | Primary user config |
| `$XDG_CONFIG_HOME/ramalama/ramalama.conf.d/*.conf` | User config drop-in files |Also applies to: 30-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ramalama.conf.5.md` around lines 18 - 25, The docs table in
docs/ramalama.conf.5.md omits the $XDG_DATA_HOME-based path that the runtime
loader actually checks; update the table rows to include an entry for
$XDG_DATA_HOME/ramalama/ramalama.conf (marked as Linux / non-Windows defaults)
and also mirror this addition where the same table appears (lines ~30-35) so
both $XDG_DATA_HOME/ramalama/ramalama.conf and
$XDG_CONFIG_HOME/ramalama/ramalama.conf are documented alongside the existing
/etc and /usr paths.
8b37ad8 to
9ede17e
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/unit/test_config_documentation.py (1)
68-75:⚠️ Potential issue | 🔴 CriticalDon't record nested subsection paths as documented config fields.
documented.add(section_name)now captures nested names likeprovider.openai, butget_config_fields()only models top-levelBaseConfigfields. Since this PR already adds[ramalama.provider.openai]indocs/ramalama.conf.5.md, thetest_no_undocumented_fields_in_*checks will report a false extra field. Track nested sections for parsing, but only add top-level section names todocumented.🔧 Proposed fix
if section_match: is_commented = section_match.group(1) == '#' section_name = section_match.group(2) - documented.add(section_name) + if '.' not in section_name: + documented.add(section_name) # Skip fields if it's a commented nested section OR if it's a subsection with its own fields in_commented_nested_section = is_commented or (section_name in subsections_with_fields) prev_line_blank = current_line_blank continueif section_match: section_name = section_match.group(1) current_section = section_name - documented.add(section_name) + if '.' not in section_name: + documented.add(section_name) continueAlso applies to: 147-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/test_config_documentation.py` around lines 68 - 75, The test records nested section names like "provider.openai" into documented via documented.add(section_name), but get_config_fields() only lists top-level BaseConfig names; change the logic where section_match is handled (the block using section_match, section_name, in_commented_nested_section) so that instead of adding the full section_name you extract and add only the top-level name (e.g., section_name.split('.', 1)[0]) or skip adding if it contains a dot; keep tracking nested sections for parsing via in_commented_nested_section, and apply the same change to the analogous handling later in the file (the block around the other documented.add usage at lines referenced as 147-152).
🧹 Nitpick comments (1)
docsite/convert_manpages.py (1)
241-243: Make the JSX escape resilient to doc formatting changes.This exact-string replacement only fixes one formatting variant. A trivial copy change in
docs/ramalama.conf.5.mdwill miss the replacement and bring the MDX build break back.♻️ Suggested hardening
- # Prevent JSX compilation error on the website - content = content.replace('**storage_folder**="<default store>/benchmarks"', '**storage_folder**="`<default store>`/benchmarks"') + # Prevent JSX compilation errors on placeholder paths like <default store>/... + content = re.sub( + r'(\*\*storage_folder\*\*=")<([^>]+)>(/benchmarks")', + r'\1`<\2>`\3', + content, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docsite/convert_manpages.py` around lines 241 - 243, The current exact-string replace on content is brittle; change it to use a regex-based replacement so any formatting/quoting/asterisk variations are handled: use re.sub on the variable content (replace the current content = content.replace(...) line in convert_manpages.py) to locate the storage_folder assignment and only wrap the literal <default store> in backticks (preserving surrounding emphasis markers and quotes) rather than matching the entire exact string; this makes the transformation resilient to minor doc formatting changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/unit/test_config_documentation.py`:
- Around line 68-75: The test records nested section names like
"provider.openai" into documented via documented.add(section_name), but
get_config_fields() only lists top-level BaseConfig names; change the logic
where section_match is handled (the block using section_match, section_name,
in_commented_nested_section) so that instead of adding the full section_name you
extract and add only the top-level name (e.g., section_name.split('.', 1)[0]) or
skip adding if it contains a dot; keep tracking nested sections for parsing via
in_commented_nested_section, and apply the same change to the analogous handling
later in the file (the block around the other documented.add usage at lines
referenced as 147-152).
---
Nitpick comments:
In `@docsite/convert_manpages.py`:
- Around line 241-243: The current exact-string replace on content is brittle;
change it to use a regex-based replacement so any formatting/quoting/asterisk
variations are handled: use re.sub on the variable content (replace the current
content = content.replace(...) line in convert_manpages.py) to locate the
storage_folder assignment and only wrap the literal <default store> in backticks
(preserving surrounding emphasis markers and quotes) rather than matching the
entire exact string; this makes the transformation resilient to minor doc
formatting changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6d6550bc-c0f7-4a0d-8637-d728395f7cd9
📒 Files selected for processing (5)
docs/Makefiledocs/clean_for_man.pydocs/ramalama.conf.5.mddocsite/convert_manpages.pytest/unit/test_config_documentation.py
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/Makefile
- docs/clean_for_man.py
9ede17e to
50e2301
Compare
@rhatdan, Thanks for pointing that out. I regenerated and verified the build locally, and the man pages compile successfully and display correctly in the terminal. Here is a quick recording of the final terminal output. olawoyin@OLAWOYIN__mnt_c_Users_DELL_Documents_ramalama_docs.2026-04-16.02-19-38.mp4 |
50e2301 to
6e64b7e
Compare
…preprocessor Signed-off-by: woyin365 <woyin365@gmail.com>
6e64b7e to
9780d52
Compare
|
A friendly reminder that this PR had no activity for 30 days. |
6 participants
This PR addresses formatting issues, content errors, and structural issues impacting readability in the Configuration File documentation (
ramalama.conf.5.md).Issue Summary
The current configuration documentation page is quite hard to follow and has few breaks, broken links, content errors, manpage-style formatting not adapted for Docusaurus, no clear section organizations, poor scanability etc as identified in issue #111 on Fedora Forge
Changes Made
Restructured Content
Fixed All Tables
Added Code Blocks with Syntax Highlighting
Fixed Grammar and Spelling
Structured Configuration Options
Added Complete Configuration Example
Screenshots
Before (Current Documentation)
After (This PR)
Video Demonstration of Local Testing of the Web Pages
Configuration.File._.RamaLama.and.4.more.pages.-.Personal.-.Microsoft_.Edge.2026-04-16.02-27-32.mp4
Terminal Formatting
Native Manpage Structural Alignment Standardized the
ramalama.conf.5.mddocumentation to perfectly align with the core
.1framework. Stripped out proprietary Docusaurus UI wrappers (e.g., :::note / :::warning) and YAML frontmatter, replacing them with readable Markdown syntax that rendered the man pages good. This guarantees flawless native rendering in both the web portal and thego-md2manterminal output for man pagesVideo Demonstration of Local Testing Terminal man pages
olawoyin@OLAWOYIN__mnt_c_Users_DELL_Documents_ramalama_docs.2026-04-16.02-19-38.mp4
Summary by Sourcery
Rewrite and restructure the RamaLama configuration file documentation for Docusaurus, improving readability, navigation, and accuracy while adding a complete configuration example.
Documentation: