Omit generated diffs and slim the split-commit prompt - #38
Merged
Conversation
- Match lockfiles, `dist`/`out`, snapshots, and minified assets as generated paths. - Return source diffs plus a numstat summary for omitted generated files from `getStagedDiff`. - Cover matching, omitted-path formatting, and staged-diff omission with unit and integration tests.
- Stop embedding the single-message prompt in `getSplitPrompt`.
- Summarize the active convention and include the diff once, without `{diff}` interpolation from custom templates.
- Assert the split prompt drops SMALL/MEDIUM/LARGE instructions and interpolates the diff once.
- Default commit generation and refine to the provider's low effort when the config sets none, keeping an explicit effort untouched via `withDefaultMinEffort`. - Return the static rules and examples from `getPrompt` as a system instruction and send only the diff as the user prompt, so providers can cache the prefix across runs. - Drop the second copy of the diff from the custom template prompt. - Mark the last Anthropic system block ephemeral on both the API key and setup-token paths. - Cover the effort default, the prompt split, and the Anthropic request shape with unit tests.
- Add `generatedOnlyMessage` to build a commit title, and a bullet body for several files, from the staged paths and the active convention. - Short-circuit `Commit.route` to that local message when every staged path matches `isGeneratedPath`, and say so in the log. - Thread a `Proposal` with optional request metadata through `interact` and the commit handlers so the local path renders no model lines. - Cover the local message and the skipped model call with unit tests.
Owner
Author
|
@codex review Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's New
Staged Diff Filtering
getStagedDiffreturns the source-file patch and a numstat line per generated path (lockfiles,dist/out, snapshots, minified assets) so those bodies never enter the LLM context.Split Commit Prompt
getSplitPromptstates the active convention inline instead of slicing the single-message prompt, which had been duplicating{diff}and carrying SMALL/MEDIUM/LARGE instructions into the partition task.Staged Diff Flow
flowchart TB A["getStagedDiff"] --> B["Load worktree root and staged paths"] B --> C{"Any staged paths?"} C -->|no| D["Reject: No staged changes found"] C -->|yes| E["Split generated vs source"] E --> F["diff --staged for source paths"] E --> G["numstat summary for generated paths"] F --> H["Concatenate body + omitted-path note"] G --> H H --> I["Return staged context"]Changed Files
src/infra/git/parsers.tssrc/infra/git/repo.tssrc/domain/commit/prompts.tstest/infra/git/parsers.test.tstest/infra/git/repo.integration.test.tstest/domain/commit/prompts.test.ts{diff}interpolation.Testing & Feedback
Check
getStagedDiffwith a lockfile staged beside source, and with only a lockfile staged. Confirm the split prompt contains the diff once and does not mention SMALL/MEDIUM/LARGE.If you find any bugs or have recommendations for improvements, please open an issue and assign it to me.