Skip to content

Prevent GitHub and Jira sync checkpoints from skipping unread tickets - #1359

Merged
WaylandYang merged 5 commits into
deeplethe:devfrom
Floating-Y:fix/source-timestamp-checkpoints-1212
Oct 26, 2026
Merged

WaylandYang merged 5 commits into
deeplethe:devfrom
Floating-Y:fix/source-timestamp-checkpoints-1212

Conversation

@Floating-Y

@Floating-Y Floating-Y commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Why

Refs #1212 and implements the GitHub/Jira part of ADR 0072 (#1213). A successful sync previously advanced from its start time after reading only 200 items or ten pages, permanently skipping unread tickets.

What changes

  • Read tickets in ascending update order and finish the current timestamp group after reaching either budget. Already-consumed timestamps use neither budget. Jira requests ORDER BY updated ASC, key ASC so equal-time results have stable page boundaries.
  • Persist progress in sources.sync_checkpoint and commit it atomically with the successful sync record. Migration 0119 starts GitHub/Jira recovery from NULL while preserving documents, versions, jobs and audit history. The CLI schema count is 107.
  • Verify the selected listing prefix before ingestion. Detected pagination movement, detail failures, write failures or finalization failures preserve the previous checkpoint. Retries reuse existing ingestion deduplication.
  • Read GitHub comments and events completely, including beyond ten pages. A failed detail page preserves the previous document instead of replacing it with truncated history.
  • Keep at most three retries per rate-limited request. Honor waits up to 30 seconds; longer hints end the run immediately rather than holding a sync slot for hours. Jira's search total remains required.
  • ADR 0072 records the listing-verification and waiting tradeoffs. Its status is one line with Custom progression/recovery open in Charge only changed items against the custom sync budget (#1212, ADR 0072) #1397; its index's Overtaken cell is empty. The design, migration comment and this description all point to Charge only changed items against the custom sync budget (#1212, ADR 0072) #1397.

The readers, checkpoint transaction, recovery migration and regression tests stay together because the guarantee that no unread tickets are skipped must be checked through the real ingestion and storage paths. Most added lines are regression coverage. Custom batch progression remains in #1397; this PR does not close #1212.

How it was checked

On Windows, current head 3cabf315526e56a8339d4e72781548b291b4a327 passed:

  • 30 GitHub/Jira reader tests, 8 timestamp-checkpoint database integration tests, and 1 GitHub sync regression: 39 passed, 0 failed, 0 ignored.
  • Both first application and repeat application of all 107 migrations on an isolated PostgreSQL 16 / pgvector database. Database regressions used UTOPIA_TEST_REQUIRE_DB=1.
  • cargo fmt --all --check, cargo clippy --locked --offline --workspace --all-targets -- -D warnings, and git diff --check.

The existing JQL test now checks the complete query both with and without a checkpoint, including key ASC; the existing pagination test also checks the emitted query. No test files or ignored tests were added for this review fix. These HTTP fixtures validate the emitted query and reader behavior, not a live Jira server's sorting engine.

Current head 3cabf315526e56a8339d4e72781548b291b4a327 passed all six CI checks: lint, test, build, backend, web and migrations. The CI log confirms the exact JQL assertions, equal-time Jira pagination and all eight checkpoint regressions ran successfully. The earlier local full-workspace run encountered a Windows Poppler/CJK-resource failure in pdf_gbk; this revision reran the related tests locally and uses the full Linux CI suite for workspace validation. No tests or expectations were disabled.

Integration and recovery

Per the reviewed merge order, this PR merges first with migration 0119/schema count 107. #1409 will move to 0121/108, and #1397 will rebase onto the updated sync_source flow.

Existing GitHub/Jira sources start with no checkpoint and rescan from the beginning. At the nominal 200-item budget, a 20,000-issue repository needs about 100 runs to catch up; a complete equal-time group may exceed that per-run budget. Existing documents, versions and jobs are retained.

Tradeoffs

The second pass adds listing requests and cannot provide an atomic snapshot; a continuously changing prefix may repeatedly fail. The Jira key tie-breaker stabilizes equal-time ordering but does not prevent updates from moving items between pages. Complete discussions increase request count, memory use and run duration. Ordinary job retries do not persist a long Retry-After deadline, so later attempts are not guaranteed to honor it.

Before review

  • Every PR commit is signed off.
  • CI passes on the final pushed commit.
  • Relevant database regressions ran against PostgreSQL/pgvector with skipping disabled.
  • Migration 0119 has no collision on the current base; the CLI schema version matches all 107 migrations.
  • ADR 0072 status and references follow the decision-record rules.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

Notes while it is a draft. The rule matches 0072 (inclusive since plus skip, Jira minute floor, an empty window keeps the checkpoint), the checkpoint and run commit together, and the tests drive the real readers. Three things to look at before marking it ready:

  1. fetch_all fails the run when one ticket has more than ten pages of events or comments. That makes every run fail from then on, so the checkpoint never passes that group and no other ticket syncs again. Skip that ticket (keep its old document, warn) rather than failing the run.
  2. The second listing pass that re-reads the prefix and fails the run if positions moved is not in 0072. It doubles listing requests every run, and on a busy repository any update inside the window between the two passes fails the run, which surfaces as last_sync_status = failed and alerts. Either write it into 0072 as a dated revision with the reason, or drop it.
  3. A Retry-After of up to 24 hours sleeps inside the worker while the source stays running. Cap it much lower and let the next run pick up.

Smaller: total on the Jira response is no longer #[serde(default)], fine for api/2 but say so; the two rate_limit_headers… tests are near-duplicates. #1367 (docs only today) will later touch the same files; no conflict yet.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
@Floating-Y
Floating-Y marked this pull request as ready for review October 25, 2026 11:00
@Floating-Y
Floating-Y marked this pull request as draft October 25, 2026 11:10
…on setup

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
@Floating-Y
Floating-Y marked this pull request as ready for review October 25, 2026 11:47

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This matches 0072, and all three points from my 10-22 comment are handled: discussions are read in full, the second listing pass is in a dated 0072 revision, and rate-limit waits are capped. The checkpoint and the run's success commit in one transaction, and the tests go through the real readers and the database. A few small things before merge:

  1. Make 0072's status line one line, per the README rule, for example In progress 2026-10-25 (#1359, migration 0119); open: custom batch progression and recovery (#1397). Move the second paragraph into the body, and clear the "Overtaken" cell in docs/decisions/README.md (that column is for later records).
  2. #1246 is closed and #1397 replaces it. Please update the references in 0072, docs/design/sources.md, the migration comment and the PR body.
  3. Jira: please use ORDER BY updated ASC, key ASC. Without a tie-breaker, offset paging over an equal-time group can repeat an issue, which fails the run, and the next run reads the same group again.

Merge order: this PR first. #1409 also uses 0119 and will move to 0121/108, and #1397 rebases onto the new sync_source branch. One note for the release: existing GitHub/Jira sources start with no checkpoint and rescan from the beginning, so a 20k-issue repository takes about 100 runs to catch up.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Floating-Y, the status line is one line, #1397 is referenced everywhere, and Jira pages are stable with key ASC. Approving and merging this first.

@WaylandYang
WaylandYang merged commit 6431a5a into deeplethe:dev Oct 26, 2026
6 checks passed
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.

2 participants