chore(scripts): update AI commit prompt to use Conventional Commits#682
chore(scripts): update AI commit prompt to use Conventional Commits#682greenc-FNAL wants to merge 3 commits into
Conversation
Update the system prompt in scripts/git-ai-commit to enforce the Conventional Commits v1.0.0 specification. This ensures that AI-generated commit messages follow a standardized format, including specific types (feat, fix, etc.), proper handling of breaking changes, and structured body/footer sections. See also: https://www.conventionalcommits.org/en/v1.0.0/#specification
📝 WalkthroughWalkthroughThis PR reworks Changesgit-ai-commit diff handling and docs
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Main as main()
participant Skip as _skip_binary_and_generated
participant Build as _build_messages
participant Trunc as _truncate_diff
Main->>Skip: staged diff
Skip-->>Main: cleaned diff
Main->>Main: compute raw_len (pre-truncation)
alt raw_len exceeds threshold
Main->>Main: select max_diff_chars = escalated cap
else
Main->>Main: select max_diff_chars = default cap
end
Main->>Build: build_messages(diff, max_diff_chars, amend_no_diff)
Build->>Trunc: truncate_diff(diff, max_diff_chars)
Trunc-->>Build: truncated diff + omission marker
Build-->>Main: prompt messages
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…changes Update the AI prompt to include a direct link to the Conventional Commits specification for better model adherence. Implement logic to handle the case where `--amend` is used but no changes are staged. In this scenario, the script now prompts the AI to rewrite the previous commit message to improve its wording and ensure it strictly follows Conventional Commits guidelines.
Implement several strategies to minimize the amount of data sent to the language model, reducing token usage and cost while maintaining message quality. Key changes: - Implement hunk-aware diff truncation with omission markers. - Skip binary and generated files based on a glob list. - Reduce recent commit history depth from 10 to 5. - Introduce two-tier context character caps for prompt escalation. - Refine the system prompt to remove redundant filler. - Update man pages and add tests for the new truncation logic. - Add comprehensive context optimization guidelines in docs/dev/.
There was a problem hiding this comment.
Pull request overview
This PR updates git-ai-commit to better control prompt content and enforce Conventional Commits formatting, while also reducing prompt bloat via diff filtering/truncation and a smaller commit-history window.
Changes:
- Update the system prompt to require Conventional Commits v1.0.0 (type/scope, breaking changes, body/footer rules).
- Add staged-diff context optimizations: skip binary/generated-file sections and apply hunk-aware truncation with a model-dependent diff-size cap.
- Reduce recent history depth from 10 to 5, and add an
--amend“no staged changes” mode that rewrites the prior message to conform to Conventional Commits.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/git-ai-commit | Implements Conventional Commits prompt rules, diff skipping/truncation, escalation ordering changes, and amend-with-no-diff rewrite behavior. |
| scripts/test/test_git_ai_commit.py | Adds unit tests for truncation, diff skipping, and max-diff-chars behavior. |
| scripts/man/man1/git-ai-commit.1 | Updates user-facing documentation for new diff handling and history depth. |
| docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md | Adds a design/plan record describing the context-optimization approach and decisions. |
| docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.json | Adds a structured plan/steps artifact for the optimization work. |
| docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md | Adds the guideline doc plus an “Implementation status” section reflecting what was implemented/rejected. |
| # Split diff into sections starting with "diff --git". | ||
| sections = re.split(r"(?=^diff --git )", diff, flags=re.MULTILINE) | ||
| kept_sections: list[str] = [] | ||
| for sec in sections: | ||
| if not sec.strip(): | ||
| continue | ||
| # Binary detection. | ||
| if "Binary files" in sec and "differ" in sec: | ||
| # Extract path from the line, e.g., "Binary files a/foo.bin and b/foo.bin differ" | ||
| m = re.search(r"Binary files ([^ \n]+) and ([^ \n]+) differ", sec) | ||
| path = m.group(2) if m else "<unknown>" | ||
| kept_sections.append(f"[skipped binary/generated file: {path}]\n") | ||
| continue | ||
| # Generated file detection via header line. | ||
| header_match = re.search(r"^diff --git a/([^ \n]+) b/([^ \n]+)", sec, flags=re.MULTILINE) | ||
| if header_match: | ||
| path = header_match.group(2) | ||
| if any(fnmatch.fnmatch(path, pat) for pat in _GENERATED_FILE_GLOBS): | ||
| kept_sections.append(f"[skipped binary/generated file: {path}]\n") | ||
| continue | ||
| # Keep original section. | ||
| kept_sections.append(sec) | ||
| return "".join(kept_sections) |
| omitted = total_files - len(kept_sections) | ||
| final = stat_block + "".join(kept_sections) | ||
| if omitted or line_boundary_cut: | ||
| final += ( | ||
| f"\n\n[diff truncated: {omitted} of {total_files} files omitted to fit context budget]" | ||
| ) | ||
| return final |
| # Determine max diff chars based on model and escalation. | ||
| if backend == "kilo" and (model == _ESCALATION_MODEL_KILO or model_explicit): | ||
| max_diff_chars = _MAX_DIFF_CHARS_ESCALATED | ||
| else: | ||
| max_diff_chars = _MAX_DIFF_CHARS_DEFAULT |
| # Build a large file section with many lines | ||
| lines = [f"+line {i}\n" for i in range(100)] | ||
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) | ||
| diff = stat_block + file_section | ||
|
|
| # Build a diff larger than 60 000 chars | ||
| lines = [f"+line {i} with some padding to make it longer\n" for i in range(1500)] | ||
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) | ||
| diff = stat_block + file_section | ||
| assert len(diff) > 60_000 |
| # Build a diff larger than 60 000 but smaller than 400 000 | ||
| lines = [f"+line {i} with some padding to make it longer\n" for i in range(1500)] | ||
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) | ||
| diff = stat_block + file_section | ||
| assert 60_000 < len(diff) < 400_000 |
| _MAX_DIFF_CHARS_DEFAULT = 60_000 | ||
| _MAX_DIFF_CHARS_ESCALATED = 400_000 # Large ceiling for escalated path | ||
|
|
||
| # When the kilo model was not explicitly chosen and the prompt exceeds this | ||
| # character count, escalate to a free high-context model rather than risk |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| The placeholder format is ``"[skipped binary/generated file: <path>]"``. | ||
| """ | ||
| import fnmatch | ||
| import re |
|
21 fixed, 1 new since branch point (cf6781a) ❌ 1 new CodeQL alert since the previous PR commit
✅ 21 CodeQL alerts resolved since the previous PR commit
❌ 1 new CodeQL alert since the branch point
✅ 21 CodeQL alerts resolved since the branch point
Review the full CodeQL report for details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/git-ai-commit`:
- Around line 241-247: The truncation summary in the diff builder is inaccurate
when a file is only partially included via the line-boundary cut path. Update
the logic around the `kept_sections` / `omitted` calculation in
`scripts/git-ai-commit` so a partial section is counted as truncated rather than
fully kept, and make the final marker message reflect that one or more files
were cut mid-file. Keep the fix localized to the truncation branch that sets
`final` and appends the `[diff truncated: ...]` note.
In `@scripts/test/test_git_ai_commit.py`:
- Around line 827-868: The large-diff tests in
test_default_cap_truncates_large_diff and test_escalated_cap_keeps_large_diff
duplicate the same fragile diff-building logic using the `.join()` separator
trick. Refactor the repeated diff construction into a small helper such as a
local `_make_large_diff(...)` fixture/helper in test_git_ai_commit.py, and reuse
it in both tests so the threshold assertions remain the same while removing
copy/paste drift.
- Around line 716-743: The test named test_single_oversized_file_line_boundary
is building many small diff sections instead of one oversized file section, so
it never hits the single-section line-boundary fallback in _truncate_diff.
Update the fixture in test_single_oversized_file_line_boundary to construct one
real diff --git section with a large body that exceeds the budget on its own, so
_truncate_diff exercises the kept_sections == false fallback path. Keep the
existing assertions, but make sure the test data targets the fallback branch in
_truncate_diff rather than the normal multi-file truncation path.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e59fc2b4-408f-4cee-9ac3-80dcccb80a69
📒 Files selected for processing (6)
docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.jsondocs/dev/git-ai-commit-context/git-ai-commit-context-optimization.mddocs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.mdscripts/git-ai-commitscripts/man/man1/git-ai-commit.1scripts/test/test_git_ai_commit.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Analyze python with CodeQL
- GitHub Check: copilot-pull-request-reviewer
⚠️ CI failures not shown inline (6)
GitHub Actions: greenc-FNAL checking Markdown format / markdown-check: greenc-FNAL checking Markdown format
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = "success" ]; then�[0m
�[36;1m echo "✅ Markdown formatting check passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::Markdown formatting check failed."�[0m
GitHub Actions: greenc-FNAL checking Markdown format / 0_markdown-check.txt: greenc-FNAL checking Markdown format
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = "success" ]; then�[0m
�[36;1m echo "✅ Markdown formatting check passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::Markdown formatting check failed."�[0m
GitHub Actions: greenc-FNAL checking Python code / python-check: greenc-FNAL checking Python code
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = 'success' ] && [ "success" = 'success' ]; then�[0m
�[36;1m echo "✅ Python checks passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::Python checks failed. Comment '@${REPO_NAME}bot python-fix' on the PR to attempt auto-fix."�[0m
GitHub Actions: greenc-FNAL checking Python code / 0_scripts-test.txt: greenc-FNAL checking Python code
Conclusion: failure
_2 PASSED [ 25%]
scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_missing_github_repository_exits_2 PASSED [ 26%]
scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_skipped_when_sarif_has_baseline PASSED [ 26%]
scripts/test/test_check_codeql_alerts.py::TestMainApiModeWithPrRef::test_api_mode_pr_ref_produces_filtered_url PASSED [ 26%]
scripts/test/test_check_codeql_alerts.py::TestMainEntrypoint::test_entrypoint_no_alerts_exits_zero PASSED [ 26%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_reads_from_file PASSED [ 26%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_reads_from_stdin PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_invalid_yaml_returns_empty PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_non_dict_yaml_returns_empty PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_missing_diagnostics_key_returns_empty PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_non_list_diagnostics_value_returns_empty PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_empty_diagnostics_returns_empty_list PASSED [ 28%]
scripts/test/test_clang_tidy_check_summary.py::TestCountUniqueDiagnostics::test_single_entry_counted PASSED [ 28%]
scripts/test/test_clang_tidy_check_summary.py::TestCountUniqueDiagnostics::test_identical_triplet_counted_once PASSED [ 28%]
scripts/test/test_clang_tidy_check_summary.py::TestCountUniqueDiagnostics::test_different_offsets_counted_separately PASSED [ 28%]
scripts/test/test_clang_tidy_check_summary.py::TestCountUniqueDiagnostics::test_multiple_checks_counted_independently PASSED [ 29%]
scripts/test/test_clang_tidy_check_summary.py::TestCountUniqueDiagnostics::test_empty_list_returns_empty_dict PASSED [ 29%]
scripts/test/test_clang_tidy_check_summary.py...
GitHub Actions: greenc-FNAL checking Python code / 1_python-check.txt: greenc-FNAL checking Python code
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = 'success' ] && [ "success" = 'success' ]; then�[0m
�[36;1m echo "✅ Python checks passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::Python checks failed. Comment '@${REPO_NAME}bot python-fix' on the PR to attempt auto-fix."�[0m
GitHub Actions: greenc-FNAL checking Python code / scripts-test: greenc-FNAL checking Python code
Conclusion: failure
##[group]Run codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f
with:
files: coverage-scripts.xml
flags: scripts
name: phlex-scripts-coverage
fail_ci_if_error: false
verbose: true
root_dir: phlex-src
***REDACTED***
disable_file_fixes: false
disable_search: false
disable_safe_directory: false
disable_telem: false
dry_run: false
git_service: github
gcov_executable: gcov
handle_no_reports_found: false
recurse_submodules: false
run_command: upload-coverage
skip_validation: false
use_legacy_upload_endpoint: false
use_oidc: false
use_pypi: false
version: latest
env:
CODECOV_***REDACTED***
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
##[group]Run missing_deps=""
�[36;1mmissing_deps=""�[0m
�[36;1m�[0m
�[36;1m# Check for always-required commands�[0m
�[36;1mfor cmd in bash git curl; do�[0m
�[36;1m if ! command -v "$cmd" >/dev/null 2>&1; then�[0m
�[36;1m missing_deps="$missing_deps $cmd"�[0m
�[36;1m fi�[0m
�[36;1mdone�[0m
�[36;1m�[0m
�[36;1m# Check for gpg only if validation is not being skipped�[0m
�[36;1mif [ "$INPUT_SKIP_VALIDATION" != "true" ]; then�[0m
�[36;1m if ! command -v gpg >/dev/null 2>&1; then�[0m
�[36;1m missing_deps="$missing_deps gpg"�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1m# Report missing required dependencies�[0m
�[36;1mif [ -n "$missing_deps" ]; then�[0m
�[36;1m echo "Error: The following required dependencies are missing:$missing_deps"�[0m
�[36;1m echo "Please install these dependencies before using this action."�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mecho "All required system dependencies are available."�[0m
shell: /usr/bin/sh -e {0}
env:
CODECOV_***REDACTED***
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
INPUT_SKIP_VALIDATION: false
...
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.md: All Markdown files must follow markdownlint rule MD012: no multiple consecutive blank lines (never more than one blank line in a row)
All Markdown files must follow markdownlint rule MD022: headings must be surrounded by exactly one blank line before and after
All Markdown files must follow markdownlint rule MD031: fenced code blocks must be surrounded by exactly one blank line before and after
All Markdown files must follow markdownlint rule MD032: lists must be surrounded by exactly one blank line before and after (including after headings and code blocks)
All Markdown files must follow markdownlint rule MD034: no bare URLs (use markdown link syntax like[text](destination)instead of plain URLs)
All Markdown files must follow markdownlint rule MD036: use # headings for titles, not Bold:
All Markdown files must follow markdownlint rule MD040: always specify code block language (for example, use 'bash', 'python', '```text', etc.)
**/*.md: Do not use multiple consecutive blank lines in Markdown (MD012)
Surround Markdown headings with exactly one blank line (MD022)
Surround Markdown fenced code blocks with exactly one blank line (MD031)
Surround Markdown lists with exactly one blank line (MD032)
Do not use bare URLs in Markdown; use[text](url)syntax instead (MD034)
Use#headings in Markdown, not**Bold**for section titles (MD036)
Always specify language on fenced code blocks in Markdown (MD040)
Files:
docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.mddocs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Use ruff for Python formatting and linting (configured inpyproject.toml); follow Google-style docstring conventions; use line length of 99 characters; use double quotes for strings
Usefrom __future__ import annotationsto enable deferred evaluation of type annotations (avoids forward-reference issues; Python >=3.12)
Type hints are recommended; mypy is configured inpyproject.toml
Avoid naming Python test scriptstypes.pyor other names that shadow standard library modules, as this causes obscure import errors
Python test scripts should follow the test structure pattern: C++ driver provides data streams, Jsonnet config wires the graph, and Python script implements algorithms
**/*.py: Enforce 99-character line limit and double quotes in Python via ruff configured inpyproject.toml
Use Google-style docstrings in Python code
Use type hints in Python code and configure mypy for type checking
Usefrom __future__ import annotationsin Python to enable deferred evaluation of type annotations
Use PEP 8 naming in Python:CapWordsfor classes,snake_casefor everything else
When using thereadtool for Python files, always use integer values foroffsetandlimitparameters, never float/double values
Files:
scripts/test/test_git_ai_commit.py
🪛 LanguageTool
docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md
[style] ~111-~111: Consider an alternative for the overused word “exactly”.
Context: ...conventional‑commit* header, which is exactly what the spec requires. - Therefore the...
(EXACTLY_PRECISELY)
docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ... via SourceFileLoader. Run in CI by .github/workflows/python-check.yaml:162 and ...
(GITHUB)
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ...ub/workflows/python-check.yaml:162and .github/workflows/coverage.yaml:272 (pytest s...
(GITHUB)
[grammar] ~124-~124: Use a hyphen to join words.
Context: ... - New tests for _truncate_diff: under budget returns unchanged; over budget ...
(QB_NEW_EN_HYPHEN)
[grammar] ~124-~124: Use a hyphen to join words.
Context: ...f`: under budget returns unchanged; over budget cuts on file/hunk boundary a...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (18)
scripts/test/test_git_ai_commit.py (4)
679-694: LGTM!
695-715: LGTM!
744-759: LGTM!
766-817: LGTM!scripts/man/man1/git-ai-commit.1 (2)
37-38: 📐 Maintainability & Code Quality | ⚡ Quick winClarify the truncation granularity.
This isn’t really “hunk-aware” in the
@@sense; the helper preserves wholediff --gitsections and falls back to a line-boundary cut for one oversized section. Also, skipped binary/generated files are replaced with a placeholder note, not silently dropped.Suggested wording
-Output of git diff --cached --stat -p, with hunk-aware truncation applied to whole file sections and binary/generated files skipped. +Output of git diff --cached --stat -p, with section-aware truncation that preserves whole file sections and replaces skipped binary/generated files with a placeholder note.
91-92: LGTM!docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.json (1)
84-86: 📐 Maintainability & Code Quality | ⚡ Quick winFix the history-depth wording.
This step still references
git log --oneline -10, which conflicts with the stated reduction to-5. As written, the instruction is self-contradictory and can send the implementation back to the old depth.docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md (1)
1-133: LGTM!docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md (1)
1-179: LGTM!scripts/git-ai-commit (9)
9-9: LGTM!Also applies to: 34-38
134-135: LGTM!
145-192: LGTM!
250-270: LGTM!
334-337: LGTM!
771-809: LGTM!
991-992: LGTM!Also applies to: 1002-1017
1030-1034: 🎯 Functional CorrectnessWorth a sanity check: the diff-cap tiers for non-kilo backends and the default-vs-threshold gap.
Two things jump out here that might be intentional but smell slightly off:
_MAX_DIFF_CHARS_DEFAULT(60k) is larger than_ESCALATION_THRESHOLD_CHARS(30k). On an auto-selected kilo model, any diff big enough to approach the 60k cap will have already tripped escalation (raw_len > 30k) and jumped to the 400k cap first. Net effect: the 60k default cap only ever actually truncates for env-pinned models. If that's the intent, cool; if you expected the default model to sometimes send up to ~60k, it won't.- For non-kilo backends there's no escalation branch, so this line pins them to 60k regardless of an explicitly chosen high-context model. Large commits on
github-models/copilotget quietly truncated with no way to opt into the bigger cap.Can you confirm both are the desired behavior? Happy to be wrong here — just want to make sure the tiers line up with intent.
1035-1071: LGTM!
| omitted = total_files - len(kept_sections) | ||
| final = stat_block + "".join(kept_sections) | ||
| if omitted or line_boundary_cut: | ||
| final += ( | ||
| f"\n\n[diff truncated: {omitted} of {total_files} files omitted to fit context budget]" | ||
| ) | ||
| return final |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The truncation note fibs when it does a line-boundary cut.
When the first file section alone blows the budget you cut it mid-file and stash the partial slice in kept_sections. That means omitted = total_files - len(kept_sections) counts the partially-shown file as fully kept. Concrete fallout: a single oversized file (total_files == 1) emits [diff truncated: 0 of 1 files omitted…] even though you very much did chop it. The model reading this prompt is being told "nothing omitted" while staring at a truncated hunk — mixed signals, like a "this is fine" dog in a burning room.
Call the partial case out explicitly so the count stays honest:
🔧 Proposed marker fix
omitted = total_files - len(kept_sections)
final = stat_block + "".join(kept_sections)
- if omitted or line_boundary_cut:
- final += (
- f"\n\n[diff truncated: {omitted} of {total_files} files omitted to fit context budget]"
- )
+ if line_boundary_cut:
+ final += (
+ f"\n\n[diff truncated: first file shown partially; "
+ f"{total_files - 1} of {total_files} files omitted to fit context budget]"
+ )
+ elif omitted:
+ final += (
+ f"\n\n[diff truncated: {omitted} of {total_files} files omitted to fit context budget]"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| omitted = total_files - len(kept_sections) | |
| final = stat_block + "".join(kept_sections) | |
| if omitted or line_boundary_cut: | |
| final += ( | |
| f"\n\n[diff truncated: {omitted} of {total_files} files omitted to fit context budget]" | |
| ) | |
| return final | |
| omitted = total_files - len(kept_sections) | |
| final = stat_block + "".join(kept_sections) | |
| if line_boundary_cut: | |
| final += ( | |
| f"\n\n[diff truncated: first file shown partially; " | |
| f"{total_files - 1} of {total_files} files omitted to fit context budget]" | |
| ) | |
| elif omitted: | |
| final += ( | |
| f"\n\n[diff truncated: {omitted} of {total_files} files omitted to fit context budget]" | |
| ) | |
| return final |
🤖 Prompt for 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.
In `@scripts/git-ai-commit` around lines 241 - 247, The truncation summary in the
diff builder is inaccurate when a file is only partially included via the
line-boundary cut path. Update the logic around the `kept_sections` / `omitted`
calculation in `scripts/git-ai-commit` so a partial section is counted as
truncated rather than fully kept, and make the final marker message reflect that
one or more files were cut mid-file. Keep the fix localized to the truncation
branch that sets `final` and appends the `[diff truncated: ...]` note.
| def test_single_oversized_file_line_boundary(self) -> None: | ||
| """Single file section larger than budget is truncated at line boundary.""" | ||
| stat_block = " git diff --cached --stat -p\n\n" | ||
| # Build a large file section with many lines | ||
| lines = [f"+line {i}\n" for i in range(100)] | ||
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) | ||
| diff = stat_block + file_section | ||
|
|
||
| # Budget: enough for stat block + the header line + a couple of content lines, | ||
| # but far less than the full file section. | ||
| budget = ( | ||
| len(stat_block) | ||
| + len("diff --git a/large.py b/large.py\n") | ||
| + len("+line 0\n") | ||
| + len("+line 1\n") | ||
| ) | ||
| result = _M._truncate_diff(diff, budget) | ||
|
|
||
| # Result should be shorter than the original diff (truncation occurred) | ||
| assert len(result) < len(diff) | ||
| # Should contain the truncation marker even for a single-file fallback cut | ||
| assert "files omitted to fit context budget" in result | ||
| # All content lines (non-marker) should be from the original diff (no mid-line cuts) | ||
| marker = "[diff truncated:" | ||
| for line in result.split("\n"): | ||
| if line and marker not in line: | ||
| assert line in diff | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
This test doesn't actually exercise the line-boundary fallback it's named for.
"diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) (Line 721) uses the header as a separator between 100 tiny lines, not as a single prefix for one big body. That produces ~99 distinct diff --git sections rather than one oversized section.
Per _truncate_diff (scripts/git-ai-commit:195-247), the line-boundary fallback only fires if not kept_sections — i.e., only when the first file section by itself blows the budget. With the current construction, the first synthetic section fits within the computed budget, so the normal multi-file truncation path runs instead, and the fallback branch this test is named for never actually executes. All the assertions (shorter result, marker present, no mid-line cuts) still pass — just under the wrong code path — so this is coverage theater for the one genuinely tricky bit of _truncate_diff.
🔧 Proposed fix: build one real oversized section
- # Build a large file section with many lines
- lines = [f"+line {i}\n" for i in range(100)]
- file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines)
- diff = stat_block + file_section
+ # Build a single, genuinely oversized file section
+ body = "".join(f"+line {i}\n" for i in range(100))
+ file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n" + body
+ diff = stat_block + file_section📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_single_oversized_file_line_boundary(self) -> None: | |
| """Single file section larger than budget is truncated at line boundary.""" | |
| stat_block = " git diff --cached --stat -p\n\n" | |
| # Build a large file section with many lines | |
| lines = [f"+line {i}\n" for i in range(100)] | |
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) | |
| diff = stat_block + file_section | |
| # Budget: enough for stat block + the header line + a couple of content lines, | |
| # but far less than the full file section. | |
| budget = ( | |
| len(stat_block) | |
| + len("diff --git a/large.py b/large.py\n") | |
| + len("+line 0\n") | |
| + len("+line 1\n") | |
| ) | |
| result = _M._truncate_diff(diff, budget) | |
| # Result should be shorter than the original diff (truncation occurred) | |
| assert len(result) < len(diff) | |
| # Should contain the truncation marker even for a single-file fallback cut | |
| assert "files omitted to fit context budget" in result | |
| # All content lines (non-marker) should be from the original diff (no mid-line cuts) | |
| marker = "[diff truncated:" | |
| for line in result.split("\n"): | |
| if line and marker not in line: | |
| assert line in diff | |
| def test_single_oversized_file_line_boundary(self) -> None: | |
| """Single file section larger than budget is truncated at line boundary.""" | |
| stat_block = " git diff --cached --stat -p\n\n" | |
| # Build a single, genuinely oversized file section | |
| body = "".join(f"+line {i}\n" for i in range(100)) | |
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n" + body | |
| diff = stat_block + file_section | |
| # Budget: enough for stat block + the header line + a couple of content lines, | |
| # but far less than the full file section. | |
| budget = ( | |
| len(stat_block) | |
| len("diff --git a/large.py b/large.py\n") | |
| len("+line 0\n") | |
| len("+line 1\n") | |
| ) | |
| result = _M._truncate_diff(diff, budget) | |
| # Result should be shorter than the original diff (truncation occurred) | |
| assert len(result) < len(diff) | |
| # Should contain the truncation marker even for a single-file fallback cut | |
| assert "files omitted to fit context budget" in result | |
| # All content lines (non-marker) should be from the original diff (no mid-line cuts) | |
| marker = "[diff truncated:" | |
| for line in result.split("\n"): | |
| if line and marker not in line: | |
| assert line in diff |
🤖 Prompt for 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.
In `@scripts/test/test_git_ai_commit.py` around lines 716 - 743, The test named
test_single_oversized_file_line_boundary is building many small diff sections
instead of one oversized file section, so it never hits the single-section
line-boundary fallback in _truncate_diff. Update the fixture in
test_single_oversized_file_line_boundary to construct one real diff --git
section with a large body that exceeds the budget on its own, so _truncate_diff
exercises the kept_sections == false fallback path. Keep the existing
assertions, but make sure the test data targets the fallback branch in
_truncate_diff rather than the normal multi-file truncation path.
| def test_default_cap_truncates_large_diff(self, tmp_path: Path) -> None: | ||
| """Large diff (> 60 000 chars) is truncated with default max_diff_chars.""" | ||
| stat_block = " git diff --cached --stat -p\n\n" | ||
| # Build a diff larger than 60 000 chars | ||
| lines = [f"+line {i} with some padding to make it longer\n" for i in range(1500)] | ||
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) | ||
| diff = stat_block + file_section | ||
| assert len(diff) > 60_000 | ||
|
|
||
| msgs = _build_messages(diff, "", "", "", tmp_path) | ||
| user_content = msgs[1]["content"] | ||
|
|
||
| # Should contain truncation marker | ||
| assert "files omitted to fit context budget" in user_content | ||
|
|
||
| def test_escalated_cap_keeps_large_diff(self, tmp_path: Path) -> None: | ||
| """Large diff fits within _MAX_DIFF_CHARS_ESCALATED (400 000).""" | ||
| stat_block = " git diff --cached --stat -p\n\n" | ||
| # Build a diff larger than 60 000 but smaller than 400 000 | ||
| lines = [f"+line {i} with some padding to make it longer\n" for i in range(1500)] | ||
| file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) | ||
| diff = stat_block + file_section | ||
| assert 60_000 < len(diff) < 400_000 | ||
|
|
||
| msgs = _build_messages( | ||
| diff, "", "", "", tmp_path, max_diff_chars=_M._MAX_DIFF_CHARS_ESCALATED | ||
| ) | ||
| user_content = msgs[1]["content"] | ||
|
|
||
| # No truncation marker should be present | ||
| assert "files omitted to fit context budget" not in user_content | ||
| # Content should be present | ||
| assert "large.py" in user_content | ||
| assert "line 0" in user_content | ||
|
|
||
| def test_default_arg_unchanged(self) -> None: | ||
| """_build_messages signature has _MAX_DIFF_CHARS_DEFAULT as max_diff_chars default.""" | ||
| import inspect | ||
|
|
||
| sig = inspect.signature(_build_messages) | ||
| param = sig.parameters["max_diff_chars"] | ||
| assert param.default == _M._MAX_DIFF_CHARS_DEFAULT |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Fine, but reuses the same fragile diff-builder pattern.
Same .join()-as-separator trick from Line 721 shows up again at Lines 831-833 and 846-848. It's harmless here since these tests only need to cross the 60k/400k thresholds rather than model a single file, but three near-duplicate diff-builder blocks is a bit of copy/paste creep. Consider extracting a small _make_large_diff(n_lines, stat_block=...) fixture/helper to DRY these up and avoid the same footgun biting a future test.
🤖 Prompt for 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.
In `@scripts/test/test_git_ai_commit.py` around lines 827 - 868, The large-diff
tests in test_default_cap_truncates_large_diff and
test_escalated_cap_keeps_large_diff duplicate the same fragile diff-building
logic using the `.join()` separator trick. Refactor the repeated diff
construction into a small helper such as a local `_make_large_diff(...)`
fixture/helper in test_git_ai_commit.py, and reuse it in both tests so the
threshold assertions remain the same while removing copy/paste drift.
Update the system prompt in scripts/git-ai-commit to enforce the
Conventional Commits v1.0.0 specification.
This ensures that AI-generated commit messages follow a standardized
format, including specific types (feat, fix, etc.), proper handling
of breaking changes, and structured body/footer sections.
See also: https://www.conventionalcommits.org/en/v1.0.0/#specification
Code
scripts/git-ai-committo better manage prompt size by:max_diff_charsthrough prompt construction.--amendbehavior for the no-staged-changes case so the model rewrites the previous commit message.Docs
Tests
_build_messagesbehavior under default and escalated diff caps.