fix(scheduler): status guards + RETURNING + single backoff source (Sourcery on #69)#70
Merged
Merged
Conversation
…urcery on PR #69) Address 3 Sourcery comments on the v0.4.0 scheduler foundation (PR #69): 1. `mark_publish_attempt_failed` now requires `WHERE status = 'publishing'` so a misfire on a draft/failed/published row cannot corrupt the retry counter. Folded the post-UPDATE state read into the same statement via `UPDATE ... RETURNING` (1 round-trip instead of 2). Adds new `PublishFailureOutcome::Skipped` variant for the no-op case so the scheduler can log it without notifying. 2. `reset_retry_state` now requires `WHERE status IN ('draft', 'failed')` so an accidental call on a published row can't silently un-publish it, and a call while the scheduler holds the row in 'publishing' can't race the lock. Signature changes from `Result<(), String>` to `Result<bool, String>` so the caller can surface the no-op to the user. 3. Backoff window check moved out of the SQL CASE into Rust against BACKOFF_MINUTES — eliminates the silent-drift risk between the const table and the duplicated `WHEN 0 THEN 0 WHEN 1 THEN 5 ...` SQL literal. New `backoff_window_elapsed` helper keeps the filter logic isolated and unit-test-friendly. Three new regression tests lock the new contracts: - mark_failed_skips_when_status_is_not_publishing - reset_retry_state_rejects_published_row - reset_retry_state_rejects_publishing_row Existing tests still green; total: 13 scheduler tests, 240 lib tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Guide du réviseurRefactorise la gestion des échecs/reculs (backoff) du planificateur afin que le SQL soit protégé et minimal tandis que Rust possède la politique de backoff, ajoute des garde-fous sur les statuts et un signalement no-op pour les chemins d’échec/réinitialisation, et étend les tests pour verrouiller les nouveaux contrats. Diagramme de séquence pour la gestion d’une tentative de publication échouée avec garde sur le statutsequenceDiagram
actor Scheduler
participant SchedulerModule as scheduler_rs
participant Db as SqlitePool
Scheduler->>SchedulerModule: mark_publish_attempt_failed(pool, post_id)
SchedulerModule->>Db: query_as(UPDATE post_history ... WHERE id = post_id AND status = 'publishing' RETURNING failed_attempts, status)
Db-->>SchedulerModule: Option(failed_attempts, status)
alt [no row updated]
SchedulerModule-->>Scheduler: PublishFailureOutcome::Skipped
else [status == failed]
SchedulerModule-->>Scheduler: PublishFailureOutcome::GaveUp { failed_attempts }
else [status != failed]
SchedulerModule-->>Scheduler: PublishFailureOutcome::WillRetry { failed_attempts }
end
Modifications au niveau des fichiers
Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre tableau de bord pour :
Obtenir de l’aide
Original review guide in EnglishReviewer's GuideRefactors the scheduler’s failure/backoff handling so SQL is guarded and minimal while Rust owns the backoff policy, adds status guards and no-op signaling for failure/reset paths, and extends tests to lock in the new contracts. Sequence diagram for handling a failed publish attempt with status guardsequenceDiagram
actor Scheduler
participant SchedulerModule as scheduler_rs
participant Db as SqlitePool
Scheduler->>SchedulerModule: mark_publish_attempt_failed(pool, post_id)
SchedulerModule->>Db: query_as(UPDATE post_history ... WHERE id = post_id AND status = 'publishing' RETURNING failed_attempts, status)
Db-->>SchedulerModule: Option(failed_attempts, status)
alt [no row updated]
SchedulerModule-->>Scheduler: PublishFailureOutcome::Skipped
else [status == failed]
SchedulerModule-->>Scheduler: PublishFailureOutcome::GaveUp { failed_attempts }
else [status != failed]
SchedulerModule-->>Scheduler: PublishFailureOutcome::WillRetry { failed_attempts }
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - j'ai trouvé 1 problème et laissé quelques retours de haut niveau :
- Il y a plusieurs chaînes de statut codées en dur (par ex. 'draft', 'failed', 'publishing', 'published') utilisées dans les comparaisons SQL et Rust ; envisagez de les centraliser sous forme de constantes ou d’un enum avec une table de correspondance pour réduire le risque de dérive et rendre les changements de statut futurs plus sûrs.
- Dans
backoff_window_elapsed, chaque appel réanalyselast_attempt_atdepuis un RFC 3339 ; si cette fonction devient une section de code critique, vous pourriez envisager de stocker une datetime déjà parsée ou une représentation numérique dans la base de données pour éviter les coûts de parsing répétés.
Prompt pour agents IA
Veuillez traiter les commentaires de cette revue de code :
## Commentaires généraux
- Il y a plusieurs chaînes de statut codées en dur (par ex. 'draft', 'failed', 'publishing', 'published') utilisées dans les comparaisons SQL et Rust ; envisagez de les centraliser sous forme de constantes ou d’un enum avec une table de correspondance pour réduire le risque de dérive et rendre les changements de statut futurs plus sûrs.
- Dans `backoff_window_elapsed`, chaque appel réanalyse `last_attempt_at` depuis un RFC 3339 ; si cette fonction devient une section de code critique, vous pourriez envisager de stocker une datetime déjà parsée ou une représentation numérique dans la base de données pour éviter les coûts de parsing répétés.
## Commentaires individuels
### Commentaire 1
<location path="src-tauri/src/db/scheduler.rs" line_range="58" />
<code_context>
-/// We avoid SQLite's `datetime('now')` here because it returns the
-/// format `'YYYY-MM-DD HH:MM:SS'` (space separator, no fractional, no
-/// Z), which sorts lexicographically before our RFC 3339 writes
+/// `now_rfc3339` is bound from Rust as the reference instant for
+/// `scheduled_at` comparisons (string-equal to what
+/// `chrono::Utc::now().to_rfc3339()` writes during scheduling). We avoid
</code_context>
<issue_to_address>
**issue (complexity):** Envisagez de simplifier la nouvelle logique de backoff et de retry en parsant les timestamps plus tôt, en renforçant les invariants, et en simplifiant le flux de données et les tests, tout en gardant Rust comme unique source de vérité.
Vous pouvez conserver l’objectif « Rust comme unique source de vérité » tout en réduisant sensiblement la complexité de la nouvelle logique de backoff et du flux de données.
### 1. Parser `last_attempt_at` une seule fois dans `row_to_post_record`
Au lieu de parser le RFC3339 à chaque appel de `backoff_window_elapsed`, parsez-le au moment de la construction et stockez une valeur typée (ou `Option`) dans `PostRecord`. Cela supprime le bruit lié au parsing et à la politique d’erreur dans le prédicat de backoff.
```rust
// Example: adjust PostRecord to carry a typed timestamp
pub struct PostRecord {
// ...
pub last_attempt_at: Option<DateTime<Utc>>,
pub failed_attempts: i64,
// ...
}
// In row_to_post_record:
fn row_to_post_record(row: &SqliteRow) -> Result<PostRecord, String> {
let last_attempt_str: Option<String> = row.try_get("last_attempt_at")?;
let last_attempt_at = match last_attempt_str {
None => None,
Some(s) => match DateTime::parse_from_rfc3339(&s) {
Ok(dt) => Some(dt.with_timezone(&Utc)),
Err(_) => None, // “fail open” here; treat malformed as never-attempted
},
};
Ok(PostRecord {
// ...
last_attempt_at,
failed_attempts: row.try_get("failed_attempts")?,
// ...
})
}
```
Avec cela, `backoff_window_elapsed` peut se concentrer uniquement sur la logique de fenêtre temporelle :
```rust
fn backoff_window_elapsed(post: &PostRecord, now: DateTime<Utc>) -> bool {
let Some(last) = post.last_attempt_at else {
return true;
};
let idx = post
.failed_attempts
.clamp(0, MAX_FAILED_ATTEMPTS) as usize;
let window_min = BACKOFF_MINUTES[idx];
last + chrono::Duration::minutes(window_min) <= now
}
```
### 2. Renforcer les invariants de backoff et l’indexation
Étant donné l’invariant selon lequel la table couvre toutes les tentatives, vous pouvez simplifier l’indexation défensive en remplaçant la gymnastique `.get().unwrap_or_else(..)` par un simple `clamp` :
```rust
#[test]
fn backoff_table_covers_all_attempts() {
assert_eq!(BACKOFF_MINUTES, [0, 5, 30, 120]);
assert_eq!(MAX_FAILED_ATTEMPTS, 3);
assert_eq!(BACKOFF_MINUTES.len(), (MAX_FAILED_ATTEMPTS + 1) as usize);
}
```
Ensuite, le `clamp` ci-dessus suffit ; vous n’avez plus besoin des fallbacks `.get()` plus `.last()`.
### 3. Éviter le `Vec<PostRecord>` intermédiaire dans `list_due_for_publish`
Vous pouvez conserver le backoff côté Rust tout en évitant la passe supplémentaire collect/filter en filtrant lors du mapping :
```rust
pub async fn list_due_for_publish(pool: &SqlitePool) -> Result<Vec<PostRecord>, String> {
let now = Utc::now();
let now_rfc3339 = now.to_rfc3339();
let rows: Vec<SqliteRow> = sqlx::query(/* same SELECT */)
.bind(&now_rfc3339)
.bind(MAX_FAILED_ATTEMPTS)
.fetch_all(pool)
.await
.map_err(|e| e.to_string())?;
let mut due = Vec::with_capacity(rows.len());
for row in rows.iter() {
let post = row_to_post_record(row)?;
if backoff_window_elapsed(&post, now) {
due.push(post);
}
}
Ok(due)
}
```
Cela maintient toute la logique en Rust (pas de `CASE` SQL) tout en réduisant le pipeline de transformation à une boucle simple.
### 4. Petit helper de test pour réduire la répétition
Les nouveaux tests offrent une bonne couverture d’état mais dupliquent un peu de SQL « lecture status/failed_attempts ». Un petit helper leur permet de se concentrer sur les transitions :
```rust
async fn fetch_attempts_and_status(
pool: &SqlitePool,
id: i64,
) -> (i64, String) {
sqlx::query_as("SELECT failed_attempts, status FROM post_history WHERE id = ?")
.bind(id)
.fetch_one(pool)
.await
.expect("fetch")
}
```
Ensuite, des tests comme `mark_failed_skips_when_status_is_not_publishing` deviennent un peu plus faciles à lire :
```rust
let (failed_attempts, status) = fetch_attempts_and_status(&pool, id).await;
assert_eq!(failed_attempts, 0);
assert_eq!(status, "draft");
```
Tout ce qui précède conserve les nouveaux comportements (backoff piloté par Rust, garde-fous, résultats) tout en rendant le flux de contrôle et la politique de backoff plus faciles à comprendre.
</issue_to_address>Sourcery est gratuit pour l’open source - si vous appréciez nos revues, merci d’en parler ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- There are multiple hard-coded status string literals (e.g. 'draft', 'failed', 'publishing', 'published') used across SQL and Rust comparisons; consider centralizing these as constants or an enum + mapping to reduce drift risk and make future status changes safer.
- In
backoff_window_elapsed, every call reparseslast_attempt_atfrom RFC 3339; if this becomes a hot path, you might consider storing a parsed datetime or a numeric representation in the DB to avoid repeated parsing costs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are multiple hard-coded status string literals (e.g. 'draft', 'failed', 'publishing', 'published') used across SQL and Rust comparisons; consider centralizing these as constants or an enum + mapping to reduce drift risk and make future status changes safer.
- In `backoff_window_elapsed`, every call reparses `last_attempt_at` from RFC 3339; if this becomes a hot path, you might consider storing a parsed datetime or a numeric representation in the DB to avoid repeated parsing costs.
## Individual Comments
### Comment 1
<location path="src-tauri/src/db/scheduler.rs" line_range="58" />
<code_context>
-/// We avoid SQLite's `datetime('now')` here because it returns the
-/// format `'YYYY-MM-DD HH:MM:SS'` (space separator, no fractional, no
-/// Z), which sorts lexicographically before our RFC 3339 writes
+/// `now_rfc3339` is bound from Rust as the reference instant for
+/// `scheduled_at` comparisons (string-equal to what
+/// `chrono::Utc::now().to_rfc3339()` writes during scheduling). We avoid
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new backoff and retry logic by parsing timestamps earlier, tightening invariants, and streamlining data flow and tests while keeping Rust as the single source of truth.
You can keep the “single source of truth in Rust” goal while trimming a fair bit of complexity from the new backoff logic and data flow.
### 1. Parse `last_attempt_at` once in `row_to_post_record`
Instead of parsing RFC3339 in every `backoff_window_elapsed` call, parse at construction time and store a typed value (or `Option`) in `PostRecord`. That removes parsing and error-policy noise from the backoff predicate.
```rust
// Example: adjust PostRecord to carry a typed timestamp
pub struct PostRecord {
// ...
pub last_attempt_at: Option<DateTime<Utc>>,
pub failed_attempts: i64,
// ...
}
// In row_to_post_record:
fn row_to_post_record(row: &SqliteRow) -> Result<PostRecord, String> {
let last_attempt_str: Option<String> = row.try_get("last_attempt_at")?;
let last_attempt_at = match last_attempt_str {
None => None,
Some(s) => match DateTime::parse_from_rfc3339(&s) {
Ok(dt) => Some(dt.with_timezone(&Utc)),
Err(_) => None, // “fail open” here; treat malformed as never-attempted
},
};
Ok(PostRecord {
// ...
last_attempt_at,
failed_attempts: row.try_get("failed_attempts")?,
// ...
})
}
```
With that, `backoff_window_elapsed` can focus purely on the time-window logic:
```rust
fn backoff_window_elapsed(post: &PostRecord, now: DateTime<Utc>) -> bool {
let Some(last) = post.last_attempt_at else {
return true;
};
let idx = post
.failed_attempts
.clamp(0, MAX_FAILED_ATTEMPTS) as usize;
let window_min = BACKOFF_MINUTES[idx];
last + chrono::Duration::minutes(window_min) <= now
}
```
### 2. Tighten the backoff invariants and indexing
Given the invariant that the table covers all attempts, you can simplify the defensive indexing, replacing the `.get().unwrap_or_else(..)` dance with a simple clamp:
```rust
#[test]
fn backoff_table_covers_all_attempts() {
assert_eq!(BACKOFF_MINUTES, [0, 5, 30, 120]);
assert_eq!(MAX_FAILED_ATTEMPTS, 3);
assert_eq!(BACKOFF_MINUTES.len(), (MAX_FAILED_ATTEMPTS + 1) as usize);
}
```
Then the `clamp` above is enough; you no longer need `.get()` plus `.last()` fallbacks.
### 3. Avoid the intermediate `Vec<PostRecord>` in `list_due_for_publish`
You can keep the Rust-side backoff while avoiding the extra collect/filter pass by filtering during mapping:
```rust
pub async fn list_due_for_publish(pool: &SqlitePool) -> Result<Vec<PostRecord>, String> {
let now = Utc::now();
let now_rfc3339 = now.to_rfc3339();
let rows: Vec<SqliteRow> = sqlx::query(/* same SELECT */)
.bind(&now_rfc3339)
.bind(MAX_FAILED_ATTEMPTS)
.fetch_all(pool)
.await
.map_err(|e| e.to_string())?;
let mut due = Vec::with_capacity(rows.len());
for row in rows.iter() {
let post = row_to_post_record(row)?;
if backoff_window_elapsed(&post, now) {
due.push(post);
}
}
Ok(due)
}
```
This keeps all logic in Rust (no SQL `CASE`) but reduces the transformation pipeline to a straightforward loop.
### 4. Small test helper to reduce repetition
The new tests add good state coverage but duplicate a bit of “read status/failed_attempts” SQL. A tiny helper keeps them focused on transitions:
```rust
async fn fetch_attempts_and_status(
pool: &SqlitePool,
id: i64,
) -> (i64, String) {
sqlx::query_as("SELECT failed_attempts, status FROM post_history WHERE id = ?")
.bind(id)
.fetch_one(pool)
.await
.expect("fetch")
}
```
Then tests like `mark_failed_skips_when_status_is_not_publishing` become a little easier to scan:
```rust
let (failed_attempts, status) = fetch_attempts_and_status(&pool, id).await;
assert_eq!(failed_attempts, 0);
assert_eq!(status, "draft");
```
All of the above keeps the new behaviors (Rust-driven backoff, guards, outcomes) intact while making the control flow and backoff policy easier to reason about.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// We avoid SQLite's `datetime('now')` here because it returns the | ||
| /// format `'YYYY-MM-DD HH:MM:SS'` (space separator, no fractional, no | ||
| /// Z), which sorts lexicographically before our RFC 3339 writes | ||
| /// `now_rfc3339` is bound from Rust as the reference instant for |
There was a problem hiding this comment.
issue (complexity): Envisagez de simplifier la nouvelle logique de backoff et de retry en parsant les timestamps plus tôt, en renforçant les invariants, et en simplifiant le flux de données et les tests, tout en gardant Rust comme unique source de vérité.
Vous pouvez conserver l’objectif « Rust comme unique source de vérité » tout en réduisant sensiblement la complexité de la nouvelle logique de backoff et du flux de données.
1. Parser last_attempt_at une seule fois dans row_to_post_record
Au lieu de parser le RFC3339 à chaque appel de backoff_window_elapsed, parsez-le au moment de la construction et stockez une valeur typée (ou Option) dans PostRecord. Cela supprime le bruit lié au parsing et à la politique d’erreur dans le prédicat de backoff.
// Example: adjust PostRecord to carry a typed timestamp
pub struct PostRecord {
// ...
pub last_attempt_at: Option<DateTime<Utc>>,
pub failed_attempts: i64,
// ...
}
// In row_to_post_record:
fn row_to_post_record(row: &SqliteRow) -> Result<PostRecord, String> {
let last_attempt_str: Option<String> = row.try_get("last_attempt_at")?;
let last_attempt_at = match last_attempt_str {
None => None,
Some(s) => match DateTime::parse_from_rfc3339(&s) {
Ok(dt) => Some(dt.with_timezone(&Utc)),
Err(_) => None, // “fail open” here; treat malformed as never-attempted
},
};
Ok(PostRecord {
// ...
last_attempt_at,
failed_attempts: row.try_get("failed_attempts")?,
// ...
})
}Avec cela, backoff_window_elapsed peut se concentrer uniquement sur la logique de fenêtre temporelle :
fn backoff_window_elapsed(post: &PostRecord, now: DateTime<Utc>) -> bool {
let Some(last) = post.last_attempt_at else {
return true;
};
let idx = post
.failed_attempts
.clamp(0, MAX_FAILED_ATTEMPTS) as usize;
let window_min = BACKOFF_MINUTES[idx];
last + chrono::Duration::minutes(window_min) <= now
}2. Renforcer les invariants de backoff et l’indexation
Étant donné l’invariant selon lequel la table couvre toutes les tentatives, vous pouvez simplifier l’indexation défensive en remplaçant la gymnastique .get().unwrap_or_else(..) par un simple clamp :
#[test]
fn backoff_table_covers_all_attempts() {
assert_eq!(BACKOFF_MINUTES, [0, 5, 30, 120]);
assert_eq!(MAX_FAILED_ATTEMPTS, 3);
assert_eq!(BACKOFF_MINUTES.len(), (MAX_FAILED_ATTEMPTS + 1) as usize);
}Ensuite, le clamp ci-dessus suffit ; vous n’avez plus besoin des fallbacks .get() plus .last().
3. Éviter le Vec<PostRecord> intermédiaire dans list_due_for_publish
Vous pouvez conserver le backoff côté Rust tout en évitant la passe supplémentaire collect/filter en filtrant lors du mapping :
pub async fn list_due_for_publish(pool: &SqlitePool) -> Result<Vec<PostRecord>, String> {
let now = Utc::now();
let now_rfc3339 = now.to_rfc3339();
let rows: Vec<SqliteRow> = sqlx::query(/* same SELECT */)
.bind(&now_rfc3339)
.bind(MAX_FAILED_ATTEMPTS)
.fetch_all(pool)
.await
.map_err(|e| e.to_string())?;
let mut due = Vec::with_capacity(rows.len());
for row in rows.iter() {
let post = row_to_post_record(row)?;
if backoff_window_elapsed(&post, now) {
due.push(post);
}
}
Ok(due)
}Cela maintient toute la logique en Rust (pas de CASE SQL) tout en réduisant le pipeline de transformation à une boucle simple.
4. Petit helper de test pour réduire la répétition
Les nouveaux tests offrent une bonne couverture d’état mais dupliquent un peu de SQL « lecture status/failed_attempts ». Un petit helper leur permet de se concentrer sur les transitions :
async fn fetch_attempts_and_status(
pool: &SqlitePool,
id: i64,
) -> (i64, String) {
sqlx::query_as("SELECT failed_attempts, status FROM post_history WHERE id = ?")
.bind(id)
.fetch_one(pool)
.await
.expect("fetch")
}Ensuite, des tests comme mark_failed_skips_when_status_is_not_publishing deviennent un peu plus faciles à lire :
let (failed_attempts, status) = fetch_attempts_and_status(&pool, id).await;
assert_eq!(failed_attempts, 0);
assert_eq!(status, "draft");Tout ce qui précède conserve les nouveaux comportements (backoff piloté par Rust, garde-fous, résultats) tout en rendant le flux de contrôle et la politique de backoff plus faciles à comprendre.
Original comment in English
issue (complexity): Consider simplifying the new backoff and retry logic by parsing timestamps earlier, tightening invariants, and streamlining data flow and tests while keeping Rust as the single source of truth.
You can keep the “single source of truth in Rust” goal while trimming a fair bit of complexity from the new backoff logic and data flow.
1. Parse last_attempt_at once in row_to_post_record
Instead of parsing RFC3339 in every backoff_window_elapsed call, parse at construction time and store a typed value (or Option) in PostRecord. That removes parsing and error-policy noise from the backoff predicate.
// Example: adjust PostRecord to carry a typed timestamp
pub struct PostRecord {
// ...
pub last_attempt_at: Option<DateTime<Utc>>,
pub failed_attempts: i64,
// ...
}
// In row_to_post_record:
fn row_to_post_record(row: &SqliteRow) -> Result<PostRecord, String> {
let last_attempt_str: Option<String> = row.try_get("last_attempt_at")?;
let last_attempt_at = match last_attempt_str {
None => None,
Some(s) => match DateTime::parse_from_rfc3339(&s) {
Ok(dt) => Some(dt.with_timezone(&Utc)),
Err(_) => None, // “fail open” here; treat malformed as never-attempted
},
};
Ok(PostRecord {
// ...
last_attempt_at,
failed_attempts: row.try_get("failed_attempts")?,
// ...
})
}With that, backoff_window_elapsed can focus purely on the time-window logic:
fn backoff_window_elapsed(post: &PostRecord, now: DateTime<Utc>) -> bool {
let Some(last) = post.last_attempt_at else {
return true;
};
let idx = post
.failed_attempts
.clamp(0, MAX_FAILED_ATTEMPTS) as usize;
let window_min = BACKOFF_MINUTES[idx];
last + chrono::Duration::minutes(window_min) <= now
}2. Tighten the backoff invariants and indexing
Given the invariant that the table covers all attempts, you can simplify the defensive indexing, replacing the .get().unwrap_or_else(..) dance with a simple clamp:
#[test]
fn backoff_table_covers_all_attempts() {
assert_eq!(BACKOFF_MINUTES, [0, 5, 30, 120]);
assert_eq!(MAX_FAILED_ATTEMPTS, 3);
assert_eq!(BACKOFF_MINUTES.len(), (MAX_FAILED_ATTEMPTS + 1) as usize);
}Then the clamp above is enough; you no longer need .get() plus .last() fallbacks.
3. Avoid the intermediate Vec<PostRecord> in list_due_for_publish
You can keep the Rust-side backoff while avoiding the extra collect/filter pass by filtering during mapping:
pub async fn list_due_for_publish(pool: &SqlitePool) -> Result<Vec<PostRecord>, String> {
let now = Utc::now();
let now_rfc3339 = now.to_rfc3339();
let rows: Vec<SqliteRow> = sqlx::query(/* same SELECT */)
.bind(&now_rfc3339)
.bind(MAX_FAILED_ATTEMPTS)
.fetch_all(pool)
.await
.map_err(|e| e.to_string())?;
let mut due = Vec::with_capacity(rows.len());
for row in rows.iter() {
let post = row_to_post_record(row)?;
if backoff_window_elapsed(&post, now) {
due.push(post);
}
}
Ok(due)
}This keeps all logic in Rust (no SQL CASE) but reduces the transformation pipeline to a straightforward loop.
4. Small test helper to reduce repetition
The new tests add good state coverage but duplicate a bit of “read status/failed_attempts” SQL. A tiny helper keeps them focused on transitions:
async fn fetch_attempts_and_status(
pool: &SqlitePool,
id: i64,
) -> (i64, String) {
sqlx::query_as("SELECT failed_attempts, status FROM post_history WHERE id = ?")
.bind(id)
.fetch_one(pool)
.await
.expect("fetch")
}Then tests like mark_failed_skips_when_status_is_not_publishing become a little easier to scan:
let (failed_attempts, status) = fetch_attempts_and_status(&pool, id).await;
assert_eq!(failed_attempts, 0);
assert_eq!(status, "draft");All of the above keeps the new behaviors (Rust-driven backoff, guards, outcomes) intact while making the control flow and backoff policy easier to reason about.
3 tasks
thierryvm
added a commit
that referenced
this pull request
May 12, 2026
Wires the data layer mergé in PR #69+#70 to a real tokio task. Polls SQLite every 60s for due drafts, atomically locks each row via try_lock_for_publish, dispatches through the existing publish_post / publish_linkedin_post commands, and surfaces a desktop notification when a post exhausts MAX_FAILED_ATTEMPTS. The scheduler stays a dumb pump: zero in-Rust retry timers, all backoff lives in db::scheduler::BACKOFF_MINUTES re-read every tick. That survives app restarts without losing track of pending posts. Architecture: - `tick` / `process_post` take traits (PublishDispatcher + Notifier) so the loop is testable without an AppHandle (real Tauri runtime is too heavy to stand up in #[tokio::test]). Production impls are thin shells: TauriDispatcher routes by network, TauriNotifier wraps tauri-plugin-notification. - Pre-check token expiry against accounts.token_expires_at (added in migration 014). Expired token short-circuits with a clear error rather than waiting for Meta/LinkedIn to 401 — the dispatch still counts as a failed attempt so the retry budget burns down even on doomed tokens. New deps: - tauri-plugin-notification = "2" + notification:default capability - async-trait = "0.1" for the dispatcher seam (already transitive) Tests: +9 in src/scheduler.rs (240 -> 249 lib tests). Cover the headline F2 E2E (overdue draft -> published after one tick), retry budget exhaustion -> notification, lock-contention skip, backoff window respect between failures, and the three token-expiry branches. Out of scope (deferred to follow-ups): - Scheduler UI badges in Composer/Dashboard (PR F3) - Sourcery #70 complexity follow-ups (task #94) — DateTime<Utc> parsing in PostRecord touches frontend serde, warrants a dedicated PR rather than smuggling it through F2. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to PR #69 addressing 3 Sourcery review comments on
src-tauri/src/db/scheduler.rs.mark_publish_attempt_failed—WHERE status = 'publishing'prevents misfires from corruptingfailed_attempts. NewPublishFailureOutcome::Skippedvariant surfaces the no-op. Collapsed into oneUPDATE ... RETURNING(was 2 round-trips).reset_retry_state—WHERE status IN ('draft', 'failed')prevents accidentally un-publishing a published post or racing the scheduler's'publishing'lock. Signature nowResult<bool, String>so callers can surface the no-op.CASEinto Rust againstBACKOFF_MINUTES. The const table is now the single source of truth; no more silent-drift risk between docs and SQL literals.Test plan
cargo check→ 0 errorscargo clippy --lib -- -D warnings→ 0 warningscargo fmt --check→ cleancargo test --lib db::scheduler::→ 13/13 pass (10 existing adapted + 3 new regression tests)New regression tests
mark_failed_skips_when_status_is_not_publishing— locks the new guard contractreset_retry_state_rejects_published_row— critical: published rows stay publishedreset_retry_state_rejects_publishing_row— protects scheduler lock from racesNotes
#[allow(dead_code)]until PR F2 consumes them.🤖 Generated with Claude Code
Summary by Sourcery
Renforcer la logique de nouvelle tentative du scheduler et centraliser la politique de backoff tout en améliorant les sémantiques de mise à jour protégées par les statuts.
Corrections de bugs :
Améliorations :
UPDATE ... RETURNINGpour la prise en compte des échecs de publication et le reporting des résultats.Tests :
Original summary in English
Summary by Sourcery
Harden scheduler retry logic and centralize backoff policy while improving status-guarded update semantics.
Bug Fixes:
Enhancements:
Tests: