diff --git a/hindsight-integrations/claude-code/scripts/recall.py b/hindsight-integrations/claude-code/scripts/recall.py index 6f9d43d641..54e91c93ad 100755 --- a/hindsight-integrations/claude-code/scripts/recall.py +++ b/hindsight-integrations/claude-code/scripts/recall.py @@ -41,6 +41,50 @@ LAST_RECALL_STATE = "last_recall.json" +def recall_filter_key(tags, tag_groups, tags_match): + def canonical_tags(value): + if not value: + return None + if not isinstance(value, list): + return value + unique = {json.dumps(item, sort_keys=True): item for item in value} + return [unique[key] for key in sorted(unique)] + + def canonical_tag_groups(value): + if not value: + return None + if isinstance(value, dict): + return { + key: canonical_tags(value[key]) if key == "tags" else canonical_tag_groups(value[key]) + for key in sorted(value) + } + if isinstance(value, list): + return [canonical_tag_groups(item) for item in value] + return value + + canonical_filter_tags = canonical_tags(tags) + canonical_filter_groups = canonical_tag_groups(tag_groups) + return json.dumps( + { + "tags": canonical_filter_tags, + "tag_groups": canonical_filter_groups, + "tags_match": tags_match if canonical_filter_tags or canonical_filter_groups else None, + }, + sort_keys=True, + ) + + +def recall_results_or_none(response, bank_id, config): + if not isinstance(response, dict) or ("results" in response and not isinstance(response["results"], list)): + debug_log(config, f"Recall from bank '{bank_id}' returned malformed results; allowing retry") + return None + results = response.get("results", []) + if any(not isinstance(result, dict) for result in results): + debug_log(config, f"Recall from bank '{bank_id}' returned malformed result items; allowing retry") + return None + return results + + def filter_by_min_scores(results: list[dict], min_scores: dict, config: dict) -> list[dict]: """Drop recall results whose numeric scores are below configured floors.""" if not min_scores: @@ -199,19 +243,35 @@ def _dbg(*a): print(f"[Hindsight] Recall failed: {e}", file=sys.stderr) return - results = response.get("results", []) + results = recall_results_or_none(response, bank_id, config) + seen_recall_keys = set() + if results is not None: + try: + primary_recall_key = (bank_id, recall_filter_key(recall_tags, tag_groups, tags_match)) + seen_recall_keys.add(primary_recall_key) + except Exception as e: + debug_log(config, f"Unable to build primary recall key for '{bank_id}': {e}") + else: + results = [] # Also recall from any additional banks (e.g. shared user profile bank) additional_banks = config.get("recallAdditionalBanks", []) for extra_bank_id in additional_banks: - extra_filter = additional_bank_filters.get(extra_bank_id, {}) - extra_tags = extra_filter.get("recallTags", recall_tags) or None - extra_tag_groups = extra_filter.get("recallTagGroups", tag_groups) or None - extra_tags_match = extra_filter.get( - "recallTagsMatch", - tags_match if extra_tags or extra_tag_groups else None, - ) try: + extra_filter = additional_bank_filters.get(extra_bank_id, {}) + extra_tags = extra_filter.get("recallTags", recall_tags) or None + extra_tag_groups = extra_filter.get("recallTagGroups", tag_groups) or None + extra_tags_match = extra_filter.get( + "recallTagsMatch", + tags_match if extra_tags or extra_tag_groups else None, + ) + recall_key = ( + extra_bank_id, + recall_filter_key(extra_tags, extra_tag_groups, extra_tags_match), + ) + if recall_key in seen_recall_keys: + debug_log(config, f"Skipping duplicate additional-bank recall for '{extra_bank_id}'") + continue extra_response = client.recall( bank_id=extra_bank_id, query=query, @@ -223,10 +283,13 @@ def _dbg(*a): tag_groups=extra_tag_groups, timeout=10, ) - extra_results = extra_response.get("results", []) + extra_results = recall_results_or_none(extra_response, extra_bank_id, config) + if extra_results is None: + continue if extra_results: debug_log(config, f"Got {len(extra_results)} memories from additional bank '{extra_bank_id}'") results = results + extra_results + seen_recall_keys.add(recall_key) except Exception as e: debug_log(config, f"Recall from additional bank '{extra_bank_id}' failed: {e}") diff --git a/hindsight-integrations/claude-code/tests/test_hooks.py b/hindsight-integrations/claude-code/tests/test_hooks.py index eafc85a8e8..ab6ba1121e 100644 --- a/hindsight-integrations/claude-code/tests/test_hooks.py +++ b/hindsight-integrations/claude-code/tests/test_hooks.py @@ -275,6 +275,534 @@ def capture_and_respond(req, timeout=None): assert captured[1]["tags"] == ["memory_type:rule"] assert captured[1]["tags_match"] == "all_strict" + def test_additional_banks_skip_primary_bank_with_same_filters(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["project-bank", "shared-bank"], + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "shared-bank"] + + def test_additional_banks_skip_primary_bank_with_reordered_equivalent_filters(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallTags": ["memory_type:rule", "tech_stack:supabase"], + "recallTagsMatch": "all_strict", + "recallAdditionalBanks": ["project-bank"], + "recallAdditionalBankFilters": { + "project-bank": { + "recallTags": ["tech_stack:supabase", "memory_type:rule"], + "recallTagsMatch": "all_strict", + } + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank"] + + def test_additional_banks_keep_distinct_tag_group_order(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallTagGroups": [ + {"op": "all", "tags": ["memory_type:rule"]}, + {"op": "any", "tags": ["tech_stack:supabase"]}, + ], + "recallAdditionalBanks": ["project-bank"], + "recallAdditionalBankFilters": { + "project-bank": { + "recallTagGroups": [ + {"op": "any", "tags": ["tech_stack:supabase"]}, + {"op": "all", "tags": ["memory_type:rule"]}, + ], + } + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "project-bank"] + + def test_additional_banks_skip_equivalent_tag_group_inner_tag_order(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallTagGroups": [{"op": "all", "tags": ["memory_type:rule", "tech_stack:supabase"]}], + "recallAdditionalBanks": ["project-bank"], + "recallAdditionalBankFilters": { + "project-bank": { + "recallTagGroups": [{"op": "all", "tags": ["tech_stack:supabase", "memory_type:rule"]}], + } + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank"] + + def test_additional_banks_skip_primary_bank_with_duplicate_filter_terms(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallTags": ["memory_type:rule"], + "recallTagsMatch": "all_strict", + "recallAdditionalBanks": ["project-bank"], + "recallAdditionalBankFilters": { + "project-bank": { + "recallTags": ["memory_type:rule", "memory_type:rule"], + "recallTagsMatch": "all_strict", + } + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank"] + + def test_additional_banks_skip_primary_bank_when_tags_match_has_no_filters(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallTagsMatch": "all_strict", + "recallAdditionalBanks": ["project-bank"], + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank"] + + def test_additional_banks_skip_primary_bank_with_empty_list_filters(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["project-bank"], + "recallAdditionalBankFilters": { + "project-bank": { + "recallTags": [], + "recallTagGroups": [], + } + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank"] + + def test_additional_banks_skip_primary_bank_with_tags_match_only_filter(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["project-bank"], + "recallAdditionalBankFilters": { + "project-bank": {"recallTagsMatch": "all_strict"}, + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank"] + + def test_additional_banks_allow_primary_bank_with_distinct_filters(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallTags": ["memory_type:task"], + "recallAdditionalBanks": ["project-bank"], + "recallAdditionalBankFilters": { + "project-bank": {"recallTags": ["memory_type:rule"]}, + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "project-bank"] + assert captured[0][1]["tags"] == ["memory_type:task"] + assert captured[1][1]["tags"] == ["memory_type:rule"] + + def test_additional_banks_skip_repeated_extra_bank_with_same_filters(self, monkeypatch, tmp_path): + captured = [] + + def capture_and_respond(req, timeout=None): + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["shared-bank", "shared-bank"], + "recallAdditionalBankFilters": { + "shared-bank": {"recallTags": ["memory_type:rule"]}, + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "shared-bank"] + assert captured[1][1]["tags"] == ["memory_type:rule"] + + def test_additional_banks_retry_repeated_extra_bank_after_failure(self, monkeypatch, tmp_path): + captured = [] + shared_calls = 0 + + def capture_and_respond(req, timeout=None): + nonlocal shared_calls + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + if bank == "shared-bank" and shared_calls == 0: + shared_calls += 1 + raise OSError("temporary recall failure") + if bank == "shared-bank": + shared_calls += 1 + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["shared-bank", "shared-bank"], + "recallAdditionalBankFilters": { + "shared-bank": {"recallTags": ["memory_type:rule"]}, + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "shared-bank", "shared-bank"] + assert shared_calls == 2 + + def test_additional_banks_retry_repeated_extra_bank_after_bad_response_shape(self, monkeypatch, tmp_path): + captured = [] + shared_calls = 0 + + def capture_and_respond(req, timeout=None): + nonlocal shared_calls + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + if bank == "shared-bank" and shared_calls == 0: + shared_calls += 1 + return FakeHTTPResponse({"results": 3}) + if bank == "shared-bank": + shared_calls += 1 + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["shared-bank", "shared-bank"], + "recallAdditionalBankFilters": { + "shared-bank": {"recallTags": ["memory_type:rule"]}, + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "shared-bank", "shared-bank"] + assert shared_calls == 2 + + def test_additional_banks_retry_repeated_extra_bank_after_bad_result_items(self, monkeypatch, tmp_path): + captured = [] + shared_calls = 0 + + def capture_and_respond(req, timeout=None): + nonlocal shared_calls + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + if bank == "shared-bank" and shared_calls == 0: + shared_calls += 1 + return FakeHTTPResponse({"results": [3]}) + if bank == "shared-bank": + shared_calls += 1 + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["shared-bank", "shared-bank"], + "recallAdditionalBankFilters": { + "shared-bank": {"recallTags": ["memory_type:rule"]}, + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "shared-bank", "shared-bank"] + assert shared_calls == 2 + + def test_additional_banks_skip_repeated_extra_bank_after_missing_results(self, monkeypatch, tmp_path): + captured = [] + shared_calls = 0 + + def capture_and_respond(req, timeout=None): + nonlocal shared_calls + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + if bank == "shared-bank": + shared_calls += 1 + return FakeHTTPResponse({}) + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["shared-bank", "shared-bank"], + "recallAdditionalBankFilters": { + "shared-bank": {"recallTags": ["memory_type:rule"]}, + }, + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "shared-bank"] + assert shared_calls == 1 + + def test_additional_banks_can_retry_primary_after_bad_response_shape(self, monkeypatch, tmp_path): + captured = [] + project_calls = 0 + + def capture_and_respond(req, timeout=None): + nonlocal project_calls + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + if bank == "project-bank" and project_calls == 0: + project_calls += 1 + return FakeHTTPResponse({"results": 3}) + if bank == "project-bank": + project_calls += 1 + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["project-bank"], + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "project-bank"] + assert project_calls == 2 + + def test_additional_banks_can_retry_primary_after_bad_result_items(self, monkeypatch, tmp_path): + captured = [] + project_calls = 0 + + def capture_and_respond(req, timeout=None): + nonlocal project_calls + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + if bank == "project-bank" and project_calls == 0: + project_calls += 1 + return FakeHTTPResponse({"results": [3]}) + if bank == "project-bank": + project_calls += 1 + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["project-bank"], + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank", "project-bank"] + assert project_calls == 2 + + def test_additional_banks_skip_primary_after_missing_results(self, monkeypatch, tmp_path): + captured = [] + project_calls = 0 + + def capture_and_respond(req, timeout=None): + nonlocal project_calls + if "/recall" in req.full_url: + bank = req.full_url.split("/banks/", 1)[1].split("/", 1)[0] + captured.append((bank, json.loads(req.data.decode()))) + if bank == "project-bank" and project_calls == 0: + project_calls += 1 + return FakeHTTPResponse({}) + if bank == "project-bank": + project_calls += 1 + return FakeHTTPResponse({"results": []}) + + hook_input = make_hook_input(prompt="What project rules apply here?") + _run_hook( + "recall", + hook_input, + monkeypatch, + tmp_path, + urlopen_side_effect=capture_and_respond, + extra_settings={ + "bankId": "project-bank", + "recallAdditionalBanks": ["project-bank"], + }, + ) + + assert [bank for bank, _body in captured] == ["project-bank"] + assert project_calls == 1 + def test_disabled_auto_recall_produces_no_output(self, monkeypatch, tmp_path): (tmp_path / "plugin_root").mkdir(exist_ok=True) (tmp_path / "plugin_data").mkdir(exist_ok=True)