Skip to content

Commit f888d2b

Browse files
hyperpolymathclaude
andcommitted
refactor: convert feedback from per-file JSON to NDJSON append log
Replace one-JSON-file-per-submission with a single NDJSON log at /tmp/panll/feedback/feedback.ndjson. NDJSON is the correct format for append-only event logs: crash-safe (partial write loses one line, not the history), grep-searchable, countable with wc -l, no orphaned files. - Use serde_json::to_string (compact) instead of to_string_pretty - Append via OpenOptions::append (atomic on POSIX) - Update both tests for NDJSON semantics and parallel-safe assertions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 212338f commit f888d2b

1 file changed

Lines changed: 68 additions & 38 deletions

File tree

src-tauri/src/main.rs

Lines changed: 68 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -631,27 +631,32 @@ fn get_vexation_index() -> f64 {
631631
}
632632

633633
/// Submits feedback to the Feedback-O-Tron collective.
634+
///
635+
/// Appends one NDJSON line to `/tmp/panll/feedback/feedback.ndjson`.
636+
/// NDJSON (Newline-Delimited JSON) is used instead of one-file-per-submission
637+
/// because feedback is an append-only log:
638+
/// - Crash-safe: a partial write loses one line, not the entire history
639+
/// - Searchable: `grep "bug" feedback.ndjson` finds all bug reports
640+
/// - Countable: `wc -l feedback.ndjson` gives the total instantly
641+
/// - No orphaned files to clean up
634642
#[tauri::command]
635643
fn submit_feedback(
636644
pane_l_state: String,
637645
pane_n_state: String,
638646
pane_w_state: String,
639647
report_type: String,
640648
) -> Result<String, String> {
641-
// Create feedback directory
642649
let feedback_dir = env::temp_dir().join("panll").join("feedback");
643650
fs::create_dir_all(&feedback_dir)
644651
.map_err(|e| format!("Failed to create feedback directory: {}", e))?;
645652

646-
// Generate timestamp and ID
647653
let timestamp = SystemTime::now()
648654
.duration_since(UNIX_EPOCH)
649655
.map_err(|e| format!("Time error: {}", e))?
650656
.as_secs();
651657

652658
let id = format!("{}-{}", report_type, timestamp);
653659

654-
// Create feedback JSON
655660
let feedback_json = json!({
656661
"id": id,
657662
"report_type": report_type,
@@ -661,14 +666,22 @@ fn submit_feedback(
661666
"timestamp": timestamp,
662667
});
663668

664-
// Write to file
665-
let filename = format!("feedback-{}-{}.json", report_type, timestamp);
666-
let filepath = feedback_dir.join(&filename);
667-
668-
fs::write(&filepath, serde_json::to_string_pretty(&feedback_json).map_err(|e| e.to_string())?)
669-
.map_err(|e| format!("Failed to write feedback file: {}", e))?;
670-
671-
Ok(format!("Feedback saved: {}", filepath.display()))
669+
// Serialize as a single compact line (no pretty-printing — NDJSON requires one object per line).
670+
let mut line = serde_json::to_string(&feedback_json).map_err(|e| e.to_string())?;
671+
line.push('\n');
672+
673+
// Append to the NDJSON log file. OpenOptions ensures atomic append on POSIX.
674+
let filepath = feedback_dir.join("feedback.ndjson");
675+
use std::io::Write;
676+
let mut file = fs::OpenOptions::new()
677+
.create(true)
678+
.append(true)
679+
.open(&filepath)
680+
.map_err(|e| format!("Failed to open feedback log: {}", e))?;
681+
file.write_all(line.as_bytes())
682+
.map_err(|e| format!("Failed to append feedback: {}", e))?;
683+
684+
Ok(format!("Feedback appended: {}", filepath.display()))
672685
}
673686

674687
#[tauri::command]
@@ -936,7 +949,14 @@ Commands:
936949
}
937950

938951
#[test]
939-
fn test_submit_feedback_creates_json_file() {
952+
fn test_submit_feedback_appends_ndjson() {
953+
let feedback_path = env::temp_dir().join("panll").join("feedback").join("feedback.ndjson");
954+
let lines_before = if feedback_path.exists() {
955+
fs::read_to_string(&feedback_path).unwrap_or_default().lines().count()
956+
} else {
957+
0
958+
};
959+
940960
let result = submit_feedback(
941961
"pane_l content".to_string(),
942962
"pane_n content".to_string(),
@@ -946,30 +966,36 @@ Commands:
946966

947967
assert!(result.is_ok());
948968
let message = result.unwrap();
949-
assert!(message.contains("Feedback saved:"));
950-
assert!(message.contains("feedback-bug-"));
951-
952-
// Clean up: extract filepath from message and verify file exists
953-
if let Some(path_str) = message.strip_prefix("Feedback saved: ") {
954-
let path = PathBuf::from(path_str);
955-
assert!(path.exists());
956-
957-
// Read and verify JSON structure
958-
let content = fs::read_to_string(&path).expect("read feedback file");
959-
let feedback: Value = serde_json::from_str(&content).expect("parse feedback JSON");
960-
961-
assert_eq!(feedback.get("report_type").and_then(Value::as_str), Some("bug"));
962-
assert_eq!(feedback.get("pane_l_state").and_then(Value::as_str), Some("pane_l content"));
963-
assert!(feedback.get("id").is_some());
964-
assert!(feedback.get("timestamp").is_some());
965-
966-
// Clean up
967-
let _ = fs::remove_file(path);
968-
}
969+
assert!(message.contains("Feedback appended:"));
970+
assert!(message.contains("feedback.ndjson"));
971+
972+
// Verify the file grew (other parallel tests may also append, so use >=).
973+
let content = fs::read_to_string(&feedback_path).expect("read feedback log");
974+
let lines: Vec<&str> = content.lines().collect();
975+
assert!(lines.len() > lines_before, "file should have grown");
976+
977+
// Find our specific entry by its unique pane_l_state value.
978+
let our_line = lines.iter().find(|line| line.contains("pane_l content"))
979+
.expect("our feedback entry should be in the log");
980+
let feedback: Value = serde_json::from_str(our_line).expect("parse NDJSON line");
981+
assert_eq!(feedback.get("report_type").and_then(Value::as_str), Some("bug"));
982+
assert_eq!(feedback.get("pane_l_state").and_then(Value::as_str), Some("pane_l content"));
983+
assert!(feedback.get("id").is_some());
984+
assert!(feedback.get("timestamp").is_some());
985+
986+
// Verify it's compact (no embedded newlines within the JSON object).
987+
assert!(!our_line.contains('\n'));
969988
}
970989

971990
#[test]
972991
fn test_submit_feedback_different_report_type() {
992+
let feedback_path = env::temp_dir().join("panll").join("feedback").join("feedback.ndjson");
993+
let lines_before = if feedback_path.exists() {
994+
fs::read_to_string(&feedback_path).unwrap_or_default().lines().count()
995+
} else {
996+
0
997+
};
998+
973999
let result = submit_feedback(
9741000
"state1".to_string(),
9751001
"state2".to_string(),
@@ -979,12 +1005,16 @@ Commands:
9791005

9801006
assert!(result.is_ok());
9811007
let message = result.unwrap();
982-
assert!(message.contains("feedback-feature-request-"));
983-
984-
// Clean up
985-
if let Some(path_str) = message.strip_prefix("Feedback saved: ") {
986-
let _ = fs::remove_file(path_str);
987-
}
1008+
assert!(message.contains("feedback.ndjson"));
1009+
1010+
// Verify the file grew and our entry is present.
1011+
let content = fs::read_to_string(&feedback_path).expect("read feedback log");
1012+
let lines: Vec<&str> = content.lines().collect();
1013+
assert!(lines.len() > lines_before, "file should have grown");
1014+
let our_line = lines.iter().find(|line| line.contains("\"state1\""))
1015+
.expect("our feedback entry should be in the log");
1016+
let last: Value = serde_json::from_str(our_line).expect("parse NDJSON line");
1017+
assert_eq!(last.get("report_type").and_then(Value::as_str), Some("feature-request"));
9881018
}
9891019
}
9901020

0 commit comments

Comments
 (0)