release: 1.4.0 to main - #317
Merged
Merged
Conversation
…y category (#312) The first live UN-FAO delivery, 2026-09-29, uploaded 109 of 110 objects and reported success. views-faoapi refused it. The missing object was the §5 GAUL sidecar: its bytes matched the previous run's, the content-addressed store correctly declined a second copy, update_document ran against the OLD file, and _ContractStorePort.upload returned a real file id for the wrong document. The consumer resolves by filename, found nothing, refused. The C-94 guard did not catch it, and could not have. It checked two entries, {"forecast": manifest_id, "historical": hist_id}, via latest_file_id({"name": ..., "category": category}). Every wire object is uploaded with category="forecast" (sink.py:227), so the newest forecast document is always the manifest, uploaded last by design. Adding the sidecar to that dict would have resolved the manifest and passed. The lookup key was wrong, not the dict. So the guard now asks two different questions, because neither implies the other: legs — does the consumer's SELECTION land on this run? (unchanged) objects — does each artefact resolve by its own filename AT ALL? (new) Per object rather than by count, and the decisive reason is not the one I started with. Pooling upstream is deterministic — measured in views-models on 2026-09-29, 25 of 25 anchor cells byte-identical across an accidental re-pool — and naming.py embeds the run id in every filename. A re-run therefore writes NEW names over IDENTICAL bytes, so the dedup path that took the sidecar takes all 110 objects at once: the store creates the documents, the count is right, nothing is servable. A count-plus-spot-check passes that. Re-running is what C-105 and C-22 tell an operator to do after a torn run, which makes it the realistic case rather than the exotic one — our own remedy is the trigger for the worst version. The query loop moved OUT of both partner managers into delivery/findability.py. Not incidental: the partner packages are under a ratcheting line budget whose own comment says the response to it binding is to move code out of the package rather than raise the number, and crafd/ sat at 699 of 700. Delegating paid for the wider call site exactly — both partners are unchanged at 690 and 699. It also leaves one copy of the rule instead of two that can disagree (the C-75 shape). An unqueryable filename degrades to UNVERIFIED, never to "invisible". filename is a declared collection attribute (pipeline-core provisioning.py:101), but if a store cannot be queried on it the honest answer is "could not ask" — a guard that manufactured an outage on every delivery would be deleted within a week (ADR-014 §3). Five mutations, each caught: report only the first missing object; accept any id as evidence; pass only the two legs (the shipped bug); stop exporting the sink's ledger; swallow a failed query as not-found. Complementary to views-pipeline-core's separate fix for the upload that reports success having written nothing. Neither waits on the other: theirs stops the lie at the source, this one stops us believing a lie from any source. Noted, not acted on: this is the first bug requiring an identical hand-patch in both partner managers, which is one of the two triggers C-33 names for extracting the partner seam. Recorded on #312; extracting it here would be a different change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9
…lookup (#312 review) Review finding from the views-models seat, verified here before acting. The per-object check queried `filename` directly. Three facts say it would not work: 1. `Query.equal("filename", ...)` appears NOWHERE on the platform. The hits in pipeline-core, faoapi and crafdapi are `Query.equal("name", filename)` against the storage BUCKET — a different call on a different object. 2. `filename` is declared in pipeline-core's collection schema (provisioning.py:101) and **no index is declared anywhere in that file**. 3. views-faoapi — the consumer being modelled — deliberately does not query it. `resolve_artifact_file_ids` (prediction/manager.py:277-295) runs a type-scoped query on {category, type} and matches `filename` in Python. Its docstring says so, and that shape ran against the real store during the 2026-09-29 incident. Left as written, the guard would return UNVERIFIED on every delivery forever while its presence read as coverage — a guard that structurally cannot fire (views-models C-155), which is worse than the bug it replaces. The failure direction was right and the query was wrong. So the check now uses faoapi's shape: scope to {name, category, type}, match filename in Python. Better on its own merits, index question aside — it asks the question the consumer actually asks, which is C-94's thesis rather than an approximation, and it costs one query per artefact TYPE rather than one per object (110 -> 3). Carrying it needed the sink's ledger to record doc_type, and the port to expose `documents()` alongside `latest_file_id`. Both partner packages came in UNDER where they started: unfao 690 -> 688, crafd 699 -> 697. The guard's docstring and success log moved into `findability.verify` with the rule they describe — one home per fact, and the budget's own instruction is to move code out of the package rather than raise the number. Moving `store_port.py` out was considered and rejected: it is byte-identical in both partners and would free 111 lines each, but it is referenced by six register entries, an anti-drift test and ADR-015, and that blast radius does not belong inside an incident fix. Six mutations, each caught: revert to a filename query; accept any id as evidence; report only the first missing object; pass no objects (the shipped bug); swallow a failed query as not-found; stop exporting the ledger. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9
…oved Three findings from /review-diff on this branch, two of them self-inflicted by the rework that answered the review. 1. WARNING — findability.py said "asked for that exact filename", "Why by filename and not by id", and "NOT FOUND by filename". After the rework the store is NOT queried on filename; it is a type-scoped query matched in Python, precisely because that attribute has no index. A reader would have concluded the opposite, and anyone simplifying would have restored the broken form. The test guards the filters; the prose was inviting the change. 2. WARNING — the CIC still said "After both legs are uploaded" and "Two distinct refusals". Both wrong: every artefact is checked now, and there are four refusals across two questions. Updated, with why the per-object half resolves the way faoapi does rather than by filename. Review date moved. 3. SUGGESTION — `objects` carried no annotation while `legs: dict` sat beside it. Annotated in verify and in both managers. Checked and NOT findings, recorded so they are not re-raised: blank-line hygiene is correct, `logger` remains used in both managers, and `assert_findable` is still reachable through the legs loop. Both partner packages remain under budget at 690 and 699. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9
…(/code-review) Six findings from /code-review medium on PR #314. It also traced the new query down through pipeline-core to Appwrite and confirmed the mechanism holds: the scope set matches faoapi's query exactly, newest-first is guaranteed by the $createdAt sort so first-wins reproduces the consumer's selection rather than approximating it, pagination is handled upstream (MAX_METADATA_PAGES 1000 x 100) so 110 objects are not truncated, and pipeline-core's own comment at file.py:1216 independently corroborates that `filename` carries no index. MEDIUM 1 — a polarity bug, and the worst kind. The parse sat OUTSIDE the try. If the store returned objects rather than dicts, or renamed its keys, `doc.get` raised, `seen` stayed empty, and EVERY object was reported not found — firing the full "NOTHING in this run is servable" refusal on a healthy delivery. A failure of the check must never quarantine the delivery; this module argues that twice, in C-99's and C-103's words, and then did the opposite. The parse is now inside the try. Mutation-proven: moving it back out fails the new test. MEDIUM 2 — silent fallback. `summary.get("uploaded_objects", [])` would have let the guard quietly shrink to the single historical object and log "preflight passed" while 108 shards, the sidecar and the manifest went unasked — the precise shape of the bug being fixed. The key is guaranteed on every path that reaches the guard (the interlock-closed path returns before it), so its absence is a defect and now reads as one (ADR-003). LOW 3 — `documents()` had no test anywhere. It is the only method the per-object check depends on, and the findability tests inject callables, so a pipeline-core rename would have surfaced during a live delivery. Now asserted at the port, which is the seam whose job is absorbing exactly that. LOW 4 — the port documented itself as four-method; `documents` makes it five. A datastore satisfying the documented contract would have failed with AttributeError mid-delivery. Also fixed, reported as not-a-finding: `unverified()` hardcoded the word "leg", so a scope rendered as "'forecast/sampled_forecast_shard objects' leg". It now takes the phrase. And the module docstring claimed "no store types" while verify reads two document KEYS — the boundary is now stated honestly rather than overclaimed. LOW 5 (SEARCH_INCOMPLETE noise growing ~110 documents per run) and LOW 6 (the budget-driven inline dict at the call site) are recorded on #312 rather than changed here: 5 needs a C-94 note and no code, and 6 costs lines neither partner package has. 522 passing. Both packages unchanged at 690 and 699. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9
…#312 re-review) The views-models seat could not find the `$createdAt` sort I had twice asserted as fact. They were right and I was wrong, and the way I was wrong is the worse part: an automated review asserted it, I relayed it to them twice without checking, and it became a rationale I was about to put in the code. Verified here. `search_files_by_metadata` builds `queries = []` and appends only `Query.equal(attribute, value)` per filter, then `limit` + `offset` (pipeline-core `modules/appwrite/file.py:1045-1050`). No `order_desc`, no `order_asc`, no sort field. The only ordering calls in that module are at `2643-2645`, inside `list_files` — a STORAGE listing, not a collection query — where `order_field` defaults to None. So document order is unspecified and "first match wins" was a coin flip dressed as a tie-break. Rather than record the caveat, the assumption is gone. `seen` now maps filename to the SET of ids carried under it, and an object is findable when the id THIS run uploaded is among them. That is the question we actually have, and it needs no ordering guarantee at all. The refusal now shows every id the store holds under a name instead of one, which is strictly more useful for an operator auditing a bucket. Two tests: the same delivery passes under three different document orders including a stale same-filename document returned first, and order-independence does not become permissiveness — a name carrying only other runs' ids is still refused. Mutation-proven by restoring first-wins, which fails both. NOT fixed here, because they are not ours, and both are now on #312: pipeline-core's `get_latest_file_id` DOCUMENTS "the newest matching file based on creation timestamp" and implements `files_list[0]` over that unsorted result — a stated guarantee with no mechanism, and the C-94 leg check has depended on it since August. views-faoapi's `resolve_artifact_file_ids` carries the same "Newest-first" assumption in its docstring. 524 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
…fact-by-name fix(delivery): verify every uploaded artefact by name, not two legs by category (#312)
…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
…tract fix(port): the documented datastore contract omitted the method documents() calls
…s tagged Bumps 1.3.0 -> 1.4.0. MINOR rather than PATCH by the same reasoning as 1.2.0: a previously-passing delivery can now stop. The guard refuses in four situations instead of two. That is a behaviour change a launcher must be told about, not a patch. Cut now rather than at convenience because the launcher installs from a git TAG (views-models tools/launcher/postprocessor.sh:57), not from PyPI and not from poetry.lock. Until this tag exists, the fix for today's unservable delivery is merged, reviewed, tested — and reachable by nothing. 1.2.0 sat unpinned for three weeks for exactly this reason. The changelog entry leads with the consequence for someone still on an older pin: a delivery can complete successfully and be unservable. Then what changed, then why taking it promptly is worth it — the deterministic-re-run case, where the dedup path that took one object takes all 110 and the documented remedy for a torn run is what triggers it. Recorded there as known-and-not-ours: pipeline-core's get_latest_file_id documents "newest by creation timestamp" over an unsorted result, which the SELECTION half of the guard has relied on since August. This release removes the equivalent assumption from the per-object half. The upstream half is filed there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9
release: 1.4.0 — the findability fix, which is unreachable until it is tagged
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.
Promotes
developmenttomainfor 1.4.0.Fixes the defect behind today's unservable UN-FAO delivery: the findability guard checked 2 of 110 artefacts and could not see the sidecar by construction. It now verifies every uploaded object by the consumer's own resolution shape, order-independently.
A previously-passing delivery can now stop. Four refusal situations instead of two, no new exception types.
CHANGELOG.mdleads with that.Reviewed by the views-models seat (non-hosting, approved), plus
/code-review mediumand/review-diff. Between them they caught three defects in the fix — the lookup key, a polarity bug that would have condemned a healthy delivery, and an ordering assumption that does not exist. All addressed, all mutation-proven.This tag is the delivery mechanism, not housekeeping: the launcher installs from it.
526 passing, ruff clean.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ANY1CCy9Xo7zjMY4XJ69v9