fix(port): the documented datastore contract omitted the method documents() calls - #315
Merged
Merged
Conversation
…ents() calls Follow-up to #312. /code-review's finding 4 said the port documented itself as four-method when `documents` made it five. I fixed the module header and missed the thing that mattered: the CLASS docstring also enumerates the datastore methods a caller must supply, and `get_predictions_by_metadata` was absent from it. Measured rather than reasoned about — a double built to the documented contract: class DocumentedContract: def get_latest_file_id(self, filters): ... def get_file_metadata(self, fid): ... def download_prediction(self, fid): ... def upload_data(self, **kw): ... AttributeError: 'DocumentedContract' object has no attribute 'get_predictions_by_metadata' Mid-delivery, after the upload has happened — which is the one place this port exists to prevent a surprise. Found while verifying, for this repo rather than taking it on trust, that a pipeline-core fix to `get_latest_file_id`'s missing sort would actually reach us. It does: this repo vendors no copy of the Appwrite client, calls `search_files_by_metadata` nowhere, and the port takes any duck-typed datastore with `views_pipeline_core.modules.datastore.DatastoreModule` passed in production. So the ordering fix reaches us with a pin bump and no change here. faoapi and crafdapi each carry their own copy of that module and are not covered. The guard is derived from the source, not a hardcoded list: it collects every `self._dsm.<method>(` the port calls and asserts each appears in the module or class docstring. Prose cannot drift from the code without failing, which is how this one rotted. Mutation-proven by removing the fifth name, which fails both partners. ADR-014 §2 — the guard also asserts it found calls at all, so a refactor that renames `_dsm` cannot leave it silently scanning nothing. 526 passing. Both partner packages unchanged at 690 and 699. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9
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.
Follow-up to #312.
/code-review's finding 4 said the port documented itself as four-method whendocumentsmade it five. I fixed the module header and missed the thing that mattered: the class docstring also enumerates the datastore methods a caller must supply, andget_predictions_by_metadatawas absent.Measured, not reasoned about — a double built to the documented contract:
Mid-delivery, after the upload. The one place this port exists to prevent a surprise.
How it surfaced
While verifying — for this repo, rather than taking a peer's word — that a pipeline-core fix to
get_latest_file_id's missing sort would actually reach us.It does. This repo vendors no copy of the Appwrite client, calls
search_files_by_metadatanowhere, and the port accepts any duck-typed datastore with pipeline-core'sDatastoreModulepassed in production. So that fix arrives with a pin bump and no change here. views-faoapi and views-crafdapi each carry their own copy of that module and are not covered.The guard
Derived from the source rather than hardcoded: it collects every
self._dsm.<method>(the port calls and asserts each appears in a docstring. Prose cannot drift from code without failing — which is exactly how this rotted. It also asserts it found calls at all, so renaming_dsmcan't leave it silently scanning nothing (ADR-014 §2).Mutation-proven by removing the fifth name: fails both partners.
Verification
526 passing, ruff clean. Both partner packages unchanged at 690 and 699.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9