Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion continual-learning/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Default cadence:
- minimum 120 minutes since the last run
- transcript mtime must advance since the previous run

Trial mode defaults (enabled in this plugin hook config):
Trial mode defaults (enabled by default in this plugin):

- minimum 3 completed turns
- minimum 15 minutes
Expand Down
64 changes: 64 additions & 0 deletions continual-learning/hooks/continual-learning-stop.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { expect, test } from "bun:test";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";

const path_hook = join(import.meta.dir, "continual-learning-stop.ts");

test("uses trial cadence by default", async () => {
const dir_workspace = mkdtempSync(join(tmpdir(), "continual-learning-"));
const path_transcript = join(dir_workspace, "transcript.jsonl");
writeFileSync(path_transcript, "{}\n");

try {
const outputs_hook = [];

for (let count_turn = 1; count_turn <= 3; count_turn += 1) {
outputs_hook.push(
await runHook(dir_workspace, path_transcript, count_turn)
);
}

expect(outputs_hook.slice(0, 2)).toEqual([{}, {}]);
expect(outputs_hook[2]).toHaveProperty("followup_message");
} finally {
rmSync(dir_workspace, { recursive: true, force: true });
}
});

async function runHook(
dir_workspace: string,
path_transcript: string,
count_turn: number

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ambiguous string hook helper args

Low Severity · Bugbot Rules

runHook takes two consecutive string parameters (dir_workspace and path_transcript), so a swapped call site type-checks but would point cwd and transcript_path at the wrong paths. This violates the review rule that same-type TypeScript parameters should use a named-args object instead.

Fix in Cursor Fix in Web

Triggered by team rule: No ambiguous args at callsite in typescript

Reviewed by Cursor Bugbot for commit 564b3e1. Configure here.

): Promise<Record<string, unknown>> {
const env_hook = { ...process.env };
delete env_hook.CONTINUAL_LEARNING_TRIAL_MODE;
delete env_hook.CONTINUOUS_LEARNING_TRIAL_MODE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fragile trial default regression test

Low Severity · Potential Edge Case

The new regression test only clears the trial-mode env keys before spawning the hook. Inherited CONTINUAL_LEARNING_MIN_TURNS or CONTINUAL_LEARNING_TRIAL_MIN_TURNS (and legacy names) can still shape cadence, so the three-turn assertion can pass without trial defaults working, or fail when trial defaults are correct.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 564b3e1. Configure here.


const process_hook = Bun.spawn(["bun", "run", path_hook], {
cwd: dir_workspace,
env: env_hook,
stdin: "pipe",
stdout: "pipe",
stderr: "pipe",
});
const input_hook = {
conversation_id: "trial-default-test",
generation_id: `generation-${count_turn}`,
status: "completed",
loop_count: 0,
transcript_path: path_transcript,
};

process_hook.stdin.write(JSON.stringify(input_hook));
process_hook.stdin.end();

const [status_exit, text_stdout, text_stderr] = await Promise.all([
process_hook.exited,
new Response(process_hook.stdout).text(),
new Response(process_hook.stderr).text(),
]);
expect(status_exit, text_stderr).toBe(0);

return JSON.parse(text_stdout) as Record<string, unknown>;
}
5 changes: 4 additions & 1 deletion continual-learning/hooks/continual-learning-stop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,10 @@ async function main(): Promise<number> {
const now = Date.now();

const trialEnabled = parseBoolean(
readEnvValue("CONTINUAL_LEARNING_TRIAL_MODE", "CONTINUOUS_LEARNING_TRIAL_MODE")
readEnvValue(
"CONTINUAL_LEARNING_TRIAL_MODE",
"CONTINUOUS_LEARNING_TRIAL_MODE"
) ?? "true"
);
if (trialEnabled && countedTurn && state.trialStartedAtMs === null) {
state.trialStartedAtMs = now;
Expand Down