Refactor: extract JsonFileHelper to eliminate duplicate JSON persistence code

[Copilot]

Three classes (NotificationHistory, ReviewRequestHistory, ReviewRequestService) each duplicated the same ~30-line Load/Save pattern for JSON file persistence — ~90 lines of structural repetition across 6 methods.

Changes

• New src/JsonFileHelper.cs: Generic static helpers centralizing all JSON file I/O:
  • Load<T>(fileName, errorContext, defaultValue) — existence check, deserialize, fallback to default on missing file or error
  • Save<T>(fileName, data, errorContext) — serialize with WriteIndented, write to file, log on error
  • Error messages include the file name for easier diagnosis
• NotificationHistory, ReviewRequestHistory, ReviewRequestService: Inline Load/Save bodies replaced with JsonFileHelper calls; unused using System.Text.Json removed from each

Before / After

// Before (repeated verbatim in all 3 classes, ~15 lines each):
private List<ReviewRequestEntry> Load()
{
    if (File.Exists(Constants.ReviewRequestDetailsFileName))
    {
        try
        {
            var json = File.ReadAllText(Constants.ReviewRequestDetailsFileName);
            return JsonSerializer.Deserialize<List<ReviewRequestEntry>>(json) ?? new List<ReviewRequestEntry>();
        }
        catch (Exception ex) { Logger.LogError($"Error loading review request details: {ex.Message}", ex); }
    }
    return new List<ReviewRequestEntry>();
}

// After:
private List<ReviewRequestEntry> Load() =>
    JsonFileHelper.Load<List<ReviewRequestEntry>>(
        Constants.ReviewRequestDetailsFileName, "review request details", new List<ReviewRequestEntry>());

Locking strategy in callers is unchanged.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Duplicate Code: JSON File Persistence Load/Save Pattern in History/Service Classes</issue_title>
<issue_description>Analysis of recent commits

Assignee: @copilot

Summary

Three classes (NotificationHistory, ReviewRequestHistory, and ReviewRequestService) contain nearly identical Load() and Save() methods for JSON-based file persistence. This results in ~90 lines of structurally duplicated code across 6 methods in 3 files, all following the exact same pattern.

Duplication Details

Pattern: JSON File Load

• Severity: High
• Occurrences: 3 instances
• Locations:
  • src/NotificationHistory.cs (lines 18–33)
  • src/ReviewRequestHistory.cs (lines 15–30)
  • src/ReviewRequestService.cs (lines 69–84)
• Code Sample (NotificationHistory — others are structurally identical):

  private List(NotificationEntry) Load()
  {
      if (File.Exists(Constants.NotificationHistoryFileName))
      {
          try
          {
              var json = File.ReadAllText(Constants.NotificationHistoryFileName);
              return JsonSerializer.Deserialize(List<NotificationEntry)>(json) ?? new List(NotificationEntry)();
          }
          catch (Exception ex)
          {
              Logger.LogError($"Error loading notification history", ex);
          }
      }
      return new List(NotificationEntry)();
  }

Pattern: JSON File Save (with lock)

• Severity: High
• Occurrences: 3 instances
• Locations:
  • src/NotificationHistory.cs (lines 36–50)
  • src/ReviewRequestHistory.cs (lines 33–49)
  • src/ReviewRequestService.cs (lines 86–99)
• Code Sample (ReviewRequestHistory — others are structurally identical):

  private void Save()
  {
      lock (_lockObject)
      {
          try
          {
              var options = new JsonSerializerOptions { WriteIndented = true };
              var list = _seenRequestIds.ToList();
              var json = JsonSerializer.Serialize(list, options);
              File.WriteAllText(Constants.ReviewRequestHistoryFileName, json);
          }
          catch (Exception ex)
          {
              Logger.LogError($"Error saving review request history: {ex.Message}", ex);
          }
      }
  }

Impact Analysis

• Maintainability: Any bug fix or improvement to file I/O logic (e.g., atomic writes, backup strategy, encoding) must be applied in 3 separate places. This is the primary risk vector for divergence.
• Bug Risk: High — if error handling, serializer options, or file-write strategy changes in one class but not others, silent inconsistencies will accumulate.
• Code Bloat: ~90 lines of duplication that could be reduced to a single generic utility (~20 lines).

Refactoring Recommendations

1. Extract a generic JsonFileStore(T) utility class

   • Create: src/Persistence/JsonFileStore.cs
   • Provide static or instance Load(T)(string filePath) and Save(T)(string filePath, T data) methods with consistent error handling
   • Estimated effort: Low (2–3 hours)
   • Benefits: Single source of truth for JSON persistence; centralized error handling; easier to add features like atomic writes

2. Alternatively, extract a PersistenceHelper static class

   • Simpler approach with static LoadJson(T) and SaveJson(T) helpers
   • No need to change class structure, just delegate the repeated body

Implementation Checklist

• Review duplication findings
• Design JsonFileStore(T) or PersistenceHelper utility
• Refactor NotificationHistory.Load() / NotificationHistory.Save()
• Refactor ReviewRequestHistory.Load() / ReviewRequestHistory.Save()
• Refactor ReviewRequestService.Load() / ReviewRequestService.Save()
• Update tests (if any cover these methods)
• Verify no functionality broken

Analysis Metadata

• Analyzed Files: 21 .cs files
• Detection Method: Pattern search + manual cross-file structural analysis
• Analysis Date: 2026-03-19

Generated by Duplicate Code Detector · ◷

To install this agentic workflow, run

gh aw add github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619

• Fixes Duplicate Code: JSON File Persistence Load/Save Pattern in History/Service Classes #158

---

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

[Copilot]

Pull request overview

Refactors JSON file persistence in the AgentSupervisor WinForms app by centralizing the repeated Load/Save JSON file I/O logic into a single helper, reducing duplication across history/service classes.

Changes:

• Added JsonFileHelper with generic Load<T> / Save<T> methods for JSON file persistence and consistent error logging.
• Updated NotificationHistory, ReviewRequestHistory, and ReviewRequestService to delegate JSON persistence to JsonFileHelper.
• Removed now-unused using System.Text.Json directives from the refactored classes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/JsonFileHelper.cs	Introduces centralized generic JSON load/save helpers with consistent error logging.
src/NotificationHistory.cs	Replaces inline JSON file persistence with JsonFileHelper calls.
src/ReviewRequestHistory.cs	Replaces inline JSON file persistence with JsonFileHelper calls (list-to-HashSet remains equivalent).
src/ReviewRequestService.cs	Replaces inline JSON file persistence with JsonFileHelper calls for review request details.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.