Open
Conversation
Update documentation and AGENTS.md following practices documented: https://softwareone.atlassian.net/wiki/spaces/~712020b14b6af3a5e341f9b4c936ce4cd948bb/pages/7130022026/Extensions+technical+documentation+Strategy+for+Repositories
📝 WalkthroughWalkthroughThis pull request introduces comprehensive repository-level documentation to guide users and AI assistants. It adds Copilot instructions, an AGENTS.md entry point, restructures the README with condensed quick-start guidance, and establishes four new documentation files covering architecture, contributing guidelines, local development setup, and testing practices. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/architecture.md`:
- Around line 94-103: The Python snippet in the docs causes ruff format --check
to fail due to uneven spacing in the inline comments; edit the code block
containing MPTClient.from_config and normalize spacing so each attribute access
line (e.g., client.catalog, client.commerce, client.billing, client.accounts,
client.audit, client.helpdesk, client.notifications) uses a single space before
the inline comment (e.g., "client.catalog # → Catalog") to satisfy the
formatter and CI.
- Around line 20-69: The markdown fences that list the project tree, the ASCII
diagram box, and the error hierarchy (the blocks beginning with
"mpt_api_client/", the box diagram, and the block starting with "MPTError") are
missing language identifiers and trigger MD040; update each opening
triple-backtick to include a language tag (use "text") for those fenced code
blocks and apply the same change to the other affected blocks referenced around
lines 75-85 and 190-194 so all fenced code blocks specify a language.
In `@docs/testing.md`:
- Around line 72-88: The test snippet fails ruff format due to missing/incorrect
blank-line spacing; update the snippet around the top-level imports (pytest,
respx, Response) and the test function test_get_product so there is a single
blank line separating the import block from the function definition and remove
any stray characters (like the displayed "+"); ensure overall file-level
blank-line conventions are respected so ruff format --check passes while keeping
the existing imports and the test_get_product function name unchanged.
- Around line 8-29: The fenced code block showing the tests/ directory tree is
missing a language tag which triggers markdownlint MD040; update the block in
docs/testing.md to include a language specifier (e.g., add "text" after the
opening ``` so the block becomes ```text) for the tests/ tree snippet to satisfy
the linter and preserve formatting.
🪄 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 YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4570fb7e-977b-43f9-818a-e510a5f9756b
📒 Files selected for processing (7)
.github/copilot-instructions.mdAGENTS.mdREADME.mddocs/architecture.mddocs/contributing.mddocs/local-development.mddocs/testing.md
Comment on lines
+20
to
+69
| ``` | ||
| mpt_api_client/ | ||
| ├── __init__.py # Public API: MPTClient, AsyncMPTClient, RQLQuery | ||
| ├── mpt_client.py # Client entry points | ||
| ├── constants.py # Shared constants (content types) | ||
| ├── exceptions.py # Error hierarchy (MPTError, MPTHttpError, MPTAPIError) | ||
| │ | ||
| ├── http/ # HTTP transport layer | ||
| │ ├── client.py # Sync HTTPClient (httpx.Client) | ||
| │ ├── async_client.py # Async AsyncHTTPClient (httpx.AsyncClient) | ||
| │ ├── base_service.py # ServiceBase — shared service logic | ||
| │ ├── service.py # Service — sync service (extends ServiceBase) | ||
| │ ├── async_service.py # AsyncService — async service (extends ServiceBase) | ||
| │ ├── query_state.py # Query parameter accumulation | ||
| │ ├── client_utils.py # URL validation helpers | ||
| │ ├── types.py # Type aliases (Response, HeaderTypes, etc.) | ||
| │ └── mixins/ # Composable HTTP operation mixins | ||
| │ ├── collection_mixin.py | ||
| │ ├── create_mixin.py | ||
| │ ├── create_file_mixin.py | ||
| │ ├── update_mixin.py | ||
| │ ├── update_file_mixin.py | ||
| │ ├── delete_mixin.py | ||
| │ ├── get_mixin.py | ||
| │ ├── enable_mixin.py | ||
| │ ├── disable_mixin.py | ||
| │ ├── download_file_mixin.py | ||
| │ ├── file_operations_mixin.py | ||
| │ ├── queryable_mixin.py | ||
| │ └── resource_mixins.py | ||
| │ | ||
| ├── models/ # Response models | ||
| │ ├── model.py # Model base class (camelCase ↔ snake_case mapping) | ||
| │ ├── collection.py # Collection[Model] — paginated result set | ||
| │ ├── meta.py # Meta / Pagination metadata | ||
| │ └── file_model.py # FileModel for binary responses | ||
| │ | ||
| ├── resources/ # API domain modules | ||
| │ ├── accounts/ # Account, Users, Buyers, Sellers, API Tokens, … | ||
| │ ├── audit/ # Audit records, Event types | ||
| │ ├── billing/ # Invoices, Ledgers, Journals, Statements, Credit memos, … | ||
| │ ├── catalog/ # Products, Listings, Price lists, Authorizations, … | ||
| │ ├── commerce/ # Agreements, Orders, Subscriptions, Assets | ||
| │ ├── helpdesk/ # Cases, Chats, Queues, Forms, … | ||
| │ └── notifications/ # Messages, Batches, Subscribers, … | ||
| │ | ||
| └── rql/ # RQL query builder | ||
| ├── query_builder.py # RQLQuery, RQLProperty, RQLValue | ||
| └── constants.py # RQL operator constants | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint.
These fences currently omit a language and trigger MD040 warnings.
💡 Suggested fix
-```
+```text
mpt_api_client/
...
-```
+```
-```
+```text
┌─────────────────────────────────────────────┐
...
-```
+```
-```
+```text
MPTError
├── MPTHttpError # generic HTTP error (status_code, message, body)
│ └── MPTAPIError # structured API error (payload, title, detail, trace_id)
-```
+```Also applies to: 75-85, 190-194
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/architecture.md` around lines 20 - 69, The markdown fences that list the
project tree, the ASCII diagram box, and the error hierarchy (the blocks
beginning with "mpt_api_client/", the box diagram, and the block starting with
"MPTError") are missing language identifiers and trigger MD040; update each
opening triple-backtick to include a language tag (use "text") for those fenced
code blocks and apply the same change to the other affected blocks referenced
around lines 75-85 and 190-194 so all fenced code blocks specify a language.
Comment on lines
+94
to
+103
| ```python | ||
| client = MPTClient.from_config(api_token="...", base_url="...") | ||
| client.catalog # → Catalog | ||
| client.commerce # → Commerce | ||
| client.billing # → Billing | ||
| client.accounts # → Accounts | ||
| client.audit # → Audit | ||
| client.helpdesk # → Helpdesk | ||
| client.notifications # → Notifications | ||
| ``` |
There was a problem hiding this comment.
Python snippet formatting currently breaks CI checks.
Pipeline reports ruff format --check failure around this example block; please reformat the snippet to formatter-compatible spacing.
💡 Suggested fix
```python
client = MPTClient.from_config(api_token="...", base_url="...")
-client.catalog # → Catalog
-client.commerce # → Commerce
-client.billing # → Billing
-client.accounts # → Accounts
-client.audit # → Audit
-client.helpdesk # → Helpdesk
+client.catalog # → Catalog
+client.commerce # → Commerce
+client.billing # → Billing
+client.accounts # → Accounts
+client.audit # → Audit
+client.helpdesk # → Helpdesk
client.notifications # → Notifications</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/architecture.md around lines 94 - 103, The Python snippet in the docs
causes ruff format --check to fail due to uneven spacing in the inline comments;
edit the code block containing MPTClient.from_config and normalize spacing so
each attribute access line (e.g., client.catalog, client.commerce,
client.billing, client.accounts, client.audit, client.helpdesk,
client.notifications) uses a single space before the inline comment (e.g.,
"client.catalog # → Catalog") to satisfy the formatter and CI.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+8
to
+29
| ``` | ||
| tests/ | ||
| ├── unit/ # Fast, isolated unit tests (default target) | ||
| │ ├── conftest.py # Shared fixtures (http_client, async_http_client, DummyModel) | ||
| │ ├── http/ # Tests for HTTP transport, services, and mixins | ||
| │ ├── models/ # Tests for Model, Collection, Meta | ||
| │ ├── resources/ # Tests for each resource domain (accounts, catalog, …) | ||
| │ ├── rql/ # Tests for the RQL query builder | ||
| │ ├── test_constants.py | ||
| │ ├── test_exceptions.py | ||
| │ └── test_mpt_client.py | ||
| │ | ||
| └── e2e/ # End-to-end tests against a live MPT API environment | ||
| ├── conftest.py # E2E fixtures (mpt_vendor, mpt_client, mpt_operations) | ||
| ├── accounts/ | ||
| ├── audit/ | ||
| ├── billing/ | ||
| ├── catalog/ | ||
| ├── commerce/ | ||
| ├── helpdesk/ | ||
| └── notifications/ | ||
| ``` |
There was a problem hiding this comment.
Specify a language for the test-structure fenced block.
This currently triggers markdownlint MD040.
💡 Suggested fix
-```
+```text
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.
Suggested change
| ``` | |
| tests/ | |
| ├── unit/ # Fast, isolated unit tests (default target) | |
| │ ├── conftest.py # Shared fixtures (http_client, async_http_client, DummyModel) | |
| │ ├── http/ # Tests for HTTP transport, services, and mixins | |
| │ ├── models/ # Tests for Model, Collection, Meta | |
| │ ├── resources/ # Tests for each resource domain (accounts, catalog, …) | |
| │ ├── rql/ # Tests for the RQL query builder | |
| │ ├── test_constants.py | |
| │ ├── test_exceptions.py | |
| │ └── test_mpt_client.py | |
| │ | |
| └── e2e/ # End-to-end tests against a live MPT API environment | |
| ├── conftest.py # E2E fixtures (mpt_vendor, mpt_client, mpt_operations) | |
| ├── accounts/ | |
| ├── audit/ | |
| ├── billing/ | |
| ├── catalog/ | |
| ├── commerce/ | |
| ├── helpdesk/ | |
| └── notifications/ | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/testing.md` around lines 8 - 29, The fenced code block showing the
tests/ directory tree is missing a language tag which triggers markdownlint
MD040; update the block in docs/testing.md to include a language specifier
(e.g., add "text" after the opening ``` so the block becomes ```text) for the
tests/ tree snippet to satisfy the linter and preserve formatting.
Comment on lines
+72
to
+88
| ```python | ||
| import pytest | ||
| import respx | ||
| from httpx import Response | ||
|
|
||
| def test_get_product(async_http_client): | ||
| # Arrange | ||
| respx.get("/catalog/products/PRD-001").mock( | ||
| return_value=Response(200, json={"id": "PRD-001", "name": "Test"}) | ||
| ) | ||
|
|
||
| # Act | ||
| result = async_http_client.request("GET", "/catalog/products/PRD-001") | ||
|
|
||
| # Assert | ||
| assert result.status_code == 200 | ||
| ``` |
There was a problem hiding this comment.
Unit-test example needs formatter-compliant spacing.
Pipeline indicates ruff format --check failure around this snippet; reformat it to avoid CI failures.
💡 Suggested fix
```python
import pytest
import respx
from httpx import Response
+
def test_get_product(async_http_client):
# Arrange
respx.get("/catalog/products/PRD-001").mock(📝 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
| ```python | |
| import pytest | |
| import respx | |
| from httpx import Response | |
| def test_get_product(async_http_client): | |
| # Arrange | |
| respx.get("/catalog/products/PRD-001").mock( | |
| return_value=Response(200, json={"id": "PRD-001", "name": "Test"}) | |
| ) | |
| # Act | |
| result = async_http_client.request("GET", "/catalog/products/PRD-001") | |
| # Assert | |
| assert result.status_code == 200 | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/testing.md` around lines 72 - 88, The test snippet fails ruff format due
to missing/incorrect blank-line spacing; update the snippet around the top-level
imports (pytest, respx, Response) and the test function test_get_product so
there is a single blank line separating the import block from the function
definition and remove any stray characters (like the displayed "+"); ensure
overall file-level blank-line conventions are respected so ruff format --check
passes while keeping the existing imports and the test_get_product function name
unchanged.
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
1 participant
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.
Update documentation and AGENTS.md following practices documented: https://softwareone.atlassian.net/wiki/spaces/~712020b14b6af3a5e341f9b4c936ce4cd948bb/pages/7130022026/Extensions+technical+documentation+Strategy+for+Repositories
Closes MPT-19004
.github/copilot-instructions.mdwith repository-specific guidance for Copilot, including quick validation commandsAGENTS.mdas the documentation index, defining the repository purpose and recommended reading order across documentation filesREADME.mdwith a condensed Quick Start section, key commands list, and a documentation table linking to architecture, contributing, testing, local development, and usage guidesdocs/architecture.mddocumenting the four-layer architecture model (client entry points, resource services, HTTP mixins, transport layer), cross-cutting components (RQL builder, models layer, error hierarchy), and directory structuredocs/contributing.mdspecifying development workflow, Python style requirements (3.12+, full type hints, Google-style docstrings, 100-character lines), linting/formatting tools, and API resource organization patternsdocs/local-development.mdwith prerequisites, step-by-step setup instructions, Makefile targets, environment variables, and Docker development environment detailsdocs/testing.mddocumenting test directory layout, testing toolchain, writing conventions (Arrange-Act-Assert structure), E2E execution requirements, and coverage collection behavior