Skip to content

Honor configured split effort and tighten commit prompts - #37

Open
rafaeelricco wants to merge 2 commits into
mainfrom
rafaeelricco/drop-split-min-effort
Open

rafaeelricco wants to merge 2 commits into
mainfrom
rafaeelricco/drop-split-min-effort

Conversation

@rafaeelricco

@rafaeelricco rafaeelricco commented Aug 18, 2026 •

Copy link
Copy Markdown
Owner

Motivation

Split-plan ignored the user's configured effort (always low/MINIMAL), so a high-effort pick never reached the provider. A prompt audit of the commit, refine, split and branch prompts also found instructions written for older models, a rule that contradicted its own examples, and a refine prompt that ignored the user's commit convention.

What's New

Split Plan Effort

  • generateSplitPlan now wraps the incoming ProviderConfig with withDefaultMinEffort instead of withMinEffort, so an explicit effort reaches the provider unchanged.
  • An unset effort still falls back to the provider's lowest level, matching commit and refine, so split calls get no slower for users who never picked an effort.
  • withMinEffort stays in src/domain/llm/effort.ts because withDefaultMinEffort builds on it, and the router.ts module header now lists split plan under withDefaultMinEffort.

Commit Prompts

  • The conventional, imperative and custom prompts replace "First, internally decide SMALL/MEDIUM/LARGE" and the stacked "Output ONLY / Do NOT" lines with one sentence explaining why: the reply is written verbatim as the commit message. The size-based format rule itself is unchanged.
  • The imperative prompt's style examples and negative example are now capitalized to match its "start with a capital letter" rule; previously they showed lowercase add user authentication as the good form. Its example body no longer ends with a literal - ... bullet.
  • Split-plan examples are labeled as showing the JSON shape only, so their imperative messages don't pull conventional-commit users off their convention.

Branch Name Prompt

  • The "forbidden anywhere" list now covers only words that come from the instructions (suggestion, prompt, snapshot, kebab-case, branch-name). It used to ban cli, model, tool and context, which collided with the area-prefix rule for changes under src/cli/.
  • The five-step <synthesis_protocol> is removed; its one criterion not stated elsewhere ("no slug may be just a subset of another") moves into <diversity_axes>.

Refine Convention

  • refineCommitMessage takes the configured commit_convention and custom_template, and getRefinePrompt states them through conventionSummary. It no longer guesses the convention from the current message's prefix, which forced imperative style onto custom-template users, and it defines the size-based format instead of naming undefined SMALL/MEDIUM/LARGE labels.

Split Plan Effort Flow

flowchart TB
    A["generateSplitPlan(config)"] --> B["withDefaultMinEffort"]
    B --> C{"config.effort set?"}
    C -->|yes| D["Keep configured effort"]
    C -->|no| E["Lowest level: low or MINIMAL"]
    D --> F["generateContent"]
    E --> F
    F --> G["Provider"]
Loading

Changed Files

File Change Type Summary
src/domain/llm/router.ts Modified Split plan uses withDefaultMinEffort; refineCommitMessage takes convention and template; module header updated.
src/domain/commit/prompts.ts Modified Prompt fixes above; getRefinePrompt takes convention and template.
src/cli/commit.ts Modified Passes commit_convention and custom_template to refine.
test/domain/llm/router.test.ts Modified Split effort pass-through and unset-to-low tests; refine call sites updated.
test/domain/commit/prompts.test.ts Modified Refine prompt states a custom template and no longer names SMALL.

Testing & Feedback

Review generateSplitPlan and refineCommitMessage in src/domain/llm/router.ts, then the prompt text in src/domain/commit/prompts.ts. The split tests should expect Just("high") for a configured high and Just("low") for no effort. The new refine test fails on the old prompt, which contained "SMALL" and no template text. The prompt rewording is covered only by unit tests, so try it on real diffs:

  • Run commit with the imperative convention and confirm the title starts with a capital letter.
  • Set a custom template, choose Adjust, and confirm the refined message keeps the template's shape.
  • Run commit branch on changes that are all under src/cli/ and check that one suggestion can start with cli-.
  • Pick high in commit effort, then run commit split and confirm the commit note reports high effort.

If you find any bugs or have recommendations for improvements, please open an issue and assign it to me.

@rafaeelricco rafaeelricco self-assigned this Aug 18, 2026
- Call `generateContent` with `withDefaultMinEffort(config)` in `generateSplitPlan`, so an explicit effort reaches the provider and an unset one still falls back to the lowest level.
- Drop the unused `withMinEffort` import from `src/domain/llm/router.ts` and update the module header to list split plan under `withDefaultMinEffort`.
- Assert a configured `high` effort reaches the provider and an unset effort defaults to `low` in the split-plan router tests.
@rafaeelricco
rafaeelricco force-pushed the rafaeelricco/drop-split-min-effort branch from e038516 to fc4a7d6 Compare September 27, 2026 14:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-27T15:23:15.376312Z 58f93f4 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

- Replace the "internally decide" and stacked "ONLY / Do NOT" output lines in the conventional, imperative, and custom prompts with one line explaining that the reply becomes the commit message verbatim.
- Capitalize the imperative prompt's style examples and negative example so they match its capital-letter rule, and make the example body bullets complete imperative sentences.
- Label the split-plan examples as illustrative so their imperative messages do not override the active convention.
- Limit the branch-name "forbidden anywhere" list to words from the instructions, so area prefixes like `cli` stay usable, and drop the `<synthesis_protocol>` steps while keeping the no-subset rule.
- Pass `commit_convention` and `custom_template` through `refineCommitMessage` into `getRefinePrompt`, which now states the configured convention and the size-based format instead of inferring the convention from the current message.
- Cover the refine prompt's use of a custom template with a unit test and update the refine call sites in the router tests.
@rafaeelricco rafaeelricco changed the title Pass configured effort through to split-plan generation Honor configured split effort and tighten commit prompts Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant