Skip to content

fix(consistency): audit batch 8 — auto-consolidate paths, embedding, cycles, feedback list#150

Merged
pszymkowiak merged 1 commit intomainfrom
fix/audit-batch-8-consistency
Apr 30, 2026
Merged

fix(consistency): audit batch 8 — auto-consolidate paths, embedding, cycles, feedback list#150
pszymkowiak merged 1 commit intomainfrom
fix/audit-batch-8-consistency

Conversation

@pszymkowiak
Copy link
Copy Markdown
Contributor

Bundle B8 from the 15-agent moyen-run validation. Closes audit findings AC1 (auto-consolidate only fired on MCP path), AC2 (consolidated memory had no embedding), CYC1 (concept link cycles silently accepted), and FB1 (feedback list subcommand mentioned in spec but missing). 324 tests passing. See commit message for the full per-fix narrative.

…cycles, feedback list

P3 / completeness fixes from the 15-agent moyen-run validation pass
(M2, M3, M11, M12).

## AC1 — Auto-consolidate consistency across all store paths

Agent M2 + M3 caught that `auto_consolidate_enabled = true` was only
honored on the MCP `tool_store` path. Memories landing via `icm store`
(CLI), the PostToolUse hook (`cmd_hook_post`), the PreCompact hook
(`cmd_hook_compact`), and the SessionEnd hook (`cmd_hook_end`) all
silently bypassed the rollup, so users on the CLI never saw their
configured threshold fire.

New `maybe_auto_consolidate(store, embedder, topic, &cfg.memory)`
helper centralises the trigger. Wired into:

* `cmd_store` — fires on the just-stored topic
* `cmd_save_project` — same
* `cmd_hook_post` — fires on `context-{project}` after extraction
* `extract_from_hook_transcript` (PreCompact + SessionEnd) — same

Same threshold + enabled flag as the MCP path, same eprintln!
notification format. Errors are logged and swallowed (consolidation
is a maintenance op, not on the critical path).

## AC2 — Consolidated memory missing embedding

Agent M2 found that the rolled-up memory had `embedding = None`,
making it invisible to hybrid recall until a manual `icm embed`
rebuild. Added a new
`SqliteStore::auto_consolidate_with_embedder(topic, threshold,
embedder)` variant that embeds the consolidated content inline. The
old `auto_consolidate(topic, threshold)` is preserved as a wrapper
that forwards `None` for backward compat with existing tests.

MCP `try_auto_consolidate` now threads the live embedder through.
CLI `maybe_auto_consolidate` passes the cmd-level embedder.

## CYC1 — Concept link cycle detection

Agent M11 confirmed that `A → B → C → A` was silently accepted,
corrupting `get_neighborhood` BFS. New `would_create_cycle(source,
target)` helper does a bounded BFS from `target` along outgoing
edges; if it reaches `source`, the new edge is rejected with
`IcmError::InvalidInput`. Also catches the trivial `source ==
target` self-link case at the application level (the schema CHECK
constraint already blocks it at the DB layer, but the new path
gives a cleaner error message).

New `IcmError::InvalidInput(String)` variant for caller-supplied
input that violates a domain invariant.

2 regression tests: `test_transitive_cycle_rejected` and
`test_dag_links_still_allowed` (sanity guard against over-strict
rejection).

## FB1 — `icm feedback list` subcommand

Agent M12 caught that the spec mentioned `feedback list --topic` but
only `record / search / stats` actually existed. Added the missing
subcommand. Mirrors `Search` without the FTS query — useful for
browsing all feedback under a topic without inventing an
intersecting keyword. Routes to the existing `FeedbackStore::
list_feedback` method (which was already implemented but
unreachable from the CLI).

## Test plan

- 321 → 324 tests passing (3 new: auto-consolidate-with-embedder,
  transitive cycle rejected, DAG links still allowed)
- `cargo clippy --workspace -- -D warnings` clean (added one targeted
  `#[allow(clippy::too_many_arguments)]` on `cmd_store` after threading
  the memory config through)
- `cargo fmt --all -- --check` clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pszymkowiak pszymkowiak merged commit 004458a into main Apr 30, 2026
9 checks passed
@pszymkowiak pszymkowiak deleted the fix/audit-batch-8-consistency branch April 30, 2026 09:03
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