Skip to content

The C-94 findability guard checks 2 of 110 artefacts — and cannot see the sidecar by construction #312

Closed

Description

@Polichinel

Reported by the views-models seat from the first live UN-FAO delivery, 2026-09-29. The run completed, logged success, sent a WandB success alert, and produced a delivery views-faoapi refuses at ingest. Bucket listing: run-0 has 110 objects, this run 109. The missing object is the §5 GAUL sidecar.

Verified here against the code before filing. The report is right about the hole and understates it.

The guard checks 2 of 110 artefacts — and cannot see the sidecar at all

unfao/managers/unfao.py:393 calls the C-94 preflight with two ids:

{"forecast": summary["manifest_file_id"], "historical": hist_file_id}

That is the commit marker and the historical leg. Not the sidecar, not the 108 shards.

But adding them to that dict would not work either, and that is the part worth reading twice. The guard resolves each entry with:

port.latest_file_id({"name": consumer_name, "category": category})

In contract/wire/sink.py:227 every wire object is uploaded with the same common dict:

common = {"name": consumer_name, "category": "forecast", "loa": "pgm"}

Shards, sidecar and manifest all carry category="forecast". So a query for the newest forecast document returns the manifest — which is uploaded last, by design (sink.py:252). The guard is structurally incapable of observing the sidecar. It is not a missing entry in a dict; the lookup key is the wrong key.

What actually failed

The sidecar's bytes were byte-identical to run-0's, the store deduplicated, update_document ran against the old file, and the uploader logged the new filename as uploaded. _ContractStorePort.upload returned a real file_id — for the wrong document. Nothing in this repository asks whether that id resolves under the name the manifest references.

views-faoapi resolves by filename. It finds nothing. It refuses. Correctly.

The consumer seat's phrasing, which belongs in the register entry:

Manifest-last protects against a run that stops early. It does not protect against an upload that returns success having written nothing.

That is C-79 one level down. C-79 is about an upload that reports failure with the file already written; this is an upload that reports success with the file not written. The guard's own docstring — "every call reported success in run-0 too, and the historical leg still stranded" — describes this exact failure and then checks the two legs that did not exhibit it.

Two things in the report that are NOT findings

Checked, because acting on them would mean changing code that is already correct:

  1. "The sidecar must land before the manifest, always." It already does. sink.py:215 — shards → sidecar → manifest LAST — and sink.py:246-252 implements it. ADR-013 already carries the rule as a dated MINOR clarification (2026-07-19, §4 audit, register C-51): "the manifest uploads only after every shard AND the sidecar." The ordering was obeyed. The manifest is in the bucket without the sidecar not because it went first, but because the sidecar's upload lied about having landed. Encoding an ordering rule again would treat a correct mechanism as the cause.

  2. The curation was right. Manifest declares expected_cell_count: 64742, land_gaul, unmapped_count: 0. Saying so because it was the pre-registered most-likely silent failure and it did not happen.

The shape of the fix — design mine, stated so it can be argued with

Check resolvability by name, for every artefact the manifest references.

  • The manifest is in hand at the call site and already carries sidecar.name + sha256 (run_manifest.py:56) and shards[*].name. Nothing new needs plumbing.
  • filename is already passed to upload_data (store_port.py:66-76), so it is a queryable field — latest_file_id passes filters straight through.
  • An id is not evidence. The id returned here was real. The check must be "does a document resolvable by this filename exist", not "did I get a non-empty id back". A guard written the second way passes this incident.
  • 110 lookups per delivery is the obvious cost objection. It is one query per object against a store the run has just written to, on a path that runs once a month. Worth measuring before optimising, not worth pre-optimising into a sampled check that would miss exactly this.

Open for review: whether the shard set is verified per-object or by count-plus-spot-check. I lean per-object — the failure mode is one object silently absent, which a count catches only if the store also fails to create a document, and here it did create one.

Relationship to the other half

views-pipeline-core is separately fixing the upload-that-reports-success. Complementary, not alternatives, and neither should wait on the other: theirs stops the lie at the source, this one stops us believing a lie from any source. A store we do not own can regress; the guard is ours.

Register: extends C-94. Related: C-79, C-99 (fail-closed on an unrecognised result), C-105 (the torn-run ledger, which also did not fire — nothing failed).

Activity

  1. Polichinel commented on Sep 29, 2026

    @Polichinel
    CollaboratorAuthor

    New evidence from the views-models seat: today's incident is the general case arriving early, and the worst case is 110 of 110

    The run-0/run-1 sidecar collision happened because the sidecar is the one artefact whose content repeats across different runs — same GAUL edition, same cell set, same bytes, while shards carry different forecast values.

    That stops being true the moment a run is repeated. The views-models seat proved pooling is deterministic today, by accident: a publish failed, re-pooled under a new run id, and 25 of 25 pre-registered anchor cells came back byte-identical on full sha256 of the draw vectors.

    What that means here, verified in this repository's code

    Every artefact's filename embeds the run id — contract/wire/naming.py:19-31:

    {run_id}__{target}__m{time_id:06d}.arrow.parquet
    {run_id}__sidecar.parquet
    {run_id}__manifest.json
    

    So a deterministic re-run produces new filenames over identical bytes. That is precisely the shape that failed today:

    1. store deduplicates on content, declines to write a second copy — correctly
    2. update_document runs against the previous run's file
    3. _ContractStorePort.upload returns a real file_id, for the wrong document
    4. we log the new filename as uploaded
    5. views-faoapi resolves by filename, finds nothing, refuses

    Today that path was walked once, by the sidecar. On a re-publish it is walked by all 110 objects — every shard, the sidecar, and the manifest — and the run reports complete success with nothing servable in the bucket.

    A re-publish is not hypothetical. It is the documented response to a torn run (C-105) and to the correction procedure (C-22), both of which say: re-run, manifest last. Our own remedy for a failed delivery is the thing that triggers the worst case of this defect.

    Consequence for the fix

    It settles the open question in the issue body. I asked whether the shard leg should be verified per-object (108 lookups) or by count-plus-spot-check, and leaned per-object because a count only catches this if the store also failed to create a document.

    Per-object, and the determinism argument is the stronger one: a count-plus-spot-check passes a republish in which nothing at all was written, because the store creates 110 documents and the count is right. Only resolution by name survives it. 108 lookups against a publish already taking ~11 seconds per shard is not a cost worth optimising against a check that would miss the total-failure case.

    What it does not change

    Still a guard defect, not an upload defect — views-pipeline-core owns the upload-reports-success half separately, and the two remain complementary. And #313 is unaffected: a per-edition sidecar can be missing too, and this evidence argues for landing the guard sooner rather than for bundling them.

    Credit where due: the determinism measurement is the views-models seat's, from today's run. I verified the filename mechanism here; I could not have seen the pooling result.

  2. Polichinel commented on Sep 29, 2026

    @Polichinel
    CollaboratorAuthor

    /code-review findings 5 and 6 — recorded, not changed

    Two lows from the PR #314 review that are better tracked than patched.

    5 — the read-back walk gets noisier every month. Each delivery now enumerates the metadata collection once per artefact type, and pipeline-core's search_files_by_metadata refuses the walk with SEARCH_INCOMPLETE when len(documents) != reported_total — which a concurrent write landing mid-walk triggers. That surfaces as FindabilityUnverifiedError, so the delivery reports UNVERIFIED rather than failing, which is the right polarity. But the collection grows by ~110 shard documents per run, so the walk lengthens monotonically and the window widens. No code change proposed: the failure direction is correct and any retry would be guessing at a store semantic we do not own. Belongs as a note in C-94 at the next /register-risk, with the trigger being the first time a delivery reports UNVERIFIED with no other explanation.

    6 — the call site's inline dict is budget-driven formatting. The historical artefact's record is appended inline across two lines with a one-space continuation, which reads as a stray statement — at the exact call site whose wrongness caused this issue. Hoisting it to a named local would cost 2–3 lines and neither partner package has them: unfao sits at 690/700 and crafd at 699/700. Recorded rather than done, because the alternative is raising a budget whose own comment forbids it or moving store_port.py out, which six register entries, an anti-drift test and ADR-015 reference.

    Both are consequences of the fix rather than defects in it, and both are cheap to revisit when the partner-seam extraction (C-33) eventually happens.

  3. Polichinel commented on Sep 29, 2026

    @Polichinel
    CollaboratorAuthor

    The ordering guarantee does not exist — three places assume it, and one of them documents it

    Found by the views-models seat on re-review, verified here. I had asserted this sort as fact twice after an automated review claimed it, without checking. Recording the mechanism because two of the three places are not ours to fix.

    What is actually there

    search_files_by_metadata builds queries = [] and appends only Query.equal(attribute, value) per filter, then Query.limit and Query.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, which is a storage listing rather than a collection query, and whose order_field defaults to None.

    So the document order returned by get_predictions_by_metadata is unspecified.

    Three consumers of an order nothing establishes

    where what it assumes ours?
    pipeline-core get_latest_file_id docstring: "the file ID of the newest matching file based on creation timestamp"; implementation is files_list[0] no
    views-faoapi resolve_artifact_file_ids docstring: "Newest-first; the first match per filename wins" no
    this repo, the #312 per-object check was "first wins" fixed

    The first is the one that matters beyond this issue. It is a documented guarantee with no mechanism, and the C-94 selection check has relied on it since August: our leg check asks get_latest_file_id for the newest document and compares it against this run's id. If the order is arbitrary, that check can return another run's document and raise a false "invisible delivery" — the failure direction this guard exists to avoid. Nothing has gone wrong yet, which means order has been favourable in practice, not that it is guaranteed.

    What changed here

    The assumption is gone rather than documented. seen maps each filename to the set of ids carried under it, and an object is findable when the id this run uploaded is among them. Order-independent by construction, and it is the question we actually have. The refusal now lists every id the store holds under a name, which is more useful to someone auditing a bucket than a single arbitrary pick.

    Two tests pin it, including the same delivery passing under three document orders with a stale same-filename document returned first. Mutation-proven.

    Not proposed here

    Fixing get_latest_file_id is pipeline-core's, and whether it should sort or its docstring should stop promising is their call. Flagging it rather than filing there, because the faoapi half is a third repo and someone should decide whether this is one cross-repo item or three local ones — that sequencing is the maintainer's.

  4. added a commit that references this issue on Sep 29, 2026
  5. Polichinel commented on Sep 29, 2026

    @Polichinel
    CollaboratorAuthor

    Merged — PR #314, development, 2026-09-29

    The guard now verifies every uploaded artefact, not two legs by category, and the 2026-09-29 incident is in the suite as a named regression test.

    What landed

    selection does the consumer's own query land on this run? (unchanged)
    per-object does each artefact resolve under its own filename at all? (new)

    Resolved the way views-faoapi resolves — type-scoped query on {name, category, type}, filename matched in Python — because filename carries no index and a direct query would have reported UNVERIFIED forever. One query per artefact type rather than one per object.

    Three things the reviews caught that the first draft got wrong

    • the lookup key (views-models seat): querying filename directly would have been the first code on the platform to do so, against an unindexed attribute. The obvious translation of "check by name" was the broken one.
    • a polarity bug (/code-review): the parse sat outside the try, so an unreadable store answer would have reported every object missing and quarantined a healthy delivery — inside the module that argues against exactly that twice.
    • an ordering assumption that does not exist (views-models seat again): I had asserted a $createdAt sort twice, relaying an automated claim without checking it. There is no sort. Removed the assumption rather than documenting it — seen now holds the set of ids under each filename, and findability means this run's id is among them. Order-independent by construction.

    Verification

    524 passing, ruff clean, CI green. Both partner packages came in under where they started — unfao 690, crafd 699 — because the query loop moved out of them into delivery/findability.py, which is what the budget's own comment prescribes.

    Mutations proven 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 sink's ledger · move the parse back outside the try · restore first-wins.

    Left open, deliberately, and not in this repo

    pipeline-core's get_latest_file_id documents "the newest matching file based on creation timestamp" and implements files_list[0] over an unsorted result. The C-94 selection check has relied on that since August, so it can in principle raise a false invisible-delivery. views-faoapi's resolve_artifact_file_ids carries the same assumption. Recorded in the comment above with file and line evidence; whether that is one cross-repo item or three local ones is the maintainer's sequencing call.

    #313 — the sidecar ontology question — is untouched by this and still needs a decision on whether to open a þing.

    Findings 5 and 6 (the widening SEARCH_INCOMPLETE window; the budget-driven inline dict) are recorded above for a C-94 note rather than changed.

  6. Polichinel commented on Sep 29, 2026

    @Polichinel
    CollaboratorAuthor

    Released as 1.4.0 — and a note on why "is it released?" had two different correct answers tonight

    Tag 1.4.0 cut from main, reachable via git ls-remote --tags, and on PyPI. views-models#439 updated to ask for it with the one-line diff.

    The reusable finding, from the views-models seat

    The pod installs views-postprocessing from a git tag (tools/launcher/postprocessor.sh:132, exact-pin, with a ref check at :164 that refuses a mismatch) and views-pipeline-core from PyPI — transitively, through views-hydranet's dependency range.

    Two different mechanisms for two halves of the same fix. So tonight, "is the fix released?" was:

    • pipeline-core's half — yes, 3.3.4 on PyPI, verified by downloading the wheel
    • this repo's half — merged, on PyPI, and still unreachable, because the pod pinned 1.1.1

    Both answers were correct and they had to be established differently. Nothing anywhere states that this is the arrangement.

    This is the same family as #310 (CI installs from poetry.lock; production resolves live with pip) and it is now the second instance in ten days. Not a defect — a pinned git ref is more reproducible than a PyPI range, which is the point of it. But the asymmetry is undocumented, and it is why a merged fix for a live production defect sat reachable by nothing while three sessions believed it had shipped.

    Worth a line in C-112 at the next /register-risk — that entry is already about not being able to see what production runs, and this is the mechanism half of it: we cannot see it partly because there is more than one way for code to arrive, and no document says which applies to whom.

    Credit: the git-vs-PyPI observation is the views-models seat's, from having to verify both halves by hand tonight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions