-
Notifications
You must be signed in to change notification settings - Fork 355
feat: add single-task repeat mode and code cleanup #166
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 10 commits
a063b27
837bf8f
39a8303
e984745
05ade13
45bff6d
a0bf467
e6a3e22
a5f2cdd
71f5ca2
1bc3373
a20b9c5
f387ef0
44f6a83
baf3f56
4e05f85
f7474df
76a326d
a77835b
66ceb9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Example Project PRD | ||
|
|
||
| Ein einfaches Beispiel-Projekt, um Ralphy zu demonstrieren. | ||
|
|
||
| ## Kontext | ||
|
|
||
| Wir bauen eine kleine CLI-Anwendung, die Markdown-Dateien in HTML konvertiert. | ||
|
|
||
| ## Tasks | ||
|
|
||
| - [ ] Projekt-Setup: package.json erstellen mit TypeScript und Vitest | ||
| - [ ] Funktion schreiben, die Markdown-Headings (#, ##, ###) in HTML-Tags konvertiert | ||
| - [ ] Funktion schreiben, die **bold** und *italic* Text konvertiert | ||
| - [ ] Funktion schreiben, die Listen (- item) in HTML-Listen konvertiert | ||
| - [ ] CLI-Entry-Point erstellen, der eine Datei einliest und konvertiert | ||
| - [ ] README.md mit Nutzungsanleitung schreiben | ||
|
|
||
| ## Akzeptanzkriterien | ||
|
|
||
| - Alle Tests gruen | ||
| - TypeScript kompiliert ohne Fehler | ||
| - CLI kann mit `npx ts-node src/index.ts input.md` ausgefuehrt werden | ||
|
|
||
| ## Notizen | ||
|
|
||
| - Keine externen Markdown-Libraries verwenden (Lernzweck) | ||
| - Einfache Regex-basierte Konvertierung reicht aus |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| import { beforeAll, describe, expect, it, mock } from "bun:test"; | ||
|
|
||
| let parseArgs: typeof import("../args.ts").parseArgs; | ||
|
|
||
| beforeAll(async () => { | ||
| mock.module("../../version.ts", () => ({ | ||
| VERSION: "test", | ||
| })); | ||
| ({ parseArgs } = await import("../args.ts")); | ||
| }); | ||
|
|
||
| function parseCliArgs(args: string[]) { | ||
| return parseArgs(["bun", "ralphy", ...args]); | ||
| } | ||
|
|
||
| describe("parseArgs repeat options", () => { | ||
| it("parses --repeat 5 with task", () => { | ||
| const { options, task } = parseCliArgs(["--repeat", "5", "do something"]); | ||
| expect(task).toBe("do something"); | ||
| expect(options.repeatCount).toBe(5); | ||
| expect(options.continueOnFailure).toBe(false); | ||
| }); | ||
|
|
||
| it("throws on --repeat 0", () => { | ||
| expect(() => parseCliArgs(["--repeat", "0", "task"])).toThrow( | ||
| "--repeat must be an integer between 1 and 10000", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws on --repeat -1", () => { | ||
| expect(() => parseCliArgs(["--repeat", "-1", "task"])).toThrow( | ||
| "--repeat must be an integer between 1 and 10000", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws on --repeat abc", () => { | ||
| expect(() => parseCliArgs(["--repeat", "abc", "task"])).toThrow( | ||
| "--repeat must be an integer between 1 and 10000", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws on --repeat 1.5", () => { | ||
| expect(() => parseCliArgs(["--repeat", "1.5", "task"])).toThrow( | ||
| "--repeat must be an integer between 1 and 10000", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws on --repeat 10001", () => { | ||
| expect(() => parseCliArgs(["--repeat", "10001", "task"])).toThrow( | ||
| "--repeat must be an integer between 1 and 10000", | ||
| ); | ||
| }); | ||
|
|
||
| it("parses --repeat with --continue-on-failure", () => { | ||
| const { options } = parseCliArgs(["--repeat", "3", "--continue-on-failure", "task"]); | ||
| expect(options.repeatCount).toBe(3); | ||
| expect(options.continueOnFailure).toBe(true); | ||
| }); | ||
|
|
||
| it("throws when --repeat is used without task", () => { | ||
| expect(() => parseCliArgs(["--repeat", "3"])).toThrow( | ||
| "--repeat and --continue-on-failure require a task argument", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws when --continue-on-failure is used without task", () => { | ||
| expect(() => parseCliArgs(["--continue-on-failure"])).toThrow( | ||
| "--repeat and --continue-on-failure require a task argument", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws when repeat options are combined with task source flags", () => { | ||
| expect(() => parseCliArgs(["--repeat", "3", "--yaml", "tasks.yaml", "task"])).toThrow( | ||
| "--repeat and --continue-on-failure cannot be used with --prd, --yaml, --json, or --github", | ||
| ); | ||
| }); | ||
|
|
||
| it("defaults to repeatCount 1", () => { | ||
| const { options } = parseCliArgs(["task"]); | ||
| expect(options.repeatCount).toBe(1); | ||
| expect(options.continueOnFailure).toBe(false); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { describe, expect, it } from "bun:test"; | ||
| import { DEFAULT_OPTIONS } from "../../config/types.ts"; | ||
| import { runSingleTaskLoop } from "./single-task-loop.ts"; | ||
|
|
||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| describe("runSingleTaskLoop", () => { | ||
| it("stops on first non-fatal failure in fail-fast mode", async () => { | ||
| let calls = 0; | ||
| const result = await runSingleTaskLoop( | ||
| "task", | ||
| { | ||
| ...DEFAULT_OPTIONS, | ||
| repeatCount: 3, | ||
| continueOnFailure: false, | ||
| }, | ||
| { | ||
| runTaskFn: async () => { | ||
| calls++; | ||
| return { success: false, fatal: false, error: "boom" }; | ||
| }, | ||
| logInfoFn: () => {}, | ||
| }, | ||
| ); | ||
|
|
||
| expect(calls).toBe(1); | ||
| expect(result.completed).toBe(0); | ||
| expect(result.failed).toBe(1); | ||
| expect(result.total).toBe(3); | ||
| }); | ||
|
|
||
| it("continues on non-fatal failures when continue-on-failure is enabled", async () => { | ||
| let call = 0; | ||
| const sequence = [ | ||
| { success: false, fatal: false, error: "first" }, | ||
| { success: true, fatal: false }, | ||
| { success: false, fatal: false, error: "last" }, | ||
| ] as const; | ||
|
|
||
| const result = await runSingleTaskLoop( | ||
| "task", | ||
| { | ||
| ...DEFAULT_OPTIONS, | ||
| repeatCount: 3, | ||
| continueOnFailure: true, | ||
| }, | ||
| { | ||
| runTaskFn: async () => sequence[call++] ?? sequence[sequence.length - 1], | ||
| logInfoFn: () => {}, | ||
| }, | ||
| ); | ||
|
|
||
| expect(call).toBe(3); | ||
| expect(result.completed).toBe(1); | ||
| expect(result.failed).toBe(2); | ||
| expect(result.total).toBe(3); | ||
| }); | ||
|
|
||
| it("always stops on fatal failures", async () => { | ||
| let calls = 0; | ||
| const result = await runSingleTaskLoop( | ||
| "task", | ||
| { | ||
| ...DEFAULT_OPTIONS, | ||
| repeatCount: 5, | ||
| continueOnFailure: true, | ||
| }, | ||
| { | ||
| runTaskFn: async () => { | ||
| calls++; | ||
| return { success: false, fatal: true, error: "auth failed" }; | ||
| }, | ||
| logInfoFn: () => {}, | ||
| }, | ||
| ); | ||
|
|
||
| expect(calls).toBe(1); | ||
| expect(result.completed).toBe(0); | ||
| expect(result.failed).toBe(1); | ||
| expect(result.total).toBe(5); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,57 @@ | ||||||||||||||||||||||
| import type { RuntimeOptions } from "../../config/types.ts"; | ||||||||||||||||||||||
| import { logInfo } from "../../ui/logger.ts"; | ||||||||||||||||||||||
| import { type TaskRunResult, runTask } from "./task.ts"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| type TaskRunner = (task: string, options: RuntimeOptions) => Promise<TaskRunResult>; | ||||||||||||||||||||||
| type InfoLogger = (message: string) => void; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export interface SingleTaskLoopResult { | ||||||||||||||||||||||
| total: number; | ||||||||||||||||||||||
| completed: number; | ||||||||||||||||||||||
| failed: number; | ||||||||||||||||||||||
|
Comment on lines
+8
to
+11
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 Consider adding
Suggested change
Then populate it in the return statement ( Prompt To Fix With AIThis is a comment left during a code review.
Path: cli/src/cli/commands/single-task-loop.ts
Line: 8-11
Comment:
**`skipped` missing from `SingleTaskLoopResult` interface**
The `skipped` count is computed inside the function body (line 52: `const skipped = total - completed - failed`) and also re-computed by callers — `index.ts` repeats `result.total - result.completed - result.failed` twice. Storing the formula in two independent places is fragile: if the semantics of `skipped` ever change, both sites need updating.
Consider adding `skipped` directly to the interface so callers can use it without re-deriving it:
```suggestion
export interface SingleTaskLoopResult {
total: number;
completed: number;
failed: number;
skipped: number;
}
```
Then populate it in the return statement (`skipped: total - completed - failed`) and remove the duplicate computation in `index.ts`.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Run the single-task flow with optional repeat behavior. | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| export async function runSingleTaskLoop( | ||||||||||||||||||||||
| task: string, | ||||||||||||||||||||||
| options: RuntimeOptions, | ||||||||||||||||||||||
| deps?: { | ||||||||||||||||||||||
| runTaskFn?: TaskRunner; | ||||||||||||||||||||||
| logInfoFn?: InfoLogger; | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| ): Promise<SingleTaskLoopResult> { | ||||||||||||||||||||||
| const runTaskFn = deps?.runTaskFn ?? runTask; | ||||||||||||||||||||||
| const logInfoFn = deps?.logInfoFn ?? logInfo; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const total = options.repeatCount; | ||||||||||||||||||||||
| let completed = 0; | ||||||||||||||||||||||
| let failed = 0; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for (let i = 1; i <= total; i++) { | ||||||||||||||||||||||
| if (total > 1) { | ||||||||||||||||||||||
| logInfoFn(`[${i}/${total}] Executing: ${task}`); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const result = await runTaskFn(task, options); | ||||||||||||||||||||||
| if (result.success) { | ||||||||||||||||||||||
| completed++; | ||||||||||||||||||||||
| continue; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| failed++; | ||||||||||||||||||||||
| if (result.fatal || !options.continueOnFailure) { | ||||||||||||||||||||||
| break; | ||||||||||||||||||||||
|
Comment on lines
+38
to
+52
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. Loop counter printed even on When The inner Prompt To Fix With AIThis is a comment left during a code review.
Path: cli/src/cli/commands/single-task-loop.ts
Line: 38-52
Comment:
**Loop counter printed even on `i = 1` when `total = 1` is impossible but guard allows `i = 1, total > 1`**
When `total = 1`, no counter is printed (guarded by `total > 1`) — this is correct. However, notice that the `logInfoFn` call at line 40 prints `[i/total]` before `runTaskFn` is called. If `runTaskFn` itself also logs a banner (e.g. `"Running task with claude…"` in `task.ts` line 35), the output for `--repeat 3` looks like:
```
[1/3] Executing: find and fix bugs
Running task with claude... ← emitted inside runTask
...
[2/3] Executing: find and fix bugs
Running task with claude...
```
The inner `logInfo` in `task.ts` fires unconditionally for every iteration. Combined with the iteration counter here, this creates duplicate/noisy output for repeat runs. The "Repetitive per-iteration log noise" concern is still unaddressed in `task.ts` — but the interplay with this counter makes it more visible.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (total > 1) { | ||||||||||||||||||||||
| const skipped = total - completed - failed; | ||||||||||||||||||||||
| const parts = [`${completed} succeeded`, `${failed} failed`]; | ||||||||||||||||||||||
| if (skipped > 0) parts.push(`${skipped} skipped`); | ||||||||||||||||||||||
| logInfoFn(`Done: ${parts.join(", ")} of ${total}`); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return { total, completed, failed }; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.