Skip to content

refactor(workflow): semantic function clustering — dedup, split, rename#21277

Merged
pelikhan merged 1 commit intomainfrom
copilot/refactor-semantic-function-clustering-0f03c7bf-31c9-480a-8dbe-bd2476416676
Mar 16, 2026
Merged

refactor(workflow): semantic function clustering — dedup, split, rename#21277
pelikhan merged 1 commit intomainfrom
copilot/refactor-semantic-function-clustering-0f03c7bf-31c9-480a-8dbe-bd2476416676

Conversation

Copy link
Contributor

Copilot AI commented Mar 16, 2026

Four findings from a semantic analysis of pkg/workflow — one functional duplicate, one tombstone file, one multi-concern file exceeding the 300-line guideline, and two naming inconsistencies.

Changes

  • Deduplicate conversion helperssafeUintToInt in frontmatter_extraction_metadata.go now delegates to safeUint64ToInt in map_helpers.go via return safeUint64ToInt(uint64(u)), removing identical overflow logic. Unused math import removed.

    // Before: identical bodies in two files
    func safeUintToInt(u uint) int {
        if u > math.MaxInt { return 0 }
        return int(u)
    }
    
    // After: single source of truth
    func safeUintToInt(u uint) int { return safeUint64ToInt(uint64(u)) }
  • Delete tombstonesafe_outputs_generation.go was 7 lines of only a comment cataloguing where code moved; deleted.

  • Split safe_outputs_jobs.go (694 lines, 3 logger namespaces) into three focused files:

    • safe_outputs_jobs.goSafeOutputJobConfig + buildSafeOutputJob
    • safe_outputs_steps.goGitHubScriptStepConfig, buildCustomActionStep, buildGitHubScriptStep, buildGitHubScriptStepWithoutDownload, buildAgentOutputDownloadSteps
    • safe_outputs_env.go — all env-var assembly functions
  • Normalize file naming — rename singular-prefix outliers to match the safe_outputs_ convention used by all other files in the package:

    • safe_output_parser.gosafe_outputs_parser.go
    • safe_output_validation_config.gosafe_outputs_validation_config.go
    • Logger namespaces updated to match (workflow:safe_outputs_parser, workflow:safe_outputs_validation_config)

- Consolidate safeUintToInt to delegate to safeUint64ToInt
- Delete tombstone file safe_outputs_generation.go
- Split safe_outputs_jobs.go into jobs/steps/env files
- Rename safe_output_parser.go -> safe_outputs_parser.go
- Rename safe_output_validation_config.go -> safe_outputs_validation_config.go
- Update logger namespaces to match renamed files

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
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

Refactors pkg/workflow safe-outputs code to reduce duplication, remove a tombstone file, and split a large multi-concern module into focused files while normalizing logger namespaces and helper usage.

Changes:

  • Deduplicates safeUintToInt by delegating to safeUint64ToInt and removes the unused math import.
  • Splits step-building and env-var assembly logic out of safe_outputs_jobs.go into new focused modules.
  • Normalizes safe-outputs logger namespaces and removes a tombstone file.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/workflow/safe_outputs_validation_config.go Updates logger namespace to the normalized safe_outputs_ prefix.
pkg/workflow/safe_outputs_steps.go New module containing safe-output step builder helpers previously in safe_outputs_jobs.go.
pkg/workflow/safe_outputs_parser.go Updates logger namespace to the normalized safe_outputs_ prefix.
pkg/workflow/safe_outputs_jobs.go Slims down to job config + job builder; removes step/env helper implementations.
pkg/workflow/safe_outputs_generation.go Deletes tombstone file that only contained a relocation comment.
pkg/workflow/safe_outputs_env.go New module containing safe-output env-var assembly helpers previously in safe_outputs_jobs.go.
pkg/workflow/frontmatter_extraction_metadata.go Deduplicates safeUintToInt implementation and removes unused import.
Comments suppressed due to low confidence (2)

pkg/workflow/safe_outputs_parser.go:5

  • safeOutputParserLog now emits the plural namespace workflow:safe_outputs_parser, but the variable name is still singular (safeOutput...). This is inconsistent with the rest of the safe-outputs loggers (e.g., safeOutputsJobsLog, safeOutputsStepsLog) and makes grepping harder. Rename the logger variable to safeOutputsParserLog (and update its uses in this file) to match the normalized naming.
    pkg/workflow/safe_outputs_validation_config.go:9
  • safeOutputValidationLog now uses the plural namespace workflow:safe_outputs_validation_config, but the variable name remains singular. To keep naming consistent with the rest of the safe-outputs code (e.g., safeOutputsJobsLog, safeOutputsEnvLog), rename this variable to safeOutputsValidationLog and update references in this file.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +75 to +79
// Get safe-outputs level token
var safeOutputsToken string
if data.SafeOutputs != nil {
safeOutputsToken = data.SafeOutputs.GitHubToken
}
@pelikhan pelikhan merged commit c1f907c into main Mar 16, 2026
92 checks passed
@pelikhan pelikhan deleted the copilot/refactor-semantic-function-clustering-0f03c7bf-31c9-480a-8dbe-bd2476416676 branch March 16, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic Function Clustering: Duplicate conversions, multi-concern file, naming inconsistencies in pkg/workflow

3 participants