Harden collection pagination and expand test coverage - #53
Merged
jhansche merged 6 commits intoAug 15, 2026
Conversation
_iter_pages raises UnexpectedResponseError when a response lacks the connection object or it is not a dict, and collection() does the same when a page is missing the expected media edges, instead of a bare KeyError, TypeError, or AttributeError. This matches the postcard paths, which already raise UnexpectedResponseError.
Add tests/fixtures/api_payloads.json (a sanitized capture of live me, collections, and new_postcards responses) with a conftest loader, plus test_api_payloads.py pinning the model shapes: the owner feeder, the new-postcard feed nodes, and the identify_postcard analysis from the collect-flow reanalyze capture.
Cover previously untested client operations via graphql_mock and the api_payloads fixture: sign-in, token refresh, refresh, collection filtering, feeder options, media sharing, power profile, and the firmware check. The power-profile and firmware-check responses are real captures: both mutations resolve asynchronously (power profile reports the old value until a refresh; an up-to-date firmware check returns a succeeded result), so the tests exercise that observed behavior.
Extend scripts/dump_payloads.py to read the ME profile and collections alongside the feeders and postcard flow, using the library's own queries.me.ME and queries.me.COLLECTIONS. This is what produced the api_payloads.json fixture; both reads are non-destructive. Also make the POSTCARD_COLLECT mutation opt-in via BB_COLLECT_POSTCARD_ID so a default run is read-only. Previously the dumper collected postcards[0] on every run, an irreversible change to the account.
Contributor
Author
|
Fyi, these are the last changes I had for pybirdbuddy at the moment, everything else I have to work on will be in ha-birdbuddy, I promise :) appreciate all the time you spent on reviews and merges 🙇♂️ |
jhansche
reviewed
Aug 14, 2026
jhansche
approved these changes
Aug 14, 2026
There was a problem hiding this comment.
Pull request overview
This PR hardens GraphQL collection pagination error handling in the BirdBuddy client, expands test coverage using a sanitized real-world payload fixture, and enhances the payload-dumping script to gather additional read-only data while making postcard collection opt-in.
Changes:
- Raise
UnexpectedResponseErrorfrom_iter_pages()/collection()when pages are malformed (instead of leakingKeyError/TypeError). - Add a sanitized real API payload fixture plus model/client tests that pin key response shapes and behaviors.
- Extend
scripts/dump_payloads.pyto capturemeandcollections, and require explicit opt-in for postcard collection.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
birdbuddy/client.py |
Hardens pagination + collection parsing to raise UnexpectedResponseError on malformed responses. |
tests/test_client.py |
Adds client behavior tests for refresh/login/token refresh/collection filtering/options/sharing/power profile/firmware check. |
tests/test_api_payloads.py |
Adds model-shape regression tests against a sanitized real API payload capture. |
tests/fixtures/api_payloads.json |
Introduces scrubbed real payload fixture used by new tests. |
tests/conftest.py |
Adds api_payloads fixture loader for the new JSON fixture. |
scripts/dump_payloads.py |
Captures me + collections and makes postcard collect opt-in via BB_COLLECT_POSTCARD_ID. |
Suppressed comments (3)
tests/fixtures/api_payloads.json:144
- This fixture is described as sanitized and other URL fields are replaced with https://example.invalid/asset, but this mapUrl still points at a real Bird Buddy asset host. Replace it with the same placeholder URL to keep the fixture fully scrubbed and avoid leaking/depending on external hosts.
"mapUrl": "https://assets.cms-api-graphql.cms-api.prod.aws.mybirdbuddy.com/asset/map/bird/a34bcd83-2d18-4d2a-8b97-2f591ff79985_Pine%20Siskin.svg"
tests/fixtures/api_payloads.json:188
- This fixture is described as sanitized and other URL fields are replaced with https://example.invalid/asset, but this mapUrl still points at a real Bird Buddy asset host. Replace it with the same placeholder URL to keep the fixture fully scrubbed and avoid leaking/depending on external hosts.
"mapUrl": "https://assets.cms-api-graphql.cms-api.prod.aws.mybirdbuddy.com/asset/map/bird/d8c20684-d366-4933-8ceb-4955ec9459a1_House%20Finch.svg"
tests/fixtures/api_payloads.json:232
- This fixture is described as sanitized and other URL fields are replaced with https://example.invalid/asset, but this mapUrl still points at a real Bird Buddy asset host. Replace it with the same placeholder URL to keep the fixture fully scrubbed and avoid leaking/depending on external hosts.
"mapUrl": "https://assets.cms-api-graphql.cms-api.prod.aws.mybirdbuddy.com/asset/map/bird/bd6ef12e-955d-488c-af4e-c5462d6979aa_Red-Breasted%20Nuthatch.svg"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Scrub the mapUrl asset host in api_payloads.json to example.invalid, and add mapUrl to the dumper's _URL_KEYS so future dumps scrub it too. - Capture the profile and collections through _capture so a server error lands in the dump instead of aborting the run before the postcard steps. - Reanalyze the same feed item that gets collected when BB_COLLECT_POSTCARD_ID is set, so the captured reanalyze and collect steps stay consistent. - Drop the stale capture date from the collect-flow fixture docstring.
Contributor
Author
|
Thanks for the review! Just pushed 587e5e2 to address the feedback items. One was a false positive (CI is green) and I commented inline. |
jhansche
approved these changes
Aug 15, 2026
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
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.
A bit of additional hardening and test coverage, plus improvements to the API dumper script.
Summary
collection()and_iter_pages()now raiseUnexpectedResponseErroron a malformed page instead of a bareKeyError/TypeError.tests/fixtures/api_payloads.jsonis a scrubbed capture; adds model tests that pin the feeder, new-postcard, and postcard-analysis objects.POSTCARD_COLLECTopt-in viaBB_COLLECT_POSTCARD_ID, so a default run is read-only (previously it collected a postcard on every run, which might negatively surprise devs).Notes
client.py58% to 70%).me,collections,new_postcards, andreanalyzeshapes all match the committed fixtures, and I confirmed that no postcard was collected.