fix(recall): scope by the domain identity remember writes (#721, slice 2) by cdeust · Pull Request #901 · cdeust/Cortex

cdeust · 2026-11-06T03:30:36Z

Summary

recall now scopes by the same domain identity remember writes. Slice 1 (#877) made remember resolve an explicit domain with resolve_explicit_domain: an exact family member maps to its canonical (cortex-cowork → cortex), any other name is kept, lowercased. recall still passed the raw argument to the store, so recall(domain="cortex-cowork") searched a scope no write used, and recall(domain="SciFact") missed rows written as scifact. The handler now resolves the argument the same way; an empty or missing domain stays unscoped.

Refs #721 (slice 2 of the remaining slices listed on the issue; the issue stays open).

Root cause: write and search resolved the explicit domain through two different paths (resolved on write, raw on read).

Type of change

Bug fix (non-breaking change which fixes an issue)

Test plan

New tests_py/handlers/test_recall_domain_scope.py (7 tests): builds a repository registry from real directories under tmp_path (cortex, cortex-cowork, harness-comparison) and reads the domain the public recall handler hands to pg_recall. It checks family → canonical, explicit name kept lowercased, recall scope == remember._resolve_domain("", name) for three names, and None/"" stay unscoped.
Without the fix: 6 failed, 1 passed (None already stays unscoped). With it: 7 passed.
pytest tests_py/handlers/ -k "recall or remember": 284 passed, 1 skipped (PostgreSQL is missing in this session).
pytest tests_py/shared/test_explicit_domain_identity.py tests_py/handlers/test_remember_resolve_domain.py: pass.
ruff check and ruff format --check (ruff 0.17.3, the pinned version) pass on both changed files. python scripts/check_craftsmanship.py passes.

Not verified here:

PostgreSQL-gated tests and the mutation gate (no PostgreSQL in this session).
Retrieval benchmarks. torch could not be downloaded here (blocked host), so no benchmark ran. Benchmark callers pass lowercase names that are not repository names (beam, benchmark), which this change leaves as they are. The one exception would be a machine whose dev roots hold a repository named exactly like the benchmark domain.
On main, ruff check / ruff format --check over the whole tree report errors in scripts/ci_pipeline_source_contract.py, scripts/tests/test_no_main_comparison.py and tests_py/scripts/test_ci_review_required.py. This PR does not touch those files.

Audit notes

Layers: handlers → shared, the same import remember.py already uses. No core/infrastructure change.
Remaining fix: avoid silently remapping explicit domain identities through substring collisions #721 slices: priority of directory over domain in remember (owner decision); recall_hierarchical and other domain filters (unified_search goes through this handler and inherits the fix); documenting aliases and migrating rows already written under a collided domain.

Coding-standards compliance

§2.2 Layer dependency direction preserved.
§4.1 / §4.2: recall.py and _handler_impl were already in the craftsmanship baseline; the gate still passes.
§8 No new numeric constant.

Breaking changes

A recall with an explicit domain that is an exact name of a repository grouped under a family now searches the family canonical, and mixed-case domains are lowercased. These are the scopes remember has written since #877.

Generated by Claude Code

…e 2) remember resolves an explicit domain with resolve_explicit_domain (a family member maps to its canonical, any other name is kept, lowercased), but recall passed the raw argument to the store. recall(domain= "cortex-cowork") therefore searched a scope no write used ("cortex"), and recall(domain="SciFact") missed rows written as "scifact". recall now resolves the argument the same way; an empty domain stays unscoped. Refs #721 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtYM7JyD4jA1eJJddp

The reranker-matrix probe executes the real body of recall._handler_impl with an explicit namespace that lists every global the body reads. The handler now calls resolve_explicit_domain, which the namespace lacked, so the unit check (run 42156979849, test_reranker_matrix_runtime) failed with NameError. The probe now passes the production resolver; its fixture query has no domain, so the resolver returns "" and reads no registry. Refs #721 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtYM7JyD4jA1eJJddp

cdeust · 2026-11-06T03:47:34Z

CI autopilot : unit rouge sur 2723af7, correctif poussé en 617d7b7.

Échec (run 42156979849) : 4 cas de tests_py/benchmarks/test_reranker_matrix_runtime.py::test_real_handler_function_bodies_expose_90_vs_30 échouent avec NameError: name 'resolve_explicit_domain' is not defined. Les 11848 autres tests passent.
Cause : benchmarks/reranker_matrix/handler_probe.py exécute le corps réel de recall._handler_impl avec un espace de noms explicite qui liste chaque global lu par le corps ; cette PR ajoute un appel à resolve_explicit_domain que la sonde ne fournissait pas. L'échec vient donc bien de la PR.
Correctif : la sonde passe le résolveur de production. Sa requête de fixture n'a pas de domaine, donc il renvoie "" sans lire de registre.
Vérifié localement : test_reranker_matrix_runtime.py et test_recall_domain_scope.py passent (21 tests), et ruff check, ruff format --check et le gate craftsmanship passent. Non vérifié : la CI complète sur 617d7b7, encore en cours.

Generated by Claude Code

…anged The unit check's mutation gate (ADR-1122, run 42158643521) mutates every function a PR changes. Editing one line of recall._handler_impl made the PR own that function's 133 pre-existing unkilled mutants. The domain is now resolved in _scoped_by_explicit_domain, the function the public handler wraps, which leaves _handler_impl and the reranker probe that executes its body identical to main. An empty or missing domain is left as given, as before; the caller's dict is copied, not changed. Refs #721 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtYM7JyD4jA1eJJddp

cdeust · 2026-11-06T06:53:43Z

CI autopilot : unit rouge sur 617d7b7 (gate de mutation), correctif poussé en 9b67a1d.

Échec (run 42158643521) : les tests passent (11852), mais scripts/ci_mutation_diff.py (ADR-1122) mutait recall._handler_impl en entier, parce que la PR en modifiait une ligne. Résultat : 133 mutants non tués (survivants et timeouts), antérieurs à la PR.
Cause et correctif : la résolution du domaine vit maintenant dans _scoped_by_explicit_domain, la fonction qu'enveloppe le handler public. _handler_impl, et la sonde handler_probe.py qui exécute son corps, sont identiques à main. Le comportement de la PR ne change pas. Les tests passent par le handler public, vérifient la copie des arguments de l'appelant, ainsi que None, "" et un domaine blanc.
Vérifié localement : GITHUB_BASE_REF=main python scripts/ci_mutation_diff.py → 1 file(s), 0 failed, 0 untested, 12/12 mutants de _scoped_by_explicit_domain tués (sans PostgreSQL ici : les tests PG du lot sont ignorés). 332 tests handlers/benchmarks passés. ruff et le gate craftsmanship passent.
Non vérifié : le gate dans la CI avec PostgreSQL, et la CI complète sur 9b67a1d, encore en cours.

Generated by Claude Code

chatgpt-codex-connector · 2026-11-06T15:11:29Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-06T15:12:26.325972Z	`9b67a1d`	Draft marked ready
🔒 Security Review	✅ Completed 2026-11-06T15:16:10.394823Z	`9b67a1d`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude · 2026-11-06T15:11:50Z

Claude finished @cdeust's task in 1m 25s —— View job

PR #901 review (1 of 3)

Gather the diff and context
Run the five reviews (quality, security, performance, tests, docs)
Verify findings and post the verdict

I found no Blockers. The full review is in this comment, with three Notes that need no action before merge. I did not run pytest or ruff here because the command was not permitted; the findings come from reading the code.

Verdict: approve

claude · 2026-11-06T15:13:20Z

Claude review 1 of 3: PR #901 at `9b67a1d`

I found no Blockers. Five reviews ran: code quality, security, performance, test coverage and documentation accuracy.

What I checked in the diff (mcp_server/handlers/recall.py, tests_py/handlers/test_recall_domain_scope.py):

Routing: every production caller of recall (unified_search, recall_hierarchical, the benchmark callers) imports the public handler, so all of them go through _scoped_by_explicit_domain. Nothing outside the tests calls _handler_impl directly.
Same scope as remember: with an explicit domain, recall now resolves it with the same resolve_explicit_domain that remember._resolve_domain uses. The caller's dict is copied, not changed.
Layers: the new import is handlers → shared. Domain is not a privacy boundary in PRIVACY.md or docs/agent-scope.md; the agent_topic scope is not affected.
Performance: _build_registry is cached with lru_cache. Recall now pays at most one synchronous registry scan per process, the same scan remember already pays.
Tests: the names the tests patch are the names the code reads. The registry cache is cleared before and after each test. Without the fix, the family, lowercase, equals-remember and blank-domain tests would fail.

I did not run pytest or ruff in this session (the commands were not permitted here), so the checks above come from reading the code.

Notes, no action before merge

The _scoped_by_explicit_domain docstring (recall.py:678) says "A missing or empty domain is left as given". A whitespace-only domain is turned into None, which means unscoped. test_blank_domain_becomes_unscoped pins that behaviour, but the docstring does not mention it.
This slice does not cover recall_hierarchical.py:247/:264, which still passes the raw domain to get_memories_for_domain. domain="cortex-cowork" returns empty at :265-266 before the flat-recall fallback runs. It is already listed in the remaining fix: avoid silently remapping explicit domain identities through substring collisions #721 slices.
The fixture's harness-comparison repository is not used by any test.

Verdict: approve

cdeust mentioned this pull request Nov 6, 2026

fix: avoid silently remapping explicit domain identities through substring collisions #721

Open

4 tasks

cdeust marked this pull request as ready for review November 6, 2026 15:11

github-actions Bot merged commit 50af470 into main Nov 6, 2026
32 checks passed

cdeust mentioned this pull request Nov 6, 2026

fix(recall): recall_hierarchical reads the domain remember writes (#721, slice 3) #914

Merged

8 of 10 tasks

Navigation Menu

Repository navigation

fix(recall): scope by the domain identity remember writes (#721, slice 2) - #901

fix(recall): scope by the domain identity remember writes (#721, slice 2)#901
github-actions[bot] merged 3 commits into
mainfrom
fix/721-recall-domain-scope

cdeust commented Nov 6, 2026

Uh oh!

cdeust commented Nov 6, 2026

Uh oh!

cdeust commented Nov 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 6, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 6, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 6, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cdeust commented Nov 6, 2026

Summary

Type of change

Test plan

Audit notes

Coding-standards compliance

Breaking changes

Uh oh!

cdeust commented Nov 6, 2026

Uh oh!

cdeust commented Nov 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

claude Bot commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR #901 review (1 of 3)

Uh oh!

claude Bot commented Nov 6, 2026

Claude review 1 of 3: PR #901 at 9b67a1d

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chatgpt-codex-connector Bot commented Nov 6, 2026 •

edited

Loading

claude Bot commented Nov 6, 2026 •

edited

Loading

Claude review 1 of 3: PR #901 at `9b67a1d`