feat: Phase 2 — Token Budget Manager and model limits - #4

Follow-up fixes on top of the Phase 2 implementation, from the review round plus three clarification questions. Two commits:

75b3999 — review findings

• Non-positive effective budgets now raise ValueError instead of emitting nonsense negative limits (guard sits in assess, so it covers constructor args and per-call overhead — an __init__-only check would have missed the latter).
• Truncation drops low-signal files first (snapshots, tests, generated code, largest-first within groups), matching the plan's strategy; char length stays documented as the cost proxy to avoid a paid count call per file.
• Working copies use dataclasses.replace() (no more 7-field manual copy to rot); identity-keyed index so duplicate paths can't collapse.
• Final NEEDS_CHUNKING total reuses the last loop recount instead of a duplicate paid API call (pinned: 3 calls, not 4, for the 2-file chunking case).
• BudgetDecision docstring now states the aliasing contract (omitted holds originals by design; included carries all files with truncated ones as note placeholders). Note wording unchanged — tests and plan language already bless it.
• Config-side notes (model IDs, from_config, overhead plumbing) deliberately deferred to Phase 3.

5a9432f — ordering and coverage questions

• _is_low_signal vs ignore_globs is a deliberate split, now stated in the docstring: "never send" (user-configured) vs "drop first under pressure" (built-in judgment, always sent whole when budget allows).
• Sort key provably exhausts the whole low-signal group largest-first before any normal file, no matter the sizes — pinned by a test where a 150-char snap truncates before a 300-char normal file, then spills over in order.
• The guard-skipped collective-overflow case (many tiny files, none individually truncatable, headers alone over budget) correctly falls through to NEEDS_CHUNKING — previously untested, now pinned.

Gate: 92 pytest green, ruff check, format --check, mypy --strict clean.

Follow-up review fixes addressing the pre-merge review items and audit findings:

1. Truncation policy decision record & simplification (Option A)

• Reverted _is_low_signal and aligned candidate sorting strictly by descending raw patch length (len(d.patch_text)).
• Documented the deliberate policy decision on TokenBudgetManager:

  "Truncation orders candidate diffs strictly by descending patch length (largest raw patch first). Signal weighting was evaluated, but common generated artifacts and lockfiles are already filtered upstream by ignore_globs; its only non-redundant contribution (test-directory deprioritization) was actively harmful, stripping critical test diffs in test-focused commits. Reverted to size-only ordering as the deliberate policy."

2. Anti-inflation guard & NEEDS_CHUNKING interaction in mixed diffs

• Fixed the summary_note formatting bug in Step 3 where mixed diffs (some files truncated, remainder guard-skipped) emitted misleading notes like "even with all 1 files truncated".
• Formatted trunc_info with proper ratios ("even with 1/3 files truncated; requires chunking").
• Added test_mixed_truncated_and_guarded_diffs_trigger_needs_chunking to pin this exact edge case.

3. template_overhead wired into GitmateConfig

• Added template_overhead: int = DEFAULT_TEMPLATE_OVERHEAD (500) to GitmateConfig.
• Added parsing, positive-integer validation, and persistence to load_config / save_config.
• Enforced budget validation at config-load time (max_context_tokens > reserved_output_tokens + template_overhead) so invalid limits are rejected upfront instead of failing when assess() is called.
• Added round-trip and validation tests in tests/test_config.py.

4. Domain typed exception

• Added InvalidBudgetError(TokenBudgetError) to replace the raw generic ValueError when effective_budget <= 0.

Gate: 97 pytest passed, ruff check, ruff format --check, and mypy --strict clean.

Follow-up commit addressing opencode's review findings:

1. Named Risk for Phase 3 (Tracked in plan.md):

   • Documented under Phase 3 in plan.md that TokenBudgetManager defaults to a 1M context window (context_window: int = 1_048_576) independent of model. In Phase 3, when wiring up provider calls for models with smaller context windows (e.g., Claude's 200k limit), the caller must pass get_context_window(cfg) or use a TokenBudgetManager.from_config(cfg, counter) factory method rather than relying on constructor defaults.

2. Validation Gap Closed for Budget Inputs (feat: Phase 1 diff/context extractor with parser hardening #2):

   • Extended InvalidBudgetError checks in TokenBudgetManager.__init__ and assess() to reject non-positive values (<= 0) for context_window, reserved_output_tokens, and template_overhead.
   • Updated and expanded tests in test_non_positive_budget_inputs_raise to verify that both 0 and negative inputs are rejected upfront with InvalidBudgetError.

3. Docstring Clarification on Aliasing (fix: filename quoting, nested globs, and small display bugs #3):

   • Updated BudgetDecision docstring to accurately describe the safer behavior: on truncate and chunk paths, all entries in included_diffs are fresh copies created via dataclasses.replace() so caller objects are never mutated in-place.

Gate: 98 pytest passed, ruff check, ruff format --check, and mypy --strict clean.