Skip to content

fix: integration idempotency suppressed banner#83

Merged
marcinpsk merged 4 commits into
mainfrom
fix/integration-idempotency-suppressed-banner
Jun 3, 2026
Merged

fix: integration idempotency suppressed banner#83
marcinpsk merged 4 commits into
mainfrom
fix/integration-idempotency-suppressed-banner

Conversation

@marcinpsk

@marcinpsk marcinpsk commented Jun 3, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Chores

    • Include the project lockfile in release artifacts and update release build command.
    • Add docstring-coverage tooling and a pre-commit hook to run it.
  • Tests

    • Make idempotency tests more robust to suppressed change-detection banners; rely on an always-present end-of-run summary.
  • Documentation

    • Clarified docstrings describing export selection behavior, image download expectations, and repository pickle constraints.

marcinpsk added 2 commits June 1, 2026 11:34
PR #74 suppresses the CHANGE DETECTION REPORT banner when there are no
new/modified types, so the second-run output no longer contains
'New device types: 0'. Update Scenarios G and H to treat a suppressed
banner as a valid no-change signal and verify idempotency against the
always-printed '0 device types created/updated' summary.
semantic-release bumped only pyproject.toml, leaving uv.lock's project
version stale (drifted to 1.6.0 while the project was at 1.6.1). Add
'uv lock' as the build_command and include uv.lock in the release commit
via assets so the lockfile stays in sync on every release. Also backfill
the current uv.lock to 1.6.1.
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f8cba221-63e3-4566-a445-83e86ea1b8ed

📥 Commits

Reviewing files that changed from the base of the PR and between 17299bc and b87a617.

📒 Files selected for processing (6)
  • .github/workflows/tests.yml
  • .pre-commit-config.yaml
  • core/export.py
  • core/repo.py
  • pyproject.toml
  • tests/integration/test_import.py

Walkthrough

Adds interrogate configuration and hooks, runs docstring checks in CI, updates Semantic Release to include uv.lock, expands internal docstrings, and makes integration tests resilient to suppressed change-detection banners by asserting end-of-run summaries.

Changes

Tooling, CI, Tests, and Docstrings

Layer / File(s) Summary
Project tooling configuration
pyproject.toml
Add [tool.interrogate] settings (coverage threshold, exclusions, ignore flags), enable branch = true, set [tool.semantic_release].build_command = "uv lock", and add uv.lock to assets.
CI workflow and pre-commit hook
.github/workflows/tests.yml, .pre-commit-config.yaml
Add a GitHub Actions step "Check docstring coverage" running uv run interrogate and a local pre-commit interrogate hook that runs uv run --native-tls interrogate always.
Integration test idempotency assertions
tests/integration/test_import.py
test_idempotency now conditionally checks change-detection banners when present and otherwise asserts banner suppression; both test_idempotency and test_update_mode always verify end-of-run 0 device types created / 0 device types updated via regex.
Expanded internal docstrings
core/export.py, core/repo.py
Clarify how export candidate sets are determined for device/module/rack types, describe device-type elevation image download behavior and success semantics, and state that _RestrictedUnpickler.find_class rejects class lookups and expects string tuples.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A little lock hops into the release tray,
Docstrings bloom so checks know the way,
CI runs a probe for every doc line,
Tests now listen for the end-of-run sign,
Quiet banners or loud, the summary says "fine."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix: integration idempotency suppressed banner' directly addresses the main change: updating integration tests to handle suppressed CHANGE DETECTION REPORT banners by validating against the always-printed end-of-run summary instead.
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.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/integration-idempotency-suppressed-banner

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@marcinpsk

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/test_import.py`:
- Around line 496-503: The regexes r"0 device types created" and r"0 device
types updated" match substrings like "10 device types created"; update them to
anchor the zero so it won't match multi-digit counts (e.g. use word boundaries):
replace those patterns with r"\b0 device types created\b" and r"\b0 device types
updated\b" in the loop that checks the end-of-run summary.
- Around line 545-551: The regexes used in the post-update idempotency checks
(the re.search calls that inspect result2.stdout after run_importer())
incorrectly match numbers like "10" or "20"; update both checks to assert an
exact zero by using a digit-boundary-aware pattern such as a negative-lookbehind
so you only match a literal "0" (e.g., replace the patterns for "0 device types
created" and "0 device types updated" with a pattern using (?<!\d)0 ... ),
keeping the same calls to run_importer(), result2, and re.search.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 83806f8b-45d4-4af3-9935-67577a1c78ef

📥 Commits

Reviewing files that changed from the base of the PR and between 5dc5aa9 and 17299bc.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • pyproject.toml
  • tests/integration/test_import.py

Comment thread tests/integration/test_import.py
Comment thread tests/integration/test_import.py
marcinpsk added 2 commits June 3, 2026 14:43
Address CodeRabbit review on the end-of-run summary checks: the bare
"0 device types created/updated" patterns also matched "10"/"20"/etc.
Anchor them with a (?<!\d) lookbehind so only a literal zero passes,
in both test_idempotency and test_update_mode.

Also configure interrogate ([tool.interrogate]) to measure docstring
coverage on first-party source only — excluding the vendored repo/ and
test functions (mirrors the ruff D102/D103 ignore for tests/**) and
nested helpers. Backfill the few missing docstrings so first-party
coverage is 100%.
- Add interrogate to pre-commit and the tests workflow so the docstring
  floor is actually gated rather than advisory.
- Bump interrogate fail-under 80 -> 95 (first-party is at 100%).
- Enable branch coverage in coverage.py; suite passes the 96% gate
  at 96.70% with branches counted.
@marcinpsk marcinpsk merged commit 95ec314 into main Jun 3, 2026
7 checks passed
@marcinpsk marcinpsk deleted the fix/integration-idempotency-suppressed-banner branch June 3, 2026 14:03
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.

1 participant