Skip to content

fix: stop the refresh loop faking convergence, and survive a failed publish - #32

Merged
gabrielspadon merged 1 commit into
mainfrom
fix/refresh-survives-transient-publish-failure
Aug 17, 2026
Merged

fix: stop the refresh loop faking convergence, and survive a failed publish#32
gabrielspadon merged 1 commit into
mainfrom
fix/refresh-survives-transient-publish-failure

Conversation

@gabrielspadon

Copy link
Copy Markdown
Collaborator

Found while diagnosing why yesterday's refresh failed. The 403 was the visible problem; the convergence bug underneath it was worse, and it was mine.

The refresh has never actually converged

Removing the response cache deleted if ! save_cache "$i"; then but left its body and its fi — and that fi was the one closing the pipeline-failure branch above:

if [ "$rc" -ne 0 ]; then
  echo "::error::Pipeline run $i failed..."
  ...                                  # ← everything below is now INSIDE this branch
  digest=$(find output ... | sha256sum ...)
  echo "::error::Cache save failed after successful pipeline run $i"
  exit 1
fi                                     # ← the orphaned fi, ~25 lines late

if/fi counts still balanced 4-to-4, so nothing caught it. But on a successful run that whole block is skipped, so digest and prev_digest are both unset and compare equal:

if [ "$digest" = "$prev_digest" ]; then   # "" = ""  → true, on iteration 1
  echo "Corpus unchanged across two consecutive runs — converged"

Run 32033375617 shows it exactly: Pipeline run 1/10, runs_completed=1, "converged" — and no Run N — corpus digest … line at all, because that echo was in the dead branch too. One pass, declared as two. The 403 accidentally prevented publishing an unverified corpus.

Fixed by closing the block where it belongs, rejecting an empty digest, and running the step under set -uo pipefail — which is what would have caught this originally.

Verified by simulating the loop with a stub pipeline: a corpus that changes for three iterations now converges at run 4, not run 1.

Publication no longer discards an hour of quota

The 403 destroyed a completed corpus because three things lined up: the push had no retry, output/ lives only on the runner, and the verify job neither re-triggered nor preserved — the re-trigger path required result == 'success', and the failure path just exit 1.

  • pushes and PR calls retry 6× over ~8 min (diagnostics to stderr, since one call sits inside $(…))
  • a failed push uploads output/ as an artifact
  • a converged-but-unpublished run re-triggers; only a run that never converged is a failure

Four contracts, each mutation-tested

Mutation Result
re-break the fi pairing CAUGHT
drop the empty-digest guard CAUGHT
remove retry from the push CAUGHT
remove the publish-failure re-trigger CAUGHT

bash -n over every run block is the one that matters: it sees mispaired blocks that a count cannot.

Verification

ruff clean, mypy clean (55 files), pyrefly 0 errors, pytest 2021 passed, 2 skipped. Every workflow block shellchecks clean at -S warning.

Note: GitHub is still mid-incident (degraded API/Actions), so CI here may be flaky for reasons unrelated to the diff.

…ublish

Removing the response cache deleted `if ! save_cache "$i"; then` but left its
body and its `fi`, and that `fi` was the one closing the pipeline-failure
branch above it. The if/fi counts still balanced, so nothing caught it, but the
digest computation moved inside the failure branch.

On a successful run the branch is skipped, so `digest` and `prev_digest` were
both unset and compared equal. Every run declared "Corpus unchanged across two
consecutive runs" after a single pass. The run on 2026-08-17 shows it exactly:
`Pipeline run 1/10`, `runs_completed=1`, converged, with no digest line ever
printed because that echo was in the dead branch too.

The block is closed where it belongs, the convergence test rejects an empty
digest, and the step runs under `set -uo pipefail`, which is what would have
caught this in the first place.

Publication is now retried and no longer discards the corpus. A platform
incident returned 403 on the push after an hour of provider calls and the run
was thrown away: the push had no retry, `output/` lived only on the runner, and
the verify job neither re-triggered (that path required success) nor preserved
anything (it just exited 1). Pushes and pull-request calls retry six times over
about eight minutes with diagnostics on stderr, a failed push uploads output/ as
an artifact, and a converged run that could not publish re-triggers instead of
ending. Only a run that never converged is still a failure.

Four contract tests cover it, each verified by mutation: every run block must
parse under `bash -n`, which is the check that sees a mispaired block where a
count cannot; convergence must reject an empty digest; the push must be retried
and the corpus preserved; and a converged-but-unpublished run must retry.
@gabrielspadon
gabrielspadon enabled auto-merge (squash) August 17, 2026 14:25
@gabrielspadon
gabrielspadon merged commit 8df90b1 into main Aug 17, 2026
11 checks passed
@gabrielspadon
gabrielspadon deleted the fix/refresh-survives-transient-publish-failure branch August 17, 2026 14:29
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

gabrielspadon added a commit that referenced this pull request Aug 17, 2026
Corrected refresh run 32039247944 ran ten passes and never converged:
digests alternated between two states rather than drifting, and every
pass logged the same `Post-run fixup: corrected 2 .bib files`.

A provider returned titles containing a literal newline. The serializer
wrote that verbatim while the strict parser collapses whitespace on
read, so those entries could never equal their own re-serialization; the
corpus digest moved every pass and the loop burned its ten-run ceiling.
Three files affected (two sources plus one a2i2 copy).

Two changes. The serializer collapses whitespace inside every field
value, so a multi-line value cannot reach a file. And the post-run fixup
compares the serialized form against the file rather than asking whether
a canonicalization rule reported a change: a file can differ from its
own serialization with no rule firing, and the old gate skipped the
write while the difference remained, which is why the damaged files
never healed.

Verified on the corpus rescued from the failed run (3742 files,
preserved by the artifact step added in #32): pass1 fixed=2, pass2
fixed=0, pass3 fixed=0, zero files churning. Both changes
mutation-tested; reverting either fails a test. The postrun one needed a
new test, since the existing suite missed that mutation.

ruff clean, mypy clean (55 files), pyrefly 0 errors, pytest 2026 passed,
2 skipped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant