Skip to content

LCORE-1374: Contributing guide #1224

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.

Sign up for GitHub

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

Merged
tisnik merged 1 commit into from
Feb 25, 2026
Merged

LCORE-1374: Contributing guide #1224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
* [Pylint](#pylint)
* [Security checks](#security-checks)
* [Code style](#code-style)
* [Function Standards](#function-standards)
* [Documentation](#documentation)
* [Type annotations](#type-annotations)
* [Naming conventions](#naming-conventions)
* [Async functions](#async-functions)
* [Error handling](#error-handling)
* [Formatting rules](#formatting-rules)
* [Docstrings style](#docstrings-style)

Expand Down Expand Up @@ -227,6 +233,33 @@ make security-check

## Code style

### Function Standards

#### Documentation

All functions require docstrings with brief descriptions

#### Type annotations

Use complete type annotations for parameters and return types

- Use `typing_extensions.Self` for model validators
- Union types: `str | int` (modern syntax)
- Optional: `Optional[Type]`

#### Naming conventions

Use snake_case with descriptive, action-oriented names (get_, validate_, check_)

#### Async functions

Use `async def` for I/O operations and external API calls

#### Error handling

- Use FastAPI `HTTPException` with appropriate status codes for API endpoints
- Handle `APIConnectionError` from Llama Stack where appropriate

### Formatting rules

Code formatting rules are checked by __Black__. More info can be found on [https://black.readthedocs.io/en/stable/](https://black.readthedocs.io/en/stable/).
Expand Down
33 changes: 33 additions & 0 deletions docs/contributing_guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
* [Pylint](#pylint)
* [Security checks](#security-checks)
* [Code style](#code-style)
* [Function Standards](#function-standards)
* [Documentation](#documentation)
* [Type annotations](#type-annotations)
* [Naming conventions](#naming-conventions)
* [Async functions](#async-functions)
* [Error handling](#error-handling)
* [Formatting rules](#formatting-rules)
* [Docstrings style](#docstrings-style)

Expand Down Expand Up @@ -227,6 +233,33 @@ make security-check

## Code style

### Function Standards

#### Documentation

All functions require docstrings with brief descriptions

#### Type annotations

Use complete type annotations for parameters and return types

- Use `typing_extensions.Self` for model validators
- Union types: `str | int` (modern syntax)
- Optional: `Optional[Type]`
Comment on lines +246 to +248
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Optional[Type] contradicts the adjacent modern-syntax recommendation.

The guide promotes PEP 604 str | int syntax (modern) on line 247, but then prescribes the legacy Optional[Type] form on line 248. Since the project requires Python 3.12+, both union and optional should use the modern union syntax (Type | None) for consistency.

✏️ Suggested update 
 - Use `typing_extensions.Self` for model validators
-- Union types: `str | int` (modern syntax)
-- Optional: `Optional[Type]`
+- Union types: use `str | int` (PEP 604 modern syntax)
+- Optional values: use `Type | None` instead of `Optional[Type]`
📝 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.

Suggested change
- Use `typing_extensions.Self` for model validators
- Union types: `str | int` (modern syntax)
- Optional: `Optional[Type]`
- Use `typing_extensions.Self` for model validators
- Union types: use `str | int` (PEP 604 modern syntax)
- Optional values: use `Type | None` instead of `Optional[Type]`
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@docs/contributing_guide.md` around lines 246 - 248, The docs currently mix
modern PEP 604 union syntax (`str | int`) with the legacy `Optional[Type]`;
update the guidance to use modern optional unions everywhere by replacing the
`Optional[Type]` recommendation with `Type | None` and note that
`typing_extensions.Self` guidance remains unchanged; ensure the examples and
bullet list consistently prefer the `X | Y` form for both unions and optionals.


#### Naming conventions

Use snake_case with descriptive, action-oriented names (get_, validate_, check_)

#### Async functions

Use `async def` for I/O operations and external API calls

#### Error handling

- Use FastAPI `HTTPException` with appropriate status codes for API endpoints
- Handle `APIConnectionError` from Llama Stack where appropriate
Comment on lines +240 to +261
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Prose lines in the new section are missing terminal periods.

Lines 240, 244, 252, and 256 each end a sentence without a full stop. This is inconsistent with the surrounding documentation style.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@docs/contributing_guide.md` around lines 240 - 261, Add missing terminal
periods to the prose sentences in the new section: add a period at the end of
the lines for "All functions require docstrings with brief descriptions", the
"Type annotations" introductory line, the "Use snake_case with descriptive,
action-oriented names (get_, validate_, check_)" line, and the "Use async def
for I/O operations and external API calls" line so that they end with full
stops; locate these within the section that contains the headings "All functions
require docstrings with brief descriptions", "#### Type annotations", "####
Naming conventions", and "#### Async functions" and update the sentences to
include a trailing period for consistency with surrounding documentation style.


### Formatting rules

Code formatting rules are checked by __Black__. More info can be found on [https://black.readthedocs.io/en/stable/](https://black.readthedocs.io/en/stable/).
Expand Down
Loading