Skip to content

Refactor duplicate JSON/JSONL parsing logic into shared utility#4574

Merged
pelikhan merged 2 commits intomainfrom
copilot/refactor-json-parsing-fallback
Nov 22, 2025
Merged

Refactor duplicate JSON/JSONL parsing logic into shared utility#4574
pelikhan merged 2 commits intomainfrom
copilot/refactor-json-parsing-fallback

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 22, 2025

Both parse_claude_log.cjs and parse_copilot_log.cjs contained ~50 lines of identical JSON/JSONL parsing fallback logic, creating maintenance burden and divergence risk.

Changes

  • New shared function: Created parseLogEntries() in log_parser_shared.cjs to handle JSON array, JSONL, and mixed format parsing with debug logs
  • parse_claude_log.cjs: Removed 47 lines of duplicate parsing code, replaced with shared function call
  • parse_copilot_log.cjs: Removed 38 lines of duplicate parsing code, replaced with shared function call
  • Test coverage: Added 11 unit tests covering all format variations and edge cases

Example

Before (duplicated in both parsers):

// First, try to parse as JSON array
try {
  logEntries = JSON.parse(logContent);
  if (!Array.isArray(logEntries)) throw new Error("Not a JSON array");
} catch (jsonArrayError) {
  // Try JSONL format - 40+ lines of identical parsing logic...
  logEntries = [];
  const lines = logContent.split("\n");
  for (const line of lines) {
    const trimmedLine = line.trim();
    // ... more parsing logic
  }
}

After:

const logEntries = parseLogEntries(logContent);
if (!logEntries) {
  return { markdown: "Log format not recognized...", mcpFailures: [], maxTurnsHit: false };
}

Net reduction: 85 lines of duplicate code eliminated.

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Duplicate Code: JSON/JSONL Parsing Fallback in Log Parsers</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Log Parser JSON Fallback

Analysis of commit 102022d

Assignee: @copilot

Summary

Both Copilot and Claude log parsers reimplement the same JSON/JSONL parsing fallback logic, even down to identical trimming, array-handling, and line-by-line parsing. This block is ~50 lines long in each file and differs only by parser name, making it easy for the parsers to drift out of sync when formats change.

Duplication Details

Pattern: JSON/JSONL parsing fallback

  • Severity: Medium
  • Occurrences: 2
  • Locations:
    • pkg/workflow/js/parse_claude_log.cjs (lines 31-89)
    • pkg/workflow/js/parse_copilot_log.cjs (lines 60-118)
  • Code Sample:
    try {
      logEntries = JSON.parse(logContent);
      if (!Array.isArray(logEntries)) {
        throw new Error("Not a JSON array");
      }
    } catch (jsonArrayError) {
      logEntries = [];
      const lines = logContent.split("\n");
      for (const line of lines) {
        const trimmedLine = line.trim();
        if (trimmedLine === "") continue;
        if (trimmedLine.startsWith("[{")) {
          // try to parse array
        }
        if (!trimmedLine.startsWith("{")) continue;
        try {
          const jsonEntry = JSON.parse(trimmedLine);
          logEntries.push(jsonEntry);
        } catch (jsonLineError) {
          continue;
        }
      }
    }

Impact Analysis

  • Maintainability: Updating parsing behavior (e.g., new log format or stricter validation) requires editing both files, inviting divergence.
  • Bug Risk: Fixes applied to one parser (Copilot or Claude) may be missed in the other, leading to inconsistent user-facing summaries for the same log format.
  • Code Bloat: ~100 duplicated lines increase file size and noise.

Refactoring Recommendations

  1. Extract shared parser helper
    • Create a utility in log_parser_shared.cjs (e.g., parseLogEntries(logContent, options)) that encapsulates the JSON array → JSONL fallback.
    • Estimated effort: 2-3 hours. Benefits: single source of truth, easier future format tweaks.
  2. Inject parser-specific hooks
    • Allow callers to supply parser-specific behaviors (e.g., Copilot’s debug log handling) through options callbacks instead of duplicating the base logic.

Implementation Checklist

  • Review duplication findings
  • Prioritize refactoring tasks
  • Create refactoring plan
  • Implement changes
  • Update tests
  • Verify no functionality broken

Analysis Metadata

  • Analyzed Files: 4
  • Detection Method: Serena semantic code analysis
  • Commit: 102022d
  • Analysis Date: 2024-06-05

AI generated by Duplicate Code Detector</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Created parseLogEntries() function in log_parser_shared.cjs
- Extracted ~100 lines of duplicate parsing code from both parsers
- Updated parse_claude_log.cjs to use shared function (-47 lines)
- Updated parse_copilot_log.cjs to use shared function (-38 lines)
- Added comprehensive unit tests for parseLogEntries()
- All existing tests pass without modification

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicate JSON/JSONL parsing fallback in log parsers Refactor duplicate JSON/JSONL parsing logic into shared utility Nov 22, 2025
Copilot AI requested a review from pelikhan November 22, 2025 22:28
@pelikhan pelikhan added the smoke label Nov 22, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 22, 2025

✅ Agentic Changeset Generator completed successfully.

@pelikhan pelikhan marked this pull request as ready for review November 22, 2025 23:47
Copilot AI review requested due to automatic review settings November 22, 2025 23:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR successfully refactors duplicate JSON/JSONL parsing logic from two log parser files into a shared utility function, eliminating 85 lines of duplicate code while maintaining functionality and adding comprehensive test coverage.

Key Changes

  • Created shared utility: New parseLogEntries() function in log_parser_shared.cjs handles JSON array, JSONL, and mixed format parsing with consistent error handling
  • Refactored parsers: Both parse_claude_log.cjs and parse_copilot_log.cjs now use the shared function, reducing code duplication
  • Comprehensive testing: Added 11 unit tests covering JSON arrays, JSONL, mixed formats, edge cases, and error conditions

Reviewed changes

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

Show a summary per file
File Description
pkg/workflow/js/log_parser_shared.cjs Added parseLogEntries() function with robust JSON/JSONL parsing logic that returns null on failure
pkg/workflow/js/parse_claude_log.cjs Removed 47 lines of duplicate parsing code, replaced with shared function call
pkg/workflow/js/parse_copilot_log.cjs Removed 38 lines of duplicate parsing code, replaced with shared function call
pkg/workflow/js/log_parser_shared.test.cjs Added 11 comprehensive test cases covering all parsing scenarios
.github/workflows/*.lock.yml Bundled workflow files updated with inlined shared function across 60+ workflows

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

@pelikhan pelikhan merged commit 19fad4f into main Nov 22, 2025
135 of 136 checks passed
@pelikhan pelikhan deleted the copilot/refactor-json-parsing-fallback branch November 22, 2025 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code: JSON/JSONL Parsing Fallback in Log Parsers

3 participants