Skip to content

Refactor: Extract duplicate comment helpers into separate modules#3565

Merged
pelikhan merged 7 commits intomainfrom
copilot/refactor-comment-output-helpers
Nov 10, 2025
Merged

Refactor: Extract duplicate comment helpers into separate modules#3565
pelikhan merged 7 commits intomainfrom
copilot/refactor-comment-output-helpers

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 10, 2025

Refactor: Extract Duplicate Comment Output Helpers

This PR addresses duplicate code in comment-related safe-output scripts by extracting shared footer generation and repository URL helper logic into separate modules following the repository's pattern.

Plan

  • Explore repository structure and understand the duplicate code
  • Verify all tests pass in current state
  • Create new shared module comment_helpers.cjs with:
    • generateFooter() function
    • getRepositoryUrl() function
  • Create comprehensive tests for the new module
  • Refactor create_issue.cjs to use shared helpers
  • Refactor add_comment.cjs to use shared helpers
  • Refactor create_pr_review_comment.cjs to use shared helpers
  • Run tests to ensure no regressions
  • Format and lint all modified files
  • Split helpers into separate files per review feedback
  • Revert bundler fix (already in main)
  • Merge main branch
  • Recompile all workflows

Changes Made

Split into separate modules following repository pattern:

  • pkg/workflow/js/generate_footer.cjs - Footer generation helper (44 lines)
  • pkg/workflow/js/generate_footer.test.cjs - Test suite (11 test cases)
  • pkg/workflow/js/get_repository_url.cjs - Repository URL helper (27 lines)
  • pkg/workflow/js/get_repository_url.test.cjs - Test suite (6 test cases)

Modified Files:

  • pkg/workflow/js/create_issue.cjs - Updated import
  • pkg/workflow/js/add_comment.cjs - Updated imports
  • pkg/workflow/js/create_pr_review_comment.cjs - Updated imports
  • pkg/workflow/js.go - Added both modules to embedded sources
  • 26 workflow .lock.yml files - Recompiled with bundled helpers

Total Impact:

  • Code removed: ~154 lines of duplicate code
  • Code added: 71 lines of helpers (split into 2 files) + 229 lines of tests
  • Net reduction: ~83 lines (excluding tests)

Benefits

✅ Single point of truth for footer generation and repository URL logic
✅ Consistent messaging across all comment types (issues, PRs, discussions)
✅ Easier maintenance and future updates
✅ Follows repository pattern (one function per file)
✅ Comprehensive test coverage (17 total test cases split across 2 files)
✅ Valid bundled JavaScript (bundler fix from main handles multi-line exports)
✅ All workflows recompiled with updated helpers
✅ No functionality broken - all tests pass

Test Results

✓ generate_footer.test.cjs (11 tests)
✓ get_repository_url.test.cjs (6 tests)
✓ create_issue.test.cjs (22 tests)
✓ add_comment.test.cjs (16 tests)
✓ create_pr_review_comment.test.cjs (16 tests)

Total: 780 JavaScript tests pass + Go unit tests pass

Merged from main

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] 🔍 Duplicate Code Detected: Comment Output Helpers</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Comment Output Helpers

Analysis of commit 7292890

Assignee: @copilot

Summary

Multiple comment-related safe-output scripts reimplement the same footer generation and repository URL helper logic. Keeping three divergent copies makes it easy to miss fixes (for example when updating footer text or URL logic) and inflates maintenance cost.

Duplication Details

Pattern: Repeated footer/repository helper functions across comment scripts

  • Severity: Medium
  • Occurrences: 5 helper copies across 3 files
  • Locations:
    • pkg/workflow/js/create_issue.cjs:19
    • pkg/workflow/js/add_comment.cjs:17
    • pkg/workflow/js/create_pr_review_comment.cjs:18
    • pkg/workflow/js/create_pr_review_comment.cjs:51
    • pkg/workflow/js/add_comment.cjs:142
  • Code Sample:
    function generateFooter(
      workflowName,
      runUrl,
      workflowSource,
      workflowSourceURL,
      triggeringIssueNumber,
      triggeringPRNumber,
      triggeringDiscussionNumber
    ) {
      let footer = `\n\n> AI generated by [${workflowName}](${runUrl})`;
      if (triggeringIssueNumber) {
        footer += ` for #${triggeringIssueNumber}`;
      } else if (triggeringPRNumber) {
        footer += ` for #${triggeringPRNumber}`;
      } else if (triggeringDiscussionNumber) {
        footer += ` for discussion #${triggeringDiscussionNumber}`;
      }
      if (workflowSource && workflowSourceURL) {
        footer += `\n>\n> To add this workflow in your repository, run \`gh aw add ${workflowSource}\``;
      }
      footer += "\n";
      return footer;
    }

Impact Analysis

  • Maintainability: Fixes to wording, attribution, or safe-output links must be applied in three places. A missed update will silently produce inconsistent comments.
  • Bug Risk: Future behavior changes (e.g., adding more context links) risk diverging between issue comments, PR review comments, and general comments.
  • Code Bloat: Each file carries ~40 extra lines of duplicated helpers, obscuring the core logic.

Refactoring Recommendations

  1. Extract shared helpers

    • Move footer generation and repository URL computation into a new shared module (e.g., pkg/workflow/js/comment_helpers.cjs).
    • Estimated effort: 2-3 hours to introduce the module, add exports, and update imports/tests.
    • Benefits: Single point of truth for messaging and URL logic, easier future evolution.
  2. Add coverage to avoid regressions

    • Extend existing unit tests to cover the new helper module and confirm each consumer uses it.
    • Estimated effort: 1 hour for focused tests around footer content and URL selection.

Implementation Checklist

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

Analysis Metadata

  • Analyzed Files: 3 primary scripts (additional utilities inspected for comparison)
  • Detection Method: Serena semantic code analysis (symbol + pattern search)
  • Commit: 7292890
  • Analysis Date: 2025-11-09

AI generated by Duplicate Code Detector</issue_description>

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


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Create new comment_helpers.cjs with generateFooter() and getRepositoryUrl()
- Add comprehensive tests for the new module (17 test cases)
- Refactor create_issue.cjs to use shared helpers
- Refactor add_comment.cjs to use shared helpers
- Refactor create_pr_review_comment.cjs to use shared helpers
- Add comment_helpers.cjs to Go embedded sources
- All tests pass (780 JavaScript tests + Go unit tests)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicate code in comment output helpers Refactor: Extract duplicate comment helpers into shared module Nov 10, 2025
Copilot AI requested a review from pelikhan November 10, 2025 14:12
Comment thread pkg/workflow/js/comment_helpers.cjs Outdated
* This helper handles trial mode where target repository URLs are different from execution context
* @returns {string} Repository URL
*/
function getRepositoryUrl() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot split helpers in different files

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Split into separate files following the repository pattern:

  • generate_footer.cjs - Footer generation helper
  • get_repository_url.cjs - Repository URL helper

Each with their own test files. All 780 JavaScript tests + Go unit tests pass.

Commit: da12d73

- Split comment_helpers.cjs into generate_footer.cjs and get_repository_url.cjs
- Create separate test files for each helper
- Update all consumers to import from separate modules
- Update Go embedded sources to register both new modules
- All tests pass (780 JavaScript tests + Go unit tests)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Refactor: Extract duplicate comment helpers into shared module Refactor: Extract duplicate comment helpers into separate modules Nov 10, 2025
Copilot AI requested a review from pelikhan November 10, 2025 14:23
Comment thread .github/workflows/ci-doctor.lock.yml Outdated
return `${githubServer}/${context.repo.owner}/${context.repo.repo}`;
}
}
generateFooter,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot looks like invalid generated JavaScript , check bundler code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Changed module.exports to single-line format to match the repository pattern (like sanitize_content.cjs). The bundler now correctly removes the entire export statement instead of leaving dangling fragments.

Before (multi-line):

module.exports = {
  generateFooter,
};

After (single-line):

module.exports = { generateFooter };

All 780 JavaScript tests + Go tests pass. Lock files now contain valid JavaScript.

Commit: 9eae7b7

- Change module.exports from multi-line to single-line format
- Fixes invalid JavaScript generation in bundled lock files
- Follows repository pattern used by other helper modules
- All 780 JavaScript tests + Go unit tests pass

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot revert previous commit, merge main

Copilot AI and others added 2 commits November 10, 2025 15:12
This reverts commit 9eae7b7. The bundler fix is already included in main branch.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Nov 10, 2025

@copilot revert previous commit, merge main

Reverted commit 9eae7b7 and merged main branch. The bundler fix from main (#3566) now handles multi-line module.exports correctly. All 780 JavaScript tests + Go unit tests pass.

Commits:

Copy link
Copy Markdown
Collaborator

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

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

@copilot recompile all

- Regenerate all 26 lock files with bundled helper functions
- generateFooter and getRepositoryUrl properly inlined
- All 780 JavaScript tests pass

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Nov 10, 2025

@copilot recompile all

Recompiled all 26 workflows. Helper functions generateFooter and getRepositoryUrl are properly bundled into lock files. All 780 JavaScript tests pass.

Commit: a7306aa

Copilot AI requested a review from pelikhan November 10, 2025 16:06
@pelikhan pelikhan marked this pull request as ready for review November 10, 2025 20:50
Copilot AI review requested due to automatic review settings November 10, 2025 20:50
@pelikhan pelikhan merged commit 15fc225 into main Nov 10, 2025
44 checks passed
@pelikhan pelikhan deleted the copilot/refactor-comment-output-helpers branch November 10, 2025 20:52
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 comment helper code by extracting shared footer generation and repository URL logic into separate, reusable modules. The refactoring eliminates ~154 lines of duplicate code across three safe-output scripts while maintaining full test coverage and following the repository's established patterns.

Key Changes:

  • Creates two focused helper modules (generate_footer.cjs and get_repository_url.cjs) with comprehensive test suites
  • Updates three consumer scripts to use the new shared helpers via require()
  • Properly registers both modules in Go's embedded JavaScript sources
  • Successfully recompiles all 26 workflow lock files with correctly bundled helper functions

Reviewed Changes

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

Show a summary per file
File Description
pkg/workflow/js/generate_footer.cjs New helper module for AI attribution footer generation with workflow context
pkg/workflow/js/generate_footer.test.cjs Comprehensive test suite with 11 test cases covering all footer variations
pkg/workflow/js/get_repository_url.cjs New helper module handling repository URL resolution in trial and normal modes
pkg/workflow/js/get_repository_url.test.cjs Test suite with 6 test cases covering all URL resolution scenarios
pkg/workflow/js/create_issue.cjs Refactored to import and use generateFooter helper
pkg/workflow/js/add_comment.cjs Refactored to import both generateFooter and getRepositoryUrl helpers
pkg/workflow/js/create_pr_review_comment.cjs Refactored to import both helpers for PR review comment creation
pkg/workflow/js.go Added embed directives and registration for both new helper modules
.github/workflows/*.lock.yml (26 files) Recompiled workflows with correctly inlined helper functions via bundler

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] 🔍 Duplicate Code Detected: Comment Output Helpers

3 participants