Skip to content

refactor: simplify endpoint validation and apply review follow-ups - #221

Merged
tjorim merged 5 commits into
stack/pr-220from
fix/pr-review-follow-up
Oct 21, 2026
Merged

tjorim merged 5 commits into
stack/pr-220from
fix/pr-review-follow-up

Conversation

@tjorim

@tjorim tjorim commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Endpoint validation repeats the public method signatures in a central rule table. Move those checks into the endpoint methods so invalid arguments fail before creating a session or sending a request, and complete the remaining review cleanup.

  • Remove _ENDPOINTS, _EndpointSpec, _frozen_spec, and _validate_params. Keep shared date, time, and fixed-choice validation, with explicit required-value and station/station_id checks in the relevant methods. Errors use public Python parameter names.
  • Preserve HTTP status 200 on InvalidResponseError when response-model deserialization fails, and only refresh an ETag from a 304 when the request sent If-None-Match.
  • Use pytest-cov for the existing 92% combined statement/branch coverage gate, removing the direct coverage[toml] dependency and updating CI and local instructions.
  • Restore useful comments explaining retry accounting, API quirks, optional fields, and release-version derivation.
  • Align client imports, examples, tests, and wheel validation with IRail, station_id, and vehicle_id. The client has no compatibility alias.

Public endpoint signatures and serialized query parameters are unchanged by the validation refactor. Regression tests exercise validation through public methods, including rejection before HTTP activity.

Validation: the combined tree passed 116 offline tests on Python 3.12 and 3.14 with 100% measured statement/branch coverage. Ruff, formatting, and ty passed. The built wheel installed in a fresh Python 3.12 environment and passed isolated import and py.typed checks. Live API tests were not run.

Integration: this PR still targets stack/pr-220, the original #220 snapshot. Integrate the latest main fixes and retarget to main before merging.

Summary by CodeRabbit

  • Breaking Changes

    • Renamed the public client from iRail to IRail.
    • Renamed request parameters for liveboards, vehicles, and compositions.
    • Liveboard requests now require exactly one of a station name or station ID.
  • Improvements

    • Added clearer validation for required values, dates, times, and timeouts.
    • Improved handling of cached responses and invalid API response data.
  • Documentation

    • Updated usage examples and testing instructions to reflect the new client name and coverage tooling.
  • Testing

    • Expanded endpoint, validation, retry, rate-limit, and response handling coverage.

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6a60106e-7cb1-4be6-b901-ae52c4d66bd9
📥 Commits

Reviewing files that changed from the base of the PR and between e52a6aa and 72d62fb.

⛔ Files ignored due to path filters: 1
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing: 12
  • .github/workflows/build.yml
  • .github/workflows/publish.yml
  • .github/workflows/test.yml
  • README.md
  • pyproject.toml
  • pyrail/__init__.py
  • pyrail/irail.py
  • pyrail/models.py
  • tests/live/conftest.py
  • tests/live/live_api.py
  • tests/live/test_live_api.py
  • tests/test_irail.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The package client is now named IRail. Endpoint methods validate required and fixed-choice parameters locally. The request handling updates ETag and deserialization behavior. Tests, documentation, and workflows reflect the client name and coverage command changes.

Changes: Client API and request handling

Layer / File(s) Summary
Public client and endpoint validation
pyrail/__init__.py, pyrail/irail.py, tests/test_irail.py, tests/live/*, README.md, .github/workflows/build.yml
The package exports IRail instead of iRail. Endpoint methods validate required arguments and accepted values, and use station_id and vehicle_id for identifiers. Unit and live tests and README examples use the updated names and arguments.
Response, cache, and retry handling
pyrail/irail.py, pyrail/models.py, tests/test_irail.py
A 304 response stores its ETag only when the request included If-None-Match and the cache generation is unchanged. Deserialization errors use status 200. Related response, cache, and retry tests are updated.
Build and coverage workflow updates
.github/workflows/*, pyproject.toml, README.md
The build import check targets IRail. Test coverage uses pytest-cov and its dependency replaces coverage[toml]. The publish workflow comments on the Git history needed by hatch-vcs.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Suggested reviewers: daanvervacke

Merge Risk: ⚪ Minimal · up to 72d62

No actionable issue remains from this review; the change is mergeable after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: simplifying endpoint validation. It also accurately references the related review follow-ups.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 7 files. (5 skipped: 5…
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 7 files. (5 skipped: 5 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the station sign,
Then calls IRail along the line.
The cache keeps tags when rules are met,
The tests catch cases not seen yet.
Coverage hops through clover green.

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

@tjorim tjorim changed the title fix: address final PR stack review findings refactor: simplify endpoint validation and apply review follow-ups Oct 21, 2026
@tjorim

tjorim commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@tjorim
tjorim merged commit 9e5cbb6 into stack/pr-220 Oct 21, 2026
6 checks passed
@tjorim
tjorim deleted the fix/pr-review-follow-up branch October 21, 2026 13:08
@tjorim
tjorim restored the fix/pr-review-follow-up branch October 21, 2026 13:15
tjorim added a commit that referenced this pull request Oct 21, 2026
Bring the combined #221 and #222 changes onto main: validate requests in endpoint methods, preserve HTTP 200 on model parsing errors, adopt pytest-cov, and restore useful comments.

Preserve newer main fixes and dependency updates while resolving integration conflicts and deduplicating an ETag regression test.
@tjorim
tjorim deleted the fix/pr-review-follow-up branch October 21, 2026 13:24
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