Skip to content

feat: configurable compaction model#17

Merged
morganlinton merged 6 commits into
GetSmallAI:mainfrom
santhreal:feat/configurable-compaction-model
Jul 15, 2026
Merged

feat: configurable compaction model#17
morganlinton merged 6 commits into
GetSmallAI:mainfrom
santhreal:feat/configurable-compaction-model

Conversation

@santhreal

Copy link
Copy Markdown
Contributor

Adds modelSystem.compaction, a setting that selects the model used to summarize the conversation during context compaction (both automatic compaction and /compact).

  • Defaults to the main conversation model (inherit); set { "backend": "...", "model": "..." } to compact with a different model (e.g. a cheaper or longer-context one).
  • Prompt-budget sizing still tracks the main model — that is the context being fit; only the summary call uses the compaction model.
  • If the configured backend isn't ready (e.g. missing API key), compaction inherits the main model and prints a warning in the compaction notice (warn-and-inherit).
  • Wired through all three compaction paths: /compact, session auto-compact, and the in-loop guard.

Also bundles a small clippy question_mark cleanup in hooks/state.rs.

Tests: 5 new (model_system round-trip + context_guard classify/notice), full suite green, clippy --all-targets -- -D warnings clean, fmt --check clean.

Add modelSystem.compaction, selecting the model used to summarize the
conversation during context compaction (both automatic compaction and
/compact).

- Defaults to the main conversation model; set { "backend": "...",
  "model": "..." } to compact with a different model.
- Prompt-budget sizing still tracks the main model; only the summary
  call uses the compaction model.
- When the configured backend is not ready (e.g. missing API key),
  compaction inherits the main model and warns in the compaction notice.
- Wired through all three compaction paths (/compact, session
  auto-compact, in-loop guard).
Collapse the else-if-let/else-return into home? per the clippy
question_mark lint.
The warn-and-inherit notice for an unavailable compaction model was only
shown on the automatic-compaction path; cmd_compact built its own notice
and dropped result.warning. Print the warning line on the manual
/compact path too, so a configured-but-not-ready compaction backend is
never a silent fallback.
The compaction model added to modelSystem was configurable in
agent.config.json but invisible in the /route command that inspects the
stack. Add it to /route status (after selector) and /route template so
it is discoverable and verifiable like every other tier, and document it
in the /route README section.
When a dedicated compaction model is configured and its backend is ready
but the summarize call itself fails or returns empty, compaction fell
back to the deterministic trim with no signal beyond the changed method
label. Report the failure in the compaction notice so an explicitly
chosen model that breaks at runtime is never a silent degrade. Carry the
model label on CompactionModel::Use for the message and add a regression
test that drives the failing-summarize path.

@morganlinton morganlinton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — clean design (budget stays on the main model; only the summary call switches), and the follow-ups for /compact warnings, /route discoverability, and failure surfacing are appreciated.

A few small nits before merge:

  1. Docs wording (CHANGELOG.md / README.md) — Prose says set modelSystem.compaction to a "provider:model" spec, but the actual config shape is a ModelRef object ({ "backend": "...", "model": "..." }). The JSON example is correct; please reword the surrounding text so it matches (e.g. “a ModelRef / { backend, model } object”).

  2. CompactResult.warning doc comment — Still describes only the unavailable → inherit-main-model case. After the failure-surfacing commit it also covers dedicated-model error/empty → deterministic trim. Please broaden the comment to match both paths.

  3. Missing Unavailable unit testclassify_compaction_model covers Inherit and ready Use; please add a case for a configured backend that isn’t ready (e.g. openrouter without an API key) asserting CompactionModel::Unavailable.

Happy to discuss separately whether a failed dedicated compaction model should retry with the main model before falling back to deterministic trim.

- Docs (CHANGELOG/README): describe modelSystem.compaction as a ModelRef
  { backend, model } object rather than a "provider:model" spec, matching
  the actual config shape.
- Broaden the CompactResult.warning doc comment to cover both the
  unavailable->inherit path and the dedicated-model failure path.
- A dedicated compaction model that errors or returns empty now retries
  with the main conversation model before falling back to the
  deterministic trim, surfacing the outcome in the compaction notice.
- Add a classify_compaction_model test for a not-ready backend asserting
  CompactionModel::Unavailable.
@santhreal

Copy link
Copy Markdown
Contributor Author

Thanks Morgan — all three addressed in 16fdc36, plus the retry idea:

  1. Docs wording — CHANGELOG and README now describe modelSystem.compaction as a ModelRef object ({ backend, model }, same shape as planner/selector) instead of a "provider:model" spec. JSON examples were already in that form.
  2. CompactResult.warning doc — broadened to cover both paths: unavailable backend → inherit main model, and a dedicated model that errors/returns-empty → retry → deterministic trim.
  3. Unavailable test — added compaction_marks_not_ready_backend_unavailable, which forces OPENROUTER_API_KEY empty (then restores it) and asserts CompactionModel::Unavailable with the key-required reason.

On the retry discussion — I went ahead and implemented it since it's clearly the better behavior: a dedicated compaction model that errors or returns empty now retries with the main conversation model before falling back to the deterministic trim. The notice reports the outcome accordingly ("… failed; summarized with <main> instead", or "… and main model both failed; trimmed deterministically"). Budget sizing still tracks the main model throughout. Happy to gate that behind a flag if you'd rather it be opt-in.

All green: 8 compaction tests (incl. the new one), clippy --all-targets -- -D warnings, and fmt --check.

@morganlinton morganlinton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good — review items addressed, and the main-model retry before deterministic trim is the right default. Approving.

@morganlinton
morganlinton merged commit 9eb8967 into GetSmallAI:main Jul 15, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants