-
Notifications
You must be signed in to change notification settings - Fork 48
Closed
Description
🔍 Duplicate Code Detected: Issue vs PR Update Scripts
Analysis of commit fe3a592
Assignee: @copilot
Summary
pkg/workflow/js/update_issue.cjs and pkg/workflow/js/update_pull_request.cjs both implement the same staged preview + update plumbing for agentic "update" actions. Each file redefines the same helper functions (context validator, number extractor, staged preview renderer, summary builder, and the runUpdateWorkflow invocation) even though the logic only differs in a few literals and the body operation handling. The shared structure exceeds 60 lines of nearly identical code per file, creating an avoidable maintenance hotspot.
Duplication Details
Pattern: Repeated staged-update scaffolding
- Severity: Medium
- Occurrences: 2
- Locations:
pkg/workflow/js/update_issue.cjs:25-100pkg/workflow/js/update_pull_request.cjs:41-153
- Code Sample:
The same duplication repeats for
// update_issue.cjs async function main() { return await runUpdateWorkflow({ itemType: "update_issue", displayName: "issue", displayNamePlural: "issues", numberField: "issue_number", outputNumberKey: "issue_number", outputUrlKey: "issue_url", isValidContext: isIssueContext, getContextNumber: getIssueNumber, supportsStatus: true, supportsOperation: false, renderStagedItem, executeUpdate: executeIssueUpdate, getSummaryLine, }); } // update_pull_request.cjs async function main() { return await runUpdateWorkflow({ itemType: "update_pull_request", displayName: "pull request", displayNamePlural: "pull requests", numberField: "pull_request_number", outputNumberKey: "pull_request_number", outputUrlKey: "pull_request_url", isValidContext: isPRContext, getContextNumber: getPRNumber, supportsStatus: false, supportsOperation: true, renderStagedItem, executeUpdate: executePRUpdate, getSummaryLine, }); }
renderStagedItem(lines 25-49 vs. 41-64) andgetSummaryLine(73-80 vs. 126-133), differing only in entity labels.
Impact Analysis
- Maintainability: Any change to the staged update flow or logging must be manually mirrored in both files, increasing the chance of drift (e.g., issues support status updates but PRs do not, so future features will be harder to roll out consistently).
- Bug Risk: Fixes to context detection or summary formatting could land in one script but not the other, leading to inconsistent behavior for workflows that expect parity between issues and PRs.
- Code Bloat: Each script reimplements ~60 lines of boilerplate even though
runUpdateWorkflowalready abstracts most of the behavior; the duplicated helpers add noise to the JS directory.
Refactoring Recommendations
-
Extract shared config for update scripts
- Create a single factory (e.g.,
createUpdateScript(config)) that accepts entity-specific settings (itemType, context predicates, staging renderer overrides) and returns the module entrypoint. Both update scripts can then be thin wrappers supplying their config objects. - Estimated effort: 4-6 hours.
- Benefits: One place to change preview rendering, summary formatting, and context validation.
- Create a single factory (e.g.,
-
Move shared render/summary helpers into
update_runner.cjsor a new utility- Provide default implementations for
renderStagedItem/getSummaryLinethat accept the entity label and field keys, so only PR-specific body handling needs a custom override. - Estimated effort: 2-3 hours.
- Benefits: Reduces drift risk and keeps future update targets (e.g., discussions) trivial to add.
- Provide default implementations for
Implementation Checklist
- Review duplication findings
- Prioritize refactoring tasks
- Create refactoring plan
- Implement changes
- Update tests
- Verify no functionality broken
Analysis Metadata
- Analyzed Files: 2
- Detection Method: Serena semantic code analysis
- Commit: fe3a592
- Analysis Date: 2024-??-??
AI generated by Duplicate Code Detector
Reactions are currently unavailable