Skip to content

feat(consolidate): add icm consolidate-all for batch/cron rollups (#179) - #339

Merged
pszymkowiak merged 2 commits into
developfrom
feat/consolidate-all-179
Jul 24, 2026
Merged

feat(consolidate): add icm consolidate-all for batch/cron rollups (#179)#339
pszymkowiak merged 2 commits into
developfrom
feat/consolidate-all-179

Conversation

@pszymkowiak

Copy link
Copy Markdown
Contributor

Implements the cron-style rollup half of #179 (the standalone command; not the in-process worker daemon).

What

icm consolidate-all --threshold N [--summarizer-provider …] [--summarizer-model …] [--dry-run] — consolidates every topic whose memory count exceeds the threshold, in one batch. Lets users get LLM-consolidation quality without paying the ~13s-per-call latency inside interactive sessions.

Why it's safe to cron

Idempotent by construction: consolidating a topic collapses it to a single memory (below the threshold), so the next run skips it. It never keeps originals — that's what makes re-running a no-op. Drive it from a systemd timer / cron / launchd.

Implementation

  • Reuses the existing per-topic cmd_consolidate (same provider resolution, model, and lexical fallback) — no duplicated consolidation logic.
  • Iterates a snapshot of list_topics_with_prefix(None) (biggest topics first) so consolidating one doesn't perturb iteration.
  • Per-topic failures are logged and don't abort the batch (Consolidated N topic(s); M failed).
  • --dry-run lists what would be consolidated, changes nothing.

Tests / validation

  • Unit: consolidates only over-threshold topics, leaves under-threshold ones untouched, second run is a no-op (idempotent); --dry-run changes nothing.
  • E2E on the binary (lexical provider): seeded alpha=5/beta=2/gamma=4, --threshold 3 → dry-run lists alpha+gamma, real run collapses both to 1, beta untouched, second run reports "nothing to consolidate".
  • cargo clippy --all-targets -- -D warnings clean; 298 cli tests pass.

Follow-up (not in scope): the in-process background queue + worker thread + icm consolidate-jobs status (mechanism #1 of #179) can come later; this delivers the cron path now.

🤖 Generated with Claude Code

patrick and others added 2 commits July 24, 2026 13:02
…179)

Adds a standalone command that consolidates every topic whose memory count
exceeds a threshold — the cron-style rollup half of #179, so users get LLM
consolidation quality without paying the ~13s latency in interactive sessions.

- `icm consolidate-all --threshold N [--summarizer-provider …] [--dry-run]`.
- **Idempotent**: a consolidated topic collapses to one memory (below the
  threshold) and is skipped next run — so it's safe to drive from systemd
  timers / cron / launchd. It never keeps originals (that would break
  idempotency).
- Reuses the existing per-topic `cmd_consolidate` (same provider/model/lexical
  fallback), iterating a snapshot of `list_topics_with_prefix`, biggest topics
  first; per-topic failures are reported and don't abort the batch.
- `--dry-run` lists the topics that would be consolidated without changing
  anything.

Tests: consolidates only over-threshold topics, leaves under-threshold ones
untouched, and a second run is a no-op (idempotent); dry-run changes nothing.
E2E verified with lexical provider. clippy `-D warnings` clean; 298 cli tests
pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d threshold 0

Adversarial review flagged two footguns in `icm consolidate-all`:

- A bare `consolidate-all` resolves the summarizer to `none` (the default), so
  a cron invocation would replace EVERY over-threshold topic with a lexical
  ' | ' join and delete the originals — a store-wide quality loss (#186) with
  only a per-topic stderr warning. Now refuse unless the user explicitly passes
  `--summarizer-provider none` to opt into lexical.
- `--threshold 0` leaves a just-consolidated single-memory topic still "over"
  the threshold (1 > 0), so every run re-consolidates everything — infinite
  churn on a timer. Now rejected.

Also drop the unnecessary `#[allow(clippy::too_many_arguments)]` (7 args, under
the lint's threshold). Added a test for both guards.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@pszymkowiak
pszymkowiak merged commit b9b4d21 into develop Jul 24, 2026
7 checks passed
@rtk-ai-icm-ci rtk-ai-icm-ci Bot mentioned this pull request Jul 24, 2026
@pszymkowiak
pszymkowiak deleted the feat/consolidate-all-179 branch July 28, 2026 20:31
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.

1 participant