refactor: enterprise-grade quality pass (38 backlog items)#2
Merged
Conversation
CRITICAL (all 6 resolved):
- C1: Split 769-line models.py into models/ subpackage (5 domain files)
- C2/C3: Extract _compute_sleep_time, _should_retry, _parse_error_body,
_parse_retry_after helpers from client.py (nesting depth reduced)
- C4: Fix in-place mutation in _remap_legacy_fields and Monitor.__init__
- C5: Typed Outage model with frozen=True; list_outages() returns list[Outage]
- C6: Remove shadow datetime import inside Maintenance.is_active()
HIGH (10 of 11 resolved):
- H1: _request return type corrected to dict | list
- H2/H3: _parse_list / _unwrap_list helpers extracted to _utils.py
- H4: _ClientProtocol in _protocols.py replaces 5 duplicate mixin stubs
- H5: Internal endpoint symbols removed from __all__; deprecated symbols
(HYPERPING_API_BASE, API_PATHS) emit DeprecationWarning via __getattr__
- H6: Race-condition docstrings + raise_on_conflict placeholder added
- H7: O(n) fetch cost documented in get_monitor_report
- H8: _validate_id() guards all resource ID URL interpolations
- H9: Bare except Exception narrowed to (ValueError, httpx.DecodingError)
- H10: HyperpingAuthError omits response_body to prevent token leakage
MEDIUM (14 of 20 resolved):
- M4: DNS-field cross-validation added to MonitorCreate
- M6: APIErrorResponse removed from __all__ (internal-only)
- M7/M8: ping() and default config comments clarified
- M9: period param Literal-typed with ValueError guard
- M10: add_subscriber validates email format client-side
- M13: CircuitBreaker.state typed CircuitState (not str)
- M14: state/failure_count reads acquire _lock (thread safety)
- M15: Debug logs sanitize sensitive field values
- M16: CircuitBreaker extracted to _circuit_breaker.py
- M17: All tests migrated from API_PATHS/HYPERPING_API_BASE to Endpoint enum
- M18: Legacy aliases removed from _incidents_mixin
- M19: _MONITOR_WRITABLE_FIELDS hoisted to module-level frozenset
- M20: params or None pattern applied
- M21/M22/M23: Missing test coverage added (update_incident, pause/resume
monitor, report parsing with nested outage details)
- M24: conftest fixture converted to yield-based (closes client after test)
LOW (all 7 resolved):
- L1: LocalizedText.get(lang, default) accessor added
- L2: f-string logging replaced with %-style args throughout
- L3: IncidentStatus/IncidentUpdateCreate emit DeprecationWarning (v0.3.0 removal)
- L4: ci.yml given permissions: contents: read
- L5: uv audit step added to ci.yml and publish.yml
- L6: httpx>=0.27,<1.0 and pydantic>=2.0,<3.0
- L7: Circuit breaker message references recovery_timeout correctly
Deferred (require semver bump or separate PR):
- M1 async client, M2 pagination, M3 per-endpoint CB, M11 URL validation,
M12 datetime coercion, H11 SHA pinning, M25 frozen request models
- Fix all 14 ruff lint errors (unused imports, N811/N813 naming, E501 line length, import sorting, dead TYPE_CHECKING block) - Fix all 6 mypy errors (stale type: ignore comments, bare dict type) - Replace 13 bare assert isinstance() calls with expect_dict() helper that survives python -O and gives clear error messages - Change _ClientProtocol from Protocol to regular base class to avoid metaclass/MRO concerns with non-Protocol mixin inheritance - Remove unused _URL_SCHEME_RE and re import from _monitor_models.py - Fix em dash in circuit breaker error message (violates project rules) - Fix CI audit step: use continue-on-error instead of || true so vulnerability findings are visible in the build summary - Remove redundant inner import in test_update_incident_not_found
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.
Summary
Resolves 38 of 45 backlog items from the enterprise-grade review (2026-04-02). All 177 tests pass at 92% coverage.
Changes
CRITICAL (all 6)
models.pyintomodels/subpackage with 5 domain files (C1)_remap_legacy_fields/Monitor.__init__(C4)Outagemodel;list_outages()now returnslist[Outage](C5)datetimeimport insideMaintenance.is_active()(C6)HIGH (10 of 11)
_utils.py:_parse_list,_unwrap_list,_validate_idhelpers (H2, H3, H8)_protocols.py:_ClientProtocolreplaces 5 duplicate mixin stubs (H4)_circuit_breaker.py:CircuitBreakerextracted fromclient.py(M16)__all__cleaned up; deprecated symbols emitDeprecationWarning(H5)raise_on_conflictplaceholder (H6, H7)except Exceptionto(ValueError, httpx.DecodingError)(H9)HyperpingAuthErroromitsresponse_bodyto prevent token leakage (H10)MEDIUM (14 of 20)
MonitorCreate(M4)periodparam Literal-typed withValueErrorguard (M9)add_subscriber(M10)CircuitBreaker.statetypedCircuitState; reads acquire lock (M13, M14)Endpointenum (M17); missing tests added (M21, M22, M23)conftest.pyfixture yield-based, closes client after each test (M24)LOW (all 7)
LocalizedText.get()accessor; %-style logging;DeprecationWarningfor aliases (L1, L2, L3)ci.ymlleast-privilege permissions +uv auditstep (L4, L5)httpx>=0.27,<1.0,pydantic>=2.0,<3.0(L6)recovery_timeoutcorrectly (L7)Deferred
Items requiring a semver bump or separate PR: M1 (async), M2 (pagination), M3 (per-endpoint CB), M11 (URL validation), M12 (datetime coercion), H11 (SHA pinning), M25 (frozen request models). All documented in BACKLOG.md.
Test plan
uv run pytest— 177 passed, 92% coverage (threshold 85%)test_update_incident_*,test_update_monitor_*,test_pause/resume_monitor,test_get_all_reports,test_get_monitor_report,test_get_monitor_report_not_found