From 8d333db2fc7067d28653bf381d2058279cb2540c Mon Sep 17 00:00:00 2001 From: Amar K P Date: Thu, 17 Sep 2026 13:29:33 +0530 Subject: [PATCH 1/2] fix: implement chunk-and-summarize execution and resolve Phase 3/4 checklist discrepancies - Implement _chunk_patch_text and summarize_omitted_diffs in fallback.py to chunk and summarize BudgetDecision.omitted_diffs when strategy is BudgetStrategy.NEEDS_CHUNKING, merging summaries into the final commit prompt - Add gemini-3.5-flash to DEFAULT_MODEL_CONTEXT_WINDOWS in config.py with comment noting gemini-3.5-flash-lite remains default - Add RetryableError alias for RetryableProviderError in providers/base.py - Calibrate DEFAULT_TEMPLATE_OVERHEAD (500 tokens) against shipped templates (~206-330 tokens) - Update plan.md to remove stale Anthropic references and align with AGENTS.md and Phase 4 chunk-and-summarize delivery - Add tests in test_config.py, test_orchestrator.py, and test_providers.py - Closes #8 --- .gitignore | 3 - plan.md | 177 ++++++++++++++++++++++++++++++++++ src/gitmate/config.py | 6 ++ src/gitmate/fallback.py | 164 ++++++++++++++++++++++++------- src/gitmate/providers/base.py | 4 + tests/test_config.py | 5 + tests/test_orchestrator.py | 80 ++++++++++++++- tests/test_providers.py | 8 ++ 8 files changed, 406 insertions(+), 41 deletions(-) create mode 100644 plan.md diff --git a/.gitignore b/.gitignore index 3eb0f44..2c61c6d 100644 --- a/.gitignore +++ b/.gitignore @@ -69,7 +69,4 @@ Thumbs.db # Agent tooling droppings (not part of this project) .agents/ -#md files -plan.md - gitmate.architecture.json \ No newline at end of file diff --git a/plan.md b/plan.md new file mode 100644 index 0000000..2207065 --- /dev/null +++ b/plan.md @@ -0,0 +1,177 @@ +# gitmate — AI Git Assistant CLI: Build Plan + +A CLI that generates commit messages, PR summaries, and changelogs from your +git diffs using an LLM — with cost control, caching, and a human always in +the loop. Goal: something you actually run every day, not just a portfolio repo. + +--- + +## Design decisions locked in up front + +| Decision | Choice | Why | +|---|---|---| +| Caching strategy | **Hash-based** (`sha256(diff)` → response) over semantic caching | Semantic caching (embedding similarity) needs a vector store, an embedding model call on every lookup, and a similarity threshold you'll forever be tuning. Hash caching is O(1), free, and correct — its only cost is a ~0% hit rate on a diff that changed by one line. That's an acceptable, explainable tradeoff for v1. Revisit semantic caching only if cache-hit-rate data (Phase 6) shows it's worth the complexity. | +| Git access | **`subprocess` calls to the `git` binary** over GitPython | You need exact control over diff flags (`--staged`, `-U0`, path filters) and want zero abstraction leakage. GitPython is a nice-to-have wrapper you don't need — it's another dependency and another place for version mismatches to bite you. | +| LLM SDK | **Google GenAI SDK (Gemini)** default, behind your own `LLMProvider` interface (Anthropic secondary/deferred) | Official SDK (`google-genai`), targets Gemini 3 Flash / Flash-Lite per AGENTS.md. Handles generation and token counting behind a swappable interface without guessing with third-party tokenizers. | +| Config format | **TOML** (`~/.config/gitmate/config.toml`) | Human-editable, standard for Python CLIs (same as `pyproject.toml`), no extra parsing dependency (`tomllib` is stdlib in 3.11+). | +| Secrets | **`keyring`** library, not plaintext config | Uses the OS credential store (macOS Keychain / Windows Credential Manager / Secret Service on Linux). No API key ever touches a file you could accidentally `git add`. | +| Cache store | **`diskcache`** over raw SQLite | Gives you TTL, LRU eviction, and thread-safety out of the box with a dict-like API — SQLite is fine but you'd be re-implementing what this library already does correctly. | +| CLI framework | **Typer** + **Rich** | Typer for decorator-based commands and auto-generated `--help`; Rich for readable diffs/diffs-with-syntax-highlighting and a nice review prompt in the terminal. | +| Packaging | **`pyproject.toml` + `hatchling`**, installed via **`pipx`** | `pipx` is the standard way people install CLI tools without polluting their global Python env. Publish to PyPI once stable. | +| Automatic trigger | **`pre-commit` framework hook**, optional `prepare-commit-msg` git hook | This is what turns it from "a script I remember to run" into "part of my git workflow." | + +--- + +## Phase 0 — Skeleton & Config (½–1 day) + +**Goal:** `gitmate config set-key` works, `gitmate --help` shows all planned commands (stubbed). + +- Set up `pyproject.toml`, package layout (`src/gitmate/`), Typer app with stub commands: `commit`, `pr-summary`, `changelog`, `doc`, `config`. +- `config` command: read/write `~/.config/gitmate/config.toml` (model choice, default commit style, budget cap); store API key via `keyring`. +- Basic Rich-styled CLI output (banner, error formatting). + +**Deliverables:** installable package (`pipx install -e .`), working `config` command, CI skeleton (GitHub Actions running `pytest` on push). + +**Tech:** Typer, Rich, `tomllib`/`tomli-w`, `keyring`, `pytest`, GitHub Actions. + +--- + +## Phase 1 — Diff/Context Extractor (1 day) + +**Goal:** `gitmate debug-diff` prints a cleaned, filtered diff for any git range. + +- Wrap `subprocess.run(["git", "diff", ...])` for three modes: staged (`--staged`), branch-vs-branch (for PR summaries), and range (for changelogs, `tagA..tagB`). +- Noise filtering: skip lockfiles (`package-lock.json`, `poetry.lock`, `Cargo.lock`), minified/generated files, binary diffs — via a configurable ignore-glob list (defaults + user overrides in config). +- Return a structured object: list of `FileDiff(path, additions, deletions, patch_text, is_binary)`, not a raw string — every downstream layer works off this. + +**Deliverables:** `DiffExtractor` class with unit tests against a scratch git repo (use `pytest` fixtures that `git init` a temp dir and make real commits — don't mock git itself, it's cheap to run for real). + +**Tech:** `subprocess`, `pytest` with `tmp_path` fixtures. + +--- + +## Phase 2 — Token Budget Manager (½–1 day) + +**Goal:** Given a `FileDiff` list, decide: send whole, truncate, or chunk-and-summarize. + +- Count tokens via the google-genai SDK's token-counting support (`GeminiTokenCounter`, provider-specific behind a `TokenCounter` Protocol). +- Strategy: if under budget, send as-is. If over, drop full patches for the largest low-signal files first (test snapshots, lockfile-adjacent generated code) and replace with a one-line "N files changed, omitted for size" note. If still over after that, fall back to chunk-and-summarize-then-merge (summarize each file's diff separately, then feed summaries into the final prompt). +- Make the strategy explicit and loggable — the user should be able to run `--verbose` and see "included 8/12 files, truncated 4." + +**Deliverables:** `TokenBudgetManager` with a documented strategy function and tests covering: fits easily, needs truncation, needs chunking. + +**Tech:** `google-genai` SDK token counting, plain Python (no extra deps needed here). + +--- + +## Phase 3 — Provider Abstraction + Cache Layer (1 day) + +**Goal:** `LLMProvider.generate(prompt) -> LLMResponse`, with caching in front of it, swappable backend. + +- Define a small `Protocol`/ABC: `LLMProvider.generate(prompt: str, model: str) -> LLMResponse`. +- Implement `GeminiProvider` first, targeting **Gemini 3 Flash / Flash-Lite** via the `google-genai` SDK (`client.models.generate_content`). This is the locked-in default per AGENTS.md; Anthropic is the secondary provider using the same interface and is deferred to the stretch-ideas list until the abstraction is proven once. Model IDs churn fast (2.0 Flash is retired; Flash-Lite previews deprecate within months), so the ID stays configurable in `config.py` rather than hardcoded. +- Cache: wrap the provider call — `diskcache.Cache` keyed on `sha256(prompt + model + template_version)`, with a configurable TTL (default: no expiry, since a given diff's ideal message doesn't go stale — but bump the key if you change your prompt template, hence `template_version` in the key). Template version is a hardcoded `"1"` placeholder until Phase 4 ships real templates, so Phase 3 cache entries don't need a key migration later. +- Cache location follows the project's XDG convention: `~/.cache/gitmate/`, overridable via a `cache_dir` config field. +- Retry with exponential backoff (`tenacity` library) on transient errors (5xx, 429 rate-limit, transport failures); on final failure, raise a typed `ProviderUnavailable` exception (Phase 4 catches this). +- **Named Risk (from Phase 2 review):** `TokenBudgetManager` has a hardcoded `context_window` default (1,048,576) that does not inspect `model`. Add a `TokenBudgetManager.from_config(cfg, counter)` factory that resolves `get_context_window(cfg)`, so non-Gemini models (e.g. Claude's 200k limit) never get silently over-budgeted to 1M. + +**Deliverables:** `LLMProvider` interface, `GeminiProvider` implementation, `CachedProvider` wrapper, `from_config` factory, tests using a fake in-memory provider (no real API calls in CI — mock at the `LLMProvider` boundary, not deeper; include a call-count test proving the cache is consulted before `generate()`). + +**Tech:** `google-genai` SDK, `diskcache`, `tenacity` for retries. (Anthropic SDK, already a dependency, stays unused until the secondary provider is implemented.) + +--- + +## Phase 4 — Graceful Degradation + Prompt Templates (½ day) + +**Goal:** If the API is down/rate-limited/offline, you still get a usable commit message. + +- Template-based fallback: derive a message from `git status` + file extensions (e.g. "Update 3 files in src/auth (added: 2, modified: 1)"). No LLM call at all. +- Prompt templates as versioned files (`templates/commit_conventional.txt`, `templates/commit_plain.txt`, `templates/pr_summary.txt`), selected by config. Bump a version string in each when you edit it — this is the `template_version` the cache key depends on. +- Wire this so `LLMProvider` failure (`ProviderUnavailable`) → caught → fallback path, with a visible warning ("⚠ API unavailable, using template fallback") so the user isn't confused about message quality. +- Chunk-and-summarize execution (deferred from Phase 2): when `TokenBudgetManager.assess()` returns `BudgetStrategy.NEEDS_CHUNKING`, consume `omitted_diffs`, summarize chunks via the provider, and merge summaries into the final prompt (with graceful fallback if the provider fails). + +**Deliverables:** fallback generator, prompt template engine with at least 2 prompt template styles, chunk-and-summarize orchestrator, tests that simulate provider failure, template rendering, and chunk-and-summarize prompt merging. + +**Tech:** plain Python, Jinja2 (optional, if templates get more than string `.format()` can cleanly handle). + +--- + +## Phase 5 — Output & Human-in-the-Loop (½ day) + +**Goal:** `gitmate commit` shows the generated message, lets you edit/accept/reject, then actually commits. + +- Default: print the message in Rich, prompt `[a]ccept / [e]dit / [r]egenerate / [c]ancel`. +- Edit path: open `$EDITOR` pre-filled with the message (mirrors `git commit -e`). +- Accept path: write to `.git/COMMIT_EDITMSG` and run `git commit -F`. +- Never auto-commit without one of these confirmations, even with `--yes` flags — make `--yes` require an explicit config opt-in, not a default flag. + +**Deliverables:** working `gitmate commit` end-to-end against a real repo; integration test using a temp git repo. + +**Tech:** Rich `Prompt`, `subprocess` for `$EDITOR` and final `git commit`. + +--- + +## Phase 6 — Metrics & the README table (½ day) + +**Goal:** Every run logs latency, tokens used, cache hit/miss, and estimated cost to a local SQLite table — this is what generates your README's metrics table, and it's also what tells you later whether semantic caching would actually help. + +- `~/.local/share/gitmate/metrics.db`: one row per invocation (timestamp, command, tokens_in, tokens_out, cache_hit, latency_ms, estimated_cost_usd, fallback_used). +- `gitmate stats` command: prints aggregate table (avg latency, cache hit rate, total estimated spend this month) — this is the exact table you paste into the README. + +**Deliverables:** metrics recording wired into Phases 3–5, `gitmate stats` command, a real 2-week's worth of your own usage data before you write the README. + +**Tech:** stdlib `sqlite3` (this one really is simple enough not to need `diskcache`'s abstraction — it's structured rows, not a KV cache). + +--- + +## Phase 7 — Beyond commits: PR summaries & changelogs (1–1.5 days) + +**Goal:** the tool earns its keep beyond `git commit` — this is what makes it a daily-use tool instead of a one-trick demo. + +- `gitmate pr-summary --base main`: diff current branch against `main`, generate a PR description (what changed, why, testing notes) using the `pr_summary` template. Copies to clipboard (`pyperclip`) and/or opens the `gh pr create` flow if the `gh` CLI is detected. +- `gitmate changelog --from v1.0.0 --to v1.1.0`: walks commit messages + diffs between two tags/refs, groups by conventional-commit type (feat/fix/chore), generates a `CHANGELOG.md` section. +- Both reuse the same Diff Extractor / Token Budget Manager / Provider stack — this is the payoff of building those as real layers instead of one script. + +**Deliverables:** two new working commands, tests, and (this is the actually-useful part) you running `pr-summary` on your next 3 real PRs instead of writing them by hand. + +**Tech:** `pyperclip`, optional `gh` CLI shell-out detection. + +--- + +## Phase 8 — Automatic trigger: git hook integration (½ day) + +**Goal:** stop having to remember to run the tool at all. + +- Ship a `prepare-commit-msg` git hook script that calls `gitmate commit --hook-mode` (non-interactive generate, writes to the commit message file git already opens in your editor — so you review inside your normal `git commit` flow, no extra step). +- Also provide a `.pre-commit-hooks.yaml` so people using the `pre-commit` framework can add it with one config line. + +**Deliverables:** `gitmate install-hook` command that symlinks/writes the hook into `.git/hooks/`, documented `pre-commit` config snippet. + +**Tech:** `pre-commit` framework config format, plain shell/Python hook script. + +--- + +## Phase 9 — Packaging, docs, and the story (½–1 day) + +**Goal:** ship it. + +- Finalize `pyproject.toml`, publish to PyPI (`pip install gitmate` or your chosen name — check it's not taken). +- README: architecture diagram (your existing one, updated with Config/Metrics/Hook boxes), the real metrics table from Phase 6, a demo GIF (`vhs` or `asciinema` + `agg` are free tools for scripted terminal GIFs), and the caching-tradeoff paragraph already decided above. +- Write the "interview story": lead with the graceful-degradation and human-in-the-loop decisions — those are the two choices that signal product judgment, not just API-calling. + +**Deliverables:** PyPI package, finished README, demo GIF. + +**Tech:** `hatchling`/`build`/`twine` for publishing, `vhs` (Charm) for demo recording. + +--- + +## Suggested order & time budget + +Phases 0→5 are the MVP (~4–5 days) and get you a working `gitmate commit`. +Phase 6 (metrics) should run *underneath* everything from Phase 3 onward even though it's written up last — start logging early so you have real data by the time you write the README. +Phases 7–8 are what make this a tool you keep using after the demo, not just for the interview — don't skip them for "future work" bullet points. + +## Stretch ideas (only after the above works and you're using it daily) +- Budget cap: hard-stop or warn when `metrics.db` shows spend exceeding a configured monthly cap. +- Multi-provider: add an `OpenAIProvider` to prove the abstraction actually swaps cleanly. +- Revisit semantic caching *only if* Phase 6 data shows hash-cache hit rate is too low to matter. diff --git a/src/gitmate/config.py b/src/gitmate/config.py index 1f73abb..4e3bf5f 100644 --- a/src/gitmate/config.py +++ b/src/gitmate/config.py @@ -28,12 +28,18 @@ #: Context window limits for known models; users override via `max_context_tokens`. DEFAULT_MODEL_CONTEXT_WINDOWS: dict[str, int] = { "gemini-3.5-flash-lite": 1_048_576, + # Standard Flash tier registered for completeness; gemini-3.5-flash-lite remains + # the configured default above. + "gemini-3.5-flash": 1_048_576, "gemini-3-flash": 1_048_576, "gemini-flash": 1_048_576, "claude-3-5-sonnet": 200_000, "claude-3-5-haiku": 200_000, } DEFAULT_RESERVED_OUTPUT_TOKENS = 2048 +#: Calibrated in Phase 4 against real templates (commit_conventional: ~329 tokens, +#: commit_plain: ~210 tokens, pr_summary: ~206 tokens); 500 tokens provides a safe +#: upper bound with ~170+ tokens of headroom for dynamic summary notes. DEFAULT_TEMPLATE_OVERHEAD = 500 diff --git a/src/gitmate/fallback.py b/src/gitmate/fallback.py index 70b2ba8..088fd24 100644 --- a/src/gitmate/fallback.py +++ b/src/gitmate/fallback.py @@ -10,7 +10,7 @@ from gitmate import config as config_mod from gitmate.config import API_KEY_ACCOUNT, GitmateConfig from gitmate.diff_extractor import FileDiff -from gitmate.prompt import render_commit_prompt +from gitmate.prompt import load_template, render_commit_prompt from gitmate.providers.base import LLMProvider, ProviderUnavailable from gitmate.providers.cache import CachedProvider from gitmate.providers.gemini import GeminiProvider @@ -213,6 +213,70 @@ def _multi_file_fallback(diffs: list[FileDiff], style: str) -> str: return f"{header}\n\n{body}" +def _chunk_patch_text(patch: str, max_chars: int = 8_000) -> list[str]: + """Split large patch text into chunks if needed to avoid blowing per-call limits.""" + if len(patch) <= max_chars: + return [patch] + lines = patch.splitlines(keepends=True) + chunks: list[str] = [] + current: list[str] = [] + current_len = 0 + for line in lines: + if current_len + len(line) > max_chars and current: + chunks.append("".join(current)) + current = [line] + current_len = len(line) + else: + current.append(line) + current_len += len(line) + if current: + chunks.append("".join(current)) + return chunks + + +def summarize_omitted_diffs( + omitted_diffs: list[FileDiff], + provider: LLMProvider, + model: str, +) -> tuple[str, int, int]: + """Summarize omitted diffs per file/chunk and return merged notes and token usage. + + Consumes BudgetDecision.omitted_diffs (which preserves untruncated patches), + prompts the provider for concise per-file summaries, and aggregates token usage. + """ + summaries: list[str] = [] + total_in = 0 + total_out = 0 + + for diff in omitted_diffs: + if diff.is_binary or not diff.patch_text.strip(): + summaries.append(f"- {diff.path}: [binary or empty file]") + continue + + chunks = _chunk_patch_text(diff.patch_text) + file_part_summaries: list[str] = [] + for i, chunk in enumerate(chunks, start=1): + part_suffix = f" (part {i}/{len(chunks)})" if len(chunks) > 1 else "" + prompt = ( + f"Summarize the following git diff for {diff.path}{part_suffix} in 1-2 concise bullet points " + f"(focus on WHAT changed and WHY):\n\n" + f"{chunk}" + ) + resp = provider.generate(prompt=prompt, model=model) + if resp.input_tokens is not None: + total_in += resp.input_tokens + if resp.output_tokens is not None: + total_out += resp.output_tokens + text = resp.text.strip() + if text: + file_part_summaries.append(text) + + joined_file_summary = "; ".join(file_part_summaries) if file_part_summaries else "Updated" + summaries.append(f"- {diff.path}: {joined_file_summary}") + + return "\n".join(summaries), total_in, total_out + + def generate_commit_message( diffs: list[FileDiff], cfg: GitmateConfig, @@ -232,25 +296,26 @@ def generate_commit_message( store = secret_store if secret_store is not None else config_mod.KeyringSecretStore() api_key = store.get_secret(API_KEY_ACCOUNT) or os.environ.get("GEMINI_API_KEY") + # Resolve template and provider upfront so chunk summarization can use the active provider + active_template = load_template(cfg.commit_style) + version_key = active_template.version_key + + active_provider: LLMProvider + if provider is not None: + active_provider = provider + else: + base_provider = GeminiProvider(api_key=api_key) + active_provider = CachedProvider( + provider=base_provider, + cache_dir=cfg.cache_dir, + template_version=version_key, + ) + # 1. Budget evaluation with fallback on counter/budget failure active_counter = counter if counter is not None else GeminiTokenCounter(api_key=api_key) try: budget_mgr = TokenBudgetManager.from_config(cfg, active_counter) decision = budget_mgr.assess(diffs) - if decision.strategy is BudgetStrategy.NEEDS_CHUNKING: - if console is not None: - console.print( - "[yellow]⚠ Diff exceeds token budget, using template fallback[/yellow]" - ) - return GenerationResult( - text=generate_fallback_message(diffs, style=cfg.commit_style), - is_fallback=True, - model=cfg.model, - fallback_reason=decision.summary_note, - ) - patch_chunks = [d.patch_text for d in decision.included_diffs if d.patch_text] - diff_text = "\n".join(patch_chunks) - summary_note = decision.summary_note except TokenBudgetError as exc: if console is not None: console.print("[yellow]⚠ API unavailable, using template fallback[/yellow]") @@ -262,35 +327,64 @@ def generate_commit_message( fallback_reason=f"Token budgeting failed: {exc}", ) - # 2. Render prompt template - prompt_text, version_key = render_commit_prompt( - style=cfg.commit_style, - diff_text=diff_text, - summary_note=summary_note, - ) + # 2. Generation / Chunk-and-summarize with graceful fallback on provider failure + try: + total_input_tokens = 0 + total_output_tokens = 0 - # 3. Resolve provider - active_provider: LLMProvider - if provider is not None: - active_provider = provider - else: - base_provider = GeminiProvider(api_key=api_key) - active_provider = CachedProvider( - provider=base_provider, - cache_dir=cfg.cache_dir, - template_version=version_key, + if decision.strategy is BudgetStrategy.NEEDS_CHUNKING: + if not decision.omitted_diffs: + if console is not None: + console.print( + "[yellow]⚠ Diff exceeds token budget, using template fallback[/yellow]" + ) + return GenerationResult( + text=generate_fallback_message(diffs, style=cfg.commit_style), + is_fallback=True, + model=cfg.model, + fallback_reason=decision.summary_note, + ) + + # Chunk and summarize omitted diffs preserving original patches + merged_summaries, chunk_in, chunk_out = summarize_omitted_diffs( + decision.omitted_diffs, + active_provider, + cfg.model, + ) + total_input_tokens += chunk_in + total_output_tokens += chunk_out + + summary_note = ( + f"{decision.summary_note}\n\nSummaries of large omitted files:\n{merged_summaries}" + ) + patch_chunks = [d.patch_text for d in decision.included_diffs if d.patch_text] + diff_text = "\n".join(patch_chunks) + else: + patch_chunks = [d.patch_text for d in decision.included_diffs if d.patch_text] + diff_text = "\n".join(patch_chunks) + summary_note = decision.summary_note + + # Render prompt template + prompt_text, _ = render_commit_prompt( + style=cfg.commit_style, + diff_text=diff_text, + summary_note=summary_note, ) - # 4. Generate with graceful degradation - try: resp = active_provider.generate(prompt=prompt_text, model=cfg.model) + if resp.input_tokens is not None: + total_input_tokens += resp.input_tokens + if resp.output_tokens is not None: + total_output_tokens += resp.output_tokens + return GenerationResult( text=resp.text, is_fallback=False, model=resp.model, - input_tokens=resp.input_tokens, - output_tokens=resp.output_tokens, + input_tokens=total_input_tokens or None, + output_tokens=total_output_tokens or None, ) + except ProviderUnavailable as exc: if console is not None: console.print("[yellow]⚠ API unavailable, using template fallback[/yellow]") diff --git a/src/gitmate/providers/base.py b/src/gitmate/providers/base.py index a7c33fa..01325fb 100644 --- a/src/gitmate/providers/base.py +++ b/src/gitmate/providers/base.py @@ -14,6 +14,10 @@ class RetryableProviderError(Exception): """Internal marker for transient failures worth retrying before giving up.""" +#: Alias for RetryableProviderError matching review checklist naming. +RetryableError = RetryableProviderError + + @dataclass class LLMResponse: """Text plus optional token usage from one provider call.""" diff --git a/tests/test_config.py b/tests/test_config.py index d3476c0..306abd8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -195,3 +195,8 @@ def test_max_context_tokens_must_exceed_reserved_plus_overhead(tmp_config_dir: P match="must be greater than 'reserved_output_tokens' \\+ 'template_overhead'", ): load_config() + + +def test_gemini_flash_non_lite_context_window() -> None: + cfg = GitmateConfig(model="gemini-3.5-flash") + assert get_context_window(cfg) == 1_048_576 diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index dcfd3ef..2f09892 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock +import pytest from rich.console import Console from gitmate.config import GitmateConfig, InMemorySecretStore @@ -160,8 +161,82 @@ def test_generate_commit_message_counter_failure_triggers_fallback() -> None: ) -def test_generate_commit_message_chunking_uses_fallback() -> None: +def test_generate_commit_message_chunk_and_summarize_success() -> None: + """When diff forces NEEDS_CHUNKING, chunk omitted diffs, summarize, and merge into final prompt.""" cfg = GitmateConfig(max_context_tokens=2_600) + provider = FakeProvider(response="feat(auth): add login service with chunking") + counter = FakeCounter() + + result = generate_commit_message( + diffs=_sample_diff(), + cfg=cfg, + provider=provider, + counter=counter, + ) + + assert result.is_fallback is False + assert result.text == "feat(auth): add login service with chunking" + assert result.model == "gemini-3.5-flash-lite" + # 1 chunk call for src/auth/login.py + 1 final commit prompt call = 2 provider calls + assert len(provider.prompts) == 2 + + # First call was chunk summarization + assert "Summarize the following git diff for src/auth/login.py" in provider.prompts[0] + assert "+class LoginService:" in provider.prompts[0] + + # Second call was final commit prompt containing merged chunk summary + assert "Summaries of large omitted files:" in provider.prompts[1] + assert "- src/auth/login.py: feat(auth): add login service" in provider.prompts[1] + + # Accumulated token usage + assert result.input_tokens == 300 # 150 + 150 + assert result.output_tokens == 50 # 25 + 25 + + +def test_generate_commit_message_chunking_provider_failure_triggers_fallback() -> None: + """If provider fails during chunk summarization, graceful degradation catches it.""" + cfg = GitmateConfig(max_context_tokens=2_600) + provider = FailingProvider(error_msg="Rate limit exceeded during chunking") + counter = FakeCounter() + console = MagicMock(spec=Console) + + result = generate_commit_message( + diffs=_sample_diff(), + cfg=cfg, + provider=provider, + counter=counter, + console=console, + ) + + assert result.is_fallback is True + assert result.text == "feat(auth): add login.py" + assert "Rate limit exceeded during chunking" in (result.fallback_reason or "") + console.print.assert_called_once_with( + "[yellow]⚠ API unavailable, using template fallback[/yellow]" + ) + + +def test_generate_commit_message_chunking_empty_omitted_diffs_falls_back( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When NEEDS_CHUNKING has no omitted diffs, fall back cleanly.""" + from gitmate.token_budget import BudgetDecision, BudgetStrategy, TokenBudgetManager + + def _mock_assess(self: TokenBudgetManager, diffs: list[FileDiff]) -> BudgetDecision: + return BudgetDecision( + strategy=BudgetStrategy.NEEDS_CHUNKING, + included_diffs=diffs, + omitted_diffs=[], + total_tokens=999_999, + budget_limit=100, + reserved_output_tokens=2048, + template_overhead=500, + model="gemini-3.5-flash-lite", + summary_note="metadata exceeded budget", + ) + + monkeypatch.setattr(TokenBudgetManager, "assess", _mock_assess) + cfg = GitmateConfig() provider = FakeProvider() counter = FakeCounter() console = MagicMock(spec=Console) @@ -176,8 +251,7 @@ def test_generate_commit_message_chunking_uses_fallback() -> None: assert result.is_fallback is True assert result.text == "feat(auth): add login.py" - assert result.fallback_reason is not None - assert "requires chunking" in result.fallback_reason + assert result.fallback_reason == "metadata exceeded budget" assert provider.prompts == [] console.print.assert_called_once_with( "[yellow]⚠ Diff exceeds token budget, using template fallback[/yellow]" diff --git a/tests/test_providers.py b/tests/test_providers.py index 01db236..4e376d8 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -201,3 +201,11 @@ def test_is_like_provider_protocol(tmp_path: Path) -> None: cached = CachedProvider(FakeProvider(), cache_dir=tmp_path) assert isinstance(cached, LLMProvider) cached.close() + + +def test_retryable_error_alias() -> None: + from gitmate.providers.base import RetryableError, RetryableProviderError + + assert RetryableError is RetryableProviderError + err = RetryableError("transient") + assert isinstance(err, RetryableProviderError) From e79a0c884f0f578bc0929a6c9665adc30d764163 Mon Sep 17 00:00:00 2001 From: Amar K P Date: Thu, 17 Sep 2026 16:23:31 +0530 Subject: [PATCH 2/2] refactor: extract chunking and chunk summarization into src/gitmate/chunking.py - Move chunk_patch_text and summarize_omitted_diffs to src/gitmate/chunking.py to cleanly separate LLM-dependent chunking logic from deterministic zero-LLM fallback.py - Re-export helpers in fallback.py for backward compatibility - Add dedicated test suite in tests/test_chunking.py (4 new tests, 163 total passing) --- src/gitmate/chunking.py | 77 ++++++++++++++++++++++++++++++++++ src/gitmate/fallback.py | 65 +--------------------------- tests/test_chunking.py | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 64 deletions(-) create mode 100644 src/gitmate/chunking.py create mode 100644 tests/test_chunking.py diff --git a/src/gitmate/chunking.py b/src/gitmate/chunking.py new file mode 100644 index 0000000..2d41589 --- /dev/null +++ b/src/gitmate/chunking.py @@ -0,0 +1,77 @@ +"""Chunking abstractions: split oversized diffs and summarize via LLMProvider.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from gitmate.diff_extractor import FileDiff + from gitmate.providers.base import LLMProvider + + +def chunk_patch_text(patch: str, max_chars: int = 8_000) -> list[str]: + """Split large patch text into chunks if needed to avoid blowing per-call limits.""" + if len(patch) <= max_chars: + return [patch] + lines = patch.splitlines(keepends=True) + chunks: list[str] = [] + current: list[str] = [] + current_len = 0 + for line in lines: + if current_len + len(line) > max_chars and current: + chunks.append("".join(current)) + current = [line] + current_len = len(line) + else: + current.append(line) + current_len += len(line) + if current: + chunks.append("".join(current)) + return chunks + + +# Compatibility alias for internal callers +_chunk_patch_text = chunk_patch_text + + +def summarize_omitted_diffs( + omitted_diffs: list[FileDiff], + provider: LLMProvider, + model: str, +) -> tuple[str, int, int]: + """Summarize omitted diffs per file/chunk and return merged notes and token usage. + + Consumes BudgetDecision.omitted_diffs (which preserves untruncated patches), + prompts the provider for concise per-file summaries, and aggregates token usage. + """ + summaries: list[str] = [] + total_in = 0 + total_out = 0 + + for diff in omitted_diffs: + if diff.is_binary or not diff.patch_text.strip(): + summaries.append(f"- {diff.path}: [binary or empty file]") + continue + + chunks = chunk_patch_text(diff.patch_text) + file_part_summaries: list[str] = [] + for i, chunk in enumerate(chunks, start=1): + part_suffix = f" (part {i}/{len(chunks)})" if len(chunks) > 1 else "" + prompt = ( + f"Summarize the following git diff for {diff.path}{part_suffix} in 1-2 concise bullet points " + f"(focus on WHAT changed and WHY):\n\n" + f"{chunk}" + ) + resp = provider.generate(prompt=prompt, model=model) + if resp.input_tokens is not None: + total_in += resp.input_tokens + if resp.output_tokens is not None: + total_out += resp.output_tokens + text = resp.text.strip() + if text: + file_part_summaries.append(text) + + joined_file_summary = "; ".join(file_part_summaries) if file_part_summaries else "Updated" + summaries.append(f"- {diff.path}: {joined_file_summary}") + + return "\n".join(summaries), total_in, total_out diff --git a/src/gitmate/fallback.py b/src/gitmate/fallback.py index 088fd24..c66c8ce 100644 --- a/src/gitmate/fallback.py +++ b/src/gitmate/fallback.py @@ -8,6 +8,7 @@ from typing import TYPE_CHECKING from gitmate import config as config_mod +from gitmate.chunking import summarize_omitted_diffs from gitmate.config import API_KEY_ACCOUNT, GitmateConfig from gitmate.diff_extractor import FileDiff from gitmate.prompt import load_template, render_commit_prompt @@ -213,70 +214,6 @@ def _multi_file_fallback(diffs: list[FileDiff], style: str) -> str: return f"{header}\n\n{body}" -def _chunk_patch_text(patch: str, max_chars: int = 8_000) -> list[str]: - """Split large patch text into chunks if needed to avoid blowing per-call limits.""" - if len(patch) <= max_chars: - return [patch] - lines = patch.splitlines(keepends=True) - chunks: list[str] = [] - current: list[str] = [] - current_len = 0 - for line in lines: - if current_len + len(line) > max_chars and current: - chunks.append("".join(current)) - current = [line] - current_len = len(line) - else: - current.append(line) - current_len += len(line) - if current: - chunks.append("".join(current)) - return chunks - - -def summarize_omitted_diffs( - omitted_diffs: list[FileDiff], - provider: LLMProvider, - model: str, -) -> tuple[str, int, int]: - """Summarize omitted diffs per file/chunk and return merged notes and token usage. - - Consumes BudgetDecision.omitted_diffs (which preserves untruncated patches), - prompts the provider for concise per-file summaries, and aggregates token usage. - """ - summaries: list[str] = [] - total_in = 0 - total_out = 0 - - for diff in omitted_diffs: - if diff.is_binary or not diff.patch_text.strip(): - summaries.append(f"- {diff.path}: [binary or empty file]") - continue - - chunks = _chunk_patch_text(diff.patch_text) - file_part_summaries: list[str] = [] - for i, chunk in enumerate(chunks, start=1): - part_suffix = f" (part {i}/{len(chunks)})" if len(chunks) > 1 else "" - prompt = ( - f"Summarize the following git diff for {diff.path}{part_suffix} in 1-2 concise bullet points " - f"(focus on WHAT changed and WHY):\n\n" - f"{chunk}" - ) - resp = provider.generate(prompt=prompt, model=model) - if resp.input_tokens is not None: - total_in += resp.input_tokens - if resp.output_tokens is not None: - total_out += resp.output_tokens - text = resp.text.strip() - if text: - file_part_summaries.append(text) - - joined_file_summary = "; ".join(file_part_summaries) if file_part_summaries else "Updated" - summaries.append(f"- {diff.path}: {joined_file_summary}") - - return "\n".join(summaries), total_in, total_out - - def generate_commit_message( diffs: list[FileDiff], cfg: GitmateConfig, diff --git a/tests/test_chunking.py b/tests/test_chunking.py new file mode 100644 index 0000000..1acbf26 --- /dev/null +++ b/tests/test_chunking.py @@ -0,0 +1,93 @@ +"""Tests for diff chunking and omitted diff summarization.""" + +from __future__ import annotations + +from gitmate.chunking import chunk_patch_text, summarize_omitted_diffs +from gitmate.diff_extractor import FileDiff +from gitmate.providers.base import LLMResponse + + +class FakeChunkProvider: + """Mock LLMProvider recording chunk summarization calls.""" + + def __init__(self, response: str = "Added feature X") -> None: + self.response = response + self.prompts: list[str] = [] + + def generate(self, prompt: str, model: str) -> LLMResponse: + self.prompts.append(prompt) + return LLMResponse(text=self.response, model=model, input_tokens=40, output_tokens=10) + + +def test_chunk_patch_text_small() -> None: + patch = "@@ -1,3 +1,3 @@\n-old\n+new\n" + chunks = chunk_patch_text(patch, max_chars=100) + assert chunks == [patch] + + +def test_chunk_patch_text_splits_at_lines() -> None: + lines = [f"+line {i}\n" for i in range(20)] + patch = "".join(lines) + chunks = chunk_patch_text(patch, max_chars=50) + assert len(chunks) > 1 + assert "".join(chunks) == patch + + +def test_summarize_omitted_diffs_binary_and_empty() -> None: + binary_diff = FileDiff( + path="assets/logo.png", + old_path=None, + status="modified", + additions=0, + deletions=0, + patch_text="", + is_binary=True, + ) + empty_diff = FileDiff( + path="empty.txt", + old_path=None, + status="modified", + additions=0, + deletions=0, + patch_text=" ", + is_binary=False, + ) + provider = FakeChunkProvider() + merged, total_in, total_out = summarize_omitted_diffs( + [binary_diff, empty_diff], provider, "gemini-3.5-flash-lite" + ) + assert "- assets/logo.png: [binary or empty file]" in merged + assert "- empty.txt: [binary or empty file]" in merged + assert provider.prompts == [] + assert total_in == 0 + assert total_out == 0 + + +def test_summarize_omitted_diffs_calls_provider_and_aggregates_tokens() -> None: + diff1 = FileDiff( + path="src/auth.py", + old_path=None, + status="modified", + additions=10, + deletions=2, + patch_text="@@ -1,5 +1,10 @@\n+auth logic\n", + is_binary=False, + ) + diff2 = FileDiff( + path="src/db.py", + old_path=None, + status="modified", + additions=5, + deletions=1, + patch_text="@@ -1,5 +1,5 @@\n+db connection\n", + is_binary=False, + ) + provider = FakeChunkProvider(response="Updated service logic") + merged, total_in, total_out = summarize_omitted_diffs( + [diff1, diff2], provider, "gemini-3.5-flash-lite" + ) + assert len(provider.prompts) == 2 + assert "- src/auth.py: Updated service logic" in merged + assert "- src/db.py: Updated service logic" in merged + assert total_in == 80 + assert total_out == 20