From ad98f95a5496d6371ec7906fba7f9ca5fdf96288 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Tue, 7 Jul 2026 13:06:17 -0500 Subject: [PATCH 1/7] feat: implement context optimization for git-ai-commit - Add hunk-aware diff truncation with whole-file-section preservation - Implement two-tier context caps (60k/400k chars) for token budgeting - Replace post-truncation raw_len calculation with untruncated input length - Skip binary and generated files (minimal glob list, extendable) - Reduce commit history depth from 10 to 5 - Trim system prompt filler while preserving Conventional Commits rules - Update man page to reflect truncation, history depth, and provider fallback - Add context optimization documentation and JSON metadata - Expand test coverage for new context handling logic --- .../git-ai-commit-context-optimization.json | 149 ++++++++ .../git-ai-commit-context-optimization.md | 178 +++++++++ .../git_ai_commit_context_guidelines.md | 133 +++++++ scripts/git-ai-commit | 348 +++++++++++++++--- scripts/man/man1/git-ai-commit.1 | 17 +- scripts/test/test_git_ai_commit.py | 232 +++++++++++- 6 files changed, 1001 insertions(+), 56 deletions(-) create mode 100644 docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.json create mode 100644 docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md create mode 100644 docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md diff --git a/docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.json b/docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.json new file mode 100644 index 000000000..80d0312e4 --- /dev/null +++ b/docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.json @@ -0,0 +1,149 @@ +{ + "goal": "Implement applicable recommendations from docs/dev/git_ai_commit_context_guidelines.md to maximize commit-message quality while minimizing prompt/context bloat in scripts/git-ai-commit, per the resolved decisions in .kilo/plans/1782915491660-git-ai-commit-context-optimization.md (hunk-aware truncation, two-tier caps, raw-length escalation, binary/generated skipping, reduced history depth, prompt trims, tests, and docs).", + "idempotent": false, + "steps": [ + { + "id": "step-1", + "description": "[coder-qwen | default free/open-weight model] Read scripts/git-ai-commit in full to capture exact current structure. Report the precise line ranges and current code for: module constants block (_KILO_MODEL_PINNED_BY_ENV ~130, _MAX_DIFF_CHARS ~134, _ESCALATION_THRESHOLD_CHARS ~141, _ESCALATION_MODEL_KILO ~142), the _SYSTEM prompt (~144), _staged_diff (~212), _recent_log (~228), _build_messages signature and body (~656-704 including the inline truncation at 671-672), and the main() escalation block (~882-925 including the sum(len(...)) computation at ~919). Do NOT edit anything in this step; produce a faithful map of anchors to be used by subsequent edit steps.", + "model": "coder-qwen", + "input": null, + "output": "Verified anchor map: exact current text/line ranges of constants, _SYSTEM, _staged_diff, _recent_log, _build_messages, and main() escalation logic.", + "gate": true, + "parallel": false, + "retry": 0, + "max_retries": 2, + "on_error": "abort", + "self_check": "Confirm all named anchors (_MAX_DIFF_CHARS at ~134, _build_messages at ~656, main escalation sum at ~919) are located and quoted; abort if any is missing since later edits depend on them." + }, + { + "id": "step-2", + "description": "[coder-qwen | default free/open-weight model] Implement Task 1: add a hunk-aware diff-truncation helper `_truncate_diff(diff: str, max_chars: int) -> str` to scripts/git-ai-commit near _build_messages. Behavior: (a) return diff unchanged when len(diff) <= max_chars; (b) preserve the leading `git diff --cached --stat -p` diffstat block, then include whole `diff --git` file sections until the char budget is reached; (c) drop remaining file sections and append a clear marker naming counts, e.g. `\\n\\n[diff truncated: N of M files omitted to fit context budget]`; (d) if a single file section alone exceeds the budget, fall back to a line-boundary (never mid-line) cut within that section plus the marker. Split file sections on the `diff --git ` boundary. Add explanatory comments. Follow file-formatting rules: exactly one trailing newline, no trailing whitespace, ruff 99-char limit / double quotes. Do not yet wire it into _build_messages.", + "model": "coder-qwen", + "input": "step-1 anchor map", + "output": "New `_truncate_diff` helper added with hunk/file-boundary logic, stat-block preservation, single-oversized-file line-boundary fallback, and omitted-count marker.", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "abort", + "self_check": "python3 -c \"from importlib.machinery import SourceFileLoader; m=SourceFileLoader('g','scripts/git-ai-commit').load_module(); assert callable(m._truncate_diff)\" — module imports and helper is callable." + }, + { + "id": "step-3", + "description": "[coder-qwen | default free/open-weight model] Implement Task 2: replace the single `_MAX_DIFF_CHARS = 60_000` constant with two module constants: `_MAX_DIFF_CHARS_DEFAULT = 60_000` (Haiku/non-escalated path) and `_MAX_DIFF_CHARS_ESCALATED` set to a large ceiling matched to Qwen's practical context (effectively a no-truncation safety net; choose a value like 400_000 and justify in a comment). Add explanatory comments describing the two-tier intent. Update any internal reference to the old name. Keep ruff/formatting rules.", + "model": "coder-qwen", + "input": "step-1 anchor map", + "output": "Two-tier cap constants (_MAX_DIFF_CHARS_DEFAULT, _MAX_DIFF_CHARS_ESCALATED) defined with comments; old single constant removed.", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "abort", + "self_check": "grep confirms both new constants exist and the bare `_MAX_DIFF_CHARS =` (without _DEFAULT/_ESCALATED suffix) no longer appears; module still imports." + }, + { + "id": "step-4", + "description": "[coder-qwen | default free/open-weight model] Implement Task 4: add a `max_diff_chars: int = _MAX_DIFF_CHARS_DEFAULT` parameter to `_build_messages` (default preserves current behavior for direct/test callers). Replace the inline slice+marker at ~671-672 with a call to `_truncate_diff(diff, max_diff_chars)`. Ensure the rest of _build_messages (system/user message assembly at ~704) is otherwise unchanged. Keep formatting rules.", + "model": "coder-qwen", + "input": "step-2 helper; step-3 constants; step-1 anchor map", + "output": "_build_messages accepts max_diff_chars (defaulting to _MAX_DIFF_CHARS_DEFAULT) and delegates truncation to _truncate_diff; inline 60k slice removed.", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "abort", + "self_check": "Inspect signature: `_build_messages` has max_diff_chars param; grep shows no remaining `diff[:_MAX_DIFF_CHARS]` inline slice; module imports." + }, + { + "id": "step-5", + "description": "[coder-qwen | default free/open-weight model] Implement Task 3 + Task 4 wiring in main() (~882-925). (a) Compute the escalation decision from the RAW pre-truncation length: build the sum of untruncated diff + status + log + context + instructions lengths BEFORE _build_messages truncates, instead of the current post-truncation `sum(len(m['content']) for m in msgs)`. Preserve existing guards: backend == 'kilo', not model_explicit, not _KILO_MODEL_PINNED_BY_ENV, threshold _ESCALATION_THRESHOLD_CHARS, and keep the existing stderr escalation notice. (b) After resolving the model, select `max_diff_chars = _MAX_DIFF_CHARS_ESCALATED` when the resolved model is the escalation model (_ESCALATION_MODEL_KILO) or a user-pinned/explicit high-context model; otherwise `_MAX_DIFF_CHARS_DEFAULT`. (c) Pass that value into the `_build_messages(...)` call at ~909, reordering so message construction happens after the model/cap is resolved. Do not double-count or regress the env-pin / explicit-model suppression logic. Keep formatting rules.", + "model": "coder-qwen", + "input": "step-4 _build_messages signature; step-3 constants; step-1 anchor map", + "output": "main() decides escalation from raw pre-truncation length, resolves model, then passes the model-appropriate max_diff_chars into _build_messages; guards and stderr notice preserved.", + "gate": false, + "parallel": false, + "retry": 1, + "max_retries": 3, + "on_error": "replan", + "self_check": "grep confirms the escalation condition no longer relies on `sum(len(m['content']) for m in msgs)` for the decision; module imports; a manual reasoning check that guards (kilo/model_explicit/env-pin/threshold) remain intact." + }, + { + "id": "step-6", + "description": "[coder-qwen | default free/open-weight model] Implement Task 5: skip binary/generated files in the diff. Add a small commented constant listing generated-file glob patterns (e.g. *.pb.cc, *.pb.h, *.gen.cpp and similar). Before prompting, filter `diff --git` sections that are either git binary sections (contain `Binary files ... differ`) or match a generated pattern, replacing each removed section with a one-line note so the model knows a file changed (e.g. `[skipped binary/generated file: ]`). Confirm no conflict with the `--stat` block (the stat line for skipped files may remain — that is acceptable). Integrate this filtering so it runs before/within truncation (apply filtering to the raw diff prior to _truncate_diff). Keep formatting rules.", + "model": "coder-qwen", + "input": "step-2 helper; step-4/step-5 wiring; step-1 anchor map", + "output": "Binary and generated-pattern diff sections are replaced with a one-line note; generated-pattern constant added; stat block left intact.", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "continue", + "self_check": "Unit-level reasoning: a synthetic diff containing a `Binary files a/x b/x differ` section and a `*.pb.cc` section yields notes, not full hunks; module imports." + }, + { + "id": "step-7", + "description": "[coder-qwen | default free/open-weight model] Implement Task 6: reduce default commit-history depth in `_recent_log` (~228) from `-10` to `-5`. Update the in-code docstring reference to `git log --oneline -10` accordingly. OPTIONAL (only if low-risk and cleanly threadable): add a `--history-depth N` argparse flag threaded into `_recent_log(depth)`, defaulting to 5, and update argparse help text and the module docstring. If the flag is omitted, add a brief comment noting it as a deliberate simplification. Do not break existing callers. Keep formatting rules.", + "model": "coder-qwen", + "input": "step-1 anchor map", + "output": "_recent_log default depth reduced to 5 (with docstring updated); optional --history-depth flag added only if clean, otherwise documented as deliberately omitted.", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "continue", + "self_check": "grep confirms `_recent_log` uses -5 (or a depth variable defaulting to 5); if flag added, argparse help and docstring updated; module imports." + }, + { + "id": "step-8", + "description": "[coder-qwen | default free/open-weight model] Implement Task 7: tighten the `_SYSTEM` prompt wording (~144) by removing only redundant/filler phrasing. MUST NOT weaken: Conventional Commits v1.0.0 rules, the 'respond with only the message' constraint, or subject/body/footer formatting requirements. Keep the change minimal (the prompt is already close to minimal-bloat form). Preserve textwrap.dedent structure and formatting rules.", + "model": "coder-qwen", + "input": "step-1 anchor map (_SYSTEM text)", + "output": "_SYSTEM prompt trimmed of filler while fully preserving Conventional Commits conformance, the message-only constraint, and formatting requirements.", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "continue", + "self_check": "Diff review confirms only filler removed; the phrases enforcing Conventional Commits, 'only the message', and subject/body/footer rules remain present; module imports." + }, + { + "id": "step-9", + "description": "[coder-qwen | default free/open-weight model] Implement Task 8: update scripts/test/test_git_ai_commit.py (loaded via SourceFileLoader). Add tests: (a) _truncate_diff under budget returns unchanged; (b) over budget cuts on file/hunk boundary and keeps the --stat header; (c) a single oversized file falls back to a line-boundary cut; (d) the omitted-count marker text is present; (e) escalation ordering: a large RAW diff triggers escalation using pre-truncation length, and the escalated path receives the larger cap (assert via _build_messages output length / absence of truncation marker when max_diff_chars is escalated); (f) binary/generated-file skipping produces the one-line note. Update any existing tests that assumed the old single 60k cap or the old inline marker string `[diff truncated — too large to show in full]`. Preserve _build_messages default-arg behavior so existing signature-based tests still pass where feasible. Keep ruff/formatting rules.", + "model": "coder-qwen", + "input": "steps 2-8 implemented source", + "output": "Test suite extended with truncation, escalation-ordering, and binary/generated-skip tests; obsolete-cap/marker assertions updated; default-arg compatibility preserved.", + "gate": false, + "parallel": false, + "retry": 1, + "max_retries": 3, + "on_error": "replan", + "self_check": "pytest scripts/test/ -v — all tests (existing + new) pass; no reference to the removed inline marker string remains except in intentionally updated assertions." + }, + { + "id": "step-10", + "description": "[coder-qwen | default free/open-weight model] Implement Task 9: update documentation. (a) scripts/man/man1/git-ai-commit.1 — reflect new hunk-aware truncation behavior, reduced history depth (10->5), any new --history-depth flag, and binary/generated-file skipping. (b) The module docstring at the top of scripts/git-ai-commit — sync any option/default changes. (c) docs/dev/git_ai_commit_context_guidelines.md — append a short 'Implementation status' section recording the disposition table from the plan (item 1 Implement-adapted, item 2 Partial, item 3 Reject, item 4 Implement, item 5 Reject, item 6 Partial, item 7 Reject-table/tune-threshold) with done/partial/rejected + rationale. Follow Markdown lint rules (MD012/MD022/MD031/MD032/MD034/MD040) and file-formatting rules (one trailing newline, no trailing whitespace).", + "model": "coder-qwen", + "input": "steps 2-9 implemented behavior; plan disposition table", + "output": "Man page, module docstring, and guidelines doc updated to match new behavior; disposition/Implementation-status section appended.", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "continue", + "self_check": "Man page mentions truncation/history-depth/binary skip; guidelines doc has an 'Implementation status' section covering all 7 report items; markdownlint-style spot check passes." + }, + { + "id": "step-11", + "description": "[coder-qwen | default free/open-weight model] Validation. Run: (1) `pytest scripts/test/ -v` — all tests must pass. (2) `ruff check scripts/git-ai-commit scripts/test/test_git_ai_commit.py` per pyproject.toml (99-char, double quotes) — fix any findings. (3) Manual dry-runs: `git ai-commit --dry-run` on a small diff (no truncation, default model, no escalation); a synthetic large diff over the escalation threshold (confirm escalation fires AND no truncation marker on the escalated path); and a diff containing a binary + a generated file (confirm both are skipped with a note). (4) `PREKCOMMAND=$(command -v prek || command -v pre-commit); $PREKCOMMAND run --all-files` and address hook findings. Report pass/fail per check with the exact commands and key output.", + "model": "coder-qwen", + "input": "all prior steps", + "output": "Validation report: pytest, ruff, three dry-run scenarios, and pre-commit hook results, with fixes applied for any failures.", + "gate": true, + "parallel": false, + "retry": 1, + "max_retries": 3, + "on_error": "replan", + "self_check": "All four checks pass; escalated-path dry-run shows no truncation marker; binary/generated dry-run shows skip notes; gate blocks completion until green." + } + ] +} diff --git a/docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md b/docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md new file mode 100644 index 000000000..20fbbcdcd --- /dev/null +++ b/docs/dev/git-ai-commit-context/git-ai-commit-context-optimization.md @@ -0,0 +1,178 @@ +# git-ai-commit context-optimization plan + +## Goal + +Implement the applicable recommendations from +`docs/dev/git_ai_commit_context_guidelines.md` to maximize commit-message +quality while minimizing unnecessary prompt/context bloat in +`scripts/git-ai-commit`. Treat the report as an idea source, not a spec: the +tool already implements several recommendations and diverges from the report's +generic assumptions (character-based budgets, not tokens; single FNAL +deployment; no full-file inclusion; no doc cache; no issue/PR fetch). + +## Authoritative facts (verified) + +- Tool: `scripts/git-ai-commit` (Python, git-subcommand convention). This repo + copy is authoritative; it is symlinked onto PATH in + `.devcontainer/post-create.sh:57`. The separate personal copy at + `/home/greenc/scripts/git-ai-commit` is **out of scope**. +- Tests: `scripts/test/test_git_ai_commit.py`, loaded via `SourceFileLoader`. + Run in CI by `.github/workflows/python-check.yaml:162` and + `.github/workflows/coverage.yaml:272` (`pytest scripts/test/`). +- Man page: `scripts/man/man1/git-ai-commit.1` (266 lines) — must stay in sync + with CLI/behavior changes. +- Diff source: `_staged_diff` returns `git diff --cached --stat -p` output, so + the string begins with a diffstat block, then per-file `diff --git` hunks. +- Current truncation: `_build_messages` (line ~671) slices the raw diff at + `_MAX_DIFF_CHARS = 60_000` mid-line and appends + `[diff truncated — too large to show in full]`. +- Current escalation: in `main()` (line ~915), if `backend == "kilo"`, model + not explicit, not pinned by env, and summed message length > + `_ESCALATION_THRESHOLD_CHARS = 30_000`, switch model to + `_ESCALATION_MODEL_KILO = "qwen/qwen3-coder-next"`. Bug: this sums the + **already-truncated** message content, and truncation runs unconditionally at + 60k regardless of the resolved (escalated) model. +- No full-file content inclusion, no `.git-ai-commit-cache.json`, no GitHub + issue/PR fetch, no style-config reading exist today. + +## Resolved decisions + +1. **Scope:** selective/pragmatic. Implement report items that improve message + quality or reduce genuine bloat; explicitly reject the rest with rationale. +2. **Truncation:** escalate-first, truncate-rarely. When escalation triggers + (or a high-context model is otherwise in use), raise/disable the char cap so + the model sees the full diff; on the non-escalated (default Haiku) path keep + a cap and cut on **hunk boundaries** with a clear marker. +3. **Escalation ordering:** two-tier cap driven by the resolved model. + `main()` decides escalation from the **raw (pre-truncation)** prompt length, + then passes a model-appropriate cap into `_build_messages`. +4. **History window:** reduce the default `git log` depth (report item 4). +5. **Prompt wording:** only trims that lose no Conventional Commits conformance + (report item 6). + +## Report item disposition (record in the report doc) + +| # | Report recommendation | Disposition | Rationale | +| --- | --- | --- | --- | +| 1 | Diff-size gating, hunk truncation, `--full-diff` | Implement (adapted) | Char-based, no tokenizer dep; hunk-aware cut + escalation-aware cap | +| 2 | Conditional file content; skip binary/generated; whitelist | Partial | Tool never includes full files; add binary/generated-file diff skipping | +| 3 | Cache static docs in `.git-ai-commit-cache.json` | Reject | Per-invocation CLI, no session; cache adds staleness + bloat; AGENTS.md dedup already limits volume | +| 4 | Limit commit-history window (`--history-depth`, default 3) | Implement | Cheap, direct bloat reduction | +| 5 | Smart issue/PR pulling | Reject | Feature absent; adds network + bloat for marginal gain | +| 6 | Prompt-wording tweaks | Partial | Trim only where conformance preserved | +| 7 | Model-window table / `--max-tokens` | Reject (table); tune threshold only | Single FNAL deployment; full table is overkill and bloats config | + +## Implementation tasks (ordered) + +1. **Add a hunk-aware diff-truncation helper.** + - New function (e.g. `_truncate_diff(diff: str, max_chars: int) -> str`). + - Preserve the leading `--stat` block, then include whole `diff --git` + file sections until the budget is reached; drop remaining files and append + a clear marker naming how many files/sections were omitted, e.g. + `\n\n[diff truncated: N of M files omitted to fit context budget]`. + - If a single file section alone exceeds the budget, fall back to a + line-boundary cut within that section (never mid-line) plus the marker. + - Return the diff unchanged when under budget. + +2. **Introduce a two-tier character cap.** + - Replace the single `_MAX_DIFF_CHARS` with two constants, e.g. + `_MAX_DIFF_CHARS_DEFAULT` (keep ~60_000 for the Haiku path) and + `_MAX_DIFF_CHARS_ESCALATED` (large ceiling matched to Qwen's practical + context, or effectively "no truncation" safety net). + - Keep values as module constants with explanatory comments. + +3. **Make escalation decision use raw (pre-truncation) length.** + - In `main()`, compute the escalation decision from the untruncated diff + + instructions + context length **before** truncation is applied. + - Preserve existing guards: `backend == "kilo"`, `not model_explicit`, + `not _KILO_MODEL_PINNED_BY_ENV`, threshold `_ESCALATION_THRESHOLD_CHARS`. + - Keep the existing stderr escalation notice. + +4. **Pass the resolved cap into `_build_messages`.** + - Add a `max_diff_chars` parameter to `_build_messages` (default preserves + current behavior for direct/test callers). + - `main()` selects `_MAX_DIFF_CHARS_ESCALATED` when the resolved model is the + escalation model or a user-pinned/explicit high-context model; otherwise + `_MAX_DIFF_CHARS_DEFAULT`. + - `_build_messages` calls `_truncate_diff(diff, max_diff_chars)` instead of + the inline slice. + +5. **Skip binary/generated files in the diff (report item 2).** + - Filter or exclude binary-file sections (git marks these + `Binary files ... differ`) and known generated patterns + (`*.pb.cc`, `*.pb.h`, `*.gen.cpp`, etc.) from the diff before prompting, + replacing each with a one-line note so the model knows a file changed. + - Keep the exclusion list as a small, commented constant. Confirm no + conflict with `--stat` (the stat line for skipped files may remain). + +6. **Reduce default commit-history depth (report item 4).** + - Change `_recent_log` from `-10` to a smaller default (recommended `-5`; + report suggested 3). Update the man page and the docstring reference to + `git log --oneline -10`. + - Optional (only if low-risk): add a `--history-depth N` flag threaded into + `_recent_log`. If added, update argparse, help text, docstring, and man + page. If omitted, note it as a deliberate simplification. + +7. **Tighten `_SYSTEM` wording where conformance is preserved (report item 6).** + - Remove redundant/filler phrasing only. Do not weaken the Conventional + Commits v1.0.0 rules, the "respond with only the message" constraint, or + the subject/body/footer formatting requirements. + - Keep the change minimal; the existing prompt is already close to the + report's suggested minimal-bloat form. + +8. **Update tests (`scripts/test/test_git_ai_commit.py`).** + - New tests for `_truncate_diff`: under budget returns unchanged; over budget + cuts on file/hunk boundary and keeps the `--stat` header; single oversized + file falls back to line-boundary cut; marker text present. + - New tests for escalation ordering: a large raw diff triggers escalation + using pre-truncation length, and the escalated path receives the larger cap + (assert via `_build_messages` output length / marker absence). + - New tests for binary/generated-file skipping. + - Update any existing tests that assumed the old single 60k cap or the old + inline marker string. Keep `_build_messages` default-arg behavior so + existing signature-based tests still pass where feasible. + +9. **Update documentation.** + - `scripts/man/man1/git-ai-commit.1`: reflect new truncation behavior, + reduced history depth, any new flag, and binary/generated skipping. + - Script module docstring (top of `scripts/git-ai-commit`): sync any option + or default changes. + - `docs/dev/git_ai_commit_context_guidelines.md`: append a short + "Implementation status" section recording the disposition table above + (done / partial / rejected + rationale) so the report reflects reality. + +## Validation + +- `pytest scripts/test/ -v` (matches CI in `python-check.yaml` and + `coverage.yaml`). All existing tests must still pass. +- `ruff` per `pyproject.toml` (99-char limit, double quotes) on the edited + script and tests. +- Manual dry-run checks (`git ai-commit --dry-run`) on: + - a small diff (no truncation, default model, no escalation), + - a synthetic large diff (> escalation threshold) confirming escalation fires + **and** the full diff is sent (no truncation marker on the escalated path), + - a diff containing a binary and a generated file (confirm they are skipped + with a note). +- `PREKCOMMAND=$(command -v prek || command -v pre-commit)`; run + `$PREKCOMMAND run --all-files` before opening a PR. + +## Risks / notes + +- The `--stat -p` header interacts with hunk truncation: keep the stat block + intact so the model still sees the full file list even when hunks are cut. +- Escalation currently measures summed message content; moving the decision to + raw pre-truncation length must not double-count or regress the existing + env-pin / explicit-model suppression logic. +- File-formatting rules (global AGENTS.md): exactly one trailing newline, no + trailing whitespace. +- Implementation requires source edits and running tests — switch to an + implementation-capable agent to execute this plan. + +## Deliberately out of scope + +- Token-based budgeting / tokenizer dependency (kept character-based). +- `.git-ai-commit-cache.json` session cache. +- GitHub issue/PR fetching. +- Per-model context-window table / `--max-tokens`. +- Reading language style configs (`.clang-format`, `pyproject.toml`, etc.). +- The personal copy at `/home/greenc/scripts/git-ai-commit`. diff --git a/docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md b/docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md new file mode 100644 index 000000000..a929223ff --- /dev/null +++ b/docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md @@ -0,0 +1,133 @@ +# Git‑AI‑Commit Context Guidelines + +## Original prompt + +Please evaluate the various sources of context to git-ai-commit for relevance/usefulness, and suggest adjustments to context inclusions or prompt wording to minimize context bloat while preserving or enhancing prompt effectiveness with various models and context window sizes. + +## Evaluation of current context sources + +| Context source | Typical size | Relevance to commit‑message generation | Typical usefulness | Comments (bloat risk) | +| --- | --- | --- | --- | --- | +| **Staged diff (`git diff --staged`)** | 0 – few KB | Highest – gives concrete code changes | essential | Can explode for large refactors; diff‑size‑based truncation is needed | +| **Full file contents of changed files** | 0 – several MB | Medium – helps model see surrounding code, definitions, imports | useful for non‑trivial changes (API tweaks, new types) | Heavy for binary files or huge generated sources | +| **Commit history (last N commits)** | 0 – few KB | Low‑medium – provides project‑wide style, recent terminology | useful for style consistency, detecting reused ticket IDs | Older history quickly becomes irrelevant; limit to “last 5” | +| **Branch name / PR number** | < 100 B | Low – can surface ticket ID, feature name | helpful for including ticket numbers, short context | negligible size | +| **Issue/PR description (GitHub API)** | 0 – few KB | Medium – clarifies intent, scope | valuable when present | Only fetch when `--issue` flag is used | +| **Project‑wide guidelines (README, CONTRIBUTING, `.gitmessage`, `kilo.json`)** | up to tens KB | Low‑medium – informs preferred tone, prefixes (e.g., “feat:”, “fix:”) | useful for enforcing conventions | Most of the file is static; cache and insert only once per session | +| **Language‑specific style configs (`.clang‑format`, `pyproject.toml`, `ruff.toml`)** | < 5 KB | Low – rarely needed for the message itself | seldom used | Can be omitted unless explicitly requested | +| **Recent CI status / test failures** | < 2 KB | Low – may affect urgency wording | occasional | Only useful for “WIP” or “hot‑fix” prompts | +| **User‑defined prompt template** | < 1 KB | High – drives tone, length, ticket format | critical | Fixed size; no bloat | + +### Key observations + +1. Diff is the single most valuable source. +2. Full file contents add marginal benefit and become a major source of bloat for large files. +3. Static project‑wide docs and style configs contribute almost no incremental information per commit; they can be cached once per session. +4. Branch/issue metadata is tiny but highly effective for ticket IDs. +5. Commit history beyond the last 3–5 entries rarely influences the current message. + +--- + +## Recommendations to shrink context while keeping effectiveness + +### 1. Diff‑size gating + +- If staged diff > `MAX_DIFF_TOKENS` (≈ 2 k tokens / ~4 KB), truncate to the first `N` hunks (default 10) and add a note: “*diff truncated – see remaining changes in file X*”. +- Provide a `--full-diff` flag for rare cases where the full diff is required. + +### 2. Conditional file‑content inclusion + +- Include only the *closest* surrounding lines (± 5–10 lines) around each changed hunk, not the whole file. +- Skip binary or generated files (`.pb.cc`, `.pb.h`, `.gen.cpp`, etc.) unless the user explicitly asks. +- Use a file‑type whitelist (`.cpp`, `.hpp`, `.py`, `.jsonnet`, `.cmake`). + +### 3. Cache static project context + +- Load once per session: `README.md`, `CONTRIBUTING.md`, `kilo.json`, `.gitmessage`. +- Pass a reference token (e.g., `<>`) that the model expands internally via a stored embedding or short summary. + +### 4. Limit commit‑history window + +- Default to **last 3 commits**; expose a `--history-depth=N` flag. +- Drop merges and revert commits unless they contain the same ticket ID. + +### 5. Smart issue/PR pulling + +- Only request issue/PR body when a ticket reference is detected in branch name or diff. +- Provide a short excerpt (first 200 chars) plus a “more” flag if the user wants the full description. + +### 6. Prompt wording tweaks to reduce token waste + +| Current wording (example) | Suggested wording | +| --- | --- | +| “Generate a conventional commit message for the following changes. Include a brief summary, a detailed body, and a footer with any relevant issue numbers.” | **“Write a conventional‑commit message for the diff below. Use a one‑line summary, optional body, and footer if an issue ID is present.”** | +| “Consider the project’s style guidelines, recent commit history, and the current branch name when forming the message.” | **“Follow the project’s conventional‑commit style. Use the branch name for any ticket ID.”** | +| “If the diff is large, you may truncate it but indicate that you omitted sections.” | **“When diff > 2 k tokens, keep the first 10 hunks and add ‘…truncated’.”** | + +- Remove filler words (e.g., “please”, “kindly”). +- Use concrete limits instead of vague adjectives. + +### 7. Model‑specific context windows + +| Model size | Max diff tokens + surrounding lines | History depth | +| --- | --- | --- | +| 4 k tokens (gpt‑3.5‑turbo) | ≤ 1 k diff tokens (≈ 2 KB) | last 2 commits | +| 8 k tokens (claude‑2.1) | ≤ 2 k diff tokens | last 3 commits | +| 16 k tokens (gpt‑4‑32k) | ≤ 4 k diff tokens | last 5 commits | +| 32 k tokens+ (Claude‑3‑Sonnet) | ≤ 6 k diff tokens | last 7 commits | + +- Expose a `--max‑tokens` option that defaults based on detected model. + +## Concrete adjustment plan for `git‑ai‑commit` + +1. Add a “context profile” enum (`tiny`, `standard`, `full`). +2. Implement diff‑truncation routine that keeps first `MAX_HUNKS` (default 10) or until token budget is hit, appending `…diff truncated…`. +3. Cache static docs in `.git‑ai‑commit‑cache.json` on first run; reuse on subsequent invocations. +4. Make issue/PR lookup lazy – only if a pattern like `([A-Z]+‑\d+)` appears. +5. Refine the default prompt template (see wording table) and allow user‑override via `--prompt‑template`. +6. Add CLI flags: `--diff-lines N`, `--history-depth N`, `--include-files PATTERN`, `--exclude‑binary`, `--max‑tokens N`. +7. Document model‑aware defaults in the README and `kilo.json` command config. + +### Minimal‑bloat default prompt (example) + +```text +Write a conventional‑commit message for the staged changes below. +Use a one‑line summary (≤ 50 chars). Add a body only if the diff +contains multiple logical parts. Include a footer with the ticket +ID if the branch name contains one (e.g. feat/1234‑...). + + +``` + +All other context (project guidelines, recent commits, full issue body) is omitted unless the corresponding flags are supplied. + +--- + +### Functional conformance check (summary) + +- The diff remains the primary source; truncation only adds a clear marker, preserving spec compliance. +- Ticket extraction, optional body, and footer handling continue to follow Conventional Commits 1.0. +- Style‑guide caching does not affect the required format. +- Prompt wording explicitly demands a **conventional‑commit** header, which is exactly what the spec requires. +- Therefore the suggested changes maintain full functional conformance while reducing token usage. + +--- + +### Implementation status + +The following table records the disposition of each recommendation from this +document as implemented in `scripts/git-ai-commit`. + +| # | Recommendation | Disposition | Notes | +| --- | --- | --- | --- | +| 1 | Hunk-aware diff truncation | Implemented | `_truncate_diff` preserves whole file sections and falls back to line-boundary cut; omission marker includes file counts. | +| 2 | Two-tier context caps | Implemented | `_MAX_DIFF_CHARS_DEFAULT = 60 000` (default); `_MAX_DIFF_CHARS_ESCALATED = 400 000` (escalated path). | +| 3 | Rewrite escalation to use raw length | Implemented | `raw_len` computed from untruncated inputs; replaces post-truncation `sum(len(m["content"]))` heuristic. | +| 4 | Binary and generated-file skipping | Implemented (partial) | Binary sections and files matching `_GENERATED_FILE_GLOBS` are replaced with a one-line note. Glob list is deliberately minimal; extend as needed. | +| 5 | Reduce commit-history depth | Implemented | `_recent_log` depth reduced from 10 to 5. No `--history-depth` flag added (kept simple). | +| 6 | Trim `_SYSTEM` prompt filler | Implemented (partial) | Removed one redundant filler sentence; essential Conventional Commits rules and the "message only" constraint preserved. | +| 7 | Reject table-driven model routing | Rejected | Auto-escalation threshold retained as-is; two-tier caps provide the necessary headroom without adding routing complexity. | + +--- + +*Generated by the Kilo `git‑ai‑commit` context‑optimization work.* diff --git a/scripts/git-ai-commit b/scripts/git-ai-commit index 99cea172a..4dc389d5d 100755 --- a/scripts/git-ai-commit +++ b/scripts/git-ai-commit @@ -6,7 +6,8 @@ Analyses the Git index of the current repository and generates a detailed, informative commit message using a language model, then optionally commits. This tool automatically gathers context from: - - Staged changes (diff) + - Staged changes (diff, with binary and generated files skipped and hunk-aware + truncation applied) - Git status and recent commit history - Repository-level guidance: AGENTS.md, AGENT.md, GEMINI.md, CLAUDE.md, CONTEXT.md, INSTRUCTIONS.md, CONTRIBUTING.md, HACKING.md, @@ -31,11 +32,11 @@ Options --context TEXT Supplemental context/prompt for the AI --backend BACKEND API backend: kilo (default), github-models, copilot, antigravity, agy - --model MODEL Model name; defaults to azure/claude-haiku-4-5 for the - kilo backend (gpt-4o for others, none for antigravity); - large prompts auto-escalate to qwen/qwen3-coder-next on - kilo unless a model is explicitly set (env: - GIT_AI_COMMIT_MODEL or GIT_AI_COMMIT_KILO_MODEL) + --model MODEL Model name; defaults to azure/claude-haiku-4-5 for the + kilo backend (gpt-4o for others, none for antigravity); + large prompts auto-escalate to qwen/qwen3-coder-next; + binary and generated files are skipped (env: + GIT_AI_COMMIT_MODEL or GIT_AI_COMMIT_KILO_MODEL) --api URL API base URL override (env: GIT_AI_COMMIT_API) --dry-run Print generated message to stdout; do not commit -y/--yes Commit without interactive confirmation @@ -77,11 +78,12 @@ Environment variables GIT_AI_COMMIT_KILO_API kilo backend base URL (default: https://litellm.fnal.gov/v1) GIT_AI_COMMIT_KILO_MODEL kilo backend default model (default: azure/claude-haiku-4-5); setting this env var suppresses auto-escalation - GIT_AI_COMMIT_KILO_PROVIDER kilo auth.json provider key (default: fnal-litellm) + GIT_AI_COMMIT_KILO_PROVIDER kilo auth.json provider key. When unset, the + split FNAL providers are tried in order (fnal-azure, fnal-ow, fnal-litellm). Token resolution (kilo backend): 1. GIT_AI_COMMIT_TOKEN - 2. ~/.local/share/kilo/auth.json (fnal-litellm key) + 2. ~/.local/share/kilo/auth.json (fnal-azure, fnal-ow, or fnal-litellm key) 3. GITHUB_TOKEN / GH_TOKEN 4. `gh auth token` 5. ~/.config/gh/hosts.yml @@ -128,10 +130,22 @@ _DEFAULT_MODEL_KILO = _KILO_MODEL_ENV or "azure/claude-haiku-4-5" # True when the user has explicitly pinned a kilo model via the environment, # which suppresses auto-escalation even if the diff is large. _KILO_MODEL_PINNED_BY_ENV = bool(_KILO_MODEL_ENV) -_DEFAULT_KILO_PROVIDER = os.environ.get("GIT_AI_COMMIT_KILO_PROVIDER", "fnal-litellm") +# auth.json provider key(s) that hold the FNAL gateway API token. The former +# single "fnal-litellm" provider was split into "fnal-azure" and "fnal-ow", +# which share the same upstream key; "fnal-litellm" is retired. When the user +# does not pin GIT_AI_COMMIT_KILO_PROVIDER, try the new keys in order (and the +# retired name last, for any un-migrated auth.json). git-ai-commit talks to +# the LiteLLM gateway directly, so the provider key only selects the auth +# token — model names remain bare (e.g. "azure/...", "qwen/..."). +_KILO_PROVIDER_ENV = os.environ.get("GIT_AI_COMMIT_KILO_PROVIDER", "").strip() +_DEFAULT_KILO_PROVIDER = _KILO_PROVIDER_ENV or "fnal-azure" +_KILO_PROVIDER_FALLBACKS = ( + [_KILO_PROVIDER_ENV] if _KILO_PROVIDER_ENV else ["fnal-azure", "fnal-ow", "fnal-litellm"] +) _DEFAULT_MODEL_OTHER = "gpt-4o" -_MAX_DIFF_CHARS = 60_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 @@ -141,21 +155,139 @@ _MAX_DIFF_CHARS = 60_000 _ESCALATION_THRESHOLD_CHARS = 30_000 _ESCALATION_MODEL_KILO = "qwen/qwen3-coder-next" +# Hunk-aware diff truncation helper + +# Binary and generated file skipping + +# Patterns for generated files to skip. Adjust as needed. +_GENERATED_FILE_GLOBS = [ + "*.pb.cc", + "*.pb.h", + "*.gen.cpp", + "*.gen.hpp", + "*.generated.cpp", + "*.generated.h", +] + + +def _skip_binary_and_generated(diff: str) -> str: + """Replace binary and generated file sections in *diff* with a placeholder note. + + Binary sections are lines containing "Binary files ... differ". + Generated files are detected via filename patterns in the "diff --git" header. + The placeholder format is ``"[skipped binary/generated file: ]"``. + """ + import fnmatch + import re + + # 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 "" + 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) + + +def _truncate_diff(diff: str, max_chars: int) -> str: + r"""Truncate *diff* to *max_chars* preserving the diffstat and whole file hunks. + + - If the diff length is within the budget, return unchanged. + - Preserve the leading "git diff --cached --stat -p" stat block (the lines before the first + "diff --git"). + - Include full ``diff --git`` file sections until adding the next section would exceed + *max_chars*. + - If a single file section alone exceeds *max_chars*, fall back to cutting at a line boundary + within that section. + - Omitted sections are replaced with a note: + ``"\n\n[diff truncated: N of M files omitted to fit context budget]"``. + """ + if len(diff) <= max_chars: + return diff + + # Split the diff into stat block + individual file sections. + # Use a zero-width lookahead so each file section retains its "diff --git " prefix. + import re as _re + + raw_sections = _re.split(r"(?=^diff --git )", diff, flags=_re.MULTILINE) + # First element is the leading diffstat block (may be empty for a bare diff). + stat_block = raw_sections[0] + file_sections = raw_sections[1:] # each starts with "diff --git " + + total_files = len(file_sections) + kept_sections: list[str] = [] + current_len = len(stat_block) + + line_boundary_cut = False # True when the first section was cut mid-file + for sec in file_sections: + if current_len + len(sec) <= max_chars: + kept_sections.append(sec) + current_len += len(sec) + else: + # If this is the first section and it alone exceeds budget, fall + # back to a line-boundary cut within it so we return something useful. + if not kept_sections: + lines = sec.splitlines(keepends=True) + acc = "" + for line in lines: + if len(stat_block) + len(acc) + len(line) > max_chars: + break + acc += line + if acc: + kept_sections.append(acc.rstrip("\n")) + line_boundary_cut = True + break + + 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 + + _SYSTEM = textwrap.dedent("""\ You are an expert software developer writing Git commit messages. CRITICAL — respond with ONLY the commit message, nothing else. - Do not write any introduction, explanation, or commentary before or - after the message. Do not wrap the message in a markdown code fence. + No preamble, commentary, or markdown fences. Your very first character must be the first character of the subject line. - Message rules: - - Subject line: imperative mood, ≤72 characters, no trailing period; - must be a complete, self-contained sentence (do not let the subject - run past the blank line into the body) - - Separate subject from body with exactly one blank line - - Body: explain *what* changed and *why*; bullet lists or inline code - spans where they genuinely aid clarity, not merely for decoration + ## Conventional Commits v1.0.0 Guidelines + Specification: https://www.conventionalcommits.org/en/v1.0.0/ + + Format: [optional scope][optional !]: + [optional body] + [optional footer(s)] + + - Common types (spec allows additional types): feat, fix, docs, style, refactor, + perf, test, build, ci, chore, revert + - Breaking changes: append '!' directly before ':' (e.g., `feat!:`) OR add + `BREAKING CHANGE: ` as a footer line + - Subject: imperative mood, no trailing period, keep it brief + - Body: Explain *what* changed and *why* (motivation, problem, tradeoffs). + Required only for non-trivial changes. Body and footer each separated from + subject/footer by a blank line. + - Footer format: `Token: value` or `Token# value` (tokens use hyphens instead + of spaces, except BREAKING CHANGE which may use spaces) + - Use bullet points for lists; avoid decorative formatting - Do NOT reproduce the diff verbatim or enumerate every changed file """) @@ -221,8 +353,10 @@ def _status() -> str: def _recent_log() -> str: + # Depth deliberately capped at 5 to keep context concise. + # Increase via a future --history-depth flag if needed. try: - return _git("log", "--oneline", "-10") + return _git("log", "--oneline", "-5") except _Error: return "(no prior commits)" @@ -372,8 +506,11 @@ def _get_instructions(root: Path) -> str: def _kilo_auth_token() -> str | None: """Read the kilo provider API key from kilo's auth.json, if present. - The provider name is taken from GIT_AI_COMMIT_KILO_PROVIDER (default: - "fnal-litellm"), matching the key used in ~/.config/kilo/kilo.jsonc. + The provider name(s) come from GIT_AI_COMMIT_KILO_PROVIDER when set; + otherwise the split FNAL providers are tried in order + ("fnal-azure", "fnal-ow", then the retired "fnal-litellm"). These share + the same upstream gateway token, so the first present key wins. The names + match the provider keys in ~/.config/kilo/kilo.jsonc. """ if not _KILO_AUTH_JSON.exists(): return None @@ -383,13 +520,48 @@ def _kilo_auth_token() -> str | None: return None if not isinstance(data, dict): return None - provider = data.get(_DEFAULT_KILO_PROVIDER) - if not isinstance(provider, dict): - return None - key = provider.get("key", "") - if not isinstance(key, str): - return None - return key.strip() or None + for provider_name in _KILO_PROVIDER_FALLBACKS: + provider = data.get(provider_name) + if not isinstance(provider, dict): + continue + key = provider.get("key", "") + if isinstance(key, str) and key.strip(): + return key.strip() + return None + + +def _max_tokens_for_prompt(prompt_size: int, model: str) -> int: + """Return max_tokens based on prompt size and model capabilities. + + Reasoning models (e.g., qwen/qwen3-coder-next, google/gemma4-31b) + spend significant internal thinking tokens before producing visible + output. A low max_tokens budget can cause them to return empty content + (finish_reason=stop with zero visible text) instead of an error. + + The max_tokens budget must be large enough for: + - System prompt + - User prompt (diff, status, log, instructions, context) + - Expected output (commit message with subject + body) + + For large prompts, increase max_tokens to avoid truncating the response. + This applies to all models, not just reasoning ones. + """ + # Base budget: enough for a decent commit message (subject + body) + base_tokens = 2048 + + # Reasoning models need higher budget to avoid empty responses + if model: + model_lower = model.lower() + if any(kw in model_lower for kw in ("reasoning", "4-5", "qwen3-coder-next", "gemma4-31b")): + return 8912 + + # Scale with prompt size to avoid truncated responses + if prompt_size > 50_000: + return 4096 + elif prompt_size > 25_000: + return 3072 + else: + return base_tokens def _gh_config_token() -> str | None: @@ -462,8 +634,8 @@ def _token(backend: str) -> str: if backend == "kilo": raise _Error( "No FNAL LiteLLM token found. " - "Ensure kilo is configured with the fnal-litellm provider, or " - "set GIT_AI_COMMIT_TOKEN." + "Ensure kilo is configured with the fnal-azure or fnal-ow " + "provider, or set GIT_AI_COMMIT_TOKEN." ) raise _Error( "No API token found. Set GIT_AI_COMMIT_TOKEN, GITHUB_TOKEN, or run 'gh auth login'." @@ -567,12 +739,28 @@ def _chat( messages: list[dict[str, str]], ) -> str: """Send *messages* to the chat completions endpoint and return the reply.""" + # Scale max_tokens with prompt size to avoid truncated responses. + # This applies to all models, not just reasoning ones. + prompt_size = sum(len(m["content"]) for m in messages) + if model: + model_lower = model.lower() + if any(kw in model_lower for kw in ("reasoning", "4-5", "qwen3-coder-next", "gemma4-31b")): + max_tokens = 8912 + elif prompt_size > 50_000: + max_tokens = 4096 + elif prompt_size > 25_000: + max_tokens = 3072 + else: + max_tokens = 2048 + else: + max_tokens = 2048 + payload = json.dumps( { "model": model, "messages": messages, "temperature": 0.2, - "max_tokens": 2048, + "max_tokens": max_tokens, } ).encode() req = urllib.request.Request( @@ -659,28 +847,47 @@ def _build_messages( root: Path, last_msg: str = "", instr: str = "", + amend_no_diff: bool = False, + max_diff_chars: int = _MAX_DIFF_CHARS_DEFAULT, ) -> list[dict[str, str]]: - """Build the system + user message list for the chat completion request.""" - if len(diff) > _MAX_DIFF_CHARS: - diff = diff[:_MAX_DIFF_CHARS] + "\n\n[diff truncated — too large to show in full]" + """Build the system + user message list for the chat completion request. + + If `amend_no_diff` is True, there are no staged changes; the model should rewrite + the previous commit message to conform to Conventional Commits guidelines. + """ + # Apply hunk-aware truncation using the helper. + diff = _truncate_diff(diff, max_diff_chars) parts: list[str] = [ f"Repository: {root}\n", f"## Staged diff (git diff --cached)\n\n```diff\n{diff}\n```", f"\n## Status (git status --short)\n\n```\n{status.strip()}\n```", - f"\n## Recent commits (git log --oneline -10)\n\n```\n{log.strip()}\n```", + f"\n## Recent commits (git log --oneline -5)\n\n```\n{log.strip()}\n```", ] if last_msg: parts.append(f"\n## Previous commit message\n\n```\n{last_msg}\n```") + if amend_no_diff: + parts.append( + "\n## Note\n\nNo staged changes are present. Rewrite the above previous commit " + "message to follow the Conventional Commits format and improve its wording." + ) if instr: parts.append(f"\n## Agent instructions\n\n{instr}") if context: parts.append(f"\n## Supplemental context\n\n{context}") - parts.append( - "\n\nWrite the commit message for the staged changes above." - " Remember: respond with the commit message text only —" - " start directly with the subject line." - ) + if amend_no_diff: + parts.append( + "\n\nRewrite the previous commit message to follow Conventional Commits guidelines." + " Do not simply repeat it; provide an improved version that complies with the format" + " and uses clearer wording." + " Respond with only the revised commit message, starting with the subject line." + ) + else: + parts.append( + "\n\nWrite the commit message for the staged changes above." + " Remember: respond with the commit message text only —" + " start directly with the subject line." + ) return [ {"role": "system", "content": _SYSTEM}, @@ -862,6 +1069,8 @@ def main() -> None: try: root = _git_root() diff = _staged_diff(args.amend) + # Skip binary and generated file sections before any truncation. + diff = _skip_binary_and_generated(diff) status = _status() log = _recent_log() last_msg = _last_commit_message() if args.amend else "" @@ -871,6 +1080,7 @@ def main() -> None: print(f"git-ai-commit: {exc}", file=sys.stderr) sys.exit(1) + amend_no_diff = False if not diff.strip(): if not args.amend: print("Nothing staged. Use 'git add' first.", file=sys.stderr) @@ -884,20 +1094,62 @@ def main() -> None: file=sys.stderr, ) sys.exit(1) + # No staged changes but we have a prior commit message; rewrite it. + amend_no_diff = True + + # Compute raw prompt length before any truncation for escalation decision. + raw_len = ( + len(_SYSTEM) + + len(f"Repository: {root}\n") + + len(diff) + + len(status) + + len(log) + + len(context) + + len(instr) + + len(last_msg) + ) + # 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 + msgs = _build_messages( + diff, + status, + log, + context, + root, + last_msg, + instr, + amend_no_diff, + max_diff_chars=max_diff_chars, + ) - msgs = _build_messages(diff, status, log, context, root, last_msg, instr) - - # Auto-escalate to a free high-context model when the prompt is large and - # the user has not explicitly chosen a model. This avoids degraded output - # (or silent truncation) from Haiku on large diffs without spending budget - # on azure models unnecessarily. + # Auto-escalate to a free high-context model when the raw (pre-truncation) + # prompt is large and the user has not explicitly chosen a model. Using the + # raw length avoids the circular dependency where truncation hides the true + # diff size from the escalation heuristic. if ( backend == "kilo" and not model_explicit and not _KILO_MODEL_PINNED_BY_ENV - and sum(len(m["content"]) for m in msgs) > _ESCALATION_THRESHOLD_CHARS + and raw_len > _ESCALATION_THRESHOLD_CHARS ): model = _ESCALATION_MODEL_KILO + # Upgrade the diff cap now that we know we are on the escalated model. + max_diff_chars = _MAX_DIFF_CHARS_ESCALATED + # Rebuild messages with the larger cap. + msgs = _build_messages( + diff, + status, + log, + context, + root, + last_msg, + instr, + amend_no_diff, + max_diff_chars=max_diff_chars, + ) print( f"Prompt exceeds {_ESCALATION_THRESHOLD_CHARS:,} chars — " f"escalating to {_ESCALATION_MODEL_KILO}.", diff --git a/scripts/man/man1/git-ai-commit.1 b/scripts/man/man1/git-ai-commit.1 index 0600e6a13..fb6da75bd 100644 --- a/scripts/man/man1/git-ai-commit.1 +++ b/scripts/man/man1/git-ai-commit.1 @@ -34,13 +34,14 @@ sent to the language model. Pass \fB\-\-no\-instructions\fR to suppress all sources except the diff, status, and log. .TP \fBStaged diff\fR -Output of \fBgit diff \-\-cached \-\-stat \-p\fR. +Output of \fBgit diff \-\-cached \-\-stat \-p\fR, with hunk\-aware truncation +applied to whole file sections and binary/generated files skipped. .TP \fBGit status\fR Output of \fBgit status \-\-short\fR. .TP \fBRecent commits\fR -Output of \fBgit log \-\-oneline \-10\fR. +Output of \fBgit log \-\-oneline \-5\fR. .TP \fBRepository guidance\fR \fBAGENTS.md\fR, \fBAGENT.md\fR, \fBGEMINI.md\fR, \fBCLAUDE.md\fR, \fBCONTEXT.md\fR, @@ -87,8 +88,8 @@ Model name to use. Defaults to \fBazure/claude\-haiku\-4\-5\fR for the When the \fBkilo\fR backend is selected and the model was not explicitly specified (neither via \fB\-\-model\fR nor via \fBGIT_AI_COMMIT_KILO_MODEL\fR / \fBGIT_AI_COMMIT_MODEL\fR), large prompts (>\~30,000 characters) are -automatically escalated to \fBqwen/qwen3\-coder\-next\fR to avoid silent -truncation. +automatically escalated to \fBqwen/qwen3\-coder\-next\fR; binary and +generated files are skipped. Overridden by the \fBGIT_AI_COMMIT_MODEL\fR environment variable. .TP \fB\-\-api\fR \fIURL\fR @@ -197,7 +198,10 @@ Default model for the \fBkilo\fR backend. Default: \fBazure/claude\-haiku\-4\-5 Setting this variable suppresses auto\-escalation even for large prompts. .TP \fBGIT_AI_COMMIT_KILO_PROVIDER\fR -Provider key in \fB~/.local/share/kilo/auth.json\fR. Default: \fBfnal\-litellm\fR. +Provider key in \fB~/.local/share/kilo/auth.json\fR. When unset, the split +FNAL providers are tried in order: \fBfnal\-azure\fR, then \fBfnal\-ow\fR, then +the retired \fBfnal\-litellm\fR. These share the same upstream gateway token, +so the first key present is used. .TP \fBEDITOR\fR, \fBVISUAL\fR Editor invoked by \fB\-\-edit\fR. Checked in order; \fBvi\fR is the final fallback. @@ -246,7 +250,8 @@ echo "Closes #1234; keep message terse" | git ai\-commit \-\-yes .SH FILES .TP \fB~/.local/share/kilo/auth.json\fR -Kilo credentials file; read for the \fBfnal\-litellm\fR provider key. +Kilo credentials file; read for the \fBfnal\-azure\fR / \fBfnal\-ow\fR provider +key (legacy \fBfnal\-litellm\fR still honored if present). .TP \fB~/.config/git\-ai\-commit/instructions.md\fR User\-level instructions included in every prompt. diff --git a/scripts/test/test_git_ai_commit.py b/scripts/test/test_git_ai_commit.py index cbce95ee0..0f718d1e3 100644 --- a/scripts/test/test_git_ai_commit.py +++ b/scripts/test/test_git_ai_commit.py @@ -67,7 +67,7 @@ class TestKiloAuthToken: """_kilo_auth_token validates the kilo credentials file before reading.""" - _PROVIDER = "fnal-litellm" + _PROVIDER = "fnal-azure" def _write(self, path: Path, obj: object) -> None: path.parent.mkdir(parents=True, exist_ok=True) @@ -75,7 +75,9 @@ def _write(self, path: Path, obj: object) -> None: def _setup(self, monkeypatch: pytest.MonkeyPatch, path: Path) -> None: monkeypatch.setattr(_M, "_KILO_AUTH_JSON", path) - monkeypatch.setattr(_M, "_DEFAULT_KILO_PROVIDER", self._PROVIDER) + # _kilo_auth_token iterates _KILO_PROVIDER_FALLBACKS; pin it to a single + # provider for the single-provider test cases below. + monkeypatch.setattr(_M, "_KILO_PROVIDER_FALLBACKS", [self._PROVIDER]) def test_file_missing_returns_none( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -173,6 +175,42 @@ def test_valid_key_no_whitespace( self._setup(monkeypatch, p) assert _kilo_auth_token() == "tok123" + def test_provider_fallback_order( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """First present provider in the fallback list wins. + + The retired fnal-litellm provider was split into fnal-azure and + fnal-ow; _kilo_auth_token tries the list in order and returns the first + provider that has a usable key, skipping absent earlier entries. + """ + p = tmp_path / "auth.json" + # fnal-azure absent; fnal-ow present -> fnal-ow key is used. + self._write(p, {"fnal-ow": {"key": "ow-token"}}) + monkeypatch.setattr(_M, "_KILO_AUTH_JSON", p) + monkeypatch.setattr( + _M, "_KILO_PROVIDER_FALLBACKS", ["fnal-azure", "fnal-ow", "fnal-litellm"] + ) + assert _kilo_auth_token() == "ow-token" + + def test_provider_fallback_prefers_earlier( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """When multiple providers are present, the earliest in the list wins.""" + p = tmp_path / "auth.json" + self._write( + p, + { + "fnal-azure": {"key": "azure-token"}, + "fnal-ow": {"key": "ow-token"}, + }, + ) + monkeypatch.setattr(_M, "_KILO_AUTH_JSON", p) + monkeypatch.setattr( + _M, "_KILO_PROVIDER_FALLBACKS", ["fnal-azure", "fnal-ow", "fnal-litellm"] + ) + assert _kilo_auth_token() == "azure-token" + # =========================================================================== # _gh_cli_token @@ -676,3 +714,193 @@ def test_main_with_antigravity_backend( # Check the second call (git commit) git_cmd = mock_run.call_args_list[1][0][0] assert git_cmd == ["git", "commit", "-m", "feat: mock commit message"] + + +# =========================================================================== +# _truncate_diff +# =========================================================================== + + +class TestTruncateDiff: + """Tests for _truncate_diff, the hunk-aware diff truncation helper.""" + + def test_under_budget_returns_unchanged(self) -> None: + """Diff shorter than max_chars is returned unchanged.""" + diff = "diff --git a/foo.py b/foo.py\n+new line\n" + result = _M._truncate_diff(diff, 1000) + assert result == diff + + def test_over_budget_cuts_on_file_boundary(self) -> None: + """Diff with multiple files is cut on file boundary; omission marker present.""" + # Diff with two file sections; first will fit, second will not + stat_block = " git diff --cached --stat -p\n\n" + file1 = "diff --git a/aaa.py b/aaa.py\nnew file mode 100644\n+line 1\n+line 2\n" + file2 = "diff --git a/bbb.py b/bbb.py\nnew file mode 100644\n+line a\n+line b\n" + diff = stat_block + file1 + file2 + # Budget that fits stat + file1 exactly, but not file2 on top + budget = len(stat_block) + len(file1) + result = _M._truncate_diff(diff, budget) + + # stat + file1 should be present + assert stat_block in result + assert "aaa.py" in result + assert "line 1" in result + # file2 should be omitted + assert "bbb.py" not in result + assert "line a" not in result + # marker should be present + assert "files omitted to fit context budget" in result + + 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_omission_marker_text(self) -> None: + """Marker text contains expected description.""" + stat_block = " git diff --cached --stat -p\n\n" + file1 = "diff --git a/a.py b/a.py\n+1\n" + file2 = "diff --git a/b.py b/b.py\n+2\n" + file3 = "diff --git a/c.py b/c.py\n+3\n" + diff = stat_block + file1 + file2 + file3 + + # Budget fits stat + file1 exactly; file2 and file3 are dropped (2 of 3 omitted) + budget = len(stat_block) + len(file1) + result = _M._truncate_diff(diff, budget) + + assert "files omitted to fit context budget" in result + # Check the marker format: 2 of 3 files omitted + assert "2 of 3 files omitted to fit context budget" in result + + +# =========================================================================== +# _skip_binary_and_generated +# =========================================================================== + + +class TestSkipBinaryAndGenerated: + """Tests for _skip_binary_and_generated, the binary/generated file filter.""" + + def test_binary_section_replaced_with_note(self) -> None: + """Binary file section replaced with [skipped binary/generated file: ...].""" + diff = ( + "git diff --cached --stat -p\n\n" + "Binary files a/img.png and b/img.png differ\n" + "diff --git a/text.txt b/text.txt\n+hello\n" + ) + result = _M._skip_binary_and_generated(diff) + + # Should contain the skip note with filename + assert "[skipped binary/generated file: b/img.png]" in result + # Should NOT contain the original binary diff line + assert "Binary files" not in result + # Normal section should pass through + assert "text.txt" in result + assert "hello" in result + + def test_generated_file_replaced_with_note(self) -> None: + """Generated file section (matching _GENERATED_FILE_GLOBS) is replaced.""" + diff = ( + "git diff --cached --stat -p\n\n" + "diff --git a/foo.pb.h b/foo.pb.h\n" + "new file mode 100644\n" + "+// Generated by protoc\n" + "+class Foo {}\n" + "diff --git a/normal.cpp b/normal.cpp\n+int main() {}\n" + ) + result = _M._skip_binary_and_generated(diff) + + # foo.pb.h is in _GENERATED_FILE_GLOBS (path stored without b/ prefix) + assert "[skipped binary/generated file: foo.pb.h]" in result + # Generated file hunk body should be absent + assert "Generated by protoc" not in result + assert "class Foo {}" not in result + # Normal file should pass through + assert "normal.cpp" in result + assert "int main()" in result + + def test_normal_file_section_kept(self) -> None: + """Plain .py file section passes through unchanged.""" + diff = ( + "git diff --cached --stat -p\n\n" + "diff --git a/program.py b/program.py\n+print('hello')\n" + ) + result = _M._skip_binary_and_generated(diff) + + assert result == diff + assert "print('hello')" in result + + +# =========================================================================== +# _build_messages max_diff_chars parameter +# =========================================================================== + + +class TestBuildMessagesMaxDiffChars: + """Tests for _build_messages with the max_diff_chars parameter.""" + + 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 From 6ebbf3d5cca95e48454635df84a7e990b0dd3fe9 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Tue, 7 Jul 2026 14:44:23 -0500 Subject: [PATCH 2/7] Potential fix for pull request finding 'CodeQL / Unused global variable' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- scripts/git-ai-commit | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/git-ai-commit b/scripts/git-ai-commit index 4dc389d5d..9236c93a2 100755 --- a/scripts/git-ai-commit +++ b/scripts/git-ai-commit @@ -138,7 +138,6 @@ _KILO_MODEL_PINNED_BY_ENV = bool(_KILO_MODEL_ENV) # the LiteLLM gateway directly, so the provider key only selects the auth # token — model names remain bare (e.g. "azure/...", "qwen/..."). _KILO_PROVIDER_ENV = os.environ.get("GIT_AI_COMMIT_KILO_PROVIDER", "").strip() -_DEFAULT_KILO_PROVIDER = _KILO_PROVIDER_ENV or "fnal-azure" _KILO_PROVIDER_FALLBACKS = ( [_KILO_PROVIDER_ENV] if _KILO_PROVIDER_ENV else ["fnal-azure", "fnal-ow", "fnal-litellm"] ) From 0f52a6feb9205275fa2f3a0478b2e2867f4c2672 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Tue, 7 Jul 2026 14:45:48 -0500 Subject: [PATCH 3/7] Potential fix for pull request finding 'CodeQL / Module is imported more than once' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- scripts/git-ai-commit | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/git-ai-commit b/scripts/git-ai-commit index 9236c93a2..3dad73d75 100755 --- a/scripts/git-ai-commit +++ b/scripts/git-ai-commit @@ -177,7 +177,6 @@ def _skip_binary_and_generated(diff: str) -> str: The placeholder format is ``"[skipped binary/generated file: ]"``. """ import fnmatch - import re # Split diff into sections starting with "diff --git". sections = re.split(r"(?=^diff --git )", diff, flags=re.MULTILINE) From 0b99d79021bf3455bbaaeed746b540d4e474103a Mon Sep 17 00:00:00 2001 From: Chris Green Date: Wed, 8 Jul 2026 08:14:56 -0500 Subject: [PATCH 4/7] refactor(test): extract large diff builder into reusable helper Extract the large diff construction logic into a private `_make_large_diff` helper method to avoid duplication between test cases and improve test readability. - Addresses https://github.com/Framework-R-D/phlex/pull/682#discussion_r3507974849 --- scripts/test/test_git_ai_commit.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/scripts/test/test_git_ai_commit.py b/scripts/test/test_git_ai_commit.py index 0f718d1e3..e72e5b722 100644 --- a/scripts/test/test_git_ai_commit.py +++ b/scripts/test/test_git_ai_commit.py @@ -862,13 +862,16 @@ def test_normal_file_section_kept(self) -> None: class TestBuildMessagesMaxDiffChars: """Tests for _build_messages with the max_diff_chars parameter.""" - def test_default_cap_truncates_large_diff(self, tmp_path: Path) -> None: - """Large diff (> 60 000 chars) is truncated with default max_diff_chars.""" + def _make_large_diff(self, *, num_lines: int = 1500) -> str: + """Construct a large diff with approximately `num_lines` added lines.""" 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)] + lines = [f"+line {i} with some padding to make it longer\n" for i in range(num_lines)] file_section = "diff --git a/large.py b/large.py\nnew file mode 100644\n".join(lines) - diff = stat_block + file_section + return stat_block + file_section + + def test_default_cap_truncates_large_diff(self, tmp_path: Path) -> None: + """Large diff (> 60 000 chars) is truncated with default max_diff_chars.""" + diff = self._make_large_diff() assert len(diff) > 60_000 msgs = _build_messages(diff, "", "", "", tmp_path) @@ -879,11 +882,7 @@ def test_default_cap_truncates_large_diff(self, tmp_path: Path) -> None: 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 + diff = self._make_large_diff() assert 60_000 < len(diff) < 400_000 msgs = _build_messages( From 7453cac8efa612afebe1785f49ab55ec55e15ca5 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Thu, 9 Jul 2026 15:31:52 -0500 Subject: [PATCH 5/7] docs: update auth token resolution order in man page Update the documentation to reflect that ~/.local/share/kilo/auth.json now supports multiple keys: fnal-azure, fnal-ow, and fnal-litellm. --- scripts/git-ai-commit | 10 +++++----- scripts/man/man1/git-ai-commit.1 | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/git-ai-commit b/scripts/git-ai-commit index 3dad73d75..000efa144 100755 --- a/scripts/git-ai-commit +++ b/scripts/git-ai-commit @@ -185,10 +185,10 @@ def _skip_binary_and_generated(diff: str) -> str: if not sec.strip(): continue # Binary detection. - if "Binary files" in sec and "differ" in sec: + m = re.search(r"^Binary files ([^ \n]+) and ([^ \n]+) differ", sec, flags=re.MULTILINE) + if m: # 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 "" + path = m.group(2) kept_sections.append(f"[skipped binary/generated file: {path}]\n") continue # Generated file detection via header line. @@ -551,7 +551,7 @@ def _max_tokens_for_prompt(prompt_size: int, model: str) -> int: if model: model_lower = model.lower() if any(kw in model_lower for kw in ("reasoning", "4-5", "qwen3-coder-next", "gemma4-31b")): - return 8912 + return 8192 # Scale with prompt size to avoid truncated responses if prompt_size > 50_000: @@ -743,7 +743,7 @@ def _chat( if model: model_lower = model.lower() if any(kw in model_lower for kw in ("reasoning", "4-5", "qwen3-coder-next", "gemma4-31b")): - max_tokens = 8912 + max_tokens = 8192 elif prompt_size > 50_000: max_tokens = 4096 elif prompt_size > 25_000: diff --git a/scripts/man/man1/git-ai-commit.1 b/scripts/man/man1/git-ai-commit.1 index fb6da75bd..a8e1b834d 100644 --- a/scripts/man/man1/git-ai-commit.1 +++ b/scripts/man/man1/git-ai-commit.1 @@ -137,7 +137,7 @@ Token resolution order: .IP "1." 4 \fBGIT_AI_COMMIT_TOKEN\fR .IP "2." 4 -\fB~/.local/share/kilo/auth.json\fR (\fIfnal\-litellm\fR key) +\fB~/.local/share/kilo/auth.json\fR (\fIfnal\-azure\fR, \fIfnal\-ow\fR, \fIfnal\-litellm\fR keys) .IP "3." 4 \fBGITHUB_TOKEN\fR / \fBGH_TOKEN\fR .IP "4." 4 From 33df23e4e474215e051447de045bb57d62832544 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Fri, 10 Jul 2026 13:18:15 -0500 Subject: [PATCH 6/7] feat: implement model-aware provider selection for kilo backend Replace hardcoded azure/claude-haiku-4-5 default with qwen/qwen3-coder-next, routing open-weight models (qwen/*, google/*, nvidia/*, BAAI/*) to fnal-ow provider to avoid HTTP 400 budget exceeded errors. Add _determine_kilo_provider() to select fnal-ow for open-weight models and fnal-azure otherwise, while maintaining fallback to fnal-ow/fnal-azure for backward compatibility. Update documentation and tests accordingly. --- .../git_ai_commit_context_guidelines.md | 8 ++ scripts/git-ai-commit | 91 +++++++++++++------ scripts/man/man1/git-ai-commit.1 | 2 +- scripts/test/test_git_ai_commit.py | 2 +- 4 files changed, 74 insertions(+), 29 deletions(-) diff --git a/docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md b/docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md index a929223ff..ae075a227 100644 --- a/docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md +++ b/docs/dev/git-ai-commit-context/git_ai_commit_context_guidelines.md @@ -128,6 +128,14 @@ document as implemented in `scripts/git-ai-commit`. | 6 | Trim `_SYSTEM` prompt filler | Implemented (partial) | Removed one redundant filler sentence; essential Conventional Commits rules and the "message only" constraint preserved. | | 7 | Reject table-driven model routing | Rejected | Auto-escalation threshold retained as-is; two-tier caps provide the necessary headroom without adding routing complexity. | +### Implementation status (subsequent fixes) + +| # | Recommendation | Disposition | Notes | +| --- | --- | --- | --- | +| 8 | Model-aware provider selection | Implemented | `_determine_kilo_provider()` routes open-weight models (qwen/*, google/*, nvidia/*, BAAI/*) to fnal-ow provider (no cost) and Azure models to fnal-azure. Default model changed from azure/claude-haiku-4-5 to qwen/qwen3-coder-next. | + --- *Generated by the Kilo `git‑ai‑commit` context‑optimization work.* + +After budget exhaustion fix (2026-07-10): updated default model and added model-aware provider selection to route open-weight models to the unlimited-budget fnal-ow provider, avoiding HTTP 400 budget exceeded errors. diff --git a/scripts/git-ai-commit b/scripts/git-ai-commit index 000efa144..d76a8d084 100755 --- a/scripts/git-ai-commit +++ b/scripts/git-ai-commit @@ -32,11 +32,12 @@ Options --context TEXT Supplemental context/prompt for the AI --backend BACKEND API backend: kilo (default), github-models, copilot, antigravity, agy - --model MODEL Model name; defaults to azure/claude-haiku-4-5 for the - kilo backend (gpt-4o for others, none for antigravity); - large prompts auto-escalate to qwen/qwen3-coder-next; - binary and generated files are skipped (env: - GIT_AI_COMMIT_MODEL or GIT_AI_COMMIT_KILO_MODEL) + --model MODEL Model name; defaults to qwen/qwen3-coder-next for the + kilo backend (gpt-4o for others, none for antigravity); + large prompts auto-escalate to qwen/qwen3-coder-next + (unless model is explicitly set); binary and generated + files are skipped (env: GIT_AI_COMMIT_MODEL or + GIT_AI_COMMIT_KILO_MODEL) --api URL API base URL override (env: GIT_AI_COMMIT_API) --dry-run Print generated message to stdout; do not commit -y/--yes Commit without interactive confirmation @@ -126,23 +127,33 @@ _KILO_AUTH_JSON = Path.home() / ".local" / "share" / "kilo" / "auth.json" # naming convention. _DEFAULT_KILO_API = os.environ.get("GIT_AI_COMMIT_KILO_API", "https://litellm.fnal.gov/v1") _KILO_MODEL_ENV = os.environ.get("GIT_AI_COMMIT_KILO_MODEL", "").strip() -_DEFAULT_MODEL_KILO = _KILO_MODEL_ENV or "azure/claude-haiku-4-5" +# Default model for kilo backend: use an open-weight model that has no cost +# and unlimited budget via the fnal-ow provider. This avoids hitting budget +# limits common with Azure/azure models. Override via GIT_AI_COMMIT_MODEL. +_DEFAULT_MODEL_KILO = _KILO_MODEL_ENV or "qwen/qwen3-coder-next" # True when the user has explicitly pinned a kilo model via the environment, # which suppresses auto-escalation even if the diff is large. _KILO_MODEL_PINNED_BY_ENV = bool(_KILO_MODEL_ENV) # auth.json provider key(s) that hold the FNAL gateway API token. The former # single "fnal-litellm" provider was split into "fnal-azure" and "fnal-ow", # which share the same upstream key; "fnal-litellm" is retired. When the user -# does not pin GIT_AI_COMMIT_KILO_PROVIDER, try the new keys in order (and the -# retired name last, for any un-migrated auth.json). git-ai-commit talks to -# the LiteLLM gateway directly, so the provider key only selects the auth -# token — model names remain bare (e.g. "azure/...", "qwen/..."). +# does not pin GIT_AI_COMMIT_KILO_PROVIDER, select the provider based on the +# model name: open-weight models (qwen/*, google/*) use fnal-ow; other models +# use fnal-azure. git-ai-commit talks to the LiteLLM gateway directly, so +# the provider key only selects the auth token — model names remain bare. _KILO_PROVIDER_ENV = os.environ.get("GIT_AI_COMMIT_KILO_PROVIDER", "").strip() -_KILO_PROVIDER_FALLBACKS = ( - [_KILO_PROVIDER_ENV] if _KILO_PROVIDER_ENV else ["fnal-azure", "fnal-ow", "fnal-litellm"] -) _DEFAULT_MODEL_OTHER = "gpt-4o" +# Open-weight model prefix patterns that should use the fnal-ow provider. +# These models have no cost and unlimited budget in the FNAL deployment. +_OPEN_WEIGHT_MODEL_PREFIXES = ("qwen/", "google/", "nvidia/", "BAAI/") + +# Default fallback providers, used when user has not pinned a provider via +# GIT_AI_COMMIT_KILO_PROVIDER, and for backward compatibility with tests. +# When model-based selection is used, the _determine_kilo_provider function +# will select the appropriate provider first. +_KILO_PROVIDER_FALLBACKS = ["fnal-azure", "fnal-ow", "fnal-litellm"] + _MAX_DIFF_CHARS_DEFAULT = 60_000 _MAX_DIFF_CHARS_ESCALATED = 400_000 # Large ceiling for escalated path @@ -501,14 +512,10 @@ def _get_instructions(root: Path) -> str: # --------------------------------------------------------------------------- -def _kilo_auth_token() -> str | None: - """Read the kilo provider API key from kilo's auth.json, if present. +def _kilo_auth_token_for_providers(providers: list[str]) -> str | None: + """Read the kilo provider API key from kilo's auth.json for the given providers. - The provider name(s) come from GIT_AI_COMMIT_KILO_PROVIDER when set; - otherwise the split FNAL providers are tried in order - ("fnal-azure", "fnal-ow", then the retired "fnal-litellm"). These share - the same upstream gateway token, so the first present key wins. The names - match the provider keys in ~/.config/kilo/kilo.jsonc. + Returns the first valid API key found for any of the specified providers. """ if not _KILO_AUTH_JSON.exists(): return None @@ -518,7 +525,7 @@ def _kilo_auth_token() -> str | None: return None if not isinstance(data, dict): return None - for provider_name in _KILO_PROVIDER_FALLBACKS: + for provider_name in providers: provider = data.get(provider_name) if not isinstance(provider, dict): continue @@ -528,6 +535,18 @@ def _kilo_auth_token() -> str | None: return None +def _kilo_auth_token() -> str | None: + """Read the kilo provider API key from kilo's auth.json, if present. + + The provider name(s) come from GIT_AI_COMMIT_KILO_PROVIDER when set; + otherwise the split FNAL providers are tried in order + ("fnal-azure", "fnal-ow", then the retired "fnal-litellm"). These share + the same upstream gateway token, so the first present key wins. The names + match the provider keys in ~/.config/kilo/kilo.jsonc. + """ + return _kilo_auth_token_for_providers(_KILO_PROVIDER_FALLBACKS) + + def _max_tokens_for_prompt(prompt_size: int, model: str) -> int: """Return max_tokens based on prompt size and model capabilities. @@ -605,16 +624,34 @@ def _gh_oauth_token() -> str | None: return _gh_config_token() -def _token(backend: str) -> str: - """Resolve the best available API token for the given backend.""" +def _determine_kilo_provider(model: str) -> str: + """Select the appropriate FNAL LiteLLM provider based on model name. + + Open-weight models (qwen/*, google/, nvidia/, BAAI/) should use fnal-ow + which has no cost and unlimited budget. Other models use fnal-azure. + """ + if model: + model_lower = model.lower() + if any(model_lower.startswith(prefix) for prefix in _OPEN_WEIGHT_MODEL_PREFIXES): + return "fnal-ow" + return "fnal-azure" + + +def _token(backend: str, model: str = "") -> str: + """Resolve the best available API token for the given backend and model.""" # GIT_AI_COMMIT_TOKEN always wins regardless of backend. if val := os.environ.get("GIT_AI_COMMIT_TOKEN", "").strip(): return val if backend == "kilo": - # Prefer the kilo-stored FNAL key; fall through to GH tokens so that - # the script still works if the kilo extension is not installed. - if tok := _kilo_auth_token(): + # Select provider based on model name, or use environment hint + # if explicitly set via GIT_AI_COMMIT_KILO_PROVIDER. + if _KILO_PROVIDER_ENV: + providers = [_KILO_PROVIDER_ENV] + else: + providers = [_determine_kilo_provider(model), "fnal-ow", "fnal-azure", "fnal-litellm"] + + if tok := _kilo_auth_token_for_providers(providers): return tok for var in ("GITHUB_TOKEN", "GH_TOKEN"): @@ -1073,7 +1110,7 @@ def main() -> None: log = _recent_log() last_msg = _last_commit_message() if args.amend else "" instr = "" if args.no_instructions else _get_instructions(root) - tok = "" if backend in ("antigravity", "agy") else _token(backend) + tok = "" if backend in ("antigravity", "agy") else _token(backend, model) except _Error as exc: print(f"git-ai-commit: {exc}", file=sys.stderr) sys.exit(1) diff --git a/scripts/man/man1/git-ai-commit.1 b/scripts/man/man1/git-ai-commit.1 index a8e1b834d..74bf17171 100644 --- a/scripts/man/man1/git-ai-commit.1 +++ b/scripts/man/man1/git-ai-commit.1 @@ -82,7 +82,7 @@ API backend to use. One of \fBkilo\fR (default), \fBgithub\-models\fR, Overridden by the \fBGIT_AI_COMMIT_BACKEND\fR environment variable. .TP \fB\-\-model\fR \fIMODEL\fR -Model name to use. Defaults to \fBazure/claude\-haiku\-4\-5\fR for the +Model name to use. Defaults to \fBqwen/qwen3\-coder\-next\fR for the \fBkilo\fR backend, \fBgpt\-4o\fR for other backends, and none for the \fBantigravity\fR backend (which lets the CLI use its default model). When the \fBkilo\fR backend is selected and the model was not explicitly diff --git a/scripts/test/test_git_ai_commit.py b/scripts/test/test_git_ai_commit.py index e72e5b722..495e2936f 100644 --- a/scripts/test/test_git_ai_commit.py +++ b/scripts/test/test_git_ai_commit.py @@ -461,7 +461,7 @@ def test_main_exits_0_when_no_staged_changes( monkeypatch.setattr(_M, "_status", lambda: "") monkeypatch.setattr(_M, "_recent_log", lambda: "") monkeypatch.setattr(_M, "_get_instructions", lambda _root: "") - monkeypatch.setattr(_M, "_token", lambda _backend: "tok") + monkeypatch.setattr(_M, "_token", lambda _backend, _model: "tok") # Simulate a non-tty (pipe) stdin with no content so the stdin-read # branch is exercised without touching pytest's capture machinery. monkeypatch.setattr(sys, "stdin", io.StringIO("")) From af2a5819294dd537322edbc18742ca160d28a8a9 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Fri, 10 Jul 2026 16:51:48 -0500 Subject: [PATCH 7/7] refactor: improve diff truncation logic and binary file detection - Add end-of-string anchor to binary file regex for stricter matching - Remove redundant `import re as _re` alias, use `re` directly - Enhance truncation message to distinguish line-boundary cuts from omitted files - Update test to correctly construct single large file section - Clarify auth token resolution in man page with provider context --- scripts/git-ai-commit | 13 ++++++++----- scripts/man/man1/git-ai-commit.1 | 2 +- scripts/test/test_git_ai_commit.py | 5 +++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/scripts/git-ai-commit b/scripts/git-ai-commit index d76a8d084..4c6acb77f 100755 --- a/scripts/git-ai-commit +++ b/scripts/git-ai-commit @@ -196,7 +196,7 @@ def _skip_binary_and_generated(diff: str) -> str: if not sec.strip(): continue # Binary detection. - m = re.search(r"^Binary files ([^ \n]+) and ([^ \n]+) differ", sec, flags=re.MULTILINE) + m = re.search(r"^Binary files ([^ \n]+) and ([^ \n]+) differ$", sec, flags=re.MULTILINE) if m: # Extract path from the line, e.g., "Binary files a/foo.bin and b/foo.bin differ" path = m.group(2) @@ -232,9 +232,7 @@ def _truncate_diff(diff: str, max_chars: int) -> str: # Split the diff into stat block + individual file sections. # Use a zero-width lookahead so each file section retains its "diff --git " prefix. - import re as _re - - raw_sections = _re.split(r"(?=^diff --git )", diff, flags=_re.MULTILINE) + raw_sections = re.split(r"(?=^diff --git )", diff, flags=re.MULTILINE) # First element is the leading diffstat block (may be empty for a bare diff). stat_block = raw_sections[0] file_sections = raw_sections[1:] # each starts with "diff --git " @@ -265,7 +263,12 @@ def _truncate_diff(diff: str, max_chars: int) -> str: omitted = total_files - len(kept_sections) final = stat_block + "".join(kept_sections) - if omitted or line_boundary_cut: + 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]" ) diff --git a/scripts/man/man1/git-ai-commit.1 b/scripts/man/man1/git-ai-commit.1 index 74bf17171..fc9be1014 100644 --- a/scripts/man/man1/git-ai-commit.1 +++ b/scripts/man/man1/git-ai-commit.1 @@ -137,7 +137,7 @@ Token resolution order: .IP "1." 4 \fBGIT_AI_COMMIT_TOKEN\fR .IP "2." 4 -\fB~/.local/share/kilo/auth.json\fR (\fIfnal\-azure\fR, \fIfnal\-ow\fR, \fIfnal\-litellm\fR keys) +\fB~/.local/share/kilo/auth.json\fR (\fIfnal\-azure\fR / \fIfnal\-ow\fR / \fIfnal\-litellm\fR keys; see \fBGIT_AI_COMMIT_KILO_PROVIDER\fR) .IP "3." 4 \fBGITHUB_TOKEN\fR / \fBGH_TOKEN\fR .IP "4." 4 diff --git a/scripts/test/test_git_ai_commit.py b/scripts/test/test_git_ai_commit.py index 495e2936f..403e838f8 100644 --- a/scripts/test/test_git_ai_commit.py +++ b/scripts/test/test_git_ai_commit.py @@ -754,9 +754,10 @@ def test_over_budget_cuts_on_file_boundary(self) -> None: 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 + # Build a large file section with many lines - this should create one large section 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) + # Create a single large file section with all the lines + 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,