Skip to content

refactor(backend): enable ruff SIM102 and collapse nested if statements - #10315

Draft
ogenstad wants to merge 1 commit into
developfrom
po-enable-ruff-sim102
Draft

refactor(backend): enable ruff SIM102 and collapse nested if statements#10315
ogenstad wants to merge 1 commit into
developfrom
po-enable-ruff-sim102

Conversation

@ogenstad

@ogenstad ogenstad commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Why

SIM102 (collapsible-if) was suppressed in two places in pyproject.toml: the global ignore
list, and a backend/tests/fixtures/**.py per-file entry sitting under a "Refactor code and remove
the ignore rule" header. Both suppressions are now gone and the rule is fully enforced.

Goal: enable the rule with zero behavioral change.

Non-goals: no logic changes, no restructuring beyond what the rule requires, no changes to the
other rules still listed in either ignore block.

What changed

Behavioral changes: none. Every edit is structural.

  • Removed "SIM102" from the global ignore list and from the
    backend/tests/fixtures/**.py per-file ignores. grep -c SIM102 pyproject.toml is now 0.
  • Fixed the 38 violations that surfaced (36 in backend/infrahub/, 2 in fixtures) across 23 files.
    Nested if A: if B: becomes if A and B:, which short-circuits identically.
  • Two sites in core/node/__init__.py where the collapsed form wrapped a multi-line await in a
    walrus inside an and were instead destructured into an assignment plus a plain if. That also
    satisfies the rule (the outer body then holds two statements) and matches the idiom already used
    a few lines above.
  • Flattened redundant parenthesization the autofix left behind - (A and B and C) and D - in
    attribute_parameters.py and groups/parsers.py. No and-within-and or or-within-or
    nesting remains anywhere in the diff.

What stayed the same: no schema changes, no API changes, no dependency changes. Three comments
moved from inside an outer if to just above the merged condition; no comment was added, removed,
or reworded.

How to review

Most of the diff is mechanical and was produced by ruff check --fix. Worth actual attention:

  1. backend/infrahub/core/node/__init__.py - the two destructured await blocks in delete(),
    and the multi-term guard chains in _collect_extra_filters / process_label.
  2. backend/infrahub/core/schema/schema_branch.py - identify_required_object_templates, the one
    condition mixing or inside and.
  3. Four sites where an elif was collapsed (graphql/app.py, graphql/field_extractor.py,
    core/diff/enricher/cardinality_one.py x2). Collapsing an elif is only sound when it is the
    terminal branch; each was verified to have no trailing else.

Two things reviewers should weigh in on:

  • New idiom. Before this PR the backend had 110 walrus operators and zero used as a boolean
    operand. This PR introduces 14 across 8 files, 8 of which put the walrus in a later operand
    (if guard and (x := f()):), where the binding is conditional. It is correct - the name is only
    read inside the body, and the binding condition is unchanged from the nested original - but it is
    a convention arriving via a lint sweep. The alternative is destructuring those 8 the way the
    delete() blocks were. Happy to do that if preferred.
  • Fixture drift. The two fixture edits live in local copies of infrahub-demo-edge. If that
    fixture is ever re-synced from upstream the violation returns, and because the per-file ignore is
    gone, CI will fail until it is re-applied. That is the intended tripwire rather than an
    oversight; a follow-up cleanup issue on opsmill/infrahub-demo-edge could remove the friction.

How to test

uv run ruff check .            # clean
uv run ruff format --check .   # clean
uv run mypy backend/infrahub/  # clean on changed files
uv run pytest backend/tests/unit -q

Beyond the standard checks, equivalence was proven mechanically rather than by eye. Both versions
of every changed file were canonicalised to a maximally-nested normal form (explode and-chains
into nested ifs; de-sugar sole-test walruses into assignment plus test; flatten same-operator
boolean nesting - all three behaviour-preserving), then compared as ASTs. All 23 files are
identical to their base version under that normal form, and comment token streams match exactly.

The canonicaliser was itself validated against 26 control cases, confirming it rejects andor,
flipped comparisons, reordered operands, dropped negations, swallowed else branches, non-terminal
elif collapses, dropped sibling statements, and/or regrouping, and - specifically for the
destructured blocks - an assignment hoisted outside its guard.

The edited fixture check was additionally differential-tested: old and new validate() driven
through 122 enumerated input combinations, comparing recorded log_error calls. Zero mismatches,
with 112 cases producing non-empty output.

Impact & rollout

  • Backward compatibility: no behavioral change; nothing to migrate.
  • Performance: unaffected.
  • Config/env changes: none at runtime; pyproject.toml lint config only.
  • Deployment notes: safe to deploy.

Checklist

  • Tests added/updated - n/a, no behavior change; existing suite covers the touched paths
  • Changelog entry - n/a, not user-facing
  • External docs updated - n/a
  • Internal .md docs updated - n/a
  • I have reviewed AI generated content

Review in cubic

Remove the SIM102 (collapsible-if) suppressions from pyproject.toml - both
the global `ignore` entry and the `backend/tests/fixtures/**.py` per-file
entry - and fix the 38 violations that surfaced.

All changes are structural: nested `if A: if B:` pairs become `if A and B:`,
which short-circuits identically. No conditions, operands, branches or
comments were altered.

Two sites in core/node/__init__.py where the collapsed form wrapped a
multi-line `await` in a walrus were destructured into an assignment plus a
plain `if` instead, which satisfies the rule and reads better.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label Aug 19, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 13 untouched benchmarks


Comparing po-enable-ruff-sim102 (56aee23) with develop (e6e28c2)

Open in CodSpeed

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 24 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would auto-approve. Refactor enables ruff SIM102 lint rule by collapsing nested ifs into equivalent compound conditions with verified zero behavioral change across 24 files.

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant