Add the postcardCollect flow and deprecate the legacy sighting API - #52
Conversation
test_schema.py only checks that the input types a query references exist; it cannot catch a malformed field selection or a missing field argument. Add graphql-core to the dev extra and a test that parses every query string in birdbuddy.queries and validates it against the committed introspection schema, so an invalid selection fails make check.
The graphql-core validation added in the previous commit flagged birds.SIGHTING_CREATE and birds.SIGHTING_CREATE_PROGRESS: the API removed their input types (SightingCreateInput, SightingCreateCheckProgressInput) and the sightingCreate mutation, so the queries no longer validate and any call already errors server side. Drop the two dead query constants and deprecate sighting_create and sighting_create_check_progress to raise NotImplementedError. Unlike the rest of the sighting flow, these have no postcard-flow equivalent to delegate to, because they drove a background media-analysis mutation the API abandoned.
Add the POSTCARD_COLLECT query string (with a CollectedPostcardFields fragment) and a CollectedPostcard model wrapping FeedItemCollectedPostcard, with accessors for species, medias, the mystery/new-species flags, inference mode/type, and the species-name identification confidence. Postcard confidence is reported by the API only as enum buckets (inferenceConfidenceLevel HIGH_CONFIDENCE/LOW_CONFIDENCE at the new-postcard stage; mediaSpeciesNameIdentificationConfidenceLevel CANNOT_DECIDE/VERY_CONFIDENT on the postcard) -- the numeric InferenceMediaRecognitionSuggestion.confidence is not reachable through the collect flow -- so the model surfaces the enum, not a score.
Add BirdBuddy.collect_postcard, which reanalyzes a postcard first (idempotent, so it is safe whether or not inference has already run) and then collects it with the species the backend recognized, returning a CollectedPostcard. Add a shared _postcard_id resolver used by both reanalyze and collect. Reanalyzing before collecting is the fix for the postcard internal-server errors: converting or collecting a postcard whose inference had not run errored server-side, and nothing ran inference first. Fixes jhansche#29 Related to jhansche/ha-birdbuddy#98
Add the RUF rule group so noqa directives that suppress an inactive or non-firing rule are reported, and remove the one it flags. Pin ruff to ~=0.16.3 so lint runs against a version that has the RUF rules, and move None to the end of the union that RUF036 flags.
Add the sighting-report preview's recognized species, the postcard media, and the feeder to POSTCARD_REANALYZE, so an analyzed postcard carries the fields a read-only preview needs. Rework the dumper's capture to send the library's own POSTCARD_REANALYZE and POSTCARD_COLLECT queries, so collect_flow.json cannot drift from what the client issues.
Add BirdBuddy.identify_postcard, which runs the AI inference and returns a PostcardAnalysis: the recognized species (from the sighting-report preview), the postcard media, and the feeder, without collecting. Add the PostcardAnalysis model. Deprecate reanalyze_postcard in favor of identify_postcard, keeping its raw inferenceExternalPostcardReanalyze return for compatibility. collect_postcard now identifies before collecting. Verified live against a real account: identifying a fresh postcard returned a recognized species with media and flipped inferenceExecutionMode to MANUAL_COMPLETED, confirmed in the app. The extended reanalyze omits reportToken/matchTokens, the payload bloat behind jhansche/ha-birdbuddy#78; that issue is fixed once the integration adopts this API.
identify_postcard and collect_postcard raise UnexpectedResponseError when the GraphQL response lacks the expected nested fields, instead of a bare KeyError or TypeError on result[...][...].
Deprecate sighting_from_postcard, finish_postcard, sighting_choose_species, and sighting_choose_mystery in favor of identify_postcard and collect_postcard. Extract _choose_species and _choose_mystery so finish_postcard can run the best-guess and mystery paths without emitting the choose methods' own deprecation warnings. A TODO marks them for removal with the deprecated flow.
Add an identify_postcard / collect_postcard usage example, and list the postcard-flow deprecations (reanalyze_postcard, the sighting-report methods, and the removed sighting_create methods) alongside latest_collections.
af5bd97 to
0b431d7
Compare
jhansche
left a comment
There was a problem hiding this comment.
LGTM - letting copilot take a pass, but I don't expect there to be anything major
There was a problem hiding this comment.
Pull request overview
This PR migrates the client’s postcard workflow to the newer postcardCollect flow, adds first-class models for postcard analysis/collection, and deprecates the legacy sighting/report API paths that no longer exist upstream. It also adds automated GraphQL query validation against the committed schema and updates developer tooling/docs to reflect the new flow.
Changes:
- Add
identify_postcard()(returnsPostcardAnalysis) andcollect_postcard()(returnsCollectedPostcard) and extend the reanalyze selection to include species/media/feeder details. - Deprecate/remove legacy sighting create/progress paths (now raise
NotImplementedError) and add supporting tests + a sanitized collect-flow fixture. - Validate all library GraphQL documents against
tests/fixtures/schema.jsonviagraphql-core, and enable RuffRUFrules.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_query_validation.py | New test validating all query constants against the committed GraphQL schema. |
| tests/test_postcards.py | New unit tests for the PostcardAnalysis model parsing behavior. |
| tests/test_client.py | Updates tests for deprecated sighting APIs; adds tests for identify/collect postcard and malformed responses. |
| tests/fixtures/collect_flow.json | Adds sanitized real-world capture fixture for the collect flow. |
| tests/conftest.py | Adds a fixture loader for the new collect-flow capture. |
| scripts/dump_payloads.py | Updates the capture script to dump the postcard collect flow using the library’s own queries. |
| README.md | Documents the new identify/collect postcard usage and updated deprecation notes. |
| pyproject.toml | Adds graphql-core to dev deps, updates Ruff pin, and enables RUF rules. |
| birdbuddy/queries/birds.py | Extends POSTCARD_REANALYZE selections and adds the POSTCARD_COLLECT mutation + fragment. |
| birdbuddy/postcards.py | Introduces PostcardAnalysis and CollectedPostcard models. |
| birdbuddy/client.py | Adds identify_postcard() / collect_postcard(), centralizes postcard id resolution, and deprecates/adjusts legacy sighting methods. |
Suppressed comments (4)
birdbuddy/client.py:962
- sighting_choose_species() makes sighting_data optional in the signature, but the implementation requires a report token (schema shows reportToken is NON_NULL in SightingChooseSpeciesInput) and will raise TypeError when sighting_data is omitted/None. Make sighting_data required in the signature to match the actual contract and avoid misleading callers/static typing.
This issue also appears on line 983 of the same file.
async def sighting_choose_species(
self,
sighting_id: str,
species_id: str,
sighting_data: SightingReport | str | None = None,
birdbuddy/client.py:1016
- sighting_choose_mystery() makes sighting_data optional in the signature, but the mutation input requires reportToken (NON_NULL in the schema) and the method raises TypeError when sighting_data is omitted/None. Make sighting_data required to match the API contract and improve type safety.
This issue also appears on line 1034 of the same file.
async def sighting_choose_mystery(
self,
sighting_id: str,
sighting_data: SightingReport | str | None = None,
) -> SightingReport:
birdbuddy/client.py:987
- _choose_species() treats sighting_data as required (it raises TypeError unless it’s a SightingReport or str token), but the signature suggests it’s optional by accepting None and providing a default. Tightening the type and removing the default better reflects the actual behavior and simplifies static analysis.
async def _choose_species(
self,
sighting_id: str,
species_id: str,
sighting_data: SightingReport | str | None = None,
birdbuddy/client.py:1038
- _choose_mystery() raises TypeError unless sighting_data is a SightingReport or str token, so allowing None in the type/default is misleading. Make sighting_data required in the helper signature to match the implementation and improve readability/type checking.
async def _choose_mystery(
self,
sighting_id: str,
sighting_data: SightingReport | str | None = None,
) -> SightingReport:
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
_postcard_id raises TypeError for an argument that is neither a str nor a FeedNode, which identify_postcard propagates and a test asserts, so list it in the Raises section (collect_postcard already does).
|
I'll have the next release out shortly (with breaking changes noted) |
Second part of splitting #41 into something more reviewable.
Summary
identify_postcard()returns aPostcardAnalysis(species, media, feeder) without collecting;collect_postcard()collects into aCollectedPostcard. AddsPOSTCARD_COLLECTand the two models.POSTCARD_REANALYZE— select the recognized species, media, and feeder.sighting_createqueries.RUFrules — tooling maintenance; flagsnoqadirectives that suppress an inactive rule. Removed one that previously slipped in by mistake.identify_postcard/collect_postcard— raiseUnexpectedResponseErroron a malformed response.reanalyze_postcard,sighting_from_postcard,finish_postcard,sighting_choose_species,sighting_choose_mystery(useidentify_postcard/collect_postcard);sighting_create/sighting_create_check_progressnow raiseNotImplementedErrorsince the API removed the mutations.Issue references
collect_postcardreanalyzes before collecting.Notes
identify_postcardon a fresh postcard returned the recognized species and media and flippedinferenceExecutionModetoMANUAL_COMPLETED(confirmed in the app, needed a pull-to-refresh but it worked!); a re-identify returnedALREADY_REANALYZED, so the internal reanalyze is idempotent.sighting_createqueries; the next commit removes them (wanted proper TDD). Happy to merge the two together if you'd rather not have a red commit in the history, thought it would be easier to review individual commits this way though.reanalyze_postcardis deprecated; its raw payload now carries the extra fields from the extended query (additive, so callers are unaffected).Follow-ups