Skip to content

Fix logs command parameter naming confusion in pagination function#2049

Merged
pelikhan merged 6 commits intomainfrom
copilot/fix-logs-command-count-argument
Oct 21, 2025
Merged

Fix logs command parameter naming confusion in pagination function#2049
pelikhan merged 6 commits intomainfrom
copilot/fix-logs-command-count-argument

Conversation

Copy link
Contributor

Copilot AI commented Oct 21, 2025

Problem

The listWorkflowRunsWithPagination function had a confusing parameter name that obscured the algorithm's behavior. The parameter was named count, which suggested it represented the total number of matching runs the user wants to find (from the -c flag). However, it was actually used as the batch size for GitHub CLI API calls.

This created ambiguity between two distinct concepts:

  1. User's count (from -c flag): Total number of matching workflow runs to find
  2. API batch size: Number of runs to fetch per gh run list call

Before

func listWorkflowRunsWithPagination(workflowName string, count int, ...) {
    if count > 0 {
        args = append(args, "--limit", strconv.Itoa(count))
    }
}

When batchSize (100 or 250) was passed as the count parameter, it was unclear whether it represented the user's desired total or the API batch size.

Solution

Renamed the parameter from count to limit to clearly indicate its purpose as the API batch size:

func listWorkflowRunsWithPagination(workflowName string, limit int, ...) {
    if limit > 0 {
        args = append(args, "--limit", strconv.Itoa(limit))
    }
}

Algorithm Behavior

When a user runs ./gh-aw logs -c 10:

  1. The outer loop continues until len(processedRuns) >= 10 (the user's count)
  2. Each iteration calls listWorkflowRunsWithPagination(workflowName, batchSize, ...)
  3. The batchSize (100 or 250) becomes the limit parameter → gh run list --limit 100
  4. Downloads up to maxDownloads = count - len(processedRuns) runs from each batch
  5. Continues until enough matching runs are found or available data is exhausted

Changes

  • pkg/cli/logs.go: Renamed parameter and enhanced documentation
  • pkg/cli/logs_filtering_test.go: Added test documenting parameter semantics

This clarification makes the code more maintainable and ensures the algorithm correctly distinguishes between the user's total desired matching runs and the API batch size per request.

Original prompt

The "logs" command download algorithm has the "count" argument confused. "count" is the number of matching workflow matching to find.

However, in the download loop, the algorithm has to run gh run list multiple times to collect enough runs. In this loop, it is incorrectly using the count as the loop counter instead of the match count.

When running ./gh-aw logs -c 10, the current algorithm SHOULD find at last 5 runs currently. If not, there is a bug.


💡 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.

Copilot AI self-assigned this Oct 21, 2025
Copilot AI and others added 3 commits October 21, 2025 00:16
The listWorkflowRunsWithPagination function had a confusing parameter name 'count'
which was actually used as the batch size limit for the GitHub CLI API call.

This was confusing because:
- The user's -c flag means "total matching runs to find"
- The 'count' parameter in listWorkflowRunsWithPagination meant "batch size per API call"

Fixed by renaming the parameter to 'limit' to clarify it represents the API batch
size (passed to gh run list --limit), not the total number of matching runs.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix count argument logic in logs command Fix logs command parameter naming confusion in pagination function Oct 21, 2025
Copilot AI requested a review from pelikhan October 21, 2025 00:22
@@ -0,0 +1,76 @@
# Fix: Logs Command Count Parameter Confusion
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot delete file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted in commit 3f8e8d1

@pelikhan pelikhan marked this pull request as ready for review October 21, 2025 00:26
@github-actions
Copy link
Contributor

Agentic Changeset Generator triggered by this pull request.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan October 21, 2025 00:28
@pelikhan pelikhan merged commit 12a9958 into main Oct 21, 2025
3 checks passed
@pelikhan pelikhan deleted the copilot/fix-logs-command-count-argument branch October 21, 2025 00:30
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.

2 participants