Add opt-in patch start mode

[BrooksFlannery]

Adds a persisted --start-mode flag with greedy as the default and naive as dependency-gated scheduling. Threads the selected mode through the runner and patch controller while rejecting --start-mode in true ad-hoc mode. Covers naive and greedy scheduler behavior with focused controller tests.

---

^{Need help on this PR? Tag @codesmith with what you need.}

• Let Codesmith autofix CI failures and bot reviews

---

Summary by cubic

Adds an opt-in patch start scheduling mode with a persisted --start-mode flag. Defaults to greedy, preserves stored config on --gameplan updates, and rejects use in true ad‑hoc runs.

• New Features

  • CLI/Config: --start-mode [greedy|naive] persisted per project; default greedy. --gameplan loads keep stored config and only apply CLI overrides; help clarifies greedy.
  • Scheduler: greedy can start downstream patches when deps have PRs; naive waits for dependency merges.

• Migration

  • Stored configs auto-migrate by adding start_mode: "greedy". No action needed; override via CLI or edit the stored config.

^{Written for commit 7cb1713. Summary will update on new commits. Review in cubic}

Summary by CodeRabbit

• New Features
  • Added --start-mode configuration option for patch scheduling with two strategies: naive (stricter dependency checking) and greedy (more relaxed), defaulting to greedy
  • Start mode setting persists in project configuration and can be set during project creation or when resuming existing projects

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
onton	Ready	Preview, Comment	May 15, 2026 4:23pm

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR introduces configurable patch scheduling modes (Naive vs Greedy) throughout the codebase. Core type definitions and dependency logic are added to patch_controller_core, threaded through the patch_controller API with optional parameters, persisted in configuration storage with migration support, wired into the CLI and runner, and validated with property and unit tests.

Changes

Patch start mode scheduling feature

Layer / File(s)	Summary
Core start mode type and dependency satisfaction lib_core/patch_controller_core.mli, lib_core/patch_controller_core.ml	Define start_mode variant (Naive/Greedy) with string converters and implement start_deps_satisfied, which selects between Graph.deps_satisfied (Greedy) and direct merged-dependency checks (Naive).
Patch controller API with start mode parameter lib/patch_controller.mli, lib/patch_controller.ml	Expose start_mode type alias and string converters; update plan_actions, plan_messages, plan_tick_messages, plan_tick, and tick to accept optional ?start_mode (default Greedy); gate Orchestrator.Start eligibility in plan_action_for_patch using the new dependency logic.
Configuration storage with start mode migration lib/project_store.mli, lib/project_store.ml	Add start_mode : string field to stored_config; extend save_config signature to persist start_mode; introduce migrate_start_mode to inject default "greedy" for older configs lacking the field.
CLI argument and runner integration bin/main.ml	Add --start-mode CLI enum (naive/greedy); extend resolve_config to accept, validate (reject for ad-hoc projects), and persist start_mode; update config record to include start_mode; pass resolved mode to plan_tick_messages at runtime.
Property and unit test coverage lib/patch_controller.ml, test/dune, test/test_patch_controller_properties.ml	Add inline unit tests validating Naive vs Greedy start eligibility; create QCheck2 property suite verifying parsing totality, round-trip conversion, chain-based blocking rules, graph invariants, and concrete witness scenarios.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

A rabbit hops with modes so bright,
Naive blocks till deps align just right,
Greedy rushes when PRs are near,
Two ways to schedule patches clear—
Patches dance to new tune! 🐰🎵

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'Add opt-in patch start mode' directly matches the primary change: introducing a new optional --start-mode CLI flag that allows users to choose between greedy and naive patch scheduling behavior, which is threaded through the entire system.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch opt-in-start-mode

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 5 files

_{You’re at about 90% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

_{Re-trigger cubic}

[flowglad-review-service]

Review Summary

The PR cleanly adds a persisted --start-mode flag (greedy/naive), threads it through the runner and patch controller, provides backward-compatible migration for existing stored configs (defaulting to greedy), correctly rejects the flag in true ad-hoc mode, and covers the new scheduling logic with four focused inline tests. The implementation is correct and the code is well-structured. No bugs, security issues, or correctness problems were found.

Severity	Count
Must-fix	0
Should-fix	0
Note	0

---

0 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 11139 in / 1615 out · Cache: 96723 created / 355956 read · Review mode: agentic · Turns: 15 · Tool calls: 18 · Forced submit: yes · Tool mix: read_file=11, search_codebase=7

[flowglad-review-service]

Code Review: 2 comments

[flowglad-review-service]

Review Summary

The PR correctly implements start_mode gating for Naive and Greedy scheduling, the migration (migrate_start_mode), and the CLI validation that rejects --start-mode in true ad-hoc mode. Two findings:

1. should-fix: The new scheduler logic has only inline %test boolean assertions. Repository conventions require QCheck2 property tests (totality, round-trip, interleaving, boundary) in a dedicated test/test_patch_controller_properties.ml file that links onton_core only.

2. should-fix: In the | _, Some gp_path (fresh gameplan) branch, omitting --start-mode always defaults to Greedy and immediately overwrites any previously persisted Naive preference. The branch should load and merge the stored start_mode the same way the | Some proj, None branch does.

Severity	Count
Must-fix	0
Should-fix	2
Note	0

---

2 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 11242 in / 4426 out · Cache: 70684 created / 355593 read · Review mode: agentic · Turns: 15 · Tool calls: 19 · Forced submit: yes · Tool mix: read_file=11, search_codebase=7, find_references=1

[flowglad-review-service]

Code Review: 2 comments

[flowglad-review-service]

Review Summary

Re-examining prop_chain_greedy_allows_child_when_mid_has_pr: it uses Bool.equal, which checks both directions of the equivalence — not just the "allows" case but also the "blocks" case when both flags are false. The test is correct. Only one minor finding remains: the unnecessary try/with wrapper around a total Result.t-returning function in prop_start_mode_parse_total.

Severity	Count
Must-fix	0
Should-fix	1
Note	1

---

2 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 15465 in / 4326 out · Cache: 91415 created / 411865 read · Review mode: agentic · Turns: 15 · Tool calls: 19 · Forced submit: yes · Tool mix: read_file=15, search_codebase=4

[flowglad-review-service]

Review Summary

The implementation is correct and complete. Key observations:

• Patch_controller_core is a clean pure module (no effectful deps) with Naive/Greedy semantics correctly implemented: Naive checks only direct deps via Graph.deps, Greedy delegates to the existing Graph.deps_satisfied.
• Config migration (migrate_start_mode) correctly defaults missing fields to "greedy", and stored_config_of_yojson will raise on missing fields without migration — the pipeline order (migrate_backend_model |> migrate_start_mode) is correct.
• All three resolve_config arms (ad-hoc/--gameplan/--project) properly resolve, persist, and thread start_mode; the previously-reported bug (stored value overwritten in the --gameplan branch) is fixed.
• The property test file covers all four required families (totality, round-trip, algebraic subset, interleaving chain), links onton_core only, and prop_start_mode_parse_total no longer wraps the total function in try/with.

No new must-fix or should-fix issues found in this diff.

Severity	Count
Must-fix	0
Should-fix	0
Note	0

---

0 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 15587 in / 2295 out · Cache: 86023 created / 411498 read · Review mode: agentic · Turns: 15 · Tool calls: 18 · Forced submit: yes · Tool mix: read_file=15, search_codebase=3

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bin/main.ml`:
- Around line 3859-3894: The code currently recomputes repo_root, GitHub
credentials and main_branch and immediately calls Project_store.save_config even
when stored_opt exists, which can overwrite previously valid stored settings;
update the logic in the block that handles stored_opt (around
repo_root_for_fresh, load_existing_config, stored_opt, merge_cli_stored,
resolve_backend_model, resolve_github_credentials, resolve_branch) to either (a)
preserve stored repo_root/github_token/github_owner/github_repo/main_branch when
stored_opt is Some (mirroring the Some proj, None branch) by merging CLI values
with stored.Project_store fields before calling Project_store.save_config, or
(b) defer calling Project_store.save_config until after start_mode_result and
the full resolved config have been validated; ensure Project_store.save_config
is invoked only with the final validated values (use
Patch_controller.start_mode_of_string/ start_mode_to_string and the merged
backend/model from resolve_backend_model).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0ac914d5-b5ae-4bf9-b7c6-7432dc8e2f2b

📥 Commits

Reviewing files that changed from the base of the PR and between 7b1fe08 and 100956d.

📒 Files selected for processing (9)

• bin/main.ml
• lib/patch_controller.ml
• lib/patch_controller.mli
• lib/project_store.ml
• lib/project_store.mli
• lib_core/patch_controller_core.ml
• lib_core/patch_controller_core.mli
• test/dune
• test/test_patch_controller_properties.ml

[flowglad-review-service]

Code Review: 1 comment

[flowglad-review-service]

Review Summary

The implementation is well-structured overall. The pure/effectful split is correct (patch_controller_core in lib_core/, handlers in lib/), the migration path correctly defaults existing configs to greedy, and all three config branches (None/None, _/Some gp_path, Some proj/None) correctly resolve and persist start_mode. The previously-outstanding issues about start_mode being overwritten in the --gameplan branch and missing property tests have been addressed in this commit.

One finding: the [Greedy] docstring in patch_controller_core.mli omits the has_pr condition from the description, making the Greedy semantics appear simpler than they are. The fix is a one-line doc update.

Severity	Count
Must-fix	0
Should-fix	1
Note	0

---

1 comment posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 15981 in / 2617 out · Cache: 101084 created / 457400 read · Review mode: agentic · Turns: 15 · Tool calls: 19 · Forced submit: yes · Tool mix: read_file=14, search_codebase=5

[flowglad-review-service]

Code Review: 1 comment

[flowglad-review-service]

Review Summary

This incremental diff correctly addresses the two outstanding Turn 2 findings: start_mode is now properly loaded from stored config in the --gameplan branch (no longer silently overwriting a stored naive preference), and ad-hoc mode correctly rejects --start-mode. The Turn 3 try/with note is resolved — prop_start_mode_parse_total no longer wraps the call. The Turn 5 docstring concern is addressed in patch_controller_core.mli.

One new finding: the gen_generated_graph generator calls Graph.of_patches, which raises on duplicate IDs. The current index-based generation avoids duplicates, but there is no explicit guard. Per repo convention, QCheck2 properties touching functions with raise paths should be protected — and in this case the protection belongs in the generator, not the property body.

Severity	Count
Must-fix	0
Should-fix	1
Note	0

---

1 comment posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 15990 in / 3101 out · Cache: 84669 created / 413867 read · Review mode: agentic · Turns: 15 · Tool calls: 17 · Forced submit: yes · Tool mix: read_file=14, search_codebase=3

[flowglad-review-service]

Review Summary

The incremental diff is clean. The outstanding prior comments (stored-config overwrite in the --gameplan branch, property test structure, and docstring accuracy) are already posted on the PR as live comments and are not re-raised here. No new bugs, correctness issues, or quality concerns are introduced by this increment.

Severity	Count
Must-fix	0
Should-fix	0
Note	0

---

0 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 15999 in / 1707 out · Cache: 91362 created / 409594 read · Review mode: agentic · Turns: 15 · Tool calls: 19 · Forced submit: yes · Tool mix: read_file=15, search_codebase=4