feat(papertrail): add automatic sync scheduling core#640
Conversation
SCIP oracle — resolution reportHeuristic→compiler edge resolution per corpus. Δ compares resolved-after to the
resolved = |
|
Bencher Report| Branch | feat/592-auto-sync-core |
| Testbed | ubuntu-latest |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
- LL Hits (hits)
- L1 Hits (hits)
- Estimated Cycles (cycles)
- Instructions (instructions)
- RAM Hits (hits)
- Total read+write (reads/writes)
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Estimated Cycles | cycles x 1e6 | Instructions | instructions x 1e6 | L1 Hits | hits x 1e6 | LL Hits | hits x 1e6 | RAM Hits | hits x 1e3 | Total read+write | reads/writes x 1e6 |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| rag_pipeline::pipeline::index cargo_resolver:resolver_config() | 📈 view plot | 2,524.14 x 1e6 | 📈 view plot | 1,655.83 x 1e6 | 📈 view plot | 2,258.85 x 1e6 | 📈 view plot | 44.02 x 1e6 | 📈 view plot | 1,291.68 x 1e3 | 📈 view plot | 2,304.16 x 1e6 |
| rag_pipeline::pipeline::query_cold cargo_resolver:resolver_built_config() | 📈 view plot | 350.56 x 1e6 | 📈 view plot | 233.04 x 1e6 | 📈 view plot | 334.77 x 1e6 | 📈 view plot | 2.96 x 1e6 | 📈 view plot | 28.31 x 1e3 | 📈 view plot | 337.76 x 1e6 |
| rag_pipeline::pipeline::query_warm cargo_resolver:resolver_index() | 📈 view plot | 336.39 x 1e6 | 📈 view plot | 223.63 x 1e6 | 📈 view plot | 321.44 x 1e6 | 📈 view plot | 2.81 x 1e6 | 📈 view plot | 25.73 x 1e3 | 📈 view plot | 324.28 x 1e6 |
|
| Branch | feat/592-auto-sync-core |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result seconds (s) (Result Δ%) | Upper Boundary seconds (s) (Limit %) |
|---|---|---|---|
| index_time/full_rebuild_cargo | 📈 view plot 🚷 view threshold | 4.79 s(-17.33%)Baseline: 5.79 s | 7.63 s (62.68%) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1191e1bf31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b92c94b0f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if change_detected { | ||
| return ScheduleDecision::Incremental; | ||
| } | ||
| if elapsed(state.last_successful_probe_ms, config.probe_interval_secs) { |
There was a problem hiding this comment.
When sync_min_interval_secs is configured lower than probe_interval_secs, a successful mirror still leaves last_successful_probe_ms unset because record_success records only last_successful_mirror_ms for mirror operations. This check then treats the probe lane as overdue as soon as the shorter minimum interval expires, even though the mirror just contacted the provider and is at least as fresh as a probe, so papertrail_sync_status and the future scheduler will request probes earlier than the configured probe cadence.
Useful? React with 👍 / 👎.
| if let Some(operation) = completed_mirror_operation(&report, full) { | ||
| record_success(conn, binding, operation, now_ms())?; | ||
| } |
There was a problem hiding this comment.
When mirror_binding returns a paused report with a provider paused_until_ms farther out than sync_min_interval_secs (for example a GitHub secondary limit with Retry-After), this path records only the attempt and discards that resume horizon. Because each sync_mirror call builds a fresh GovernorRegistry, the in-memory retry hold is also gone, so status/the scheduler can consider the binding eligible again after the minimum interval and re-hit the provider before the reported pause expires.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30f01505ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8d6342fd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
skakri
force-pushed
the
feat/592-auto-sync-core
branch
from
d8d6342 to
7151bfa
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7151bfa0f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf96fff7a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
skakri
force-pushed
the
feat/592-auto-sync-core
branch
from
bf96fff to
2cfe1c9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cfe1c9adc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let overdue = decide_schedule(now, &ctx.schedule, health, filter_changed) | ||
| != ScheduleDecision::Skip; |
There was a problem hiding this comment.
For GitLab/Bitbucket/Jira bindings, tracker_synchronization reports ProviderClientPending and sync_mirror can only return the intentional provider_client_pending result, but this path still runs decide_schedule against their health row. With no last_full_walk_ms (or once the min interval has elapsed after a manual attempt), papertrail_sync_status advertises overdue: true for work the product cannot run, so consumers of the new scheduling status will keep prompting or retrying unsupported provider mirrors. Gate overdue on native synchronization, or force Skip for pending providers.
Useful? React with 👍 / 👎.
| error_class, | ||
| error_detail, | ||
| overdue, | ||
| failed: error_class.is_some(), |
There was a problem hiding this comment.
When a mirror hits a rate-governed pause, record_pause persists error_class='rate_limited' together with retry_not_before_ms, and this line derives failed from any error class. In that rate-limited scenario the status reports failed: true even though the transport documents Paused as not a failure and the binding just needs to resume later, which obscures whether operator action is needed; derive failed only for non-RateLimited classes or expose a separate paused state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 918e537937
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "UPDATE papertrail_sync_cursor | ||
| SET last_successful_probe_ms=last_probe_ms | ||
| WHERE last_successful_probe_ms IS NULL AND last_probe_ms IS NOT NULL", |
There was a problem hiding this comment.
When upgrading an index that already completed the normal non---full mirror backfill before this change, the cursor has backfill_done=1 and a recent last_probe_ms, but old save_cursor only populated last_full_sync_ms for forced full runs. This migration seeds probe freshness only, so load_persisted_health still returns last_full_walk_ms=None and decide_schedule immediately marks the binding due for a Full rewalk after upgrade even though the historical backfill is complete. Fresh evidence beyond the earlier initial-backfill comment is that this affects pre-existing migrated cursor rows, not the new completed_full_walk path.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8842a700ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.full_rewalk { | ||
| return MirrorContinuation::Full; | ||
| } | ||
| if !self.backfill_done |
There was a problem hiding this comment.
When record_attempt creates a cursor row and the client fails before mirror_binding saves any cursor state (for example a missing configured token on the first sync), the row has backfill_done=0 but no resumable cursor fields. This branch classifies that bare row as Incremental, so decide_schedule returns before the last_full_walk_ms due check even though no full walk ever completed; the recovery run is scheduled as an incremental resume instead of the required initial full walk, which can skip full-rewalk pruning for an upgraded cache that already has items.
Useful? React with 👍 / 👎.
| let probe_interval_secs = raw.probe_interval_secs.unwrap_or(default.probe_interval_secs); | ||
| let sync_min_interval_secs = | ||
| raw.sync_min_interval_secs.unwrap_or(default.sync_min_interval_secs); | ||
| let full_sync_interval_secs = | ||
| raw.full_sync_interval_secs.unwrap_or(default.full_sync_interval_secs); |
There was a problem hiding this comment.
This now accepts the three cadence keys, but docs/config.md:624-626 still says probe_interval_secs, sync_min_interval_secs, and full_sync_interval_secs remain rejected until the scheduler consumes them. Users following the shipped config docs will think the newly activated settings are invalid, so update that section with the supported defaults and scheduling semantics alongside this parser change.
Useful? React with 👍 / 👎.
| "UPDATE papertrail_sync_cursor | ||
| SET last_full_sync_ms=last_probe_ms | ||
| WHERE backfill_done=1 AND last_full_sync_ms IS NULL AND last_probe_ms IS NOT NULL", |
There was a problem hiding this comment.
For an upgraded cursor that already completed its initial mirror (backfill_done=1 with last_probe_ms populated), this migration stamps last_full_sync_ms but leaves the new last_successful_mirror_ms NULL. papertrail_sync_status will therefore report a completed full walk with no successful mirror until the next sync, losing the existing health state that the new status field is supposed to surface.
Useful? React with 👍 / 👎.
| record_failure( | ||
| conn, | ||
| binding, | ||
| PapertrailErrorClass::Provider, | ||
| Some(&error.to_string()), |
There was a problem hiding this comment.
When mirror_binding returns any error, this path persists it as Provider, so a network outage from TransportError::Http or a local SQLite/write failure is surfaced through papertrail_sync_status as a provider failure even though the enum now has Network and Storage classes. That makes the new health state misleading in exactly those operational scenarios; inspect the error chain here before choosing the persisted class.
Useful? React with 👍 / 👎.
1 participant
Closes the scheduling/persistence half of #592.
What changed
probe_interval_secs,sync_min_interval_secs, andfull_sync_interval_secsSkip,Probe,Incremental, andFulldecisionspapertrail_sync_statusDesign boundary
This is the first of two PR-sized slices. Hook/watcher triggers, the single-in-flight coalescing worker, timer/backstop execution, and concurrent binding dispatch will stack on this PR.
No new network operation runs under the maintenance write lock. This slice only adds policy/persistence and instruments the existing mirror boundary.
Validation
cargo test -p rag-rat-core index::papertrail::schedule::tests --lib --no-fail-fastcargo test -p rag-rat-core config::tests::papertrail_scheduling_intervals_are_parsed --libcargo test -p rag-rat-core index::schema_bootstrap_tests::papertrail_tests::migration_064_is_the_tip_and_persists_binding_health --libcargo clippy -p rag-rat-core --lib -- -D warningscargo +nightly fmt --allgit diff --check