Skip to content

[file-diet] File Diet: Refactor pkg/cli/experiments_command.go (1120 lines) #53696

Description

@github-actions

Overview

The file pkg/cli/experiments_command.go has grown to 1120 lines, making it difficult to maintain and test. This task involves refactoring it into smaller, focused files with improved test coverage.

Current State

  • File: pkg/cli/experiments_command.go
  • Size: 1120 lines
  • Test Coverage: pkg/cli/experiments_command_test.go is 539 lines (~48% test-to-source ratio)
  • Complexity: Contains ~40 top-level functions spanning several distinct concerns: CLI command wiring, list/analyze orchestration, remote & local data fetching (git branches, GitHub API), JSONL state parsing, git-ref safety validation, and console rendering.
Full File Analysis

Detailed Breakdown

Functions and their approximate responsibilities:

  • Cobra command wiring (lines 94–195): NewExperimentsCommand, NewExperimentsListSubcommand, NewExperimentsAnalyzeSubcommand
  • Command orchestration (lines 195–330): RunExperimentsList, RunExperimentsAnalyze, computeExperimentAnalyses
  • Metric eval loading/summarizing (lines 331–411): loadLocalMetricEvalResults, loadRemoteMetricEvalResults, summarizeMetricEvalResults
  • Experiment config discovery (lines 412–612): loadLocalExperimentConfigs, loadRemoteExperimentConfigs, findRemoteWorkflowFilenameForExperiment, matchWorkflowFilenameByExperiment, findWorkflowFileForExperiment, workflowFileCandidates
  • Experiment listing/details fetch (local & remote) (lines 613–762): fetchLocalExperiments, fetchRemoteExperiments, fetchLocalExperimentDetails, fetchRemoteExperimentDetails, experimentStateFilenames, readLocalExperimentState
  • Git ref/path safety validation (lines 762–851): buildSafeGitShowObjectArg, isSafeExperimentStateRef, isHexObjectIDPrefix, isSafeGitRefName, isSafeGitTreePath
  • Experiment state parsing & aggregation (lines 851–1023): readRemoteExperimentState, appendExperimentRun, parseExperimentStateJSONL, parseExperimentState, emptyExperimentState, experimentInfoFromState, experimentDetailsFromState, experimentTotalRuns, experimentLastRun, extractExperimentName, gitRefExists
  • Rendering (lines 1024–1106): printExperimentDetails, formatAssignments
  • Generic helper (lines 1106+): parsePagedJSONArray[T any]

Notable coupling/complexity:

  • Local vs. remote variants of many functions (fetchLocal*/fetchRemote*, loadLocal*/loadRemote*) duplicate structural logic while differing only in the data source (local git state vs. remote GitHub API), a strong signal for extracting a shared interface/abstraction.
  • The git-ref/path safety validators (isSafe*) are a self-contained security-sensitive module that would benefit from isolation and dedicated, exhaustive testing.
  • JSONL/state parsing functions are independent of CLI/cobra concerns and could be moved to a state-only file.

Refactoring Strategy

Proposed File Splits

  1. experiments_command.go (kept, slimmed down)

    • Functions: NewExperimentsCommand, NewExperimentsListSubcommand, NewExperimentsAnalyzeSubcommand, RunExperimentsList, RunExperimentsAnalyze, computeExperimentAnalyses
    • Responsibility: Cobra command definitions and top-level orchestration only
    • Estimated LOC: ~250
  2. experiments_fetch.go

    • Functions: fetchLocalExperiments, fetchRemoteExperiments, fetchLocalExperimentDetails, fetchRemoteExperimentDetails, loadLocalExperimentConfigs, loadRemoteExperimentConfigs, findRemoteWorkflowFilenameForExperiment, matchWorkflowFilenameByExperiment, findWorkflowFileForExperiment, workflowFileCandidates, loadLocalMetricEvalResults, loadRemoteMetricEvalResults, summarizeMetricEvalResults
    • Responsibility: Discovering and loading experiment configs/metrics from local git state and remote GitHub API
    • Estimated LOC: ~420
  3. experiments_state.go

    • Functions: experimentStateFilenames, readLocalExperimentState, readRemoteExperimentState, appendExperimentRun, parseExperimentStateJSONL, parseExperimentState, emptyExperimentState, experimentInfoFromState, experimentDetailsFromState, experimentTotalRuns, experimentLastRun, extractExperimentName, gitRefExists
    • Responsibility: Parsing and aggregating experiment state (JSONL records → structured results), independent of CLI wiring
    • Estimated LOC: ~270
  4. experiments_git_safety.go

    • Functions: buildSafeGitShowObjectArg, isSafeExperimentStateRef, isHexObjectIDPrefix, isSafeGitRefName, isSafeGitTreePath
    • Responsibility: Security-sensitive validation of git refs/paths used when reading experiment state via git show
    • Estimated LOC: ~90
  5. experiments_render.go

    • Functions: printExperimentDetails, formatAssignments
    • Responsibility: Console rendering of experiment details
    • Estimated LOC: ~90

Shared Utilities

  • experiments_json_utils.go: Extract parsePagedJSONArray[T any] here since it's a generic helper usable by any paged-JSON-array consumer beyond experiments (or move to an existing generic utils file if one already fits, e.g. a shared json_utils.go).

Interface Abstractions

  • Introduce a small experimentDataSource interface (e.g., FetchExperiments() ([]ExperimentInfo, error), FetchExperimentDetails(id string) (*ExperimentDetails, error)) with localExperimentDataSource and remoteExperimentDataSource implementations. This removes the pervasive Local/Remote function-name duplication and lets RunExperimentsList/RunExperimentsAnalyze select an implementation based on repoOverride instead of branching per call site.
Test Coverage Plan
  1. experiments_fetch_test.go

    • Test cases: local config discovery with matching/mismatched workflow IDs, remote fetch fallback paths, metric eval summarization with malformed/empty JSON, workflow filename matching heuristics
    • Target coverage: >80%
  2. experiments_state_test.go

    • Test cases: JSONL parsing with valid/partial/corrupt lines, state aggregation across multiple runs, empty-state defaults, extractExperimentName edge cases (missing prefix, unusual ref formats)
    • Target coverage: >80%
  3. experiments_git_safety_test.go

    • Test cases: valid/invalid git ref names, path traversal attempts in tree paths, hex object ID prefix boundary cases, injection attempts in buildSafeGitShowObjectArg
    • Target coverage: >90% (security-sensitive)
  4. experiments_render_test.go

    • Test cases: formatting assignments with empty/singleton/multiple entries, printing details with nil/partial fields
    • Target coverage: >80%

Implementation Guidelines

  1. Preserve Behavior: Ensure all existing functionality works identically
  2. Maintain Exports: Keep public API unchanged (exported functions/types)
  3. Add Tests First: Write tests for each new file before refactoring
  4. Incremental Changes: Split one module at a time
  5. Run Tests Frequently: Verify make test-unit passes after each split
  6. Update Imports: Ensure all import paths are correct
  7. Document Changes: Add comments explaining module boundaries

Acceptance Criteria

  • Original file is split into 5 focused files
  • Each new file is under 500 lines
  • All tests pass (make test-unit)
  • Test coverage is ≥80% for new files
  • No breaking changes to public API
  • Code passes linting (make lint)
  • Build succeeds (make build)
Additional Context
  • Repository Guidelines: Follow patterns in .github/agents/developer.instructions.agent.md
  • Code Organization: Prefer many small files grouped by functionality
  • Testing: Match existing test patterns in pkg/cli/*_test.go

Priority: Medium
Effort: Medium (5 new files, existing test coverage at ~48% needs to grow alongside the split)
Expected Impact: Improved maintainability, easier testing, reduced complexity

Generated by 🧹 Daily File Diet · auto · 89.6 AIC · ⌖ 11.3 AIC · ⊞ 10.1K ·

  • expires on Aug 20, 2026, 5:06 AM UTC-08:00

Activity

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

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions