-
Notifications
You must be signed in to change notification settings - Fork 355
feat: add --github-order flag to control issue execution order #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
edf4ef9
e7686d1
76d062e
b1e5a2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
idorozin marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,9 +25,10 @@ export class GitHubTaskSource implements TaskSource { | |
| private owner: string; | ||
| private repo: string; | ||
| private label?: string; | ||
| private order?: number[]; | ||
| private cache: GitHubCache | null = null; | ||
|
|
||
| constructor(repoPath: string, label?: string) { | ||
| constructor(repoPath: string, label?: string, order?: number[]) { | ||
| // Parse owner/repo format | ||
| const [owner, repo] = repoPath.split("/"); | ||
| if (!owner || !repo) { | ||
|
|
@@ -37,6 +38,7 @@ export class GitHubTaskSource implements TaskSource { | |
| this.owner = owner; | ||
| this.repo = repo; | ||
| this.label = label; | ||
| this.order = order; | ||
|
|
||
| // Use GITHUB_TOKEN from environment | ||
| this.octokit = new Octokit({ | ||
|
|
@@ -75,13 +77,35 @@ export class GitHubTaskSource implements TaskSource { | |
| per_page: 100, | ||
| }); | ||
|
|
||
| const tasks = issues.map((issue) => ({ | ||
| let tasks = issues.map((issue) => ({ | ||
| id: `${issue.number}:${issue.title}`, | ||
| title: issue.title, | ||
| body: issue.body || undefined, | ||
| completed: false, | ||
| })); | ||
|
|
||
| // Sort by specified order if provided | ||
| if (this.order && this.order.length > 0) { | ||
| const orderMap = new Map(this.order.map((num, idx) => [num, idx])); | ||
| const tagged = tasks.map((t) => ({ | ||
| task: t, | ||
| issueNum: Number.parseInt(t.id.split(":")[0], 10), | ||
| })); | ||
| const filtered = tagged.filter(({ issueNum }) => orderMap.has(issueNum)); | ||
|
|
||
| const foundNums = new Set(filtered.map(({ issueNum }) => issueNum)); | ||
| const missing = this.order.filter((n) => !foundNums.has(n)); | ||
| if (missing.length > 0) { | ||
| console.warn( | ||
| `[ralphy] Warning: --github-order specified issues not found among open issues: ${missing.join(", ")}`, | ||
| ); | ||
| } | ||
|
Comment on lines
+96
to
+102
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The missing-issue warning is emitted on every cache refresh, not just the first time. During normal operation, once issue This means a user running Consider filtering // Only warn on the initial fetch, not on subsequent refreshes after issues are closed
const isFirstFetch = !this.cache;
const foundNums = new Set(filtered.map(({ issueNum }) => issueNum));
const missing = this.order.filter((n) => !foundNums.has(n));
if (isFirstFetch && missing.length > 0) {
console.warn(
`[ralphy] Warning: --github-order specified issues not found among open issues: ${missing.join(", ")}`,
);
}Prompt To Fix With AIThis is a comment left during a code review.
Path: cli/src/tasks/github.ts
Line: 96-102
Comment:
**Warning fires as false positive after issues are closed**
The missing-issue warning is emitted on every cache refresh, not just the first time. During normal operation, once issue `#2` is processed and closed via `markComplete()`, the cache is invalidated. The next call to `fetchOpenIssues()` fetches only `state: "open"` issues, so issue `#2` is no longer in the results. The check on line 97 then flags it as missing and emits the warning — even though it was just successfully completed.
This means a user running `--github-order 2,3,4,5` will see "Warning: issues not found among open issues: 2" after processing the very first issue, then "...2, 3" after the second, and so on. This is confusing and may cause users to think something went wrong when the workflow is proceeding correctly.
Consider filtering `this.order` to exclude issues that have already been closed before computing `missing`. You can cross-reference with the closed-count query, or simply only warn on the first fetch (e.g. when `this.cache` is null):
```ts
// Only warn on the initial fetch, not on subsequent refreshes after issues are closed
const isFirstFetch = !this.cache;
const foundNums = new Set(filtered.map(({ issueNum }) => issueNum));
const missing = this.order.filter((n) => !foundNums.has(n));
if (isFirstFetch && missing.length > 0) {
console.warn(
`[ralphy] Warning: --github-order specified issues not found among open issues: ${missing.join(", ")}`,
);
}
```
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| tasks = filtered | ||
| .sort((a, b) => orderMap.get(a.issueNum)! - orderMap.get(b.issueNum)!) | ||
| .map(({ task }) => task); | ||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
|
|
||
| // Update cache (preserve closed count if we have it) | ||
| this.cache = { | ||
| openIssues: tasks, | ||
|
|
@@ -139,7 +163,12 @@ export class GitHubTaskSource implements TaskSource { | |
| per_page: 100, | ||
| }); | ||
|
|
||
| const closedCount = issues.length; | ||
| // When order is specified, only count closed issues that are in the order list | ||
| let closedCount = issues.length; | ||
| if (this.order && this.order.length > 0) { | ||
| const orderSet = new Set(this.order); | ||
| closedCount = issues.filter((issue) => orderSet.has(issue.number)).length; | ||
| } | ||
|
|
||
| // Update cache with closed count | ||
| if (this.cache) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.