feat: follow up plan-review-pass changes

[axeldelafosse]

Summary

• extract codex rendering into src/loop/codex-render.ts and wire it into the runner flow
• simplify runner/task wiring and update related CLI constants/types
• update README usage examples to match the new behavior
• add/update tests for args, runner, task, and codex-render

Validation

• bun run check

[gemini-code-assist]

Summary of Changes

Hello @axeldelafosse, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant enhancement to the agent loop runner by adding an automatic plan review step for plain-text prompts, allowing for better validation of initial task plans. It also refactors the internal handling of Codex agent output into a dedicated rendering module, improving modularity and maintainability. These changes streamline the workflow for users and provide more control over the planning and review phases of agent execution.

Highlights

• New Plan Review Feature: Introduced a new --review-plan CLI option that allows for an automatic review pass of the PLAN.md file after it's created from a plain-text prompt. This review can be performed by 'other' (the non-primary model by default), 'claude', 'codex', or skipped entirely with 'none'.
• Codex Rendering Refactor: Extracted the logic for rendering raw Codex events into a dedicated codex-render.ts module. This new module handles parsing and formatting of agent messages, ensuring consistent output for both 'pretty' and 'raw' formats.
• Optional Proof Criteria: The --proof argument is now optional, removing the previous requirement for a proof criteria to be provided for task completion.
• CLI Argument and Type Updates: Updated CLI argument parsing in args.ts to support the new --review-plan option and adjusted related types (PlanReviewMode) to accommodate the new review modes.
• Enhanced Test Coverage: Added new tests for the codex-render module and updated existing tests for argument parsing (args.test.ts), prompt generation (prompts.test.ts), agent runner logic (runner.test.ts), and task resolution (task.test.ts) to reflect the new features and refactorings.
• README Examples Refreshed: The README.md file has been updated with new examples demonstrating the usage of the --review-plan option and clarifying the optional nature of the --proof argument.

Changelog

• README.md
  • Updated --proof argument description to reflect its optional status.
  • Added --review-plan option with available modes and default behavior.
  • Included new examples demonstrating --review-plan usage, including overriding the reviewer and skipping the review.
• src/loop/args.ts
  • Removed REQUIRED_PROOF_ERROR constant.
  • Imported PlanReviewMode type.
  • Implemented parsePlanReviewValue to validate --review-plan argument values.
  • Added parsePlanReviewArg function to handle --review-plan argument parsing.
  • Integrated --review-plan parsing into the consumeArg function.
  • Removed the check for a required --proof value from parseArgs.
• src/loop/codex-app-server.ts
  • Defined RunCodexTurnCallbacks interface for agent turn callbacks.
  • Added NOOP_CALLBACK constant for empty callback functions.
  • Modified runCodexTurn to use the new RunCodexTurnCallbacks interface and default onParsed to NOOP_CALLBACK if not provided.
• src/loop/codex-render.ts
  • Added new file to encapsulate Codex event rendering logic.
  • Defined interfaces AppServerEvent, CodexRenderState, CodexRendererConfig, and CodexRenderer.
  • Implemented utility functions asRecord, asString, parseJsonLine, collectText, parseDeltaText, parseCompletedMessage, and parseItemId.
  • Developed appendChunk, markMessageBoundary, handleDeltaLine, and handleCompletedLine for processing different event types.
  • Exported createCodexRenderer to initialize a renderer instance with specified format and write function.
• src/loop/constants.ts
  • Adjusted formatting in the HELP message for better alignment.
  • Updated the description for --proof to indicate it is no longer required.
  • Added --review-plan option description to the HELP message.
• src/loop/prompts.ts
  • Added appendProofRequirements helper function to conditionally add proof text.
  • Introduced buildPlanReviewPrompt to generate prompts specifically for reviewing PLAN.md.
  • Updated buildWorkPrompt to use appendProofRequirements.
  • Updated buildReviewPrompt to use appendProofRequirements.
• src/loop/runner.ts
  • Imported createCodexRenderer from the new module.
  • Modified runCodexAgent to utilize createCodexRenderer for handling raw Codex output.
  • Replaced manual parsing and state management with the CodexRenderer instance.
  • Added logic to ensure a trailing newline is written in 'pretty' format after agent completion if necessary.
• src/loop/task.ts
  • Imported buildPlanReviewPrompt and PlanReviewMode.
  • Implemented resolvePlanReviewer to determine the appropriate agent for plan review based on options.
  • Modified runPlanMode to include an optional plan review step after PLAN.md creation.
  • Added logging for plan review status and error handling for non-zero exit codes during plan review.
• src/loop/types.ts
  • Defined new type PlanReviewMode to represent options for plan review agents.
  • Added reviewPlan property of type PlanReviewMode to the Options interface.
• tests/loop/args.test.ts
  • Removed test case for parseArgs throwing on missing required proof.
  • Updated the default options test to reflect that --proof is no longer required and proof defaults to an empty string.
  • Added test cases for parsing --review-plan argument, including bare usage, specific agent values, none, and equals-form syntax.
  • Added a test case to reject invalid --review-plan values.
• tests/loop/codex-render.test.ts
  • Added new test file for createCodexRenderer.
  • Included tests for completed message concatenation, delta extraction priority, completed extraction priority, and handling of whitespace-only content.
  • Added a test for raw mode output behavior.
• tests/loop/prompts.test.ts
  • Added a test case for buildPlanReviewPrompt to verify its content and instructions.
  • Added a test case to confirm buildReviewPrompt omits proof requirements when the proof string is empty.
• tests/loop/runner.test.ts
  • Updated the mock signature for runCodexTurn to include the new callbacks parameter.
  • Added a test to verify runAgent inserts a blank line after message completion in pretty mode.
  • Added a test to ensure runAgent preserves nested delta newline content in pretty mode.
• tests/loop/task.test.ts
  • Modified makeOptions to accept partial overrides for flexibility in tests.
  • Updated resolveTask tests to assert the correct number of runAgent calls, including the new plan review step.
  • Added tests to confirm resolveTask reviews PLAN.md with the 'other' model by default and correctly resolves the reviewer when the primary agent is Claude.
  • Added a test to verify resolveTask skips plan review when reviewPlan is set to 'none'.
  • Added a test to ensure resolveTask throws an error when the plan review agent exits with a non-zero code.

Activity

• The pull request introduces new functionality and refactors existing code, as detailed in the highlights and changelog.
• New tests have been added and existing tests updated to ensure the correctness and stability of the changes.
• The README.md has been updated to reflect the new CLI options and their usage.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a plan review pass, making the --proof argument optional and adding a new --review-plan argument. The implementation includes extracting codex rendering logic into a new src/loop/codex-render.ts file, which is a great improvement for modularity. The argument parsing, task running logic, and prompts are all updated accordingly. The changes are well-tested, with new test files and updates to existing ones. Overall, this is a solid contribution. I have one suggestion to improve the argument parsing logic for better clarity and maintainability.

[gemini-code-assist]

Using a try-catch block for control flow can make the code harder to understand and maintain. It's generally better to use explicit checks. In this case, you can directly check if the next argument is one of the valid values for --review-plan.

This change makes the logic for handling the presence or absence of a value for --review-plan more explicit and readable.

Suggested change

	try {
	opts.reviewPlan = parsePlanReviewValue(next);
	return index + 1;
	} catch {
	opts.reviewPlan = "other";
	return index;
	}
	if (next === "other" || next === "claude" || next === "codex" || next === "none") {
	opts.reviewPlan = next;
	return index + 1;
	} else {
	opts.reviewPlan = "other";
	return index;
	}