@@ -177,32 +177,13 @@ pub struct UpdatePrRequest<'a> {
177177}
178178
179179// ============== PR Cache Types ==============
180-
181- /// Cache for closed PR data, keyed by repo full name (e.g., "owner/repo")
182- #[ derive( Debug , Default , Serialize , Deserialize ) ]
183- pub struct PrCache {
184- /// Version for cache format migrations
185- #[ serde( default ) ]
186- pub version : u32 ,
187- /// Per-repo PR caches
188- #[ serde( default ) ]
189- pub repos : std:: collections:: HashMap < String , RepoPrCache > ,
190- }
191-
192- /// Cache for a single repository's closed PRs
193- #[ derive( Debug , Clone , Default , Serialize , Deserialize ) ]
194- pub struct RepoPrCache {
195- /// Watermark: the `updated_at` timestamp of the most recently updated PR we've seen
196- /// Format: ISO 8601 string (e.g., "2025-01-02T15:30:00Z")
197- #[ serde( default ) ]
198- pub watermark : String ,
199- /// Cached closed PRs, keyed by head branch name
200- #[ serde( default ) ]
201- pub closed_prs : std:: collections:: HashMap < String , CachedPullRequest > ,
202- }
180+ //
181+ // The cache storage itself (schema, per-repo scoped access) lives in `crate::pr_cache`. The
182+ // types below are the cached PR shapes it stores; they stay here since they mirror the API
183+ // response types (`PullRequest` et al.) above.
203184
204185/// Full PR metadata for caching (mirrors PullRequest with Serialize)
205- #[ derive( Debug , Clone , Serialize , Deserialize ) ]
186+ #[ derive( Debug , Clone , PartialEq , Serialize , Deserialize ) ]
206187pub struct CachedPullRequest {
207188 pub number : u64 ,
208189 pub state : PrState ,
@@ -221,21 +202,21 @@ pub struct CachedPullRequest {
221202}
222203
223204/// Cached branch reference (mirrors PrBranchRef with Serialize)
224- #[ derive( Debug , Clone , Serialize , Deserialize ) ]
205+ #[ derive( Debug , Clone , PartialEq , Serialize , Deserialize ) ]
225206pub struct CachedPrBranchRef {
226207 pub ref_name : String ,
227208 pub sha : String ,
228209 pub repo : Option < CachedPrRepoRef > ,
229210}
230211
231212/// Cached repo reference
232- #[ derive( Debug , Clone , Serialize , Deserialize ) ]
213+ #[ derive( Debug , Clone , PartialEq , Serialize , Deserialize ) ]
233214pub struct CachedPrRepoRef {
234215 pub full_name : String ,
235216}
236217
237218/// Cached user reference
238- #[ derive( Debug , Clone , Serialize , Deserialize ) ]
219+ #[ derive( Debug , Clone , PartialEq , Serialize , Deserialize ) ]
239220pub struct CachedPrUser {
240221 pub login : String ,
241222}
@@ -519,68 +500,91 @@ impl GitHubClient {
519500 /// List closed PRs with caching support.
520501 ///
521502 /// Uses a watermark timestamp strategy:
522- /// 1. Loads cached closed PRs from the provided cache
503+ /// 1. Loads cached closed PRs for this repo from the cache handle
523504 /// 2. Fetches PRs from API sorted by `updated_at` descending
524505 /// 3. Stops fetching when encountering a PR older than the watermark
525506 /// 4. Merges fresh data with cache (fresh data wins for any branch name)
526- /// 5. Updates watermark to most recent `updated_at` seen
507+ /// 5. Persists the merged data and an updated watermark (best-effort; a persistence
508+ /// failure only costs the *next* call's warm cache, not this call's result)
527509 pub fn list_closed_prs_with_cache (
528510 & self ,
529511 repo : & RepoIdentifier ,
530- cache : & mut PrCache ,
512+ cache : & crate :: pr_cache :: PrCacheHandle ,
531513 on_progress : Option < & dyn Fn ( usize , usize ) > ,
532514 ) -> Result < PrListResult , GitHubError > {
533515 let repo_key = repo. full_name ( ) ;
534516
535- // Get existing cache for this repo
536- let repo_cache = cache. repos . entry ( repo_key. clone ( ) ) . or_default ( ) ;
537-
538- let watermark = if repo_cache. watermark . is_empty ( ) {
517+ let mut closed_prs = cache. closed_prs_for_repo ( & repo_key) . unwrap_or_else ( |e| {
518+ tracing:: warn!( "Failed to read PR cache for {}: {}" , repo_key, e) ;
519+ std:: collections:: HashMap :: new ( )
520+ } ) ;
521+ let watermark = cache. watermark ( & repo_key) . unwrap_or_else ( |e| {
522+ tracing:: warn!( "Failed to read PR cache watermark for {}: {}" , repo_key, e) ;
539523 None
540- } else {
541- Some ( repo_cache. watermark . clone ( ) )
542- } ;
524+ } ) ;
525+ tracing:: debug!(
526+ "PR cache for {}: {} cached closed PRs, watermark={:?}" ,
527+ repo_key,
528+ closed_prs. len( ) ,
529+ watermark
530+ ) ;
543531
544532 // Fetch PRs with early termination based on watermark
545533 let fresh_prs =
546534 self . list_prs_until_watermark ( repo, "closed" , watermark. as_deref ( ) , on_progress) ?;
535+ tracing:: debug!(
536+ "Fetched {} fresh closed PRs for {} (a small number means the watermark cache hit; \
537+ a number near the repo's total closed-PR count means a full backfill happened)",
538+ fresh_prs. len( ) ,
539+ repo_key
540+ ) ;
547541
548542 // Track the newest updated_at for new watermark
549543 let mut newest_updated_at: Option < String > = None ;
544+ let mut fresh_cached: std:: collections:: HashMap < String , CachedPullRequest > =
545+ std:: collections:: HashMap :: new ( ) ;
550546
551- // Merge fresh PRs into cache
552547 for ( branch_name, pr) in & fresh_prs {
553- // Track newest timestamp
554548 if newest_updated_at
555549 . as_ref ( )
556550 . is_none_or ( |ts| pr. updated_at > * ts)
557551 {
558552 newest_updated_at = Some ( pr. updated_at . clone ( ) ) ;
559553 }
560554
561- // Update cache with fresh data
562- repo_cache
563- . closed_prs
564- . insert ( branch_name. clone ( ) , CachedPullRequest :: from ( pr) ) ;
555+ let cached_pr = CachedPullRequest :: from ( pr) ;
556+ closed_prs. insert ( branch_name. clone ( ) , cached_pr. clone ( ) ) ;
557+ fresh_cached. insert ( branch_name. clone ( ) , cached_pr) ;
565558 }
566559
567- // Update watermark if we saw newer data
568- if let Some ( ts) = newest_updated_at
569- && ( repo_cache. watermark . is_empty ( ) || ts > repo_cache. watermark )
570- {
571- repo_cache. watermark = ts;
560+ let new_watermark = match ( & watermark, & newest_updated_at) {
561+ ( None , Some ( ts) ) => Some ( ts. clone ( ) ) ,
562+ ( Some ( current) , Some ( ts) ) if ts > current => Some ( ts. clone ( ) ) ,
563+ _ => None ,
564+ } ;
565+
566+ tracing:: debug!(
567+ "Updated PR cache watermark for {}: {:?} -> {:?}" ,
568+ repo_key,
569+ watermark,
570+ new_watermark
571+ ) ;
572+ if let Err ( e) = cache. commit_fresh_prs (
573+ & repo_key,
574+ fresh_cached. iter ( ) . map ( |( k, v) | ( k. as_str ( ) , v) ) ,
575+ new_watermark. as_deref ( ) ,
576+ ) {
577+ tracing:: warn!( "Failed to persist PR cache for {}: {}" , repo_key, e) ;
572578 }
573579
574580 // Collect all authors from cache before filtering (for pruning decisions)
575- let all_authors: std:: collections:: HashMap < String , String > = repo_cache
576- . closed_prs
581+ let all_authors: std:: collections:: HashMap < String , String > = closed_prs
577582 . iter ( )
578583 . map ( |( branch, cached_pr) | ( branch. clone ( ) , cached_pr. user . login . clone ( ) ) )
579584 . collect ( ) ;
580585
581586 // Convert cache to return type, applying filters
582- let prs: std:: collections:: HashMap < String , PullRequest > = repo_cache
583- . closed_prs
587+ let prs: std:: collections:: HashMap < String , PullRequest > = closed_prs
584588 . iter ( )
585589 . map ( |( k, v) | ( k. clone ( ) , PullRequest :: from ( v) ) )
586590 . filter ( |( _, pr) | self . should_include_pr ( pr) )
@@ -1233,42 +1237,6 @@ pub fn login_interactive() -> Result<String> {
12331237 }
12341238}
12351239
1236- // ============== PR Cache Functions ==============
1237-
1238- /// Get path to PR cache file
1239- fn get_pr_cache_path ( ) -> Result < PathBuf > {
1240- let base_dirs = xdg:: BaseDirectories :: with_prefix ( env ! ( "CARGO_PKG_NAME" ) ) ;
1241- base_dirs
1242- . place_state_file ( "pr_cache.yaml" )
1243- . context ( "Failed to determine PR cache file path" )
1244- }
1245-
1246- /// Load PR cache from disk
1247- pub fn load_pr_cache ( ) -> Result < PrCache > {
1248- let cache_path = get_pr_cache_path ( ) ?;
1249- if !cache_path. exists ( ) {
1250- return Ok ( PrCache :: default ( ) ) ;
1251- }
1252- let contents = fs:: read_to_string ( & cache_path) . context ( "Failed to read PR cache file" ) ?;
1253- serde_yaml:: from_str ( & contents) . context ( "Failed to parse PR cache file" )
1254- }
1255-
1256- /// Save PR cache to disk
1257- pub fn save_pr_cache ( cache : & PrCache ) -> Result < ( ) > {
1258- let cache_path = get_pr_cache_path ( ) ?;
1259- let contents = serde_yaml:: to_string ( cache) ?;
1260- write_file_secure ( & cache_path, & contents) ?;
1261- Ok ( ( ) )
1262- }
1263-
1264- /// Clear PR cache for a specific repo
1265- pub fn clear_pr_cache ( repo_full_name : & str ) -> Result < ( ) > {
1266- let mut cache = load_pr_cache ( ) . unwrap_or_default ( ) ;
1267- cache. repos . remove ( repo_full_name) ;
1268- save_pr_cache ( & cache) ?;
1269- Ok ( ( ) )
1270- }
1271-
12721240// ============== Cache Conversion Traits ==============
12731241
12741242impl From < & PullRequest > for CachedPullRequest {
0 commit comments