mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(memory): apply turn_id scope filter even without agent_id (#2130)
## Description `SQLiteMemoryStore._build_query_conditions()` dropped `turn_id` when a query specified `session_id` and `turn_id` without also specifying `agent_id`. That made a single-turn scope return every memory in the session, and `count()` uses the same helper. ## Fix - Apply `agent_id` and `turn_id` as independent narrowing predicates inside the `session_id` branch. - Preserve the existing `agent_id`-only behavior. - Add direct query-condition regression tests for turn-only, agent+turn, and agent-only scopes. - Merge current `main` to refresh mergeability and stale lint results. ## Testing ```text uvx ruff@0.15.17 check headroom/memory/adapters/sqlite.py tests/test_memory/test_query_conditions.py headroom/memory/factory.py All checks passed! uvx ruff@0.15.17 format --check headroom/memory/adapters/sqlite.py tests/test_memory/test_query_conditions.py headroom/memory/factory.py 3 files already formatted git diff --check headroomlabs/main...HEAD # no output uv run --extra dev python -m pytest tests/test_memory/test_query_conditions.py -q 3 passed ``` ## Review Readiness - [x] Ready for review - [x] Regression tests added - [x] CHANGELOG updated Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
This commit is contained in:
parent
5e0f1a219f
commit
9e38905a7d
3 changed files with 49 additions and 3 deletions
|
|
@ -111,6 +111,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
* **wrap/doctor:** make the Claude Remote Control gate warning accurate and stop it firing for users who never had the feature ([#1779](https://github.com/headroomlabs-ai/headroom/issues/1779)). Claude Code 2.1.196 added a client-side check that **deterministically** disables first-party Remote Control (`/remote-control` / `/rc`) whenever `ANTHROPIC_BASE_URL` points at a non-`api.anthropic.com` host — which Headroom always does. The old notice hedged ("may hide the Remote Control menu"); it now states the disable as fact, names the `/rc` command, and detects the installed Claude Code version so the wording is exact (`2.1.196` when known, `2.1.196+` when not). The gate is upstream and RC's control-plane talks to `claude.ai` (not the API host), so Headroom cannot restore it — the warning tells you to run Claude without Headroom for RC sessions. The warning is suppressed for auth modes that never had Remote Control (API-key/PAYG via `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN`, and Bedrock/Vertex/Foundry cloud IAM) and on Claude Code builds older than 2.1.196 where RC is unaffected by a custom base URL. Both the `headroom wrap claude` launch banner and `headroom doctor` co-report the sibling base-URL gates Headroom *does* restore — on-demand tool loading (#746, automatic) and the 1M context window (#1158, via `--1m`) — and the wrap-side co-report is session-accurate: it says "already restored via --1m" when the flag is in effect and reports tool deferral OFF (not falsely "kept on") when the user chose `--tool-search false`/`ENABLE_TOOL_SEARCH=false`; the `ENABLE_TOOL_SEARCH=...` banner line got the same accuracy fix. `is_custom_anthropic_base_url` now recognizes scheme-less values (`myproxy.local:8080`, `127.0.0.1:8787`) as custom hosts and degrades gracefully on malformed URLs instead of crashing `doctor`. `doctor` resolves the Claude Code version lazily, so runs with no custom base URL never pay the `claude --version` subprocess. No request bytes are touched (cache-safe); this is UX/notice-only.
|
||||
* **install:** default the docker image to `ghcr.io/headroomlabs-ai/headroom:latest` instead of the dead `ghcr.io/chopratejas/headroom:latest`. After the repo moved to the `headroomlabs-ai` org, GHCR did not redirect the old package, so `headroom install` / `headroom init` and the install scripts pulled a frozen `0.27.0` image while current releases publish to the new path ([#1867](https://github.com/headroomlabs-ai/headroom/issues/1867)).
|
||||
* **transforms/content-router:** stop a profile-derived `read_protection_window` kwarg from weakening an explicit `--protect-tool-results` guarantee. `ContentRouter.apply()` computes `read_protection_window` from `protect_recent_reads_fraction`, where `0.0` (the sentinel `--protect-tool-results` sets) means "protect all excluded-tool output regardless of conversation depth" per #1374's documented contract — but the method then unconditionally overwrote that window with a `read_protection_window` kwarg whenever one was present. `proxy_pipeline_kwargs()` supplies that kwarg on every request from the active `AgentSavingsProfile.protect_recent` (the default `coding` profile sets `protect_recent=2`), so in practice only the last 2 messages ever kept read-protection and older excluded-tool output silently fell through to lossy compression. The runtime kwarg may now only narrow the window when `protect_recent_reads_fraction > 0`; it can no longer shrink the "protect everything" guarantee set by `--protect-tool-results`.
|
||||
* **memory:** apply the `turn_id` scope filter even when `agent_id` is absent. In `SQLiteMemoryStore._build_query_conditions` the `turn_id` condition was nested inside the `agent_id` block, so a query filtered by `user_id` + `session_id` + `turn_id` (no `agent_id`) dropped the `turn_id` predicate entirely and returned every memory in the session instead of the single turn — an over-broad result that leaks sibling-turn memories into recall (and makes `count()` wrong for that scope). `agent_id` and `turn_id` are now applied independently.
|
||||
* **transforms:** stop the lossless `diff` fold from silently dropping lines out of non-diff content. `ContentRouter._lossless_first` tries every `compact_lossless` fold on all content, but the `diff` kind (`diff_strip_index`) is the only one with no exact-inverse check — it removes any line shaped like `index <hex>..<hex>`. Applied to arbitrary text/log/search payloads that happen to contain such a line, that line was deleted with no CCR marker, so it was unrecoverable — a violation of the lossless no-loss contract the method's own docstring promises. The `diff` fold now runs only when the strategy is `DIFF` or the content is diff-shaped (`_looks_like_diff`); genuine diffs still have their `index` bookkeeping folded.
|
||||
* **memory:** include the Ollama server URL in the embedder cache key so a second backend can't get an embedder bound to the wrong server. `_create_embedder` cached by `(backend, model)` only, but the Ollama embedder is constructed with `base_url=config.ollama_base_url`. Two configs in the same process that shared a backend and model but pointed at different Ollama servers (e.g. a per-project storage router) collided on one cache slot, so the second silently reused the first's embedder and embedded against the wrong host. The cache key now also includes `ollama_base_url`.
|
||||
* **tokenizers:** use `o200k_base` for the gpt-4.1 / gpt-4.5 / o4 families in `get_encoding_for_model`. `gpt-4.1*` and `gpt-4.5*` matched the broad `gpt-4` prefix and were encoded with `cl100k_base`, and `o4*` matched no prefix and fell through to the `cl100k_base` default — all three use `o200k_base`, so their token counts were computed with the wrong vocabulary. Added explicit `gpt-4.1`/`gpt-4.5` prefixes ahead of `gpt-4` and an `o4` prefix; `gpt-4` and `gpt-3.5` snapshots still resolve to `cl100k_base`.
|
||||
|
|
|
|||
|
|
@ -428,13 +428,18 @@ class SQLiteMemoryStore:
|
|||
conditions.append("session_id = ?")
|
||||
params.append(filter.session_id)
|
||||
|
||||
# agent_id and turn_id are independent narrowing constraints:
|
||||
# turn_id must be applied even when agent_id is absent. Nesting
|
||||
# the turn_id check inside the agent_id block dropped the turn
|
||||
# filter for a (session_id + turn_id, no agent_id) query, so it
|
||||
# returned the whole session instead of the one turn.
|
||||
if filter.agent_id is not None:
|
||||
conditions.append("agent_id = ?")
|
||||
params.append(filter.agent_id)
|
||||
|
||||
if filter.turn_id is not None:
|
||||
conditions.append("turn_id = ?")
|
||||
params.append(filter.turn_id)
|
||||
if filter.turn_id is not None:
|
||||
conditions.append("turn_id = ?")
|
||||
params.append(filter.turn_id)
|
||||
elif filter.agent_id is not None:
|
||||
# Agent without session - unusual but supported
|
||||
conditions.append("agent_id = ?")
|
||||
|
|
|
|||
40
tests/test_memory/test_query_conditions.py
Normal file
40
tests/test_memory/test_query_conditions.py
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
"""SQLiteMemoryStore._build_query_conditions scope filtering.
|
||||
|
||||
`_build_query_conditions` only reads the filter, so it is exercised directly via
|
||||
``object.__new__`` (no DB, no embedder).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from headroom.memory.adapters.sqlite import SQLiteMemoryStore
|
||||
from headroom.memory.ports import MemoryFilter
|
||||
|
||||
|
||||
def _conditions(**kwargs) -> tuple[list[str], list]:
|
||||
store = object.__new__(SQLiteMemoryStore)
|
||||
return store._build_query_conditions(MemoryFilter(**kwargs))
|
||||
|
||||
|
||||
def test_turn_id_is_applied_without_agent_id():
|
||||
"""A (user, session, turn) filter without agent_id must still narrow to the
|
||||
turn — previously the turn_id condition was nested inside the agent_id block
|
||||
and silently dropped, returning the whole session."""
|
||||
conditions, params = _conditions(user_id="u", session_id="s", turn_id="t")
|
||||
|
||||
assert "turn_id = ?" in conditions
|
||||
assert "t" in params
|
||||
|
||||
|
||||
def test_agent_id_and_turn_id_both_applied():
|
||||
conditions, params = _conditions(user_id="u", session_id="s", agent_id="a", turn_id="t")
|
||||
|
||||
assert "agent_id = ?" in conditions
|
||||
assert "turn_id = ?" in conditions
|
||||
assert "a" in params and "t" in params
|
||||
|
||||
|
||||
def test_agent_id_only_still_applied():
|
||||
conditions, _ = _conditions(user_id="u", session_id="s", agent_id="a")
|
||||
|
||||
assert "agent_id = ?" in conditions
|
||||
assert "turn_id = ?" not in conditions
|
||||
Loading…
Add table
Add a link
Reference in a new issue