Skip to content

Record why source syncs stop at timestamp boundaries - #1213

Merged
WaylandYang merged 4 commits into
deeplethe:devfrom
Floating-Y:fix/source-window-1212
Oct 21, 2026
Merged

WaylandYang merged 4 commits into
deeplethe:devfrom
Floating-Y:fix/source-window-1212

Conversation

@Floating-Y

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

Copy link
Copy Markdown
Contributor

Why

Source syncs can import only the first 200 items and then advance their checkpoint past the rest. A timestamp watermark also stalls if a group sharing one update time extends beyond the readers' ten-page limit, or if the next run spends its budgets rereading that completed group.

Refs #1212. This documentation PR records the agreed direction and the follow-up boundary clarification; it does not implement or close the issue.

What changes

Record ADR 0072: A source checkpoint ends at a timestamp boundary, and add it to the sources index. 0071 is used by #1319.

  • GitHub and Jira finish the current timestamp group when either budget runs out, then checkpoint that group's time.
  • The next run consumes only items updated strictly after the checkpoint. Returned items at or before it are skipped and count against neither budget; pages containing only skipped items cannot exhaust the listing budget. The record accounts for GitHub's inclusive since and Jira's minute-rounded bound.
  • The 1,001-item GitHub and 501-item Jira examples explain the boundary rule. Acceptance also requires the run after the 1,001-item group at T to import a later group and advance past T.
  • Persisted continuation state, response snapshots and saved offsets are rejected approaches. Custom counts only changed items against its cap; its implementation remains separate in Do not advance the source checkpoint when a batch is truncated (#1212) #1246.

The ADR keeps the recovery policy and implementation acceptance cases, while removing the revision-history paragraph and PR-scope sentence requested in review. Its status remains Proposed. This PR changes only the ADR and index; production code, schema and tests are not included.

Validation

Checked on Windows for documentation commit 075604d, using a clean checkout without the pre-existing local reader probes:

  • git diff --cached --check and cargo fmt --all --check passed.
  • cargo clippy --locked --workspace --all-targets -- -D warnings passed.
  • pnpm install --frozen-lockfile, pnpm build and pnpm test passed; 36 test files and 268 frontend tests passed.
  • Verified all 78 relative links in the ADR and index, removed the outdated boundary wording, and reviewed the full diff against the requested changes.
  • Only the ADR changed in this revision. The original worktree's three uncommitted reader/test probes are unchanged and excluded from the commit.

CI for 075604d passed all six jobs: lint, test, build, backend, migrations and web.

Local Rust workspace/database tests were not rerun because a local test database was unavailable; CI ran them against PostgreSQL with UTOPIA_TEST_REQUIRE_DB=1. No live-provider validation is claimed. The new acceptance case is a requirement for the later implementation, not a claim that the fix already passes it.

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 changed the title Record why source checkpoints need bounded continuation Record why source syncs stop at timestamp boundaries Oct 19, 2026
@Floating-Y
Floating-Y marked this pull request as ready for review October 19, 2026 08:12
@Floating-Y

Copy link
Copy Markdown
Contributor Author

@WaylandYang, the ADR is ready for another review in 19083ff.

It now follows your decision in #1212: finish the current timestamp group when either budget runs out, checkpoint that group's time, and start the next run after it. The reader counterexamples explain the rule; persisted continuation and offsets are now rejected approaches. The custom implementation remains separate in #1246.

I used 0072 because #1319 took 0071 after your comment. The index, PR title and description are updated. This PR still changes only the ADR and index; production code and the local probe tests are not included.

Documentation checks passed, and all six CI jobs passed. Could you review the revised record? I marked the PR ready, but GitHub does not permit this account to request a reviewer directly.

@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.

Thank you, Floating-Y. This reads exactly as we agreed: the boundary rule up front, your 1,001/501 counterexample as the reason, and the continuation designs under rejected approaches. Moving to 0072 is right; #1319 keeps 0071.

One thing needs fixing before it lands: where the next run starts. GitHub's since is inclusive in practice, and Jira's updated >= is rounded to the minute, so both hand back the group we just finished. If the next run reads those again, a 1,001-item group at T uses up the page budget inside T again, the run finishes T, and the checkpoint never moves. Could you replace lines 41–46 with the rule itself? Something like: "The next run consumes only items updated strictly after the checkpoint. Items a provider returns at or before it (GitHub's inclusive since, Jira's minute-rounded bound) are skipped and count against neither budget; for GitHub, since can simply be the checkpoint plus one second." Then fix the "exclusive boundary" wording in Acceptance, and add a case where the run after the 1,001-item group moves past T.

Optional: the revision note and the last Acceptance sentence are about this PR's history rather than the decision, so they could go.

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

Copy link
Copy Markdown
Contributor Author

@WaylandYang, addressed in 075604d.

The ADR now states that the next run consumes only items updated strictly after the checkpoint. Returned items at or before it are skipped and count against neither budget, including pages containing only skipped items. It describes GitHub's inclusive since, the checkpoint-plus-one-second option, and Jira's minute-rounded bound.

Acceptance now requires the run after the 1,001-item group at T to skip the returned prefix, import a later group and advance the checkpoint past T. I also removed the revision-history paragraph and the PR-scope sentence as suggested.

This revision changes only the ADR. Format, workspace Clippy, frontend build and all 268 frontend tests passed locally; all six CI jobs passed. Please take another look.

@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.

All four points from the 2026-10-19 review landed in 075604d: the next run consumes only items strictly after the checkpoint and charges skipped items to neither budget, GitHub's inclusive boundary and Jira's minute rounding are named as such, the acceptance case after the 1,001-item group is there, and the history paragraph is gone. 0072 is free on dev and in every open PR. Approving; this supersedes the change request.

@WaylandYang

Copy link
Copy Markdown
Contributor

Approved, and the only thing holding the merge is the up-to-date rule: dev has moved since this was opened and the branch needs the current dev merged in before the required checks count. Your fork does not allow maintainer edits, so I cannot press Update branch for you; a git merge origin/dev (or rebase) and push is all it needs, and it merges once CI is green.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
@WaylandYang
WaylandYang merged commit ffc1e8d into deeplethe:dev Oct 21, 2026
6 checks passed
gedaiu added a commit to re-cinq/utopia that referenced this pull request Oct 22, 2026
Sync upstream dev into re-dev (2026-10-21, second): deeplethe#1213
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