Skip to content

MPT-22046 reduce _process_value cognitive complexity below threshold#342

Merged
jentyk merged 1 commit into
mainfrom
refactor/MPT-22046
Jun 11, 2026
Merged

MPT-22046 reduce _process_value cognitive complexity below threshold#342
jentyk merged 1 commit into
mainfrom
refactor/MPT-22046

Conversation

@jentyk

@jentyk jentyk commented Jun 11, 2026

Copy link
Copy Markdown
Member

What

Refactors BaseModel._process_value in mpt_api_client/models/model.py to reduce its Cognitive Complexity from 18 to ~4, under the allowed 15 (SonarQube finding).

The dict- and list-conversion branches are extracted into two flat module-level helpers:

  • _build_model_from_dict(value, target_class) — builds a BaseModel/subclass from a raw dict.
  • _resolve_list_model_class(target_class) — resolves the element model class for a list[...]-typed field via a guard clause instead of three levels of nesting.

_process_value is now three flat if statements.

Why

The cost was concentrated in the list branch, where if target_class → if origin is list → if args and … reached nesting level 3 (each if costs 1 + nesting_depth). Flattening into helpers removes the nesting penalty. Helpers are kept module-level (matching the existing to_snake_case/to_camel_case pattern) to avoid pushing BaseModel past WPS214's method-count limit.

Notes for reviewers

  • Behaviour is unchanged. The old trailing if isinstance(value, BaseModel): return value is folded into the final return value (both returned the value unchanged).
  • The # noqa: WPS231 C901 suppression on _process_value is removed (no longer needed).
  • _build_model_from_dict narrows target_class through a typed local so mypy sees a BaseModel return rather than Any.

Verification

make check passes clean (ruff format, ruff check, flake8/WPS, mypy, uv lock --check); 118 model unit tests pass.

Context

Split out of MPT-21922 — this refactor was originally committed on that bug's branch but is unrelated to it. Tracked under MPT-22046.

Closes MPT-22046

  • Refactored BaseModel._process_value to reduce Cognitive Complexity from 18 to approximately 4 (below the SonarQube threshold of 15)
  • Extracted dict-to-model conversion logic into new private helper _build_model_from_dict(value, target_class)
  • Extracted list element-type resolution logic into new private helper _resolve_list_model_class(target_class)
  • Simplified _process_value to three flat if statements, improving code readability and maintainability
  • Removed noqa suppression comment from _process_value
  • Preserved existing behavior: all 118 model unit tests pass, and all linting checks (ruff, flake8, WPS, mypy) pass

… threshold

Extract the dict- and list-conversion branches of
BaseModel._process_value into module-level helpers
(_build_model_from_dict and _resolve_list_model_class). This flattens
the nested conditionals that drove cognitive complexity to 18, bringing
the method under the allowed 15. Behaviour is unchanged; the redundant
trailing isinstance(value, BaseModel) check folds into the final return.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jentyk jentyk requested a review from a team as a code owner June 11, 2026 10:38
@jentyk jentyk requested review from d3rky and svazquezco June 11, 2026 10:38
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 32850806-42d4-4ed8-9a78-2a6a211f5936

📥 Commits

Reviewing files that changed from the base of the PR and between 2dc276c and c5d6ad6.

📒 Files selected for processing (1)
  • mpt_api_client/models/model.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • softwareone-platform/mpt-extension-skills (manual)
📜 Recent review details
⏰ 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
🧰 Additional context used
📓 Path-based instructions (2)
**/*

⚙️ CodeRabbit configuration file

**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved

Files:

  • mpt_api_client/models/model.py
**

⚙️ CodeRabbit configuration file

**: # AGENTS.md

Working protocol for any task in this repository:

  1. Identify the task type and select only the local repository files that are relevant to that task.
  2. Read only those relevant local files before making changes.
  3. If any selected local file references shared standards or shared operational guidance that are relevant to the same task, read those shared documents too before proceeding.
  4. Treat repository-local documents as repository-specific additions, restrictions, or overrides to shared guidance.
  5. If a repository-local rule conflicts with a shared rule, the local repository rule takes precedence.

Python API client for the SoftwareONE Marketplace Platform (MPT) API. Provides synchronous
(MPTClient) and asynchronous (AsyncMPTClient) clients built on httpx, with typed
resource services, mixin-based HTTP operations, and an RQL query builder.

Documentation Reading Order

When applicable, read the repository documentation in this order:

  1. README.md — repository overview, quick start, and documentation map
  2. docs/usage.md — installation, configuration, Python usage examples, and supported Docker-based commands
  3. docs/architecture.md — layered architecture, directory structure, and key abstractions
  4. docs/local-development.md — Docker-only setup and execution model
  5. docs/testing.md — repository-specific testing strategy and command mapping
  6. docs/contributing.md — repository-specific workflow and links to shared standards
  7. docs/documentation.md — repository-specific documentation rules

Then inspect the code paths relevant to the task:

  • mpt_api_client/mpt_client.py — public sync and async client entry points
  • mpt_api_client/http/ — HTTP clients, services, query state, and reusable mixins
  • mpt_api_client/resources/ — domain resource groups such as catalog, commerce, billing, and integration
  • mpt_api_client/models/ — response model layer and collection wrappers
  • mpt_api_client/rql/ — fluent RQL query ...

Files:

  • mpt_api_client/models/model.py
🧠 Learnings (2)
📚 Learning: 2026-02-17T10:04:00.873Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 210
File: mpt_api_client/rql/query_builder.py:18-18
Timestamp: 2026-02-17T10:04:00.873Z
Learning: In this repository, Ruff and flake8 with wemake-python-styleguide are used together. Do not remove WPS* noqa directives (e.g., WPS231) even if Ruff flags them as unknown in RUF102. Keep the directives to satisfy flake8 rules; ensure tooling configuration accounts for both linters to avoid false positives.

Applied to files:

  • mpt_api_client/models/model.py
📚 Learning: 2026-04-16T13:00:41.320Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 303
File: tests/e2e/helpdesk/chats/participants/conftest.py:25-31
Timestamp: 2026-04-16T13:00:41.320Z
Learning: In mpt-api-python-client, do not treat list-wrapped arguments to CreateMixin.create() / AsyncCreateMixin.create() as an error. ResourceData is intentionally typed as Resource | list[Resource] (see mpt_api_client/models/model.py), so create() should accept either a single resource dict or a list of resource dicts (e.g., create([chat_participant_data])) to perform a batch create and return a ModelCollection. Therefore, reviewers should only flag list-wrapped create() arguments when there is evidence they violate the expected API contract beyond this documented batch-create behavior.

Applied to files:

  • mpt_api_client/models/model.py
🪛 Ruff (0.15.15)
mpt_api_client/models/model.py

[warning] 129-129: Invalid rule code in # noqa: WPS221

Add non-Ruff rule codes to the lint.external configuration option

(RUF102)

🔇 Additional comments (3)
mpt_api_client/models/model.py (3)

116-122: LGTM!


124-132: LGTM!


204-211: LGTM!


📝 Walkthrough

Walkthrough

This PR refactors the BaseModel class to improve code maintainability by extracting nested value conversion logic into private helper functions. Two new helpers—_build_model_from_dict and _resolve_list_model_class—replace inline branching in _process_value, simplifying the method while preserving all existing behavior for dict-to-model and list conversions.

Changes

BaseModel nested value conversion refactoring

Layer / File(s) Summary
Extract helper functions for model construction and list type resolution
mpt_api_client/models/model.py
Added _build_model_from_dict to construct BaseModel/subclass instances from raw dicts using an optional target class, and _resolve_list_model_class to determine element model classes from list[...] type hints via typing.get_origin/get_args.
Simplify _process_value by delegating to new helpers
mpt_api_client/models/model.py
Refactored _process_value to delegate dict and list conversions to the new helper functions, removing the prior inline branching for target model class selection while maintaining the same nested structure conversion outcomes.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Pr And Commit Formatting ✅ Passed PR title follows Conventional Commits with JIRA-ID, description explains changes and rationale, single commit with body explaining what/why, and linear history with no merge commits.
Documentation Up To Date ✅ Passed This is a pure refactoring with no documentation impact: internal helper functions extracted from BaseModel._process_value, behavior unchanged, no public API/config/user-facing changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud

Copy link
Copy Markdown

@jentyk jentyk changed the title refactor(MPT-22046): reduce _process_value cognitive complexity below threshold MPT-22046 reduce _process_value cognitive complexity below threshold Jun 11, 2026
@jentyk jentyk merged commit 1d6ff11 into main Jun 11, 2026
4 checks passed
@jentyk jentyk deleted the refactor/MPT-22046 branch June 11, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants