Record import outcome per source, instead of inferring it from job state

[njakobsen]

spatial_processing_status_cache is defined as the state of a queued job, but every consumer reads it as the outcome of the last import. The two agree only when imports are always queued.

How it's defined

#update_spatial_processing_status derives the value entirely from Delayed::Job rows, and nils it when there is no job:

latest_job = spatial_processing_jobs(method_name).last

if !latest_job
  update_cached_status(self, method_name, nil)
elsif latest_job.failed_at?  then :failure
elsif latest_job.locked_at?  then :processing
else                              :queued
end

It is written only by SpatialProcessingJob's lifecycle hooks and by that method. An import run inline — record.update_features!, or the class-level ::update_features! — has no job, so nothing writes a status.

How it's read

#updating_features_failed? is the basis for "this record's import is broken": Connect Engine gates its failure banner on it, and any sweep that retries broken records selects on it. Both ask about the import, not about a job.

What that produces

An inline import that succeeds leaves the record flagged as failed, permanently. Observed on a real record (Homalco referral submission 1691) — set to failure, then imported inline:

inline success, cache: "failure"    # 2 features imported
after recompute:       "failure"

The recompute doesn't rescue it: Delayed::Worker.destroy_failed_jobs = false in the hosts, so the historical failed job stays the newest row and the status recomputes straight back to failure. #feature_update_error reads last_error off that same row, so the reason outlives the failure too.

Not hypothetical — stolo_connect's nightly geometry task imports inline, so anything it repairs keeps its stale failure indefinitely:

Referral.update_features_without_indexing! allow_blank: true, skip_invalid: true
Site.update_features_without_indexing!     allow_blank: true, skip_invalid: true

The UI mostly masks this — the banner also requires !features? — which is why it hasn't surfaced. It bites anything trusting the flag alone, such as the retry sweep added in #58, which over-selects records that already recovered.

Direction: a per-source import ledger

Record the outcome of importing each source, written by the import itself so inline and queued runs land in the same place. A later run by any means updates the entry, so a repaired source stops reading as broken no matter how it was repaired.

Two arguments this is the right shape rather than a patch:

It can't be derived from anything that exists. A source that fails produces zero features rows, so the thing most worth recording is the thing that leaves no trace. That is why the status ended up on the model to begin with.

source_identifier is already the key, present on Feature and on the importers, and already used to attribute partial-import warnings. Today those warnings are a flat array with the source string-prefixed into each message ("archive.zip/layer.kml: Skipped 2 network-linked layers…") — the shape you get from having nowhere per-source to put them. A ledger gives them a structural home.

Where it lives

Neither existing column is universal, and they don't overlap cleanly:

	tables
spatial_processing_status_cache	attachments, map_layers, referral_submissions, referrals, sites
features_hash	communities, community_layers, map_layers, referrals, sea_areas, sites, spatial_caches, tribes

So reusing either still means adding it to the tables that lack it — the per-table opt-in worth escaping. A polymorphic table avoids that. The cost: this gem ships no migrations and no generators today, so either every host writes the DDL or the gem starts providing it.

Absorbing features_hash

More than tidying — it would make imports incremental, which they are not today. #update_features! hashes every source together and features.delete_alls the lot, so one changed file re-imports all of them, and a model without the column never short-circuits at all. Homalco submission 1636 re-imports 674 features across seven archives on every run because referral_submissions has no features_hash. Per-source keys plus Feature's existing source_identifier scope would let a run replace only what changed.

Open questions

Pruning. When a source is removed, its entry has to go, or the ledger accumulates failures for files that no longer exist. The import must reconcile against the current source list, not just upsert.

Whose outcome wins. A lenient run and a strict run are different facts about the same source. The nightly above runs allow_blank: true, skip_invalid: true; if that marks a source green, a strict import that would have refused it never gets retried. Last-writer-wins is simplest, but leniency then wins by being last. Same tension as the rejected option below.

Sources with no identifier. Only Importers::File sets a meaningful source_identifier. Base defaults it to nil, so ArcGISMapLayer's ESRIGeoJSON and Geomark imports would collapse onto a single nil key — a fallback is needed, probably the import: data method plus index.

Queue state. :queued and :processing are genuinely job state and would stay job-derived. Only outcome moves to the ledger.

Considered and rejected

Having an inline run cancel any queued job for the same record. A job stores the arguments it was enqueued with and replays them (@args → @record.send(@method_name, *@args, **options)), so a queued import is not interchangeable with an inline one — the nightly is deliberately lenient, a queued import is strict. Cancelling would let a lenient run that discarded invalid geometry suppress the strict import a user's upload had just queued, with nobody told. That is the failure mode culturecode/stolo_connect#1962 exists to fix.

It would also remove a self-correction: for models carrying features_hash, a queued job running after an equivalent inline import short-circuits on #features_cache_key_matches? and does nearly no work, but its success hook still fires and clears the stale status.

Urgency

None immediate — nothing is visibly broken. Worth deciding before more code starts trusting the flag on its own.