@@ -9,7 +9,7 @@ use anyhow::{Result, bail};
99use clap:: Parser ;
1010use serde:: Serialize ;
1111use std:: io:: { self , Write } ;
12- use std:: path:: PathBuf ;
12+ use std:: path:: { Path , PathBuf } ;
1313
1414/// Feedback CLI command.
1515#[ derive( Debug , Parser ) ]
@@ -294,19 +294,10 @@ async fn run_history(args: FeedbackHistoryArgs) -> Result<()> {
294294 return Ok ( ( ) ) ;
295295 }
296296
297- let mut entries = Vec :: new ( ) ;
297+ let ( mut entries, warnings ) = load_feedback_entries ( & feedback_dir ) ;
298298
299- // Read feedback files
300- if let Ok ( dir_entries) = std:: fs:: read_dir ( & feedback_dir) {
301- for entry in dir_entries. flatten ( ) {
302- let path = entry. path ( ) ;
303- if path. extension ( ) . is_some_and ( |e| e == "json" )
304- && let Ok ( content) = std:: fs:: read_to_string ( & path)
305- && let Ok ( entry) = serde_json:: from_str :: < FeedbackEntry > ( & content)
306- {
307- entries. push ( entry) ;
308- }
309- }
299+ for warning in & warnings {
300+ eprintln ! ( "{}" , warning) ;
310301 }
311302
312303 // Sort by timestamp (newest first)
@@ -318,7 +309,11 @@ async fn run_history(args: FeedbackHistoryArgs) -> Result<()> {
318309 if args. json {
319310 println ! ( "{}" , serde_json:: to_string_pretty( & entries) ?) ;
320311 } else if entries. is_empty ( ) {
321- println ! ( "No feedback history found." ) ;
312+ if warnings. is_empty ( ) {
313+ println ! ( "No feedback history found." ) ;
314+ } else {
315+ println ! ( "Feedback history contains unreadable entries. See warnings above." ) ;
316+ }
322317 } else {
323318 println ! ( "Feedback History:" ) ;
324319 println ! ( "{}" , "-" . repeat( 60 ) ) ;
@@ -338,6 +333,62 @@ async fn run_history(args: FeedbackHistoryArgs) -> Result<()> {
338333 Ok ( ( ) )
339334}
340335
336+ fn load_feedback_entries ( feedback_dir : & Path ) -> ( Vec < FeedbackEntry > , Vec < String > ) {
337+ let mut entries = Vec :: new ( ) ;
338+ let mut warnings = Vec :: new ( ) ;
339+
340+ match std:: fs:: read_dir ( feedback_dir) {
341+ Ok ( dir_entries) => {
342+ for entry in dir_entries {
343+ let entry = match entry {
344+ Ok ( entry) => entry,
345+ Err ( error) => {
346+ warnings. push ( format ! (
347+ "Warning: Failed to read feedback directory entry in {}: {}" ,
348+ feedback_dir. display( ) ,
349+ error
350+ ) ) ;
351+ continue ;
352+ }
353+ } ;
354+
355+ let path = entry. path ( ) ;
356+ if !path. extension ( ) . is_some_and ( |ext| ext == "json" ) {
357+ continue ;
358+ }
359+
360+ let content = match std:: fs:: read_to_string ( & path) {
361+ Ok ( content) => content,
362+ Err ( error) => {
363+ warnings. push ( format ! (
364+ "Warning: Failed to read feedback file {}: {}" ,
365+ path. display( ) ,
366+ error
367+ ) ) ;
368+ continue ;
369+ }
370+ } ;
371+
372+ match serde_json:: from_str :: < FeedbackEntry > ( & content) {
373+ Ok ( entry) => entries. push ( entry) ,
374+ Err ( error) => warnings. push ( format ! (
375+ "Warning: Failed to parse feedback file {}: {}" ,
376+ path. display( ) ,
377+ error
378+ ) ) ,
379+ }
380+ }
381+ }
382+ Err ( error) => warnings. push ( format ! (
383+ "Warning: Failed to read feedback directory {}: {}" ,
384+ feedback_dir. display( ) ,
385+ error
386+ ) ) ,
387+ }
388+
389+ ( entries, warnings)
390+ }
391+
341392/// Submit feedback (save locally and optionally upload).
342393async fn submit_feedback (
343394 category : & str ,
@@ -410,6 +461,7 @@ fn read_single_line() -> Result<String> {
410461#[ cfg( test) ]
411462mod tests {
412463 use super :: * ;
464+ use tempfile:: tempdir;
413465
414466 #[ test]
415467 fn test_feedback_entry_serialization_with_session ( ) {
@@ -573,4 +625,56 @@ mod tests {
573625 serde_json:: from_str ( & pretty_json) . expect ( "deserialization should succeed" ) ;
574626 assert_eq ! ( parsed. id, entry. id) ;
575627 }
628+
629+ #[ test]
630+ fn test_load_feedback_entries_warns_on_invalid_json ( ) {
631+ let temp_dir = tempdir ( ) . expect ( "temp dir should be created" ) ;
632+ let feedback_dir = temp_dir. path ( ) ;
633+
634+ std:: fs:: write (
635+ feedback_dir. join ( "bad.json" ) ,
636+ r#"{"id":"broken","# ,
637+ )
638+ . expect ( "invalid file should be written" ) ;
639+
640+ let ( entries, warnings) = load_feedback_entries ( feedback_dir) ;
641+
642+ assert ! ( entries. is_empty( ) , "invalid JSON should not produce entries" ) ;
643+ assert_eq ! ( warnings. len( ) , 1 , "invalid JSON should produce one warning" ) ;
644+ assert ! (
645+ warnings[ 0 ] . contains( "Failed to parse feedback file" ) ,
646+ "warning should mention parse failure"
647+ ) ;
648+ }
649+
650+ #[ test]
651+ fn test_load_feedback_entries_keeps_valid_entries_and_collects_warnings ( ) {
652+ let temp_dir = tempdir ( ) . expect ( "temp dir should be created" ) ;
653+ let feedback_dir = temp_dir. path ( ) ;
654+
655+ let valid_entry = FeedbackEntry {
656+ id : "valid-entry" . to_string ( ) ,
657+ timestamp : "2024-09-01T10:00:00Z" . to_string ( ) ,
658+ category : "general" . to_string ( ) ,
659+ message : "This one is valid" . to_string ( ) ,
660+ session_id : None ,
661+ } ;
662+
663+ std:: fs:: write (
664+ feedback_dir. join ( "good.json" ) ,
665+ serde_json:: to_string ( & valid_entry) . expect ( "valid JSON should serialize" ) ,
666+ )
667+ . expect ( "valid file should be written" ) ;
668+ std:: fs:: write (
669+ feedback_dir. join ( "bad.json" ) ,
670+ r#"{"id":"broken","# ,
671+ )
672+ . expect ( "invalid file should be written" ) ;
673+
674+ let ( entries, warnings) = load_feedback_entries ( feedback_dir) ;
675+
676+ assert_eq ! ( entries. len( ) , 1 , "valid entry should still be loaded" ) ;
677+ assert_eq ! ( entries[ 0 ] . id, "valid-entry" ) ;
678+ assert_eq ! ( warnings. len( ) , 1 , "invalid entry should still be reported" ) ;
679+ }
576680}
0 commit comments