Summary
reingest.py should reingest each group's latest successful execution, not skip groups whose newest run happens to have failed. Now that get_execution_group_and_latest_filtered exposes a pre-rank latest_successful knob, the recovery path can ask the right question.
Background
There are two distinct "latest execution" questions, now both available on get_execution_group_and_latest_filtered:
successful (post-rank): "is the latest run successful?" — keeps a group only if its newest execution succeeded.latest_successful (pre-rank): "what is the latest successful run?" — ranks over successful executions only, so a group whose newest run failed but succeeded earlier is still returned, pointing at that earlier success.
The reingest case
get_reingest_targets (packages/climate-ref/src/climate_ref/executor/reingest.py:400) currently passes successful=None if include_failed else True, i.e. the post-rank filter. For a group whose newest run failed after an earlier success, this drops the group entirely — even though a good bundle exists to reingest from. A recovery/reingest caller almost certainly wants the pre-rank latest_successful=True instead, so it recovers the last good run rather than skipping the group.
Proposed change
Switch the include_failed=False path from successful=True to latest_successful=True:
results = get_execution_group_and_latest_filtered(
database.session,
diagnostic_filters=diagnostic_filters,
provider_filters=provider_filters,
latest_successful=None if include_failed else True,
include_superseded=True,
)Why it wasn't done in the same change
This alters recovery/reingest behavior (which groups get reprocessed and from which execution), so it deserves its own PR with a focused test rather than riding along with the read-only primitive refactor that introduced latest_successful. The downstream logic in get_reingest_targets already re-checks oldest.successful and selects eg.executions[0], so the interaction with the pre-rank population needs a deliberate test (e.g. a group with success -> fail -> and confirm it now reingests the good run).
Acceptance criteria
Summary
reingest.pyshould reingest each group's latest successful execution, not skip groups whose newest run happens to have failed. Now thatget_execution_group_and_latest_filteredexposes a pre-ranklatest_successfulknob, the recovery path can ask the right question.Background
There are two distinct "latest execution" questions, now both available on
get_execution_group_and_latest_filtered:successful(post-rank): "is the latest run successful?" — keeps a group only if its newest execution succeeded.latest_successful(pre-rank): "what is the latest successful run?" — ranks over successful executions only, so a group whose newest run failed but succeeded earlier is still returned, pointing at that earlier success.The reingest case
get_reingest_targets(packages/climate-ref/src/climate_ref/executor/reingest.py:400) currently passessuccessful=None if include_failed else True, i.e. the post-rank filter. For a group whose newest run failed after an earlier success, this drops the group entirely — even though a good bundle exists to reingest from. A recovery/reingest caller almost certainly wants the pre-ranklatest_successful=Trueinstead, so it recovers the last good run rather than skipping the group.Proposed change
Switch the
include_failed=Falsepath fromsuccessful=Truetolatest_successful=True:Why it wasn't done in the same change
This alters recovery/reingest behavior (which groups get reprocessed and from which execution), so it deserves its own PR with a focused test rather than riding along with the read-only primitive refactor that introduced
latest_successful. The downstream logic inget_reingest_targetsalready re-checksoldest.successfuland selectseg.executions[0], so the interaction with the pre-rank population needs a deliberate test (e.g. a group with success -> fail -> and confirm it now reingests the good run).Acceptance criteria
include_failed=Falsereingest targets a group whose latest run failed but which has an earlier successful run.include_failed=Truebehavior is unchanged.