[rollroyces]

Closing in favour of #1397, which implements this the way 0072 specifies: walk the whole response, charge only Created/Updated against the cap, keep the checkpoint where it was while items remain, and test that item 201 arrives on the second run. Thanks for the correction — the take(MAX_NEW_PER_SYNC) shape was the rejected one and I should have read 0072 before proposing it.

[WaylandYang]

Thank you for picking up #1212. Both CI failures are mechanical. cargo fmt wants the ResponseTemplate::new(200).set_body_json(...) chain in source_checkpoint_tests.rs on one line. The test fails with a foreign-key error because it calls start_run for the source before the sources row exists; insert the source first.

The bigger problem is that keeping the old since does not get past the cap. The next run gets the same response, takes the same first 200 items (now all Unchanged), and never reaches item 201. The source then reports failed on every run from then on: the window only grows while the checkpoint stays put, so it never recovers, and anything new that the endpoint sorts after position 200 is blocked too. Each of those runs also raises a SOURCE_SYNC_FAILED error alert, and after 50 of them the keep-50 pruning in finish_run deletes the last ok run, so since falls back to None. That is worse than today, where only part of one window is lost. The ADR draft in #1213 lists this exact approach as rejected for the same reason.

For custom, the fix discussed in #1212 is small and fits this PR: walk the whole response (it is already complete) and count only Created/Updated against MAX_NEW_PER_SYNC. Mark the run truncated only when you stop with items left. Each retry then moves forward, and the run ends ok once everything is in. Please also extend the test to sync twice and assert that the 201st item arrives and the second run is ok; as written, the test would pass with the stall. Two smaller things: sync_custom never sets stats.discovered, so the message reads "(0 > 200)"; and whether a capped batch should raise an error alert at all is still open in #1212.

GitHub and Jira are not covered (GitHub's reader stops at ten pages without saying so), and their design is still being settled in #1212 and #1213. Please scope this PR to custom and change "Closes #1212" to "Refs #1212". Running cargo fmt and this test locally with UTOPIA_DATABASE_URL set before pushing would have caught both CI failures.

[WaylandYang]

A correction to what I wrote on #1212, sorry: this PR doesn't fit "as is". Its sync_custom still takes the first MAX_NEW_PER_SYNC items and marks the run failed, so a retry replays the same 200 and never reaches the 201st, which is the approach 0072 (#1213) rejects. What fits is the change from the first review: walk the whole response, count only Created/Updated against the cap, and keep the checkpoint where it was while items remain, so each run moves forward and the source ends ok. With a test that syncs twice and sees the 201st item arrive, "Part of #1212" instead of "Closes", and the CI failures fixed (cargo fmt, and inserting the source before start_run), this is ready to go. Thank you for sticking with it.

[WaylandYang]

The request for changes stands, and 0072 (#1213) now states the rule this has to implement. The current code still does take(MAX_NEW_PER_SYNC) and marks the run failed, so every retry replays the same 200 items and never reaches item 201; that is the approach 0072 lists under Rejected.

What it needs: walk the whole response, charge only Created / Updated against the cap, keep the checkpoint while items remain and end the run ok; a test that syncs twice and sees item 201; set stats.discovered (the message currently reads (0 > 200)); Refs #1212, not Closes. CI: rustfmt at source_checkpoint_tests.rs:201, and the test fails on source_sync_runs_source_id_fkey because start_run runs before the sources INSERT.

If there is no update within a week I will close this and reassign the fix.

[rollroyces]

Rewritten per your review — the take(MAX_NEW_PER_SYNC) shape was the one 0072 lists under Rejected, and I should have read it before proposing.

#1397 replaces this PR. It walks the whole response and charges only Created/Updated against the budget, so on a retry the unchanged prefix costs nothing and the run reaches item 201; stats.discovered is the response size, and the run is written failed while items remain so last_successful_sync_start holds the checkpoint. The test syncs twice and asserts item 201 arrives. Refs #1212 — the GitHub and Jira halves are not in it.

cargo fmt is clean and the sources INSERT now runs before start_run.